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

Allow Configuration Validators To Satisfy DataSourceConfigValidator, ProviderConfigValidator, and ResourceConfigValidator Interfaces #404

Closed
bflad opened this issue Jul 8, 2022 · 1 comment · Fixed by #405
Assignees
Labels
breaking-change This PR introduces a breaking change or the resolution of this issue may require a breaking change. enhancement New feature or request
Milestone

Comments

@bflad
Copy link
Contributor

bflad commented Jul 8, 2022

Module version

v0.9.0

Use-cases

Provider developers may wish to develop generic "top level" schema data based configuration validators that satisfy all the following interfaces:

  • tfsdk.DataSourceConfigValidator
  • tfsdk.ProviderConfigValidator
  • tfsdk.ResourceConfigValidator

Such as these:

These *ConfigValidator interfaces were originally split during the initial validation implementation to allow separate feature handling, should the protocol individually change over time for one or more of the Validate*Config RPCs. I think the split use cases is very valid and follows the original design principles of this framework, however I made a simple mistake in the actual code for the interfaces, where they each use the same Validate() method name with differently typed parameters. Oops. They should use different method names so all three can be implemented, if desired. The validator developer can choose to share the implementation details across them to reduce code, however that is outside the scope of the framework anyways.

Attempted Solutions

Individual data source, provider, and resource configuration validators. 😢

Proposal

Adjust the interfaces slightly from:

type DataSourceConfigValidator interface {
	// ... other fields ...

	Validate(context.Context, ValidateDataSourceConfigRequest, *ValidateDataSourceConfigResponse)
}

type ProviderConfigValidator interface {
	// ... other fields ...

	Validate(context.Context, ValidateProviderConfigRequest, *ValidateProviderConfigResponse)
}

type ResourceConfigValidator interface {
	// ... other fields ...

	Validate(context.Context, ValidateResourceConfigRequest, *ValidateResourceConfigResponse)
}

To:

type DataSourceConfigValidator interface {
	// ... other fields ...

	ValidateDataSource(context.Context, ValidateDataSourceConfigRequest, *ValidateDataSourceConfigResponse)
}

type ProviderConfigValidator interface {
	// ... other fields ...

	ValidateProvider(context.Context, ValidateProviderConfigRequest, *ValidateProviderConfigResponse)
}

type ResourceConfigValidator interface {
	// ... other fields ...

	ValidateResource(context.Context, ValidateResourceConfigRequest, *ValidateResourceConfigResponse)
}

One potential consideration here is a separate breaking change to refactor data source, provider, and resource specific types into their own packages, but I think this change can be made beforehand without too much repercussions given that these are likely rare in the wild at the moment (as compared to attribute validators) and when that refactoring potentially happens it can just be a breaking change of the type name itself while keeping the newly specific Validate* method names.

References

@bflad bflad added enhancement New feature or request breaking-change This PR introduces a breaking change or the resolution of this issue may require a breaking change. labels Jul 8, 2022
@bflad bflad added this to the v1.0.0 milestone Jul 8, 2022
bflad added a commit that referenced this issue Jul 8, 2022
…oviderConfigValidator, and ResourceConfigValidator interfaces

Reference: #404
@bflad bflad self-assigned this Jul 8, 2022
@bflad bflad modified the milestones: v1.0.0, v0.10.0 Jul 8, 2022
@bflad bflad closed this as completed in #405 Jul 8, 2022
bflad added a commit that referenced this issue Jul 8, 2022
…oviderConfigValidator, and ResourceConfigValidator interfaces (#405)

Reference: #404
@github-actions
Copy link

github-actions bot commented Aug 8, 2022

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 Aug 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking-change This PR introduces a breaking change or the resolution of this issue may require a breaking change. enhancement New feature or request
Projects
None yet
1 participant