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

Create a dedicated schema Validation method #22484

Closed
wants to merge 6 commits into from

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Aug 15, 2019

Due to both the nature of dynamic blocks, and the need for resources to
sometimes communicate incomplete values, we cannot validate MinItems and
MaxItems during decoding or in CoerceValue.

Since we're reducing the early automatic validation, add a
Block.Validate method to precisely validate values. The number of items in the
block will be validated once the values is wholly known.

If a NestingGroup block has any RequiredAttributes, automatically
instantiating that would create a block missing those required
attributes, which will fail with the new validation. This means that the
block itself is essentially required, which we can enforce early in InternalValidate.
Since nothing currently makes use of NestingGroup, we can change the
semantics slightly to better align with the validation rules already present.

This also turns on InternalValidate for the schema. While the generated configschema
for existing providers should follow the expectations of InternalValidate, this will
only return warnings for the time being to avoid breaking any existing providers.

Fixes #22414

@jbardin jbardin requested a review from a team August 15, 2019 23:06
@jbardin jbardin force-pushed the jbardin/configschema-validate branch from f14c1e3 to 8247472 Compare August 16, 2019 13:43
@jbardin jbardin changed the title [WIP] Create a dedicated schema Validation method Create a dedicated schema Validation method Aug 16, 2019
@jbardin jbardin marked this pull request as ready for review August 16, 2019 14:43
@jbardin jbardin force-pushed the jbardin/configschema-validate branch from 51d757f to 8fca378 Compare August 16, 2019 21:00
@jbardin jbardin changed the title Create a dedicated schema Validation method [WIP] Create a dedicated schema Validation method Aug 16, 2019
@jbardin jbardin force-pushed the jbardin/configschema-validate branch from 8fca378 to de524fb Compare August 16, 2019 21:14
@jbardin jbardin changed the title [WIP] Create a dedicated schema Validation method Create a dedicated schema Validation method Aug 16, 2019
Due to both the nature of dynamic blocks, and the need for resources to
sometimes communicate incomplete values, we cannot validate MinItems and
MaxItems during decoding or CoerceValue.

Since we're reducing the early automatic validation, add a
Block.Validate method to precisely validate values, once they are fully
known.
If a NestingGroup block has any RequiredAttributes, automatically
instantiating that would create a block missing those required
attributes. This means that the block itself is essentially required,
which we can enforce early in InternalValidate.
Calling this during EvalDiff allows us to validate the final
configuration, immediately after all values have been evaluated. This
allows us to ensure that values passed to providers always conform to
the provider's schema.
This wasn't used until now but we should enforce the schema consistency,
especially when providers start generating their own rather than having
it done by helper/schema.

These will be presented as a warning initially to prevent any
unexpected breakage.
@jbardin jbardin force-pushed the jbardin/configschema-validate branch from 5b56d40 to 23a36cf Compare August 19, 2019 16:47
@jbardin
Copy link
Member Author

jbardin commented Aug 19, 2019

Rebased to squash a couple commits, hopefully the last cleanup for this PR

@jbardin
Copy link
Member Author

jbardin commented Aug 20, 2019

I'm still not completely happy with the validation concessions we need here.
Since this PR has gotten quite messy, I'm going to close this in favor of something a little simpler.

@ghost
Copy link

ghost commented Sep 20, 2019

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 Sep 20, 2019
@jbardin jbardin deleted the jbardin/configschema-validate branch October 31, 2019 16:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
1 participant