Skip to content

Commit 385fd21

Browse files
authored
Merge pull request kubernetes#126743 from neolit123/1.32-add-get-proxy-env-vars-test
kubeadm: sort the merged env vars and improve related tests
2 parents 2f3e7f5 + a9f681d commit 385fd21

File tree

4 files changed

+66
-11
lines changed

4 files changed

+66
-11
lines changed

cmd/kubeadm/app/phases/addons/proxy/proxy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ func createKubeProxyAddon(cfg *kubeadmapi.ClusterConfiguration, client clientset
259259
}
260260
// Propagate the http/https proxy host environment variables to the container
261261
env := &kubeproxyDaemonSet.Spec.Template.Spec.Containers[0].Env
262-
*env = append(*env, kubeadmutil.MergeKubeadmEnvVars(kubeadmutil.GetProxyEnvVars())...)
262+
*env = append(*env, kubeadmutil.MergeKubeadmEnvVars(kubeadmutil.GetProxyEnvVars(nil))...)
263263

264264
// Create the DaemonSet for kube-proxy or update it in case it already exists
265265
return []byte(""), apiclient.CreateOrUpdateDaemonSet(client, kubeproxyDaemonSet)

cmd/kubeadm/app/phases/controlplane/manifests.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func GetStaticPodSpecs(cfg *kubeadmapi.ClusterConfiguration, endpoint *kubeadmap
5252
// Get the required hostpath mounts
5353
mounts := getHostPathVolumesForTheControlPlane(cfg)
5454
if proxyEnvs == nil {
55-
proxyEnvs = kubeadmutil.GetProxyEnvVars()
55+
proxyEnvs = kubeadmutil.GetProxyEnvVars(nil)
5656
}
5757
componentHealthCheckTimeout := kubeadmapi.GetActiveTimeouts().ControlPlaneComponentHealthCheck
5858

cmd/kubeadm/app/util/env.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,25 @@ package util
1818

1919
import (
2020
"os"
21+
"sort"
2122
"strings"
2223

2324
v1 "k8s.io/api/core/v1"
2425

2526
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
2627
)
2728

28-
// GetProxyEnvVars builds a list of environment variables in order to use the right proxy
29-
func GetProxyEnvVars() []kubeadmapi.EnvVar {
29+
// GetProxyEnvVars builds a list of environment variables in order to use the right proxy.
30+
// Passing nil for environment will make the function use the OS environment.
31+
func GetProxyEnvVars(environment []string) []kubeadmapi.EnvVar {
3032
envs := []kubeadmapi.EnvVar{}
31-
for _, env := range os.Environ() {
33+
if environment == nil {
34+
environment = os.Environ()
35+
}
36+
for _, env := range environment {
3237
pos := strings.Index(env, "=")
3338
if pos == -1 {
34-
// malformed environment variable, skip it.
39+
// Malformed environment variable, skip it.
3540
continue
3641
}
3742
name := env[:pos]
@@ -59,5 +64,8 @@ func MergeKubeadmEnvVars(envList ...[]kubeadmapi.EnvVar) []v1.EnvVar {
5964
for _, v := range m {
6065
merged = append(merged, v)
6166
}
67+
sort.Slice(merged, func(i, j int) bool {
68+
return merged[i].Name < merged[j].Name
69+
})
6270
return merged
6371
}

cmd/kubeadm/app/util/env_test.go

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,9 @@ limitations under the License.
1717
package util
1818

1919
import (
20+
"reflect"
2021
"testing"
2122

22-
"github.com/stretchr/testify/assert"
23-
2423
v1 "k8s.io/api/core/v1"
2524

2625
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
@@ -40,10 +39,10 @@ func TestMergeKubeadmEnvVars(t *testing.T) {
4039
name: "normal case without duplicated env",
4140
proxyEnv: []kubeadmapi.EnvVar{
4241
{
43-
EnvVar: v1.EnvVar{Name: "Foo1", Value: "Bar1"},
42+
EnvVar: v1.EnvVar{Name: "Foo2", Value: "Bar2"},
4443
},
4544
{
46-
EnvVar: v1.EnvVar{Name: "Foo2", Value: "Bar2"},
45+
EnvVar: v1.EnvVar{Name: "Foo1", Value: "Bar1"},
4746
},
4847
},
4948
extraEnv: []kubeadmapi.EnvVar{
@@ -82,9 +81,57 @@ func TestMergeKubeadmEnvVars(t *testing.T) {
8281
for _, test := range tests {
8382
t.Run(test.name, func(t *testing.T) {
8483
envs := MergeKubeadmEnvVars(test.proxyEnv, test.extraEnv)
85-
if !assert.ElementsMatch(t, envs, test.mergedEnv) {
84+
if !reflect.DeepEqual(envs, test.mergedEnv) {
8685
t.Errorf("expected env: %v, got: %v", test.mergedEnv, envs)
8786
}
8887
})
8988
}
9089
}
90+
91+
func TestGetProxyEnvVars(t *testing.T) {
92+
var tests = []struct {
93+
name string
94+
environment []string
95+
expected []kubeadmapi.EnvVar
96+
}{
97+
{
98+
name: "environment with lowercase proxy vars",
99+
environment: []string{
100+
"http_proxy=p1",
101+
"foo1=bar1",
102+
"https_proxy=p2",
103+
"foo2=bar2",
104+
"no_proxy=p3",
105+
},
106+
expected: []kubeadmapi.EnvVar{
107+
{EnvVar: v1.EnvVar{Name: "http_proxy", Value: "p1"}},
108+
{EnvVar: v1.EnvVar{Name: "https_proxy", Value: "p2"}},
109+
{EnvVar: v1.EnvVar{Name: "no_proxy", Value: "p3"}},
110+
},
111+
},
112+
{
113+
name: "environment with uppercase proxy vars",
114+
environment: []string{
115+
"HTTP_PROXY=p1",
116+
"foo1=bar1",
117+
"HTTPS_PROXY=p2",
118+
"foo2=bar2",
119+
"NO_PROXY=p3",
120+
},
121+
expected: []kubeadmapi.EnvVar{
122+
{EnvVar: v1.EnvVar{Name: "HTTP_PROXY", Value: "p1"}},
123+
{EnvVar: v1.EnvVar{Name: "HTTPS_PROXY", Value: "p2"}},
124+
{EnvVar: v1.EnvVar{Name: "NO_PROXY", Value: "p3"}},
125+
},
126+
},
127+
}
128+
129+
for _, test := range tests {
130+
t.Run(test.name, func(t *testing.T) {
131+
envs := GetProxyEnvVars(test.environment)
132+
if !reflect.DeepEqual(envs, test.expected) {
133+
t.Errorf("expected env: %v, got: %v", test.expected, envs)
134+
}
135+
})
136+
}
137+
}

0 commit comments

Comments
 (0)