Skip to content

Don't select skipped Gateways and Ingresses for TLS work - #19220

Merged
Mitch Denny (mitchdenny) merged 4 commits into
mainfrom
mitchdenny-fix-skipped-gateway-fqdn-wait
Aug 11, 2026
Merged

Don't select skipped Gateways and Ingresses for TLS work#19220
Mitch Denny (mitchdenny) merged 4 commits into
mainfrom
mitchdenny-fix-skipped-gateway-fqdn-wait

Conversation

@mitchdenny

@mitchdenny Mitch Denny (mitchdenny) commented Aug 11, 2026

Copy link
Copy Markdown
Member

Description

Deploying an AppHost that declares a Gateway without any routes made aspire deploy hang for roughly 15 minutes before failing. The reported symptom turned out to be one instance of a broader problem, so this PR fixes the class rather than the single case.

A Gateway with no routes, and an Ingress with no paths and no default backend, are intentionally skipped during materialization — they are never written into the deployment artifacts. But five other sites re-selected those same resources independently, with no eligibility check, so deployment steps ran against Kubernetes objects that are never created:

Site Effect on a skipped resource
CollectGatewaysNeedingFqdnDiscovery The reported hangtls-fqdn-discovery-{env} polls kubectl get gateway 180 × 5s (~15 min) waiting for an address on a Gateway that was never rendered
CertManagerExtensions.AppendSolver Emits an HTTP-01 solver parentRef to the missing Gateway
CollectGatewaysWithTls Field-manager cleanup targets a non-existent Gateway
CollectTlsSecrets (gateway loop) Orphaned self-signed Secret
CollectTlsSecrets (ingress loop) Orphaned self-signed Secret

The cert-manager case is the most damaging and is not visible in the original issue. WithTls(issuer) sets the cert-manager.io/cluster-issuer annotation, so a route-less Gateway still reached the solver and became a parentRef to a Gateway that never exists. The solver's HTTPRoute has nothing to attach to, the ACME challenge URL is unreachable, and Certificates sit in Pending indefinitely. Worse, AppendSolver already had a guard that warns when no eligible Gateway is found — written precisely to surface this at deploy time — but the ineligible Gateway made parentGateways.Count == 1, so the one diagnostic that would have explained the failure was suppressed.

User-facing behavior

Given an AppHost with a Gateway that has no routes:

var builder = DistributedApplication.CreateBuilder(args);

var k8s = builder.AddKubernetesEnvironment("env");

// No .WithRoute(...) call, so this Gateway is never materialized.
builder.AddGateway("public")
       .WithParent(k8s)
       .WithTls(issuer);

builder.Build().Run();

Before: aspire deploy registered tls-fqdn-discovery-env and blocked for ~15 minutes polling for a Gateway that would never appear, then failed. If cert-manager was in play, the ClusterIssuer manifest also shipped a dangling parentRef and no warning was emitted.

After: no TLS or gateway steps are registered for the skipped Gateway, deployment does not stall, no dangling parentRef is emitted, and a warning names what was omitted:

warn: Gateway 'public' has no routes configured. The Gateway, routes, TLS certificate, and load-balancer frontend will not be created.
warn: ClusterIssuer 'letsencrypt' has an HTTP-01 solver but no Gateway in environment 'env' is both annotated with
      cert-manager.io/cluster-issuer=letsencrypt 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)).

Implementation

The eligibility rule is centralized as internal bool ShouldMaterialize on KubernetesGatewayResource (Routes.Count > 0) and KubernetesIngressResource (Paths.Count > 0 || DefaultBackend is not null), and applied at all five selection sites plus the two existing materialization skips. Previously the rule was inlined in exactly one place, which is what allowed the other sites to drift.

ShouldMaterialize is a static eligibility rule, and it is evaluated while pipeline steps are being collected. DistributedApplicationPipeline.ResolveStepsAsync builds the complete step list before any step executes, so at selection time KubernetesIngressResource.GeneratedIngress is always null. That leaves one residual gap: BuildIngressObject can still return null later, when an Ingress path's backend cannot be resolved, so a tls-bootstrap step could remain registered for an Ingress that never reaches the chart, creating an orphaned Secret.

CollectTlsSecrets therefore returns List<TlsSecretRequest> (secret name, hostname, owning resource) instead of a HashSet<(ReferenceExpression, ReferenceExpression)>, and BootstrapTlsSecretsAsync re-checks OwnerWasMaterialized(owner) at action time. That step depends on helm-deploy-{env}, so it runs after prepare-deployment-targets and GeneratedIngress is authoritative there. A seen set preserves the secret+host de-duplication the HashSet previously provided. Gateways need no second check: BuildGatewayObjects always emits the Gateway once ShouldMaterialize is true, and only skips individual HTTPRoutes on resolution failure.

The 15-minute retry budget (MaxRetryAttempts = 179, 5s delay) is deliberately unchanged. AGC and other Gateway controllers legitimately take 5–10 minutes to assign an address to a materialized Gateway; the bug was waiting for a Gateway that would never exist, not waiting too long for one that would.

Validation

Each fix was confirmed to be load-bearing by temporarily reverting only the selection-site filters — leaving the materialization skips intact, i.e. the exact pre-fix state — and re-running the suite:

failed CertManagerTests.BuildClusterIssuerManifest_RouteLessGateway_IsNotEmittedAsParentRef
       Assert.DoesNotContain() Failure: Sub-string found
failed KubernetesGatewayTests.AddGateway_NoRoutes_DoesNotGenerateYamlOrTlsSteps(hasTls: True, hasHostname: True)
       Assert.Empty() Failure: Collection was not empty
failed KubernetesGatewayTests.AddGateway_NoRoutes_DoesNotGenerateYamlOrTlsSteps(hasTls: True, hasHostname: False)
       Assert.Empty() Failure: Collection was not empty
failed KubernetesIngressTests.AddIngress_NoPathsWithTls_DoesNotRegisterTlsBootstrapStep
       Assert.Empty() Failure: Collection was not empty

The hasTls: False variant correctly passed in both states, confirming it is genuine additional coverage rather than a false positive. With the fixes in place, Aspire.Hosting.Kubernetes builds with 0 warnings and all 277 tests in Aspire.Hosting.Kubernetes.Tests pass with no snapshot drift.

One existing test had codified the cert-manager bug: BuildClusterIssuerManifest_EmitsExpectedYamlForLetsEncryptHttp01 used a route-less Gateway and asserted the parentRef was present. It now gives the Gateway a route, and a new test covers the route-less case.

Tests assert the complete set of TLS/gateway step names rather than using Assert.DoesNotContain. The regression class here is "a new gateway step is added without the eligibility filter", which a name-by-name absence check cannot catch — and Assert.DoesNotContain is discouraged by AGENTS.md as a weak assertion. The duplicated per-file pipeline-step harness was extracted into a shared PipelineStepTestHelpers.

Both diagnostics are now asserted by tests. They were previously emitted but unverified, so the route-less Gateway warning and the cert-manager solver warning could each have been weakened or dropped silently — and those messages are the only signal a user gets that their Gateway or certificate was skipped.

No new deployment E2E test is added. The regression mechanism is pure pipeline-step selection: deterministic, cluster-independent, and fully observable from the step assertions above, which fail without the fix. The existing Kubernetes deployment E2E suite was run against this branch on real Azure and all five classes passed, including KubernetesGatewayTlsDeploymentTests and both cert-manager classes (run 31481521922).

Fixes #19217

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

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
Copilot AI balanced review requested due to automatic review settings August 11, 2026 04:09
@github-actions

Copy link
Copy Markdown
Contributor

🚀 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 -- 19220

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19220"

@github-actions github-actions Bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 11, 2026
@github-actions

This comment has been minimized.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prevents skipped Kubernetes Gateways and Ingresses from entering TLS workflows, addressing #19217’s deployment stall and invalid TLS artifacts.

Changes:

  • Centralizes materialization eligibility.
  • Filters TLS discovery, bootstrap, cleanup, and cert-manager selection.
  • Adds focused pipeline-step and manifest regression tests.
Show a summary per file
File Description
src/Aspire.Hosting.Kubernetes/KubernetesGatewayResource.cs Adds Gateway eligibility predicate.
src/Aspire.Hosting.Kubernetes/KubernetesIngressResource.cs Adds Ingress eligibility predicate.
src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs Filters pipeline selections and improves warnings.
src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs Excludes skipped Gateways from solver references.
tests/Aspire.Hosting.Kubernetes.Tests/PipelineStepTestHelpers.cs Adds shared pipeline-step inspection helpers.
tests/Aspire.Hosting.Kubernetes.Tests/KubernetesGatewayTests.cs Covers skipped and materialized Gateway steps.
tests/Aspire.Hosting.Kubernetes.Tests/KubernetesIngressTests.cs Covers skipped Ingress TLS steps.
tests/Aspire.Hosting.Kubernetes.Tests/CertManagerTests.cs Covers cert-manager parent references.

Review details

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

Comment thread src/Aspire.Hosting.Kubernetes/KubernetesIngressResource.cs
Comment thread src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs
Comment thread src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs
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
Copilot AI review requested due to automatic review settings August 11, 2026 04:41
@mitchdenny

Copy link
Copy Markdown
Member Author

PR Testing Report

PR Information

Artifact Version Verification

  • Expected Commit: 9d0d9425
  • Installed Version: 13.6.0-pr.19220.g9d0d9425
  • Status: ✅ Verified

Testing was done A/B against a real Kubernetes cluster. Note that the fix ships in the
Aspire.Hosting.Kubernetes package (not the CLI binary), so each scenario pins the package
version explicitly via the PR hive.

  • Baseline app → stable 13.5.0 CLI, packages 13.5.0-preview.1.26410.5
  • PR app → PR CLI, packages 13.6.0-pr.19220.g9d0d9425

Changes Analyzed

Files Changed

  • src/Aspire.Hosting.Kubernetes/KubernetesGatewayResource.cs — adds ShouldMaterialize
  • src/Aspire.Hosting.Kubernetes/KubernetesIngressResource.cs — adds ShouldMaterialize
  • src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs — applies the rule at 5 selection sites
  • src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs — filters route-less Gateways out of solver parentRefs
  • tests/Aspire.Hosting.Kubernetes.Tests/PipelineStepTestHelpers.cs (new shared helper)
  • tests/Aspire.Hosting.Kubernetes.Tests/{KubernetesGatewayTests,CertManagerTests,KubernetesIngressTests}.cs

Change Categories

  • CLI changes
  • Hosting integration changes (Aspire.Hosting.Kubernetes)
  • Dashboard changes
  • CI infrastructure changes
  • VS Code extension changes
  • Test changes

Test Scenarios Executed

Scenario 0: Baseline reproduction (pre-fix behavior)

Objective: Confirm the bug actually reproduces on shipped bits, so the PR result is meaningful.
Coverage Type: Baseline / regression reproduction
Status: ✅ Bug reproduced

Steps:

  1. Created an AppHost pinned to stable 13.5.0-preview.1.26410.5, with a cert-manager issuer and a
    route-less AddGateway("public").WithGatewayClass("nginx").WithTls(issuer).
  2. Ran aspire publish → confirmed no Gateway YAML is generated (the Gateway is skipped).
  3. Ran aspire deploy against docker-desktop.

Observations:

  • aspire publish emitted only Chart.yaml, values.yaml, and the dashboard templates — no public.yaml,
    confirming the Gateway was never materialized.
  • aspire deploy nevertheless registered tls-fqdn-discovery-env and logged:
    (tls-fqdn-discovery-env) [INF] Waiting for Gateway 'public' to be assigned a hostname address...
  • It then stalled for 11m03s (14:17:30 → 14:28:33) before I killed it. Left alone it would have
    consumed the full ~15 minute retry budget (179 attempts × 5s).
  • Meanwhile kubectl get gateway -A returned
    error: the server doesn't have a resource type "gateway" — it was polling for an object whose CRD
    wasn't even installed, and the retry loop cannot distinguish that from "no address yet".
  • It also created an orphaned public-tls Secret in default (confirmed with kubectl get secret public-tls),
    independently demonstrating the orphaned-secret defect.

Scenario 1: Route-less TLS Gateway with the PR build (headline A/B)

Objective: The exact same app must no longer register FQDN discovery or stall.
Coverage Type: Happy path (primary fix)
Status: ✅ Passed

Steps:

  1. Same AppHost, only the two version pins changed to 13.6.0-pr.19220.g9d0d9425.
  2. Deleted the orphaned public-tls Secret left by the baseline so the assertion was clean.
  3. Ran aspire deploy.

Observations:

  • grep -c tls-fqdn-discovery over the deploy log → 0. The step is not registered at all.
  • gateway-field-cleanup-env is also absent (correctly skipped for a non-materialized Gateway).
  • No orphaned public-tls Secret was recreated (Error from server (NotFound)).
  • The improved warning fired:
    (prepare-deployment-targets-env) [WRN] Gateway 'public' has no routes configured. The Gateway, routes, TLS certificate, and load-balancer frontend ...
  • No 15-minute stall. The run terminated in ~5 minutes at an unrelated step (see note below).

