-
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
[4.0] azurerm_application_gateway
- upgrade API version to 2023-11-01
#26776
Conversation
…m into app_gateway_2023-11-01
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 sorting out this upgrade for 4.0 @teowa. Could you take a look at the comments I left in-line regarding the tests? I can see that you've updated the configs to ensure the tests pass, but I think with security defaults changing in this SDK upgrade the test cases should be updated to validate current security standards.
ssl_policy { | ||
policy_type = "Predefined" | ||
policy_name = "AppGwSslPolicy20150501" | ||
} |
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.
Can we update the check for this test instead to ensure the API is returning the documented default value AppGwSslPolicy20220101 is being returned
terraform-provider-azurerm/internal/services/network/application_gateway_resource_test.go
Line 1061 in c092e2f
check.That(data.ResourceName).Key("ssl_profile.0.ssl_policy.0.policy_name").HasValue("AppGwSslPolicy20150501"), |
ssl_policy { | |
policy_type = "Predefined" | |
policy_name = "AppGwSslPolicy20150501" | |
} |
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.
updated
ssl_policy { | ||
policy_type = "Predefined" | ||
policy_name = "AppGwSslPolicy20150501" | ||
} |
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.
Same here
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.
updated
@@ -7662,6 +7672,12 @@ resource "azurerm_application_gateway" "test" { | |||
priority = 10 | |||
} | |||
|
|||
ssl_policy { | |||
policy_type = "Custom" | |||
min_protocol_version = "TLSv1_1" |
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.
The minimum protocol version default will now be TLSv1_2
so can we update the checks in the test instead of the config. It's a better practice for the tests to verify that current security standards/defaults are working in the API than to ensure outdated security defaults and practices still work.
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.
updated
…m into app_gateway_2023-11-01
|
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 @teowa LGTM 👍
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Community Note
Description
context: #25844
https://learn.microsoft.com/en-us/azure/application-gateway/application-gateway-ssl-policy-overview#default-tls-policy
upgrade API version to
2023-11-01
, there is breaking change on default SSL policy, so must be done in 4.0PR 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.
azurerm_application_gateway
- upgrade API version to2023-11-01
This is a (please select all that apply):
Related Issue(s)
Fixes #0000
Note
If this PR changes meaningfully during the course of review please update the title and description as required.