diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/output_resource.go b/pkg/reconciler/v1alpha1/taskrun/resources/output_resource.go index 39dc6f82f86..da16bcf6cac 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/output_resource.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/output_resource.go @@ -30,6 +30,13 @@ import ( var ( outputDir = "/workspace/output/" + + // allowedOutputResource checks if an output resource type produces + // an output that should be copied to the PVC + allowedOutputResources = map[v1alpha1.PipelineResourceType]bool{ + v1alpha1.PipelineResourceTypeStorage: true, + v1alpha1.PipelineResourceTypeGit: true, + } ) // AddOutputResources reads the output resources and adds the corresponding container steps @@ -105,8 +112,12 @@ func AddOutputResources( } } + // Workaround for issue #401. Unless all resource types are implemented as + // outputs, or until we have metadata on the resource that declares whether + // the output should be copied to the PVC, only copy git and storage output + // resources. // copy to pvc if pvc is present - if pipelineRunpvcName != "" { + if allowedOutputResources[resource.Spec.Type] && pipelineRunpvcName != "" { var newSteps []corev1.Container for _, dPath := range boundResource.Paths { newSteps = append(newSteps, []corev1.Container{{ @@ -186,3 +197,10 @@ func addStoreUploadStep(build *buildv1alpha1.Build, build.Spec.Steps = append(build.Spec.Steps, buildSteps...) return nil } + +// allowedOutputResource checks if an output resource type produces +// an output that should be copied to the PVC +func allowedOutputResource(resourceType v1alpha1.PipelineResourceType) bool { + + return allowedOutputResources[resourceType] +} diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go index a75b65c6210..702b3ee006b 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go @@ -85,6 +85,14 @@ func outputResourcesetUp() { FieldName: "STORAGE_CREDS", }}, }, + }, { + ObjectMeta: metav1.ObjectMeta{ + Name: "source-image", + Namespace: "marshmallow", + }, + Spec: v1alpha1.PipelineResourceSpec{ + Type: "image", + }, }} for _, r := range rs { @@ -549,6 +557,45 @@ func Test_Valid_OutputResources(t *testing.T) { Secret: &corev1.SecretVolumeSource{SecretName: "sname"}, }, }}, + }, { + name: "image resource as output", + desc: "image resource defined only in output", + taskRun: &v1alpha1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-taskrun-run-only-output-step", + Namespace: "marshmallow", + OwnerReferences: []metav1.OwnerReference{{ + Kind: "PipelineRun", + Name: "pipelinerun", + }}, + }, + Spec: v1alpha1.TaskRunSpec{ + Outputs: v1alpha1.TaskRunOutputs{ + Resources: []v1alpha1.TaskResourceBinding{{ + Name: "source-workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "source-image", + }, + }}, + }, + }, + }, + task: &v1alpha1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task1", + Namespace: "marshmallow", + }, + Spec: v1alpha1.TaskSpec{ + Outputs: &v1alpha1.Outputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "source-workspace", + Type: "image", + }}, + }, + }, + }, + wantSteps: nil, + build: build(), }} { t.Run(c.name, func(t *testing.T) { outputResourcesetUp() @@ -716,3 +763,29 @@ func Test_InValid_OutputResources(t *testing.T) { }) } } + +func Test_AllowedOutputResource(t *testing.T) { + for _, c := range []struct { + desc string + resourceType v1alpha1.PipelineResourceType + expectedAllowed bool + }{{ + desc: "storage type is allowed", + resourceType: v1alpha1.PipelineResourceTypeStorage, + expectedAllowed: true, + }, { + desc: "git type is allowed", + resourceType: v1alpha1.PipelineResourceTypeGit, + expectedAllowed: true, + }, { + desc: "anything else is not allowed", + resourceType: "fooType", + expectedAllowed: false, + }} { + t.Run(c.desc, func(t *testing.T) { + if c.expectedAllowed != allowedOutputResources[c.resourceType] { + t.Fatalf("Test AllowedOutputResource %s expected %t but got %t", c.desc, c.expectedAllowed, allowedOutputResources[c.resourceType]) + } + }) + } +}