[release/13.5] Don't select skipped Gateways and Ingresses for TLS work - #19254
Conversation
A Gateway with no routes, and an Ingress with no paths and no default backend, are skipped during materialization. But five other sites re-selected those resources independently with no eligibility check, so deployment steps ran against objects that are never created: - tls-fqdn-discovery polled `kubectl get gateway` 180 x 5s (~15 min) before failing, for a Gateway that was never rendered. - The cert-manager HTTP-01 solver emitted a parentRef to the missing Gateway, leaving the solver HTTPRoute orphaned and Certificates stuck in Pending. This also suppressed the guard warning written to surface exactly that misconfiguration, since the ineligible Gateway made parentGateways.Count non-zero. - Field-manager cleanup and TLS secret bootstrap targeted non-existent Gateways and Ingresses, leaving orphaned self-signed Secrets. Centralize the rule as ShouldMaterialize on KubernetesGatewayResource and KubernetesIngressResource so every selection site shares one definition, and apply it at all five sites. Also reword the route-less Gateway and path-less Ingress warnings to name the omitted artifacts, and widen the cert-manager warning to cover both causes now that route-less Gateways reach that branch. Fixes #19217 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20d0667c-9a79-4368-85ff-e1876d55df72
The XML doc claimed 60 retries / 5 minutes, but the policy uses 179 attempts with 5-second delays (~15 minutes). Also call out that kubectl failures are indistinguishable from 'no address yet', so callers must only pass Gateways that were actually materialized. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20d0667c-9a79-4368-85ff-e1876d55df72
ShouldMaterialize is evaluated when pipeline steps are collected, which happens before any step runs, so it can only reject Ingresses that are statically ineligible. BuildIngressObject can still return null later (unresolvable backend), leaving a tls-bootstrap step registered for an Ingress that never appears in the chart and creating an orphaned Secret. Carry the owning resource through to deploy time via TlsSecretRequest and re-check effective materialization inside the bootstrap action, where GeneratedIngress is authoritative. Basing the original selection on GeneratedIngress is not possible: it is always null during step collection, so that would disable TLS bootstrap for every Ingress. Also assert the route-less Gateway and cert-manager solver warnings, which were previously emitted but untested. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20d0667c-9a79-4368-85ff-e1876d55df72
Two gaps found while reviewing the change as a whole. The Ingress "will not be created" warning was reworded alongside the Gateway one but never asserted, which is the same weakness that review already called out for the Gateway and cert-manager warnings. More importantly, nothing pinned the invariant that CollectTlsSecrets must not depend on GeneratedIngress. In production the pipeline builds every step before running any of them, so collection always sees GeneratedIngress as null; moving the materialization check there would disable TLS bootstrap for every Ingress. The existing tests could not catch that, because they inspect collection after app.Run() has already populated the generated objects. The new test collects before app.Run() and fails against that exact mutation. Also fixes a brace placed on the declaration line in CollectTlsSecrets. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20d0667c-9a79-4368-85ff-e1876d55df72
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19254Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19254" |
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.
Adds filtering and deploy-time rechecks to prevent TLS/cert-manager automation from acting on Gateway/Ingress resources that won’t be materialized, and strengthens tests around those scenarios.
Changes:
- Introduces
ShouldMaterializeon Gateway/Ingress resources and uses it to filter TLS/FQDN/cert-manager selection. - Tracks TLS secret “owners” and re-checks materialization at deploy time before bootstrapping certificates.
- Adds/updates tests and logging assertions to ensure skipped resources don’t generate artifacts or pipeline steps.
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/PipelineStepTestHelpers.cs | Adds helper to reconstruct pipeline steps in tests and assert on gateway/TLS-related step names. |
| tests/Aspire.Hosting.Kubernetes.Tests/KubernetesIngressTests.cs | Adds tests ensuring empty ingresses don’t generate templates, TLS bootstrap steps, and emits an explicit warning. |
| tests/Aspire.Hosting.Kubernetes.Tests/KubernetesGatewayTests.cs | Adds pipeline-step assertions, broadens “no routes” coverage, and asserts warning content when gateway is skipped. |
| tests/Aspire.Hosting.Kubernetes.Tests/CertManagerTests.cs | Ensures cert-manager solver parentRefs exclude route-less gateways and asserts a warning is logged. |
| tests/Aspire.Hosting.Kubernetes.Tests/Aspire.Hosting.Kubernetes.Tests.csproj | Adds diagnostics testing package to support logging test utilities. |
| src/Aspire.Hosting.Kubernetes/KubernetesIngressResource.cs | Adds ShouldMaterialize to represent configured eligibility for ingress materialization. |
| src/Aspire.Hosting.Kubernetes/KubernetesGatewayResource.cs | Adds ShouldMaterialize to represent configured eligibility for gateway materialization. |
| src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs | Filters selections by ShouldMaterialize, introduces TlsSecretRequest with owner, and re-checks materialization before TLS bootstrap. |
| src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs | Excludes route-less gateways from solver parentRefs and improves warning messaging. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| /// <summary> | ||
| /// Gets a value indicating whether this ingress is emitted into the deployment artifacts. | ||
| /// An ingress with neither path rules nor a default backend is skipped, so TLS secret | ||
| /// collection must not select it — bootstrapping a secret for an Ingress that is never | ||
| /// created leaves an orphaned self-signed certificate in the cluster. | ||
| /// </summary> | ||
| internal bool ShouldMaterialize => Paths.Count > 0 || DefaultBackend is not null; |
| "ClusterIssuer '{IssuerName}' has an HTTP-01 solver but no Gateway in environment '{EnvironmentName}' is both annotated with " + | ||
| ClusterIssuerAnnotationKey + "={IssuerName} and configured with at least one route. cert-manager will not be able to satisfy " + | ||
| "ACME challenges until at least one routed Gateway adopts this issuer (e.g. via WithRoute(...) and WithTls(issuer)).", |
| [Theory] | ||
| [InlineData(false, false)] | ||
| [InlineData(true, false)] | ||
| [InlineData(true, true)] | ||
| public async Task AddGateway_NoRoutes_DoesNotGenerateYamlOrTlsSteps(bool hasTls, bool hasHostname) |
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| #pragma warning disable ASPIREPIPELINES001 |
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. 3 / 100 test projects · 3 jobs, from 9 changed files. Selected test projects (3 / 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.
Chatted aabout this one offline. Important to get in as it is a valid scenario. Approved.
11a1277
into
release/13.5
|
✅ No documentation update needed. Step 5 branch taken: "excluded → backport (base_branch_is_release, head_branch_is_backport, title_release_prefix, body_backport_marker)"
No documentation PR created per the exclusion rule for backport PRs. |
Backport of #19220 to release/13.5
/cc Mitch Denny (@mitchdenny)
Customer Impact
aspire deployappears to hang for ~15 minutes and then fails when a Kubernetes AppHost has a TLS Gateway with no routes. The Gateway is deliberately skipped when rendering the chart, but FQDN discovery still polls for it (180 attempts x 5s), with nothing in the logs to indicate the Gateway will never be created — the user just seesWaiting for Gateway 'gateway' to be assigned a hostname address...until it times out.The same run also leaves an orphaned bootstrap TLS Secret in the cluster, and points a cert-manager HTTP-01 solver at the missing Gateway, which orphans the HTTPRoute and leaves Certificates stuck
Pending.Reproduced against stable 13.5.0 (11m03s stall before failure). With this fix the user gets an actionable warning naming exactly what will not be created, and the unusable TLS work is skipped.
Testing
Aspire.Hosting.Kubernetes.Tests, all passing. New coverage asserts each skip warning is emitted, that no gateway/TLS pipeline steps are registered for skipped resources, and that TLS secret collection does not depend on rendered objects. That last invariant was confirmed load-bearing by mutation testing — reintroducing the mistake it guards against fails the suite.public-tlsSecret on docker-desktop against stable 13.5.0, then confirmed both were gone with a build of this change (0tls-fqdn-discoverysteps registered, no orphaned Secret).Risk
Low. Confined to
Aspire.Hosting.Kubernetes, with no public API change — the newShouldMaterializepredicate isinternaland no/api/baseline files are touched.The predicate is exactly the negation of the conditions already used to skip rendering, so any configuration that deploys successfully today evaluates it as
trueand behaves identically. The behavioral change is limited to configurations that are already broken. One deploy-time re-check was added so an Ingress that drops out late (no resolvable backends) does not get a bootstrap certificate.Cherry-pick applied cleanly; net content is identical to what merged to
main.Regression?
No. The route-less Gateway skip (#16487, 2026-04-28) predates FQDN discovery (#16551, 2026-04-30), so the two have disagreed since FQDN discovery was first introduced. Both shipped in 13.4 and 13.5, making this a latent bug rather than something that regressed in 13.5.