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: Online/Network/API validation #101

Open
radeksimko opened this issue Mar 23, 2018 · 1 comment
Open

helper/schema: Online/Network/API validation #101

radeksimko opened this issue Mar 23, 2018 · 1 comment
Labels
enhancement New feature or request

Comments

@radeksimko
Copy link
Member

Depends on hashicorp/terraform#15895
Or it's probably better if it hashicorp/terraform#15895 is implemented first.


It is certainly undesirable to perform any "slow" validation which requires network access by default in terraform validate, but there's still value in having such validation.

It can be opt-in for validate command and it can also (more importantly) run as part of plan.

The implementation can be very much similar to ValidateFunc, except that the interface needs access to provider's meta.

Example use cases in AWS provider:

"vpc_security_group_ids": {
	Type:     schema.TypeString,
	Optional: true,
	NetworkValidateFunc: func(k string, v, meta interface{}) (ws []string, es []error) {
		if hasEc2Classic(meta.(*AWSClient).supportedplatforms) {
			es = append(es, fmt.Errorf("Use security_groups (with SG names) in EC2 Classic-enabled region"))
			return
		}
		return
	},
},
"security_groups": {
	Type:     schema.TypeString,
	Optional: true,
	NetworkValidateFunc: func(k string, v, meta interface{}) (ws []string, es []error) {
		if !hasEc2Classic(meta.(*AWSClient).supportedplatforms) {
			es = append(es, fmt.Errorf("Use security_group_ids (with SG IDs) in VPC-enabled region"))
			return
		}
		return
	},
},

I'm not sure if NetworkValidateFunc is the best name, but this is rather a simple "reminder" rather than full-blown proposal with all answers.

Related: hashicorp/terraform-provider-aws#3897

@apparentlymart
Copy link
Contributor

This sort of validation was one of the intended use-cases of the CustomizeDiff functionality, which runs as part of terraform plan and is therefore allowed to do network-sensitive validations.

The [ValidateValue] helper function allows this to be expressed in a more declarative way by function composition:

  CustomizeDiff: customdiff.All(
      customdiff.ValidateValue("security_groups", func (value, meta interface{}) error {
          if !hasEc2Classic(meta.(*AWSClient).supportedplatforms) {
              return fmt.Errorf("Use security_group_ids (with SG IDs) in VPC-enabled region"))
          }
      }),
      // ...
  ),

Of course, there are some ergonomic problems here. Most notably, the validation rule is expressed outside of the schema.Schema instance and this may not be easy to find in a resource with a very large schema.

Having a new schema-specific function for this seems fine to me, but from the perspective of Terraform Core I'd prefer to have the provider SDK do it as part of the existing Diff method (the same as CustomizeDiff) rather than introducing a new idea of "network validation". Once you've got the provider configured enough that it could implement network validation there isn't really any harm in running a full plan, even if you intend to ignore the result.

I think I'd approach this by using the very general CustomizeDiff for this sort of problem right now and then that experience will help us understand which parts of its functionality would be valuable to include as schema.Schema features too. For example, I think we can achieve the sort of validation you described here for hashicorp/terraform-provider-aws#3897 with the features we already have, and then convert to a different approach later.

@hashibot hashibot transferred this issue from hashicorp/terraform Sep 26, 2019
@hashibot hashibot added the enhancement New feature or request label Oct 2, 2019
@radeksimko radeksimko changed the title helper/schema: Network validation helper/schema: Online/Network validation Oct 29, 2019
@radeksimko radeksimko changed the title helper/schema: Online/Network validation helper/schema: Online/Network/API validation Oct 30, 2019
@radeksimko radeksimko added the upstream-protocol Requires change of protocol specification, i.e. can't be done under the current protocol label Dec 8, 2019
@paddycarver paddycarver removed the upstream-protocol Requires change of protocol specification, i.e. can't be done under the current protocol label Aug 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants