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

Updating and adding guidance for brownfield deployments #10

Merged
merged 10 commits into from
Sep 22, 2023

Conversation

Brunoga-MS
Copy link
Contributor

@Brunoga-MS Brunoga-MS commented Sep 21, 2023

Overview/Summary

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

This PR fixes/adds/changes/removes

  1. Added Cleaning up preview version deployment document (file is: docs\content\patterns\alz\Cleaning-Up-Preview-Version-Deployment.md)
  2. Added script Start-ALZMonitorCleanup.ps1 (file is patterns\alz\scripts\Start-ALZMonitorCleanup.ps1) to allow for complete cleanup of preview version if needed.
  3. Added additional cases to Known Issues
  4. Fixed links and minor cosmetics in Cleaning up a Deployment
  5. Added clarification in Deploy with Azure PowerShell to include brownfield scenarios, steps. Changes are also related to links and cosmetics
  6. cosmetics in Introduction to deploying the ALZ Pattern
  7. Links and cosmetics in Remediate Policies
  8. Fixed alzArm template to avoid permission issue on root tenant USER STORY 30878

Breaking Changes

  1. NONE

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 or ADO Work Items (Internal Only)
  • Ensured my code/branch is up-to-date with the latest changes in the main branch
  • Ensured PR tests are passing
  • Updated relevant and associated documentation (e.g. Contribution Guide, Docs etc.)

@JoeyBarnes JoeyBarnes merged commit 988ed94 into Azure:main Sep 22, 2023
1 of 4 checks passed
Copy link
Contributor

@paulgrimley paulgrimley left a comment

Choose a reason for hiding this comment

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

I've reviewed a couple of files for grammar, the rest will need @arjenhuitema to comment

- Policy Set Definitions
- Policy Assignment remediation identity role assignments

All resources deployed as part of the initial ALZ Monitor deployment and the resources created by dynamically by 'deploy if not exist' policies are either tagged, marked in metadata, or in description (depending on what the resource supports) with the value `_deployed_by_alz_monitor` or `_deployed_by_alz_monitor=True`. This metadata is used to execute the cleanup of deployed resources; _if it has been removed or modified the cleanup script will not include those resources_.
Copy link
Contributor

Choose a reason for hiding this comment

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

grammar "by dynamically by"


All resources deployed as part of the initial ALZ Monitor deployment and the resources created by dynamically by 'deploy if not exist' policies are either tagged, marked in metadata, or in description (depending on what the resource supports) with the value `_deployed_by_alz_monitor` or `_deployed_by_alz_monitor=True`. This metadata is used to execute the cleanup of deployed resources; _if it has been removed or modified the cleanup script will not include those resources_.

## Cleanup Script Execution
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment that the script should be thoroughly tested before uisng in production environments and we assume no responsibility in the event of resource or data loss (I think Jan has something he has written previously) and there is one for ALZ clean-up that has something in the comments.

arjenhuitema pushed a commit that referenced this pull request Mar 20, 2024
Updating noEmail with latest from Main
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.

3 participants