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

helper/schema: implement validation for TypeList/TypeSet. #6508

Closed

Conversation

mattmoyer
Copy link
Contributor

This was previously supported only on primitive schema types. It allows providers to validate properties of the entire collection in addition to the properties of individual elements.

This is similar to the functionality in #4348 but applied only to the elements of a particular TypeList or TypeSet schema rather than the resource as a whole.

My specific use case for this collection-level validation is an addition to the Fastly provider (WIP here) that adds support for a new vcl block in the fastly_service_v1 resource. The way the Fastly API is set up, you have multiple VCL blocks but exactly one of them can be marked as a "main" block (the rest are treated as includable libraries). This functionality lets me validate this property and issue a nice warning/error before doing anything else.

This was previously supported only on primitive schema types.
@phinze
Copy link
Contributor

phinze commented May 5, 2016

Hi @mattmoyer! I'm excited to have you diving in to helper/schema!

Unfortunately this one is not as straightforward as it seems. I'll do my best to explain here and then I can field any follow up questions you may have.

Your implementation here works fine iff there are no "computed" values in the relevant config. A "computed" value is one that is not yet known for a variety of reasons - most commonly because it references an attribute of a resource that has not yet been created.

If you check out validatePrimitive you'll notice we return early - before ValidateFunc handling - if the value is computed. It's a tradeoff we have to make at the validation step because we just don't know whether or not the value is going to be valid or not.

With the non-primitive data types TypeList and TypeSet, not only can they be computed in their entirety, they can also be partially computed. For example, imagine a VCL block that references an IP address of an aws_instance.

I started wrestling with these questions in the context of resource-level validation over in #4348 and my latest (not yet spiked out) proposal is to significantly restructure the validation code to allow validation authors to "opt-in" to the fields that are required to express their given validation. That way the calling code can know if a given validation can be executed at plan time or if it needs to be saved until JIT during the apply. I think something similar might work for sets and lists as well!

Anyways - sorry to complicate your ostensibly easy win! 😝 Hope all this makes sense. Hit me with any questions. 👍

@mattmoyer
Copy link
Contributor Author

Ooh, wow yeah that is a pretty tricky. I'll take a deeper look at this tomorrow and decide what to do next.

@mattmoyer
Copy link
Contributor Author

@phinze thanks for the detailed feedback on what a tricky problem this is right now. I don't think I have time now to finish implementing this using the strategy proposed in #4348 (comment), so I'll close for now. I'm happy to take another try at this once those changes are in place.

@ghost
Copy link

ghost commented Apr 25, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants