Preserve permissions when copying to/from an artifact storage bucket

We were calling `gsutil cp -r ...`, which doesn't preserve
permissions. That means that if you have an executable file in one
`TaskRun` pod and copy it to another `TaskRun` pod via an artifact
storage bucket, it won't be executable when it gets there. So let's
fix that.

fixes #1047

Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>

pkg/apis/pipeline/v1alpha1/artifact_bucket.go

@@ -77,7 +77,7 @@ func (b *ArtifactBucket) StorageBasePath(pr *PipelineRun) string {
 
 // GetCopyFromStorageToContainerSpec returns a container used to download artifacts from temporary storage
 func (b *ArtifactBucket) GetCopyFromStorageToContainerSpec(name, sourcePath, destinationPath string) []corev1.Container {
-	args := []string{"-args", fmt.Sprintf("cp -r %s %s", fmt.Sprintf("%s/%s/*", b.Location, sourcePath), destinationPath)}
+	args := []string{"-args", fmt.Sprintf("cp -P -r %s %s", fmt.Sprintf("%s/%s/*", b.Location, sourcePath), destinationPath)}
 
 	envVars, secretVolumeMount := getSecretEnvVarsAndVolumeMounts("bucket", secretVolumeMountPath, b.Secrets)
 
@@ -100,7 +100,7 @@ func (b *ArtifactBucket) GetCopyFromStorageToContainerSpec(name, sourcePath, des
 
 // GetCopyToStorageFromContainerSpec returns a container used to upload artifacts for temporary storage
 func (b *ArtifactBucket) GetCopyToStorageFromContainerSpec(name, sourcePath, destinationPath string) []corev1.Container {
-	args := []string{"-args", fmt.Sprintf("cp -r %s %s", sourcePath, fmt.Sprintf("%s/%s", b.Location, destinationPath))}
+	args := []string{"-args", fmt.Sprintf("cp -P -r %s %s", sourcePath, fmt.Sprintf("%s/%s", b.Location, destinationPath))}
 
 	envVars, secretVolumeMount := getSecretEnvVarsAndVolumeMounts("bucket", secretVolumeMountPath, b.Secrets)
 

pkg/apis/pipeline/v1alpha1/artifact_bucket_test.go

@@ -55,7 +55,7 @@ func TestBucketGetCopyFromContainerSpec(t *testing.T) {
 		Name:         "artifact-copy-from-workspace-mz4c7",
 		Image:        "override-with-gsutil-image:latest",
 		Command:      []string{"/ko-app/gsutil"},
-		Args:         []string{"-args", "cp -r gs://fake-bucket/src-path/* /workspace/destination"},
+		Args:         []string{"-args", "cp -P -r gs://fake-bucket/src-path/* /workspace/destination"},
 		Env:          []corev1.EnvVar{{Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: fmt.Sprintf("/var/bucketsecret/%s/serviceaccount", secretName)}},
 		VolumeMounts: []corev1.VolumeMount{{Name: expectedVolumeName, MountPath: fmt.Sprintf("/var/bucketsecret/%s", secretName)}},
 	}}
@@ -72,7 +72,7 @@ func TestBucketGetCopyToContainerSpec(t *testing.T) {
 		Name:         "artifact-copy-to-workspace-9l9zj",
 		Image:        "override-with-gsutil-image:latest",
 		Command:      []string{"/ko-app/gsutil"},
-		Args:         []string{"-args", "cp -r src-path gs://fake-bucket/workspace/destination"},
+		Args:         []string{"-args", "cp -P -r src-path gs://fake-bucket/workspace/destination"},
 		Env:          []corev1.EnvVar{{Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: fmt.Sprintf("/var/bucketsecret/%s/serviceaccount", secretName)}},
 		VolumeMounts: []corev1.VolumeMount{{Name: expectedVolumeName, MountPath: fmt.Sprintf("/var/bucketsecret/%s", secretName)}},
 	}}

pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go

@@ -937,7 +937,7 @@ func TestAddStepsToTaskWithBucketFromConfigMap(t *testing.T) {
 				Name:    "artifact-copy-from-gitspace-78c5n",
 				Image:   "override-with-gsutil-image:latest",
 				Command: []string{"/ko-app/gsutil"},
-				Args:    []string{"-args", "cp -r gs://fake-bucket/prev-task-path/* /workspace/gitspace"},
+				Args:    []string{"-args", "cp -P -r gs://fake-bucket/prev-task-path/* /workspace/gitspace"},
 			}},
 		},
 	}, {
@@ -975,7 +975,7 @@ func TestAddStepsToTaskWithBucketFromConfigMap(t *testing.T) {
 				Name:    "artifact-copy-from-workspace-j2tds",
 				Image:   "override-with-gsutil-image:latest",
 				Command: []string{"/ko-app/gsutil"},
-				Args:    []string{"-args", "cp -r gs://fake-bucket/prev-task-path/* /workspace/gcs-dir"},
+				Args:    []string{"-args", "cp -P -r gs://fake-bucket/prev-task-path/* /workspace/gcs-dir"},
 			}},
 		},
 	}} {

pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go

@@ -771,7 +771,7 @@ func TestValidOutputResourcesWithBucketStorage(t *testing.T) {
 			Name:    "artifact-copy-to-source-git-9l9zj",
 			Image:   "override-with-gsutil-image:latest",
 			Command: []string{"/ko-app/gsutil"},
-			Args:    []string{"-args", "cp -r /workspace/source-workspace gs://fake-bucket/pipeline-task-name"},
+			Args:    []string{"-args", "cp -P -r /workspace/source-workspace gs://fake-bucket/pipeline-task-name"},
 		}},
 	}, {
 		name: "git resource in output only with bucket storage",
@@ -815,7 +815,7 @@ func TestValidOutputResourcesWithBucketStorage(t *testing.T) {
 			Name:    "artifact-copy-to-source-git-9l9zj",
 			Image:   "override-with-gsutil-image:latest",
 			Command: []string{"/ko-app/gsutil"},
-			Args:    []string{"-args", "cp -r /workspace/output/source-workspace gs://fake-bucket/pipeline-task-name"},
+			Args:    []string{"-args", "cp -P -r /workspace/output/source-workspace gs://fake-bucket/pipeline-task-name"},
 		}},
 	}, {
 		name: "git resource in output",

test/artifact_bucket_test.go

@@ -32,7 +32,7 @@ import (
 const (
 	helloworldResourceName    = "helloworldgit"
 	addFileTaskName           = "add-file-to-resource-task"
-	readFileTaskName          = "read-new-file-task"
+	runFileTaskName           = "run-new-file-task"
 	bucketTestPipelineName    = "bucket-test-pipeline"
 	bucketTestPipelineRunName = "bucket-test-pipeline-run"
 	systemNamespace           = "tekton-pipelines"
@@ -129,22 +129,22 @@ func TestStorageBucketPipelineRun(t *testing.T) {
 		tb.TaskInputs(tb.InputsResource(helloworldResourceName, v1alpha1.PipelineResourceTypeGit)),
 		tb.TaskOutputs(tb.OutputsResource(helloworldResourceName, v1alpha1.PipelineResourceTypeGit)),
 		tb.Step("addfile", "ubuntu", tb.Command("/bin/bash"),
-			tb.Args("-c", "echo stuff > /workspace/helloworldgit/newfile"),
+			tb.Args("-c", "'#!/bin/bash\necho hello' > /workspace/helloworldgit/newfile"),
 		),
+		tb.Step("make-executable", "ubuntu", tb.Command("chmod"),
+			tb.Args("+x", "/workspace/helloworldgit/newfile")),
 	))
 	if _, err := c.TaskClient.Create(addFileTask); err != nil {
 		t.Fatalf("Failed to create Task `%s`: %s", addFileTaskName, err)
 	}
 
-	t.Logf("Creating Task %s", readFileTaskName)
-	readFileTask := tb.Task(readFileTaskName, namespace, tb.TaskSpec(
+	t.Logf("Creating Task %s", runFileTaskName)
+	readFileTask := tb.Task(runFileTaskName, namespace, tb.TaskSpec(
 		tb.TaskInputs(tb.InputsResource(helloworldResourceName, v1alpha1.PipelineResourceTypeGit)),
-		tb.Step("readfile", "ubuntu", tb.Command("/bin/bash"),
-			tb.Args("-c", "cat /workspace/helloworldgit/newfile"),
-		),
+		tb.Step("runfile", "ubuntu", tb.Command("/workspace/helloworld/newfile")),
 	))
 	if _, err := c.TaskClient.Create(readFileTask); err != nil {
-		t.Fatalf("Failed to create Task `%s`: %s", readFileTaskName, err)
+		t.Fatalf("Failed to create Task `%s`: %s", runFileTaskName, err)
 	}
 
 	t.Logf("Creating Pipeline %s", bucketTestPipelineName)
@@ -154,7 +154,7 @@ func TestStorageBucketPipelineRun(t *testing.T) {
 			tb.PipelineTaskInputResource("helloworldgit", "source-repo"),
 			tb.PipelineTaskOutputResource("helloworldgit", "source-repo"),
 		),
-		tb.PipelineTask("readfile", readFileTaskName,
+		tb.PipelineTask("runfile", runFileTaskName,
 			tb.PipelineTaskInputResource("helloworldgit", "source-repo", tb.From("addfile")),
 		),
 	))