Added WithConfiguration extension method for configuring PVCs

[Christopher Brown (cdbrown2018)]

Summary

This PR adds support for customizing the Kubernetes PersistentVolumeClaim generated for first-class persistent volumes in Aspire. It introduces a new WithConfiguration(...) API so callers can mutate the generated claim during manifest generation, enabling customization of metadata and spec fields that aren’t covered by the existing volume configuration methods.

withPersistentVolumeName(...) is available to Polyglot AppHosts, while a more robust WithConfiguration(...) is available on C# AppHosts.

Fixes: #19633

What changed

• Added KubernetesPersistentVolumeCustomizationAnnotation to carry a PVC configuration callback.
• Added KubernetesPersistentVolumeExtensions.WithConfiguration(...) for configuring the generated PersistentVolumeClaim.
• Added polyglot WithPersistentVolumeName with associated plumbing for setting the name of an existing volume on cluster.
• Added polyglot WithoutStorageClass, used to explicitly exclude storageClasses from PVC specs.
• Applied the customization callback when building the PVC in KubernetesEnvironmentResource.
• Added an explicit PreserveEmptyStringAttribute that short circuits the YamlIEnumerableSkipEmptyObjectGraphVisitor which was causing empty strings to be omitted from YAML serialization.
• Added tests covering:
  • TypeScript capability generation for withPersistentVolumeName, withoutStorageClass
  • End-to-end Kubernetes publishing with PVC customization applied to the generated manifest

Why

Users need a way to further customize generated PVCs without forking manifests or relying on post-processing. This makes the first-class persistent volume API more flexible while keeping the existing experience intact.

Example

C# Only implementation:

var data = k8s.AddPersistentVolume("data")
    .WithConfiguration(claim =>
    {
        claim.Metadata.Labels["example.com/retention"] = "retain";
        claim.Spec.VolumeMode = "Block";
        claim.Spec.VolumeName = "test-volume";                         // Fixes #19633
    });

C# and Polyglot compatible: All contribute to fixing #19633.

// Explicitly exclude storageClassName from the manifest. Useful for claiming PVs that use the cluster's default storage class.
var data = k8s.AddPersistentVolume("data").WithoutStorageClass();

// Set the storageClassName to "", telling Kubernetes to bind to a classless PV.
var data = k8s.AddPersistentVolume("data").WithStorageClass("");

// Claim an existing volume named "postgres-storage" that has a storage class "efs-block".
// Note: For a PVC to successfully bind to an existing PV, they must have matching storageClassNames.
var data = k8s.AddPersistentVolume("data")
    .WithStorageClass("efs-block")
    .WithPersistentVolumeName("postgres-storage");

Notes

• This is an opt-in API and should be backward-compatible.
• Changing claim.Metadata.Name will alter the generated PVC identity, so workload references should be updated accordingly.
• WithConfiguration is NOT polyglot compatible. None of the Kubernetes models are currently exposed using [AspireExport]. The typescript docs currently inaccurately depict an exposed workload property on KubernetesResource. Exposing all of the Kubernetes manifests over [AspireExport] is outside the scope of this ticket. Exposing only a subset (those applicable to PersistentVolumeClaim) would both necessarily expose ObjectMetaV1, touching numerous areas in the codebase; and it would create an inconsistent API. Therefore, WithConfiguration was kept C# only.

Checklist

• Is this feature complete?
  • Yes. Ready to ship.
  • No. Follow-up changes expected.
• Are you including unit tests for the changes and scenario tests if relevant?
  • Yes
  • No
• Did you add public API?
  • Yes
    • If yes, did you have an API Review for it?
      • Yes
      • No
    • Did you add <remarks /> and <code /> elements on your triple slash comments?
      • Yes
      • No
  • No
• Does the change make any security assumptions or guarantees?
  • Yes
    • If yes, have you done a threat model and had a security review?
      • Yes
      • No
  • No

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19900

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19900"

[Copilot]

🟡 Changes recommended

Generated SDK callbacks cannot access PVC metadata or specification because their nested types are not ATS-exported.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds PVC customization during Kubernetes manifest generation.

Changes:

• Adds WithConfiguration and its annotation.
• Applies callbacks when generating PVCs.
• Adds Kubernetes publishing and TypeScript generation tests.

File summaries

File	Description
KubernetesPublisherTests.cs	Tests customized PVC output.
AtsTypeScriptCodeGeneratorTests.cs	Tests generated callback signature.
PersistentVolumeClaimV1.cs	Exposes PVC through ATS.
KubernetesPersistentVolumeExtensions.cs	Adds the customization API.
KubernetesEnvironmentResource.cs	Applies PVC callbacks.
KubernetesPersistentVolumeCustomizationAnnotation.cs	Stores customization callbacks.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

TypeScript cannot access the nested PVC properties, and the targeted VolumeName regression remains untested.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

tests/Aspire.Hosting.Kubernetes.Tests/KubernetesPublisherTests.cs:1757

• The regression test never exercises PersistentVolumeClaimSpecV1.VolumeName, even though binding a named pre-existing PV is the behavior required by #19633. It would still pass if VolumeName were omitted from the model or YAML serialization. Set claim.Spec.VolumeName and assert both the generated claim and volumeName YAML output.

src/Aspire.Hosting.Kubernetes/Resources/PersistentVolumeClaimV1.cs:18

• ExposeProperties = true does not make Metadata or Spec usable in generated SDKs because their custom types (ObjectMetaV1 and PersistentVolumeClaimSpecV1) are not ATS-exported. AtsCapabilityScanner consequently filters those property capabilities as unresolved types, so TypeScript receives a PersistentVolumeClaim handle but cannot perform the documented label or VolumeName mutations. Export a selective nested editor/model surface (or a purpose-built PVC editor) and assert the generated metadata/spec members.

[AspireExport(ExposeProperties = true)]

• Files reviewed: 6/6 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Public API consistency and the stated TypeScript contract need resolution before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Classless existing volumes remain unsupported for polyglot callers, and the tests validate an undeployable block-volume configuration without deployment coverage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

tests/Aspire.Hosting.Kubernetes.Tests/KubernetesPublisherTests.cs:1737

• This regression coverage only verifies serialized YAML, but the changed behavior is Kubernetes binding to an existing PV. Extend AksPersistentVolumeDeploymentTests (or add equivalent deployment coverage) to create a PV, call WithPersistentVolumeName, deploy, and verify the pod binds and can use it. That would catch admission and storage-class compatibility failures that this snapshot cannot detect.
  tests/Aspire.Hosting.Kubernetes.Tests/KubernetesPublisherTests.cs:1740
• This test combines volumeMode: Block with WithPersistentVolume(..., mountPath), which emits a filesystem volumeMount. Kubernetes raw-block PVCs must instead be attached through volumeDevices, so the accepted snapshot represents a workload that cannot deploy. Use Filesystem for this customization test (and update the snapshot), unless block-device plumbing is added and tested.

• Files reviewed: 7/7 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Omitting storageClassName does not disable the cluster default and can prevent binding to an existing classless volume.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 5
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The new storage-class annotations are never resolved, causing explicit storage classes to be ignored.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

src/Aspire.Hosting.Kubernetes/Annotations/KubernetesPersistentVolumeStorageClassAnnotation.cs:23

• This condition only runs when omitStorageClassName is true, but the exception reports that it was false, which would send users debugging the wrong input if the invariant is triggered.

            throw new ArgumentException($"A Persistent Volume Claim was configured with {nameof(omitStorageClassName)} = false, but a {nameof(storageClassName)} was provided.");

src/Aspire.Hosting.Kubernetes/Annotations/KubernetesPersistentVolumeStorageClassAnnotation.cs:9

• Correct the spelling of “ommission” to “omission.”

/// Represents an annotation that enables the explicit ommission of storageClassName from a PVC.

• Files reviewed: 13/13 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Storage-class ordering and empty-value inconsistencies can generate incorrect PVC manifests, while named-PV binding lacks deployment regression coverage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/Aspire.Hosting.Kubernetes/KubernetesPersistentVolumeExtensions.cs:183

• The new publish test verifies only that spec.volumeName is serialized. Existing KinD deployment coverage dynamically provisions its PVs, so nothing verifies the issue's actual contract: a pre-created named PV binds to this claim and is mounted by the workload. Extend the Kubernetes deployment E2E scenario to create a compatible PV first, deploy with WithPersistentVolumeName, and assert the PVC's bound volume name.

src/Aspire.Hosting.Kubernetes/KubernetesPersistentVolumeExtensions.cs:106

• The parameter overload has the same stale-state problem: after WithoutStorageClass(), setting a parameterized storage class leaves ShouldRequestStorageClassName false, so ResolveStorageClass discards the parameter. Set the flag back to true before assigning the expression.

        builder.Resource.StorageClassName = ReferenceExpression.Create($"{storageClassName.Resource}");

• Files reviewed: 12/12 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Static PV binding lacks deployment validation, and public documentation omits the critical null-versus-empty storage-class distinction.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 12/12 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Parameterized whitespace handling regresses, and static binding lacks deployment-level verification.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/Aspire.Hosting.Kubernetes/KubernetesPersistentVolumeExtensions.cs:190

• The named-volume path is only exercised through generated YAML and capability checks. Existing KinD deployment tests use dynamic local-path provisioning and never create a pre-existing PV, so the #19633 scenario could remain Pending while all current tests pass. Extend a Kubernetes deploy E2E test to pre-create a PV, use this API with matching storage-class semantics, and assert that the PVC binds to that exact PV.

        builder.Resource.PersistentVolumeName = ReferenceExpression.Create($"{persistentVolumeName}");

• Files reviewed: 13/13 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The AKS test cannot complete static rebinding because it deletes an in-use PVC, retains the stale claim reference, and expects the recreated PVC to keep its old UID.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

tests/Aspire.Deployment.EndToEnd.Tests/AksPersistentVolumeDeploymentTests.cs:400

• Waiting for the retained PV to reach Released is not sufficient for reuse. Its spec.claimRef still contains the deleted claim's UID, so Kubernetes reserves it for that old claim and the new PVC remains pending even when spec.volumeName matches. Remove spec.claimRef after the PV is released so it becomes available for the second claim.

            "phase=''; for i in $(seq 1 60); do " +
            "phase=$(kubectl get persistentvolume \"$EXISTING_PV_NAME\" -o jsonpath='{.status.phase}' 2>/dev/null || true); " +
            "if [ \"$phase\" = \"Released\" ]; then break; fi; sleep 2; done; " +
            "test \"$phase\" = \"Released\" && " +

• Files reviewed: 13/13 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Generated polyglot documentation omits the key empty-string storage-class behavior.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/Aspire.Hosting.Kubernetes/KubernetesPersistentVolumeExtensions.cs:72

• <ats-summary> overrides the richer C# summary in generated SDK documentation, so polyglot users will not see the new distinction that an empty string requests a classless volume. Include the empty-string behavior in the ATS summary (or an <ats-param>), otherwise the advertised TypeScript path is not discoverable from its generated API docs.

• Files reviewed: 13/13 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The modified AKS test removes existing coverage that normal redeployment preserves the original PVC.

Review details

Suppressed comments (1)

tests/Aspire.Deployment.EndToEnd.Tests/AksPersistentVolumeDeploymentTests.cs:142

• This removes the only real-deployment coverage that an ordinary redeploy preserves the existing PVC: the test now deletes the StatefulSet and claim before redeploying, and the updated assertion requires a new PVC UID. Keep the original redeploy/reuse assertion and exercise static rebinding in a separate test or an additional deployment phase so regressions that replace a user's claim during a normal update remain detectable.

            await RetainPersistentVolumeForStaticBindingAsync(auto, counter);

• Files reviewed: 13/13 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Adam Buchanan (RejectKid)]

Chris, I put together a few fixes on top of your branch: PR here.

The main change keeps the original test that checks a normal redeploy preserves the PVC. It then does a third deployment to check that a new claim can bind to the retained disk, with the data and permissions still intact.

I also changed the example and snapshot to Filesystem, since the test uses a filesystem mount, and added tests for storage-class call order, empty and whitespace values, and multiple configuration callbacks. Blank PV names now get rejected up front.

CI passed on Windows and Linux: 350 Kubernetes tests and 129 TypeScript tests on each, with none skipped. The deployment test project builds too. The full managed build passed locally; I didn't test native AOT.

I haven't run the AKS test against a real cluster. After these changes are pulled into your branch and the PR build is available, someone with Microsoft access can run deployment-tests.yml with pr_number=19900 and test_filter=AksPersistentVolumeDeploymentTests. When I checked, the upstream CI runs were waiting for approval, and API review was still open.

One small thing for the PR description: the VolumeMode = "Block" example should also be Filesystem if it's meant to be used with a filesystem mount.