Remove validation of parameter variables after substitution

This commit removes validation for fields containing a parameter variable
after the parameter value has already been substituted in TaskRun and PipelineRun.
This is done to allow parameter values to contain literal strings like $(params.a); for example, in #4690.

pkg/apis/pipeline/v1beta1/pipeline_validation.go

@@ -48,6 +48,16 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
 	if equality.Semantic.DeepEqual(ps, &PipelineSpec{}) {
 		errs = errs.Also(apis.ErrGeneric("expected at least one, got none", "description", "params", "resources", "tasks", "workspaces"))
 	}
+
+	if apis.IsSubstituted(ctx) {
+		// Validate the pipeline's workspaces only.
+		errs = errs.Also(validatePipelineWorkspacesDeclarations(ps.Workspaces))
+		errs = errs.Also(validatePipelineWorkspacesUsage(ps.Workspaces, ps.Tasks).ViaField("tasks"))
+		errs = errs.Also(validatePipelineWorkspacesUsage(ps.Workspaces, ps.Finally).ViaField("finally"))
+
+		return errs
+	}
+
 	// PipelineTask must have a valid unique label and at least one of taskRef or taskSpec should be specified
 	errs = errs.Also(ValidatePipelineTasks(ctx, ps.Tasks, ps.Finally))
 	// All declared resources should be used, and the Pipeline shouldn't try to use any resources

pkg/apis/pipeline/v1beta1/task_validation.go

@@ -48,6 +48,15 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
 	if len(ts.Steps) == 0 {
 		errs = errs.Also(apis.ErrMissingField("steps"))
 	}
+
+	if apis.IsSubstituted(ctx) {
+		// Validate the task's workspaces only.
+		errs = errs.Also(validateDeclaredWorkspaces(ts.Workspaces, ts.Steps, ts.StepTemplate).ViaField("workspaces"))
+		errs = errs.Also(validateWorkspaceUsages(ctx, ts))
+
+		return errs
+	}
+
 	errs = errs.Also(ValidateVolumes(ts.Volumes).ViaField("volumes"))
 	errs = errs.Also(validateDeclaredWorkspaces(ts.Workspaces, ts.Steps, ts.StepTemplate).ViaField("workspaces"))
 	errs = errs.Also(validateWorkspaceUsages(ctx, ts))

pkg/apis/pipeline/v1beta1/task_validation_test.go

@@ -1245,3 +1245,85 @@ func getContextBasedOnFeatureFlag(featureFlag string) context.Context {
 
 	return config.ToContext(context.Background(), cfg)
 }
+func TestSubstitutedContext(t *testing.T) {
+	type fields struct {
+		Params            []v1beta1.ParamSpec
+		Steps             []v1beta1.Step
+		ValidateWorkspace bool
+	}
+	tests := []struct {
+		name          string
+		fields        fields
+		expectedError apis.FieldError
+	}{{
+		name: "variable not substituted",
+		fields: fields{
+			Steps: []v1beta1.Step{{
+				Image: "my-image",
+				Args:  []string{"params"},
+				Script: `
+				#!/usr/bin/env  bash
+				hello "$(params.a)"`,
+			}},
+			ValidateWorkspace: false,
+		},
+		expectedError: apis.FieldError{
+			Message: `non-existent variable in "\n\t\t\t\t#!/usr/bin/env  bash\n\t\t\t\thello \"$(params.a)\""`,
+			Paths:   []string{"steps[0].script"},
+		},
+	}, {
+		name: "variable substituted double quoted",
+		fields: fields{
+			Steps: []v1beta1.Step{{
+				Image: "my-image",
+				Args:  []string{"params"},
+				Script: `
+				#!/usr/bin/env  bash
+				hello "$(params.a)"`,
+			}},
+			ValidateWorkspace: true,
+		},
+	}, {
+		name: "variable substituted not quoted",
+		fields: fields{
+			Steps: []v1beta1.Step{{
+				Image: "my-image",
+				Args:  []string{"params"},
+				Script: `
+				#!/usr/bin/env  bash
+				hello $(params.a)`,
+			}},
+			ValidateWorkspace: true,
+		},
+	}, {
+		name: "variable substituted single quoted",
+		fields: fields{
+			Steps: []v1beta1.Step{{
+				Image:  "my-image",
+				Args:   []string{"params"},
+				Script: "echo `$(params.a)`",
+			}},
+			ValidateWorkspace: true,
+		},
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ts := &v1beta1.TaskSpec{
+				Params: tt.fields.Params,
+				Steps:  tt.fields.Steps,
+			}
+			ctx := context.Background()
+			ts.SetDefaults(ctx)
+			if tt.fields.ValidateWorkspace {
+				ctx = apis.WithinSubstituted(ctx)
+			}
+			err := ts.Validate(ctx)
+			if err == nil && tt.expectedError.Error() != "" {
+				t.Fatalf("Expected an error, got nothing for %v", ts)
+			}
+			if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
+				t.Errorf("TaskSpec.Validate() errors diff %s", diff.PrintWantGot(d))
+			}
+		})
+	}
+}

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -376,7 +376,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get
 		return controller.NewPermanentError(err)
 	}
 
-	if err := pipelineSpec.Validate(ctx); err != nil {
+	if err := pipelineSpec.Validate(apis.WithinSubstituted(ctx)); err != nil {
 		// This Run has failed, so we need to mark it as failed and stop reconciling it
 		pr.Status.MarkFailed(ReasonFailedValidation,
 			"Pipeline %s/%s can't be Run; it has an invalid spec: %s",

pkg/reconciler/taskrun/taskrun.go

@@ -667,7 +667,7 @@ func (c *Reconciler) createPod(ctx context.Context, tr *v1beta1.TaskRun, rtr *re
 	// Apply step exitCode path substitution
 	ts = resources.ApplyStepExitCodePath(ts)
 
-	if validateErr := ts.Validate(ctx); validateErr != nil {
+	if validateErr := ts.Validate(apis.WithinSubstituted(ctx)); validateErr != nil {
 		logger.Errorf("Failed to create a pod for taskrun: %s due to task validation error %v", tr.Name, validateErr)
 		return nil, validateErr
 	}