-
Notifications
You must be signed in to change notification settings - Fork 456
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
[CI Environment] [MAJOR/BREAKING] Introducing OIDC and dual environment support #1608
base: main
Are you sure you want to change the base?
Conversation
* Push updated Readme file(s) * Fix environement files * Updated actions to use powershell action in gh * reset readme * restore main * Test login using OCID * Update all workflows to use Engineering environment :D * Update workflows to use OIDC * Added envs to dependency pipeline * Removing commented out code * reset global.var and settings Co-authored-by: CARMLPipelinePrincipal <[email protected]>
Unit Test Results 1 files ±0 1 suites ±0 16s ⏱️ +3s Results for commit 6393e1d. ± Comparison against base commit 27b1952. This pull request removes 45 and adds 49 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
* enable publishing in different subscription Co-authored-by: Marius Storhaug <[email protected]>
env: | ||
AZURE_CREDENTIALS: ${{ secrets.AZURE_CREDENTIALS }} | ||
AZURE_CREDENTIALS: ${{ secrets.AZURE_CREDENTIALS }} # TODO: Update this to use OIDC |
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.
Note: here is a TODO
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.
Yepp, this is deffo not ready for wider review. Think I just wanted initial thoughts on the changes. But lets discuss in the Thursday call. Don't want to doc yet until we agree on more aspects on this PR.
@@ -405,8 +405,14 @@ on: | |||
- 'network-hub-rg/Parameters/**' | |||
- '.github/workflows/network-hub.yml' | |||
|
|||
permissions: |
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.
Is this needed? Especially as 2 permissions call out the publishing of test results even though the example is a deployment pipeline
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.
The two OIDC permissions would be needed here if we want to update this example to use OIDC as well. Thoughts?
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.
Right, be we list 4 here
@@ -127,11 +127,12 @@ For _GitHub_, you have to perform the following environment-specific steps: | |||
|
|||
To use the environment's pipelines you should use the information you gathered during the [Azure setup](#1-configure-your-azure-environment) to set up the following repository secrets: | |||
|
|||
<!-- TODO: Update this to use OIDC --> |
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.
Note, TODO
@@ -157,6 +158,7 @@ To use the environment's pipelines you should use the information you gathered d | |||
|
|||
<p> | |||
|
|||
<!-- TODO: Update this to use OIDC --> |
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.
Note: TODO
@@ -258,12 +260,13 @@ The variable group `PLATFORM_VARIABLES` must be set up in Azure DevOps as descri | |||
|
|||
Based on the information you gathered in the [Azure setup](#1-configure-your-azure-environment), you must configure the following secrets in the variable group: | |||
|
|||
<!-- TODO: Update this to use OIDC --> |
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.
Note: TODO
AZURE_CREDENTIALS: ${{ secrets.AZURE_CREDENTIALS }} | ||
ARM_SUBSCRIPTION_ID: '${{ secrets.ARM_SUBSCRIPTION_ID }}' | ||
AZURE_CLIENT_ID: '${{ secrets.AZURE_CLIENT_ID }}' | ||
AZURE_SUBSCRIPTION_ID: '${{ secrets.AZURE_SUBSCRIPTION_ID }}' | ||
ARM_MGMTGROUP_ID: '${{ secrets.ARM_MGMTGROUP_ID }}' |
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.
Should we not also rename the managemet group to AZURE_MANAGEMENTGROUP_ID
?
@MariusStorhaug great work! What I don't get: When you are using Was it on purpose or is this just missing? I think we should stick to one pattern here. What do you think? |
Description
Adding support for 2 environments with separate subscriptions and Service Principals for:
ADO:
GH:
Validation
environment.Publishing
environment.Validation
environment.Update
Getting started - scenario 2
documentation.Pipeline references
Type of Change
Please delete options that are not relevant.
Checklist