From 1542c4d2f420528f52b41c9002afa536b2bf6e52 Mon Sep 17 00:00:00 2001 From: Yash Desai Date: Sat, 1 Jun 2019 09:41:10 -0700 Subject: [PATCH] Allow composing env var from existing env vars. (#643) Example: env name: FOO value: "foo" name: BAR value: "bar" name: FOOBAR value: "${FOO}${BAR}" <-- should expand to: "foobar" Added testcase for the same as well. Change is based on kubelet_pods.go. Simplified some of the existing code. --- Gopkg.lock | 4 +- .../forked/golang/expansion/expand.go | 102 +++++++++++++++++ vkubelet/env.go | 104 +++++++++--------- vkubelet/env_internal_test.go | 65 +++++++++++ 4 files changed, 219 insertions(+), 56 deletions(-) create mode 100644 vendor/k8s.io/kubernetes/third_party/forked/golang/expansion/expand.go diff --git a/Gopkg.lock b/Gopkg.lock index c5592d306..b88eb80f8 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -1272,7 +1272,7 @@ revision = "0317810137be915b9cf888946c6e115c1bfac693" [[projects]] - digest = "1:4ff70ea1888545c3a245a76657ba38a308d2c068bd26fa4310f63560ebaf265c" + digest = "1:b78561dbff036b36f419a270993ca22bb6fdafd84085686f24e4a5f8e31056be" name = "k8s.io/kubernetes" packages = [ "pkg/api/v1/pod", @@ -1285,6 +1285,7 @@ "pkg/kubelet/apis/stats/v1alpha1", "pkg/kubelet/envvars", "pkg/kubelet/server/remotecommand", + "third_party/forked/golang/expansion", ] pruneopts = "NUT" revision = "c27b913fddd1a6c480c229191a087698aa92f0b1" @@ -1409,6 +1410,7 @@ "k8s.io/kubernetes/pkg/kubelet/apis/stats/v1alpha1", "k8s.io/kubernetes/pkg/kubelet/envvars", "k8s.io/kubernetes/pkg/kubelet/server/remotecommand", + "k8s.io/kubernetes/third_party/forked/golang/expansion", ] solver-name = "gps-cdcl" solver-version = 1 diff --git a/vendor/k8s.io/kubernetes/third_party/forked/golang/expansion/expand.go b/vendor/k8s.io/kubernetes/third_party/forked/golang/expansion/expand.go new file mode 100644 index 000000000..6bf0ea8ce --- /dev/null +++ b/vendor/k8s.io/kubernetes/third_party/forked/golang/expansion/expand.go @@ -0,0 +1,102 @@ +package expansion + +import ( + "bytes" +) + +const ( + operator = '$' + referenceOpener = '(' + referenceCloser = ')' +) + +// syntaxWrap returns the input string wrapped by the expansion syntax. +func syntaxWrap(input string) string { + return string(operator) + string(referenceOpener) + input + string(referenceCloser) +} + +// MappingFuncFor returns a mapping function for use with Expand that +// implements the expansion semantics defined in the expansion spec; it +// returns the input string wrapped in the expansion syntax if no mapping +// for the input is found. +func MappingFuncFor(context ...map[string]string) func(string) string { + return func(input string) string { + for _, vars := range context { + val, ok := vars[input] + if ok { + return val + } + } + + return syntaxWrap(input) + } +} + +// Expand replaces variable references in the input string according to +// the expansion spec using the given mapping function to resolve the +// values of variables. +func Expand(input string, mapping func(string) string) string { + var buf bytes.Buffer + checkpoint := 0 + for cursor := 0; cursor < len(input); cursor++ { + if input[cursor] == operator && cursor+1 < len(input) { + // Copy the portion of the input string since the last + // checkpoint into the buffer + buf.WriteString(input[checkpoint:cursor]) + + // Attempt to read the variable name as defined by the + // syntax from the input string + read, isVar, advance := tryReadVariableName(input[cursor+1:]) + + if isVar { + // We were able to read a variable name correctly; + // apply the mapping to the variable name and copy the + // bytes into the buffer + buf.WriteString(mapping(read)) + } else { + // Not a variable name; copy the read bytes into the buffer + buf.WriteString(read) + } + + // Advance the cursor in the input string to account for + // bytes consumed to read the variable name expression + cursor += advance + + // Advance the checkpoint in the input string + checkpoint = cursor + 1 + } + } + + // Return the buffer and any remaining unwritten bytes in the + // input string. + return buf.String() + input[checkpoint:] +} + +// tryReadVariableName attempts to read a variable name from the input +// string and returns the content read from the input, whether that content +// represents a variable name to perform mapping on, and the number of bytes +// consumed in the input string. +// +// The input string is assumed not to contain the initial operator. +func tryReadVariableName(input string) (string, bool, int) { + switch input[0] { + case operator: + // Escaped operator; return it. + return input[0:1], false, 1 + case referenceOpener: + // Scan to expression closer + for i := 1; i < len(input); i++ { + if input[i] == referenceCloser { + return input[1:i], true, i + 1 + } + } + + // Incomplete reference; return it. + return string(operator) + string(referenceOpener), false, 1 + default: + // Not the beginning of an expression, ie, an operator + // that doesn't begin an expression. Return the operator + // and the first rune in the string. + return (string(operator) + string(input[0])), false, 1 + } +} diff --git a/vkubelet/env.go b/vkubelet/env.go index 140720ec0..542c15110 100755 --- a/vkubelet/env.go +++ b/vkubelet/env.go @@ -16,6 +16,7 @@ import ( v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" fieldpath "k8s.io/kubernetes/pkg/fieldpath" "k8s.io/kubernetes/pkg/kubelet/envvars" + "k8s.io/kubernetes/third_party/forked/golang/expansion" "github.com/virtual-kubelet/virtual-kubelet/log" "github.com/virtual-kubelet/virtual-kubelet/manager" @@ -78,12 +79,13 @@ func populateEnvironmentVariables(ctx context.Context, pod *corev1.Pod, rm *mana // populateContainerEnvironment populates the environment of a single container in the specified pod. func populateContainerEnvironment(ctx context.Context, pod *corev1.Pod, container *corev1.Container, rm *manager.ResourceManager, recorder record.EventRecorder) error { // Create an "environment map" based on the value of the specified container's ".envFrom" field. - envFrom, err := makeEnvironmentMapBasedOnEnvFrom(ctx, pod, container, rm, recorder) + tmpEnv, err := makeEnvironmentMapBasedOnEnvFrom(ctx, pod, container, rm, recorder) if err != nil { return err } - // Create an "environment map" based on the value of the specified container's ".env" field. - env, err := makeEnvironmentMapBasedOnEnv(ctx, pod, container, rm, recorder) + // Create the final "environment map" for the container using the ".env" and ".envFrom" field + // and service environment variables. + err = makeEnvironmentMap(ctx, pod, container, rm, recorder, tmpEnv) if err != nil { return err } @@ -92,7 +94,17 @@ func populateContainerEnvironment(ctx context.Context, pod *corev1.Pod, containe // This is in accordance with what the Kubelet itself does. // https://github.com/kubernetes/kubernetes/blob/v1.13.1/pkg/kubelet/kubelet_pods.go#L557-L558 container.EnvFrom = []corev1.EnvFromSource{} - container.Env = mergeEnvironments(envFrom, env) + + res := make([]corev1.EnvVar, 0) + + for key, val := range tmpEnv { + res = append(res, corev1.EnvVar{ + Name: key, + Value: val, + }) + } + container.Env = res + return nil } @@ -265,17 +277,37 @@ loop: return res, nil } -// makeEnvironmentMapBasedOnEnv returns a map representing the resolved environment of the specified container after being populated from the entries in the ".env" field. -func makeEnvironmentMapBasedOnEnv(ctx context.Context, pod *corev1.Pod, container *corev1.Container, rm *manager.ResourceManager, recorder record.EventRecorder) (map[string]string, error) { - // Create a map to hold the resolved environment variables. - res := make(map[string]string, len(container.Env)) +// makeEnvironmentMap returns a map representing the resolved environment of the specified container after being populated from the entries in the ".env" and ".envFrom" field. +func makeEnvironmentMap(ctx context.Context, pod *corev1.Pod, container *corev1.Container, rm *manager.ResourceManager, recorder record.EventRecorder, res map[string]string) error { + + // TODO If pod.Spec.EnableServiceLinks is nil then fail as per 1.14 kubelet. + enableServiceLinks := corev1.DefaultEnableServiceLinks + if pod.Spec.EnableServiceLinks != nil { + enableServiceLinks = *pod.Spec.EnableServiceLinks + } + + // Note that there is a race between Kubelet seeing the pod and kubelet seeing the service. + // To avoid this users can: (1) wait between starting a service and starting; or (2) detect + // missing service env var and exit and be restarted; or (3) use DNS instead of env vars + // and keep trying to resolve the DNS name of the service (recommended). + svcEnv, err := getServiceEnvVarMap(rm, pod.Namespace, enableServiceLinks) + if err != nil { + return err + } + + // If the variable's Value is set, expand the `$(var)` references to other + // variables in the .Value field; the sources of variables are the declared + // variables of the container and the service environment variables. + mappingFunc := expansion.MappingFuncFor(res, svcEnv) + // Iterate over environment variables in order to populate the map. loop: for _, env := range container.Env { switch { // Handle values that have been directly provided. case env.Value != "": - res[env.Name] = env.Value + // Expand variable references + res[env.Name] = expansion.Expand(env.Value, mappingFunc) continue loop // Handle population from a configmap key. case env.ValueFrom != nil && env.ValueFrom.ConfigMapKeyRef != nil: @@ -303,10 +335,10 @@ loop: // Hence, we should return a meaningful error. if errors.IsNotFound(err) { recorder.Eventf(pod, corev1.EventTypeWarning, ReasonMandatoryConfigMapNotFound, "configmap %q not found", vf.Name) - return nil, fmt.Errorf("configmap %q not found", vf.Name) + return fmt.Errorf("configmap %q not found", vf.Name) } recorder.Eventf(pod, corev1.EventTypeWarning, ReasonFailedToReadMandatoryConfigMap, "failed to read configmap %q", vf.Name) - return nil, fmt.Errorf("failed to read configmap %q: %v", vf.Name, err) + return fmt.Errorf("failed to read configmap %q: %v", vf.Name, err) } // At this point we have successfully fetched the target configmap. // We must now try to grab the requested key. @@ -325,7 +357,7 @@ loop: // At this point we know the key reference is mandatory. // Hence, we should fail. recorder.Eventf(pod, corev1.EventTypeWarning, ReasonMandatoryConfigMapKeyNotFound, "key %q does not exist in configmap %q", vf.Key, vf.Name) - return nil, fmt.Errorf("configmap %q doesn't contain the %q key required by pod %s", vf.Name, vf.Key, pod.Name) + return fmt.Errorf("configmap %q doesn't contain the %q key required by pod %s", vf.Name, vf.Key, pod.Name) } // Populate the environment variable and continue on to the next reference. res[env.Name] = keyValue @@ -355,10 +387,10 @@ loop: // Hence, we should return a meaningful error. if errors.IsNotFound(err) { recorder.Eventf(pod, corev1.EventTypeWarning, ReasonMandatorySecretNotFound, "secret %q not found", vf.Name) - return nil, fmt.Errorf("secret %q not found", vf.Name) + return fmt.Errorf("secret %q not found", vf.Name) } recorder.Eventf(pod, corev1.EventTypeWarning, ReasonFailedToReadMandatorySecret, "failed to read secret %q", vf.Name) - return nil, fmt.Errorf("failed to read secret %q: %v", vf.Name, err) + return fmt.Errorf("failed to read secret %q: %v", vf.Name, err) } // At this point we have successfully fetched the target secret. // We must now try to grab the requested key. @@ -377,7 +409,7 @@ loop: // At this point we know the key reference is mandatory. // Hence, we should fail. recorder.Eventf(pod, corev1.EventTypeWarning, ReasonMandatorySecretKeyNotFound, "key %q does not exist in secret %q", vf.Key, vf.Name) - return nil, fmt.Errorf("secret %q doesn't contain the %q key required by pod %s", vf.Name, vf.Key, pod.Name) + return fmt.Errorf("secret %q doesn't contain the %q key required by pod %s", vf.Name, vf.Key, pod.Name) } // Populate the environment variable and continue on to the next reference. res[env.Name] = string(keyValue) @@ -389,7 +421,7 @@ loop: runtimeVal, err := podFieldSelectorRuntimeValue(vf, pod) if err != nil { - return res, err + return err } res[env.Name] = runtimeVal @@ -402,21 +434,6 @@ loop: } } - // TODO If pod.Spec.EnableServiceLinks is nil then fail as per 1.14 kubelet. - enableServiceLinks := corev1.DefaultEnableServiceLinks - if pod.Spec.EnableServiceLinks != nil { - enableServiceLinks = *pod.Spec.EnableServiceLinks - } - - // Note that there is a race between Kubelet seeing the pod and kubelet seeing the service. - // To avoid this users can: (1) wait between starting a service and starting; or (2) detect - // missing service env var and exit and be restarted; or (3) use DNS instead of env vars - // and keep trying to resolve the DNS name of the service (recommended). - svcEnv, err := getServiceEnvVarMap(rm, pod.Namespace, enableServiceLinks) - if err != nil { - return nil, err - } - // Append service env vars. for k, v := range svcEnv { if _, present := res[k]; !present { @@ -424,8 +441,7 @@ loop: } } - // Return the populated environment. - return res, nil + return nil } // podFieldSelectorRuntimeValue returns the runtime value of the given @@ -444,25 +460,3 @@ func podFieldSelectorRuntimeValue(fs *corev1.ObjectFieldSelector, pod *corev1.Po } return fieldpath.ExtractFieldPathAsString(pod, internalFieldPath) } - -// mergeEnvironments creates the final environment for a container by merging "envFrom" and "env". -// Values in "env" override any values with the same key defined in "envFrom". -// This is in accordance with what the Kubelet itself does. -// https://github.com/kubernetes/kubernetes/blob/v1.13.1/pkg/kubelet/kubelet_pods.go#L557-L558 -func mergeEnvironments(envFrom map[string]string, env map[string]string) []corev1.EnvVar { - tmp := make(map[string]string, 0) - res := make([]corev1.EnvVar, 0) - for key, val := range envFrom { - tmp[key] = val - } - for key, val := range env { - tmp[key] = val - } - for key, val := range tmp { - res = append(res, corev1.EnvVar{ - Name: key, - Value: val, - }) - } - return res -} diff --git a/vkubelet/env_internal_test.go b/vkubelet/env_internal_test.go index ff71d65af..c0ec982af 100644 --- a/vkubelet/env_internal_test.go +++ b/vkubelet/env_internal_test.go @@ -23,6 +23,10 @@ const ( envVarValue1 = "foo_value" // envVarName2 is a string that can be used as the name of an environment value. envVarName2 = "BAR" + // envVarValue2 is a string meant to be used as the value of the "envVarName2" environment value. + envVarValue2 = "bar_value" + // envVarName12 is a key that can be used as the name of an environment variable. + envVarName12 = "FOOBAR" // envVarName3 is a string that can be used as the name of an environment value. envVarName3 = "CHO" // envVarName4 is a string that can be used as the name of an environment value. @@ -1011,3 +1015,64 @@ func TestServiceEnvVar(t *testing.T) { } } + +// TestComposingEnv tests that env var can be composed from the existing env vars. +func TestComposingEnv(t *testing.T) { + rm := testutil.FakeResourceManager() + er := testutil.FakeEventRecorder(defaultEventRecorderBufferSize) + + // Create a pod object having a single container. + // The container's third environment variable is composed of the previous two. + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: "pod-0", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Env: []corev1.EnvVar{ + { + Name: envVarName1, + Value: envVarValue1, + }, + { + Name: envVarName2, + Value: envVarValue2, + }, + { + Name: envVarName12, + Value: "$(" + envVarName1 + ")$(" + envVarName2 + ")", // "$(envVarName1)$(envVarName2)" + }, + }, + }, + }, + EnableServiceLinks: &bFalse, + }, + } + + // Populate the pods's environment. + err := populateEnvironmentVariables(context.Background(), pod, rm, er) + assert.Check(t, err) + + // Make sure that the container's environment contains all the expected keys and values. + assert.Check(t, is.DeepEqual(pod.Spec.Containers[0].Env, []corev1.EnvVar{ + { + Name: envVarName1, + Value: envVarValue1, + }, + { + Name: envVarName2, + Value: envVarValue2, + }, + { + Name: envVarName12, + Value: envVarValue1 + envVarValue2, + }, + }, + sortOpt, + )) + + // Make sure that no events have been recorded. + assert.Check(t, is.Len(er.Events, 0)) +}