[Feature Request]: AVM alignment tracker #4020

AlexanderSehr · 2023-09-25T11:43:30Z

Description

This issue is intended to track the efforts for alinging the CARML modules to the Azure Verified Module (AVM) specs, ultimately enabling us to publish the CARML modules to the Public Bicep Registry.

Bulk edits

These can ideally be updated 'on scale', with the already defined interfaces & conventions (e.g., folder structure), that must be updated in all modules (while taking individual characterists into account).

Note: Some of these changes may require changes to the CI environment.

Checklist

Per-module edits

Side-by-side / following the alignment to the extension interface, the following list should be used to track the 'full' alignment of the modules (e.g., PSRule compliance, etc.)

Checklist

Migration Guide

This section provides a checklist of things to look out for per module to ensure they're AVM compliant, both as per module specifications & the Contribution Guide.

Checklist

Tests (ref)
- Rename test folder and add nested e2e folder (ref)
- Rename min folder to defaults
- Rename common folder to max
- Add waf-aligned folder (e.g., based on common). This test should not fail PSRule & show the module being deployed with best-practices
- For each folder,
- Update the serviceShort parameter to align with the new naming (e.g., waf for waf-aligned). For now, we should continue using min for defaults to align with PSRule.
- Update the namePrefix input parameter value from [[namePrefix]] to #_namePrefix_# (the reason being that Bicep has a compilation issue because of the prefix & suffix in another location)
- Update the ../../main.bicep module template reference to ../../../main.bicep
- If a resource group is deployed, update the RG parameter name to the new format that also uses the namePrefix. For example:
```
@description('Optional. The name of the resource group to deploy for testing purposes.') 
@maxLength(90)
param resourceGroupName string = 'dep-${namePrefix}-network.privateendpoints-${serviceShort}-rg'
```
- (Optionally) add a block like the following below the target scope to render a more meaningful example in the ReadMe
```
metadata name = 'Using only defaults'
metadata description = 'This instance deploys the module with the minimum set of required parameters.'
```
- You should also try and test idempotency if possible. You can do this by updating the test invocation to
```
@batchSize(1)
module testDeployment '../../../main.bicep' = [for iteration in [ 'init', 'idem' ]: {
  scope: resourceGroup
  name: '${uniqueString(deployment().name, location)}-test-${serviceShort}-${iteration}'
  (...)
}]
```
For each module that supports
Diagnostic Settings

Reference to AVM specs
- Add the diagnosticSettingType described in the above reference to a // Definitions block at the bottom of the template file
- Remove any of the current diagnosticSetting parameters & variables
- Add the the new diagnosticSettings parameter as per the specs to the template
- Updated the deployment block as per the specs to enable it to work with the new object type
- Check if any of the tests must be updated. The new block may look like
```
diagnosticSettings: [
  {
    name: 'customSetting'
    eventHubName: diagnosticDependencies.outputs.eventHubNamespaceEventHubName
    eventHubAuthorizationRuleResourceId: diagnosticDependencies.outputs.eventHubAuthorizationRuleId
    storageAccountResourceId: diagnosticDependencies.outputs.storageAccountResourceId
    workspaceResourceId: diagnosticDependencies.outputs.logAnalyticsWorkspaceResourceId
  }
]
```
NOTE: ⚠️ Make sure that if the module does not support e.g. metrics, that you update the logic accordingly
Role Assignments

Reference to AVM specs
- Add the roleAssignmentType described in the above reference to a // Definitions block at the bottom of the template file
- Update the current roleAssignments parameter as per the specs (- should now reference the User-defined-type)
- Take the current list of builtInRoleNames from the nested_roleAssignments.bicep file and add them to the variables block of the main template. The new schema does not require the nested template. Also, reduce the list of specified roles to only those that make sense for this resource (ref)/ For, for example, Cognitive Services, we should only provide the important ones as Owner, Contributor, etc. + all service specific roles such as 'Cognitive Services User'.
- Replace the original module deployment block with the new resource deployment block
- Check if any of the tests must be updated. The new block may look like
```
roleAssignments: [
  {
    roleDefinitionIdOrName: 'Reader'
    principalId: nestedDependencies.outputs.managedIdentityPrincipalId
    principalType: 'ServicePrincipal'
  }
]
```
Resource Locks

