From 7ed2ec73db1e4c45d32f0414a79fc5e785597f89 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Thu, 10 Oct 2024 18:38:48 -0400 Subject: [PATCH 1/6] Always inject GraphQL services as they are too intertwined with the server to exclude --- src/Tgstation.Server.Host/Core/Application.cs | 89 +++++++++---------- 1 file changed, 44 insertions(+), 45 deletions(-) diff --git a/src/Tgstation.Server.Host/Core/Application.cs b/src/Tgstation.Server.Host/Core/Application.cs index 3a310a7ec9..429cbc8989 100644 --- a/src/Tgstation.Server.Host/Core/Application.cs +++ b/src/Tgstation.Server.Host/Core/Application.cs @@ -295,55 +295,54 @@ void ConfigureNewtonsoftJsonSerializerSettingsForApi(JsonSerializerSettings sett services.AddSingleton(); // configure graphql - if (postSetupServices.InternalConfiguration.EnableGraphQL) - services - .AddScoped() - .AddGraphQLServer() - .AddAuthorization() - .ModifyOptions(options => - { - options.EnsureAllNodesCanBeResolved = true; - options.EnableFlagEnums = true; - }) + services + .AddScoped() + .AddGraphQLServer() + .AddAuthorization() + .ModifyOptions(options => + { + options.EnsureAllNodesCanBeResolved = true; + options.EnableFlagEnums = true; + }) #if DEBUG - .ModifyCostOptions(options => - { - options.EnforceCostLimits = false; - }) + .ModifyCostOptions(options => + { + options.EnforceCostLimits = false; + }) #endif - .AddMutationConventions() - .AddInMemorySubscriptions( - new SubscriptionOptions - { - TopicBufferCapacity = 1024, // mainly so high for tests, not possible to DoS the server without authentication and some other access to generate messages - }) - .AddGlobalObjectIdentification() - .AddQueryFieldToMutationPayloads() - .ModifyOptions(options => - { - options.EnableDefer = true; - }) - .ModifyPagingOptions(pagingOptions => + .AddMutationConventions() + .AddInMemorySubscriptions( + new SubscriptionOptions { - pagingOptions.IncludeTotalCount = true; - pagingOptions.RequirePagingBoundaries = false; - pagingOptions.DefaultPageSize = ApiController.DefaultPageSize; - pagingOptions.MaxPageSize = ApiController.MaximumPageSize; + TopicBufferCapacity = 1024, // mainly so high for tests, not possible to DoS the server without authentication and some other access to generate messages }) - .AddFiltering() - .AddSorting() - .AddHostTypes() - .AddErrorFilter() - .AddType() - .AddType() - .AddType() - .AddType() - .AddType() - .BindRuntimeType() - .TryAddTypeInterceptor() - .AddQueryType() - .AddMutationType() - .AddSubscriptionType(); + .AddGlobalObjectIdentification() + .AddQueryFieldToMutationPayloads() + .ModifyOptions(options => + { + options.EnableDefer = true; + }) + .ModifyPagingOptions(pagingOptions => + { + pagingOptions.IncludeTotalCount = true; + pagingOptions.RequirePagingBoundaries = false; + pagingOptions.DefaultPageSize = ApiController.DefaultPageSize; + pagingOptions.MaxPageSize = ApiController.MaximumPageSize; + }) + .AddFiltering() + .AddSorting() + .AddHostTypes() + .AddErrorFilter() + .AddType() + .AddType() + .AddType() + .AddType() + .AddType() + .BindRuntimeType() + .TryAddTypeInterceptor() + .AddQueryType() + .AddMutationType() + .AddSubscriptionType(); void AddTypedContext() where TContext : DatabaseContext From 80c12ee218d4a5cdf0eeb26bc85acca0dddec6a6 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Thu, 10 Oct 2024 18:39:23 -0400 Subject: [PATCH 2/6] Version bump to 6.11.1 --- build/Version.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Version.props b/build/Version.props index 32469c8252..dc59a0b959 100644 --- a/build/Version.props +++ b/build/Version.props @@ -3,7 +3,7 @@ - 6.11.0 + 6.11.1 5.3.0 10.10.0 0.2.0 From d3082b2ffc3f7d7b273359e82956fb8842c96dc6 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Thu, 10 Oct 2024 18:44:24 -0400 Subject: [PATCH 3/6] Do not blanket enable GraphQL in CI --- .github/workflows/ci-pipeline.yml | 1 - tests/Tgstation.Server.Tests/Live/LiveTestingServer.cs | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci-pipeline.yml b/.github/workflows/ci-pipeline.yml index 41bef2a36a..ccd353acf7 100644 --- a/.github/workflows/ci-pipeline.yml +++ b/.github/workflows/ci-pipeline.yml @@ -41,7 +41,6 @@ env: TGS_WEBPANEL_NODE_VERSION: 20.x TGS_TEST_GITHUB_TOKEN: ${{ secrets.LIVE_TESTS_TOKEN }} PACKAGING_PRIVATE_KEY_PASSPHRASE: ${{ secrets.PACKAGING_PRIVATE_KEY_PASSPHRASE }} - Internal__EnableGraphQL: true concurrency: group: "ci-${{ (github.event_name != 'push' && github.event_name != 'schedule' && github.event.inputs.pull_request_number) || github.run_id }}-${{ github.event_name }}" diff --git a/tests/Tgstation.Server.Tests/Live/LiveTestingServer.cs b/tests/Tgstation.Server.Tests/Live/LiveTestingServer.cs index f9ed0b926a..bbd6bb176c 100644 --- a/tests/Tgstation.Server.Tests/Live/LiveTestingServer.cs +++ b/tests/Tgstation.Server.Tests/Live/LiveTestingServer.cs @@ -158,6 +158,9 @@ public LiveTestingServer(SwarmConfiguration swarmConfiguration, bool enableOAuth "Telemetry:DisableVersionReporting=true", }; + if (MultiServerClient.UseGraphQL) + args.Add("Internal:EnableGraphQL=true"); + swarmArgs = new List(); if (swarmConfiguration != null) { From 772af9cd83c43489a8301a22d8351857eb6cf6fe Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Thu, 10 Oct 2024 19:52:24 -0400 Subject: [PATCH 4/6] Fix cases of MultiServerClient not being able to handle GraphQL being disabled --- .../Tgstation.Server.Tests/Live/MultiServerClient.cs | 9 +++++++-- tests/Tgstation.Server.Tests/Live/TestLiveServer.cs | 12 +++++++----- tests/Tgstation.Server.Tests/Live/UsersTest.cs | 3 +++ 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/tests/Tgstation.Server.Tests/Live/MultiServerClient.cs b/tests/Tgstation.Server.Tests/Live/MultiServerClient.cs index 5dbf153d78..9cc8f280b8 100644 --- a/tests/Tgstation.Server.Tests/Live/MultiServerClient.cs +++ b/tests/Tgstation.Server.Tests/Live/MultiServerClient.cs @@ -28,7 +28,7 @@ public MultiServerClient(IRestServerClient restServerClient, IGraphQLServerClien public ValueTask DisposeAsync() => ValueTaskExtensions.WhenAll( RestClient.DisposeAsync(), - GraphQLClient.DisposeAsync()); + GraphQLClient?.DisposeAsync() ?? ValueTask.CompletedTask); public ValueTask Execute( Func restAction, @@ -48,6 +48,11 @@ public ValueTask Execute( where TGraphQLResult : class { var restTask = restAction(RestClient); + if (!UseGraphQL) + { + return (await restTask, null); + } + var graphQLResult = await GraphQLClient.RunOperation(graphQLAction, cancellationToken); graphQLResult.EnsureNoErrors(); @@ -60,6 +65,6 @@ public ValueTask Execute( } public ValueTask Subscribe(Func>> operationExecutor, IObserver> observer, CancellationToken cancellationToken) where TResultData : class - => GraphQLClient.Subscribe(operationExecutor, observer, cancellationToken); + => GraphQLClient?.Subscribe(operationExecutor, observer, cancellationToken) ?? ValueTask.FromResult(new CancellationTokenSource()); } } diff --git a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs index 2092e0819f..edd368bec1 100644 --- a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs +++ b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs @@ -1930,11 +1930,13 @@ async ValueTask CreateClient( password, cancellationToken: cancellationToken); using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - graphQLClientTask = graphQLClientFactory.CreateFromLogin( - url, - username, - password, - cancellationToken: cts.Token); + graphQLClientTask = MultiServerClient.UseGraphQL + ? graphQLClientFactory.CreateFromLogin( + url, + username, + password, + cancellationToken: cts.Token) + : ValueTask.FromResult(null); IRestServerClient restClient; try diff --git a/tests/Tgstation.Server.Tests/Live/UsersTest.cs b/tests/Tgstation.Server.Tests/Live/UsersTest.cs index ec6220d8fa..8d918804fe 100644 --- a/tests/Tgstation.Server.Tests/Live/UsersTest.cs +++ b/tests/Tgstation.Server.Tests/Live/UsersTest.cs @@ -48,6 +48,9 @@ await ValueTaskExtensions.WhenAll( await TestPagination(cancellationToken); + if (!MultiServerClient.UseGraphQL) + return; + Assert.IsFalse(observer.Completed); Assert.AreEqual(0U, observer.ErrorCount); From e1f7f209c9307c777ab8fad209c45f0a59448cd2 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Fri, 11 Oct 2024 00:00:17 -0400 Subject: [PATCH 5/6] Fix more cases of tests using GraphQL indiscriminately --- .../Live/TestLiveServer.cs | 99 ++++++++++--------- 1 file changed, 55 insertions(+), 44 deletions(-) diff --git a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs index edd368bec1..9140a29df3 100644 --- a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs +++ b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs @@ -1372,12 +1372,13 @@ async Task TestTgsInternal(bool openDreamOnly, CancellationToken hardCancellatio var firstAdminRestClient = firstAdminMultiClient.RestClient; - await using (var tokenOnlyGraphQLClient = graphQLClientFactory.CreateFromToken(server.RootUrl, firstAdminRestClient.Token.Bearer)) - { - // just testing auth works the same here - var result = await tokenOnlyGraphQLClient.RunOperation(client => client.ServerVersion.ExecuteAsync(cancellationToken), cancellationToken); - Assert.IsTrue(result.IsSuccessResult()); - } + if (MultiServerClient.UseGraphQL) + await using (var tokenOnlyGraphQLClient = graphQLClientFactory.CreateFromToken(server.RootUrl, firstAdminRestClient.Token.Bearer)) + { + // just testing auth works the same here + var result = await tokenOnlyGraphQLClient.RunOperation(client => client.ServerVersion.ExecuteAsync(cancellationToken), cancellationToken); + Assert.IsTrue(result.IsSuccessResult()); + } await using (var tokenOnlyRestClient = restClientFactory.CreateFromToken(server.RootUrl, firstAdminRestClient.Token)) { @@ -1393,32 +1394,33 @@ async Task TestTgsInternal(bool openDreamOnly, CancellationToken hardCancellatio } // basic graphql test, to be used everywhere eventually - await using (var unauthenticatedGraphQLClient = graphQLClientFactory.CreateUnauthenticated(server.RootUrl)) - { - // check auth works as expected - var result = await unauthenticatedGraphQLClient.RunOperation(client => client.ServerVersion.ExecuteAsync(cancellationToken), cancellationToken); - Assert.IsTrue(result.IsErrorResult()); + if (MultiServerClient.UseGraphQL) + await using (var unauthenticatedGraphQLClient = graphQLClientFactory.CreateUnauthenticated(server.RootUrl)) + { + // check auth works as expected + var result = await unauthenticatedGraphQLClient.RunOperation(client => client.ServerVersion.ExecuteAsync(cancellationToken), cancellationToken); + Assert.IsTrue(result.IsErrorResult()); - // test getting server info - var unAuthedMultiClient = new MultiServerClient(firstAdminRestClient, unauthenticatedGraphQLClient); + // test getting server info + var unAuthedMultiClient = new MultiServerClient(firstAdminRestClient, unauthenticatedGraphQLClient); - await unauthenticatedGraphQLClient.RunQueryEnsureNoErrors( - gqlClient => gqlClient.UnauthenticatedServerInformation.ExecuteAsync(cancellationToken), - cancellationToken); + await unauthenticatedGraphQLClient.RunQueryEnsureNoErrors( + gqlClient => gqlClient.UnauthenticatedServerInformation.ExecuteAsync(cancellationToken), + cancellationToken); - var testObserver = new HoldLastObserver>(); - using var subscription = await unauthenticatedGraphQLClient.Subscribe( - gql => gql.SessionInvalidation.Watch(), - testObserver, - cancellationToken); + var testObserver = new HoldLastObserver>(); + using var subscription = await unauthenticatedGraphQLClient.Subscribe( + gql => gql.SessionInvalidation.Watch(), + testObserver, + cancellationToken); - await Task.Delay(1000, cancellationToken); + await Task.Delay(1000, cancellationToken); - Assert.AreEqual(0U, testObserver.ErrorCount); - Assert.AreEqual(1U, testObserver.ResultCount); - Assert.IsTrue(testObserver.LastValue.IsAuthenticationError()); - Assert.IsTrue(testObserver.Completed); - } + Assert.AreEqual(0U, testObserver.ErrorCount); + Assert.AreEqual(1U, testObserver.ResultCount); + Assert.IsTrue(testObserver.LastValue.IsAuthenticationError()); + Assert.IsTrue(testObserver.Completed); + } async ValueTask CreateUserWithNoInstancePerms() { @@ -1477,9 +1479,10 @@ async Task FailFast(Task task) if (!openDreamOnly) { // force a session refresh if necessary - await firstAdminMultiClient.GraphQLClient.RunQueryEnsureNoErrors( - gql => gql.ReadCurrentUser.ExecuteAsync(cancellationToken), - cancellationToken); + if (MultiServerClient.UseGraphQL) + await firstAdminMultiClient.GraphQLClient.RunQueryEnsureNoErrors( + gql => gql.ReadCurrentUser.ExecuteAsync(cancellationToken), + cancellationToken); jobsHubTestTask = FailFast(await jobsHubTest.Run(cancellationToken)); // returns Task var rootTest = FailFast(RawRequestTests.Run(restClientFactory, firstAdminRestClient, cancellationToken)); @@ -1610,14 +1613,19 @@ await FailFast( initialSessionId = dd.SessionId.Value; // force a session refresh if necessary - await firstAdminMultiClient.GraphQLClient.RunQueryEnsureNoErrors( - gql => gql.ReadCurrentUser.ExecuteAsync(cancellationToken), - cancellationToken); + if (MultiServerClient.UseGraphQL) + { + await firstAdminMultiClient.GraphQLClient.RunQueryEnsureNoErrors( + gql => gql.ReadCurrentUser.ExecuteAsync(cancellationToken), + cancellationToken); - restartSubscription = await firstAdminMultiClient.GraphQLClient.Subscribe( - gql => gql.SessionInvalidation.Watch(), - restartObserver, - cancellationToken); + restartSubscription = await firstAdminMultiClient.GraphQLClient.Subscribe( + gql => gql.SessionInvalidation.Watch(), + restartObserver, + cancellationToken); + } + else + restartSubscription = null; try { @@ -1628,7 +1636,7 @@ await firstAdminMultiClient.GraphQLClient.RunQueryEnsureNoErrors( } catch { - restartSubscription.Dispose(); + restartSubscription?.Dispose(); throw; } } @@ -1638,15 +1646,18 @@ await firstAdminMultiClient.GraphQLClient.RunQueryEnsureNoErrors( await Task.WhenAny(serverTask, Task.Delay(TimeSpan.FromMinutes(1), cancellationToken)); Assert.IsTrue(serverTask.IsCompleted); - Assert.AreEqual(0U, restartObserver.ErrorCount); - Assert.AreEqual(1U, restartObserver.ResultCount); - restartObserver.LastValue.EnsureNoErrors(); - Assert.IsTrue(restartObserver.Completed); - Assert.AreEqual(SessionInvalidationReason.ServerShutdown, restartObserver.LastValue.Data.SessionInvalidated); + if (MultiServerClient.UseGraphQL) + { + Assert.AreEqual(0U, restartObserver.ErrorCount); + Assert.AreEqual(1U, restartObserver.ResultCount); + restartObserver.LastValue.EnsureNoErrors(); + Assert.IsTrue(restartObserver.Completed); + Assert.AreEqual(SessionInvalidationReason.ServerShutdown, restartObserver.LastValue.Data.SessionInvalidated); + } } finally { - restartSubscription.Dispose(); + restartSubscription?.Dispose(); } // test the reattach message queueing From a1bf9310c8dd0670e9808e9107b9379814cf3974 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Fri, 11 Oct 2024 11:26:43 -0400 Subject: [PATCH 6/6] Fix another graphQL test --- tests/Tgstation.Server.Tests/Live/RawRequestTests.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/Tgstation.Server.Tests/Live/RawRequestTests.cs b/tests/Tgstation.Server.Tests/Live/RawRequestTests.cs index 447ee2f621..f04ce92007 100644 --- a/tests/Tgstation.Server.Tests/Live/RawRequestTests.cs +++ b/tests/Tgstation.Server.Tests/Live/RawRequestTests.cs @@ -457,6 +457,9 @@ await serverClient.Users.Update(new UserUpdateRequest static async Task TestGraphQLLogin(IRestServerClientFactory clientFactory, IRestServerClient restClient, CancellationToken cancellationToken) { + if (!MultiServerClient.UseGraphQL) + return; + await using var gqlClient = new GraphQLServerClientFactory(clientFactory).CreateUnauthenticated(restClient.Url); var result = await gqlClient.RunOperation(client => client.Login.ExecuteAsync(cancellationToken), cancellationToken);