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

[AVM Question/Feedback]: Should there be a way to merge default assignments? #125

Open
1 task done
JWilkinsonMB opened this issue Oct 3, 2024 · 4 comments
Open
1 task done
Assignees
Labels
Language: Terraform 🌐 This is related to the Terraform IaC language Type: Feature Request ➕ New feature or request

Comments

@JWilkinsonMB
Copy link

Check for previous/existing GitHub issues

  • I have checked for previous/existing GitHub issues

Description

Are there any thoughts around supporting one or both of:

  1. Allowing polices / assignments defined in the root library to be replaced with custom versions with the same name
  2. Allowing default assignments to be merged, e.g. where a default such as the log analytics workspace is defined in the root library, but there is a custom policy assignment that should use the same value.

As an example, I currently need to use a custom version of the policy assignment Deploy-MDFC-Config-H224 & the policy set Deploy-MDFC-Config_20240319. The reason for this is that this policy set as provided will only deploy Defender for servers P2, whereas I want to deploy P1, so I've made this configurable.

The long term fix would be to try and get the ALZ policy updated so that it supports this ability, but that could take a while. In the meantime:

Without 1. I have to update any archetypes where this policy is used and override them to remove the version that's included in the library, and add my version which has to have a different name.

Without 2. I have to create a new default with a different name but the same value as the original because otherwise I get an error about the default already existing. This requires changes to the terraform code as well as the library files.

I understand that implementing 1. could lead to the risk of inadvertently overwriting a policy and risk causing more problems than the potential convenience is worth, so if that was the design decision behind removing the overwrite support (which used to exist in v0.13 of the provider with lib_overwrite_enabled) then I can get behind that. It would be good to have that discussion though and if it's possible / sensible to make it configurable as it used to be so the user can make that judgment call.

I think 2. could be supported safely as policy_assignments can be merged.

Technically I believe the implementation is in alzlib but thought the question better belongs here as it's more of a design decision than implementation. Happy to move it over there if that's preferred though.

Thanks!

@JWilkinsonMB JWilkinsonMB added Language: Terraform 🌐 This is related to the Terraform IaC language Needs: Triage 🔍 Maintainers need to triage still Type: Question/Feedback 🙋 Further information is requested or just some feedback labels Oct 3, 2024
@matt-FFFFFF
Copy link
Member

Hi!

Thanks for the question. It should still possible to overwrite library artefacts, though we changed the attribute name to fit in with the move to go-getter refs.

Let me know if this option doesn't work for you...

https://registry.terraform.io/providers/Azure/alz/latest/docs#library_overwrite_enabled

@matt-FFFFFF
Copy link
Member

#RR

@matt-FFFFFF matt-FFFFFF self-assigned this Oct 3, 2024
@matt-FFFFFF matt-FFFFFF removed the Needs: Triage 🔍 Maintainers need to triage still label Oct 3, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Author Feedback 👂 Awaiting feedback from the issue/PR author label Oct 3, 2024
@JWilkinsonMB
Copy link
Author

Thanks @matt-FFFFFF, not sure how I missed that!

library_overwrite_enabled answers the 1st ask. However, if you set a default with the same name it overwrites it entirely rather than merging it, so it doesn't cover the 2nd point.

Particularly with the log analytics workspace I could see it being useful to be able to re-use the same default for custom policy assignments, although not a big deal, there's an easy workaround by creating an additional default.

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs: Attention 👋 Reply has been added to issue, maintainer to review and removed Needs: Author Feedback 👂 Awaiting feedback from the issue/PR author labels Oct 4, 2024
@matt-FFFFFF
Copy link
Member

I think merging defaults is one for a future release. For now you can clone them and add your own in another library reference.

I'll add the feature request to this

@matt-FFFFFF matt-FFFFFF added Type: Feature Request ➕ New feature or request and removed Type: Question/Feedback 🙋 Further information is requested or just some feedback Needs: Attention 👋 Reply has been added to issue, maintainer to review labels Oct 5, 2024
@JWilkinsonMB JWilkinsonMB changed the title [AVM Question/Feedback]: Should there be a way to overwrite policies and / or merge default assignments? [AVM Question/Feedback]: Should there be a way to merge default assignments? Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language: Terraform 🌐 This is related to the Terraform IaC language Type: Feature Request ➕ New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants