From 4d629484ca894c9ca03cd315bf6ac93d3a71a8d0 Mon Sep 17 00:00:00 2001 From: Scott Date: Thu, 7 May 2020 10:50:33 -0400 Subject: [PATCH] Git PipelineResource errs when HOME != /tekton/home & UID is non-zero When the disable-home-env-overwrite flag is "true"... Currently the git pipelineresource errors out if a securityContext is set on a TaskRun. This happens because it will attempt to lock $HOME/.gitconfig. If UID is non-zero and HOME is not set then it tries to lock `//.gitconfig` which is in a root-owned directory. This commit sets an explicit home directory of /tekton/home for the git pipelineresource. This volume mount always exists, even if the user has set the disable-home-env-overwrite flag to "true", and it's always world-writeable since it's an emptyDir with no permissions set on it. --- pkg/apis/pipeline/paths.go | 2 + .../resource/v1alpha1/git/git_resource.go | 3 ++ .../v1alpha1/git/git_resource_test.go | 9 ++++ pkg/pod/creds_init.go | 3 +- pkg/pod/pod.go | 6 +-- pkg/pod/pod_test.go | 2 +- .../taskrun/resources/input_resource_test.go | 42 +++++++++++++++---- pkg/reconciler/taskrun/taskrun_test.go | 9 ++++ 8 files changed, 61 insertions(+), 15 deletions(-) diff --git a/pkg/apis/pipeline/paths.go b/pkg/apis/pipeline/paths.go index 37d8305aa7e..c0c62f73776 100644 --- a/pkg/apis/pipeline/paths.go +++ b/pkg/apis/pipeline/paths.go @@ -21,4 +21,6 @@ const ( WorkspaceDir = "/workspace" // DefaultResultPath is the path for task result DefaultResultPath = "/tekton/results" + // HomeDir is the HOME directory of PipelineResources + HomeDir = "/tekton/home" ) diff --git a/pkg/apis/resource/v1alpha1/git/git_resource.go b/pkg/apis/resource/v1alpha1/git/git_resource.go index 5e0f16df3a0..f80adf26551 100644 --- a/pkg/apis/resource/v1alpha1/git/git_resource.go +++ b/pkg/apis/resource/v1alpha1/git/git_resource.go @@ -169,6 +169,9 @@ func (s *Resource) GetInputTaskModifier(_ *v1beta1.TaskSpec, path string) (v1bet env := []corev1.EnvVar{{ Name: "TEKTON_RESOURCE_NAME", Value: s.Name, + }, { + Name: "HOME", + Value: pipeline.HomeDir, }} if len(s.HTTPProxy) != 0 { diff --git a/pkg/apis/resource/v1alpha1/git/git_resource_test.go b/pkg/apis/resource/v1alpha1/git/git_resource_test.go index 63419e7e01d..1cd0dae1b00 100644 --- a/pkg/apis/resource/v1alpha1/git/git_resource_test.go +++ b/pkg/apis/resource/v1alpha1/git/git_resource_test.go @@ -21,6 +21,7 @@ import ( "github.com/google/go-cmp/cmp" tb "github.com/tektoncd/pipeline/internal/builder/v1beta1" + pipeline "github.com/tektoncd/pipeline/pkg/apis/pipeline" resourcev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1/git" @@ -401,6 +402,7 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) { WorkingDir: "/workspace", Env: []corev1.EnvVar{ {Name: "TEKTON_RESOURCE_NAME", Value: "git-resource"}, + {Name: "HOME", Value: pipeline.HomeDir}, {Name: "HTTP_PROXY", Value: "http-proxy.git.com"}, {Name: "HTTPS_PROXY", Value: "https-proxy.git.com"}, {Name: "NO_PROXY", Value: "no-proxy.git.com"}, @@ -438,6 +440,7 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) { WorkingDir: "/workspace", Env: []corev1.EnvVar{ {Name: "TEKTON_RESOURCE_NAME", Value: "git-resource"}, + {Name: "HOME", Value: pipeline.HomeDir}, {Name: "HTTP_PROXY", Value: "http-proxy.git.com"}, {Name: "HTTPS_PROXY", Value: "https-proxy.git.com"}, {Name: "NO_PROXY", Value: "no-proxy.git.com"}, @@ -476,6 +479,7 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) { WorkingDir: "/workspace", Env: []corev1.EnvVar{ {Name: "TEKTON_RESOURCE_NAME", Value: "git-resource"}, + {Name: "HOME", Value: pipeline.HomeDir}, {Name: "HTTP_PROXY", Value: "http-proxy.git.com"}, {Name: "HTTPS_PROXY", Value: "https-proxy.git.com"}, {Name: "NO_PROXY", Value: "no-proxy.git.com"}, @@ -514,6 +518,7 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) { WorkingDir: "/workspace", Env: []corev1.EnvVar{ {Name: "TEKTON_RESOURCE_NAME", Value: "git-resource"}, + {Name: "HOME", Value: pipeline.HomeDir}, {Name: "HTTP_PROXY", Value: "http-proxy.git.com"}, {Name: "HTTPS_PROXY", Value: "https-proxy.git.com"}, {Name: "NO_PROXY", Value: "no-proxy.git.com"}, @@ -551,6 +556,7 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) { WorkingDir: "/workspace", Env: []corev1.EnvVar{ {Name: "TEKTON_RESOURCE_NAME", Value: "git-resource"}, + {Name: "HOME", Value: pipeline.HomeDir}, {Name: "HTTPS_PROXY", Value: "https-proxy.git.com"}, {Name: "NO_PROXY", Value: "no-proxy.git.com"}, }, @@ -587,6 +593,7 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) { WorkingDir: "/workspace", Env: []corev1.EnvVar{ {Name: "TEKTON_RESOURCE_NAME", Value: "git-resource"}, + {Name: "HOME", Value: pipeline.HomeDir}, {Name: "HTTP_PROXY", Value: "http-proxy.git.com"}, {Name: "NO_PROXY", Value: "no-proxy.git.com"}, }, @@ -623,6 +630,7 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) { WorkingDir: "/workspace", Env: []corev1.EnvVar{ {Name: "TEKTON_RESOURCE_NAME", Value: "git-resource"}, + {Name: "HOME", Value: pipeline.HomeDir}, {Name: "HTTP_PROXY", Value: "http-proxy.git.com"}, {Name: "HTTPS_PROXY", Value: "https-proxy.git.com"}, }, @@ -661,6 +669,7 @@ func TestGitResource_GetDownloadTaskModifier(t *testing.T) { WorkingDir: "/workspace", Env: []corev1.EnvVar{ {Name: "TEKTON_RESOURCE_NAME", Value: "git-resource"}, + {Name: "HOME", Value: pipeline.HomeDir}, {Name: "HTTP_PROXY", Value: "http-proxy.git.com"}, {Name: "HTTPS_PROXY", Value: "https-proxy.git.com"}, {Name: "NO_PROXY", Value: "no-proxy.git.com"}, diff --git a/pkg/pod/creds_init.go b/pkg/pod/creds_init.go index d9742c31a43..3940a3e4ae6 100644 --- a/pkg/pod/creds_init.go +++ b/pkg/pod/creds_init.go @@ -19,6 +19,7 @@ package pod import ( "fmt" + "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/credentials" "github.com/tektoncd/pipeline/pkg/credentials/dockercreds" "github.com/tektoncd/pipeline/pkg/credentials/gitcreds" @@ -120,7 +121,7 @@ func credsInit(credsImage string, serviceAccountName, namespace string, kubeclie // with /tekton/home. func CredentialsPath(shouldOverrideHomeEnv bool) string { if shouldOverrideHomeEnv { - return homeDir + return pipeline.HomeDir } return credsInitHomeDir } diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index f42b322cade..98ffae2c62a 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -42,8 +42,6 @@ func GetFeatureFlagsConfigName() string { } const ( - homeDir = "/tekton/home" - // ResultsDir is the folder used by default to create the results file ResultsDir = "/tekton/results" @@ -69,7 +67,7 @@ var ( MountPath: pipeline.WorkspaceDir, }, { Name: "tekton-internal-home", - MountPath: homeDir, + MountPath: pipeline.HomeDir, }, { Name: "tekton-internal-results", MountPath: ResultsDir, @@ -101,7 +99,7 @@ func MakePod(images pipeline.Images, taskRun *v1beta1.TaskRun, taskSpec v1beta1. if overrideHomeEnv { implicitEnvVars = append(implicitEnvVars, corev1.EnvVar{ Name: "HOME", - Value: homeDir, + Value: pipeline.HomeDir, }) } else { // Add the volume that creds-init will write to when diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index 89e35b769b1..1a41b6ac942 100644 --- a/pkg/pod/pod_test.go +++ b/pkg/pod/pod_test.go @@ -48,7 +48,7 @@ func TestMakePod(t *testing.T) { implicitEnvVars := []corev1.EnvVar{{ Name: "HOME", - Value: homeDir, + Value: pipeline.HomeDir, }} secretsVolumeMount := corev1.VolumeMount{ Name: "tekton-internal-secret-volume-multi-creds-9l9zj", diff --git a/pkg/reconciler/taskrun/resources/input_resource_test.go b/pkg/reconciler/taskrun/resources/input_resource_test.go index 1ddfa11972a..49e2baac206 100644 --- a/pkg/reconciler/taskrun/resources/input_resource_test.go +++ b/pkg/reconciler/taskrun/resources/input_resource_test.go @@ -370,7 +370,10 @@ func TestAddInputResourceToTask(t *testing.T) { Command: []string{"/ko-app/git-init"}, Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "master", "-path", "/workspace/gitspace"}, WorkingDir: "/workspace", - Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "the-git"}}, + Env: []corev1.EnvVar{ + {Name: "TEKTON_RESOURCE_NAME", Value: "the-git"}, + {Name: "HOME", Value: pipeline.HomeDir}, + }, }}}, Resources: &v1beta1.TaskResources{ Inputs: gitInputs, @@ -408,7 +411,10 @@ func TestAddInputResourceToTask(t *testing.T) { Command: []string{"/ko-app/git-init"}, Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/gitspace"}, WorkingDir: "/workspace", - Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}}, + Env: []corev1.EnvVar{ + {Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}, + {Name: "HOME", Value: pipeline.HomeDir}, + }, }}}, Resources: &v1beta1.TaskResources{ Inputs: gitInputs, @@ -453,14 +459,20 @@ func TestAddInputResourceToTask(t *testing.T) { Command: []string{"/ko-app/git-init"}, Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/gitspace"}, WorkingDir: "/workspace", - Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}}, + Env: []corev1.EnvVar{ + {Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}, + {Name: "HOME", Value: pipeline.HomeDir}, + }, }}, {Container: corev1.Container{ Name: "git-source-the-git-with-branch-9l9zj", Image: "override-with-git:latest", Command: []string{"/ko-app/git-init"}, Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/git-duplicate-space"}, WorkingDir: "/workspace", - Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}}, + Env: []corev1.EnvVar{ + {Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}, + {Name: "HOME", Value: pipeline.HomeDir}, + }, }}}, Resources: &v1beta1.TaskResources{ Inputs: multipleGitInputs, @@ -498,14 +510,17 @@ func TestAddInputResourceToTask(t *testing.T) { Command: []string{"/ko-app/git-init"}, Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "master", "-path", "/workspace/gitspace"}, WorkingDir: "/workspace", - Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "the-git"}}, + Env: []corev1.EnvVar{ + {Name: "TEKTON_RESOURCE_NAME", Value: "the-git"}, + {Name: "HOME", Value: pipeline.HomeDir}, + }, }}}, Resources: &v1beta1.TaskResources{ Inputs: gitInputs, }, }, }, { - desc: "set revision to provdided branch", + desc: "set revision to provided branch", task: task, taskRun: &v1beta1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ @@ -536,7 +551,10 @@ func TestAddInputResourceToTask(t *testing.T) { Command: []string{"/ko-app/git-init"}, Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/gitspace"}, WorkingDir: "/workspace", - Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}}, + Env: []corev1.EnvVar{ + {Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}, + {Name: "HOME", Value: pipeline.HomeDir}, + }, }}}, Resources: &v1beta1.TaskResources{ Inputs: gitInputs, @@ -622,7 +640,10 @@ func TestAddInputResourceToTask(t *testing.T) { Command: []string{"/ko-app/git-init"}, Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/gitspace", "-sslVerify=false"}, WorkingDir: "/workspace", - Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-sslVerify-false"}}, + Env: []corev1.EnvVar{ + {Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-sslVerify-false"}, + {Name: "HOME", Value: pipeline.HomeDir}, + }, }}}, Resources: &v1beta1.TaskResources{ Inputs: gitInputs, @@ -922,7 +943,10 @@ gsutil cp gs://fake-bucket/rules.zip /workspace/gcs-dir Command: []string{"/ko-app/git-init"}, Args: []string{"-url", "https://github.com/grafeas/kritis", "-revision", "branch", "-path", "/workspace/gitspace"}, WorkingDir: "/workspace", - Env: []corev1.EnvVar{{Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}}, + Env: []corev1.EnvVar{ + {Name: "TEKTON_RESOURCE_NAME", Value: "the-git-with-branch"}, + {Name: "HOME", Value: pipeline.HomeDir}, + }, }}}, Resources: &v1beta1.TaskResources{ Inputs: optionalGitInputs, diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index a2864db453d..b0caca0372a 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -732,6 +732,7 @@ func TestReconcile(t *testing.T) { tb.WorkingDir(workspaceDir), tb.EnvVar("HOME", "/tekton/home"), tb.EnvVar("TEKTON_RESOURCE_NAME", "git-resource"), + tb.EnvVar("HOME", "/tekton/home"), tb.VolumeMount("tekton-internal-tools", "/tekton/tools"), tb.VolumeMount("tekton-internal-workspace", workspaceDir), tb.VolumeMount("tekton-internal-home", "/tekton/home"), @@ -816,8 +817,12 @@ func TestReconcile(t *testing.T) { "/workspace/workspace", ), tb.WorkingDir(workspaceDir), + // Note: the duplication of HOME env var here is intentional: our pod builder + // adds it first and the git pipelineresource adds its own to ensure that HOME + // is set even when disable-home-env-overwrite feature flag is "true". tb.EnvVar("HOME", "/tekton/home"), tb.EnvVar("TEKTON_RESOURCE_NAME", "git-resource"), + tb.EnvVar("HOME", "/tekton/home"), tb.VolumeMount("tekton-internal-tools", "/tekton/tools"), tb.VolumeMount("tekton-internal-downward", "/tekton/downward"), tb.VolumeMount("tekton-internal-workspace", workspaceDir), @@ -920,8 +925,12 @@ func TestReconcile(t *testing.T) { "-path", "/workspace/workspace"), tb.WorkingDir(workspaceDir), + // Note: the duplication of HOME env var here is intentional: our pod builder + // adds it first and the git pipelineresource adds its own to ensure that HOME + // is set even when disable-home-env-overwrite feature flag is "true". tb.EnvVar("HOME", "/tekton/home"), tb.EnvVar("TEKTON_RESOURCE_NAME", "workspace"), + tb.EnvVar("HOME", "/tekton/home"), tb.VolumeMount("tekton-internal-tools", "/tekton/tools"), tb.VolumeMount("tekton-internal-downward", "/tekton/downward"), tb.VolumeMount("tekton-internal-workspace", workspaceDir),