-
Notifications
You must be signed in to change notification settings - Fork 885
Require external endpoint for Kubernetes ingress/gateway routes #17232
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
5f5c748
Require external endpoint for Kubernetes ingress/gateway routes
4795698
Address PR review feedback
a56bb56
Fix CLI E2E test to use Starter template for AppHost layout
6048d7c
Update stale EmptyAppHost comment to reference Starter template
1cde3b2
Use 'using' for TemporaryWorkspace in K8s publish E2E test
5d58570
Document external-endpoint requirement in deployment skill
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
57 changes: 57 additions & 0 deletions
57
src/Aspire.Hosting.Kubernetes/Extensions/EndpointRoutingValidation.cs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using Aspire.Hosting.ApplicationModel; | ||
|
|
||
| namespace Aspire.Hosting.Kubernetes.Extensions; | ||
|
|
||
| /// <summary> | ||
| /// Helpers that validate publish-time intent for endpoints that are being | ||
| /// routed by ingress / gateway-style resources. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Ingress and Gateway resources expose their backing service to traffic that | ||
| /// originates outside the cluster, so it is a privacy/security footgun to | ||
| /// route an <see cref="EndpointReference"/> that the resource owner never | ||
| /// flagged as external. The check is performed during publish (when the | ||
| /// Helm chart is materialized) rather than at <c>WithRoute</c> call time | ||
| /// because authoring order is not significant: a user may legitimately | ||
| /// register the route before calling | ||
| /// <see cref="ResourceBuilderExtensions.WithExternalHttpEndpoints{T}"/> or | ||
| /// setting <see cref="EndpointAnnotation.IsExternal"/> directly. | ||
| /// </remarks> | ||
| internal static class EndpointRoutingValidation | ||
| { | ||
| /// <summary> | ||
| /// Throws an <see cref="InvalidOperationException"/> when the supplied | ||
| /// endpoint is not marked external on its <see cref="EndpointAnnotation"/>. | ||
| /// </summary> | ||
| /// <param name="endpoint">The routed endpoint reference.</param> | ||
| /// <param name="routingResourceKind">The kind of routing resource (e.g., <c>"Ingress"</c> or <c>"Gateway"</c>).</param> | ||
| /// <param name="routingResourceName">The name of the routing resource that owns the route.</param> | ||
| public static void ThrowIfEndpointNotExternal( | ||
| EndpointReference endpoint, | ||
| string routingResourceKind, | ||
| string routingResourceName) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(endpoint); | ||
| ArgumentException.ThrowIfNullOrEmpty(routingResourceKind); | ||
| ArgumentException.ThrowIfNullOrEmpty(routingResourceName); | ||
|
|
||
| // EndpointAnnotation captures publish-time intent — the endpoint | ||
| // is external if and only if the author explicitly opted in, either | ||
| // through .WithExternalHttpEndpoints() or by passing isExternal: true | ||
| // when creating the endpoint annotation. | ||
| if (endpoint.EndpointAnnotation.IsExternal) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| throw new InvalidOperationException( | ||
| $"Resource '{endpoint.Resource.Name}' endpoint '{endpoint.EndpointName}' is not marked as external " + | ||
| $"but is being routed by {routingResourceKind} '{routingResourceName}'. " + | ||
| $"Call .WithExternalHttpEndpoints() on the target resource or pass isExternal: true when " + | ||
| $"creating the endpoint annotation. {routingResourceKind} routes may only expose endpoints " + | ||
| $"that are explicitly marked external."); | ||
| } | ||
| } |
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
159 changes: 159 additions & 0 deletions
159
tests/Aspire.Cli.EndToEnd.Tests/KubernetesPublishRequiresExternalEndpointTests.cs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Globalization; | ||
| using Aspire.Cli.EndToEnd.Tests.Helpers; | ||
| using Aspire.Cli.Tests.Utils; | ||
| using Hex1b.Automation; | ||
| using Xunit; | ||
|
|
||
| namespace Aspire.Cli.EndToEnd.Tests; | ||
|
|
||
| /// <summary> | ||
| /// End-to-end coverage for the Kubernetes ingress/gateway validation that | ||
| /// requires routed endpoints to be marked external (see | ||
| /// <c>EndpointRoutingValidation.ThrowIfEndpointNotExternal</c>). The CLI | ||
| /// surface check matters because the validation throws during model | ||
| /// materialization on the <c>aspire publish</c> path and we want a regression | ||
| /// guard that exercises the full publish pipeline, not just the unit-level | ||
| /// helper. | ||
| /// </summary> | ||
| public sealed class KubernetesPublishRequiresExternalEndpointTests(ITestOutputHelper output) | ||
| { | ||
| private const string ProjectName = "AspireK8sExternalCheck"; | ||
|
|
||
| [Fact] | ||
| [CaptureWorkspaceOnFailure] | ||
| public async Task IngressWithoutExternalEndpoint_FailsPublishWithGuidance() | ||
| { | ||
| await RunPublishFailureScenarioAsync( | ||
| // Wire an ingress that routes a non-external HTTP endpoint. The | ||
| // publish-time validation in EndpointRoutingValidation should | ||
| // throw before any Helm output is generated. | ||
| appHostBodyExtension: """ | ||
| #pragma warning disable ASPIREHOSTINGAZURE001 | ||
| var kube = builder.AddKubernetesEnvironment("kube"); | ||
| var api = builder.AddContainer("api", "nginx").WithHttpEndpoint(targetPort: 80); | ||
| kube.AddIngress("public").WithRoute("/", api.GetEndpoint("http")); | ||
| #pragma warning restore ASPIREHOSTINGAZURE001 | ||
| """); | ||
| } | ||
|
|
||
| [Fact] | ||
| [CaptureWorkspaceOnFailure] | ||
| public async Task GatewayWithoutExternalEndpoint_FailsPublishWithGuidance() | ||
| { | ||
| await RunPublishFailureScenarioAsync( | ||
| appHostBodyExtension: """ | ||
| #pragma warning disable ASPIREHOSTINGAZURE001 | ||
| var kube = builder.AddKubernetesEnvironment("kube"); | ||
| var api = builder.AddContainer("api", "nginx").WithHttpEndpoint(targetPort: 80); | ||
| kube.AddGateway("public").WithRoute("/", api.GetEndpoint("http")); | ||
| #pragma warning restore ASPIREHOSTINGAZURE001 | ||
| """); | ||
| } | ||
|
|
||
| private async Task RunPublishFailureScenarioAsync(string appHostBodyExtension) | ||
| { | ||
| var repoRoot = CliE2ETestHelpers.GetRepoRoot(); | ||
| var strategy = CliInstallStrategy.Detect(output.WriteLine); | ||
| var workspace = TemporaryWorkspace.Create(output); | ||
|
|
||
| using var terminal = CliE2ETestHelpers.CreateDockerTestTerminal(repoRoot, strategy, output, workspace: workspace); | ||
|
|
||
| var pendingRun = terminal.RunAsync(TestContext.Current.CancellationToken); | ||
|
mitchdenny marked this conversation as resolved.
|
||
|
|
||
| var counter = new SequenceCounter(); | ||
| var auto = new Hex1bTerminalAutomator(terminal, defaultTimeout: TimeSpan.FromSeconds(500)); | ||
| var testBodyFailed = false; | ||
|
|
||
| try | ||
| { | ||
| await auto.PrepareDockerEnvironmentAsync(counter, workspace); | ||
| await auto.InstallAspireCliAsync(strategy, counter); | ||
|
|
||
| // EmptyAppHost gives us a minimal csproj-based AppHost we can mutate. | ||
| await auto.AspireNewAsync(ProjectName, counter, template: AspireTemplate.EmptyAppHost); | ||
|
|
||
| // cd into the project so subsequent `aspire add` and `aspire publish` | ||
| // commands resolve the AppHost via repo-root discovery. | ||
| await auto.TypeAsync($"cd {ProjectName}"); | ||
| await auto.EnterAsync(); | ||
| await auto.WaitForSuccessPromptAsync(counter); | ||
|
|
||
| // The Kubernetes hosting package is required to compile the AppHost code | ||
| // we're about to write. `aspire add` resolves the version against the | ||
| // same feed configuration the rest of the CLI uses (including PR builds). | ||
| await auto.TypeAsync("aspire add Aspire.Hosting.Kubernetes"); | ||
| await auto.EnterAsync(); | ||
| await auto.WaitForAspireAddCompletionAsync(counter, TimeSpan.FromSeconds(180)); | ||
|
|
||
| // Patch AppHost.cs in-place. The EmptyAppHost template emits a single | ||
| // `builder.Build().Run();` line we replace; failing to find it should | ||
| // surface as a clear test failure rather than a silently no-op publish. | ||
|
mitchdenny marked this conversation as resolved.
Outdated
|
||
| var projectDir = Path.Combine(workspace.WorkspaceRoot.FullName, ProjectName); | ||
| var appHostDir = Path.Combine(projectDir, $"{ProjectName}.AppHost"); | ||
| var appHostFilePath = Path.Combine(appHostDir, "AppHost.cs"); | ||
| var content = File.ReadAllText(appHostFilePath); | ||
| const string buildRunPattern = "builder.Build().Run();"; | ||
| Assert.Contains(buildRunPattern, content); | ||
| content = content.Replace(buildRunPattern, appHostBodyExtension + Environment.NewLine + buildRunPattern); | ||
| File.WriteAllText(appHostFilePath, content); | ||
|
|
||
| // ASPIRE_PLAYGROUND interferes with `--non-interactive`. See | ||
| // KubernetesPublishTests for full context. | ||
| await auto.TypeAsync("unset ASPIRE_PLAYGROUND"); | ||
| await auto.EnterAsync(); | ||
| await auto.WaitForSuccessPromptAsync(counter); | ||
|
|
||
| // Drive aspire publish. The validation throws an InvalidOperationException | ||
| // during model materialization, so publish should exit with a non-zero code | ||
| // and surface our guidance message verbatim in stderr/stdout. | ||
| await auto.TypeAsync("aspire publish -o helm-output --non-interactive"); | ||
| await auto.EnterAsync(); | ||
|
|
||
| var expectedCounter = counter.Value; | ||
| // We don't pin to a specific exit code — the publish pipeline currently | ||
| // surfaces validation failures as exit 1, but treating any non-zero | ||
| // ERR:* prompt as the success condition keeps this test stable across | ||
| // future exit-code refactors. | ||
| var errorPromptSearcher = new CellPatternSearcher() | ||
| .FindPattern(expectedCounter.ToString(CultureInfo.InvariantCulture)) | ||
| .RightText(" ERR:"); | ||
|
|
||
| await auto.WaitUntilAsync( | ||
| snapshot => errorPromptSearcher.Search(snapshot).Count > 0, | ||
| TimeSpan.FromMinutes(5), | ||
| description: "waiting for aspire publish to fail"); | ||
| counter.Increment(); | ||
|
|
||
| // After the publish exits, scrape the screen for the guidance fragments. | ||
| // We use a generous WaitUntilTextAsync so any in-progress rendering | ||
| // settles before we assert. | ||
| await auto.WaitUntilTextAsync("WithExternalHttpEndpoints", timeout: TimeSpan.FromSeconds(30)); | ||
| await auto.WaitUntilTextAsync("'api'", timeout: TimeSpan.FromSeconds(30)); | ||
| await auto.WaitUntilTextAsync("'public'", timeout: TimeSpan.FromSeconds(30)); | ||
| } | ||
| catch | ||
| { | ||
| testBodyFailed = true; | ||
| throw; | ||
| } | ||
| finally | ||
| { | ||
| try | ||
| { | ||
| await auto.TypeAsync("exit"); | ||
| await auto.EnterAsync(); | ||
| await pendingRun; | ||
| } | ||
| catch | ||
| { | ||
| if (!testBodyFailed) | ||
| { | ||
| throw; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.