Fix gsutil authentication bug

Fixes tektoncd#372 what: gsutil previosuly supported authentication using environment variables. Build pipeline gcs resource implementation relied on this feature to provide access for private gcs buckets. In recent version of gsutil there is a bug which did not recognize env variable GOOGLE_APPLICATION_CREDENTIALS and users noticed that they could not access private gcs artifacts how: authentication method is updated to use more concrete method using gcloud auth command. This PR does not change user contract to authenticate but only implementation on how client authentication is done behind the screen. e2e test pipelinerun test is updated not to have gcs input resource as gcloud auth command requires valid service account json(in new authentication world). New e2e test is added to replace old test and in this test gcloud service account json is used for test setup. This test checks whether developer has gcloud service account is set and if not test is skipped with helpeful msg
bobcatfish · Jan 21, 2019 · e8caf46 · e8caf46
1 parent 02e06bc
commit e8caf46
Show file tree

Hide file tree

Showing 9 changed files with 233 additions and 117 deletions.
diff --git a/cmd/gsutil/main.go b/cmd/gsutil/main.go
@@ -15,14 +15,16 @@ limitations under the License.
 */
 
 /*
-This tool is for executing `gsutil` command
-gsutil command is prefixed with `-m` flag(default) for faster download and uploads.
+This tool is for executing `gsutil` and `gcloud` command
 To override gsutil base image update `.ko.yaml` file.
 
+`gcloud auth` is used to authenticate with private buckets
+
 To use it, run
 ```
 image: github.com/build-pipeline/cmd/gsutil
 args: ['-args', 'ARGUMENTS_FOR_GSUTIL_COMMAND']
+
 ```
 
 For help, run
@@ -37,13 +39,17 @@ Following example executes gsutil sub-command `cp`
 ```
 image: github.com/build-pipeline/cmd/gsutil
 args: ['-args', 'cp', 'gs://fake-bucket/rules.zip', '/workspace/gcs-dir'
+env:
+  - name: GOOGLE_APPLICATION_CREDENTIALS
+    value: /var/secret/auth/key.json
 ```
 */
 
 package main
 
 import (
 	"flag"
+	"os"
 	"os/exec"
 	"strings"
 
@@ -58,12 +64,27 @@ func main() {
 	flag.Parse()
 	logger, _ := logging.NewLogger("", "gsutil")
 	defer logger.Sync()
-	gsutilPath, checkgsutilErr := exec.LookPath("gsutil")
-	if checkgsutilErr != nil {
-		logger.Fatalf("Error executing command 'gsutil verson'; err %s; output %s", checkgsutilErr.Error(), gsutilPath)
+
+	authFilePath := os.Getenv("GOOGLE_APPLICATION_CREDENTIALS")
+	if authFilePath != "" {
+		_, err := os.Stat(authFilePath)
+		if os.IsNotExist(err) {
+			logger.Fatalf("File does not exist at path %s; got err %s", authFilePath, err)
+		}
+
+		if _, err := exec.LookPath("gcloud"); err != nil {
+			logger.Fatalf("Error executing command 'gcloud'; err %s", err)
+		}
+
+		if authCmdstdoutStderr, err := exec.Command("gcloud", "auth", "activate-service-account", "--key-file", authFilePath).CombinedOutput(); err != nil {
+			logger.Fatalf("Error executing command %s; cmd output: %s", err, authCmdstdoutStderr)
+		}
+		logger.Info("Successfully authenticated with gcloud")
 	}
 
-	logger.Infof("gsutil binary at path %s", gsutilPath)
+	if _, err := exec.LookPath("gsutil"); err != nil {
+		logger.Fatalf("Error executing command 'gsutil'; err %s", err)
+	}
 
 	cmd := exec.Command("gsutil")
 	cmd.Args = append(cmd.Args, strings.Split(*args, " ")...)

diff --git a/pkg/apis/pipeline/v1alpha1/gcs_resource.go b/pkg/apis/pipeline/v1alpha1/gcs_resource.go
@@ -150,22 +150,29 @@ func (s *GCSResource) GetDownloadContainerSpec() ([]corev1.Container, error) {
 
 func getSecretEnvVarsAndVolumeMounts(resourceName string, s []SecretParam) ([]corev1.EnvVar, []corev1.VolumeMount) {
 	mountPaths := make(map[string]struct{})
-	var envVars []corev1.EnvVar
-	var secretVolumeMount []corev1.VolumeMount
-	for _, secretParam := range s {
-		mountPath := filepath.Join(gcsSecretVolumeMountPath, secretParam.SecretName)
+	var (
+		envVars           []corev1.EnvVar
+		secretVolumeMount []corev1.VolumeMount
+		authVar           bool
+	)
 
-		envVars = append(envVars, corev1.EnvVar{
-			Name:  strings.ToUpper(secretParam.FieldName),
-			Value: filepath.Join(mountPath, secretParam.SecretKey),
-		})
+	for _, secretParam := range s {
+		if secretParam.FieldName == "GOOGLE_APPLICATION_CREDENTIALS" && !authVar {
+			authVar = true
+			mountPath := filepath.Join(gcsSecretVolumeMountPath, secretParam.SecretName)
 
-		if _, ok := mountPaths[mountPath]; !ok {
-			secretVolumeMount = append(secretVolumeMount, corev1.VolumeMount{
-				Name:      fmt.Sprintf("volume-%s-%s", resourceName, secretParam.SecretName),
-				MountPath: mountPath,
+			envVars = append(envVars, corev1.EnvVar{
+				Name:  strings.ToUpper(secretParam.FieldName),
+				Value: filepath.Join(mountPath, secretParam.SecretKey),
 			})
-			mountPaths[mountPath] = struct{}{}
+
+			if _, ok := mountPaths[mountPath]; !ok {
+				secretVolumeMount = append(secretVolumeMount, corev1.VolumeMount{
+					Name:      fmt.Sprintf("volume-%s-%s", resourceName, secretParam.SecretName),
+					MountPath: mountPath,
+				})
+				mountPaths[mountPath] = struct{}{}
+			}
 		}
 	}
 	return envVars, secretVolumeMount

diff --git a/pkg/apis/pipeline/v1alpha1/gcs_resource_test.go b/pkg/apis/pipeline/v1alpha1/gcs_resource_test.go
@@ -216,31 +216,21 @@ func Test_GetDownloadContainerSpec(t *testing.T) {
 			TypeDir:        true,
 			Secrets: []SecretParam{{
 				SecretName: "secretName",
-				FieldName:  "fieldName",
-				SecretKey:  "key.json",
-			}, {
+				FieldName:  "GOOGLE_APPLICATION_CREDENTIALS",
 				SecretKey:  "key.json",
-				SecretName: "secretNameSomethingelse",
-				FieldName:  "GOOGLE_ANOTHER_CREDENTIALS",
 			}},
 		},
 		wantContainers: []corev1.Container{{
 			Name:  "storage-fetch-gcs-valid",
 			Image: "override-with-gsutil-image:latest",
 			Args:  []string{"-args", "cp -r gs://some-bucket/** /workspace"},
 			Env: []corev1.EnvVar{{
-				Name:  "FIELDNAME",
+				Name:  "GOOGLE_APPLICATION_CREDENTIALS",
 				Value: "/var/secret/secretName/key.json",
-			}, {
-				Name:  "GOOGLE_ANOTHER_CREDENTIALS",
-				Value: "/var/secret/secretNameSomethingelse/key.json",
 			}},
 			VolumeMounts: []corev1.VolumeMount{{
 				Name:      "volume-gcs-valid-secretName",
 				MountPath: "/var/secret/secretName",
-			}, {
-				Name:      "volume-gcs-valid-secretNameSomethingelse",
-				MountPath: "/var/secret/secretNameSomethingelse",
 			}},
 		}},
 	}, {
@@ -256,18 +246,15 @@ func Test_GetDownloadContainerSpec(t *testing.T) {
 			}, {
 				SecretKey:  "key.json",
 				SecretName: "secretName",
-				FieldName:  "GOOGLE_ANOTHER_CREDENTIALS",
+				FieldName:  "GOOGLE_APPLICATION_CREDENTIALS",
 			}},
 		},
 		wantContainers: []corev1.Container{{
 			Name:  "storage-fetch-gcs-valid",
 			Image: "override-with-gsutil-image:latest",
 			Args:  []string{"-args", "cp gs://some-bucket /workspace"},
 			Env: []corev1.EnvVar{{
-				Name:  "FIELDNAME",
-				Value: "/var/secret/secretName/key.json",
-			}, {
-				Name:  "GOOGLE_ANOTHER_CREDENTIALS",
+				Name:  "GOOGLE_APPLICATION_CREDENTIALS",
 				Value: "/var/secret/secretName/key.json",
 			}},
 			VolumeMounts: []corev1.VolumeMount{{
@@ -311,15 +298,15 @@ func Test_GetUploadContainerSpec(t *testing.T) {
 			TypeDir:        true,
 			Secrets: []SecretParam{{
 				SecretName: "secretName",
-				FieldName:  "fieldName",
+				FieldName:  "GOOGLE_APPLICATION_CREDENTIALS",
 				SecretKey:  "key.json",
 			}},
 		},
 		wantContainers: []corev1.Container{{
 			Name:  "storage-upload-gcs-valid",
 			Image: "override-with-gsutil-image:latest",
 			Args:  []string{"-args", "cp -r /workspace/* gs://some-bucket"},
-			Env:   []corev1.EnvVar{{Name: "FIELDNAME", Value: "/var/secret/secretName/key.json"}},
+			Env:   []corev1.EnvVar{{Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: "/var/secret/secretName/key.json"}},
 			VolumeMounts: []corev1.VolumeMount{{
 				Name:      "volume-gcs-valid-secretName",
 				MountPath: "/var/secret/secretName",
@@ -333,21 +320,20 @@ func Test_GetUploadContainerSpec(t *testing.T) {
 			DestinationDir: "/workspace",
 			Secrets: []SecretParam{{
 				SecretName: "secretName",
-				FieldName:  "fieldName",
+				FieldName:  "GOOGLE_APPLICATION_CREDENTIALS",
 				SecretKey:  "key.json",
 			}, {
 				SecretKey:  "key.json",
 				SecretName: "secretName",
-				FieldName:  "GOOGLE_ANOTHER_CREDENTIALS",
+				FieldName:  "GOOGLE_APPLICATION_CREDENTIALS",
 			}},
 		},
 		wantContainers: []corev1.Container{{
 			Name:  "storage-upload-gcs-valid",
 			Image: "override-with-gsutil-image:latest",
 			Args:  []string{"-args", "cp /workspace/* gs://some-bucket"},
 			Env: []corev1.EnvVar{
-				{Name: "FIELDNAME", Value: "/var/secret/secretName/key.json"},
-				{Name: "GOOGLE_ANOTHER_CREDENTIALS", Value: "/var/secret/secretName/key.json"},
+				{Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: "/var/secret/secretName/key.json"},
 			},
 			VolumeMounts: []corev1.VolumeMount{{
 				Name:      "volume-gcs-valid-secretName",

diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go
@@ -135,7 +135,7 @@ func setUp() {
 			SecretParams: []v1alpha1.SecretParam{{
 				SecretKey:  "key.json",
 				SecretName: "secret-name",
-				FieldName:  "GOOGLE_CREDENTIALS",
+				FieldName:  "GOOGLE_APPLICATION_CREDENTIALS",
 			}, {
 				SecretKey:  "token",
 				SecretName: "secret-name2",
@@ -967,12 +967,10 @@ func Test_StorageInputResource(t *testing.T) {
 					Args:  []string{"-args", "cp -r gs://fake-bucket/rules.zip/** /workspace/storage-gcs-keys"},
 					VolumeMounts: []corev1.VolumeMount{{
 						Name: "volume-storage-gcs-keys-secret-name", MountPath: "/var/secret/secret-name"}, {
-						Name: "volume-storage-gcs-keys-secret-name2", MountPath: "/var/secret/secret-name2"}, {
 						Name: "workspace", MountPath: "/workspace"},
 					},
 					Env: []corev1.EnvVar{
-						{Name: "GOOGLE_CREDENTIALS", Value: "/var/secret/secret-name/key.json"},
-						{Name: "GOOGLE_TOKEN", Value: "/var/secret/secret-name2/token"},
+						{Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: "/var/secret/secret-name/key.json"},
 					},
 				}},
 				Volumes: []corev1.Volume{{

diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/output_resource_test.go
@@ -82,7 +82,7 @@ func outputResourcesetUp() {
 			SecretParams: []v1alpha1.SecretParam{{
 				SecretKey:  "key.json",
 				SecretName: "sname",
-				FieldName:  "STORAGE_CREDS",
+				FieldName:  "GOOGLE_APPLICATION_CREDENTIALS",
 			}},
 		},
 	}}
@@ -326,7 +326,7 @@ func Test_Valid_OutputResources(t *testing.T) {
 			}},
 			Args: []string{"-args", "cp -r /workspace/faraway-disk/* gs://some-bucket"},
 			Env: []corev1.EnvVar{{
-				Name: "STORAGE_CREDS", Value: "/var/secret/sname/key.json",
+				Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: "/var/secret/sname/key.json",
 			}},
 		}, {
 			Name:         "source-mkdir-source-gcs",
@@ -393,7 +393,7 @@ func Test_Valid_OutputResources(t *testing.T) {
 				Name: "workspace", MountPath: "/workspace",
 			}},
 			Env: []corev1.EnvVar{{
-				Name: "STORAGE_CREDS", Value: "/var/secret/sname/key.json",
+				Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: "/var/secret/sname/key.json",
 			}},
 			Args: []string{"-args", "cp -r /workspace/output/source-workspace/* gs://some-bucket"},
 		}, {
@@ -457,7 +457,7 @@ func Test_Valid_OutputResources(t *testing.T) {
 				Name: "workspace", MountPath: "/workspace",
 			}},
 			Env: []corev1.EnvVar{{
-				Name: "STORAGE_CREDS", Value: "/var/secret/sname/key.json",
+				Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: "/var/secret/sname/key.json",
 			}},
 			Args: []string{"-args", "cp -r /workspace/output/source-workspace/* gs://some-bucket"},
 		}},
@@ -510,7 +510,7 @@ func Test_Valid_OutputResources(t *testing.T) {
 				Name: "workspace", MountPath: "/workspace",
 			}},
 			Env: []corev1.EnvVar{{
-				Name: "STORAGE_CREDS", Value: "/var/secret/sname/key.json",
+				Name: "GOOGLE_APPLICATION_CREDENTIALS", Value: "/var/secret/sname/key.json",
 			}},
 			Args: []string{"-args", "cp -r /workspace/output/source-workspace/* gs://some-bucket"},
 		}},

diff --git a/test/README.md b/test/README.md
@@ -104,17 +104,21 @@ pipelineRunsInformer.Informer().GetIndexer().Add(obj)
 
 ### Setup
 
-Besides the environment variable `KO_DOCKER_REPO`, you may also need the
-permissions inside the Build to run the Kaniko e2e test. If so, setting
-`KANIKO_SECRET_CONFIG_FILE` as the path of the GCP service account JSON key
-which has permissions to push to the registry specified in `KO_DOCKER_REPO` will
-enable Kaniko to use those credentials when pushing.
+Besides the environment variable `KO_DOCKER_REPO`, you may also need the permissions 
+inside the TaskRun to run the Kaniko e2e test and GCS taskrun test.
+- In Kaniko e2e test, setting `KANIKO_SECRET_CONFIG_FILE` as the path of the GCP service account 
+JSON key which has permissions to push to the registry specified in `KO_DOCKER_REPO` will 
+enable Kaniko to use those credentials when pushing an image.
+- In GCS taskrun test, GCP service account JSON key file at path `KANIKO_SECRET_CONFIG_FILE` is used to generate 
+Kubernetes secret to access GCS bucket. This e2e test requires valid service account configuration 
+json but it does not require any role binding. 
 
-To create a service account usable in the e2e tests:
+To reduce e2e test setup developers can use the same environment variable for both Kaniko e2e test and GCS taskrun test. To create a service account usable in the e2e tests:
 
-```shell
+```bash
 PROJECT_ID=your-gcp-project
 ACCOUNT_NAME=service-account-name
+# gcloud configure project
 gcloud config set project $PROJECT_ID
 
 # create the service account