Note on the non-blocking failure: this deploy ultimately failed at helm-install-cert-manager-chart
(Deployment/cert-manager-chart not ready ... Progress deadline exceeded). This is environmental, not a PR regression
helm history shows revision 1 failed at 14:17:31 during the baseline run too, with the identical error.
docker-desktop simply cannot bring up cert-manager here. Both sides fail identically at that step;
the difference under test (FQDN discovery) is unaffected.


Scenario 2: cert-manager solver parentRefs (Finding 1)

Objective: A skipped Gateway must not be emitted as an HTTP-01 solver parentRef.
Coverage Type: Happy path (secondary fix found during code review)
Status: ✅ Passed

Observations:

  • With the PR build, the route-less Gateway produced no dangling parentRefs entry, and the
    reworded cert-manager warning correctly covers the route-less cause.
  • Pre-fix, this would have emitted a solver HTTPRoute pointed at a Gateway that is never created,
    leaving Certificates stuck Pending — and because parentGateways.Count == 1, the pre-existing
    guard warning was suppressed.

Scenario 3: Routed TLS Gateway — positive control

Objective: Guard against over-filtering. A Gateway with a route must still fully materialize and
still register all its TLS steps.
Coverage Type: Positive control / regression guard
Status: ✅ Passed

Steps:

  1. Same shape, but with a real container backend and .WithRoute("/", api.GetEndpoint("http")).
  2. Installed Gateway API CRDs (standard-install.yaml v1.2.0) — an initial run aborted at
    helm-deploy-env purely because docker-desktop had no Gateway CRDs.
  3. Ran aspire publish then aspire deploy.

Observations:

  • aspire publish did emit templates/public/public.yaml and templates/public/route.yaml.
    The Gateway YAML contains the HTTPS listener with certificateRefs: [public-tls] and the
    cert-manager.io/cluster-issuer: le-prod annotation — fully intact.
  • aspire deploy registered both gateway-field-cleanup-env and tls-fqdn-discovery-env.
  • A real Gateway object was created in the cluster:
    default public nginx <no address> Unknown 2m51s
  • tls-fqdn-discovery-env then polled it and legitimately created the bootstrap secret
    (Creating bootstrap TLS secret 'public-tls' for 'bootstrap.invalid'). This is correct behavior —
    the Gateway exists, it just has no address because docker-desktop has no load-balancer controller.

This is the key result: the fix filters only non-materialized resources. Working Gateways are untouched.


Scenario 4: Path-less Ingress with TLS (Finding 2)

Objective: A path-less Ingress with TLS must not bootstrap an orphaned Secret.
Coverage Type: Unhappy path
Expected Outcome: No Ingress YAML, no TLS bootstrap step, no orphaned Secret, deploy still succeeds.
Status: ✅ Passed

Observations:

  • aspire publish emitted no Ingress YAML (only Chart.yaml, values.yaml, dashboard templates).
  • aspire deploy returned EXIT=0 — a fully clean, successful deploy.
  • grep -cE "tls-bootstrap|tls-fqdn-discovery"0.
  • kubectl get secret web-tlsError from server (NotFound). No orphaned Secret.

Scenario 5: Unit test suite

Objective: Verify the regression tests are load-bearing.
Status: ✅ Passed

  • 273/273 Aspire.Hosting.Kubernetes.Tests pass; 0 build warnings; no snapshot drift.
  • Verified the tests are meaningful by reverting only the selection filters → 4 tests failed;
    restoring the fix → all pass.

Summary

Scenario Status Notes
0. Baseline reproduction ✅ Bug reproduced Stalled 11m03s + orphaned public-tls Secret
1. Route-less Gateway (PR) ✅ Passed tls-fqdn-discovery absent; no stall; no orphaned Secret
2. cert-manager parentRefs ✅ Passed No dangling solver parentRef; warning reworded
3. Routed Gateway (positive control) ✅ Passed Gateway YAML + object + both TLS steps intact
4. Path-less Ingress + TLS ✅ Passed Deploy exit 0; no TLS step; no orphaned Secret
5. Unit tests ✅ Passed 273/273, 0 warnings, filters proven load-bearing

Overall Result

✅ PR VERIFIED

The headline defect is fixed and directly demonstrated A/B on a live cluster: the identical AppHost
stalls 11+ minutes on shipped bits and does not register the step at all with the PR build.
The two additional defects found during code review (cert-manager solver parentRefs, orphaned
Ingress TLS Secret) are also confirmed fixed. The positive control proves the change does not
over-filter — routed Gateways still materialize and still run their full TLS pipeline.

