-
Notifications
You must be signed in to change notification settings - Fork 82
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
feat: aad group creation #155
Comments
@matt-FFFFFF and @kewalaka I understand this from a convenience perspective. I'm just wondering if this moving the module into the monolith territory and perhaps composition would be an alternative approach with some example documentation. I am obviously slightly biased given I am the owner of the module, but we do have an AVM module that could help with this. https://github.com/Azure/terraform-azurerm-avm-res-authorization-roleassignment This change will add a dependency on the azuread provider in the lz-vending module that could be avoided if we used composition instead. Not sure if that is a bad thing, but just something to consider. I'm honestly not sure which is the 'correct' approach here, but wanted to raise it as a suggestion. I also appreciate the AVM module doesn't support conditions or creating the aad group yet, but they could be added fairly quickly if desired. |
hi @jaredfholgate I see this more as adding support for AAD groups rather than a role assignments thing. I grant I was perhaps a little lazy in the way I did role assignments - I included it in the module because it saved me having to create a separate locals map to compute the combination of "AAD to associated RA". This is particularly useful to hide complexity in my use case because I'm using the "map of yaml files" pattern in the root module, which already makes for some fun locals. Whether it fits in the module - it started life as an external component for a customer LZ vending machine based on this module, and it works grand that way. I hear you on avoiding this being the tipping point leading to a super-LZ-module, so if you decide not to accept I am just fine about it. For me - it was written - so why not share 😊 |
@jaredfholgate i've looked at your RA module properly now and I admit I had not been paying attention - I see all the things it does re managing the translation of names and the various targets, not just RBAC. Nice 💖. This definitely motivates me to do a refactor - I have my own home-brew approach for AAD group membership & this is deffo for the chop now! |
@jaredfholgate a problem I've found with using an external role assignments module where the resources that I want to permissions to are created in the LZ module. For this scenario, I place a 'depends_on' the role assignment module so that it waits for the LZ module to finish. The depends on triggers the data blocks within the role assignment module to be re-calculated, this makes attributes on the permissions "known after apply" and therefore get dropped and re-created. As a potential workaround, I'm experimenting with The code isn't public so i can't share - hopefully the above makes sense. |
i ve been trying to have the ad groups created separately with another module, but then when referencing it in the vending module i got the for_each limitation with the map: Error: Invalid for_each argument Any idea ? |
You need to make the map keys static strings. It won't work if the map keys are unknown. |
Description
Sub-module to create n number of AAD groups and assign them to the subscription (or child scopes)
Describe the solution you'd like
Something like this...
Additional context
The text was updated successfully, but these errors were encountered: