Skip to content

Adding support for filtering whatif output#482

Merged
daltondhcp merged 34 commits intoAzure:mainfrom
vancopayments:main
Dec 27, 2021
Merged

Adding support for filtering whatif output#482
daltondhcp merged 34 commits intoAzure:mainfrom
vancopayments:main

Conversation

@jasonbrisbin
Copy link
Copy Markdown
Contributor

@jasonbrisbin jasonbrisbin commented Nov 18, 2021

Overview/Summary

Adding support for filtering whatif results using the ExcludeChangeType parameter for the Get-Az****DeploymentWhatIfResult cmdlets. This is in response to issue 475 that I opened. I propose using a PSF config value for controlling this i.e. AzOps.Core.WhatifExcludedChangeTypes which would be an array containing the list of change types to exclude. https://docs.microsoft.com/en-us/azure/azure-resource-manager/templates/deploy-what-if?tabs=azure-powershell#change-types

Also I am suggesting a slight change to the formatting of output.json such that the output will be a valid JSON document when there is more than 1 template deployed with changes. Currently the file has a simple text append function which when processing multiple templates in a single run will produce a ill formatted document.

Before

{
  "results":"first template"
}
{
  "results":"second template"
}

After

[
  {
    "results":"first template"
  },
  {
    "results":"second template"
  }
]

Replace this with a brief description of what this Pull Request fixes, changes, etc.

This PR fixes/adds/changes/removes

  1. Whatif exclusions #475

Breaking Changes

  1. None

Testing Evidence

Please provide any testing evidence to show that your Pull Request works/fixes as described and planned (include screenshots, if appropriate).

As part of this Pull Request I have

  • Checked for duplicate Pull Requests
  • Associated it with relevant issues, for tracking and closure.
  • Ensured my code/branch is up-to-date with the latest changes in the main branch
  • Performed testing and provided evidence.
  • Updated relevant and associated documentation.
  • Updated the "What's New?" wiki page (located in the Enterprise-Scale repo in the directory: /docs/wiki/whats-new.md)

Settings.json with whatif exclusions for NoChange and Ignore
image

Post change sample output with obfuscated names/guids. This is the output.json, note the Change Type field and the top level array format of the document.
sample.txt

…pe parameter for the Get-Az****DeploymentWhatIfResult cmdlets. This is in response to issue 475 that I opened. I propose using a PSF config value for controlling this i.e. AzOps.Core.WhatifExcludedChangeTypes which would be an array containing the list of change types to exclude. https://docs.microsoft.com/en-us/azure/azure-resource-manager/templates/deploy-what-if?tabs=azure-powershell#change-types

Also I am suggesting a slight change to the formatting of output.json such that the output will be a valid JSON document when there is more than 1 template deployed with changes. Currently the file has a simple text append function which when processing multiple templates in a single run will produce a ill formatted document.
@jasonbrisbin jasonbrisbin requested a review from a team as a code owner November 18, 2021 21:26
@daltondhcp daltondhcp added the do-not-merge Not ready to merge label Nov 19, 2021
Copy link
Copy Markdown
Contributor

@daltondhcp daltondhcp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution @jasonbrisbin.

Based on what is currently in the PR, it doesn't look like you have touched the MD file at all (which is what is added to the PR comment). https://github.com/Azure/AzOps-Accelerator/blob/main/.github/workflows/validate.yml#L115
It also looks like we potentially have bit of a clash with #470 that was merged yesterday

Anyway, to move forward with this, can you please do the following:

  1. Share examples (screenshots) of what the output would look like so we understand how it helps to highlight what has been changed and reduce noise.
  2. Validate the end-to-end flow in a pipeline/GitHub actions as per https://github.com/azure/azops/wiki/pre-release.
  3. Update https://github.com/Azure/AzOps/blob/main/src/internal/configurations/Core.ps1 and https://github.com/Azure/AzOps-Accelerator/blob/main/settings.json with the new setting/suggested default values.
  4. Update https://github.com/azure/azops/wiki/settings so the we have the new setting documented

@jasonbrisbin
Copy link
Copy Markdown
Contributor Author

jasonbrisbin commented Nov 19, 2021

@Jefajers I reran today with the published changes from 470 and I really like the output it does a good job cleaning up the output and making it more human readable.

image

@daltondhcp That said, it would be ideal (for me) if the json file produced output was valid for multiple deploys. Here is a sanitized output.json from my most recent run. Notice that there are multiple independent JSON objects at the top level and no commas between them (lines 61/62,122/123,183/184). My update adds a top level array and each json object is a member of that array. I have completed steps 3 and 4 without much trouble. I do not see the push.yaml/pull.yaml files referenced in step 2, can you point me in the right direction?
OUTPUT.txt

@daltondhcp
Copy link
Copy Markdown
Contributor

daltondhcp commented Nov 20, 2021

@jasonbrisbin , so you'd want to keep the recently introduced, 'human readable' MD output, but change/enable the new feature for output.json so you can use that for other purposes in the pipeline?

2 is essentially the ability to install the module from your fork/branch. See example how I use it in my repo @ https://github.com/daltondhcp/azops-test2/blob/main/.github/workflows/pull.yml#L168 .

@jasonbrisbin
Copy link
Copy Markdown
Contributor Author

Absolutely, we want to keep the changes from #470. My update should compliment that change. I specifically was trying to accomplish the following.

  1. Ensure output.json is a well formatted document. As it stands today, I am not sure the current ouput.json is useful when dealing with more than one template being deployed.
  2. Adds support for excluding specific change types from the whatif output. The format of the output is great but it still includes all possible results. Below is an excerpt from this article

https://docs.microsoft.com/en-us/azure/azure-resource-manager/templates/deploy-what-if?tabs=azure-powershell#change-types

Change types

The what-if operation lists six different types of changes:

Create: The resource doesn't currently exist but is defined in the template. The resource will be created.

Delete: This change type only applies when using complete mode for deployment. The resource exists, but isn't defined in the template. With complete mode, the resource will be deleted. Only resources that support complete mode deletion are included in this change type.

Ignore: The resource exists, but isn't defined in the template. The resource won't be deployed or modified.

NoChange: The resource exists, and is defined in the template. The resource will be redeployed, but the properties of the resource won't change. This change type is returned when ResultFormat is set to FullResourcePayloads, which is the default value.

Modify: The resource exists, and is defined in the template. The resource will be redeployed, and the properties of the resource will change. This change type is returned when ResultFormat is set to FullResourcePayloads, which is the default value.

Deploy: The resource exists, and is defined in the template. The resource will be redeployed. The properties of the resource may or may not change. The operation returns this change type when it doesn't have enough information to determine if any properties will change. You only see this condition when ResultFormat is set to ResourceIdOnly.

I will setup my pipeline like yours to complete #2.

Thanks @daltondhcp

@daltondhcp
Copy link
Copy Markdown
Contributor

@jasonbrisbin, thank you for the detailed explanation! Looking forward to your updated PR 👍

@daltondhcp daltondhcp added the waiting-for-response Maintainers have replied and are awaiting a response from the bug/issue/feature creator label Nov 26, 2021
@daltondhcp
Copy link
Copy Markdown
Contributor

Hey @jasonbrisbin - have you had the time to take this any further?

Thanks in advance!

@jasonbrisbin
Copy link
Copy Markdown
Contributor Author

I was out last week, but will be working on it this week.

@jasonbrisbin
Copy link
Copy Markdown
Contributor Author

@daltondhcp, I copied your pull pipeline and it runs successfully as shown here

https://github.com/vancopayments/AzOps/actions/runs/1527490982

This should address item 2.

@daltondhcp
Copy link
Copy Markdown
Contributor

Sorry for the delay @jasonbrisbin - I have been on leave for almost two weeks - will take a look and get back to you around this during the week.

Copy link
Copy Markdown
Contributor

@daltondhcp daltondhcp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just some cosmetic changes needed to ensure we are aligned with rest of the code base.

Comment thread docs/wiki/Settings.md Outdated
Comment thread settings.json Outdated
Comment thread src/internal/configurations/Core.ps1 Outdated
Comment thread src/internal/functions/New-AzOpsDeployment.ps1 Outdated
Comment thread src/internal/functions/New-AzOpsDeployment.ps1 Outdated
Comment thread src/internal/functions/New-AzOpsDeployment.ps1 Outdated
Comment thread src/internal/functions/New-AzOpsDeployment.ps1 Outdated
Comment thread src/internal/functions/New-AzOpsDeployment.ps1
Comment thread src/internal/functions/New-AzOpsDeployment.ps1 Outdated
Comment thread src/internal/functions/New-AzOpsDeployment.ps1 Outdated
@jasonbrisbin
Copy link
Copy Markdown
Contributor Author

Looks good, just some cosmetic changes needed to ensure we are aligned with rest of the code base.

@daltondhcp I have made the changes you suggested.

Copy link
Copy Markdown
Contributor

@daltondhcp daltondhcp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more things, thank you for your efforts and patience! :)

Comment thread src/internal/functions/New-AzOpsDeployment.ps1 Outdated
Comment thread src/internal/functions/New-AzOpsDeployment.ps1 Outdated
Comment thread .github/CODEOWNERS
Comment thread .gitignore
Comment thread src/internal/functions/Set-AzOpsWhatIfOutput.ps1 Outdated
@daltondhcp daltondhcp merged commit 6cdd417 into Azure:main Dec 27, 2021
@daltondhcp daltondhcp linked an issue Jan 5, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge Not ready to merge waiting-for-response Maintainers have replied and are awaiting a response from the bug/issue/feature creator

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Whatif exclusions

2 participants