-
Notifications
You must be signed in to change notification settings - Fork 177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Only fetch provision application oid via API if not supplied #2592
Conversation
The following pipelines have been queued for testing: |
@@ -45,6 +45,10 @@ param ( | |||
[ValidatePattern('^[0-9a-f]{8}(-[0-9a-f]{4}){3}-[0-9a-f]{12}$')] | |||
[string] $ProvisionerApplicationId, | |||
|
|||
[Parameter(ParameterSetName = 'Provisioner', Mandatory = $false)] | |||
[ValidatePattern('^[0-9a-f]{8}(-[0-9a-f]{4}){3}-[0-9a-f]{12}$')] | |||
[string] $ProvisionerApplicationOid, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have this already in the sub-config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have the value (via TestApplicationOid
which is the same as provisioner for CI), but the key is not set. I set the parameter mandatory as $false
so it will still be backwards compatible, and we can then update the sub configs (starting with the dogfood related ones).
Hello @azure-sdk! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
Currently there are some problems with the
Az.Resources
cmdlet v5.2.0 related to fetching service principals via thegraph API in private azure clouds. This manifests either as an error like below:
Or, if the
-MicrosoftGraphEndpointResourceId
parameter is supplied to theAdd-AzEnvironment
call, then the APIrequest returns a 500:
I'm pretty sure there is still some missing functionality as support for non-public clouds was only recently added and
the AAD->MSGraph breaking change in azure powershell has had related issues.
Regardless of all this, we don't need to be doing an API call to get the OID in CI environments, because we already have
that value available and can pass it in as a parameter. So even if the above problems were fixed, I would still want to
make this change to reduce our test script running time. This update happens to bypass the problematic
Get-AzAdServicePrincipal
call and fix the issue at hand.Fixes Azure/azure-sdk-for-js#19886