Recommendations

  • None blocking.
  • Follow-up (optional, out of scope): DiscoverGatewayFqdnAsync swallows all kubectl errors
    (ThrowOnNonZeroReturnCode = false + empty OnErrorData, retrying whenever the result is null), so a
    missing CRD, a deleted object, or an unreachable cluster are all indistinguishable from "no address yet"
    and each burn the full ~15-minute budget. This PR removes the selection bug that made it easy to hit;
    distinguishing hard failures from "not ready yet" would make the step fail fast for the remaining causes.
    A doc-correction commit (b2d2531) has been pushed noting the real budget and this caveat.

@github-actions

This comment has been minimized.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (4)

src/Aspire.Hosting.Kubernetes/KubernetesIngressResource.cs:94

  • ShouldMaterialize can still be true when no Ingress is emitted. BuildIngressObject returns null when every configured path/default backend lacks a deployment target or endpoint mapping (KubernetesEnvironmentResource.cs:827-830), but this predicate then lets CollectTlsSecrets register the bootstrap step anyway, leaving the same orphaned secret this change is intended to prevent. Base TLS selection on successful materialization (or otherwise include backend resolvability in the eligibility decision).
    internal bool ShouldMaterialize => Paths.Count > 0 || DefaultBackend is not null;

src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs:639

  • The route-less regression test passes NullLogger, so it does not verify this diagnostic even though the actionable warning is part of the user-facing fix. Removing this warning, or losing the issuer/environment/WithRoute guidance, would leave all added tests green. Capture the logger output and assert the key diagnostic fields.
                        logger.LogWarning(
                            "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)).",

src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs:886

  • The updated route-less Gateway test verifies artifacts and pipeline-step names but never asserts this new actionable warning. Since the issue explicitly requires users to be told which Gateway outputs are omitted, capture the publish logs and assert the Gateway name plus the omitted-resource guidance so this behavior cannot disappear unnoticed.
                logger.LogWarning(
                    "Gateway '{GatewayName}' has no routes configured. The Gateway, routes, TLS certificate, and load-balancer frontend will not be created.",
                    gatewayResource.Name);

tests/Aspire.Hosting.Kubernetes.Tests/CertManagerTests.cs:195

  • These assertions use the weak Assert.DoesNotContain pattern discouraged by the repository guidance for generated artifacts. The test can still pass if the manifest loses unrelated required content. Snapshot the complete generated YAML with Verify so both the solver shape and absence of parentRefs are covered together.
        Assert.DoesNotContain("parentRefs:", yaml);
        Assert.DoesNotContain("name: public", yaml);
  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

Copy link
Copy Markdown
Contributor

🚀 Deployment tests starting on PR #19220...

This will deploy to real Azure infrastructure. Results will be posted here when complete.

View workflow run

@mitchdenny

Copy link
Copy Markdown
Member Author

✅ Deployment E2E tests passed (real Azure)

Ran the Kubernetes deployment E2E suite against this PR's branch on real Azure infrastructure.

Run: 31461921150 · Conclusion: success · Commit: b2d2531 · ~29 min wall clock

Dispatched deployment-tests.yml with test_filter=Kubernetes, which selected exactly the 5 classes covering this PR's blast radius (Aspire.Hosting.Kubernetes gateway/ingress selection + cert-manager solver parentRefs).

Deployment test class Result Duration
KubernetesGatewayTlsDeploymentTests ✅ Passed 18m
AksAzureKubernetesEnvironmentGatewayDeploymentTests ✅ Passed 14m
AksAzureKubernetesEnvironmentCertManagerDeploymentTests ✅ Passed 20m
AksAzureKubernetesEnvironmentCertManagerTypeScriptDeploymentTests ✅ Passed 19m
KubernetesHelmChartDeploymentTests ✅ Passed 8m

Each job reported total: 1, failed: 0, succeeded: 1, skipped: 0 — confirming the tests genuinely executed and were not skipped for missing Azure auth.

Why this is the right coverage for this PR

KubernetesGatewayTlsDeploymentTests is a direct, real-infrastructure regression guard for the two riskiest parts of this change. It deploys to a real AKS cluster with the ALB controller, Gateway API, and cert-manager with a Let's Encrypt HTTP-01 ClusterIssuer (gatewayHTTPRoute solver), using a routed Gateway with .WithRoute("/", ...) + .WithTls(), and asserts:

  1. The Gateway gets an FQDN assigned by AGC.
  2. The FQDN discovery pipeline step patches the hostname onto the HTTPS listener — this is precisely the tls-fqdn-discovery step this PR filters out for route-less Gateways. Its passing proves the filter does not over-apply to real, materialized Gateways.
  3. cert-manager issues a real TLS certificate via HTTP-01 — this exercises the solver parentRefs path changed in CertManagerExtensions.AppendSolver, confirming that filtering route-less Gateways out of parentGateways does not break real certificate issuance.
  4. The app is reachable over HTTPS.

In other words, the change's two highest-risk behaviors (dropping a Gateway from FQDN discovery, and dropping it from the cert-manager solver) are validated end-to-end against live infrastructure, not just unit tests.

Combined with the earlier local A/B (route-less Gateway stalls 11m+ on shipped bits vs. the step never being registered with this PR) and the 273 passing unit tests, the fix is verified from both directions: the broken path is gone, and the working path is untouched.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving with the expectation that the three existing unresolved review threads are addressed before merge, particularly the ingress materialization/TLS bootstrap correctness issue.

Validation: I reviewed the affected code paths locally and ran Aspire.Hosting.Kubernetes.Tests at b2d2531: 273 passed, 0 failed, 0 skipped. The PR testing report also records successful real-Azure Kubernetes deployment E2E coverage at this commit; I did not independently run that Azure deployment suite.

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
Copilot AI review requested due to automatic review settings August 11, 2026 09:40
@github-actions

This comment has been minimized.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (3)

src/Aspire.Hosting.Kubernetes/KubernetesIngressResource.cs:92

  • This documentation describes ShouldMaterialize as an authoritative emitted-state signal, but an Ingress with configured paths can still be omitted when all backends fail resolution, as the new tests demonstrate. Describe this as configuration-time eligibility so future callers do not mistake it for GeneratedIngress is not null.
    /// 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.

src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs:2077

  • The new unresolvable-backend tests only call OwnerWasMaterialized directly; they never execute this guard or the tls-bootstrap step. Removing this if leaves those tests green, so the orphaned-secret regression is not protected at the point where the secret is actually skipped. Add a test that executes the bootstrap action with an unmaterialized Ingress and verifies that no kubectl/secret creation is attempted, with a materialized positive control.
            if (!OwnerWasMaterialized(tlsSecret.Owner))

src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs:1222

  • PublishAsync does not populate these generated objects; PrepareDeploymentTargetsAsync does so before the publish/Helm steps. Naming the actual preparation phase matters here because this method's correctness depends on that ordering.
    /// Determines whether the routing resource that requested a TLS secret actually made it into
    /// the rendered chart. Called at deploy time, once publish has populated the generated objects.
  • Files reviewed: 9/9 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

Copy link
Copy Markdown
Contributor

🚀 Deployment tests starting on PR #19220...

This will deploy to real Azure infrastructure. Results will be posted here when complete.

View workflow run

@mitchdenny

Copy link
Copy Markdown
Member Author

Deployment E2E re-run after the review fixes

The earlier E2E run predated commit d1f13b07b4, which changed the TLS bootstrap action path (CollectTlsSecrets now returns List<TlsSecretRequest> and BootstrapTlsSecretsAsync re-checks materialization). Re-ran the Kubernetes deployment suite against real Azure on the current head.

Run 31481521922 — all green, 28m19s (d1f13b07b4)

Test class Result
KubernetesGatewayTlsDeploymentTests ✅ success
AksAzureKubernetesEnvironmentGatewayDeploymentTests ✅ success
AksAzureKubernetesEnvironmentCertManagerDeploymentTests ✅ success
AksAzureKubernetesEnvironmentCertManagerTypeScriptDeploymentTests ✅ success
KubernetesHelmChartDeploymentTests ✅ success

The three classes that exercise the changed code paths end-to-end — gateway TLS and both cert-manager classes — all pass, so the TlsSecretRequest refactor does not regress the happy path where Gateways and Ingresses are materialized. That was the main risk of moving the materialization check to action time.

Merge state

  • Full CI green on d1f13b07b4: 337 checks pass, 3 skipped, 0 failing.
  • All 277 unit tests in Aspire.Hosting.Kubernetes.Tests pass; Aspire.Hosting.Kubernetes builds with 0 warnings.
  • All review threads resolved.
  • MERGEABLE / CLEAN, no conflicts with main.

One thing for the approver to note: James Newton-King (@JamesNK)'s approval predates d1f13b07b4, which added 73 lines of production code to KubernetesEnvironmentResource.cs in response to review feedback. Worth a second look before merging.

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
Copilot AI review requested due to automatic review settings August 11, 2026 12:36
@github-actions

Copy link
Copy Markdown
Contributor

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)

