Skip to content

Commit

Permalink
Revert ResolutionRequestSpec back to a map of parameters
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
abayer committed Aug 16, 2022
1 parent 8a1129d commit 2045ac9
Show file tree
Hide file tree
Showing 13 changed files with 3,275 additions and 386 deletions.
292 changes: 124 additions & 168 deletions docs/pipeline-api.md

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions pkg/apis/pipeline/v1/pipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/apis/pipeline/v1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}}
Expand Down Expand Up @@ -275,11 +275,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) {
Expand Down
30 changes: 17 additions & 13 deletions pkg/apis/pipeline/v1/taskref_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 11 additions & 11 deletions pkg/apis/pipeline/v1/taskref_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.NewStructuredValues("https://github.com/tektoncd/pipeline.git"),
}, {
Name: "branch",
Value: "baz",
Value: *v1.NewStructuredValues("baz"),
}}}},
wc: config.EnableAlphaAPIFields,
}}
Expand Down Expand Up @@ -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"),
Expand All @@ -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.NewStructuredValues("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 {
Expand Down
17 changes: 6 additions & 11 deletions pkg/apis/pipeline/v1beta1/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions pkg/apis/pipeline/v1beta1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}}},
},
Expand Down Expand Up @@ -337,7 +337,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",
Expand Down
26 changes: 8 additions & 18 deletions pkg/apis/pipeline/v1beta1/resolver_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,20 @@ import (

func (rr ResolverRef) convertTo(ctx context.Context, sink *v1.ResolverRef) {
sink.Resolver = v1.ResolverName(rr.Resolver)
sink.Resource = nil
for _, r := range rr.Resource {
new := v1.ResolverParam{}
sink.Params = nil
for _, r := range rr.Params {
new := v1.Param{}
r.convertTo(ctx, &new)
sink.Resource = append(sink.Resource, new)
sink.Params = append(sink.Params, new)
}
}

func (rr *ResolverRef) convertFrom(ctx context.Context, source v1.ResolverRef) {
rr.Resolver = ResolverName(source.Resolver)
rr.Resource = nil
for _, r := range source.Resource {
new := ResolverParam{}
rr.Params = nil
for _, r := range source.Params {
new := Param{}
new.convertFrom(ctx, r)
rr.Resource = append(rr.Resource, new)
rr.Params = append(rr.Params, new)
}
}

func (rp ResolverParam) convertTo(ctx context.Context, sink *v1.ResolverParam) {
sink.Name = rp.Name
sink.Value = rp.Value
}

func (rp *ResolverParam) convertFrom(ctx context.Context, source v1.ResolverParam) {
rp.Name = source.Name
rp.Value = source.Value
}
Loading

0 comments on commit 2045ac9

Please sign in to comment.