Not required as ?? takes the first non-null value. Having null as an 'else' case means it's just ignored 😄

The idea was to actually supply a null value. Having any value in changeFeedRetentionInDays means its not stored indefinetely.

ChangefeedRetention works as follows: values from 1 to 146000 sets a specific point in time when the data gets deleted. setting null (not 0!) causes the data to be retained indefinitely - which is the goal here. So setting ?? should make sense, right?

Nono absolutely. But as the parameter is nullable, not setting it results in a null and a null would be passed to the resource.

With

• retentionInDays: changeFeedRetentionInDays ?? null all you're getting after the evaluation is
• retentionInDays: null ?? null

and Bicep will pick any null.

@fblix, could we move forward on this? This PR has been open for a very very long time. If bad comes to worse I can also create the contribution, yet would need to you review and approve it.

Why removing the default? Would it not make more sense to move it into the network ACLS like suggested here: #1472 (comment)

Just wondering how we could make it so that a firewall would be enabled by default, but could be disabled if needed. I guess with the implementation in the comment it's also not exactly possible, right? 😄

Somehow in my mind it doesn't check out...

@eriqua / @ChrisSidebotham any ideas?

Removing the default value was the only solution I was able to come up with. Everything else just fails the linting spectacularly, maybe you guys have a better idea on how to achieve this?

@eriqua / @ChrisSidebotham any ideas?

@AlexanderSehr bare with me, still need to look into this more closely.

The rule of thumb, if that helps, is to make it more difficult, rather than easier, to deviate from the value we want to enforce as default. That is only in case the intention of the default is to make the resource secure by default/WAF compliant/pass PSRule.

Regarding the implementation, please consider the condition for public network access, normally enabled or disabled by default also based on network ACL values.

Just had some spare time again, and I still think it would be the easiest/cleanest solution to just have networkACLs set as mandatory. Default being the Action: Deny. If someone then wants to open the firewall again, it would be as easy as setting Action: Allow. What do you think? @AlexanderSehr @eriqua @ChrisSidebotham

Hey @fblix,
I took another stab.

How about this to avoid making it mandatory.

Template

@description('Required. Networks ACLs, this value contains IPs to whitelist and/or Subnet information. If in use, bypass needs to be supplied. For security reasons, it is recommended to set the DefaultAction Deny.')
param networkAcls networkAclsType // Removed default as nullable types cannot have defaults

(...)
    networkAcls: !empty(networkAcls)
      ? {
          resourceAccessRules: networkAcls.?resourceAccessRules
          bypass: networkAcls.?bypass
          defaultAction: networkAcls.?defaultAction ?? 'Deny'
          virtualNetworkRules: networkAcls.?virtualNetworkRules
          ipRules: networkAcls.?ipRules
        }
      : { // New default case that enables the firewall by default
          bypass: 'AzureServices'
          defaultAction: 'Deny'
        }

type networkAclsType = {
  (...)  
}? // Nullable => optional
(...)

Test case (defaults test)

module testDeployment '../../../main.bicep' = [
  for iteration in ['init', 'idem']: {
    scope: resourceGroup
    name: '${uniqueString(deployment().name, resourceLocation)}-test-${serviceShort}-${iteration}'
    params: {
      name: '${namePrefix}${serviceShort}001'
      allowBlobPublicAccess: false
      location: resourceLocation
      networkAcls: {
        defaultAction: 'Allow'
        bypass: 'None'
      }
    }
  }
]

In Azure, that showed up as

So the firewall was disabled.

When I remove the explicit specification / it falls back to the default it shows

+1 on @AlexanderSehr proposed implementation.

Not sure I follow on the defaults test case though.
Shouldn't we just use required parameters there? I think I'm missing something.

You're correct @eriqua. I just used the default test as a showcase to not muddy the waters.

Must be updated to reflect the changefeed example