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

Add DdosEnabled toggle and fix logic modPolicyAssignmentConnEnableDdos #810

Merged
merged 8 commits into from
Jul 9, 2024

Conversation

oZakari
Copy link
Contributor

@oZakari oZakari commented Jul 9, 2024

Overview/Summary

Closed #701 but copied over PR form as author is on PTO and I am unable to make necessary changes into their PR branch.

Closes Bug #596 and @jamiepla1 flagged this. Policy issue around parDdosProtectionPlanId, because by default parDdosProtectionPlanId is populated during creation of ALZ-Bicep Accelerator. If you don't clean (empty) or catch this, it will deploy a policy Enable-DDoS-VNET. This will block the creation of Virtual Networks in the Connectivity Subscription if you disable/false parDdosEnabled in other areas of ALZ-Bicep. To get around this, simply adding the boolean/toggle of parDdosEnabled will now make this not enable by default even if parDdosProtectionPlanId. Making this uniformed across the ALZ-Bicep deployment.

This PR fixes/adds/changes/removes

@sys.description('Switch to enable/disable DDoS Network Protection deployment. True will enforce policy Enable-DDoS-VNET at connectivity or landing zone Management Groups. False will not enforce policy Enable-DDoS-VNET.')
param parDdosEnabled bool = true

Within the Module modPolicyAssignmentConnEnableDdosVnet the variable parPolicyAssignmentIdentityType, parDisableAlzDefaultPolicies was removed in favor of having parDdosEnabled. This allows the policy to be added, but not inforced if parDdosEnabled is false. If the end-user forgets to set parDdosEnabled, then they will get an error when deploying networking within Connectivity Subscription.

parPolicyAssignmentEnforcementMode: !parDdosEnabled ? 'DoNotEnforce' : varPolicyAssignmentEnableDDoSVNET.libDefinition.properties.enforcementMode

When deploying Hub in Connectivity Subscription the following happens if Enable-DDoS-VNET is enforced

The deployment 'alz-Hub-and-SpokeDeploy-20231209T1712405048Z'
     | failed with error(s). Showing 1 out of 1 error(s). Status Message:
     | Resource
     | /subscriptions/*******-****-****-****-************/resourceGroups/rg-alz-connectivity/providers/Microsoft.Network/ddosProtectionPlans/alz-ddos-plan referenced by resource /subscriptions/*******-****-****-****-************/resourceGroups/rg-alz-connectivity/providers/Microsoft.Network/virtualNetworks/alz-hub-swedencentral was not found. Please make sure that the referenced resource exists. (Code: InvalidGlobalResourceReference)

**Disclaimer
It should be documented that DDoS Protection is a recommendation, therefor the policy is in a non-compliant state as when it is turned to parDdosEnabled = false, as it is not being enforced.
image

Breaking Changes

  1. N/A

Testing Evidence

image

As part of this Pull Request I have

@oZakari oZakari changed the title Fix_modPolicyAssignmentConnEnableDdos Add DdosEnabled toggle and fix logic modPolicyAssignmentConnEnableDdos Jul 9, 2024
@oZakari oZakari requested a review from jtracey93 July 9, 2024 04:08
@oZakari oZakari added Type: Bug 🪲 Something isn't working Area: Policy 📝 Issues / PR's related to Policy Area: Networking 🌐 Issues / PR's related to Networking labels Jul 9, 2024
@jtracey93
Copy link
Collaborator

/azp run validateazcloud

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@oZakari oZakari merged commit 5759b89 into Azure:main Jul 9, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Networking 🌐 Issues / PR's related to Networking Area: Policy 📝 Issues / PR's related to Policy Type: Bug 🪲 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants