Add Radius recipe parameters and secret management#18834
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18834Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18834" |
There was a problem hiding this comment.
Pull request overview
Adds publish/deploy-only secret-provider and recipe-configuration capabilities to the Radius deployment-target integration.
Changes:
- Adds scoped recipe parameters with late-bound values.
- Adds inline, existing, and sealed secret stores.
- Adds validation, Bicep generation, deployment orchestration, documentation, and tests.
Reviewed changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
tests/.../SecretStoreValidationTests.cs |
Tests store validation diagnostics. |
tests/.../SecretStoreDefaultPathTests.cs |
Tests default and Run-mode behavior. |
tests/.../SecretStoreConsumerTests.cs |
Tests consumer annotations. |
tests/.../SealedSecretManifestTests.cs |
Tests manifest security validation. |
tests/.../SealedSecretArtifactTests.cs |
Tests artifact paths. |
tests/.../SealedSecretApplyStepTests.cs |
Tests apply and synchronization logic. |
tests/.../AddRadiusSecretStoreTests.cs |
Tests store APIs. |
tests/.../WithRecipeParametersTests.cs |
Tests parameter registration. |
tests/.../RecipeParameterIsolationAndFidelityTests.cs |
Tests environment isolation. |
tests/.../RecipeParameterAdvancedTests.cs |
Tests nested and late-bound parameters. |
tests/.../RecipePackParametersBicepTests.cs |
Tests recipe Bicep output. |
tests/.../SecretStoreConsumerBicepTests.cs |
Tests consumer Bicep output. |
tests/.../SealedSecretPublishTests.cs |
Tests sealed-secret publishing. |
tests/.../RadiusDeployParametersTests.cs |
Tests deploy parameter bindings. |
tests/.../InlineSecretStoreBicepTests.cs |
Tests inline-store output. |
tests/.../ExistingSecretStoreBicepTests.cs |
Tests existing-secret output. |
Secrets/SealedSecretManifest.cs |
Validates encrypted manifests. |
Secrets/SealedSecretArtifact.cs |
Resolves artifact paths. |
Secrets/RadiusSecretStoreValidation.cs |
Implements fail-fast validation. |
Secrets/RadiusSecretStoreType.cs |
Defines store types and encodings. |
Secrets/RadiusSecretStoreResource.cs |
Models secret stores. |
Secrets/RadiusSecretStorePopulation.cs |
Models population modes. |
Secrets/RadiusSecretStoreNaming.cs |
Validates store names. |
Secrets/RadiusSecretStoreExtensions.cs |
Adds store builder APIs. |
Secrets/RadiusSecretStoreConsumerExtensions.cs |
Adds consumer APIs. |
Secrets/RadiusSecretStoreConsumer.cs |
Models consumer wiring. |
Recipes/RadiusRecipeParameterExtensions.cs |
Adds recipe parameter APIs. |
Recipes/RadiusProviderReference.cs |
Defines provider references. |
README.md |
Documents new features. |
RadiusEnvironmentResource.cs |
Adds validation/apply pipeline stages. |
Publishing/RadiusInfrastructureOptions.cs |
Carries generated secret and parameter state. |
Publishing/RadiusBicepPublishingContext.cs |
Copies sealed manifests. |
Publishing/Constructs/RadiusSecretStoreConstruct.cs |
Models secret-store Bicep resources. |
Publishing/BicepPostProcessor.cs |
Emits and validates new symbols. |
CloudProviders/RadiusCloudProviderExtensions.cs |
Documents export suppression. |
Aspire.Hosting.Radius.csproj |
Adds YamlDotNet. |
Annotations/RadiusSecretStoresAnnotation.cs |
Stores consumer wiring. |
Annotations/RadiusRecipeParametersAnnotation.cs |
Stores scoped recipe parameters. |
| // Sealed-secrets apply/wait gate: applies each SealedSecret manifest to the workspace's | ||
| // cluster and waits for the underlying Secret to materialize before rad deploy. Scheduled | ||
| // after publish and RequiredBy deploy; a no-op when no sealed store is declared. | ||
| var applySealedSecretsStep = new SealedSecretApplyStep(this).CreatePipelineStep(); |
There was a problem hiding this comment.
Good call on end-to-end coverage. A SealedSecret apply/materialize deployment test requires a live cluster (kind + the Sealed Secrets controller) and belongs in tests/Aspire.Deployment.EndToEnd.Tests rather than the unit suite, so it's out of scope for this PR. The apply/sync/verify logic is unit-tested against an injected kubectl runner (including the new permanent-vs-transient handling). I'll track the live E2E separately. Leaving this thread open as a reminder.
| public static IResourceBuilder<RadiusEnvironmentResource> WithRecipeParameters( | ||
| this IResourceBuilder<RadiusEnvironmentResource> builder, | ||
| Action<IDictionary<string, object>> configure) |
There was a problem hiding this comment.
Agreed a deployed recipe-parameter round-trip is valuable, but it needs a live Radius environment and belongs in the deployment E2E suite, so it's out of scope here. Recipe-parameter emission is covered by the publish/bicep unit tests. I'll track the live E2E separately and am leaving this thread open.
Harden and tighten the Radius secret-store surface in response to Copilot review comments: - SealedSecretManifest: fail closed in EmbedsPlaintextSecret when the embedded last-applied-configuration has duplicate/missing/non-string kind or duplicate data/stringData, so a crafted payload cannot smuggle plaintext past a single TryGetProperty lookup. - WithSealedSecret: resolve a relative manifest path against the AppHost directory (matching other hosting file APIs) so it works regardless of the process working directory. - RadiusSecretStoreNaming: align the store-name grammar with Aspire's resource-name grammar (letters/digits/hyphens, start-with-letter, no consecutive/trailing hyphen, <=64) so publish mode (AddResource) never rejects a name run mode accepted. Update the ASPIRERADIUS049 message. - WithData: add a single-key convenience overload binding a data key to a secret parameter, delegating transactionally to the callback form. - SealedSecretApplyStep.GetSealedSecretStatusAsync: only treat SealedSecret NotFound or transient connectivity failures as retryable; surface permanent kubectl failures (auth/RBAC/context/missing plugin) immediately instead of waiting out the whole materialization budget. Adds 22 targeted tests (357 total, all passing). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b76dc20d-c553-47b7-8f5e-6fcdc0f10aa3
Harden and tighten the Radius secret-store surface in response to Copilot review comments: - SealedSecretManifest: fail closed in EmbedsPlaintextSecret when the embedded last-applied-configuration has duplicate/missing/non-string kind or duplicate data/stringData, so a crafted payload cannot smuggle plaintext past a single TryGetProperty lookup. - WithSealedSecret: resolve a relative manifest path against the AppHost directory (matching other hosting file APIs) so it works regardless of the process working directory. - RadiusSecretStoreNaming: align the store-name grammar with Aspire's resource-name grammar (letters/digits/hyphens, start-with-letter, no consecutive/trailing hyphen, <=64) so publish mode (AddResource) never rejects a name run mode accepted. Update the ASPIRERADIUS049 message. - WithData: add a single-key convenience overload binding a data key to a secret parameter, delegating transactionally to the callback form. - SealedSecretApplyStep.GetSealedSecretStatusAsync: only treat SealedSecret NotFound or transient connectivity failures as retryable; surface permanent kubectl failures (auth/RBAC/context/missing plugin) immediately instead of waiting out the whole materialization budget. Adds 22 targeted tests (357 total, all passing). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b76dc20d-c553-47b7-8f5e-6fcdc0f10aa3
6f5c0f3 to
0013878
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 40 out of 40 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/Aspire.Hosting.Radius/Secrets/RadiusSecretStoreNaming.cs:69
- Uppercase letters pass this check, but an inline store with no
resourcereference is materialized by Radius as a Kubernetes Secret whosemetadata.nameis the store name. Kubernetes object names reject uppercase, so names such asDbCredspass AppHost validation and then fail only during deployment. Enforce the target-compatible lowercase DNS grammar, or separate the Aspire resource name from the physical secret-store/Secret name.
else if (char.IsAsciiLetterOrDigit(c))
| public static IResourceBuilder<RadiusEnvironmentResource> WithSecretStore( | ||
| this IResourceBuilder<RadiusEnvironmentResource> radius, | ||
| [ResourceName] string name, | ||
| RadiusSecretStoreType type, | ||
| Action<IResourceBuilder<RadiusSecretStoreResource>> configure) |
There was a problem hiding this comment.
Good catch — this is a real limitation. Today an environment-scoped store uses name as both the Aspire model identity and the physical Radius store name, so two environments cannot each declare e.g. db-creds (the second AddResource is rejected).
I'm leaving this one open as a follow-up rather than fixing it in this PR: the clean fix is additive API surface — an optional physical store name threaded through AddRadiusSecretStore/WithSecretStore, a StoreName on the resource, and all emission sites (bicep name:, the UCP reference, and the sealed Secret name). That's a larger, riskier change than the other re-review fixes, and it only bites when two Radius environments each declare a same-named env-local store. Happy to implement it in a dedicated PR if you'd prefer — let me know.
…e inline key rejection, existing-secret reference validation Re-review of PR microsoft#18834 raised three additional findings on the Radius secret-store surface: - Terraform Git PAT auth now requires the referenced store to declare a 'pat' key (optionally 'username'), matching what Radius reads for recipeConfig.terraform.authentication.git.pat, instead of accepting a basicAuthentication username/password store it cannot use (ASPIRERADIUS051). - RadiusSecretStoreDataBuilder.Add rejects a duplicate inline key via TryAdd instead of silently overwriting through the dictionary indexer, so a double-declared key surfaces at the call site (ASPIRERADIUS043). - WithExistingSecret validates the '[namespace/]name' reference at the API boundary (single separator, DNS-1123 name/namespace) rather than deferring a malformed reference to deploy time (ASPIRERADIUS046). Adds targeted tests and updates the README diagnostics table and xmldoc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b76dc20d-c553-47b7-8f5e-6fcdc0f10aa3
…ialization timeout, validate sealed manifest names A second re-review round on PR microsoft#18834 raised three findings: - ConfigureRadiusInfrastructure exposes secret stores (opts.SecretStores), but the identifier snapshot/rewire logic ignored them. Renaming a store construct left recipeConfig referencing the old <identifier>.id, and renaming the legacy application/environment left the store's ApplicationId/EnvironmentId pointing at stale symbols. Extended IdentifierSnapshot and added RewireSecretStoreReferences to rewrite both consumer references and parent scope IDs. - WithMaterializationTimeout accepted values above int.MaxValue milliseconds (e.g. TimeSpan.MaxValue) that CancellationTokenSource.CancelAfter / the deploy deadline cannot represent, so they passed publish validation but threw at deploy. Reject them at configuration time. - SealedSecret manifest parsing accepted any nonblank metadata.name (e.g. Bad_Name, a/b, an overlong namespace), deferring the failure to kubectl apply. Validate the name as a DNS-1123 subdomain and any explicit namespace as a DNS-1123 label at publish time, mirroring the WithExistingSecret fast-fail. Extracted the DNS-1123 helpers into a shared KubernetesName class and added tests for all three findings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b76dc20d-c553-47b7-8f5e-6fcdc0f10aa3
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 42 out of 42 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (2)
src/Aspire.Hosting.Radius/Secrets/RadiusSecretStoreExtensions.cs:204
- Resolve the manifest path before changing the population mode.
Path.GetFullPathcan throw for malformed paths; after that exception the resource remains marked as sealed and a corrected retry is rejected by ASPIRERADIUS065.
population.HasSealedSecret = true;
// Resolve a relative manifest path against the AppHost directory (not the process working
// directory), matching other hosting file APIs (e.g. KeycloakResourceBuilderExtensions), so
// the documented `./secrets/...` usage works regardless of where the AppHost is launched.
population.SealedManifestPath = Path.GetFullPath(manifestPath, store.ApplicationBuilder.AppHostDirectory);
population.Keys.AddRange(validatedKeys);
src/Aspire.Hosting.Radius/Secrets/RadiusSecretStoreNaming.cs:57
- These rules allow uppercase letters and a 64-character name, but this value is also used as the name of a Radius-created Kubernetes Secret. Kubernetes object names must be lowercase DNS-1123 names, with each label limited to 63 characters, so accepted inputs such as
DB-Credsor 64as will fail only at deployment. Validate the physical store name against the Kubernetes naming contract (or map it to a separate valid physical name).
if (!char.IsAsciiLetter(name[0]) || name[^1] == '-')
{
return false;
}
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Second slice of the Aspire→Radius integration, layered on the merged compute-environment baseline (microsoft#18696). Recipe parameters (`WithRecipeParameters`): - Flow environment-wide and resource-type-scoped values onto the recipe pack and legacy environment recipe entries. - ParameterResource-backed values emit as valueless `@secure()` Bicep params bound at deploy time; provider references resolve at publish. - ASPIRERADIUS028 guards recipe-parameter identifier collisions. Secret management (gated behind experimental ASPIRERADIUS006): - `AddRadiusSecretStore` / `WithSecretStore` with inline, existing-secret, and sealed-secret population modes plus `recipeConfig` consumers. - Sealed secrets are applied via a dedicated pipeline step after publish and before deploy; secret values are never inlined into Bicep. - Validation surfaces actionable ASPIRERADIUS040–055 diagnostics. Recipe and secret parameters are unified with the existing container env-var parameter registry so a parameter shared across both emits a single Bicep `param` and one deploy binding; distinct parameters whose names collide to the same identifier fail loudly (ASPIRERADIUS056) rather than silently misbinding. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f19583f4-384b-4601-a94b-290d9b0d4ed5
Address four findings from the code review of the sealed-secret path: - Replace the regex/line-oriented SealedSecret manifest validator with a YamlDotNet parser. Reject duplicate keys, multi-document/non-mapping roots, anchors/aliases/merge keys, top-level data/stringData, and non-empty spec.template.data so a plaintext Kubernetes Secret can no longer be smuggled past validation into published artifacts or applied to a cluster. - Close the validate/copy TOCTOU: read and validate the manifest once, carry the validated bytes, and write those exact bytes into the artifact instead of re-reading the source file. - Gate the deploy-time materialization wait on the SealedSecret's status (observedGeneration match + Synced=True) before confirming the Secret, so a redeploy no longer proceeds with a stale Secret. Fast-fail on Synced=False or generation advance, enforce a per-probe remaining-time budget, and fail with ASPIRERADIUS058 on timeout. Raw kubectl -o json output is never logged. - Fail closed (ASPIRERADIUS059) when the active rad workspace's Kubernetes context cannot be resolved, instead of silently using kubectl's ambient context, with an ASPIRE_RADIUS_KUBE_CONTEXT override escape hatch. Update the README diagnostics table (broaden 044; add 058/059) and add tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ddaf7f6d-b633-4b7e-9578-ec5b40e3c8e9
…tic doc Address review feedback on the Radius secret-store API: - Rename FromExistingSecret/FromSealedSecret to WithExistingSecret/ WithSealedSecret so the configuration methods follow Aspire's With* convention (they configure an existing resource builder). Update all call sites, XML docs, validation messages, README examples, and tests. - Remove the retired ASPIRERADIUS046 entry from the README diagnostics table; its "sealed Secret never materialized before deploy" meaning is now covered by the status-gated ASPIRERADIUS058. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ddaf7f6d-b633-4b7e-9578-ec5b40e3c8e9
Fixes five design findings surfaced by architecture/hosting/api reviews on the Radius recipe & secret-store surface: 1. Gateway TLS (WithTlsCertificate) now fails loud instead of silently dropping: rejected at the validation gate (ASPIRERADIUS060), with a defensive throw at emission. 2. Remove the unusable WithTerraformProviderSecret API (and its enum member, ASPIRERADIUS053 validation, emission case, test, and README refs). 3. Restrict WithMaterializationTimeout to sealed stores: reject an explicit override on a non-sealed store (ASPIRERADIUS062). 4. Validate declared keys exist in the materialized sealed Secret at deploy (ASPIRERADIUS061); Secret values are never logged, only key names read. 5. ATS export conformance: add Reason to every [AspireExportIgnore] on the new Radius surface and [ResourceName] to the secret-store name params. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ddaf7f6d-b633-4b7e-9578-ec5b40e3c8e9
- SealedSecretManifest.ValidateStructure: guard Stack.Pop() on MappingEnd/ SequenceEnd so a malformed or out-of-sync YAML event stream fails as ASPIRERADIUS044 instead of escaping as a raw InvalidOperationException. - README: use the modern canonical resource-type string Radius.Data/redisCaches in the recipe-parameter scoping example (matches WithRecipeParametersTests and RadiusResourceTypes.RedisCaches); Applications.Datastores/redisCaches is only the legacy fallback emission type. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ddaf7f6d-b633-4b7e-9578-ec5b40e3c8e9
WaitForSealedSecretSynced_StatusMatchesButSecretAbsent_KeepsWaitingThenTimesOut used a 10ms wall-clock timeout and asserted secretCalls > 1. On a loaded CI runner a slow cold-JIT first iteration could consume the whole 10ms budget before a second poll, so only one secretExists call happened and the assertion failed. The probe now reports the Secret absent on the first call (so the loop keeps waiting past one iteration) and hangs on the second so the SUT's remaining-budget guard cancels it and surfaces the real ASPIRERADIUS058 timeout, guaranteeing at least two polls regardless of runner speed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ddaf7f6d-b633-4b7e-9578-ec5b40e3c8e9
…ostics Address re-review feedback on PR microsoft#18778: - SealedSecretManifest.ReadValidated: File.ReadAllBytes can throw ArgumentException (empty/whitespace/invalid-character path) and PathTooLongException, which previously escaped the catch filter without the ASPIRERADIUS044 wrapping promised by the XML-doc/README contract. Broaden the filter so every unreadable-manifest failure normalizes to ASPIRERADIUS044. - SealedSecretApplyStep.ParseGeneration: the "did not return metadata.generation" throw omitted the SealedSecret/store identity and a diagnostic code. Thread the store name + namespace/name through and map the failure to ASPIRERADIUS058 for consistent triage. Adds targeted tests for both paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ddaf7f6d-b633-4b7e-9578-ec5b40e3c8e9
Fixes findings from code-review, rubber-duck, security, architecture, and hosting-integration-authoring passes on the recipe-parameter and secret-store work in Aspire.Hosting.Radius. Secret handling: - Reject plaintext `kind: Secret` embedded in a SealedSecret's last-applied-configuration annotation; fail closed on malformed/non-scalar annotation values and non-object data/stringData (ASPIRERADIUS063). - Require an explicit metadata.namespace for application-scoped sealed stores so the namespace can't silently diverge per environment (ASPIRERADIUS055). - Bound the kubectl apply/sync/verify sequence with a shared timeout budget (ASPIRERADIUS066); tolerate transient/NotFound `kubectl get sealedsecret` failures during polling instead of aborting the deploy. - Reject key-specific envSecret consumers against keyless stores (ASPIRERADIUS064) and repeated population of a store (ASPIRERADIUS065); validate keys before mutating so a bad first key can't poison a retry. - Fire the targeted duplicate-identifier diagnostic (ASPIRERADIUS048) for app-vs-env Bicep identifier collisions, matching the actual emission scope. Recipe parameters: - Convert ParameterResource/RadiusProviderReference nested inside recipe parameter objects/arrays recursively so they stay late-bound, with cycle and depth guards. Cleanup: - Remove the dead WithTlsCertificate/GatewayTls consumer API and stale gateway-TLS doc comments. Adds unit tests for each finding and updates the README diagnostics table. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b76dc20d-c553-47b7-8f5e-6fcdc0f10aa3
Harden and tighten the Radius secret-store surface in response to Copilot review comments: - SealedSecretManifest: fail closed in EmbedsPlaintextSecret when the embedded last-applied-configuration has duplicate/missing/non-string kind or duplicate data/stringData, so a crafted payload cannot smuggle plaintext past a single TryGetProperty lookup. - WithSealedSecret: resolve a relative manifest path against the AppHost directory (matching other hosting file APIs) so it works regardless of the process working directory. - RadiusSecretStoreNaming: align the store-name grammar with Aspire's resource-name grammar (letters/digits/hyphens, start-with-letter, no consecutive/trailing hyphen, <=64) so publish mode (AddResource) never rejects a name run mode accepted. Update the ASPIRERADIUS049 message. - WithData: add a single-key convenience overload binding a data key to a secret parameter, delegating transactionally to the callback form. - SealedSecretApplyStep.GetSealedSecretStatusAsync: only treat SealedSecret NotFound or transient connectivity failures as retryable; surface permanent kubectl failures (auth/RBAC/context/missing plugin) immediately instead of waiting out the whole materialization budget. Adds 22 targeted tests (357 total, all passing). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b76dc20d-c553-47b7-8f5e-6fcdc0f10aa3
…e inline key rejection, existing-secret reference validation Re-review of PR microsoft#18834 raised three additional findings on the Radius secret-store surface: - Terraform Git PAT auth now requires the referenced store to declare a 'pat' key (optionally 'username'), matching what Radius reads for recipeConfig.terraform.authentication.git.pat, instead of accepting a basicAuthentication username/password store it cannot use (ASPIRERADIUS051). - RadiusSecretStoreDataBuilder.Add rejects a duplicate inline key via TryAdd instead of silently overwriting through the dictionary indexer, so a double-declared key surfaces at the call site (ASPIRERADIUS043). - WithExistingSecret validates the '[namespace/]name' reference at the API boundary (single separator, DNS-1123 name/namespace) rather than deferring a malformed reference to deploy time (ASPIRERADIUS046). Adds targeted tests and updates the README diagnostics table and xmldoc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b76dc20d-c553-47b7-8f5e-6fcdc0f10aa3
…ialization timeout, validate sealed manifest names A second re-review round on PR microsoft#18834 raised three findings: - ConfigureRadiusInfrastructure exposes secret stores (opts.SecretStores), but the identifier snapshot/rewire logic ignored them. Renaming a store construct left recipeConfig referencing the old <identifier>.id, and renaming the legacy application/environment left the store's ApplicationId/EnvironmentId pointing at stale symbols. Extended IdentifierSnapshot and added RewireSecretStoreReferences to rewrite both consumer references and parent scope IDs. - WithMaterializationTimeout accepted values above int.MaxValue milliseconds (e.g. TimeSpan.MaxValue) that CancellationTokenSource.CancelAfter / the deploy deadline cannot represent, so they passed publish validation but threw at deploy. Reject them at configuration time. - SealedSecret manifest parsing accepted any nonblank metadata.name (e.g. Bad_Name, a/b, an overlong namespace), deferring the failure to kubectl apply. Validate the name as a DNS-1123 subdomain and any explicit namespace as a DNS-1123 label at publish time, mirroring the WithExistingSecret fast-fail. Extracted the DNS-1123 helpers into a shared KubernetesName class and added tests for all three findings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b76dc20d-c553-47b7-8f5e-6fcdc0f10aa3
- WithExistingSecret validates reference and keys before marking the store populated so a corrected retry is not blocked by ASPIRERADIUS065 (R1). - Reject secret data keys that are not valid Kubernetes Secret keys at the API boundary for inline/existing/sealed populations (ASPIRERADIUS067) (R2). - rad workspace kube-context fallback now returns the single distinct context only; multiple contexts fail closed instead of guessing (R3). - Application-scoped store declared without any Radius environment now fails fast via a store-registered pipeline gate (ASPIRERADIUS068) (R4). - Add remarks/example xmldoc to AddRadiusSecretStore and WithRecipeParameters (R5/R6). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b76dc20d-c553-47b7-8f5e-6fcdc0f10aa3
d9b1415 to
d6a46d7
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 42 out of 42 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
src/Aspire.Hosting.Radius/Secrets/RadiusSecretStoreNaming.cs:28
- Radius secret-store resource names are capped at 63 characters upstream, but this validator accepts 64. A 64-character name therefore passes AppHost validation and only fails when Radius processes the generated
Applications.Core/secretStoresresource, defeating the fail-fast guarantee. Align this limit, its diagnostic text, and the 64-character acceptance test with Radius's 63-characterResourceNameStringcontract.
internal const int MaxNameLength = 64;
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 42 out of 42 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
src/Aspire.Hosting.Radius/Secrets/KubernetesName.cs:61
- This accepts
.and.., and it has no 253-character limit. Kubernetes rejects those values as Secret data keys, soWithData/WithExistingSecret/WithSealedSecretcan pass AppHost validation and then fail at deployment. Enforce the path-segment exclusions and maximum length here, update both error messages, and add boundary tests.
if (value.Length == 0)
{
return false;
}
- IsValidSecretDataKey now matches apimachinery IsConfigMapKey semantics: reject keys >253 chars, '.'/'..', and keys starting with '..' (N1). - IsTransientKubectlFailure no longer treats permanent x509 TLS trust failures as retryable; they fail fast instead of polling to the sync timeout (N2). - Secret-store validation test uses Directory.CreateTempSubdirectory (N3). - SealedSecret manifest requires the exact bitnami.com/v1alpha1 group/version instead of a bitnami.com/ prefix (N4). - Embedded SealedSecret in last-applied-configuration is now validated recursively for plaintext spec.template.data/stringData (ASPIRERADIUS063) (N5). - Recipe-parameter Merge validates all keys before applying (transactional) (N6). - Deploy-parameters test now exercises the production WriteDeployParametersFile helper and asserts the owner-only ARM JSON file contract (N7). - README secret-store diagnostics range lists only secret-store codes and notes 056/057 are separate and 053/054/060 retired (N8). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b76dc20d-c553-47b7-8f5e-6fcdc0f10aa3
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
The hand-rolled line/regex parser in ParseActiveWorkspaceContext did not honor real YAML syntax, so valid rad configs (trailing inline comments on values such as `default: prod # active`, quoted mapping keys, or flow-style mappings) failed to resolve the active workspace context and aborted every SealedSecret deployment with ASPIRERADIUS059 unless the override was set. Parse ~/.rad/config.yaml with YamlDotNet (already a dependency), reading workspaces.default then workspaces.items.<default>.connection.context, and keep the fail-closed behavior for missing/ambiguous values (default present but context missing -> null; no default -> single distinct context only). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b76dc20d-c553-47b7-8f5e-6fcdc0f10aa3
- SealedSecret apply: close TOCTOU by streaming the exact validated manifest bytes to `kubectl apply -f -` over stdin instead of re-reading the (mutable) manifest path. - Reject uppercase inline secret-store names: the name is used verbatim as the backing Kubernetes Secret name (DNS-1123), so require lowercase to fail fast at the API/publish boundary instead of at deploy. - ASPIRERADIUS067: include the 253-char limit and `.`/`..` prefix restrictions in the invalid-secret-data-key message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a79a2e75-6c34-40f1-8b75-dcd5ca8b2b16
| // other failure will never resolve by waiting, so surface it immediately instead of burning | ||
| // the whole materialization timeout. NotFound stderr: | ||
| // Error from server (NotFound): secrets "my-secret" not found | ||
| if (IsNotFound(stderr, name)) |
| await process.StandardInput.BaseStream.WriteAsync(input, cancellationToken).ConfigureAwait(false); | ||
| await process.StandardInput.BaseStream.FlushAsync(cancellationToken).ConfigureAwait(false); |
| private static void CopySealedSecretManifests(RadiusInfrastructureOptions options, string outputDir, ILogger logger) | ||
| { | ||
| foreach (var (storeName, manifest) in options.SealedSecretManifests) |
Description
This adds recipe parameter customization and secret management to the
Aspire.Hosting.Radiusintegration, so an AppHost can tune how Radius recipes provision backing resources and can supply the secrets those resources need — declaratively, in the app model — instead of configuring them out-of-band against the cluster.What you can now do:
ParameterResource/RadiusProviderReferencevalues, including when nested inside objects/arrays.builder.AddRadiusSecretStore(...)or environment-scoped viaradius.WithSecretStore(...)— and populate them three ways: inlineWithData, a reference to an existing KubernetesSecret(WithExistingSecret), or an encrypted Bitnami SealedSecret manifest (WithSealedSecret) that is applied and its materialization awaited at deploy time.Validation is fail-fast with a dedicated
ASPIRERADIUS0xxdiagnostic range covering population, publish, and deploy errors, so misconfiguration surfaces at publish time with an actionable message rather than as an opaque cluster failure.User-facing usage
Recipe parameters (C# AppHost):
Secret stores (C# AppHost):
Security considerations
This change handles secret material and executes external tooling, so it is security-relevant:
@secure()parameters, never as literals. SealedSecret manifests are validated to be a single encrypted BitnamiSealedSecret, and the parser fails closed if a manifest embeds a plaintextkind: Secret(including inside alast-applied-configurationannotation), has malformed/ambiguous structure, or uses YAML anchors/aliases/merge keys/duplicate keys (ASPIRERADIUS044,063).kubectlinvocations use discreteArgumentListargv entries withUseShellExecute=false; secret-returning calls suppress stdout logging so no secret material reaches logs, temp files, or error messages. The apply/sync/verify sequence is bounded by a materialization-timeout budget (ASPIRERADIUS066) and tolerates transient/NotFound poll failures rather than aborting the deploy..., no leading/trailing., no Windows reserved device names) before being used as Bicep symbols and filesystem path segments, preventing path traversal.radworkspace's Kubernetes context cannot be resolved, rather than applying tokubectl's ambient context (ASPIRERADIUS059).A security review of these changes was performed and found no high-confidence issues.
Checklist
<remarks />and<code />elements on your triple slash comments?