Merge branch 'main' into lindluni/code-scanning-exclusion

Author: jfine

src/Octoshift/Factories/GithubApiFactory.cs

@@ -31,7 +31,8 @@ GithubApi ISourceGithubApiFactory.Create(string apiUrl, string uploadsUrl, strin
         apiUrl ??= DEFAULT_API_URL;
         uploadsUrl ??= DEFAULT_UPLOADS_URL;
         sourcePersonalAccessToken ??= _environmentVariableProvider.SourceGithubPersonalAccessToken();
-        var githubClient = new GithubClient(_octoLogger, _clientFactory.CreateClient("Default"), _versionProvider, _retryPolicy, _dateTimeProvider, sourcePersonalAccessToken);
+        var clientRetryPolicy = (_retryPolicy ?? new RetryPolicy(_octoLogger)).WithServiceName("GitHub");
+        var githubClient = new GithubClient(_octoLogger, _clientFactory.CreateClient("Default"), _versionProvider, clientRetryPolicy, _dateTimeProvider, sourcePersonalAccessToken);
         var multipartUploader = new ArchiveUploader(githubClient, uploadsUrl, _octoLogger, _retryPolicy, _environmentVariableProvider);
         return new GithubApi(githubClient, apiUrl, _retryPolicy, multipartUploader);
     }
@@ -41,7 +42,8 @@ GithubApi ISourceGithubApiFactory.CreateClientNoSsl(string apiUrl, string upload
         apiUrl ??= DEFAULT_API_URL;
         uploadsUrl ??= DEFAULT_UPLOADS_URL;
         sourcePersonalAccessToken ??= _environmentVariableProvider.SourceGithubPersonalAccessToken();
-        var githubClient = new GithubClient(_octoLogger, _clientFactory.CreateClient("NoSSL"), _versionProvider, _retryPolicy, _dateTimeProvider, sourcePersonalAccessToken);
+        var clientRetryPolicy = (_retryPolicy ?? new RetryPolicy(_octoLogger)).WithServiceName("GitHub");
+        var githubClient = new GithubClient(_octoLogger, _clientFactory.CreateClient("NoSSL"), _versionProvider, clientRetryPolicy, _dateTimeProvider, sourcePersonalAccessToken);
         var multipartUploader = new ArchiveUploader(githubClient, uploadsUrl, _octoLogger, _retryPolicy, _environmentVariableProvider);
         return new GithubApi(githubClient, apiUrl, _retryPolicy, multipartUploader);
     }
@@ -51,7 +53,8 @@ GithubApi ITargetGithubApiFactory.Create(string apiUrl, string uploadsUrl, strin
         apiUrl ??= DEFAULT_API_URL;
         uploadsUrl ??= DEFAULT_UPLOADS_URL;
         targetPersonalAccessToken ??= _environmentVariableProvider.TargetGithubPersonalAccessToken();
-        var githubClient = new GithubClient(_octoLogger, _clientFactory.CreateClient("Default"), _versionProvider, _retryPolicy, _dateTimeProvider, targetPersonalAccessToken);
+        var clientRetryPolicy = (_retryPolicy ?? new RetryPolicy(_octoLogger)).WithServiceName("GitHub");
+        var githubClient = new GithubClient(_octoLogger, _clientFactory.CreateClient("Default"), _versionProvider, clientRetryPolicy, _dateTimeProvider, targetPersonalAccessToken);
         var multipartUploader = new ArchiveUploader(githubClient, uploadsUrl, _octoLogger, _retryPolicy, _environmentVariableProvider);
         return new GithubApi(githubClient, apiUrl, _retryPolicy, multipartUploader);
     }

src/Octoshift/RetryPolicy.cs

