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

💡 Feature Request - Flexibility to exclude individual ALZ default policy assignments #481

Closed
stalejohnsen opened this issue Mar 23, 2023 · 4 comments · Fixed by #482
Closed
Labels
Needs: Attention 👋 Needs attention from the maintainers

Comments

@stalejohnsen
Copy link
Contributor

Describe the solution you'd like

The module alzDefaultPolicyAssignments assigns all the recommended ALZ reference architecture policies but it would be great to have the flexibility to exclude individual policy assignments as parameter, similar to how ALZ terraform module can do exclusion on the built in archetypes.

If one of the ALZ reference architecture polices does not fit the requirements for an organization you will have to modify the alzDefaultPolicyAssignments module, but that is something you might not want to do if you also want to have a easy way to stay up to date with new versions of ALZ Bicep, ref this guidance:

Option 2 will make it easier to pull in updates from ALZ-Bicep for the ALZ Default Policy Assignments module if there are any >changes to the defaults and will reduce chances of merge conflicts and manual remediation to merge them. https://github.com/Azure/ALZ-Bicep/wiki/AssigningPolicies

By having the option to exclude policy assignments from ALZ default assignments module (like we already do for DDoS Protection) in the parameters file only, no modifications of the official module is required.

Describe alternatives you've considered

Having a custom copy of the alzDefaultPolicyAssignments module allows modifications without editing the official upstream version but this is unnecessary management overhead. Will use separate policy assignments orchestration module for other additional policy assignments.

Additional context

@ghost ghost added the Needs: Triage 🔍 Needs triaging by the team label Mar 23, 2023
@jtracey93
Copy link
Contributor

Good ask @stalejohnsen and may park this one until we release the accelerator as it may address this. Should be ready in a few weeks, we are finishing docs and testing 👍

However, one thought I've had which keeps it super simple, and handles git merges in a good way, would be to simply just comment out an assignment as per the below example:

// Modules - Policy Assignments - Intermediate Root Management Group
// Module - Policy Assignment - Deploy-MDFC-Config
module modPolicyAssignmentIntRootDeployMdfcConfig '../../../policy/assignments/policyAssignmentManagementGroup.bicep' = {
  scope: managementGroup(varManagementGroupIds.intRoot)
  name: varModuleDeploymentNames.modPolicyAssignmentIntRootDeployMdfcConfig
  params: {
    parPolicyAssignmentDefinitionId: varPolicyAssignmentDeployMDFCConfig.definitionId
    parPolicyAssignmentName: varPolicyAssignmentDeployMDFCConfig.libDefinition.name
    parPolicyAssignmentDisplayName: varPolicyAssignmentDeployMDFCConfig.libDefinition.properties.displayName
    parPolicyAssignmentDescription: varPolicyAssignmentDeployMDFCConfig.libDefinition.properties.description
    parPolicyAssignmentParameters: varPolicyAssignmentDeployMDFCConfig.libDefinition.properties.parameters
    parPolicyAssignmentParameterOverrides: {
      emailSecurityContact: {
        value: parMsDefenderForCloudEmailSecurityContact
      }
      ascExportResourceGroupLocation: {
        value: parLogAnalyticsWorkSpaceAndAutomationAccountLocation
      }
      logAnalytics: {
        value: parLogAnalyticsWorkspaceResourceId
      }
    }
    parPolicyAssignmentIdentityType: varPolicyAssignmentDeployMDFCConfig.libDefinition.identity.type
    parPolicyAssignmentIdentityRoleDefinitionIds: [
      varRbacRoleDefinitionIds.owner
    ]
    parPolicyAssignmentEnforcementMode: parDisableAlzDefaultPolicies ? 'DoNotEnforce' : varPolicyAssignmentDeployMDFCConfig.libDefinition.properties.enforcementMode
    parTelemetryOptOut: parTelemetryOptOut
  }
}

// Commented out to not assign due to reason XYZ
// Module - Policy Assignment - Deploy-AzActivity-Log
//module modPolicyAssignmentIntRootDeployAzActivityLog '../../../policy/assignments/policyAssignmentManagementGroup.bicep' = {
//  scope: managementGroup(varManagementGroupIds.intRoot)
//  name: varModuleDeploymentNames.modPolicyAssignmentIntRootDeployAzActivityLog
//  params: {
//    parPolicyAssignmentDefinitionId: varPolicyAssignmentDeployAzActivityLog.definitionId
//    parPolicyAssignmentName: varPolicyAssignmentDeployAzActivityLog.libDefinition.name
//    parPolicyAssignmentDisplayName: varPolicyAssignmentDeployAzActivityLog.libDefinition.properties.displayName
//    parPolicyAssignmentDescription: varPolicyAssignmentDeployAzActivityLog.libDefinition.properties.description
//    parPolicyAssignmentParameters: varPolicyAssignmentDeployAzActivityLog.libDefinition.properties.parameters
//    parPolicyAssignmentParameterOverrides: {
//      logAnalytics: {
//        value: parLogAnalyticsWorkspaceResourceId
//      }
//    }
//    parPolicyAssignmentIdentityType: varPolicyAssignmentDeployAzActivityLog.libDefinition.identity.type
//    parPolicyAssignmentIdentityRoleDefinitionIds: [
//      varRbacRoleDefinitionIds.owner
//    ]
//    parPolicyAssignmentEnforcementMode: parDisableAlzDefaultPolicies ? 'DoNotEnforce' : varPolicyAssignmentDeployAzActivityLog.libDefinition.properties.enforcementMode
//    parTelemetryOptOut: parTelemetryOptOut
//  }
//}

Thoughts?

@jtracey93 jtracey93 added Needs: Author Feedback and removed Needs: Triage 🔍 Needs triaging by the team labels Mar 23, 2023
@stalejohnsen
Copy link
Contributor Author

Thank you for your feedback, @jtracey93. Accelerator will definitively help with the separation of customization and release upgrades but in this situation I think I still would have to have a customized version of alzDefaultPolicyAssignments? Comments would be my alternative to solve this right now but an exclude parameter would be my preferred solution as that would be zero module customization.

@ghost ghost added Needs: Attention 👋 Needs attention from the maintainers and removed Needs: Author Feedback labels Mar 23, 2023
@jtracey93
Copy link
Contributor

Going to close this as we added the flexibility for the troublesome policy for the customer, thanks @stalejohnsen, in #482.

We also provided a workaround in the comment #481 (comment)

@jtracey93 jtracey93 linked a pull request Mar 24, 2023 that will close this issue
10 tasks
@stalejohnsen
Copy link
Contributor Author

Hi @jtracey93. I think this feature request could be more relevant again with the latest policy refresh. I personally really like all the great work that has been done in this refresh with new policies and update of existing ones, but when we add more opiniated alz default policies the chances that organization need to exclude some of them will increase. The portal experience and alz terraform has a user friendly way of excluding specific policies, I would like to see something similar in ALZ bicep without having to customize alzDefaultPolicyAssignments.

I did a quick mockup here, what do you think? https://github.com/stalejohnsen/ALZ-Bicep-fork/commit/cc009d3b8e4ac80abc4953d77016bd294cbb54c8

As an example, adding the assignment names from the lib definition to this new parameter would then give the exclude flexibility:
"parExcludeAlzDefaultPolicies": {
"value": ["Deploy-VM-Backup","Deploy-VM-Monitoring","Deploy-VMSS-Monitoring"]
}

@ghost ghost locked as resolved and limited conversation to collaborators May 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Needs: Attention 👋 Needs attention from the maintainers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants