diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index b89b21370bc..6e5ee4b0ab0 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -3512,9 +3512,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

@@ -4477,9 +4475,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 ccef95cf767..3cfd436d25d 100644 --- a/pkg/apis/pipeline/v1/container_types.go +++ b/pkg/apis/pipeline/v1/container_types.go @@ -222,8 +222,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. @@ -342,8 +340,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 6425ec1de3b..97009643432 100644 --- a/pkg/apis/pipeline/v1/openapi_generated.go +++ b/pkg/apis/pipeline/v1/openapi_generated.go @@ -2327,7 +2327,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: "", }, @@ -3013,7 +3013,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 76d325bfff0..399835a458a 100644 --- a/pkg/apis/pipeline/v1/swagger.json +++ b/pkg/apis/pipeline/v1/swagger.json @@ -1214,7 +1214,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": { @@ -1587,7 +1587,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 e85179d60d1..75d898fb50b 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -88,7 +88,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)) @@ -148,18 +148,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 b04d43f190e..2b51d346dfe 100644 --- a/pkg/apis/pipeline/v1/task_validation_test.go +++ b/pkg/apis/pipeline/v1/task_validation_test.go @@ -1370,6 +1370,55 @@ func TestTaskSpecValidateError(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 @@ -1383,7 +1432,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: {}