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

Adding RBAC constrained delegation parameters and guidance in the roleAssignment modules #816

Merged
merged 14 commits into from
Aug 1, 2024

Conversation

sebassem
Copy link
Contributor

@sebassem sebassem commented Jul 25, 2024

Adding the conditions parameters to the roleAssignment modules with examples and guidance to enable customers to securely delegate role assignments.

Related Issues/Work Items

Fixes AB#36173

Breaking Changes

N/A

Testing Evidence

image

image

As part of this Pull Request I have

@sebassem sebassem requested review from oZakari and jtracey93 July 25, 2024 12:14
@sebassem sebassem self-assigned this Jul 25, 2024
@sebassem sebassem added Area: RBAC 🛂 Issues / PR's related to RBAC Type: Documentation 📄 Improvements or additions to documentation Type: Enhancement ✨ New feature or request labels Jul 25, 2024
jtracey93
jtracey93 previously approved these changes Jul 25, 2024
Copy link
Contributor

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

LGTM

@sebassem great work. Just to confirm we aren't adding the condition building simplification to these modules right, just the sub vending ones?

@jtracey93
Copy link
Contributor

/azp run validateazcloud

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@oZakari
Copy link
Contributor

oZakari commented Jul 25, 2024

/azp run validateazcloud

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@sebassem
Copy link
Contributor Author

LGTM

@sebassem great work. Just to confirm we aren't adding the condition building simplification to these modules right, just the sub vending ones?

Correct, just showing how to use the condition parameter in the roleAssignment modules

@oZakari
Copy link
Contributor

oZakari commented Jul 26, 2024

@sebassem Im looking into that pipeline error but it's unrelated to your changes so nothing to do on your end.

@oZakari
Copy link
Contributor

oZakari commented Aug 1, 2024

@sebassem Looking into it closer, it is related to these changes. 😞 The policy assignment management group module references the role assignment modules. In main currently, json is approx 3.4 MB and with your changes it comes out to 4.2 MB. As far as I'm aware the only way we can get around this is by splitting up the module deployments (likely the ALZ Default Policy Assignments module). However, I'd rather avoid this if at all possible due to it being messy and requiring a lot of work and primarily with the AVM transition work in play.

@oZakari
Copy link
Contributor

oZakari commented Aug 1, 2024

If I remove the customer telemetry module references, I can get it down to 3.5 MB. Will think on it a bit more and update here but thinking that's going to be the easiest/quickest option. Ping me if any options come to your mind as well.

@sebassem
Copy link
Contributor Author

sebassem commented Aug 1, 2024

If I remove the customer telemetry module references, I can get it down to 3.5 MB. Will think on it a bit more and update here but thinking that's going to be the easiest/quickest option. Ping me if any options come to your mind as well.

Maybe its the long description of the parameter. I just pushed a commit to reduce the description, if that solved it, then I can move the instructions to generate a condition in md file ?

image

@oZakari
Copy link
Contributor

oZakari commented Aug 1, 2024

Awesome. thanks! That got us to 4.011 and tested locally and no error so let's go with that approach for now.

I am going to hit it again with the policy refresh, but good enough for now.

@sebassem
Copy link
Contributor Author

sebassem commented Aug 1, 2024

Awesome. thanks! That got us to 4.011 and tested locally and no error so let's go with that approach for now.

I am going to hit it again with the policy refresh, but good enough for now.

Just pushed the changes with having the instructions in the readme

@oZakari
Copy link
Contributor

oZakari commented Aug 1, 2024

/azp run validateazcloud

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@oZakari oZakari merged commit 2a32d0b into Azure:main Aug 1, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: RBAC 🛂 Issues / PR's related to RBAC Type: Documentation 📄 Improvements or additions to documentation Type: Enhancement ✨ New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants