[Modules] storage account module : conditional nfsv3 parameter

[ghost]

Azure Storage account : Only the NFSv3 parameter when set to true as this can only be added during storage account creation

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.

Pipeline references

For module/pipeline changes, please create and attach the status badge of your successful run.

Pipeline

Type of Change

Please delete options that are not relevant.

• Bugfix (non-breaking change which fixes an issue)

Checklist

• I'm sure there are no other open Pull Requests for the same update/change
• My corresponding pipelines / checks run clean and green without any errors or warnings
• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation (readme)
• I did format my code

[ghost]

tested this
and with an exiting storage account with nfsv3 disabled this continues to work
also a new storage account deployment with NFSv3 still works

[ahmadabdalla]

@robbertwortell I've done initial investigations on this, and would like to add the following:

I've created a small ARM template with the following:

Template

{
    "$schema": "http://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#",
    "contentVersion": "1.0.0.0",
    "parameters": {
        "location": {
            "type": "String"
        },
        "storageAccountName": {
            "type": "String"
        },
        "accountType": {
            "type": "String"
        },
        "kind": {
            "type": "String"
        },
        "accessTier": {
            "type": "String"
        },
        "publicNetworkAccess": {
            "type": "String"
        },
        "networkAclsBypass": {
            "type": "String"
        },
        "networkAclsDefaultAction": {
            "type": "String"
        },
        "isHnsEnabled": {
            "type": "Bool"
        },
        "isNfsV3Enabled": {
            "type": "Bool"
        },
        "isSftpEnabled": {
            "type": "Bool"
        }
    },
    "variables": {},
    "resources": [
        {
            "type": "Microsoft.Storage/storageAccounts",
            "apiVersion": "2022-05-01",
            "name": "[parameters('storageAccountName')]",
            "location": "[parameters('location')]",
            "dependsOn": [],
            "tags": {},
            "sku": {
                "name": "[parameters('accountType')]"
            },
            "kind": "[parameters('kind')]",
            "properties": {
                "accessTier": "[parameters('accessTier')]",
                "publicNetworkAccess": "[parameters('publicNetworkAccess')]",
                "networkAcls": {
                    "bypass": "[parameters('networkAclsBypass')]",
                    "defaultAction": "[parameters('networkAclsDefaultAction')]",
                    "ipRules": []
                },
                "isHnsEnabled": "[parameters('isHnsEnabled')]",
                "isNfsV3Enabled": "[parameters('isNfsV3Enabled')]",
                "isSftpEnabled": "[parameters('isSftpEnabled')]"
            }
        }
    ],
    "outputs": {}
}

Parameters

{
    "$schema": "https://schema.management.azure.com/schemas/2015-01-01/deploymentParameters.json#",
    "contentVersion": "1.0.0.0",
    "parameters": {
        "location": {
            "value": "westeurope"
        },
        "storageAccountName": {
            "value": "teststoragecarmls"
        },
        "accountType": {
            "value": "Standard_LRS"
        },
        "kind": {
            "value": "StorageV2"
        },
        "accessTier": {
            "value": "Hot"
        },
        "publicNetworkAccess": {
            "value": "Enabled"
        },
        "networkAclsBypass": {
            "value": "AzureServices"
        },
        "networkAclsDefaultAction": {
            "value": "Deny"
        },
        "isHnsEnabled": {
            "value": true
        },
        "isNfsV3Enabled": {
            "value": true
        },
        "isSftpEnabled": {
            "value": false
        }
    }
}

I can deploy this template over and over without issues, meaning that the isNfsV3Enabled property is not impacting the template deployment.

Is there something in our module that may cause this issue? Can you please help share more context on the error you are getting please?

[ahmadabdalla]

Added a comment in the PR

[ghost]

@ahmadabdalla

The issue seems to be with an existing storage account created without sending the NFSv3 parameter.
And then trying to send the NFSv3 paramter (either true or false)

so the path is :
scenario 1:
tag 0.9:
SA is created without param NFSv3 (if you export you don't see this)

scenario 2:
tag 0.10:
SA will be created with NFSv3 (true or false)

scenario 3:
SA created with 0.9 and there after the modules are upgraded from 0.9>0.10
this fails as MS does not allow to add this param.
Current solotion is todo an union, to only add this parameter condional,
But maybe the same solution as this following example could also work for NFSv3?

isHnsEnabled: enableHierarchicalNamespace ? enableHierarchicalNamespace : null

so:
isNFSv3Enabled: isNfsV3Enabled? isNfsV3Enabled: null

[ahmadabdalla]

OK now I see where this is happening and my updated testing criteria

Test Scenarios

v1

I've used the same template above but with NO NFS properties, and then stored it as v1 template spec.

v2

Updated that template to include NFS properties like our CARML pattern to set it to false if left default, then stored it as v2 template spec.

    isHnsEnabled: enableHierarchicalNamespace ? enableHierarchicalNamespace : null
    isSftpEnabled: enableSftp
    isNfsV3Enabled: enableNfsV3

v3

Updated that template to now set the NFS properties as null if not set to true then stored it as v3:

    isHnsEnabled: enableHierarchicalNamespace ? enableHierarchicalNamespace : null
    isSftpEnabled: enableSftp ? enableSftp : null
    isNfsV3Enabled: enableNfsV3 ? enableNfsV3 : null

Observations:

• Deploying a storage account using V1 is SUCCESS. Then querying the Storage account using REST, the NFS properties are not even returned.
• Deploying the same storage account again using V2 will FAIL the deployment.

{
      "code": "AccountPropertyCannotBeUpdated",
      "message": "The property 'isNfsV3Enabled' was specified in the input, but it cannot be updated as it is read-only. For more information, see - https://aka.ms/storageaccountupdate"
}

• Deploying the same storage account where NFS properties are default to false but are now set to null as per the template logic is SUCCESS

This indicates we do not need to perform the union function to maintain backwards compatibility between 0.9 and 0.10.

What are your thoughts here?

[ghost]

@ahmadabdalla i assumed (wrongly) that your V3 didn't work as the original was added differently then all the others.
But i never tested this.
But for sure V3 would be the best. (uses the same logic and layout as the rest and is most readable.
Ill update the code

[ahmadabdalla]

@robbertwortell how did you go with the tests?

[ahmadabdalla]

@robbertwortell there are new breaking changes to the CARML CI and module folder structure that has recently been merged. Can you please sync from main and re-apply your changes please. If you'd like me to support you please let me know :) :)

[ghost]

@ahmadabdalla i moved to a personal account
this pull request can be ignored (will remove later and checkout the new PR)
#3290