@@ -10,14 +10,26 @@ namespace OctoshiftCLI
     public class RetryPolicy
     {
         private readonly OctoLogger _log;
+        private readonly string _serviceName;
         internal int _httpRetryInterval = 1000;
         internal int _retryInterval = 4000;
 
-        public RetryPolicy(OctoLogger log)
+        public RetryPolicy(OctoLogger log, string serviceName = null)
         {
             _log = log;
+            _serviceName = serviceName;
         }
 
+        /// <summary>
+        /// Returns a new RetryPolicy with the same configuration as this one, but tagged with a service name
+        /// for more actionable error messages (e.g. on 401 Unauthorized).
+        /// </summary>
+        public RetryPolicy WithServiceName(string serviceName) => new(_log, serviceName)
+        {
+            _httpRetryInterval = _httpRetryInterval,
+            _retryInterval = _retryInterval
+        };
+
         public async Task<T> HttpRetry<T>(Func<Task<T>> func, Func<HttpRequestException, bool> filter)
         {
             var policy = Policy.Handle(filter)
@@ -26,7 +38,15 @@ public async Task<T> HttpRetry<T>(Func<Task<T>> func, Func<HttpRequestException,
                                    _log?.LogVerbose($"Call failed with HTTP {((HttpRequestException)ex).StatusCode}, retrying...");
                                });
 
-            return await policy.ExecuteAsync(func);
+            try
+            {
+                return await policy.ExecuteAsync(func);
+            }
+            catch (HttpRequestException ex) when (ex.StatusCode == System.Net.HttpStatusCode.Unauthorized)
+            {
+                ThrowUnauthorizedException(ex);
+                throw; // unreachable, but required by compiler
+            }
         }
 
         public async Task<PolicyResult<T>> RetryOnResult<T>(Func<Task<T>> func, Func<T, bool> resultPredicate, string retryLogMessage = null)
@@ -53,15 +73,20 @@ private AsyncRetryPolicy CreateRetryPolicyForException<TException>() where TExce
                         _log?.LogVerbose($"[HTTP ERROR {(int?)httpEx.StatusCode}] {ex}");
                         if (httpEx.StatusCode == System.Net.HttpStatusCode.Unauthorized)
                         {
-                            // We should not retry on an unathorized error; instead, log and bubble up the error
-                            throw new OctoshiftCliException($"Unauthorized. Please check your token and try again", ex);
-                        };
+                            ThrowUnauthorizedException(httpEx);
+                        }
                     }
                     else
                     {
                         _log?.LogVerbose(ex.ToString());
                     }
                     _log?.LogVerbose("Retrying...");
                 });
+
+        private void ThrowUnauthorizedException(HttpRequestException ex)
+        {
+            var serviceContext = _serviceName is not null ? $" ({_serviceName})" : "";
+            throw new OctoshiftCliException($"Unauthorized{serviceContext}. Please check your token and try again", ex);
+        }
     }
 }

src/Octoshift/Services/GithubApi.cs

@@ -561,14 +561,14 @@ ... on Migration {
         {
             return await _retryPolicy.Retry(async () =>
             {
-                var data = await _client.PostGraphQLAsync(url, payload);
+                var data = await _client.PostGraphQLWithRetryAsync(url, payload);
 
                 return (
-                    State: (string)data["data"]["node"]["state"],
-                    RepositoryName: (string)data["data"]["node"]["repositoryName"],
-                    WarningsCount: (int)data["data"]["node"]["warningsCount"],
-                    FailureReason: (string)data["data"]["node"]["failureReason"],
-                    MigrationLogUrl: (string)data["data"]["node"]["migrationLogUrl"]);
+                        State: (string)data["data"]["node"]["state"],
+                        RepositoryName: (string)data["data"]["node"]["repositoryName"],
+                        WarningsCount: (int)data["data"]["node"]["warningsCount"],
+                        FailureReason: (string)data["data"]["node"]["failureReason"],
+                        MigrationLogUrl: (string)data["data"]["node"]["migrationLogUrl"]);
             });
         }
         catch (Exception ex)

src/Octoshift/Services/GithubClient.cs

@@ -25,6 +25,9 @@ public class GithubClient
     private const int SECONDARY_RATE_LIMIT_MAX_RETRIES = 3;
     private const int SECONDARY_RATE_LIMIT_DEFAULT_DELAY = 60; // 60 seconds default delay
 
+    internal int _secondaryRateLimitMaxRetries = SECONDARY_RATE_LIMIT_MAX_RETRIES; // Exposed for testing purposes
+    internal int _secondaryRateLimitDefaultDelay = SECONDARY_RATE_LIMIT_DEFAULT_DELAY; // Exposed for testing purposes
+
     public GithubClient(OctoLogger log, HttpClient httpClient, IVersionProvider versionProvider, RetryPolicy retryPolicy, DateTimeProvider dateTimeProvider, string personalAccessToken)
     {
         _log = log;
@@ -93,6 +96,36 @@ public virtual async Task<JToken> PostGraphQLAsync(
         return data;
     }
 
+    public virtual async Task<JToken> PostGraphQLWithRetryAsync(
+        string url,
+        object body,
+        Dictionary<string, string> customHeaders = null,
+        int retryCount = 0)
+    {
+        var currentRetryCount = retryCount;
+
+        while (true)
+        {
+            var (response, headers) = await PostWithRetry(url, body, customHeaders);
+
+            if (IsGraphQLSecondaryRateLimit(response))
+            {
+                (response, _) = await HandleSecondaryRateLimit(HttpMethod.Post, url, body, HttpStatusCode.OK, customHeaders, headers, currentRetryCount);
+
+                if (IsGraphQLSecondaryRateLimit(response))
+                {
+                    currentRetryCount++;
+                    continue;
+                }
+            }
+
+            var data = JObject.Parse(response);
+            EnsureSuccessGraphQLResponse(data);
+
+            return data;
+        }
+    }
+
     public virtual async IAsyncEnumerable<JToken> PostGraphQLWithPaginationAsync(
         string url,
         object body,
@@ -156,6 +189,13 @@ public virtual async Task<string> PatchAsync(string url, object body, Dictionary
         HttpStatusCode expectedStatus = HttpStatusCode.OK) =>
         await _retryPolicy.Retry(async () => await SendAsync(HttpMethod.Get, url, customHeaders: customHeaders, expectedStatus: expectedStatus));
 
+    private async Task<(string Content, KeyValuePair<string, IEnumerable<string>>[] ResponseHeaders)> PostWithRetry(
+        string url,
+        object body,
+        Dictionary<string, string> customHeaders = null,
+        HttpStatusCode expectedStatus = HttpStatusCode.OK) =>
+        await _retryPolicy.Retry(async () => await SendAsync(HttpMethod.Post, url, body, customHeaders: customHeaders, expectedStatus: expectedStatus));
+
     private async Task<(string Content, KeyValuePair<string, IEnumerable<string>>[] ResponseHeaders)> SendAsync(
         HttpMethod httpMethod,
         string url,
@@ -312,6 +352,22 @@ private bool IsSecondaryRateLimit(HttpStatusCode statusCode, string content)
                statusCode == HttpStatusCode.TooManyRequests;
     }
 
+    private bool IsGraphQLSecondaryRateLimit(string content)
+    {
+        var response = JObject.Parse(content);
+
+        if (response.TryGetValue("errors", out var jErrors) && jErrors is JArray { Count: > 0 } errors)
+        {
+            var error = (JObject)errors[0];
+            if (error.TryGetValue("type", out var jType) && jType.Type == JTokenType.String)
+            {
+                return ((string)jType) == "RATE_LIMIT";
+            }
+        }
+
+        return false;
+    }
+
     private async Task<(string Content, KeyValuePair<string, IEnumerable<string>>[] ResponseHeaders)> HandleSecondaryRateLimit(
         HttpMethod httpMethod,
         string url,
@@ -321,7 +377,7 @@ private bool IsSecondaryRateLimit(HttpStatusCode statusCode, string content)
         KeyValuePair<string, IEnumerable<string>>[] headers,
         int retryCount = 0)
     {
-        if (retryCount >= SECONDARY_RATE_LIMIT_MAX_RETRIES)
+        if (retryCount >= _secondaryRateLimitMaxRetries)
         {
             throw new OctoshiftCliException($"Secondary rate limit exceeded. Maximum retries ({SECONDARY_RATE_LIMIT_MAX_RETRIES}) reached. Please wait before retrying your request.");
         }
@@ -359,6 +415,6 @@ private int GetSecondaryRateLimitDelay(KeyValuePair<string, IEnumerable<string>>
         }
 
         // Otherwise use exponential backoff: 1m → 2m → 4m
-        return SECONDARY_RATE_LIMIT_DEFAULT_DELAY * (int)Math.Pow(2, retryCount);
+        return _secondaryRateLimitDefaultDelay * (int)Math.Pow(2, retryCount);
     }
 }

src/OctoshiftCLI.IntegrationTests/AdoToGithub.cs

@@ -23,19 +23,23 @@ protected AdoToGithub(ITestOutputHelper output, string adoServerUrl = "https://d
             StartTime = DateTime.Now;
             _output = output;
 
+            TestHelper.AssertCredentialsPresent(
+                (adoPatEnvVar, "Azure DevOps personal access token"),
+                ("GHEC_PAT", "GitHub Enterprise Cloud personal access token"));
+
             var logger = new OctoLogger(x => { }, x => _output.WriteLine(x), x => { }, x => { });
 
             _versionClient = new HttpClient();
             var adoToken = Environment.GetEnvironmentVariable(adoPatEnvVar);
             _adoHttpClient = new HttpClient();
-            var retryPolicy = new RetryPolicy(logger);
+            var retryPolicy = new RetryPolicy(logger, $"Azure DevOps ({adoPatEnvVar})");
             var adoClient = new AdoClient(logger, _adoHttpClient, new VersionChecker(_versionClient, logger), retryPolicy, adoToken);
             var adoApi = new AdoApi(adoClient, adoServerUrl, logger);
 
             var githubToken = Environment.GetEnvironmentVariable("GHEC_PAT");
             _githubHttpClient = new HttpClient();
-            var githubClient = new GithubClient(logger, _githubHttpClient, new VersionChecker(_versionClient, logger), new RetryPolicy(logger), new DateTimeProvider(), githubToken);
-            var githubApi = new GithubApi(githubClient, "https://api.github.com", new RetryPolicy(logger), null);
+            var githubClient = new GithubClient(logger, _githubHttpClient, new VersionChecker(_versionClient, logger), new RetryPolicy(logger, "GitHub (GHEC_PAT)"), new DateTimeProvider(), githubToken);
+            var githubApi = new GithubApi(githubClient, "https://api.github.com", new RetryPolicy(logger, "GitHub (GHEC_PAT)"), null);
 
             Tokens = new Dictionary<string, string>
             {

src/OctoshiftCLI.IntegrationTests/BbsToGithub.cs

@@ -39,12 +39,19 @@ public BbsToGithub(ITestOutputHelper output)
         _startTime = DateTime.Now;
         _output = output;
 
+        var azureStorageEnvVar = $"AZURE_STORAGE_CONNECTION_STRING_BBS_{TestHelper.GetOsName().ToUpperInvariant()}";
+        TestHelper.AssertCredentialsPresent(
+            ("BBS_USERNAME", "Bitbucket Server username"),
+            ("BBS_PASSWORD", "Bitbucket Server password"),
+            ("GHEC_PAT", "GitHub Enterprise Cloud personal access token"),
+            (azureStorageEnvVar, "Azure blob storage connection string for BBS migration archives"));
+
         _logger = new OctoLogger(_ => { }, x => _output.WriteLine(x), _ => { }, _ => { });
 
         var sourceBbsUsername = Environment.GetEnvironmentVariable("BBS_USERNAME");
         var sourceBbsPassword = Environment.GetEnvironmentVariable("BBS_PASSWORD");
         var targetGithubToken = Environment.GetEnvironmentVariable("GHEC_PAT");
-        _azureStorageConnectionString = Environment.GetEnvironmentVariable($"AZURE_STORAGE_CONNECTION_STRING_BBS_{TestHelper.GetOsName().ToUpper()}");
+        _azureStorageConnectionString = Environment.GetEnvironmentVariable(azureStorageEnvVar);
         _tokens = new Dictionary<string, string>
         {
             ["BBS_USERNAME"] = sourceBbsUsername,
@@ -55,14 +62,14 @@ public BbsToGithub(ITestOutputHelper output)
         _versionClient = new HttpClient();
 
         _sourceBbsHttpClient = new HttpClient();
-        _sourceBbsClient = new BbsClient(_logger, _sourceBbsHttpClient, new VersionChecker(_versionClient, _logger), new RetryPolicy(_logger), sourceBbsUsername, sourceBbsPassword);
+        _sourceBbsClient = new BbsClient(_logger, _sourceBbsHttpClient, new VersionChecker(_versionClient, _logger), new RetryPolicy(_logger, "Bitbucket Server (BBS_USERNAME/BBS_PASSWORD)"), sourceBbsUsername, sourceBbsPassword);
 
         _targetGithubHttpClient = new HttpClient();
-        _targetGithubClient = new GithubClient(_logger, _targetGithubHttpClient, new VersionChecker(_versionClient, _logger), new RetryPolicy(_logger), new DateTimeProvider(), targetGithubToken);
-        var retryPolicy = new RetryPolicy(_logger);
+        _targetGithubClient = new GithubClient(_logger, _targetGithubHttpClient, new VersionChecker(_versionClient, _logger), new RetryPolicy(_logger, "GitHub (GHEC_PAT)"), new DateTimeProvider(), targetGithubToken);
+        var retryPolicy = new RetryPolicy(_logger, "GitHub (GHEC_PAT)");
         var environmentVariableProvider = new EnvironmentVariableProvider(_logger);
         _archiveUploader = new ArchiveUploader(_targetGithubClient, UPLOADS_URL, _logger, retryPolicy, environmentVariableProvider);
-        _targetGithubApi = new GithubApi(_targetGithubClient, "https://api.github.com", new RetryPolicy(_logger), _archiveUploader);
+        _targetGithubApi = new GithubApi(_targetGithubClient, "https://api.github.com", new RetryPolicy(_logger, "GitHub (GHEC_PAT)"), _archiveUploader);
 
         _blobServiceClient = new BlobServiceClient(_azureStorageConnectionString);
 

src/OctoshiftCLI.IntegrationTests/GhesToGithub.cs

@@ -36,29 +36,35 @@ public GhesToGithub(ITestOutputHelper output)
         _startTime = DateTime.Now;
         _output = output;
 
+        var azureStorageEnvVar = $"AZURE_STORAGE_CONNECTION_STRING_GHES_{TestHelper.GetOsName().ToUpperInvariant()}";
+        TestHelper.AssertCredentialsPresent(
+            ("GHES_PAT", "GitHub Enterprise Server personal access token"),
+            ("GHEC_PAT", "GitHub Enterprise Cloud personal access token"),
+            (azureStorageEnvVar, "Azure blob storage connection string for GHES migration archives"));
+
         var logger = new OctoLogger(_ => { }, x => _output.WriteLine(x), _ => { }, _ => { });
 
         var sourceGithubToken = Environment.GetEnvironmentVariable("GHES_PAT");
         var targetGithubToken = Environment.GetEnvironmentVariable("GHEC_PAT");
-        _azureStorageConnectionString = Environment.GetEnvironmentVariable($"AZURE_STORAGE_CONNECTION_STRING_GHES_{TestHelper.GetOsName().ToUpper()}");
+        _azureStorageConnectionString = Environment.GetEnvironmentVariable(azureStorageEnvVar);
         _tokens = new Dictionary<string, string>
         {
             ["GH_SOURCE_PAT"] = sourceGithubToken,
             ["GH_PAT"] = targetGithubToken,
         };
 
         _versionClient = new HttpClient();
-        var retryPolicy = new RetryPolicy(logger);
+        var retryPolicy = new RetryPolicy(logger, "GHES (GHES_PAT)");
         var environmentVariableProvider = new EnvironmentVariableProvider(logger);
 
         _sourceGithubHttpClient = new HttpClient();
-        _sourceGithubClient = new GithubClient(logger, _sourceGithubHttpClient, new VersionChecker(_versionClient, logger), new RetryPolicy(logger), new DateTimeProvider(), sourceGithubToken);
+        _sourceGithubClient = new GithubClient(logger, _sourceGithubHttpClient, new VersionChecker(_versionClient, logger), new RetryPolicy(logger, "GHES (GHES_PAT)"), new DateTimeProvider(), sourceGithubToken);
         _archiveUploader = new ArchiveUploader(_targetGithubClient, UPLOADS_URL, logger, retryPolicy, environmentVariableProvider);
-        _sourceGithubApi = new GithubApi(_sourceGithubClient, GHES_API_URL, new RetryPolicy(logger), _archiveUploader);
+        _sourceGithubApi = new GithubApi(_sourceGithubClient, GHES_API_URL, new RetryPolicy(logger, "GHES (GHES_PAT)"), _archiveUploader);
 
         _targetGithubHttpClient = new HttpClient();
-        _targetGithubClient = new GithubClient(logger, _targetGithubHttpClient, new VersionChecker(_versionClient, logger), new RetryPolicy(logger), new DateTimeProvider(), targetGithubToken);
-        _targetGithubApi = new GithubApi(_targetGithubClient, "https://api.github.com", new RetryPolicy(logger), _archiveUploader);
+        _targetGithubClient = new GithubClient(logger, _targetGithubHttpClient, new VersionChecker(_versionClient, logger), new RetryPolicy(logger, "GitHub (GHEC_PAT)"), new DateTimeProvider(), targetGithubToken);
+        _targetGithubApi = new GithubApi(_targetGithubClient, "https://api.github.com", new RetryPolicy(logger, "GitHub (GHEC_PAT)"), _archiveUploader);
 
         _blobServiceClient = new BlobServiceClient(_azureStorageConnectionString);