refactor: Adopt simpler proxy generator funcs

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
 			}

pkg/handlers/httpproxy/systemd_proxy_config.go

@@ -16,19 +16,19 @@ 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",
 		"/etc/systemd/system/kubelet.service.d/http-proxy.conf",
 	}
 )
 
-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
 }

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))
+		})
+	}
 }