From ffbc494a4a317e2673cf8186bc15d674658e458f Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Fri, 16 Aug 2024 17:36:33 +0300 Subject: [PATCH 1/2] kubeadm: add unit test for GetProxyEnvVars --- cmd/kubeadm/app/phases/addons/proxy/proxy.go | 2 +- .../app/phases/controlplane/manifests.go | 2 +- cmd/kubeadm/app/util/env.go | 12 +++-- cmd/kubeadm/app/util/env_test.go | 49 +++++++++++++++++++ 4 files changed, 59 insertions(+), 6 deletions(-) diff --git a/cmd/kubeadm/app/phases/addons/proxy/proxy.go b/cmd/kubeadm/app/phases/addons/proxy/proxy.go index 6c9fdd28492..35f9d7335e1 100644 --- a/cmd/kubeadm/app/phases/addons/proxy/proxy.go +++ b/cmd/kubeadm/app/phases/addons/proxy/proxy.go @@ -259,7 +259,7 @@ func createKubeProxyAddon(cfg *kubeadmapi.ClusterConfiguration, client clientset } // Propagate the http/https proxy host environment variables to the container env := &kubeproxyDaemonSet.Spec.Template.Spec.Containers[0].Env - *env = append(*env, kubeadmutil.MergeKubeadmEnvVars(kubeadmutil.GetProxyEnvVars())...) + *env = append(*env, kubeadmutil.MergeKubeadmEnvVars(kubeadmutil.GetProxyEnvVars(nil))...) // Create the DaemonSet for kube-proxy or update it in case it already exists return []byte(""), apiclient.CreateOrUpdateDaemonSet(client, kubeproxyDaemonSet) diff --git a/cmd/kubeadm/app/phases/controlplane/manifests.go b/cmd/kubeadm/app/phases/controlplane/manifests.go index 11b93e083db..2c2b7f8b35e 100644 --- a/cmd/kubeadm/app/phases/controlplane/manifests.go +++ b/cmd/kubeadm/app/phases/controlplane/manifests.go @@ -52,7 +52,7 @@ func GetStaticPodSpecs(cfg *kubeadmapi.ClusterConfiguration, endpoint *kubeadmap // Get the required hostpath mounts mounts := getHostPathVolumesForTheControlPlane(cfg) if proxyEnvs == nil { - proxyEnvs = kubeadmutil.GetProxyEnvVars() + proxyEnvs = kubeadmutil.GetProxyEnvVars(nil) } componentHealthCheckTimeout := kubeadmapi.GetActiveTimeouts().ControlPlaneComponentHealthCheck diff --git a/cmd/kubeadm/app/util/env.go b/cmd/kubeadm/app/util/env.go index 5534b1d212a..e0f984798fe 100644 --- a/cmd/kubeadm/app/util/env.go +++ b/cmd/kubeadm/app/util/env.go @@ -25,13 +25,17 @@ import ( kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" ) -// GetProxyEnvVars builds a list of environment variables in order to use the right proxy -func GetProxyEnvVars() []kubeadmapi.EnvVar { +// GetProxyEnvVars builds a list of environment variables in order to use the right proxy. +// Passing nil for environment will make the function use the OS environment. +func GetProxyEnvVars(environment []string) []kubeadmapi.EnvVar { envs := []kubeadmapi.EnvVar{} - for _, env := range os.Environ() { + if environment == nil { + environment = os.Environ() + } + for _, env := range environment { pos := strings.Index(env, "=") if pos == -1 { - // malformed environment variable, skip it. + // Malformed environment variable, skip it. continue } name := env[:pos] diff --git a/cmd/kubeadm/app/util/env_test.go b/cmd/kubeadm/app/util/env_test.go index ad70a4e8584..0f788528ac3 100644 --- a/cmd/kubeadm/app/util/env_test.go +++ b/cmd/kubeadm/app/util/env_test.go @@ -17,6 +17,7 @@ limitations under the License. package util import ( + "reflect" "testing" "github.com/stretchr/testify/assert" @@ -88,3 +89,51 @@ func TestMergeKubeadmEnvVars(t *testing.T) { }) } } + +func TestGetProxyEnvVars(t *testing.T) { + var tests = []struct { + name string + environment []string + expected []kubeadmapi.EnvVar + }{ + { + name: "environment with lowercase proxy vars", + environment: []string{ + "http_proxy=p1", + "foo1=bar1", + "https_proxy=p2", + "foo2=bar2", + "no_proxy=p3", + }, + expected: []kubeadmapi.EnvVar{ + {EnvVar: v1.EnvVar{Name: "http_proxy", Value: "p1"}}, + {EnvVar: v1.EnvVar{Name: "https_proxy", Value: "p2"}}, + {EnvVar: v1.EnvVar{Name: "no_proxy", Value: "p3"}}, + }, + }, + { + name: "environment with uppercase proxy vars", + environment: []string{ + "HTTP_PROXY=p1", + "foo1=bar1", + "HTTPS_PROXY=p2", + "foo2=bar2", + "NO_PROXY=p3", + }, + expected: []kubeadmapi.EnvVar{ + {EnvVar: v1.EnvVar{Name: "HTTP_PROXY", Value: "p1"}}, + {EnvVar: v1.EnvVar{Name: "HTTPS_PROXY", Value: "p2"}}, + {EnvVar: v1.EnvVar{Name: "NO_PROXY", Value: "p3"}}, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + envs := GetProxyEnvVars(test.environment) + if !reflect.DeepEqual(envs, test.expected) { + t.Errorf("expected env: %v, got: %v", test.expected, envs) + } + }) + } +} From a9f681d40a29bd9a6491db7d392ebc4a1a045c79 Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Fri, 16 Aug 2024 17:37:23 +0300 Subject: [PATCH 2/2] kubeadm: sort the results of MergeKubeadmEnvVars MergeKubeadmEnvVars use a map which results in non deterministic output in the end slice EnvVar objects. Before returning the slice, sort it by the Name field. Update the unit test to capture the sorting aspect. --- cmd/kubeadm/app/util/env.go | 4 ++++ cmd/kubeadm/app/util/env_test.go | 8 +++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/cmd/kubeadm/app/util/env.go b/cmd/kubeadm/app/util/env.go index e0f984798fe..d34a9a4ce2a 100644 --- a/cmd/kubeadm/app/util/env.go +++ b/cmd/kubeadm/app/util/env.go @@ -18,6 +18,7 @@ package util import ( "os" + "sort" "strings" v1 "k8s.io/api/core/v1" @@ -63,5 +64,8 @@ func MergeKubeadmEnvVars(envList ...[]kubeadmapi.EnvVar) []v1.EnvVar { for _, v := range m { merged = append(merged, v) } + sort.Slice(merged, func(i, j int) bool { + return merged[i].Name < merged[j].Name + }) return merged } diff --git a/cmd/kubeadm/app/util/env_test.go b/cmd/kubeadm/app/util/env_test.go index 0f788528ac3..7442edec49e 100644 --- a/cmd/kubeadm/app/util/env_test.go +++ b/cmd/kubeadm/app/util/env_test.go @@ -20,8 +20,6 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" - v1 "k8s.io/api/core/v1" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" @@ -41,10 +39,10 @@ func TestMergeKubeadmEnvVars(t *testing.T) { name: "normal case without duplicated env", proxyEnv: []kubeadmapi.EnvVar{ { - EnvVar: v1.EnvVar{Name: "Foo1", Value: "Bar1"}, + EnvVar: v1.EnvVar{Name: "Foo2", Value: "Bar2"}, }, { - EnvVar: v1.EnvVar{Name: "Foo2", Value: "Bar2"}, + EnvVar: v1.EnvVar{Name: "Foo1", Value: "Bar1"}, }, }, extraEnv: []kubeadmapi.EnvVar{ @@ -83,7 +81,7 @@ func TestMergeKubeadmEnvVars(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { envs := MergeKubeadmEnvVars(test.proxyEnv, test.extraEnv) - if !assert.ElementsMatch(t, envs, test.mergedEnv) { + if !reflect.DeepEqual(envs, test.mergedEnv) { t.Errorf("expected env: %v, got: %v", test.mergedEnv, envs) } })