Skip to content

Commit 080b666

Browse files
authored
Merge pull request #469 from slauger/fix/code-volume-empty-mountpath
fix: default environmentPath for code volume mount
2 parents 3072a22 + 5de5096 commit 080b666

5 files changed

Lines changed: 55 additions & 5 deletions

File tree

internal/controller/config_controller_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,15 @@ func TestConfigReconcile_PuppetConfRendering(t *testing.T) {
104104
})},
105105
contains: []string{"environmentpath = /custom/code/environments"},
106106
},
107+
{
108+
// Regression for #463: an empty environmentPath must still emit the
109+
// default so puppet.conf and the code volume mountPath stay consistent.
110+
name: "default environmentPath when unset",
111+
opts: []configOption{withPuppetSpec(openvoxv1alpha1.PuppetSpec{
112+
Reports: "puppetdb",
113+
})},
114+
contains: []string{"environmentpath = /etc/puppetlabs/code/environments"},
115+
},
107116
{
108117
name: "custom hieraConfig",
109118
opts: []configOption{withPuppetSpec(openvoxv1alpha1.PuppetSpec{

internal/controller/config_rendering.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@ func (r *ConfigReconciler) renderPuppetConf(ctx context.Context, cfg *openvoxv1a
2121
sb.WriteString("rundir = /var/run/puppetlabs\n")
2222
sb.WriteString("manage_internal_file_permissions = false\n")
2323

24-
if cfg.Spec.Puppet.EnvironmentPath != "" {
25-
fmt.Fprintf(&sb, "environmentpath = %s\n", cfg.Spec.Puppet.EnvironmentPath)
26-
}
24+
fmt.Fprintf(&sb, "environmentpath = %s\n", resolveEnvironmentPath(cfg))
2725

2826
if cfg.Spec.Puppet.HieraConfig != "" {
2927
fmt.Fprintf(&sb, "hiera_config = %s\n", cfg.Spec.Puppet.HieraConfig)

internal/controller/helpers.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,23 @@ func caInternalServiceName(caName string) string {
169169
return fmt.Sprintf("%s-internal", caName)
170170
}
171171

172+
// defaultEnvironmentPath is the fallback puppet environmentpath used when
173+
// spec.puppet.environmentPath is unset. It mirrors the kubebuilder default on
174+
// PuppetSpec.EnvironmentPath, which is not applied by the API server when the
175+
// whole spec.puppet object is omitted (nested defaults require the parent object
176+
// to be present). Resolving it here keeps hand-written Configs from rendering an
177+
// empty volume mountPath, which the Kubernetes API rejects.
178+
const defaultEnvironmentPath = "/etc/puppetlabs/code/environments"
179+
180+
// resolveEnvironmentPath returns the configured puppet environmentpath, falling
181+
// back to defaultEnvironmentPath when unset.
182+
func resolveEnvironmentPath(cfg *openvoxv1alpha1.Config) string {
183+
if cfg.Spec.Puppet.EnvironmentPath != "" {
184+
return cfg.Spec.Puppet.EnvironmentPath
185+
}
186+
return defaultEnvironmentPath
187+
}
188+
172189
// resolveCode determines the code source for a Server.
173190
// Priority: Server override > Config default.
174191
func resolveCode(server *openvoxv1alpha1.Server, cfg *openvoxv1alpha1.Config) *openvoxv1alpha1.CodeSpec {

internal/controller/server_deployment.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ func (r *ServerReconciler) buildPodSpec(server *openvoxv1alpha1.Server, cfg *ope
296296
if code := resolveCode(server, cfg); code != nil {
297297
volumeMounts = append(volumeMounts, corev1.VolumeMount{
298298
Name: "code",
299-
MountPath: cfg.Spec.Puppet.EnvironmentPath,
299+
MountPath: resolveEnvironmentPath(cfg),
300300
ReadOnly: true,
301301
})
302302
switch {
@@ -332,7 +332,7 @@ func (r *ServerReconciler) buildPodSpec(server *openvoxv1alpha1.Server, cfg *ope
332332
// filesystem prevents creating the directory and the pod crash-loops.
333333
volumeMounts = append(volumeMounts, corev1.VolumeMount{
334334
Name: "code",
335-
MountPath: cfg.Spec.Puppet.EnvironmentPath,
335+
MountPath: resolveEnvironmentPath(cfg),
336336
})
337337
volumes = append(volumes, corev1.Volume{
338338
Name: "code",

internal/controller/server_deployment_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,32 @@ func TestBuildPodSpec_CodeVolumeImage(t *testing.T) {
143143
t.Error("code volume not found")
144144
}
145145

146+
// TestBuildPodSpec_CodeVolumeDefaultsMountPath is a regression test for #463:
147+
// when spec.code is set but spec.puppet.environmentPath is empty (e.g. a
148+
// hand-written Config that omits the whole spec.puppet block, so the CRD default
149+
// is never applied), the code volume must still render a valid mountPath rather
150+
// than "", which the Kubernetes API rejects.
151+
func TestBuildPodSpec_CodeVolumeDefaultsMountPath(t *testing.T) {
152+
cfg := newConfig("production", withCodeImage("ghcr.io/slauger/puppet-code:v1.0"))
153+
cfg.Spec.Puppet.EnvironmentPath = ""
154+
server := newServer("test-server", withServerRole(true))
155+
156+
podSpec := testBuildPodSpec(server, cfg)
157+
158+
found := false
159+
for _, vm := range podSpec.Containers[0].VolumeMounts {
160+
if vm.Name == "code" {
161+
found = true
162+
if vm.MountPath != defaultEnvironmentPath {
163+
t.Errorf("expected code mountPath %q, got %q", defaultEnvironmentPath, vm.MountPath)
164+
}
165+
}
166+
}
167+
if !found {
168+
t.Fatal("code volume mount not found")
169+
}
170+
}
171+
146172
func TestBuildPodSpec_CodeVolumePVC(t *testing.T) {
147173
cfg := newConfig("production", withCodePVC("puppet-code-pvc"))
148174
server := newServer("test-server", withServerRole(true))

0 commit comments

Comments
 (0)