From 213d9e9ea43de5eee2945be14f9e064a4b564513 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 31 Aug 2024 21:08:12 -0400 Subject: [PATCH 01/22] Add `launchTime` field to `DreamDaemonResponse` --- .../Models/Internal/DreamDaemonApiBase.cs | 10 +++++++++- .../Components/Session/SessionController.cs | 3 +++ .../Components/Watchdog/IWatchdog.cs | 5 +++++ .../Components/Watchdog/WatchdogBase.cs | 3 +++ .../Controllers/DreamDaemonController.cs | 1 + .../System/IProcessBase.cs | 8 +++++++- src/Tgstation.Server.Host/System/Process.cs | 17 +++++++++++++++++ .../Live/Instance/WatchdogTest.cs | 5 +++++ 8 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/Tgstation.Server.Api/Models/Internal/DreamDaemonApiBase.cs b/src/Tgstation.Server.Api/Models/Internal/DreamDaemonApiBase.cs index 6bde7948d03..ea5c16bc15b 100644 --- a/src/Tgstation.Server.Api/Models/Internal/DreamDaemonApiBase.cs +++ b/src/Tgstation.Server.Api/Models/Internal/DreamDaemonApiBase.cs @@ -1,4 +1,6 @@ -namespace Tgstation.Server.Api.Models.Internal +using System; + +namespace Tgstation.Server.Api.Models.Internal { /// /// Base class for DreamDaemon API models. @@ -11,6 +13,12 @@ public abstract class DreamDaemonApiBase : DreamDaemonSettings [ResponseOptions] public long? SessionId { get; set; } + /// + /// When the current server execution was started. + /// + [ResponseOptions] + public DateTimeOffset? LaunchTime { get; set; } + /// /// If the server is undergoing a soft reset. This may be automatically set by changes to other fields. /// diff --git a/src/Tgstation.Server.Host/Components/Session/SessionController.cs b/src/Tgstation.Server.Host/Components/Session/SessionController.cs index 754b672b7d6..77a25fd979a 100644 --- a/src/Tgstation.Server.Host/Components/Session/SessionController.cs +++ b/src/Tgstation.Server.Host/Components/Session/SessionController.cs @@ -124,6 +124,9 @@ async Task Wrap() /// public long? MemoryUsage => process.MemoryUsage; + /// + public DateTimeOffset? LaunchTime => process.LaunchTime; + /// /// The for the . /// diff --git a/src/Tgstation.Server.Host/Components/Watchdog/IWatchdog.cs b/src/Tgstation.Server.Host/Components/Watchdog/IWatchdog.cs index 2220c569327..38ab770d33d 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/IWatchdog.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/IWatchdog.cs @@ -19,6 +19,11 @@ public interface IWatchdog : IComponentService, IAsyncDisposable, IEventConsumer /// long? SessionId { get; } + /// + /// When the current server executions was started. + /// + DateTimeOffset? LaunchTime { get; } + /// /// The current . /// diff --git a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs index 5d9326ce7dd..21871a680c1 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs @@ -38,6 +38,9 @@ abstract class WatchdogBase : IWatchdog, ICustomCommandHandler, IRestartHandler /// public long? SessionId => GetActiveController()?.ReattachInformation.Id; + /// + public DateTimeOffset? LaunchTime => GetActiveController()?.LaunchTime; + /// public WatchdogStatus Status { diff --git a/src/Tgstation.Server.Host/Controllers/DreamDaemonController.cs b/src/Tgstation.Server.Host/Controllers/DreamDaemonController.cs index 79e03ebb845..6b4ceb1a858 100644 --- a/src/Tgstation.Server.Host/Controllers/DreamDaemonController.cs +++ b/src/Tgstation.Server.Host/Controllers/DreamDaemonController.cs @@ -366,6 +366,7 @@ ValueTask ReadImpl(DreamDaemonSettings? settings, bool knownForce firstIteration = false; result.Status = dd.Status; result.SessionId = dd.SessionId; + result.LaunchTime = dd.LaunchTime; } while (result.Status == WatchdogStatus.Online && !result.SessionId.HasValue); // this is the one invalid combo, it's not that racy diff --git a/src/Tgstation.Server.Host/System/IProcessBase.cs b/src/Tgstation.Server.Host/System/IProcessBase.cs index 6ae88dce502..b95b0bf9282 100644 --- a/src/Tgstation.Server.Host/System/IProcessBase.cs +++ b/src/Tgstation.Server.Host/System/IProcessBase.cs @@ -1,4 +1,5 @@ -using System.Threading; +using System; +using System.Threading; using System.Threading.Tasks; namespace Tgstation.Server.Host.System @@ -8,6 +9,11 @@ namespace Tgstation.Server.Host.System /// public interface IProcessBase { + /// + /// When the process was started. + /// + DateTimeOffset? LaunchTime { get; } + /// /// The resulting in the exit code of the process or if the process was detached. /// diff --git a/src/Tgstation.Server.Host/System/Process.cs b/src/Tgstation.Server.Host/System/Process.cs index e180cfc3cc6..76f9ef56f36 100644 --- a/src/Tgstation.Server.Host/System/Process.cs +++ b/src/Tgstation.Server.Host/System/Process.cs @@ -16,6 +16,23 @@ sealed class Process : IProcess /// public int Id { get; } + /// + public DateTimeOffset? LaunchTime + { + get + { + try + { + return handle.StartTime; + } + catch (Exception ex) + { + logger.LogWarning(ex, "Failed to get PID {pid}'s memory usage!", Id); + return null; + } + } + } + /// public Task Startup { get; } diff --git a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs index 08f436b26df..241c156dba8 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs @@ -682,6 +682,7 @@ async Task RunBasicTest(CancellationToken cancellationToken) Assert.AreEqual(WatchdogStatus.Offline, daemonStatus.Status.Value); Assert.IsNotNull(daemonStatus.ActiveCompileJob); Assert.IsFalse(daemonStatus.SessionId.HasValue); + Assert.IsFalse(daemonStatus.LaunchTime.HasValue); Assert.IsNull(daemonStatus.StagedCompileJob); Assert.AreEqual(DMApiConstants.InteropVersion, daemonStatus.ActiveCompileJob.DMApiVersion); Assert.AreEqual(DreamDaemonSecurity.Trusted, daemonStatus.ActiveCompileJob.MinimumSecurityLevel); @@ -730,6 +731,7 @@ async Task RunBasicTest(CancellationToken cancellationToken) daemonStatus = await instanceClient.DreamDaemon.Read(cancellationToken); Assert.AreEqual(WatchdogStatus.Offline, daemonStatus.Status.Value); Assert.IsFalse(daemonStatus.SessionId.HasValue); + Assert.IsFalse(daemonStatus.LaunchTime.HasValue); await ExpectGameDirectoryCount(1, cancellationToken); await CheckDMApiFail(daemonStatus.ActiveCompileJob, cancellationToken, false); @@ -741,12 +743,15 @@ async Task RunBasicTest(CancellationToken cancellationToken) }, cancellationToken); Assert.AreEqual(string.Empty, daemonStatus.AdditionalParameters); Assert.IsFalse(daemonStatus.SessionId.HasValue); + Assert.IsFalse(daemonStatus.LaunchTime.HasValue); } long? sessionIdTracker; void ValidateSessionId(DreamDaemonResponse daemonStatus, bool? knownIncrease) { Assert.IsTrue(daemonStatus.SessionId.HasValue, $"Expected a session ID in the DreamDaemonResponse"); + Assert.IsTrue(daemonStatus.LaunchTime.HasValue); + Assert.IsTrue(daemonStatus.LaunchTime.Value >= DateTimeOffset.UtcNow.AddHours(-1)); if (sessionIdTracker.HasValue) if (knownIncrease.HasValue) From c5341b447137dd1bbcc158d96aacd1e92ec6ba52 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 31 Aug 2024 21:16:00 -0400 Subject: [PATCH 02/22] Move SystemD arg responsibility from service to Host Watchdog --- build/Version.props | 2 +- build/tgstation-server.service | 2 +- src/Tgstation.Server.Host.Console/Program.cs | 7 +++++++ .../Tgstation.Server.Host.Console.csproj | 2 ++ 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/build/Version.props b/build/Version.props index af7c24e51a7..2b9c5d5073f 100644 --- a/build/Version.props +++ b/build/Version.props @@ -11,7 +11,7 @@ 16.0.0 7.2.1 5.9.0 - 1.4.1 + 1.5.0 1.2.1 2.0.0 netstandard2.0 diff --git a/build/tgstation-server.service b/build/tgstation-server.service index 98c03fee57c..44ce74fe2e4 100644 --- a/build/tgstation-server.service +++ b/build/tgstation-server.service @@ -11,7 +11,7 @@ User=tgstation-server Type=notify-reload NotifyAccess=all WorkingDirectory=/opt/tgstation-server -ExecStart=/usr/bin/dotnet Tgstation.Server.Host.Console.dll --appsettings-base-path=/etc/tgstation-server --General:SetupWizardMode=Never --Internal:UsingSystemD=true +ExecStart=/usr/bin/dotnet Tgstation.Server.Host.Console.dll --appsettings-base-path=/etc/tgstation-server --General:SetupWizardMode=Never TimeoutStartSec=600 Restart=always KillMode=process diff --git a/src/Tgstation.Server.Host.Console/Program.cs b/src/Tgstation.Server.Host.Console/Program.cs index c8383865af4..26f5b8f5b55 100644 --- a/src/Tgstation.Server.Host.Console/Program.cs +++ b/src/Tgstation.Server.Host.Console/Program.cs @@ -1,10 +1,12 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Reflection; using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.Hosting.Systemd; using Microsoft.Extensions.Logging; using Tgstation.Server.Common; @@ -44,6 +46,11 @@ internal static async Task Main(string[] args) var trace = arguments.Remove("--trace-host-watchdog"); var debug = arguments.Remove("--debug-host-watchdog"); + const string SystemDArg = "--Internal:UsingSystemD=true"; + if (!arguments.Any(arg => arg.Equals(SystemDArg, StringComparison.OrdinalIgnoreCase)) + && SystemdHelpers.IsSystemdService()) + arguments.Add(SystemDArg); + using var loggerFactory = LoggerFactory.Create(builder => { if (trace) diff --git a/src/Tgstation.Server.Host.Console/Tgstation.Server.Host.Console.csproj b/src/Tgstation.Server.Host.Console/Tgstation.Server.Host.Console.csproj index 3cbf1c1a163..a405553d55a 100644 --- a/src/Tgstation.Server.Host.Console/Tgstation.Server.Host.Console.csproj +++ b/src/Tgstation.Server.Host.Console/Tgstation.Server.Host.Console.csproj @@ -11,6 +11,8 @@ + + From 442ad8001745dbaedce2a760fbbaf0c8762dc102 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 31 Aug 2024 21:58:17 -0400 Subject: [PATCH 03/22] Add client count to DreamDaemon responses Closes #1909 --- build/Version.props | 4 ++-- src/DMAPI/tgs.dm | 2 +- src/DMAPI/tgs/v5/__interop_version.dm | 2 +- src/DMAPI/tgs/v5/_defines.dm | 1 + src/DMAPI/tgs/v5/topic.dm | 4 +++- src/DMAPI/tgs/v5/undefs.dm | 3 ++- .../Models/Internal/DreamDaemonApiBase.cs | 6 ++++++ .../Components/Interop/Topic/TopicResponse.cs | 5 +++++ .../Components/Watchdog/IWatchdog.cs | 5 +++++ .../Components/Watchdog/WatchdogBase.cs | 8 ++++++++ .../Controllers/DreamDaemonController.cs | 1 + .../Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs | 5 +++++ 12 files changed, 40 insertions(+), 6 deletions(-) diff --git a/build/Version.props b/build/Version.props index 2b9c5d5073f..a6365b35adb 100644 --- a/build/Version.props +++ b/build/Version.props @@ -9,8 +9,8 @@ 7.0.0 13.7.0 16.0.0 - 7.2.1 - 5.9.0 + 7.3.0 + 5.10.0 1.5.0 1.2.1 2.0.0 diff --git a/src/DMAPI/tgs.dm b/src/DMAPI/tgs.dm index 4766b3dfe66..42f2d5fc31f 100644 --- a/src/DMAPI/tgs.dm +++ b/src/DMAPI/tgs.dm @@ -1,7 +1,7 @@ // tgstation-server DMAPI // The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in IETF RFC 2119. -#define TGS_DMAPI_VERSION "7.2.1" +#define TGS_DMAPI_VERSION "7.3.0" // All functions and datums outside this document are subject to change with any version and should not be relied on. diff --git a/src/DMAPI/tgs/v5/__interop_version.dm b/src/DMAPI/tgs/v5/__interop_version.dm index f4806f7adb9..29ea239ad84 100644 --- a/src/DMAPI/tgs/v5/__interop_version.dm +++ b/src/DMAPI/tgs/v5/__interop_version.dm @@ -1 +1 @@ -"5.9.0" +"5.10.0" diff --git a/src/DMAPI/tgs/v5/_defines.dm b/src/DMAPI/tgs/v5/_defines.dm index 92c7a8388a7..a47bfd78000 100644 --- a/src/DMAPI/tgs/v5/_defines.dm +++ b/src/DMAPI/tgs/v5/_defines.dm @@ -95,6 +95,7 @@ #define DMAPI5_TOPIC_PARAMETER_NEW_SERVER_VERSION "newServerVersion" #define DMAPI5_TOPIC_PARAMETER_BROADCAST_MESSAGE "broadcastMessage" +#define DMAPI5_TOPIC_RESPONSE_CLIENT_COUNT "clientCount" #define DMAPI5_TOPIC_RESPONSE_COMMAND_RESPONSE "commandResponse" #define DMAPI5_TOPIC_RESPONSE_COMMAND_RESPONSE_MESSAGE "commandResponseMessage" #define DMAPI5_TOPIC_RESPONSE_CHAT_RESPONSES "chatResponses" diff --git a/src/DMAPI/tgs/v5/topic.dm b/src/DMAPI/tgs/v5/topic.dm index e1f2cb63857..59e5e63e5cd 100644 --- a/src/DMAPI/tgs/v5/topic.dm +++ b/src/DMAPI/tgs/v5/topic.dm @@ -149,7 +149,9 @@ if(DMAPI5_TOPIC_COMMAND_HEALTHCHECK) if(event_handler && event_handler.receive_health_checks) event_handler.HandleEvent(TGS_EVENT_HEALTH_CHECK) - return TopicResponse() + var/list/health_check_response = TopicResponse() + health_check_response[DMAPI5_TOPIC_RESPONSE_CLIENT_COUNT] = TGS_CLIENT_COUNT + return health_check_response; if(DMAPI5_TOPIC_COMMAND_WATCHDOG_REATTACH) detached = FALSE diff --git a/src/DMAPI/tgs/v5/undefs.dm b/src/DMAPI/tgs/v5/undefs.dm index 237207fdfd0..d8dbf7a7818 100644 --- a/src/DMAPI/tgs/v5/undefs.dm +++ b/src/DMAPI/tgs/v5/undefs.dm @@ -17,8 +17,8 @@ #undef DMAPI5_BRIDGE_COMMAND_EVENT #undef DMAPI5_PARAMETER_ACCESS_IDENTIFIER +#undef DMAPI5_PARAMETER_CLIENT_COUNT #undef DMAPI5_PARAMETER_CUSTOM_COMMANDS -#undef DMAPI5_PARAMETER_TOPIC_PORT #undef DMAPI5_CHUNK #undef DMAPI5_CHUNK_PAYLOAD @@ -95,6 +95,7 @@ #undef DMAPI5_TOPIC_PARAMETER_NEW_SERVER_VERSION #undef DMAPI5_TOPIC_PARAMETER_BROADCAST_MESSAGE +#undef DMAPI5_TOPIC_RESPONSE_CLIENT_COUNT #undef DMAPI5_TOPIC_RESPONSE_COMMAND_RESPONSE #undef DMAPI5_TOPIC_RESPONSE_COMMAND_RESPONSE_MESSAGE #undef DMAPI5_TOPIC_RESPONSE_CHAT_RESPONSES diff --git a/src/Tgstation.Server.Api/Models/Internal/DreamDaemonApiBase.cs b/src/Tgstation.Server.Api/Models/Internal/DreamDaemonApiBase.cs index ea5c16bc15b..e3411b8115f 100644 --- a/src/Tgstation.Server.Api/Models/Internal/DreamDaemonApiBase.cs +++ b/src/Tgstation.Server.Api/Models/Internal/DreamDaemonApiBase.cs @@ -19,6 +19,12 @@ public abstract class DreamDaemonApiBase : DreamDaemonSettings [ResponseOptions] public DateTimeOffset? LaunchTime { get; set; } + /// + /// The last known count of connected players. Requires to not be 0 and a game server interop version >= 5.10.0 to populate. + /// + [ResponseOptions] + public int? ClientCount { get; set; } + /// /// If the server is undergoing a soft reset. This may be automatically set by changes to other fields. /// diff --git a/src/Tgstation.Server.Host/Components/Interop/Topic/TopicResponse.cs b/src/Tgstation.Server.Host/Components/Interop/Topic/TopicResponse.cs index 12db6f3ef17..e5b30150de8 100644 --- a/src/Tgstation.Server.Host/Components/Interop/Topic/TopicResponse.cs +++ b/src/Tgstation.Server.Host/Components/Interop/Topic/TopicResponse.cs @@ -36,5 +36,10 @@ sealed class TopicResponse : DMApiResponse, IMissingPayloadsCommunication /// public IReadOnlyCollection? MissingChunks { get; set; } + + /// + /// The number of connected clients to the game. Added in Interop 5.10.0. + /// + public int? ClientCount { get; } } } diff --git a/src/Tgstation.Server.Host/Components/Watchdog/IWatchdog.cs b/src/Tgstation.Server.Host/Components/Watchdog/IWatchdog.cs index 38ab770d33d..35da1504f83 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/IWatchdog.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/IWatchdog.cs @@ -24,6 +24,11 @@ public interface IWatchdog : IComponentService, IAsyncDisposable, IEventConsumer /// DateTimeOffset? LaunchTime { get; } + /// + /// Last known client count queried from the DMAPI. Requires health checks to be enabled to populate. + /// + int? ClientCount { get; } + /// /// The current . /// diff --git a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs index 21871a680c1..3c67ca495c1 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs @@ -38,6 +38,9 @@ abstract class WatchdogBase : IWatchdog, ICustomCommandHandler, IRestartHandler /// public long? SessionId => GetActiveController()?.ReattachInformation.Id; + /// + public int? ClientCount { get; private set; } + /// public DateTimeOffset? LaunchTime => GetActiveController()?.LaunchTime; @@ -570,6 +573,7 @@ protected async ValueTask LaunchNoLock( // since neither server is running, this is safe to do LastLaunchParameters = ActiveLaunchParameters; healthChecksMissed = 0; + ClientCount = null; try { @@ -667,6 +671,7 @@ protected async ValueTask ReattachFailure(CancellationToken cancellationToken) // we lost the server, just restart entirely // DCT: Operation must always run await DisposeAndNullControllers(CancellationToken.None); + ClientCount = null; const string FailReattachMessage = "Unable to properly reattach to server! Restarting watchdog..."; Logger.LogWarning(FailReattachMessage); @@ -1183,7 +1188,10 @@ async ValueTask HandleHealthCheck(CancellationToken cancellationT } } else + { healthChecksMissed = 0; + ClientCount = response.ClientCount; + } return MonitorAction.Continue; } diff --git a/src/Tgstation.Server.Host/Controllers/DreamDaemonController.cs b/src/Tgstation.Server.Host/Controllers/DreamDaemonController.cs index 6b4ceb1a858..e6760f0f177 100644 --- a/src/Tgstation.Server.Host/Controllers/DreamDaemonController.cs +++ b/src/Tgstation.Server.Host/Controllers/DreamDaemonController.cs @@ -367,6 +367,7 @@ ValueTask ReadImpl(DreamDaemonSettings? settings, bool knownForce result.Status = dd.Status; result.SessionId = dd.SessionId; result.LaunchTime = dd.LaunchTime; + result.ClientCount = dd.ClientCount; } while (result.Status == WatchdogStatus.Online && !result.SessionId.HasValue); // this is the one invalid combo, it's not that racy diff --git a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs index 241c156dba8..d7930e0cc61 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs @@ -753,6 +753,9 @@ void ValidateSessionId(DreamDaemonResponse daemonStatus, bool? knownIncrease) Assert.IsTrue(daemonStatus.LaunchTime.HasValue); Assert.IsTrue(daemonStatus.LaunchTime.Value >= DateTimeOffset.UtcNow.AddHours(-1)); + if (daemonStatus.ClientCount.HasValue) + Assert.AreEqual(0, daemonStatus.ClientCount.Value); + if (sessionIdTracker.HasValue) if (knownIncrease.HasValue) if (knownIncrease.Value) @@ -861,6 +864,8 @@ async Task RunHealthCheckTest(bool checkDump, CancellationToken cancellationToke // Ensure it's responding to health checks await Task.WhenAny(Task.Delay(7000, cancellationToken), ourProcessHandler.Lifetime); Assert.IsFalse(ddProc.HasExited); + var daemonStatus = await instanceClient.DreamDaemon.Read(cancellationToken); + Assert.AreEqual(0, daemonStatus.ClientCount); // check DD agrees var topicRequestResult = await SendTestTopic( From 33dd07152dee981bb7d4359b11d7c0e8f11079f5 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 31 Aug 2024 21:58:27 -0400 Subject: [PATCH 04/22] API Bump to 10.8 --- build/Version.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Version.props b/build/Version.props index a6365b35adb..92901d070f6 100644 --- a/build/Version.props +++ b/build/Version.props @@ -5,7 +5,7 @@ 6.9.2 5.2.0 - 10.7.0 + 10.8.0 7.0.0 13.7.0 16.0.0 From debeba2aa22c8eced4966fed3a3e2ab38a1c6b51 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 31 Aug 2024 21:58:43 -0400 Subject: [PATCH 05/22] Version bump to 6.10 --- build/Version.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Version.props b/build/Version.props index 92901d070f6..c6573e2f149 100644 --- a/build/Version.props +++ b/build/Version.props @@ -3,7 +3,7 @@ - 6.9.2 + 6.10.0 5.2.0 10.8.0 7.0.0 From 2a972427758a1cdbde75415862b8fe078fc3b938 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Mon, 2 Sep 2024 20:58:28 -0400 Subject: [PATCH 06/22] Minor spacing fix --- .github/workflows/auto-approve-dominions-prs.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/auto-approve-dominions-prs.yml b/.github/workflows/auto-approve-dominions-prs.yml index 791c34fc591..6bb495b2611 100644 --- a/.github/workflows/auto-approve-dominions-prs.yml +++ b/.github/workflows/auto-approve-dominions-prs.yml @@ -29,7 +29,7 @@ jobs: - name: GitHub API Call run: | curl --request POST \ - --url https://api.github.com/repos/${{github.repository}}/pulls/${{github.event.number}}/reviews \ + --url https://api.github.com/repos/${{ github.repository }}/pulls/${{github.event.number}}/reviews \ --header 'authorization: Bearer ${{ steps.app-token-generation.outputs.token }}' \ --header 'content-type: application/json' \ -d '{"event":"APPROVE"}' \ From 0f9448a39d4c3f4d7845b38376eafb5505793d0b Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Mon, 2 Sep 2024 20:59:22 -0400 Subject: [PATCH 07/22] Support for GitHub Apps as instance repository credentials --- .github/workflows/ci-pipeline.yml | 16 +++- .../Models/Internal/RepositorySettings.cs | 9 +- .../Models/RemoteGitProvider.cs | 2 +- .../Remote/GitHubRemoteDeploymentManager.cs | 50 +++++++++-- .../Repository/GitHubRemoteFeatures.cs | 50 ++++++----- .../Core/VersionReportingService.cs | 4 +- .../Utils/GitHub/GitHubClientFactory.cs | 43 +++++---- .../Utils/GitHub/GitHubServiceFactory.cs | 17 +++- .../Utils/GitHub/IGitHubClientFactory.cs | 10 +-- .../Utils/GitHub/IGitHubServiceFactory.cs | 9 ++ .../Utils/GitHub/RepositoryIdentifier.cs | 77 ++++++++++++++++ .../Utils/GitHub/TestGitHubClientFactory.cs | 88 +++++++++++++++++++ 12 files changed, 311 insertions(+), 64 deletions(-) create mode 100644 src/Tgstation.Server.Host/Utils/GitHub/RepositoryIdentifier.cs diff --git a/.github/workflows/ci-pipeline.yml b/.github/workflows/ci-pipeline.yml index 14080009664..07491565d23 100644 --- a/.github/workflows/ci-pipeline.yml +++ b/.github/workflows/ci-pipeline.yml @@ -432,8 +432,6 @@ jobs: matrix: configuration: ["Debug", "Release"] env: - TGS_TEST_DISCORD_TOKEN: ${{ secrets.DISCORD_TOKEN }} - TGS_TEST_IRC_CONNECTION_STRING: ${{ secrets.IRC_CONNECTION_STRING }} TGS_TELEMETRY_KEY_FILE: /tmp/tgs_telemetry_key.txt runs-on: ubuntu-latest steps: @@ -486,6 +484,12 @@ jobs: - name: Run Unit Tests run: sudo dotnet test --no-build --logger "GitHubActions;summary.includePassedTests=true;summary.includeSkippedTests=true" --filter TestCategory!=RequiresDatabase -c ${{ matrix.configuration }}NoWindows --collect:"XPlat Code Coverage" --settings build/ci.runsettings --results-directory ./TestResults tgstation-server.sln + env: + TGS_TEST_DISCORD_TOKEN: ${{ secrets.DISCORD_TOKEN }} + TGS_TEST_IRC_CONNECTION_STRING: ${{ secrets.IRC_CONNECTION_STRING }} + TGS_TEST_APP_ID: ${{ secrets.APP_ID }} + TGS_TEST_APP_PRIVATE_KEY: ${{ secrets.APP_PRIVATE_KEY }} + TGS_TEST_REPO_SLUG: ${{ github.repository }} - name: Store Code Coverage uses: actions/upload-artifact@v4 @@ -500,8 +504,6 @@ jobs: matrix: configuration: ["Debug", "Release"] env: - TGS_TEST_DISCORD_TOKEN: ${{ secrets.DISCORD_TOKEN }} - TGS_TEST_IRC_CONNECTION_STRING: ${{ secrets.IRC_CONNECTION_STRING }} TGS_TELEMETRY_KEY_FILE: C:/tgs_telemetry_key.txt runs-on: windows-latest steps: @@ -550,6 +552,12 @@ jobs: - name: Run Unit Tests run: dotnet test --no-build --logger "GitHubActions;summary.includePassedTests=true;summary.includeSkippedTests=true" --filter TestCategory!=RequiresDatabase -c ${{ matrix.configuration }}NoWix --collect:"XPlat Code Coverage" --settings build/ci.runsettings --results-directory ./TestResults tgstation-server.sln + env: + TGS_TEST_DISCORD_TOKEN: ${{ secrets.DISCORD_TOKEN }} + TGS_TEST_IRC_CONNECTION_STRING: ${{ secrets.IRC_CONNECTION_STRING }} + TGS_TEST_APP_ID: ${{ secrets.APP_ID }} + TGS_TEST_APP_PRIVATE_KEY: ${{ secrets.APP_PRIVATE_KEY }} + TGS_TEST_REPO_SLUG: ${{ github.repository }} - name: Store Code Coverage uses: actions/upload-artifact@v4 diff --git a/src/Tgstation.Server.Api/Models/Internal/RepositorySettings.cs b/src/Tgstation.Server.Api/Models/Internal/RepositorySettings.cs index 75a77ff00a7..7b916352d2a 100644 --- a/src/Tgstation.Server.Api/Models/Internal/RepositorySettings.cs +++ b/src/Tgstation.Server.Api/Models/Internal/RepositorySettings.cs @@ -7,6 +7,11 @@ namespace Tgstation.Server.Api.Models.Internal /// public class RepositorySettings { + /// + /// Prefix for TGS encoded app private keys. This is encoded in the format PREFIX + (APP_ID OR CLIENT_ID) + ':' + BASE64(APP_PRIVATE_KEY). + /// + public const string TgsAppPrivateKeyPrefix = "TGS_GHPK_"; + /// /// The name of the committer. /// @@ -23,14 +28,14 @@ public class RepositorySettings public string? CommitterEmail { get; set; } /// - /// The username to access the git repository with. + /// The username to access the git repository with. If using a TGS encoded app private key for , this should be the app's name. /// [StringLength(Limits.MaximumStringLength)] [ResponseOptions] public string? AccessUser { get; set; } /// - /// The token/password to access the git repository with. + /// The token/password to access the git repository with. Can also be a TGS encoded app private key. for details. /// [StringLength(Limits.MaximumStringLength)] [ResponseOptions(Presence = FieldPresence.Ignored)] diff --git a/src/Tgstation.Server.Api/Models/RemoteGitProvider.cs b/src/Tgstation.Server.Api/Models/RemoteGitProvider.cs index b90016a4229..80233a7422a 100644 --- a/src/Tgstation.Server.Api/Models/RemoteGitProvider.cs +++ b/src/Tgstation.Server.Api/Models/RemoteGitProvider.cs @@ -3,7 +3,7 @@ /// /// Indicates the remote git host. /// - public enum RemoteGitProvider + public enum RemoteGitProvider // Note if adding more: There's an assumption that all but unknown can derive a slug { /// /// Unknown remote git provider. diff --git a/src/Tgstation.Server.Host/Components/Deployment/Remote/GitHubRemoteDeploymentManager.cs b/src/Tgstation.Server.Host/Components/Deployment/Remote/GitHubRemoteDeploymentManager.cs index 9a04c0ac94e..a5283ed469b 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/Remote/GitHubRemoteDeploymentManager.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/Remote/GitHubRemoteDeploymentManager.cs @@ -76,11 +76,21 @@ await databaseContextFactory.UseContext( var instanceAuthenticated = repositorySettings!.AccessToken != null; IAuthenticatedGitHubService? authenticatedGitHubService; - IGitHubService gitHubService; + IGitHubService? gitHubService; if (instanceAuthenticated) { - authenticatedGitHubService = await gitHubServiceFactory.CreateService(repositorySettings.AccessToken!, cancellationToken); - gitHubService = authenticatedGitHubService; + authenticatedGitHubService = await gitHubServiceFactory.CreateService( + repositorySettings.AccessToken!, + new RepositoryIdentifier(remoteInformation), + cancellationToken); + + if (authenticatedGitHubService == null) + { + Logger.LogWarning("Can't create GitHub deployment as authentication for repository failed!"); + gitHubService = await gitHubServiceFactory.CreateService(cancellationToken); + } + else + gitHubService = authenticatedGitHubService; } else { @@ -99,12 +109,12 @@ await databaseContextFactory.UseContext( Logger.LogTrace("Not creating deployment"); else if (!instanceAuthenticated) Logger.LogWarning("Can't create GitHub deployment as no access token is set for repository!"); - else + else if (authenticatedGitHubService != null) { Logger.LogTrace("Creating deployment..."); try { - compileJob.GitHubDeploymentId = await authenticatedGitHubService!.CreateDeployment( + compileJob.GitHubDeploymentId = await authenticatedGitHubService.CreateDeployment( new NewDeployment(compileJob.RevisionInformation.CommitSha) { AutoMerge = false, @@ -175,7 +185,11 @@ public override async ValueTask> RemoveMergedTest } var gitHubService = repositorySettings.AccessToken != null - ? await gitHubServiceFactory.CreateService(repositorySettings.AccessToken, cancellationToken) + ? await gitHubServiceFactory.CreateService( + repositorySettings.AccessToken, + new RepositoryIdentifier(repository), + cancellationToken) + ?? await gitHubServiceFactory.CreateService(cancellationToken) : await gitHubServiceFactory.CreateService(cancellationToken); var tasks = revisionInformation @@ -255,7 +269,16 @@ protected override async ValueTask CommentOnTestMergeSource( int testMergeNumber, CancellationToken cancellationToken) { - var gitHubService = await gitHubServiceFactory.CreateService(repositorySettings.AccessToken!, cancellationToken); + var gitHubService = await gitHubServiceFactory.CreateService( + repositorySettings.AccessToken!, + new RepositoryIdentifier(remoteRepositoryOwner, remoteRepositoryName), + cancellationToken); + + if (gitHubService == null) + { + Logger.LogWarning("Error posting GitHub comment: Authentication failed!"); + return; + } try { @@ -343,7 +366,18 @@ await databaseContextFactory.UseContext( return; } - var gitHubService = await gitHubServiceFactory.CreateService(gitHubAccessToken, cancellationToken); + var gitHubService = await gitHubServiceFactory.CreateService( + gitHubAccessToken, + new RepositoryIdentifier(compileJob.GitHubRepoId.Value), + cancellationToken); + + if (gitHubService == null) + { + Logger.LogWarning( + "GitHub authentication failed, can't update to {deploymentState}!", + deploymentState); + return; + } try { diff --git a/src/Tgstation.Server.Host/Components/Repository/GitHubRemoteFeatures.cs b/src/Tgstation.Server.Host/Components/Repository/GitHubRemoteFeatures.cs index 69289bb5d40..7d34f78e542 100644 --- a/src/Tgstation.Server.Host/Components/Repository/GitHubRemoteFeatures.cs +++ b/src/Tgstation.Server.Host/Components/Repository/GitHubRemoteFeatures.cs @@ -49,33 +49,39 @@ public GitHubRemoteFeatures(IGitHubServiceFactory gitHubServiceFactory, ILogger< CancellationToken cancellationToken) { var gitHubService = repositorySettings.AccessToken != null - ? await gitHubServiceFactory.CreateService(repositorySettings.AccessToken, cancellationToken) + ? await gitHubServiceFactory.CreateService( + repositorySettings.AccessToken, + new RepositoryIdentifier(this), + cancellationToken) : await gitHubServiceFactory.CreateService(cancellationToken); PullRequest? pr = null; ApiException? exception = null; string? errorMessage = null; - try - { - pr = await gitHubService.GetPullRequest(RemoteRepositoryOwner, RemoteRepositoryName, parameters.Number, cancellationToken); - } - catch (RateLimitExceededException ex) - { - // you look at your anonymous access and sigh - errorMessage = "GITHUB API ERROR: RATE LIMITED"; - exception = ex; - } - catch (AuthorizationException ex) - { - errorMessage = "GITHUB API ERROR: BAD CREDENTIALS"; - exception = ex; - } - catch (NotFoundException ex) - { - // you look at your shithub and sigh - errorMessage = "GITHUB API ERROR: PULL REQUEST NOT FOUND"; - exception = ex; - } + if (gitHubService == null) + errorMessage = "GITHUB API ERROR: AUTH FAILURE"; + else + try + { + pr = await gitHubService.GetPullRequest(RemoteRepositoryOwner, RemoteRepositoryName, parameters.Number, cancellationToken); + } + catch (RateLimitExceededException ex) + { + // you look at your anonymous access and sigh + errorMessage = "GITHUB API ERROR: RATE LIMITED"; + exception = ex; + } + catch (AuthorizationException ex) + { + errorMessage = "GITHUB API ERROR: BAD CREDENTIALS"; + exception = ex; + } + catch (NotFoundException ex) + { + // you look at your shithub and sigh + errorMessage = "GITHUB API ERROR: PULL REQUEST NOT FOUND"; + exception = ex; + } if (exception != null) Logger.LogWarning(exception, "Error retrieving pull request metadata!"); diff --git a/src/Tgstation.Server.Host/Core/VersionReportingService.cs b/src/Tgstation.Server.Host/Core/VersionReportingService.cs index 6ed4b2812fe..3a4feeed378 100644 --- a/src/Tgstation.Server.Host/Core/VersionReportingService.cs +++ b/src/Tgstation.Server.Host/Core/VersionReportingService.cs @@ -222,9 +222,9 @@ async ValueTask TryReportVersion(Guid telemetryId, string serializedPem, l : $"\"{serverFriendlyName}\""); try { - var gitHubClient = await gitHubClientFactory.CreateInstallationClient( + var gitHubClient = await gitHubClientFactory.CreateClientForRepository( serializedPem, - repositoryId, + new RepositoryIdentifier(repositoryId), cancellationToken); if (gitHubClient == null) diff --git a/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs b/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs index 4cf31a5d289..d23874a302e 100644 --- a/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs +++ b/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs @@ -13,6 +13,7 @@ using Octokit; +using Tgstation.Server.Api.Models.Internal; using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.System; @@ -94,18 +95,18 @@ public async ValueTask CreateClient(string accessToken, Cancellat cancellationToken))!; /// - public ValueTask CreateInstallationClient(string serializedPem, long repositoryId, CancellationToken cancellationToken) - => GetOrCreateClient(serializedPem, repositoryId, cancellationToken); + public ValueTask CreateClientForRepository(string accessString, RepositoryIdentifier repositoryIdentifier, CancellationToken cancellationToken) + => GetOrCreateClient(accessString, repositoryIdentifier, cancellationToken); /// - /// Retrieve a from the or add a new one based on a given . + /// Retrieve a from the or add a new one based on a given . /// - /// Optional access token to use as credentials or GitHub App private key. If using a private key, must be set. - /// Setting this specifies is a private key and a GitHub App installation authenticated client will be returned. + /// Optional access token to use as credentials or GitHub App private key. If using a TGS encoded app private key, must be set. + /// The optional for the GitHub ID that the client will be used to connect to. Must be set if is a TGS encoded app private key. /// The for the operation. - /// A resulting in the for the given or if authentication failed. + /// A resulting in the for the given or if authentication failed. #pragma warning disable CA1506 // TODO: Decomplexify - async ValueTask GetOrCreateClient(string? accessTokenOrSerializedPem, long? installationRepositoryId, CancellationToken cancellationToken) + async ValueTask GetOrCreateClient(string? accessString, RepositoryIdentifier? repositoryIdentifier, CancellationToken cancellationToken) #pragma warning restore CA1506 { GitHubClient client; @@ -114,13 +115,13 @@ public async ValueTask CreateClient(string accessToken, Cancellat using (await SemaphoreSlimContext.Lock(clientCacheSemaphore, cancellationToken)) { string cacheKey; - if (String.IsNullOrWhiteSpace(accessTokenOrSerializedPem)) + if (String.IsNullOrWhiteSpace(accessString)) { - accessTokenOrSerializedPem = null; + accessString = null; cacheKey = DefaultCacheKey; } else - cacheKey = accessTokenOrSerializedPem; + cacheKey = accessString; cacheHit = clientCache.TryGetValue(cacheKey, out var tuple); @@ -134,12 +135,15 @@ public async ValueTask CreateClient(string accessToken, Cancellat product.Name, product.Version)); - if (accessTokenOrSerializedPem != null) + if (accessString != null) { - if (installationRepositoryId.HasValue) + if (accessString.StartsWith(RepositorySettings.TgsAppPrivateKeyPrefix)) { - logger.LogTrace("Performing GitHub App authentication for installation on repository {installationRepositoryId}", installationRepositoryId.Value); - var splits = accessTokenOrSerializedPem.Split(':'); + if (repositoryIdentifier == null) + throw new InvalidOperationException("Cannot create app installation key without target repositoryIdentifier!"); + + logger.LogTrace("Performing GitHub App authentication for installation on repository {installationRepositoryId}", repositoryIdentifier); + var splits = accessString.Split(':'); if (splits.Length != 2) { logger.LogError("Failed to parse serialized Client ID & PEM! Expected 2 chunks, got {chunkCount}", splits.Length); @@ -167,9 +171,11 @@ public async ValueTask CreateClient(string accessToken, Cancellat var nowDateTime = DateTime.UtcNow; + var appOrClientId = splits[0][RepositorySettings.TgsAppPrivateKeyPrefix.Length..]; + var jwt = jwtSecurityTokenHandler.CreateToken(new SecurityTokenDescriptor { - Issuer = splits[0], + Issuer = appOrClientId, Expires = nowDateTime.AddMinutes(10), IssuedAt = nowDateTime, SigningCredentials = signingCredentials, @@ -182,7 +188,10 @@ public async ValueTask CreateClient(string accessToken, Cancellat Installation installation; try { - installation = await client.GitHubApps.GetRepositoryInstallationForCurrent(installationRepositoryId.Value); + var installationTask = repositoryIdentifier.IsSlug + ? client.GitHubApps.GetRepositoryInstallationForCurrent(repositoryIdentifier.Owner, repositoryIdentifier.Name) + : client.GitHubApps.GetRepositoryInstallationForCurrent(repositoryIdentifier.RepositoryId.Value); + installation = await installationTask; } catch (Exception ex) { @@ -204,7 +213,7 @@ public async ValueTask CreateClient(string accessToken, Cancellat } } else - client.Credentials = new Credentials(accessTokenOrSerializedPem); + client.Credentials = new Credentials(accessString); } clientCache.Add(cacheKey, (Client: client, LastUsed: now)); diff --git a/src/Tgstation.Server.Host/Utils/GitHub/GitHubServiceFactory.cs b/src/Tgstation.Server.Host/Utils/GitHub/GitHubServiceFactory.cs index a38b84b271d..1fb2b351cc2 100644 --- a/src/Tgstation.Server.Host/Utils/GitHub/GitHubServiceFactory.cs +++ b/src/Tgstation.Server.Host/Utils/GitHub/GitHubServiceFactory.cs @@ -53,9 +53,20 @@ public async ValueTask CreateService(CancellationToken cancellat /// public async ValueTask CreateService(string accessToken, CancellationToken cancellationToken) => CreateServiceImpl( - await gitHubClientFactory.CreateClient( - accessToken ?? throw new ArgumentNullException(nameof(accessToken)), - cancellationToken)); + await gitHubClientFactory.CreateClient(accessToken, cancellationToken)); + + /// + public async ValueTask CreateService(string accessString, RepositoryIdentifier repositoryIdentifier, CancellationToken cancellationToken) + { + var client = await gitHubClientFactory.CreateClientForRepository( + accessString ?? throw new ArgumentNullException(nameof(accessString)), + repositoryIdentifier, + cancellationToken); + if (client == null) + return null; + + return CreateServiceImpl(client); + } /// /// Create a . diff --git a/src/Tgstation.Server.Host/Utils/GitHub/IGitHubClientFactory.cs b/src/Tgstation.Server.Host/Utils/GitHub/IGitHubClientFactory.cs index 1808b4ca3c3..944bcb0cf92 100644 --- a/src/Tgstation.Server.Host/Utils/GitHub/IGitHubClientFactory.cs +++ b/src/Tgstation.Server.Host/Utils/GitHub/IGitHubClientFactory.cs @@ -26,12 +26,12 @@ public interface IGitHubClientFactory ValueTask CreateClient(string accessToken, CancellationToken cancellationToken); /// - /// Creates a GitHub App client for an installation. + /// Creates a GitHub client that will only be used for a given . /// - /// The private key . - /// The GitHub repository ID. + /// The GitHub personal access token or TGS encoded app private key . + /// The for the GitHub ID that the client will be used to connect to. /// The for the operation. - /// A resulting in a new for the given or if authentication failed. - ValueTask CreateInstallationClient(string pem, long repositoryId, CancellationToken cancellationToken); + /// A resulting in a new for the given or if authentication failed. + ValueTask CreateClientForRepository(string accessString, RepositoryIdentifier repositoryIdentifier, CancellationToken cancellationToken); } } diff --git a/src/Tgstation.Server.Host/Utils/GitHub/IGitHubServiceFactory.cs b/src/Tgstation.Server.Host/Utils/GitHub/IGitHubServiceFactory.cs index adb0ec88ebd..236df579f38 100644 --- a/src/Tgstation.Server.Host/Utils/GitHub/IGitHubServiceFactory.cs +++ b/src/Tgstation.Server.Host/Utils/GitHub/IGitHubServiceFactory.cs @@ -22,5 +22,14 @@ public interface IGitHubServiceFactory /// The for the operation. /// A resulting in a new . public ValueTask CreateService(string accessToken, CancellationToken cancellationToken); + + /// + /// Create an . + /// + /// The access token to use for communication with GitHub. + /// The for the repository the service will be talking with. + /// The for the operation. + /// A resulting in a new . + public ValueTask CreateService(string accessString, RepositoryIdentifier repositoryIdentifier, CancellationToken cancellationToken); } } diff --git a/src/Tgstation.Server.Host/Utils/GitHub/RepositoryIdentifier.cs b/src/Tgstation.Server.Host/Utils/GitHub/RepositoryIdentifier.cs new file mode 100644 index 00000000000..89cb21554bb --- /dev/null +++ b/src/Tgstation.Server.Host/Utils/GitHub/RepositoryIdentifier.cs @@ -0,0 +1,77 @@ +using System; +using System.Diagnostics.CodeAnalysis; + +using Tgstation.Server.Api.Models; +using Tgstation.Server.Api.Models.Internal; + +namespace Tgstation.Server.Host.Utils.GitHub +{ + /// + /// Identifies a repository either by its or and . + /// + public sealed class RepositoryIdentifier + { + /// + /// If the is using and . + /// + [MemberNotNullWhen(false, nameof(RepositoryId))] + [MemberNotNullWhen(true, nameof(Owner))] + [MemberNotNullWhen(true, nameof(Name))] + public bool IsSlug => !RepositoryId.HasValue; + + /// + /// The repository ID. + /// + public long? RepositoryId { get; } + + /// + /// The repository's owning entity. + /// + public string? Owner { get; } + + /// + /// The repository's name. + /// + public string? Name { get; } + + /// + /// Initializes a new instance of the class. + /// + /// The value of . + public RepositoryIdentifier(long repositoryId) + { + RepositoryId = repositoryId; + } + + /// + /// Initializes a new instance of the class. + /// + /// The value of . + /// The value of . + public RepositoryIdentifier(string owner, string name) + { + Owner = owner ?? throw new ArgumentNullException(nameof(owner)); + Name = name ?? throw new ArgumentNullException(nameof(name)); + } + + /// + /// Initializes a new instance of the class. + /// + /// The to build from. + public RepositoryIdentifier(IGitRemoteInformation gitRemoteInformation) + { + ArgumentNullException.ThrowIfNull(gitRemoteInformation); + if (gitRemoteInformation.RemoteGitProvider == RemoteGitProvider.Unknown) + throw new ArgumentException("Cannot cast IGitRemoteInformation of Unknown provider to RepositoryIdentifier!", nameof(gitRemoteInformation)); + + Owner = gitRemoteInformation.RemoteRepositoryOwner; + Name = gitRemoteInformation.RemoteRepositoryName; + } + + /// + public override string ToString() + => IsSlug + ? $"{Owner}/{Name}" + : $"ID {RepositoryId.Value}"; + } +} diff --git a/tests/Tgstation.Server.Host.Tests/Utils/GitHub/TestGitHubClientFactory.cs b/tests/Tgstation.Server.Host.Tests/Utils/GitHub/TestGitHubClientFactory.cs index 33058be75f4..605083ca121 100644 --- a/tests/Tgstation.Server.Host.Tests/Utils/GitHub/TestGitHubClientFactory.cs +++ b/tests/Tgstation.Server.Host.Tests/Utils/GitHub/TestGitHubClientFactory.cs @@ -1,5 +1,6 @@ using System; using System.Net.Http.Headers; +using System.Text; using System.Threading; using System.Threading.Tasks; @@ -11,6 +12,7 @@ using Octokit; +using Tgstation.Server.Api.Models.Internal; using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.System; @@ -96,6 +98,92 @@ public async Task TestCreateTokenClient() mockApp.VerifyAll(); } + [TestMethod] + public async Task TestCreateEncodedClientRealKey() + { + var mockApp = new Mock(); + mockApp.SetupGet(x => x.ProductInfoHeaderValue).Returns(new ProductInfoHeaderValue("TGSTests", "1.2.3")).Verifiable(); + + var mockOptions = new Mock>(); + mockOptions.SetupGet(x => x.Value).Returns(new GeneralConfiguration()); + var factory = new GitHubClientFactory(mockApp.Object, loggerFactory.CreateLogger(), mockOptions.Object); + + var appID = Environment.GetEnvironmentVariable("TGS_TEST_APP_ID"); + var privateKey = Environment.GetEnvironmentVariable("TGS_TEST_APP_PRIVATE_KEY"); + var repoSlug = Environment.GetEnvironmentVariable("TGS_TEST_REPO_SLUG"); + + if (String.IsNullOrWhiteSpace(appID)) + Assert.Inconclusive("Missing App ID"); + + if (String.IsNullOrWhiteSpace(privateKey)) + Assert.Inconclusive("Missing App Private Key"); + + if (String.IsNullOrWhiteSpace(repoSlug)) + Assert.Inconclusive("Missing repo slug"); + + var fakeAccessString = $"{RepositorySettings.TgsAppPrivateKeyPrefix}{appID}:{Convert.ToBase64String(Encoding.UTF8.GetBytes(privateKey))}"; + var slugSplits = repoSlug.Split('/'); + + Assert.AreEqual(2, slugSplits.Length); + + var client = await factory.CreateClientForRepository(fakeAccessString, new RepositoryIdentifier(slugSplits[0], slugSplits[1]), CancellationToken.None); + Assert.IsNotNull(client); + + var credentials = await client.Connection.CredentialStore.GetCredentials(); + + Assert.AreEqual(AuthenticationType.Oauth, credentials.AuthenticationType); + + mockApp.VerifyAll(); + } + + [TestMethod] + public async Task TestCreateEncodedClientFakeKey() + { + var mockApp = new Mock(); + mockApp.SetupGet(x => x.ProductInfoHeaderValue).Returns(new ProductInfoHeaderValue("TGSTests", "1.2.3")).Verifiable(); + + var mockOptions = new Mock>(); + mockOptions.SetupGet(x => x.Value).Returns(new GeneralConfiguration()); + var factory = new GitHubClientFactory(mockApp.Object, loggerFactory.CreateLogger(), mockOptions.Object); + + await Assert.ThrowsExceptionAsync(() => factory.CreateClient(null, CancellationToken.None).AsTask()); + + const string FakePrivateKey = @"-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEAq3oP6NMRwRZY8eMbm4GRLyfJ07LNpHzjRcjTvMf8LGGSVb8v +DBApR/5+TvWB5qnnh5fJBrR40sboJhIXUXkpyebIu/7lDqXhfjroAk8oNJhyLpW7 +u+8DpwTuZYQSSUgdMvNqrWBt7SMrFbhTtEVvQLW5LcwXL9E+V83pwQ1q37wCsedq +fsQbZSxXMMu4efaFoGJ1G60WqYiCCBSsJMPZY5St2G5Tn2caUVQX4V/kyU09gGyN +lJWFpZJP37zBdfmX+VkNj+UFusmcUe7GnklzYhyiXX2sVxzCekCAUxZKO3WKDHbg +CA02oZ8mfQiIbxCYG/uqYmAoAMbQJKqc7iu/0wIDAQABAoIBAEa37F/EzImpQb1g +QD59zPZ5nk7katLvfnuFO22bvHBBPSyH0EtVTvEWD9lYft42K/pLquhM/ZdP2OX6 +iAtdwNI3j4mYsba8yqZYfN6W7rojRRTZQ7dZ91OmQPs04KXAS+p7YP9nyW4HFvm6 +Lyslh6BUUa6FgPqDfQaRMVogwnbKUilob1eNncXRTw1PUQ+YOK28COYwp+XtNdEM +lecUVqZv5HKSMvP643sMKnj93PXWyisaj77pEdw/1CDQkP1cgqTyX4SaEgx2QDhR +ljaSIQH41JqQCDpWqbqrx3XiqpaWrIAnmUHhP0WRUlD8wCaW5PxY/HwkT/McgyGD +QUofyuECgYEA1XH6850JOrqYZagmTwgSzfbuOmQyYWxdvnq/oRjyy2rpcrISRdMh +3NdpwDX41EhWvMEjfA7l1AYccf72AbwGK+kzosqXn53NHcCjsVd1VzNo9Zm7l9Wj +SJE3ZLSrQ/8nlvrwGr0tgybhq3kLJZLZ+blBSvuYx2YHC4PQEuAo3DcCgYEAzaoO +tPCvpo23jHcevYaQQCYh1sAts9m99LUg2GUYHQ/U11VGq+DTUfReTI7LhXZuyvyx +V+bR5e6NPTuL0XBmESjDSUnj+SCVK8x+NMJMQKqS4OcGSZmx0CCtQCAykZvOA9NQ +zoXW8DkTtiwyQ+AMh9msYnWpJJj2y86/6pOqQ0UCgYB/B+Lu8dr4VO02Myj5iDiI +1BlcLx281Z3FK5C48/wsDGj7lfdCDzHsGVgayQRacuMMW3Ye807dLPXo8nC+/4Q8 +xgGxNRmgKW5V8rx5Yy+2wiYJZYE8EC2plqN9D/mN8mFBff9AKq7Xi2BriRKVPhz0 +fsjZM3vt0E8JD13angYzaQKBgHHkfBJ9u3grwPrbuL1SOK4dr92iPWz85zIN4GuV +yH3Hl6HMCsACWGRpRJN2/IQjawWkXH2GSLThn3vKbwqECTH1dfgvID2FarZ/n2CO +PPYOwBomNhgqMgtFHUyGyBpUwwjhTD2iZr5PjXf0D74A5E+THuDDsfCfeQSysRsx +vTdVAoGBAI/jjUMdjkY43zhe3w2piwT0fhGfqm9ikdAB9IcgcptuS0ML0ZaWV/eO +8a/G4KMV387BgLUPotWL29EHTdD5ir2yVIhF4JtQEpPucktacruu2TbGUm/5gV96 +0+rEPfoEdN+gHI+3w7N6owFmir7Wdz0VK9K8OxsjMLshW3sRlphg +-----END RSA PRIVATE KEY-----"; + + var fakeAccessString = $"{RepositorySettings.TgsAppPrivateKeyPrefix}123:{Convert.ToBase64String(Encoding.UTF8.GetBytes(FakePrivateKey))}"; + + var client = await factory.CreateClientForRepository(fakeAccessString, new RepositoryIdentifier("fake_owner", "fake_name"), CancellationToken.None); + Assert.IsNull(client); + + mockApp.VerifyAll(); + } + [TestMethod] public async Task TestClientCaching() { From 0013b5046310223ee8b23e04774265b59c7ab4b9 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Mon, 2 Sep 2024 21:01:31 -0400 Subject: [PATCH 08/22] Bump nuget versions --- build/Version.props | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build/Version.props b/build/Version.props index c6573e2f149..079fb9d43b2 100644 --- a/build/Version.props +++ b/build/Version.props @@ -7,8 +7,8 @@ 5.2.0 10.8.0 7.0.0 - 13.7.0 - 16.0.0 + 13.8.0 + 16.1.0 7.3.0 5.10.0 1.5.0 From 4bf7473579d4d7b3beb4add8d5ed7d1de55edfd1 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Mon, 2 Sep 2024 21:48:57 -0400 Subject: [PATCH 09/22] Note on required app permissions --- README.md | 49 +++++++++++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index a646eda7c05..3ecd2a91423 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ If you're just a hobbyist server host, you can probably get away with using SQLi _HOWEVER_ -SQLite is not a battle-ready relational database. It doesn't scale well for any use case. TGS *strongly* recommends you use one of its supported standalone databases. Setting one of these up is more involved but worth the effort. +SQLite is not a battle-ready relational database. It doesn't scale well for any use case. TGS _strongly_ recommends you use one of its supported standalone databases. Setting one of these up is more involved but worth the effort. The supported standalone databases are: @@ -38,8 +38,9 @@ The supported standalone databases are: - MySQL TGS will require either: + - No pre-existing database WITH schema creation permissions. -or + or - Exclusive access to a database schema that TGS has full control over. ### Installation @@ -63,11 +64,13 @@ Note: If you use the `/silent` or `/passive` arguments to the installer, you wil [winget](https://github.com/microsoft/winget-cli) installed is the easiest way to install the latest version of tgstation-server (provided Microsoft has approved the most recent package manifest). Check if you have `winget` by running the following command. + ``` winget --version ``` If it returns an error that means you don't have winget. You can easily install it by running the following commands in an administrative Windows Powershell instance: + ``` Import-Module Appx Invoke-WebRequest -Uri https://www.nuget.org/api/v2/package/Microsoft.UI.Xaml/2.7.3 -OutFile .\microsoft.ui.xaml.2.7.3.zip @@ -158,6 +161,7 @@ Alternatively, to launch the server in the current shell, run `./tgs.sh` in the tgstation-server supports running in a docker container. The official image repository is located at https://hub.docker.com/r/tgstation/server. It can also be built locally by running `docker build . -f build/Dockerfile -t ` in the repository root. To create a container run + ```sh docker run \ -ti \ # Start with interactive terminal the first time to run the setup wizard @@ -174,6 +178,7 @@ docker run \ -v /path/to/your/log/folder:/tgs_logs \ # Recommended, create a volume mapping for server logs tgstation/server[:] ``` + with any additional options you desire (i.e. You'll have to expose more game ports in order to host more than one instance). When launching the container for the first time, you'll be prompted with the setup wizard and then the container will exit. Start the container again to launch the server. @@ -199,14 +204,15 @@ OpenDream currently requires [.NET SDK 8.0](https://dotnet.microsoft.com/en-us/d
How to handle a different SDK version than the ASP.NET runtime of TGS. - On Linux, as long as OpenDream and TGS do not use the same .NET major version, you cannot achieve this with the package manager as they will conflict. For example, the 7.0 SDK can be added to an 8.0 runtime installation via the following steps. +On Linux, as long as OpenDream and TGS do not use the same .NET major version, you cannot achieve this with the package manager as they will conflict. For example, the 7.0 SDK can be added to an 8.0 runtime installation via the following steps. + +1. Install `tgstation-server` using any of the above methods. +1. [Download the Linux SDK binaries](https://dotnet.microsoft.com/en-us/download/dotnet/7.0) for your selected architecture. +1. Extract everything EXCEPT the `dotnet` executable, `LICENSE.txt`, and `ThirdPartyNotices.txt` in the `.tar.gz` on top of the existing installation directory `/usr/share/dotnet/` +1. Run `sudo chown -R root /usr/share/dotnet` - 1. Install `tgstation-server` using any of the above methods. - 1. [Download the Linux SDK binaries](https://dotnet.microsoft.com/en-us/download/dotnet/7.0) for your selected architecture. - 1. Extract everything EXCEPT the `dotnet` executable, `LICENSE.txt`, and `ThirdPartyNotices.txt` in the `.tar.gz` on top of the existing installation directory `/usr/share/dotnet/` - 1. Run `sudo chown -R root /usr/share/dotnet` +You should now be able to run the `dotnet --list-sdks` command and see an entry for `7.0.XXX [/usr/share/dotnet/sdk]`. - You should now be able to run the `dotnet --list-sdks` command and see an entry for `7.0.XXX [/usr/share/dotnet/sdk]`.
### Configuring @@ -261,13 +267,14 @@ Create an `appsettings.Production.yml` file next to `appsettings.yml`. This will - `ControlPanel:Enable`: Enable the javascript based control panel to be served from the server via /index.html -- `ControlPanel:AllowAnyOrigin`: Set the Access-Control-Allow-Origin header to * for all responses (also enables all headers and methods) +- `ControlPanel:AllowAnyOrigin`: Set the Access-Control-Allow-Origin header to \* for all responses (also enables all headers and methods) - `ControlPanel:AllowedOrigins`: Set the Access-Control-Allow-Origin headers to this list of origins for all responses (also enables all headers and methods). This is overridden by `ControlPanel:AllowAnyOrigin` - `ControlPanel:PublicPath`: URL from which the webpanel can be accessed, defaults to "/app/". Must be an absolute path (https://example.org/path/to/webpanel) or a path starting from root (/path/to/webpanel). Note that this option does not relocate the webpanel for you; you will need a reverse proxy to relocate the webpanel - `Elasticsearch`: tgstation-server also supports automatically ingesting its logs to ElasticSearch. You can set this up in the setup wizard, or with the following configuration: + ```yml Elasticsearch: Enable: true @@ -291,6 +298,7 @@ Create an `appsettings.Production.yml` file next to `appsettings.yml`. This will - `Swarm:UpdateRequiredNodeCount`: Should be set to the total number of servers in your swarm minus 1. Prevents updates from occurring unless the non-controller server count in the swarm is greater than or equal to this value. - `Security:OAuth:`: Sets the OAuth client ID and secret for a given ``. The currently supported providers are `Keycloak`, `GitHub`, `Discord`, `InvisionCommunity` and `TGForums`. Setting these fields to `null` disables logins with the provider, but does not stop users from associating their accounts using the API. Sample Entry: + ```yml Security: OAuth: @@ -301,6 +309,7 @@ Security: ServerUrl: "..." UserInformationUrlOverride: "..." # For power users, leave out of configuration for most cases. Not supported by GitHub provider. ``` + The following providers use the `RedirectUrl` setting: - GitHub @@ -366,6 +375,7 @@ The DMAPI is fully backwards compatible and should function with any tgstation-s Here is a bare minimum example project that implements the essential code changes for integrating the DMAPI Before `tgs.dm`: + ```dm //Remember, every codebase is different, you probably have better methods for these defines than the ones given here #define TGS_EXTERNAL_CONFIGURATION @@ -382,6 +392,7 @@ Before `tgs.dm`: ``` Anywhere else: + ```dm var/global/client_count = 0 @@ -436,11 +447,13 @@ _NOTE: Your reverse proxy setup may interfere with SSE (Server-Sent Events) whic 1. Setup a basic website configuration. Instructions on how to do so are out of scope. 2. In your Caddyfile, under a server entry, add the following (replace 5000 with the port TGS is hosted on): + ``` https://your.site.here { reverse_proxy localhost:5000 } ``` + 3. For this setup, your configuration's `ControlPanel:PublicPath` needs to be blank. If you have a path in `PublicPath`, it needs to be in "reverse_proxy PublicPathHere localhost:5000". See https://caddyserver.com/docs/caddyfile/directives/reverse_proxy @@ -450,6 +463,7 @@ See https://caddyserver.com/docs/caddyfile/directives/reverse_proxy 1. Setup a basic website configuration. Instructions on how to do so are out of scope. 2. Acquire an HTTPS certificate, likely via Let's Encrypt, and configure NGINX to use it. 3. Setup a path under a server like the following (replace 8080 with the port TGS is hosted on): + ``` location /tgs { proxy_pass http://127.0.0.1:8080; @@ -465,6 +479,7 @@ See https://docs.nginx.com/nginx/admin-guide/web-server/reverse-proxy/ 2. Setup a basic website configuration. Instructions on how to do so are out of scope. 3. Acquire an HTTPS certificate, likely via Let's Encrypt, and configure Apache to use it. 4. Under a VirtualHost entry, setup the following (replace 8080 with the port TGS is hosted on): + ``` ProxyPass / http://127.0.0.1:8080 ProxyPassReverse / http://127.0.0.1:8080 @@ -473,6 +488,7 @@ ProxyPassReverse / http://127.0.0.1:8080 See https://httpd.apache.org/docs/2.4/howto/reverse_proxy.html Example VirtualHost Entry + ``` @@ -548,7 +564,7 @@ All users with access to an instance have an InstanceUser object associated with The `Repository` folder is a git repository containing the code of the game you wish to host. It can be cloned from any public or private remote repository and has capabilities to affect changes back to it. All the standard benefits of git are utilized (i.e. check out any revision or reference). -Additional features become available if the remote repository is hosted on https://github.com/. Namely the Test Merge feature, which allows you to take a pull request opened on the repository and compile it into a game deployment for testing. Information about test merges is available in game via the DMAPI and via the main API as well. +Additional features become available if the remote repository is hosted on https://github.com/. Namely the Test Merge feature, which allows you to take a pull request opened on the repository and compile it into a game deployment for testing. Information about test merges is available in game via the DMAPI and via the main API as well. These require a valid personal access token with `repo` scope or an installed GitHub App with the following permissions: `contents: write`, `pull_requests: write`, and `deployments: write`. Manual operations on the repository while an instance is running may lead to git data corruption. Thankfully, it's simple enough to delete and reclone the repository via the API. @@ -586,10 +602,7 @@ Bots have a set of built-in commands that can be triggered via `!tgs`, mentionin All files in game code deployments are considered transient by default, meaning when new code is deployed, changes will be lost. Static files allow you to specify which files and folders stick around throughout all deployments. -The `StaticFiles` folder contains 3 root folders which cannot be deleted and operate under special rules - - `CodeModifications` - - `EventScripts` - - `GameStaticFiles` +The `StaticFiles` folder contains 3 root folders which cannot be deleted and operate under special rules - `CodeModifications` - `EventScripts` - `GameStaticFiles` These files can be modified either in host mode or system user mode. In host mode, TGS itself is responsible for reading and writing the files. In system user mode read and write actions are performed using the system account of the logged on User, enabling the use of ACLs to control access to files. Database users will not be able to use the static file system if this mode is configured for an instance. @@ -663,12 +676,12 @@ Feel free to ask for help [on the discussions page](https://github.com/tgstation ## Contributing -* See [CONTRIBUTING.md](.github/CONTRIBUTING.md) +- See [CONTRIBUTING.md](.github/CONTRIBUTING.md) ## Licensing -* The DMAPI for the project is licensed under the MIT license. -* The /tg/station 13 icon is licensed under [Creative Commons 3.0 BY-SA](http://creativecommons.org/licenses/by-sa/3.0/). -* The remainder of the project is licensed under [GNU AGPL v3](http://www.gnu.org/licenses/agpl-3.0.html) +- The DMAPI for the project is licensed under the MIT license. +- The /tg/station 13 icon is licensed under [Creative Commons 3.0 BY-SA](http://creativecommons.org/licenses/by-sa/3.0/). +- The remainder of the project is licensed under [GNU AGPL v3](http://www.gnu.org/licenses/agpl-3.0.html) See the files in the `/src/DMAPI` tree for the MIT license From 157934730c4f9e145a304b40ee23b177e4d4b312 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Tue, 3 Sep 2024 06:57:57 -0400 Subject: [PATCH 10/22] Fix `DummyGitHubServiceFactory` --- .../Live/DummyGitHubServiceFactory.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/Tgstation.Server.Tests/Live/DummyGitHubServiceFactory.cs b/tests/Tgstation.Server.Tests/Live/DummyGitHubServiceFactory.cs index d05faed9a65..7b19388fd4a 100644 --- a/tests/Tgstation.Server.Tests/Live/DummyGitHubServiceFactory.cs +++ b/tests/Tgstation.Server.Tests/Live/DummyGitHubServiceFactory.cs @@ -29,6 +29,14 @@ public ValueTask CreateService(string accessToken, return ValueTask.FromResult(CreateDummyService()); } + public ValueTask CreateService(string accessString, RepositoryIdentifier repositoryIdentifier, CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(accessString); + ArgumentNullException.ThrowIfNull(repositoryIdentifier); + + return ValueTask.FromResult(CreateDummyService()); + } + TestingGitHubService CreateDummyService() => new TestingGitHubService(cryptographySuite, logger); } } From 59dd0351b8a8ab76360620f41b151c3dfa9554a4 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Tue, 3 Sep 2024 07:01:53 -0400 Subject: [PATCH 11/22] Move `RepositorySettings` out of Internal namespace Make abstract --- build/Version.props | 4 ++-- src/Tgstation.Server.Api/Models/ErrorCode.cs | 12 ++++++------ .../Models/{Internal => }/RepositorySettings.cs | 4 ++-- src/Tgstation.Server.Api/Rights/RepositoryRights.cs | 10 +++++----- .../Repository/DefaultGitRemoteFeatures.cs | 2 +- .../Components/Repository/GitHubRemoteFeatures.cs | 1 - .../Components/Repository/GitLabRemoteFeatures.cs | 1 - .../Components/Repository/GitRemoteFeaturesBase.cs | 1 - .../Components/Repository/Repository.cs | 1 - .../Components/Repository/RepositoryUpdateService.cs | 4 ++-- .../Controllers/InstanceController.cs | 2 +- .../Controllers/RepositoryController.cs | 4 ++-- .../20181124231534_MSToggleTestmergeComments.cs | 2 +- .../20181124231549_MYToggleTestmergeComments.cs | 2 +- .../Models/RepositorySettings.cs | 4 ++-- .../Utils/GitHub/GitHubClientFactory.cs | 2 +- .../Utils/GitHub/TestGitHubClientFactory.cs | 2 +- 17 files changed, 27 insertions(+), 31 deletions(-) rename src/Tgstation.Server.Api/Models/{Internal => }/RepositorySettings.cs (97%) diff --git a/build/Version.props b/build/Version.props index 079fb9d43b2..17a15a5068c 100644 --- a/build/Version.props +++ b/build/Version.props @@ -7,8 +7,8 @@ 5.2.0 10.8.0 7.0.0 - 13.8.0 - 16.1.0 + 14.0.0 + 17.0.0 7.3.0 5.10.0 1.5.0 diff --git a/src/Tgstation.Server.Api/Models/ErrorCode.cs b/src/Tgstation.Server.Api/Models/ErrorCode.cs index 9e1fea2fe96..ca7abbbc8da 100644 --- a/src/Tgstation.Server.Api/Models/ErrorCode.cs +++ b/src/Tgstation.Server.Api/Models/ErrorCode.cs @@ -185,7 +185,7 @@ public enum ErrorCode : uint SwarmIntegrityCheckFailed, /// - /// One of and is set while the other isn't. + /// One of and is set while the other isn't. /// [Description("Either both accessUser and accessToken must be set or neither!")] RepoMismatchUserAndAccessToken, @@ -239,13 +239,13 @@ public enum ErrorCode : uint RepoDuplicateTestMerge, /// - /// Attempted to set a whitespace . + /// Attempted to set a whitespace . /// [Description("committerName cannot be whitespace!")] RepoWhitespaceCommitterName, /// - /// Attempted to set a whitespace . + /// Attempted to set a whitespace . /// [Description("committerEmail cannot be whitespace!")] RepoWhitespaceCommitterEmail, @@ -377,19 +377,19 @@ public enum ErrorCode : uint InstanceMissingDreamMakerSettings, /// - /// Missing in database. + /// Missing in database. /// [Description("Could not retrieve Repository settings from the database!")] InstanceMissingRepositorySettings, /// - /// Performing an automatic update with the flag resulted in merge conflicts. + /// Performing an automatic update with the flag resulted in merge conflicts. /// [Description("Performing this automatic update as a merge would result in conficts. Aborting!")] InstanceUpdateTestMergeConflict, /// - /// and are required for this operation. + /// and are required for this operation. /// [Description("Git credentials are required for this operation!")] RepoCredentialsRequired, diff --git a/src/Tgstation.Server.Api/Models/Internal/RepositorySettings.cs b/src/Tgstation.Server.Api/Models/RepositorySettings.cs similarity index 97% rename from src/Tgstation.Server.Api/Models/Internal/RepositorySettings.cs rename to src/Tgstation.Server.Api/Models/RepositorySettings.cs index 7b916352d2a..f54e1e4a6e1 100644 --- a/src/Tgstation.Server.Api/Models/Internal/RepositorySettings.cs +++ b/src/Tgstation.Server.Api/Models/RepositorySettings.cs @@ -1,11 +1,11 @@ using System.ComponentModel.DataAnnotations; -namespace Tgstation.Server.Api.Models.Internal +namespace Tgstation.Server.Api.Models { /// /// Represents configurable settings for a git repository. /// - public class RepositorySettings + public abstract class RepositorySettings { /// /// Prefix for TGS encoded app private keys. This is encoded in the format PREFIX + (APP_ID OR CLIENT_ID) + ':' + BASE64(APP_PRIVATE_KEY). diff --git a/src/Tgstation.Server.Api/Rights/RepositoryRights.cs b/src/Tgstation.Server.Api/Rights/RepositoryRights.cs index 407437ce5d1..fdd8f4e048f 100644 --- a/src/Tgstation.Server.Api/Rights/RepositoryRights.cs +++ b/src/Tgstation.Server.Api/Rights/RepositoryRights.cs @@ -19,7 +19,7 @@ public enum RepositoryRights : ulong CancelPendingChanges = 1 << 0, /// - /// User may clone the repository if it does not exist. This also allows setting , , and at clone time. + /// User may clone the repository if it does not exist. This also allows setting , , and at clone time. /// SetOrigin = 1 << 1, @@ -39,17 +39,17 @@ public enum RepositoryRights : ulong UpdateBranch = 1 << 4, /// - /// User may change and . + /// User may change and . /// ChangeCommitter = 1 << 5, /// - /// User may change , , and . + /// User may change , , and . /// ChangeTestMergeCommits = 1 << 6, /// - /// User may read and change and . + /// User may read and change and . /// ChangeCredentials = 1 << 7, @@ -64,7 +64,7 @@ public enum RepositoryRights : ulong Read = 1 << 9, /// - /// User may change and . + /// User may change and . /// ChangeAutoUpdateSettings = 1 << 10, diff --git a/src/Tgstation.Server.Host/Components/Repository/DefaultGitRemoteFeatures.cs b/src/Tgstation.Server.Host/Components/Repository/DefaultGitRemoteFeatures.cs index 05d9408a677..1c526e9295b 100644 --- a/src/Tgstation.Server.Host/Components/Repository/DefaultGitRemoteFeatures.cs +++ b/src/Tgstation.Server.Host/Components/Repository/DefaultGitRemoteFeatures.cs @@ -29,7 +29,7 @@ sealed class DefaultGitRemoteFeatures : IGitRemoteFeatures /// public ValueTask GetTestMerge( TestMergeParameters parameters, - Api.Models.Internal.RepositorySettings repositorySettings, + RepositorySettings repositorySettings, CancellationToken cancellationToken) => throw new NotSupportedException(); } } diff --git a/src/Tgstation.Server.Host/Components/Repository/GitHubRemoteFeatures.cs b/src/Tgstation.Server.Host/Components/Repository/GitHubRemoteFeatures.cs index 7d34f78e542..97eded304c1 100644 --- a/src/Tgstation.Server.Host/Components/Repository/GitHubRemoteFeatures.cs +++ b/src/Tgstation.Server.Host/Components/Repository/GitHubRemoteFeatures.cs @@ -6,7 +6,6 @@ using Octokit; using Tgstation.Server.Api.Models; -using Tgstation.Server.Api.Models.Internal; using Tgstation.Server.Host.Utils.GitHub; namespace Tgstation.Server.Host.Components.Repository diff --git a/src/Tgstation.Server.Host/Components/Repository/GitLabRemoteFeatures.cs b/src/Tgstation.Server.Host/Components/Repository/GitLabRemoteFeatures.cs index 9cf32f3f7bd..ffbd577ac2a 100644 --- a/src/Tgstation.Server.Host/Components/Repository/GitLabRemoteFeatures.cs +++ b/src/Tgstation.Server.Host/Components/Repository/GitLabRemoteFeatures.cs @@ -6,7 +6,6 @@ using Microsoft.Extensions.Logging; using Tgstation.Server.Api.Models; -using Tgstation.Server.Api.Models.Internal; namespace Tgstation.Server.Host.Components.Repository { diff --git a/src/Tgstation.Server.Host/Components/Repository/GitRemoteFeaturesBase.cs b/src/Tgstation.Server.Host/Components/Repository/GitRemoteFeaturesBase.cs index d42e9287929..4aa58fd1fc4 100644 --- a/src/Tgstation.Server.Host/Components/Repository/GitRemoteFeaturesBase.cs +++ b/src/Tgstation.Server.Host/Components/Repository/GitRemoteFeaturesBase.cs @@ -6,7 +6,6 @@ using Microsoft.Extensions.Logging; using Tgstation.Server.Api.Models; -using Tgstation.Server.Api.Models.Internal; namespace Tgstation.Server.Host.Components.Repository { diff --git a/src/Tgstation.Server.Host/Components/Repository/Repository.cs b/src/Tgstation.Server.Host/Components/Repository/Repository.cs index 9a71bcdcebf..1aa2fa08427 100644 --- a/src/Tgstation.Server.Host/Components/Repository/Repository.cs +++ b/src/Tgstation.Server.Host/Components/Repository/Repository.cs @@ -12,7 +12,6 @@ using Microsoft.Extensions.Logging; using Tgstation.Server.Api.Models; -using Tgstation.Server.Api.Models.Internal; using Tgstation.Server.Host.Components.Events; using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.Extensions; diff --git a/src/Tgstation.Server.Host/Components/Repository/RepositoryUpdateService.cs b/src/Tgstation.Server.Host/Components/Repository/RepositoryUpdateService.cs index 17e820433a7..341db7f6172 100644 --- a/src/Tgstation.Server.Host/Components/Repository/RepositoryUpdateService.cs +++ b/src/Tgstation.Server.Host/Components/Repository/RepositoryUpdateService.cs @@ -25,7 +25,7 @@ sealed class RepositoryUpdateService /// /// The current for the . /// - readonly RepositorySettings currentModel; + readonly Models.RepositorySettings currentModel; /// /// The that initiated the repository update. @@ -50,7 +50,7 @@ sealed class RepositoryUpdateService /// The value of . /// The value of . public RepositoryUpdateService( - RepositorySettings currentModel, + Models.RepositorySettings currentModel, User initiatingUser, ILogger logger, long instanceId) diff --git a/src/Tgstation.Server.Host/Controllers/InstanceController.cs b/src/Tgstation.Server.Host/Controllers/InstanceController.cs index dbcb03b70b3..aff77e4869d 100644 --- a/src/Tgstation.Server.Host/Controllers/InstanceController.cs +++ b/src/Tgstation.Server.Host/Controllers/InstanceController.cs @@ -770,7 +770,7 @@ public async ValueTask GrantPermissions(long id, CancellationToke AutoUpdateInterval = initialSettings.AutoUpdateInterval ?? 0, AutoUpdateCron = initialSettings.AutoUpdateCron ?? String.Empty, ChatBotLimit = initialSettings.ChatBotLimit ?? Models.Instance.DefaultChatBotLimit, - RepositorySettings = new RepositorySettings + RepositorySettings = new Models.RepositorySettings { CommitterEmail = Components.Repository.Repository.DefaultCommitterEmail, CommitterName = Components.Repository.Repository.DefaultCommitterName, diff --git a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs index 735b9efa839..4e7f576fd6e 100644 --- a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs +++ b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs @@ -369,7 +369,7 @@ public async ValueTask Update([FromBody] RepositoryUpdateRequest if (currentModel == default) return this.Gone(); - bool CheckModified(Expression> expression, RepositoryRights requiredRight) + bool CheckModified(Expression> expression, RepositoryRights requiredRight) { var memberSelectorExpression = (MemberExpression)expression.Body; var property = (PropertyInfo)memberSelectorExpression.Member; @@ -531,7 +531,7 @@ async ValueTask PopulateApi( /// /// The current . /// A new . - RepositoryUpdateService CreateRepositoryUpdateService(RepositorySettings currentModel) + RepositoryUpdateService CreateRepositoryUpdateService(Models.RepositorySettings currentModel) => new( currentModel, AuthenticationContext.User, diff --git a/src/Tgstation.Server.Host/Database/Migrations/20181124231534_MSToggleTestmergeComments.cs b/src/Tgstation.Server.Host/Database/Migrations/20181124231534_MSToggleTestmergeComments.cs index 30b821ee66a..d33370d6a86 100644 --- a/src/Tgstation.Server.Host/Database/Migrations/20181124231534_MSToggleTestmergeComments.cs +++ b/src/Tgstation.Server.Host/Database/Migrations/20181124231534_MSToggleTestmergeComments.cs @@ -5,7 +5,7 @@ namespace Tgstation.Server.Host.Database.Migrations { /// - /// Add the column for MSSQL. + /// Add the column for MSSQL. /// public partial class MSToggleTestmergeComments : Migration { diff --git a/src/Tgstation.Server.Host/Database/Migrations/20181124231549_MYToggleTestmergeComments.cs b/src/Tgstation.Server.Host/Database/Migrations/20181124231549_MYToggleTestmergeComments.cs index f91e86a6db4..b8c35a193c2 100644 --- a/src/Tgstation.Server.Host/Database/Migrations/20181124231549_MYToggleTestmergeComments.cs +++ b/src/Tgstation.Server.Host/Database/Migrations/20181124231549_MYToggleTestmergeComments.cs @@ -5,7 +5,7 @@ namespace Tgstation.Server.Host.Database.Migrations { /// - /// Add the column for MySQL/MariaDB. + /// Add the column for MySQL/MariaDB. /// public partial class MYToggleTestmergeComments : Migration { diff --git a/src/Tgstation.Server.Host/Models/RepositorySettings.cs b/src/Tgstation.Server.Host/Models/RepositorySettings.cs index e8fb7f60197..d8041627182 100644 --- a/src/Tgstation.Server.Host/Models/RepositorySettings.cs +++ b/src/Tgstation.Server.Host/Models/RepositorySettings.cs @@ -4,8 +4,8 @@ namespace Tgstation.Server.Host.Models { - /// - public sealed class RepositorySettings : Api.Models.Internal.RepositorySettings, IApiTransformable + /// + public sealed class RepositorySettings : Api.Models.RepositorySettings, IApiTransformable { /// /// The row Id. diff --git a/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs b/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs index d23874a302e..1ba4d8d523d 100644 --- a/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs +++ b/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs @@ -13,7 +13,7 @@ using Octokit; -using Tgstation.Server.Api.Models.Internal; +using Tgstation.Server.Api.Models; using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.System; diff --git a/tests/Tgstation.Server.Host.Tests/Utils/GitHub/TestGitHubClientFactory.cs b/tests/Tgstation.Server.Host.Tests/Utils/GitHub/TestGitHubClientFactory.cs index 605083ca121..5132f8ab018 100644 --- a/tests/Tgstation.Server.Host.Tests/Utils/GitHub/TestGitHubClientFactory.cs +++ b/tests/Tgstation.Server.Host.Tests/Utils/GitHub/TestGitHubClientFactory.cs @@ -12,7 +12,7 @@ using Octokit; -using Tgstation.Server.Api.Models.Internal; +using Tgstation.Server.Api.Models; using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.System; From b2c8f2fa9099a2116fbb6e377e36f5d03d59321a Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Tue, 3 Sep 2024 07:09:37 -0400 Subject: [PATCH 12/22] Unfuck the readme --- README.md | 47 +++++++++++++++++------------------------------ 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 3ecd2a91423..ed43da7fb11 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ If you're just a hobbyist server host, you can probably get away with using SQLi _HOWEVER_ -SQLite is not a battle-ready relational database. It doesn't scale well for any use case. TGS _strongly_ recommends you use one of its supported standalone databases. Setting one of these up is more involved but worth the effort. +SQLite is not a battle-ready relational database. It doesn't scale well for any use case. TGS *strongly* recommends you use one of its supported standalone databases. Setting one of these up is more involved but worth the effort. The supported standalone databases are: @@ -38,9 +38,8 @@ The supported standalone databases are: - MySQL TGS will require either: - - No pre-existing database WITH schema creation permissions. - or +or - Exclusive access to a database schema that TGS has full control over. ### Installation @@ -64,13 +63,11 @@ Note: If you use the `/silent` or `/passive` arguments to the installer, you wil [winget](https://github.com/microsoft/winget-cli) installed is the easiest way to install the latest version of tgstation-server (provided Microsoft has approved the most recent package manifest). Check if you have `winget` by running the following command. - ``` winget --version ``` If it returns an error that means you don't have winget. You can easily install it by running the following commands in an administrative Windows Powershell instance: - ``` Import-Module Appx Invoke-WebRequest -Uri https://www.nuget.org/api/v2/package/Microsoft.UI.Xaml/2.7.3 -OutFile .\microsoft.ui.xaml.2.7.3.zip @@ -161,7 +158,6 @@ Alternatively, to launch the server in the current shell, run `./tgs.sh` in the tgstation-server supports running in a docker container. The official image repository is located at https://hub.docker.com/r/tgstation/server. It can also be built locally by running `docker build . -f build/Dockerfile -t ` in the repository root. To create a container run - ```sh docker run \ -ti \ # Start with interactive terminal the first time to run the setup wizard @@ -178,7 +174,6 @@ docker run \ -v /path/to/your/log/folder:/tgs_logs \ # Recommended, create a volume mapping for server logs tgstation/server[:] ``` - with any additional options you desire (i.e. You'll have to expose more game ports in order to host more than one instance). When launching the container for the first time, you'll be prompted with the setup wizard and then the container will exit. Start the container again to launch the server. @@ -204,15 +199,14 @@ OpenDream currently requires [.NET SDK 8.0](https://dotnet.microsoft.com/en-us/d
How to handle a different SDK version than the ASP.NET runtime of TGS. -On Linux, as long as OpenDream and TGS do not use the same .NET major version, you cannot achieve this with the package manager as they will conflict. For example, the 7.0 SDK can be added to an 8.0 runtime installation via the following steps. - -1. Install `tgstation-server` using any of the above methods. -1. [Download the Linux SDK binaries](https://dotnet.microsoft.com/en-us/download/dotnet/7.0) for your selected architecture. -1. Extract everything EXCEPT the `dotnet` executable, `LICENSE.txt`, and `ThirdPartyNotices.txt` in the `.tar.gz` on top of the existing installation directory `/usr/share/dotnet/` -1. Run `sudo chown -R root /usr/share/dotnet` + On Linux, as long as OpenDream and TGS do not use the same .NET major version, you cannot achieve this with the package manager as they will conflict. For example, the 7.0 SDK can be added to an 8.0 runtime installation via the following steps. -You should now be able to run the `dotnet --list-sdks` command and see an entry for `7.0.XXX [/usr/share/dotnet/sdk]`. + 1. Install `tgstation-server` using any of the above methods. + 1. [Download the Linux SDK binaries](https://dotnet.microsoft.com/en-us/download/dotnet/7.0) for your selected architecture. + 1. Extract everything EXCEPT the `dotnet` executable, `LICENSE.txt`, and `ThirdPartyNotices.txt` in the `.tar.gz` on top of the existing installation directory `/usr/share/dotnet/` + 1. Run `sudo chown -R root /usr/share/dotnet` + You should now be able to run the `dotnet --list-sdks` command and see an entry for `7.0.XXX [/usr/share/dotnet/sdk]`.
### Configuring @@ -267,14 +261,13 @@ Create an `appsettings.Production.yml` file next to `appsettings.yml`. This will - `ControlPanel:Enable`: Enable the javascript based control panel to be served from the server via /index.html -- `ControlPanel:AllowAnyOrigin`: Set the Access-Control-Allow-Origin header to \* for all responses (also enables all headers and methods) +- `ControlPanel:AllowAnyOrigin`: Set the Access-Control-Allow-Origin header to * for all responses (also enables all headers and methods) - `ControlPanel:AllowedOrigins`: Set the Access-Control-Allow-Origin headers to this list of origins for all responses (also enables all headers and methods). This is overridden by `ControlPanel:AllowAnyOrigin` - `ControlPanel:PublicPath`: URL from which the webpanel can be accessed, defaults to "/app/". Must be an absolute path (https://example.org/path/to/webpanel) or a path starting from root (/path/to/webpanel). Note that this option does not relocate the webpanel for you; you will need a reverse proxy to relocate the webpanel - `Elasticsearch`: tgstation-server also supports automatically ingesting its logs to ElasticSearch. You can set this up in the setup wizard, or with the following configuration: - ```yml Elasticsearch: Enable: true @@ -298,7 +291,6 @@ Create an `appsettings.Production.yml` file next to `appsettings.yml`. This will - `Swarm:UpdateRequiredNodeCount`: Should be set to the total number of servers in your swarm minus 1. Prevents updates from occurring unless the non-controller server count in the swarm is greater than or equal to this value. - `Security:OAuth:`: Sets the OAuth client ID and secret for a given ``. The currently supported providers are `Keycloak`, `GitHub`, `Discord`, `InvisionCommunity` and `TGForums`. Setting these fields to `null` disables logins with the provider, but does not stop users from associating their accounts using the API. Sample Entry: - ```yml Security: OAuth: @@ -309,7 +301,6 @@ Security: ServerUrl: "..." UserInformationUrlOverride: "..." # For power users, leave out of configuration for most cases. Not supported by GitHub provider. ``` - The following providers use the `RedirectUrl` setting: - GitHub @@ -375,7 +366,6 @@ The DMAPI is fully backwards compatible and should function with any tgstation-s Here is a bare minimum example project that implements the essential code changes for integrating the DMAPI Before `tgs.dm`: - ```dm //Remember, every codebase is different, you probably have better methods for these defines than the ones given here #define TGS_EXTERNAL_CONFIGURATION @@ -392,7 +382,6 @@ Before `tgs.dm`: ``` Anywhere else: - ```dm var/global/client_count = 0 @@ -447,13 +436,11 @@ _NOTE: Your reverse proxy setup may interfere with SSE (Server-Sent Events) whic 1. Setup a basic website configuration. Instructions on how to do so are out of scope. 2. In your Caddyfile, under a server entry, add the following (replace 5000 with the port TGS is hosted on): - ``` https://your.site.here { reverse_proxy localhost:5000 } ``` - 3. For this setup, your configuration's `ControlPanel:PublicPath` needs to be blank. If you have a path in `PublicPath`, it needs to be in "reverse_proxy PublicPathHere localhost:5000". See https://caddyserver.com/docs/caddyfile/directives/reverse_proxy @@ -463,7 +450,6 @@ See https://caddyserver.com/docs/caddyfile/directives/reverse_proxy 1. Setup a basic website configuration. Instructions on how to do so are out of scope. 2. Acquire an HTTPS certificate, likely via Let's Encrypt, and configure NGINX to use it. 3. Setup a path under a server like the following (replace 8080 with the port TGS is hosted on): - ``` location /tgs { proxy_pass http://127.0.0.1:8080; @@ -479,7 +465,6 @@ See https://docs.nginx.com/nginx/admin-guide/web-server/reverse-proxy/ 2. Setup a basic website configuration. Instructions on how to do so are out of scope. 3. Acquire an HTTPS certificate, likely via Let's Encrypt, and configure Apache to use it. 4. Under a VirtualHost entry, setup the following (replace 8080 with the port TGS is hosted on): - ``` ProxyPass / http://127.0.0.1:8080 ProxyPassReverse / http://127.0.0.1:8080 @@ -488,7 +473,6 @@ ProxyPassReverse / http://127.0.0.1:8080 See https://httpd.apache.org/docs/2.4/howto/reverse_proxy.html Example VirtualHost Entry - ``` @@ -602,7 +586,10 @@ Bots have a set of built-in commands that can be triggered via `!tgs`, mentionin All files in game code deployments are considered transient by default, meaning when new code is deployed, changes will be lost. Static files allow you to specify which files and folders stick around throughout all deployments. -The `StaticFiles` folder contains 3 root folders which cannot be deleted and operate under special rules - `CodeModifications` - `EventScripts` - `GameStaticFiles` +The `StaticFiles` folder contains 3 root folders which cannot be deleted and operate under special rules + - `CodeModifications` + - `EventScripts` + - `GameStaticFiles` These files can be modified either in host mode or system user mode. In host mode, TGS itself is responsible for reading and writing the files. In system user mode read and write actions are performed using the system account of the logged on User, enabling the use of ACLs to control access to files. Database users will not be able to use the static file system if this mode is configured for an instance. @@ -676,12 +663,12 @@ Feel free to ask for help [on the discussions page](https://github.com/tgstation ## Contributing -- See [CONTRIBUTING.md](.github/CONTRIBUTING.md) +* See [CONTRIBUTING.md](.github/CONTRIBUTING.md) ## Licensing -- The DMAPI for the project is licensed under the MIT license. -- The /tg/station 13 icon is licensed under [Creative Commons 3.0 BY-SA](http://creativecommons.org/licenses/by-sa/3.0/). -- The remainder of the project is licensed under [GNU AGPL v3](http://www.gnu.org/licenses/agpl-3.0.html) +* The DMAPI for the project is licensed under the MIT license. +* The /tg/station 13 icon is licensed under [Creative Commons 3.0 BY-SA](http://creativecommons.org/licenses/by-sa/3.0/). +* The remainder of the project is licensed under [GNU AGPL v3](http://www.gnu.org/licenses/agpl-3.0.html) See the files in the `/src/DMAPI` tree for the MIT license From 9fe0c97d2aac9d7ff8c22b70100aac64ff64bca7 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Tue, 3 Sep 2024 21:56:50 -0400 Subject: [PATCH 13/22] Fix failing tests --- .../Components/Interop/Topic/TopicResponse.cs | 2 +- .../Utils/GitHub/GitHubServiceFactory.cs | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Interop/Topic/TopicResponse.cs b/src/Tgstation.Server.Host/Components/Interop/Topic/TopicResponse.cs index e5b30150de8..45bae93f754 100644 --- a/src/Tgstation.Server.Host/Components/Interop/Topic/TopicResponse.cs +++ b/src/Tgstation.Server.Host/Components/Interop/Topic/TopicResponse.cs @@ -40,6 +40,6 @@ sealed class TopicResponse : DMApiResponse, IMissingPayloadsCommunication /// /// The number of connected clients to the game. Added in Interop 5.10.0. /// - public int? ClientCount { get; } + public int? ClientCount { get; set; } } } diff --git a/src/Tgstation.Server.Host/Utils/GitHub/GitHubServiceFactory.cs b/src/Tgstation.Server.Host/Utils/GitHub/GitHubServiceFactory.cs index 1fb2b351cc2..1d5fb5ef318 100644 --- a/src/Tgstation.Server.Host/Utils/GitHub/GitHubServiceFactory.cs +++ b/src/Tgstation.Server.Host/Utils/GitHub/GitHubServiceFactory.cs @@ -53,7 +53,9 @@ public async ValueTask CreateService(CancellationToken cancellat /// public async ValueTask CreateService(string accessToken, CancellationToken cancellationToken) => CreateServiceImpl( - await gitHubClientFactory.CreateClient(accessToken, cancellationToken)); + await gitHubClientFactory.CreateClient( + accessToken ?? throw new ArgumentNullException(nameof(accessToken)), + cancellationToken)); /// public async ValueTask CreateService(string accessString, RepositoryIdentifier repositoryIdentifier, CancellationToken cancellationToken) From 91f56e5f8526911ef7c4e68aecd31b4921c42ae1 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Tue, 3 Sep 2024 23:13:11 -0400 Subject: [PATCH 14/22] Add git credentials validation Closes #1876 --- src/Tgstation.Server.Api/Models/ErrorCode.cs | 6 + .../Repository/GitRemoteFeaturesFactory.cs | 14 +- .../Repository/IGitRemoteFeatures.cs | 2 +- .../Repository/IGitRemoteFeaturesFactory.cs | 11 +- .../Controllers/RepositoryController.cs | 116 ++++++++++++++++ .../Utils/GitHub/GitHubClientFactory.cs | 130 ++++++++++++------ .../Utils/GitHub/IGitHubClientFactory.cs | 7 + 7 files changed, 234 insertions(+), 52 deletions(-) diff --git a/src/Tgstation.Server.Api/Models/ErrorCode.cs b/src/Tgstation.Server.Api/Models/ErrorCode.cs index ca7abbbc8da..cb30e83915f 100644 --- a/src/Tgstation.Server.Api/Models/ErrorCode.cs +++ b/src/Tgstation.Server.Api/Models/ErrorCode.cs @@ -657,5 +657,11 @@ public enum ErrorCode : uint /// [Description("Could not load configured .dme due to it being outside the deployment directory! This should be a relative path.")] DeploymentWrongDme, + + /// + /// Entered wrong username for a repository access token. + /// + [Description("Provided repository username doesn't match the user of the corresponding access token!")] + RepoTokenUsernameMismatch, } } diff --git a/src/Tgstation.Server.Host/Components/Repository/GitRemoteFeaturesFactory.cs b/src/Tgstation.Server.Host/Components/Repository/GitRemoteFeaturesFactory.cs index 70694d0b940..712ce06d5c9 100644 --- a/src/Tgstation.Server.Host/Components/Repository/GitRemoteFeaturesFactory.cs +++ b/src/Tgstation.Server.Host/Components/Repository/GitRemoteFeaturesFactory.cs @@ -45,18 +45,24 @@ public GitRemoteFeaturesFactory( public IGitRemoteFeatures CreateGitRemoteFeatures(IRepository repository) { ArgumentNullException.ThrowIfNull(repository); + return CreateGitRemoteFeatures(repository.Origin); + } + + /// + public IGitRemoteFeatures CreateGitRemoteFeatures(Uri origin) + { + ArgumentNullException.ThrowIfNull(origin); - var primaryRemote = repository.Origin; - var remoteGitProvider = ParseRemoteGitProviderFromOrigin(primaryRemote); + var remoteGitProvider = ParseRemoteGitProviderFromOrigin(origin); return remoteGitProvider switch { RemoteGitProvider.GitHub => new GitHubRemoteFeatures( gitHubServiceFactory, loggerFactory.CreateLogger(), - primaryRemote), + origin), RemoteGitProvider.GitLab => new GitLabRemoteFeatures( loggerFactory.CreateLogger(), - primaryRemote), + origin), RemoteGitProvider.Unknown => new DefaultGitRemoteFeatures(), _ => throw new InvalidOperationException($"Unknown RemoteGitProvider: {remoteGitProvider}!"), }; diff --git a/src/Tgstation.Server.Host/Components/Repository/IGitRemoteFeatures.cs b/src/Tgstation.Server.Host/Components/Repository/IGitRemoteFeatures.cs index 0971fdc74d9..4f48e8048da 100644 --- a/src/Tgstation.Server.Host/Components/Repository/IGitRemoteFeatures.cs +++ b/src/Tgstation.Server.Host/Components/Repository/IGitRemoteFeatures.cs @@ -3,7 +3,7 @@ /// /// Provides features for remote git services. /// - interface IGitRemoteFeatures : IGitRemoteAdditionalInformation + public interface IGitRemoteFeatures : IGitRemoteAdditionalInformation { /// /// Gets a formatter string which creates the remote refspec for fetching the HEAD of passed in test merge number. diff --git a/src/Tgstation.Server.Host/Components/Repository/IGitRemoteFeaturesFactory.cs b/src/Tgstation.Server.Host/Components/Repository/IGitRemoteFeaturesFactory.cs index 42a55e6285c..4ee6ac7a12b 100644 --- a/src/Tgstation.Server.Host/Components/Repository/IGitRemoteFeaturesFactory.cs +++ b/src/Tgstation.Server.Host/Components/Repository/IGitRemoteFeaturesFactory.cs @@ -7,15 +7,22 @@ namespace Tgstation.Server.Host.Components.Repository /// /// Factory for creating . /// - interface IGitRemoteFeaturesFactory + public interface IGitRemoteFeaturesFactory { /// /// Create the for a given . /// - /// The to create for. + /// The containing the to create for. /// A new instance. IGitRemoteFeatures CreateGitRemoteFeatures(IRepository repository); + /// + /// Create the for a given . + /// + /// The of the origing URL. + /// A new instance. + IGitRemoteFeatures CreateGitRemoteFeatures(Uri origin); + /// /// Gets the for a given . /// diff --git a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs index 4e7f576fd6e..6b4a6f3175a 100644 --- a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs +++ b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs @@ -2,10 +2,13 @@ using System.Globalization; using System.Linq; using System.Linq.Expressions; +using System.Net; using System.Reflection; using System.Threading; using System.Threading.Tasks; +using GitLabApiClient; + using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; @@ -23,6 +26,7 @@ using Tgstation.Server.Host.Models; using Tgstation.Server.Host.Security; using Tgstation.Server.Host.Utils; +using Tgstation.Server.Host.Utils.GitHub; namespace Tgstation.Server.Host.Controllers { @@ -43,6 +47,16 @@ public sealed class RepositoryController : InstanceRequiredController /// readonly IJobManager jobManager; + /// + /// The for the . + /// + readonly IGitRemoteFeaturesFactory gitRemoteFeaturesFactory; + + /// + /// The for the . + /// + readonly IGitHubClientFactory gitHubClientFactory; + /// /// Initializes a new instance of the class. /// @@ -52,6 +66,8 @@ public sealed class RepositoryController : InstanceRequiredController /// The for the . /// The value of . /// The value of . + /// The value of . + /// The value of . /// The for the . public RepositoryController( IDatabaseContext databaseContext, @@ -60,6 +76,8 @@ public RepositoryController( IInstanceManager instanceManager, ILoggerFactory loggerFactory, IJobManager jobManager, + IGitRemoteFeaturesFactory gitRemoteFeaturesFactory, + IGitHubClientFactory gitHubClientFactory, IApiHeadersProvider apiHeaders) : base( databaseContext, @@ -70,6 +88,8 @@ public RepositoryController( { this.loggerFactory = loggerFactory ?? throw new ArgumentNullException(nameof(loggerFactory)); this.jobManager = jobManager ?? throw new ArgumentNullException(nameof(jobManager)); + this.gitRemoteFeaturesFactory = gitRemoteFeaturesFactory ?? throw new ArgumentNullException(nameof(gitRemoteFeaturesFactory)); + this.gitHubClientFactory = gitHubClientFactory ?? throw new ArgumentNullException(nameof(gitHubClientFactory)); } /// @@ -109,6 +129,11 @@ public async ValueTask Create([FromBody] RepositoryCreateRequest return this.Gone(); currentModel.UpdateSubmodules = model.UpdateSubmodules ?? true; + + var earlyOut = await ValidateCredentials(model, model.Origin, cancellationToken); + if (earlyOut != null) + return earlyOut; + currentModel.AccessToken = model.AccessToken; currentModel.AccessUser = model.AccessUser; @@ -425,6 +450,11 @@ bool CheckModified(Expression> express using var repo = await repoManager.LoadRepository(cancellationToken); if (repo == null) return Conflict(new ErrorMessageResponse(ErrorCode.RepoMissing)); + + var credAuthFailure = await ValidateCredentials(model, repo.Origin, cancellationToken); + if (credAuthFailure != null) + return credAuthFailure; + await PopulateApi(api, repo, DatabaseContext, Instance, cancellationToken); return null; @@ -537,5 +567,91 @@ RepositoryUpdateService CreateRepositoryUpdateService(Models.RepositorySettings AuthenticationContext.User, loggerFactory.CreateLogger(), Instance.Require(x => x.Id)); + + /// + /// Validates the of a given if it is set. + /// + /// The to validate. + /// The repository's origin . + /// The for the operation. + /// A resulting in on success, or an on validation failure. + async ValueTask ValidateCredentials(Api.Models.RepositorySettings model, Uri origin, CancellationToken cancellationToken) + { + if (String.IsNullOrWhiteSpace(model.AccessToken)) + return null; + + Logger.LogDebug("Repository access token updated, performing auth check..."); + var remoteFeatures = gitRemoteFeaturesFactory.CreateGitRemoteFeatures(origin); + switch (remoteFeatures.RemoteGitProvider!.Value) + { + case RemoteGitProvider.GitHub: + var gitHubClient = await gitHubClientFactory.CreateClientForRepository( + model.AccessToken, + new RepositoryIdentifier( + remoteFeatures.RemoteRepositoryOwner!, + remoteFeatures.RemoteRepositoryName!), + cancellationToken); + if (gitHubClient == null) + { + return this.StatusCode(HttpStatusCode.FailedDependency, new ErrorMessageResponse(ErrorCode.RemoteApiError) + { + AdditionalData = "GitHub authentication failed!", + }); + } + + try + { + string username; + if (!model.AccessToken.StartsWith(Api.Models.RepositorySettings.TgsAppPrivateKeyPrefix)) + { + var user = await gitHubClient.User.Current(); + username = user.Login; + } + else + { + // we literally need to app auth again to get the damn bot username + var appClient = gitHubClientFactory.CreateAppClient(model.AccessToken)!; + var app = await appClient.GitHubApps.GetCurrent(); + username = app.Name; + } + + if (username != model.AccessUser) + return Conflict(new ErrorMessageResponse(ErrorCode.RepoTokenUsernameMismatch)); + } + catch (Exception ex) + { + return this.StatusCode(HttpStatusCode.FailedDependency, new ErrorMessageResponse(ErrorCode.RemoteApiError) + { + AdditionalData = $"GitHub Authentication Failure: {ex.Message}", + }); + } + + break; + case RemoteGitProvider.GitLab: + // need to abstract this eventually + var gitLabClient = new GitLabClient(GitLabRemoteFeatures.GitLabUrl, model.AccessToken); + try + { + var user = await gitLabClient.Users.GetCurrentSessionAsync(); + if (user.Username != model.AccessUser) + return Conflict(new ErrorMessageResponse(ErrorCode.RepoTokenUsernameMismatch)); + } + catch (Exception ex) + { + return this.StatusCode(HttpStatusCode.FailedDependency, new ErrorMessageResponse(ErrorCode.RemoteApiError) + { + AdditionalData = $"GitLab Authentication Failure: {ex.Message}", + }); + } + + break; + case RemoteGitProvider.Unknown: + default: + Logger.LogWarning("RemoteGitProvider is {provider}, no auth check implemented!", remoteFeatures.RemoteGitProvider.Value); + break; + } + + return null; + } } } diff --git a/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs b/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs index 1ba4d8d523d..6a8d9b7a8c3 100644 --- a/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs +++ b/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs @@ -98,6 +98,10 @@ public async ValueTask CreateClient(string accessToken, Cancellat public ValueTask CreateClientForRepository(string accessString, RepositoryIdentifier repositoryIdentifier, CancellationToken cancellationToken) => GetOrCreateClient(accessString, repositoryIdentifier, cancellationToken); + /// + public IGitHubClient? CreateAppClient(string tgsEncodedAppPrivateKey) + => CreateAppClientInternal(tgsEncodedAppPrivateKey ?? throw new ArgumentNullException(nameof(tgsEncodedAppPrivateKey))); + /// /// Retrieve a from the or add a new one based on a given . /// @@ -109,7 +113,7 @@ public async ValueTask CreateClient(string accessToken, Cancellat async ValueTask GetOrCreateClient(string? accessString, RepositoryIdentifier? repositoryIdentifier, CancellationToken cancellationToken) #pragma warning restore CA1506 { - GitHubClient client; + GitHubClient? client; bool cacheHit; DateTimeOffset? lastUsed; using (await SemaphoreSlimContext.Lock(clientCacheSemaphore, cancellationToken)) @@ -129,11 +133,6 @@ public async ValueTask CreateClient(string accessToken, Cancellat if (!cacheHit) { logger.LogTrace("Creating new GitHubClient..."); - var product = assemblyInformationProvider.ProductInfoHeaderValue.Product!; - client = new GitHubClient( - new ProductHeaderValue( - product.Name, - product.Version)); if (accessString != null) { @@ -143,47 +142,10 @@ public async ValueTask CreateClient(string accessToken, Cancellat throw new InvalidOperationException("Cannot create app installation key without target repositoryIdentifier!"); logger.LogTrace("Performing GitHub App authentication for installation on repository {installationRepositoryId}", repositoryIdentifier); - var splits = accessString.Split(':'); - if (splits.Length != 2) - { - logger.LogError("Failed to parse serialized Client ID & PEM! Expected 2 chunks, got {chunkCount}", splits.Length); - return null; - } - byte[] pemBytes; - try - { - pemBytes = Convert.FromBase64String(splits[1]); - } - catch (Exception ex) - { - logger.LogError(ex, "Failed to parse supposed base64 PEM!"); + client = CreateAppClientInternal(accessString); + if (client == null) return null; - } - - var pem = Encoding.UTF8.GetString(pemBytes); - - using var rsa = RSA.Create(); - rsa.ImportFromPem(pem); - - var signingCredentials = new SigningCredentials(new RsaSecurityKey(rsa), SecurityAlgorithms.RsaSha256); - var jwtSecurityTokenHandler = new JwtSecurityTokenHandler { SetDefaultTimesOnTokenCreation = false }; - - var nowDateTime = DateTime.UtcNow; - - var appOrClientId = splits[0][RepositorySettings.TgsAppPrivateKeyPrefix.Length..]; - - var jwt = jwtSecurityTokenHandler.CreateToken(new SecurityTokenDescriptor - { - Issuer = appOrClientId, - Expires = nowDateTime.AddMinutes(10), - IssuedAt = nowDateTime, - SigningCredentials = signingCredentials, - }); - - var jwtStr = jwtSecurityTokenHandler.WriteToken(jwt); - - client.Credentials = new Credentials(jwtStr, AuthenticationType.Bearer); Installation installation; try @@ -213,8 +175,13 @@ public async ValueTask CreateClient(string accessToken, Cancellat } } else + { + client = CreateUnauthenticatedClient(); client.Credentials = new Credentials(accessString); + } } + else + client = CreateUnauthenticatedClient(); clientCache.Add(cacheKey, (Client: client, LastUsed: now)); lastUsed = null; @@ -271,5 +238,78 @@ public async ValueTask CreateClient(string accessToken, Cancellat return client; } + + /// + /// Create an App (not installation) authenticated . + /// + /// The TGS encoded app private key string. + /// A new app auth for the given on success on failure. + GitHubClient? CreateAppClientInternal(string tgsEncodedAppPrivateKey) + { + var splits = tgsEncodedAppPrivateKey.Split(':'); + if (splits.Length != 2) + { + logger.LogError("Failed to parse serialized Client ID & PEM! Expected 2 chunks, got {chunkCount}", splits.Length); + return null; + } + + byte[] pemBytes; + try + { + pemBytes = Convert.FromBase64String(splits[1]); + } + catch (Exception ex) + { + logger.LogError(ex, "Failed to parse supposed base64 PEM!"); + return null; + } + + var pem = Encoding.UTF8.GetString(pemBytes); + + using var rsa = RSA.Create(); + rsa.ImportFromPem(pem); + + var signingCredentials = new SigningCredentials( + new RsaSecurityKey(rsa), + SecurityAlgorithms.RsaSha256) + { + // https://stackoverflow.com/questions/62307933/rsa-disposed-object-error-every-other-test + CryptoProviderFactory = new CryptoProviderFactory + { + CacheSignatureProviders = false, + }, + }; + var jwtSecurityTokenHandler = new JwtSecurityTokenHandler { SetDefaultTimesOnTokenCreation = false }; + + var nowDateTime = DateTime.UtcNow; + + var appOrClientId = splits[0][RepositorySettings.TgsAppPrivateKeyPrefix.Length..]; + + var jwt = jwtSecurityTokenHandler.CreateToken(new SecurityTokenDescriptor + { + Issuer = appOrClientId, + Expires = nowDateTime.AddMinutes(10), + IssuedAt = nowDateTime, + SigningCredentials = signingCredentials, + }); + + var jwtStr = jwtSecurityTokenHandler.WriteToken(jwt); + var client = CreateUnauthenticatedClient(); + client.Credentials = new Credentials(jwtStr, AuthenticationType.Bearer); + return client; + } + + /// + /// Creates an unauthenticated . + /// + /// A new . + GitHubClient CreateUnauthenticatedClient() + { + var product = assemblyInformationProvider.ProductInfoHeaderValue.Product!; + return new GitHubClient( + new ProductHeaderValue( + product.Name, + product.Version)); + } } } diff --git a/src/Tgstation.Server.Host/Utils/GitHub/IGitHubClientFactory.cs b/src/Tgstation.Server.Host/Utils/GitHub/IGitHubClientFactory.cs index 944bcb0cf92..be630c152f9 100644 --- a/src/Tgstation.Server.Host/Utils/GitHub/IGitHubClientFactory.cs +++ b/src/Tgstation.Server.Host/Utils/GitHub/IGitHubClientFactory.cs @@ -33,5 +33,12 @@ public interface IGitHubClientFactory /// The for the operation. /// A resulting in a new for the given or if authentication failed. ValueTask CreateClientForRepository(string accessString, RepositoryIdentifier repositoryIdentifier, CancellationToken cancellationToken); + + /// + /// Create an App (not installation) authenticated . + /// + /// The TGS encoded app private key string. + /// A new app auth for the given on success on failure. + IGitHubClient? CreateAppClient(string tgsEncodedAppPrivateKey); } } From 3a7f344eea45ef3d01249681d66a56c21c0d659e Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Tue, 3 Sep 2024 23:17:15 -0400 Subject: [PATCH 15/22] Fix test auth conditional --- tests/Tgstation.Server.Tests/Live/Instance/RepositoryTest.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/Tgstation.Server.Tests/Live/Instance/RepositoryTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/RepositoryTest.cs index 86fe4ac9119..06de6115451 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/RepositoryTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/RepositoryTest.cs @@ -128,9 +128,7 @@ public async Task AbortLongCloneAndCloneSomethingQuick(Task longClo updated = await Checkout(new RepositoryUpdateRequest { Reference = "master" }, false, true, cancellationToken); // enable the good shit if possible - if (TestingUtils.RunningInGitHubActions - || String.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable("TGS_TEST_GITHUB_TOKEN")) - || Environment.MachineName.Equals("CYBERSTATIONXVI", StringComparison.OrdinalIgnoreCase)) + if (!String.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable("TGS_TEST_GITHUB_TOKEN"))) await repositoryClient.Update(new RepositoryUpdateRequest { CreateGitHubDeployments = true, From 72ef272f0825bdb60b6c3e08010f0c29588eb7a0 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Tue, 3 Sep 2024 23:18:54 -0400 Subject: [PATCH 16/22] Strip GitHub from private key prefix --- src/Tgstation.Server.Api/Models/RepositorySettings.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Api/Models/RepositorySettings.cs b/src/Tgstation.Server.Api/Models/RepositorySettings.cs index f54e1e4a6e1..66ecd82b06c 100644 --- a/src/Tgstation.Server.Api/Models/RepositorySettings.cs +++ b/src/Tgstation.Server.Api/Models/RepositorySettings.cs @@ -10,7 +10,7 @@ public abstract class RepositorySettings /// /// Prefix for TGS encoded app private keys. This is encoded in the format PREFIX + (APP_ID OR CLIENT_ID) + ':' + BASE64(APP_PRIVATE_KEY). /// - public const string TgsAppPrivateKeyPrefix = "TGS_GHPK_"; + public const string TgsAppPrivateKeyPrefix = "TGS_PK_"; /// /// The name of the committer. From a778af4f6cb606b0288f27f2e5937183d43897fa Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Tue, 3 Sep 2024 23:20:24 -0400 Subject: [PATCH 17/22] Remove unused undef --- src/DMAPI/tgs/v5/undefs.dm | 1 - 1 file changed, 1 deletion(-) diff --git a/src/DMAPI/tgs/v5/undefs.dm b/src/DMAPI/tgs/v5/undefs.dm index d8dbf7a7818..acd19dfa641 100644 --- a/src/DMAPI/tgs/v5/undefs.dm +++ b/src/DMAPI/tgs/v5/undefs.dm @@ -17,7 +17,6 @@ #undef DMAPI5_BRIDGE_COMMAND_EVENT #undef DMAPI5_PARAMETER_ACCESS_IDENTIFIER -#undef DMAPI5_PARAMETER_CLIENT_COUNT #undef DMAPI5_PARAMETER_CUSTOM_COMMANDS #undef DMAPI5_CHUNK From f2e87b0b15cdb51c0e84957166a09302deb38bb5 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Tue, 3 Sep 2024 23:21:27 -0400 Subject: [PATCH 18/22] Better documentation comment --- src/Tgstation.Server.Api/Models/ErrorCode.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Api/Models/ErrorCode.cs b/src/Tgstation.Server.Api/Models/ErrorCode.cs index cb30e83915f..052941d9222 100644 --- a/src/Tgstation.Server.Api/Models/ErrorCode.cs +++ b/src/Tgstation.Server.Api/Models/ErrorCode.cs @@ -659,7 +659,7 @@ public enum ErrorCode : uint DeploymentWrongDme, /// - /// Entered wrong username for a repository access token. + /// Entered wrong for a . /// [Description("Provided repository username doesn't match the user of the corresponding access token!")] RepoTokenUsernameMismatch, From 7415005d6ecfb7ed9dd6aba1d12edaf904c028b4 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Tue, 3 Sep 2024 23:22:31 -0400 Subject: [PATCH 19/22] Client count is unsigned for the love of god --- .../Models/Internal/DreamDaemonApiBase.cs | 2 +- .../Components/Interop/Topic/TopicResponse.cs | 2 +- src/Tgstation.Server.Host/Components/Watchdog/IWatchdog.cs | 2 +- src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs | 2 +- tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Tgstation.Server.Api/Models/Internal/DreamDaemonApiBase.cs b/src/Tgstation.Server.Api/Models/Internal/DreamDaemonApiBase.cs index e3411b8115f..c0c6f63374f 100644 --- a/src/Tgstation.Server.Api/Models/Internal/DreamDaemonApiBase.cs +++ b/src/Tgstation.Server.Api/Models/Internal/DreamDaemonApiBase.cs @@ -23,7 +23,7 @@ public abstract class DreamDaemonApiBase : DreamDaemonSettings /// The last known count of connected players. Requires to not be 0 and a game server interop version >= 5.10.0 to populate. /// [ResponseOptions] - public int? ClientCount { get; set; } + public uint? ClientCount { get; set; } /// /// If the server is undergoing a soft reset. This may be automatically set by changes to other fields. diff --git a/src/Tgstation.Server.Host/Components/Interop/Topic/TopicResponse.cs b/src/Tgstation.Server.Host/Components/Interop/Topic/TopicResponse.cs index 45bae93f754..a374bd6750c 100644 --- a/src/Tgstation.Server.Host/Components/Interop/Topic/TopicResponse.cs +++ b/src/Tgstation.Server.Host/Components/Interop/Topic/TopicResponse.cs @@ -40,6 +40,6 @@ sealed class TopicResponse : DMApiResponse, IMissingPayloadsCommunication /// /// The number of connected clients to the game. Added in Interop 5.10.0. /// - public int? ClientCount { get; set; } + public uint? ClientCount { get; set; } } } diff --git a/src/Tgstation.Server.Host/Components/Watchdog/IWatchdog.cs b/src/Tgstation.Server.Host/Components/Watchdog/IWatchdog.cs index 35da1504f83..c1dd923c002 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/IWatchdog.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/IWatchdog.cs @@ -27,7 +27,7 @@ public interface IWatchdog : IComponentService, IAsyncDisposable, IEventConsumer /// /// Last known client count queried from the DMAPI. Requires health checks to be enabled to populate. /// - int? ClientCount { get; } + uint? ClientCount { get; } /// /// The current . diff --git a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs index 3c67ca495c1..e0aff03f405 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs @@ -39,7 +39,7 @@ abstract class WatchdogBase : IWatchdog, ICustomCommandHandler, IRestartHandler public long? SessionId => GetActiveController()?.ReattachInformation.Id; /// - public int? ClientCount { get; private set; } + public uint? ClientCount { get; private set; } /// public DateTimeOffset? LaunchTime => GetActiveController()?.LaunchTime; diff --git a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs index d7930e0cc61..2966ec20e69 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs @@ -754,7 +754,7 @@ void ValidateSessionId(DreamDaemonResponse daemonStatus, bool? knownIncrease) Assert.IsTrue(daemonStatus.LaunchTime.Value >= DateTimeOffset.UtcNow.AddHours(-1)); if (daemonStatus.ClientCount.HasValue) - Assert.AreEqual(0, daemonStatus.ClientCount.Value); + Assert.AreEqual(0U, daemonStatus.ClientCount.Value); if (sessionIdTracker.HasValue) if (knownIncrease.HasValue) @@ -865,7 +865,7 @@ async Task RunHealthCheckTest(bool checkDump, CancellationToken cancellationToke await Task.WhenAny(Task.Delay(7000, cancellationToken), ourProcessHandler.Lifetime); Assert.IsFalse(ddProc.HasExited); var daemonStatus = await instanceClient.DreamDaemon.Read(cancellationToken); - Assert.AreEqual(0, daemonStatus.ClientCount); + Assert.AreEqual(0U, daemonStatus.ClientCount); // check DD agrees var topicRequestResult = await SendTestTopic( From 984fb9b54d3dfa24f5e7acedbd02d95c2dd786e3 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Wed, 4 Sep 2024 06:40:11 -0400 Subject: [PATCH 20/22] Add exception handler wrapper for `Tgstation.Server.Host.Console` --- src/Tgstation.Server.Host.Console/Program.cs | 57 +++++++++++--------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/src/Tgstation.Server.Host.Console/Program.cs b/src/Tgstation.Server.Host.Console/Program.cs index 26f5b8f5b55..8e28416db4e 100644 --- a/src/Tgstation.Server.Host.Console/Program.cs +++ b/src/Tgstation.Server.Host.Console/Program.cs @@ -61,37 +61,46 @@ internal static async Task Main(string[] args) builder.AddConsole(); }); - if (trace && debug) - { - loggerFactory.CreateLogger(nameof(Program)).LogCritical("Please specify only 1 of --trace-host-watchdog or --debug-host-watchdog!"); - return 2; - } - - using var cts = new CancellationTokenSource(); - void AppDomainHandler(object? a, EventArgs b) => cts.Cancel(); - AppDomain.CurrentDomain.ProcessExit += AppDomainHandler; + var logger = loggerFactory.CreateLogger(nameof(Program)); try { - System.Console.CancelKeyPress += (a, b) => + if (trace && debug) { - b.Cancel = true; - cts.Cancel(); - }; + logger.LogCritical("Please specify only 1 of --trace-host-watchdog or --debug-host-watchdog!"); + return 2; + } - var watchdog = WatchdogFactory.CreateWatchdog( - RuntimeInformation.IsOSPlatform(OSPlatform.Windows) - ? new NoopSignalChecker() - : new PosixSignalChecker( - loggerFactory.CreateLogger()), - loggerFactory); + using var cts = new CancellationTokenSource(); + void AppDomainHandler(object? a, EventArgs b) => cts.Cancel(); + AppDomain.CurrentDomain.ProcessExit += AppDomainHandler; + try + { + System.Console.CancelKeyPress += (a, b) => + { + b.Cancel = true; + cts.Cancel(); + }; + + var watchdog = WatchdogFactory.CreateWatchdog( + RuntimeInformation.IsOSPlatform(OSPlatform.Windows) + ? new NoopSignalChecker() + : new PosixSignalChecker( + loggerFactory.CreateLogger()), + loggerFactory); - return await watchdog.RunAsync(false, arguments.ToArray(), cts.Token) - ? 0 - : 1; + return await watchdog.RunAsync(false, arguments.ToArray(), cts.Token) + ? 0 + : 1; + } + finally + { + AppDomain.CurrentDomain.ProcessExit -= AppDomainHandler; + } } - finally + catch (Exception ex) { - AppDomain.CurrentDomain.ProcessExit -= AppDomainHandler; + logger.LogCritical(ex, "Failed to run!"); + return 3; } } } From 927ea68421dce22636296ff17956da85e1075a10 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Wed, 4 Sep 2024 06:46:57 -0400 Subject: [PATCH 21/22] Fix fake PEM test on Linux --- .../Utils/GitHub/GitHubClientFactory.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs b/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs index 6a8d9b7a8c3..bf4ce1b69ef 100644 --- a/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs +++ b/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs @@ -267,7 +267,16 @@ public async ValueTask CreateClient(string accessToken, Cancellat var pem = Encoding.UTF8.GetString(pemBytes); using var rsa = RSA.Create(); - rsa.ImportFromPem(pem); + + try + { + rsa.ImportFromPem(pem); + } + catch (Exception ex) + { + logger.LogWarning(ex, "Failed to parse PEM!"); + return null; + } var signingCredentials = new SigningCredentials( new RsaSecurityKey(rsa), From 02a9f268a9070900cb68cff5a248670340b0fcf0 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Wed, 4 Sep 2024 07:02:54 -0400 Subject: [PATCH 22/22] Adjusting code to satisfy tests --- src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs b/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs index bf4ce1b69ef..68d9dfac5b9 100644 --- a/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs +++ b/src/Tgstation.Server.Host/Utils/GitHub/GitHubClientFactory.cs @@ -246,6 +246,7 @@ public async ValueTask CreateClient(string accessToken, Cancellat /// A new app auth for the given on success on failure. GitHubClient? CreateAppClientInternal(string tgsEncodedAppPrivateKey) { + var client = CreateUnauthenticatedClient(); var splits = tgsEncodedAppPrivateKey.Split(':'); if (splits.Length != 2) { @@ -303,7 +304,6 @@ public async ValueTask CreateClient(string accessToken, Cancellat }); var jwtStr = jwtSecurityTokenHandler.WriteToken(jwt); - var client = CreateUnauthenticatedClient(); client.Credentials = new Credentials(jwtStr, AuthenticationType.Bearer); return client; }