[release/13.5] Set a default fsGroup for Kubernetes persistent volumes - #19401
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19401Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19401" |
There was a problem hiding this comment.
Pull request overview
Note
Copilot could not run the full agentic suite for this review because it was automatically requested on a bot-authored pull request. Request a review from Copilot under Reviewers to retry with the full agentic suite. Improved support for bot-authored pull requests is coming soon.
Updates Kubernetes publishing behavior to apply a default pod securityContext (fsGroup + change policy) when binding to first-class persistent volumes, and adds/updates tests to validate defaults and customizations.
Changes:
- Apply default
fsGroup(2000) andfsGroupChangePolicy(OnRootMismatch) for workloads with first-class persistent volume bindings. - Add unit/snapshot coverage for overriding and removing the default pod security context.
- Expand AKS end-to-end coverage to verify fsGroup behavior and add an additional API write path.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Kubernetes.Tests/Snapshots/KubernetesPublisherTests.PublishAsync_WithFirstClassPersistentVolume_OnProject_BindsViaMountPathOverload#00.verified.yaml | Snapshot updated to include default pod securityContext for PV binding. |
| tests/Aspire.Hosting.Kubernetes.Tests/Snapshots/KubernetesPublisherTests.PublishAsync_WithFirstClassPersistentVolume_KubernetesCustomizationOverridesDefaultFsGroup.verified.yaml | New snapshot validating fsGroup override while retaining default change policy. |
| tests/Aspire.Hosting.Kubernetes.Tests/Snapshots/KubernetesPublisherTests.PublishAsync_WithFirstClassPersistentVolume_KubernetesCustomizationCanRemoveDefaultSecurityContext.verified.yaml | New snapshot validating securityContext removal via customization. |
| tests/Aspire.Hosting.Kubernetes.Tests/Snapshots/KubernetesPublisherTests.PublishAsync_WithFirstClassPersistentVolume_FallsThroughForUnboundVolumes#00.verified.yaml | Snapshot updated to include default pod securityContext. |
| tests/Aspire.Hosting.Kubernetes.Tests/Snapshots/KubernetesPublisherTests.PublishAsync_WithFirstClassPersistentVolume_BindsByName_PromotesToStatefulSet#00.verified.yaml | Snapshot updated to include default pod securityContext. |
| tests/Aspire.Hosting.Kubernetes.Tests/KubernetesPublisherTests.cs | New unit tests + assertions around generated securityContext behavior. |
| tests/Aspire.Deployment.EndToEnd.Tests/AksPersistentVolumeDeploymentTests.cs | E2E now verifies filesystem group on AKS and adds a new write action. |
| src/Aspire.Hosting.Kubernetes/KubernetesResource.cs | Implements default fsGroup/fsGroupChangePolicy when PV binding annotation is present. |
| src/Aspire.Hosting.Kubernetes/KubernetesPersistentVolumeExtensions.cs | Docs updated to describe default fsGroup behavior and customization mechanism. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| @@ -0,0 +1,40 @@ | |||
| --- | |||
| /// <c>mountPath</c> instead. The generated pod uses an Aspire-managed | ||
| /// <c>fsGroup</c> of <c>2000</c> with an <c>OnRootMismatch</c> change policy so | ||
| /// non-root containers can access supported volumes without matching the image's | ||
| /// primary group. Use |
| var securityContext = Workload.PodTemplate.Spec.SecurityContext ??= new(); | ||
| securityContext.FsGroup ??= DefaultPersistentVolumeFsGroup; | ||
| securityContext.FsGroupChangePolicy ??= DefaultPersistentVolumeFsGroupChangePolicy; |
| await auto.RunCommandAsync( | ||
| $"FS_GROUP=$(kubectl get statefulset apiservice-statefulset --namespace \"$NS\" -o jsonpath='{{.spec.template.spec.securityContext.fsGroup}}') && " + | ||
| $"test \"$FS_GROUP\" = \"{expectedFsGroup}\" && " + | ||
| // Verify the Linux identity and mount ownership reported as: | ||
| // id -u: 1654 | ||
| // id -G: 1654 2000 | ||
| // stat -c %g /srv/data: 2000 | ||
| // This proves the write succeeds through group access rather than root privileges. | ||
| "PROCESS_UID=$(kubectl exec pod/apiservice-statefulset-0 --namespace \"$NS\" -- id -u) && " + | ||
| "PROCESS_GROUPS=$(kubectl exec pod/apiservice-statefulset-0 --namespace \"$NS\" -- id -G) && " + | ||
| "VOLUME_GROUP=$(kubectl exec pod/apiservice-statefulset-0 --namespace \"$NS\" -- stat -c %g /srv/data) && " + | ||
| "test \"$PROCESS_UID\" != \"0\" && " + | ||
| $"printf ' %s ' \"$PROCESS_GROUPS\" | grep --fixed-strings --quiet ' {expectedFsGroup} ' && " + | ||
| $"test \"$VOLUME_GROUP\" = \"{expectedFsGroup}\" && " + | ||
| $"echo \"StatefulSet uses fsGroup {expectedFsGroup}; pod UID is $PROCESS_UID with groups $PROCESS_GROUPS; /srv/data group is $VOLUME_GROUP\"", | ||
| counter); |
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: efbea589-4f81-4baf-9e17-4f1af07af5ee
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: efbea589-4f81-4baf-9e17-4f1af07af5ee
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: efbea589-4f81-4baf-9e17-4f1af07af5ee
e0f5a16 to
03ebfce
Compare
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. 4 / 100 test projects · 3 jobs, from 9 changed files. Selected test projects (4 / 100)
Selected jobs (3)
How these were chosen — grouped by what changed🔧 🔧 🧪 🧪 🧪 🧪 🧪 🧪 🧪 Job reasons
Selection computed for commit |
Jose Perez Rodriguez (joperezr)
left a comment
Jose Perez Rodriguez (joperezr)
left a comment
There was a problem hiding this comment.
Important change for the K8s story. Small scope and low risk. Approved.
ca6cbec
into
release/13.5
|
✅ No documentation update needed. Step 5 branch taken: "excluded → backport" Exclusion reasons (from Evidence:
This PR is a backport of already-merged PR #19374. Per workflow policy, backports are out of scope for docs generation since documentation should be authored against the original (forward) PR on the default branch, not duplicated for each backport. No triggered signals were evaluated since |
Backport of #19374 to release/13.5
/cc Adam Ratzman (@adamint) Mitch Denny (@mitchdenny)
Customer Impact
Customers using the new first-class Kubernetes persistent-volume support in Aspire 13.5 can get a healthy pod and bound PVC, but non-root workloads fail with permission denied on their first write. Without this fix, users must manually configure
fsGroupor use a root init-container workaround.Testing
Publisher tests verify the default
fsGroup: 2000andOnRootMismatchpolicy, override to3000, opt-out, and read-only mounts. A live AKS E2E passed, proving non-root UID1654can write, the same PVC survives redeployment, persisted data remains readable, and the replacement pod can write after changing to group3000. Backport CI is running with no failures.Risk
Low. The behavior is limited to the new experimental first-class
WithPersistentVolume(...)path; ordinary workloads and legacy PVC generation are unchanged. Users can override or remove the generated security context.Regression?
No. First-class Kubernetes persistent volumes are new in Aspire 13.5, so this completes their usable default behavior rather than fixing functionality that worked in an earlier release.