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; 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) 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/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/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" } } 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', + ); + } }