qbeyond / terraform-azurerm-archetype-lib

Repository with q.beyond designed Azure-archetypes, including policies
MIT License
0 stars 0 forks source link

Error on missing parameter #25

Open ghost opened 7 months ago

ghost commented 7 months ago

Is your feature request related to a problem? Please describe. I can deploy policies without problem, even though I don't provide the required parameters. This is due to the assignments containing all parameters with values even though, they are useless.

Describe the solution you'd like When I forget to provide a a parameter for an assignment, that is required, I want to get an error or at least a warning.

One possible solution would be to remove the required parameters from the assignments. If I don't set them via CAF, I should get an error from azure.

Describe alternatives you've considered Another solution would be to check in this module, that the default values where actually replaced in the resources. But that would require knowledge of the CAF in this module. It is possible, but I don't like it. But this would only produce a warning and not an error.

Aditional Context The CAF is also just filling useless parameters. But I don't think that makes it better.

ghost commented 7 months ago
  1. We have no control over all assignments. For example, the MDFC assignment comes from the CAF and also has bullshit parameters filled in
  2. We still have the problem that we cannot build customer specific archetypes. If we forbid bullshit parameters, then every policy would have to be configured for every customer, even if the customer doesn't want it. And then you would have to enter bullshit again, only this time in the customer repo @QBY-MarkusMaring
  1. It is true, that the CAF also does it. But we may try to change that there? If we prevent that here, at least we are a bit better.
  2. Do we currently have assignments, that are not for every customer? And if a customer doesn't use an assignment, I think it is reasonable, to add this (in form of useless parameters) to the customer repo, as you should disable the enforcment of the assignment anyhow.

Problem with checks is, that you would need to pass all assignments created by CAF back to this module to implement the checks. This should work, as the dependency graph doesn't care about modules. And it would have the advantage, that we could not only check for our assignments.

Still preventing errors is usually better than warn about them. Maybe do both?

QBY-MarkusMaring commented 7 months ago

Sounds good to me. For 1 we might need to go into discussion with the CAF maintainers. I tried searching for existing discussion/issues. There are many related issues but I didn't yet read through most of them yet. From what I read in their wiki it seems like intended behavior, as it is expected to be overwritten: https://github.com/Azure/terraform-azurerm-caf-enterprise-scale/wiki/%5BUser-Guide%5D-Archetype-Definitions#working-with-the-archetype_config-object I will check later if I can find related discussion and find out if there is a reason for parameters that the CAF doesn't replace (like: "security_contact@replace_me")

For 2: All our in house assignments are for every customer. So it would be good to at least enable errors for those :+1: If we really want to disable some of them adding placeholders and disabling enforcement is fine for me.