Fix Kubernetes values for embedded environment parameters - #19429
Fix Kubernetes values for embedded environment parameters#19429Mitch Denny (mitchdenny) wants to merge 2 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19429Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19429" |
|
🚀 Deployment tests starting on PR #19429... This will deploy to real Azure infrastructure. Results will be posted here when complete. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Fixes Kubernetes Helm value declarations and deploy-time resolution for parameters embedded in composite environment expressions.
Changes:
- Tracks embedded config and secret parameters without creating synthetic environment variables.
- Merges tracked parameters into Helm values generation.
- Adds publish and deploy regression coverage for secret and non-secret parameters.
Show a summary per file
| File | Description |
|---|---|
src/Aspire.Hosting.Kubernetes/KubernetesResource.cs |
Tracks embedded parameter metadata. |
src/Aspire.Hosting.Kubernetes/KubernetesPublishingContext.cs |
Adds embedded parameters to Helm values. |
tests/Aspire.Hosting.Kubernetes.Tests/KubernetesPublisherTests.cs |
Tests generated chart output. |
tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs |
Tests deploy-time resolution. |
tests/Aspire.Hosting.Kubernetes.Tests/Snapshots/KubernetesPublisherTests.PublishAsync_HandlesSpecialResourceName#01.verified.yaml |
Updates existing values snapshot. |
tests/Aspire.Hosting.Kubernetes.Tests/Snapshots/KubernetesPublisherTests.PublishAsync_EmbeddedParametersInEnvironmentExpressionsPopulateValues#00.verified.yaml |
Verifies values declarations. |
tests/Aspire.Hosting.Kubernetes.Tests/Snapshots/KubernetesPublisherTests.PublishAsync_EmbeddedParametersInEnvironmentExpressionsPopulateValues#01.verified.yaml |
Verifies ConfigMap output. |
tests/Aspire.Hosting.Kubernetes.Tests/Snapshots/KubernetesPublisherTests.PublishAsync_EmbeddedParametersInEnvironmentExpressionsPopulateValues#02.verified.yaml |
Verifies Secret output. |
tests/Aspire.Hosting.Kubernetes.Tests/Snapshots/KubernetesDeployTests.EmbeddedParametersInEnvironmentExpressions_EndToEnd_PublishAndResolve.verified.yaml |
Verifies resolved overrides. |
Review details
Suppressed comments (1)
src/Aspire.Hosting.Kubernetes/KubernetesPublishingContext.cs:201
- The same silent collision occurs for secrets: if
Secretsalready contains the embedded parameter's key,TryAdddiscards itsParameterSource, while the composite expression continues to reference that secret path. Deploy then resolves the path from the unrelated secret entry or leaves it unresolved. Reject conflicting mappings or give embedded secret parameters a distinct values path.
foreach (var kvp in resourceContext.AdditionalSecretValues)
{
secretItems.TryAdd(kvp.Key, kvp.Value);
- Files reviewed: 9/9 changed files
- Comments generated: 2
- Review effort level: Balanced
| foreach (var kvp in resourceContext.AdditionalConfigValues) | ||
| { | ||
| configItems.TryAdd(kvp.Key, kvp.Value); |
| foreach (var kvp in resourceContext.AdditionalConfigValues) | ||
| { | ||
| configItems.TryAdd(kvp.Key, kvp.Value); |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a27416ca-ff25-4524-b57f-94daeb337f40
|
🚀 Deployment tests starting on PR #19429... This will deploy to real Azure infrastructure. Results will be posted here when complete. |
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 / 101 test projects · 4 jobs, from 21 changed files. Selected test projects (4 / 101)
Selected jobs (4)
How these were chosen — grouped by what changed🔧 🔧 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 Job reasons
Selection computed for commit |
Description
Parameters embedded in composite Kubernetes environment expressions were emitted as Helm references without declaring the referenced values. Published charts could therefore render incomplete values such as
http:///test, andaspire deploycould not resolve the parent expression.This change preserves embedded parameter metadata in values-only config and secret collections. The Kubernetes publisher adds those entries to
values.yamlwithout creating synthetic environment variables, and deploy-time overrides resolve both the nested parameter and the parent composite value.Expanded expression coverage also identified parameter-driven conditionals with secret-valued branches being omitted from the chart. Those final environment values are now routed through the generated Kubernetes Secret.
Characterization coverage now locks in:
aspire publishtohelm templaterendering for embedded config and secret parameters.User-facing usage
The generated Helm values now declare the nested parameter:
The deployment override resolves
SOME_URLtohttp://localhost/test.Validation
Aspire.Hosting.Kubernetes.Tests: 288 passed.RenderEmbeddedEnvironmentExpressionsWithHelm: passed using a locally built Linux CLI archive and real Helm rendering.AksStarterWithRedisHelmDeploymentTests: passed against commit53abb98040, including Helm chart generation, AKS installation, pod and deployment readiness, service endpoint checks, Redis-backed application behavior, and resource cleanup.Fixes #11140
Checklist
<remarks />and<code />elements on your triple slash comments?