From 554627226fd33e4575d52710fabe27704bdc0588 Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Mon, 10 Sep 2018 09:31:12 -0400 Subject: [PATCH 1/8] Add GitHubAccessToken general configuration option --- .../Configuration/GeneralConfiguration.cs | 5 +++++ src/Tgstation.Server.Host/appsettings.Docker.json | 4 +++- src/Tgstation.Server.Host/appsettings.json | 3 ++- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Tgstation.Server.Host/Configuration/GeneralConfiguration.cs b/src/Tgstation.Server.Host/Configuration/GeneralConfiguration.cs index 6b7884b399..0451038e48 100644 --- a/src/Tgstation.Server.Host/Configuration/GeneralConfiguration.cs +++ b/src/Tgstation.Server.Host/Configuration/GeneralConfiguration.cs @@ -24,5 +24,10 @@ /// Minimum length of database user passwords /// public uint MinimumPasswordLength { get; set; } + + /// + /// A GitHub personal access token to use for bypassing rate limits on requests. Requires no scopes + /// + public string GitHubAccessToken { get; set; } } } diff --git a/src/Tgstation.Server.Host/appsettings.Docker.json b/src/Tgstation.Server.Host/appsettings.Docker.json index d90c93f5a5..f7d5860d86 100644 --- a/src/Tgstation.Server.Host/appsettings.Docker.json +++ b/src/Tgstation.Server.Host/appsettings.Docker.json @@ -1,6 +1,8 @@ { "General": { - "LogFileDirectory": "/tgs_logs" + "LogFileDirectory": "/tgs_logs", + "MinimumPasswordLength": 15, + "GitHubAccessToken": null }, "Database": { "DatabaseType": "SqlServer or MySQL or MariaDB", diff --git a/src/Tgstation.Server.Host/appsettings.json b/src/Tgstation.Server.Host/appsettings.json index d16d0bb4f5..6caafa8898 100644 --- a/src/Tgstation.Server.Host/appsettings.json +++ b/src/Tgstation.Server.Host/appsettings.json @@ -2,7 +2,8 @@ "General": { "LogFileDirectory": null, //use the default path "DisableFileLogging": false, - "MinimumPasswordLength": 15 + "MinimumPasswordLength": 15, + "GitHubAccessToken": null }, "Logging": { "IncludeScopes": false, From 992ab646d2419adfd8632a6029f75bc25ab8a0ae Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Mon, 10 Sep 2018 09:55:30 -0400 Subject: [PATCH 2/8] Use configured GitHubAccessToken when available for Octokit operations --- .../Controllers/AdministrationController.cs | 19 +++++++++++++++---- .../Controllers/RepositoryController.cs | 12 ++++++++++-- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/Tgstation.Server.Host/Controllers/AdministrationController.cs b/src/Tgstation.Server.Host/Controllers/AdministrationController.cs index a401011001..f73a6edc84 100644 --- a/src/Tgstation.Server.Host/Controllers/AdministrationController.cs +++ b/src/Tgstation.Server.Host/Controllers/AdministrationController.cs @@ -31,9 +31,9 @@ namespace Tgstation.Server.Host.Controllers const string RestartNotSupportedException = "This deployment of tgstation-server is lacking the Tgstation.Server.Host.Watchdog component. Restarts and version changes cannot be completed!"; /// - /// The for the + /// The for the /// - readonly IGitHubClient gitHubClient; + readonly IGitHubClientFactory gitHubClientFactory; /// /// The for the @@ -55,6 +55,11 @@ namespace Tgstation.Server.Host.Controllers /// readonly UpdatesConfiguration updatesConfiguration; + /// + /// The for the + /// + readonly GeneralConfiguration generalConfiguration; + /// /// Construct an /// @@ -66,13 +71,15 @@ namespace Tgstation.Server.Host.Controllers /// The value of /// The for the /// The containing value of - public AdministrationController(IDatabaseContext databaseContext, IAuthenticationContextFactory authenticationContextFactory, IGitHubClient gitHubClient, IServerControl serverUpdater, IApplication application, IIOManager ioManager, ILogger logger, IOptions updatesConfigurationOptions) : base(databaseContext, authenticationContextFactory, logger, false) + /// The containing value of + public AdministrationController(IDatabaseContext databaseContext, IAuthenticationContextFactory authenticationContextFactory, IGitHubClientFactory gitHubClientFactory, IServerControl serverUpdater, IApplication application, IIOManager ioManager, ILogger logger, IOptions updatesConfigurationOptions, IOptions generalConfigurationOptions) : base(databaseContext, authenticationContextFactory, logger, false) { - this.gitHubClient = gitHubClient ?? throw new ArgumentNullException(nameof(gitHubClient)); + this.gitHubClientFactory = gitHubClientFactory ?? throw new ArgumentNullException(nameof(gitHubClientFactory)); this.serverUpdater = serverUpdater ?? throw new ArgumentNullException(nameof(serverUpdater)); this.application = application ?? throw new ArgumentNullException(nameof(application)); this.ioManager = ioManager ?? throw new ArgumentNullException(nameof(ioManager)); updatesConfiguration = updatesConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(updatesConfigurationOptions)); + generalConfiguration = generalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(generalConfigurationOptions)); } StatusCodeResult RateLimit(RateLimitExceededException exception) @@ -83,6 +90,8 @@ namespace Tgstation.Server.Host.Controllers return StatusCode(429); } + IGitHubClient GetGitHubClient() => String.IsNullOrEmpty(generalConfiguration.GitHubAccessToken) ? gitHubClientFactory.CreateClient() : gitHubClientFactory.CreateClient(generalConfiguration.GitHubAccessToken); + /// [TgsAuthorize] public override async Task Read(CancellationToken cancellationToken) @@ -93,6 +102,7 @@ namespace Tgstation.Server.Host.Controllers Uri repoUrl = null; try { + var gitHubClient = GetGitHubClient(); var repositoryTask = gitHubClient.Repository.Get(updatesConfiguration.GitHubRepositoryId); var releases = (await gitHubClient.Repository.Release.GetAll(updatesConfiguration.GitHubRepositoryId).ConfigureAwait(false)).Where(x => x.TagName.StartsWith(updatesConfiguration.GitTagPrefix, StringComparison.InvariantCulture)); @@ -137,6 +147,7 @@ namespace Tgstation.Server.Host.Controllers IEnumerable releases; try { + var gitHubClient = GetGitHubClient(); releases = (await gitHubClient.Repository.Release.GetAll(updatesConfiguration.GitHubRepositoryId).ConfigureAwait(false)).Where(x => x.TagName.StartsWith(updatesConfiguration.GitTagPrefix, StringComparison.InvariantCulture)); } catch (RateLimitExceededException e) diff --git a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs index 721e8b6bd7..50e95ce65d 100644 --- a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs +++ b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs @@ -2,6 +2,7 @@ using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using System; using System.Collections.Generic; using System.Globalization; @@ -15,6 +16,7 @@ using Tgstation.Server.Api; using Tgstation.Server.Api.Models; using Tgstation.Server.Api.Rights; using Tgstation.Server.Host.Components; +using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.Core; using Tgstation.Server.Host.Models; using Tgstation.Server.Host.Security; @@ -42,6 +44,11 @@ namespace Tgstation.Server.Host.Controllers /// readonly IJobManager jobManager; + /// + /// The for the + /// + readonly GeneralConfiguration generalConfiguration; + /// /// Construct a /// @@ -51,11 +58,12 @@ namespace Tgstation.Server.Host.Controllers /// The value of /// The value of /// The for the - public RepositoryController(IDatabaseContext databaseContext, IAuthenticationContextFactory authenticationContextFactory, IInstanceManager instanceManager, IGitHubClientFactory gitHubClientFactory, IJobManager jobManager, ILogger logger) : base(databaseContext, authenticationContextFactory, logger, true) + public RepositoryController(IDatabaseContext databaseContext, IAuthenticationContextFactory authenticationContextFactory, IInstanceManager instanceManager, IGitHubClientFactory gitHubClientFactory, IJobManager jobManager, ILogger logger, IOptions generalConfigurationOptions) : base(databaseContext, authenticationContextFactory, logger, true) { this.instanceManager = instanceManager ?? throw new ArgumentNullException(nameof(instanceManager)); this.gitHubClientFactory = gitHubClientFactory ?? throw new ArgumentNullException(nameof(gitHubClientFactory)); this.jobManager = jobManager ?? throw new ArgumentNullException(nameof(jobManager)); + generalConfiguration = generalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(generalConfigurationOptions)); } static async Task LoadRevisionInformation(Components.Repository.IRepository repository, IDatabaseContext databaseContext, Models.Instance instance, string lastOriginCommitSha, Action revInfoSink, CancellationToken cancellationToken) @@ -474,7 +482,7 @@ namespace Tgstation.Server.Host.Controllers foreach (var I in model.NewTestMerges.Where(x => String.IsNullOrWhiteSpace(x.PullRequestRevision))) I.PullRequestRevision = null; - var gitHubClient = currentModel.AccessToken != null ? gitHubClientFactory.CreateClient(currentModel.AccessToken) : gitHubClientFactory.CreateClient(); + var gitHubClient = String.IsNullOrEmpty(generalConfiguration.GitHubAccessToken) ? (currentModel.AccessToken != null ? gitHubClientFactory.CreateClient(currentModel.AccessToken) : gitHubClientFactory.CreateClient()) : gitHubClientFactory.CreateClient(generalConfiguration.GitHubAccessToken); var repoOwner = repo.GitHubOwner; var repoName = repo.GitHubRepoName; From b12bc979cdc5516b7883422b44a1c475357a6100 Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Mon, 10 Sep 2018 09:57:07 -0400 Subject: [PATCH 3/8] Add documentation about General:GitHubAccessToken --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 603fa83bbc..69d6028c32 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,8 @@ Create an `appsettings.Production.json` file next to `appsettings.json`. This wi - `General:MinimumPasswordLength`: Minimum password length requirement for database users +- `General:GitHubAccessToken`: Specify a GitHub personal access token with no scopes here to highly mitigate the possiblity of 429 response codes from GitHub requests + - `Logging:LogLevel:Default`: Can be one of `Trace`, `Debug`, `Information`, `Warning`, `Error`, or `Critical`. Restricts what is put into the log files. Currently `Debug` is reccommended for help with error reporting. - `Database:DatabaseType`: Can be one of `SqlServer`, `MariaDB`, or `MySql` From b2ae012598085ed86961105e461cac59f19d53a8 Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Mon, 10 Sep 2018 09:59:17 -0400 Subject: [PATCH 4/8] Add GitHubAccessToken support to the tests --- tests/Tgstation.Server.Tests/TestingServer.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/Tgstation.Server.Tests/TestingServer.cs b/tests/Tgstation.Server.Tests/TestingServer.cs index 3e30fbfa06..71cd1195eb 100644 --- a/tests/Tgstation.Server.Tests/TestingServer.cs +++ b/tests/Tgstation.Server.Tests/TestingServer.cs @@ -1,5 +1,6 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using System; +using System.Collections.Generic; using System.Globalization; using System.IO; using System.Threading; @@ -28,6 +29,7 @@ namespace Tgstation.Server.Tests //we have to rely on env vars var databaseType = Environment.GetEnvironmentVariable("TGS4_TEST_DATABASE_TYPE"); var connectionString = Environment.GetEnvironmentVariable("TGS4_TEST_CONNECTION_STRING"); + var gitHubAccessToken = Environment.GetEnvironmentVariable("TGS4_TEST_GITHUB_TOKEN"); if (String.IsNullOrEmpty(databaseType)) Assert.Fail("No database type configured in env var TGS4_TEST_DATABASE_TYPE!"); @@ -35,14 +37,19 @@ namespace Tgstation.Server.Tests if (String.IsNullOrEmpty(connectionString)) Assert.Fail("No connection string configured in env var TGS4_TEST_CONNECTION_STRING!"); - realServer = new ServerFactory().CreateServer(new string[] + var args = new List() { "--urls", Url.ToString(), String.Format(CultureInfo.InvariantCulture, "Database:DatabaseType={0}", databaseType), String.Format(CultureInfo.InvariantCulture, "Database:ConnectionString={0}", connectionString), "Database:DropDatabase=true" - }, null); + }; + + if (!String.IsNullOrEmpty(gitHubAccessToken)) + args.Add(String.Format(CultureInfo.InvariantCulture, "General:GitHubAccessToken={0}", gitHubAccessToken)); + + realServer = new ServerFactory().CreateServer(args.ToArray(), null); } public void Dispose() From caa5ce9ebccb341046c7f5c267afe21480b1d109 Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Mon, 10 Sep 2018 10:01:27 -0400 Subject: [PATCH 5/8] Add GitHubAccessToken support to appveyor --- appveyor.yml | 2 +- build/BuildDox.ps1 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 9bd18e311d..4874f5497a 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -4,7 +4,7 @@ pull_requests: environment: TGS4_TEST_DATABASE_TYPE: SqlServer TGS4_TEST_CONNECTION_STRING: Server=(local)\SQL2017;Initial Catalog=TGS_Test;User ID=sa;Password=Password12! - repo_token: + TGS4_TEST_GITHUB_TOKEN: secure: lJNGAXwiB5HlWdthz3K4PetqpTG5IEAyRgKaiKxFMQ8HW8CcOjRtB97B05op7BsK branches: only: diff --git a/build/BuildDox.ps1 b/build/BuildDox.ps1 index 81e88b9666..9087e16101 100644 --- a/build/BuildDox.ps1 +++ b/build/BuildDox.ps1 @@ -30,6 +30,6 @@ if($publish_dox){ echo "" > .nojekyll git add --all git commit -m "Deploy code docs to GitHub Pages for Appveyor build $Env:APPVEYOR_BUILD_NUMBER" -m "Commit: $Env:APPVEYOR_REPO_COMMIT" - git push -f "https://$Env:repo_token@$github_url" 2>&1 | out-null + git push -f "https://$Env:TGS4_TEST_GITHUB_TOKEN@$github_url" 2>&1 | out-null cd "$bf" } From 899c291123a1812a755e8dfc8d063c2b77ac844a Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Mon, 10 Sep 2018 10:25:41 -0400 Subject: [PATCH 6/8] Fix doc comments --- .../Controllers/AdministrationController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Controllers/AdministrationController.cs b/src/Tgstation.Server.Host/Controllers/AdministrationController.cs index f73a6edc84..e3d8fb5e6a 100644 --- a/src/Tgstation.Server.Host/Controllers/AdministrationController.cs +++ b/src/Tgstation.Server.Host/Controllers/AdministrationController.cs @@ -65,7 +65,7 @@ namespace Tgstation.Server.Host.Controllers /// /// The for the /// The for the - /// The value of + /// The value of /// The value of /// The value of /// The value of From 4377f1b0962c414869a232a17caa2879496bbb12 Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Mon, 10 Sep 2018 10:26:22 -0400 Subject: [PATCH 7/8] Remove IGitHubClient from dependency container --- src/Tgstation.Server.Host/Core/Application.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Core/Application.cs b/src/Tgstation.Server.Host/Core/Application.cs index a397f43b17..adccb77f87 100644 --- a/src/Tgstation.Server.Host/Core/Application.cs +++ b/src/Tgstation.Server.Host/Core/Application.cs @@ -175,7 +175,6 @@ namespace Tgstation.Server.Host.Core services.AddSingleton(); services.AddSingleton(); - services.AddSingleton(x => x.GetRequiredService().CreateClient()); if (isWindows) { From 93a78ed59cfd7cf2a87c11dd4d141f0e9fea89c8 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Mon, 10 Sep 2018 11:42:29 -0400 Subject: [PATCH 8/8] Add missing doc comment --- src/Tgstation.Server.Host/Controllers/RepositoryController.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs index 50e95ce65d..fe1040dee3 100644 --- a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs +++ b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs @@ -58,6 +58,7 @@ namespace Tgstation.Server.Host.Controllers /// The value of /// The value of /// The for the + /// The containing value of public RepositoryController(IDatabaseContext databaseContext, IAuthenticationContextFactory authenticationContextFactory, IInstanceManager instanceManager, IGitHubClientFactory gitHubClientFactory, IJobManager jobManager, ILogger logger, IOptions generalConfigurationOptions) : base(databaseContext, authenticationContextFactory, logger, true) { this.instanceManager = instanceManager ?? throw new ArgumentNullException(nameof(instanceManager));