Revert ResolutionRequestSpec back to a map of parameters

This is to make sure we're replicating the CRD exactly the same as it is currently in Resolution - I think that in a followup release, we should add ResolutionRequest v1alpha2 with the Params syntax. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
tektoncd · Aug 8, 2022 · ab51f0f · ab51f0f
1 parent 8402818
commit ab51f0f
Show file tree

Hide file tree

Showing 9 changed files with 42 additions and 189 deletions.
diff --git a/pkg/apis/pipeline/v1/pipeline_types.go b/pkg/apis/pipeline/v1/pipeline_types.go
@@ -256,8 +256,8 @@ func (pt PipelineTask) validateTask(ctx context.Context) (errs *apis.FieldError)
 			if pt.TaskRef.Resolver != "" {
 				errs = errs.Also(apis.ErrDisallowedFields("taskref.resolver"))
 			}
-			if len(pt.TaskRef.Resource) > 0 {
-				errs = errs.Also(apis.ErrDisallowedFields("taskref.resource"))
+			if len(pt.TaskRef.Params) > 0 {
+				errs = errs.Also(apis.ErrDisallowedFields("taskref.params"))
 			}
 		}
 	}

diff --git a/pkg/apis/pipeline/v1/pipeline_types_test.go b/pkg/apis/pipeline/v1/pipeline_types_test.go
@@ -208,9 +208,9 @@ func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) {
 		},
 		enableAPIFields: true,
 	}, {
-		name: "pipeline task - use of resource with the feature flag set",
+		name: "pipeline task - use of resolver params with the feature flag set",
 		tasks: PipelineTask{
-			TaskRef: &TaskRef{Name: "boo", ResolverRef: ResolverRef{Resource: []ResolverParam{{}}}},
+			TaskRef: &TaskRef{Name: "boo", ResolverRef: ResolverRef{Params: []Param{{}}}},
 		},
 		enableAPIFields: true,
 	}}
@@ -274,11 +274,11 @@ func TestPipelineTask_ValidateRegularTask_Failure(t *testing.T) {
 		},
 		expectedError: *apis.ErrDisallowedFields("taskref.resolver"),
 	}, {
-		name: "pipeline task - use of resource without the feature flag set",
+		name: "pipeline task - use of resolver params without the feature flag set",
 		task: PipelineTask{
-			TaskRef: &TaskRef{Name: "boo", ResolverRef: ResolverRef{Resource: []ResolverParam{{}}}},
+			TaskRef: &TaskRef{Name: "boo", ResolverRef: ResolverRef{Params: []Param{{}}}},
 		},
-		expectedError: *apis.ErrDisallowedFields("taskref.resource"),
+		expectedError: *apis.ErrDisallowedFields("taskref.params"),
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

diff --git a/pkg/apis/pipeline/v1/taskref_validation.go b/pkg/apis/pipeline/v1/taskref_validation.go
@@ -31,21 +31,25 @@ func (ref *TaskRef) Validate(ctx context.Context) (errs *apis.FieldError) {
 		return
 	}
 
-	switch {
-	case ref.Resolver != "":
-		errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "resolver", config.AlphaAPIFields).ViaField("resolver"))
-		if ref.Name != "" {
-			errs = errs.Also(apis.ErrMultipleOneOf("name", "resolver"))
+	if ref.Resolver != "" || ref.Params != nil {
+		if ref.Resolver != "" {
+			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "resolver", config.AlphaAPIFields).ViaField("resolver"))
+			if ref.Name != "" {
+				errs = errs.Also(apis.ErrMultipleOneOf("name", "resolver"))
+			}
 		}
-	case ref.Resource != nil:
-		errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "resource", config.AlphaAPIFields).ViaField("resource"))
-		if ref.Name != "" {
-			errs = errs.Also(apis.ErrMultipleOneOf("name", "resource"))
+		if ref.Params != nil {
+			errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "params", config.AlphaAPIFields).ViaField("params"))
+			if ref.Name != "" {
+				errs = errs.Also(apis.ErrMultipleOneOf("name", "params"))
+			}
+			if ref.Resolver == "" {
+				errs = errs.Also(apis.ErrMissingField("resolver"))
+			}
+			// TODO(abayer): Uncomment this when taskrun_validation.go is added with the ValidateParameters function.
+			// errs = errs.Also(ValidateParameters(ctx, ref.Params))
 		}
-		if ref.Resolver == "" {
-			errs = errs.Also(apis.ErrMissingField("resolver"))
-		}
-	case ref.Name == "":
+	} else if ref.Name == "" {
 		errs = errs.Also(apis.ErrMissingField("name"))
 	}
 	return

