From 9e5487494ed766a3922ab5621abf4cedf8a69aee Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Thu, 24 Aug 2023 19:46:40 -0400 Subject: [PATCH 1/6] Fix configuration "reloading" during setup wizard It runs again after setup so it's completely unnecessary. --- src/Tgstation.Server.Host/ServerFactory.cs | 6 +++++- .../Setup/SetupWizard.cs | 20 +------------------ 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/src/Tgstation.Server.Host/ServerFactory.cs b/src/Tgstation.Server.Host/ServerFactory.cs index 0256395b7d..2dd323022d 100644 --- a/src/Tgstation.Server.Host/ServerFactory.cs +++ b/src/Tgstation.Server.Host/ServerFactory.cs @@ -24,6 +24,11 @@ namespace Tgstation.Server.Host /// sealed class ServerFactory : IServerFactory { + /// + /// Name of the appsettings file. + /// + public const string AppSettings = "appsettings"; + /// /// The for the . /// @@ -60,7 +65,6 @@ namespace Tgstation.Server.Host args[oldArgs.Length] = "--hostBuilder:reloadConfigOnChange=false"; } - const string AppSettings = "appsettings"; const string AppSettingsRelocationKey = $"--{AppSettings}-base-path="; var appsettingsRelativeBasePathArgument = args.FirstOrDefault(arg => arg.StartsWith(AppSettingsRelocationKey, StringComparison.Ordinal)); diff --git a/src/Tgstation.Server.Host/Setup/SetupWizard.cs b/src/Tgstation.Server.Host/Setup/SetupWizard.cs index 3ea5f694f6..92b3d53c42 100644 --- a/src/Tgstation.Server.Host/Setup/SetupWizard.cs +++ b/src/Tgstation.Server.Host/Setup/SetupWizard.cs @@ -87,11 +87,6 @@ namespace Tgstation.Server.Host.Setup /// readonly InternalConfiguration internalConfiguration; - /// - /// A that will complete when the is reloaded. - /// - TaskCompletionSource reloadTcs; - /// /// Initializes a new instance of the class. /// @@ -131,12 +126,6 @@ namespace Tgstation.Server.Host.Setup generalConfiguration = generalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(generalConfigurationOptions)); internalConfiguration = internalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(internalConfigurationOptions)); - - configuration - .GetReloadToken() - .RegisterChangeCallback( - state => reloadTcs?.TrySetResult(), - null); } /// @@ -1014,19 +1003,12 @@ namespace Tgstation.Server.Host.Setup var configBytes = Encoding.UTF8.GetBytes(serializedYaml); - reloadTcs = new TaskCompletionSource(); - try { await ioManager.WriteAllBytes( userConfigFileName, configBytes, cancellationToken); - - // Ensure the reload - if (generalConfiguration.SetupWizardMode != SetupWizardMode.Only) - using (cancellationToken.Register(() => reloadTcs.TrySetCanceled())) - await reloadTcs.Task; } catch (OperationCanceledException) { @@ -1108,7 +1090,7 @@ namespace Tgstation.Server.Host.Setup var userConfigFileName = ioManager.ConcatPath( internalConfiguration.AppSettingsBasePath, - String.Format(CultureInfo.InvariantCulture, "appsettings.{0}.yml", hostingEnvironment.EnvironmentName)); + $"{ServerFactory.AppSettings}.{hostingEnvironment.EnvironmentName}.yml"); async Task HandleSetupCancel() { From fd1e73dfd49300ba24b85f86ba55edfec1fb7d7b Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Thu, 24 Aug 2023 19:47:00 -0400 Subject: [PATCH 2/6] Add prompt for SQLServer connection encryption --- src/Tgstation.Server.Host/Setup/SetupWizard.cs | 8 +++++--- .../Tgstation.Server.Host.Tests/Setup/TestSetupWizard.cs | 2 ++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Tgstation.Server.Host/Setup/SetupWizard.cs b/src/Tgstation.Server.Host/Setup/SetupWizard.cs index 92b3d53c42..35024c47bb 100644 --- a/src/Tgstation.Server.Host/Setup/SetupWizard.cs +++ b/src/Tgstation.Server.Host/Setup/SetupWizard.cs @@ -501,16 +501,16 @@ namespace Tgstation.Server.Host.Setup } while (true); - bool useWinAuth; + var useWinAuth = false; + var encrypt = false; if (databaseConfiguration.DatabaseType == DatabaseType.SqlServer && platformIdentifier.IsWindows) { var defaultResponse = serverAddressEntry?.AddressList.Any(IPAddress.IsLoopback) ?? false ? (bool?)true : null; useWinAuth = await PromptYesNo("Use Windows Authentication?", defaultResponse, cancellationToken); + encrypt = await PromptYesNo("Use encrypted connection?", false, cancellationToken); } - else - useWinAuth = false; await console.WriteAsync(null, true, cancellationToken); @@ -566,6 +566,8 @@ namespace Tgstation.Server.Host.Setup csb.Password = password; } + csb.Encrypt = encrypt; + CreateTestConnection(csb.ConnectionString); csb.InitialCatalog = databaseName; databaseConfiguration.ConnectionString = csb.ConnectionString; diff --git a/tests/Tgstation.Server.Host.Tests/Setup/TestSetupWizard.cs b/tests/Tgstation.Server.Host.Tests/Setup/TestSetupWizard.cs index 7ad15d9ad0..c60d7308c2 100644 --- a/tests/Tgstation.Server.Host.Tests/Setup/TestSetupWizard.cs +++ b/tests/Tgstation.Server.Host.Tests/Setup/TestSetupWizard.cs @@ -181,6 +181,8 @@ namespace Tgstation.Server.Host.Setup.Tests "no", //test winauth "yes", + // encrypt + "YES", //sql server will always fail so reconfigure with maria nameof(DatabaseType.MariaDB), "127.0.0.1", From e9eaa42f5ca6e4db44b696560d13618ec5072fdc Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Thu, 24 Aug 2023 22:15:09 -0400 Subject: [PATCH 3/6] Update to web control panel 4.24.0 Version bump to 5.15.0 --- build/ControlPanelVersion.props | 2 +- build/Version.props | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/build/ControlPanelVersion.props b/build/ControlPanelVersion.props index fbfe1438e0..ce868e2875 100644 --- a/build/ControlPanelVersion.props +++ b/build/ControlPanelVersion.props @@ -1,6 +1,6 @@ - 4.23.1 + 4.24.0 diff --git a/build/Version.props b/build/Version.props index 6d63f5b310..de451f3b0e 100644 --- a/build/Version.props +++ b/build/Version.props @@ -3,7 +3,7 @@ - 5.14.1 + 5.15.0 4.7.1 9.12.0 6.0.1 From 643f369aa33816110ccd14d118f2f04faae1f513 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Thu, 24 Aug 2023 22:42:47 -0400 Subject: [PATCH 4/6] Fix setup wizard tests --- .../Setup/SetupWizard.cs | 8 +--- .../Setup/TestSetupWizard.cs | 44 +++++-------------- 2 files changed, 11 insertions(+), 41 deletions(-) diff --git a/src/Tgstation.Server.Host/Setup/SetupWizard.cs b/src/Tgstation.Server.Host/Setup/SetupWizard.cs index 35024c47bb..f265520b4d 100644 --- a/src/Tgstation.Server.Host/Setup/SetupWizard.cs +++ b/src/Tgstation.Server.Host/Setup/SetupWizard.cs @@ -110,7 +110,6 @@ namespace Tgstation.Server.Host.Setup IPlatformIdentifier platformIdentifier, IAsyncDelayer asyncDelayer, IHostApplicationLifetime applicationLifetime, - IConfiguration configuration, IOptions generalConfigurationOptions, IOptions internalConfigurationOptions) { @@ -122,7 +121,6 @@ namespace Tgstation.Server.Host.Setup this.platformIdentifier = platformIdentifier ?? throw new ArgumentNullException(nameof(platformIdentifier)); this.asyncDelayer = asyncDelayer ?? throw new ArgumentNullException(nameof(asyncDelayer)); this.applicationLifetime = applicationLifetime ?? throw new ArgumentNullException(nameof(applicationLifetime)); - ArgumentNullException.ThrowIfNull(configuration); generalConfiguration = generalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(generalConfigurationOptions)); internalConfiguration = internalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(internalConfigurationOptions)); @@ -1012,11 +1010,7 @@ namespace Tgstation.Server.Host.Setup configBytes, cancellationToken); } - catch (OperationCanceledException) - { - throw; - } - catch (Exception e) + catch (Exception e) when (e is not OperationCanceledException) { await console.WriteAsync(e.Message, true, cancellationToken); await console.WriteAsync(null, true, cancellationToken); diff --git a/tests/Tgstation.Server.Host.Tests/Setup/TestSetupWizard.cs b/tests/Tgstation.Server.Host.Tests/Setup/TestSetupWizard.cs index c60d7308c2..3b9da3b2b3 100644 --- a/tests/Tgstation.Server.Host.Tests/Setup/TestSetupWizard.cs +++ b/tests/Tgstation.Server.Host.Tests/Setup/TestSetupWizard.cs @@ -27,28 +27,25 @@ namespace Tgstation.Server.Host.Setup.Tests [TestMethod] public void TestConstructionThrows() { - Assert.ThrowsException(() => new SetupWizard(null, null, null, null, null, null, null, null, null, null, null)); + Assert.ThrowsException(() => new SetupWizard(null, null, null, null, null, null, null, null, null, null)); var mockIOManager = new Mock(); - Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, null, null, null, null, null, null, null, null, null, null)); + Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, null, null, null, null, null, null, null, null, null)); var mockConsole = new Mock(); - Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, null, null, null, null, null, null, null, null, null)); + Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, null, null, null, null, null, null, null, null)); var mockHostingEnvironment = new Mock(); - Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, null, null, null, null, null, null, null, null)); + Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, null, null, null, null, null, null, null)); var mockAssemblyInfoProvider = new Mock(); - Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, null, null, null, null, null, null, null)); + Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, null, null, null, null, null, null)); var mockDBConnectionFactory = new Mock(); - Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, null, null, null, null, null, null)); + Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, null, null, null, null, null)); var mockPlatformIdentifier = new Mock(); - Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, null, null, null, null, null)); + Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, null, null, null, null)); var mockAsyncDelayer = new Mock(); - Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, mockAsyncDelayer.Object, null, null, null, null)); + Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, mockAsyncDelayer.Object, null, null, null)); var mockLifetime = new Mock(); - Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, mockAsyncDelayer.Object, mockLifetime.Object, null, null, null)); - var mockConfiguration = new Mock(); - mockConfiguration.Setup(x => x.GetReloadToken()).Returns(Mock.Of()); - Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, mockAsyncDelayer.Object, mockLifetime.Object, mockConfiguration.Object, null, null)); + Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, mockAsyncDelayer.Object, mockLifetime.Object, null, null)); var mockGeneralConfigurationOptions = Options.Create(new GeneralConfiguration()); - Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, mockAsyncDelayer.Object, mockLifetime.Object, mockConfiguration.Object, mockGeneralConfigurationOptions, null)); + Assert.ThrowsException(() => new SetupWizard(mockIOManager.Object, mockConsole.Object, mockHostingEnvironment.Object, mockAssemblyInfoProvider.Object, mockDBConnectionFactory.Object, mockPlatformIdentifier.Object, mockAsyncDelayer.Object, mockLifetime.Object, mockGeneralConfigurationOptions, null)); } [TestMethod] @@ -64,23 +61,6 @@ namespace Tgstation.Server.Host.Setup.Tests var mockInternalConfigurationOptions = new Mock>(); var mockPlatformIdentifier = new Mock(); var mockAsyncDelayer = new Mock(); - var mockConfiguration = new Mock(); - var mockChangeToken = new Mock(); - - object configReloadCallbackState = null; - Action configReloadCallback = null; - mockChangeToken - .Setup(x => x.RegisterChangeCallback(It.IsNotNull>(), null)) - .Callback, object>((callback, state) => - { - configReloadCallback = callback; - configReloadCallbackState = state; - }); - - mockConfiguration - .Setup(x => x.GetReloadToken()) - .Returns(mockChangeToken.Object) - .Verifiable(); var testGeneralConfig = new GeneralConfiguration { @@ -103,7 +83,6 @@ namespace Tgstation.Server.Host.Setup.Tests mockPlatformIdentifier.Object, mockAsyncDelayer.Object, mockLifetime.Object, - mockConfiguration.Object, mockGeneralConfigurationOptions.Object, mockInternalConfigurationOptions.Object); @@ -128,7 +107,6 @@ namespace Tgstation.Server.Host.Setup.Tests mockIOManager.Setup(x => x.ReadAllBytes(It.IsNotNull(), It.IsAny())).Returns(Task.FromResult(Encoding.UTF8.GetBytes("less profane"))).Verifiable(); mockIOManager .Setup(x => x.WriteAllBytes(It.IsNotNull(), It.IsNotNull(), It.IsAny())) - .Callback(() => configReloadCallback(configReloadCallbackState)) .Returns(Task.CompletedTask) .Verifiable(); @@ -369,8 +347,6 @@ namespace Tgstation.Server.Host.Setup.Tests mockAssemblyInfoProvider.VerifyAll(); mockPlatformIdentifier.VerifyAll(); mockAsyncDelayer.VerifyAll(); - mockConfiguration.VerifyAll(); - mockChangeToken.VerifyAll(); } } } From c2b78e7edf356c37d4f4abdee7bece089c7e90af Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Thu, 24 Aug 2023 22:48:23 -0400 Subject: [PATCH 5/6] Remove bad doc comment --- src/Tgstation.Server.Host/Setup/SetupWizard.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Setup/SetupWizard.cs b/src/Tgstation.Server.Host/Setup/SetupWizard.cs index f265520b4d..fb171af566 100644 --- a/src/Tgstation.Server.Host/Setup/SetupWizard.cs +++ b/src/Tgstation.Server.Host/Setup/SetupWizard.cs @@ -98,7 +98,6 @@ namespace Tgstation.Server.Host.Setup /// The value of . /// The value of . /// The value of . - /// The in use. /// The containing the value of . /// The containing the value of . public SetupWizard( From 9e55d453aa2d5a9bc5e3dea01e3f18ace7f0428f Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Thu, 24 Aug 2023 22:51:00 -0400 Subject: [PATCH 6/6] Remove unused using --- src/Tgstation.Server.Host/Setup/SetupWizard.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Setup/SetupWizard.cs b/src/Tgstation.Server.Host/Setup/SetupWizard.cs index fb171af566..40656cad2e 100644 --- a/src/Tgstation.Server.Host/Setup/SetupWizard.cs +++ b/src/Tgstation.Server.Host/Setup/SetupWizard.cs @@ -12,7 +12,6 @@ using System.Threading.Tasks; using Microsoft.Data.SqlClient; using Microsoft.Data.Sqlite; -using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options;