Reference to AVM specs
- Add the lockType described in the above reference to a // Definitions block at the bottom of the template file
- Update the current lock parameter as per the specs (- should now reference the User-defined-type)
- Updated the deployment block as per the specs to enable it to work with the new object type
- Check if any of the tests must be updated. The new block may look like
```
lock: {
  kind: 'CanNotDelete'
  name: 'myCustomLockName'
}
```
Tags

Reference to AVM specs
- Update the current tags parameter as per the specs
Managed Identities

Reference to AVM specs
- Add the managedIdentitiesType described in the above reference to a // Definitions block at the bottom of the template file
- Remove any of the current identity parameters & variables
- Add the the new managedIdentities parameter as per the specs to the template
- Updated the deployment block as per the specs to enable it to work with the new object type
- Check if any of the tests must be updated. The new block may look like
```
managedIdentities: {
  systemAssigned: true
  userAssignedResourcesIds: [
    nestedDependencies.outputs.managedIdentityResourceId
  ]
}
```
NOTE: ⚠️ Make sure that if the module does not support e.g. user-assigned-identities, that you update the logic accordingly
Private Endpoints

Reference to AVM specs
- Add the privateEndpointType described in the above reference to a // Definitions block at the bottom of the template file
- Update the current privateEndpoints parameter as per the specs (- should now reference the User-defined-type)
- Updated the deployment block as per the specs to enable it to work with the new object type.
  
  Note: For any resource that only supports one service/groupID (e.g. 'vault' for KeyVault, but NOT 'blob' for StorageAccount) we can provide a default value for that property (hence there are 2 variants in the spec).
- Check if any of the tests must be updated. The new block may look like
```
privateEndpoints: [
  {
    privateDnsZoneResourceIds: [
      nestedDependencies.outputs.privateDNSZoneResourceId
    ]
    subnetResourceId: nestedDependencies.outputs.subnetResourceId
    tags: {
      'hidden-title': 'This is visible in the resource name'
      Environment: 'Non-Prod'
      Role: 'DeploymentValidation'
    }
  }
]
```
Customer Managed Keys

Reference to AVM specs
- Add the customerManagedKeyType described in the above reference to a // Definitions block at the bottom of the template file
- Remove any of the current customer-managed-key parameters & variables
- Add the the new customerManagedKey parameter as per the specs to the template
- Update the existing resource references as per the specs
- Updated the deployment block as per the specs to enable it to work with the new object type
  - ⚠️ BEWARE module-specific differences
  - Note also that the new schema SHOULD support system-assigned-identities. As this cannot be done in a single deployment, you can find a reference how this would look like here
- Check if any of the tests must be updated. The new block may look like
```
customerManagedKey: {
  keyName: nestedDependencies.outputs.keyVaultKeyName
  keyVaultResourceId: nestedDependencies.outputs.keyVaultResourceId
  userAssignedIdentityResourceId: nestedDependencies.outputs.managedIdentityResourceId
}
```
NOTE: ⚠️ Make sure that if the module does not support e.g. metrics, that you update the logic accordingly

Other

Set version in version.json back to 0.1

(Optional) Introduce the new nullable feature for parameters where-ever it makes sense to you (and ensure to test it). This enables us to simplify logic like in the following example

// Old
param attributesExp int = -1
resource secret 'Microsoft.KeyVault/vaults/secrets@2022-07-01' = {
  (...)
  properties: {
    attributes: {
      exp: attributesExp != -1 ? attributesExp : null
    }
  }
}

// New
param attributesExp int?
resource secret 'Microsoft.KeyVault/vaults/secrets@2022-07-01' = {
  (...)
  properties: {
    attributes: {
      exp: attributesExp
    }
  }
}

ReadMe
- Regenerate all module ReadMe's & compile all module Bicep templates from the ground up. Ideally, remove the ReadMEs and regenerate them completely. Take note of any extra content (e.g., 'considerations') that were added manually and add them to the module's description metadata in the respective main.bicep file

Helper script (work in progress)

Snippet

#region helper functions

function Convert-Folders {

    [CmdletBinding()]
    param (
        [Parameter(Mandatory = $true)]
        [string] $FolderPath
    )

    # .bicep
    (Get-ChildItem -Path $FolderPath -Directory -Recurse -Filter '.bicep').FullName | Where-Object { $_ } | ForEach-Object {
        if (-not (Test-Path (Join-Path (Split-Path $_) 'modules'))) {
            $null = Rename-Item -Path $_ -NewName 'modules' -Force
        }
    }

    # .test
    # Add [e2e] folder
    (Get-ChildItem -Path $FolderPath -Directory -Recurse -Filter '.test').FullName | ForEach-Object {
        $newPath = Join-Path $_ 'e2e'
        if (-not (Test-Path $newPath)) {
            $null = New-Item -ItemType 'Directory' -Path $newPath
        }
    }

    # Move tests to new sub folder
    $testTopFolders = (Get-ChildItem -Path $FolderPath -Directory -Recurse -Filter '.test').FullName

    foreach ($testfolderPath in $testTopFolders) {

        $testCaseFolderPaths = (Get-ChildItem $testfolderPath -Directory -Exclude 'e2e').FullName

        foreach ($testFolderPath in $testCaseFolderPaths) {

            $expectedFolderName = Split-Path $testfolderPath -Leaf

            $newTestFolderPath = Join-Path (Split-Path $testFolderPath -Parent) 'e2e' $expectedFolderName
            if (-not (Test-Path $newTestFolderPath)) {
                $null = New-Item -ItemType 'Directory' -Path $newTestFolderPath
            }

            $originalTestFilePaths = Get-ChildItem $testfolderPath -File -Recurse

            foreach ($originalTestFilePath in $originalTestFilePaths) {
                $null = Move-Item -Path $originalTestFilePath -Destination $newTestFolderPath -Force
            }

            # If default folder [min/common], rename to new names [defaults/max]
            switch ($expectedFolderName) {
                'min' {
                    if (-not (Test-Path (Join-Path (Split-Path $newTestFolderPath) 'defaults'))) {
                        $null = Rename-Item -Path $newTestFolderPath -NewName 'defaults' -Force
                    }
                }
                'common' {
                    if (-not (Test-Path (Join-Path (Split-Path $newTestFolderPath) 'max'))) {
                        $null = Rename-Item -Path $newTestFolderPath -NewName 'max' -Force
                    }
                }
                Default {}
            }

            # Delete original folder
            $null = Remove-Item -Path $testFolderPath -Force
        }
    }

    # Rename test folder
    (Get-ChildItem -Path $FolderPath -Directory -Recurse -Filter '.test').FullName | Rename-Item -NewName 'tests'

    # Generate WAF folder
    $wafFolderPath = Join-Path $FolderPath 'tests' 'e2e' 'waf-aligned'
    if (-not (Test-Path $wafFolderPath)) {
        # Duplicate 'max' test folder
        $null = New-Item -Path $wafFolderPath -ItemType 'Directory' -Force
        $maxTestPath = Join-Path $FolderPath 'tests' 'e2e' 'max'
        $null = Copy-Item -Path "$maxTestPath/*" -Destination $wafFolderPath -Recurse
    }
}

function Set-ReadMe {

    [CmdletBinding()]
    param (
        [Parameter(Mandatory = $true)]
        [string] $FolderPath
    )

    $readMePaths = (Get-ChildItem -Path $FolderPath -File -Recurse -Filter 'readme.md').FullName

    # Remove original ReadMes
    $readMePaths | ForEach-Object { $null = Remove-Item -Path $_ -Force }

    # Regenerate new ReadMes
    # Set-AVMModule -ModuleFolderPath $FolderPath -Recurse
}

function Set-VersionFile {

    [CmdletBinding()]
    param (
        [Parameter(Mandatory = $true)]
        [string] $FolderPath
    )

    $versionPaths = (Get-ChildItem -Path $FolderPath -File -Recurse -Filter 'version.json').FullName

    foreach ($path in $versionPaths) {
        $originalContent = Get-Content -Path $path -Raw | ConvertFrom-Json
        $originalContent.version = '0.1'
        $originalContent | ConvertTo-Json -Depth 100 | Set-Content -Path $path -Force
    }
}

function Set-TestFiles {

    [CmdletBinding()]
    param (
        [Parameter(Mandatory = $true)]
        [string] $FolderPath
    )

    $testFilePaths = (Get-ChildItem -Path $FolderPath -File -Recurse -Filter 'main.test.bicep').FullName
    $testFolderPaths = $testFilePaths | ForEach-Object { Split-Path $_ }

    # [1] Update service shorts
    foreach ($testFolderPath in $testFolderPaths) {

        $testFolderName = Split-Path $testFolderPath -Leaf

        # If default folder [min/common], rename to new names [defaults/max]
        switch ($testFolderName) {
            'defaults' {
                $testContent = Get-Content (Join-Path $testFolderPath 'main.test.bicep') -Raw
                # Should remain `min` for now to work with PSRule config
                # $testContent = $testContent -replace "(param serviceShort string = '[a-zA-Z]+)min'", ("`$1{0}'" -f 'min')
                $null = Set-Content (Join-Path $testFolderPath 'main.test.bicep') -Value $testContent -Force
            }
            'max' {
                $testContent = Get-Content (Join-Path $testFolderPath 'main.test.bicep') -Raw
                $testContent = $testContent -replace "(param serviceShort string = '[a-zA-Z]+)com'", ("`$1{0}'" -f 'max')
                $null = Set-Content (Join-Path $testFolderPath 'main.test.bicep') -Value $testContent -Force
            }
            'waf-aligned' {
                $testContent = Get-Content (Join-Path $testFolderPath 'main.test.bicep') -Raw
                $testContent = $testContent -replace "(param serviceShort string = '[a-zA-Z]+)com'", ("`$1{0}'" -f 'waf')
                $null = Set-Content (Join-Path $testFolderPath 'main.test.bicep') -Value $testContent -Force
            }
            Default {}
        }
    }

    foreach ($testFilePath in $testFilePaths) {
        $testContent = Get-Content $testFilePath -Raw

        # [2] Remove telemetry
        $testContent = $testContent -replace "@description\('.+\(GUID\)\.'\)(\r\n|\r|\n){1}param enableDefaultTelemetry.+(\r\n|\r|\n){2}", ''
        $testContent = $testContent -replace '.*enableDefaultTelemetry: enableDefaultTelemetry.*(\r\n|\r|\n){1}', ''

        # [3] Update RG name
        $testContent = $testContent -replace "(param resourceGroupName string = ')ms.(.+)", '$1dep-${namePrefix}-$2'

        # [4] Idempotency & main.bicep reference
        $testContent = $testContent -replace "module testDeployment '\.\.\/\.\.\/main\.bicep' = {", "@batchSize(1)`r`nmodule testDeployment '../../../main.bicep' = [for iteration in [ 'init', 'idem' ]: {"
        $testContent = $testContent -replace "name: '\`${uniqueString\(deployment\(\)\.name, location\)}-test-\`${serviceShort}'", "name: '`${uniqueString(deployment().name, location)}-test-`${serviceShort}-`${iteration}'"

        # [5] diagnosticsTemplateReference
        $testContent = $testContent -replace '.+diagnostic.dependencies.bicep', "module diagnosticDependencies '../../../../../../utilities/e2e-template-assets/templates/diagnostic.dependencies.bicep"

        # [6] Replace namePrefix token with new format
        $testContent = $testContent -replace '\[\[namePrefix\]\]', '#_namePrefix_#'

        $null = Set-Content -Path $testFilePath -Value $testContent -Force
    }
}
#endregion

function Convert-CarmlToAvm {

    [CmdletBinding()]
    param (
        [Parameter(Mandatory = $true)]
        [string] $FolderPath,

        [Parameter(Mandatory = $false)]
        [string] $RepoRoot = 'C:\dev\ip\Azure-ResourceModules\ResourceModules'
    )

    # Load external functions
    # . (Join-Path $RepoRoot 'utilities' 'tools' 'Set-AVMModule.ps1')

    # [1] Convert folders
    Convert-Folders -FolderPath $FolderPath

    # [2] Set version.json back to 0.1
    Set-VersionFile -FolderPath $FolderPath

    # [3] Test file changes
    Set-TestFiles -FolderPath $FolderPath

    # [4] Regenerate ReadMe
    Set-ReadMe -FolderPath $FolderPath

}
Convert-CarmlToAvm -FolderPath 'C:\dev\ip\Azure-ResourceModules\ResourceModules\modules\key-vault\vault'

