From eca463d6d0c6ef7f0f88e15c78e2ee437f581e3e Mon Sep 17 00:00:00 2001 From: Djuradj Kurepa <91743470+dkurepa@users.noreply.github.com> Date: Tue, 29 Oct 2024 18:37:34 +0100 Subject: [PATCH 1/5] Rewrite GitHub webooks using Octokit.Webhooks (#4111) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Přemek Vysoký --- Directory.Packages.props | 1 + NuGet.config | 2 +- .../GitHubWebhookEventProcessor.cs | 151 ++++++++++++++ .../Controllers/WebHooksController.cs | 189 ------------------ .../PcsStartup.cs | 11 + .../ProductConstructionService.Api.csproj | 1 + 6 files changed, 165 insertions(+), 190 deletions(-) create mode 100644 src/ProductConstructionService/ProductConstructionService.Api/Controllers/GitHubWebhookEventProcessor.cs delete mode 100644 src/ProductConstructionService/ProductConstructionService.Api/Controllers/WebHooksController.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index 92440fc2b8..4e2d87c600 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -133,6 +133,7 @@ + diff --git a/NuGet.config b/NuGet.config index b23b52a674..4a6b4a6b4f 100644 --- a/NuGet.config +++ b/NuGet.config @@ -147,7 +147,7 @@ - + diff --git a/src/ProductConstructionService/ProductConstructionService.Api/Controllers/GitHubWebhookEventProcessor.cs b/src/ProductConstructionService/ProductConstructionService.Api/Controllers/GitHubWebhookEventProcessor.cs new file mode 100644 index 0000000000..553c09a520 --- /dev/null +++ b/src/ProductConstructionService/ProductConstructionService.Api/Controllers/GitHubWebhookEventProcessor.cs @@ -0,0 +1,151 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Reflection; +using Maestro.Data; +using Microsoft.DotNet.GitHub.Authentication; +using Octokit.Internal; +using Octokit; +using Octokit.Webhooks; +using Octokit.Webhooks.Events; +using Octokit.Webhooks.Events.Installation; +using Octokit.Webhooks.Events.InstallationRepositories; +using Microsoft.EntityFrameworkCore; +using System.Diagnostics.CodeAnalysis; + +namespace ProductConstructionService.Api.Controllers; + +public class GitHubWebhookEventProcessor : WebhookEventProcessor +{ + private readonly ILogger _logger; + private readonly IGitHubTokenProvider _tokenProvider; + private readonly BuildAssetRegistryContext _context; + + public GitHubWebhookEventProcessor( + ILogger logger, + IGitHubTokenProvider tokenProvider, + BuildAssetRegistryContext context) + { + _logger = logger; + _tokenProvider = tokenProvider; + _context = context; + } + + protected override async Task ProcessInstallationWebhookAsync( + WebhookHeaders headers, + InstallationEvent installationEvent, + InstallationAction action) + { + switch(installationEvent.Action) + { + case InstallationActionValue.Deleted: + await RemoveInstallationRepositoriesAsync(installationEvent.Installation.Id); + break; + case InstallationActionValue.Created: + await SynchronizeInstallationRepositoriesAsync(installationEvent.Installation.Id); + break; + default: + _logger.LogError("Received unknown installation action `{action}`", installationEvent.Action); + break; + } + } + + protected override async Task ProcessInstallationRepositoriesWebhookAsync(WebhookHeaders headers, InstallationRepositoriesEvent installationRepositoriesEvent, InstallationRepositoriesAction action) + { + await SynchronizeInstallationRepositoriesAsync(installationRepositoriesEvent.Installation.Id); + } + + private async Task RemoveInstallationRepositoriesAsync(long installationId) + { + foreach (Maestro.Data.Models.Repository repo in await _context.Repositories.Where(r => r.InstallationId == installationId).ToListAsync()) + { + repo.InstallationId = 0; + _context.Update(repo); + } + + await _context.SaveChangesAsync(); + } + + private async Task SynchronizeInstallationRepositoriesAsync(long installationId) + { + var token = await _tokenProvider.GetTokenForInstallationAsync(installationId); + IReadOnlyList gitHubRepos = await GetAllInstallationRepositories(token); + + List barRepos = + await _context.Repositories.Where(r => r.InstallationId == installationId).ToListAsync(); + + List updatedRepos = []; + // Go through all Repos that currently have a given installation id. If the repo is already present in BAR, make sure it has the correct installationId, + // and add it to the list of update repos. + // If not, add it to BAR + foreach (Repository repo in gitHubRepos) + { + Maestro.Data.Models.Repository? existing = await _context.Repositories.FindAsync(repo.HtmlUrl); + + if (existing == null) + { + _context.Repositories.Add( + new Maestro.Data.Models.Repository + { + RepositoryName = repo.HtmlUrl, + InstallationId = installationId, + }); + } + else + { + existing.InstallationId = installationId; + _context.Update(existing); + updatedRepos.Add(existing); + } + } + + // If a repo is present in BAR, but not in the updateRepos list, that means the GH app was uninstalled from it. Set its installationID to 0 + foreach (Maestro.Data.Models.Repository repo in barRepos.Except(updatedRepos, new RepositoryNameComparer())) + { + repo.InstallationId = 0; + _context.Update(repo); + } + + await _context.SaveChangesAsync(); + } + + private static Task> GetAllInstallationRepositories(string token) + { + var product = new ProductHeaderValue( + "Maestro", + Assembly.GetEntryAssembly()? + .GetCustomAttribute()? + .InformationalVersion); + var client = new GitHubClient(product) { Credentials = new Credentials(token, AuthenticationType.Bearer) }; + var pagination = new ApiPagination(); + var uri = new Uri("installation/repositories", UriKind.Relative); + + async Task>> GetInstallationRepositories(Uri requestUri) + { + IApiResponse response = + await client.Connection.Get(requestUri, parameters: null); + return new ApiResponse>(response.HttpResponse, response.Body.Repositories); + } + + return pagination.GetAllPages( + async () => new ReadOnlyPagedCollection( + await GetInstallationRepositories(uri), + GetInstallationRepositories), + uri); + } + + public class InstallationRepositoriesResponse + { + public int TotalCount { get; set; } + public StringEnum RepositorySelection { get; set; } + public required List Repositories { get; set; } + } + + private class RepositoryNameComparer : IEqualityComparer + { + public bool Equals(Maestro.Data.Models.Repository? x, Maestro.Data.Models.Repository? y) => + string.Equals(x?.RepositoryName, y?.RepositoryName); + + public int GetHashCode([DisallowNull] Maestro.Data.Models.Repository obj) => throw new NotImplementedException(); + } +} diff --git a/src/ProductConstructionService/ProductConstructionService.Api/Controllers/WebHooksController.cs b/src/ProductConstructionService/ProductConstructionService.Api/Controllers/WebHooksController.cs deleted file mode 100644 index a5b8d8caa9..0000000000 --- a/src/ProductConstructionService/ProductConstructionService.Api/Controllers/WebHooksController.cs +++ /dev/null @@ -1,189 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Reflection; -using Maestro.Data; -using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.WebHooks; -using Microsoft.DotNet.GitHub.Authentication; -using Microsoft.EntityFrameworkCore; -using Newtonsoft.Json.Linq; -using Octokit; -using Octokit.Internal; -using ProductConstructionService.Api.Api; - -namespace ProductConstructionService.Api.Controllers; - -public class WebHooksController : Controller -{ - private readonly Lazy _context; - - public WebHooksController( - ILogger logger, - IGitHubTokenProvider gitHubTokenProvider, - Lazy context) - { - _context = context; - Logger = logger; - GitHubTokenProvider = gitHubTokenProvider; - } - - public ILogger Logger { get; } - public IGitHubTokenProvider GitHubTokenProvider { get; } - public BuildAssetRegistryContext Context => _context.Value; - - [GitHubWebHook(EventName = "installation")] - [ValidateModelState] - public async Task InstallationHandler(JObject data) - { - var ser = new SimpleJsonSerializer(); - var payload = ser.Deserialize(data.ToString()); - switch (payload.Action) - { - case "deleted": - await RemoveInstallationRepositoriesAsync(payload.Installation.Id); - break; - case "created": // installation was created - case "removed": // repository removed from installation - case "added": // repository added to installation - await SynchronizeInstallationRepositoriesAsync(payload.Installation.Id); - break; - default: - Logger.LogError( - "Received Unknown action '{action}' for installation event. Payload: {payload}", - payload.Action, - data.ToString()); - break; - } - - return Ok(); - } - - private async Task RemoveInstallationRepositoriesAsync(long installationId) - { - foreach (Maestro.Data.Models.Repository repo in await Context.Repositories.Where(r => r.InstallationId == installationId).ToListAsync()) - { - Context.Update(repo); - repo.InstallationId = 0; - } - - await Context.SaveChangesAsync(); - } - - [GitHubWebHook(EventName = "installation_repositories")] - [ValidateModelState] - public async Task InstallationRepositoriesHandler(JObject data) - { - var ser = new SimpleJsonSerializer(); - var payload = ser.Deserialize(data.ToString()); - await SynchronizeInstallationRepositoriesAsync(payload.Installation.Id); - return Ok(); - } - - private async Task SynchronizeInstallationRepositoriesAsync(long installationId) - { - var token = await GitHubTokenProvider.GetTokenForInstallationAsync(installationId); - IReadOnlyList gitHubRepos = await GetAllInstallationRepositories(token); - - List toRemove = - await Context.Repositories.Where(r => r.InstallationId == installationId).ToListAsync(); - - foreach (Repository repo in gitHubRepos) - { - Maestro.Data.Models.Repository? existing = await Context.Repositories.FindAsync(repo.HtmlUrl); - if (existing == null) - { - Context.Repositories.Add( - new Maestro.Data.Models.Repository - { - RepositoryName = repo.HtmlUrl, - InstallationId = installationId - }); - } - else - { - toRemove.Remove(existing); - existing.InstallationId = installationId; - Context.Update(existing); - } - } - - foreach (Maestro.Data.Models.Repository repository in toRemove) - { - repository.InstallationId = 0; - Context.Update(repository); - } - - await Context.SaveChangesAsync(); - } - - private static Task> GetAllInstallationRepositories(string token) - { - var product = new ProductHeaderValue( - "Maestro", - Assembly.GetEntryAssembly()? - .GetCustomAttribute()? - .InformationalVersion); - var client = new GitHubClient(product) { Credentials = new Credentials(token, AuthenticationType.Bearer) }; - var pagination = new ApiPagination(); - var uri = new Uri("installation/repositories", UriKind.Relative); - - async Task>> GetInstallationRepositories(Uri requestUri) - { - IApiResponse response = - await client.Connection.Get(requestUri, parameters: null); - return new ApiResponse>(response.HttpResponse, response.Body.Repositories); - } - - return pagination.GetAllPages( - async () => new ReadOnlyPagedCollection( - await GetInstallationRepositories(uri), - GetInstallationRepositories), - uri); - } - - [GitHubWebHook] - [ValidateModelState] - public IActionResult GitHubHandler(string id, string @event, JObject data) - { - Logger.LogWarning("Received unhandled event {eventName}", @event); - return NoContent(); - } - -#nullable disable - public class InstallationEvent - { - public string Action { get; set; } - public Installation Installation { get; set; } - public List Repositories { get; set; } - public User Sender { get; set; } - } - - #nullable disable - public class InstallationRepository - { - public int Id { get; set; } - public string Name { get; set; } - public string FullName { get; set; } - public bool Private { get; set; } - } - - #nullable disable - public class InstallationRepositoriesEvent - { - public string Action { get; set; } - public Installation Installation { get; set; } - public StringEnum RepositorySelection { get; set; } - public List RepositoriesAdded { get; set; } - public List RepositoriesRemoved { get; set; } - public User Sender { get; set; } - } - - #nullable disable - public class InstallationRepositoriesResponse - { - public int TotalCount { get; set; } - public StringEnum RepositorySelection { get; set; } - public List Repositories { get; set; } - } -} diff --git a/src/ProductConstructionService/ProductConstructionService.Api/PcsStartup.cs b/src/ProductConstructionService/ProductConstructionService.Api/PcsStartup.cs index 59bd42ce31..c26bbfc16b 100644 --- a/src/ProductConstructionService/ProductConstructionService.Api/PcsStartup.cs +++ b/src/ProductConstructionService/ProductConstructionService.Api/PcsStartup.cs @@ -39,12 +39,16 @@ using ProductConstructionService.DependencyFlow; using ProductConstructionService.ServiceDefaults; using Microsoft.Extensions.DependencyInjection.Extensions; +using Octokit.Webhooks.AspNetCore; +using Octokit.Webhooks; +using ProductConstructionService.Api.Controllers; namespace ProductConstructionService.Api; internal static class PcsStartup { private const string SqlConnectionStringUserIdPlaceholder = "USER_ID_PLACEHOLDER"; + private const string GitHubWebHooksPath = "/api/webhooks/incoming/github"; private static class ConfigurationKeys { @@ -54,6 +58,7 @@ private static class ConfigurationKeys // Secrets coming from the KeyVault public const string GitHubClientId = $"{KeyVaultSecretPrefix}github-app-id"; public const string GitHubClientSecret = $"{KeyVaultSecretPrefix}github-app-private-key"; + public const string GitHubAppWebhook = $"{KeyVaultSecretPrefix}github-app-webhook-secret"; // Configuration from appsettings.json public const string AzureDevOpsConfiguration = "AzureDevOps"; @@ -274,6 +279,8 @@ internal static async Task ConfigurePcs( options.Cookie.IsEssential = true; }); + builder.Services.AddTransient(); + if (addSwagger) { builder.ConfigureSwagger(); @@ -312,6 +319,10 @@ public static void ConfigureApi(this IApplicationBuilder app, bool isDevelopment { controllers.AllowAnonymous(); } + + e.MapGitHubWebhooks( + path: GitHubWebHooksPath, + secret: app.ApplicationServices.GetRequiredService()[ConfigurationKeys.GitHubAppWebhook]); }); } diff --git a/src/ProductConstructionService/ProductConstructionService.Api/ProductConstructionService.Api.csproj b/src/ProductConstructionService/ProductConstructionService.Api/ProductConstructionService.Api.csproj index 1e44e60298..49e833ca1e 100644 --- a/src/ProductConstructionService/ProductConstructionService.Api/ProductConstructionService.Api.csproj +++ b/src/ProductConstructionService/ProductConstructionService.Api/ProductConstructionService.Api.csproj @@ -33,6 +33,7 @@ + From f6a3e9463dcff8f8c27250d15ac0efba9a7cd89c Mon Sep 17 00:00:00 2001 From: Oleksandr Didyk <106967057+oleksandr-didyk@users.noreply.github.com> Date: Wed, 30 Oct 2024 12:29:07 +0100 Subject: [PATCH 2/5] Fixup GitHub token name (#4118) --- eng/templates/stages/deploy.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/templates/stages/deploy.yaml b/eng/templates/stages/deploy.yaml index 6840c2bdd6..1319daed7d 100644 --- a/eng/templates/stages/deploy.yaml +++ b/eng/templates/stages/deploy.yaml @@ -129,7 +129,7 @@ stages: --repo dotnet/arcade-services displayName: Create GitHub release env: - GH_TOKEN: $(dotnet-bot-arcade-services-content-write) + GH_TOKEN: $(dotnet-bot-arcade-services-content-rw) continueOnError: true - stage: validateDeployment From 110f1900f60242746cb6af9e80dd898854048dff Mon Sep 17 00:00:00 2001 From: Oleksandr Didyk <106967057+oleksandr-didyk@users.noreply.github.com> Date: Mon, 4 Nov 2024 10:12:57 +0100 Subject: [PATCH 3/5] Remove obsolete var group (#4122) --- eng/templates/jobs/e2e-pcs-tests.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/eng/templates/jobs/e2e-pcs-tests.yml b/eng/templates/jobs/e2e-pcs-tests.yml index 87d3d625e4..a88d5fbd4d 100644 --- a/eng/templates/jobs/e2e-pcs-tests.yml +++ b/eng/templates/jobs/e2e-pcs-tests.yml @@ -21,7 +21,6 @@ jobs: # Required for dotnet-bot-maestro-auth-test-content-rw - group: Arcade-Services-Scenario-Tests - ${{ if not(or(startsWith(variables['Build.SourceBranch'], 'refs/heads/production'), startsWith(variables['Build.SourceBranch'], 'refs/heads/production-'), eq(variables['Build.SourceBranch'], 'refs/heads/production'))) }}: - - group: MaestroInt KeyVault - name: PcsTestEndpoint value: https://product-construction-int.delightfuldune-c0f01ab0.westus2.azurecontainerapps.io - name: ScenarioTestSubscription @@ -29,7 +28,6 @@ jobs: - name: MaestroAppId value: $(MaestroStagingAppClientId) - ${{ else }}: - - group: MaestroProd KeyVault - name: PcsTestEndpoint value: https://product-construction-prod.wittysky-0c79e3cc.westus2.azurecontainerapps.io - name: ScenarioTestSubscription From 78cf8ade63c3092f83c90f56bf944096cd0ea9b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Mon, 4 Nov 2024 14:36:07 +0100 Subject: [PATCH 4/5] Fix DI for the `darc vmr get-version` command (#4120) --- .../GetRepoVersionCommandLineOptions.cs | 7 ++ .../VirtualMonoRepo/VmrCommandLineOptions.cs | 34 +------- .../VmrCommandLineOptionsBase.cs | 37 +++++++++ test/Maestro.Web.Tests/TestDatabase.cs | 77 ++++++++---------- .../DependencyRegistrationTests.cs | 22 +++--- .../TestDatabase.cs | 79 ++++++++----------- 6 files changed, 124 insertions(+), 132 deletions(-) diff --git a/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/GetRepoVersionCommandLineOptions.cs b/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/GetRepoVersionCommandLineOptions.cs index b78203418d..5b500dd8c1 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/GetRepoVersionCommandLineOptions.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/GetRepoVersionCommandLineOptions.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using CommandLine; using Microsoft.DotNet.Darc.Operations.VirtualMonoRepo; +using Microsoft.Extensions.DependencyInjection; #nullable enable namespace Microsoft.DotNet.Darc.Options.VirtualMonoRepo; @@ -14,4 +15,10 @@ internal class GetRepoVersionCommandLineOptions : VmrCommandLineOptionsBase Repositories { get; set; } = Array.Empty(); + + public override IServiceCollection RegisterServices(IServiceCollection services) + { + RegisterVmrServices(services, tmpPath: null); + return base.RegisterServices(services); + } } diff --git a/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/VmrCommandLineOptions.cs b/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/VmrCommandLineOptions.cs index 5777e64d79..325bc387b2 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/VmrCommandLineOptions.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/VmrCommandLineOptions.cs @@ -1,15 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.IO; using CommandLine; -using Microsoft.DotNet.Darc.Helpers; -using Microsoft.DotNet.DarcLib.VirtualMonoRepo; -using Microsoft.DotNet.DarcLib; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging.Abstractions; -using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.DotNet.Darc.Operations; +using Microsoft.Extensions.DependencyInjection; namespace Microsoft.DotNet.Darc.Options.VirtualMonoRepo; @@ -20,31 +14,7 @@ internal abstract class VmrCommandLineOptions : VmrCommandLineOptionsBase public override IServiceCollection RegisterServices(IServiceCollection services) { - string tmpPath = Path.GetFullPath(TmpPath ?? Path.GetTempPath()); - LocalSettings localDarcSettings = null; - - var gitHubToken = GitHubPat; - var azureDevOpsToken = AzureDevOpsPat; - - // Read tokens from local settings if not provided - // We silence errors because the VMR synchronization often works with public repositories where tokens are not required - if (gitHubToken == null || azureDevOpsToken == null) - { - try - { - localDarcSettings = LocalSettings.GetSettings(this, NullLogger.Instance); - } - catch (DarcException) - { - // The VMR synchronization often works with public repositories where tokens are not required - } - - gitHubToken ??= localDarcSettings?.GitHubToken; - azureDevOpsToken ??= localDarcSettings?.AzureDevOpsToken; - } - - services.AddVmrManagers(GitLocation, VmrPath, tmpPath, gitHubToken, azureDevOpsToken); - services.TryAddTransient(); + RegisterVmrServices(services, TmpPath); return base.RegisterServices(services); } } diff --git a/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/VmrCommandLineOptionsBase.cs b/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/VmrCommandLineOptionsBase.cs index 6005938b0a..cf31484c2b 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/VmrCommandLineOptionsBase.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/VmrCommandLineOptionsBase.cs @@ -3,7 +3,14 @@ using System; using CommandLine; +using Microsoft.DotNet.Darc.Helpers; using Microsoft.DotNet.Darc.Operations; +using Microsoft.DotNet.DarcLib.VirtualMonoRepo; +using Microsoft.DotNet.DarcLib; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Logging.Abstractions; +using System.IO; #nullable enable namespace Microsoft.DotNet.Darc.Options.VirtualMonoRepo; @@ -12,4 +19,34 @@ internal abstract class VmrCommandLineOptionsBase : CommandLineOptions whe { [Option("vmr", HelpText = "Path to the VMR; defaults to nearest git root above the current working directory.")] public string VmrPath { get; set; } = Environment.CurrentDirectory; + + protected void RegisterVmrServices(IServiceCollection services, string? tmpPath) + { + LocalSettings? localDarcSettings = null; + + var gitHubToken = GitHubPat; + var azureDevOpsToken = AzureDevOpsPat; + + // Read tokens from local settings if not provided + // We silence errors because the VMR synchronization often works with public repositories where tokens are not required + if (gitHubToken == null || azureDevOpsToken == null) + { + try + { + localDarcSettings = LocalSettings.GetSettings(this, NullLogger.Instance); + } + catch (DarcException) + { + // The VMR synchronization often works with public repositories where tokens are not required + } + + gitHubToken ??= localDarcSettings?.GitHubToken; + azureDevOpsToken ??= localDarcSettings?.AzureDevOpsToken; + } + + tmpPath = Path.GetFullPath(tmpPath ?? Path.GetTempPath()); + + services.AddVmrManagers(GitLocation, VmrPath, tmpPath, gitHubToken, azureDevOpsToken); + services.TryAddTransient(); + } } diff --git a/test/Maestro.Web.Tests/TestDatabase.cs b/test/Maestro.Web.Tests/TestDatabase.cs index 3a71ea2369..fe1849afd3 100644 --- a/test/Maestro.Web.Tests/TestDatabase.cs +++ b/test/Maestro.Web.Tests/TestDatabase.cs @@ -42,64 +42,55 @@ private class SharedTestDatabase : TestDatabase public class TestDatabase : IDisposable { private const string TestDatabasePrefix = "TFD_"; - private string _databaseName; - private readonly SemaphoreSlim _createLock = new(1); - - protected TestDatabase() - { - } + private readonly Lazy> _databaseName = new( + InitializeDatabaseAsync, + LazyThreadSafetyMode.ExecutionAndPublication); public void Dispose() { using var connection = new SqlConnection(BuildAssetRegistryContextFactory.GetConnectionString("master")); connection.Open(); DropAllTestDatabases(connection).GetAwaiter().GetResult(); + GC.SuppressFinalize(this); } - public async Task GetConnectionString() + public async Task GetConnectionString() => BuildAssetRegistryContextFactory.GetConnectionString(await _databaseName.Value); + + private static async Task InitializeDatabaseAsync() { - if (_databaseName != null) - { - return ConnectionString; - } + string databaseName = TestDatabasePrefix + + $"_{TestContext.CurrentContext.Test.ClassName!.Split('.').Last()}" + + $"_{TestContext.CurrentContext.Test.MethodName}" + + $"_{DateTime.Now:yyyyMMddHHmmss}"; + + TestContext.WriteLine($"Creating database '{databaseName}'"); - await _createLock.WaitAsync(); - try + await using (var connection = new SqlConnection(BuildAssetRegistryContextFactory.GetConnectionString("master"))) { - string databaseName = $"{TestDatabasePrefix}_{TestContext.CurrentContext.Test.ClassName.Split('.').Last()}_{TestContext.CurrentContext.Test.MethodName}_{DateTime.Now:yyyyMMddHHmmss}"; - TestContext.WriteLine($"Creating database '{databaseName}'"); - await using (var connection = new SqlConnection(BuildAssetRegistryContextFactory.GetConnectionString("master"))) + await connection.OpenAsync(); + await DropAllTestDatabases(connection); + await using (SqlCommand createCommand = connection.CreateCommand()) { - await connection.OpenAsync(); - - await DropAllTestDatabases(connection); - - await using (SqlCommand createCommand = connection.CreateCommand()) - { - createCommand.CommandText = $"CREATE DATABASE {databaseName}"; - await createCommand.ExecuteNonQueryAsync(); - } + createCommand.CommandText = $"CREATE DATABASE {databaseName}"; + await createCommand.ExecuteNonQueryAsync(); } + } - var collection = new ServiceCollection(); - collection.AddSingleton(new HostingEnvironment - {EnvironmentName = Environments.Development}); - collection.AddBuildAssetRegistry(o => - { - o.UseSqlServer(BuildAssetRegistryContextFactory.GetConnectionString(databaseName)); - o.EnableServiceProviderCaching(false); - }); + var collection = new ServiceCollection(); + collection.AddSingleton(new HostingEnvironment + { + EnvironmentName = Environments.Development + }); + collection.AddBuildAssetRegistry(o => + { + o.UseSqlServer(BuildAssetRegistryContextFactory.GetConnectionString(databaseName)); + o.EnableServiceProviderCaching(false); + }); - await using ServiceProvider provider = collection.BuildServiceProvider(); - await provider.GetRequiredService().Database.MigrateAsync(); + await using ServiceProvider provider = collection.BuildServiceProvider(); + await provider.GetRequiredService().Database.MigrateAsync(); - _databaseName = databaseName; - return ConnectionString; - } - finally - { - _createLock.Dispose(); - } + return databaseName; } private static async Task DropAllTestDatabases(SqlConnection connection) @@ -123,6 +114,4 @@ private static async Task DropAllTestDatabases(SqlConnection connection) await command.ExecuteNonQueryAsync(); } } - - private string ConnectionString => BuildAssetRegistryContextFactory.GetConnectionString(_databaseName); } diff --git a/test/Microsoft.DotNet.Darc.Tests/DependencyRegistrationTests.cs b/test/Microsoft.DotNet.Darc.Tests/DependencyRegistrationTests.cs index 7a0c790893..809938ca26 100644 --- a/test/Microsoft.DotNet.Darc.Tests/DependencyRegistrationTests.cs +++ b/test/Microsoft.DotNet.Darc.Tests/DependencyRegistrationTests.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Collections.Generic; using System.Linq; using FluentAssertions; using Microsoft.DotNet.Darc.Options; @@ -15,19 +14,20 @@ namespace Microsoft.DotNet.Darc.Tests; [TestFixture] public class DependencyRegistrationTests { + /// + /// Tests instantiating the operations + /// [Test] - public void AreDependenciesRegistered() + public void AreDarcOperationsRegistered() { - DependencyInjectionValidation.IsDependencyResolutionCoherent(services => + foreach (Type optionType in Program.GetOptions().Concat(Program.GetVmrOptions())) { - // Tests instantiating the operations - IEnumerable optionTypes = Program.GetOptions().Concat(Program.GetVmrOptions()); - foreach (Type optionType in optionTypes) + DependencyInjectionValidation.IsDependencyResolutionCoherent(services => { // Register the option type services.AddTransient(optionType); - var operationOption = (CommandLineOptions) Activator.CreateInstance(optionType); + var operationOption = (CommandLineOptions)Activator.CreateInstance(optionType); // Set IsCi to true to avoid login pop up operationOption.IsCi = true; operationOption.RegisterServices(services); @@ -35,10 +35,10 @@ public void AreDependenciesRegistered() // Verify we can create the operation var operation = operationOption.GetOperation(provider); - operation.Should().NotBeNull($"Operation of {optionType.Name} could not be created"); + operation.Should().NotBeNull($"Operation for {optionType.Name} could not be created"); services.AddTransient(operation.GetType()); - } - }, - out string message).Should().BeTrue(message); + }, + out string message).Should().BeTrue(message); + } } } diff --git a/test/ProductConstructionService.Api.Tests/TestDatabase.cs b/test/ProductConstructionService.Api.Tests/TestDatabase.cs index db10dad529..a8decba67b 100644 --- a/test/ProductConstructionService.Api.Tests/TestDatabase.cs +++ b/test/ProductConstructionService.Api.Tests/TestDatabase.cs @@ -36,64 +36,55 @@ private class SharedTestDatabase : TestDatabase public class TestDatabase : IDisposable { private const string TestDatabasePrefix = "TFD_"; - private string _databaseName = null!; - private readonly SemaphoreSlim _createLock = new(1); - - protected TestDatabase() - { - } + private readonly Lazy> _databaseName = new( + InitializeDatabaseAsync, + LazyThreadSafetyMode.ExecutionAndPublication); public void Dispose() { using var connection = new SqlConnection(BuildAssetRegistryContextFactory.GetConnectionString("master")); connection.Open(); DropAllTestDatabases(connection).GetAwaiter().GetResult(); + GC.SuppressFinalize(this); } - public async Task GetConnectionString() + public async Task GetConnectionString() => BuildAssetRegistryContextFactory.GetConnectionString(await _databaseName.Value); + + private static async Task InitializeDatabaseAsync() { - if (_databaseName != null) - { - return ConnectionString; - } + string databaseName = TestDatabasePrefix + + $"_{TestContext.CurrentContext.Test.ClassName!.Split('.').Last()}" + + $"_{TestContext.CurrentContext.Test.MethodName}" + + $"_{DateTime.Now:yyyyMMddHHmmss}"; + + TestContext.WriteLine($"Creating database '{databaseName}'"); - await _createLock.WaitAsync(); - try + await using (var connection = new SqlConnection(BuildAssetRegistryContextFactory.GetConnectionString("master"))) { - var databaseName = $"{TestDatabasePrefix}_{TestContext.CurrentContext.Test.ClassName!.Split('.').Last()}_{TestContext.CurrentContext.Test.MethodName}_{DateTime.Now:yyyyMMddHHmmss}"; - TestContext.WriteLine($"Creating database '{databaseName}'"); - await using (var connection = new SqlConnection(BuildAssetRegistryContextFactory.GetConnectionString("master"))) + await connection.OpenAsync(); + await DropAllTestDatabases(connection); + await using (SqlCommand createCommand = connection.CreateCommand()) { - await connection.OpenAsync(); - - await DropAllTestDatabases(connection); - - await using (SqlCommand createCommand = connection.CreateCommand()) - { - createCommand.CommandText = $"CREATE DATABASE {databaseName}"; - await createCommand.ExecuteNonQueryAsync(); - } + createCommand.CommandText = $"CREATE DATABASE {databaseName}"; + await createCommand.ExecuteNonQueryAsync(); } + } - var collection = new ServiceCollection(); - collection.AddSingleton(new HostingEnvironment - { EnvironmentName = Environments.Development }); - collection.AddBuildAssetRegistry(o => - { - o.UseSqlServer(BuildAssetRegistryContextFactory.GetConnectionString(databaseName)); - o.EnableServiceProviderCaching(false); - }); + var collection = new ServiceCollection(); + collection.AddSingleton(new HostingEnvironment + { + EnvironmentName = Environments.Development + }); + collection.AddBuildAssetRegistry(o => + { + o.UseSqlServer(BuildAssetRegistryContextFactory.GetConnectionString(databaseName)); + o.EnableServiceProviderCaching(false); + }); - await using ServiceProvider provider = collection.BuildServiceProvider(); - await provider.GetRequiredService().Database.MigrateAsync(); + await using ServiceProvider provider = collection.BuildServiceProvider(); + await provider.GetRequiredService().Database.MigrateAsync(); - _databaseName = databaseName; - return ConnectionString; - } - finally - { - _createLock.Dispose(); - } + return databaseName; } private static async Task DropAllTestDatabases(SqlConnection connection) @@ -109,7 +100,7 @@ private static async Task DropAllTestDatabases(SqlConnection connection) } } - foreach (var db in previousTestDbs) + foreach (string db in previousTestDbs) { TestContext.WriteLine($"Dropping test database '{db}'"); await using SqlCommand command = connection.CreateCommand(); @@ -117,6 +108,4 @@ private static async Task DropAllTestDatabases(SqlConnection connection) await command.ExecuteNonQueryAsync(); } } - - private string ConnectionString => BuildAssetRegistryContextFactory.GetConnectionString(_databaseName); } From fc65aa74ba3967334f94c538037bf67b2b34cb92 Mon Sep 17 00:00:00 2001 From: Oleksandr Didyk <106967057+oleksandr-didyk@users.noreply.github.com> Date: Mon, 4 Nov 2024 15:03:47 +0100 Subject: [PATCH 5/5] Add policy for admin role (#4124) --- .../Maestro.Authentication/AuthenticationConfiguration.cs | 7 +++++++ .../Controllers/StatusController.cs | 2 ++ 2 files changed, 9 insertions(+) diff --git a/src/Maestro/Maestro.Authentication/AuthenticationConfiguration.cs b/src/Maestro/Maestro.Authentication/AuthenticationConfiguration.cs index 5e80786dca..83789763ec 100644 --- a/src/Maestro/Maestro.Authentication/AuthenticationConfiguration.cs +++ b/src/Maestro/Maestro.Authentication/AuthenticationConfiguration.cs @@ -23,6 +23,7 @@ public static class AuthenticationConfiguration { public const string EntraAuthorizationPolicyName = "Entra"; public const string MsftAuthorizationPolicyName = "msft"; + public const string AdminAuthorizationPolicyName = "RequireAdminAccess"; public const string AccountSignInRoute = "/Account/SignIn"; @@ -111,6 +112,12 @@ public static void ConfigureAuthServices(this IServiceCollection services, IConf || context.User.IsInRole(prodconSvcsRole); }); }); + options.AddPolicy(AdminAuthorizationPolicyName, policy => + { + policy.AddAuthenticationSchemes(AuthenticationSchemes); + policy.RequireAuthenticatedUser(); + policy.RequireRole("Admin"); + }); }); services.Configure( diff --git a/src/ProductConstructionService/ProductConstructionService.Api/Controllers/StatusController.cs b/src/ProductConstructionService/ProductConstructionService.Api/Controllers/StatusController.cs index 829fc0dbec..8b287a41dc 100644 --- a/src/ProductConstructionService/ProductConstructionService.Api/Controllers/StatusController.cs +++ b/src/ProductConstructionService/ProductConstructionService.Api/Controllers/StatusController.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Net; +using Maestro.Authentication; using Microsoft.AspNetCore.ApiVersioning; using Microsoft.AspNetCore.ApiVersioning.Swashbuckle; using Microsoft.AspNetCore.Authorization; @@ -12,6 +13,7 @@ namespace ProductConstructionService.Api.Controllers; [Route("status")] [ApiVersion("2020-02-20")] +[Authorize(Policy = AuthenticationConfiguration.AdminAuthorizationPolicyName)] public class StatusController(IReplicaWorkItemProcessorStateCacheFactory replicaWorkItemProcessorStateCacheFactory) : ControllerBase { private readonly IReplicaWorkItemProcessorStateCacheFactory _replicaWorkItemProcessorStateCacheFactory = replicaWorkItemProcessorStateCacheFactory;