diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index eead110f31e..113bfc69e98 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -1109,7 +1109,7 @@ func schema_pkg_apis_pipeline_v1beta1_PipelineRef(ref common.ReferenceCallback) return common.OpenAPIDefinition{ Schema: spec.Schema{ SchemaProps: spec.SchemaProps{ - Description: "PipelineRef can be used to refer to a specific instance of a Pipeline. Copied from CrossVersionObjectReference: https://github.com/kubernetes/kubernetes/blob/169df7434155cbbc22f1532cba8e0a9588e29ad8/pkg/apis/autoscaling/types.go#L64", + Description: "PipelineRef can be used to refer to a specific instance of a Pipeline.", Type: []string{"object"}, Properties: map[string]spec.Schema{ "name": { @@ -3763,7 +3763,7 @@ func schema_pkg_apis_pipeline_v1beta1_TaskRef(ref common.ReferenceCallback) comm return common.OpenAPIDefinition{ Schema: spec.Schema{ SchemaProps: spec.SchemaProps{ - Description: "TaskRef can be used to refer to a specific instance of a task. Copied from CrossVersionObjectReference: https://github.com/kubernetes/kubernetes/blob/169df7434155cbbc22f1532cba8e0a9588e29ad8/pkg/apis/autoscaling/types.go#L64", + Description: "TaskRef can be used to refer to a specific instance of a task.", Type: []string{"object"}, Properties: map[string]spec.Schema{ "name": { diff --git a/pkg/apis/pipeline/v1beta1/pipelineref_types.go b/pkg/apis/pipeline/v1beta1/pipelineref_types.go new file mode 100644 index 00000000000..17bd57428cc --- /dev/null +++ b/pkg/apis/pipeline/v1beta1/pipelineref_types.go @@ -0,0 +1,35 @@ +/* +Copyright 2022 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta1 + +// PipelineRef can be used to refer to a specific instance of a Pipeline. +type PipelineRef struct { + // Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names + Name string `json:"name,omitempty"` + // API version of the referent + // +optional + APIVersion string `json:"apiVersion,omitempty"` + // Bundle url reference to a Tekton Bundle. + // +optional + Bundle string `json:"bundle,omitempty"` + + // ResolverRef allows referencing a Pipeline in a remote location + // like a git repo. This field is only supported when the alpha + // feature gate is enabled. + // +optional + ResolverRef `json:",omitempty"` +} diff --git a/pkg/apis/pipeline/v1beta1/pipelineref_validation.go b/pkg/apis/pipeline/v1beta1/pipelineref_validation.go index 1a61b7b219b..45300a54801 100644 --- a/pkg/apis/pipeline/v1beta1/pipelineref_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipelineref_validation.go @@ -18,71 +18,58 @@ package v1beta1 import ( "context" + "fmt" "github.com/google/go-containerregistry/pkg/name" "github.com/tektoncd/pipeline/pkg/apis/config" + "github.com/tektoncd/pipeline/pkg/apis/version" "knative.dev/pkg/apis" ) // Validate ensures that a supplied PipelineRef field is populated // correctly. No errors are returned for a nil PipelineRef. func (ref *PipelineRef) Validate(ctx context.Context) (errs *apis.FieldError) { - cfg := config.FromContextOrDefaults(ctx) if ref == nil { return } - if cfg.FeatureFlags.EnableAPIFields == config.AlphaAPIFields { - errs = errs.Also(ref.validateAlphaRef(ctx)) - } else { - errs = errs.Also(ref.validateInTreeRef(ctx)) - } - return -} - -// validateInTreeRef returns errors if the given pipelineRef is not -// valid for Pipelines' built-in resolution machinery. -func (ref *PipelineRef) validateInTreeRef(ctx context.Context) (errs *apis.FieldError) { - cfg := config.FromContextOrDefaults(ctx) - if ref.Resolver != "" { - errs = errs.Also(apis.ErrDisallowedFields("resolver")) - } - if ref.Resource != nil { - errs = errs.Also(apis.ErrDisallowedFields("resource")) - } - if ref.Name == "" { - errs = errs.Also(apis.ErrMissingField("name")) - } - if cfg.FeatureFlags.EnableTektonOCIBundles { - if ref.Bundle != "" && ref.Name == "" { - errs = errs.Also(apis.ErrMissingField("name")) - } - if ref.Bundle != "" { - if _, err := name.ParseReference(ref.Bundle); err != nil { - errs = errs.Also(apis.ErrInvalidValue("invalid bundle reference", "bundle", err.Error())) - } - } - } else if ref.Bundle != "" { - errs = errs.Also(apis.ErrDisallowedFields("bundle")) - } - return -} -// validateAlphaRef ensures that the user has passed either a -// valid remote resource reference or a valid in-tree resource reference, -// but not both. -func (ref *PipelineRef) validateAlphaRef(ctx context.Context) (errs *apis.FieldError) { switch { - case ref.Resolver == "" && ref.Resource != nil: - errs = errs.Also(apis.ErrMissingField("resolver")) - case ref.Resolver == "": - errs = errs.Also(ref.validateInTreeRef(ctx)) - default: + 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.Bundle != "" { errs = errs.Also(apis.ErrMultipleOneOf("bundle", "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.Bundle != "" { + errs = errs.Also(apis.ErrMultipleOneOf("bundle", "resource")) + } + if ref.Resolver == "" { + errs = errs.Also(apis.ErrMissingField("resolver")) + } + case ref.Name == "": + errs = errs.Also(apis.ErrMissingField("name")) + case ref.Bundle != "": + errs = errs.Also(validateBundleFeatureFlag(ctx, "bundle", true).ViaField("bundle")) + if _, err := name.ParseReference(ref.Bundle); err != nil { + errs = errs.Also(apis.ErrInvalidValue("invalid bundle reference", "bundle", err.Error())) + } } return } + +func validateBundleFeatureFlag(ctx context.Context, featureName string, wantValue bool) *apis.FieldError { + flagValue := config.FromContextOrDefaults(ctx).FeatureFlags.EnableTektonOCIBundles + if flagValue != wantValue { + var errs *apis.FieldError + message := fmt.Sprintf(`%s requires "enable-tekton-oci-bundles" feature gate to be %t but it is %t`, featureName, wantValue, flagValue) + return errs.Also(apis.ErrGeneric(message)) + } + return nil +} diff --git a/pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go new file mode 100644 index 00000000000..0344ad5213a --- /dev/null +++ b/pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go @@ -0,0 +1,200 @@ +/* +Copyright 2020 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta1_test + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/config" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "github.com/tektoncd/pipeline/test/diff" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/apis" + logtesting "knative.dev/pkg/logging/testing" +) + +func TestPipelineRef_Invalid(t *testing.T) { + tests := []struct { + name string + ref *v1beta1.PipelineRef + wantErr *apis.FieldError + withContext func(context.Context) context.Context + }{{ + name: "use of bundle without the feature flag set", + ref: &v1beta1.PipelineRef{ + Name: "my-pipeline", + Bundle: "docker.io/foo", + }, + wantErr: apis.ErrGeneric("bundle requires \"enable-tekton-oci-bundles\" feature gate to be true but it is false"), + }, { + name: "bundle missing name", + ref: &v1beta1.PipelineRef{ + Bundle: "docker.io/foo", + }, + wantErr: apis.ErrMissingField("name"), + withContext: enableTektonOCIBundles(t), + }, { + name: "invalid bundle reference", + ref: &v1beta1.PipelineRef{ + Name: "my-pipeline", + Bundle: "not a valid reference", + }, + wantErr: apis.ErrInvalidValue("invalid bundle reference", "bundle", "could not parse reference: not a valid reference"), + withContext: enableTektonOCIBundles(t), + }, { + name: "pipelineRef without Pipeline Name", + ref: &v1beta1.PipelineRef{}, + wantErr: apis.ErrMissingField("name"), + }, { + name: "pipelineref resolver disallowed without alpha feature gate", + ref: &v1beta1.PipelineRef{ + ResolverRef: v1beta1.ResolverRef{ + Resolver: "foo", + }, + }, + wantErr: apis.ErrGeneric("resolver requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\""), + }, { + name: "pipelineref resource disallowed without alpha feature gate", + ref: &v1beta1.PipelineRef{ + ResolverRef: v1beta1.ResolverRef{ + Resource: []v1beta1.ResolverParam{}, + }, + }, + wantErr: apis.ErrMissingField("resolver").Also(apis.ErrGeneric("resource requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"")), + }, { + name: "pipelineref resource disallowed without resolver", + ref: &v1beta1.PipelineRef{ + ResolverRef: v1beta1.ResolverRef{ + Resource: []v1beta1.ResolverParam{}, + }, + }, + wantErr: apis.ErrMissingField("resolver"), + withContext: enableAlphaAPIFields, + }, { + name: "pipelineref resolver disallowed in conjunction with pipelineref name", + ref: &v1beta1.PipelineRef{ + Name: "foo", + ResolverRef: v1beta1.ResolverRef{ + Resolver: "bar", + }, + }, + wantErr: apis.ErrMultipleOneOf("name", "resolver"), + withContext: enableAlphaAPIFields, + }, { + name: "pipelineref resolver disallowed in conjunction with pipelineref bundle", + ref: &v1beta1.PipelineRef{ + Bundle: "foo", + ResolverRef: v1beta1.ResolverRef{ + Resolver: "baz", + }, + }, + wantErr: apis.ErrMultipleOneOf("bundle", "resolver"), + withContext: enableAlphaAPIFields, + }, { + name: "pipelineref resource disallowed in conjunction with taskref name", + ref: &v1beta1.PipelineRef{ + Name: "bar", + ResolverRef: v1beta1.ResolverRef{ + Resource: []v1beta1.ResolverParam{{ + Name: "foo", + Value: "bar", + }}, + }, + }, + wantErr: apis.ErrMultipleOneOf("name", "resource").Also(apis.ErrMissingField("resolver")), + withContext: enableAlphaAPIFields, + }, { + name: "pipelineref resource disallowed in conjunction with taskref bundle", + ref: &v1beta1.PipelineRef{ + Bundle: "bar", + ResolverRef: v1beta1.ResolverRef{ + Resource: []v1beta1.ResolverParam{{ + Name: "foo", + Value: "bar", + }}, + }, + }, + wantErr: apis.ErrMultipleOneOf("bundle", "resource").Also(apis.ErrMissingField("resolver")), + withContext: enableAlphaAPIFields, + }} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + if tc.withContext != nil { + ctx = tc.withContext(ctx) + } + err := tc.ref.Validate(ctx) + if d := cmp.Diff(tc.wantErr.Error(), err.Error()); d != "" { + t.Error(diff.PrintWantGot(d)) + } + }) + } +} + +func TestPipelineRef_Valid(t *testing.T) { + tests := []struct { + name string + ref *v1beta1.PipelineRef + wc func(context.Context) context.Context + }{{ + name: "no pipelineRef", + ref: nil, + }, { + name: "alpha feature: valid resolver", + ref: &v1beta1.PipelineRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}}, + wc: enableAlphaAPIFields, + }, { + name: "alpha feature: valid resolver with resource parameters", + ref: &v1beta1.PipelineRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git", Resource: []v1beta1.ResolverParam{{ + Name: "repo", + Value: "https://github.com/tektoncd/pipeline.git", + }, { + Name: "branch", + Value: "baz", + }}}}, + wc: enableAlphaAPIFields, + }} + + for _, ts := range tests { + t.Run(ts.name, func(t *testing.T) { + ctx := context.Background() + if ts.wc != nil { + ctx = ts.wc(ctx) + } + if err := ts.ref.Validate(ctx); err != nil { + t.Error(err) + } + }) + } +} + +func enableTektonOCIBundles(t *testing.T) func(context.Context) context.Context { + return func(ctx context.Context) context.Context { + s := config.NewStore(logtesting.TestLogger(t)) + s.OnConfigChanged(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName()}, + Data: map[string]string{ + "enable-tekton-oci-bundles": "true", + }, + }) + return s.ToContext(ctx) + } +} diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go index a9e75f33b6d..e8f5b047fff 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go @@ -265,25 +265,6 @@ const ( PipelineRunSpecStatusPending = "PipelineRunPending" ) -// PipelineRef can be used to refer to a specific instance of a Pipeline. -// Copied from CrossVersionObjectReference: https://github.com/kubernetes/kubernetes/blob/169df7434155cbbc22f1532cba8e0a9588e29ad8/pkg/apis/autoscaling/types.go#L64 -type PipelineRef struct { - // Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names - Name string `json:"name,omitempty"` - // API version of the referent - // +optional - APIVersion string `json:"apiVersion,omitempty"` - // Bundle url reference to a Tekton Bundle. - // +optional - Bundle string `json:"bundle,omitempty"` - - // ResolverRef allows referencing a Pipeline in a remote location - // like a git repo. This field is only supported when the alpha - // feature gate is enabled. - // +optional - ResolverRef `json:",omitempty"` -} - // PipelineRunStatus defines the observed state of PipelineRun type PipelineRunStatus struct { duckv1beta1.Status `json:",inline"` diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go index ce82e89697a..dbf45fc5d85 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go @@ -29,7 +29,6 @@ import ( corev1resources "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" - logtesting "knative.dev/pkg/logging/testing" ) func TestPipelineRun_Invalid(t *testing.T) { @@ -93,49 +92,6 @@ func TestPipelineRun_Invalid(t *testing.T) { }, }, want: apis.ErrInvalidValue("PipelineRunCancell should be Cancelled, CancelledRunFinally, StoppedRunFinally or PipelineRunPending", "spec.status"), - }, { - name: "use of bundle without the feature flag set", - pr: v1beta1.PipelineRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pipelinelinename", - }, - Spec: v1beta1.PipelineRunSpec{ - PipelineRef: &v1beta1.PipelineRef{ - Name: "my-pipeline", - Bundle: "docker.io/foo", - }, - }, - }, - want: apis.ErrDisallowedFields("spec.pipelineRef.bundle"), - }, { - name: "bundle missing name", - pr: v1beta1.PipelineRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pipelinelinename", - }, - Spec: v1beta1.PipelineRunSpec{ - PipelineRef: &v1beta1.PipelineRef{ - Bundle: "docker.io/foo", - }, - }, - }, - want: apis.ErrMissingField("spec.pipelineRef.name"), - wc: enableTektonOCIBundles(t), - }, { - name: "invalid bundle reference", - pr: v1beta1.PipelineRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pipelinelinename", - }, - Spec: v1beta1.PipelineRunSpec{ - PipelineRef: &v1beta1.PipelineRef{ - Name: "my-pipeline", - Bundle: "not a valid reference", - }, - }, - }, - want: apis.ErrInvalidValue("invalid bundle reference", "spec.pipelineRef.bundle", "could not parse reference: not a valid reference"), - wc: enableTektonOCIBundles(t), }, { name: "pipelinerun pending while running", pr: v1beta1.PipelineRun{ @@ -319,34 +275,6 @@ func TestPipelineRun_Validate(t *testing.T) { }, }, }, - }, { - name: "alpha feature: valid resolver", - pr: v1beta1.PipelineRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pr", - }, - Spec: v1beta1.PipelineRunSpec{ - PipelineRef: &v1beta1.PipelineRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}}, - }, - }, - wc: enableAlphaAPIFields, - }, { - name: "alpha feature: valid resolver with resource parameters", - pr: v1beta1.PipelineRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pr", - }, - Spec: v1beta1.PipelineRunSpec{ - PipelineRef: &v1beta1.PipelineRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git", Resource: []v1beta1.ResolverParam{{ - Name: "repo", - Value: "https://github.com/tektoncd/pipeline.git", - }, { - Name: "branch", - Value: "baz", - }}}}, - }, - }, - wc: enableAlphaAPIFields, }, { name: "alpha feature: sidecar and step overrides", pr: v1beta1.PipelineRun{ @@ -397,12 +325,6 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) { wantErr *apis.FieldError withContext func(context.Context) context.Context }{{ - name: "pipelineRef without Pipeline Name", - spec: v1beta1.PipelineRunSpec{ - PipelineRef: &v1beta1.PipelineRef{}, - }, - wantErr: apis.ErrMissingField("pipelineRef.name"), - }, { name: "pipelineRef and pipelineSpec together", spec: v1beta1.PipelineRunSpec{ PipelineRef: &v1beta1.PipelineRef{ @@ -455,63 +377,6 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) { "workspaces[0].volumeclaimtemplate", }, }, - }, { - name: "pipelineref resolver disallowed without alpha feature gate", - spec: v1beta1.PipelineRunSpec{ - PipelineRef: &v1beta1.PipelineRef{ - Name: "foo", - ResolverRef: v1beta1.ResolverRef{ - Resolver: "foo", - }, - }, - }, - wantErr: apis.ErrDisallowedFields("resolver").ViaField("pipelineRef"), - }, { - name: "pipelineref resource disallowed without alpha feature gate", - spec: v1beta1.PipelineRunSpec{ - PipelineRef: &v1beta1.PipelineRef{ - Name: "foo", - ResolverRef: v1beta1.ResolverRef{ - Resource: []v1beta1.ResolverParam{}, - }, - }, - }, - wantErr: apis.ErrDisallowedFields("resource").ViaField("pipelineRef"), - }, { - name: "pipelineref resource disallowed without resolver", - spec: v1beta1.PipelineRunSpec{ - PipelineRef: &v1beta1.PipelineRef{ - ResolverRef: v1beta1.ResolverRef{ - Resource: []v1beta1.ResolverParam{}, - }, - }, - }, - wantErr: apis.ErrMissingField("resolver").ViaField("pipelineRef"), - withContext: enableAlphaAPIFields, - }, { - name: "pipelineref resolver disallowed in conjunction with pipelineref name", - spec: v1beta1.PipelineRunSpec{ - PipelineRef: &v1beta1.PipelineRef{ - Name: "foo", - ResolverRef: v1beta1.ResolverRef{ - Resolver: "bar", - }, - }, - }, - wantErr: apis.ErrMultipleOneOf("name", "resolver").ViaField("pipelineRef"), - withContext: enableAlphaAPIFields, - }, { - name: "pipelineref resolver disallowed in conjunction with pipelineref bundle", - spec: v1beta1.PipelineRunSpec{ - PipelineRef: &v1beta1.PipelineRef{ - Bundle: "foo", - ResolverRef: v1beta1.ResolverRef{ - Resolver: "baz", - }, - }, - }, - wantErr: apis.ErrMultipleOneOf("bundle", "resolver").ViaField("pipelineRef"), - withContext: enableAlphaAPIFields, }, { name: "duplicate stepOverride names", spec: v1beta1.PipelineRunSpec{ @@ -975,19 +840,6 @@ func TestPipelineRunWithTimeout_Validate(t *testing.T) { } } -func enableTektonOCIBundles(t *testing.T) func(context.Context) context.Context { - return func(ctx context.Context) context.Context { - s := config.NewStore(logtesting.TestLogger(t)) - s.OnConfigChanged(&corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName()}, - Data: map[string]string{ - "enable-tekton-oci-bundles": "true", - }, - }) - return s.ToContext(ctx) - } -} - func enableAlphaAPIFields(ctx context.Context) context.Context { featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{ "enable-api-fields": "alpha", diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 0fbcefe17f8..033808a2d7c 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -707,7 +707,7 @@ } }, "v1beta1.PipelineRef": { - "description": "PipelineRef can be used to refer to a specific instance of a Pipeline. Copied from CrossVersionObjectReference: https://github.com/kubernetes/kubernetes/blob/169df7434155cbbc22f1532cba8e0a9588e29ad8/pkg/apis/autoscaling/types.go#L64", + "description": "PipelineRef can be used to refer to a specific instance of a Pipeline.", "type": "object", "properties": { "apiVersion": { @@ -2134,7 +2134,7 @@ } }, "v1beta1.TaskRef": { - "description": "TaskRef can be used to refer to a specific instance of a task. Copied from CrossVersionObjectReference: https://github.com/kubernetes/kubernetes/blob/169df7434155cbbc22f1532cba8e0a9588e29ad8/pkg/apis/autoscaling/types.go#L64", + "description": "TaskRef can be used to refer to a specific instance of a task.", "type": "object", "properties": { "apiVersion": { diff --git a/pkg/apis/pipeline/v1beta1/task_types.go b/pkg/apis/pipeline/v1beta1/task_types.go index c0d168b0a77..957b0aef697 100644 --- a/pkg/apis/pipeline/v1beta1/task_types.go +++ b/pkg/apis/pipeline/v1beta1/task_types.go @@ -134,36 +134,3 @@ type TaskList struct { metav1.ListMeta `json:"metadata,omitempty"` Items []Task `json:"items"` } - -// TaskRef can be used to refer to a specific instance of a task. -// Copied from CrossVersionObjectReference: https://github.com/kubernetes/kubernetes/blob/169df7434155cbbc22f1532cba8e0a9588e29ad8/pkg/apis/autoscaling/types.go#L64 -type TaskRef struct { - // Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names - Name string `json:"name,omitempty"` - // TaskKind indicates the kind of the task, namespaced or cluster scoped. - Kind TaskKind `json:"kind,omitempty"` - // API version of the referent - // +optional - APIVersion string `json:"apiVersion,omitempty"` - // Bundle url reference to a Tekton Bundle. - // +optional - Bundle string `json:"bundle,omitempty"` - - // ResolverRef allows referencing a Task in a remote location - // like a git repo. This field is only supported when the alpha - // feature gate is enabled. - // +optional - ResolverRef `json:",omitempty"` -} - -// Check that Pipeline may be validated and defaulted. - -// TaskKind defines the type of Task used by the pipeline. -type TaskKind string - -const ( - // NamespacedTaskKind indicates that the task type has a namespaced scope. - NamespacedTaskKind TaskKind = "Task" - // ClusterTaskKind indicates that task type has a cluster scope. - ClusterTaskKind TaskKind = "ClusterTask" -) diff --git a/pkg/apis/pipeline/v1beta1/taskref_types.go b/pkg/apis/pipeline/v1beta1/taskref_types.go new file mode 100644 index 00000000000..07aeb436d79 --- /dev/null +++ b/pkg/apis/pipeline/v1beta1/taskref_types.go @@ -0,0 +1,49 @@ +/* +Copyright 2022 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta1 + +// TaskRef can be used to refer to a specific instance of a task. +type TaskRef struct { + // Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names + Name string `json:"name,omitempty"` + // TaskKind indicates the kind of the task, namespaced or cluster scoped. + Kind TaskKind `json:"kind,omitempty"` + // API version of the referent + // +optional + APIVersion string `json:"apiVersion,omitempty"` + // Bundle url reference to a Tekton Bundle. + // +optional + Bundle string `json:"bundle,omitempty"` + + // ResolverRef allows referencing a Task in a remote location + // like a git repo. This field is only supported when the alpha + // feature gate is enabled. + // +optional + ResolverRef `json:",omitempty"` +} + +// Check that Pipeline may be validated and defaulted. + +// TaskKind defines the type of Task used by the pipeline. +type TaskKind string + +const ( + // NamespacedTaskKind indicates that the task type has a namespaced scope. + NamespacedTaskKind TaskKind = "Task" + // ClusterTaskKind indicates that task type has a cluster scope. + ClusterTaskKind TaskKind = "ClusterTask" +) diff --git a/pkg/apis/pipeline/v1beta1/taskref_validation.go b/pkg/apis/pipeline/v1beta1/taskref_validation.go index dc5e97611f0..652eed6cff8 100644 --- a/pkg/apis/pipeline/v1beta1/taskref_validation.go +++ b/pkg/apis/pipeline/v1beta1/taskref_validation.go @@ -21,81 +21,43 @@ import ( "github.com/google/go-containerregistry/pkg/name" "github.com/tektoncd/pipeline/pkg/apis/config" + "github.com/tektoncd/pipeline/pkg/apis/version" "knative.dev/pkg/apis" ) // Validate ensures that a supplied TaskRef field is populated // correctly. No errors are returned for a nil TaskRef. func (ref *TaskRef) Validate(ctx context.Context) (errs *apis.FieldError) { - cfg := config.FromContextOrDefaults(ctx) if ref == nil { return } - if cfg.FeatureFlags.EnableAPIFields == config.AlphaAPIFields { - errs = errs.Also(ref.validateAlphaRef(ctx)) - } else { - errs = errs.Also(ref.validateInTreeRef(ctx)) - } - return -} -// validateInTreeRef returns errors if the given taskRef is not valid for -// Pipelines' built-in resolution machinery. -func (ref *TaskRef) validateInTreeRef(ctx context.Context) (errs *apis.FieldError) { - cfg := config.FromContextOrDefaults(ctx) - if ref.Resolver != "" { - errs = errs.Also(apis.ErrDisallowedFields("resolver")) - } - if ref.Resource != nil { - errs = errs.Also(apis.ErrDisallowedFields("resource")) - } - if ref.Name == "" { - errs = errs.Also(apis.ErrMissingField("name")) - } - if cfg.FeatureFlags.EnableTektonOCIBundles { - if ref.Bundle != "" && ref.Name == "" { - errs = errs.Also(apis.ErrMissingField("name")) + 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.Bundle != "" { - if _, err := name.ParseReference(ref.Bundle); err != nil { - errs = errs.Also(apis.ErrInvalidValue("invalid bundle reference", "bundle", err.Error())) - } - } - } else if ref.Bundle != "" { - errs = errs.Also(apis.ErrDisallowedFields("bundle")) - } - return -} - -// validateAlphaRef ensures that the user has passed either a -// valid remote resource reference or a valid in-tree resource reference, -// but not both. -func (ref *TaskRef) validateAlphaRef(ctx context.Context) (errs *apis.FieldError) { - hasResolver := ref.Resolver != "" - hasResource := ref.Resource != nil - hasName := ref.Name != "" - hasBundle := ref.Bundle != "" - if hasName { - if hasResolver { - errs = errs.Also(apis.ErrMultipleOneOf("name", "resolver")) + errs = errs.Also(apis.ErrMultipleOneOf("bundle", "resolver")) } - if hasResource { + 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 hasBundle { - if hasResolver { - errs = errs.Also(apis.ErrMultipleOneOf("bundle", "resolver")) - } - if hasResource { + if ref.Bundle != "" { errs = errs.Also(apis.ErrMultipleOneOf("bundle", "resource")) } - } - if !hasResolver { - if hasResource { + if ref.Resolver == "" { errs = errs.Also(apis.ErrMissingField("resolver")) - } else { - errs = errs.Also(ref.validateInTreeRef(ctx)) + } + case ref.Name == "": + errs = errs.Also(apis.ErrMissingField("name")) + case ref.Bundle != "": + errs = errs.Also(validateBundleFeatureFlag(ctx, "bundle", true).ViaField("bundle")) + if _, err := name.ParseReference(ref.Bundle); err != nil { + errs = errs.Also(apis.ErrInvalidValue("invalid bundle reference", "bundle", err.Error())) } } return diff --git a/pkg/apis/pipeline/v1beta1/taskref_validation_test.go b/pkg/apis/pipeline/v1beta1/taskref_validation_test.go new file mode 100644 index 00000000000..9e0be193f2d --- /dev/null +++ b/pkg/apis/pipeline/v1beta1/taskref_validation_test.go @@ -0,0 +1,189 @@ +/* +Copyright 2019 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta1_test + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "github.com/tektoncd/pipeline/test/diff" + "knative.dev/pkg/apis" +) + +func TestTaskRef_Valid(t *testing.T) { + tests := []struct { + name string + taskRef *v1beta1.TaskRef + wc func(context.Context) context.Context + }{{ + name: "nil taskref", + }, { + name: "simple taskref", + taskRef: &v1beta1.TaskRef{Name: "taskrefname"}, + }, { + name: "alpha feature: valid resolver", + taskRef: &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}}, + wc: enableAlphaAPIFields, + }, { + name: "alpha feature: valid resolver with resource parameters", + taskRef: &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git", Resource: []v1beta1.ResolverParam{{ + Name: "repo", + Value: "https://github.com/tektoncd/pipeline.git", + }, { + Name: "branch", + Value: "baz", + }}}}, + wc: enableAlphaAPIFields, + }, { + name: "valid bundle", + taskRef: &v1beta1.TaskRef{ + Name: "bundled-task", + Bundle: "gcr.io/my-bundle"}, + wc: enableAlphaAPIFields, + }} + for _, ts := range tests { + t.Run(ts.name, func(t *testing.T) { + ctx := context.Background() + if ts.wc != nil { + ctx = ts.wc(ctx) + } + if err := ts.taskRef.Validate(ctx); err != nil { + t.Errorf("TaskRef.Validate() error = %v", err) + } + }) + } +} + +func TestTaskRef_Invalid(t *testing.T) { + tests := []struct { + name string + taskRef *v1beta1.TaskRef + wantErr *apis.FieldError + wc func(context.Context) context.Context + }{{ + name: "missing taskref name", + taskRef: &v1beta1.TaskRef{}, + wantErr: apis.ErrMissingField("name"), + }, { + name: "use of bundle without the feature flag set", + taskRef: &v1beta1.TaskRef{ + Name: "my-task", + Bundle: "docker.io/foo", + }, + wantErr: apis.ErrGeneric("bundle requires \"enable-tekton-oci-bundles\" feature gate to be true but it is false"), + }, { + name: "bundle missing name", + taskRef: &v1beta1.TaskRef{ + Bundle: "docker.io/foo", + }, + wantErr: apis.ErrMissingField("name"), + wc: enableTektonOCIBundles(t), + }, { + name: "invalid bundle reference", + taskRef: &v1beta1.TaskRef{ + Name: "my-task", + Bundle: "invalid reference", + }, + wantErr: apis.ErrInvalidValue("invalid bundle reference", "bundle", "could not parse reference: invalid reference"), + wc: enableTektonOCIBundles(t), + }, { + name: "taskref resolver disallowed without alpha feature gate", + taskRef: &v1beta1.TaskRef{ + ResolverRef: v1beta1.ResolverRef{ + Resolver: "git", + }, + }, + wantErr: apis.ErrGeneric("resolver requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\""), + }, { + name: "taskref resource disallowed without alpha feature gate", + taskRef: &v1beta1.TaskRef{ + ResolverRef: v1beta1.ResolverRef{ + Resource: []v1beta1.ResolverParam{}, + }, + }, + wantErr: apis.ErrMissingField("resolver").Also(apis.ErrGeneric("resource requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"")), + }, { + name: "taskref resource disallowed without resolver", + taskRef: &v1beta1.TaskRef{ + ResolverRef: v1beta1.ResolverRef{ + Resource: []v1beta1.ResolverParam{}, + }, + }, + wantErr: apis.ErrMissingField("resolver"), + wc: enableAlphaAPIFields, + }, { + name: "taskref resolver disallowed in conjunction with taskref name", + taskRef: &v1beta1.TaskRef{ + Name: "foo", + ResolverRef: v1beta1.ResolverRef{ + Resolver: "git", + }, + }, + wantErr: apis.ErrMultipleOneOf("name", "resolver"), + wc: enableAlphaAPIFields, + }, { + name: "taskref resolver disallowed in conjunction with taskref bundle", + taskRef: &v1beta1.TaskRef{ + Bundle: "bar", + ResolverRef: v1beta1.ResolverRef{ + Resolver: "git", + }, + }, + wantErr: apis.ErrMultipleOneOf("bundle", "resolver"), + wc: enableAlphaAPIFields, + }, { + name: "taskref resource disallowed in conjunction with taskref name", + taskRef: &v1beta1.TaskRef{ + Name: "bar", + ResolverRef: v1beta1.ResolverRef{ + Resource: []v1beta1.ResolverParam{{ + Name: "foo", + Value: "bar", + }}, + }, + }, + wantErr: apis.ErrMultipleOneOf("name", "resource").Also(apis.ErrMissingField("resolver")), + wc: enableAlphaAPIFields, + }, { + name: "taskref resource disallowed in conjunction with taskref bundle", + taskRef: &v1beta1.TaskRef{ + Bundle: "bar", + ResolverRef: v1beta1.ResolverRef{ + Resource: []v1beta1.ResolverParam{{ + Name: "foo", + Value: "bar", + }}, + }, + }, + wantErr: apis.ErrMultipleOneOf("bundle", "resource").Also(apis.ErrMissingField("resolver")), + wc: enableAlphaAPIFields, + }} + for _, ts := range tests { + t.Run(ts.name, func(t *testing.T) { + ctx := context.Background() + if ts.wc != nil { + ctx = ts.wc(ctx) + } + err := ts.taskRef.Validate(ctx) + if d := cmp.Diff(ts.wantErr.Error(), err.Error()); d != "" { + t.Error(diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go index cbd30e966bf..b2e7d28ec30 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go @@ -58,49 +58,11 @@ func TestTaskRun_Validate(t *testing.T) { taskRun *v1beta1.TaskRun wc func(context.Context) context.Context }{{ - name: "simple taskref", - taskRun: &v1beta1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "taskrname", - }, - Spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{Name: "taskrefname"}, - }, - }, - }, { name: "do not validate spec on delete", taskRun: &v1beta1.TaskRun{ ObjectMeta: metav1.ObjectMeta{Name: "taskrname"}, }, wc: apis.WithinDelete, - }, { - name: "alpha feature: valid resolver", - taskRun: &v1beta1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "tr", - }, - Spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}}, - }, - }, - wc: enableAlphaAPIFields, - }, { - name: "alpha feature: valid resolver with resource parameters", - taskRun: &v1beta1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "tr", - }, - Spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git", Resource: []v1beta1.ResolverParam{{ - Name: "repo", - Value: "https://github.com/tektoncd/pipeline.git", - }, { - Name: "branch", - Value: "baz", - }}}}, - }, - }, - wc: enableAlphaAPIFields, }, { name: "alpha feature: valid step and sidecar overrides", taskRun: &v1beta1.TaskRun{ @@ -194,12 +156,6 @@ func TestTaskRunSpec_Invalidate(t *testing.T) { name: "invalid taskspec", spec: v1beta1.TaskRunSpec{}, wantErr: apis.ErrMissingOneOf("taskRef", "taskSpec"), - }, { - name: "missing taskref name", - spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{}, - }, - wantErr: apis.ErrMissingField("taskRef.name"), }, { name: "invalid taskref and taskspec together", spec: v1beta1.TaskRunSpec{ @@ -287,34 +243,6 @@ func TestTaskRunSpec_Invalidate(t *testing.T) { }, wantErr: apis.ErrMultipleOneOf("params[myobjectparam].name"), wc: enableAlphaAPIFields, - }, { - name: "use of bundle without the feature flag set", - spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{ - Name: "my-task", - Bundle: "docker.io/foo", - }, - }, - wantErr: apis.ErrDisallowedFields("taskRef.bundle"), - }, { - name: "bundle missing name", - spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{ - Bundle: "docker.io/foo", - }, - }, - wantErr: apis.ErrMissingField("taskRef.name"), - wc: enableTektonOCIBundles(t), - }, { - name: "invalid bundle reference", - spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{ - Name: "my-task", - Bundle: "invalid reference", - }, - }, - wantErr: apis.ErrInvalidValue("invalid bundle reference", "taskRef.bundle", "could not parse reference: invalid reference"), - wc: enableTektonOCIBundles(t), }, { name: "using debug when apifields stable", spec: v1beta1.TaskRunSpec{ @@ -338,95 +266,6 @@ func TestTaskRunSpec_Invalidate(t *testing.T) { }, wantErr: apis.ErrInvalidValue("breakito is not a valid breakpoint. Available valid breakpoints include [onFailure]", "debug.breakpoint"), wc: enableAlphaAPIFields, - }, { - name: "taskref resolver disallowed without alpha feature gate", - spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{ - Name: "foo", - ResolverRef: v1beta1.ResolverRef{ - Resolver: "git", - }, - }, - }, - wantErr: apis.ErrDisallowedFields("resolver").ViaField("taskRef"), - }, { - name: "taskref resource disallowed without alpha feature gate", - spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{ - Name: "foo", - ResolverRef: v1beta1.ResolverRef{ - Resource: []v1beta1.ResolverParam{}, - }, - }, - }, - wantErr: apis.ErrDisallowedFields("resource").ViaField("taskRef"), - }, { - name: "taskref resource disallowed without resolver", - spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{ - ResolverRef: v1beta1.ResolverRef{ - Resource: []v1beta1.ResolverParam{}, - }, - }, - }, - wantErr: apis.ErrMissingField("resolver").ViaField("taskRef"), - wc: enableAlphaAPIFields, - }, { - name: "taskref resolver disallowed in conjunction with taskref name", - spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{ - Name: "foo", - ResolverRef: v1beta1.ResolverRef{ - Resolver: "git", - }, - }, - }, - wantErr: apis.ErrMultipleOneOf("name", "resolver").ViaField("taskRef"), - wc: enableAlphaAPIFields, - }, { - name: "taskref resolver disallowed in conjunction with taskref bundle", - spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{ - Bundle: "bar", - ResolverRef: v1beta1.ResolverRef{ - Resolver: "git", - }, - }, - }, - wantErr: apis.ErrMultipleOneOf("bundle", "resolver").ViaField("taskRef"), - wc: enableAlphaAPIFields, - }, { - name: "taskref resource disallowed in conjunction with taskref name", - spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{ - Name: "bar", - ResolverRef: v1beta1.ResolverRef{ - Resource: []v1beta1.ResolverParam{{ - Name: "foo", - Value: "bar", - }}, - }, - }, - }, - wantErr: apis.ErrMultipleOneOf("name", "resource").ViaField("taskRef").Also( - apis.ErrMissingField("resolver").ViaField("taskRef")), - wc: enableAlphaAPIFields, - }, { - name: "taskref resource disallowed in conjunction with taskref bundle", - spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{ - Bundle: "bar", - ResolverRef: v1beta1.ResolverRef{ - Resource: []v1beta1.ResolverParam{{ - Name: "foo", - Value: "bar", - }}, - }, - }, - }, - wantErr: apis.ErrMultipleOneOf("bundle", "resource").ViaField("taskRef").Also( - apis.ErrMissingField("resolver").ViaField("taskRef")), - wc: enableAlphaAPIFields, }, { name: "stepOverride disallowed without alpha feature gate", spec: v1beta1.TaskRunSpec{