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 missing All, Any, and friends validators #122

Closed
1 task done
joneshf opened this issue Mar 25, 2023 · 1 comment · Fixed by #158
Closed
1 task done

Add missing All, Any, and friends validators #122

joneshf opened this issue Mar 25, 2023 · 1 comment · Fixed by #158
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Milestone

Comments

@joneshf
Copy link

joneshf commented Mar 25, 2023

Terraform CLI and Framework Versions

$ terraform version
Terraform v1.3.6
on darwin_amd64

Your version of Terraform is out of date! The latest version
is 1.4.2. You can update by downloading from https://www.terraform.io/downloads.html
$ go list -m github.com/hashicorp/terraform-plugin-framework
github.com/hashicorp/terraform-plugin-framework v1.1.1

Use Cases or Problem Statement

In #80, the metavalidator package was replaced with concrete implementations for most of the data types. Unfortunately, the boolvalidator, datasourcevalidator, providervalidator, and resourcevalidator packages didn't get All, Any, and friends like the rest of the *validator packages.

The lack of feature parity in the these package makes it very hard to express logic on boolean attributes in the same way you can on other data types. You either have to roll your own implementation (which is the hard part), or not validate. Both choices are unideal.

Proposal

In order of preference:

  1. Add back the metavalidator package. However, I think this ship has sailed. So it's fine if this isn't the selected option.

    My opinion as a provider author is that having to use a different function in a different package for the sake of type safety is more a bug than a feature. It feels like between one of generics and interfaces, there's a solution that would let provider authors have the ease of use to create validations with the same function while also giving the framework the type safety it requires.

  2. Add the All, Any, and friends versions to the boolvalidator, datasourcevalidator, providervalidator, and resourcevalidator packages.

    I would hope that any validation could be combined with All, Any, and friends.

  3. Add the All, Any, and friends versions to the boolvalidator package.

    I could imagine there might be strong opinions about whether the datasourcevalidator, providervalidator, and resourcevalidator packages need All, Any and friends. I would rather drop my implementation of AnyWithAllWarnings for booleans by at least having the boolvalidator package implement this behavior, than get bogged down in that opinion. It makes sense to me that anything should be able to combine validators in these ways, but I also am not maintaining this framework so my opinion is only worth so much.

    If there's opinions about the other three, then just punt and at least have feature parity for the boolean stuff.

Additional Information

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@joneshf joneshf added the enhancement New feature or request label Mar 25, 2023
joneshf added a commit to joneshf/terraform-provider-openwrt that referenced this issue Mar 25, 2023
The way we setup `peerdns` isn't correct. We don't want to require both
that the `proto` be `"dhcp"` and `"dhcpv6"`. That doesn't even make
sense. And the fact that we can say that makes even less sense.

In order to show the failure, we add a failing test. We then implement a
fix by adding a validator that disjoins multiple other validators. The
test is effectively what we want in reality, so it passing here should
give good confidence that we can do what we want.

It's unfortunate that we have to write this validator. Hopefully, it's
an oversight and not intentional. We don't want to maintain this
indefinitely. See
hashicorp/terraform-plugin-framework-validators#122
for the upstream issue.

Branch: joneshf/failing-peerdns-test
Pull-Request: #110
@bookshelfdave bookshelfdave added help wanted Extra attention is needed good first issue Good for newcomers and removed tf-devex-triage labels Jul 10, 2023
@SBGoods SBGoods self-assigned this Aug 1, 2023
@bflad bflad added this to the v0.12.0 milestone Aug 21, 2023
@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
4 participants