From 13518c88c027eabf46cb653a37aa4222e48efdd8 Mon Sep 17 00:00:00 2001 From: Arun Gopinathan Date: Mon, 31 Aug 2026 16:04:00 -0500 Subject: [PATCH 1/4] Send auth on cluster recommendations and surface fallbacks GetClusterRecommendationsAsync hardcoded authHeader: null, so the call was always anonymous even when the same client authenticated the create that followed it. The service therefore could not identify the caller, and every SDK caller resolved to the default service tier regardless of configuration. Pass the token from the existing userTokenCallback, which is what every other request already uses. The service rejects an invalid or expired token before the controller runs, so it does not degrade to treating the caller as anonymous - it returns 401. A rejected token is now retried once without the header, so a token problem falls back to the previous anonymous behaviour instead of losing recommendations entirely. Clients with no token are unaffected: no header is sent and no retry is attempted. CreateTunnelAsync caught every recommendation failure with an empty block. The create still succeeds via global routing, so a caller had no way to tell that recommendation-based placement had stopped working - it looks identical to normal operation while silently reverting to nearest-by-latency selection. Failures are now classified and reported two ways: a ClusterSelected event for embedders, and an X-Tunnel-Cluster-Source header on the create so the path is visible in service telemetry rather than only on the client. The header requires a service-side change to be logged before it is observable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 09709668-833e-4c9a-bea4-fdb93f91b8e5 --- .../TunnelClusterSelectionEventArgs.cs | 114 ++++++++ cs/src/Management/TunnelManagementClient.cs | 116 ++++++-- .../TunnelManagementClientTests.cs | 250 ++++++++++++++++++ 3 files changed, 465 insertions(+), 15 deletions(-) create mode 100644 cs/src/Management/TunnelClusterSelectionEventArgs.cs diff --git a/cs/src/Management/TunnelClusterSelectionEventArgs.cs b/cs/src/Management/TunnelClusterSelectionEventArgs.cs new file mode 100644 index 00000000..16bbc028 --- /dev/null +++ b/cs/src/Management/TunnelClusterSelectionEventArgs.cs @@ -0,0 +1,114 @@ +// +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. +// + +using System; + +namespace Microsoft.DevTunnels.Management +{ + /// + /// How the cluster for a tunnel create request was chosen. + /// + /// + /// When a create request does not specify a cluster, the client asks the recommendations + /// API which cluster to use. That call can fail, and when it does the client falls back to + /// global (Traffic Manager) routing, which still works but picks the nearest cluster by + /// latency rather than the recommended one. The fallback is therefore invisible to the + /// caller, so this value records which path was actually taken. + /// + public enum TunnelClusterSource + { + /// + /// The caller specified the cluster, so no recommendation was requested. + /// + Explicit, + + /// + /// The recommendations API was called and its cluster was used. + /// + Recommended, + + /// + /// The recommendations API rejected the caller's token, and the retry without a token + /// succeeded. Routing is correct but the caller was not identified, so it is treated as + /// anonymous and cannot be assigned a service tier. This indicates a token problem on + /// the caller's side that would otherwise be invisible. + /// + RecommendedAfterAuthRejected, + + /// + /// The recommendations API returned unauthorized even without a token, so global + /// routing was used instead. + /// + FallbackAuthFailed, + + /// + /// The recommendations API returned no cluster, so global routing was used instead. + /// + FallbackEmpty, + + /// + /// The recommendations API call failed, so global routing was used instead. + /// + FallbackError, + } + + /// + /// Event args reporting how the cluster for a tunnel create request was chosen. + /// + public class TunnelClusterSelectionEventArgs : EventArgs + { + /// + /// Creates a new instance of the class. + /// + public TunnelClusterSelectionEventArgs( + TunnelClusterSource source, + string? clusterId = null, + Exception? exception = null) + { + this.Source = source; + this.ClusterId = clusterId; + this.Exception = exception; + } + + /// + /// Gets how the cluster was chosen. + /// + public TunnelClusterSource Source { get; } + + /// + /// Gets the cluster that was selected, if one was. + /// + public string? ClusterId { get; } + + /// + /// Gets the failure that caused a fallback, if there was one. + /// + public Exception? Exception { get; } + + /// + /// Gets a value indicating whether the recommendations API was bypassed or failed, so + /// the tunnel was placed by global routing rather than by recommendation. + /// + public bool IsFallback => + this.Source == TunnelClusterSource.FallbackAuthFailed || + this.Source == TunnelClusterSource.FallbackEmpty || + this.Source == TunnelClusterSource.FallbackError; + + /// + /// Converts a to the stable wire value sent to the + /// service, which is what makes the client-side path visible in service telemetry. + /// + public static string ToHeaderValue(TunnelClusterSource source) => source switch + { + TunnelClusterSource.Explicit => "explicit", + TunnelClusterSource.Recommended => "recommended", + TunnelClusterSource.RecommendedAfterAuthRejected => "recommended-after-auth-rejected", + TunnelClusterSource.FallbackAuthFailed => "fallback-auth-failed", + TunnelClusterSource.FallbackEmpty => "fallback-empty", + TunnelClusterSource.FallbackError => "fallback-error", + _ => "unknown", + }; + } +} diff --git a/cs/src/Management/TunnelManagementClient.cs b/cs/src/Management/TunnelManagementClient.cs index 8ce2694f..b3540e03 100644 --- a/cs/src/Management/TunnelManagementClient.cs +++ b/cs/src/Management/TunnelManagementClient.cs @@ -47,6 +47,7 @@ public class TunnelManagementClient : ITunnelManagementClient private const string RequestIdHeaderName = "VsSaaS-Request-Id"; private const string CheckAvailableSubPath = ":checkNameAvailability"; private const string EnterprisePolicyFailureHeaderName = "X-Enterprise-Policy-Failure"; + private const string ClusterSourceHeaderName = "X-Tunnel-Cluster-Source"; private const int CreateNameRetries = 3; private static readonly string[] ManageAccessTokenScope = @@ -80,6 +81,18 @@ public class TunnelManagementClient : ITunnelManagementClient /// public event EventHandler? ReportProgress; + /// + /// Event raised when a tunnel create request selected a cluster via the recommendations + /// API, reporting which path was taken. + /// + /// + /// Subscribe to surface recommendation failures. When the recommendations call fails the + /// create still succeeds via global routing, so without this event a caller has no way to + /// tell that recommendation-based placement stopped working. Raised only when the caller + /// did not specify a cluster. + /// + public event EventHandler? ClusterSelected; + /// /// ApiVersion that will be used if one is not specified /// @@ -1088,29 +1101,59 @@ public async Task CreateTunnelAsync( // If the caller didn't specify a cluster, auto-select one via the // recommendations API. Failures fall back to global routing. + var clusterSource = TunnelClusterSource.Explicit; if (string.IsNullOrEmpty(tunnel.ClusterId)) { + Exception? recommendationError = null; try { - var recommendations = await GetClusterRecommendationsAsync( - preferredClusterId: null, - requiredGeo: options.RequiredGeo, - cancellation); + var (recommendations, authRejected) = + await GetClusterRecommendationsInternalAsync( + preferredClusterId: null, + requiredGeo: options.RequiredGeo, + cancellation); if (!string.IsNullOrEmpty(recommendations?.RecommendedClusterId)) { tunnel.ClusterId = recommendations!.RecommendedClusterId; + clusterSource = authRejected + ? TunnelClusterSource.RecommendedAfterAuthRejected + : TunnelClusterSource.Recommended; + } + else + { + clusterSource = TunnelClusterSource.FallbackEmpty; } } - catch (Exception) when (!cancellation.IsCancellationRequested) + catch (Exception ex) when (!cancellation.IsCancellationRequested) { // Fall through to global (Traffic Manager) routing if the - // recommendations request fails for any reason. + // recommendations request fails for any reason. The failure is reported + // rather than swallowed, because global routing still succeeds and the + // caller would otherwise have no indication that the cluster it asked for + // was not the cluster it got. + recommendationError = ex; + clusterSource = ex is UnauthorizedAccessException + ? TunnelClusterSource.FallbackAuthFailed + : TunnelClusterSource.FallbackError; } + + this.ClusterSelected?.Invoke( + this, + new TunnelClusterSelectionEventArgs( + clusterSource, tunnel.ClusterId, recommendationError)); } options.AdditionalHeaders ??= new List>(); options.AdditionalHeaders = options.AdditionalHeaders.Append( new KeyValuePair("If-None-Match", "*")); + + // Report the client-side selection path to the service. Recommendation fallbacks + // are otherwise invisible in service telemetry: a create that fell back looks + // identical to one that was never recommended at all. + options.AdditionalHeaders = options.AdditionalHeaders.Append( + new KeyValuePair( + ClusterSourceHeaderName, + TunnelClusterSelectionEventArgs.ToHeaderValue(clusterSource))); var tunnelId = tunnel.TunnelId; var idGenerated = string.IsNullOrEmpty(tunnelId); if (idGenerated) @@ -1624,6 +1667,57 @@ public async Task GetClusterRecommendationsAsync( string? preferredClusterId = null, string? requiredGeo = null, CancellationToken cancellation = default) + { + var (response, _) = await GetClusterRecommendationsInternalAsync( + preferredClusterId, requiredGeo, cancellation); + return response!; + } + + /// + /// Requests cluster recommendations, reporting whether the caller's token was rejected. + /// + /// + /// The token is sent so the service can identify the caller and apply its service tier. + /// If the token is rejected the request is retried without it, because the service + /// rejects a bad token before the controller runs and does not fall back to treating the + /// caller as anonymous. Without the retry, one expired token would silently disable + /// recommendation-based routing for that caller. + /// + private async Task<(ClusterRecommendationResponse? Response, bool AuthRejected)> + GetClusterRecommendationsInternalAsync( + string? preferredClusterId, + string? requiredGeo, + CancellationToken cancellation) + { + var uri = BuildClusterRecommendationsUri(preferredClusterId, requiredGeo); + + var authHeader = await this.userTokenCallback(); + if (authHeader == null) + { + // No token to offer, so this is an ordinary anonymous request rather than a + // rejected one. + var anonymous = await SendRequestAsync( + HttpMethod.Get, uri, options: null, authHeader: null, body: null, cancellation); + return (anonymous, false); + } + + try + { + var response = await SendRequestAsync( + HttpMethod.Get, uri, options: null, authHeader, body: null, cancellation); + return (response, false); + } + catch (UnauthorizedAccessException) when (!cancellation.IsCancellationRequested) + { + var response = await SendRequestAsync( + HttpMethod.Get, uri, options: null, authHeader: null, body: null, cancellation); + return (response, true); + } + } + + private Uri BuildClusterRecommendationsUri( + string? preferredClusterId, + string? requiredGeo) { var baseAddress = this.httpClient.BaseAddress!; var builder = new UriBuilder(baseAddress); @@ -1648,15 +1742,7 @@ public async Task GetClusterRecommendationsAsync( } builder.Query = string.Join("&", queryParts); - - var response = await SendRequestAsync( - HttpMethod.Get, - builder.Uri, - options: null, - authHeader: null, - body: null, - cancellation); - return response!; + return builder.Uri; } /// diff --git a/cs/test/TunnelsSDK.Test/TunnelManagementClientTests.cs b/cs/test/TunnelsSDK.Test/TunnelManagementClientTests.cs index bf7fdf4e..e1f7ee27 100644 --- a/cs/test/TunnelsSDK.Test/TunnelManagementClientTests.cs +++ b/cs/test/TunnelsSDK.Test/TunnelManagementClientTests.cs @@ -635,6 +635,256 @@ public async Task CreateTunnelAsync_FallsBackOnRecommendFailure() Assert.NotNull(resultTunnel); } + [Fact] + public async Task GetClusterRecommendationsAsync_SendsAuthTokenWhenAvailable() + { + AuthenticationHeaderValue capturedAuth = null; + var handler = new MockHttpMessageHandler( + (message, ct) => + { + capturedAuth = message.Headers.Authorization; + return Task.FromResult(RecommendationResponse(message, "usw4")); + }); + + var client = new TunnelManagementClient( + this.userAgent, + () => Task.FromResult(new AuthenticationHeaderValue("Bearer", "token1")), + this.tunnelServiceUri, + handler); + + await client.GetClusterRecommendationsAsync(cancellation: this.timeout); + + // Without this the service cannot identify the caller, so every caller resolves to + // the default tier no matter what tiers are configured. + Assert.NotNull(capturedAuth); + Assert.Equal("Bearer", capturedAuth.Scheme); + Assert.Equal("token1", capturedAuth.Parameter); + } + + [Fact] + public async Task GetClusterRecommendationsAsync_SendsNoAuthHeaderWhenNoToken() + { + var requests = 0; + var sawAuthHeader = false; + var handler = new MockHttpMessageHandler( + (message, ct) => + { + requests++; + sawAuthHeader |= message.Headers.Authorization != null; + return Task.FromResult(RecommendationResponse(message, "usw4")); + }); + + // No token callback, which is the default for an unauthenticated client. + var client = new TunnelManagementClient( + this.userAgent, null, this.tunnelServiceUri, handler); + + var response = await client.GetClusterRecommendationsAsync(cancellation: this.timeout); + + // The anonymous path must be unchanged: a header-less request still succeeds, and no + // retry is attempted because nothing was rejected. + Assert.False(sawAuthHeader); + Assert.Equal(1, requests); + Assert.Equal("usw4", response.RecommendedClusterId); + } + + [Fact] + public async Task GetClusterRecommendationsAsync_RetriesAnonymouslyWhenTokenRejected() + { + var authedAttempts = 0; + var anonymousAttempts = 0; + var handler = new MockHttpMessageHandler( + (message, ct) => + { + if (message.Headers.Authorization != null) + { + authedAttempts++; + return Task.FromResult(new HttpResponseMessage(HttpStatusCode.Unauthorized) + { + RequestMessage = message, + }); + } + + anonymousAttempts++; + return Task.FromResult(RecommendationResponse(message, "usw4")); + }); + + var client = new TunnelManagementClient( + this.userAgent, + () => Task.FromResult(new AuthenticationHeaderValue("Bearer", "expired")), + this.tunnelServiceUri, + handler); + + var response = await client.GetClusterRecommendationsAsync(cancellation: this.timeout); + + // The service rejects a bad token before the controller runs, so it never degrades to + // anonymous on its own. Without the retry one expired token silently disables + // recommendation-based routing and the caller falls back to Traffic Manager. + Assert.Equal(1, authedAttempts); + Assert.Equal(1, anonymousAttempts); + Assert.Equal("usw4", response.RecommendedClusterId); + } + + [Fact] + public async Task CreateTunnelAsync_ReportsRecommendedClusterSource() + { + var (client, events, headers) = CreateClientCapturingClusterSource( + (message, ct) => Task.FromResult(RecommendationResponse(message, "usw4")), + userToken: null); + + await client.CreateTunnelAsync( + new Tunnel { TunnelId = TunnelId }, options: null, this.timeout); + + var selection = Assert.Single(events); + Assert.Equal(TunnelClusterSource.Recommended, selection.Source); + Assert.Equal("usw4", selection.ClusterId); + Assert.False(selection.IsFallback); + Assert.Null(selection.Exception); + Assert.Equal("recommended", Assert.Single(headers)); + } + + [Fact] + public async Task CreateTunnelAsync_ReportsWhenTokenWasRejectedButRoutingRecovered() + { + var (client, events, headers) = CreateClientCapturingClusterSource( + (message, ct) => Task.FromResult( + message.Headers.Authorization != null + ? new HttpResponseMessage(HttpStatusCode.Unauthorized) { RequestMessage = message } + : RecommendationResponse(message, "usw4")), + userToken: "expired"); + + await client.CreateTunnelAsync( + new Tunnel { TunnelId = TunnelId }, options: null, this.timeout); + + // This is the case that is otherwise completely invisible: the tunnel lands on the + // right cluster, so nothing looks wrong, but the caller was not identified and cannot + // be assigned a tier. + var selection = Assert.Single(events); + Assert.Equal(TunnelClusterSource.RecommendedAfterAuthRejected, selection.Source); + Assert.Equal("usw4", selection.ClusterId); + Assert.False(selection.IsFallback); + Assert.Equal("recommended-after-auth-rejected", Assert.Single(headers)); + } + + [Fact] + public async Task CreateTunnelAsync_ReportsFallbackWhenRecommendationFails() + { + var (client, events, headers) = CreateClientCapturingClusterSource( + (message, ct) => Task.FromResult( + new HttpResponseMessage(HttpStatusCode.InternalServerError) + { + RequestMessage = message, + }), + userToken: null); + + await client.CreateTunnelAsync( + new Tunnel { TunnelId = TunnelId }, options: null, this.timeout); + + var selection = Assert.Single(events); + Assert.Equal(TunnelClusterSource.FallbackError, selection.Source); + Assert.True(selection.IsFallback); + Assert.NotNull(selection.Exception); + Assert.Equal("fallback-error", Assert.Single(headers)); + } + + [Fact] + public async Task CreateTunnelAsync_ReportsFallbackWhenRecommendationReturnsNoCluster() + { + var (client, events, headers) = CreateClientCapturingClusterSource( + (message, ct) => Task.FromResult(RecommendationResponse(message, clusterId: null)), + userToken: null); + + await client.CreateTunnelAsync( + new Tunnel { TunnelId = TunnelId }, options: null, this.timeout); + + var selection = Assert.Single(events); + Assert.Equal(TunnelClusterSource.FallbackEmpty, selection.Source); + Assert.True(selection.IsFallback); + Assert.Null(selection.Exception); + Assert.Equal("fallback-empty", Assert.Single(headers)); + } + + [Fact] + public async Task CreateTunnelAsync_ReportsExplicitSourceWithoutCallingRecommendations() + { + var recommendationCalls = 0; + var (client, events, headers) = CreateClientCapturingClusterSource( + (message, ct) => + { + recommendationCalls++; + return Task.FromResult(RecommendationResponse(message, "usw4")); + }, + userToken: null); + + await client.CreateTunnelAsync( + new Tunnel { TunnelId = TunnelId, ClusterId = ClusterId }, + options: null, + this.timeout); + + Assert.Equal(0, recommendationCalls); + Assert.Empty(events); + Assert.Equal("explicit", Assert.Single(headers)); + } + + private static HttpResponseMessage RecommendationResponse( + HttpRequestMessage message, string clusterId) + { + var json = clusterId == null + ? "{\"recommendations\":[]}" + : $"{{\"recommendedClusterId\":\"{clusterId}\",\"recommendations\":[]}}"; + return new HttpResponseMessage(HttpStatusCode.OK) + { + RequestMessage = message, + Content = new StringContent(json, System.Text.Encoding.UTF8, "application/json"), + }; + } + + /// + /// Builds a client whose recommendation responses are supplied by the caller, capturing + /// both the raised selection events and the cluster-source header sent on create. + /// + private (TunnelManagementClient Client, + List Events, + List Headers) CreateClientCapturingClusterSource( + Func> onRecommendation, + string userToken) + { + var headers = new List(); + var handler = new MockHttpMessageHandler( + async (message, ct) => + { + if (message.RequestUri!.AbsolutePath.EndsWith("/recommendations")) + { + return await onRecommendation(message, ct); + } + + if (message.Headers.TryGetValues("X-Tunnel-Cluster-Source", out var values)) + { + headers.AddRange(values); + } + + var sentTunnel = await message.Content!.ReadFromJsonAsync( + cancellationToken: ct); + return new HttpResponseMessage(HttpStatusCode.OK) + { + RequestMessage = message, + Content = JsonContent.Create( + new Tunnel { TunnelId = sentTunnel!.TunnelId }), + }; + }); + + var client = new TunnelManagementClient( + this.userAgent, + userToken == null + ? null + : () => Task.FromResult(new AuthenticationHeaderValue("Bearer", userToken)), + new Uri("https://global.rel.tunnels.api.visualstudio.com/"), + handler); + + var events = new List(); + client.ClusterSelected += (_, e) => events.Add(e); + return (client, events, headers); + } + private sealed class MockHttpMessageHandler : DelegatingHandler { private readonly Func> handler; From 345529504760dc1fa9351f3efc055aa4972b0cfa Mon Sep 17 00:00:00 2001 From: Arun Gopinathan Date: Mon, 31 Aug 2026 16:43:41 -0500 Subject: [PATCH 2/4] Send auth on cluster recommendations and report fallbacks (Go SDK) Mirrors the C# change in the Go SDK. The recommendations call was always anonymous, so the service could not identify the caller or apply their service tier. It now sends the caller's token. The service rejects a bad token before the controller runs rather than treating the caller as anonymous, so a rejected token is retried once without it. Without that retry a single expired token would silently disable recommendation-based routing for that caller. When the recommendations call fails, the create still succeeds via global routing, so the fallback was invisible. The selection path is now reported two ways: an X-Tunnel-Cluster-Source header, so it is visible in service telemetry without any cooperation from the embedder, and an optional OnClusterSelected callback for embedders that want to log it locally. The header values match the C# ones exactly. The header is passed explicitly to sendRequest because TunnelRequestOptions.AdditionalHeaders is written in four places but never read when the request is built; turning that on would also start sending If-Match and If-Not-Match, which is a separate behaviour change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 09709668-833e-4c9a-bea4-fdb93f91b8e5 --- go/tunnels/cluster_recommendations_test.go | 351 +++++++++++++++++++++ go/tunnels/cluster_selection.go | 61 ++++ go/tunnels/manager.go | 100 +++++- 3 files changed, 504 insertions(+), 8 deletions(-) create mode 100644 go/tunnels/cluster_recommendations_test.go create mode 100644 go/tunnels/cluster_selection.go diff --git a/go/tunnels/cluster_recommendations_test.go b/go/tunnels/cluster_recommendations_test.go new file mode 100644 index 00000000..fd6b50e5 --- /dev/null +++ b/go/tunnels/cluster_recommendations_test.go @@ -0,0 +1,351 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package tunnels + +import ( + "context" + "net/http" + "net/url" + "strings" + "testing" +) + +// recordedRequest captures what the client sent so tests can assert on auth and headers. +type recordedRequest struct { + path string + authorization string + clusterSource string +} + +// newRecordingManager returns a Manager whose transport is driven by the given handler and +// records every request it makes. A round-tripper is used rather than a real test server +// because the create path rewrites the request host to include the cluster ID, which no +// local server can serve. +func newRecordingManager( + t *testing.T, + token string, + handler func(r *http.Request) (*http.Response, error), +) (*Manager, *[]recordedRequest) { + t.Helper() + + serviceURL, err := url.Parse("https://example.test/") + if err != nil { + t.Fatalf("parsing url: %v", err) + } + + var requests []recordedRequest + client := &http.Client{Transport: roundTripperFunc(func(r *http.Request) (*http.Response, error) { + requests = append(requests, recordedRequest{ + path: r.URL.Path, + authorization: r.Header.Get("Authorization"), + clusterSource: r.Header.Get(clusterSourceHeaderName), + }) + return handler(r) + })} + + manager, err := NewManager( + userAgentManagerTest, func() string { return token }, serviceURL, client, "2023-09-27-preview") + if err != nil { + t.Fatalf("creating manager: %v", err) + } + + return manager, &requests +} + +func isRecommendationsRequest(r *http.Request) bool { + return strings.Contains(r.URL.Path, "recommendations") +} + +// createResponse returns a minimal created-tunnel response for the requested tunnel. +func createResponse(r *http.Request) (*http.Response, error) { + return responseWithStatus(http.StatusOK, + `{"tunnelId":"`+tunnelIDFromPath(r.URL.Path)+`"}`), nil +} + +// recommendationResponse returns a recommendations response naming the given cluster, or +// an empty recommendation when clusterID is empty. +func recommendationResponse(clusterID string) (*http.Response, error) { + if clusterID == "" { + return responseWithStatus(http.StatusOK, `{"recommendations":[]}`), nil + } + return responseWithStatus(http.StatusOK, + `{"recommendedClusterId":"`+clusterID+`","recommendations":[]}`), nil +} + +// lastClusterSource returns the cluster-source header from the final recorded request, +// which is the create itself. +func lastClusterSource(requests []recordedRequest) string { + if len(requests) == 0 { + return "" + } + return requests[len(requests)-1].clusterSource +} + +// The recommendations call must send the caller's token so the service can identify them +// and apply their service tier. Before this change it was always anonymous, so every +// caller looked untiered. +func TestGetClusterRecommendationsSendsAuthorizationHeader(t *testing.T) { + manager, requests := newRecordingManager(t, "Bearer test-token", func(r *http.Request) (*http.Response, error) { + return recommendationResponse("euw") + }) + + recommendations, err := manager.GetClusterRecommendations(context.Background(), "", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if recommendations.RecommendedClusterID != "euw" { + t.Errorf("got cluster %q, want %q", recommendations.RecommendedClusterID, "euw") + } + if len(*requests) != 1 { + t.Fatalf("got %d requests, want 1", len(*requests)) + } + if (*requests)[0].authorization != "Bearer test-token" { + t.Errorf("got Authorization %q, want %q", (*requests)[0].authorization, "Bearer test-token") + } +} + +// With no token configured the call must still work anonymously, and must not send an +// empty Authorization header, which the service would reject. +func TestGetClusterRecommendationsWithoutTokenSendsNoAuthorizationHeader(t *testing.T) { + manager, requests := newRecordingManager(t, "", func(r *http.Request) (*http.Response, error) { + return recommendationResponse("euw") + }) + + if _, err := manager.GetClusterRecommendations(context.Background(), "", ""); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(*requests) != 1 { + t.Fatalf("got %d requests, want 1", len(*requests)) + } + if (*requests)[0].authorization != "" { + t.Errorf("got Authorization %q, want none", (*requests)[0].authorization) + } +} + +// The service rejects a bad token before the controller runs, so it never falls back to +// treating the caller as anonymous. Without the retry, one expired token would silently +// disable recommendation-based routing for that caller. +func TestGetClusterRecommendationsRetriesAnonymouslyAfterUnauthorized(t *testing.T) { + manager, requests := newRecordingManager(t, "Bearer expired-token", func(r *http.Request) (*http.Response, error) { + if r.Header.Get("Authorization") != "" { + return responseWithStatus(http.StatusUnauthorized, `{"title":"Unauthorized"}`), nil + } + return recommendationResponse("euw") + }) + + recommendations, err := manager.GetClusterRecommendations(context.Background(), "", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if recommendations.RecommendedClusterID != "euw" { + t.Errorf("got cluster %q, want %q", recommendations.RecommendedClusterID, "euw") + } + if len(*requests) != 2 { + t.Fatalf("got %d requests, want 2 (authenticated then anonymous)", len(*requests)) + } + if (*requests)[1].authorization != "" { + t.Errorf("retry sent Authorization %q, want none", (*requests)[1].authorization) + } +} + +// A non-auth failure must not trigger the anonymous retry; retrying would not help and +// would double the latency of every failure. +func TestGetClusterRecommendationsDoesNotRetryOnServerError(t *testing.T) { + manager, requests := newRecordingManager(t, "Bearer test-token", func(r *http.Request) (*http.Response, error) { + return responseWithStatus(http.StatusInternalServerError, `{"title":"Server error"}`), nil + }) + + if _, err := manager.GetClusterRecommendations(context.Background(), "", ""); err == nil { + t.Fatal("expected an error") + } + if len(*requests) != 1 { + t.Errorf("got %d requests, want 1 (no retry)", len(*requests)) + } +} + +// When the caller specifies a cluster there is nothing to recommend, so no call is made +// and the create reports that the cluster was chosen explicitly. +func TestCreateTunnelWithExplicitClusterReportsExplicitSource(t *testing.T) { + var selections []ClusterSelection + manager, requests := newRecordingManager(t, "Bearer test-token", func(r *http.Request) (*http.Response, error) { + if isRecommendationsRequest(r) { + t.Error("recommendations should not be requested when a cluster is specified") + } + return createResponse(r) + }) + manager.OnClusterSelected = func(s ClusterSelection) { selections = append(selections, s) } + + if _, err := manager.CreateTunnel(context.Background(), &Tunnel{ClusterID: "euw"}, nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(selections) != 0 { + t.Errorf("got %d selection callbacks, want 0", len(selections)) + } + if got := lastClusterSource(*requests); got != string(ClusterSourceExplicit) { + t.Errorf("got cluster source header %q, want %q", got, ClusterSourceExplicit) + } +} + +// The happy path: the recommendation is used and reported as such. +func TestCreateTunnelReportsRecommendedCluster(t *testing.T) { + var selections []ClusterSelection + manager, requests := newRecordingManager(t, "Bearer test-token", func(r *http.Request) (*http.Response, error) { + if isRecommendationsRequest(r) { + return recommendationResponse("euw") + } + return createResponse(r) + }) + manager.OnClusterSelected = func(s ClusterSelection) { selections = append(selections, s) } + + if _, err := manager.CreateTunnel(context.Background(), &Tunnel{}, nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(selections) != 1 { + t.Fatalf("got %d selection callbacks, want 1", len(selections)) + } + if selections[0].Source != ClusterSourceRecommended { + t.Errorf("got source %q, want %q", selections[0].Source, ClusterSourceRecommended) + } + if selections[0].ClusterID != "euw" { + t.Errorf("got cluster %q, want %q", selections[0].ClusterID, "euw") + } + if got := lastClusterSource(*requests); got != string(ClusterSourceRecommended) { + t.Errorf("got cluster source header %q, want %q", got, ClusterSourceRecommended) + } +} + +// Routing is correct here but the caller was not identified, so they cannot be assigned a +// service tier. That is a token problem the caller would otherwise never learn about. +func TestCreateTunnelReportsRecommendationAfterAuthRejected(t *testing.T) { + var selections []ClusterSelection + manager, requests := newRecordingManager(t, "Bearer expired-token", func(r *http.Request) (*http.Response, error) { + if isRecommendationsRequest(r) { + if r.Header.Get("Authorization") != "" { + return responseWithStatus(http.StatusUnauthorized, `{"title":"Unauthorized"}`), nil + } + return recommendationResponse("euw") + } + return createResponse(r) + }) + manager.OnClusterSelected = func(s ClusterSelection) { selections = append(selections, s) } + + if _, err := manager.CreateTunnel(context.Background(), &Tunnel{}, nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(selections) != 1 { + t.Fatalf("got %d selection callbacks, want 1", len(selections)) + } + if selections[0].Source != ClusterSourceRecommendedAfterAuthRejected { + t.Errorf("got source %q, want %q", selections[0].Source, ClusterSourceRecommendedAfterAuthRejected) + } + if got := lastClusterSource(*requests); got != string(ClusterSourceRecommendedAfterAuthRejected) { + t.Errorf("got cluster source header %q, want %q", got, ClusterSourceRecommendedAfterAuthRejected) + } +} + +// The create still succeeds via global routing, so the fallback is invisible without the +// callback and the header. +func TestCreateTunnelReportsFallbackOnRecommendationError(t *testing.T) { + var selections []ClusterSelection + manager, requests := newRecordingManager(t, "Bearer test-token", func(r *http.Request) (*http.Response, error) { + if isRecommendationsRequest(r) { + return responseWithStatus(http.StatusInternalServerError, `{"title":"Server error"}`), nil + } + return createResponse(r) + }) + manager.OnClusterSelected = func(s ClusterSelection) { selections = append(selections, s) } + + tunnel, err := manager.CreateTunnel(context.Background(), &Tunnel{}, nil) + if err != nil { + t.Fatalf("create should still succeed via global routing: %v", err) + } + if tunnel == nil { + t.Fatal("expected a tunnel") + } + if len(selections) != 1 { + t.Fatalf("got %d selection callbacks, want 1", len(selections)) + } + if selections[0].Source != ClusterSourceFallbackError { + t.Errorf("got source %q, want %q", selections[0].Source, ClusterSourceFallbackError) + } + if selections[0].Err == nil { + t.Error("expected the underlying error to be reported") + } + if !selections[0].Source.IsFallback() { + t.Error("expected the source to be classified as a fallback") + } + if got := lastClusterSource(*requests); got != string(ClusterSourceFallbackError) { + t.Errorf("got cluster source header %q, want %q", got, ClusterSourceFallbackError) + } +} + +// An auth failure that survives the anonymous retry is distinguished from a generic +// failure, because it points at the service rejecting the request rather than being down. +func TestCreateTunnelReportsFallbackWhenAuthFailsEvenAnonymously(t *testing.T) { + var selections []ClusterSelection + manager, requests := newRecordingManager(t, "Bearer test-token", func(r *http.Request) (*http.Response, error) { + if isRecommendationsRequest(r) { + return responseWithStatus(http.StatusUnauthorized, `{"title":"Unauthorized"}`), nil + } + return createResponse(r) + }) + manager.OnClusterSelected = func(s ClusterSelection) { selections = append(selections, s) } + + if _, err := manager.CreateTunnel(context.Background(), &Tunnel{}, nil); err != nil { + t.Fatalf("create should still succeed via global routing: %v", err) + } + if len(selections) != 1 { + t.Fatalf("got %d selection callbacks, want 1", len(selections)) + } + if selections[0].Source != ClusterSourceFallbackAuthFailed { + t.Errorf("got source %q, want %q", selections[0].Source, ClusterSourceFallbackAuthFailed) + } + if got := lastClusterSource(*requests); got != string(ClusterSourceFallbackAuthFailed) { + t.Errorf("got cluster source header %q, want %q", got, ClusterSourceFallbackAuthFailed) + } +} + +// A successful call that recommends nothing is a distinct condition from a failure, and +// means the service had no cluster to offer. +func TestCreateTunnelReportsFallbackOnEmptyRecommendation(t *testing.T) { + var selections []ClusterSelection + manager, requests := newRecordingManager(t, "Bearer test-token", func(r *http.Request) (*http.Response, error) { + if isRecommendationsRequest(r) { + return recommendationResponse("") + } + return createResponse(r) + }) + manager.OnClusterSelected = func(s ClusterSelection) { selections = append(selections, s) } + + if _, err := manager.CreateTunnel(context.Background(), &Tunnel{}, nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(selections) != 1 { + t.Fatalf("got %d selection callbacks, want 1", len(selections)) + } + if selections[0].Source != ClusterSourceFallbackEmpty { + t.Errorf("got source %q, want %q", selections[0].Source, ClusterSourceFallbackEmpty) + } + if got := lastClusterSource(*requests); got != string(ClusterSourceFallbackEmpty) { + t.Errorf("got cluster source header %q, want %q", got, ClusterSourceFallbackEmpty) + } +} + +// The callback is optional, so a nil callback must not panic and the header must still be +// sent. +func TestCreateTunnelWithoutCallbackStillSendsHeader(t *testing.T) { + manager, requests := newRecordingManager(t, "Bearer test-token", func(r *http.Request) (*http.Response, error) { + if isRecommendationsRequest(r) { + return recommendationResponse("euw") + } + return createResponse(r) + }) + + if _, err := manager.CreateTunnel(context.Background(), &Tunnel{}, nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := lastClusterSource(*requests); got != string(ClusterSourceRecommended) { + t.Errorf("got cluster source header %q, want %q", got, ClusterSourceRecommended) + } +} diff --git a/go/tunnels/cluster_selection.go b/go/tunnels/cluster_selection.go new file mode 100644 index 00000000..aaf6aedb --- /dev/null +++ b/go/tunnels/cluster_selection.go @@ -0,0 +1,61 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package tunnels + +// ClusterSource describes how the cluster for a tunnel create request was chosen. +// +// When a create request does not specify a cluster, the client asks the recommendations +// API which cluster to use. That call can fail, and when it does the client falls back to +// global (Traffic Manager) routing, which still works but picks the nearest cluster by +// latency rather than the recommended one. The fallback is therefore invisible to the +// caller, so this value records which path was actually taken. +type ClusterSource string + +const ( + // ClusterSourceExplicit means the caller specified the cluster, so no recommendation + // was requested. + ClusterSourceExplicit ClusterSource = "explicit" + + // ClusterSourceRecommended means the recommendations API was called and its cluster + // was used. + ClusterSourceRecommended ClusterSource = "recommended" + + // ClusterSourceRecommendedAfterAuthRejected means the recommendations API rejected the + // caller's token and the retry without a token succeeded. Routing is correct but the + // caller was not identified, so it is treated as anonymous and cannot be assigned a + // service tier. This indicates a token problem that would otherwise be invisible. + ClusterSourceRecommendedAfterAuthRejected ClusterSource = "recommended-after-auth-rejected" + + // ClusterSourceFallbackAuthFailed means the recommendations API returned unauthorized + // even without a token, so global routing was used instead. + ClusterSourceFallbackAuthFailed ClusterSource = "fallback-auth-failed" + + // ClusterSourceFallbackEmpty means the recommendations API returned no cluster, so + // global routing was used instead. + ClusterSourceFallbackEmpty ClusterSource = "fallback-empty" + + // ClusterSourceFallbackError means the recommendations API call failed, so global + // routing was used instead. + ClusterSourceFallbackError ClusterSource = "fallback-error" +) + +// IsFallback reports whether the tunnel was placed by global routing rather than by +// recommendation. +func (s ClusterSource) IsFallback() bool { + return s == ClusterSourceFallbackAuthFailed || + s == ClusterSourceFallbackEmpty || + s == ClusterSourceFallbackError +} + +// ClusterSelection reports how the cluster for a tunnel create request was chosen. +type ClusterSelection struct { + // Source is how the cluster was chosen. + Source ClusterSource + + // ClusterID is the cluster that was selected, if one was. + ClusterID string + + // Err is the failure that caused a fallback, if there was one. + Err error +} diff --git a/go/tunnels/manager.go b/go/tunnels/manager.go index 808281df..263c3f6b 100644 --- a/go/tunnels/manager.go +++ b/go/tunnels/manager.go @@ -59,6 +59,7 @@ const ( endpointsApiSubPath = "/endpoints" portsApiSubPath = "/ports" tunnelAuthenticationScheme = "Tunnel" + clusterSourceHeaderName = "X-Tunnel-Cluster-Source" goUserAgent = "Dev-Tunnels-Service-Go-SDK/" + PackageVersion createNameRetries = 3 ) @@ -100,6 +101,15 @@ type Manager struct { userAgents []UserAgent apiVersion string isCustomDomain bool + + // OnClusterSelected is an optional callback invoked when a create request selected a + // cluster via the recommendations API, reporting which path was taken. + // + // Set it to surface recommendation failures. When the recommendations call fails the + // create still succeeds via global routing, so without this a caller has no way to tell + // that recommendation-based placement stopped working. Invoked only when the caller did + // not specify a cluster. + OnClusterSelected func(ClusterSelection) } // Creates a new Manager used for interacting with the Tunnels APIs. @@ -233,13 +243,46 @@ func (m *Manager) CreateTunnel(ctx context.Context, tunnel *Tunnel, options *Tun // If the caller didn't specify a cluster, auto-select one via the // recommendations API. Failures fall back to global routing. + clusterSource := ClusterSourceExplicit if tunnel.ClusterID == "" { - recommendations, recErr := m.GetClusterRecommendations(ctx, "", options.RequiredGeo) - if recErr == nil && recommendations != nil && recommendations.RecommendedClusterID != "" { + recommendations, authRejected, recErr := m.getClusterRecommendations(ctx, "", options.RequiredGeo) + switch { + case recErr != nil: + // Global routing still succeeds, so without reporting this the caller has no + // indication that recommendation-based placement stopped working. + if isUnauthorized(recErr) { + clusterSource = ClusterSourceFallbackAuthFailed + } else { + clusterSource = ClusterSourceFallbackError + } + case recommendations != nil && recommendations.RecommendedClusterID != "": tunnel.ClusterID = recommendations.RecommendedClusterID + if authRejected { + clusterSource = ClusterSourceRecommendedAfterAuthRejected + } else { + clusterSource = ClusterSourceRecommended + } + default: + clusterSource = ClusterSourceFallbackEmpty + } + + if m.OnClusterSelected != nil { + m.OnClusterSelected(ClusterSelection{ + Source: clusterSource, + ClusterID: tunnel.ClusterID, + Err: recErr, + }) } } + // Report the client-side selection path to the service. Recommendation fallbacks are + // otherwise invisible in service telemetry: a create that fell back looks identical to + // one that was never recommended at all. + // + // This is passed explicitly rather than via options.AdditionalHeaders because that map + // is never read when the request is built. + clusterSourceHeader := map[string]string{clusterSourceHeaderName: string(clusterSource)} + convertedTunnel, err := tunnel.requestObject() convertedTunnel.TunnelID = tunnel.TunnelID if err != nil { @@ -252,7 +295,7 @@ func (m *Manager) CreateTunnel(ctx context.Context, tunnel *Tunnel, options *Tun if err != nil { return nil, fmt.Errorf("error creating request url: %w", err) } - response, err = m.sendTunnelRequest(ctx, tunnel, options, http.MethodPut, url, convertedTunnel, nil, manageAccessTokenScope, false) + response, err = m.sendTunnelRequest(ctx, tunnel, options, http.MethodPut, url, convertedTunnel, nil, manageAccessTokenScope, false, clusterSourceHeader) if err == nil { break } @@ -720,6 +763,21 @@ func (m *Manager) ListClusters(ctx context.Context) (clusters []*ClusterDetails, func (m *Manager) GetClusterRecommendations( ctx context.Context, preferredClusterId string, requiredGeo string, ) (recommendations *ClusterRecommendationResponse, err error) { + recommendations, _, err = m.getClusterRecommendations(ctx, preferredClusterId, requiredGeo) + return recommendations, err +} + +// getClusterRecommendations requests cluster recommendations, reporting whether the +// caller's token was rejected. +// +// The token is sent so the service can identify the caller and apply its service tier. +// If the token is rejected the request is retried without it, because the service rejects +// a bad token before the controller runs and does not fall back to treating the caller as +// anonymous. Without the retry, one expired token would silently disable +// recommendation-based routing for that caller. +func (m *Manager) getClusterRecommendations( + ctx context.Context, preferredClusterId string, requiredGeo string, +) (recommendations *ClusterRecommendationResponse, authRejected bool, err error) { queryValues := url.Values{} if preferredClusterId != "" { queryValues.Set("preferredClusterId", preferredClusterId) @@ -730,18 +788,34 @@ func (m *Manager) GetClusterRecommendations( path := clustersApiPath + recommendationsApiSubPath url := m.buildUri("", path, nil, queryValues.Encode()) - response, err := m.sendRequest(ctx, http.MethodGet, url, nil, nil, "", false) + + token := m.tokenProvider() + response, err := m.sendRequest(ctx, http.MethodGet, url, nil, nil, token, false) + if err != nil && token != "" && isUnauthorized(err) { + response, err = m.sendRequest(ctx, http.MethodGet, url, nil, nil, "", false) + authRejected = err == nil + } if err != nil { - return nil, fmt.Errorf("error getting cluster recommendations: %w", err) + return nil, authRejected, fmt.Errorf("error getting cluster recommendations: %w", err) } err = json.Unmarshal(response, &recommendations) if err != nil { - return nil, fmt.Errorf("error parsing response json to ClusterRecommendationResponse: %w", err) + return nil, authRejected, fmt.Errorf("error parsing response json to ClusterRecommendationResponse: %w", err) } - return recommendations, nil + return recommendations, authRejected, nil +} + +// isUnauthorized reports whether the error is a 401 or 403 from the service. +func isUnauthorized(err error) bool { + var requestErr *requestError + if !errors.As(err, &requestErr) { + return false + } + return requestErr.statusCode == http.StatusUnauthorized || + requestErr.statusCode == http.StatusForbidden } // Checks if tunnel name is available @@ -775,11 +849,14 @@ func (m *Manager) sendTunnelRequest( partialFields []string, accessTokenScopes []TunnelAccessScope, allowNotFound bool, + extraHeaders ...map[string]string, ) ([]byte, error) { authHeaderValue := m.getAccessToken(tunnel, tunnelRequestOptions, accessTokenScopes) - return m.sendRequest(ctx, method, uri, requestObject, partialFields, authHeaderValue, allowNotFound) + return m.sendRequest(ctx, method, uri, requestObject, partialFields, authHeaderValue, allowNotFound, extraHeaders...) } +// sendRequest sends a request to the service. extraHeaders is optional and adds +// per-request headers on top of the manager-wide ones. func (m *Manager) sendRequest( ctx context.Context, method string, @@ -788,6 +865,7 @@ func (m *Manager) sendRequest( partialFields []string, authHeaderValue string, allowNotFound bool, + extraHeaders ...map[string]string, ) ([]byte, error) { request, err := m.createRequest(ctx, method, uri, requestObject, partialFields) if err != nil { @@ -819,6 +897,12 @@ func (m *Manager) sendRequest( request.Header.Add(header, headerValue) } + for _, headers := range extraHeaders { + for header, headerValue := range headers { + request.Header.Set(header, headerValue) + } + } + result, err := m.httpClient.Do(request) if err != nil { return nil, fmt.Errorf("error sending request: %w", err) From df68e6834f3ed946bacf2f1b55d388b3556b369d Mon Sep 17 00:00:00 2001 From: Arun Gopinathan Date: Mon, 31 Aug 2026 17:06:11 -0500 Subject: [PATCH 3/4] Send auth on cluster recommendations and report cluster source (TypeScript) The recommendations call now sends the caller token so the service can identify them and apply their service tier. A rejected token is retried anonymously, because the service rejects a bad token before the controller runs and would otherwise silently disable recommendation-based routing for that caller. createTunnel no longer swallows recommendation failures in an empty catch. It classifies the outcome and sends it in the X-Tunnel-Cluster-Source header, so a silent fallback to Traffic Manager routing is visible service-side, and raises onClusterSelected for embedders that want to log it. Matches the C# and Go changes on this branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 09709668-833e-4c9a-bea4-fdb93f91b8e5 --- ts/src/management/index.ts | 1 + ts/src/management/tunnelClusterSelection.ts | 78 +++++ .../management/tunnelManagementHttpClient.ts | 119 +++++++- ts/test/tunnels-test/tunnelManagementTests.ts | 274 +++++++++++++++++- 4 files changed, 462 insertions(+), 10 deletions(-) create mode 100644 ts/src/management/tunnelClusterSelection.ts diff --git a/ts/src/management/index.ts b/ts/src/management/index.ts index c399ac63..e4145593 100644 --- a/ts/src/management/index.ts +++ b/ts/src/management/index.ts @@ -5,3 +5,4 @@ export * from './tunnelManagementHttpClient'; export * from './tunnelManagementClient'; export * from './tunnelRequestOptions'; export * from './tunnelAccessTokenProperties'; +export * from './tunnelClusterSelection'; diff --git a/ts/src/management/tunnelClusterSelection.ts b/ts/src/management/tunnelClusterSelection.ts new file mode 100644 index 00000000..265fec79 --- /dev/null +++ b/ts/src/management/tunnelClusterSelection.ts @@ -0,0 +1,78 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +/** + * Describes how the cluster for a tunnel create request was chosen. + * + * When a create request does not specify a cluster, the client asks the recommendations + * API which cluster to use. That call can fail, and when it does the client falls back to + * global (Traffic Manager) routing, which still works but picks the nearest cluster by + * latency rather than the recommended one. The fallback is therefore invisible to the + * caller, so this value records which path was actually taken. + */ +export enum TunnelClusterSource { + /** + * The caller specified the cluster, so no recommendation was requested. + */ + explicit = 'explicit', + + /** + * The recommendations API was called and its cluster was used. + */ + recommended = 'recommended', + + /** + * The recommendations API rejected the caller's token and the retry without a token + * succeeded. Routing is correct but the caller was not identified, so it is treated as + * anonymous and cannot be assigned a service tier. This indicates a token problem that + * would otherwise be invisible. + */ + recommendedAfterAuthRejected = 'recommended-after-auth-rejected', + + /** + * The recommendations API returned unauthorized even without a token, so global + * routing was used instead. + */ + fallbackAuthFailed = 'fallback-auth-failed', + + /** + * The recommendations API returned no cluster, so global routing was used instead. + */ + fallbackEmpty = 'fallback-empty', + + /** + * The recommendations API call failed, so global routing was used instead. + */ + fallbackError = 'fallback-error', +} + +/** + * Reports how the cluster for a tunnel create request was chosen. + */ +export interface TunnelClusterSelectionEventArgs { + /** + * How the cluster was chosen. + */ + source: TunnelClusterSource; + + /** + * The cluster that was selected, if one was. + */ + clusterId?: string; + + /** + * The failure that caused a fallback, if there was one. + */ + error?: Error; +} + +/** + * Gets whether the tunnel was placed by global routing rather than by recommendation. + */ +export function isClusterFallback(source: TunnelClusterSource): boolean { + return ( + source === TunnelClusterSource.fallbackAuthFailed || + source === TunnelClusterSource.fallbackEmpty || + source === TunnelClusterSource.fallbackError + ); +} diff --git a/ts/src/management/tunnelManagementHttpClient.ts b/ts/src/management/tunnelManagementHttpClient.ts index fe70c891..b676480c 100644 --- a/ts/src/management/tunnelManagementHttpClient.ts +++ b/ts/src/management/tunnelManagementHttpClient.ts @@ -33,6 +33,10 @@ import axios, { AxiosAdapter, AxiosError, AxiosRequestConfig, AxiosResponse, Met import * as https from 'https'; import { TunnelPlanTokenProperties } from './tunnelPlanTokenProperties'; import { IdGeneration } from './idGeneration'; +import { + TunnelClusterSelectionEventArgs, + TunnelClusterSource, +} from './tunnelClusterSelection'; type NullableIfNotBoolean = T extends boolean ? T : T | null; @@ -43,6 +47,7 @@ const portsApiSubPath = '/ports'; const eventsApiSubPath = '/events'; const clustersApiPath = '/clusters'; const recommendationsSubPath = '/recommendations'; +const clusterSourceHeaderName = 'X-Tunnel-Cluster-Source'; const tunnelAuthentication = 'Authorization'; const checkAvailablePath = ':checkNameAvailability'; const createNameRetries = 3; @@ -54,6 +59,14 @@ function comparePorts(a: TunnelPort, b: TunnelPort) { return (a.portNumber ?? Number.MAX_SAFE_INTEGER) - (b.portNumber ?? Number.MAX_SAFE_INTEGER); } +/** + * Gets whether the error is a 401 or 403 from the service. + */ +function isUnauthorizedError(error: unknown): boolean { + const status = (error as AxiosError)?.response?.status; + return status === 401 || status === 403; +} + function parseDate(value?: string | Date) { return typeof value === 'string' ? new Date(Date.parse(value)) : value; } @@ -135,6 +148,20 @@ export class TunnelManagementHttpClient implements TunnelManagementClient { private readonly reportProgressEmitter = new Emitter(); + private readonly clusterSelectedEmitter = new Emitter(); + + /** + * Event that is raised when a create request selected a cluster via the + * recommendations API, reporting which path was taken. + * + * Subscribe to surface recommendation failures. When the recommendations call fails the + * create still succeeds via global routing, so without this a caller has no way to tell + * that recommendation-based placement stopped working. Raised only when the caller did + * not specify a cluster. + */ + public readonly onClusterSelected: Event = + this.clusterSelectedEmitter.event; + /** * Event that is raised to report tunnel management progress. * @@ -333,20 +360,37 @@ export class TunnelManagementHttpClient implements TunnelManagementClient { // If the caller didn't specify a cluster, auto-select one via the // recommendations API. Failures fall back to global routing. + let clusterSource = TunnelClusterSource.explicit; if (!tunnel.clusterId) { + let error: Error | undefined; try { - const recommendations = await this.getClusterRecommendations( + const { response, authRejected } = await this.getClusterRecommendationsInternal( undefined, options.requiredGeo, cancellation, ); - if (recommendations?.recommendedClusterId) { - tunnel.clusterId = recommendations.recommendedClusterId; + if (response?.recommendedClusterId) { + tunnel.clusterId = response.recommendedClusterId; + clusterSource = authRejected + ? TunnelClusterSource.recommendedAfterAuthRejected + : TunnelClusterSource.recommended; + } else { + clusterSource = TunnelClusterSource.fallbackEmpty; } - } catch { - // Fall through to global (Traffic Manager) routing if the - // recommendations request fails for any reason. + } catch (e) { + // Global routing still succeeds, so without reporting this the caller has + // no indication that recommendation-based placement stopped working. + error = e as Error; + clusterSource = isUnauthorizedError(e) + ? TunnelClusterSource.fallbackAuthFailed + : TunnelClusterSource.fallbackError; } + + this.clusterSelectedEmitter.fire({ + source: clusterSource, + clusterId: tunnel.clusterId, + error, + }); } options = { @@ -354,6 +398,10 @@ export class TunnelManagementHttpClient implements TunnelManagementClient { additionalHeaders: { ...options.additionalHeaders, 'If-None-Match': '*', + // Report the client-side selection path to the service. Recommendation + // fallbacks are otherwise invisible in service telemetry: a create that + // fell back looks identical to one that was never recommended at all. + [clusterSourceHeaderName]: clusterSource, }, }; @@ -732,6 +780,27 @@ export class TunnelManagementHttpClient implements TunnelManagementClient { requiredGeo?: string, cancellation?: CancellationToken, ): Promise { + return (await this.getClusterRecommendationsInternal( + preferredClusterId, + requiredGeo, + cancellation, + )).response; + } + + /** + * Requests cluster recommendations, reporting whether the caller's token was rejected. + * + * The token is sent so the service can identify the caller and apply its service tier. + * If the token is rejected the request is retried without it, because the service + * rejects a bad token before the controller runs and does not fall back to treating the + * caller as anonymous. Without the retry, one expired token would silently disable + * recommendation-based routing for that caller. + */ + private async getClusterRecommendationsInternal( + preferredClusterId?: string, + requiredGeo?: string, + cancellation?: CancellationToken, + ): Promise<{ response: ClusterRecommendationResponse; authRejected: boolean }> { const queryParts: string[] = []; if (preferredClusterId) { queryParts.push(`preferredClusterId=${encodeURIComponent(preferredClusterId)}`); @@ -741,13 +810,44 @@ export class TunnelManagementHttpClient implements TunnelManagementClient { } const query = queryParts.length > 0 ? queryParts.join('&') : undefined; - return (await this.sendRequest( - 'GET', + try { + return { + response: await this.sendClusterRecommendationsRequest(query, false, cancellation), + authRejected: false, + }; + } catch (error) { + if (!isUnauthorizedError(error)) { + throw error; + } + + return { + response: await this.sendClusterRecommendationsRequest(query, true, cancellation), + authRejected: true, + }; + } + } + + private async sendClusterRecommendationsRequest( + query: string | undefined, + suppressUserToken: boolean, + cancellation?: CancellationToken, + ): Promise { + const uri = await this.buildUri( undefined, clustersApiPath + recommendationsSubPath, query, + ); + const config = await this.getAxiosRequestConfig( undefined, undefined, + undefined, + suppressUserToken, + ); + return (await this.request( + 'GET', + uri, + undefined, + config, false, cancellation, ))!; @@ -1136,6 +1236,7 @@ export class TunnelManagementHttpClient implements TunnelManagementClient { tunnel?: Tunnel, options?: TunnelRequestOptions, accessTokenScopes?: string[], + suppressUserToken?: boolean, ): Promise { // Get access token header const headers: { [name: string]: string } = {}; @@ -1146,7 +1247,7 @@ export class TunnelManagementHttpClient implements TunnelManagementClient { ] = `${TunnelAuthenticationSchemes.tunnel} ${options.accessToken}`; } - if (!(tunnelAuthentication in headers) && this.userTokenCallback) { + if (!(tunnelAuthentication in headers) && this.userTokenCallback && !suppressUserToken) { const token = await this.userTokenCallback(); if (token) { headers[tunnelAuthentication] = token; diff --git a/ts/test/tunnels-test/tunnelManagementTests.ts b/ts/test/tunnels-test/tunnelManagementTests.ts index 3fb5d2d8..5f63e639 100644 --- a/ts/test/tunnels-test/tunnelManagementTests.ts +++ b/ts/test/tunnels-test/tunnelManagementTests.ts @@ -5,7 +5,7 @@ import * as assert from 'assert'; import axios, { Axios, AxiosHeaders, AxiosError, AxiosPromise, AxiosRequestConfig, AxiosResponse, Method } from 'axios'; import * as https from 'https'; import { suite, test, slow, timeout } from '@testdeck/mocha'; -import { ManagementApiVersions, TunnelManagementHttpClient } from '@microsoft/dev-tunnels-management'; +import { ManagementApiVersions, TunnelManagementHttpClient, TunnelClusterSource, TunnelClusterSelectionEventArgs, isClusterFallback } from '@microsoft/dev-tunnels-management'; import { Tunnel, TunnelPort, TunnelProgress, TunnelReportProgressEventArgs, ClusterRecommendationResponse, ClusterAvailability } from '@microsoft/dev-tunnels-contracts'; import { CancellationToken, CancellationTokenSource } from 'vscode-jsonrpc'; @@ -713,4 +713,276 @@ export class TunnelManagementTests { const url = new URL(capturedUri!); assert.ok(url.hostname.startsWith('usw2.'), `Expected hostname to start with usw2., got ${url.hostname}`); } + + private static readonly clusterSourceHeaderName = 'X-Tunnel-Cluster-Source'; + + /** + * Creates a client whose transport is driven by the given handler and records every + * request it makes, so tests can assert on the auth and cluster-source headers. + */ + private createRecordingClient( + token: string | undefined, + handler: (config: AxiosRequestConfig) => AxiosResponse, + ): { client: TunnelManagementHttpClient; requests: AxiosRequestConfig[] } { + const requests: AxiosRequestConfig[] = []; + const client = new TunnelManagementHttpClient( + 'test/0.0.0', + ManagementApiVersions.Version20230927preview, + token ? async () => token : undefined, + TunnelManagementTests.testServiceUri, + ); + (client).axiosRequest = async (config: AxiosRequestConfig) => { + requests.push(config); + return handler(config); + }; + return { client, requests }; + } + + private static isRecommendationsRequest(config: AxiosRequestConfig): boolean { + return (config.url ?? '').includes('/clusters/recommendations'); + } + + private static okResponse(config: AxiosRequestConfig, data: any): AxiosResponse { + return { data, status: 200, statusText: 'OK', headers: {}, config } as AxiosResponse; + } + + private static failure(config: AxiosRequestConfig, status: number): AxiosError { + const error = new AxiosError('request failed'); + error.response = { + data: {}, + status, + statusText: '', + headers: {} as AxiosHeaders, + config, + } as AxiosResponse; + return error; + } + + private static authorizationOf(config: AxiosRequestConfig): string | undefined { + return (config.headers as { [name: string]: string })?.['Authorization']; + } + + private static clusterSourceOf(config: AxiosRequestConfig): string | undefined { + return (config.headers as { [name: string]: string })?.[ + TunnelManagementTests.clusterSourceHeaderName + ]; + } + + // The recommendations call must send the caller's token so the service can identify + // them and apply their service tier. + @test + public async getClusterRecommendationsSendsAuthorizationHeader() { + const { client, requests } = this.createRecordingClient('Bearer test-token', (config) => + TunnelManagementTests.okResponse(config, { + recommendedClusterId: 'euw', + }), + ); + + const result = await client.getClusterRecommendations(); + + assert.strictEqual(result.recommendedClusterId, 'euw'); + assert.strictEqual(requests.length, 1); + assert.strictEqual(TunnelManagementTests.authorizationOf(requests[0]), 'Bearer test-token'); + } + + // With no token configured the call must still work anonymously, and must not send an + // empty Authorization header, which the service would reject. + @test + public async getClusterRecommendationsWithoutTokenSendsNoAuthorizationHeader() { + const { client, requests } = this.createRecordingClient(undefined, (config) => + TunnelManagementTests.okResponse(config, { + recommendedClusterId: 'euw', + }), + ); + + await client.getClusterRecommendations(); + + assert.strictEqual(requests.length, 1); + assert.strictEqual(TunnelManagementTests.authorizationOf(requests[0]), undefined); + } + + // The service rejects a bad token before the controller runs, so it never falls back to + // treating the caller as anonymous. Without the retry, one expired token would silently + // disable recommendation-based routing for that caller. + @test + public async getClusterRecommendationsRetriesAnonymouslyAfterUnauthorized() { + const { client, requests } = this.createRecordingClient('Bearer expired-token', (config) => { + if (TunnelManagementTests.authorizationOf(config)) { + throw TunnelManagementTests.failure(config, 401); + } + return TunnelManagementTests.okResponse(config, { + recommendedClusterId: 'euw', + }); + }); + + const result = await client.getClusterRecommendations(); + + assert.strictEqual(result.recommendedClusterId, 'euw'); + assert.strictEqual(requests.length, 2); + assert.strictEqual(TunnelManagementTests.authorizationOf(requests[1]), undefined); + } + + // A non-auth failure must not trigger the anonymous retry; retrying would not help and + // would double the latency of every failure. + @test + public async getClusterRecommendationsDoesNotRetryOnServerError() { + const { client, requests } = this.createRecordingClient('Bearer test-token', (config) => { + throw TunnelManagementTests.failure(config, 500); + }); + + await assert.rejects(async () => await client.getClusterRecommendations()); + assert.strictEqual(requests.length, 1); + } + + // When the caller specifies a cluster there is nothing to recommend, so no call is made + // and the create reports that the cluster was chosen explicitly. + @test + public async createTunnelWithExplicitClusterReportsExplicitSource() { + const selections: TunnelClusterSelectionEventArgs[] = []; + const { client, requests } = this.createRecordingClient('Bearer test-token', (config) => { + assert.ok( + !TunnelManagementTests.isRecommendationsRequest(config), + 'recommendations should not be requested when a cluster is specified', + ); + return TunnelManagementTests.okResponse(config, { tunnelId: 'tnnl0001' }); + }); + client.onClusterSelected((e) => selections.push(e)); + + await client.createTunnel({ clusterId: 'euw' }); + + assert.strictEqual(selections.length, 0); + assert.strictEqual( + TunnelManagementTests.clusterSourceOf(requests[requests.length - 1]), + 'explicit', + ); + } + + // The happy path: the recommendation is used and reported as such. + @test + public async createTunnelReportsRecommendedCluster() { + const selections: TunnelClusterSelectionEventArgs[] = []; + const { client, requests } = this.createRecordingClient('Bearer test-token', (config) => { + if (TunnelManagementTests.isRecommendationsRequest(config)) { + return TunnelManagementTests.okResponse(config, { + recommendedClusterId: 'euw', + }); + } + return TunnelManagementTests.okResponse(config, { tunnelId: 'tnnl0001' }); + }); + client.onClusterSelected((e) => selections.push(e)); + + await client.createTunnel({}); + + assert.strictEqual(selections.length, 1); + assert.strictEqual(selections[0].source, TunnelClusterSource.recommended); + assert.strictEqual(selections[0].clusterId, 'euw'); + assert.strictEqual( + TunnelManagementTests.clusterSourceOf(requests[requests.length - 1]), + 'recommended', + ); + } + + // Routing is correct here but the caller was not identified, so they cannot be assigned + // a service tier. That is a token problem the caller would otherwise never learn about. + @test + public async createTunnelReportsRecommendationAfterAuthRejected() { + const selections: TunnelClusterSelectionEventArgs[] = []; + const { client, requests } = this.createRecordingClient('Bearer expired-token', (config) => { + if (TunnelManagementTests.isRecommendationsRequest(config)) { + if (TunnelManagementTests.authorizationOf(config)) { + throw TunnelManagementTests.failure(config, 401); + } + return TunnelManagementTests.okResponse(config, { + recommendedClusterId: 'euw', + }); + } + return TunnelManagementTests.okResponse(config, { tunnelId: 'tnnl0001' }); + }); + client.onClusterSelected((e) => selections.push(e)); + + await client.createTunnel({}); + + assert.strictEqual(selections.length, 1); + assert.strictEqual( + selections[0].source, + TunnelClusterSource.recommendedAfterAuthRejected, + ); + assert.strictEqual( + TunnelManagementTests.clusterSourceOf(requests[requests.length - 1]), + 'recommended-after-auth-rejected', + ); + } + + // The create still succeeds via global routing, so the fallback is invisible without + // the event and the header. + @test + public async createTunnelReportsFallbackOnRecommendationError() { + const selections: TunnelClusterSelectionEventArgs[] = []; + const { client, requests } = this.createRecordingClient('Bearer test-token', (config) => { + if (TunnelManagementTests.isRecommendationsRequest(config)) { + throw TunnelManagementTests.failure(config, 500); + } + return TunnelManagementTests.okResponse(config, { tunnelId: 'tnnl0001' }); + }); + client.onClusterSelected((e) => selections.push(e)); + + const tunnel = await client.createTunnel({}); + + assert.ok(tunnel, 'create should still succeed via global routing'); + assert.strictEqual(selections.length, 1); + assert.strictEqual(selections[0].source, TunnelClusterSource.fallbackError); + assert.ok(selections[0].error, 'expected the underlying error to be reported'); + assert.ok(isClusterFallback(selections[0].source)); + assert.strictEqual( + TunnelManagementTests.clusterSourceOf(requests[requests.length - 1]), + 'fallback-error', + ); + } + + // An auth failure that survives the anonymous retry is distinguished from a generic + // failure, because it points at the service rejecting the request rather than being + // down. + @test + public async createTunnelReportsFallbackWhenAuthFailsEvenAnonymously() { + const selections: TunnelClusterSelectionEventArgs[] = []; + const { client, requests } = this.createRecordingClient('Bearer test-token', (config) => { + if (TunnelManagementTests.isRecommendationsRequest(config)) { + throw TunnelManagementTests.failure(config, 401); + } + return TunnelManagementTests.okResponse(config, { tunnelId: 'tnnl0001' }); + }); + client.onClusterSelected((e) => selections.push(e)); + + await client.createTunnel({}); + + assert.strictEqual(selections.length, 1); + assert.strictEqual(selections[0].source, TunnelClusterSource.fallbackAuthFailed); + assert.strictEqual( + TunnelManagementTests.clusterSourceOf(requests[requests.length - 1]), + 'fallback-auth-failed', + ); + } + + // A successful call that recommends nothing is a distinct condition from a failure, and + // means the service had no cluster to offer. + @test + public async createTunnelReportsFallbackOnEmptyRecommendation() { + const selections: TunnelClusterSelectionEventArgs[] = []; + const { client, requests } = this.createRecordingClient('Bearer test-token', (config) => { + if (TunnelManagementTests.isRecommendationsRequest(config)) { + return TunnelManagementTests.okResponse(config, {}); + } + return TunnelManagementTests.okResponse(config, { tunnelId: 'tnnl0001' }); + }); + client.onClusterSelected((e) => selections.push(e)); + + await client.createTunnel({}); + + assert.strictEqual(selections.length, 1); + assert.strictEqual(selections[0].source, TunnelClusterSource.fallbackEmpty); + assert.strictEqual( + TunnelManagementTests.clusterSourceOf(requests[requests.length - 1]), + 'fallback-empty', + ); + } } From 0a1906dd7d8f644a52527d20b465b35bb5dc11dc Mon Sep 17 00:00:00 2001 From: Arun Gopinathan Date: Mon, 31 Aug 2026 17:13:00 -0500 Subject: [PATCH 4/4] Bump Go package version and TS interpackage dependency floors Required by the PR checklist. The Go PackageVersion goes to 0.1.28. The TS management and connections manifests still required >1.3.50 while the published packages are at 1.3.55, so yarn could resolve an older contracts or management package than the one these changes are built against and produce a mismatched dependency set. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 09709668-833e-4c9a-bea4-fdb93f91b8e5 --- go/tunnels/tunnels.go | 2 +- ts/src/connections/package.json | 4 ++-- ts/src/management/package.json | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go/tunnels/tunnels.go b/go/tunnels/tunnels.go index 8c9b9aae..8ecb62f3 100644 --- a/go/tunnels/tunnels.go +++ b/go/tunnels/tunnels.go @@ -10,7 +10,7 @@ import ( "github.com/rodaine/table" ) -const PackageVersion = "0.1.27" +const PackageVersion = "0.1.28" func (tunnel *Tunnel) requestObject() (*Tunnel, error) { convertedTunnel := &Tunnel{ diff --git a/ts/src/connections/package.json b/ts/src/connections/package.json index f7e4c3c5..77d004bb 100644 --- a/ts/src/connections/package.json +++ b/ts/src/connections/package.json @@ -18,8 +18,8 @@ "buffer": "^5.2.1", "debug": "^4.1.1", "vscode-jsonrpc": "^4.0.0", - "@microsoft/dev-tunnels-contracts": ">1.3.50", - "@microsoft/dev-tunnels-management": ">1.3.50", + "@microsoft/dev-tunnels-contracts": ">1.3.55", + "@microsoft/dev-tunnels-management": ">1.3.55", "uuid": "^3.3.3", "await-semaphore": "^0.1.3", "websocket": "^1.0.28", diff --git a/ts/src/management/package.json b/ts/src/management/package.json index 4fe55824..f66dad4f 100644 --- a/ts/src/management/package.json +++ b/ts/src/management/package.json @@ -18,7 +18,7 @@ "buffer": "^5.2.1", "debug": "^4.1.1", "vscode-jsonrpc": "^4.0.0", - "@microsoft/dev-tunnels-contracts": ">1.3.50", + "@microsoft/dev-tunnels-contracts": ">1.3.55", "axios": "^1.8.4" } }