ManagementGroup: Run WhatIf test on e2e after deployment #250

olljanat · 2022-05-26T12:35:11Z

Overview/Summary

Run WhatIf deployment for management group on e2e tests. Also added new optional parameter parTopLevelManagementGroupParentId which was needed for it.

This PR fixes/adds/changes/removes

First step to solve 💡 Feature Request - Make all modules compatible with WhatIf mode #249

Breaking Changes

N/A

Testing Evidence

Without parTopLevelManagementGroupParentId parameter new WhatIf test step failed to error and it that it passed without issues.

Note: The result may contain false positive predictions (noise).
You can help us improve the accuracy of the result by opening an issue here: https://aka.ms/WhatIfIssues

Resource and property changes are indicated with these symbols:
  - Delete
  ~ Modify

The deployment will update the following scope:

Scope: /

  ~ Microsoft.Management/ManagementGroups/PR-test [2021-04-01]
    - properties.details:

        parent.id: "/providers/Microsoft.Management/managementGroups/13a69a3b-cf5f-4204-b274-3e9ce5240a60"


Resource changes: 1 to modify.

As part of this Pull Request I have

Read the Contribution Guide and ensured this PR is compliant with the guide
Checked for duplicate Pull Requests
Associated it with relevant GitHub Issues
(ALZ Bicep Core Team Only) Associated it with relevant ADO Items
Ensured my code/branch is up-to-date with the latest changes in the main branch
Performed testing and provided evidence.
Updated tests (if required) Unit - Linting - E2E (End-To-End)
Updated relevant and associated documentation (e.g. Contribution Guide, Module READMEs, Wiki Docs etc.)
If relevant, created or updated Code Tours here

jtracey93 · 2022-05-26T15:57:02Z

Thanks @olljanat for this work. As mentioned yesterday (in another discussion IIRC) we are not merging any other changes until #227 is complete and merged as this is a big PR and we need to close it out first before we keep making more merge conflicts we need to resolve.

Can I propose we put this on hold until #227 is merged (hopefully next week) and then you can update this PR with the latest from the upstream (this repo) and then we can review and take it from there?

Let me know if all okay with that 👍

olljanat · 2022-05-26T16:19:33Z

@jtracey93 that's fine. I added my self to #227 notification list and will rebase my PRs after it is merged.

olljanat · 2022-06-14T14:50:30Z

@jtracey93 rebased with main

jtracey93

Could you update the "all" parameters file with this new input please?

@jfaurskov can I also get your input here as I know you are re working the testing already.

jfaurskov · 2022-06-16T12:28:05Z

/azp run ValidateAzCloud

azure-pipelines · 2022-06-16T12:28:10Z

No pipelines are associated with this pull request.

jfaurskov · 2022-06-16T12:41:53Z

/azp run ValidateAzCloud

azure-pipelines · 2022-06-16T12:42:00Z

Azure Pipelines failed to run 1 pipeline(s).

jfaurskov · 2022-06-16T13:24:05Z

/azp run ValidateAzCloud

azure-pipelines · 2022-06-16T13:24:13Z

Azure Pipelines successfully started running 1 pipeline(s).

olljanat · 2022-06-17T08:17:45Z

@jtracey93 updated "all" parameters file and rebased with main.

jtracey93 · 2022-06-17T09:50:59Z

/azp run validateazcloud

azure-pipelines · 2022-06-17T09:51:08Z

Azure Pipelines successfully started running 1 pipeline(s).

jtracey93

@olljanat a few more bits.

Also requesting @jfaurskov to review this PR too

tests/pipelines/bicep-build-to-validate.yml

infra-as-code/bicep/modules/managementGroups/managementGroups.bicep

jfaurskov

Looking good to me. We should expand on this going forward as we split validation and deployment tests up into individual chunks.

jtracey93 · 2022-06-17T11:04:30Z

Looking good to me. We should expand on this going forward as we split validation and deployment tests up into individual chunks.

Agreed and think these should be included on the deployment phase as needs to run again post deployment as a check that the what-if doesn't kick off saying there's a change, when there clearly wouldn't be 👍

infra-as-code/bicep/modules/managementGroups/managementGroups.bicep

olljanat · 2022-06-20T08:56:11Z

