-
Notifications
You must be signed in to change notification settings - Fork 387
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
feat: Update avm/res/web/static-site
- Add additional param for publicNetworkAccess
#4286
Open
ChrisSidebotham
wants to merge
77
commits into
Azure:main
Choose a base branch
from
ChrisSidebotham:swa-updates
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+54
−7
Open
Changes from 75 commits
Commits
Show all changes
77 commits
Select commit
Hold shift + click to select a range
7ecd1a5
adding deployment files
ChrisSidebotham 0f15c86
ficing rp ref
ChrisSidebotham c6993a6
Merge branch 'main' of https://github.com/Azure/bicep-registry-modules
ChrisSidebotham 0e65d72
Merge branch 'main' of https://github.com/Azure/bicep-registry-modules
ChrisSidebotham 482add6
Adding workflow file
ChrisSidebotham 1bffa61
Merge branch 'Azure:main' into main
ChrisSidebotham 8c358e7
Merge branch 'main' of https://github.com/Azure/bicep-registry-modules
ChrisSidebotham 14a5d0d
Merge branch 'main' of https://github.com/ChrisSidebotham/bicep-regis…
ChrisSidebotham fba5375
fixing path
ChrisSidebotham 05152a8
Fixing Workflow name
ChrisSidebotham f8d04c6
adding workflow
ChrisSidebotham d22a6c7
Merge branch 'main' of https://github.com/Azure/bicep-registry-modules
ChrisSidebotham 6993c4d
Adding Workflow file
ChrisSidebotham cf44304
Adding workflow file
ChrisSidebotham 5260056
Merge branch 'anchor' of https://github.com/ChrisSidebotham/bicep-reg…
ChrisSidebotham 3740075
Merge branch 'main' of https://github.com/Azure/bicep-registry-modules
ChrisSidebotham c9b6b4c
Adding workflow file
ChrisSidebotham a3d83a4
adding workflow
ChrisSidebotham 915d12c
Fixing workflow name
ChrisSidebotham 1c56bb3
update workflow
ChrisSidebotham 454bcc5
Merge branch 'main' of https://github.com/Azure/bicep-registry-modules
ChrisSidebotham fe017c1
updating Test Case s location to fix Pipeline
ChrisSidebotham ddb6e04
Merge branch 'main' of https://github.com/Azure/bicep-registry-modules
ChrisSidebotham 1931d9a
Merge branch 'main' of https://github.com/Azure/bicep-registry-modules
ChrisSidebotham 657311c
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham fff3279
Merge branch 'main' of https://github.com/Azure/bicep-registry-modules
ChrisSidebotham c5f6e4f
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham ffc5837
Merge branch 'main' of https://github.com/Azure/bicep-registry-modules
ChrisSidebotham e9b2e80
adding workflow
ChrisSidebotham 018c016
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 9a3e2bc
Merge branch 'main' of https://github.com/Azure/bicep-registry-modules
ChrisSidebotham ebed1d1
Merge branch 'main' of https://github.com/Azure/bicep-registry-modules
ChrisSidebotham d587154
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 18a9a54
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham bb9037e
Merge branch 'main' of https://github.com/Azure/bicep-registry-modules
ChrisSidebotham de4e665
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 8994eff
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 88df6f6
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham cdfb68d
Adding workflow
ChrisSidebotham 0e3020f
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 40614c1
Update forwardTo property to null if empty
ChrisSidebotham 4240d6c
Update templateHash values in Service Bus Namespace and Topic
ChrisSidebotham 6e16d58
Merge branch 'main' into anchor
ChrisSidebotham d41dec0
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 689f23d
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 6cd9703
Merge branch 'main' of https://github.com/Azure/bicep-registry-modules
ChrisSidebotham 27de0ec
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 3249154
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 65802d0
Merge branch 'Azure:main' into anchor
ChrisSidebotham 8a0d70b
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham e19c970
Merge branch 'Azure:main' into main
ChrisSidebotham dc5e12d
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 379534f
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 77e2f8c
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 6e0b216
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham e4cf2a0
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham bacee7d
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham e8c9b85
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 4ca1290
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 3211b7c
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham bb507f9
#2578
ChrisSidebotham 7ec58b0
typo
ChrisSidebotham 88d09f1
typo
ChrisSidebotham 0a7cefc
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 64d7206
Merge branch 'anchor' of https://github.com/ChrisSidebotham/bicep-reg…
ChrisSidebotham ff6facc
Adding logic to skip for non resource modules
ChrisSidebotham d0dadd0
chore: Remove trailing whitespace in main.json file
ChrisSidebotham 77014ff
Merge branch 'anchor' of https://github.com/ChrisSidebotham/bicep-reg…
ChrisSidebotham c679dd1
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 3dff828
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 6c9b47e
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 6579a77
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 783a91c
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham 739c32a
Merge branch 'main' of https://github.com/Azure/bicep-registry-module…
ChrisSidebotham f7ebd4f
Changes to support [AVM Module Issue]: Static Web App missing `public…
ChrisSidebotham 1d694a3
update to execution type
ChrisSidebotham eb676fb
refactor: Update publicNetworkAccess parameter description and allowe…
ChrisSidebotham File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Requires a regeneration There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. applied |
||||||||||||||||
|
||||||||||||||||
var formattedUserAssignedIdentities = reduce( | ||||||||||||||||
map((managedIdentities.?userAssignedResourceIds ?? []), (id) => { '${id}': {} }), | ||||||||||||||||
{}, | ||||||||||||||||
|
@@ -155,7 +158,7 @@ resource avmTelemetry 'Microsoft.Resources/deployments@2024-03-01' = if (enableT | |||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
resource staticSite 'Microsoft.Web/staticSites@2021-03-01' = { | ||||||||||||||||
resource staticSite 'Microsoft.Web/staticSites@2024-04-01' = { | ||||||||||||||||
name: name | ||||||||||||||||
location: location | ||||||||||||||||
tags: tags | ||||||||||||||||
|
@@ -174,6 +177,7 @@ resource staticSite 'Microsoft.Web/staticSites@2021-03-01' = { | |||||||||||||||
repositoryToken: repositoryToken | ||||||||||||||||
repositoryUrl: repositoryUrl | ||||||||||||||||
templateProperties: templateProperties | ||||||||||||||||
publicNetworkAccess: publicNetworkAccess | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am about t commit another push to this which updates it to the same configuration as we use for PaaS Services, but I am removing
networkacl
as this is not supportted for this resource