Skip to content

Commit 85a68e0

Browse files
jimmidysonjimmidyson
committed
refactor: Adopt simpler proxy generator funcs
Small tweak that satisfies my desire for side-effect free functions and I have also updated tests to have slightly better checks.
1 parent a1b14cb commit 85a68e0

File tree

3 files changed

+115
-129
lines changed

3 files changed

+115
-129
lines changed

‎pkg/handlers/httpproxy/inject.go

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,7 @@ const (
2929
)
3030

3131
type httpProxyPatchHandler struct {
32-
decoder runtime.Decoder
33-
generator *systemdConfigGenerator
32+
decoder runtime.Decoder
3433
}
3534

3635
var (
@@ -47,9 +46,6 @@ func NewPatch() *httpProxyPatchHandler {
4746
controlplanev1.GroupVersion,
4847
bootstrapv1.GroupVersion,
4948
),
50-
generator: &systemdConfigGenerator{
51-
template: templates.Lookup("systemd.conf.tmpl"),
52-
},
5349
}
5450
}
5551

@@ -101,14 +97,15 @@ func (h *httpProxyPatchHandler) GeneratePatches(
10197
if err := generatePatch(
10298
obj, variables, &holderRef, controlPlaneSelector, log,
10399
func(obj *controlplanev1.KubeadmControlPlaneTemplate) error {
104-
var err error
105100
log.WithValues("namespacedName", types.NamespacedName{
106101
Name: obj.Name,
107102
Namespace: obj.Namespace,
108103
}).Info("adding files to kubeadm config spec")
109-
obj.Spec.Template.Spec.KubeadmConfigSpec.Files, err = h.generator.AddSystemdFiles(
110-
httpProxyVariable, obj.Spec.Template.Spec.KubeadmConfigSpec.Files)
111-
return err
104+
obj.Spec.Template.Spec.KubeadmConfigSpec.Files = append(
105+
obj.Spec.Template.Spec.KubeadmConfigSpec.Files,
106+
generateSystemdFiles(httpProxyVariable)...,
107+
)
108+
return nil
112109
}); err != nil {
113110
return err
114111
}
@@ -127,13 +124,15 @@ func (h *httpProxyPatchHandler) GeneratePatches(
127124
if err := generatePatch(
128125
obj, variables, &holderRef, defaultWorkerSelector, log,
129126
func(obj *bootstrapv1.KubeadmConfigTemplate) error {
130-
var err error
131127
log.WithValues("namespacedName", types.NamespacedName{
132128
Name: obj.Name,
133129
Namespace: obj.Namespace,
134130
}).Info("adding files to worker node kubeadm config template")
135-
obj.Spec.Template.Spec.Files, err = h.generator.AddSystemdFiles(httpProxyVariable, obj.Spec.Template.Spec.Files)
136-
return err
131+
obj.Spec.Template.Spec.Files = append(
132+
obj.Spec.Template.Spec.Files,
133+
generateSystemdFiles(httpProxyVariable)...,
134+
)
135+
return nil
137136
}); err != nil {
138137
return err
139138
}

‎pkg/handlers/httpproxy/systemd_proxy_config.go

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,19 @@ var (
1616
//go:embed templates
1717
sources embed.FS
1818

19-
templates *template.Template = template.Must(template.ParseFS(sources, "templates/*.tmpl"))
19+
templates *template.Template = template.Must(template.ParseFS(sources, "templates/systemd.conf.tmpl")).Templates()[0]
2020

2121
systemdUnitPaths = []string{
2222
"/etc/systemd/system/containerd.service.d/http-proxy.conf",
2323
"/etc/systemd/system/kubelet.service.d/http-proxy.conf",
2424
}
2525
)
2626

27-
type systemdConfigGenerator struct {
28-
template *template.Template
29-
}
27+
func generateSystemdFiles(vars HTTPProxyVariables) []bootstrapv1.File {
28+
if vars.HTTP == "" && vars.HTTPS == "" && len(vars.NO) == 0 {
29+
return nil
30+
}
3031

31-
func (g *systemdConfigGenerator) Generate(vars HTTPProxyVariables) (string, error) {
3232
tplVars := struct {
3333
HTTP string
3434
HTTPS string
@@ -41,23 +41,19 @@ func (g *systemdConfigGenerator) Generate(vars HTTPProxyVariables) (string, erro
4141

4242
var buf bytes.Buffer
4343
err := templates.ExecuteTemplate(&buf, "systemd.conf.tmpl", tplVars)
44-
return buf.String(), err
45-
}
46-
47-
func (g *systemdConfigGenerator) AddSystemdFiles(
48-
vars HTTPProxyVariables, dest []bootstrapv1.File,
49-
) ([]bootstrapv1.File, error) {
50-
content, err := g.Generate(vars)
5144
if err != nil {
52-
return nil, err
45+
panic(err) // This should be impossible with the simple template we have.
5346
}
5447

48+
files := make([]bootstrapv1.File, 0, len(systemdUnitPaths))
49+
5550
for _, path := range systemdUnitPaths {
56-
dest = append(dest, bootstrapv1.File{
51+
files = append(files, bootstrapv1.File{
5752
Path: path,
5853
Permissions: "0640",
59-
Content: content,
54+
Content: buf.String(),
55+
Owner: "root",
6056
})
6157
}
62-
return dest, nil
58+
return files
6359
}

‎pkg/handlers/httpproxy/systemd_proxy_config_test.go

Lines changed: 92 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -10,107 +10,98 @@ import (
1010
bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1"
1111
)
1212

13-
func newTestGenerator() *systemdConfigGenerator {
14-
return &systemdConfigGenerator{
15-
template: templates.Lookup("systemd.conf.tmpl"),
16-
}
17-
}
18-
19-
func TestGenerate(t *testing.T) {
20-
g := NewWithT(t)
21-
22-
content, err := newTestGenerator().Generate(HTTPProxyVariables{
23-
HTTP: "http://example.com",
24-
HTTPS: "https://example.com",
25-
NO: []string{
26-
"https://no-proxy.example.com",
13+
func TestGenerateSystemdFiles(t *testing.T) {
14+
t.Parallel()
15+
16+
tests := []struct {
17+
name string
18+
vars HTTPProxyVariables
19+
expectedContents string
20+
}{{
21+
name: "no proxy configuration",
22+
}, {
23+
name: "all vars set",
24+
vars: HTTPProxyVariables{
25+
HTTP: "http://example.com",
26+
HTTPS: "https://example.com",
27+
NO: []string{
28+
"https://no-proxy.example.com",
29+
},
2730
},
28-
})
29-
g.Expect(err).NotTo(HaveOccurred())
30-
g.Expect(content).To(And(
31-
ContainSubstring(`Environment="HTTP_PROXY=http://example.com"`),
32-
ContainSubstring(`Environment="http_proxy=http://example.com"`),
33-
ContainSubstring(`Environment="HTTPS_PROXY=https://example.com"`),
34-
ContainSubstring(`Environment="https_proxy=https://example.com"`),
35-
ContainSubstring(`Environment="NO_PROXY=https://no-proxy.example.com"`),
36-
ContainSubstring(`Environment="no_proxy=https://no-proxy.example.com"`),
37-
))
38-
}
39-
40-
func TestGenerate_OnlyHTTP(t *testing.T) {
41-
g := NewWithT(t)
42-
43-
content, err := newTestGenerator().Generate(HTTPProxyVariables{
44-
HTTP: "http://example.com",
45-
})
46-
g.Expect(err).NotTo(HaveOccurred())
47-
g.Expect(content).To(And(
48-
ContainSubstring(`Environment="HTTP_PROXY=http://example.com"`),
49-
ContainSubstring(`Environment="http_proxy=http://example.com"`),
50-
Not(ContainSubstring(`Environment="HTTPS_PROXY=`)),
51-
Not(ContainSubstring(`Environment="https_proxy=`)),
52-
Not(ContainSubstring(`Environment="NO_PROXY=`)),
53-
Not(ContainSubstring(`Environment="no_proxy=`)),
54-
))
55-
}
56-
57-
func TestGenerate_OnlyHTTPS(t *testing.T) {
58-
g := NewWithT(t)
59-
60-
content, err := newTestGenerator().Generate(HTTPProxyVariables{
61-
HTTPS: "https://example.com",
62-
})
63-
g.Expect(err).NotTo(HaveOccurred())
64-
g.Expect(content).To(And(
65-
Not(ContainSubstring(`Environment="HTTP_PROXY=http://example.com"`)),
66-
Not(ContainSubstring(`Environment="http_proxy=http://example.com"`)),
67-
ContainSubstring(`Environment="HTTPS_PROXY=https://example.com"`),
68-
ContainSubstring(`Environment="https_proxy=https://example.com"`),
69-
Not(ContainSubstring(`Environment="NO_PROXY=https://no-proxy.example.com"`)),
70-
Not(ContainSubstring(`Environment="no_proxy=https://no-proxy.example.com"`)),
71-
))
72-
}
73-
74-
func TestGenerate_OnlyNoProxy(t *testing.T) {
75-
g := NewWithT(t)
76-
77-
content, err := newTestGenerator().Generate(HTTPProxyVariables{
78-
NO: []string{"https://no-proxy.example.com"},
79-
})
80-
g.Expect(err).NotTo(HaveOccurred())
81-
g.Expect(content).To(And(
82-
Not(ContainSubstring(`Environment="HTTP_PROXY="`)),
83-
Not(ContainSubstring(`Environment="http_proxy="`)),
84-
Not(ContainSubstring(`Environment="HTTPS_PROXY="`)),
85-
Not(ContainSubstring(`Environment="https_proxy=`)),
86-
ContainSubstring(`Environment="NO_PROXY=https://no-proxy.example.com"`),
87-
ContainSubstring(`Environment="no_proxy=https://no-proxy.example.com"`),
88-
))
89-
}
90-
91-
func TestGenerate_NoProxyMultipleURLs(t *testing.T) {
92-
g := NewWithT(t)
93-
94-
content, err := newTestGenerator().Generate(HTTPProxyVariables{
95-
NO: []string{
96-
"https://no-proxy.example.com",
97-
"https://no-proxy-1.example.com",
31+
expectedContents: `[Service]
32+
Environment="HTTP_PROXY=http://example.com"
33+
Environment="http_proxy=http://example.com"
34+
Environment="HTTPS_PROXY=https://example.com"
35+
Environment="https_proxy=https://example.com"
36+
Environment="NO_PROXY=https://no-proxy.example.com"
37+
Environment="no_proxy=https://no-proxy.example.com"
38+
`,
39+
}, {
40+
name: "http only",
41+
vars: HTTPProxyVariables{
42+
HTTP: "http://example.com",
9843
},
99-
})
100-
g.Expect(err).NotTo(HaveOccurred())
101-
g.Expect(content).To(And(
102-
ContainSubstring(
103-
`Environment="NO_PROXY=https://no-proxy.example.com,https://no-proxy-1.example.com"`,
104-
),
105-
ContainSubstring(
106-
`Environment="no_proxy=https://no-proxy.example.com,https://no-proxy-1.example.com"`,
107-
),
108-
))
109-
}
110-
111-
func TestAddSystemdFiles(t *testing.T) {
112-
g := NewWithT(t)
113-
114-
dst := []bootstrapv1.File{}
115-
g.Expect(newTestGenerator().AddSystemdFiles(HTTPProxyVariables{}, dst)).To(HaveLen(2))
44+
expectedContents: `[Service]
45+
Environment="HTTP_PROXY=http://example.com"
46+
Environment="http_proxy=http://example.com"
47+
`,
48+
}, {
49+
name: "https only",
50+
vars: HTTPProxyVariables{
51+
HTTPS: "https://example.com",
52+
},
53+
expectedContents: `[Service]
54+
Environment="HTTPS_PROXY=https://example.com"
55+
Environment="https_proxy=https://example.com"
56+
`,
57+
}, {
58+
name: "no proxy only",
59+
vars: HTTPProxyVariables{
60+
NO: []string{
61+
"https://no-proxy.example.com",
62+
},
63+
},
64+
expectedContents: `[Service]
65+
Environment="NO_PROXY=https://no-proxy.example.com"
66+
Environment="no_proxy=https://no-proxy.example.com"
67+
`,
68+
}, {
69+
name: "multiple no proxy only",
70+
vars: HTTPProxyVariables{
71+
NO: []string{
72+
"https://no-proxy.example.com",
73+
"https://no-proxy-1.example.com",
74+
},
75+
},
76+
expectedContents: `[Service]
77+
Environment="NO_PROXY=https://no-proxy.example.com,https://no-proxy-1.example.com"
78+
Environment="no_proxy=https://no-proxy.example.com,https://no-proxy-1.example.com"
79+
`,
80+
}}
81+
82+
for idx := range tests {
83+
tt := tests[idx]
84+
t.Run(tt.name, func(t *testing.T) {
85+
t.Parallel()
86+
87+
g := NewWithT(t)
88+
89+
var expectedFiles []bootstrapv1.File
90+
if tt.expectedContents != "" {
91+
expectedFiles = []bootstrapv1.File{{
92+
Path: systemdUnitPaths[0],
93+
Content: tt.expectedContents,
94+
Permissions: "0640",
95+
Owner: "root",
96+
}, {
97+
Path: systemdUnitPaths[1],
98+
Content: tt.expectedContents,
99+
Permissions: "0640",
100+
Owner: "root",
101+
}}
102+
}
103+
104+
g.Expect(generateSystemdFiles(tt.vars)).Should(Equal(expectedFiles))
105+
})
106+
}
116107
}

0 commit comments

Comments
 (0)