diff --git a/pkg/apis/pipeline/v1/taskref_validation_test.go b/pkg/apis/pipeline/v1/taskref_validation_test.go
@@ -43,12 +43,12 @@ func TestTaskRef_Valid(t *testing.T) {
 		wc:      config.EnableAlphaAPIFields,
 	}, {
 		name: "alpha feature: valid resolver with resource parameters",
-		taskRef: &v1.TaskRef{ResolverRef: v1.ResolverRef{Resolver: "git", Resource: []v1.ResolverParam{{
+		taskRef: &v1.TaskRef{ResolverRef: v1.ResolverRef{Resolver: "git", Params: []v1.Param{{
 			Name:  "repo",
-			Value: "https://github.com/tektoncd/pipeline.git",
+			Value: *v1.NewArrayOrString("https://github.com/tektoncd/pipeline.git"),
 		}, {
 			Name:  "branch",
-			Value: "baz",
+			Value: *v1.NewArrayOrString("baz"),
 		}}}},
 		wc: config.EnableAlphaAPIFields,
 	}}
@@ -87,15 +87,15 @@ func TestTaskRef_Invalid(t *testing.T) {
 		name: "taskref resource disallowed without alpha feature gate",
 		taskRef: &v1.TaskRef{
 			ResolverRef: v1.ResolverRef{
-				Resource: []v1.ResolverParam{},
+				Params: []v1.Param{},
 			},
 		},
-		wantErr: apis.ErrMissingField("resolver").Also(apis.ErrGeneric("resource requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"")),
+		wantErr: apis.ErrMissingField("resolver").Also(apis.ErrGeneric("params requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"")),
 	}, {
-		name: "taskref resource disallowed without resolver",
+		name: "taskref resolver params disallowed without resolver",
 		taskRef: &v1.TaskRef{
 			ResolverRef: v1.ResolverRef{
-				Resource: []v1.ResolverParam{},
+				Params: []v1.Param{},
 			},
 		},
 		wantErr: apis.ErrMissingField("resolver"),
@@ -111,17 +111,17 @@ func TestTaskRef_Invalid(t *testing.T) {
 		wantErr: apis.ErrMultipleOneOf("name", "resolver"),
 		wc:      config.EnableAlphaAPIFields,
 	}, {
-		name: "taskref resource disallowed in conjunction with taskref name",
+		name: "taskref resolver params disallowed in conjunction with taskref name",
 		taskRef: &v1.TaskRef{
 			Name: "bar",
 			ResolverRef: v1.ResolverRef{
-				Resource: []v1.ResolverParam{{
+				Params: []v1.Param{{
 					Name:  "foo",
-					Value: "bar",
+					Value: *v1.NewArrayOrString("bar"),
 				}},
 			},
 		},
-		wantErr: apis.ErrMultipleOneOf("name", "resource").Also(apis.ErrMissingField("resolver")),
+		wantErr: apis.ErrMultipleOneOf("name", "params").Also(apis.ErrMissingField("resolver")),
 		wc:      config.EnableAlphaAPIFields,
 	}}
 	for _, ts := range tests {

diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go
@@ -253,7 +253,7 @@ func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) {
 		},
 		enableAPIFields: true,
 	}, {
-		name: "pipeline task - use of resource with the feature flag set",
+		name: "pipeline task - use of params with the feature flag set",
 		tasks: PipelineTask{
 			TaskRef: &TaskRef{Name: "boo", ResolverRef: ResolverRef{Params: []Param{}}},
 		},
@@ -336,7 +336,7 @@ func TestPipelineTask_ValidateRegularTask_Failure(t *testing.T) {
 		},
 		expectedError: *apis.ErrDisallowedFields("taskref.resolver"),
 	}, {
-		name: "pipeline task - use of resource without the feature flag set",
+		name: "pipeline task - use of resolver params without the feature flag set",
 		task: PipelineTask{
 			TaskRef: &TaskRef{Name: "boo", ResolverRef: ResolverRef{Params: []Param{{
 				Name: "bar",

diff --git a/pkg/apis/resolution/v1alpha1/resolution_request_types.go b/pkg/apis/resolution/v1alpha1/resolution_request_types.go
@@ -17,7 +17,6 @@ limitations under the License.
 package v1alpha1
 
 import (
-	pipelinev1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	duckv1 "knative.dev/pkg/apis/duck/v1"
 )
@@ -62,8 +61,7 @@ type ResolutionRequestSpec struct {
 	// resource being requested. For example: repo URL, commit SHA,
 	// path to file, the kind of authentication to leverage, etc.
 	// +optional
-	// +listType=atomic
-	Params []pipelinev1beta1.Param `json:"params,omitempty"`
+	Params map[string]string `json:"params,omitempty"`
 }
 
 // ResolutionRequestStatus are all the fields in a ResolutionRequest's

diff --git a/pkg/apis/resolution/v1alpha1/resolution_request_validation.go b/pkg/apis/resolution/v1alpha1/resolution_request_validation.go
@@ -19,7 +19,6 @@ package v1alpha1
 import (
 	"context"
 
-	pipelinev1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"github.com/tektoncd/resolution/pkg/common"
 	"knative.dev/pkg/apis"
 )
@@ -33,7 +32,7 @@ func (rr *ResolutionRequest) Validate(ctx context.Context) (errs *apis.FieldErro
 
 // Validate checks the the spec field of a ResolutionRequest is valid.
 func (rs *ResolutionRequestSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
-	return errs.Also(pipelinev1beta1.ValidateParameters(ctx, rs.Params)).ViaField("params")
+	return nil
 }
 
 func validateTypeLabel(rr *ResolutionRequest) *apis.FieldError {

diff --git a/pkg/apis/resolution/v1alpha1/resolution_request_validation_test.go b/pkg/apis/resolution/v1alpha1/resolution_request_validation_test.go
diff --git a/pkg/apis/resolution/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/resolution/v1alpha1/zz_generated.deepcopy.go