@jtracey93 looks that your idea to use tenant().tenantId handles the WhatIf case nicely so there is no even need to have separate parameter at all \o/

jtracey93 · 2022-06-20T09:19:57Z

@jtracey93 looks that your idea to use tenant().tenantId handles the WhatIf case nicely so there is no even need to have separate parameter at all \o/

Hey @olljanat I actually think having this as an input parameter is a nice idea as it enables some further use cases for nested ALZ deployments in some more complex scenarios 👍

olljanat · 2022-06-20T13:50:39Z

@jtracey93 OK. Added it back but updated description because it is not really needed by WhatIf anymore (and not used in e2e because of that).

jtracey93 · 2022-06-20T13:57:39Z

/azp run validateazcloud

azure-pipelines · 2022-06-20T13:57:49Z

Azure Pipelines successfully started running 1 pipeline(s).

jtracey93

Couple of changes to make it more robust for all users @olljanat (thanks for your patience)

jtracey93 · 2022-06-20T14:20:00Z

infra-as-code/bicep/modules/managementGroups/managementGroups.bicep

@@ -69,6 +72,11 @@ resource resTopLevelMg 'Microsoft.Management/managementGroups@2021-04-01' = {
  name: parTopLevelManagementGroupPrefix
  properties: {
    displayName: parTopLevelManagementGroupDisplayName
+    details: {
+      parent: {
+        id: parTopLevelManagementGroupParentId


Can we add the logic back here that if an empty string is provided that we pass in /providers/Microsoft.Management/managementGroups/${tenant().tenantId}?

If any other string is provided im happy for the error, like our test just picked up 😄

ERROR: {"status":"Failed","error":{"code":"DeploymentFailed","message":"At least one resource deployment operation failed. Please list deployment operations for details. Please see https://aka.ms/DeployOperations for usage details.","details":[{"code":"BadRequest","message":"{\r\n \"error\": {\r\n \"code\": \"BadRequest\",\r\n \"message\": \"Invalid management group creation parameters: expected 'parentId' format: '/providers/Microsoft.Management/managementGroups/ID'\",\r\n \"details\": null\r\n }\r\n}"}]}}

If you can make the change, then we can get this one merged 👍

jtracey93 · 2022-06-20T14:22:48Z

infra-as-code/bicep/modules/managementGroups/README.md

@@ -20,6 +20,7 @@ The module requires the following inputs:

 | Parameter                             | Type   | Description                                                                                                                                                     | Requirements                      | Example               |
 | ------------------------------------- | ------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------- | --------------------- |
+| parTopLevelManagementGroupParentId    | string | Optional Tenant Root Group ID. Used as top level management group parent.                                                          |                                   |                       |


Can we add this to the example column for this please to help show what is expected: /providers/Microsoft.Management/managementGroups/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx

Thanks 👍

ghost · 2022-06-27T15:01:14Z

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

jtracey93 · 2022-07-04T08:07:51Z

Closing this as have wrapped into #276

ghost added the Needs: Triage 🔍 Needs triaging by the team label May 26, 2022

olljanat mentioned this pull request May 26, 2022

💡 Feature Request - Make all modules compatible with WhatIf mode #249

Open

jtracey93 added the do not merge label May 26, 2022

jtracey93 added external community contribution and removed Needs: Triage 🔍 Needs triaging by the team labels May 26, 2022

olljanat mentioned this pull request Jun 8, 2022

Module Bicep template and parameter file hygiene #227

Merged

9 tasks

olljanat force-pushed the msg-whatif-test branch from 8ca6ad3 to 3e52b26 Compare June 14, 2022 14:49

jtracey93 requested changes Jun 14, 2022

View reviewed changes

ghost added the Needs: Author Feedback label Jun 14, 2022

olljanat force-pushed the msg-whatif-test branch from 3e52b26 to 3785296 Compare June 17, 2022 08:16

ghost removed the Needs: Author Feedback label Jun 17, 2022

jtracey93 requested changes Jun 17, 2022

View reviewed changes

tests/pipelines/bicep-build-to-validate.yml Outdated Show resolved Hide resolved

tests/pipelines/bicep-build-to-validate.yml Outdated Show resolved Hide resolved

infra-as-code/bicep/modules/managementGroups/managementGroups.bicep Outdated Show resolved Hide resolved

ghost added the Needs: Author Feedback label Jun 17, 2022

jfaurskov self-requested a review June 17, 2022 10:53

jfaurskov reviewed Jun 17, 2022

View reviewed changes

ghost removed the Needs: Author Feedback label Jun 17, 2022

jtracey93 reviewed Jun 17, 2022

View reviewed changes

infra-as-code/bicep/modules/managementGroups/managementGroups.bicep Outdated Show resolved Hide resolved

jtracey93 reviewed Jun 17, 2022

View reviewed changes

infra-as-code/bicep/modules/managementGroups/managementGroups.bicep Outdated Show resolved Hide resolved

olljanat force-pushed the msg-whatif-test branch from d3cb169 to d1e6a23 Compare June 20, 2022 08:51

ManagementGroup: Add after deployment WhatIf test to e2e

9743ede

olljanat force-pushed the msg-whatif-test branch from d1e6a23 to 9743ede Compare June 20, 2022 13:49

jtracey93 requested changes Jun 20, 2022

View reviewed changes

ghost added the Needs: Author Feedback label Jun 20, 2022

ghost added the Status: No Recent Activity label Jun 27, 2022

jtracey93 closed this Jul 4, 2022

ghost removed the Status: No Recent Activity label Jul 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ManagementGroup: Run WhatIf test on e2e after deployment #250

ManagementGroup: Run WhatIf test on e2e after deployment #250

olljanat commented May 26, 2022

jtracey93 commented May 26, 2022

olljanat commented May 26, 2022

olljanat commented Jun 14, 2022

jtracey93 left a comment

jfaurskov commented Jun 16, 2022

azure-pipelines bot commented Jun 16, 2022

jfaurskov commented Jun 16, 2022

azure-pipelines bot commented Jun 16, 2022

jfaurskov commented Jun 16, 2022

azure-pipelines bot commented Jun 16, 2022

olljanat commented Jun 17, 2022

jtracey93 commented Jun 17, 2022

azure-pipelines bot commented Jun 17, 2022

jtracey93 left a comment

jfaurskov left a comment

jtracey93 commented Jun 17, 2022

olljanat commented Jun 20, 2022

jtracey93 commented Jun 20, 2022

olljanat commented Jun 20, 2022

jtracey93 commented Jun 20, 2022

azure-pipelines bot commented Jun 20, 2022

jtracey93 left a comment •

edited

Loading

jtracey93 Jun 20, 2022

jtracey93 Jun 20, 2022

ghost commented Jun 27, 2022

jtracey93 commented Jul 4, 2022

ManagementGroup: Run WhatIf test on e2e after deployment #250

ManagementGroup: Run WhatIf test on e2e after deployment #250

Conversation

olljanat commented May 26, 2022

Overview/Summary

This PR fixes/adds/changes/removes

Breaking Changes

Testing Evidence

As part of this Pull Request I have

jtracey93 commented May 26, 2022

olljanat commented May 26, 2022

olljanat commented Jun 14, 2022

jtracey93 left a comment

Choose a reason for hiding this comment

jfaurskov commented Jun 16, 2022

azure-pipelines bot commented Jun 16, 2022

jfaurskov commented Jun 16, 2022

azure-pipelines bot commented Jun 16, 2022

jfaurskov commented Jun 16, 2022

azure-pipelines bot commented Jun 16, 2022

olljanat commented Jun 17, 2022

jtracey93 commented Jun 17, 2022

azure-pipelines bot commented Jun 17, 2022

jtracey93 left a comment

Choose a reason for hiding this comment

jfaurskov left a comment

Choose a reason for hiding this comment

jtracey93 commented Jun 17, 2022

olljanat commented Jun 20, 2022

jtracey93 commented Jun 20, 2022

olljanat commented Jun 20, 2022

jtracey93 commented Jun 20, 2022

azure-pipelines bot commented Jun 20, 2022

jtracey93 left a comment • edited Loading

Choose a reason for hiding this comment

jtracey93 Jun 20, 2022

Choose a reason for hiding this comment

jtracey93 Jun 20, 2022

Choose a reason for hiding this comment

ghost commented Jun 27, 2022

jtracey93 commented Jul 4, 2022

jtracey93 left a comment •

edited

Loading