Support multiple volume items in the same directory using subPath#301
Support multiple volume items in the same directory using subPath#301guicassolato wants to merge 2 commits into
Conversation
… avoid clashing with the default tls cert paths Signed-off-by: Guilherme Cassolato <guicassolato@gmail.com>
📝 WalkthroughWalkthroughAuthorinoDeployment's volume mount logic now creates per-item VolumeMounts when a Volume's Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/reconcilers/deployment.go`:
- Around line 109-117: The current logic treats a MountPath that already ends
with a single item's filename as a base and appends additional item paths, which
produces invalid file/dir mixes when volume.Items has multiple entries; change
the logic around volume.MountPath handling so the "file-style" shortcut is only
applied when len(volume.Items) == 1 (i.e., preserve a file MountPath only for
single-item volumes). For cases where len(volume.Items) > 1, either
enforce/normalize MountPath to be a directory (ensure it ends with "/") before
appending each item or return an explicit validation error rejecting a
file-style MountPath for multi-item volumes; update the code around the
mountPath calculation (references: volume.MountPath, volume.Items, mountPath,
path) accordingly and add a unit test that covers the multi-item +
file-MountPath regression.
- Around line 119-123: The current VolumeMount creation sets SubPath
(volumeMount with fields Name, MountPath, SubPath) which prevents live updates
for Secret/ConfigMap volumes; either remove SubPath to mount the whole volume or
implement an explicit rollout on secret/configmap updates: prefer removing
SubPath in the volumeMount construction when the backing Volume is a Secret or
ConfigMap, and if partial-path mounting is required, compute a checksum of the
referenced Secret/ConfigMap and add it as an annotation on the
Deployment/PodTemplate (so the reconciler updates podTemplate.annotations and
triggers a rollout) — update the volumeMount creation site (volumeMount) and the
deployment reconcile logic that builds the pod template to apply one of these
fixes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1a6fffa6-4efd-4065-ba99-a11e7d797251
📒 Files selected for processing (2)
pkg/reconcilers/deployment.gopkg/reconcilers/deployment_test.go
| volumeMount := k8score.VolumeMount{ | ||
| Name: volume.Name, | ||
| MountPath: mountPath, | ||
| SubPath: path, | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Kubernetes, do subPath volume mounts backed by Secret, ConfigMap, or projected volumes receive live updates when the source object changes, or is a pod restart required?
💡 Result:
In Kubernetes, subPath volume mounts backed by Secret, ConfigMap, or projected volumes do not receive live updates when the source object changes. A pod restart is required to pick up the changes.
Citations:
- 1: https://kubernetes.io/docs/concepts/storage/projected-volumes/
- 2: https://kubernetes.io/docs/concepts/storage/volumes/
- 3: https://kubernetes.io/docs/concepts/configuration/configmap
- 4: https://kubernetes.io/docs/concepts/storage/projected-volumes
SubPath mounts prevent automatic Secret/ConfigMap updates.
Using subPath with Secret or ConfigMap volumes disables live refresh behaviour. When the source Secret or ConfigMap is updated, pods with subPath mounts will not receive the changes until restarted. Either keep whole-volume mounts where feasible, or implement an explicit pod rollout strategy on source updates.
Code location
Lines 119–123 in pkg/reconcilers/deployment.go
volumeMount := k8score.VolumeMount{
Name: volume.Name,
MountPath: mountPath,
SubPath: path,
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/reconcilers/deployment.go` around lines 119 - 123, The current
VolumeMount creation sets SubPath (volumeMount with fields Name, MountPath,
SubPath) which prevents live updates for Secret/ConfigMap volumes; either remove
SubPath to mount the whole volume or implement an explicit rollout on
secret/configmap updates: prefer removing SubPath in the volumeMount
construction when the backing Volume is a Secret or ConfigMap, and if
partial-path mounting is required, compute a checksum of the referenced
Secret/ConfigMap and add it as an annotation on the Deployment/PodTemplate (so
the reconciler updates podTemplate.annotations and triggers a rollout) — update
the volumeMount creation site (volumeMount) and the deployment reconcile logic
that builds the pod template to apply one of these fixes.
Signed-off-by: Guilherme Cassolato <guicassolato@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/reconcilers/deployment.go`:
- Around line 104-107: Guard against an empty mount path by checking if both
item.Path and item.Key are empty before using them: after computing path :=
item.Path; if path == "" { path = item.Key } add a conditional that if path ==
"" then log a warning (including item identifier) and continue/skip this item so
you never set VolumeMount.SubPath or compute mountPath with an empty string;
update any code that builds mountPath to assume path is non-empty (or trim
trailing slashes) to avoid producing a mountPath that ends with "/".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2da0879f-5814-436e-ad83-54dad19b4fbb
📒 Files selected for processing (2)
pkg/reconcilers/deployment.gopkg/reconcilers/deployment_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/reconcilers/deployment_test.go
| path := item.Path | ||
| if path == "" { | ||
| path = item.Key | ||
| } |
There was a problem hiding this comment.
Guard against empty path when both item.Path and item.Key are unset.
If both fields are empty strings, path remains empty, resulting in an empty SubPath (which mounts the entire volume instead of a specific file) and a malformed mountPath ending with just /. Consider skipping items with no resolvable path or logging a warning.
🛡️ Proposed defensive guard
for _, item := range volume.Items {
path := item.Path
if path == "" {
path = item.Key
}
+ if path == "" {
+ // Skip items with no resolvable path to avoid malformed mounts
+ continue
+ }
mountPath := volume.MountPath📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| path := item.Path | |
| if path == "" { | |
| path = item.Key | |
| } | |
| path := item.Path | |
| if path == "" { | |
| path = item.Key | |
| } | |
| if path == "" { | |
| // Skip items with no resolvable path to avoid malformed mounts | |
| continue | |
| } | |
| mountPath := volume.MountPath |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/reconcilers/deployment.go` around lines 104 - 107, Guard against an empty
mount path by checking if both item.Path and item.Key are empty before using
them: after computing path := item.Path; if path == "" { path = item.Key } add a
conditional that if path == "" then log a warning (including item identifier)
and continue/skip this item so you never set VolumeMount.SubPath or compute
mountPath with an empty string; update any code that builds mountPath to assume
path is non-empty (or trim trailing slashes) to avoid producing a mountPath that
ends with "/".
1 participant
Support multiple volume items in the same directory using subPath
Summary
This PR enhances the volume mounting functionality in the Authorino Operator to support mounting multiple files from different volumes into the same directory without conflicts. This is achieved by automatically using Kubernetes
subPathvolume mounts when the user specifiesitemsin the volume configuration.Key improvements:
/etc/ssl/certs/alongside Authorino's default TLS server certificates without conflictsmountPathcan be specified as a directory, and individual files are automatically mounted with appropriate subpathsUse cases enabled:
/etc/ssl/certs/for custom trust storesImplementation Details
The implementation modifies
pkg/reconcilers/deployment.goto intelligently handle themountPathfield:itemsare NOT specified: The entire volume is mounted atmountPath(existing behavior, unchanged)itemsARE specified:mountPathalready ends with/<item-path>→ use as-is (backward compatible)mountPathis a directory → automatically append the item pathThis allows users to write intuitive volume specifications like:
Which results in the file being mounted at
/etc/ssl/certs/custom-ca.crtwithsubPath: custom-ca.crt.Tests Implemented
Comprehensive unit tests were added in
pkg/reconcilers/deployment_test.gocovering:TestAuthorinoDeployment_VolumeMounts (9 test cases)
TestAuthorinoDeployment_VolumeProjections (4 test cases)
TestAuthorinoDeployment_DefaultMode (1 test case)
All tests validate the exact volumeMount structure (name, mountPath, subPath, readOnly) and projected volume source configurations.
Related Issues
This is a follow-up to #282, which was closed after a workaround was found. However, the core issue of mounting multiple files in the same directory remained unresolved. This PR addresses that limitation properly by leveraging Kubernetes subPath mounts.
Thanks to @jhernand for the collaboration on the original issue!
How to test this PR
To test this branch in a local Kubernetes Kind cluster:
Create a Kind cluster:
Build and load the operator image:
Deploy or update the operator:
Create test certificates with cert-manager:
Deploy an Authorino instance with multiple volumes:
Verify the volume mounts:
Co-authored by: 🤖 Claude Code
Summary by CodeRabbit
Bug Fixes
Tests