Skip to content
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

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

MariusStorhaug
Copy link
Contributor

@MariusStorhaug MariusStorhaug commented Jun 26, 2022

Description

Adding support for 2 environments with separate subscriptions and Service Principals for:

  • ADO:

    • Service connection for Validation and Publishing.
      image
    • Variables for subscription IDs for validation and publishing.
      image
    • Module pipelines now deploy:
      • Deploying validation to a validation service connection.
        image
      • Publishing template specs and bicep registry using publishing service connection.
        image
        image
    • Dependency pipeline use validation service connection.
      image
    • Update publishing script to support specifying subscription when using a MG level SvcCon. (Should we support this?)
  • GH:

    • OIDC profiles for publishing and validation
      image
    • Environments and secrets for publishing and validation
      image
    • Module workflows now deploy:
      • Validation steps with Validation environment.
        image
        image
      • Publishing steps with Publishing environment.
        image
    • Dependency workflow uses Validation environment.
      image
  • Update Getting started - scenario 2 documentation.

Pipeline references

Pipeline
.Platform: Dependencies
Batch: BatchAccounts
Compute: Disks

Type of Change

Please delete options that are not relevant.

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Update to documentation

Checklist

  • I'm sure there are no other open Pull Requests for the same update/change
  • My corresponding pipelines / checks run clean and green without any errors or warnings
  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (readme)
  • I did format my code

* 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]>
@MariusStorhaug MariusStorhaug temporarily deployed to Validation June 26, 2022 13:36 Inactive
@MariusStorhaug MariusStorhaug temporarily deployed to Validation June 26, 2022 13:37 Inactive
@github-actions
Copy link

github-actions bot commented Jun 26, 2022

Unit Test Results

  1 files  ±0    1 suites  ±0   16s ⏱️ +3s
49 tests +4  49 ✔️ +4  0 💤 ±0  0 ±0 
50 runs  +5  50 ✔️ +5  0 💤 ±0  0 ±0 

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.
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.DesktopVirtualization/scalingplans] used resource type [diagnosticsettings] should use one of the recent API version(s). Currently using [2021-05-01-preview]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.DesktopVirtualization/scalingplans] used resource type [roleassignments] should use one of the recent API version(s). Currently using [2020-10-01-preview]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.DesktopVirtualization/scalingplans] used resource type [scalingPlans] should use one of the recent API version(s). Currently using [2021-09-03-preview]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.DesktopVirtualization/scalingplans] All apiVersion properties should be set to a static, hard-coded value
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.DesktopVirtualization/scalingplans] All parameters in parameters files exist in template file (deploy.json)
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.DesktopVirtualization/scalingplans] All required parameters in template file (deploy.json) should exist in parameters files
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.DesktopVirtualization/scalingplans] CUA ID deployment should be present in the template
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.DesktopVirtualization/scalingplans] Conditional parameters' description should contain 'Required if' followed by the condition making the parameter required.
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.DesktopVirtualization/scalingplans] If delete lock is implemented, the template should have a lock parameter with the default value of ['']
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.DesktopVirtualization/scalingplans] Location output should be returned for resources that use it
…
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.Batch/batchAccounts] used resource type [batchAccounts] should use one of the recent API version(s). Currently using [2022-01-01]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.Batch/batchAccounts] used resource type [diagnosticsettings] should use one of the recent API version(s). Currently using [2021-05-01-preview]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ API version tests [All apiVersions in the template should be 'recent'].In [Microsoft.Batch/batchAccounts] used resource type [locks] should use one of the recent API version(s). Currently using [2017-04-01]
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Batch/batchAccounts] All apiVersion properties should be set to a static, hard-coded value
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Batch/batchAccounts] All parameters in parameters files exist in template file (deploy.json)
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Batch/batchAccounts] All required parameters in template file (deploy.json) should exist in parameters files
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Batch/batchAccounts] CUA ID deployment should be present in the template
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Batch/batchAccounts] Conditional parameters' description should contain 'Required if' followed by the condition making the parameter required.
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Batch/batchAccounts] If delete lock is implemented, the template should have a lock parameter with the default value of ['']
/home/runner/work/ResourceModules/ResourceModules/arm/.global/global.module.tests.ps1 ‑ Deployment template tests.Deployment template tests.[Microsoft.Batch/batchAccounts] Location output should be returned for resources that use it
…

♻️ This comment has been updated with latest results.

@MariusStorhaug MariusStorhaug temporarily deployed to Validation June 26, 2022 13:38 Inactive
@MariusStorhaug MariusStorhaug temporarily deployed to Validation June 26, 2022 13:38 Inactive
@MariusStorhaug MariusStorhaug temporarily deployed to Validation June 26, 2022 13:38 Inactive
@MariusStorhaug MariusStorhaug temporarily deployed to Validation June 26, 2022 13:38 Inactive
@MariusStorhaug MariusStorhaug temporarily deployed to Publishing June 26, 2022 13:42 Inactive
@MariusStorhaug MariusStorhaug changed the title [CI Environment] [MAJOR/BREAKING] Authentication overhaul [CI Environment] [MAJOR/BREAKING] Introducing OIDC and dual environment support Jun 26, 2022
@AlexanderSehr AlexanderSehr added enhancement New feature or request [cat] pipelines category: pipelines [cat] github category: GitHub labels Jun 27, 2022
env:
AZURE_CREDENTIALS: ${{ secrets.AZURE_CREDENTIALS }}
AZURE_CREDENTIALS: ${{ secrets.AZURE_CREDENTIALS }} # TODO: Update this to use OIDC
Copy link
Contributor

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

Copy link
Contributor Author

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:
Copy link
Contributor

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

Copy link
Contributor Author

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?

Copy link
Contributor

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 -->
Copy link
Contributor

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 -->
Copy link
Contributor

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 -->
Copy link
Contributor

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 }}'
Copy link
Contributor

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 MariusStorhaug requested a review from a team as a code owner June 27, 2022 19:14
@MariusStorhaug MariusStorhaug marked this pull request as draft June 27, 2022 19:55
@SeSeicht
Copy link
Contributor

SeSeicht commented Jul 4, 2022

@MariusStorhaug great work! What I don't get: When you are using ARM_ and when Azure_ in variable names?
For example, you renamed env.ARM_SUBSCRIPTION_ID -> env.AZURE_SUBSCRIPTION_ID here
but did not change env.ARM_MGMTGROUP_ID here

Was it on purpose or is this just missing? I think we should stick to one pattern here. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[cat] github category: GitHub [cat] pipelines category: pipelines enhancement New feature or request
Projects
None yet
3 participants