-
Notifications
You must be signed in to change notification settings - Fork 517
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
Feature addvalidation #539
Conversation
@microsoft-github-policy-service agree [company="Airwair International Limited"]
|
@microsoft-github-policy-service agree company="Airwair International Ltd" |
Great to have some more validation before deployment! 👍 The PR itself adds an approval gate, so maybe there is not need for the requirement to use environments in github. The pipelines should limit the number of duplicated steps, it would be simpler to define each step only one time and trigger pipeline runs on both pull request into main and push into main. Some of the steps could have if-expressions to only run in certain scenarios:
Also I had a look at the updates to the Deploy-ALZ*.ps1 scripts. Just using the -WhatIf flag only prints the pending changes to the host/console, without any way to store the result in a variable. If the result of a WhatIf should be published as text into the PR; have a look at Get-AzManagementGroupDeploymentWhatIfResult, etc. ExampleCode examples collapsede.g.
e.g on:
push:
branches: [ "main" ]
pull_request:
branches: [ "main" ]
workflow_dispatch: {}
env:
ENV_FILE: ".env"
WHATIF_TO_PR: false
permissions:
id-token: write
contents: read
pull-requests: write
...
- name: What-If
id: whatif
if: github.event_name == 'pull_request'
uses: azure/powershell@v1
with:
inlineScript: |
$whatif = .\pipeline-scripts\Deploy-ALZSubscriptionPlacement.ps1 -WhatIf $true
$whatif | Out-String
-join (1..15 | ForEach {[char]((48..57)+(65..90)+(97..122) | Get-Random)}) | Set-Variable -Name EOF
"WHATIF<<$EOF" >> $env:GITHUB_OUTPUT
$whatif >> $env:GITHUB_OUTPUT
"$EOF" >> $env:GITHUB_OUTPUT
azPSVersion: "latest"
- name: Push Whatif Output to PR
if: github.event_name == 'pull_request' && env.WHATIF_TO_PR == true
uses: actions/github-script@v6
env:
WHATIF: "${{ steps.whatif.outputs.WHATIF }}"
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
const output = `#### Deployment WhatIf 📖\`${{ steps.whatif.outcome }}\`
<details><summary>Show Plan</summary>
\`\`\`text\n
${process.env.WHATIF}
\`\`\`
</details>
*Pushed by: @${{ github.actor }}, Action: \`${{ github.event_name }}\`*`;
github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: output
})
- name: Deploy Subscription Placement
if: github.ref == 'refs/heads/main' && github.event_name == 'push'
uses: azure/powershell@v1
with:
inlineScript: |
.\pipeline-scripts\Deploy-ALZSubscriptionPlacement.ps1
azPSVersion: "latest" |
@picccard Thanks for the feedback, i was working on using the output - perhaps for the step summary. One change at a time i guess. Agree about the duplication of code and i changed my PR to minimise this. Also, the environment approval runs before the deployment job runs, straight after the validation - unlike the approval gate via the branch protection which runs when there is a PR. Try it yourself and you will see it flows better. Take your point about changing the triggers, i have similar workflows for some of our repo's. I am just trying to get validation in to the workflow with what we have at the moment and we can work on the rest next? |
Thanks @MilesCameron-DMs! I like the idea of using environments, especially if we end up incorporating the option to deploy to We were also thinking about adding the output to a comment as @picccard suggested as well. I will play around with this a bit this week and I may end up opening a PR with some potential changes as a heads up. |
@MilesCameron-DMs I just opened a PR against your branch if you wouldn't mind taking a look. I am holding off on adding on adding the outputs to comments as the approach I want to go with requires some modification of the ALZ Powershell module. |
@oZakari thanks for that. I have reviewed and merged. Much neater solution 👍 |
Thank you, @MilesCameron-DMs! I resubmitted a PR as I realized I originally requested it against your main branch and not your feature branch. |
/azp run e2e |
No commit pushedDate could be found for PR 539 in repo Azure/ALZ-Bicep |
Overview/Summary
#536
Adds validation by adding a validate job for each deploy job. Also adds dependencies for validate to run successfully before deploying.
This PR fixes/adds/changes/removes
whatif
to the deploy jobsneeds
in the GitHub workflowproduction
Breaking Changes
production
to be added to the GitHub repo.This has been added to the wiki
Testing Evidence
Limited testing as in the test environment VWAN is not deployed:
As part of this Pull Request I have
.bicep
file/s I am adding/editing are using the latest API version possiblemain
branch