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

Custom role definition as user-defined type #673

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

picccard
Copy link
Contributor

@picccard picccard commented Nov 5, 2023

Overview/Summary

The module for customRoleDefinitions currently includes the CAF recommended custom roles, with no way to add additional roles. Customers who would like more custom roles have these options:

  1. Modify the module directly (not recommended and would brake on newer releases)
  2. Copy the module to config/custom-modules and maintain it themselves
  3. Other (such as bash/powershell-script, etc.)

This issue of no clear way to extend the usage of the module is also found in #646

This PR aims to allow extention of the module, with more custom roles through the parameter file. This together with a user-defined type for customRoles adds IntelliSense when using bicepparam instead of json for the parameterfile. The parameter parAdditionalRoles is added and prefilled with example roles as of now.

Now the drawback of this approach is that roleDefinitions isn't seperated into their own file, everything is just mashed together into one big parameterfile.

I'm looking for feedback on this approach. Would it be bad design to not keep roleDefs in their own file? Should the CAF recommended custom roles be included in the parameter aswell or should they be held separately?

Please mark the PR don't merge.

This PR fixes/adds/changes/removes

Instead of customers changing the released module this PR opens the possibility to add more custom roles through the parameter file.

Breaking Changes

  1. None yet.

@oZakari oZakari added Status: Do Not Merge ⛔ Do not merge PRs with this label attached as they are not ready etc. and removed do not merge labels Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Do Not Merge ⛔ Do not merge PRs with this label attached as they are not ready etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants