-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[Enhancement:] azurerm_cdn_frontdoor_firewall_policy
- js_challenge_cookie_expiration_in_minutes
causing BadRequest
error with Standard_AzureFrontDoor
SKU
#28726
Conversation
js_challenge_cookie_expiration_in_minutes
causing issues with Standard_AzureFrontDoor
SKUsazurerm_cdn_frontdoor_firewall_policy
- Address issue of new field js_challenge_cookie_expiration_in_minutes
causing issues with Standard_AzureFrontDoor
SKUs
azurerm_cdn_frontdoor_firewall_policy
- Address issue of new field js_challenge_cookie_expiration_in_minutes
causing issues with Standard_AzureFrontDoor
SKUsazurerm_cdn_frontdoor_firewall_policy
- js_challenge_cookie_expiration_in_minutes
causing BadRequest
error with Standard_AzureFrontDoor
SKU
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @WodansSon - Just a few items to take a look at, feel free to ping me in the off-line thread on these as needed as I know it's time sensitive (and I get so many GH notifications I'd not want to miss a reply 🙈)
internal/services/cdn/cdn_frontdoor_firewall_policy_resource.go
Outdated
Show resolved
Hide resolved
internal/services/cdn/cdn_frontdoor_firewall_policy_resource.go
Outdated
Show resolved
Hide resolved
if *model.Sku.Name == waf.SkuNamePremiumAzureFrontDoor && policy.JavascriptChallengeExpirationInMinutes != nil { | ||
d.Set("js_challenge_cookie_expiration_in_minutes", int(pointer.From(policy.JavascriptChallengeExpirationInMinutes))) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is now O+C
, should we not always set it? (relating to the comment on the schema default above...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, because if the resource is a Standard
sku the javascriptChallengeExpirationInMinutes
field will be nil
, it will only have a value if it is a Premium
sku, so I think some sort of filtering logic needs to be applied here. I have updated the logic in the Read
function to only write the value of the javascriptChallengeExpirationInMinutes
field to the state
file if the javascriptChallengeExpirationInMinutes
field is not nil
. That said, I could flip the logic to always write the value if the sku is not Standard
or to always write the value if the sku is Premium
, at the end of the day it's six of one half dozen of the other, it kind of all works out in the wash whichever way we go. I am open any of the above options...
Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes @WodansSon - This LGTM now 👍
Tests are all green too:
* CHANGELOG.md for v4.19.0 * Update CHANGELOG.md #28523 * Update CHANGELOG.md #28691 * Updated to include #28717 * Update for #26680 * Update CHANGELOG.md #28633 * Update CHANGELOG.md for #28703 * Update CHANGELOG.md for #28391 * Update CHANGELOG.md #28725 * Update #28733 * Update CHANGELOG.md #28659 * Update for #28741 * Update CHANGELOG.md #28712 * Update CHANGELOG.md #28441 * Update CHANGELOG.md #28441 * Update CHANGELOG.md #28441 * Update CHANGELOG.md for #28602 * Update for #27424 * Update CHANGELOG.md for #28524 * Update CHANGELOG.md #28726 * Update for #28767 * Update for #28195 * prep for release v4.19.0 --------- Co-authored-by: sreallymatt <[email protected]> Co-authored-by: Wodans Son <[email protected]> Co-authored-by: stephybun <[email protected]> Co-authored-by: Wyatt Fry <[email protected]> Co-authored-by: Matthew Frahry <[email protected]> Co-authored-by: jackofallops <[email protected]>
* Initial Check-In... * Fix lint error... * Update test case names... * Mostly working now, need to fix one test... * Fix lint error... * Add test cases and CustomizeDiffShim... * Add custom_rule definition to jsChallengePolicyStandardSku test case... * Fix error return value... * Slightly tweak documentation and expose a CustomizeDiffShim for JSChallenge policy validation... * Add test case for invalid JSChallenge Policy... * Update tests... * Update documentation... * Format documentation... * Address PR comments...
* CHANGELOG.md for v4.19.0 * Update CHANGELOG.md hashicorp#28523 * Update CHANGELOG.md hashicorp#28691 * Updated to include hashicorp#28717 * Update for hashicorp#26680 * Update CHANGELOG.md hashicorp#28633 * Update CHANGELOG.md for hashicorp#28703 * Update CHANGELOG.md for hashicorp#28391 * Update CHANGELOG.md hashicorp#28725 * Update hashicorp#28733 * Update CHANGELOG.md hashicorp#28659 * Update for hashicorp#28741 * Update CHANGELOG.md hashicorp#28712 * Update CHANGELOG.md hashicorp#28441 * Update CHANGELOG.md hashicorp#28441 * Update CHANGELOG.md hashicorp#28441 * Update CHANGELOG.md for hashicorp#28602 * Update for hashicorp#27424 * Update CHANGELOG.md for hashicorp#28524 * Update CHANGELOG.md hashicorp#28726 * Update for hashicorp#28767 * Update for hashicorp#28195 * prep for release v4.19.0 --------- Co-authored-by: sreallymatt <[email protected]> Co-authored-by: Wodans Son <[email protected]> Co-authored-by: stephybun <[email protected]> Co-authored-by: Wyatt Fry <[email protected]> Co-authored-by: Matthew Frahry <[email protected]> Co-authored-by: jackofallops <[email protected]>
azurerm_cdn_frontdoor_firewall_policy
- addedsku_name
downgrade validationazurerm_cdn_frontdoor_profile
- addedsku_name
downgrade validationCommunity Note
Description
PR Checklist
For example: “
resource_name_here
- description of change e.g. adding propertynew_property_name_here
”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
BUG FIXES:
azurerm_cdn_frontdoor_firewall_policy
- fixed an issue where thejs_challenge_cookie_expiration_in_minutes
policiesdefault
value was causingStandard_AzureFrontDoor
skus to receive aBadRequest
error [GH-00000]This is a (please select all that apply):
Related PR(s)
#28717
#28750
Related Issue(s)
Fixes #28716
Fixes #28713
Note
If this PR changes meaningfully during the course of review please update the title and description as required.