Skip to content
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

Management Group Diagnostic Settings Enablement - New Module #321

Merged
merged 67 commits into from
Oct 9, 2022

Conversation

lachaves
Copy link
Contributor

@lachaves lachaves commented Sep 29, 2022

Overview/Summary

New module under orchestration folder that calls another new module to enable Diagnostic Settings for Management Groups inthe ALZ Management Group Hierarchy.

This PR fixes/adds/changes/removes

  1. Fixes 💡 Feature Request -Enable Diagnostic Settings to LAW on all created Management Groups in ALZ-Bicep #323 and ADO User Story 23555

Breaking Changes

NA

Testing Evidence

Validated with several deployments
Validated using https://learn.microsoft.com/en-us/rest/api/monitor/management-group-diagnostic-settings/get?tabs=HTTP&tryIt=true&source=docs#code-try-0

As part of this Pull Request I have

@ghost ghost added the Needs: Triage 🔍 Needs triaging by the team label Sep 29, 2022
@jtracey93 jtracey93 changed the title Mgdiagnosticto law Management Group Diagnostic Settings Enablement - New Module Sep 29, 2022
@jtracey93
Copy link
Collaborator

Hey @lachaves,

Could you please edit and fill in the PR template for this PR?

#321 (comment)

@lachaves
Copy link
Contributor Author

@jtracey93 Ready for review

Copy link
Collaborator

@jtracey93 jtracey93 left a comment

Choose a reason for hiding this comment

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

Nice work @lachaves have left a number of comments and suggestions, ping me on Teams if any questions but hopefully most of it is self-explanatory 👍

On top of the comments on the code and files here are some other bits:

  • Can you add parameters folders and files to the mgDiagSettings module directory for consistency across all modules
  • Can you add to each of the ADO Pipelines here that we use for testing: https://github.com/Azure/ALZ-Bicep/tree/main/tests/pipelines
    • Reach out to me or @jfaurskov if you have any questions around the conditional logic we use for the bicep-build-to-validate.yml or mc-base-unit-validate.yml

Thanks again and looking forward to seeing the comments adressed and then we can test and merge. 👍

Co-authored-by: Jack Tracey <41163455+jtracey93@users.noreply.github.com>
@ghost ghost removed the Needs: Author Feedback label Sep 30, 2022
lachaves and others added 3 commits September 30, 2022 07:16
Co-authored-by: Jack Tracey <41163455+jtracey93@users.noreply.github.com>
Co-authored-by: Jack Tracey <41163455+jtracey93@users.noreply.github.com>
Co-authored-by: Jack Tracey <41163455+jtracey93@users.noreply.github.com>
@jtracey93
Copy link
Collaborator

/azp run validateazcloud

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jtracey93
Copy link
Collaborator

/azp run validateazcloud

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jtracey93
Copy link
Collaborator

/azp run validateazcloud

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jtracey93
Copy link
Collaborator

/azp run validateazcloud

@azure-pipelines
Copy link

Azure Pipelines failed to run 1 pipeline(s).

@jtracey93
Copy link
Collaborator

/azp run validateazcloud

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jtracey93
Copy link
Collaborator

/azp run e2e

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jtracey93
Copy link
Collaborator

/azp run e2e

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jtracey93
Copy link
Collaborator

/azp run validateazcloud

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jtracey93
Copy link
Collaborator

Merging new module addition

@jtracey93 jtracey93 merged commit 2266f95 into Azure:main Oct 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

💡 Feature Request -Enable Diagnostic Settings to LAW on all created Management Groups in ALZ-Bicep
2 participants