Skip to content
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

Align authorization/policy-assignment to AVM specs #4122

Closed
45 tasks
elanzel opened this issue Oct 21, 2023 · 2 comments
Closed
45 tasks

Align authorization/policy-assignment to AVM specs #4122

elanzel opened this issue Oct 21, 2023 · 2 comments
Labels
blocked if an issue is blocked

Comments

@elanzel
Copy link
Contributor

elanzel commented Oct 21, 2023

To do

Checklist
  1. 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)
    • Remove the enableDefaultTelemetry parameter and reference in the test
    • 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}'
        (...)
      }]
  2. 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

  3. 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
          }
        }
      }
  4. 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
@AlexanderSehr
Copy link
Contributor

Put on hold as it's a special beast and we may or may not want to publish it as is in the first place. The main value comes through it's child modules and those are not published in AVM.

@AlexanderSehr
Copy link
Contributor

AlexanderSehr commented May 19, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked if an issue is blocked
Projects
None yet
Development

No branches or pull requests

2 participants