Skip to content

Commit

Permalink
refactor: Adopt simpler proxy generator funcs
Browse files Browse the repository at this point in the history 
Small tweak that satisfies my desire for side-effect free functions
and I have also updated tests to have slightly better checks.
  • Loading branch information
@jimmidyson
jimmidyson committed Aug 30, 2023
1 parent a1b14cb commit 85a68e0
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 129 deletions.
23 changes: 11 additions & 12 deletions pkg/handlers/httpproxy/inject.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ const (
)

type httpProxyPatchHandler struct {
decoder runtime.Decoder
generator *systemdConfigGenerator
decoder runtime.Decoder
}

var (
Expand All @@ -47,9 +46,6 @@ func NewPatch() *httpProxyPatchHandler {
controlplanev1.GroupVersion,
bootstrapv1.GroupVersion,
),
generator: &systemdConfigGenerator{
template: templates.Lookup("systemd.conf.tmpl"),
},
}
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
28 changes: 12 additions & 16 deletions pkg/handlers/httpproxy/systemd_proxy_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
193 changes: 92 additions & 101 deletions pkg/handlers/httpproxy/systemd_proxy_config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
}
}

0 comments on commit 85a68e0

Please sign in to comment.