From 4308ef26d4e01f14849b986f50d9880a8ca8f266 Mon Sep 17 00:00:00 2001 From: Piyush Garg Date: Mon, 30 Nov 2020 08:43:44 +0530 Subject: [PATCH] Add support for env in podtemplate This will help to specificy env default, env at pipelinerun and taskrun level. Env in pod template at taskrun and pipelinerun level will take precedence over one defined in step and stepTemplate Env at pipelinerun and taskrun level will override the default podtemplate Fix #1606 --- docs/podtemplates.md | 4 + .../v1beta1/taskruns/podtemplate-env.yaml | 15 + pkg/apis/pipeline/pod/template.go | 6 + .../pipeline/pod/zz_generated.deepcopy.go | 7 + .../pipeline/v1beta1/openapi_generated.go | 21 +- pkg/apis/pipeline/v1beta1/swagger.json | 9 + pkg/pod/pod.go | 43 +++ pkg/pod/pod_test.go | 258 +++++++++++++++++- test/artifact_bucket_test.go | 6 +- 9 files changed, 363 insertions(+), 6 deletions(-) create mode 100644 examples/v1beta1/taskruns/podtemplate-env.yaml diff --git a/docs/podtemplates.md b/docs/podtemplates.md index 42850e55555..47b179f9f6a 100644 --- a/docs/podtemplates.md +++ b/docs/podtemplates.md @@ -49,6 +49,10 @@ Pod templates support fields listed in the table below. volumes Specifies a list of volumes that containers within the Pod can mount. This allows you to specify a volume type for each volumeMount in a Task. + + envs + Specifies a list of environments that all containers within the Pod will be populated + runtimeClassName Specifies the runtime class for the Pod. diff --git a/examples/v1beta1/taskruns/podtemplate-env.yaml b/examples/v1beta1/taskruns/podtemplate-env.yaml new file mode 100644 index 00000000000..060b29c9bd4 --- /dev/null +++ b/examples/v1beta1/taskruns/podtemplate-env.yaml @@ -0,0 +1,15 @@ +apiVersion: tekton.dev/v1beta1 +kind: TaskRun +metadata: + generateName: podtemplate-env- +spec: + podTemplate: + env: + - name: MY_VAR1 + value: foo + taskSpec: + steps: + - image: ubuntu + script: | + #!/usr/bin/env bash + [[ $MY_VAR1 == foo ]] diff --git a/pkg/apis/pipeline/pod/template.go b/pkg/apis/pipeline/pod/template.go index f5244b795f5..c24526b1f4c 100644 --- a/pkg/apis/pipeline/pod/template.go +++ b/pkg/apis/pipeline/pod/template.go @@ -52,6 +52,12 @@ type Template struct { // +patchStrategy=merge,retainKeys Volumes []corev1.Volume `json:"volumes,omitempty" patchStrategy:"merge,retainKeys" patchMergeKey:"name" protobuf:"bytes,1,rep,name=volumes"` + // List of environment variables that can be provided to the containers belonging to the pod. + // +optional + // +patchMergeKey=name + // +patchStrategy=merge,retainKeys + Env []corev1.EnvVar `json:"env,omitempty" patchStrategy:"merge,retainKeys" patchMergeKey:"name" protobuf:"bytes,1,rep,name=env"` + // RuntimeClassName refers to a RuntimeClass object in the node.k8s.io // group, which should be used to run this pod. If no RuntimeClass resource // matches the named class, the pod will not be run. If unset or empty, the diff --git a/pkg/apis/pipeline/pod/zz_generated.deepcopy.go b/pkg/apis/pipeline/pod/zz_generated.deepcopy.go index 77cb864fedb..4c1e1855437 100644 --- a/pkg/apis/pipeline/pod/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/pod/zz_generated.deepcopy.go @@ -58,6 +58,13 @@ func (in *Template) DeepCopyInto(out *Template) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Env != nil { + in, out := &in.Env, &out.Env + *out = make([]v1.EnvVar, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.RuntimeClassName != nil { in, out := &in.RuntimeClassName, &out.RuntimeClassName *out = new(string) diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index e06d6844bd2..58aa2398aaa 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -175,6 +175,25 @@ func schema_pkg_apis_pipeline_pod_Template(ref common.ReferenceCallback) common. }, }, }, + "env": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-patch-merge-key": "name", + "x-kubernetes-patch-strategy": "merge,retainKeys", + }, + }, + SchemaProps: spec.SchemaProps{ + Description: "List of environment variables that can be provided to the containers belonging to the pod.", + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Ref: ref("k8s.io/api/core/v1.EnvVar"), + }, + }, + }, + }, + }, "runtimeClassName": { SchemaProps: spec.SchemaProps{ Description: "RuntimeClassName refers to a RuntimeClass object in the node.k8s.io group, which should be used to run this pod. If no RuntimeClass resource matches the named class, the pod will not be run. If unset or empty, the \"legacy\" RuntimeClass will be used, which is an implicit class with an empty definition that uses the default runtime handler. More info: https://git.k8s.io/enhancements/keps/sig-node/runtime-class.md This is a beta feature as of Kubernetes v1.14.", @@ -247,7 +266,7 @@ func schema_pkg_apis_pipeline_pod_Template(ref common.ReferenceCallback) common. }, }, Dependencies: []string{ - "k8s.io/api/core/v1.Affinity", "k8s.io/api/core/v1.LocalObjectReference", "k8s.io/api/core/v1.PodDNSConfig", "k8s.io/api/core/v1.PodSecurityContext", "k8s.io/api/core/v1.Toleration", "k8s.io/api/core/v1.Volume"}, + "k8s.io/api/core/v1.Affinity", "k8s.io/api/core/v1.EnvVar", "k8s.io/api/core/v1.LocalObjectReference", "k8s.io/api/core/v1.PodDNSConfig", "k8s.io/api/core/v1.PodSecurityContext", "k8s.io/api/core/v1.Toleration", "k8s.io/api/core/v1.Volume"}, } } diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index e2156137759..a74d63050f6 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -31,6 +31,15 @@ "description": "EnableServiceLinks indicates whether information about services should be injected into pod's environment variables, matching the syntax of Docker links. Optional: Defaults to true.", "type": "boolean" }, + "env": { + "description": "List of environment variables that can be provided to the containers belonging to the pod.", + "type": "array", + "items": { + "$ref": "#/definitions/v1.EnvVar" + }, + "x-kubernetes-patch-merge-key": "name", + "x-kubernetes-patch-strategy": "merge,retainKeys" + }, "hostNetwork": { "description": "HostNetwork specifies whether the pod may use the node network namespace", "type": "boolean" diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index 5932c133c25..d399e9b1295 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -217,6 +217,30 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec podTemplate = *taskRun.Spec.PodTemplate } + // Add envs specified in podtemplate to all step containers + // Envs specified in podtemplate should take precedence over + // the one specified in step and step template + for i := range stepContainers { + envs := updateAndMergeEnv(stepContainers[i].Env, podTemplate.Env) + stepContainers[i].Env = envs + } + + // Add envs specified in podtemplate to all sidecar containers + // Envs specified in podtemplate should take precedence over + // the one specified in sidecar and step template + for i := range sidecarContainers { + envs := updateAndMergeEnv(sidecarContainers[i].Env, podTemplate.Env) + sidecarContainers[i].Env = envs + } + + // Add envs specified in podtemplate to all initContainers + // Envs specified in podtemplate should take precedence over + // the one specified in init and step template + for i := range initContainers { + envs := updateAndMergeEnv(initContainers[i].Env, podTemplate.Env) + initContainers[i].Env = envs + } + // Add podTemplate Volumes to the explicitly declared use volumes volumes = append(volumes, taskSpec.Volumes...) volumes = append(volumes, podTemplate.Volumes...) @@ -403,3 +427,22 @@ func shouldAddReadyAnnotationOnPodCreate(ctx context.Context, sidecars []v1beta1 cfg := config.FromContextOrDefaults(ctx) return !cfg.FeatureFlags.RunningInEnvWithInjectedSidecars } + +// updateAndMergeEnv will merge two slices of env +// precedence will be given to second input if exist with same name key +func updateAndMergeEnv(containerenvs []corev1.EnvVar, podtemplateEnvs []corev1.EnvVar) []corev1.EnvVar { + for _, env := range podtemplateEnvs { + var updated bool + for i := range containerenvs { + if env.Name == containerenvs[i].Name { + containerenvs[i].Value = env.Value + containerenvs[i].ValueFrom = env.ValueFrom + updated = true + } + } + if !updated { + containerenvs = append(containerenvs, env) + } + } + return containerenvs +} diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index 4194bd564ad..8cdfbd4514a 100644 --- a/pkg/pod/pod_test.go +++ b/pkg/pod/pod_test.go @@ -62,6 +62,16 @@ func TestPodBuild(t *testing.T) { Name: "HOME", Value: homeDir, }} + implicitEnvVarsWithOtherEnv := []corev1.EnvVar{ + { + Name: "HOME", + Value: homeDir, + }, + { + Name: "FOO", + Value: "bar", + }, + } secretsVolume := corev1.Volume{ Name: "tekton-internal-secret-volume-multi-creds-9l9zj", VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: "multi-creds"}}, @@ -72,6 +82,20 @@ func TestPodBuild(t *testing.T) { Command: []string{"/ko-app/entrypoint", "cp", "/ko-app/entrypoint", "/tekton/tools/entrypoint"}, VolumeMounts: []corev1.VolumeMount{toolsMount}, } + placeToolsInitWithEnv := corev1.Container{ + Name: "place-tools", + Image: images.EntrypointImage, + Command: []string{"/ko-app/entrypoint", "cp", "/ko-app/entrypoint", "/tekton/tools/entrypoint"}, + VolumeMounts: []corev1.VolumeMount{toolsMount}, + Env: []corev1.EnvVar{{Name: "FOO", Value: "bar"}}, + } + placeToolsInitWithHomeEnv := corev1.Container{ + Name: "place-tools", + Image: images.EntrypointImage, + Command: []string{"/ko-app/entrypoint", "cp", "/ko-app/entrypoint", "/tekton/tools/entrypoint"}, + VolumeMounts: []corev1.VolumeMount{toolsMount}, + Env: []corev1.EnvVar{{Name: "HOME", Value: "/tekton/home/new"}, {Name: "FOO", Value: "bar"}}, + } runtimeClassName := "gvisor" automountServiceAccountToken := false dnsPolicy := corev1.DNSNone @@ -245,6 +269,12 @@ func TestPodBuild(t *testing.T) { {Name: "net.ipv4.tcp_syncookies", Value: "1"}, }, }, + Env: []corev1.EnvVar{ + { + Name: "FOO", + Value: "bar", + }, + }, RuntimeClassName: &runtimeClassName, AutomountServiceAccountToken: &automountServiceAccountToken, DNSPolicy: &dnsPolicy, @@ -258,7 +288,7 @@ func TestPodBuild(t *testing.T) { }, want: &corev1.PodSpec{ RestartPolicy: corev1.RestartPolicyNever, - InitContainers: []corev1.Container{placeToolsInit}, + InitContainers: []corev1.Container{placeToolsInitWithEnv}, Containers: []corev1.Container{{ Name: "step-name", Image: "image", @@ -275,7 +305,101 @@ func TestPodBuild(t *testing.T) { "cmd", "--", }, - Env: implicitEnvVars, + Env: implicitEnvVarsWithOtherEnv, + VolumeMounts: append([]corev1.VolumeMount{ + toolsMount, + downwardMount, + {Name: "tekton-creds-init-home-9l9zj", MountPath: "/tekton/creds"}, + }, implicitVolumeMounts...), + WorkingDir: pipeline.WorkspaceDir, + Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, + TerminationMessagePath: "/tekton/termination", + }}, + Volumes: append(implicitVolumes, toolsVolume, downwardVolume, corev1.Volume{ + Name: "tekton-creds-init-home-9l9zj", + VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}}, + }), + SecurityContext: &corev1.PodSecurityContext{ + Sysctls: []corev1.Sysctl{ + {Name: "net.ipv4.tcp_syncookies", Value: "1"}, + }, + }, + RuntimeClassName: &runtimeClassName, + AutomountServiceAccountToken: &automountServiceAccountToken, + DNSPolicy: dnsPolicy, + DNSConfig: &corev1.PodDNSConfig{ + Nameservers: []string{"8.8.8.8"}, + Searches: []string{"tekton.local"}, + }, + EnableServiceLinks: &enableServiceLinks, + PriorityClassName: priorityClassName, + }, + }, { + desc: "with-pod-template-overide", + ts: v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{Container: corev1.Container{ + Name: "name", + Image: "image", + Command: []string{"cmd"}, // avoid entrypoint lookup. + }}}, + }, + trs: v1beta1.TaskRunSpec{ + PodTemplate: &pod.Template{ + SecurityContext: &corev1.PodSecurityContext{ + Sysctls: []corev1.Sysctl{ + {Name: "net.ipv4.tcp_syncookies", Value: "1"}, + }, + }, + Env: []corev1.EnvVar{ + { + Name: "HOME", + Value: "/tekton/home/new", + }, + { + Name: "FOO", + Value: "bar", + }, + }, + RuntimeClassName: &runtimeClassName, + AutomountServiceAccountToken: &automountServiceAccountToken, + DNSPolicy: &dnsPolicy, + DNSConfig: &corev1.PodDNSConfig{ + Nameservers: []string{"8.8.8.8"}, + Searches: []string{"tekton.local"}, + }, + EnableServiceLinks: &enableServiceLinks, + PriorityClassName: &priorityClassName, + }, + }, + want: &corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, + InitContainers: []corev1.Container{placeToolsInitWithHomeEnv}, + Containers: []corev1.Container{{ + Name: "step-name", + Image: "image", + Command: []string{"/tekton/tools/entrypoint"}, + Args: []string{ + "-wait_file", + "/tekton/downward/ready", + "-wait_file_content", + "-post_file", + "/tekton/tools/0", + "-termination_path", + "/tekton/termination", + "-entrypoint", + "cmd", + "--", + }, + Env: []corev1.EnvVar{ + { + Name: "HOME", + Value: "/tekton/home/new", + }, + { + Name: "FOO", + Value: "bar", + }, + }, VolumeMounts: append([]corev1.VolumeMount{ toolsMount, downwardMount, @@ -1439,3 +1563,133 @@ func TestShouldAddReadyAnnotationonPodCreate(t *testing.T) { }) } } + +func TestUpdateAndMergeEnv(t *testing.T) { + tcs := []struct { + name string + containerEnv []corev1.EnvVar + podtemplateEnv []corev1.EnvVar + env []corev1.EnvVar + }{ + { + name: "different env", + containerEnv: []corev1.EnvVar{ + { + Name: "key1", + Value: "value1", + }, + }, + podtemplateEnv: []corev1.EnvVar{ + { + Name: "key", + Value: "value", + }, + }, + env: []corev1.EnvVar{ + { + Name: "key1", + Value: "value1", + }, + { + Name: "key", + Value: "value", + }, + }, + }, + { + name: "podtemplate env empty", + containerEnv: []corev1.EnvVar{ + { + Name: "key1", + Value: "value1", + }, + }, + podtemplateEnv: []corev1.EnvVar{}, + env: []corev1.EnvVar{ + { + Name: "key1", + Value: "value1", + }, + }, + }, + { + name: "containerTemplate env empty", + containerEnv: []corev1.EnvVar{}, + podtemplateEnv: []corev1.EnvVar{ + { + Name: "key", + Value: "value", + }, + }, + env: []corev1.EnvVar{ + { + Name: "key", + Value: "value", + }, + }, + }, + { + name: "both env empty", + containerEnv: []corev1.EnvVar{}, + podtemplateEnv: []corev1.EnvVar{}, + env: []corev1.EnvVar{}, + }, + { + name: "merge and update", + containerEnv: []corev1.EnvVar{ + { + Name: "key1", + Value: "value1", + }, + { + Name: "key2", + Value: "value2", + }, + { + Name: "key3", + Value: "value3", + }, + }, + podtemplateEnv: []corev1.EnvVar{ + { + Name: "key", + Value: "value", + }, + { + Name: "key2", + Value: "value4", + }, + { + Name: "key3", + Value: "value6", + }, + }, + env: []corev1.EnvVar{ + { + Name: "key1", + Value: "value1", + }, + { + Name: "key2", + Value: "value4", + }, + { + Name: "key3", + Value: "value6", + }, + { + Name: "key", + Value: "value", + }, + }, + }, + } + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + result := updateAndMergeEnv(tc.containerEnv, tc.podtemplateEnv) + if d := cmp.Diff(result, tc.env); d != "" { + t.Errorf("expected: %v Received: %v", tc.env, result) + } + }) + } +} diff --git a/test/artifact_bucket_test.go b/test/artifact_bucket_test.go index e7af76d07bc..b03985a5767 100644 --- a/test/artifact_bucket_test.go +++ b/test/artifact_bucket_test.go @@ -85,9 +85,6 @@ func TestStorageBucketPipelineRun(t *testing.T) { Name: "bucket-secret-volume", MountPath: fmt.Sprintf("/var/secret/%s", bucketSecretName), }}, - Env: []corev1.EnvVar{{ - Name: "CREDENTIALS", Value: fmt.Sprintf("/var/secret/%s/%s", bucketSecretName, bucketSecretKey), - }}, }}}, Volumes: []corev1.Volume{{ Name: "bucket-secret-volume", @@ -109,6 +106,9 @@ func TestStorageBucketPipelineRun(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "createbuckettaskrun", Namespace: namespace}, Spec: v1beta1.TaskRunSpec{ TaskRef: &v1beta1.TaskRef{Name: "createbuckettask"}, + PodTemplate: &v1beta1.PodTemplate{ + Env: []corev1.EnvVar{{Name: "CREDENTIALS", Value: fmt.Sprintf("/var/secret/%s/%s", bucketSecretName, bucketSecretKey)}}, + }, }, }