feat: Update avm/res/web/static-site - Add additional param for publicNetworkAccess
#4286
Conversation
microsoft-github-policy-service
bot
added
the
Needs: Triage 🔍
|
Important The "Needs: Triage 🔍" 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. |
microsoft-github-policy-service
bot
added
the
Type: AVM 🅰️ ✌️ Ⓜ️
|
Important If this is a module-related PR, being submitted by the sole owner of the module, the AVM core team must review and approve it (as module owners can't approve their own PRs). To indicate this PR needs the core team''s attention, apply the "Needs: Core Team 🧞" label! The core team will only review and approve PRs that have this label applied! |
ChrisSidebotham
added
the
Needs: Core Team 🧞
| @@ -88,6 +88,9 @@ param functionAppSettings object = {} | |||
| @description('Optional. The custom domains associated with this static site. The deployment will fail as long as the validation records are not present.') | |||
| param customDomains array = [] | |||
|
|
|||
| @description('Optional. The public network access settings for the static site. `Disabled` is configured by default.') | |||
| param publicNetworkAccess string = 'Disabled' | |||
There was a problem hiding this comment.
| param publicNetworkAccess string = 'Disabled' | |
| @allowed([ | |
| '' | |
| 'Enabled' | |
| 'Disabled' | |
| ]) | |
| param publicNetworkAccess string = 'Disabled' |
Requires a regeneration
| @@ -88,6 +88,9 @@ param functionAppSettings object = {} | |||
| @description('Optional. The custom domains associated with this static site. The deployment will fail as long as the validation records are not present.') | |||
| param customDomains array = [] | |||
|
|
|||
| @description('Optional. The public network access settings for the static site. `Disabled` is configured by default.') | |||
There was a problem hiding this comment.
Leveraging the PR to trigger a discussion about this setting in general and current implementation across the library.
I see the inconsistency is strong with this one.
In CARML we started disabling publicNetworkAccess within the resource implementation instead of through its default value, and only in case private endpoint is enabled. This is still the current implementation for multiple modules, such as for example storage-account
The reasoning behind was that it won't be useful to disable public network access by default if private endpoint is not configured. In these cases the default value is set to empty ''.
Meanwhile, many other resource modules are now disabling publicNetworkAccess by default via default values in AVM. An example is RSV. In these cases the default value is set to Disabled.
Needless to say, we also have cases where the default value is set to Enabled.
I think it would be nice to align the behavior.
There was a problem hiding this comment.
I could imagine that some resources may have PSRule tests that request one or the other. Don't have an example at hand though (for RSV, for example, this test does not exist, so I guess the person implementing the parameter simply decided that it would be a good solution).
One added caveat is that aside from PEs, one could also navigate to a resource like a storage account using service endpoints, making the matter even harder to define a default for.
eriqua
changed the title
publicNetworkAccess in avm-res-web-static-siteavm/res/web/static-site - Add additional param for publicNetworkAccess
eriqua
added
Class: Resource Module 📦
Description
#Closes #4283
Pipeline Reference
Type of Change
version.json:version.json.version.json.Checklist
Set-AVMModulelocally to generate the supporting module files.