diff --git a/pkg/handlers/httpproxy/inject.go b/pkg/handlers/httpproxy/inject.go index 6ff598075..7a90fb499 100644 --- a/pkg/handlers/httpproxy/inject.go +++ b/pkg/handlers/httpproxy/inject.go @@ -29,8 +29,7 @@ const ( ) type httpProxyPatchHandler struct { - decoder runtime.Decoder - generator *systemdConfigGenerator + decoder runtime.Decoder } var ( @@ -47,9 +46,6 @@ func NewPatch() *httpProxyPatchHandler { controlplanev1.GroupVersion, bootstrapv1.GroupVersion, ), - generator: &systemdConfigGenerator{ - template: templates.Lookup("systemd.conf.tmpl"), - }, } } @@ -101,14 +97,15 @@ func (h *httpProxyPatchHandler) GeneratePatches( if err := generatePatch( obj, variables, &holderRef, controlPlaneSelector, log, func(obj *controlplanev1.KubeadmControlPlaneTemplate) error { - var err error log.WithValues("namespacedName", types.NamespacedName{ Name: obj.Name, Namespace: obj.Namespace, }).Info("adding files to kubeadm config spec") - obj.Spec.Template.Spec.KubeadmConfigSpec.Files, err = h.generator.AddSystemdFiles( - httpProxyVariable, obj.Spec.Template.Spec.KubeadmConfigSpec.Files) - return err + obj.Spec.Template.Spec.KubeadmConfigSpec.Files = append( + obj.Spec.Template.Spec.KubeadmConfigSpec.Files, + generateSystemdFiles(httpProxyVariable)..., + ) + return nil }); err != nil { return err } @@ -127,13 +124,15 @@ func (h *httpProxyPatchHandler) GeneratePatches( if err := generatePatch( obj, variables, &holderRef, defaultWorkerSelector, log, func(obj *bootstrapv1.KubeadmConfigTemplate) error { - var err error log.WithValues("namespacedName", types.NamespacedName{ Name: obj.Name, Namespace: obj.Namespace, }).Info("adding files to worker node kubeadm config template") - obj.Spec.Template.Spec.Files, err = h.generator.AddSystemdFiles(httpProxyVariable, obj.Spec.Template.Spec.Files) - return err + obj.Spec.Template.Spec.Files = append( + obj.Spec.Template.Spec.Files, + generateSystemdFiles(httpProxyVariable)..., + ) + return nil }); err != nil { return err } diff --git a/pkg/handlers/httpproxy/systemd_proxy_config.go b/pkg/handlers/httpproxy/systemd_proxy_config.go index dd4d0d653..8d43e8fbc 100644 --- a/pkg/handlers/httpproxy/systemd_proxy_config.go +++ b/pkg/handlers/httpproxy/systemd_proxy_config.go @@ -16,7 +16,7 @@ var ( //go:embed templates sources embed.FS - templates *template.Template = template.Must(template.ParseFS(sources, "templates/*.tmpl")) + templates *template.Template = template.Must(template.ParseFS(sources, "templates/systemd.conf.tmpl")).Templates()[0] systemdUnitPaths = []string{ "/etc/systemd/system/containerd.service.d/http-proxy.conf", @@ -24,11 +24,11 @@ var ( } ) -type systemdConfigGenerator struct { - template *template.Template -} +func generateSystemdFiles(vars HTTPProxyVariables) []bootstrapv1.File { + if vars.HTTP == "" && vars.HTTPS == "" && len(vars.NO) == 0 { + return nil + } -func (g *systemdConfigGenerator) Generate(vars HTTPProxyVariables) (string, error) { tplVars := struct { HTTP string HTTPS string @@ -41,23 +41,19 @@ func (g *systemdConfigGenerator) Generate(vars HTTPProxyVariables) (string, erro var buf bytes.Buffer err := templates.ExecuteTemplate(&buf, "systemd.conf.tmpl", tplVars) - return buf.String(), err -} - -func (g *systemdConfigGenerator) AddSystemdFiles( - vars HTTPProxyVariables, dest []bootstrapv1.File, -) ([]bootstrapv1.File, error) { - content, err := g.Generate(vars) if err != nil { - return nil, err + panic(err) // This should be impossible with the simple template we have. } + files := make([]bootstrapv1.File, 0, len(systemdUnitPaths)) + for _, path := range systemdUnitPaths { - dest = append(dest, bootstrapv1.File{ + files = append(files, bootstrapv1.File{ Path: path, Permissions: "0640", - Content: content, + Content: buf.String(), + Owner: "root", }) } - return dest, nil + return files } diff --git a/pkg/handlers/httpproxy/systemd_proxy_config_test.go b/pkg/handlers/httpproxy/systemd_proxy_config_test.go index bdfa58558..6c2361eb8 100644 --- a/pkg/handlers/httpproxy/systemd_proxy_config_test.go +++ b/pkg/handlers/httpproxy/systemd_proxy_config_test.go @@ -10,107 +10,98 @@ import ( bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" ) -func newTestGenerator() *systemdConfigGenerator { - return &systemdConfigGenerator{ - template: templates.Lookup("systemd.conf.tmpl"), - } -} - -func TestGenerate(t *testing.T) { - g := NewWithT(t) - - content, err := newTestGenerator().Generate(HTTPProxyVariables{ - HTTP: "http://example.com", - HTTPS: "https://example.com", - NO: []string{ - "https://no-proxy.example.com", +func TestGenerateSystemdFiles(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + vars HTTPProxyVariables + expectedContents string + }{{ + name: "no proxy configuration", + }, { + name: "all vars set", + vars: HTTPProxyVariables{ + HTTP: "http://example.com", + HTTPS: "https://example.com", + NO: []string{ + "https://no-proxy.example.com", + }, }, - }) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(content).To(And( - ContainSubstring(`Environment="HTTP_PROXY=http://example.com"`), - ContainSubstring(`Environment="http_proxy=http://example.com"`), - ContainSubstring(`Environment="HTTPS_PROXY=https://example.com"`), - ContainSubstring(`Environment="https_proxy=https://example.com"`), - ContainSubstring(`Environment="NO_PROXY=https://no-proxy.example.com"`), - ContainSubstring(`Environment="no_proxy=https://no-proxy.example.com"`), - )) -} - -func TestGenerate_OnlyHTTP(t *testing.T) { - g := NewWithT(t) - - content, err := newTestGenerator().Generate(HTTPProxyVariables{ - HTTP: "http://example.com", - }) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(content).To(And( - ContainSubstring(`Environment="HTTP_PROXY=http://example.com"`), - ContainSubstring(`Environment="http_proxy=http://example.com"`), - Not(ContainSubstring(`Environment="HTTPS_PROXY=`)), - Not(ContainSubstring(`Environment="https_proxy=`)), - Not(ContainSubstring(`Environment="NO_PROXY=`)), - Not(ContainSubstring(`Environment="no_proxy=`)), - )) -} - -func TestGenerate_OnlyHTTPS(t *testing.T) { - g := NewWithT(t) - - content, err := newTestGenerator().Generate(HTTPProxyVariables{ - HTTPS: "https://example.com", - }) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(content).To(And( - Not(ContainSubstring(`Environment="HTTP_PROXY=http://example.com"`)), - Not(ContainSubstring(`Environment="http_proxy=http://example.com"`)), - ContainSubstring(`Environment="HTTPS_PROXY=https://example.com"`), - ContainSubstring(`Environment="https_proxy=https://example.com"`), - Not(ContainSubstring(`Environment="NO_PROXY=https://no-proxy.example.com"`)), - Not(ContainSubstring(`Environment="no_proxy=https://no-proxy.example.com"`)), - )) -} - -func TestGenerate_OnlyNoProxy(t *testing.T) { - g := NewWithT(t) - - content, err := newTestGenerator().Generate(HTTPProxyVariables{ - NO: []string{"https://no-proxy.example.com"}, - }) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(content).To(And( - Not(ContainSubstring(`Environment="HTTP_PROXY="`)), - Not(ContainSubstring(`Environment="http_proxy="`)), - Not(ContainSubstring(`Environment="HTTPS_PROXY="`)), - Not(ContainSubstring(`Environment="https_proxy=`)), - ContainSubstring(`Environment="NO_PROXY=https://no-proxy.example.com"`), - ContainSubstring(`Environment="no_proxy=https://no-proxy.example.com"`), - )) -} - -func TestGenerate_NoProxyMultipleURLs(t *testing.T) { - g := NewWithT(t) - - content, err := newTestGenerator().Generate(HTTPProxyVariables{ - NO: []string{ - "https://no-proxy.example.com", - "https://no-proxy-1.example.com", + expectedContents: `[Service] +Environment="HTTP_PROXY=http://example.com" +Environment="http_proxy=http://example.com" +Environment="HTTPS_PROXY=https://example.com" +Environment="https_proxy=https://example.com" +Environment="NO_PROXY=https://no-proxy.example.com" +Environment="no_proxy=https://no-proxy.example.com" +`, + }, { + name: "http only", + vars: HTTPProxyVariables{ + HTTP: "http://example.com", }, - }) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(content).To(And( - ContainSubstring( - `Environment="NO_PROXY=https://no-proxy.example.com,https://no-proxy-1.example.com"`, - ), - ContainSubstring( - `Environment="no_proxy=https://no-proxy.example.com,https://no-proxy-1.example.com"`, - ), - )) -} - -func TestAddSystemdFiles(t *testing.T) { - g := NewWithT(t) - - dst := []bootstrapv1.File{} - g.Expect(newTestGenerator().AddSystemdFiles(HTTPProxyVariables{}, dst)).To(HaveLen(2)) + expectedContents: `[Service] +Environment="HTTP_PROXY=http://example.com" +Environment="http_proxy=http://example.com" +`, + }, { + name: "https only", + vars: HTTPProxyVariables{ + HTTPS: "https://example.com", + }, + expectedContents: `[Service] +Environment="HTTPS_PROXY=https://example.com" +Environment="https_proxy=https://example.com" +`, + }, { + name: "no proxy only", + vars: HTTPProxyVariables{ + NO: []string{ + "https://no-proxy.example.com", + }, + }, + expectedContents: `[Service] +Environment="NO_PROXY=https://no-proxy.example.com" +Environment="no_proxy=https://no-proxy.example.com" +`, + }, { + name: "multiple no proxy only", + vars: HTTPProxyVariables{ + NO: []string{ + "https://no-proxy.example.com", + "https://no-proxy-1.example.com", + }, + }, + expectedContents: `[Service] +Environment="NO_PROXY=https://no-proxy.example.com,https://no-proxy-1.example.com" +Environment="no_proxy=https://no-proxy.example.com,https://no-proxy-1.example.com" +`, + }} + + for idx := range tests { + tt := tests[idx] + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + g := NewWithT(t) + + var expectedFiles []bootstrapv1.File + if tt.expectedContents != "" { + expectedFiles = []bootstrapv1.File{{ + Path: systemdUnitPaths[0], + Content: tt.expectedContents, + Permissions: "0640", + Owner: "root", + }, { + Path: systemdUnitPaths[1], + Content: tt.expectedContents, + Permissions: "0640", + Owner: "root", + }} + } + + g.Expect(generateSystemdFiles(tt.vars)).Should(Equal(expectedFiles)) + }) + } }