diff --git a/pkg/linter/script.go b/pkg/linter/script.go index d847f3fa..f4033b59 100644 --- a/pkg/linter/script.go +++ b/pkg/linter/script.go @@ -22,6 +22,7 @@ import ( "github.com/tektoncd/catlin/pkg/parser" "github.com/tektoncd/catlin/pkg/validator" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" ) @@ -88,20 +89,20 @@ func NewScriptLinter(r *parser.Resource) *taskLinter { } // nolint: staticcheck -func (t *taskLinter) validateScript(taskName string, s v1beta1.Step, configs []config) validator.Result { +func (t *taskLinter) validateScript(taskName string, script string, configs []config, stepName string) validator.Result { result := validator.Result{} // use /bin/sh by default if no shbang - if s.Script[0:2] != "#!" { - s.Script = "#!/usr/bin/env sh\n" + s.Script + if script[0:2] != "#!" { + script = "#!/usr/bin/env sh\n" + script } else { // using a shbang, check if we have /usr/bin/env - if s.Script[0:14] != "#!/usr/bin/env" { + if script[0:14] != "#!/usr/bin/env" { result.Warn("step: %s is not using #!/usr/bin/env ", taskName) } } for _, config := range configs { - matched, err := regexp.MatchString(`^#!`+config.regexp+`\n`, s.Script) + matched, err := regexp.MatchString(`^#!`+config.regexp+`\n`, script) if err != nil { result.Error("Invalid regexp: %s", config.regexp) return result @@ -123,7 +124,7 @@ func (t *taskLinter) validateScript(taskName string, s v1beta1.Step, configs []c return result } defer os.Remove(tmpfile.Name()) // clean up - if _, err := tmpfile.Write([]byte(s.Script)); err != nil { + if _, err := tmpfile.Write([]byte(script)); err != nil { result.Error("Cannot write to temporary files") return result } @@ -138,7 +139,7 @@ func (t *taskLinter) validateScript(taskName string, s v1beta1.Step, configs []c cmd := exec.Command(execpath, append(linter.args, tmpfile.Name())...) out, err := cmd.CombinedOutput() if err != nil { - outt := strings.ReplaceAll(string(out), tmpfile.Name(), taskName+"-"+s.Name) + outt := strings.ReplaceAll(string(out), tmpfile.Name(), taskName+"-"+stepName) result.Error("%s, %s failed:\n%s", execpath, linter.args, outt) } } @@ -148,10 +149,18 @@ func (t *taskLinter) validateScript(taskName string, s v1beta1.Step, configs []c } // nolint: staticcheck -func (t *taskLinter) collectOverSteps(steps []v1beta1.Step, name string, result *validator.Result) { - for _, step := range steps { - if step.Script != "" { - result.Append(t.validateScript(name, step, t.configs)) +func (t *taskLinter) collectOverSteps(steps interface{}, name string, result *validator.Result) { + if s, ok := steps.([]v1beta1.Step); ok { + for _, step := range s { + if step.Script != "" { + result.Append(t.validateScript(name, step.Script, t.configs, step.Name)) + } + } + } else if s, ok := steps.([]v1.Step); ok { + for _, step := range s { + if step.Script != "" { + result.Append(t.validateScript(name, step.Script, t.configs, step.Name)) + } } } } @@ -167,8 +176,14 @@ func (t *taskLinter) Validate() validator.Result { switch strings.ToLower(t.res.Kind) { case "task": - task := res.(*v1beta1.Task) - t.collectOverSteps(task.Spec.Steps, task.ObjectMeta.Name, &result) + if res.(*v1.Task) != nil { + task := res.(*v1.Task) + t.collectOverSteps(task.Spec.Steps, task.ObjectMeta.Name, &result) + } else { + task := res.(*v1beta1.Task) + t.collectOverSteps(task.Spec.Steps, task.ObjectMeta.Name, &result) + } + case "clustertask": task := res.(*v1beta1.ClusterTask) t.collectOverSteps(task.Spec.Steps, task.ObjectMeta.Name, &result) diff --git a/pkg/linter/script_test.go b/pkg/linter/script_test.go index 9da73d03..4c9af621 100644 --- a/pkg/linter/script_test.go +++ b/pkg/linter/script_test.go @@ -40,6 +40,23 @@ spec: echo "hello moto" ` +const taskScriptValidatorGoodV1 = ` +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: hello-moto +spec: + steps: + - name: s1 + image: image1 + script: | + #!/usr/bin/env sh + echo "Hello world" + script: | + #!/bin/sh + echo "hello moto" +` + const taskScriptValidatorNoGood = ` apiVersion: tekton.dev/v1beta1 kind: Task @@ -58,6 +75,24 @@ spec: #!/bin/sh echo "hello world" ` +const taskScriptValidatorNoGoodV1 = ` +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: hello-moto +spec: + steps: + - name: nogood + image: image1 + script: | + #!/usr/bin/env sh + ' + - name: warn + image: image1 + script: | + #!/bin/sh + echo "hello world" +` const clusterTaskTest = ` apiVersion: tekton.dev/v1beta1 @@ -84,6 +119,17 @@ spec: taskRef: name: task-1` +const pipelineWithTaskRefV1 = ` +apiVersion: tekton.dev/v1 +kind: Pipeline +metadata: + name: pipelien-1 +spec: + tasks: + - name: pipeline-1 + taskRef: + name: task-1` + var configSh = []config{ { regexp: `(/usr/bin/env |/bin/)sh`, @@ -110,6 +156,20 @@ func TestTaskLint_Script_good(t *testing.T) { result := tl.Validate() assert.Equal(t, 0, result.Errors) } +func TestTaskLint_Script_goodV1(t *testing.T) { + r := strings.NewReader(taskScriptValidatorGoodV1) + parser := parser.ForReader(r) + + res, err := parser.Parse() + assert.NilError(t, err) + + tl := &taskLinter{ + res: res, + configs: configSh, + } + result := tl.Validate() + assert.Equal(t, 0, result.Errors) +} func TestTaskLint_Script_no_good(t *testing.T) { r := strings.NewReader(taskScriptValidatorNoGood) @@ -125,6 +185,20 @@ func TestTaskLint_Script_no_good(t *testing.T) { result := tl.Validate() assert.Equal(t, 1, result.Errors) } +func TestTaskLint_Script_no_goodV1(t *testing.T) { + r := strings.NewReader(taskScriptValidatorNoGoodV1) + parser := parser.ForReader(r) + + res, err := parser.Parse() + assert.NilError(t, err) + + tl := &taskLinter{ + res: res, + configs: configSh, + } + result := tl.Validate() + assert.Equal(t, 1, result.Errors) +} func Test_Pipeline_skip(t *testing.T) { r := strings.NewReader(pipelineWithTaskRef) @@ -140,6 +214,20 @@ func Test_Pipeline_skip(t *testing.T) { result := tl.Validate() assert.Assert(t, is.Nil(result.Lints)) } +func Test_Pipeline_skipV1(t *testing.T) { + r := strings.NewReader(pipelineWithTaskRefV1) + parser := parser.ForReader(r) + + res, err := parser.Parse() + assert.NilError(t, err) + + tl := &taskLinter{ + res: res, + configs: configSh, + } + result := tl.Validate() + assert.Assert(t, is.Nil(result.Lints)) +} func Test_ClusterTaskParse(t *testing.T) { r := strings.NewReader(clusterTaskTest) diff --git a/pkg/parser/parser.go b/pkg/parser/parser.go index 1a7a9d24..0d1f5640 100644 --- a/pkg/parser/parser.go +++ b/pkg/parser/parser.go @@ -29,10 +29,14 @@ import ( "knative.dev/pkg/apis" "github.com/tektoncd/catlin/pkg/consts" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" ) func registerSchema() { + v1 := runtime.NewSchemeBuilder(v1.AddToScheme) + _ = v1.AddToScheme(scheme.Scheme) + beta1 := runtime.NewSchemeBuilder(v1beta1.AddToScheme) _ = beta1.AddToScheme(scheme.Scheme) } @@ -152,12 +156,19 @@ type tektonResource interface { // nolint: staticcheck func typeForKind(kind string) (tektonResource, error) { + isV1 := (&v1.Task{} != nil) switch kind { case "Task": + if isV1 { + return &v1.Task{}, nil + } return &v1beta1.Task{}, nil case "ClusterTask": return &v1beta1.ClusterTask{}, nil case "Pipeline": + if isV1 { + return &v1.Pipeline{}, nil + } return &v1beta1.Pipeline{}, nil } @@ -166,5 +177,5 @@ func typeForKind(kind string) (tektonResource, error) { func isTektonKind(gvk *schema.GroupVersionKind) bool { id := gvk.GroupVersion().Identifier() - return id == v1beta1.SchemeGroupVersion.Identifier() + return id == v1.SchemeGroupVersion.Identifier() || id == v1beta1.SchemeGroupVersion.Identifier() } diff --git a/pkg/validator/task_validator.go b/pkg/validator/task_validator.go index 90de3b66..52e21ea3 100644 --- a/pkg/validator/task_validator.go +++ b/pkg/validator/task_validator.go @@ -20,7 +20,9 @@ import ( "strings" "github.com/google/go-containerregistry/pkg/name" + v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + corev1 "k8s.io/api/core/v1" "github.com/tektoncd/catlin/pkg/parser" ) @@ -50,36 +52,66 @@ func (t *taskValidator) Validate() Result { return result } - task := res.(*v1beta1.Task) - for _, step := range task.Spec.Steps { - result.Append(t.validateStep(step)) + switch task := res.(type) { + case *v1.Task: + for _, step := range task.Spec.Steps { + result.Append(t.validateStep(step)) + } + case *v1beta1.Task: + for _, step := range task.Spec.Steps { + result.Append(t.validateStep(step)) + } } + return result } -func (t *taskValidator) validateStep(s v1beta1.Step) Result { +func (t *taskValidator) validateStep(step interface{}) Result { result := Result{} - step := s.Name - img := s.Image + + var ( + version, stepName, img, script string + env []corev1.EnvVar + envFrom []corev1.EnvFromSource + ) + + switch s := step.(type) { + case v1.Step: + version = "v1" + stepName = s.Name + img = s.Image + env = s.Env + envFrom = s.EnvFrom + script = s.Script + case v1beta1.Step: + version = "v1beta1" + stepName = s.Name + img = s.Image + env = s.Env + envFrom = s.EnvFrom + script = s.Script + default: + return result + } if _, usesVars := extractExpressionFromString(img, ""); usesVars { - result.Warn("Step %q uses image %q that contains variables; skipping validation", step, img) + result.Warn("Step %q uses image %q that contains variables; skipping validation", stepName, img) return result } if !strings.Contains(img, "/") || !isValidRegistry(img) { - result.Warn("Step %q uses image %q; consider using a fully qualified name - e.g. docker.io/library/ubuntu:1.0", step, img) + result.Warn("Step %q uses image %q; consider using a fully qualified name - e.g. docker.io/library/ubuntu:1.0", stepName, img) } if strings.Contains(img, "@sha256") { rep, err := name.NewDigest(img, name.WeakValidation) if err != nil { - result.Error("Step %q uses image %q with an invalid digest. Error: %s", step, img, err) + result.Error("Step %q uses image %q with an invalid digest. Error: %s", stepName, img, err) return result } if !tagWithDigest(rep.String()) { - result.Warn("Step %q uses image %q; consider using a image tagged with specific version along with digest eg. abc.io/img:v1@sha256:abcde", step, img) + result.Warn("Step %q uses image %q; consider using an image tagged with specific version along with digest eg. abc.io/img:v1@sha256:abcde", stepName, img) } return result @@ -87,35 +119,48 @@ func (t *taskValidator) validateStep(s v1beta1.Step) Result { ref, err := name.NewTag(img, name.WeakValidation) if err != nil { - result.Error("Step %q uses image %q with an invalid tag. Error: %s", step, img, err) + result.Error("Step %q uses image %q with an invalid tag. Error: %s", stepName, img, err) return result } if strings.EqualFold(ref.Identifier(), "latest") { - result.Error("Step %q uses image %q which must be tagged with a specific version", step, img) + result.Error("Step %q uses image %q which must be tagged with a specific version", stepName, img) } // According to [CIS benchmarks](https://cloud.google.com/kubernetes-engine/docs/concepts/cis-benchmarks). // > 5.4.1 Prefer using secrets as files over secrets as environment variables - for _, env := range s.Env { - if env.ValueFrom == nil || env.ValueFrom.SecretKeyRef == nil { - continue + for _, e := range env { + switch version { + case "v1": + if e.ValueFrom != nil && e.ValueFrom.SecretKeyRef != nil { + result.Warn("Step %q uses secret to populate env %q. Prefer using secrets as files over secrets as environment variables", stepName, e.Name) + } + + case "v1beta1": + if e.ValueFrom != nil && e.ValueFrom.SecretKeyRef != nil { + result.Warn("Step %q uses secret to populate env %q. Prefer using secrets as files over secrets as environment variables", stepName, e.Name) + } } - result.Warn("Step %q uses secret to populate env %q. Prefer using secrets as files over secrets as environment variables", step, env.Name) } - for _, envFrom := range s.EnvFrom { - if envFrom.SecretRef == nil { - continue + for _, e := range envFrom { + switch version { + case "v1": + if e.SecretRef != nil { + result.Warn("Step %q uses secret as environment variables. Prefer using secrets as files over secrets as environment variables", stepName) + } + case "v1beta1": + if e.SecretRef != nil { + result.Warn("Step %q uses secret as environment variables. Prefer using secrets as files over secrets as environment variables", stepName) + } } - result.Warn("Step %q uses secret as environment variables. Prefer using secrets as files over secrets as environment variables", step) } - if s.Script != "" { - expr, _ := extractExpressionFromString(s.Script, "params") + if script != "" { + expr, _ := extractExpressionFromString(script, "params") if expr != "" { result.Warn( "Step %q references %q directly from its script block. For reliability and security, consider putting the param into an environment variable of the Step and accessing that environment variable in your script instead.", - step, + stepName, expr) } } diff --git a/pkg/validator/task_validator_test.go b/pkg/validator/task_validator_test.go index 87be665f..7c085dba 100644 --- a/pkg/validator/task_validator_test.go +++ b/pkg/validator/task_validator_test.go @@ -67,6 +67,51 @@ spec: - name: s11 image: quay.io/foo/bar:stable-2.12-latest ` +const taskWithValidImageRefV1 = ` +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: valid-image-refs + labels: + app.kubernetes.io/version: a,b,c + annotations: + tekton.dev/tags: a,b,c + tekton.dev/pipelines.minVersion: "0.12" + tekton.dev/displayName: My Example Task +spec: + description: |- + A summary of the resource + + A para about this valid task + + params: + - name: img_ver + type: string + + steps: + - name: s1 + image: docker.io/foo:bar + - name: s2 + image: docker.io/abc/foo:baz + - name: s3 + image: quay.io/foo:bar + - name: s4 + image: r.j3ss.co/clisp:1.0 + - name: s5 + image: gcr.io/foo/bar/baz:bar + - name: s6 + image: abc.io/fedora:1.0@sha256:deadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33f + - name: s7 + image: abc.io/xyz/fedora:v123@sha256:deadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33f + - name: s8 + image: 172.16.3.4:3000/foo/bar:baz + - name: s9 + image: abc.io/xyz/fedora:latest-0.2@sha256:deadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33f + - name: s10 + image: abc.io/xyz/fedora:latest0.2@sha256:deadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33f + - name: s11 + image: quay.io/foo/bar:stable-2.12-latest +` const taskWithInvalidImageRef = ` apiVersion: tekton.dev/v1beta1 @@ -119,6 +164,57 @@ spec: - name: s13 image: docker.io/abc/foo:$(params.version) ` +const taskWithInvalidImageRefV1 = ` +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: invalid-image-refs + labels: + app.kubernetes.io/version: a,b,c + annotations: + tekton.dev/tags: a,b,c + tekton.dev/pipelines.minVersion: "0.12" + tekton.dev/displayName: My Example Task +spec: + description: |- + A summary of the resource + + A para about this valid task + + params: + - name: foo + type: string + - name: version + type: string + + steps: + - name: s1 + image: ubuntu + - name: s2 + image: api:latest + - name: s3 + image: docker.io/foo + - name: s4 + image: r.j3ss.co/clisp:latest + - name: s5 + image: docker.io/abc/foo + - name: s6 + image: gcr.io/foo/bar/baz:latest + - name: s7 + image: abc.io/fedora@sha256:deadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33f + - name: s8 + image: abc.io/fedora:latest@sha256:deadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33f + - name: s9 + image: gcr.io/k8s-staging-boskos/boskosctl@sha256:a7fc984732 + - name: s10 + image: localhost:5000/foo:bar + - name: s11 + image: $(input.params.foo) + - name: s12 + image: docker.io/xyz/$(params.foo) + - name: s13 + image: docker.io/abc/foo:$(params.version) +` const taskWithEnvFromSecret = ` apiVersion: tekton.dev/v1beta1 @@ -172,6 +268,57 @@ spec: name: $(params.configmap) ` +const taskWithEnvFromSecretV1 = ` +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: invalid-env-or-envfrom + labels: + app.kubernetes.io/version: a,b,c + annotations: + tekton.dev/tags: a,b,c + tekton.dev/pipelines.minVersion: "0.12" + tekton.dev/displayName: My Example Task +spec: + description: |- + A summary of the resource + + A para about this valid task + + params: + - name: secret + type: string + - name: configmap + type: string + + steps: + - name: s1 + image: docker.io/foo:bar + env: + - name: PASS + valueFrom: + secretKeyRef: + name: $(params.secret) + key: PASS + - name: s2 + image: docker.io/foo:bar + env: + - name: USER + valueFrom: + configMapKeyRef: + name: $(params.configmap) + key: USER + - name: s3 + image: docker.io/foo:bar + envFrom: + - secretRef: + name: $(params.secret) + - name: s4 + image: docker.io/foo:bar + envFrom: + - configMapRef: + name: $(params.configmap) +` const taskWithScriptUsingParams = ` apiVersion: tekton.dev/v1beta1 kind: Task @@ -199,6 +346,33 @@ spec: #!/usr/bin/env python print("""$(params.secret)""") ` +const taskWithScriptUsingParamsV1 = ` +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: invalid-env-or-envfrom + labels: + app.kubernetes.io/version: a,b,c + annotations: + tekton.dev/tags: a,b,c + tekton.dev/pipelines.minVersion: "0.12" + tekton.dev/displayName: My Example Task +spec: + description: foo + params: + - name: secret + type: string + steps: + - name: s1 + image: docker.io/foo:bar + script: | + echo "$(params.secret)" + - name: s2 + image: docker.io/foo:bar + script: | + #!/usr/bin/env python + print("""$(params.secret)""") +` func TestTaskValidator_ValidImageRef(t *testing.T) { r := strings.NewReader(taskWithValidImageRef) @@ -215,6 +389,21 @@ func TestTaskValidator_ValidImageRef(t *testing.T) { assert.Equal(t, 0, len(lints)) } +func TestTaskValidator_ValidImageRefV1(t *testing.T) { + r := strings.NewReader(taskWithValidImageRefV1) + parser := parser.ForReader(r) + + res, err := parser.Parse() + assert.NilError(t, err) + + v := ForKind(res) + result := v.Validate() + assert.Equal(t, 0, result.Errors) + + lints := result.Lints + assert.Equal(t, 0, len(lints)) +} + func TestTaskValidator_InvalidImageRef(t *testing.T) { r := strings.NewReader(taskWithInvalidImageRef) parser := parser.ForReader(r) @@ -259,11 +448,82 @@ func TestTaskValidator_InvalidImageRef(t *testing.T) { // image with digest and a tag assert.Equal(t, Warning, lints[8].Kind) - assert.Equal(t, `Step "s7" uses image "abc.io/fedora@sha256:deadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33f"; consider using a image tagged with specific version along with digest eg. abc.io/img:v1@sha256:abcde`, result.Lints[8].Message) + assert.Equal(t, `Step "s7" uses image "abc.io/fedora@sha256:deadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33f"; consider using an image tagged with specific version along with digest eg. abc.io/img:v1@sha256:abcde`, result.Lints[8].Message) // image with digest and latest tag assert.Equal(t, Warning, lints[9].Kind) - assert.Equal(t, `Step "s8" uses image "abc.io/fedora:latest@sha256:deadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33f"; consider using a image tagged with specific version along with digest eg. abc.io/img:v1@sha256:abcde`, result.Lints[9].Message) + assert.Equal(t, `Step "s8" uses image "abc.io/fedora:latest@sha256:deadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33f"; consider using an image tagged with specific version along with digest eg. abc.io/img:v1@sha256:abcde`, result.Lints[9].Message) + + // image with invalid digest + assert.Equal(t, Error, lints[10].Kind) + assert.Equal(t, `Step "s9" uses image "gcr.io/k8s-staging-boskos/boskosctl@sha256:a7fc984732" with an invalid digest. Error: invalid checksum digest length`, result.Lints[10].Message) + + // image with invalid registry + assert.Equal(t, Warning, lints[11].Kind) + assert.Equal(t, `Step "s10" uses image "localhost:5000/foo:bar"; consider using a fully qualified name - e.g. docker.io/library/ubuntu:1.0`, result.Lints[11].Message) + + // image with variable + assert.Equal(t, Warning, lints[12].Kind) + assert.Equal(t, `Step "s11" uses image "$(input.params.foo)" that contains variables; skipping validation`, result.Lints[12].Message) + + // image with variable + assert.Equal(t, Warning, lints[13].Kind) + assert.Equal(t, `Step "s12" uses image "docker.io/xyz/$(params.foo)" that contains variables; skipping validation`, result.Lints[13].Message) + + // image with variable + assert.Equal(t, Warning, lints[14].Kind) + assert.Equal(t, `Step "s13" uses image "docker.io/abc/foo:$(params.version)" that contains variables; skipping validation`, result.Lints[14].Message) +} + +func TestTaskValidator_InvalidImageRefV1(t *testing.T) { + r := strings.NewReader(taskWithInvalidImageRefV1) + parser := parser.ForReader(r) + + res, err := parser.Parse() + assert.NilError(t, err) + + v := ForKind(res) + result := v.Validate() + assert.Equal(t, 7, result.Errors) + + lints := result.Lints + assert.Equal(t, 15, len(lints)) + + // image without full reference + assert.Equal(t, Warning, lints[0].Kind) + assert.Equal(t, `Step "s1" uses image "ubuntu"; consider using a fully qualified name - e.g. docker.io/library/ubuntu:1.0`, lints[0].Message) + assert.Equal(t, Error, lints[1].Kind) + assert.Equal(t, `Step "s1" uses image "ubuntu" which must be tagged with a specific version`, result.Lints[1].Message) + + // image without full reference but with latest tag + assert.Equal(t, Warning, lints[2].Kind) + assert.Equal(t, `Step "s2" uses image "api:latest"; consider using a fully qualified name - e.g. docker.io/library/ubuntu:1.0`, lints[2].Message) + assert.Equal(t, Error, lints[3].Kind) + assert.Equal(t, `Step "s2" uses image "api:latest" which must be tagged with a specific version`, result.Lints[3].Message) + + // image without a tag + assert.Equal(t, Error, lints[4].Kind) + assert.Equal(t, `Step "s3" uses image "docker.io/foo" which must be tagged with a specific version`, result.Lints[4].Message) + + // image with latest tag + assert.Equal(t, Error, lints[5].Kind) + assert.Equal(t, `Step "s4" uses image "r.j3ss.co/clisp:latest" which must be tagged with a specific version`, result.Lints[5].Message) + + // image without a tag + assert.Equal(t, Error, lints[6].Kind) + assert.Equal(t, `Step "s5" uses image "docker.io/abc/foo" which must be tagged with a specific version`, result.Lints[6].Message) + + // image with latest tag + assert.Equal(t, Error, lints[7].Kind) + assert.Equal(t, `Step "s6" uses image "gcr.io/foo/bar/baz:latest" which must be tagged with a specific version`, result.Lints[7].Message) + + // image with digest and a tag + assert.Equal(t, Warning, lints[8].Kind) + assert.Equal(t, `Step "s7" uses image "abc.io/fedora@sha256:deadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33f"; consider using an image tagged with specific version along with digest eg. abc.io/img:v1@sha256:abcde`, result.Lints[8].Message) + + // image with digest and latest tag + assert.Equal(t, Warning, lints[9].Kind) + assert.Equal(t, `Step "s8" uses image "abc.io/fedora:latest@sha256:deadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33fdeadb33f"; consider using an image tagged with specific version along with digest eg. abc.io/img:v1@sha256:abcde`, result.Lints[9].Message) // image with invalid digest assert.Equal(t, Error, lints[10].Kind) @@ -308,6 +568,28 @@ func TestTaskValidator_ValidEnvFromSecret(t *testing.T) { assert.Equal(t, `Step "s3" uses secret as environment variables. Prefer using secrets as files over secrets as environment variables`, result.Lints[1].Message) } +func TestTaskValidator_ValidEnvFromSecretV1(t *testing.T) { + r := strings.NewReader(taskWithEnvFromSecretV1) + parser := parser.ForReader(r) + + res, err := parser.Parse() + assert.NilError(t, err) + + v := ForKind(res) + result := v.Validate() + assert.Equal(t, 0, result.Errors) + + assert.Equal(t, 2, len(result.Lints)) + + // env.secretKeyRef generates a warning + assert.Equal(t, Warning, result.Lints[0].Kind) + assert.Equal(t, `Step "s1" uses secret to populate env "PASS". Prefer using secrets as files over secrets as environment variables`, result.Lints[0].Message) + + // envFrom.secretRef generates a warning + assert.Equal(t, Warning, result.Lints[1].Kind) + assert.Equal(t, `Step "s3" uses secret as environment variables. Prefer using secrets as files over secrets as environment variables`, result.Lints[1].Message) +} + func TestTaskValidator_ScriptUsingParams(t *testing.T) { r := strings.NewReader(taskWithScriptUsingParams) parser := parser.ForReader(r) @@ -327,3 +609,22 @@ func TestTaskValidator_ScriptUsingParams(t *testing.T) { assert.Equal(t, Warning, result.Lints[1].Kind) assert.Equal(t, `Step "s2" references "$(params.secret)" directly from its script block. For reliability and security, consider putting the param into an environment variable of the Step and accessing that environment variable in your script instead.`, result.Lints[1].Message) } +func TestTaskValidator_ScriptUsingParamsV1(t *testing.T) { + r := strings.NewReader(taskWithScriptUsingParamsV1) + parser := parser.ForReader(r) + + res, err := parser.Parse() + assert.NilError(t, err) + + v := ForKind(res) + result := v.Validate() + assert.Equal(t, 0, result.Errors) + + assert.Equal(t, 2, len(result.Lints)) + + assert.Equal(t, Warning, result.Lints[0].Kind) + assert.Equal(t, `Step "s1" references "$(params.secret)" directly from its script block. For reliability and security, consider putting the param into an environment variable of the Step and accessing that environment variable in your script instead.`, result.Lints[0].Message) + + assert.Equal(t, Warning, result.Lints[1].Kind) + assert.Equal(t, `Step "s2" references "$(params.secret)" directly from its script block. For reliability and security, consider putting the param into an environment variable of the Step and accessing that environment variable in your script instead.`, result.Lints[1].Message) +}