Migrate Identity Live tests deployment to Fed Auth. #42635
Conversation
g2vinay
changed the title
…to update-identity-live-tests
g2vinay
force-pushed
the
update-identity-live-tests
branch
from
c8f73e7
to
3cd1232
Compare
sdk/identity/ci.yml
Outdated
| EnvVars: | ||
| SYSTEM_ACCESSTOKEN: $(System.AccessToken) | ||
| ARM_OIDC_TOKEN: $(ARM_OIDC_TOKEN) |
There was a problem hiding this comment.
In order to get these variables I think you will need to plumb the PersistOidcToken token open similar to what @benbp is doing in js at Azure/azure-sdk-for-js#31335
There was a problem hiding this comment.
Yep, and we should revert usage of ARM_CLIENT_ID and ARM_TENANT_ID, since these variables are already set as IDENTITY_*_ID
sdk/identity/ci.yml
Outdated
| ARM_CLIENT_ID: $(ARM_CLIENT_ID) | ||
| ARM_TENANT_ID: $(ARM_TENANT_ID) | ||
| inputs: | ||
| azureSubscription: azure-sdk-tests |
There was a problem hiding this comment.
See my other comment https://github.com/Azure/azure-sdk-for-java/pull/42635/files#r1819384979 but you should not need this PreStep and as it currently setup it will not work because we need to have the subscription match the deployment.
| String spClientId = configuration.get("IDENTITY_CLIENT_ID"); | ||
| String secret = configuration.get("IDENTITY_CLIENT_SECRET"); | ||
| String oidc = configuration.get("ARM_OIDC_TOKEN"); |
There was a problem hiding this comment.
If you want to use this token at the running of the tests you will also need to set it as an env variable in the tests.yml file.
sdk/identity/ci.yml
Outdated
| EnvVars: | ||
| SYSTEM_ACCESSTOKEN: $(System.AccessToken) | ||
| # EnvVars: | ||
| # AZ_TENANT_ID: $(IDENTITY_TENANT_ID) |
There was a problem hiding this comment.
You will need to map ARM_OIDC_TOKEN: $(ARM_OIDC_TOKEN) into the env to use it at test runtime.
| SYSTEM_ACCESSTOKEN: $(System.AccessToken) | ||
| # EnvVars: | ||
| # AZ_TENANT_ID: $(IDENTITY_TENANT_ID) | ||
| # AZ_CLIENT_ID: $(IDENTITY_CLIENT_ID) | ||
| CloudConfig: | ||
| Public: | ||
| SubscriptionConfigurations: |
There was a problem hiding this comment.
You should remove $(sub-config-azure-cloud-test-resources) and investigate if $(sub-config-identity-test-resources) is needed any longer as well and if not remove it also.
There was a problem hiding this comment.
Does referencing IDENTITY_TENANT_ID directly in LiveManagedIdentityTests.java not work?
There was a problem hiding this comment.
We don't end up setting those variables so I had him switch to the ones we set for the azurepipelinecredential
|
|
||
|
|
||
|
|
| AZ_TENANT_ID: $(IDENTITY_TENANT_ID) | ||
| AZ_CLIENT_ID: $(IDENTITY_CLIENT_ID) |
There was a problem hiding this comment.
| AZ_TENANT_ID: $(IDENTITY_TENANT_ID) | |
| AZ_CLIENT_ID: $(IDENTITY_CLIENT_ID) |
I don't think those are needed.
sdk/identity/test-resources-post.ps1
Outdated
| @@ -42,8 +42,7 @@ $azBuildToolsRootPom = "$PSScriptRoot/../../eng/code-quality-reports/pom.xml" | | |||
| $funcAppRoot = "$PSScriptRoot/live-test-apps/identity-test-function" | Resolve-Path | |||
| $funcAppPom = "$funcAppRoot/pom.xml" | Resolve-Path | |||
|
|
|||
| az login --service-principal -u $(getVariable('IDENTITY_CLIENT_ID')) -p $(getVariable('IDENTITY_CLIENT_SECRET')) --tenant $(getVariable('IDENTITY_TENANT_ID')) | |||
| az account set --subscription $(getVariable('IDENTITY_SUBSCRIPTION_ID')) | |||
| # az account set --subscription $(getVariable('IDENTITY_SUBSCRIPTION_ID')) | |||
There was a problem hiding this comment.
Should this be removed now if it isn't needed?
| TokenCredential pipelinesCredential = getIdentityTestCredential(interceptorManager); | ||
|
|
||
| AccessToken accessToken = pipelinesCredential | ||
| .getTokenSync(new TokenRequestContext().addScopes("https://management.azure.com/.default")); |
There was a problem hiding this comment.
If we hardcode this scope then I don't think tests will work in sovereign clouds.
sdk/identity/test-resources-post.ps1
Outdated
| Write-Host "##vso[task.setvariable variable=ARM_OIDC_TOKEN]$ARM_OIDC_TOKEN" | ||
| $env:ARM_OIDC_TOKEN = $ARM_OIDC_TOKEN | ||
|
|
||
| # Set ARM_OIDC_TOKEN as a global pipeline variable for all subsequent steps |
There was a problem hiding this comment.
What are you attempting to do here? We need to be careful about output a secret and also looks like this might clash with the existing token we are writing.
No description provided.