Require external endpoint for Kubernetes ingress/gateway routes#17232
Merged
Conversation
Throw InvalidOperationException at publish time when an ingress or gateway
route references an endpoint that was not flagged as external (via
WithExternalHttpEndpoints or isExternal: true). Validation runs inside
KubernetesEnvironmentResource.Process{Ingress,Gateway}Resources so it
covers both the standard Kubernetes hosting and the AKS wrappers, and
mirrors the existing Front Door check.
Updates existing unit and deployment E2E tests whose routed endpoints
were not previously marked external, and adds focused unit tests +
a CLI E2E test that asserts aspire publish fails with the guidance
message pointing at WithExternalHttpEndpoints.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17232Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17232" |
mitchdenny
commented
May 19, 2026
Member
Author
mitchdenny
left a comment
mitchdenny
left a comment
There was a problem hiding this comment.
Two issues found:
- Test regression in
WithLoadBalancer_RespectsExplicitGatewayClass(deleted assertions). - Wrong pragma diagnostic ID in the new CLI E2E test.
- Restore the two AGC annotation assertions in WithLoadBalancer_RespectsExplicitGatewayClass that were accidentally dropped when reformatting the test for the new validation cases. - Remove the misleading #pragma warning disable ASPIREHOSTINGAZURE001 blocks from the new CLI E2E test. None of the K8s extension APIs the embedded AppHost calls carries that diagnostic, so the pragma was silently ignored and only added noise. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The EmptyAppHost template lays out the project as
`{ProjectName}/apphost.cs` (file-based app), not
`{ProjectName}/{ProjectName}.AppHost/AppHost.cs`, which caused the
test to fail with DirectoryNotFoundException when reading the AppHost
to mutate it. Switch to the Starter template so the mutation path
matches KubernetesPublishTests.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
/deployment-test |
Contributor
|
🚀 Deployment tests starting on PR #17232... This will deploy to real Azure infrastructure. Results will be posted here when complete. |
Contributor
|
❌ Deployment E2E Tests failed — 36 passed, 2 failed, 0 cancelled View test results and recordings
|
mitchdenny
commented
May 19, 2026
Member
Author
mitchdenny
left a comment
mitchdenny
left a comment
There was a problem hiding this comment.
One stale-comment nit found in the latest changes.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds publish-time validation requiring that endpoints routed via Kubernetes AddIngress/AddGateway (and the inherited AKS path) be explicitly marked external. Today these APIs route any endpoint, which is a privacy/security footgun since ingress/gateway expose backing services to outside-cluster traffic. The new check throws a clear InvalidOperationException pointing users at WithExternalHttpEndpoints() before any Helm output is generated.
Changes:
- New internal helper
EndpointRoutingValidation.ThrowIfEndpointNotExternalinvoked fromKubernetesEnvironmentResource.ProcessIngressResources/ProcessGatewayResources, covering routes and ingressDefaultBackend. - Unit tests added for K8s + AKS positive/negative paths; existing K8s/AKS/deployment E2E tests updated to opt into
WithExternalHttpEndpoints()for routed endpoints. - New CLI E2E test scaffolds an EmptyAppHost, wires a non-external ingress/gateway, and asserts
aspire publish --non-interactivefails with the guidance text.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Hosting.Kubernetes/Extensions/EndpointRoutingValidation.cs | New internal validation helper that throws if an endpoint is not external. |
| src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs | Calls validation in ingress and gateway processing prior to materializing objects. |
| tests/Aspire.Hosting.Kubernetes.Tests/KubernetesIngressTests.cs | Adds positive/negative ingress tests; updates existing tests to call WithExternalHttpEndpoints(). |
| tests/Aspire.Hosting.Kubernetes.Tests/KubernetesGatewayTests.cs | Adds positive/negative gateway tests; updates existing tests similarly. |
| tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesIngressTests.cs | Adds AKS ingress/gateway negative tests and updates the existing positive test. |
| tests/Aspire.Deployment.EndToEnd.Tests/KubernetesGatewayTlsDeploymentTests.cs | Marks the webfrontend endpoint external to satisfy new validation. |
| tests/Aspire.Deployment.EndToEnd.Tests/AksAzureKubernetesEnvironmentGatewayDeploymentTests.cs | Marks apiService external in the AKS gateway E2E. |
| tests/Aspire.Deployment.EndToEnd.Tests/AksAzureKubernetesEnvironmentCertManagerDeploymentTests.cs | Marks apiService external in the cert-manager AKS E2E. |
| tests/Aspire.Cli.EndToEnd.Tests/KubernetesPublishRequiresExternalEndpointTests.cs | New CLI E2E test exercising the full aspire publish failure path with guidance text assertions. |
Copilot's findings
- Files reviewed: 9/9 changed files
- Comments generated: 0
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JamesNK
approved these changes
May 19, 2026
Member
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
Looks good — clean validation with clear error messages, comprehensive test coverage, and correct placement at publish time. One minor nit on workspace disposal in the E2E test.
Match the established pattern in the sibling KubernetesPublishTests so the temp directory is cleaned up after the test runs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update the aspire-deployment skill's Kubernetes reference to note that endpoints routed via AddIngress/AddGateway must be explicitly marked external, and that publish fails fast otherwise. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
❓ CLI E2E Tests unknown — 90 passed, 0 failed, 2 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #26076445443 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Today the Kubernetes
AddIngress/AddGatewayAPIs will happily route any endpoint passed toWithRoute(...)/WithDefaultBackend(...), even one that the resource owner never marked external. That is a privacy / security footgun: an ingress or gateway exposes its backing service to traffic from outside the cluster, so the routed endpoint should have been an explicit opt-in viaWithExternalHttpEndpoints()(orisExternal: true). Front Door already enforces this; the K8s and AKS paths did not.This change adds that check on the publish path. A new internal helper
EndpointRoutingValidation.ThrowIfEndpointNotExternalis called fromKubernetesEnvironmentResource.ProcessIngressResources/ProcessGatewayResourcesduring model materialization, so the user gets a clearInvalidOperationException(with the resource name, endpoint name, ingress/gateway name, and a pointer atWithExternalHttpEndpoints()) before any Helm output is generated. AKS is covered automatically becauseAddAzureKubernetesEnvironmentdelegates ingress/gateway routing to the same code path.Validation runs at publish time rather than at
WithRoute(...)call time on purpose: authoring order is not significant. Users may legitimately register a route and then callWithExternalHttpEndpoints()afterwards, so we want to evaluate intent once the model is finalized.Tests
WithRoute,WithHostRoute, andWithDefaultBackend.WithExternalHttpEndpoints()on the target resource (Polyglot Go AppHost needed no change because it does not callWithRoute).KubernetesPublishRequiresExternalEndpointTestsscaffolds an EmptyAppHost, wires an ingress (and a gateway, in a sibling fact) at a non-external endpoint, runsaspire publish --non-interactive, and asserts the CLI exits non-zero with the guidance text on screen.Notes for reviewers
AggregateExceptionat theapp.Run()boundary. The new unit tests flatten and assert on theInvalidOperationExceptionwhose message containsWithExternalHttpEndpoints.GetOriginEndpointcheck on the same shared helper is intentionally out of scope for this PR; the K8s path is the immediate gap.Fixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?