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

Fixes to remove old assignment and 10 char limit #15

Merged
merged 5 commits into from
Sep 27, 2023

Conversation

Brunoga-MS
Copy link
Contributor

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

Overview/Summary

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

This PR fixes/adds/changes/removes

  1. Cleanup script BUG 30923 - Several identities not found are left in the IAM of the MG
  2. Description and metadata addition to DINE-* policy files to correctly allow cleanup (https://dev.azure.com/CSUSolEng/Azure%20Landing%20Zones/_backlogs/backlog/Azure%20Landing%20Zones%20Team/Stories/?workitem=30923)
  3. Added known issue about subsequent deployment to different locations
  4. Removing 10 chars limitation for enterpriseScaleCompanyPrefix BUG 30883 - Deployment on pseudo MG name longer than 10 chars fails

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.)

-  cleanup script
- Description and metadata addition to DINE-* policy files
- Added known issue about subsequent deployment to different locations
- Removing 10 chars limitation for enterpriseScaleCompanyPrefix
Copy link
Contributor

@arjenhuitema arjenhuitema 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, thank you Bruno. Just one comment about the metadata/description.

Also, can you provide some evidence with these changes. For example, screenshot of a successful deployment, successful cleanup, deployment to a management group that has more than 10 characters, etc. Thanks!

@@ -135,7 +135,7 @@ ForEach ($identity in $policyAssignmentIdentities) {

ForEach ($roleAssignment in $identityRoleAssignments) {

If ($roleAssignment.Description -like '*_deployed_by_amba*') {
If ($roleAssignment.Description -eq '_deployed_by_amba') {
Copy link
Contributor

Choose a reason for hiding this comment

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

I see "_deployed_by_amba" was added in the metadata of the role assignment, however here the if statement the evaluations is done on ".Description" can you validate whether that is correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as the script line goes, I made the change to look for exact value (case insensitive). Wildcard in this case can lead to something different added by the customer ... very unlikely but possible.

Here they are the screenshots:

  • Successful deployment with a pseudoMG name which is more than 10 chars
    image

  • Description for role assignment correctly populated
    image

  • Successful cleanup
    image

image

image

  • Successful deployment
    image

image

Copy link
Contributor

@arjenhuitema arjenhuitema left a comment

Choose a reason for hiding this comment

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

LGTM

@arjenhuitema arjenhuitema merged commit bd020cf into Azure:main Sep 27, 2023
2 of 4 checks passed
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.

2 participants