-
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
azurerm_windows_web_app
, azurerm_windows_function_app
, azurerm_linux_function_app
, azurerm_linux_web_app
- support for minimum_tls_cipher_suite
property
#26584
base: main
Are you sure you want to change the base?
Conversation
azurerm_windows_web_app
, azurerm_windows_function_app
, azurerm_linux_function_app
, azurerm_linux_web_app
- add minimum_tls_cipher_suite
propertyazurerm_windows_web_app
, azurerm_windows_function_app
, azurerm_linux_function_app
, azurerm_linux_web_app
- support for minimum_tls_cipher_suite
property
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 @xiaxyi - This looks good, but we'll end up with the related slot
resources out of step, can you either add the same property there, or open a 2nd PR with those in so we can merge it all together?
Thanks
Thanks @jackofallops for the review! Added the property to the slot in this pr. Let me know if anything needed. |
@xiaxyi @jackofallops what outstanding items are there before the pr can be approved for release? |
Any updates on this please, our client is asking for this feature. |
Also interested to get this feature! Any news on the progress? |
@jackofallops, Code for slots was added in August. Is there anything else you wanted updated? |
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.
Hi @xiaxyi - Thanks for making those changes. I've left one comment below regarding what appears to be a copy-paste error if you can take a look. Also, some tests are now failing with errors of the following type:
unexpected status 400 (400 Bad Request) with response:
{"Code":"BadRequest","Message":"Tls13 does not support re-negotiation. Please
either disable ClientCert, or set ClientCertMode to Application or Required
(without exclsuion paths).","Target":null,"Details":[{"Message":"Tls13 does
not support re-negotiation. Please either disable ClientCert, or set
ClientCertMode to Application or Required (without exclsuion
paths)
(The typo in the error message there is from the API)
I'll link you the regression test run from the CI server offline.
This PR is being labeled as "stale" because it has not been updated for 30 or more days. If this PR is still valid, please remove the "stale" label. If this PR is blocked, please add it to the "Blocked" milestone. If you need some help completing this PR, please leave a comment letting us know. Thank you! |
…o app/props/minTlsCipherSuite # Conflicts: # internal/services/appservice/linux_function_app_slot_resource_test.go
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.
Hi @xiaxyi - Thanks for the updates, looks like there's still a test that is failing due to a ForceNew
change.
------- Stdout: -------
=== RUN TestAccLinuxFunctionApp_tlsSettingUpdate
=== PAUSE TestAccLinuxFunctionApp_tlsSettingUpdate
=== CONT TestAccLinuxFunctionApp_tlsSettingUpdate
testcase.go:173: Step 4/12 error: Pre-apply plan check(s) failed:
'azurerm_linux_function_app.test' - expected action to not be Replace, path: [[name]] tried to update a value that is ForceNew
--- FAIL: TestAccLinuxFunctionApp_tlsSettingUpdate (433.71s)
Community 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.
azurerm_windows_web_app
,azurerm_windows_function_app
,azurerm_linux_function_app
,azurerm_linux_web_app
- support for theminimum_tls_cipher_suite
property.This is a (please select all that apply):
Related Issue(s)
Fixes #24223
Note
If this PR changes meaningfully during the course of review please update the title and description as required.