-
Notifications
You must be signed in to change notification settings - Fork 184
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
Add template to merge multiple subscription configurations #1560
Conversation
The following pipelines have been queued for testing: |
The following pipelines have been queued for testing: |
The following pipelines have been queued for testing: |
The following pipelines have been queued for testing: |
Passing test with normal Test using new config merging: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=847902&view=results |
'@ | ConvertFrom-Json -AsHashtable | ||
|
||
foreach ($pair in $addToConfig.GetEnumerator()) { | ||
if ($pair.Value -is [Hashtable]) { |
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.
This assumes only one level of nesting is that safe to assume?
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.
Technically, someone could update a config they manage to contain more levels of nesting, but it would be ignored by all our scripts anyway, since we explicitly only look for EnvironmentVariablers
and ArmTemplateParameters
.
It may be helpful to publish a json schema somewhere for authors of subscription configs. I'm planning on writing a big doc next week to map out all aspects of configuring multi-cloud tests, so I'll include some config schema guidance as well.
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.
Left a couple comments but otherwise looks reasonable. I assume you will integrate this into use as part of another PR as I don't see those changes here.
@weshaggard yep, usage of this goes into the
As well as changing references to |
Co-authored-by: Wes Haggard <[email protected]>
The following pipelines have been queued for testing: |
The following pipelines have been queued for testing: |
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 (
|
/check-enforcer override |
This PR adds a new step that will merge multiple subscription configuration objects (initially sourced as json from keyvault via devops variable groups), and output a single object as a devops variable. This will allow service owners to manage additional subscription configurations independent of the ones we maintain. For example, the communication service has the need for their own config to store strings that vary per-cloud (e.g. static testing phone number assets and connection strings), but that also must be combined with our base configs. Right now we have to keep a separate config that, since it contains our testing principal, only the azure sdk engsys team has access to. Making it easier to maintain multiple configurations will reduce our config duplication, and also lay groundwork for keeping access credential configs separate from cloud configs for environment variables and arm template parameters.
The usage in the live.tests.yml files changes slightly to:
The usage for a service owner tests.yml file looks like below, showing an example of both single and multiple subscription configurations for a cloud: