From cd0b12d18f0dc2e6e04915d4a6d558df3e4e32bc Mon Sep 17 00:00:00 2001 From: Eric Zhang Date: Fri, 24 May 2024 11:36:51 -0400 Subject: [PATCH] chore: rebase --- .../alpha/consume-artifacts-from-task.yaml | 11 ++- pkg/apis/pipeline/v1/pipeline_validation.go | 8 +- .../pipeline/v1/pipeline_validation_test.go | 78 +++++++++++++++++++ .../pipeline/v1beta1/pipeline_validation.go | 8 +- .../v1beta1/pipeline_validation_test.go | 78 +++++++++++++++++++ 5 files changed, 171 insertions(+), 12 deletions(-) diff --git a/examples/v1/pipelineruns/alpha/consume-artifacts-from-task.yaml b/examples/v1/pipelineruns/alpha/consume-artifacts-from-task.yaml index 8cf0aaadb18..023130ee9e8 100644 --- a/examples/v1/pipelineruns/alpha/consume-artifacts-from-task.yaml +++ b/examples/v1/pipelineruns/alpha/consume-artifacts-from-task.yaml @@ -50,8 +50,11 @@ spec: - produce-artifacts-task taskSpec: steps: - - name: write - image: bash:latest + - name: artifacts-consumer-python + image: python:latest script: | - #!/usr/bin/env bash - echo $(tasks.produce-artifacts-task.outputs) \ No newline at end of file + #!/usr/bin/env python3 + import json + data = json.loads('$(tasks.produce-artifacts-task.outputs)') + if data[0]['uri'] != "pkg:github/package-url/purl-spec@244fd47e07d1004f0aed9c": + exit(1) \ No newline at end of file diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 2c9eef47d74..504fcc3574b 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -889,17 +889,17 @@ func validateArtifactReference(ctx context.Context, tasks []PipelineTask, finalT if config.FromContextOrDefaults(ctx).FeatureFlags.EnableArtifacts { return errs } - for _, t := range tasks { + for i, t := range tasks { for _, v := range t.Params.extractValues() { if len(artifactref.TaskArtifactRegex.FindAllStringSubmatch(v, -1)) > 0 { - return errs.Also(apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to use artifacts feature.", config.EnableArtifacts), "")) + return errs.Also(apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to use artifacts feature.", config.EnableArtifacts), "").ViaField("params").ViaFieldIndex("tasks", i)) } } } - for _, t := range finalTasks { + for i, t := range finalTasks { for _, v := range t.Params.extractValues() { if len(artifactref.TaskArtifactRegex.FindAllStringSubmatch(v, -1)) > 0 { - return errs.Also(apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to use artifacts feature.", config.EnableArtifacts), "")) + return errs.Also(apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to use artifacts feature.", config.EnableArtifacts), "").ViaField("params").ViaFieldIndex("finally", i)) } } } diff --git a/pkg/apis/pipeline/v1/pipeline_validation_test.go b/pkg/apis/pipeline/v1/pipeline_validation_test.go index 236283d2630..f32bb6babe3 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1/pipeline_validation_test.go @@ -173,6 +173,31 @@ func TestPipeline_Validate_Success(t *testing.T) { }}, }, }, + }, { + name: "valid pipeline with pipeline task and final task referencing artifacts in task params with enable-artifacts flag true", + p: &Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: PipelineSpec{ + Description: "this is an invalid pipeline referencing artifacts with enable-artifacts flag false", + Tasks: []PipelineTask{{ + Name: "pre-task", + TaskRef: &TaskRef{Name: "foo-task"}, + }, { + Name: "consume-artifacts-task", + Params: Params{{Name: "aaa", Value: ParamValue{ + Type: ParamTypeString, + StringVal: "$(tasks.produce-artifacts-task.outputs)", + }}}, + TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, + }}, + }, + }, + wc: func(ctx context.Context) context.Context { + return cfgtesting.SetFeatureFlags(ctx, t, + map[string]string{ + "enable-artifacts": "true", + "enable-api-fields": "alpha"}) + }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1154,6 +1179,59 @@ func TestPipelineSpec_Validate_Failure(t *testing.T) { Message: `missing field(s)`, Paths: []string{"tasks[1].when[0]", "finally[0].when[0]"}, }, + }, { + name: "invalid pipeline with one pipeline task referencing artifacts in task params with enable-artifacts flag false", + ps: &PipelineSpec{ + Description: "this is an invalid pipeline referencing artifacts with enable-artifacts flag false", + Tasks: []PipelineTask{{ + Name: "pre-task", + TaskRef: &TaskRef{Name: "foo-task"}, + }, { + Name: "consume-artifacts-task", + Params: Params{{Name: "aaa", Value: ParamValue{ + Type: ParamTypeString, + StringVal: "$(tasks.produce-artifacts-task.outputs)", + }}}, + TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, + }}, + }, + expectedError: apis.FieldError{ + Message: `feature flag enable-artifacts should be set to true to use artifacts feature.`, + Paths: []string{"tasks[1].params"}, + }, + wc: func(ctx context.Context) context.Context { + return cfgtesting.SetFeatureFlags(ctx, t, + map[string]string{ + "enable-artifacts": "false", + "enable-api-fields": "alpha"}) + }, + }, { + name: "invalid pipeline with one final pipeline task referencing artifacts in params with enable-artifacts flag false", + ps: &PipelineSpec{ + Description: "this is an invalid pipeline referencing artifacts with enable-artifacts flag false", + Tasks: []PipelineTask{{ + Name: "pre-task", + TaskRef: &TaskRef{Name: "foo-task"}, + }}, + Finally: []PipelineTask{{ + Name: "consume-artifacts-task", + Params: Params{{Name: "aaa", Value: ParamValue{ + Type: ParamTypeString, + StringVal: "$(tasks.produce-artifacts-task.outputs)", + }}}, + TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, + }}, + }, + wc: func(ctx context.Context) context.Context { + return cfgtesting.SetFeatureFlags(ctx, t, + map[string]string{ + "enable-artifacts": "false", + "enable-api-fields": "alpha"}) + }, + expectedError: apis.FieldError{ + Message: `feature flag enable-artifacts should be set to true to use artifacts feature.`, + Paths: []string{"finally[1].params"}, + }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index 382fb7fbb4f..63aeb4e1b1e 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -893,17 +893,17 @@ func validateArtifactReference(ctx context.Context, tasks []PipelineTask, finalT if config.FromContextOrDefaults(ctx).FeatureFlags.EnableArtifacts { return errs } - for _, t := range tasks { + for i, t := range tasks { for _, v := range t.Params.extractValues() { if len(artifactref.TaskArtifactRegex.FindAllStringSubmatch(v, -1)) > 0 { - return errs.Also(apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to use artifacts feature.", config.EnableArtifacts), "")) + return errs.Also(apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to use artifacts feature.", config.EnableArtifacts), "").ViaField("params").ViaFieldIndex("tasks", i)) } } } - for _, t := range finalTasks { + for i, t := range finalTasks { for _, v := range t.Params.extractValues() { if len(artifactref.TaskArtifactRegex.FindAllStringSubmatch(v, -1)) > 0 { - return errs.Also(apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to use artifacts feature.", config.EnableArtifacts), "")) + return errs.Also(apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to use artifacts feature.", config.EnableArtifacts), "").ViaField("params").ViaFieldIndex("finally", i)) } } } diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index 0774662563e..bc60a8ef406 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -175,6 +175,31 @@ func TestPipeline_Validate_Success(t *testing.T) { }}, }, }, + }, { + name: "valid pipeline with pipeline task and final task referencing artifacts in task params with enable-artifacts flag true", + p: &Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: PipelineSpec{ + Description: "this is an invalid pipeline referencing artifacts with enable-artifacts flag false", + Tasks: []PipelineTask{{ + Name: "pre-task", + TaskRef: &TaskRef{Name: "foo-task"}, + }, { + Name: "consume-artifacts-task", + Params: Params{{Name: "aaa", Value: ParamValue{ + Type: ParamTypeString, + StringVal: "$(tasks.produce-artifacts-task.outputs)", + }}}, + TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, + }}, + }, + }, + wc: func(ctx context.Context) context.Context { + return cfgtesting.SetFeatureFlags(ctx, t, + map[string]string{ + "enable-artifacts": "true", + "enable-api-fields": "alpha"}) + }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1197,6 +1222,59 @@ func TestPipelineSpec_Validate_Failure(t *testing.T) { Message: `must not set the field(s)`, Paths: []string{"finally[0].taskSpec.resources"}, }, + }, { + name: "invalid pipeline with one pipeline task referencing artifacts in task params with enable-artifacts flag false", + ps: &PipelineSpec{ + Description: "this is an invalid pipeline referencing artifacts with enable-artifacts flag false", + Tasks: []PipelineTask{{ + Name: "pre-task", + TaskRef: &TaskRef{Name: "foo-task"}, + }, { + Name: "consume-artifacts-task", + Params: Params{{Name: "aaa", Value: ParamValue{ + Type: ParamTypeString, + StringVal: "$(tasks.produce-artifacts-task.outputs)", + }}}, + TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, + }}, + }, + expectedError: apis.FieldError{ + Message: `feature flag enable-artifacts should be set to true to use artifacts feature.`, + Paths: []string{"tasks[1].params"}, + }, + wc: func(ctx context.Context) context.Context { + return cfgtesting.SetFeatureFlags(ctx, t, + map[string]string{ + "enable-artifacts": "false", + "enable-api-fields": "alpha"}) + }, + }, { + name: "invalid pipeline with one final pipeline task referencing artifacts in params with enable-artifacts flag false", + ps: &PipelineSpec{ + Description: "this is an invalid pipeline referencing artifacts with enable-artifacts flag false", + Tasks: []PipelineTask{{ + Name: "pre-task", + TaskRef: &TaskRef{Name: "foo-task"}, + }}, + Finally: []PipelineTask{{ + Name: "consume-artifacts-task", + Params: Params{{Name: "aaa", Value: ParamValue{ + Type: ParamTypeString, + StringVal: "$(tasks.produce-artifacts-task.outputs)", + }}}, + TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, + }}, + }, + wc: func(ctx context.Context) context.Context { + return cfgtesting.SetFeatureFlags(ctx, t, + map[string]string{ + "enable-artifacts": "false", + "enable-api-fields": "alpha"}) + }, + expectedError: apis.FieldError{ + Message: `feature flag enable-artifacts should be set to true to use artifacts feature.`, + Paths: []string{"finally[1].params"}, + }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {