Fix bundle conversion for pipelineRef

This commit fixes the bundle conversion for PipelineRef which should
be converting to the bundle Resolver with a Pipeline kind. It also adds
the conversion integration tests for the bundle logics to prevent such
error. It extracts the setup for bundle and makes it into a helper function.

pkg/apis/pipeline/v1beta1/pipelineref_conversion.go

@@ -56,7 +56,7 @@ func (pr PipelineRef) convertBundleToResolver(sink *v1.PipelineRef) {
 				Value: v1.ParamValue{StringVal: pr.Name, Type: v1.ParamTypeString},
 			}, {
 				Name:  "kind",
-				Value: v1.ParamValue{StringVal: "Task", Type: v1.ParamTypeString},
+				Value: v1.ParamValue{StringVal: "Pipeline", Type: v1.ParamTypeString},
 			}},
 		}
 	}

pkg/apis/pipeline/v1beta1/pipelinerun_conversion_test.go

@@ -395,7 +395,7 @@ func TestPipelineRunConversionFromDeprecated(t *testing.T) {
 						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"}},
+							{Name: "kind", Value: v1beta1.ParamValue{StringVal: "Pipeline", Type: "string"}},
 						},
 					},
 				},

test/conversion_test.go

@@ -49,6 +49,10 @@ var (
 	filterV1StepState              = cmpopts.IgnoreFields(v1.StepState{}, "Name", "ImageID", "Container")
 	filterV1beta1StepState         = cmpopts.IgnoreFields(v1beta1.StepState{}, "Name", "ImageID", "ContainerName")
 	filterReleaseAnnotation        = cmpopts.IgnoreMapEntries(ignoreReleaseAnnotation)
+	filterV1TaskRunSA              = cmpopts.IgnoreFields(v1.TaskRunSpec{}, "ServiceAccountName")
+	filterV1beta1TaskRunSA         = cmpopts.IgnoreFields(v1beta1.TaskRunSpec{}, "ServiceAccountName")
+	filterV1PipelineRunSA          = cmpopts.IgnoreFields(v1.PipelineTaskRunTemplate{}, "ServiceAccountName")
+	filterV1beta1PipelineRunSA     = cmpopts.IgnoreFields(v1beta1.PipelineRunSpec{}, "ServiceAccountName")
 
 	filterMetadata                 = []cmp.Option{filterTypeMeta, filterObjectMeta}
 	filterV1TaskRunFields          = []cmp.Option{filterTypeMeta, filterObjectMeta, filterLabels, filterCondition, filterReleaseAnnotation, filterV1TaskRunStatus, filterContainerStateTerminated, filterV1StepState}
@@ -655,6 +659,207 @@ status:
       name: %s-fetch-secure-data
       pipelineTaskName: fetch-secure-data
 `
+
+	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:
+    kind: Task
+    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
+`
+
+	v1PipelineRunWithBundleExpectedYaml = `
+metadata:
+  name: %s
+  namespace: %s
+spec:
+  taskRunTemplate:
+  timeouts: 
+    pipeline: 1h
+  pipelineRef:
+    kind: Pipeline
+    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:
+        kind: Task
+        resolver: bundles
+        params:
+        - name: bundle
+          value: %s
+        - name: name
+          value: %s
+  childReferences:
+  - apiVersion: tekton.dev/v1beta1
+    kind: TaskRun
+    name: %s-hello-world
+    pipelineTaskName: hello-world
+`
+
+	v1beta1TaskRunWithBundleRoundTripYaml = `
+metadata:
+  name: %s
+  namespace: %s
+spec:
+  timeout: 1h
+  taskRef:
+    kind: Task
+    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:
+    kind: Pipeline
+    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:
+        kind: Task
+        resolver: bundles
+        params:
+        - name: bundle
+          value: %s
+        - name: name
+          value: %s
+  childReferences:
+  - apiVersion: tekton.dev/v1beta1
+    kind: TaskRun
+    name: %s-hello-world
+    pipelineTaskName: hello-world
+`
 )
 
 // TestTaskCRDConversion first creates a v1beta1 Task CRD using v1beta1Clients and
@@ -926,3 +1131,98 @@ 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 := fmt.Sprintf("%s:5000/tektonbundlessimple", getRegistryServiceIP(ctx, t, c, namespace))
+	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),
+	}
+	v1beta1TaskRunRoundTripExpected.Status.Provenance = &v1beta1.Provenance{
+		FeatureFlags: getFeatureFlagsBaseOnAPIFlag(t),
+	}
+
+	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(filterV1TaskRunFields, filterV1TaskRunSA)...); 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(filterV1beta1TaskRunFields, filterV1beta1TaskRunSA)...); 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),
+	}
+	v1beta1PRRoundTripExpected.Status.Provenance = &v1beta1.Provenance{
+		FeatureFlags: getFeatureFlagsBaseOnAPIFlag(t),
+	}
+
+	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(filterV1PipelineRunFields, filterV1PipelineRunSA)...); 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(filterV1beta1PipelineRunFields, filterV1beta1PipelineRunSA)...); d != "" {
+		t.Errorf("-want, +got: %v", d)
+	}
+}