Skip to content

fix(operator): mount configMap keys as subPaths to preserve image defaults#267

Merged
lockwobr merged 1 commit into
mainfrom
worktree-configmap-clobber-bug
Jun 8, 2026
Merged

fix(operator): mount configMap keys as subPaths to preserve image defaults#267
lockwobr merged 1 commit into
mainfrom
worktree-configmap-clobber-bug

Conversation

@lockwobr

@lockwobr lockwobr commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes #208.

A package's configMap was mounted as a single directory volume at /skyhook-package/configmaps. Per standard Kubernetes ConfigMap-as-volume behavior, a directory mount replaces the entire directory, hiding any files the package image baked in at that path. This forced an all-or-nothing choice on package authors: ship a complete config via the SCR, or accept whatever the image provides; you could not ship sensible defaults in the image and let users override individual files.

This change mounts each ConfigMap key as its own subPath (MountPath: /skyhook-package/configmaps/<key>, SubPath: <key>, ReadOnly: true). The underlying volume is still mounted once; only the specific keys the user supplies overlay on top of the image content, so image-side files for un-supplied keys remain visible. Keys are iterated in sorted order so the generated pod spec is deterministic.

Behavior change

Scenario Before After
Image ships file A; user supplies {B} only B (whole dir replaced) A (image) + B (configMap)
User supplies {A, B} A, B from configMap same
No image defaults; user supplies {A} only A only A (unchanged)
ConfigMap edited mid-pod-run propagates (dir mount) does not propagate (subPath is static)

The last row is the only regression, and it was never actually exercised: package pods are short-lived and recreated by the operator on version bump / generation change, so they never observed live ConfigMap edits.

Tests

  • New envtest Ginkgo spec in operator/internal/controller/skyhook_controller_test.go verifying per-key subPath mounts, sorted order, read-only, no bare directory mount, and a single backing volume. Written test-first (RED → GREEN).
  • Full controller suite: 167/167 pass. go vet and golangci-lint clean.
  • Updated the explicit-uninstall chainsaw e2e, which asserted the old directory mountPath, to assert the new per-key subPath mounts. (Chainsaw run pending a cluster; static assertions match the verified pod spec.)

Docs

  • README package section documents the new overlay behavior at /skyhook-package/configmaps.

Note: no CHANGELOG.md edit — the operator changelog is generated by git-cliff from the conventional-commit message.

…aults

A package's configMap was mounted as a single directory volume at
/skyhook-package/configmaps, which (per standard Kubernetes
ConfigMap-as-volume behavior) replaced the entire directory and hid any
files the package image baked in at that path. Package authors could not
ship default files and let users override individual keys.

Mount each configMap key as its own subPath instead, so user-supplied
files overlay on top of image content rather than replacing the whole
directory; image-side files for keys the user did not supply remain
visible. Keys are iterated in sorted order for a deterministic pod spec.

subPath mounts do not receive live configMap updates, but package pods
are recreated per stage / version bump and never observed live edits in
practice.

Closes #208

Signed-off-by: Brian Lockwood <lockwobr@gmail.com>
@github-actions github-actions Bot added doc Documentation change (PR path label; doc issues use the Documentation type) component/operator Skyhook operator (controller-manager) component/ci CI workflows, GitHub Actions, and repo tooling component/tests End-to-end / chainsaw test suites (k8s-tests) labels Jun 5, 2026
@lockwobr lockwobr self-assigned this Jun 5, 2026
@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Lost in the diff? Review this PR in Change Stack to follow the change map from intent to exact ranges.

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: dfb98b54-d473-44fd-90cb-a025187acdc2

📥 Commits

Reviewing files that changed from the base of the PR and between 4c3c3be and 7c15fe4.

📒 Files selected for processing (4)
  • README.md
  • k8s-tests/chainsaw/skyhook/explicit-uninstall/chainsaw-test.yaml
  • operator/internal/controller/skyhook_controller.go
  • operator/internal/controller/skyhook_controller_test.go

📝 Walkthrough

Walkthrough

This PR refactors ConfigMap volume mounting in package pod generation. Previously, mounting a ConfigMap as a directory volume at /skyhook-package/configmaps hid any files the package image baked in at that path. The change mounts each ConfigMap key individually via subPath at /skyhook-package/configmaps/<key>, allowing user-supplied keys to overlay image defaults rather than replacing the entire directory. The implementation sorts keys for deterministic pod specs, adds a unit test validating per-key mounting behavior, updates the integration test expectations, and documents the new behavior in the README.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: mounting ConfigMap keys as subPaths instead of a single directory, enabling image defaults to be preserved.
Description check ✅ Passed The description comprehensively explains the fix, behavior changes, test coverage, and documentation updates related to the ConfigMap mounting strategy.
Linked Issues check ✅ Passed All acceptance criteria from #208 are met: per-key subPath mounts implemented [208], deterministic ordering via sorted keys [208], image defaults preserved [208], unit and e2e tests added [208], and documentation updated [208].
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing #208: controller code for per-key subPath mounting, unit tests validating the new behavior, e2e test updates, and README documentation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-configmap-clobber-bug

Comment @coderabbitai help to get the list of available commands and usage tips.

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 27040174901

Warning

No base build found for commit 4c3c3be on main.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 81.63%

Details

  • Patch coverage: 23 of 23 lines across 1 file are fully covered (100%).

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →


Coverage Stats

Coverage Status
Relevant Lines: 9880
Covered Lines: 8065
Line Coverage: 81.63%
Coverage Strength: 9.31 hits per line

💛 - Coveralls

@lockwobr lockwobr merged commit 571f9e9 into main Jun 8, 2026
34 of 37 checks passed
@lockwobr lockwobr deleted the worktree-configmap-clobber-bug branch June 8, 2026 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/ci CI workflows, GitHub Actions, and repo tooling component/operator Skyhook operator (controller-manager) component/tests End-to-end / chainsaw test suites (k8s-tests) doc Documentation change (PR path label; doc issues use the Documentation type)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stop ConfigMap volume from clobbering files baked into the package image at /skyhook-package/configmaps

3 participants