Skip to content

Commit

Permalink
feat: Use null value to evaluate behavior in publicNetworkAccess inst…
Browse files Browse the repository at this point in the history
…ead of empty string - `avm/res/cognitive-services/account` (#1949)

## Description

<!--
>Thank you for your contribution !
> Please include a summary of the change and which issue is fixed.
> Please also include the context.
> List any dependencies that are required for this change.

Fixes #123
Fixes #456
Closes #123
Closes #456
-->
Request addresses the conditional statement logic for network access
status based on the values of publicNetworkAccess and networkAcls. The
expression ensures that if publicNetworkAccess is not empty, that value
is used; otherwise, if networkAcls is not empty, the status is
considered enabled; otherwise, it’s disabled. This aligns with the
intended behavior of prioritizing publicNetworkAccess over networkAcls
for determining network access status.

This change makes it so that an empty string passed to the module, will
fail the deployment. If the value is not passed, then a null value will
be interpreted as the default 'Disabled' behavior. This can only be
overridden by explicitly passing 'Enabled'.

## Pipeline Reference

<!-- Insert your Pipeline Status Badge below -->

| Pipeline |
| -------- |
|
[![avm.res.cognitive-services.account](https://github.com/jceval/bicep-registry-modules/actions/workflows/avm.res.cognitive-services.account.yml/badge.svg)](https://github.com/jceval/bicep-registry-modules/actions/workflows/avm.res.cognitive-services.account.yml)
|

## Type of Change

<!-- Use the check-boxes [x] on the options that are relevant. -->

- [ ] Update to CI Environment or utlities (Non-module effecting
changes)
- [ ] Azure Verified Module updates:
- [ ] Bugfix containing backwards compatible bug fixes, and I have NOT
bumped the MAJOR or MINOR version in `version.json`:
- [ ] Someone has opened a bug report issue, and I have included "Closes
#{bug_report_issue_number}" in the PR description.
- [x] The bug was found by the module author, and no one has opened an
issue to report it yet.
- [ ] Feature update backwards compatible feature updates, and I have
bumped the MINOR version in `version.json`.
- [ ] Breaking changes and I have bumped the MAJOR version in
`version.json`.
  - [ ] Update to documentation

## Checklist

- [x] I'm sure there are no other open Pull Requests for the same
update/change
- [ ] I have run `Set-AVMModule` locally to generate the supporting
module files.
- [ ] My corresponding pipelines / checks run clean and green without
any errors or warnings

<!-- Please keep up to day with the contribution guide at
https://aka.ms/avm/contribute/bicep -->

---------

Co-authored-by: ChrisSidebotham-MSFT <48600046+ChrisSidebotham@users.noreply.github.com>
  • Loading branch information
jceval and ChrisSidebotham authored Jul 19, 2024
1 parent 299c7cd commit 0c380c4
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 9 deletions.
2 changes: 0 additions & 2 deletions avm/res/cognitive-services/account/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1755,11 +1755,9 @@ Whether or not public network access is allowed for this resource. For security

- Required: No
- Type: string
- Default: `''`
- Allowed:
```Bicep
[
''
'Disabled'
'Enabled'
]
Expand Down
5 changes: 2 additions & 3 deletions avm/res/cognitive-services/account/main.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,10 @@ param diagnosticSettings diagnosticSettingType

@description('Optional. Whether or not public network access is allowed for this resource. For security reasons it should be disabled. If not specified, it will be disabled by default if private endpoints are set and networkAcls are not set.')
@allowed([
''
'Enabled'
'Disabled'
])
param publicNetworkAccess string = ''
param publicNetworkAccess string?

@description('Conditional. Subdomain name used for token-based authentication. Required if \'networkAcls\' or \'privateEndpoints\' are set.')
param customSubDomainName string?
Expand Down Expand Up @@ -305,7 +304,7 @@ resource cognitiveService 'Microsoft.CognitiveServices/accounts@2023-05-01' = {
ipRules: networkAcls.?ipRules ?? []
}
: null
publicNetworkAccess: !empty(publicNetworkAccess)
publicNetworkAccess: publicNetworkAccess != null
? publicNetworkAccess
: (!empty(networkAcls) ? 'Enabled' : 'Disabled')
allowedFqdnList: allowedFqdnList
Expand Down
7 changes: 3 additions & 4 deletions avm/res/cognitive-services/account/main.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"_generator": {
"name": "bicep",
"version": "0.28.1.47646",
"templateHash": "7966489832259464647"
"templateHash": "16063130167042223341"
},
"name": "Cognitive Services",
"description": "This module deploys a Cognitive Service.",
Expand Down Expand Up @@ -628,9 +628,8 @@
},
"publicNetworkAccess": {
"type": "string",
"defaultValue": "",
"nullable": true,
"allowedValues": [
"",
"Enabled",
"Disabled"
],
Expand Down Expand Up @@ -859,7 +858,7 @@
"properties": {
"customSubDomainName": "[parameters('customSubDomainName')]",
"networkAcls": "[if(not(empty(coalesce(parameters('networkAcls'), createObject()))), createObject('defaultAction', tryGet(parameters('networkAcls'), 'defaultAction'), 'virtualNetworkRules', coalesce(tryGet(parameters('networkAcls'), 'virtualNetworkRules'), createArray()), 'ipRules', coalesce(tryGet(parameters('networkAcls'), 'ipRules'), createArray())), null())]",
"publicNetworkAccess": "[if(not(empty(parameters('publicNetworkAccess'))), parameters('publicNetworkAccess'), if(not(empty(parameters('networkAcls'))), 'Enabled', 'Disabled'))]",
"publicNetworkAccess": "[if(not(equals(parameters('publicNetworkAccess'), null())), parameters('publicNetworkAccess'), if(not(empty(parameters('networkAcls'))), 'Enabled', 'Disabled'))]",
"allowedFqdnList": "[parameters('allowedFqdnList')]",
"apiProperties": "[parameters('apiProperties')]",
"disableLocalAuth": "[parameters('disableLocalAuth')]",
Expand Down

0 comments on commit 0c380c4

Please sign in to comment.