Aspire.Hosting.Azure.Kubernetes.Tests, Aspire.Hosting.Docker.Tests, Aspire.Hosting.Kubernetes.Tests

Selected jobs (3)

deployment-e2e, extension-e2e, typescript-api-compat


How these were chosen — grouped by what changed

🔧 src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs (changed source)
1 directly: Aspire.Hosting.Kubernetes.Tests
2 via the project graph: Aspire.Hosting.Azure.Kubernetes.Tests (2 hops), Aspire.Hosting.Docker.Tests

🔧 src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs (changed source)
1 directly: Aspire.Hosting.Kubernetes.Tests

🔧 src/Aspire.Hosting.Kubernetes/KubernetesGatewayResource.cs (changed source)
1 directly: Aspire.Hosting.Kubernetes.Tests

🔧 src/Aspire.Hosting.Kubernetes/KubernetesIngressResource.cs (changed source)
1 directly: Aspire.Hosting.Kubernetes.Tests

🧪 tests/Aspire.Hosting.Kubernetes.Tests/Aspire.Hosting.Kubernetes.Tests.csproj (changed test)
1 directly: Aspire.Hosting.Kubernetes.Tests

🧪 tests/Aspire.Hosting.Kubernetes.Tests/CertManagerTests.cs (changed test)
1 directly: Aspire.Hosting.Kubernetes.Tests

🧪 tests/Aspire.Hosting.Kubernetes.Tests/KubernetesGatewayTests.cs (changed test)
1 directly: Aspire.Hosting.Kubernetes.Tests

🧪 tests/Aspire.Hosting.Kubernetes.Tests/KubernetesIngressTests.cs (changed test)
1 directly: Aspire.Hosting.Kubernetes.Tests

🧪 tests/Aspire.Hosting.Kubernetes.Tests/PipelineStepTestHelpers.cs (changed test)
1 directly: Aspire.Hosting.Kubernetes.Tests

Job reasons

Job Triggered by
deployment-e2e affected project Aspire.Hosting.Azure.Kubernetes
extension-e2e src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs, src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs, src/Aspire.Hosting.Kubernetes/KubernetesGatewayResource.cs, src/Aspire.Hosting.Kubernetes/KubernetesIngressResource.cs
• affected project Aspire.Hosting.Kubernetes
typescript-api-compat affected project Aspire.Hosting.Kubernetes

Selection computed for commit 4d60c9d.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (3)

src/Aspire.Hosting.Kubernetes/KubernetesIngressResource.cs:93

  • This summary overstates the property's contract: true does not mean the Ingress is emitted, because BuildIngressObject can still return null when no backend resolves. Describe this as static/configuration eligibility so callers do not rely on a false materialization guarantee.
    /// <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>

tests/Aspire.Hosting.Kubernetes.Tests/CertManagerTests.cs:197

  • This generated-YAML test uses negative substring assertions, contrary to the repository guidance in AGENTS.md:267-268. It can pass if unrelated required solver content is accidentally dropped along with parentRefs. Verify the complete YAML with a snapshot (or an exact full-value assertion) instead.
        Assert.Contains("- http01:", yaml);
        Assert.Contains("gatewayHTTPRoute:", yaml);
        Assert.DoesNotContain("parentRefs:", yaml);
        Assert.DoesNotContain("name: public", yaml);

src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs:2083

  • The new ingress tests verify that OwnerWasMaterialized returns false, but none executes this tls-bootstrap action. Removing this guard would therefore leave the added tests green while restoring the orphaned-secret behavior. Add a focused test that invokes the captured bootstrap step for an unmaterialized Ingress and verifies that no kubectl process is started.
            if (!OwnerWasMaterialized(tlsSecret.Owner))
            {
                context.Logger.LogDebug(
                    "Skipping TLS bootstrap for '{ResourceName}' because it was not included in the generated chart.",
                    tlsSecret.Owner.Name);
                continue;
  • Files reviewed: 9/9 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@mitchdenny
Mitch Denny (mitchdenny) merged commit bba9d7f into main Aug 11, 2026
677 of 680 checks passed
@mitchdenny
Mitch Denny (mitchdenny) deleted the mitchdenny-fix-skipped-gateway-fqdn-wait branch August 11, 2026 22:44
@mitchdenny

Copy link
Copy Markdown
Member Author

/backport to release/13.2

@microsoft-github-policy-service microsoft-github-policy-service Bot added this to the 13.6 milestone Aug 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/13.2 (link to workflow run)

@aspire-repo-bot

Copy link
Copy Markdown
Contributor

Mitch Denny (@mitchdenny) backporting to release/13.2 failed, the patch most likely resulted in conflicts. Please backport manually!

git am output
$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Don't select skipped Gateways and Ingresses for TLS work
Using index info to reconstruct a base tree...
A	src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs
M	src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs
A	src/Aspire.Hosting.Kubernetes/KubernetesGatewayResource.cs
A	src/Aspire.Hosting.Kubernetes/KubernetesIngressResource.cs
A	tests/Aspire.Hosting.Kubernetes.Tests/CertManagerTests.cs
A	tests/Aspire.Hosting.Kubernetes.Tests/KubernetesGatewayTests.cs
A	tests/Aspire.Hosting.Kubernetes.Tests/KubernetesIngressTests.cs
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs deleted in HEAD and modified in Don't select skipped Gateways and Ingresses for TLS work.  Version Don't select skipped Gateways and Ingresses for TLS work of src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs left in tree.
Auto-merging src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs
CONFLICT (content): Merge conflict in src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs
CONFLICT (modify/delete): src/Aspire.Hosting.Kubernetes/KubernetesGatewayResource.cs deleted in HEAD and modified in Don't select skipped Gateways and Ingresses for TLS work.  Version Don't select skipped Gateways and Ingresses for TLS work of src/Aspire.Hosting.Kubernetes/KubernetesGatewayResource.cs left in tree.
CONFLICT (modify/delete): src/Aspire.Hosting.Kubernetes/KubernetesIngressResource.cs deleted in HEAD and modified in Don't select skipped Gateways and Ingresses for TLS work.  Version Don't select skipped Gateways and Ingresses for TLS work of src/Aspire.Hosting.Kubernetes/KubernetesIngressResource.cs left in tree.
CONFLICT (modify/delete): tests/Aspire.Hosting.Kubernetes.Tests/CertManagerTests.cs deleted in HEAD and modified in Don't select skipped Gateways and Ingresses for TLS work.  Version Don't select skipped Gateways and Ingresses for TLS work of tests/Aspire.Hosting.Kubernetes.Tests/CertManagerTests.cs left in tree.
CONFLICT (modify/delete): tests/Aspire.Hosting.Kubernetes.Tests/KubernetesGatewayTests.cs deleted in HEAD and modified in Don't select skipped Gateways and Ingresses for TLS work.  Version Don't select skipped Gateways and Ingresses for TLS work of tests/Aspire.Hosting.Kubernetes.Tests/KubernetesGatewayTests.cs left in tree.
CONFLICT (modify/delete): tests/Aspire.Hosting.Kubernetes.Tests/KubernetesIngressTests.cs deleted in HEAD and modified in Don't select skipped Gateways and Ingresses for TLS work.  Version Don't select skipped Gateways and Ingresses for TLS work of tests/Aspire.Hosting.Kubernetes.Tests/KubernetesIngressTests.cs left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Don't select skipped Gateways and Ingresses for TLS work
Error: The process '/usr/bin/git' failed with exit code 128

Link to workflow output

aspire-repo-bot Bot added a commit to microsoft/aspire.dev that referenced this pull request Aug 11, 2026
Documents the warnings emitted when a route-less Gateway or path-less
Ingress is skipped during deployment, per microsoft/aspire#19220.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@aspire-repo-bot

Copy link
Copy Markdown
Contributor

Pull request created: #1470

Generated by PR Documentation Check · auto · 63.3 AIC · ⌖ 15.1 AIC · ⊞ 19.6K

@aspire-repo-bot

Copy link
Copy Markdown
Contributor

📝 Documentation has been drafted in microsoft/aspire.dev#1470 targeting release/13.5.

Added troubleshooting guidance to two existing AKS deployment pages covering the new warnings introduced by this fix.

  • kubernetes-gateway-aks.mdx: new "Gateway has no routes configured" subsection explaining the route-less Gateway warning and the ClusterIssuer/HTTP-01 solver warning, with the fix (add WithRoute(...)).
  • kubernetes-ingress-aks.mdx: new "Ingress has no paths configured" subsection explaining that a path-less/backend-less Ingress is skipped.

Note

This draft PR needs human review before merging.

@mitchdenny

Copy link
Copy Markdown
Member Author

/backport to release/13.5

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/13.5 (link to workflow run)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TLS FQDN discovery waits 15 minutes for skipped route-less Gateway

3 participants