ENG-12435: Uptake of new Listener API with override repo tls flag and tls mode. #441
Conversation
…erride repo tls flag and tls mode.
|
Kudos, SonarCloud Quality Gate passed!
|
There was a problem hiding this comment.
Changes look good @gengdahlCyral, I just have a few suggestions to improve the resource usability and the docs a little bit.
| Description: "TLS mode. " + | ||
| "Default value generated by API if not provided. " + | ||
| "Note! This field is in effect only if OverrideRepoClientTlsSettings is set to true or the listener " + | ||
| "is a SMART port (enabled in more than one binding). " + |
There was a problem hiding this comment.
Suggestion: Is there a link in our public doc regarding smart ports? Might be interesting to add this link in this description as a reference for customers that want to understand how it works. I would also recommend adding a reference to the cyral_repository_binding resource here, something like:
| "is a SMART port (enabled in more than one binding). " + | |
| "is a SMART port (enabled in more than one binding - see [`cyral_repository_binding`](../resources/repository_binding.md)). " + |
There was a problem hiding this comment.
Regarding the link to our public docs, please refer to the latest version instead of some specific release.
| Optional: true, | ||
| Computed: true, | ||
| }, | ||
| TlsModeKey: { |
There was a problem hiding this comment.
@gengdahlCyral could you please also add validation for the possible values that are valid for this field? I understand that the API already does this validation and that it is easier to maintain if we don't duplicate the validation here in the terraform provider, but since the validation messages returned by the API are often not clear enough, adding a validation that is computed during the terraform plan instead of during the execution, and that also provides a proper message that explicitly mentions the error and which values are allowed for this field would improve the terraform provider UX overall, which is something we are aiming for in a recent conversation that I had with @wcmjunior. The validation could be something like:
ValidateFunc: validation.StringInSlice(tlsModes(), false),| TlsModeKey: { | ||
| Description: "TLS mode. " + | ||
| "Default value generated by API if not provided. " + | ||
| "Note! This field is in effect only if OverrideRepoClientTlsSettings is set to true or the listener " + |
There was a problem hiding this comment.
| "Note! This field is in effect only if OverrideRepoClientTlsSettings is set to true or the listener " + | |
| "Note! This field is in effect only if `override_repo_client_tls_settings` is set to `true` or the listener " + |
wcmjunior
changed the title
Description of the change
The background to the changes made in the Cyral Terraform Provider is the updated listener API from ENG-12432
{{baseUrl}}/sidecars/:sidecarID/listeners.This API has been extended with two primitive attributes:
{true,false}Optional, server generated if omitted (false){allow,require,disable}Optional, server generated if omitted (allow)These two settings applies to any listener regardless of the repo type it is associated to.
Short description of changes in this PR:
Updated Makefile and Sidecar Listener
In the Makefile:
up-depsto update module and test dependencies to the latest minor and patch levels.In cyral/data_source_cyral_sidecar_listener.go:
In cyral/resource_cyral_sidecar_listener.go:
override_repo_client_tls_settingsandtls_modeto SidecarListener struct.In cyral/resource_cyral_sidecar_listener_test.go:
override_repo_client_tls_settingsandtls_mode.In docs/data-sources/sidecar_listener.md:
override_repo_client_tls_settingsandtls_mode.In docs/resources/sidecar_listener.md:
override_repo_client_tls_settingsandtls_mode.This PR introduces improvements to the Makefile and adjusts the Sidecar Listener resource code to support new fields related to TLS settings and repository client overrides. Test cases and documentation have been updated accordingly.
Type of change
Checklists
Development
Code review
Testing
Created listener without giving tls_mode or override_repo_client_tls_settings, default values where generated by server ok
updated with incorrect tls_mode -> API error returned (expected)
updated with correct parameters -> listener updated
listed listeners, tls fields where shown.