Using ConflictsWith with Lists or Sets

[jtopjian]

I'm trying to use ConflictsWith on a TypeList and it isn't working. I think I've narrowed down the issue, but I'm not entirely sure.

The openstack_compute_instance_v2 resource has a network attribute of TypeList. The list defines several nested attributes that can all be used to create a network.

Some of the attributes can't be used together. For example, port and floating_ip. So I made the following changes:

"port": &schema.Schema{
        Type:     schema.TypeString,
        Optional: true,
        ForceNew: true,
        Computed: true,
        ConflictsWith: []string{"network.floating_ip"},
},
"floating_ip": &schema.Schema{
        Type:     schema.TypeString,
        Optional: true,
        ForceNew: true,
        Computed: true,
        ConflictsWith: []string{"network.port"},
},

Then, given something simple like:

resource "openstack_compute_instance_v2" "instance_1" {
  name = "instance_1"
  network {
    floating_ip = "foo"
    port = "bar"
  }
}

I would expect an error to be throw, however, one isn't. But, if I change the schema to:

"port": &schema.Schema{
        Type:     schema.TypeString,
        Optional: true,
        ForceNew: true,
        Computed: true,
        ConflictsWith: []string{"network.0.floating_ip"},
},
"floating_ip": &schema.Schema{
        Type:     schema.TypeString,
        Optional: true,
        ForceNew: true,
        Computed: true,
        ConflictsWith: []string{"network.0.port"},
},

Then I get a validation error. Of course, that is only applicable to the first defined network.

Throwing some debug output into helper/schema/schema.go and terraform/resource.go, I think the following is happening:

During the loop for a conflicting key, s is network.0.port but conflicting_key is network.floating_ip. When network.floating_ip is checked here, since no index is specified, no value is returned.

[jtopjian]

I think this is more of a core bug than specific to OpenStack since it deals with the core schema and resource packages. However, if I'm wrong about this, please let me know :)

[mitchellh]

Going to tag this with enhancement since I think this is more of a feature request, I don't think this is anything we ever supported. :)

[jtopjian]

That sounds good to me! I'm in no rush to implement this - I can work around it.

[rosbo]

Hi,

Having ConflictsWith work properly on a TypeList is something that would also be useful for the Google Cloud Provider.

One example among many: a Google Compute Instance can define its network_interface by referring to a network or a subnetwork. These two options are mutually exclusive. An instance can have multiple network interfaces:

resource "google_compute_instance" "i1" {
  name = "test-instance1"
  machine_type = "f1-micro"
  can_ip_forward = false
  zone = "us-east1-c"

  boot_disk {
    initialize_params {
      image = "debian-cloud/debian-8"
    }
  }

  network_interface {
    subnetwork = "${google_compute_subnetwork.network1-sub1.self_link}"
    // OR
    network = "${google_compute_network.network2.self_link}"
   // Should fail if both are defined.
  }
}

Our workaround is to fail in the Create method if both are set for a network interface. It means that terraform plan succeeds but then it fails at terraform apply. We would love to bring the validation to the planning phase.

Additionally, the schema internal validation on ConflictsWith accepts invalid path and silently do no conflict detection. We had one case in the google provider where the ConflictsWith was misused and the schema validation didn't complain. Fixed this PR

...
Schema: map[string]*schema.Schema{
  "node_pool": {
		Type:     schema.TypeList,
		Optional: true,
		Computed: true,
		ForceNew: true,
		Elem: &schema.Resource{
			Schema: map[string]*schema.Schema{
				"initial_node_count": {
					Type:     schema.TypeInt,
					Required: true,
                                        // WRONG: This does nothing and the schema validation does not fail!
                                        ConflictsWith: []string{"node_pool.name_prefix"},
					ForceNew: true,
				},

				"name": {
					Type:     schema.TypeString,
					Optional: true,
					Computed: true,
					ForceNew: true,
				},

				"name_prefix": {
					Type:     schema.TypeString,
					Optional: true,
					ForceNew: true,
				},
			},
		},
	},
},

ConflictsWith: []string{"node_pool.name_prefix"} does nothing because it only looks if a value is defined based on the ConflictsWith path you pass in.
https://github.com/hashicorp/terraform/blob/master/helper/schema/schema.go?utf8=%E2%9C%93#L1175

c.Get("node_pool.name_prefix") will never be set and therefor will never conflict. network_interface.0.subnetwork...network_interface.x.subnetwork are set, never the non-indexed version.

The code performing the schema validation is here:
https://github.com/hashicorp/terraform/blob/master/helper/schema/schema.go?utf8=%E2%9C%93#L590

It should make sure that the index is defined when referencing a nested field in the ConflictsWith or accept it but do proper conflict detection.

Thank you

[bflad]

Hi folks 👋 Non-SDK team member providing some additional information here.

We tightened up the validation of the AtLeastOneOf/ConflictsWith/ExactlyOneOf/RequiredWhen fields so the references can be checked via unit testing. As of the upcoming v2, the SDK should return errors if you attempt a reference of config_block_attr.child_attr instead of something like config_block_attr.0.child_attr. This validation should also be ensuring that the configuration block attributes are Type: TypeList and MaxItems: 1, to match the only supported behaviors at the moment.

As for the enhancement request, it seems like the ConflictsWith fields (and friends) could benefit from supporting relative references so that they could handle TypeList and TypeSet attributes, e.g.

Schema: map[string]*schema.Schema{
	"config_block_attr": {
		Type:     schema.TypeSet,
		Optional: true,
		Elem: &schema.Resource{
			Schema: map[string]*schema.Schema{
				"child_attr1": {
					Type:     schema.TypeString,
					Optional: true,
					ConflictsWith: []string{".child_attr2"},
				},
				"child_attr2": {
					Type:     schema.TypeString,
					Optional: true,
					ConflictsWith: []string{".child_attr1"},
				},
			},
		},
	},
},

Where the configuration validation could occur at the same parent-children and level of schema. It would probably need to be restricted to this specific setup rather than allowing arbitrary schema decisions. Attributes cannot begin with a period, so this new syntax should not conflict with (😅 ) existing syntax.

This is just a personal design opinion though that the SDK maintainers would need to validate if its possible and review/approve the design.

[acwest]

Hi folks 👋 Non-SDK team member providing some additional information here.

We tightened up the validation of the AtLeastOneOf/ConflictsWith/ExactlyOneOf/RequiredWhen fields so the references can be checked via unit testing. As of the upcoming v2, the SDK should return errors if you attempt a reference of config_block_attr.child_attr instead of something like config_block_attr.0.child_attr. This validation should also be ensuring that the configuration block attributes are Type: TypeList and MaxItems: 1, to match the only supported behaviors at the moment.

As for the enhancement request, it seems like the ConflictsWith fields (and friends) could benefit from supporting relative references so that they could handle TypeList and TypeSet attributes, e.g.

Schema: map[string]*schema.Schema{
	"config_block_attr": {
		Type:     schema.TypeSet,
		Optional: true,
		Elem: &schema.Resource{
			Schema: map[string]*schema.Schema{
				"child_attr1": {
					Type:     schema.TypeString,
					Optional: true,
					ConflictsWith: []string{".child_attr2"},
				},
				"child_attr2": {
					Type:     schema.TypeString,
					Optional: true,
					ConflictsWith: []string{".child_attr1"},
				},
			},
		},
	},
},

Where the configuration validation could occur at the same parent-children and level of schema. It would probably need to be restricted to this specific setup rather than allowing arbitrary schema decisions. Attributes cannot begin with a period, so this new syntax should not conflict with (😅 ) existing syntax.

This is just a personal design opinion though that the SDK maintainers would need to validate if its possible and review/approve the design.

This issue has been around a long time... Is there any possibility of getting relative paths in attribute validation working??