diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index 1aa5ffaaefe..2acc4130672 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -3572,9 +3572,7 @@ string (Optional)

Image reference name. -More info: https://kubernetes.io/docs/concepts/containers/images -This field is optional to allow higher level config management to default or override -container images in workload controllers like Deployments and StatefulSets.

+More info: https://kubernetes.io/docs/concepts/containers/images

@@ -4665,9 +4663,7 @@ string (Optional)

Image reference name. -More info: https://kubernetes.io/docs/concepts/containers/images -This field is optional to allow higher level config management to default or override -container images in workload controllers like Deployments and StatefulSets.

+More info: https://kubernetes.io/docs/concepts/containers/images

diff --git a/pkg/apis/pipeline/v1/container_types.go b/pkg/apis/pipeline/v1/container_types.go index a15655521f7..b7da45e0b57 100644 --- a/pkg/apis/pipeline/v1/container_types.go +++ b/pkg/apis/pipeline/v1/container_types.go @@ -249,8 +249,6 @@ func (s *Step) GetVarSubstitutionExpressions() []string { type StepTemplate struct { // Image reference name. // More info: https://kubernetes.io/docs/concepts/containers/images - // This field is optional to allow higher level config management to default or override - // container images in workload controllers like Deployments and StatefulSets. // +optional Image string `json:"image,omitempty" protobuf:"bytes,2,opt,name=image"` // Entrypoint array. Not executed within a shell. @@ -369,8 +367,6 @@ type Sidecar struct { Name string `json:"name" protobuf:"bytes,1,opt,name=name"` // Image reference name. // More info: https://kubernetes.io/docs/concepts/containers/images - // This field is optional to allow higher level config management to default or override - // container images in workload controllers like Deployments and StatefulSets. // +optional Image string `json:"image,omitempty" protobuf:"bytes,2,opt,name=image"` // Entrypoint array. Not executed within a shell. diff --git a/pkg/apis/pipeline/v1/openapi_generated.go b/pkg/apis/pipeline/v1/openapi_generated.go index 0fb74d29b15..fe5d925166f 100644 --- a/pkg/apis/pipeline/v1/openapi_generated.go +++ b/pkg/apis/pipeline/v1/openapi_generated.go @@ -2364,7 +2364,7 @@ func schema_pkg_apis_pipeline_v1_Sidecar(ref common.ReferenceCallback) common.Op }, "image": { SchemaProps: spec.SchemaProps{ - Description: "Image reference name. More info: https://kubernetes.io/docs/concepts/containers/images This field is optional to allow higher level config management to default or override container images in workload controllers like Deployments and StatefulSets.", + Description: "Image reference name. More info: https://kubernetes.io/docs/concepts/containers/images", Type: []string{"string"}, Format: "", }, @@ -3160,7 +3160,7 @@ func schema_pkg_apis_pipeline_v1_StepTemplate(ref common.ReferenceCallback) comm Properties: map[string]spec.Schema{ "image": { SchemaProps: spec.SchemaProps{ - Description: "Image reference name. More info: https://kubernetes.io/docs/concepts/containers/images This field is optional to allow higher level config management to default or override container images in workload controllers like Deployments and StatefulSets.", + Description: "Image reference name. More info: https://kubernetes.io/docs/concepts/containers/images", Type: []string{"string"}, Format: "", }, diff --git a/pkg/apis/pipeline/v1/swagger.json b/pkg/apis/pipeline/v1/swagger.json index d99469bffd7..8093c7e8389 100644 --- a/pkg/apis/pipeline/v1/swagger.json +++ b/pkg/apis/pipeline/v1/swagger.json @@ -1232,7 +1232,7 @@ "x-kubernetes-list-type": "atomic" }, "image": { - "description": "Image reference name. More info: https://kubernetes.io/docs/concepts/containers/images This field is optional to allow higher level config management to default or override container images in workload controllers like Deployments and StatefulSets.", + "description": "Image reference name. More info: https://kubernetes.io/docs/concepts/containers/images", "type": "string" }, "imagePullPolicy": { @@ -1664,7 +1664,7 @@ "x-kubernetes-list-type": "atomic" }, "image": { - "description": "Image reference name. More info: https://kubernetes.io/docs/concepts/containers/images This field is optional to allow higher level config management to default or override container images in workload controllers like Deployments and StatefulSets.", + "description": "Image reference name. More info: https://kubernetes.io/docs/concepts/containers/images", "type": "string" }, "imagePullPolicy": { diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index d9686b67351..86c277ca968 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -89,7 +89,7 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) { } errs = errs.Also(validateSteps(ctx, mergedSteps).ViaField("steps")) - errs = errs.Also(validateSidecarNames(ts.Sidecars)) + errs = errs.Also(validateSidecars(ts.Sidecars).ViaField("sidecars")) errs = errs.Also(ValidateParameterTypes(ctx, ts.Params).ViaField("params")) errs = errs.Also(ValidateParameterVariables(ctx, ts.Steps, ts.Params)) errs = errs.Also(validateTaskContextVariables(ctx, ts.Steps)) @@ -149,18 +149,36 @@ func ValidateObjectParamsHaveProperties(ctx context.Context, params ParamSpecs) return errs } -func validateSidecarNames(sidecars []Sidecar) (errs *apis.FieldError) { +func validateSidecars(sidecars []Sidecar) (errs *apis.FieldError) { for _, sc := range sidecars { - if sc.Name == pipeline.ReservedResultsSidecarName { - errs = errs.Also(&apis.FieldError{ - Message: fmt.Sprintf("Invalid: cannot use reserved sidecar name %v ", sc.Name), - Paths: []string{"sidecars"}, - }) + errs = errs.Also(validateSidecarName(sc)) + + if sc.Image == "" { + errs = errs.Also(apis.ErrMissingField("image")) + } + + if sc.Script != "" { + if len(sc.Command) > 0 { + errs = errs.Also(&apis.FieldError{ + Message: "script cannot be used with command", + Paths: []string{"script"}, + }) + } } } return errs } +func validateSidecarName(sc Sidecar) (errs *apis.FieldError) { + if sc.Name == pipeline.ReservedResultsSidecarName { + errs = errs.Also(&apis.FieldError{ + Message: fmt.Sprintf("Invalid: cannot use reserved sidecar name %v ", sc.Name), + Paths: []string{"name"}, + }) + } + return errs +} + func validateResults(ctx context.Context, results []TaskResult) (errs *apis.FieldError) { for index, result := range results { errs = errs.Also(result.Validate(ctx).ViaIndex(index)) diff --git a/pkg/apis/pipeline/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go index 08e7657873a..6f38a2d38b0 100644 --- a/pkg/apis/pipeline/v1/task_validation_test.go +++ b/pkg/apis/pipeline/v1/task_validation_test.go @@ -1617,6 +1617,55 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { } } +func TestTaskSpecValidateErrorSidecars(t *testing.T) { + tests := []struct { + name string + sidecars []v1.Sidecar + expectedError apis.FieldError + }{{ + name: "missing image", + sidecars: []v1.Sidecar{{ + Name: "tekton-invalid-side-car", + }}, + expectedError: apis.FieldError{ + Message: "missing field(s)", + Paths: []string{"sidecars.image"}, + }, + }, { + name: "invalid command with script", + sidecars: []v1.Sidecar{{ + Name: "tekton-invalid-side-car", + Image: "ubuntu", + Command: []string{"command foo"}, + Script: ` + #!/usr/bin/env bash + echo foo`, + }}, + expectedError: apis.FieldError{ + Message: "script cannot be used with command", + Paths: []string{"sidecars.script"}, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := &v1.TaskSpec{ + Steps: []v1.Step{{ + Name: "does-not-matter", + Image: "does-not-matter", + }}, + Sidecars: tt.sidecars, + } + err := ts.Validate(context.Background()) + if err == nil { + 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)) + } + }) + } +} + func TestTaskSpecValidateErrorSidecarName(t *testing.T) { tests := []struct { name string @@ -1630,7 +1679,7 @@ func TestTaskSpecValidateErrorSidecarName(t *testing.T) { }}, expectedError: apis.FieldError{ Message: fmt.Sprintf("Invalid: cannot use reserved sidecar name %v ", pipeline.ReservedResultsSidecarName), - Paths: []string{"sidecars"}, + Paths: []string{"sidecars.name"}, }, }} for _, tt := range tests { diff --git a/test/conversion_test.go b/test/conversion_test.go index 96896acd296..9977390de9e 100644 --- a/test/conversion_test.go +++ b/test/conversion_test.go @@ -115,7 +115,6 @@ spec: volumeMounts: - name: messages mountPath: /messages - script: echo test volumes: - name: messages emptyDir: {} @@ -189,7 +188,6 @@ spec: mountPath: /messages securityContext: runAsNonRoot: true - script: echo test volumes: - name: messages emptyDir: {}