From 0c72d59f8269bb06e691e831cdf68769bdd9bbb2 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Wed, 3 Apr 2024 16:08:31 +0200 Subject: [PATCH] Remove taskref/pipelineref deprecated bundle field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This field has been deprecated for about a year and half. So this is "removing" this field from v1beta1 (it's not present in v1 already). The field is kept in the go code to provide a backward compatibility for client code (like chains, …) but it will be disallowed by the webhook. It will also be completely ignore by the rest of the code. Signed-off-by: Vincent Demeester --- docs/deprecations.md | 3 +- docs/migrating-v1beta1-to-v1.md | 3 + docs/pipeline-api.md | 6 +- docs/pipelineruns.md | 15 - docs/pipelines.md | 65 +- docs/taskruns.md | 44 -- .../taskruns/no-ci/tekton-bundles.yaml | 9 - pkg/apis/config/feature_flags.go | 49 +- pkg/apis/config/feature_flags_test.go | 7 - .../pipeline/v1beta1/openapi_generated.go | 4 +- .../pipeline/v1beta1/pipeline_types_test.go | 282 ++++----- .../pipeline/v1beta1/pipeline_validation.go | 20 - .../v1beta1/pipeline_validation_test.go | 563 ++++++++++-------- .../v1beta1/pipelineref_conversion.go | 26 +- .../pipeline/v1beta1/pipelineref_types.go | 2 + .../v1beta1/pipelineref_validation.go | 28 +- .../v1beta1/pipelineref_validation_test.go | 56 -- .../v1beta1/pipelinerun_conversion_test.go | 72 +-- .../v1beta1/pipelinerun_validation_test.go | 98 +-- pkg/apis/pipeline/v1beta1/swagger.json | 4 +- .../pipeline/v1beta1/taskref_conversion.go | 27 +- pkg/apis/pipeline/v1beta1/taskref_types.go | 1 + .../pipeline/v1beta1/taskref_validation.go | 18 +- .../v1beta1/taskref_validation_test.go | 54 -- .../v1beta1/taskrun_conversion_test.go | 550 +++++++++-------- .../v1beta1/taskrun_validation_test.go | 15 + pkg/reconciler/taskrun/taskrun_test.go | 2 - test/conversion_test.go | 337 +---------- test/tektonbundles_test.go | 262 -------- 29 files changed, 851 insertions(+), 1771 deletions(-) delete mode 100644 examples/v1beta1/taskruns/no-ci/tekton-bundles.yaml diff --git a/docs/deprecations.md b/docs/deprecations.md index 34a6bc6c937..2421432ee42 100644 --- a/docs/deprecations.md +++ b/docs/deprecations.md @@ -23,7 +23,6 @@ The following features are deprecated but have not yet been removed. |------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------|-------------------------------------| | [Several fields of Task.Step are deprecated](https://github.com/tektoncd/pipeline/issues/4737) | v0.36.0 | Beta | Feb 25, 2023 | | [ClusterTask is deprecated](https://github.com/tektoncd/pipeline/issues/4476) | v0.41.0 | Beta | July 13, 2023 | -| [`pipelineRef.bundle` and `taskRef.bundle` are deprecated](https://github.com/tektoncd/pipeline/issues/5514) | v0.41.0 | Alpha | July 13, 2023 | | [The `config-trusted-resources` configMap is deprecated](https://github.com/tektoncd/pipeline/issues/5852) | v0.45.0 | Alpha | v0.46.0 | | [The `default-cloud-events-sink` setting in the `config-defaults` configMap is deprecated](https://github.com/tektoncd/pipeline/pull/6883) in favour of the new `config-events` configMap. | v0.50.0 | N/A | v0.59.0 | | [v1beta1 Tasks, TaskRuns, Pipelines, and PipelineRuns are deprecated in favor of v1](https://github.com/tektoncd/pipeline/issues/5541) | v0.50.0 | Beta | v0.62.0 | @@ -67,4 +66,4 @@ See [TEP-0074](https://github.com/tektoncd/community/blob/main/teps/0074-depreca - The generic pipelineResources functions including inputs and outputs resources and the `from` type -- [TaskRun.Status.ResourcesResult is deprecated and tombstoned #6301](https://github.com/tektoncd/pipeline/issues/6325) \ No newline at end of file +- [TaskRun.Status.ResourcesResult is deprecated and tombstoned #6301](https://github.com/tektoncd/pipeline/issues/6325) diff --git a/docs/migrating-v1beta1-to-v1.md b/docs/migrating-v1beta1-to-v1.md index badb5503b2f..925488d9135 100644 --- a/docs/migrating-v1beta1-to-v1.md +++ b/docs/migrating-v1beta1-to-v1.md @@ -50,6 +50,9 @@ In Tekton `v1`, the following fields have been changed: `PipelineResources` and the `resources` fields of Task, TaskRun, Pipeline and PipelineRun have been removed. Please use `Tasks` instead. For more information, see [Replacing PipelineResources](https://github.com/tektoncd/pipeline/blob/main/docs/pipelineresources.md) ## Replacing `taskRef.bundle` and `pipelineRef.bundle` with Bundle Resolver + +**Note: `taskRef.bundle` and `pipelineRef.bundle` are now removed from `v1beta1`. This is kept for "history" purposes**. + Bundle resolver in remote resolution should be used instead of `taskRun.spec.taskRef.bundle` and `pipelineRun.spec.pipelineRef.bundle`. The [`enable-bundles-resolver`](https://github.com/tektoncd/pipeline/blob/main/docs/install.md#customizing-the-pipelines-controller-behavior) feature flag must be enabled to use this feature. diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index d635d006cd6..1539352104e 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -11138,7 +11138,8 @@ string (Optional)

Bundle url reference to a Tekton Bundle.

-

Deprecated: Please use ResolverRef with the bundles resolver instead.

+

Deprecated: Please use ResolverRef with the bundles resolver instead. +The field is staying there for go client backward compatibility, but is not used/allowed anymore.

@@ -14887,7 +14888,8 @@ string (Optional)

Bundle url reference to a Tekton Bundle.

-

Deprecated: Please use ResolverRef with the bundles resolver instead.

+

Deprecated: Please use ResolverRef with the bundles resolver instead. +The field is staying there for go client backward compatibility, but is not used/allowed anymore.

diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md index 716bbb040b3..6507066cde7 100644 --- a/docs/pipelineruns.md +++ b/docs/pipelineruns.md @@ -158,10 +158,6 @@ A `Tekton Bundle` is an OCI artifact that contains Tekton resources like `Tasks` You can reference a `Tekton bundle` in a `TaskRef` in both `v1` and `v1beta1` using [remote resolution](./bundle-resolver.md#pipeline-resolution). The example syntax shown below for `v1` uses remote resolution and requires enabling [beta features](./additional-configs.md#beta-features). -In `v1beta1`, you can also reference a `Tekton bundle` using OCI bundle syntax, which has been deprecated in favor of remote resolution. The example shown below for `v1beta1` uses OCI bundle syntax, and requires enabling `enable-tekton-oci-bundles: "true"` feature flag. - -{{< tabs >}} -{{% tab "v1 & v1beta1" %}} ```yaml spec: pipelineRef: @@ -174,17 +170,6 @@ spec: - name: kind value: Pipeline ``` -{{% /tab %}} - -{{% tab "v1beta1" %}} - ```yaml - spec: - pipelineRef: - name: mypipeline - bundle: docker.io/myrepo/mycatalog:v1.0 - ``` -{{% /tab %}} -{{< /tabs >}} The syntax and caveats are similar to using `Tekton Bundles` for `Task` references in [Pipelines](pipelines.md#tekton-bundles) or [TaskRuns](taskruns.md#tekton-bundles). diff --git a/docs/pipelines.md b/docs/pipelines.md index ebc0b2f9b0b..10ef6c64b78 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -585,45 +585,24 @@ There is currently a hard limit of 20 objects in a bundle. You can reference a `Tekton bundle` in a `TaskRef` in both `v1` and `v1beta1` using [remote resolution](./bundle-resolver.md#pipeline-resolution). The example syntax shown below for `v1` uses remote resolution and requires enabling [beta features](./additional-configs.md#beta-features). -In `v1beta1`, you can also reference a `Tekton bundle` using OCI bundle syntax, which has been deprecated in favor of remote resolution. The example shown below for `v1beta1` uses OCI bundle syntax, and requires enabling `enable-tekton-oci-bundles: "true"` feature flag. - - -{{< tabs >}} -{{% tab "v1 & v1beta1" %}} -```yaml -spec: - taskRef: - resolver: bundles - params: - - name: bundle - value: docker.io/myrepo/mycatalog - - name: name - value: echo-task - - name: kind - value: Task -``` -{{% /tab %}} - -{{% tab "v1beta1" %}} ```yaml spec: tasks: - name: hello-world taskRef: - name: echo-task - bundle: docker.com/myrepo/mycatalog + resolver: bundles + params: + - name: bundle + value: docker.io/myrepo/mycatalog + - name: name + value: echo-task + - name: kind + value: Task ``` -{{% /tab %}} -{{< /tabs >}} - -Here, the `bundle` field is the full reference url to the artifact. The name is the -`metadata.name` field of the `Task`. You may also specify a `tag` as you would with a Docker image which will give you a fixed, repeatable reference to a `Task`. -{{< tabs >}} -{{% tab "v1 & v1beta1" %}} ```yaml spec: taskRef: @@ -636,24 +615,9 @@ spec: - name: kind value: Task ``` -{{% /tab %}} - -{{% tab "v1beta1" %}} -```yaml -spec: - tasks: - - name: hello-world - taskRef: - name: echo-task - bundle: docker.com/myrepo/mycatalog:v1.0.1 -``` -{{% /tab %}} -{{< /tabs >}} You may also specify a fixed digest instead of a tag. -{{< tabs >}} -{{% tab "v1 & v1beta1" %}} ```yaml spec: taskRef: @@ -666,19 +630,6 @@ spec: - name: kind value: Task ``` -{{% /tab %}} - -{{% tab "v1beta1" %}} -```yaml -spec: - tasks: - - name: hello-world - taskRef: - name: echo-task - bundle: docker.io/myrepo/mycatalog@sha256:abc123 -``` -{{% /tab %}} -{{< /tabs >}} Any of the above options will fetch the image using the `ImagePullSecrets` attached to the `ServiceAccount` specified in the `PipelineRun`. diff --git a/docs/taskruns.md b/docs/taskruns.md index ae93416eb5e..213454c1815 100644 --- a/docs/taskruns.md +++ b/docs/taskruns.md @@ -120,10 +120,6 @@ A `Tekton Bundle` is an OCI artifact that contains Tekton resources like `Tasks` You can reference a `Tekton bundle` in a `TaskRef` in both `v1` and `v1beta1` using [remote resolution](./bundle-resolver.md#pipeline-resolution). The example syntax shown below for `v1` uses remote resolution and requires enabling [beta features](./additional-configs.md#beta-features). -In `v1beta1`, you can also reference a `Tekton bundle` using OCI bundle syntax, which has been deprecated in favor of remote resolution. The example shown below for `v1beta1` uses OCI bundle syntax, and requires enabling `enable-tekton-oci-bundles: "true"` feature flag. - -{{< tabs >}} -{{% tab "v1 & v1beta1" %}} ```yaml spec: taskRef: @@ -136,25 +132,9 @@ spec: - name: kind value: Task ``` -{{% /tab %}} - -{{% tab "v1beta1" %}} -```yaml -spec: -taskRef: - name: echo-task - bundle: docker.io/myrepo/mycatalog -``` -{{% /tab %}} -{{< /tabs >}} - -Here, the `bundle` field is the full reference url to the artifact. The name is the -`metadata.name` field of the `Task`. You may also specify a `tag` as you would with a Docker image which will give you a repeatable reference to a `Task`. -{{< tabs >}} -{{% tab "v1 & v1beta1" %}} ```yaml spec: taskRef: @@ -167,22 +147,9 @@ spec: - name: kind value: Task ``` -{{% /tab %}} - -{{% tab "v1beta1" %}} -```yaml -spec: -taskRef: - name: echo-task - bundle: docker.io/myrepo/mycatalog:v1.0.1 -``` -{{% /tab %}} -{{< /tabs >}} You may also specify a fixed digest instead of a tag which ensures the referenced task is constant. -{{< tabs >}} -{{% tab "v1 & v1beta1" %}} ```yaml spec: taskRef: @@ -195,17 +162,6 @@ spec: - name: kind value: Task ``` -{{% /tab %}} - -{{% tab "v1beta1" %}} -```yaml -spec: -taskRef: - name: echo-task - bundle: docker.io/myrepo/mycatalog@sha256:abc123 -``` -{{% /tab %}} -{{< /tabs >}} A working example can be found [here](../examples/v1beta1/taskruns/no-ci/tekton-bundles.yaml). diff --git a/examples/v1beta1/taskruns/no-ci/tekton-bundles.yaml b/examples/v1beta1/taskruns/no-ci/tekton-bundles.yaml deleted file mode 100644 index ac659ad8520..00000000000 --- a/examples/v1beta1/taskruns/no-ci/tekton-bundles.yaml +++ /dev/null @@ -1,9 +0,0 @@ -# TODO: Move the example image to a tekton owned repo. -apiVersion: tekton.dev/v1beta1 -kind: TaskRun -metadata: - name: remote-task-reference -spec: - taskRef: - name: hello-world - bundle: docker.io/ptasci67/example-oci@sha256:053a6cb9f3711d4527dd0d37ac610e8727ec0288a898d5dfbd79b25bcaa29828 diff --git a/pkg/apis/config/feature_flags.go b/pkg/apis/config/feature_flags.go index d9c65cc21ae..6d0541b8391 100644 --- a/pkg/apis/config/feature_flags.go +++ b/pkg/apis/config/feature_flags.go @@ -118,16 +118,16 @@ const ( runningInEnvWithInjectedSidecarsKey = "running-in-environment-with-injected-sidecars" awaitSidecarReadinessKey = "await-sidecar-readiness" requireGitSSHSecretKnownHostsKey = "require-git-ssh-secret-known-hosts" //nolint:gosec - enableTektonOCIBundles = "enable-tekton-oci-bundles" - enableAPIFields = "enable-api-fields" - sendCloudEventsForRuns = "send-cloudevents-for-runs" - enforceNonfalsifiability = "enforce-nonfalsifiability" - verificationNoMatchPolicy = "trusted-resources-verification-no-match-policy" - enableProvenanceInStatus = "enable-provenance-in-status" - resultExtractionMethod = "results-from" - maxResultSize = "max-result-size" - setSecurityContextKey = "set-security-context" - coscheduleKey = "coschedule" + // enableTektonOCIBundles = "enable-tekton-oci-bundles" + enableAPIFields = "enable-api-fields" + sendCloudEventsForRuns = "send-cloudevents-for-runs" + enforceNonfalsifiability = "enforce-nonfalsifiability" + verificationNoMatchPolicy = "trusted-resources-verification-no-match-policy" + enableProvenanceInStatus = "enable-provenance-in-status" + resultExtractionMethod = "results-from" + maxResultSize = "max-result-size" + setSecurityContextKey = "set-security-context" + coscheduleKey = "coschedule" ) // DefaultFeatureFlags holds all the default configurations for the feature flags configmap. @@ -184,13 +184,13 @@ type FeatureFlags struct { DisableCredsInit bool RunningInEnvWithInjectedSidecars bool RequireGitSSHSecretKnownHosts bool - EnableTektonOCIBundles bool - ScopeWhenExpressionsToTask bool - EnableAPIFields string - SendCloudEventsForRuns bool - AwaitSidecarReadiness bool - EnforceNonfalsifiability string - EnableKeepPodOnCancel bool + // EnableTektonOCIBundles bool // Deprecated: this is now ignored + ScopeWhenExpressionsToTask bool + EnableAPIFields string + SendCloudEventsForRuns bool + AwaitSidecarReadiness bool + EnforceNonfalsifiability string + EnableKeepPodOnCancel bool // VerificationNoMatchPolicy is the feature flag for "trusted-resources-verification-no-match-policy" // VerificationNoMatchPolicy can be set to "ignore", "warn" and "fail" values. // ignore: skip trusted resources verification when no matching verification policies found @@ -305,25 +305,14 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) { if err := setPerFeatureFlag(EnableArtifacts, DefaultEnableArtifacts, &tc.EnableArtifacts); err != nil { return nil, err } + if err := setFeatureInlineSpec(cfgMap, DisableInlineSpec, DefaultDisableInlineSpec, &tc.DisableInlineSpec); err != nil { return nil, err } if err := setPerFeatureFlag(EnableConciseResolverSyntax, DefaultEnableConciseResolverSyntax, &tc.EnableConciseResolverSyntax); err != nil { return nil, err } - // Given that they are alpha features, Tekton Bundles and Custom Tasks should be switched on if - // enable-api-fields is "alpha". If enable-api-fields is not "alpha" then fall back to the value of - // each feature's individual flag. - // - // Note: the user cannot enable "alpha" while disabling bundles or custom tasks - that would - // defeat the purpose of having a single shared gate for all alpha features. - if tc.EnableAPIFields == AlphaAPIFields { - tc.EnableTektonOCIBundles = true - } else { - if err := setFeature(enableTektonOCIBundles, DefaultEnableTektonOciBundles, &tc.EnableTektonOCIBundles); err != nil { - return nil, err - } - } + return &tc, nil } diff --git a/pkg/apis/config/feature_flags_test.go b/pkg/apis/config/feature_flags_test.go index 08d33e12caa..176a7a5637d 100644 --- a/pkg/apis/config/feature_flags_test.go +++ b/pkg/apis/config/feature_flags_test.go @@ -43,7 +43,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { RequireGitSSHSecretKnownHosts: false, DisableCredsInit: config.DefaultDisableCredsInit, AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness, - EnableTektonOCIBundles: config.DefaultEnableTektonOciBundles, EnableAPIFields: config.DefaultEnableAPIFields, SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns, VerificationNoMatchPolicy: config.DefaultNoMatchPolicyConfig, @@ -68,7 +67,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { RunningInEnvWithInjectedSidecars: false, AwaitSidecarReadiness: false, RequireGitSSHSecretKnownHosts: true, - EnableTektonOCIBundles: true, EnableAPIFields: "alpha", SendCloudEventsForRuns: true, EnforceNonfalsifiability: "spire", @@ -93,7 +91,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableAPIFields: "alpha", // These are prescribed as true by enabling "alpha" API fields, even // if the submitted text value is "false". - EnableTektonOCIBundles: true, EnforceNonfalsifiability: config.DefaultEnforceNonfalsifiability, DisableAffinityAssistant: config.DefaultDisableAffinityAssistant, DisableCredsInit: config.DefaultDisableCredsInit, @@ -119,7 +116,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { { expectedConfig: &config.FeatureFlags{ EnableAPIFields: "stable", - EnableTektonOCIBundles: true, EnforceNonfalsifiability: config.DefaultEnforceNonfalsifiability, DisableAffinityAssistant: config.DefaultDisableAffinityAssistant, DisableCredsInit: config.DefaultDisableCredsInit, @@ -141,7 +137,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { { expectedConfig: &config.FeatureFlags{ EnableAPIFields: "beta", - EnableTektonOCIBundles: config.DefaultEnableTektonOciBundles, EnforceNonfalsifiability: config.DefaultEnforceNonfalsifiability, DisableAffinityAssistant: config.DefaultDisableAffinityAssistant, DisableCredsInit: config.DefaultDisableCredsInit, @@ -164,7 +159,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { expectedConfig: &config.FeatureFlags{ EnableAPIFields: config.DefaultEnableAPIFields, EnforceNonfalsifiability: config.EnforceNonfalsifiabilityWithSpire, - EnableTektonOCIBundles: config.DefaultEnableTektonOciBundles, VerificationNoMatchPolicy: config.DefaultNoMatchPolicyConfig, RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars, AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness, @@ -220,7 +214,6 @@ func TestNewFeatureFlagsFromEmptyConfigMap(t *testing.T) { RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars, AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness, RequireGitSSHSecretKnownHosts: config.DefaultRequireGitSSHSecretKnownHosts, - EnableTektonOCIBundles: config.DefaultEnableTektonOciBundles, EnableAPIFields: config.DefaultEnableAPIFields, SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns, EnforceNonfalsifiability: config.DefaultEnforceNonfalsifiability, diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index 6a06a0867d2..6856c5ce804 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -1692,7 +1692,7 @@ func schema_pkg_apis_pipeline_v1beta1_PipelineRef(ref common.ReferenceCallback) }, "bundle": { SchemaProps: spec.SchemaProps{ - Description: "Bundle url reference to a Tekton Bundle.\n\nDeprecated: Please use ResolverRef with the bundles resolver instead.", + Description: "Bundle url reference to a Tekton Bundle.\n\nDeprecated: Please use ResolverRef with the bundles resolver instead. The field is staying there for go client backward compatibility, but is not used/allowed anymore.", Type: []string{"string"}, Format: "", }, @@ -4866,7 +4866,7 @@ func schema_pkg_apis_pipeline_v1beta1_TaskRef(ref common.ReferenceCallback) comm }, "bundle": { SchemaProps: spec.SchemaProps{ - Description: "Bundle url reference to a Tekton Bundle.\n\nDeprecated: Please use ResolverRef with the bundles resolver instead.", + Description: "Bundle url reference to a Tekton Bundle.\n\nDeprecated: Please use ResolverRef with the bundles resolver instead. The field is staying there for go client backward compatibility, but is not used/allowed anymore.", Type: []string{"string"}, Format: "", }, diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go index 28fdcf2382d..e7189d17bb9 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go @@ -506,39 +506,6 @@ func TestPipelineTask_ValidateCustomTask(t *testing.T) { } } -func TestPipelineTask_ValidateBundle_Failure(t *testing.T) { - tests := []struct { - name string - p PipelineTask - expectedError apis.FieldError - }{{ - name: "bundle - invalid reference", - p: PipelineTask{ - Name: "foo", - TaskRef: &TaskRef{Name: "bar", Bundle: "invalid reference"}, - }, - expectedError: *apis.ErrInvalidValue("invalid bundle reference (could not parse reference: invalid reference)", "taskRef.bundle"), - }, { - name: "bundle - missing taskRef name", - p: PipelineTask{ - Name: "foo", - TaskRef: &TaskRef{Bundle: "valid-bundle"}, - }, - expectedError: *apis.ErrMissingField("taskRef.name"), - }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := tt.p.validateBundle() - if err == nil { - t.Error("PipelineTask.ValidateBundles() did not return error for invalid bundle in a pipelineTask") - } - if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" { - t.Errorf("Pipeline.ValidateBundles() errors diff %s", diff.PrintWantGot(d)) - } - }) - } -} - func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) { tests := []struct { name string @@ -593,13 +560,6 @@ func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) { TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar", Params: Params{}}}, }, configMap: map[string]string{"enable-api-fields": "beta"}, - }, { - name: "pipeline task - use of bundle with the feature flag set", - tasks: PipelineTask{ - Name: "foo", - TaskRef: &TaskRef{Name: "bar", Bundle: "docker.io/foo"}, - }, - configMap: map[string]string{"enable-tekton-oci-bundles": "true"}, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -648,13 +608,6 @@ func TestPipelineTask_ValidateRegularTask_Failure(t *testing.T) { Message: `missing field(s)`, Paths: []string{"taskRef.name"}, }, - }, { - name: "pipeline task - use of bundle without the feature flag set", - task: PipelineTask{ - Name: "foo", - TaskRef: &TaskRef{Name: "bar", Bundle: "docker.io/foo"}, - }, - expectedError: *apis.ErrGeneric("bundle requires \"enable-tekton-oci-bundles\" feature gate to be true but it is false"), }, { name: "pipeline task - taskRef with resolver and k8s style name", task: PipelineTask{ @@ -754,7 +707,8 @@ func TestPipelineTask_Validate_Failure(t *testing.T) { p: PipelineTask{Name: "foo", TaskSpec: &EmbeddedTask{ TypeMeta: runtime.TypeMeta{ APIVersion: "example.com", - }}}, + }, + }}, expectedError: *apis.ErrInvalidValue("custom task spec must specify kind", "taskSpec.kind"), }, { name: "custom task reference in taskref missing apiversion", @@ -765,19 +719,9 @@ func TestPipelineTask_Validate_Failure(t *testing.T) { p: PipelineTask{Name: "foo", TaskSpec: &EmbeddedTask{ TypeMeta: runtime.TypeMeta{ Kind: "Example", - }}}, + }, + }}, expectedError: *apis.ErrInvalidValue("custom task spec must specify apiVersion", "taskSpec.apiVersion"), - }, { - name: "invalid bundle without bundle name", - p: PipelineTask{ - Name: "invalid-bundle", - TaskRef: &TaskRef{Bundle: "bundle"}, - }, - expectedError: apis.FieldError{ - Message: `missing field(s)`, - Paths: []string{"taskRef.name"}, - }, - wc: enableFeatures(t, []string{"enable-tekton-oci-bundles"}), }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -833,35 +777,44 @@ func TestPipelineTaskList_Deps(t *testing.T) { }, }, { name: "valid pipeline with Task Results deps", - tasks: []PipelineTask{{ - Name: "task-1", - }, { - Name: "task-2", - Params: Params{{ - Value: ParamValue{ - Type: "string", - StringVal: "$(tasks.task-1.results.result)", - }}, - }}, + tasks: []PipelineTask{ + { + Name: "task-1", + }, { + Name: "task-2", + Params: Params{ + { + Value: ParamValue{ + Type: "string", + StringVal: "$(tasks.task-1.results.result)", + }, + }, + }, + }, }, expectedDeps: map[string][]string{ "task-2": {"task-1"}, }, }, { name: "valid pipeline with Task Results in Matrix deps", - tasks: []PipelineTask{{ - Name: "task-1", - }, { - Name: "task-2", - Matrix: &Matrix{ - Params: Params{{ - Value: ParamValue{ - Type: ParamTypeArray, - ArrayVal: []string{ - "$(tasks.task-1.results.result)", + tasks: []PipelineTask{ + { + Name: "task-1", + }, { + Name: "task-2", + Matrix: &Matrix{ + Params: Params{ + { + Value: ParamValue{ + Type: ParamTypeArray, + ArrayVal: []string{ + "$(tasks.task-1.results.result)", + }, + }, }, - }}, - }}}, + }, + }, + }, }, expectedDeps: map[string][]string{ "task-2": {"task-1"}, @@ -899,67 +852,38 @@ func TestPipelineTaskList_Validate(t *testing.T) { expectedError *apis.FieldError wc func(context.Context) context.Context }{{ - name: "validate all three valid custom task, bundle, and regular task", + name: "validate all valid custom task, and regular task", tasks: PipelineTaskList{{ Name: "valid-custom-task", TaskRef: &TaskRef{APIVersion: "example.com", Kind: "custom"}, - }, { - Name: "valid-bundle", - TaskRef: &TaskRef{Bundle: "bundle", Name: "bundle"}, }, { Name: "valid-task", TaskRef: &TaskRef{Name: "task"}, }}, path: "tasks", - wc: enableFeatures(t, []string{"enable-tekton-oci-bundles"}), }, { - name: "validate list of tasks with valid custom task and bundle but invalid regular task", + name: "validate list of tasks with valid custom task and invalid regular task", tasks: PipelineTaskList{{ Name: "valid-custom-task", TaskRef: &TaskRef{APIVersion: "example.com", Kind: "custom"}, - }, { - Name: "valid-bundle", - TaskRef: &TaskRef{Bundle: "bundle", Name: "bundle"}, }, { Name: "invalid-task-without-name", TaskRef: &TaskRef{Name: ""}, }}, path: "tasks", - expectedError: apis.ErrGeneric(`missing field(s)`, "tasks[2].taskRef.name"), - wc: enableFeatures(t, []string{"enable-tekton-oci-bundles"}), - }, { - name: "validate list of tasks with valid custom task but invalid bundle and invalid regular task", - tasks: PipelineTaskList{{ - Name: "valid-custom-task", - TaskRef: &TaskRef{APIVersion: "example.com", Kind: "custom"}, - }, { - Name: "invalid-bundle", - TaskRef: &TaskRef{Bundle: "bundle"}, - }, { - Name: "invalid-task-without-name", - TaskRef: &TaskRef{Name: ""}, - }}, - path: "tasks", - expectedError: apis.ErrGeneric(`missing field(s)`, "tasks[2].taskRef.name").Also( - apis.ErrGeneric(`missing field(s)`, "tasks[1].taskRef.name")), - wc: enableFeatures(t, []string{"enable-tekton-oci-bundles"}), + expectedError: apis.ErrGeneric(`missing field(s)`, "tasks[1].taskRef.name"), }, { - name: "validate all three invalid tasks - custom task, bundle and regular task", + name: "validate all invalid tasks - custom task and regular task", tasks: PipelineTaskList{{ Name: "invalid-custom-task", TaskRef: &TaskRef{APIVersion: "example.com"}, - }, { - Name: "invalid-bundle", - TaskRef: &TaskRef{Bundle: "bundle"}, }, { Name: "invalid-task", TaskRef: &TaskRef{Name: ""}, }}, path: "tasks", - expectedError: apis.ErrGeneric(`missing field(s)`, "tasks[2].taskRef.name").Also( - apis.ErrGeneric(`missing field(s)`, "tasks[1].taskRef.name")).Also( + expectedError: apis.ErrGeneric(`missing field(s)`, "tasks[1].taskRef.name").Also( apis.ErrGeneric(`invalid value: custom task ref must specify kind`, "tasks[0].taskRef.kind")), - wc: enableFeatures(t, []string{"enable-tekton-oci-bundles"}), }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -992,7 +916,8 @@ func TestPipelineTask_ValidateMatrix(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }}}, + }}, + }, Params: Params{{ Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, }}, @@ -1003,12 +928,15 @@ func TestPipelineTask_ValidateMatrix(t *testing.T) { pt: &PipelineTask{ Name: "task", Matrix: &Matrix{ - Include: IncludeParamsList{{ - Name: "duplicate-param", - Params: Params{{ - Name: "duplicate", Value: ParamValue{Type: ParamTypeString, StringVal: "foo"}, - }}}, - }}, + Include: IncludeParamsList{ + { + Name: "duplicate-param", + Params: Params{{ + Name: "duplicate", Value: ParamValue{Type: ParamTypeString, StringVal: "foo"}, + }}, + }, + }, + }, Params: Params{{ Name: "duplicate", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, }}, @@ -1023,7 +951,8 @@ func TestPipelineTask_ValidateMatrix(t *testing.T) { Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, }, { Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo-1", "bar-1"}}, - }}}, + }}, + }, }, wantErrs: &apis.FieldError{ Message: `parameter names must be unique, the parameter "foobar" is also defined at`, @@ -1036,7 +965,8 @@ func TestPipelineTask_ValidateMatrix(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }}}, + }}, + }, Params: Params{{ Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}}, }}, @@ -1046,14 +976,17 @@ func TestPipelineTask_ValidateMatrix(t *testing.T) { pt: &PipelineTask{ Name: "task", Matrix: &Matrix{ - Include: IncludeParamsList{{ - Name: "invalid-include", - Params: Params{{ - Name: "foobar", Value: ParamValue{Type: ParamTypeString, StringVal: "foo"}, - }, { - Name: "foobar", Value: ParamValue{Type: ParamTypeString, StringVal: "foo-1"}, - }}}, - }}, + Include: IncludeParamsList{ + { + Name: "invalid-include", + Params: Params{{ + Name: "foobar", Value: ParamValue{Type: ParamTypeString, StringVal: "foo"}, + }, { + Name: "foobar", Value: ParamValue{Type: ParamTypeString, StringVal: "foo-1"}, + }}, + }, + }, + }, }, wantErrs: &apis.FieldError{ Message: `parameter names must be unique, the parameter "foobar" is also defined at`, @@ -1073,7 +1006,8 @@ func TestPipelineTask_ValidateMatrix(t *testing.T) { Name: "foo", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.foobar[*])"}, }, { Name: "bar", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.barfoo[*])"}, - }}}, + }}, + }, }, }, { name: "parameters in matrix contain result references", @@ -1082,7 +1016,8 @@ func TestPipelineTask_ValidateMatrix(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.foo-task.results.a-result)"}}, - }}}, + }}, + }, }, }, { name: "count of combinations of parameters in the matrix exceeds the maximum", @@ -1093,7 +1028,8 @@ func TestPipelineTask_ValidateMatrix(t *testing.T) { Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac", "windows"}}, }, { Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "firefox", "safari"}}, - }}}, + }}, + }, }, wantErrs: &apis.FieldError{ Message: "expected 0 <= 9 <= 4", @@ -1108,7 +1044,8 @@ func TestPipelineTask_ValidateMatrix(t *testing.T) { Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac"}}, }, { Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "firefox"}}, - }}}, + }}, + }, }, }, { name: "valid matrix emitting string results consumed in aggregate by another pipelineTask", @@ -1281,7 +1218,8 @@ func TestPipelineTask_IsMatrixed(t *testing.T) { }, }, expected: true, - }, { + }, + { name: "matrixed with include", arg: arg{ Matrix: &Matrix{ @@ -1290,12 +1228,14 @@ func TestPipelineTask_IsMatrixed(t *testing.T) { Params: Params{{ Name: "IMAGE", Value: ParamValue{Type: ParamTypeString, StringVal: "image-1"}, }, { - Name: "DOCKERFILE", Value: ParamValue{Type: ParamTypeString, StringVal: "path/to/Dockerfile1"}}}, + Name: "DOCKERFILE", Value: ParamValue{Type: ParamTypeString, StringVal: "path/to/Dockerfile1"}, + }}, }}, }, }, expected: true, - }, { + }, + { name: "matrixed with params and include", arg: arg{ Matrix: &Matrix{ @@ -1305,7 +1245,8 @@ func TestPipelineTask_IsMatrixed(t *testing.T) { Include: IncludeParamsList{{ Name: "common-package", Params: Params{{ - Name: "package", Value: ParamValue{Type: ParamTypeString, StringVal: "path/to/common/package/"}}}, + Name: "package", Value: ParamValue{Type: ParamTypeString, StringVal: "path/to/common/package/"}, + }}, }}, }, }, @@ -1330,36 +1271,37 @@ func TestEmbeddedTask_IsCustomTask(t *testing.T) { name string et *EmbeddedTask want bool - }{{ - name: "not a custom task - APIVersion and Kind are not set", - et: &EmbeddedTask{}, - want: false, - }, { - name: "not a custom task - APIVersion is not set", - et: &EmbeddedTask{ - TypeMeta: runtime.TypeMeta{ - Kind: "Example", + }{ + { + name: "not a custom task - APIVersion and Kind are not set", + et: &EmbeddedTask{}, + want: false, + }, { + name: "not a custom task - APIVersion is not set", + et: &EmbeddedTask{ + TypeMeta: runtime.TypeMeta{ + Kind: "Example", + }, }, - }, - want: false, - }, { - name: "not a custom task - Kind is not set", - et: &EmbeddedTask{ - TypeMeta: runtime.TypeMeta{ - APIVersion: "example/v0", + want: false, + }, { + name: "not a custom task - Kind is not set", + et: &EmbeddedTask{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "example/v0", + }, }, - }, - want: false, - }, { - name: "custom task - APIVersion and Kind are set", - et: &EmbeddedTask{ - TypeMeta: runtime.TypeMeta{ - Kind: "Example", - APIVersion: "example/v0", + want: false, + }, { + name: "custom task - APIVersion and Kind are set", + et: &EmbeddedTask{ + TypeMeta: runtime.TypeMeta{ + Kind: "Example", + APIVersion: "example/v0", + }, }, + want: true, }, - want: true, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1379,7 +1321,8 @@ func TestPipelineChecksum(t *testing.T) { pipeline: &Pipeline{ TypeMeta: metav1.TypeMeta{ APIVersion: "tekton.dev/v1beta1", - Kind: "Pipeline"}, + Kind: "Pipeline", + }, ObjectMeta: metav1.ObjectMeta{ Name: "pipeline", Namespace: "pipeline-ns", @@ -1394,7 +1337,8 @@ func TestPipelineChecksum(t *testing.T) { pipeline: &Pipeline{ TypeMeta: metav1.TypeMeta{ APIVersion: "tekton.dev/v1beta1", - Kind: "Pipeline"}, + Kind: "Pipeline", + }, ObjectMeta: metav1.ObjectMeta{ Name: "pipeline", Namespace: "pipeline-ns", diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index f1c34eee5e5..2ac8f08f580 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -21,7 +21,6 @@ import ( "fmt" "strings" - "github.com/google/go-containerregistry/pkg/name" "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/validate" "github.com/tektoncd/pipeline/pkg/internal/resultref" @@ -205,7 +204,6 @@ func (pt PipelineTask) Validate(ctx context.Context) (errs *apis.FieldError) { } } - cfg := config.FromContextOrDefaults(ctx) // Pipeline task having taskRef/taskSpec with APIVersion is classified as custom task switch { case pt.TaskRef != nil && !taskKinds[pt.TaskRef.Kind]: @@ -216,9 +214,6 @@ func (pt PipelineTask) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(pt.validateCustomTask()) case pt.TaskSpec != nil && pt.TaskSpec.APIVersion != "": errs = errs.Also(pt.validateCustomTask()) - // If EnableTektonOCIBundles feature flag is on, validate bundle specifications - case cfg.FeatureFlags.EnableTektonOCIBundles && pt.TaskRef != nil && pt.TaskRef.Bundle != "": - errs = errs.Also(pt.validateBundle()) default: errs = errs.Also(pt.validateTask(ctx)) } @@ -356,21 +351,6 @@ func (pt PipelineTask) validateCustomTask() (errs *apis.FieldError) { return errs } -// validateBundle validates bundle specifications - checking name and bundle -func (pt PipelineTask) validateBundle() (errs *apis.FieldError) { - // bundle requires a TaskRef to be specified - if (pt.TaskRef != nil && pt.TaskRef.Bundle != "") && pt.TaskRef.Name == "" { - errs = errs.Also(apis.ErrMissingField("taskRef.name")) - } - // If a bundle url is specified, ensure it is parsable - if pt.TaskRef != nil && pt.TaskRef.Bundle != "" { - if _, err := name.ParseReference(pt.TaskRef.Bundle); err != nil { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("invalid bundle reference (%s)", err.Error()), "taskRef.bundle")) - } - } - return errs -} - // validateTask validates a pipeline task or a final task for taskRef and taskSpec func (pt PipelineTask) validateTask(ctx context.Context) (errs *apis.FieldError) { if pt.TaskSpec != nil { diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index adb5810009b..360ca72854b 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -25,13 +25,11 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/config" cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing" "github.com/tektoncd/pipeline/test/diff" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/selection" "k8s.io/apimachinery/pkg/util/sets" "knative.dev/pkg/apis" - logtesting "knative.dev/pkg/logging/testing" ) func TestPipeline_Validate_Success(t *testing.T) { @@ -60,7 +58,8 @@ func TestPipeline_Validate_Success(t *testing.T) { p: &Pipeline{ ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, Spec: PipelineSpec{ - Tasks: []PipelineTask{{Name: "foo", + Tasks: []PipelineTask{{ + Name: "foo", TaskSpec: &EmbeddedTask{ TypeMeta: runtime.TypeMeta{ APIVersion: "example.dev/v0", @@ -68,7 +67,8 @@ func TestPipeline_Validate_Success(t *testing.T) { }, Spec: runtime.RawExtension{ Raw: []byte(`{"field1":123,"field2":"value"}`), - }}, + }, + }, }}, }, }, @@ -1285,64 +1285,66 @@ func TestFinallyTaskResultsToPipelineResults_Success(t *testing.T) { name string p *Pipeline wc func(context.Context) context.Context - }{{ - name: "valid pipeline with pipeline results", - p: &Pipeline{ - ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, - Spec: PipelineSpec{ - Results: []PipelineResult{{ - Name: "initialized", - Value: *NewStructuredValues("$(tasks.clone-app-repo.results.initialized)"), - }}, - Tasks: []PipelineTask{{ - Name: "clone-app-repo", - TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ - Results: []TaskResult{{ - Name: "initialized", - Type: "string", - }}, - Steps: []Step{{ - Name: "foo", Image: "bar", + }{ + { + name: "valid pipeline with pipeline results", + p: &Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: PipelineSpec{ + Results: []PipelineResult{{ + Name: "initialized", + Value: *NewStructuredValues("$(tasks.clone-app-repo.results.initialized)"), + }}, + Tasks: []PipelineTask{{ + Name: "clone-app-repo", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Results: []TaskResult{{ + Name: "initialized", + Type: "string", + }}, + Steps: []Step{{ + Name: "foo", Image: "bar", + }}, }}, }}, - }}, + }, }, - }}, { - name: "referencing existent finally task result", - p: &Pipeline{ - ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, - Spec: PipelineSpec{ - Results: []PipelineResult{{ - Name: "initialized", - Value: *NewStructuredValues("$(finally.check-git-commit.results.init)"), - }}, - Tasks: []PipelineTask{{ - Name: "clone-app-repo", - TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ - Results: []TaskResult{{ - Name: "current-date-unix-timestamp", - Type: "string", - }}, - Steps: []Step{{ - Name: "foo", Image: "bar", - }}, + }, { + name: "referencing existent finally task result", + p: &Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: PipelineSpec{ + Results: []PipelineResult{{ + Name: "initialized", + Value: *NewStructuredValues("$(finally.check-git-commit.results.init)"), }}, - }}, - Finally: []PipelineTask{{ - Name: "check-git-commit", - TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ - Results: []TaskResult{{ - Name: "init", - Type: "string", + Tasks: []PipelineTask{{ + Name: "clone-app-repo", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Results: []TaskResult{{ + Name: "current-date-unix-timestamp", + Type: "string", + }}, + Steps: []Step{{ + Name: "foo", Image: "bar", + }}, }}, - Steps: []Step{{ - Name: "foo2", Image: "bar", + }}, + Finally: []PipelineTask{{ + Name: "check-git-commit", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Results: []TaskResult{{ + Name: "init", + Type: "string", + }}, + Steps: []Step{{ + Name: "foo2", Image: "bar", + }}, }}, }}, - }}, + }, }, }, - }, } for _, tt := range tests { @@ -1586,7 +1588,8 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(params.baz)", "and", "$(params.foo-is-baz)"}}, - }}}, + }}, + }, }}, }, { name: "valid star array parameter variables in matrix", @@ -1601,7 +1604,8 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(params.baz[*])", "and", "$(params.foo-is-baz[*])"}}, - }}}, + }}, + }, }}, }, { name: "array param - using the whole variable as a param's value that is intended to be array type", @@ -1627,9 +1631,13 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) { Matrix: &Matrix{ Include: IncludeParamsList{{ Name: "build-1", - Params: Params{{ - Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.baz)"}}, - }}}}, + Params: Params{ + { + Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.baz)"}, + }, + }, + }}, + }, }}, }, { name: "object param - using single individual variable in string param", @@ -1721,7 +1729,8 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(params.myObject.key1)", "and", "$(params.myObject.key2)"}}, - }}}, + }}, + }, }}, }, { name: "object param - using the whole variable as a param's value that is intended to be object type", @@ -1903,7 +1912,8 @@ func TestValidatePipelineDeclaredParameterUsage_Failure(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(params.does-not-exist)"}}, - }}}, + }}, + }, }}, expectedError: apis.FieldError{ Message: `non-existent variable in "$(params.does-not-exist)"`, @@ -1920,7 +1930,8 @@ func TestValidatePipelineDeclaredParameterUsage_Failure(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(params.foo)", "and", "$(params.does-not-exist)"}}, - }}}, + }}, + }, }}, expectedError: apis.FieldError{ Message: `non-existent variable in "$(params.does-not-exist)"`, @@ -1938,7 +1949,9 @@ func TestValidatePipelineDeclaredParameterUsage_Failure(t *testing.T) { Params: Params{{ Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(params.foo)"}}, }, { - Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(params.does-not-exist)"}}}}}, + Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(params.does-not-exist)"}}, + }}, + }, }}, expectedError: apis.FieldError{ Message: `non-existent variable in "$(params.does-not-exist)"`, @@ -1959,7 +1972,8 @@ func TestValidatePipelineDeclaredParameterUsage_Failure(t *testing.T) { }, { Name: "b-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.does-not-exist)"}, }}, - }}}, + }}, + }, }}, expectedError: apis.FieldError{ Message: `non-existent variable in "$(params.does-not-exist)"`, @@ -2098,7 +2112,8 @@ func TestValidatePipelineDeclaredParameterUsage_Failure(t *testing.T) { Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(params.myObject.key1)"}}, }, { Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(params.myObject.non-exist-key)"}}, - }}}, + }}, + }, }}, expectedError: apis.FieldError{ Message: `non-existent variable in "$(params.myObject.non-exist-key)"`, @@ -2126,7 +2141,8 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) { expectedError apis.FieldError configMap map[string]string }{ - {name: "param enum with array type - failure", + { + name: "param enum with array type - failure", params: []ParamSpec{{ Name: "param2", Type: ParamTypeArray, @@ -2351,7 +2367,8 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) { Message: `parameter names must be unique, the parameter "duplicate-param" is also defined at`, Paths: []string{"[0].params[1].name, [0].params[2].name"}, }, - }} + }, + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() @@ -3254,7 +3271,8 @@ func TestContextValid(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param-mat", Value: ParamValue{ArrayVal: []string{"$(context.pipeline.name)"}}, - }}}, + }}, + }, }}, }, { name: "valid string context variable for PipelineRun name", @@ -3267,7 +3285,8 @@ func TestContextValid(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param-mat", Value: ParamValue{ArrayVal: []string{"$(context.pipelineRun.name)"}}, - }}}, + }}, + }, }}, }, { name: "valid string context variable for PipelineRun namespace", @@ -3280,7 +3299,8 @@ func TestContextValid(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param-mat", Value: ParamValue{ArrayVal: []string{"$(context.pipelineRun.namespace)"}}, - }}}, + }}, + }, }}, }, { name: "valid string context variable for PipelineRun uid", @@ -3293,7 +3313,8 @@ func TestContextValid(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param-mat", Value: ParamValue{ArrayVal: []string{"$(context.pipelineRun.uid)"}}, - }}}, + }}, + }, }}, }, { name: "valid array context variables for Pipeline and PipelineRun names", @@ -3306,7 +3327,8 @@ func TestContextValid(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param-mat", Value: ParamValue{ArrayVal: []string{"$(context.pipeline.name)", "and", "$(context.pipelineRun.name)"}}, - }}}, + }}, + }, }}, }, { name: "valid string context variable for PipelineTask retries", @@ -3319,7 +3341,8 @@ func TestContextValid(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param", Value: ParamValue{StringVal: "$(context.pipelineTask.retries)"}, - }}}, + }}, + }, }}, }, { name: "valid array context variable for PipelineTask retries", @@ -3332,7 +3355,8 @@ func TestContextValid(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param-mat", Value: ParamValue{ArrayVal: []string{"$(context.pipelineTask.retries)"}}, - }}}, + }}, + }, }}, }, { name: "valid string context variable for Pipeline name in include params", @@ -3346,8 +3370,10 @@ func TestContextValid(t *testing.T) { Include: IncludeParamsList{{ Name: "build-1", Params: Params{{ - Name: "a-param-mat", Value: ParamValue{Type: ParamTypeString, StringVal: "$(context.pipeline.name)"}}}, - }}}, + Name: "a-param-mat", Value: ParamValue{Type: ParamTypeString, StringVal: "$(context.pipeline.name)"}, + }}, + }}, + }, }}, }, { name: "valid string context variable for PipelineTask retries in matrix include", @@ -3361,8 +3387,10 @@ func TestContextValid(t *testing.T) { Include: IncludeParamsList{{ Name: "build-1", Params: Params{{ - Name: "a-param-mat", Value: ParamValue{Type: ParamTypeString, StringVal: "$(context.pipelineTask.retries)"}}}, - }}}, + Name: "a-param-mat", Value: ParamValue{Type: ParamTypeString, StringVal: "$(context.pipelineTask.retries)"}, + }}, + }}, + }, }}, }} for _, tt := range tests { @@ -3390,7 +3418,8 @@ func TestContextInvalid(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param-foo", Value: ParamValue{ArrayVal: []string{"$(context.pipeline.missing-foo)"}}, - }}}, + }}, + }, }}, expectedError: *apis.ErrGeneric("").Also(&apis.FieldError{ Message: `non-existent variable in "$(context.pipeline.missing)"`, @@ -3410,7 +3439,8 @@ func TestContextInvalid(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param-foo", Value: ParamValue{ArrayVal: []string{"$(context.pipelineRun.missing-foo)"}}, - }}}, + }}, + }, }}, expectedError: *apis.ErrGeneric("").Also(&apis.FieldError{ Message: `non-existent variable in "$(context.pipelineRun.missing)"`, @@ -3430,7 +3460,8 @@ func TestContextInvalid(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param-foo", Value: ParamValue{ArrayVal: []string{"$(context.pipelineTask.missing-foo)"}}, - }}}, + }}, + }, }}, expectedError: *apis.ErrGeneric("").Also(&apis.FieldError{ Message: `non-existent variable in "$(context.pipelineTask.missing)"`, @@ -3450,7 +3481,8 @@ func TestContextInvalid(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param", Value: ParamValue{ArrayVal: []string{"$(context.pipeline.missing-foo)", "$(context.pipelineTask.missing-foo)", "$(context.pipelineRun.missing-foo)"}}, - }}}, + }}, + }, }}, expectedError: *apis.ErrGeneric(`non-existent variable in "$(context.pipeline.missing)"`, "value"). Also(apis.ErrGeneric(`non-existent variable in "$(context.pipelineRun.missing)"`, "value")). @@ -3467,8 +3499,10 @@ func TestContextInvalid(t *testing.T) { Include: IncludeParamsList{{ Name: "build-1", Params: Params{{ - Name: "a-param-foo", Value: ParamValue{Type: ParamTypeString, StringVal: "$(context.pipeline.missing)"}}}, - }}}, + Name: "a-param-foo", Value: ParamValue{Type: ParamTypeString, StringVal: "$(context.pipeline.missing)"}, + }}, + }}, + }, }}, expectedError: *apis.ErrGeneric("").Also(&apis.FieldError{ Message: `non-existent variable in "$(context.pipeline.missing)"`, @@ -3483,8 +3517,10 @@ func TestContextInvalid(t *testing.T) { Include: IncludeParamsList{{ Name: "build-1", Params: Params{{ - Name: "a-param-foo", Value: ParamValue{Type: ParamTypeString, StringVal: "$(context.pipelineRun.missing)"}}}, - }}}, + Name: "a-param-foo", Value: ParamValue{Type: ParamTypeString, StringVal: "$(context.pipelineRun.missing)"}, + }}, + }}, + }, }}, expectedError: *apis.ErrGeneric("").Also(&apis.FieldError{ Message: `non-existent variable in "$(context.pipelineRun.missing)"`, @@ -3499,8 +3535,10 @@ func TestContextInvalid(t *testing.T) { Include: IncludeParamsList{{ Name: "build-1", Params: Params{{ - Name: "a-param-foo", Value: ParamValue{Type: ParamTypeString, StringVal: "$(context.pipelineTask.missing)"}}}, - }}}, + Name: "a-param-foo", Value: ParamValue{Type: ParamTypeString, StringVal: "$(context.pipelineTask.missing)"}, + }}, + }}, + }, }}, expectedError: *apis.ErrGeneric("").Also(&apis.FieldError{ Message: `non-existent variable in "$(context.pipelineTask.missing)"`, @@ -3780,7 +3818,8 @@ func TestMatrixIncompatibleAPIVersions(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }}}, + }}, + }, } tests := []struct { name string @@ -3843,7 +3882,8 @@ func Test_validateMatrix(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }}}, + }}, + }, Params: Params{{ Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, }}, @@ -3857,7 +3897,8 @@ func Test_validateMatrix(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }}}, + }}, + }, Params: Params{{ Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}}, }}, @@ -3870,14 +3911,16 @@ func Test_validateMatrix(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.foo-task.results.a-result)"}}, - }}}, + }}, + }, }, { Name: "b-task", TaskRef: &TaskRef{Name: "b-task"}, Matrix: &Matrix{ Params: Params{{ Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.bar-task.results.b-result)"}}, - }}}, + }}, + }, }}, }, { name: "parameters in matrix contain whole array results references", @@ -3887,7 +3930,8 @@ func Test_validateMatrix(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.foo-task.results.a-task-results[*])"}}, - }}}, + }}, + }, }}, }, { name: "results from matrixed task consumed in tasks through parameters", @@ -3897,7 +3941,8 @@ func Test_validateMatrix(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }}}, + }}, + }, }, { Name: "b-task", TaskRef: &TaskRef{Name: "b-task"}, @@ -3913,7 +3958,8 @@ func Test_validateMatrix(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }}}, + }}, + }, }}, finally: PipelineTaskList{{ Name: "b-task", @@ -3930,7 +3976,8 @@ func Test_validateMatrix(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }}}, + }}, + }, }, { Name: "b-task", TaskRef: &TaskRef{Name: "b-task"}, @@ -3953,7 +4000,8 @@ func Test_validateMatrix(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }}}, + }}, + }, }, { Name: "b-task", TaskRef: &TaskRef{Name: "b-task"}, @@ -3971,7 +4019,8 @@ func Test_validateMatrix(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }}}, + }}, + }, }}, finally: PipelineTaskList{{ Name: "b-task", @@ -3990,7 +4039,8 @@ func Test_validateMatrix(t *testing.T) { Matrix: &Matrix{ Params: Params{{ Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }}}, + }}, + }, }, { Name: "b-task", TaskRef: &TaskRef{Name: "b-task"}, @@ -4019,7 +4069,8 @@ func Test_validateMatrix(t *testing.T) { Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac"}}, }, { Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "safari"}}, - }}}, + }}, + }, }, { Name: "echoarrayurl", TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ @@ -4041,7 +4092,8 @@ func Test_validateMatrix(t *testing.T) { Params: ParamSpecs{{ Name: "platform", }, { - Name: "browser"}}, + Name: "browser", + }}, Results: []TaskResult{{ Name: "report-url", Type: ResultsTypeString, @@ -4050,7 +4102,8 @@ func Test_validateMatrix(t *testing.T) { Name: "produce-report-url", Image: "alpine", Script: ` | - echo -n "https://api.example/get-report/$(params.platform)-$(params.browser)" | tee $(results.report-url.path)`}}, + echo -n "https://api.example/get-report/$(params.platform)-$(params.browser)" | tee $(results.report-url.path)`, + }}, }}, }, { Name: "task-consuming-results", @@ -4084,7 +4137,8 @@ func Test_validateMatrix(t *testing.T) { Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac"}}, }, { Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "safari"}}, - }}}, + }}, + }, }, { Name: "task-consuming-results", TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ @@ -4114,14 +4168,16 @@ func Test_validateMatrix(t *testing.T) { Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac"}}, }, { Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "safari"}}, - }}}, + }}, + }, }, { Name: "taskwithresult", TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ Params: ParamSpecs{{ Name: "platform", }, { - Name: "browser"}}, + Name: "browser", + }}, Results: []TaskResult{{ Name: "report-url", Type: ResultsTypeString, @@ -4130,7 +4186,8 @@ func Test_validateMatrix(t *testing.T) { Name: "produce-report-url", Image: "alpine", Script: ` | - echo -n "https://api.example/get-report/$(params.platform)-$(params.browser)" | tee $(results.report-url.path)`}}, + echo -n "https://api.example/get-report/$(params.platform)-$(params.browser)" | tee $(results.report-url.path)`, + }}, }}, }, { Name: "task-consuming-results", @@ -4163,7 +4220,8 @@ func Test_validateMatrix(t *testing.T) { Params: ParamSpecs{{ Name: "platform", }, { - Name: "browser"}}, + Name: "browser", + }}, Results: []TaskResult{{ Name: "array-result", Type: ResultsTypeArray, @@ -4172,21 +4230,24 @@ func Test_validateMatrix(t *testing.T) { Name: "produce-array-result", Image: "alpine", Script: ` | - echo -n "[\"${params.platform}\",\"${params.browser}\"]" | tee $(results.array-result.path)`}}, + echo -n "[\"${params.platform}\",\"${params.browser}\"]" | tee $(results.array-result.path)`, + }}, }}, Matrix: &Matrix{ Params: Params{{ Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac"}}, }, { Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "safari"}}, - }}}, + }}, + }, }, { Name: "taskwithresult", TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ Params: ParamSpecs{{ Name: "platform", }, { - Name: "browser"}}, + Name: "browser", + }}, Results: []TaskResult{{ Name: "array-result", Type: ResultsTypeArray, @@ -4195,7 +4256,8 @@ func Test_validateMatrix(t *testing.T) { Name: "produce-array-result", Image: "alpine", Script: ` | - echo -n "https://api.example/get-report/$(params.platform)-$(params.browser)" | tee $(results.array-result.path)`}}, + echo -n "https://api.example/get-report/$(params.platform)-$(params.browser)" | tee $(results.array-result.path)`, + }}, }}, }, { Name: "task-consuming-results", @@ -4228,7 +4290,8 @@ func Test_validateMatrix(t *testing.T) { Params: ParamSpecs{{ Name: "platform", }, { - Name: "browser"}}, + Name: "browser", + }}, Results: []TaskResult{{ Name: "array-result", Type: ResultsTypeArray, @@ -4237,21 +4300,24 @@ func Test_validateMatrix(t *testing.T) { Name: "produce-array-result", Image: "alpine", Script: ` | - echo -n "[\"${params.platform}\",\"${params.browser}\"]" | tee $(results.array-result.path)`}}, + echo -n "[\"${params.platform}\",\"${params.browser}\"]" | tee $(results.array-result.path)`, + }}, }}, Matrix: &Matrix{ Params: Params{{ Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac"}}, }, { Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "safari"}}, - }}}, + }}, + }, }, { Name: "task-consuming-results", TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ Params: ParamSpecs{{ Name: "platform", }, { - Name: "browser"}}, + Name: "browser", + }}, Results: []TaskResult{{ Name: "report-url", Type: ResultsTypeString, @@ -4260,7 +4326,8 @@ func Test_validateMatrix(t *testing.T) { Name: "produce-report-url", Image: "alpine", Script: ` | - echo -n "https://api.example/get-report/$(params.platform)-$(params.browser)" | tee $(results.report-url.path)`}}, + echo -n "https://api.example/get-report/$(params.platform)-$(params.browser)" | tee $(results.report-url.path)`, + }}, }}, Params: Params{{ Name: "b-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(tasks.matrix-emitting-results-embedded.results.report-url[0])"}, @@ -4277,14 +4344,16 @@ func Test_validateMatrix(t *testing.T) { Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac"}}, }, { Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "safari"}}, - }}}, + }}, + }, }, { Name: "taskwithresult", TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ Params: ParamSpecs{{ Name: "platform", }, { - Name: "browser"}}, + Name: "browser", + }}, Results: []TaskResult{{ Name: "array-result", Type: ResultsTypeArray, @@ -4293,7 +4362,8 @@ func Test_validateMatrix(t *testing.T) { Name: "produce-array-result", Image: "alpine", Script: ` | - echo -n "[\"${params.platform}\",\"${params.browser}\"]" | tee $(results.array-result.path)`}}, + echo -n "[\"${params.platform}\",\"${params.browser}\"]" | tee $(results.array-result.path)`, + }}, }}, }, { Name: "task-consuming-results", @@ -4333,22 +4403,6 @@ func getTaskSpec() TaskSpec { } } -func enableFeatures(t *testing.T, features []string) func(context.Context) context.Context { - t.Helper() - return func(ctx context.Context) context.Context { - s := config.NewStore(logtesting.TestLogger(t)) - data := make(map[string]string) - for _, f := range features { - data[f] = "true" - } - s.OnConfigChanged(&corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName()}, - Data: data, - }) - return s.ToContext(ctx) - } -} - // TestPipelineWithBetaFields tests the beta API-driven features of // PipelineSpec are correctly governed `enable-api-fields`, which must // be set to "alpha" or "beta". @@ -4418,142 +4472,143 @@ func TestGetIndexingReferencesToArrayParams(t *testing.T) { name string spec PipelineSpec want sets.String - }{{ - name: "references in task params", - spec: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeString}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[1])")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("$(params.second-param[0])")}, - {Name: "first-task-third-param", Value: *NewStructuredValues("static value")}, + }{ + { + name: "references in task params", + spec: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeString}, }, - }}, - }, - want: sets.NewString("$(params.first-param[1])", "$(params.second-param[0])"), - }, { - name: "references in when expression", - spec: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeString}, - }, - Tasks: []PipelineTask{{ - WhenExpressions: []WhenExpression{{ - Input: "$(params.first-param[1])", - Operator: selection.In, - Values: []string{"$(params.second-param[0])"}, + Tasks: []PipelineTask{{ + Params: Params{ + {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[1])")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("$(params.second-param[0])")}, + {Name: "first-task-third-param", Value: *NewStructuredValues("static value")}, + }, }}, - }}, - }, - want: sets.NewString("$(params.first-param[1])", "$(params.second-param[0])"), - }, { - name: "nested references in task params", - spec: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("$(input.workspace.$(params.first-param[0]))")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("$(input.workspace.$(params.second-param[1]))")}, + want: sets.NewString("$(params.first-param[1])", "$(params.second-param[0])"), + }, { + name: "references in when expression", + spec: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeString}, }, - }}, - }, - want: sets.NewString("$(params.first-param[0])", "$(params.second-param[1])"), - }, { - name: "array parameter", - spec: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default", "array", "value")}, - {Name: "second-param", Type: ParamTypeArray}, + Tasks: []PipelineTask{{ + WhenExpressions: []WhenExpression{{ + Input: "$(params.first-param[1])", + Operator: selection.In, + Values: []string{"$(params.second-param[0])"}, + }}, + }}, }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("firstelement", "$(params.first-param)")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("firstelement", "$(params.second-param[0])")}, + want: sets.NewString("$(params.first-param[1])", "$(params.second-param[0])"), + }, { + name: "nested references in task params", + spec: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, }, - }}, - }, - want: sets.NewString("$(params.second-param[0])"), - }, { - name: "references in finally params", - spec: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray}, + Tasks: []PipelineTask{{ + Params: Params{ + {Name: "first-task-first-param", Value: *NewStructuredValues("$(input.workspace.$(params.first-param[0]))")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("$(input.workspace.$(params.second-param[1]))")}, + }, + }}, }, - Finally: []PipelineTask{{ - Params: Params{ - {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, - {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[1])")}, + want: sets.NewString("$(params.first-param[0])", "$(params.second-param[1])"), + }, { + name: "array parameter", + spec: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default", "array", "value")}, + {Name: "second-param", Type: ParamTypeArray}, }, - }}, - }, - want: sets.NewString("$(params.first-param[0])", "$(params.second-param[1])"), - }, { - name: "references in finally when expressions", - spec: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray}, + Tasks: []PipelineTask{{ + Params: Params{ + {Name: "first-task-first-param", Value: *NewStructuredValues("firstelement", "$(params.first-param)")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("firstelement", "$(params.second-param[0])")}, + }, + }}, }, - Finally: []PipelineTask{{ - WhenExpressions: WhenExpressions{{ - Input: "$(params.first-param[0])", - Operator: selection.In, - Values: []string{"$(params.second-param[1])"}, + want: sets.NewString("$(params.second-param[0])"), + }, { + name: "references in finally params", + spec: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Finally: []PipelineTask{{ + Params: Params{ + {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, + {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[1])")}, + }, }}, - }}, - }, - want: sets.NewString("$(params.first-param[0])", "$(params.second-param[1])"), - }, { - name: "parameter references with bracket notation and special characters", - spec: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second/param", Type: ParamTypeArray}, - {Name: "third.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "fourth/param", Type: ParamTypeArray}, }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues(`$(params["first.param"][0])`)}, - {Name: "first-task-second-param", Value: *NewStructuredValues(`$(params["first.param"][0])`)}, - {Name: "first-task-third-param", Value: *NewStructuredValues(`$(params['third.param'][1])`)}, - {Name: "first-task-fourth-param", Value: *NewStructuredValues(`$(params['fourth/param'][1])`)}, - {Name: "first-task-fifth-param", Value: *NewStructuredValues("static value")}, + want: sets.NewString("$(params.first-param[0])", "$(params.second-param[1])"), + }, { + name: "references in finally when expressions", + spec: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray}, }, - }}, - }, - want: sets.NewString(`$(params["first.param"][0])`, `$(params["first.param"][0])`, `$(params['third.param'][1])`, `$(params['fourth/param'][1])`), - }, { - name: "single parameter in workspace subpath", - spec: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray}, + Finally: []PipelineTask{{ + WhenExpressions: WhenExpressions{{ + Input: "$(params.first-param[0])", + Operator: selection.In, + Values: []string{"$(params.second-param[1])"}, + }}, + }}, }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("static value")}, + want: sets.NewString("$(params.first-param[0])", "$(params.second-param[1])"), + }, { + name: "parameter references with bracket notation and special characters", + spec: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second/param", Type: ParamTypeArray}, + {Name: "third.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "fourth/param", Type: ParamTypeArray}, }, - Workspaces: []WorkspacePipelineTaskBinding{ - { - Name: "first-workspace", - Workspace: "first-workspace", - SubPath: "$(params.second-param[1])", + Tasks: []PipelineTask{{ + Params: Params{ + {Name: "first-task-first-param", Value: *NewStructuredValues(`$(params["first.param"][0])`)}, + {Name: "first-task-second-param", Value: *NewStructuredValues(`$(params["first.param"][0])`)}, + {Name: "first-task-third-param", Value: *NewStructuredValues(`$(params['third.param'][1])`)}, + {Name: "first-task-fourth-param", Value: *NewStructuredValues(`$(params['fourth/param'][1])`)}, + {Name: "first-task-fifth-param", Value: *NewStructuredValues("static value")}, }, + }}, + }, + want: sets.NewString(`$(params["first.param"][0])`, `$(params["first.param"][0])`, `$(params['third.param'][1])`, `$(params['fourth/param'][1])`), + }, { + name: "single parameter in workspace subpath", + spec: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray}, }, - }}, + Tasks: []PipelineTask{{ + Params: Params{ + {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, + {Name: "first-task-second-param", Value: *NewStructuredValues("static value")}, + }, + Workspaces: []WorkspacePipelineTaskBinding{ + { + Name: "first-workspace", + Workspace: "first-workspace", + SubPath: "$(params.second-param[1])", + }, + }, + }}, + }, + want: sets.NewString("$(params.first-param[0])", "$(params.second-param[1])"), }, - want: sets.NewString("$(params.first-param[0])", "$(params.second-param[1])"), - }, } { tt := tt // capture range variable t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1beta1/pipelineref_conversion.go b/pkg/apis/pipeline/v1beta1/pipelineref_conversion.go index ae794c8b738..efa399b6b85 100644 --- a/pkg/apis/pipeline/v1beta1/pipelineref_conversion.go +++ b/pkg/apis/pipeline/v1beta1/pipelineref_conversion.go @@ -23,14 +23,11 @@ import ( ) func (pr PipelineRef) convertTo(ctx context.Context, sink *v1.PipelineRef) { - if pr.Bundle == "" { - sink.Name = pr.Name - } + sink.Name = pr.Name sink.APIVersion = pr.APIVersion new := v1.ResolverRef{} pr.ResolverRef.convertTo(ctx, &new) sink.ResolverRef = new - pr.convertBundleToResolver(sink) } func (pr *PipelineRef) convertFrom(ctx context.Context, source v1.PipelineRef) { @@ -40,24 +37,3 @@ func (pr *PipelineRef) convertFrom(ctx context.Context, source v1.PipelineRef) { new.convertFrom(ctx, source.ResolverRef) pr.ResolverRef = new } - -// convertBundleToResolver converts v1beta1 bundle string to a remote reference with the bundle resolver in v1. -// The conversion from Resolver to Bundle is not being supported since remote resolution would be turned on by -// default and it will be in beta before the stored version of CRD getting swapped to v1. -func (pr PipelineRef) convertBundleToResolver(sink *v1.PipelineRef) { - if pr.Bundle != "" { - sink.ResolverRef = v1.ResolverRef{ - Resolver: "bundles", - Params: v1.Params{{ - Name: "bundle", - Value: v1.ParamValue{StringVal: pr.Bundle, Type: v1.ParamTypeString}, - }, { - Name: "name", - Value: v1.ParamValue{StringVal: pr.Name, Type: v1.ParamTypeString}, - }, { - Name: "kind", - Value: v1.ParamValue{StringVal: "Pipeline", Type: v1.ParamTypeString}, - }}, - } - } -} diff --git a/pkg/apis/pipeline/v1beta1/pipelineref_types.go b/pkg/apis/pipeline/v1beta1/pipelineref_types.go index ab943a3242f..cf83442be63 100644 --- a/pkg/apis/pipeline/v1beta1/pipelineref_types.go +++ b/pkg/apis/pipeline/v1beta1/pipelineref_types.go @@ -23,9 +23,11 @@ type PipelineRef struct { // API version of the referent // +optional APIVersion string `json:"apiVersion,omitempty"` + // Bundle url reference to a Tekton Bundle. // // Deprecated: Please use ResolverRef with the bundles resolver instead. + // The field is staying there for go client backward compatibility, but is not used/allowed anymore. // +optional Bundle string `json:"bundle,omitempty"` diff --git a/pkg/apis/pipeline/v1beta1/pipelineref_validation.go b/pkg/apis/pipeline/v1beta1/pipelineref_validation.go index b5780112560..efe352d6371 100644 --- a/pkg/apis/pipeline/v1beta1/pipelineref_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipelineref_validation.go @@ -21,7 +21,6 @@ import ( "fmt" "strings" - "github.com/google/go-containerregistry/pkg/name" "github.com/tektoncd/pipeline/pkg/apis/config" "k8s.io/apimachinery/pkg/util/validation" "knative.dev/pkg/apis" @@ -33,6 +32,9 @@ func (ref *PipelineRef) Validate(ctx context.Context) (errs *apis.FieldError) { if ref == nil { return errs } + if apis.IsInCreate(ctx) && ref.Bundle != "" { + errs = errs.Also(apis.ErrDisallowedFields("bundle")) + } switch { case ref.Resolver != "" || ref.Params != nil: if ref.Params != nil { @@ -40,9 +42,6 @@ func (ref *PipelineRef) Validate(ctx context.Context) (errs *apis.FieldError) { if ref.Name != "" { errs = errs.Also(apis.ErrMultipleOneOf("name", "params")) } - if ref.Bundle != "" { - errs = errs.Also(apis.ErrMultipleOneOf("bundle", "params")) - } if ref.Resolver == "" { errs = errs.Also(apis.ErrMissingField("resolver")) } @@ -61,17 +60,6 @@ func (ref *PipelineRef) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(apis.ErrInvalidValue(err, "name")) } } - if ref.Bundle != "" { - errs = errs.Also(apis.ErrMultipleOneOf("bundle", "resolver")) - } - } - case ref.Bundle != "": - if ref.Name == "" { - errs = errs.Also(apis.ErrMissingField("name")) - } - 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())) } case ref.Name != "": // ref name can be a Url-like format. @@ -97,13 +85,3 @@ func (ref *PipelineRef) Validate(ctx context.Context) (errs *apis.FieldError) { } return //nolint:nakedret } - -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 index a5a00617dc1..76e04868366 100644 --- a/pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go @@ -21,14 +21,10 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "github.com/tektoncd/pipeline/pkg/apis/config" cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing" "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) { @@ -38,28 +34,6 @@ func TestPipelineRef_Invalid(t *testing.T) { 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"), @@ -137,22 +111,6 @@ func TestPipelineRef_Invalid(t *testing.T) { wantErr: &apis.FieldError{ Message: `feature flag enable-concise-resolver-syntax should be set to true to use concise resolver syntax`, }, - }, { - name: "pipelineref params disallowed in conjunction with pipelineref bundle", - ref: &v1beta1.PipelineRef{ - Bundle: "bar", - ResolverRef: v1beta1.ResolverRef{ - Params: v1beta1.Params{{ - Name: "foo", - Value: v1beta1.ParamValue{ - Type: v1beta1.ParamTypeString, - StringVal: "bar", - }, - }}, - }, - }, - wantErr: apis.ErrMultipleOneOf("bundle", "params").Also(apis.ErrMissingField("resolver")), - withContext: enableTektonOCIBundles(t), }} for _, tc := range tests { @@ -215,17 +173,3 @@ func TestPipelineRef_Valid(t *testing.T) { }) } } - -func enableTektonOCIBundles(t *testing.T) func(context.Context) context.Context { - t.Helper() - 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_conversion_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_conversion_test.go index 04397c0708b..d0812253a8e 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_conversion_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_conversion_test.go @@ -88,13 +88,14 @@ var ( }, }, }, - Sidecars: []v1beta1.SidecarState{{ContainerState: corev1.ContainerState{ - Terminated: &corev1.ContainerStateTerminated{ - ExitCode: 1, - Reason: "Error", - Message: "Error", + Sidecars: []v1beta1.SidecarState{{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + ExitCode: 1, + Reason: "Error", + Message: "Error", + }, }, - }, Name: "error", ImageID: "image-id", ContainerName: "sidecar-error", @@ -268,17 +269,21 @@ func TestPipelineRunConversion(t *testing.T) { }, HostNetwork: false, }, - StepOverrides: []v1beta1.TaskRunStepOverride{{ - Name: "test-so", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, - }}, + StepOverrides: []v1beta1.TaskRunStepOverride{ + { + Name: "test-so", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, + }, + }, }, - SidecarOverrides: []v1beta1.TaskRunSidecarOverride{{ - Name: "test-so", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, - }}, + SidecarOverrides: []v1beta1.TaskRunSidecarOverride{ + { + Name: "test-so", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, + }, + }, }, Metadata: &v1beta1.PipelineTaskMetadata{ Labels: map[string]string{ @@ -315,7 +320,8 @@ func TestPipelineRunConversion(t *testing.T) { Value: *v1beta1.NewObject(map[string]string{ "pkey1": "val1", "pkey2": "rae", - })}, { + }), + }, { Name: "pipeline-result-2", Value: *v1beta1.NewObject(map[string]string{ "pkey1": "val2", @@ -460,38 +466,6 @@ func TestPipelineRunConversionFromDeprecated(t *testing.T) { }, }, }, - }, { - name: "bundle", - in: &v1beta1.PipelineRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - Namespace: "bar", - }, - Spec: v1beta1.PipelineRunSpec{ - PipelineRef: &v1beta1.PipelineRef{ - Name: "test-bundle-name", - Bundle: "test-bundle", - }, - }, - }, - want: &v1beta1.PipelineRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - Namespace: "bar", - }, - Spec: v1beta1.PipelineRunSpec{ - PipelineRef: &v1beta1.PipelineRef{ - ResolverRef: v1beta1.ResolverRef{ - Resolver: "bundles", - Params: v1beta1.Params{ - {Name: "bundle", Value: v1beta1.ParamValue{StringVal: "test-bundle", Type: "string"}}, - {Name: "name", Value: v1beta1.ParamValue{StringVal: "test-bundle-name", Type: "string"}}, - {Name: "kind", Value: v1beta1.ParamValue{StringVal: "Pipeline", Type: "string"}}, - }, - }, - }, - }, - }, }} for _, test := range tests { versions := []apis.Convertible{&v1.PipelineRun{}} diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go index 15e044591a4..706078d181c 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go @@ -505,6 +505,21 @@ func TestPipelineRun_Invalid(t *testing.T) { }, }, want: &apis.FieldError{Message: "must not set the field(s)", Paths: []string{"spec.resources"}}, + }, { + name: "uses bundle (deprecated) on creation is disallowed", + pr: v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipelinerunname", + }, + Spec: v1beta1.PipelineRunSpec{ + PipelineRef: &v1beta1.PipelineRef{ + Name: "foo", + Bundle: "example.com/foo/bar", + }, + }, + }, + want: &apis.FieldError{Message: "must not set the field(s)", Paths: []string{"spec.pipelineRef.bundle"}}, + wc: apis.WithinCreate, }} for _, tc := range tests { @@ -944,17 +959,21 @@ func TestPipelineRun_Validate(t *testing.T) { TaskRunSpecs: []v1beta1.PipelineTaskRunSpec{ { PipelineTaskName: "bar", - StepOverrides: []v1beta1.TaskRunStepOverride{{ - Name: "task-1", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, - }}, + StepOverrides: []v1beta1.TaskRunStepOverride{ + { + Name: "task-1", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, + }, + }, }, - SidecarOverrides: []v1beta1.TaskRunSidecarOverride{{ - Name: "task-1", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, - }}, + SidecarOverrides: []v1beta1.TaskRunSidecarOverride{ + { + Name: "task-1", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, + }, + }, }, }, }, @@ -994,7 +1013,8 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) { TaskRef: &v1beta1.TaskRef{ Name: "mytask", }, - }}}, + }}, + }, }, wantErr: apis.ErrMultipleOneOf("pipelineRef", "pipelineSpec"), }, { @@ -1095,11 +1115,13 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) { TaskRunSpecs: []v1beta1.PipelineTaskRunSpec{ { PipelineTaskName: "bar", - StepOverrides: []v1beta1.TaskRunStepOverride{{ - Name: "task-1", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, - }}, + StepOverrides: []v1beta1.TaskRunStepOverride{ + { + Name: "task-1", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, + }, + }, }, }, }, @@ -1113,11 +1135,13 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) { TaskRunSpecs: []v1beta1.PipelineTaskRunSpec{ { PipelineTaskName: "bar", - SidecarOverrides: []v1beta1.TaskRunSidecarOverride{{ - Name: "task-1", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, - }}, + SidecarOverrides: []v1beta1.TaskRunSidecarOverride{ + { + Name: "task-1", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, + }, + }, }, }, }, @@ -1131,10 +1155,12 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) { TaskRunSpecs: []v1beta1.PipelineTaskRunSpec{ { PipelineTaskName: "bar", - StepOverrides: []v1beta1.TaskRunStepOverride{{ - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, - }}, + StepOverrides: []v1beta1.TaskRunStepOverride{ + { + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, + }, + }, }, }, }, @@ -1163,10 +1189,12 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) { TaskRunSpecs: []v1beta1.PipelineTaskRunSpec{ { PipelineTaskName: "bar", - SidecarOverrides: []v1beta1.TaskRunSidecarOverride{{ - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, - }}, + SidecarOverrides: []v1beta1.TaskRunSidecarOverride{ + { + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, + }, + }, }, }, }, @@ -1180,11 +1208,13 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) { TaskRunSpecs: []v1beta1.PipelineTaskRunSpec{ { PipelineTaskName: "pipelineTask", - StepOverrides: []v1beta1.TaskRunStepOverride{{ - Name: "stepOverride", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, - }}, + StepOverrides: []v1beta1.TaskRunStepOverride{ + { + Name: "stepOverride", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, + }, + }, }, ComputeResources: &corev1.ResourceRequirements{ Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("2Gi")}, diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 91cf8fe17fd..c5eb4db1a56 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -829,7 +829,7 @@ "type": "string" }, "bundle": { - "description": "Bundle url reference to a Tekton Bundle.\n\nDeprecated: Please use ResolverRef with the bundles resolver instead.", + "description": "Bundle url reference to a Tekton Bundle.\n\nDeprecated: Please use ResolverRef with the bundles resolver instead. The field is staying there for go client backward compatibility, but is not used/allowed anymore.", "type": "string" }, "name": { @@ -2668,7 +2668,7 @@ "type": "string" }, "bundle": { - "description": "Bundle url reference to a Tekton Bundle.\n\nDeprecated: Please use ResolverRef with the bundles resolver instead.", + "description": "Bundle url reference to a Tekton Bundle.\n\nDeprecated: Please use ResolverRef with the bundles resolver instead. The field is staying there for go client backward compatibility, but is not used/allowed anymore.", "type": "string" }, "kind": { diff --git a/pkg/apis/pipeline/v1beta1/taskref_conversion.go b/pkg/apis/pipeline/v1beta1/taskref_conversion.go index c089e9e0dbf..1b163970b31 100644 --- a/pkg/apis/pipeline/v1beta1/taskref_conversion.go +++ b/pkg/apis/pipeline/v1beta1/taskref_conversion.go @@ -23,15 +23,12 @@ import ( ) func (tr TaskRef) convertTo(ctx context.Context, sink *v1.TaskRef) { - if tr.Bundle == "" { - sink.Name = tr.Name - } + sink.Name = tr.Name sink.Kind = v1.TaskKind(tr.Kind) sink.APIVersion = tr.APIVersion new := v1.ResolverRef{} tr.ResolverRef.convertTo(ctx, &new) sink.ResolverRef = new - tr.convertBundleToResolver(sink) } // ConvertFrom converts v1beta1 TaskRef from v1 TaskRef @@ -43,25 +40,3 @@ func (tr *TaskRef) ConvertFrom(ctx context.Context, source v1.TaskRef) { new.convertFrom(ctx, source.ResolverRef) tr.ResolverRef = new } - -// convertBundleToResolver converts v1beta1 bundle string to a remote reference with the bundle resolver in v1. -// The conversion from Resolver to Bundle is not being supported since remote resolution would be turned on by -// default and it will be in beta before the stored version of CRD getting swapped to v1. -func (tr TaskRef) convertBundleToResolver(sink *v1.TaskRef) { - if tr.Bundle != "" { - sink.Kind = "" - sink.ResolverRef = v1.ResolverRef{ - Resolver: "bundles", - Params: v1.Params{{ - Name: "bundle", - Value: v1.ParamValue{StringVal: tr.Bundle, Type: v1.ParamTypeString}, - }, { - Name: "name", - Value: v1.ParamValue{StringVal: tr.Name, Type: v1.ParamTypeString}, - }, { - Name: "kind", - Value: v1.ParamValue{StringVal: "Task", Type: v1.ParamTypeString}, - }}, - } - } -} diff --git a/pkg/apis/pipeline/v1beta1/taskref_types.go b/pkg/apis/pipeline/v1beta1/taskref_types.go index f8f231cd961..9781a4a2133 100644 --- a/pkg/apis/pipeline/v1beta1/taskref_types.go +++ b/pkg/apis/pipeline/v1beta1/taskref_types.go @@ -32,6 +32,7 @@ type TaskRef struct { // Bundle url reference to a Tekton Bundle. // // Deprecated: Please use ResolverRef with the bundles resolver instead. + // The field is staying there for go client backward compatibility, but is not used/allowed anymore. // +optional Bundle string `json:"bundle,omitempty"` diff --git a/pkg/apis/pipeline/v1beta1/taskref_validation.go b/pkg/apis/pipeline/v1beta1/taskref_validation.go index 7db46c0d9c5..866dfadd27d 100644 --- a/pkg/apis/pipeline/v1beta1/taskref_validation.go +++ b/pkg/apis/pipeline/v1beta1/taskref_validation.go @@ -21,7 +21,6 @@ import ( "fmt" "strings" - "github.com/google/go-containerregistry/pkg/name" "github.com/tektoncd/pipeline/pkg/apis/config" "k8s.io/apimachinery/pkg/util/validation" "knative.dev/pkg/apis" @@ -33,6 +32,9 @@ func (ref *TaskRef) Validate(ctx context.Context) (errs *apis.FieldError) { if ref == nil { return errs } + if apis.IsInCreate(ctx) && ref.Bundle != "" { + errs = errs.Also(apis.ErrDisallowedFields("bundle")) + } switch { case ref.Resolver != "" || ref.Params != nil: if ref.Params != nil { @@ -40,9 +42,6 @@ func (ref *TaskRef) Validate(ctx context.Context) (errs *apis.FieldError) { if ref.Name != "" { errs = errs.Also(apis.ErrMultipleOneOf("name", "params")) } - if ref.Bundle != "" { - errs = errs.Also(apis.ErrMultipleOneOf("bundle", "params")) - } if ref.Resolver == "" { errs = errs.Also(apis.ErrMissingField("resolver")) } @@ -61,17 +60,6 @@ func (ref *TaskRef) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(apis.ErrInvalidValue(err, "name")) } } - if ref.Bundle != "" { - errs = errs.Also(apis.ErrMultipleOneOf("bundle", "resolver")) - } - } - case ref.Bundle != "": - if ref.Name == "" { - errs = errs.Also(apis.ErrMissingField("name")) - } - 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())) } case ref.Name != "": // ref name can be a Url-like format. diff --git a/pkg/apis/pipeline/v1beta1/taskref_validation_test.go b/pkg/apis/pipeline/v1beta1/taskref_validation_test.go index 09136f812c3..41f5e43b0f2 100644 --- a/pkg/apis/pipeline/v1beta1/taskref_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/taskref_validation_test.go @@ -60,12 +60,6 @@ func TestTaskRef_Valid(t *testing.T) { StringVal: "baz", }, }}}}, - }, { - name: "valid bundle", - taskRef: &v1beta1.TaskRef{ - Name: "bundled-task", - Bundle: "gcr.io/my-bundle"}, - wc: enableTektonOCIBundles(t), }} for _, ts := range tests { t.Run(ts.name, func(t *testing.T) { @@ -90,28 +84,6 @@ func TestTaskRef_Invalid(t *testing.T) { 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 with resolver and k8s style name", taskRef: &v1beta1.TaskRef{ @@ -152,32 +124,6 @@ func TestTaskRef_Invalid(t *testing.T) { wantErr: &apis.FieldError{ Message: `feature flag enable-concise-resolver-syntax should be set to true to use concise resolver syntax`, }, - }, { - 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: enableTektonOCIBundles(t), - }, { - name: "taskref params disallowed in conjunction with taskref bundle", - taskRef: &v1beta1.TaskRef{ - Bundle: "bar", - ResolverRef: v1beta1.ResolverRef{ - Params: v1beta1.Params{{ - Name: "foo", - Value: v1beta1.ParamValue{ - Type: v1beta1.ParamTypeString, - StringVal: "bar", - }, - }}, - }, - }, - wantErr: apis.ErrMultipleOneOf("bundle", "params").Also(apis.ErrMissingField("resolver")), - wc: enableTektonOCIBundles(t), }, { name: "invalid taskref name", taskRef: &v1beta1.TaskRef{Name: "_foo"}, diff --git a/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go b/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go index b9e1565dda9..9bf4eae0cda 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go @@ -50,296 +50,307 @@ func TestTaskRunConversion(t *testing.T) { tests := []struct { name string in *v1beta1.TaskRun - }{{ - name: "simple taskrun", - in: &v1beta1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - Namespace: "bar", - }, - Spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{Name: "test-task"}, - }, - }, - }, { - name: "taskrun conversion deprecated step fields", - in: &v1beta1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - Namespace: "bar", - }, - Spec: v1beta1.TaskRunSpec{ - TaskSpec: &v1beta1.TaskSpec{ - Steps: []v1beta1.Step{{ - DeprecatedLivenessProbe: &corev1.Probe{InitialDelaySeconds: 1}, - DeprecatedReadinessProbe: &corev1.Probe{InitialDelaySeconds: 2}, - DeprecatedPorts: []corev1.ContainerPort{{Name: "port"}}, - DeprecatedStartupProbe: &corev1.Probe{InitialDelaySeconds: 3}, - DeprecatedLifecycle: &corev1.Lifecycle{PostStart: &corev1.LifecycleHandler{Exec: &corev1.ExecAction{ - Command: []string{"lifecycle command"}, - }}}, - DeprecatedTerminationMessagePath: "path", - DeprecatedTerminationMessagePolicy: corev1.TerminationMessagePolicy("policy"), - DeprecatedStdin: true, - DeprecatedStdinOnce: true, - DeprecatedTTY: true, - }}, - StepTemplate: &v1beta1.StepTemplate{ - DeprecatedName: "name", - DeprecatedLivenessProbe: &corev1.Probe{InitialDelaySeconds: 1}, - DeprecatedReadinessProbe: &corev1.Probe{InitialDelaySeconds: 2}, - DeprecatedPorts: []corev1.ContainerPort{{Name: "port"}}, - DeprecatedStartupProbe: &corev1.Probe{InitialDelaySeconds: 3}, - DeprecatedLifecycle: &corev1.Lifecycle{PostStart: &corev1.LifecycleHandler{Exec: &corev1.ExecAction{ - Command: []string{"lifecycle command"}, - }}}, - DeprecatedTerminationMessagePath: "path", - DeprecatedTerminationMessagePolicy: corev1.TerminationMessagePolicy("policy"), - DeprecatedStdin: true, - DeprecatedStdinOnce: true, - DeprecatedTTY: true, + }{ + { + name: "simple taskrun", + in: &v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: v1beta1.TaskRunSpec{ + TaskRef: &v1beta1.TaskRef{Name: "test-task"}, + }, + }, + }, { + name: "taskrun conversion deprecated step fields", + in: &v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: v1beta1.TaskRunSpec{ + TaskSpec: &v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{ + DeprecatedLivenessProbe: &corev1.Probe{InitialDelaySeconds: 1}, + DeprecatedReadinessProbe: &corev1.Probe{InitialDelaySeconds: 2}, + DeprecatedPorts: []corev1.ContainerPort{{Name: "port"}}, + DeprecatedStartupProbe: &corev1.Probe{InitialDelaySeconds: 3}, + DeprecatedLifecycle: &corev1.Lifecycle{PostStart: &corev1.LifecycleHandler{Exec: &corev1.ExecAction{ + Command: []string{"lifecycle command"}, + }}}, + DeprecatedTerminationMessagePath: "path", + DeprecatedTerminationMessagePolicy: corev1.TerminationMessagePolicy("policy"), + DeprecatedStdin: true, + DeprecatedStdinOnce: true, + DeprecatedTTY: true, + }}, + StepTemplate: &v1beta1.StepTemplate{ + DeprecatedName: "name", + DeprecatedLivenessProbe: &corev1.Probe{InitialDelaySeconds: 1}, + DeprecatedReadinessProbe: &corev1.Probe{InitialDelaySeconds: 2}, + DeprecatedPorts: []corev1.ContainerPort{{Name: "port"}}, + DeprecatedStartupProbe: &corev1.Probe{InitialDelaySeconds: 3}, + DeprecatedLifecycle: &corev1.Lifecycle{PostStart: &corev1.LifecycleHandler{Exec: &corev1.ExecAction{ + Command: []string{"lifecycle command"}, + }}}, + DeprecatedTerminationMessagePath: "path", + DeprecatedTerminationMessagePolicy: corev1.TerminationMessagePolicy("policy"), + DeprecatedStdin: true, + DeprecatedStdinOnce: true, + DeprecatedTTY: true, + }, }, }, }, - }, - }, { - name: "taskrun with step Results in step state", - in: &v1beta1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - Namespace: "bar", - }, - Spec: v1beta1.TaskRunSpec{}, - Status: v1beta1.TaskRunStatus{ - TaskRunStatusFields: v1beta1.TaskRunStatusFields{ - Steps: []v1beta1.StepState{{ - Results: []v1beta1.TaskRunStepResult{{ - Name: "foo", - Type: v1beta1.ResultsTypeString, - Value: v1beta1.ResultValue{ - Type: v1beta1.ParamTypeString, - StringVal: "bar", - }, + }, { + name: "taskrun with step Results in step state", + in: &v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: v1beta1.TaskRunSpec{}, + Status: v1beta1.TaskRunStatus{ + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + Steps: []v1beta1.StepState{{ + Results: []v1beta1.TaskRunStepResult{{ + Name: "foo", + Type: v1beta1.ResultsTypeString, + Value: v1beta1.ResultValue{ + Type: v1beta1.ParamTypeString, + StringVal: "bar", + }, + }}, }}, - }}, - }, - }, - }, - }, { - name: "taskrun conversion all non deprecated fields", - in: &v1beta1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - Namespace: "bar", - }, - Spec: v1beta1.TaskRunSpec{ - Debug: &v1beta1.TaskRunDebug{ - Breakpoints: &v1beta1.TaskBreakpoints{ - OnFailure: "enabled", }, }, - Params: v1beta1.Params{{ - Name: "param-task-1", - Value: v1beta1.ParamValue{ - ArrayVal: []string{"value-task-1"}, - Type: "string", - }, - }}, - ServiceAccountName: "test-sa", - TaskRef: &v1beta1.TaskRef{Name: "test-task"}, - TaskSpec: &v1beta1.TaskSpec{ - Params: []v1beta1.ParamSpec{{ - Name: "param-name", - Type: "string", - }}, + }, + }, { + name: "taskrun conversion all non deprecated fields", + in: &v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", }, - Status: "test-task-run-spec-status", - StatusMessage: v1beta1.TaskRunSpecStatusMessage("test-status-message"), - Timeout: &metav1.Duration{Duration: 5 * time.Second}, - PodTemplate: &pod.Template{ - NodeSelector: map[string]string{ - "label": "value", + Spec: v1beta1.TaskRunSpec{ + Debug: &v1beta1.TaskRunDebug{ + Breakpoints: &v1beta1.TaskBreakpoints{ + OnFailure: "enabled", + }, }, - }, - Workspaces: []v1beta1.WorkspaceBinding{{ - Name: "workspace-volumeclaimtemplate", - SubPath: "/foo/bar/baz", - VolumeClaimTemplate: &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pvc", + Params: v1beta1.Params{{ + Name: "param-task-1", + Value: v1beta1.ParamValue{ + ArrayVal: []string{"value-task-1"}, + Type: "string", }, - Spec: corev1.PersistentVolumeClaimSpec{}, + }}, + ServiceAccountName: "test-sa", + TaskRef: &v1beta1.TaskRef{Name: "test-task"}, + TaskSpec: &v1beta1.TaskSpec{ + Params: []v1beta1.ParamSpec{{ + Name: "param-name", + Type: "string", + }}, }, - }, { - Name: "workspace-pvc", - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{}, - }, { - Name: "workspace-emptydir", - EmptyDir: &corev1.EmptyDirVolumeSource{}, - }, { - Name: "workspace-configmap", - ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "configbar", + Status: "test-task-run-spec-status", + StatusMessage: v1beta1.TaskRunSpecStatusMessage("test-status-message"), + Timeout: &metav1.Duration{Duration: 5 * time.Second}, + PodTemplate: &pod.Template{ + NodeSelector: map[string]string{ + "label": "value", }, }, - }, { - Name: "workspace-secret", - Secret: &corev1.SecretVolumeSource{SecretName: "sname"}, - }, { - Name: "workspace-projected", - Projected: &corev1.ProjectedVolumeSource{ - Sources: []corev1.VolumeProjection{{ - ConfigMap: &corev1.ConfigMapProjection{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "projected-configmap", + Workspaces: []v1beta1.WorkspaceBinding{ + { + Name: "workspace-volumeclaimtemplate", + SubPath: "/foo/bar/baz", + VolumeClaimTemplate: &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pvc", }, + Spec: corev1.PersistentVolumeClaimSpec{}, }, - Secret: &corev1.SecretProjection{ + }, { + Name: "workspace-pvc", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{}, + }, { + Name: "workspace-emptydir", + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, { + Name: "workspace-configmap", + ConfigMap: &corev1.ConfigMapVolumeSource{ LocalObjectReference: corev1.LocalObjectReference{ - Name: "projected-secret", + Name: "configbar", }, }, - ServiceAccountToken: &corev1.ServiceAccountTokenProjection{ - Audience: "projected-sat", + }, { + Name: "workspace-secret", + Secret: &corev1.SecretVolumeSource{SecretName: "sname"}, + }, { + Name: "workspace-projected", + Projected: &corev1.ProjectedVolumeSource{ + Sources: []corev1.VolumeProjection{{ + ConfigMap: &corev1.ConfigMapProjection{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "projected-configmap", + }, + }, + Secret: &corev1.SecretProjection{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "projected-secret", + }, + }, + ServiceAccountToken: &corev1.ServiceAccountTokenProjection{ + Audience: "projected-sat", + }, + }}, + }, + }, { + Name: "workspace-csi", + CSI: &corev1.CSIVolumeSource{ + NodePublishSecretRef: &corev1.LocalObjectReference{ + Name: "projected-csi", + }, + VolumeAttributes: map[string]string{"key": "attribute-val"}, }, - }}, - }, - }, { - Name: "workspace-csi", - CSI: &corev1.CSIVolumeSource{ - NodePublishSecretRef: &corev1.LocalObjectReference{ - Name: "projected-csi", }, - VolumeAttributes: map[string]string{"key": "attribute-val"}, }, - }, - }, - StepOverrides: []v1beta1.TaskRunStepOverride{{ - Name: "task-1", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, - }}, - }, - SidecarOverrides: []v1beta1.TaskRunSidecarOverride{{ - Name: "task-1", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, - }}, - }, - ComputeResources: &corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceMemory: corev1resources.MustParse("1Gi"), + StepOverrides: []v1beta1.TaskRunStepOverride{ + { + Name: "task-1", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, + }, + }, }, - }, - }, - Status: v1beta1.TaskRunStatus{ - Status: duckv1.Status{ - Conditions: []apis.Condition{ + SidecarOverrides: []v1beta1.TaskRunSidecarOverride{ { - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - Reason: "Completed", - Message: "All tasks finished running", + Name: "task-1", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")}, + }, + }, + }, + ComputeResources: &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: corev1resources.MustParse("1Gi"), }, }, - ObservedGeneration: 1, }, - TaskRunStatusFields: v1beta1.TaskRunStatusFields{ - PodName: "pod-name", - StartTime: &metav1.Time{Time: time.Now()}, - CompletionTime: &metav1.Time{Time: time.Now().Add(1 * time.Minute)}, - Steps: []v1beta1.StepState{{ - ContainerState: corev1.ContainerState{ - Terminated: &corev1.ContainerStateTerminated{ - ExitCode: 123, - }}, + Status: v1beta1.TaskRunStatus{ + Status: duckv1.Status{ + Conditions: []apis.Condition{ + { + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + Reason: "Completed", + Message: "All tasks finished running", + }, + }, + ObservedGeneration: 1, + }, + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + PodName: "pod-name", + StartTime: &metav1.Time{Time: time.Now()}, + CompletionTime: &metav1.Time{Time: time.Now().Add(1 * time.Minute)}, + Steps: []v1beta1.StepState{{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + ExitCode: 123, + }, + }, - Name: "failure", - ContainerName: "step-failure", - ImageID: "image-id", - }}, - Sidecars: []v1beta1.SidecarState{{ - ContainerState: corev1.ContainerState{ - Terminated: &corev1.ContainerStateTerminated{ - ExitCode: 123, - }}, + Name: "failure", + ContainerName: "step-failure", + ImageID: "image-id", + }}, + Sidecars: []v1beta1.SidecarState{{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + ExitCode: 123, + }, + }, - Name: "failure", - ContainerName: "step-failure", - ImageID: "image-id", - }}, - RetriesStatus: []v1beta1.TaskRunStatus{{ - Status: duckv1.Status{ - Conditions: []apis.Condition{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionFalse, - }}, - }, - }}, - TaskRunResults: []v1beta1.TaskRunResult{{ - Name: "resultName", - Type: v1beta1.ResultsTypeObject, - Value: *v1beta1.NewObject(map[string]string{"hello": "world"}), - }}, - TaskSpec: &v1beta1.TaskSpec{ - Description: "test", - Steps: []v1beta1.Step{{ - Image: "foo", + Name: "failure", + ContainerName: "step-failure", + ImageID: "image-id", }}, - Volumes: []corev1.Volume{{}}, - Params: []v1beta1.ParamSpec{{ - Name: "param-1", - Type: v1beta1.ParamTypeString, - Description: "My first param", + RetriesStatus: []v1beta1.TaskRunStatus{{ + Status: duckv1.Status{ + Conditions: []apis.Condition{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionFalse, + }}, + }, }}, - }, - Provenance: &v1beta1.Provenance{ - RefSource: &v1beta1.RefSource{ - URI: "test-uri", - Digest: map[string]string{"sha256": "digest"}, + TaskRunResults: []v1beta1.TaskRunResult{{ + Name: "resultName", + Type: v1beta1.ResultsTypeObject, + Value: *v1beta1.NewObject(map[string]string{"hello": "world"}), + }}, + TaskSpec: &v1beta1.TaskSpec{ + Description: "test", + Steps: []v1beta1.Step{{ + Image: "foo", + }}, + Volumes: []corev1.Volume{{}}, + Params: []v1beta1.ParamSpec{{ + Name: "param-1", + Type: v1beta1.ParamTypeString, + Description: "My first param", + }}, }, - FeatureFlags: config.DefaultFeatureFlags.DeepCopy(), - }}, - }, - }, - }, { - name: "taskrun with stepArtifacts in step state", - in: &v1beta1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - Namespace: "bar", + Provenance: &v1beta1.Provenance{ + RefSource: &v1beta1.RefSource{ + URI: "test-uri", + Digest: map[string]string{"sha256": "digest"}, + }, + FeatureFlags: config.DefaultFeatureFlags.DeepCopy(), + }, + }, + }, }, - Spec: v1beta1.TaskRunSpec{}, - Status: v1beta1.TaskRunStatus{ - TaskRunStatusFields: v1beta1.TaskRunStatusFields{ - Steps: []v1beta1.StepState{{ - Inputs: []v1beta1.TaskRunStepArtifact{{ - Name: "Input", - Values: []v1beta1.ArtifactValue{ - {Uri: "git:example.com", - Digest: map[v1beta1.Algorithm]string{ - "sha256": "49149151d283ac77d3fd4594825242f076c999903261bd95f79a8b261811c11a", - "sha1": "22b80854ba81d11d980794952f2343fedf2189d5", + }, { + name: "taskrun with stepArtifacts in step state", + in: &v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: v1beta1.TaskRunSpec{}, + Status: v1beta1.TaskRunStatus{ + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + Steps: []v1beta1.StepState{{ + Inputs: []v1beta1.TaskRunStepArtifact{{ + Name: "Input", + Values: []v1beta1.ArtifactValue{ + { + Uri: "git:example.com", + Digest: map[v1beta1.Algorithm]string{ + "sha256": "49149151d283ac77d3fd4594825242f076c999903261bd95f79a8b261811c11a", + "sha1": "22b80854ba81d11d980794952f2343fedf2189d5", + }, }, }, - }, - }}, - Outputs: []v1beta1.TaskRunStepArtifact{{ - Name: "Output", - Values: []v1beta1.ArtifactValue{ - {Uri: "docker:example.aaa/bbb:latest", - Digest: map[v1beta1.Algorithm]string{ - "sha256": "f05a847a269ccafc90af40ad55aedef62d165227475e4d95ef6812f7c5daa21a", + }}, + Outputs: []v1beta1.TaskRunStepArtifact{{ + Name: "Output", + Values: []v1beta1.ArtifactValue{ + { + Uri: "docker:example.aaa/bbb:latest", + Digest: map[v1beta1.Algorithm]string{ + "sha256": "f05a847a269ccafc90af40ad55aedef62d165227475e4d95ef6812f7c5daa21a", + }, }, }, - }, + }}, }}, - }}, + }, }, }, }, - }, } for _, test := range tests { @@ -510,37 +521,6 @@ func TestTaskRunConversionFromDeprecated(t *testing.T) { }, }, }, - }}, - }, { - name: "bundle", - in: &v1beta1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - Namespace: "bar", - }, - Spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{ - Name: "test-bundle-name", - Bundle: "test-bundle", - }, - }, - }, - want: &v1beta1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - Namespace: "bar", - }, - Spec: v1beta1.TaskRunSpec{ - TaskRef: &v1beta1.TaskRef{ - ResolverRef: v1beta1.ResolverRef{ - Resolver: "bundles", - Params: v1beta1.Params{ - {Name: "bundle", Value: v1beta1.ParamValue{StringVal: "test-bundle", Type: "string"}}, - {Name: "name", Value: v1beta1.ParamValue{StringVal: "test-bundle-name", Type: "string"}}, - {Name: "kind", Value: v1beta1.ParamValue{StringVal: "Task", Type: "string"}}, - }, - }, - }, }, }, }, { @@ -748,7 +728,8 @@ func TestTaskRunConvertTo(t *testing.T) { }}, }, }, - }}} + }, + }} for _, test := range tests { versions := []apis.Convertible{&v1.TaskRun{}} for _, version := range versions { @@ -813,7 +794,8 @@ func TestTaskRunConvertFrom(t *testing.T) { }}, }, }, - }}} + }, + }} for _, test := range tests { versions := []apis.Convertible{&v1beta1.TaskRun{}} for _, version := range versions { diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go index 3bab6c0e77c..e63fdfd21da 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go @@ -143,6 +143,21 @@ func TestTaskRun_Invalidate(t *testing.T) { Message: `missing field(s)`, Paths: []string{"spec.task-words.properties"}, }, + }, { + name: "uses bundle (deprecated) on creation is disallowed", + taskRun: &v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "taskrunrunname", + }, + Spec: v1beta1.TaskRunSpec{ + TaskRef: &v1beta1.TaskRef{ + Name: "foo", + Bundle: "example.com/foo/bar", + }, + }, + }, + want: &apis.FieldError{Message: "must not set the field(s)", Paths: []string{"spec.taskRef.bundle"}}, + wc: apis.WithinCreate, }} for _, ts := range tests { t.Run(ts.name, func(t *testing.T) { diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index e945a7d1fe3..22a39c39c4c 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -1695,7 +1695,6 @@ status: provenance: featureFlags: RunningInEnvWithInjectedSidecars: true - EnableTektonOCIBundles: true EnforceNonfalsifiability: "none" EnableAPIFields: "alpha" AwaitSidecarReadiness: true @@ -1708,7 +1707,6 @@ status: provenance: featureFlags: RunningInEnvWithInjectedSidecars: true - EnableTektonOCIBundles: true EnableAPIFields: "alpha" EnforceNonfalsifiability: "none" AwaitSidecarReadiness: true diff --git a/test/conversion_test.go b/test/conversion_test.go index 489fad5aced..a8031cb36f3 100644 --- a/test/conversion_test.go +++ b/test/conversion_test.go @@ -33,21 +33,17 @@ import ( ) var ( - filterLabels = cmpopts.IgnoreFields(metav1.ObjectMeta{}, "Labels") - filterAnnotations = cmpopts.IgnoreFields(metav1.ObjectMeta{}, "Annotations") - filterV1TaskRunStatus = cmpopts.IgnoreFields(v1.TaskRunStatusFields{}, "StartTime", "CompletionTime") - filterV1PipelineRunStatus = cmpopts.IgnoreFields(v1.PipelineRunStatusFields{}, "StartTime", "CompletionTime") - filterV1beta1TaskRunStatus = cmpopts.IgnoreFields(v1beta1.TaskRunStatusFields{}, "StartTime", "CompletionTime") - filterV1beta1PipelineRunStatus = cmpopts.IgnoreFields(v1beta1.PipelineRunStatusFields{}, "StartTime", "CompletionTime") - filterContainerStateTerminated = cmpopts.IgnoreFields(corev1.ContainerStateTerminated{}, "StartedAt", "FinishedAt", "ContainerID", "Message") - filterV1StepState = cmpopts.IgnoreFields(v1.StepState{}, "Name", "ImageID", "Container") - filterV1beta1StepState = cmpopts.IgnoreFields(v1beta1.StepState{}, "Name", "ImageID", "ContainerName") - filterV1TaskRunSA = cmpopts.IgnoreFields(v1.TaskRunSpec{}, "ServiceAccountName") - filterV1beta1TaskRunSA = cmpopts.IgnoreFields(v1beta1.TaskRunSpec{}, "ServiceAccountName") - filterV1PipelineRunSA = cmpopts.IgnoreFields(v1.PipelineTaskRunTemplate{}, "ServiceAccountName") - filterV1beta1PipelineRunSA = cmpopts.IgnoreFields(v1beta1.PipelineRunSpec{}, "ServiceAccountName") - filterV1RefSourceImageDigest = cmpopts.IgnoreFields(v1.RefSource{}, "Digest") - filterV1beta1RefSourceImageDigest = cmpopts.IgnoreFields(v1beta1.RefSource{}, "Digest") + filterLabels = cmpopts.IgnoreFields(metav1.ObjectMeta{}, "Labels") + filterAnnotations = cmpopts.IgnoreFields(metav1.ObjectMeta{}, "Annotations") + filterV1TaskRunStatus = cmpopts.IgnoreFields(v1.TaskRunStatusFields{}, "StartTime", "CompletionTime") + filterV1PipelineRunStatus = cmpopts.IgnoreFields(v1.PipelineRunStatusFields{}, "StartTime", "CompletionTime") + filterV1beta1TaskRunStatus = cmpopts.IgnoreFields(v1beta1.TaskRunStatusFields{}, "StartTime", "CompletionTime") + filterV1beta1PipelineRunStatus = cmpopts.IgnoreFields(v1beta1.PipelineRunStatusFields{}, "StartTime", "CompletionTime") + filterContainerStateTerminated = cmpopts.IgnoreFields(corev1.ContainerStateTerminated{}, "StartedAt", "FinishedAt", "ContainerID", "Message") + filterV1StepState = cmpopts.IgnoreFields(v1.StepState{}, "Name", "ImageID", "Container") + filterV1beta1StepState = cmpopts.IgnoreFields(v1beta1.StepState{}, "Name", "ImageID", "ContainerName") + filterV1TaskRunSA = cmpopts.IgnoreFields(v1.TaskRunSpec{}, "ServiceAccountName") + filterV1PipelineRunSA = cmpopts.IgnoreFields(v1.PipelineTaskRunTemplate{}, "ServiceAccountName") filterMetadata = []cmp.Option{filterTypeMeta, filterObjectMeta, filterAnnotations} filterV1TaskRunFields = []cmp.Option{filterTypeMeta, filterObjectMeta, filterLabels, filterAnnotations, filterCondition, filterV1TaskRunStatus, filterContainerStateTerminated, filterV1StepState} @@ -654,202 +650,6 @@ status: name: %s-hello-task pipelineTaskName: hello-task ` - - v1beta1TaskWithBundleYaml = ` -metadata: - name: %s - namespace: %s -spec: - steps: - - name: hello - image: alpine - script: 'echo Hello' -` - - v1beta1PipelineWithBundleYaml = ` -metadata: - name: %s - namespace: %s -spec: - tasks: - - name: hello-world - taskRef: - resolver: bundles - params: - - name: bundle - value: %s - - name: name - value: %s -` - - v1beta1TaskRunWithBundleYaml = ` -metadata: - name: %s - namespace: %s -spec: - taskRef: - name: %s - bundle: %s -` - - v1beta1PipelineRunWithBundleYaml = ` -metadata: - name: %s - namespace: %s -spec: - pipelineRef: - name: %s - bundle: %s -` - - v1TaskRunWithBundleExpectedYaml = ` -metadata: - name: %s - namespace: %s -spec: - serviceAccountName: default - timeout: 1h - taskRef: - resolver: bundles - params: - - name: bundle - value: %s - - name: name - value: %s - - name: kind - value: Task -status: - conditions: - - type: Succeeded - status: "True" - reason: "Succeeded" - podName: %s-pod - taskSpec: - steps: - - computeResources: {} - image: alpine - name: hello - script: 'echo Hello' - steps: - - image: alpine - name: hello - script: 'echo Hello' - terminationReason: Completed - terminated: - reason: Completed -` - - v1PipelineRunWithBundleExpectedYaml = ` -metadata: - name: %s - namespace: %s -spec: - taskRunTemplate: - timeouts: - pipeline: 1h - pipelineRef: - resolver: bundles - params: - - name: bundle - value: %s - - name: name - value: %s - - name: kind - value: Pipeline -status: - conditions: - - type: Succeeded - status: "True" - reason: "Succeeded" - pipelineSpec: - tasks: - - name: hello-world - taskRef: - resolver: bundles - params: - - name: bundle - value: %s - - name: name - value: %s - childReferences: - - apiVersion: tekton.dev/v1 - kind: TaskRun - name: %s-hello-world - pipelineTaskName: hello-world -` - - v1beta1TaskRunWithBundleRoundTripYaml = ` -metadata: - name: %s - namespace: %s -spec: - timeout: 1h - taskRef: - resolver: bundles - params: - - name: bundle - value: %s - - name: name - value: %s - - name: kind - value: Task -status: - conditions: - - type: Succeeded - status: "True" - reason: "Succeeded" - podName: %s-pod - taskSpec: - steps: - - computeResources: {} - image: alpine - name: hello - script: 'echo Hello' - steps: - - image: alpine - name: hello - script: 'echo Hello' - terminated: - reason: Completed -` - - v1beta1PipelineRunWithBundleRoundTripYaml = ` -metadata: - name: %s - namespace: %s -spec: - timeouts: - pipeline: 1h - pipelineRef: - resolver: bundles - params: - - name: bundle - value: %s - - name: name - value: %s - - name: kind - value: Pipeline -status: - conditions: - - type: Succeeded - status: "True" - reason: "Succeeded" - pipelineSpec: - tasks: - - name: hello-world - taskRef: - resolver: bundles - params: - - name: bundle - value: %s - - name: name - value: %s - childReferences: - - apiVersion: tekton.dev/v1 - kind: TaskRun - name: %s-hello-world - pipelineTaskName: hello-world -` ) // TestTaskCRDConversion first creates a v1beta1 Task CRD using v1beta1Clients and @@ -1148,118 +948,3 @@ func TestPipelineRunCRDConversion(t *testing.T) { t.Errorf("-want, +got: %v", d) } } - -// TestBundleConversion tests v1beta1 bundle syntax converted into v1 since it has -// been deprecated in v1 and it would be converted into bundle resolver in pipelineRef -// and taskRef. It sets up a registry for a bundle of a v1beta1 Task and Pipeline -// and uses the v1beta1 TaskRef/ PipelineRef to test the conversion from v1beta1 bundle -// syntax to a v1 bundle resolver and then it tests roundtrip back to v1beta1 bundle -// resolver syntax. -func TestBundleConversion(t *testing.T) { - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) - defer cancel() - - t.Parallel() - - c, namespace := setup(ctx, t, withRegistry, bundleFeatureFlags) - knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf) - defer tearDown(ctx, t, c, namespace) - - repo := getRegistryServiceIP(ctx, t, c, namespace) + ":5000/tektonbundlessimple" - taskName := helpers.ObjectNameForTest(t) - pipelineName := helpers.ObjectNameForTest(t) - task := parse.MustParseV1beta1Task(t, fmt.Sprintf(v1beta1TaskWithBundleYaml, taskName, namespace)) - pipeline := parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(v1beta1PipelineWithBundleYaml, pipelineName, namespace, repo, taskName)) - setupBundle(ctx, t, c, namespace, repo, task, pipeline) - - v1beta1TaskRunName := helpers.ObjectNameForTest(t) - v1beta1TaskRun := parse.MustParseV1beta1TaskRun(t, fmt.Sprintf(v1beta1TaskRunWithBundleYaml, v1beta1TaskRunName, namespace, taskName, repo)) - v1TaskRunExpected := parse.MustParseV1TaskRun(t, fmt.Sprintf(v1TaskRunWithBundleExpectedYaml, v1beta1TaskRunName, namespace, repo, taskName, v1beta1TaskRunName)) - v1beta1TaskRunRoundTripExpected := parse.MustParseV1beta1TaskRun(t, fmt.Sprintf(v1beta1TaskRunWithBundleRoundTripYaml, v1beta1TaskRunName, namespace, repo, taskName, v1beta1TaskRunName)) - - v1TaskRunExpected.Status.Provenance = &v1.Provenance{ - FeatureFlags: getFeatureFlagsBaseOnAPIFlag(t), - RefSource: &v1.RefSource{ - URI: repo, - Digest: map[string]string{"sha256": "a123"}, - EntryPoint: taskName, - }, - } - v1beta1TaskRunRoundTripExpected.Status.Provenance = &v1beta1.Provenance{ - FeatureFlags: getFeatureFlagsBaseOnAPIFlag(t), - RefSource: &v1beta1.RefSource{ - URI: repo, - Digest: map[string]string{"sha256": "a123"}, - EntryPoint: taskName, - }, - } - - if _, err := c.V1beta1TaskRunClient.Create(ctx, v1beta1TaskRun, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create v1beta1 TaskRun: %s", err) - } - if err := WaitForTaskRunState(ctx, c, v1beta1TaskRunName, Succeed(v1beta1TaskRunName), v1beta1TaskRunName, "v1beta1"); err != nil { - t.Fatalf("Failed waiting for v1beta1 TaskRun done: %v", err) - } - - v1TaskRunGot, err := c.V1TaskRunClient.Get(ctx, v1beta1TaskRunName, metav1.GetOptions{}) - if err != nil { - t.Fatalf("Couldn't get expected v1 TaskRun for %s: %s", v1beta1TaskRunName, err) - } - if d := cmp.Diff(v1TaskRunExpected, v1TaskRunGot, append([]cmp.Option{filterV1RefSourceImageDigest, filterV1TaskRunSA}, filterV1TaskRunFields...)...); d != "" { - t.Errorf("-want, +got: %v", d) - } - - v1beta1TaskRunRoundTrip := &v1beta1.TaskRun{} - if err := v1beta1TaskRunRoundTrip.ConvertFrom(context.Background(), v1TaskRunGot); err != nil { - t.Fatalf("Failed to convert roundtrip v1beta1TaskRunGot ConvertFrom v1 = %v", err) - } - if d := cmp.Diff(v1beta1TaskRunRoundTripExpected, v1beta1TaskRunRoundTrip, append([]cmp.Option{filterV1beta1RefSourceImageDigest, filterV1beta1TaskRunSA}, filterV1beta1TaskRunFields...)...); d != "" { - t.Errorf("-want, +got: %v", d) - } - - v1beta1ToV1PipelineRunName := helpers.ObjectNameForTest(t) - v1beta1PipelineRun := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(v1beta1PipelineRunWithBundleYaml, v1beta1ToV1PipelineRunName, namespace, pipelineName, repo)) - v1PipelineRunExpected := parse.MustParseV1PipelineRun(t, fmt.Sprintf(v1PipelineRunWithBundleExpectedYaml, v1beta1ToV1PipelineRunName, namespace, repo, pipelineName, repo, taskName, v1beta1ToV1PipelineRunName)) - v1beta1PRRoundTripExpected := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(v1beta1PipelineRunWithBundleRoundTripYaml, v1beta1ToV1PipelineRunName, namespace, repo, pipelineName, repo, taskName, v1beta1ToV1PipelineRunName)) - - v1PipelineRunExpected.Status.Provenance = &v1.Provenance{ - FeatureFlags: getFeatureFlagsBaseOnAPIFlag(t), - RefSource: &v1.RefSource{ - URI: repo, - Digest: map[string]string{"sha256": "a123"}, - EntryPoint: pipelineName, - }, - } - v1beta1PRRoundTripExpected.Status.Provenance = &v1beta1.Provenance{ - FeatureFlags: getFeatureFlagsBaseOnAPIFlag(t), - RefSource: &v1beta1.RefSource{ - URI: repo, - Digest: map[string]string{"sha256": "a123"}, - EntryPoint: pipelineName, - }, - } - - if _, err := c.V1beta1PipelineRunClient.Create(ctx, v1beta1PipelineRun, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create v1beta1 PipelineRun: %s", err) - } - if err := WaitForPipelineRunState(ctx, c, v1beta1ToV1PipelineRunName, timeout, Succeed(v1beta1ToV1PipelineRunName), v1beta1ToV1PipelineRunName, "v1beta1"); err != nil { - t.Fatalf("Failed waiting for v1beta1 PipelineRun done: %v", err) - } - - v1PipelineRunGot, err := c.V1PipelineRunClient.Get(ctx, v1beta1ToV1PipelineRunName, metav1.GetOptions{}) - if err != nil { - t.Fatalf("Couldn't get expected v1 PipelineRun for %s: %s", v1beta1ToV1PipelineRunName, err) - } - if d := cmp.Diff(v1PipelineRunExpected, v1PipelineRunGot, append([]cmp.Option{filterV1RefSourceImageDigest, filterV1PipelineRunSA}, filterV1PipelineRunFields...)...); d != "" { - t.Errorf("-want, +got: %v", d) - } - - v1beta1PRRoundTrip := &v1beta1.PipelineRun{} - if err := v1beta1PRRoundTrip.ConvertFrom(context.Background(), v1PipelineRunGot); err != nil { - t.Fatalf("Error roundtrip v1beta1PipelineRun ConvertFrom v1PipelineRunGot = %v", err) - } - if d := cmp.Diff(v1beta1PRRoundTripExpected, v1beta1PRRoundTrip, append([]cmp.Option{filterV1beta1RefSourceImageDigest, filterV1beta1PipelineRunSA}, filterV1beta1PipelineRunFields...)...); d != "" { - t.Errorf("-want, +got: %v", d) - } -} diff --git a/test/tektonbundles_test.go b/test/tektonbundles_test.go index 3515dcaeb41..d21a2cc8d85 100644 --- a/test/tektonbundles_test.go +++ b/test/tektonbundles_test.go @@ -36,9 +36,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1/layout" "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/google/go-containerregistry/pkg/v1/tarball" - pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" - "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun" "github.com/tektoncd/pipeline/test/parse" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -48,108 +46,11 @@ import ( "sigs.k8s.io/yaml" ) -var bundleFeatureFlags = requireAnyGate(map[string]string{ - "enable-tekton-oci-bundles": "true", - "enable-api-fields": "alpha", -}) - var resolverFeatureFlags = requireAllGates(map[string]string{ "enable-bundles-resolver": "true", "enable-api-fields": "beta", }) -// TestTektonBundlesSimpleWorkingExample is an integration test which tests a simple, working Tekton bundle using OCI -// images. -func TestTektonBundlesSimpleWorkingExample(t *testing.T) { - ctx := context.Background() - c, namespace := setup(ctx, t, withRegistry, bundleFeatureFlags) - - t.Parallel() - - knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf) - defer tearDown(ctx, t, c, namespace) - - taskName := helpers.ObjectNameForTest(t) - pipelineName := helpers.ObjectNameForTest(t) - pipelineRunName := helpers.ObjectNameForTest(t) - repo := getRegistryServiceIP(ctx, t, c, namespace) + ":5000/tektonbundlessimple" - task := parse.MustParseV1beta1Task(t, fmt.Sprintf(` -metadata: - name: %s - namespace: %s -spec: - steps: - - name: hello - image: alpine - script: 'echo Hello' -`, taskName, namespace)) - - pipeline := parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` -metadata: - name: %s - namespace: %s -spec: - tasks: - - name: hello-world - taskRef: - name: %s - bundle: %s -`, pipelineName, namespace, taskName, repo)) - - setupBundle(ctx, t, c, namespace, repo, task, pipeline) - - // Now generate a PipelineRun to invoke this pipeline and task. - pr := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(` -metadata: - name: %s -spec: - pipelineRef: - name: %s - bundle: %s -`, pipelineRunName, pipelineName, repo)) - if _, err := c.V1beta1PipelineRunClient.Create(ctx, pr, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create PipelineRun: %s", err) - } - - t.Logf("Waiting for PipelineRun in namespace %s to finish", namespace) - if err := WaitForPipelineRunState(ctx, c, pipelineRunName, timeout, PipelineRunSucceed(pipelineRunName), "PipelineRunCompleted", v1beta1Version); err != nil { - t.Errorf("Error waiting for PipelineRun to finish with error: %s", err) - } - - trs, err := c.V1beta1TaskRunClient.List(ctx, metav1.ListOptions{}) - if err != nil { - t.Errorf("Error retrieving taskrun: %s", err) - } - if len(trs.Items) != 1 { - t.Fatalf("Expected 1 TaskRun but found %d", len(trs.Items)) - } - - tr := trs.Items[0] - if tr.Status.GetCondition(apis.ConditionSucceeded).IsFalse() { - t.Errorf("Expected TaskRun to succeed but instead found condition: %s", tr.Status.GetCondition(apis.ConditionSucceeded)) - } - - if tr.Status.PodName == "" { - t.Fatal("Error getting a PodName (empty)") - } - p, err := c.KubeClient.CoreV1().Pods(namespace).Get(ctx, tr.Status.PodName, metav1.GetOptions{}) - if err != nil { - t.Fatalf("Error getting pod `%s` in namespace `%s`", tr.Status.PodName, namespace) - } - for _, stat := range p.Status.ContainerStatuses { - if strings.Contains(stat.Name, "step-hello") { - req := c.KubeClient.CoreV1().Pods(namespace).GetLogs(p.Name, &corev1.PodLogOptions{Container: stat.Name}) - logContent, err := req.Do(ctx).Raw() - if err != nil { - t.Fatalf("Error getting pod logs for pod `%s` and container `%s` in namespace `%s`", tr.Status.PodName, stat.Name, namespace) - } - if !strings.Contains(string(logContent), "Hello") { - t.Fatalf("Expected logs to say hello but received %v", logContent) - } - } - } -} - // TestTektonBundlesResolver is an integration test which tests a simple, working Tekton bundle using OCI // images using the remote resolution bundles resolver. func TestTektonBundlesResolver(t *testing.T) { @@ -253,169 +154,6 @@ spec: } } -// TestTektonBundlesUsingRegularImage is an integration test which passes a non-Tekton bundle as a task reference. -func TestTektonBundlesUsingRegularImage(t *testing.T) { - ctx := context.Background() - c, namespace := setup(ctx, t, withRegistry, bundleFeatureFlags) - - t.Parallel() - - knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf) - defer tearDown(ctx, t, c, namespace) - - taskName := helpers.ObjectNameForTest(t) - pipelineName := helpers.ObjectNameForTest(t) - pipelineRunName := helpers.ObjectNameForTest(t) - repo := getRegistryServiceIP(ctx, t, c, namespace) + ":5000/tektonbundlesregularimage" - - pipeline := parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` -metadata: - name: %s - namespace: %s -spec: - tasks: - - name: hello-world - taskRef: - name: %s - bundle: registry -`, pipelineName, namespace, taskName)) - - setupBundle(ctx, t, c, namespace, repo, nil, pipeline) - - // Now generate a PipelineRun to invoke this pipeline and task. - pr := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(` -metadata: - name: %s -spec: - pipelineRef: - name: %s - bundle: %s -`, pipelineRunName, pipelineName, repo)) - if _, err := c.V1beta1PipelineRunClient.Create(ctx, pr, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create PipelineRun: %s", err) - } - - t.Logf("Waiting for PipelineRun in namespace %s to finish", namespace) - if err := WaitForPipelineRunState(ctx, c, pipelineRunName, timeout, - Chain( - FailedWithReason(pipelinev1.PipelineRunReasonCouldntGetTask.String(), pipelineRunName), - FailedWithMessage("does not contain a dev.tekton.image.apiVersion annotation", pipelineRunName), - ), "PipelineRunFailed", v1beta1Version); err != nil { - t.Fatalf("Error waiting for PipelineRun to finish with expected error: %s", err) - } -} - -// TestTektonBundlesUsingImproperFormat is an integration test which passes an improperly formatted Tekton bundle as a -// task reference. -func TestTektonBundlesUsingImproperFormat(t *testing.T) { - ctx := context.Background() - c, namespace := setup(ctx, t, withRegistry, bundleFeatureFlags) - - t.Parallel() - - knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf) - defer tearDown(ctx, t, c, namespace) - - taskName := helpers.ObjectNameForTest(t) - pipelineName := helpers.ObjectNameForTest(t) - pipelineRunName := helpers.ObjectNameForTest(t) - repo := getRegistryServiceIP(ctx, t, c, namespace) + ":5000/tektonbundlesimproperformat" - - ref, err := name.ParseReference(repo) - if err != nil { - t.Fatalf("Failed to parse %s as an OCI reference: %s", repo, err) - } - - task := parse.MustParseV1beta1Task(t, fmt.Sprintf(` -metadata: - name: %s - namespace: %s -spec: - steps: - - name: hello - image: alpine - script: 'echo Hello' -`, taskName, namespace)) - - pipeline := parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` -metadata: - name: %s - namespace: %s -spec: - tasks: - - name: hello-world - taskRef: - name: %s - bundle: %s -`, pipelineName, namespace, taskName, repo)) - - // Write the pipeline into an image to the registry in the proper format. Write the task using incorrect - // annotations. - rawTask, err := yaml.Marshal(task) - if err != nil { - t.Fatalf("Failed to marshal task to yaml: %s", err) - } - - rawPipeline, err := yaml.Marshal(pipeline) - if err != nil { - t.Fatalf("Failed to marshal task to yaml: %s", err) - } - - img := empty.Image - taskLayer, err := tarball.LayerFromReader(bytes.NewBuffer(rawTask)) - if err != nil { - t.Fatalf("Failed to create oci layer from task: %s", err) - } - pipelineLayer, err := tarball.LayerFromReader(bytes.NewBuffer(rawPipeline)) - if err != nil { - t.Fatalf("Failed to create oci layer from pipeline: %s", err) - } - img, err = mutate.Append(img, mutate.Addendum{ - Layer: taskLayer, - Annotations: map[string]string{ - // intentionally invalid name annotation - "org.opencontainers.image.title": taskName, - "dev.tekton.image.kind": strings.ToLower(task.Kind), - "dev.tekton.image.apiVersion": task.APIVersion, - }, - }, mutate.Addendum{ - Layer: pipelineLayer, - Annotations: map[string]string{ - "dev.tekton.image.name": pipelineName, - "dev.tekton.image.kind": strings.ToLower(pipeline.Kind), - "dev.tekton.image.apiVersion": pipeline.APIVersion, - }, - }) - if err != nil { - t.Fatalf("Failed to create an oci image from the task and pipeline layers: %s", err) - } - - // Publish this image to the in-cluster registry. - publishImg(ctx, t, c, namespace, img, ref) - - // Now generate a PipelineRun to invoke this pipeline and task. - pr := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(` -metadata: - name: %s -spec: - pipelineRef: - name: %s - bundle: %s -`, pipelineRunName, pipelineName, repo)) - if _, err := c.V1beta1PipelineRunClient.Create(ctx, pr, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create PipelineRun: %s", err) - } - - t.Logf("Waiting for PipelineRun in namespace %s to finish", namespace) - if err := WaitForPipelineRunState(ctx, c, pipelineRunName, timeout, - Chain( - FailedWithReason(pipelinerun.ReasonCouldntGetPipeline, pipelineRunName), - FailedWithMessage("does not contain a dev.tekton.image.name annotation", pipelineRunName), - ), "PipelineRunFailed", v1beta1Version); err != nil { - t.Fatalf("Error waiting for PipelineRun to finish with expected error: %s", err) - } -} - func tarImageInOCIFormat(namespace string, img v1.Image) ([]byte, error) { // Write the image in the OCI layout and then tar it up. dir, err := os.MkdirTemp(os.TempDir(), namespace)