Azure / bicep-registry-modules

Bicep registry modules
MIT License
465 stars 308 forks source link

[AVM Module Issue]: Key Vault Diagnostic setting in portal doesn't match AVM module #3050

Open Gossef opened 4 weeks ago

Gossef commented 4 weeks ago

Check for previous/existing GitHub issues

Issue Type?

Bug

Module Name

avm/res/key-vault/vault

(Optional) Module Version

0.7.0

Description

In the portal, the diagnostic setting categories for Key Vaults are:

However, in the AVM module the category "Audit Logs" or "AuditLogs" isn't accepted, as it should be AuditEvent.

This makes it difficult to easily copy these settings to the AVM Module's configuration as the documentation doesn't necessarily list all options either.

(Optional) Correlation Id

No response

microsoft-github-policy-service[bot] commented 4 weeks ago

[!IMPORTANT] The "Needs: Triage :mag:" label must be removed once the triage process is complete!

[!TIP] For additional guidance on how to triage this issue/PR, see the BRM Issue Triage documentation.

avm-team-linter[bot] commented 4 weeks ago

@Gossef, thanks for submitting this issue for the avm/res/key-vault/vault module!

[!IMPORTANT] A member of the @Azure/avm-res-keyvault-vault-module-owners-bicep or @Azure/avm-res-keyvault-vault-module-contributors-bicep team will review it soon!

microsoft-github-policy-service[bot] commented 3 weeks ago

[!WARNING] Tagging the AVM Core Team (@Azure/avm-core-team-technical-bicep) due to a module owner or contributor having not responded to this issue within 3 business days. The AVM Core Team will attempt to contact the module owners/contributors directly.

[!TIP]

  • To prevent further actions to take effect, the "Status: Response Overdue 🚩" label must be removed, once this issue has been responded to.
  • To avoid this rule being (re)triggered, the ""Needs: Triage :mag:" label must be removed as part of the triage process (when the issue is first responded to)!
AlexanderSehr commented 3 weeks ago

Hey @Gossef, thank you for the issue. You're raising a good point that we actually tried to address in the preceeding CARML modules. There, we tried to provide up-to-date @allowed([...]) lists for logs & metrics. When transitioning to AVM we dropped those however. That begs the question as to 'why?'. And the simple answer is unfortunately maintainability. For one, these allowed list has always be up to date as it's not allowed to contain wildcards. That means, when we had an allowed list it could only set what's in the allowed ist. However, if the RP added new categories, you would not be able to use them unless the module is updated. We tried to solve that issue in return with some automation. That is, try to fetch the allowed values from whatever documentation we could find and create an issue (or even an automated PR) to update the list. That, in theory, was great as it would keep that timeframe where something is missing small - but - we never found a reliable source. We experimented with some automation using those 2 urls:

If you happen to know a reliable source to fetch these options from (without the need to have the resource deployed first and then fetch it from the portal), I think we'd have a strong case of adding the feature again.

Your request makes absolute sense afterall.

I hope the above provided some insights. 💪