From f47384b46e4b67de3c1c61e4c57d882f2125620c Mon Sep 17 00:00:00 2001 From: Yang Cao Date: Fri, 6 Oct 2023 12:01:28 -0400 Subject: [PATCH] Use RawHttpMessageHandler and VssHttpRetryMessageHandler in ResultsHttpClient (#2908) * Set an explicit timeout on ResultsHttpClient * Hook up retry and standard message handler to ResultsHttpClient * Remove explicit timeout constructor * Fix linter --- src/Runner.Common/ResultsServer.cs | 25 ++++++++- src/Runner.Sdk/Util/VssUtil.cs | 54 ++++++++++--------- .../Common/Authentication/NoOpCredentials.cs | 22 ++++++++ .../Common/Common/RawHttpMessageHandler.cs | 13 +++-- 4 files changed, 84 insertions(+), 30 deletions(-) create mode 100644 src/Sdk/Common/Common/Authentication/NoOpCredentials.cs diff --git a/src/Runner.Common/ResultsServer.cs b/src/Runner.Common/ResultsServer.cs index 4b5afb557..ef97ebcfc 100644 --- a/src/Runner.Common/ResultsServer.cs +++ b/src/Runner.Common/ResultsServer.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Net.Http; using System.Net.Http.Headers; using System.Net.WebSockets; using System.Security; @@ -52,8 +53,8 @@ namespace GitHub.Runner.Common public void InitializeResultsClient(Uri uri, string liveConsoleFeedUrl, string token) { - var httpMessageHandler = HostContext.CreateHttpClientHandler(); - this._resultsClient = new ResultsHttpClient(uri, httpMessageHandler, token, disposeHandler: true); + this._resultsClient = CreateHttpClient(uri, token); + _token = token; if (!string.IsNullOrEmpty(liveConsoleFeedUrl)) { @@ -62,6 +63,26 @@ namespace GitHub.Runner.Common } } + public ResultsHttpClient CreateHttpClient(Uri uri, string token) + { + // Using default 100 timeout + RawClientHttpRequestSettings settings = VssUtil.GetHttpRequestSettings(null); + + // Create retry handler + IEnumerable delegatingHandlers = new List(); + if (settings.MaxRetryRequest > 0) + { + delegatingHandlers = new DelegatingHandler[] { new VssHttpRetryMessageHandler(settings.MaxRetryRequest) }; + } + + // Setup RawHttpMessageHandler without credentials + var httpMessageHandler = new RawHttpMessageHandler(new NoOpCredentials(null), settings); + + var pipeline = HttpClientFactory.CreatePipeline(httpMessageHandler, delegatingHandlers); + + return new ResultsHttpClient(uri, pipeline, token, disposeHandler: true); + } + public Task CreateResultsStepSummaryAsync(string planId, string jobId, Guid stepId, string file, CancellationToken cancellationToken) { diff --git a/src/Runner.Sdk/Util/VssUtil.cs b/src/Runner.Sdk/Util/VssUtil.cs index 1107be9f4..bf74fa16c 100644 --- a/src/Runner.Sdk/Util/VssUtil.cs +++ b/src/Runner.Sdk/Util/VssUtil.cs @@ -85,6 +85,35 @@ namespace GitHub.Runner.Sdk VssCredentials credentials, IEnumerable additionalDelegatingHandler = null, TimeSpan? timeout = null) + { + RawClientHttpRequestSettings settings = GetHttpRequestSettings(timeout); + RawConnection connection = new(serverUri, new RawHttpMessageHandler(credentials.Federated, settings), additionalDelegatingHandler); + return connection; + } + + public static VssCredentials GetVssCredential(ServiceEndpoint serviceEndpoint) + { + ArgUtil.NotNull(serviceEndpoint, nameof(serviceEndpoint)); + ArgUtil.NotNull(serviceEndpoint.Authorization, nameof(serviceEndpoint.Authorization)); + ArgUtil.NotNullOrEmpty(serviceEndpoint.Authorization.Scheme, nameof(serviceEndpoint.Authorization.Scheme)); + + if (serviceEndpoint.Authorization.Parameters.Count == 0) + { + throw new ArgumentOutOfRangeException(nameof(serviceEndpoint)); + } + + VssCredentials credentials = null; + string accessToken; + if (serviceEndpoint.Authorization.Scheme == EndpointAuthorizationSchemes.OAuth && + serviceEndpoint.Authorization.Parameters.TryGetValue(EndpointAuthorizationParameters.AccessToken, out accessToken)) + { + credentials = new VssCredentials(new VssOAuthAccessTokenCredential(accessToken), CredentialPromptType.DoNotPrompt); + } + + return credentials; + } + + public static RawClientHttpRequestSettings GetHttpRequestSettings(TimeSpan? timeout = null) { RawClientHttpRequestSettings settings = RawClientHttpRequestSettings.Default.Clone(); @@ -116,30 +145,7 @@ namespace GitHub.Runner.Sdk // settings are applied to an HttpRequestMessage. settings.AcceptLanguages.Remove(CultureInfo.InvariantCulture); - RawConnection connection = new(serverUri, new RawHttpMessageHandler(credentials.Federated, settings), additionalDelegatingHandler); - return connection; - } - - public static VssCredentials GetVssCredential(ServiceEndpoint serviceEndpoint) - { - ArgUtil.NotNull(serviceEndpoint, nameof(serviceEndpoint)); - ArgUtil.NotNull(serviceEndpoint.Authorization, nameof(serviceEndpoint.Authorization)); - ArgUtil.NotNullOrEmpty(serviceEndpoint.Authorization.Scheme, nameof(serviceEndpoint.Authorization.Scheme)); - - if (serviceEndpoint.Authorization.Parameters.Count == 0) - { - throw new ArgumentOutOfRangeException(nameof(serviceEndpoint)); - } - - VssCredentials credentials = null; - string accessToken; - if (serviceEndpoint.Authorization.Scheme == EndpointAuthorizationSchemes.OAuth && - serviceEndpoint.Authorization.Parameters.TryGetValue(EndpointAuthorizationParameters.AccessToken, out accessToken)) - { - credentials = new VssCredentials(new VssOAuthAccessTokenCredential(accessToken), CredentialPromptType.DoNotPrompt); - } - - return credentials; + return settings; } } } diff --git a/src/Sdk/Common/Common/Authentication/NoOpCredentials.cs b/src/Sdk/Common/Common/Authentication/NoOpCredentials.cs new file mode 100644 index 000000000..dcb88b88a --- /dev/null +++ b/src/Sdk/Common/Common/Authentication/NoOpCredentials.cs @@ -0,0 +1,22 @@ +using System; +using System.Threading; +using System.Threading.Tasks; + +namespace GitHub.Services.Common +{ + // Set of classes used to bypass token operations + // Results Service and External services follow a different auth model but + // we are required to pass in a credentials object to create a RawHttpMessageHandler + public class NoOpCredentials : FederatedCredential + { + public NoOpCredentials(IssuedToken initialToken) : base(initialToken) + { + } + + public override VssCredentialsType CredentialType { get; } + protected override IssuedTokenProvider OnCreateTokenProvider(Uri serverUrl, IHttpResponse response) + { + return null; + } + } +} diff --git a/src/Sdk/Common/Common/RawHttpMessageHandler.cs b/src/Sdk/Common/Common/RawHttpMessageHandler.cs index fdd5a2cd0..09cd2d59a 100644 --- a/src/Sdk/Common/Common/RawHttpMessageHandler.cs +++ b/src/Sdk/Common/Common/RawHttpMessageHandler.cs @@ -109,7 +109,7 @@ namespace GitHub.Services.Common lock (m_thisLock) { // Ensure that we attempt to use the most appropriate authentication mechanism by default. - if (m_tokenProvider == null) + if (m_tokenProvider == null && !(this.Credentials is NoOpCredentials)) { m_tokenProvider = this.Credentials.CreateTokenProvider(request.RequestUri, null, null); } @@ -121,7 +121,8 @@ namespace GitHub.Services.Common HttpResponseMessageWrapper responseWrapper; Boolean lastResponseDemandedProxyAuth = false; - Int32 retries = m_maxAuthRetries; + // do not retry if we cannot recreate tokens + Int32 retries = this.Credentials is NoOpCredentials ? 0 : m_maxAuthRetries; try { tokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); @@ -138,8 +139,12 @@ namespace GitHub.Services.Common } // Let's start with sending a token - IssuedToken token = await m_tokenProvider.GetTokenAsync(null, tokenSource.Token).ConfigureAwait(false); - ApplyToken(request, token, applyICredentialsToWebProxy: lastResponseDemandedProxyAuth); + IssuedToken token = null; + if (m_tokenProvider != null) + { + token = await m_tokenProvider.GetTokenAsync(null, tokenSource.Token).ConfigureAwait(false); + ApplyToken(request, token, applyICredentialsToWebProxy: lastResponseDemandedProxyAuth); + } // The WinHttpHandler will chunk any content that does not have a computed length which is // not what we want. By loading into a buffer up-front we bypass this behavior and there is