Skip to content

Add support of policy exemption resource type#14474

Merged
wyunchi-ms merged 10 commits intoAzure:masterfrom
robga:robga/exemption
Mar 13, 2021
Merged

Add support of policy exemption resource type#14474
wyunchi-ms merged 10 commits intoAzure:masterfrom
robga:robga/exemption

Conversation

@robga
Copy link
Copy Markdown
Member

@robga robga commented Mar 8, 2021

Description

  1. Add support of policy exemption resource type

Checklist

  • I have read the Submitting Changes section of CONTRIBUTING.md
  • The title of the PR is clear and informative
  • The appropriate ChangeLog.md file(s) has been updated:
    • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
    • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
  • The PR does not introduce breaking changes
  • If applicable, the changes made in the PR have proper test coverage
  • For public API changes to cmdlets:
    • a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
      • {Please put the link here}
    • the markdown help files have been regenerated using the commands listed here

@robga robga requested review from DCtheGeek and mentat9 March 8, 2021 16:46
DCtheGeek
DCtheGeek reviewed Mar 8, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@DCtheGeek DCtheGeek left a comment

Choose a reason for hiding this comment

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

Minor language comments. I think we need to work on being consistent in regards to "policy set (initiative)" in all of the texts.

@mentat9
Copy link
Copy Markdown
Member

mentat9 commented Mar 9, 2021

except for:

Would also be good to add tests here to validate combinations of parameter values for the new cmdlets. These should break if parameter sets or parameters get changed in a breaking way.

Refers to: src/Resources/Resources.Test/ScenarioTests/PolicyTests.ps1:1252 in 6ed9241. [](commit_id = 6ed9241, deletion_comment = False)

@mentat9
Copy link
Copy Markdown
Member

mentat9 commented Mar 9, 2021 •
edited
Loading

function Test-PolicyObjectPiping

Would be good to add tests for piping an assignment into New-AzPolicyExemption and any piping scenarios for Set-AzPolicyExemption that aren't already tested below (maybe modifying metadata, etc.).

Refers to: src/Resources/Resources.Test/ScenarioTests/PolicyTests.ps1:1127 in 6ed9241. [](commit_id = 6ed9241, deletion_comment = False)

mentat9
mentat9 suggested changes Mar 9, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@mentat9 mentat9 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 overall, left some comments.

@BethanyZhou
Copy link
Copy Markdown
Contributor

BethanyZhou commented Mar 10, 2021

Hi @robga , please follow help generation guide to add help markdown files for Get-AzPolicyExemption, New-AzPolicyExemption, Remove-AzPolicyExemption and Set-AzPolicyExemption.

@BethanyZhou BethanyZhou self-assigned this Mar 10, 2021
robga
robga commented Mar 10, 2021
View reviewed changes
Copy link
Copy Markdown
Member Author

@robga robga left a comment

Choose a reason for hiding this comment

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

Minor language comments. I think we need to work on being consistent in regards to "policy set (initiative)" in all of the texts.

Updated all references in the PolicyHelpStrings file at least.

@robga
Copy link
Copy Markdown
Member Author

robga commented Mar 10, 2021

function Test-PolicyObjectPiping

Would be good to add tests for piping an assignment into New-AzPolicyExemption and any piping scenarios for Set-AzPolicyExemption that aren't already tested below (maybe modifying metadata, etc.).

Refers to: src/Resources/Resources.Test/ScenarioTests/PolicyTests.ps1:1127 in 6ed9241. [](commit_id = 6ed9241, deletion_comment = False)

I wrote some pipeline test in Test-PolicyExemptionCRUDOnPolicySet. The pattern in the three tests I added are similar. So, I mixed pipeline test into them.

@robga
Copy link
Copy Markdown
Member Author

robga commented Mar 10, 2021

except for:

Would also be good to add tests here to validate combinations of parameter values for the new cmdlets. These should break if parameter sets or parameters get changed in a breaking way.

Refers to: src/Resources/Resources.Test/ScenarioTests/PolicyTests.ps1:1252 in 6ed9241. [](commit_id = 6ed9241, deletion_comment = False)

Added parameter validation tests

@robga
Copy link
Copy Markdown
Member Author

robga commented Mar 10, 2021

Hi @robga , please follow help generation guide to add help markdown files for Get-AzPolicyExemption, New-AzPolicyExemption, Remove-AzPolicyExemption and Set-AzPolicyExemption.

Hi @BethanyZhou , I am not sure how to add ms.assetid in the help doc. I left it empty.

mentat9
mentat9 previously approved these changes Mar 10, 2021
View reviewed changes
BethanyZhou
BethanyZhou reviewed Mar 11, 2021
View reviewed changes
BethanyZhou
BethanyZhou reviewed Mar 11, 2021
View reviewed changes
BethanyZhou
BethanyZhou reviewed Mar 11, 2021
View reviewed changes
BethanyZhou
BethanyZhou reviewed Mar 11, 2021
View reviewed changes
BethanyZhou
BethanyZhou reviewed Mar 11, 2021
View reviewed changes
wyunchi-ms
wyunchi-ms reviewed Mar 11, 2021
View reviewed changes
@robga robga requested review from BethanyZhou and wyunchi-ms March 11, 2021 19:29
@wyunchi-ms wyunchi-ms merged commit cb97af9 into Azure:master Mar 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wyunchi-ms wyunchi-ms wyunchi-ms approved these changes

@BethanyZhou BethanyZhou Awaiting requested review from BethanyZhou

+2 more reviewers

@DCtheGeek DCtheGeek DCtheGeek left review comments

@mentat9 mentat9 mentat9 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@BethanyZhou BethanyZhou

Labels

needs-revision

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@robga @mentat9 @BethanyZhou @DCtheGeek @wyunchi-ms