Final steps: Migration

For the final AVM contribution a few more changes will be necessary as described in the following

Update the reference to the diagnostic dependencies file in each test file that uses it to avm/utilities/e2e-template-assets/templates/diagnostic.dependencies.bicep
Update the telemetry implementation as per it's new schema (ref). If the module references any external module, make sure to pass the enableTelemetry flag through like enableTelemetry: enableTelemetry to enable users to enable/disable for the entire deployment. Child resources should remain with the telemetry switched off for now.
Remove telemetry from test cases
Remove version.json files from child modules (as can't publish them yet)
Align app/managed-environment to AVM specs #4409

This module has already been migrated to AVM. Only the AVM version is expected to receive updates / new features. Please do not work on improving this module in CARML.
If the migrated module will be orphaned, add a ORPHANED.md file with the following content to the module in AVM only

This module is currently orphaned. Only security and bug fixes are being handled by the AVM core team at present. If interested in becoming a module owner (must be Microsoft FTE) for this orphaned module please comment on the issue here

These changes should be done after creating a fork of the public bicep registory respository and the module is added in the /avm/res/ folder. From there you can not only test the module using the AVM CI, but also open the Pull Request to the Upstream repository.

The text was updated successfully, but these errors were encountered:

ChrisSidebotham · 2023-10-12T15:58:40Z

@AlexanderSehr - Defaults will need to be serviceShort = min for PSRule to validate the non use of tags & Zones

AlexanderSehr added the enhancement New feature or request label Sep 25, 2023

AlexanderSehr self-assigned this Sep 25, 2023

AlexanderSehr added documentation Improvements or additions to documentation [cat] pipelines category: pipelines [cat] modules category: modules [cat] testing category: testing [cat] utilities category: utilities labels Sep 25, 2023

AlexanderSehr pinned this issue Sep 25, 2023

AlexanderSehr assigned AlexanderSehr and unassigned AlexanderSehr Oct 2, 2023

AlexanderSehr added the help wanted Extra attention is needed label Oct 8, 2023

rahalan mentioned this issue Oct 12, 2023

Align insights-actiongroup to AVM specs #4074

Closed

AlexanderSehr linked a pull request Oct 18, 2023 that will close this issue

[AVM] Updated Readme's to support AVM transition - Part (1) #4072

Merged

10 tasks

eriqua mentioned this issue Oct 22, 2023

[CI environment] Add nameprefix to rg test names #4130

Merged

10 tasks

eriqua closed this as completed in #4130 Oct 22, 2023

AlexanderSehr reopened this Oct 22, 2023

JPEasier mentioned this issue Nov 6, 2023

Align container-service/managed-cluster to AVM specs #4194

Closed

AlexanderSehr mentioned this issue Feb 7, 2024

[Feature request from Discussions] RFC: Microsoft.Web/sites should retain existing app configuration over deployment Azure/bicep-registry-modules#949

Open

arnoldna mentioned this issue Feb 20, 2024

Moved from CARML to AVM #4501

Merged

8 tasks

elanzel mentioned this issue Feb 22, 2024

Align automation/automation-account to AVM specs #4311

Closed

47 tasks

AlexanderSehr linked a pull request May 19, 2024 that will close this issue

CARML - AVM module transition complete #4541

Open

10 tasks

AlexanderSehr added this to the Azure Verified Modules (AVM) - V1 milestone May 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature Request]: AVM alignment tracker #4020

[Feature Request]: AVM alignment tracker #4020

AlexanderSehr commented Sep 25, 2023 •

edited

ChrisSidebotham commented Oct 12, 2023

[Feature Request]: AVM alignment tracker #4020

[Feature Request]: AVM alignment tracker #4020

Comments

AlexanderSehr commented Sep 25, 2023 • edited

Description

Bulk edits

Per-module edits

Migration Guide

Helper script (work in progress)

Final steps: Migration

ChrisSidebotham commented Oct 12, 2023

AlexanderSehr commented Sep 25, 2023 •

edited