From 2d803afc980ad33d0c17b68af78bd4903200b96b Mon Sep 17 00:00:00 2001 From: Michael Fraenkel Date: Tue, 29 Nov 2016 21:57:35 -0500 Subject: [PATCH] ConfigMaps populate environment variables --- pkg/api/testing/fuzzer.go | 11 ++ pkg/api/types.go | 28 +++++ pkg/api/v1/types.go | 28 +++++ pkg/api/validation/validation.go | 24 +++++ pkg/api/validation/validation_test.go | 59 ++++++++++ pkg/kubectl/describe.go | 18 ++++ pkg/kubectl/describe_test.go | 13 ++- pkg/kubelet/kubelet_pods.go | 51 ++++++++- pkg/kubelet/kubelet_pods_test.go | 148 +++++++++++++++++++++++++- test/e2e/common/configmap.go | 54 ++++++++++ 10 files changed, 424 insertions(+), 10 deletions(-) diff --git a/pkg/api/testing/fuzzer.go b/pkg/api/testing/fuzzer.go index 6c993c4a26f..94bb5623be2 100644 --- a/pkg/api/testing/fuzzer.go +++ b/pkg/api/testing/fuzzer.go @@ -376,6 +376,17 @@ func FuzzerFor(t *testing.T, version schema.GroupVersion, src rand.Source) *fuzz ev.ValueFrom.FieldRef.FieldPath = c.RandString() } }, + func(ev *api.EnvFromSource, c fuzz.Continue) { + if c.RandBool() { + ev.Prefix = "p_" + } + if c.RandBool() { + c.Fuzz(&ev.ConfigMapRef) + } + }, + func(cm *api.ConfigMapEnvSource, c fuzz.Continue) { + c.FuzzNoCustom(cm) // fuzz self without calling this function again + }, func(sc *api.SecurityContext, c fuzz.Continue) { c.FuzzNoCustom(sc) // fuzz self without calling this function again if c.RandBool() { diff --git a/pkg/api/types.go b/pkg/api/types.go index 80e88f91113..d830223695a 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -1133,6 +1133,26 @@ type SecretKeySelector struct { Key string } +// EnvFromSource represents the source of a set of ConfigMaps +type EnvFromSource struct { + // An optional identifier to prepend to each key in the ConfigMap. Must be a C_IDENTIFIER. + // +optional + Prefix string + // The ConfigMap to select from. + //+optional + ConfigMapRef *ConfigMapEnvSource +} + +// ConfigMapEnvSource selects a ConfigMap to populate the environment +// variables with. +// +// The contents of the target ConfigMap's Data field will represent the +// key-value pairs as environment variables. +type ConfigMapEnvSource struct { + // The ConfigMap to select from. + LocalObjectReference +} + // HTTPHeader describes a custom header to be used in HTTP probes type HTTPHeader struct { // The header field name @@ -1274,6 +1294,14 @@ type Container struct { WorkingDir string // +optional Ports []ContainerPort + // List of sources to populate environment variables in the container. + // The keys defined within a source must be a C_IDENTIFIER. An invalid key + // will prevent the container from starting. When a key exists in multiple + // sources, the value associated with the last source will take precedence. + // Values defined by an Env with a duplicate key will take precedence. + // Cannot be updated. + // +optional + EnvFrom []EnvFromSource // +optional Env []EnvVar // Compute resource requirements. diff --git a/pkg/api/v1/types.go b/pkg/api/v1/types.go index 168f5994348..67af1d17bb7 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -1233,6 +1233,26 @@ type SecretKeySelector struct { Key string `json:"key" protobuf:"bytes,2,opt,name=key"` } +// EnvFromSource represents the source of a set of ConfigMaps +type EnvFromSource struct { + // An optional identifer to prepend to each key in the ConfigMap. Must be a C_IDENTIFIER. + // +optional + Prefix string `json:"prefix,omitempty" protobuf:"bytes,1,opt,name=prefix"` + // The ConfigMap to select from + // +optional + ConfigMapRef *ConfigMapEnvSource `json:"configMapRef,omitempty" protobuf:"bytes,2,opt,name=configMapRef"` +} + +// ConfigMapEnvSource selects a ConfigMap to populate the environment +// variables with. +// +// The contents of the target ConfigMap's Data field will represent the +// key-value pairs as environment variables. +type ConfigMapEnvSource struct { + // The ConfigMap to select from. + LocalObjectReference `json:",inline" protobuf:"bytes,1,opt,name=localObjectReference"` +} + // HTTPHeader describes a custom header to be used in HTTP probes type HTTPHeader struct { // The header field name @@ -1409,6 +1429,14 @@ type Container struct { // Cannot be updated. // +optional Ports []ContainerPort `json:"ports,omitempty" patchStrategy:"merge" patchMergeKey:"containerPort" protobuf:"bytes,6,rep,name=ports"` + // List of sources to populate environment variables in the container. + // The keys defined within a source must be a C_IDENTIFIER. An invalid key + // will prevent the container from starting. When a key exists in multiple + // sources, the value associated with the last source will take precedence. + // Values defined by an Env with a duplicate key will take precedence. + // Cannot be updated. + // +optional + EnvFrom []EnvFromSource `json:"envFrom,omitempty" protobuf:"bytes,19,rep,name=envFrom"` // List of environment variables to set in the container. // Cannot be updated. // +optional diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 22abd7b5ce2..75ea1bb311f 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -1435,6 +1435,30 @@ func validateContainerResourceFieldSelector(fs *api.ResourceFieldSelector, expre return allErrs } +func validateEnvFrom(vars []api.EnvFromSource, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + for i, ev := range vars { + idxPath := fldPath.Index(i) + if len(ev.Prefix) > 0 { + for _, msg := range validation.IsCIdentifier(ev.Prefix) { + allErrs = append(allErrs, field.Invalid(idxPath.Child("prefix"), ev.Prefix, msg)) + } + } + if ev.ConfigMapRef != nil { + allErrs = append(allErrs, validateConfigMapEnvSource(ev.ConfigMapRef, idxPath.Child("configMapRef"))...) + } + } + return allErrs +} + +func validateConfigMapEnvSource(configMapSource *api.ConfigMapEnvSource, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if len(configMapSource.Name) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("name"), "")) + } + return allErrs +} + var validContainerResourceDivisorForCPU = sets.NewString("1m", "1") var validContainerResourceDivisorForMemory = sets.NewString("1", "1k", "1M", "1G", "1T", "1P", "1E", "1Ki", "1Mi", "1Gi", "1Ti", "1Pi", "1Ei") diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 4f86aca53a7..8cfe98da4c2 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -2719,6 +2719,65 @@ func TestValidateEnv(t *testing.T) { } } +func TestValidateEnvFrom(t *testing.T) { + successCase := []api.EnvFromSource{ + { + ConfigMapRef: &api.ConfigMapEnvSource{ + LocalObjectReference: api.LocalObjectReference{Name: "abc"}, + }, + }, + { + Prefix: "pre_", + ConfigMapRef: &api.ConfigMapEnvSource{ + LocalObjectReference: api.LocalObjectReference{Name: "abc"}, + }, + }, + } + if errs := validateEnvFrom(successCase, field.NewPath("field")); len(errs) != 0 { + t.Errorf("expected success: %v", errs) + } + + errorCases := []struct { + name string + envs []api.EnvFromSource + expectedError string + }{ + { + name: "zero-length name", + envs: []api.EnvFromSource{ + { + ConfigMapRef: &api.ConfigMapEnvSource{ + LocalObjectReference: api.LocalObjectReference{Name: ""}}, + }, + }, + expectedError: "field[0].configMapRef.name: Required value", + }, + { + name: "invalid prefix", + envs: []api.EnvFromSource{ + { + Prefix: "a.b", + ConfigMapRef: &api.ConfigMapEnvSource{ + LocalObjectReference: api.LocalObjectReference{Name: "abc"}}, + }, + }, + expectedError: `field[0].prefix: Invalid value: "a.b": ` + idErrMsg, + }, + } + for _, tc := range errorCases { + if errs := validateEnvFrom(tc.envs, field.NewPath("field")); len(errs) == 0 { + t.Errorf("expected failure for %s", tc.name) + } else { + for i := range errs { + str := errs[i].Error() + if str != "" && !strings.Contains(str, tc.expectedError) { + t.Errorf("%s: expected error detail either empty or %q, got %q", tc.name, tc.expectedError, str) + } + } + } + } +} + func TestValidateVolumeMounts(t *testing.T) { volumes := sets.NewString("abc", "123", "abc-123") diff --git a/pkg/kubectl/describe.go b/pkg/kubectl/describe.go index bd21ce5b82e..9d0b9079c20 100644 --- a/pkg/kubectl/describe.go +++ b/pkg/kubectl/describe.go @@ -890,6 +890,7 @@ func describeContainers(label string, containers []api.Container, containerStatu } describeContainerProbe(container, w) describeContainerVolumes(container, w) + describeContainerEnvFrom(container, resolverFn, w) describeContainerEnvVars(container, resolverFn, w) } } @@ -1012,6 +1013,7 @@ func describeContainerEnvVars(container api.Container, resolverFn EnvVarResolver none = "\t" } w.Write(LEVEL_2, "Environment Variables:%s\n", none) + for _, e := range container.Env { if e.ValueFrom == nil { w.Write(LEVEL_3, "%s:\t%s\n", e.Name, e.Value) @@ -1043,6 +1045,22 @@ func describeContainerEnvVars(container api.Container, resolverFn EnvVarResolver } } +func describeContainerEnvFrom(container api.Container, resolverFn EnvVarResolverFunc, w *PrefixWriter) { + none := "" + if len(container.EnvFrom) == 0 { + none = "\t" + } + w.Write(LEVEL_2, "Environment Variables from:%s\n", none) + + for _, e := range container.EnvFrom { + if len(e.Prefix) == 0 { + w.Write(LEVEL_3, "%s\tConfigMap\n", e.ConfigMapRef.Name) + } else { + w.Write(LEVEL_3, "%s\tConfigMap with prefix '%s'\n", e.ConfigMapRef.Name, e.Prefix) + } + } +} + // DescribeProbe is exported for consumers in other API groups that have probes func DescribeProbe(probe *api.Probe) string { attrs := fmt.Sprintf("delay=%ds timeout=%ds period=%ds #success=%d #failure=%d", probe.InitialDelaySeconds, probe.TimeoutSeconds, probe.PeriodSeconds, probe.SuccessThreshold, probe.FailureThreshold) diff --git a/pkg/kubectl/describe_test.go b/pkg/kubectl/describe_test.go index d03abd38f74..6f6a9b08fe8 100644 --- a/pkg/kubectl/describe_test.go +++ b/pkg/kubectl/describe_test.go @@ -288,13 +288,22 @@ func TestDescribeContainers(t *testing.T) { }, // Env { - container: api.Container{Name: "test", Image: "image", Env: []api.EnvVar{{Name: "envname", Value: "xyz"}}}, + container: api.Container{Name: "test", Image: "image", Env: []api.EnvVar{{Name: "envname", Value: "xyz"}}, EnvFrom: []api.EnvFromSource{{ConfigMapRef: &api.ConfigMapEnvSource{LocalObjectReference: api.LocalObjectReference{Name: "a123"}}}}}, status: api.ContainerStatus{ Name: "test", Ready: true, RestartCount: 7, }, - expectedElements: []string{"test", "State", "Waiting", "Ready", "True", "Restart Count", "7", "Image", "image", "envname", "xyz"}, + expectedElements: []string{"test", "State", "Waiting", "Ready", "True", "Restart Count", "7", "Image", "image", "envname", "xyz", "a123\tConfigMap"}, + }, + { + container: api.Container{Name: "test", Image: "image", Env: []api.EnvVar{{Name: "envname", Value: "xyz"}}, EnvFrom: []api.EnvFromSource{{Prefix: "p_", ConfigMapRef: &api.ConfigMapEnvSource{LocalObjectReference: api.LocalObjectReference{Name: "a123"}}}}}, + status: api.ContainerStatus{ + Name: "test", + Ready: true, + RestartCount: 7, + }, + expectedElements: []string{"test", "State", "Waiting", "Ready", "True", "Restart Count", "7", "Image", "image", "envname", "xyz", "a123\tConfigMap with prefix 'p_'"}, }, // Command { diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index ff7a4645fd6..2b4dd49fe16 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -414,6 +414,47 @@ func (kl *Kubelet) makeEnvironmentVariables(pod *v1.Pod, container *v1.Container return result, err } + var ( + configMaps = make(map[string]*v1.ConfigMap) + tmpEnv = make(map[string]string) + ) + + // Env will override EnvFrom variables. + // Process EnvFrom first then allow Env to replace existing values. + for _, envFrom := range container.EnvFrom { + if envFrom.ConfigMapRef != nil { + name := envFrom.ConfigMapRef.Name + configMap, ok := configMaps[name] + if !ok { + if kl.kubeClient == nil { + return result, fmt.Errorf("Couldn't get configMap %v/%v, no kubeClient defined", pod.Namespace, name) + } + configMap, err = kl.kubeClient.Core().ConfigMaps(pod.Namespace).Get(name, metav1.GetOptions{}) + + if err != nil { + return result, err + } + configMaps[name] = configMap + } + for k, v := range configMap.Data { + if errMsgs := utilvalidation.IsCIdentifier(k); len(errMsgs) != 0 { + return result, fmt.Errorf("Invalid environment variable name, %v, from configmap %v/%v: %s", k, pod.Namespace, name, errMsgs[0]) + } + + if len(envFrom.Prefix) > 0 { + k = envFrom.Prefix + k + } + // Accesses apiserver+Pods. + // So, the master may set service env vars, or kubelet may. In case both are doing + // it, we delete the key from the kubelet-generated ones so we don't have duplicate + // env vars. + // TODO: remove this next line once all platforms use apiserver+Pods. + delete(serviceEnv, k) + tmpEnv[k] = v + } + } + } + // Determine the final values of variables: // // 1. Determine the final value of each variable: @@ -424,8 +465,6 @@ func (kl *Kubelet) makeEnvironmentVariables(pod *v1.Pod, container *v1.Container // 2. Create the container's environment in the order variables are declared // 3. Add remaining service environment vars var ( - tmpEnv = make(map[string]string) - configMaps = make(map[string]*v1.ConfigMap) secrets = make(map[string]*v1.Secret) mappingFunc = expansion.MappingFuncFor(tmpEnv, serviceEnv) ) @@ -434,7 +473,7 @@ func (kl *Kubelet) makeEnvironmentVariables(pod *v1.Pod, container *v1.Container // So, the master may set service env vars, or kubelet may. In case both are doing // it, we delete the key from the kubelet-generated ones so we don't have duplicate // env vars. - // TODO: remove this net line once all platforms use apiserver+Pods. + // TODO: remove this next line once all platforms use apiserver+Pods. delete(serviceEnv, envVar.Name) runtimeVal := envVar.Value @@ -499,7 +538,11 @@ func (kl *Kubelet) makeEnvironmentVariables(pod *v1.Pod, container *v1.Container } tmpEnv[envVar.Name] = runtimeVal - result = append(result, kubecontainer.EnvVar{Name: envVar.Name, Value: tmpEnv[envVar.Name]}) + } + + // Append the env vars + for k, v := range tmpEnv { + result = append(result, kubecontainer.EnvVar{Name: k, Value: v}) } // Append remaining service env vars. diff --git a/pkg/kubelet/kubelet_pods_test.go b/pkg/kubelet/kubelet_pods_test.go index f8a7982e3e7..3ae5c0b462a 100644 --- a/pkg/kubelet/kubelet_pods_test.go +++ b/pkg/kubelet/kubelet_pods_test.go @@ -268,7 +268,9 @@ func TestMakeEnvironmentVariables(t *testing.T) { container *v1.Container // the container to use masterServiceNs string // the namespace to read master service info from nilLister bool // whether the lister should be nil + configMap *v1.ConfigMap // an optional ConfigMap to pull from expectedEnvs []kubecontainer.EnvVar // a set of expected environment vars + expectedError bool // does the test fail }{ { name: "api server = Y, kubelet = Y", @@ -605,6 +607,132 @@ func TestMakeEnvironmentVariables(t *testing.T) { }, }, }, + { + name: "configmap", + ns: "test1", + container: &v1.Container{ + EnvFrom: []v1.EnvFromSource{ + { + ConfigMapRef: &v1.ConfigMapEnvSource{LocalObjectReference: v1.LocalObjectReference{Name: "test-config-map"}}, + }, + { + Prefix: "p_", + ConfigMapRef: &v1.ConfigMapEnvSource{LocalObjectReference: v1.LocalObjectReference{Name: "test-config-map"}}, + }, + }, + Env: []v1.EnvVar{ + { + Name: "TEST_LITERAL", + Value: "test-test-test", + }, + { + Name: "EXPANSION_TEST", + Value: "$(REPLACE_ME)", + }, + { + Name: "DUPE_TEST", + Value: "ENV_VAR", + }, + }, + }, + masterServiceNs: "nothing", + nilLister: false, + configMap: &v1.ConfigMap{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "test1", + Name: "test-configmap", + }, + Data: map[string]string{ + "REPLACE_ME": "FROM_CONFIG_MAP", + "DUPE_TEST": "CONFIG_MAP", + }, + }, + expectedEnvs: []kubecontainer.EnvVar{ + { + Name: "TEST_LITERAL", + Value: "test-test-test", + }, + { + Name: "TEST_SERVICE_HOST", + Value: "1.2.3.3", + }, + { + Name: "TEST_SERVICE_PORT", + Value: "8083", + }, + { + Name: "TEST_PORT", + Value: "tcp://1.2.3.3:8083", + }, + { + Name: "TEST_PORT_8083_TCP", + Value: "tcp://1.2.3.3:8083", + }, + { + Name: "TEST_PORT_8083_TCP_PROTO", + Value: "tcp", + }, + { + Name: "TEST_PORT_8083_TCP_PORT", + Value: "8083", + }, + { + Name: "TEST_PORT_8083_TCP_ADDR", + Value: "1.2.3.3", + }, + { + Name: "REPLACE_ME", + Value: "FROM_CONFIG_MAP", + }, + { + Name: "EXPANSION_TEST", + Value: "FROM_CONFIG_MAP", + }, + { + Name: "DUPE_TEST", + Value: "ENV_VAR", + }, + { + Name: "p_REPLACE_ME", + Value: "FROM_CONFIG_MAP", + }, + { + Name: "p_DUPE_TEST", + Value: "CONFIG_MAP", + }, + }, + }, + { + name: "configmap_missing", + ns: "test1", + container: &v1.Container{ + EnvFrom: []v1.EnvFromSource{ + {ConfigMapRef: &v1.ConfigMapEnvSource{LocalObjectReference: v1.LocalObjectReference{Name: "test-config-map"}}}, + }, + }, + masterServiceNs: "nothing", + expectedError: true, + }, + { + name: "configmap_invalid_keys", + ns: "test1", + container: &v1.Container{ + EnvFrom: []v1.EnvFromSource{ + {ConfigMapRef: &v1.ConfigMapEnvSource{LocalObjectReference: v1.LocalObjectReference{Name: "test-config-map"}}}, + }, + }, + masterServiceNs: "nothing", + configMap: &v1.ConfigMap{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "test1", + Name: "test-configmap", + }, + Data: map[string]string{ + "-1234": "abc", + }, + }, + expectedError: true, + }, } for _, tc := range testCases { @@ -617,6 +745,14 @@ func TestMakeEnvironmentVariables(t *testing.T) { kl.serviceLister = testServiceLister{services} } + testKubelet.fakeKubeClient.AddReactor("get", "configmaps", func(action core.Action) (bool, runtime.Object, error) { + var err error + if tc.configMap == nil { + err = errors.New("no configmap defined") + } + return true, tc.configMap, err + }) + testPod := &v1.Pod{ ObjectMeta: v1.ObjectMeta{ Namespace: tc.ns, @@ -630,11 +766,15 @@ func TestMakeEnvironmentVariables(t *testing.T) { podIP := "1.2.3.4" result, err := kl.makeEnvironmentVariables(testPod, tc.container, podIP) - assert.NoError(t, err, "[%s]", tc.name) + if tc.expectedError { + assert.Error(t, err, tc.name) + } else { + assert.NoError(t, err, "[%s]", tc.name) - sort.Sort(envs(result)) - sort.Sort(envs(tc.expectedEnvs)) - assert.Equal(t, tc.expectedEnvs, result, "[%s] env entries", tc.name) + sort.Sort(envs(result)) + sort.Sort(envs(tc.expectedEnvs)) + assert.Equal(t, tc.expectedEnvs, result, "[%s] env entries", tc.name) + } } } diff --git a/test/e2e/common/configmap.go b/test/e2e/common/configmap.go index f160686d5ac..4ac26bc409a 100644 --- a/test/e2e/common/configmap.go +++ b/test/e2e/common/configmap.go @@ -196,6 +196,46 @@ var _ = framework.KubeDescribe("ConfigMap", func() { }) }) + It("should be consumable via the environment [Conformance]", func() { + name := "configmap-test-" + string(uuid.NewUUID()) + configMap := newEnvFromConfigMap(f, name) + By(fmt.Sprintf("Creating configMap %v/%v", f.Namespace.Name, configMap.Name)) + var err error + if configMap, err = f.ClientSet.Core().ConfigMaps(f.Namespace.Name).Create(configMap); err != nil { + framework.Failf("unable to create test configMap %s: %v", configMap.Name, err) + } + + pod := &v1.Pod{ + ObjectMeta: v1.ObjectMeta{ + Name: "pod-configmaps-" + string(uuid.NewUUID()), + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "env-test", + Image: "gcr.io/google_containers/busybox:1.24", + Command: []string{"sh", "-c", "env"}, + EnvFrom: []v1.EnvFromSource{ + { + ConfigMapRef: &v1.ConfigMapEnvSource{LocalObjectReference: v1.LocalObjectReference{Name: name}}, + }, + { + Prefix: "p_", + ConfigMapRef: &v1.ConfigMapEnvSource{LocalObjectReference: v1.LocalObjectReference{Name: name}}, + }, + }, + }, + }, + RestartPolicy: v1.RestartPolicyNever, + }, + } + + f.TestContainerOutput("consume configMaps", pod, 0, []string{ + "data_1=value-1", "data_2=value-2", "data_3=value-3", + "p_data_1=value-1", "p_data_2=value-2", "p_data_3=value-3", + }) + }) + It("should be consumable in multiple volumes in the same pod [Conformance]", func() { var ( name = "configmap-test-volume-" + string(uuid.NewUUID()) @@ -283,6 +323,20 @@ func newConfigMap(f *framework.Framework, name string) *v1.ConfigMap { } } +func newEnvFromConfigMap(f *framework.Framework, name string) *v1.ConfigMap { + return &v1.ConfigMap{ + ObjectMeta: v1.ObjectMeta{ + Namespace: f.Namespace.Name, + Name: name, + }, + Data: map[string]string{ + "data_1": "value-1", + "data_2": "value-2", + "data_3": "value-3", + }, + } +} + func doConfigMapE2EWithoutMappings(f *framework.Framework, uid, fsGroup int64, defaultMode *int32) { var ( name = "configmap-test-volume-" + string(uuid.NewUUID())