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

Add validation to provider fields in both SDK and PF implementations of provider schema #6358

Conversation

modular-magician
Copy link
Collaborator

Description

Closes hashicorp/terraform-provider-google#14447

This PR adds explicit validation feedback to users, to accompany the changes in GoogleCloudPlatform/magic-modules#9014

The PR above means that "" values will be processed by all the usual provider configuration logic but the error messages returned to users may be confusing and result in GitHub issues being opened. By adding explicit validation users will be able to identify and address the problems themselves.

I've chosen to add empty string validation to the more popular fields and avoiding things like request_reason because I don't know if there's a valid use case for setting that value to an empty string or not?

Testing

I've added:

  • unit tests for the new empty string validators
  • acceptance tests showing the validators are present in the provider config and throw errors in appropriate scenarios

Here's a screenshot from a manual test. NOTE: there are always 2 errors, as both the SDK and PF validators are rejecting the bad input

Screenshot 2023-09-22 at 18 46 31

Misc details

Previously I thought that provider-level validation protecting against empty strings was a problem due to the SDK being weird/making it easy to footgun when handling zero values. After some manual tests I found this wasn't the case, so added anti-empty string validation to the SDK and PF versions of the provider config code.

NOTE: If validation exists on one version of the provider config but not the other then there's a fundamental error in the provider, as provider schemas must match when muxed together.


Release Note Template for Downstream PRs (will be copied)

provider: added provider-level validation so these fields are not set as empty strings in a user's config: `credentials`, `access_token`, `impersonate_service_account`, `project`, `billing_project`, `region`, `zone`

Derived from GoogleCloudPlatform/magic-modules#9050

…of provider schema (#9050)

* Add empty-string validator for PF provider

* Add empty-string validator for SDK provider, move SDK validators to separate file

* Add empty string validators to : credentials, access_token, impersonate_service_account, project, billing_project, region, zone

* Add unit tests for `ValidateEmptyStrings`

* Remove empty string test case from `ValidateCredentials`

* Add acceptace tests showing that empty strings in provider block results in a validation error, and empty provider blocks have no validation errors

* Make the SDK provider's `ValidateCredentials` validator reject empty strings

* Update acceptance test after change in `credentials` validation

* Fix test definitions to avoid fall-through

* Update validation error message in code and tests

Signed-off-by: Modular Magician <[email protected]>
@modular-magician modular-magician merged commit 5eb5f99 into hashicorp:FEATURE-BRANCH-major-release-5.0.0 Sep 23, 2023
3 checks passed
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.

1 participant