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

TeamsGroupPolicyAssignment: Skip assignments that have orphaned/deleted groups or without display name #4420

Merged
merged 3 commits into from
Mar 18, 2024

Conversation

ricmestre
Copy link
Contributor

Pull Request (PR) description

If an assignment has a group that has been deleted, and/or orphaned, then skip it instead of throwing an error otherwise all assignments won't be saved into the blueprint even for the ones that were successfully exported up until the error occurred.

This Pull Request (PR) fixes the following issues

@NikCharlebois
Copy link
Collaborator

We recently ran into a similar issue with AADConditionalAccessPolicy, where we were simply skipping invalid groups instead of throwing an error. The customer made a typo in the groups name, which resulted in the group being skipped and the policy got updated with 0 group in the ExcludedGroup policy which locked them out of their system. I believe we should use it as a rule of thumb to always error out when an invalid User or Group is specified as a value. Thoughts?

@ricmestre
Copy link
Contributor Author

ricmestre commented Mar 12, 2024

@NikCharlebois My understanding was that whenever such issues happened it needed to be solved in the customer's tenant rather then at the code level, but @malauter raised PR #4408 to solve a similar issue and we discussed about it, that's why I actually raised this PR.

But regardless we really need to reach a consensus on this topic so this doubt doesn't come up again.

EDIT: Just FYI, in my case because of one bogus Teams group assignment none of the assignments were actually exported and I can't export them for the customer until they solve this issue with their MS support.

@NikCharlebois
Copy link
Collaborator

After much thinking on this topic, the consensus should be that if an invalid group is specified, that the Set-targetResource should always throw the error, because otherwise it risks bringing the environment in an invalid state. From an Export perspective, it is fine to skip over those in my opinion. I will merge the PR.

@NikCharlebois NikCharlebois merged commit d4fb236 into microsoft:Dev Mar 18, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TeamsGroupPolicyAssignment: Export stops if it finds an assignment with an orphaned/deleted group
2 participants