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

update docs so that users know Firewall Premium is the default #460

Merged
merged 13 commits into from
Oct 22, 2021

Conversation

glennmusa
Copy link
Contributor

Description

Sets the firewallSkuTier parameter in the Bicep deployment to Premium in clouds that support it and Standard elsewhere.

Updates documentation so that a user can override this value at deployment time.

Issue reference

The issue this PR will close: #357

Checklist

Please make sure you've completed the relevant tasks for this PR out of the following list:

  • All acceptance criteria in the backlog item are met
  • The documentation is updated to cover any new or changed features
  • Manual tests have passed
  • Relevant issues are linked to this PR

@shawngib
Copy link
Member

Is this truly needed beyond documenting as premium will be released in GOV, we should also consider this along with other settings for none public/gov deployments.

@glennmusa
Copy link
Contributor Author

Is this truly needed beyond documenting as premium will be released in GOV, we should also consider this along with other settings for none public/gov deployments.

I'm for documenting "if your deployment isn't into one of these regions, then set firewallSkuTier=Standard" but that sounds error-prone given we currently default to Premium.

Ideally, a custom validator/function would be helpful here that could check an ARM authoritative set of supported regions, but, given that a Firewall is not optional I think this change increases the likelihood a quickstart deploys without user intervention in whichever cloud the deployment occurs in. I will still update the docs for supported regions since that makes sense to warn of potential failure ahead of deployment.

I think your second point, design considerations for region-specific resource availability, is valuable, but both the design and implementation of that is new and undefined scope. It smells like maintaining where resource X and resource Y work in every region across clouds is expensive to maintain, changes with each provider version, and is an unknown and will not be implemented for air gapped clouds.

@glennmusa glennmusa marked this pull request as draft October 14, 2021 16:56
@glennmusa glennmusa marked this pull request as ready for review October 19, 2021 15:49
@shawngib
Copy link
Member

Just to be clear by what I mean do we still need this. "Development word on the street is that Azure FW Premium is available for use in USSec. We will try to get a verification of this, this week" quote from Byron. If premium becomes supported in all, sooner rather than later are we forcing something we shouldn't?

@glennmusa
Copy link
Contributor Author

Just to be clear by what I mean do we still need this. "Development word on the street is that Azure FW Premium is available for use in USSec. We will try to get a verification of this, this week" quote from Byron. If premium becomes supported in all, sooner rather than later are we forcing something we shouldn't?

after our other discussions about user's decisions always being first, I'm in agreement that we default something and provide docs to point them to when they need to change something, will update.

@glennmusa glennmusa marked this pull request as draft October 21, 2021 14:40
@glennmusa glennmusa changed the title Use Firewall Premium where available in Bicep deployments update docs so that users know Firewall Premium is the default Oct 21, 2021
@glennmusa glennmusa marked this pull request as ready for review October 21, 2021 15:04
@glennmusa glennmusa enabled auto-merge (squash) October 21, 2021 15:06
@glennmusa glennmusa self-assigned this Oct 21, 2021
@glennmusa glennmusa removed their assignment Oct 21, 2021
Copy link
Member

@shawngib shawngib left a comment

Choose a reason for hiding this comment

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

Perfect!!

@glennmusa glennmusa merged commit d1c20f8 into main Oct 22, 2021
@glennmusa glennmusa deleted the glenn/detectFirewallPremium branch October 22, 2021 13:40
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.

Use environment() function to determine whether or not to use Azure Firewall Premium
2 participants