diff --git a/.woodpecker/pre-commit.yaml b/.woodpecker/pre-commit.yaml index bc12ad7..b311f00 100644 --- a/.woodpecker/pre-commit.yaml +++ b/.woodpecker/pre-commit.yaml @@ -17,3 +17,18 @@ steps: commands: - test -z "$(gofmt -l .)" - go vet ./... + + - name: shellcheck + image: koalaman/shellcheck-alpine:stable + backend_options: + kubernetes: + serviceAccountName: kea-operator-ci + resources: + requests: + memory: 256Mi + cpu: "500m" + limits: + memory: 512Mi + cpu: "1" + commands: + - shellcheck --shell=sh internal/kea/scripts/*.sh diff --git a/README.md b/README.md index 6c0a1bf..d541587 100644 --- a/README.md +++ b/README.md @@ -35,9 +35,18 @@ Kea HA needs a **stable per-peer identity** (each server must know which peer it is, and peers reference each other by stable URL). That is exactly why this operator (like bind-operator) uses a **StatefulSet** rather than a bare Deployment: pods get stable ordinals (`-0`, `-1`) and headless -DNS, the entrypoint derives `this-server-name` from the ordinal, and the peer +DNS, an initContainer derives `this-server-name` from the ordinal, and the peer URLs are DNS names (never pod IPs, so the config hash never loops). +Startup preconditions live in a `kea-init` initContainer (a committed, +shellcheck-clean `internal/kea/scripts/init.sh` embedded via `go:embed` and +parameterised by env vars — no shell is interpolated in Go). It hardens the +shared run dir to `0750`, substitutes `this-server-name` from the pod ordinal, +stages both configs into the shared `emptyDir`, and bounded-waits for the HA +peer DNS to resolve (`kea-dhcp4 -t`, failing loud after the cap so the kubelet +restarts it). The main `kea-dhcp4` and `kea-ctrl-agent` containers then exec kea +directly with no wrapper shell. + ### Anycast routing caveat (deployment follow-up) The DHCP `Service` is a `LoadBalancer` intended to receive a PureLB anycast IP; diff --git a/internal/controller/keacluster_controller.go b/internal/controller/keacluster_controller.go index 8c3470b..d10ffb1 100644 --- a/internal/controller/keacluster_controller.go +++ b/internal/controller/keacluster_controller.go @@ -3,6 +3,7 @@ package controller import ( "context" "fmt" + "strconv" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -173,10 +174,9 @@ func (r *KeaClusterReconciler) reconcileConfigMap(ctx context.Context, c *v1alph _, err = ctrl.CreateOrUpdate(ctx, r.Client, cm, func() error { cm.Labels = commonLabels(c.Name) cm.Data = map[string]string{ - "kea-dhcp4.conf": dhcp4, - "kea-ctrl-agent.conf": agent, - "entrypoint-dhcp4.sh": kea.EntrypointDHCP4(), - "entrypoint-ctrlagent.sh": kea.EntrypointCtrlAgent(), + "kea-dhcp4.conf": dhcp4, + "kea-ctrl-agent.conf": agent, + "init.sh": kea.InitScript(), } return ctrl.SetControllerReference(c, cm, r.Scheme) }) @@ -288,10 +288,21 @@ func (r *KeaClusterReconciler) podTemplate(c *v1alpha1.KeaCluster, image, hash s {Name: "run", MountPath: kea.RunDir}, } + // The initContainer finalises the per-pod config and bounded-waits for HA + // peer DNS; the main containers then exec kea directly with no wrapper shell. + initC := corev1.Container{ + Name: kea.ContainerInit, + Image: image, + Command: []string{"/bin/sh", kea.InitScriptPath}, + Env: initEnv(), + Resources: c.Spec.Resources, + VolumeMounts: mounts, + } + dhcp4 := corev1.Container{ Name: kea.ContainerDHCP4, Image: image, - Command: []string{"/bin/sh", kea.ConfigDir + "/entrypoint-dhcp4.sh"}, + Command: []string{kea.DHCP4Bin, "-c", kea.DHCP4ConfPath}, Resources: c.Spec.Resources, Ports: []corev1.ContainerPort{ {Name: "dhcp", ContainerPort: kea.DHCP4Port, Protocol: corev1.ProtocolUDP}, @@ -301,7 +312,7 @@ func (r *KeaClusterReconciler) podTemplate(c *v1alpha1.KeaCluster, image, hash s agent := corev1.Container{ Name: kea.ContainerCtrlAgent, Image: image, - Command: []string{"/bin/sh", kea.ConfigDir + "/entrypoint-ctrlagent.sh"}, + Command: []string{kea.CtrlAgentBin, "-c", kea.CtrlAgentConfPath}, Resources: c.Spec.Resources, Ports: []corev1.ContainerPort{ {Name: "ctrl", ContainerPort: kea.CtrlAgentPort, Protocol: corev1.ProtocolTCP}, @@ -320,15 +331,35 @@ func (r *KeaClusterReconciler) podTemplate(c *v1alpha1.KeaCluster, image, hash s Annotations: map[string]string{"kea.unkin.net/config-hash": hash}, }, Spec: corev1.PodSpec{ - Containers: []corev1.Container{dhcp4, agent}, - Volumes: []corev1.Volume{volProjected, volRun}, - NodeSelector: c.Spec.NodeSelector, - Tolerations: c.Spec.Tolerations, - Affinity: c.Spec.Affinity, + InitContainers: []corev1.Container{initC}, + Containers: []corev1.Container{dhcp4, agent}, + Volumes: []corev1.Volume{volProjected, volRun}, + NodeSelector: c.Spec.NodeSelector, + Tolerations: c.Spec.Tolerations, + Affinity: c.Spec.Affinity, }, } } +// initEnv is the environment the embedded init.sh reads. Passing paths and +// tunables as env vars (rather than interpolating them into the script text) +// keeps init.sh static, committed and shellcheck-clean. +func initEnv() []corev1.EnvVar { + return []corev1.EnvVar{ + {Name: kea.EnvPodName, ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{FieldPath: "metadata.name"}, + }}, + {Name: kea.EnvRunDir, Value: kea.RunDir}, + {Name: kea.EnvConfigDir, Value: kea.ConfigDir}, + {Name: kea.EnvThisServerPlaceholder, Value: kea.ThisServerPlaceholder}, + {Name: kea.EnvDHCP4Bin, Value: kea.DHCP4Bin}, + {Name: kea.EnvDHCP4Conf, Value: kea.DHCP4ConfPath}, + {Name: kea.EnvCtrlAgentConf, Value: kea.CtrlAgentConfPath}, + {Name: kea.EnvWaitAttempts, Value: strconv.Itoa(kea.WaitAttempts)}, + {Name: kea.EnvWaitSleep, Value: strconv.Itoa(kea.WaitSleepSeconds)}, + } +} + // reloadReadyPods best-effort hot-reloads config on ready pods via the // ctrl-agent REST channel, analogous to bind-operator's rndc reconfig. func (r *KeaClusterReconciler) reloadReadyPods(ctx context.Context, c *v1alpha1.KeaCluster) { diff --git a/internal/controller/keacluster_test.go b/internal/controller/keacluster_test.go index df91d54..0681cd3 100644 --- a/internal/controller/keacluster_test.go +++ b/internal/controller/keacluster_test.go @@ -91,6 +91,95 @@ func TestKeaClusterReconcileCreatesWorkload(t *testing.T) { } } +// TestReconcileWiresInitContainer asserts the entrypoint refactor's pod shape: +// a kea-init initContainer prepares config + waits for HA peer DNS, and the two +// main containers exec kea directly with no wrapper shell. The init script comes +// from the ConfigMap (so its hash rolls the STS), and the old per-container +// entrypoint scripts are gone. +func TestReconcileWiresInitContainer(t *testing.T) { + scheme := testScheme(t) + cluster := newClusterFixture() + cl := fake.NewClientBuilder(). + WithScheme(scheme). + WithStatusSubresource(&v1alpha1.KeaCluster{}). + WithObjects(cluster). + Build() + r := &KeaClusterReconciler{Client: cl, Scheme: scheme} + if _, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Namespace: "dhcp-system", Name: "pxe"}}); err != nil { + t.Fatalf("reconcile: %v", err) + } + + // ConfigMap carries init.sh and no longer the per-container entrypoints. + var cm corev1.ConfigMap + if err := cl.Get(context.Background(), types.NamespacedName{Namespace: "dhcp-system", Name: "pxe-config"}, &cm); err != nil { + t.Fatalf("configmap not created: %v", err) + } + if cm.Data["init.sh"] == "" { + t.Errorf("configmap missing init.sh") + } + for _, gone := range []string{"entrypoint-dhcp4.sh", "entrypoint-ctrlagent.sh"} { + if _, ok := cm.Data[gone]; ok { + t.Errorf("configmap must not still carry %q", gone) + } + } + + var sts appsv1.StatefulSet + if err := cl.Get(context.Background(), types.NamespacedName{Namespace: "dhcp-system", Name: "pxe"}, &sts); err != nil { + t.Fatalf("statefulset not created: %v", err) + } + spec := sts.Spec.Template.Spec + + // Exactly one initContainer, named kea-init, running init.sh with POD_NAME + // sourced from the downward API. + if len(spec.InitContainers) != 1 || spec.InitContainers[0].Name != "kea-init" { + t.Fatalf("expected one kea-init initContainer, got %#v", spec.InitContainers) + } + initC := spec.InitContainers[0] + if got := initC.Command; len(got) != 2 || got[0] != "/bin/sh" || got[1] != "/etc/kea-operator/init.sh" { + t.Errorf("init command = %v, want [/bin/sh /etc/kea-operator/init.sh]", got) + } + var podNameFromDownward bool + for _, e := range initC.Env { + if e.Name == "POD_NAME" && e.ValueFrom != nil && e.ValueFrom.FieldRef != nil && e.ValueFrom.FieldRef.FieldPath == "metadata.name" { + podNameFromDownward = true + } + } + if !podNameFromDownward { + t.Errorf("init container must source POD_NAME from downward API metadata.name, env=%#v", initC.Env) + } + + // Main containers exec kea directly (no /bin/sh wrapper). + wantCmd := map[string][]string{ + "kea-dhcp4": {"/usr/sbin/kea-dhcp4", "-c", "/var/run/kea/kea-dhcp4.conf"}, + "kea-ctrl-agent": {"/usr/sbin/kea-ctrl-agent", "-c", "/var/run/kea/kea-ctrl-agent.conf"}, + } + for _, ctr := range spec.Containers { + want, ok := wantCmd[ctr.Name] + if !ok { + t.Errorf("unexpected container %q", ctr.Name) + continue + } + if len(ctr.Command) == 0 || ctr.Command[0] == "/bin/sh" { + t.Errorf("container %q must exec kea directly, got %v", ctr.Name, ctr.Command) + } + if !equalStrings(ctr.Command, want) { + t.Errorf("container %q command = %v, want %v", ctr.Name, ctr.Command, want) + } + } +} + +func equalStrings(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + // TestConfigHashChangesWithSubnets guards the roll trigger: adding a subnet // must change the pod-template config hash (so the STS rolls). func TestConfigHashChangesWithSubnets(t *testing.T) { diff --git a/internal/kea/agent.go b/internal/kea/agent.go index e5b848c..b564e77 100644 --- a/internal/kea/agent.go +++ b/internal/kea/agent.go @@ -1,6 +1,6 @@ package kea -import "fmt" +import _ "embed" type ctrlAgentRoot struct { ControlAgent controlAgent `json:"Control-agent"` @@ -27,36 +27,15 @@ func RenderCtrlAgent() (string, error) { }}) } -// EntrypointDHCP4 is the kea-dhcp4 container entrypoint. It derives this pod's -// HA peer name from the StatefulSet ordinal, substitutes the placeholder in the -// projected config, and execs the server. -func EntrypointDHCP4() string { - return fmt.Sprintf(`#!/bin/sh -set -e -ORD="${HOSTNAME##*-}" -mkdir -p %[1]s -chmod 0750 %[1]s -sed "s/%[2]s/server${ORD}/g" %[3]s/kea-dhcp4.conf > %[4]s -# The HA hook resolves peer URL hostnames once at load; on a cold container -# start the StatefulSet peer DNS records may not resolve yet, and kea exits -# hard instead of retrying. Wait for the config to validate before starting. -i=0 -until %[5]s -t %[4]s >/dev/null 2>&1; do - i=$((i+1)) - if [ "$i" -ge 60 ]; then break; fi - sleep 2 -done -exec %[5]s -c %[4]s -`, RunDir, ThisServerPlaceholder, ConfigDir, DHCP4ConfPath, DHCP4Bin) -} +// initScript is the initContainer entrypoint. It is a committed, shellcheck-clean +// shell file (no fmt.Sprintf interpolation) parameterised entirely by the +// environment variables the operator sets on the initContainer. It prepares the +// shared run dir, finalises this pod's kea-dhcp4 config from the StatefulSet +// ordinal, and bounded-waits for the HA peer DNS to resolve before the main +// kea-dhcp4 / kea-ctrl-agent containers exec kea directly. +// +//go:embed scripts/init.sh +var initScript string -// EntrypointCtrlAgent is the kea-ctrl-agent container entrypoint. -func EntrypointCtrlAgent() string { - return fmt.Sprintf(`#!/bin/sh -set -e -mkdir -p %[1]s -chmod 0750 %[1]s -cp %[2]s/kea-ctrl-agent.conf %[3]s -exec %[4]s -c %[3]s -`, RunDir, ConfigDir, CtrlAgentConfPath, CtrlAgentBin) -} +// InitScript returns the initContainer entrypoint shell script. +func InitScript() string { return initScript } diff --git a/internal/kea/config_test.go b/internal/kea/config_test.go index 396296d..419360e 100644 --- a/internal/kea/config_test.go +++ b/internal/kea/config_test.go @@ -237,29 +237,44 @@ func TestRenderCtrlAgent(t *testing.T) { } } -func TestEntrypointsHardenSocketDir(t *testing.T) { +func TestInitScriptHardensSocketDir(t *testing.T) { // Kea 2.6+ rejects a socket dir "more relaxed than 750"; the emptyDir mount - // defaults to 0777, so the entrypoints must chmod it before exec'ing kea. - want := "chmod 0750 " + RunDir - if ep := EntrypointDHCP4(); !strings.Contains(ep, want) { - t.Errorf("dhcp4 entrypoint must %q, got:\n%s", want, ep) - } - if ep := EntrypointCtrlAgent(); !strings.Contains(ep, want) { - t.Errorf("ctrl-agent entrypoint must %q, got:\n%s", want, ep) + // defaults to 0777, so the initContainer must chmod it (via $RUN_DIR) before + // the main containers exec kea. + if s := InitScript(); !strings.Contains(s, `chmod 0750 "$RUN_DIR"`) { + t.Errorf("init script must chmod 0750 the run dir, got:\n%s", s) } } -func TestEntrypointWaitsForConfigToValidate(t *testing.T) { - // The dhcp4 entrypoint must gate startup on `kea-dhcp4 -t` so a cold-start - // HA peer DNS resolution failure retries instead of crash-looping. - ep := EntrypointDHCP4() - for _, want := range []string{"until " + DHCP4Bin + " -t " + DHCP4ConfPath, "exec " + DHCP4Bin + " -c " + DHCP4ConfPath} { - if !strings.Contains(ep, want) { - t.Errorf("dhcp4 entrypoint must contain %q, got:\n%s", want, ep) +func TestInitScriptWaitsForConfigToValidate(t *testing.T) { + // The init script must gate startup on `kea-dhcp4 -t` so a cold-start HA peer + // DNS resolution failure retries (bounded) instead of crash-looping, and must + // fail loud after the cap so the kubelet restarts the initContainer. + s := InitScript() + for _, want := range []string{ + `until "$DHCP4_BIN" -t "$DHCP4_CONF"`, + `"$i" -ge "$WAIT_ATTEMPTS"`, + "exit 1", + } { + if !strings.Contains(s, want) { + t.Errorf("init script must contain %q, got:\n%s", want, s) } } } +func TestInitScriptIsInterpolationFree(t *testing.T) { + // The whole point of the refactor: the script is a static embedded file, not + // a Go-interpolated string. It must carry no fmt verbs and must derive the + // HA peer name from the pod ordinal via $POD_NAME. + s := InitScript() + if strings.Contains(s, "%[") || strings.Contains(s, "%s") || strings.Contains(s, "%d") { + t.Errorf("init script must not contain fmt verbs:\n%s", s) + } + if !strings.Contains(s, `ord="${POD_NAME##*-}"`) { + t.Errorf("init script must derive the ordinal from $POD_NAME, got:\n%s", s) + } +} + func TestControlSocketPathAllowedByKea(t *testing.T) { if !strings.HasPrefix(CtrlSocketPath, "/var/run/kea/") { t.Errorf("CtrlSocketPath %q must live under /var/run/kea (kea 2.6+ restriction)", CtrlSocketPath) diff --git a/internal/kea/consts.go b/internal/kea/consts.go index cb48b9b..4ad67d0 100644 --- a/internal/kea/consts.go +++ b/internal/kea/consts.go @@ -7,6 +7,9 @@ const ( ContainerDHCP4 = "kea-dhcp4" // ContainerCtrlAgent is the kea-ctrl-agent container name. ContainerCtrlAgent = "kea-ctrl-agent" + // ContainerInit is the initContainer that finalises config and waits for + // HA peer DNS before the main containers start. + ContainerInit = "kea-init" // ConfigDir is where projected config is mounted read-only. ConfigDir = "/etc/kea-operator" @@ -22,8 +25,9 @@ const ( CtrlAgentConfPath = RunDir + "/kea-ctrl-agent.conf" // CtrlSocketPath is the unix control socket between ctrl-agent and dhcp4. CtrlSocketPath = RunDir + "/kea4-ctrl-socket" - // EntrypointPath is the generated container entrypoint. - EntrypointPath = ConfigDir + "/entrypoint.sh" + // InitScriptPath is where the initContainer entrypoint is projected from the + // ConfigMap. + InitScriptPath = ConfigDir + "/init.sh" // DHCP4Bin is the kea-dhcp4 server binary. DHCP4Bin = "/usr/sbin/kea-dhcp4" @@ -47,4 +51,35 @@ const ( // ClientArchOption is the DHCP option code carrying PXE client arch. ClientArchOption = 93 + + // WaitAttempts caps the initContainer's bounded wait for the HA peer DNS to + // resolve (i.e. for the rendered config to pass "kea-dhcp4 -t"). + WaitAttempts = 60 + // WaitSleepSeconds is the delay between bounded-wait attempts. + WaitSleepSeconds = 2 +) + +// Environment variable names the operator sets on the initContainer. The +// embedded init.sh reads only these; keeping the names here means the script +// stays free of any Go string interpolation. +const ( + // EnvPodName carries the pod name (downward API metadata.name); its ordinal + // suffix selects this pod's HA peer name. + EnvPodName = "POD_NAME" + // EnvRunDir is the shared run/socket dir path. + EnvRunDir = "RUN_DIR" + // EnvConfigDir is the read-only projected config dir path. + EnvConfigDir = "CONFIG_DIR" + // EnvThisServerPlaceholder is the token replaced with this pod's HA peer name. + EnvThisServerPlaceholder = "THIS_SERVER_PLACEHOLDER" + // EnvDHCP4Bin is the kea-dhcp4 binary path (used for the -t config check). + EnvDHCP4Bin = "DHCP4_BIN" + // EnvDHCP4Conf is the finalized kea-dhcp4 config path in the shared run dir. + EnvDHCP4Conf = "DHCP4_CONF" + // EnvCtrlAgentConf is the staged kea-ctrl-agent config path in the run dir. + EnvCtrlAgentConf = "CTRL_AGENT_CONF" + // EnvWaitAttempts is the bounded-wait attempt cap. + EnvWaitAttempts = "WAIT_ATTEMPTS" + // EnvWaitSleep is the per-attempt sleep in seconds. + EnvWaitSleep = "WAIT_SLEEP" ) diff --git a/internal/kea/scripts/init.sh b/internal/kea/scripts/init.sh new file mode 100644 index 0000000..803729e --- /dev/null +++ b/internal/kea/scripts/init.sh @@ -0,0 +1,41 @@ +#!/bin/sh +# kea-operator initContainer. +# +# Prepares the shared run dir, finalises this pod's kea-dhcp4 config (the HA +# this-server-name is derived from the StatefulSet ordinal, known only at pod +# start), and waits for the HA peer DNS to resolve before the main kea +# containers start. Every input arrives as an environment variable set by the +# operator; nothing is interpolated into this file, so it is shellcheck-clean +# and testable on its own. +set -eu + +# StatefulSet pod names are "-"; the ordinal is this HA peer's id. +ord="${POD_NAME##*-}" + +# The run dir is a shared emptyDir. Kea 2.6+ refuses a control socket in a +# world-accessible directory, so tighten it to 0750. +mkdir -p "$RUN_DIR" +chmod 0750 "$RUN_DIR" + +# Finalise the per-pod dhcp4 config: substitute this pod's HA peer name into the +# shared (pod-independent) config projected from the ConfigMap. +sed "s/${THIS_SERVER_PLACEHOLDER}/server${ord}/g" \ + "${CONFIG_DIR}/kea-dhcp4.conf" >"$DHCP4_CONF" + +# The ctrl-agent config is pod-independent; stage it in the shared run dir so +# the main container can exec kea directly with no wrapper. +cp "${CONFIG_DIR}/kea-ctrl-agent.conf" "$CTRL_AGENT_CONF" + +# The HA hook resolves peer URL hostnames once at load; on a cold start the +# StatefulSet peer DNS records may not resolve yet and kea exits hard instead of +# retrying. Wait (bounded) for the config to validate, then fail loud so the +# kubelet restarts this initContainer rather than starting a doomed server. +i=0 +until "$DHCP4_BIN" -t "$DHCP4_CONF" >/dev/null 2>&1; do + i=$((i + 1)) + if [ "$i" -ge "$WAIT_ATTEMPTS" ]; then + echo "kea-init: config failed to validate after ${WAIT_ATTEMPTS} attempts" >&2 + exit 1 + fi + sleep "$WAIT_SLEEP" +done