From c78bbc6101cc401beb875f29d1180a78f6b34f24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Tue, 6 Aug 2024 11:53:34 +0200 Subject: [PATCH 1/9] Bump Swagger/Swashbuckle (#3792) --- Directory.Packages.props | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index 47f50839ad..390ad97dbb 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -127,7 +127,7 @@ - + @@ -143,4 +143,4 @@ - + \ No newline at end of file From d1631817cdbb0cd33c10ef941d771abb7c23d2f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Tue, 6 Aug 2024 13:18:54 +0200 Subject: [PATCH 2/9] Fix un-escaped redirect paths (#3793) --- src/Maestro/Maestro.Web/Pages/Account/AccountController.cs | 4 ++-- src/Maestro/Maestro.Web/Pages/_Layout.cshtml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Maestro/Maestro.Web/Pages/Account/AccountController.cs b/src/Maestro/Maestro.Web/Pages/Account/AccountController.cs index a6576e447c..8021279440 100644 --- a/src/Maestro/Maestro.Web/Pages/Account/AccountController.cs +++ b/src/Maestro/Maestro.Web/Pages/Account/AccountController.cs @@ -17,7 +17,7 @@ public class AccountController : Controller public new async Task SignOut() { await HttpContext.SignOutAsync(); - return RedirectToPage("/"); + return Redirect($"{Request.Scheme}://{Request.Host}"); } [HttpGet(AuthenticationConfiguration.AccountSignInRoute)] @@ -25,7 +25,7 @@ public class AccountController : Controller public IActionResult SignIn(string returnUrl = null) { return Challenge( - new AuthenticationProperties() { RedirectUri = "/" }, + new AuthenticationProperties() { RedirectUri = $"{Request.Scheme}://{Request.Host}" }, OpenIdConnectDefaults.AuthenticationScheme); } } diff --git a/src/Maestro/Maestro.Web/Pages/_Layout.cshtml b/src/Maestro/Maestro.Web/Pages/_Layout.cshtml index 32674d47d9..daecbff2ea 100644 --- a/src/Maestro/Maestro.Web/Pages/_Layout.cshtml +++ b/src/Maestro/Maestro.Web/Pages/_Layout.cshtml @@ -98,7 +98,7 @@ else {
  • - Sign In + Sign In
    • } From c288eb80a278c2cb87061a7400aa771c2b0d538f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Tue, 6 Aug 2024 14:51:12 +0200 Subject: [PATCH 3/9] Fix connection string CodeQL violations (#3790) --- .../Maestro.Data/BuildAssetRegistryContext.cs | 9 +++++++-- test/Maestro.Web.Tests/TestDatabase.cs | 13 ++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Maestro/Maestro.Data/BuildAssetRegistryContext.cs b/src/Maestro/Maestro.Data/BuildAssetRegistryContext.cs index 4c07240f5a..c79fb3e590 100644 --- a/src/Maestro/Maestro.Data/BuildAssetRegistryContext.cs +++ b/src/Maestro/Maestro.Data/BuildAssetRegistryContext.cs @@ -23,8 +23,7 @@ public class BuildAssetRegistryContextFactory : IDesignTimeDbContextFactory, int> diff --git a/test/Maestro.Web.Tests/TestDatabase.cs b/test/Maestro.Web.Tests/TestDatabase.cs index 580ab0a085..3a71ea2369 100644 --- a/test/Maestro.Web.Tests/TestDatabase.cs +++ b/test/Maestro.Web.Tests/TestDatabase.cs @@ -51,7 +51,7 @@ protected TestDatabase() public void Dispose() { - using var connection = new SqlConnection("Data Source=localhost\\SQLEXPRESS;Initial Catalog=master;Integrated Security=true"); // lgtm [SM03452] This 'connection string' is only for the local SQLExpress instance and has no credentials, Encrypt=true is unnecessary + using var connection = new SqlConnection(BuildAssetRegistryContextFactory.GetConnectionString("master")); connection.Open(); DropAllTestDatabases(connection).GetAwaiter().GetResult(); } @@ -68,7 +68,7 @@ public async Task GetConnectionString() { 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("Data Source=localhost\\SQLEXPRESS;Initial Catalog=master;Integrated Security=true")) // CodeQL [SM03452] This 'connection string' is only for the local SQLExpress instance and has no credentials, Encrypt=true is unnecessary + await using (var connection = new SqlConnection(BuildAssetRegistryContextFactory.GetConnectionString("master"))) { await connection.OpenAsync(); @@ -86,7 +86,7 @@ public async Task GetConnectionString() {EnvironmentName = Environments.Development}); collection.AddBuildAssetRegistry(o => { - o.UseSqlServer(GetConnectionString(databaseName)); + o.UseSqlServer(BuildAssetRegistryContextFactory.GetConnectionString(databaseName)); o.EnableServiceProviderCaching(false); }); @@ -124,10 +124,5 @@ private static async Task DropAllTestDatabases(SqlConnection connection) } } - private string ConnectionString => GetConnectionString(_databaseName); - - private static string GetConnectionString(string databaseName) - { - return $@"Data Source=localhost\SQLEXPRESS;Initial Catalog={databaseName};Integrated Security=true"; - } + private string ConnectionString => BuildAssetRegistryContextFactory.GetConnectionString(_databaseName); } From c40c2cd1c6401a7c76ea42b0fa42dd57475a5b56 Mon Sep 17 00:00:00 2001 From: Djuradj Kurepa <91743470+dkurepa@users.noreply.github.com> Date: Tue, 6 Aug 2024 17:04:00 +0200 Subject: [PATCH 4/9] Fix Darc DI registration test and enable it (#3794) --- .../Microsoft.DotNet.Darc.Tests/DependencyRegistrationTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/Microsoft.DotNet.Darc.Tests/DependencyRegistrationTests.cs b/test/Microsoft.DotNet.Darc.Tests/DependencyRegistrationTests.cs index 88c07ae333..7a0c790893 100644 --- a/test/Microsoft.DotNet.Darc.Tests/DependencyRegistrationTests.cs +++ b/test/Microsoft.DotNet.Darc.Tests/DependencyRegistrationTests.cs @@ -16,7 +16,6 @@ namespace Microsoft.DotNet.Darc.Tests; public class DependencyRegistrationTests { [Test] - [Ignore("Test skipped because it hangs in CI. https://github.com/dotnet/arcade-services/issues/3745")] public void AreDependenciesRegistered() { DependencyInjectionValidation.IsDependencyResolutionCoherent(services => @@ -29,6 +28,8 @@ public void AreDependenciesRegistered() services.AddTransient(optionType); var operationOption = (CommandLineOptions) Activator.CreateInstance(optionType); + // Set IsCi to true to avoid login pop up + operationOption.IsCi = true; operationOption.RegisterServices(services); var provider = services.BuildServiceProvider(); From a5222c98e0e78bde89d9d730ce35318b1f07e869 Mon Sep 17 00:00:00 2001 From: Djuradj Kurepa <91743470+dkurepa@users.noreply.github.com> Date: Tue, 6 Aug 2024 17:44:20 +0200 Subject: [PATCH 5/9] Add second build to channel before triggering the subscription (#3796) --- test/Maestro.ScenarioTests/EndToEndFlowLogic.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Maestro.ScenarioTests/EndToEndFlowLogic.cs b/test/Maestro.ScenarioTests/EndToEndFlowLogic.cs index 46e83afccd..778bcd730c 100644 --- a/test/Maestro.ScenarioTests/EndToEndFlowLogic.cs +++ b/test/Maestro.ScenarioTests/EndToEndFlowLogic.cs @@ -320,6 +320,7 @@ public async Task NonBatchedUpdatingGitHubFlowTestBase(string targetBranch, stri TestContext.WriteLine("Set up another build for intake into target repository"); Build build2 = await CreateBuildAsync(sourceRepoUri, sourceBranch, TestRepository.CoherencyTestRepo2Commit, Source2BuildNumber, source1AssetsUpdated); + await AddBuildToChannelAsync(build2.Id, testChannelName); TestContext.WriteLine("Trigger the dependency update"); await TriggerSubscriptionAsync(subscription1Id.Value); From cb73b4a55675e352945eec62b34f47632646873d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Wed, 7 Aug 2024 17:14:32 +0200 Subject: [PATCH 6/9] Fix asset validation in E2E tests (#3813) --- test/Maestro.ScenarioTests/ScenarioTests_GitHubFlow.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/Maestro.ScenarioTests/ScenarioTests_GitHubFlow.cs b/test/Maestro.ScenarioTests/ScenarioTests_GitHubFlow.cs index 2f66a33893..38849241e4 100644 --- a/test/Maestro.ScenarioTests/ScenarioTests_GitHubFlow.cs +++ b/test/Maestro.ScenarioTests/ScenarioTests_GitHubFlow.cs @@ -87,18 +87,18 @@ public ScenarioTests_GitHubFlow() new DependencyDetail { Name = "Foo", - Version = "1.1.0", + Version = "1.17.0", RepoUri = sourceRepoUri, - Commit = TestRepository.CoherencyTestRepo1Commit, + Commit = TestRepository.CoherencyTestRepo2Commit, Type = DependencyType.Product, Pinned = false }, new DependencyDetail { Name = "Bar", - Version = "2.1.0", + Version = "2.17.0", RepoUri = sourceRepoUri, - Commit = TestRepository.CoherencyTestRepo1Commit, + Commit = TestRepository.CoherencyTestRepo2Commit, Type = DependencyType.Product, Pinned = false } From d2e740d513977d05fe96dc3b53327465bf85db1c Mon Sep 17 00:00:00 2001 From: Djuradj Kurepa <91743470+dkurepa@users.noreply.github.com> Date: Fri, 9 Aug 2024 11:13:56 +0200 Subject: [PATCH 7/9] Fix Darc GitHubFlow_NonBatched test (#3816) --- test/Maestro.ScenarioTests/MaestroScenarioTestBase.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/Maestro.ScenarioTests/MaestroScenarioTestBase.cs b/test/Maestro.ScenarioTests/MaestroScenarioTestBase.cs index c8e8c4876a..3a0a98cbed 100644 --- a/test/Maestro.ScenarioTests/MaestroScenarioTestBase.cs +++ b/test/Maestro.ScenarioTests/MaestroScenarioTestBase.cs @@ -31,6 +31,8 @@ internal abstract class MaestroScenarioTestBase { private TestParameters _parameters = null!; private List _baseDarcRunArgs = []; + // We need this for tests where we have multiple updates + private Dictionary _lastUpdatedPrTimes = new(); protected IMaestroApi MaestroApi => _parameters.MaestroApi; @@ -68,6 +70,12 @@ public void SetTestParameters(TestParameters parameters) if (prs.Count == 1) { + // We use this method when we're creating the PR, and when we're fetching the updated PR + // We only want to set the Creation time when we're creating it + if (!_lastUpdatedPrTimes.ContainsKey(prs[0].Id)) + { + _lastUpdatedPrTimes[prs[0].Id] = prs[0].CreatedAt; + } return prs[0]; } @@ -91,8 +99,9 @@ public void SetTestParameters(TestParameters parameters) { pr = await GitHubApi.PullRequest.Get(repo.Id, pr.Number); - if (pr.CreatedAt != pr.UpdatedAt) + if (_lastUpdatedPrTimes[pr.Id] != pr.UpdatedAt) { + _lastUpdatedPrTimes[pr.Id] = pr.UpdatedAt; return pr; } From e2987106ae9e879299bb28b6e5742140cb0458f2 Mon Sep 17 00:00:00 2001 From: Oleksandr Didyk <106967057+oleksandr-didyk@users.noreply.github.com> Date: Mon, 12 Aug 2024 11:06:44 +0200 Subject: [PATCH 8/9] Update maestro token for e2e tests (#3817) --- azure-pipelines.yml | 21 +------- eng/templates/stages/deploy.yaml | 89 +++++++++++++++++++------------- 2 files changed, 56 insertions(+), 54 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 33d086a80e..71d1ff029e 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -68,7 +68,7 @@ extends: variables: # DotNet-Symbol-Server-Pats provides: microsoft-symbol-server-pat, symweb-symbol-server-pat - # Publish-Build-Assets provides: MaestroAccessToken, BotAccount-dotnet-maestro-bot-PAT + # Publish-Build-Assets provides: MaestroAppClientId, MaestroStagingAppClientId, BotAccount-dotnet-maestro-bot-PAT - group: DotNet-Symbol-Server-Pats - group: Publish-Build-Assets - _InternalBuildArgs: /p:DotNetSignType=$(_SignType) /p:TeamName=$(_TeamName) @@ -163,24 +163,7 @@ extends: - template: /eng/templates/stages/deploy.yaml@self parameters: - ${{ if ne(variables['Build.SourceBranch'], 'refs/heads/production') }}: - DeploymentEnvironment: Staging - MaestroTestEndpoints: https://maestro-int.westus2.cloudapp.azure.com,https://maestro.int-dot.net - PublishProfile: Int - Subscription: NetHelixStaging - ScenarioTestSubscription: "Darc: Maestro Staging" - VariableGroup: MaestroInt KeyVault - BarConnectionString: "Data Source=tcp:maestro-int-server.database.windows.net,1433; Initial Catalog=BuildAssetRegistry; Authentication=Active Directory Default; Persist Security Info=False; MultipleActiveResultSets=True; Connect Timeout=120; Encrypt=True; TrustServerCertificate=False; User Id=736067df-a2c8-4c63-ad30-12db55c186cc" - BarMigrationSubscription: BarMigrationInt - ${{ else }}: - DeploymentEnvironment: Production - MaestroTestEndpoints: https://maestro-prod.westus2.cloudapp.azure.com,https://maestro.dot.net - PublishProfile: Prod - Subscription: NetHelix - ScenarioTestSubscription: "Darc: Maestro Production" - VariableGroup: MaestroProd KeyVault - BarConnectionString: "Data Source=tcp:maestro-prod.database.windows.net,1433; Initial Catalog=BuildAssetRegistry; Authentication=Active Directory Default; Persist Security Info=False; MultipleActiveResultSets=True; Connect Timeout=120; Encrypt=True; TrustServerCertificate=False; User Id=1093df3b-c754-4788-a4ae-ea33b86b82aa" - BarMigrationSubscription: BarMigrationProd + isProd: ${{ eq(variables['Build.SourceBranch'], 'refs/heads/production') }} - ${{ if in(variables['Build.SourceBranch'], 'refs/heads/main', 'refs/heads/production') }}: - template: /eng/common/templates-official/post-build/post-build.yml@self diff --git a/eng/templates/stages/deploy.yaml b/eng/templates/stages/deploy.yaml index acf8e1f63f..578299c086 100644 --- a/eng/templates/stages/deploy.yaml +++ b/eng/templates/stages/deploy.yaml @@ -1,24 +1,8 @@ parameters: -- name: Subscription - type: string -- name: ScenarioTestSubscription - type: string -- name: PublishProfile - type: string - values: ['Int', 'Prod'] -- name: DeploymentEnvironment - type: string -- name: VariableGroup - type: string -- name: MaestroTestEndpoints - type: string -- name: BarConnectionString - type: string -- name: BarMigrationSubscription - type: string +- name: isProd + type: boolean # --- Secret Variable group requirements --- -# scenario-test-maestro-token # dn-bot-dnceng-build-rw-code-rw-release-rw # maestro-scenario-test-github-token @@ -30,12 +14,15 @@ stages: - stage: approval dependsOn: - build - - ${{ if eq(variables['Build.SourceBranch'], 'refs/heads/production')}}: + - ${{ if parameters.isProd }}: - publish_using_darc jobs: - deployment: approval displayName: deployment approval (conditional) - environment: ${{ parameters.DeploymentEnvironment }} + ${{ if parameters.isProd }}: + environment: Production + ${{ else }}: + environment: Staging pool: server strategy: runOnce: @@ -48,9 +35,29 @@ stages: - approval variables: + - ${{ if parameters.isProd }}: + - group: MaestroProd KeyVault + - name: PublishProfile + value: Prod + - name: Subscription + value: NetHelix + - name: BarConnectionString + value: "Data Source=tcp:maestro-prod.database.windows.net,1433; Initial Catalog=BuildAssetRegistry; Authentication=Active Directory Default; Persist Security Info=False; MultipleActiveResultSets=True; Connect Timeout=120; Encrypt=True; TrustServerCertificate=False; User Id=1093df3b-c754-4788-a4ae-ea33b86b82aa" + - name: BarMigrationSubscription + value: BarMigrationProd + - ${{ else }}: + - group: MaestroInt KeyVault + - name: PublishProfile + value: Int + - name: Subscription + value: NetHelixStaging + - name: BarConnectionString + value: "Data Source=tcp:maestro-int-server.database.windows.net,1433; Initial Catalog=BuildAssetRegistry; Authentication=Active Directory Default; Persist Security Info=False; MultipleActiveResultSets=True; Connect Timeout=120; Encrypt=True; TrustServerCertificate=False; User Id=736067df-a2c8-4c63-ad30-12db55c186cc" + - name: BarMigrationSubscription + value: BarMigrationInt + - name: currentDate value: $[format('{0:yyyy}-{0:MM}-{0:dd}', pipeline.startTime)] - - group: ${{ parameters.VariableGroup }} jobs: - job: updateDatabase @@ -62,7 +69,7 @@ stages: - task: AzureCLI@2 displayName: Entity Framework update inputs: - azureSubscription: ${{ parameters.BarMigrationSubscription }} + azureSubscription: ${{ variables.BarMigrationSubscription }} scriptType: ps scriptLocation: inlineScript inlineScript: | @@ -74,7 +81,7 @@ stages: --no-build ` --verbose env: - BUILD_ASSET_REGISTRY_DB_CONNECTION_STRING: ${{ parameters.BarConnectionString }} + BUILD_ASSET_REGISTRY_DB_CONNECTION_STRING: ${{ variables.BarConnectionString }} - job: deployMaestro displayName: Deploy Maestro @@ -92,7 +99,7 @@ stages: - task: AzurePowerShell@5 displayName: Deploy Service Fabric Application (Maestro) inputs: - azureSubscription: ${{ parameters.Subscription }} + azureSubscription: ${{ variables.Subscription }} scriptType: inlineScript azurePowerShellVersion: LatestVersion inline: | @@ -105,7 +112,7 @@ stages: } eng/deployment/deploy.ps1 -obj $env:BUILD_ARTIFACTSTAGINGDIRECTORY -appPackagePath $env:ApplicationPackage -publishProfile $env:PublishProfilePath -autoRollBack $autoRollBack -location westus2 env: - PublishProfilePath: $(Pipeline.Workspace)/MaestroApplication/projectartifacts/PublishProfiles/${{ parameters.PublishProfile }}.xml + PublishProfilePath: $(Pipeline.Workspace)/MaestroApplication/projectartifacts/PublishProfiles/${{ variables.PublishProfile }}.xml ApplicationPackage: $(Pipeline.Workspace)/MaestroApplication/applicationpackage - ${{ if eq(variables['Build.SourceBranch'], 'refs/heads/production')}}: @@ -134,7 +141,23 @@ stages: - deploy variables: - - group: ${{ parameters.VariableGroup }} + - group: Publish-Build-Assets + - ${{ if parameters.isProd }}: + - group: MaestroProd KeyVault + - name: MaestroTestEndpoints + value: https://maestro-prod.westus2.cloudapp.azure.com,https://maestro.dot.net + - name: ScenarioTestSubscription + value: "Darc: Maestro Production" + - name: MaestroAppClientId + value: $(MaestroAppClientId) + - ${{ else }}: + - group: MaestroInt KeyVault + - name: MaestroTestEndpoints + value: https://maestro-int.westus2.cloudapp.azure.com,https://maestro.int-dot.net + - name: ScenarioTestSubscription + value: "Darc: Maestro Staging" + - name: MaestroAppClientId + value: $(MaestroStagingAppClientId) jobs: - job: scenario @@ -172,13 +195,13 @@ stages: name: GetAuthInfo displayName: Get auth information inputs: - azureSubscription: ${{ parameters.ScenarioTestSubscription }} + azureSubscription: ${{ variables.ScenarioTestSubscription }} addSpnToEnvironment: true scriptType: ps scriptLocation: inlineScript inlineScript: | # Fetch token used for scenario tests - $token = (az account get-access-token --resource "$env:servicePrincipalId" | ConvertFrom-Json).accessToken + $token = (az account get-access-token --resource "${{ variables.MaestroAppClientId }}" | ConvertFrom-Json).accessToken echo "##vso[task.setvariable variable=Token;isOutput=true;isSecret=true]$token" # Set variables with auth info for tests below @@ -187,7 +210,7 @@ stages: echo "##vso[task.setvariable variable=TenantId;isOutput=true]$env:tenantId" # Either of the URIs will do - $barUri = "${{ split(parameters.MaestroTestEndpoints, ',')[0] }}" + $barUri = "${{ split(variables.MaestroTestEndpoints, ',')[0] }}" echo "##vso[task.setvariable variable=BarUri;isOutput=true]$barUri" - powershell: @@ -197,11 +220,7 @@ stages: displayName: Test Azure CLI auth - powershell: - .\darc\darc.exe get-default-channels --source-repo arcade-services --ci -t "$(GetAuthInfo.FederatedToken)" --bar-uri "$(GetAuthInfo.BarUri)" --debug - displayName: Test Federated token auth - - - powershell: - .\darc\darc.exe get-default-channels --source-repo arcade-services --ci --password "$(scenario-test-maestro-token)" --bar-uri "$(GetAuthInfo.BarUri)" --debug + .\darc\darc.exe get-default-channels --source-repo arcade-services --ci --password "$(GetAuthInfo.Token)" --bar-uri "$(GetAuthInfo.BarUri)" --debug displayName: Test BAR token auth - task: VSTest@2 @@ -213,7 +232,7 @@ stages: searchFolder: $(Pipeline.Workspace)/Maestro.ScenarioTests runInParallel: true env: - MAESTRO_BASEURIS: ${{ parameters.MaestroTestEndpoints }} + MAESTRO_BASEURIS: ${{ variables.MaestroTestEndpoints }} MAESTRO_TOKEN: $(GetAuthInfo.Token) GITHUB_TOKEN: $(maestro-scenario-test-github-token) AZDO_TOKEN: $(dn-bot-dnceng-build-rw-code-rw-release-rw) From b2b501bdb3bfa3c5f17c5868be50ae0f10d65cc1 Mon Sep 17 00:00:00 2001 From: Oleksandr Didyk <106967057+oleksandr-didyk@users.noreply.github.com> Date: Mon, 12 Aug 2024 13:34:14 +0200 Subject: [PATCH 9/9] Remove federated token auth (#3819) --- src/Maestro/Client/src/MaestroApiFactory.cs | 6 ------ src/Maestro/Client/src/MaestroApiOptions.cs | 4 +--- .../Maestro.Common/AppCredentials/AppCredential.cs | 14 -------------- .../AppCredentials/AppCredentialResolver.cs | 12 +++--------- .../AppCredentials/CredentialResolverOptions.cs | 5 ----- .../AzureDevOpsTokens/AzureDevOpsTokenProvider.cs | 13 +++---------- src/Maestro/Maestro.Web/Startup.cs | 1 - .../src/PushMetadataToBuildAssetRegistry.cs | 1 - .../Darc/Helpers/RemoteFactory.cs | 1 - .../Darc/Options/CommandLineOptions.cs | 6 ------ .../Darc/Options/ICommandLineOptions.cs | 1 - src/Microsoft.DotNet.Darc/DarcLib/BarApiClient.cs | 6 +++--- .../Configuration/PcsConfiguration.cs | 1 - .../ProductConstructionServiceApiOptions.cs | 1 - test/Maestro.ScenarioTests/TestParameters.cs | 1 - .../VmrTestsBase.cs | 1 - 16 files changed, 10 insertions(+), 64 deletions(-) diff --git a/src/Maestro/Client/src/MaestroApiFactory.cs b/src/Maestro/Client/src/MaestroApiFactory.cs index 71770fde56..bc60def6d6 100644 --- a/src/Maestro/Client/src/MaestroApiFactory.cs +++ b/src/Maestro/Client/src/MaestroApiFactory.cs @@ -13,13 +13,11 @@ public static class MaestroApiFactory /// /// URI of the build asset registry service to use. /// Optional BAR token. When provided, will be used as the primary auth method. - /// Optional federated credentials token. /// Managed Identity to use for the auth /// Whether to include interactive login flows public static IMaestroApi GetAuthenticated( string baseUri, string? accessToken, - string? federatedToken, string? managedIdentityId, bool disableInteractiveAuth) { @@ -27,7 +25,6 @@ public static IMaestroApi GetAuthenticated( baseUri, accessToken, managedIdentityId, - federatedToken, disableInteractiveAuth)); } @@ -35,12 +32,10 @@ public static IMaestroApi GetAuthenticated( /// Obtains API client for authenticated access to Maestro. /// /// Optional BAR token. When provided, will be used as the primary auth method. - /// Optional federated token. When provided, will be used as the primary auth method. /// Managed Identity to use for the auth /// Whether to include interactive login flows public static IMaestroApi GetAuthenticated( string? accessToken, - string? federatedToken, string? managedIdentityId, bool disableInteractiveAuth) { @@ -48,7 +43,6 @@ public static IMaestroApi GetAuthenticated( MaestroApiOptions.StagingBuildAssetRegistryBaseUri, accessToken, managedIdentityId, - federatedToken, disableInteractiveAuth)); } diff --git a/src/Maestro/Client/src/MaestroApiOptions.cs b/src/Maestro/Client/src/MaestroApiOptions.cs index 9b98163bd2..bba86e5697 100644 --- a/src/Maestro/Client/src/MaestroApiOptions.cs +++ b/src/Maestro/Client/src/MaestroApiOptions.cs @@ -39,9 +39,8 @@ public partial class MaestroApiOptions /// API base URI /// Optional BAR token. When provided, will be used as the primary auth method. /// Managed Identity to use for the auth - /// Optional federated token. When provided, will be used as the primary auth method. /// Whether to include interactive login flows - public MaestroApiOptions(string baseUri, string accessToken, string managedIdentityId, string federatedToken, bool disableInteractiveAuth) + public MaestroApiOptions(string baseUri, string accessToken, string managedIdentityId, bool disableInteractiveAuth) : this( new Uri(baseUri), AppCredentialResolver.CreateCredential( @@ -49,7 +48,6 @@ public MaestroApiOptions(string baseUri, string accessToken, string managedIdent { DisableInteractiveAuth = disableInteractiveAuth, Token = accessToken, - FederatedToken = federatedToken, ManagedIdentityId = managedIdentityId, UserScope = APP_USER_SCOPE, })) diff --git a/src/Maestro/Maestro.Common/AppCredentials/AppCredential.cs b/src/Maestro/Maestro.Common/AppCredentials/AppCredential.cs index 571e4e759e..866a4ed932 100644 --- a/src/Maestro/Maestro.Common/AppCredentials/AppCredential.cs +++ b/src/Maestro/Maestro.Common/AppCredentials/AppCredential.cs @@ -116,20 +116,6 @@ private static InteractiveBrowserCredential GetInteractiveCredential( return credential; } - /// - /// Use this for invocations from pipelines with a federated token - /// - public static AppCredential CreateFederatedCredential(string appId, string federatedToken) - { - var credential = new ClientAssertionCredential( - TENANT_ID, - appId, - token => Task.FromResult(federatedToken)); - - var requestContext = new TokenRequestContext([$"api://{appId}/.default"]); - return new AppCredential(credential, requestContext); - } - /// /// Use this for invocations from services using an MI. /// ID can be "system" for system-assigned identity or GUID for a user assigned one. diff --git a/src/Maestro/Maestro.Common/AppCredentials/AppCredentialResolver.cs b/src/Maestro/Maestro.Common/AppCredentials/AppCredentialResolver.cs index 6feadf090b..bc679f6d32 100644 --- a/src/Maestro/Maestro.Common/AppCredentials/AppCredentialResolver.cs +++ b/src/Maestro/Maestro.Common/AppCredentials/AppCredentialResolver.cs @@ -18,25 +18,19 @@ public static TokenCredential CreateCredential(AppCredentialResolverOptions opti return new ResolvedCredential(options.Token!); } - // 2. Federated token that can be used to fetch an app token (for CI scenarios) - if (!string.IsNullOrEmpty(options.FederatedToken)) - { - return AppCredential.CreateFederatedCredential(options.AppId, options.FederatedToken!); - } - - // 3. Managed identity (for server-to-server scenarios - e.g. PCS->Maestro) + // 2. Managed identity (for server-to-server scenarios - e.g. PCS->Maestro) if (!string.IsNullOrEmpty(options.ManagedIdentityId)) { return AppCredential.CreateManagedIdentityCredential(options.AppId, options.ManagedIdentityId!); } - // 4. Azure CLI authentication setup by the caller (for CI scenarios) + // 3. Azure CLI authentication setup by the caller (for CI scenarios) if (options.DisableInteractiveAuth) { return AppCredential.CreateNonUserCredential(options.AppId); } - // 5. Interactive login (user-based scenario) + // 4. Interactive login (user-based scenario) return AppCredential.CreateUserCredential(options.AppId, options.UserScope); } } diff --git a/src/Maestro/Maestro.Common/AppCredentials/CredentialResolverOptions.cs b/src/Maestro/Maestro.Common/AppCredentials/CredentialResolverOptions.cs index 25871eb8c7..b66ab70969 100644 --- a/src/Maestro/Maestro.Common/AppCredentials/CredentialResolverOptions.cs +++ b/src/Maestro/Maestro.Common/AppCredentials/CredentialResolverOptions.cs @@ -15,11 +15,6 @@ public class CredentialResolverOptions /// public string? Token { get; set; } - /// - /// Federated token to use for fetching the token. If none supplied, will try other flows. - /// - public string? FederatedToken { get; set; } - /// /// Managed Identity to use for the auth /// diff --git a/src/Maestro/Maestro.Common/AzureDevOpsTokens/AzureDevOpsTokenProvider.cs b/src/Maestro/Maestro.Common/AzureDevOpsTokens/AzureDevOpsTokenProvider.cs index 0e871de32b..0bb9ed58a8 100644 --- a/src/Maestro/Maestro.Common/AzureDevOpsTokens/AzureDevOpsTokenProvider.cs +++ b/src/Maestro/Maestro.Common/AzureDevOpsTokens/AzureDevOpsTokenProvider.cs @@ -126,14 +126,7 @@ private static Dictionary GetCredentials( continue; } - // 2. Federated token that can be used to fetch an app token (for CI scenarios) - if (!string.IsNullOrEmpty(option.FederatedToken)) - { - credentials[account] = AppCredential.CreateFederatedCredential(option.AppId, option.FederatedToken!); - continue; - } - - // 3. Managed identity (for server-to-AzDO scenarios) + // 2. Managed identity (for server-to-AzDO scenarios) if (!string.IsNullOrEmpty(option.ManagedIdentityId)) { credentials[account] = option.ManagedIdentityId == "system" @@ -142,14 +135,14 @@ private static Dictionary GetCredentials( continue; } - // 4. Azure CLI authentication setup by the caller (for CI scenarios) + // 3. Azure CLI authentication setup by the caller (for CI scenarios) if (option.DisableInteractiveAuth) { credentials[account] = AppCredential.CreateNonUserCredential(option.AppId); continue; } - // 5. Interactive login (user-based scenario) + // 4. Interactive login (user-based scenario) credentials[account] = new DefaultAzureCredential(includeInteractiveCredentials: true); } diff --git a/src/Maestro/Maestro.Web/Startup.cs b/src/Maestro/Maestro.Web/Startup.cs index 8990475448..9cfda66ace 100644 --- a/src/Maestro/Maestro.Web/Startup.cs +++ b/src/Maestro/Maestro.Web/Startup.cs @@ -263,7 +263,6 @@ public override void ConfigureServices(IServiceCollection services) targetUri, accessToken: token, managedIdentityId: managedIdentityId, - federatedToken: null, disableInteractiveAuth: !IsLocalKestrelDevMode)); } diff --git a/src/Maestro/Microsoft.DotNet.Maestro.Tasks/src/PushMetadataToBuildAssetRegistry.cs b/src/Maestro/Microsoft.DotNet.Maestro.Tasks/src/PushMetadataToBuildAssetRegistry.cs index b3e6dd4fc6..9037e4c2b3 100644 --- a/src/Maestro/Microsoft.DotNet.Maestro.Tasks/src/PushMetadataToBuildAssetRegistry.cs +++ b/src/Maestro/Microsoft.DotNet.Maestro.Tasks/src/PushMetadataToBuildAssetRegistry.cs @@ -145,7 +145,6 @@ public async Task PushMetadataAsync(CancellationToken cancellationToken) IMaestroApi client = MaestroApiFactory.GetAuthenticated( MaestroApiEndpoint, BuildAssetRegistryToken, - federatedToken: null, managedIdentityId: null, !AllowInteractive); diff --git a/src/Microsoft.DotNet.Darc/Darc/Helpers/RemoteFactory.cs b/src/Microsoft.DotNet.Darc/Darc/Helpers/RemoteFactory.cs index a94452e498..f077080842 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Helpers/RemoteFactory.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Helpers/RemoteFactory.cs @@ -28,7 +28,6 @@ public static IRemote GetRemote(ICommandLineOptions options, string repoUrl, ILo public static IBarApiClient GetBarClient(ICommandLineOptions options, ILogger logger) => new BarApiClient( options.BuildAssetRegistryToken, - options.FederatedToken, managedIdentityId: null, options.IsCi, options.BuildAssetRegistryBaseUri); diff --git a/src/Microsoft.DotNet.Darc/Darc/Options/CommandLineOptions.cs b/src/Microsoft.DotNet.Darc/Darc/Options/CommandLineOptions.cs index 02600260d5..2ea7184e39 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Options/CommandLineOptions.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Options/CommandLineOptions.cs @@ -33,11 +33,6 @@ public abstract class CommandLineOptions : ICommandLineOptions [RedactFromLogging] public string BuildAssetRegistryToken { get; set; } - [Option('t', "federated-token", - HelpText = "Federated credentials token used to authenticate to BAR. If it or the generic token are omitted, auth falls back to Azure CLI or an interactive browser login flow.")] - [RedactFromLogging] - public string FederatedToken { get; set; } - [Option("github-pat", HelpText = "Token used to authenticate GitHub.")] [RedactFromLogging] public string GitHubPat { get; set; } @@ -158,7 +153,6 @@ public virtual IServiceCollection RegisterServices(IServiceCollection services) o["default"] = new AzureDevOpsCredentialResolverOptions { Token = AzureDevOpsPat, - FederatedToken = FederatedToken, DisableInteractiveAuth = IsCi, }; }); diff --git a/src/Microsoft.DotNet.Darc/Darc/Options/ICommandLineOptions.cs b/src/Microsoft.DotNet.Darc/Darc/Options/ICommandLineOptions.cs index eec3ce231e..b7054ada55 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Options/ICommandLineOptions.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Options/ICommandLineOptions.cs @@ -13,7 +13,6 @@ public interface ICommandLineOptions { string AzureDevOpsPat { get; set; } string BuildAssetRegistryBaseUri { get; set; } - string FederatedToken { get; set; } string BuildAssetRegistryToken { get; set; } string GitHubPat { get; set; } string GitLocation { get; set; } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/BarApiClient.cs b/src/Microsoft.DotNet.Darc/DarcLib/BarApiClient.cs index c760331a64..4a3d29fc99 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/BarApiClient.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/BarApiClient.cs @@ -20,11 +20,11 @@ public class BarApiClient : IBarApiClient { private readonly IMaestroApi _barClient; - public BarApiClient(string? buildAssetRegistryPat, string? federatedToken, string? managedIdentityId, bool disableInteractiveAuth, string? buildAssetRegistryBaseUri = null) + public BarApiClient(string? buildAssetRegistryPat, string? managedIdentityId, bool disableInteractiveAuth, string? buildAssetRegistryBaseUri = null) { _barClient = !string.IsNullOrEmpty(buildAssetRegistryBaseUri) - ? MaestroApiFactory.GetAuthenticated(buildAssetRegistryBaseUri, buildAssetRegistryPat, federatedToken, managedIdentityId, disableInteractiveAuth) - : MaestroApiFactory.GetAuthenticated(buildAssetRegistryPat, managedIdentityId, federatedToken, disableInteractiveAuth); + ? MaestroApiFactory.GetAuthenticated(buildAssetRegistryBaseUri, buildAssetRegistryPat, managedIdentityId, disableInteractiveAuth) + : MaestroApiFactory.GetAuthenticated(buildAssetRegistryPat, managedIdentityId, disableInteractiveAuth); } #region Channel Operations diff --git a/src/ProductConstructionService/ProductConstructionService.Api/Configuration/PcsConfiguration.cs b/src/ProductConstructionService/ProductConstructionService.Api/Configuration/PcsConfiguration.cs index be7b6ee53c..f646899be8 100644 --- a/src/ProductConstructionService/ProductConstructionService.Api/Configuration/PcsConfiguration.cs +++ b/src/ProductConstructionService/ProductConstructionService.Api/Configuration/PcsConfiguration.cs @@ -91,7 +91,6 @@ public static void ConfigurePcs( uri, accessToken: null, managedIdentityId: managedIdentityId, - federatedToken: null, disableInteractiveAuth: true); }); diff --git a/src/ProductConstructionService/ProductConstructionService.Client/ProductConstructionServiceApiOptions.cs b/src/ProductConstructionService/ProductConstructionService.Client/ProductConstructionServiceApiOptions.cs index 1dbfe0212f..e27da4b9d6 100644 --- a/src/ProductConstructionService/ProductConstructionService.Client/ProductConstructionServiceApiOptions.cs +++ b/src/ProductConstructionService/ProductConstructionService.Client/ProductConstructionServiceApiOptions.cs @@ -30,7 +30,6 @@ public ProductConstructionServiceApiOptions(string baseUri, string accessToken, { DisableInteractiveAuth = true, // the client is only used in Maestro for now Token = accessToken, - FederatedToken = null, ManagedIdentityId = managedIdentityId, })) { diff --git a/test/Maestro.ScenarioTests/TestParameters.cs b/test/Maestro.ScenarioTests/TestParameters.cs index 1717cd5f34..378b73c1b3 100644 --- a/test/Maestro.ScenarioTests/TestParameters.cs +++ b/test/Maestro.ScenarioTests/TestParameters.cs @@ -66,7 +66,6 @@ public static async Task GetAsync(bool useNonPrimaryEndpoint = f maestroBaseUri, maestroToken, managedIdentityId: null, - federatedToken: null, disableInteractiveAuth: isCI); string? darcRootDir = darcDir; diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs index fe40bfdab9..47b7647995 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs @@ -84,7 +84,6 @@ public void DeleteCurrentTestDirectory() .AddVmrManagers("git", VmrPath, TmpPath, null, null) .AddSingleton(new BarApiClient( buildAssetRegistryPat: null, - federatedToken: null, managedIdentityId: null, disableInteractiveAuth: true, buildAssetRegistryBaseUri: MaestroApiOptions.StagingBuildAssetRegistryBaseUri));