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

ConflictsWith should flag a violation only if the field has a non zero value set. #85

Closed
rosbo opened this issue Nov 6, 2017 · 3 comments
Labels
enhancement New feature or request subsystem/types Issues and feature requests related to the type system of Terraform and our shims around it.

Comments

@rosbo
Copy link

rosbo commented Nov 6, 2017

Hi there,

The issue originally came up in the terraform-provider-google.
hashicorp/terraform-provider-google#683

As of now, when ConflictsWith looks for violations, it only checks if the other field is set. Ideally, it would check that the field is set and that it doesn't have the zero value (at least for strings).

The code checking for violations of ConflictsWith:

for _, conflicting_key := range schema.ConflictsWith {
	if value, ok := c.Get(conflicting_key); ok {
		return fmt.Errorf("%q: conflicts with %s (%#v)", k, conflicting_key, value)
	}
}

https://github.com/hashicorp/terraform/blob/master/helper/schema/schema.go?#L1235

if c.Get could have a behavior similar to the GetOk method, it would be nice:
https://github.com/hashicorp/terraform/blob/master/helper/schema/resource_data.go#L89

It makes it easy to create parametrizable module where you have two conflicting field and the user specifies one or the other.

Here is a concrete example in the google provider where this would be useful:

Specifying an empty string value for org_id or folder_id does not "unset" the argument for the google_project resource.

My scenario is to have a module that parameterizes the creation of a project:

resource "google_project" "host" {
  name            = "${var.project_name}"
  org_id          = "${var.org_id}"
  folder_id       = "${var.folder_id}"
  project_id      = "${var.project_id == "" ? random_id.default-project-id.hex : var.project_id}"
  billing_account = "${var.billing_account}"
}

In this example I can't provide both org_id and folder_id even if one is empty "" without getting this error:

Error: module.shared-vpc.google_project.host: "folder_id": conflicts with org_id ("1234567890")

Error: module.shared-vpc.google_project.host: "org_id": conflicts with folder_id ("")

If I could pass an empty value for either org_id or folder_id, that would be fantastic.

/cc @danisla

@apparentlymart
Copy link
Contributor

Hi @rosbo!

The configuration language revamp that is underway will introduce a proper idea of "unset" as separate from zero-value, and introduce null as an expression that can be assigned to express "unset", which I think will eventually get us to a place where we can robustly implement this sort of conditional attribute assignment.

# NOT YET IMPLEMENTED; may change before release

resource "google_project" "host" {
  name            = "${var.project_name}"
  org_id          = "${var.org_id != "" ? var.org_id : null}"
  folder_id       = "${var.folder_id != "" ? var.folder_id : null}"
  project_id      = "${var.project_id == "" ? random_id.default-project-id.hex : var.project_id}"
  billing_account = "${var.billing_account}"
}

For now there may be another way to achieve this goal for this resource in particular. In 0.11.0-beta1 we introduced CustomizeDiff as a new function a resource can implement, which gives it the opportunity to inspect the diff, reject certain combinations of changes, and add additional information. In principle this could be used to introduce some custom validation behavior here instead of using ConflictsWith. For example:

func projectCustomizeDiff(d *ResourceDiff, meta interface{}) error {
    orgId := d.Get("org_id")
    folderId := d.Get("folder_id")
    if orgId != "" && folderId != "" {
        return errors.New("org_id and folder_id arguments cannot both be set")
    }
    return nil
}

So far this function is only available in the beta, so it may be worth waiting until we reach 0.11.0 final before updating providers to vendor the new version, in case there are some bugs that need to be dealt with. In principle this function can replace quite a lot of the declarative validations we currently support on schemas, though I'd suggest using it sparingly since the declarative validation rules do, at least, help ensure more consistency between the behaviors of resources.

@hashibot hashibot transferred this issue from hashicorp/terraform Sep 26, 2019
@hashibot hashibot added the enhancement New feature or request label Oct 2, 2019
@paddycarver paddycarver added the subsystem/types Issues and feature requests related to the type system of Terraform and our shims around it. label Jan 6, 2021
@bflad
Copy link
Contributor

bflad commented Mar 30, 2022

Since this issue was raised, the Terraform configuration language in CLI version 0.12 and later has implemented support for null values. When this value is used in the context of configuring an attribute, Terraform will treat the attribute as if it was not set in the configuration at all. Using this style of (non 😄 ) configuration allows ConflictsWith to work as expected.

@bflad bflad closed this as completed Mar 30, 2022
@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 Apr 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request subsystem/types Issues and feature requests related to the type system of Terraform and our shims around it.
Projects
None yet
Development

No branches or pull requests

5 participants