Skip to content

Azure DDoS protection #107#114

Merged
hosungsmsft merged 4 commits into
Azure:masterfrom
fbouteruche:ddos-protection
May 18, 2018
Merged

Azure DDoS protection #107#114
hosungsmsft merged 4 commits into
Azure:masterfrom
fbouteruche:ddos-protection

Conversation

@fbouteruche
Copy link
Copy Markdown

Pull request for the issue #107

Add ddosSwitch parameter to enable the deployment of a Azure DDoS protection plan in azuredepoy.json

Add a ddosWitch member in the moodleCommon object.
Add a nested template network-vnet-ddos.json that manages the creation
of ddos protection plan and the update of the vnet.
Call network-vnet-ddos.json from the network.json template if ddosSwith is true

In the .gitignore file, exlude .vs to ignore Visual Studio stuff.

…tection plan in azuredepoy.json

Add a ddosWitch member in the moodleCommon object.
Add a nested template network-vnet-ddos.json that manages the creation
of ddos protection plan and the update of the vnet.
Call network-vnet-ddos.json from the network.json template if ddosSwith is true

In the .gitignore file, exlude .vs to ignore Visual Studio stuff.
Copy link
Copy Markdown

@hosungsmsft hosungsmsft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really great! Just a really minor change requested and one small reconfirmation requested as well.

I think it'll be also great if you can add a documentation (like docs/ddos.md) that explains how to add/configure a DDoS protection plan when a cluster was deployed without the DDoS feature turned on, but I'd say that's purely optional and up to you. Thanks much again!

"[resourceId('Microsoft.Network/ddosProtectionPlans', parameters('moodleCommon').ddosPlanName)]"
],
"location": "[resourceGroup().location]",
"name": "[parameters('vnetName')]",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. So you did confirm that this works regardless of whether this vnet already exists or is to be created, right?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I've ran 4 tests

  1. existing vNet and ddosSwitch equal to true
  2. existing vNet and ddosSwitch equal to false
  3. no existing vNet and ddosSwitch equal to true
  4. no existing vNet and ddosSwitch equal to false
    Each one of these 4 deployment succeed and I can browse the homepage.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added documentation in the Parameters.md file and the Manage.md file.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for your quick follow-up. This is super! I'm merging this PR right after. Thanks again!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're welcome. Thanks for the great work you're doing !

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is DDoS protection included with all deployments automatically?

Comment thread nested/network-vnet-ddos.json Outdated
},
"enableDdosProtection": "[parameters('moodleCommon').ddosSwitch]"
},
"type": "Microsoft.Network/virtualNetworks"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really minor, but ARM team prefers properties like this (type) to be at the top, so please move this to before properties property.

@hosungsmsft hosungsmsft merged commit e83136b into Azure:master May 18, 2018
naioja pushed a commit that referenced this pull request Dec 29, 2025
vibehero100 pushed a commit to vibehero100/Moodle that referenced this pull request Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants