Skip to content

Commit

Permalink
Use MSI for CI instead of service principal
Browse files Browse the repository at this point in the history
- Updated cipool BICEP to provision a managed identity and give it the
  required roles.
- Updated Taskfile to support MSI-based az login when running in the
  context of a GitHub action.
- Removed all reference to AZURE_CLIENT_ID and AZURE_CLIENT_SECRET from
  GitHub workflows.
  • Loading branch information
matthchr committed May 9, 2024
1 parent f1ae2f0 commit 851f884
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 38 deletions.
4 changes: 1 addition & 3 deletions .github/workflows/create-release-experimental.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,9 @@ jobs:
- name: Docker login
run: |
container_id=${{env.container_id}}
docker exec -e AZURE_CLIENT_ID -e AZURE_CLIENT_SECRET -e DOCKER_REGISTRY "$container_id" task VERSION=${{ env.ARTIFACT_VERSION }} LATEST_VERSION_TAG=${{ env.ARTIFACT_VERSION }} docker-login
docker exec -e DOCKER_REGISTRY "$container_id" task VERSION=${{ env.ARTIFACT_VERSION }} LATEST_VERSION_TAG=${{ env.ARTIFACT_VERSION }} docker-login
env:
DOCKER_REGISTRY: ${{ secrets.REGISTRY_LOGIN }}
AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }}

- name: Build, tag and push docker image
run: |
Expand Down
8 changes: 2 additions & 6 deletions .github/workflows/create-release-official.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,9 @@ jobs:
- name: Docker login
run: |
container_id=${{env.container_id}}
docker exec -e AZURE_CLIENT_ID -e AZURE_CLIENT_SECRET -e DOCKER_REGISTRY "$container_id" task docker-login
docker exec -e DOCKER_REGISTRY "$container_id" task docker-login
env:
DOCKER_REGISTRY: ${{ secrets.REGISTRY_LOGIN }}
AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }}

- name: Build, tag and push docker image
run: |
Expand All @@ -68,11 +66,9 @@ jobs:
- name: Protect image
run: |
container_id=${{env.container_id}}
docker exec -e DOCKER_PUSH_TARGET -e DOCKER_REGISTRY -e AZURE_TENANT_ID -e AZURE_CLIENT_ID -e AZURE_CLIENT_SECRET -e AZURE_SUBSCRIPTION_ID "$container_id" task controller:acr-protect-image
docker exec -e GITHUB_ACTIONS -e DOCKER_PUSH_TARGET -e DOCKER_REGISTRY -e AZURE_TENANT_ID -e AZURE_SUBSCRIPTION_ID "$container_id" task controller:acr-protect-image
env:
DOCKER_PUSH_TARGET: ${{ secrets.REGISTRY_PUBLIC }}
DOCKER_REGISTRY: ${{ secrets.REGISTRY_LOGIN }}
AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }}
AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }}
AZURE_SUBSCRIPTION_ID: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
8 changes: 2 additions & 6 deletions .github/workflows/live-validation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,17 @@ jobs:
echo "container_id=$container_id" >> $GITHUB_ENV
- name: Run CI tasks against live resources
run: docker exec --env HOSTROOT=$GITHUB_WORKSPACE -e AZURE_TENANT_ID -e AZURE_CLIENT_ID -e AZURE_CLIENT_SECRET -e AZURE_SUBSCRIPTION_ID "${{env.container_id }}" task ci-live
run: docker exec --env HOSTROOT=$GITHUB_WORKSPACE -e GITHUB_ACTIONS -e AZURE_TENANT_ID -e AZURE_SUBSCRIPTION_ID "${{env.container_id }}" task ci-live
env:
AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }}
AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }}
AZURE_SUBSCRIPTION_ID: ${{ secrets.AZURE_SUBSCRIPTION_ID }}

# We explicitly do not upload coverage for live tests as it messes with the diffs for PRs,
# since they will not exercize the authorization codepaths.

- name: Cleanup test resources
if: always()
run: docker exec -e AZURE_TENANT_ID -e AZURE_CLIENT_ID -e AZURE_CLIENT_SECRET -e AZURE_SUBSCRIPTION_ID "${{ env.container_id }}" task cleanup-azure-resources
run: docker exec -e GITHUB_ACTIONS -e AZURE_TENANT_ID -e AZURE_SUBSCRIPTION_ID "${{ env.container_id }}" task cleanup-azure-resources
env:
AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }}
AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }}
AZURE_SUBSCRIPTION_ID: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
4 changes: 1 addition & 3 deletions .github/workflows/pr-validation-fork.yml
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,9 @@ jobs:
- name: Run integration tests
run: |
container_id=${{ env.container_id }}
docker exec --env HOSTROOT=$GITHUB_WORKSPACE -e AZURE_TENANT_ID -e AZURE_CLIENT_ID -e AZURE_CLIENT_SECRET -e AZURE_SUBSCRIPTION_ID "$container_id" task controller:ci-integration-tests
docker exec --env HOSTROOT=$GITHUB_WORKSPACE -e GITHUB_ACTIONS -e AZURE_TENANT_ID -e AZURE_SUBSCRIPTION_ID "$container_id" task controller:ci-integration-tests
env:
AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }}
AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }}
AZURE_SUBSCRIPTION_ID: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
if: steps.check-changes.outputs.code-changed == 'true'

Expand Down
4 changes: 1 addition & 3 deletions .github/workflows/pr-validation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -236,11 +236,9 @@ jobs:
- name: Run integration tests
run: |
container_id=${{ env.container_id }}
docker exec --env HOSTROOT=$GITHUB_WORKSPACE -e AZURE_TENANT_ID -e AZURE_CLIENT_ID -e AZURE_CLIENT_SECRET -e AZURE_SUBSCRIPTION_ID "$container_id" task controller:ci-integration-tests
docker exec --env HOSTROOT=$GITHUB_WORKSPACE -e GITHUB_ACTIONS -e AZURE_TENANT_ID -e AZURE_SUBSCRIPTION_ID "$container_id" task controller:ci-integration-tests
env:
AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }}
AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }}
AZURE_SUBSCRIPTION_ID: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
if:
steps.check-changes.outputs.code-changed == 'true'
Expand Down
8 changes: 2 additions & 6 deletions .github/workflows/pre-release-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,14 @@ jobs:
echo "container_id=$container_id" >> $GITHUB_ENV
- name: Run Pre Release Tests
run: docker exec -e AZURE_TENANT_ID -e AZURE_CLIENT_ID -e AZURE_CLIENT_SECRET -e AZURE_SUBSCRIPTION_ID "${{ env.container_id }}" task controller:test-upgrade
run: docker exec -e GITHUB_ACTIONS -e AZURE_TENANT_ID -e AZURE_SUBSCRIPTION_ID "${{ env.container_id }}" task controller:test-upgrade
env:
AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }}
AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }}
AZURE_SUBSCRIPTION_ID: ${{ secrets.AZURE_SUBSCRIPTION_ID }}

- name: Cleanup test resources
if: always()
run: docker exec -e AZURE_TENANT_ID -e AZURE_CLIENT_ID -e AZURE_CLIENT_SECRET -e AZURE_SUBSCRIPTION_ID "${{ env.container_id }}" task cleanup-azure-resources
run: docker exec -e GITHUB_ACTIONS -e AZURE_TENANT_ID -e AZURE_SUBSCRIPTION_ID "${{ env.container_id }}" task cleanup-azure-resources
env:
AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }}
AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }}
AZURE_SUBSCRIPTION_ID: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
17 changes: 9 additions & 8 deletions Taskfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1147,20 +1147,21 @@ tasks:
- '{{.SCRIPTS_ROOT}}/delete-old-resourcegroups.sh -p "{{.TEST_LIVE_RESOURCE_PREFIX}}" -a 10800'

az-login:
desc: Runs AZ login
cmds:
# az cli has 2 tokens, an access token (valid for ~60m) and a refres token (valid for ~12h)
# If the access token is expired, but the refresh token is still valid, the below command will get a new access token via the refresh token without asking for a re-auth
# If the refresh token is expired, re-auth is required.
# TODO: It's likely that this will not work out of the box in the CI env?
- if ! az account get-access-token --tenant {{.AZURE_TENANT_ID}} --query "expiresOn" --output tsv > /dev/null; then az login; fi
desc: Runs AZ login. In the context of GitHub actions, uses an MSI token, in a local (non-actions) context, uses an interactive login
cmds:
# For interactive login: az cli has 2 tokens, an access token (valid for ~60m) and a refresh token (valid for ~12h)
# If the access token is expired, but the refresh token is still valid, the below command will get a new access token
# via the refresh token without asking for re-authentication.
# If the refresh token is expired, re-authentication is required.
# In practice this means that you should be asked for authentication once a (working) day, unless you're working for more
# than 12 hours in which case you'll be prompted twice.
- if [ "${GITHUB_ACTIONS}" = true ]; then az login --identity; elif ! az account get-access-token --tenant {{.AZURE_TENANT_ID}} --query "expiresOn" --output tsv > /dev/null; then az login; fi
- az account set --subscription {{.AZURE_SUBSCRIPTION_ID}}

docker-login:
desc: Docker login
cmds:
- 'if [ -z "{{.DOCKER_REGISTRY}}" ]; then echo "Error: DOCKER_REGISTRY must be set"; exit 1; fi'
# - docker login {{.DOCKER_REGISTRY}} --username {{.AZURE_CLIENT_ID}} --password {{.AZURE_CLIENT_SECRET}} # TODO: This needs to be fixed so that it works with a non-SP CI, I think by using az acr login instead
- az acr login --name {{.DOCKER_REGISTRY}}

header-check:
Expand Down
38 changes: 35 additions & 3 deletions scripts/v2/cipool/deploy.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ var sku = 'Standard_D8ds_v4'
// Base 1ES Image Resource IDs
var ubuntu2204GalleryVersionResourceId = '/subscriptions/723b64f0-884d-4994-b6de-8960d049cb7e/resourceGroups/CloudTestImages/providers/Microsoft.Compute/galleries/CloudTestGallery/images/MMSUbuntu22.04-Secure/versions/latest'


var poolSettings = {
maxPoolSize: 4 // We run 2 jobs per CI run on this pool, so a max of 4 means we can run 2 CI builds in parallel
maxPoolSize: 8 // We run 2 jobs per CI run on this pool, so a max of 8 means we can run 4 CI builds in parallel
resourcePredictions: [
{
'21:00': 2 // 9 AM Monday NZT
Expand Down Expand Up @@ -39,6 +38,13 @@ var poolSettings = {
]
}

var msiName = 'aso-ci-identity'

resource msi 'Microsoft.ManagedIdentity/userAssignedIdentities@2023-01-31' = {
name: msiName
location: location
}

resource agentImage 'Microsoft.CloudTest/images@2020-05-07' = {
name: '1es-ubuntu-22.04'
location: location
Expand All @@ -48,7 +54,6 @@ resource agentImage 'Microsoft.CloudTest/images@2020-05-07' = {
}
}


resource hostedPool 'Microsoft.CloudTest/hostedpools@2020-05-07' = {
name: poolName
location: location
Expand All @@ -74,5 +79,32 @@ resource hostedPool 'Microsoft.CloudTest/hostedpools@2020-05-07' = {
type: 'Stateless'
resourcePredictions: poolSettings.resourcePredictions
}
networkProfile: {
firewallProfile: {
policyName: 'Default' // allows access to everything apart from a list of known malicious endpoints
}
}
}
identity: {
type: 'UserAssigned'
userAssignedIdentities: {
'${msi.id}': {}
}
}
}

var roleDefinitionId = '8e3af657-a8ff-443c-a75c-2fe8c4bcb635' // Owner

// This has to be in a module due to the following error:
// Error BCP120: This expression is being used in an assignment to the "name" property of the "Microsoft.Authorization/roleAssignments"
// type, which requires a value that can be calculated at the start of the deployment. Properties of msi which can be
// calculated at the start include "apiVersion", "id", "name", "type".
module roleAssignment 'identity.bicep' = {
name: '${deployment().name}-identity'
scope: subscription()
params: {
principalId: msi.properties.principalId
principalType: 'ServicePrincipal'
roleDefinitionId: roleDefinitionId
}
}
29 changes: 29 additions & 0 deletions scripts/v2/cipool/identity.bicep
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// ======
// Params
// ======
@description('The principalId of the identity')
param principalId string

@description('The principal type of the identity')
param principalType string

@description('The roleDefinitionId of the role to grant')
param roleDefinitionId string


// ==========
// Deployment
// ==========

targetScope = 'subscription' // We're scoping this assignment to the subscription

// Create the role assignment
var roleAssignmentName = guid(subscription().id, principalId, roleDefinitionId)
resource roleAssignment 'Microsoft.Authorization/roleAssignments@2022-04-01' = {
name: roleAssignmentName
properties: {
roleDefinitionId: resourceId('Microsoft.Authorization/roleDefinitions', roleDefinitionId)
principalType: principalType
principalId: principalId
}
}

0 comments on commit 851f884

Please sign in to comment.