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

Drop "Removed" functionality from schema.Schema #320

Closed
c2thorn opened this issue Feb 12, 2020 · 5 comments · Fixed by #414
Closed

Drop "Removed" functionality from schema.Schema #320

c2thorn opened this issue Feb 12, 2020 · 5 comments · Fixed by #414
Labels
enhancement New feature or request
Milestone

Comments

@c2thorn
Copy link

c2thorn commented Feb 12, 2020

SDK version

v1.4.0

Relevant provider source code

			"ip_allocation_policy": {
				Type:          schema.TypeList,
				MaxItems:      1,
				ForceNew:      true,
				Optional:      true,
				ConflictsWith: []string{"cluster_ipv4_cidr"},
				Elem: &schema.Resource{
					Schema: map[string]*schema.Schema{
	 						
						... some irrelevant attributes ... 

						"use_ip_aliases": {
							Type:     schema.TypeBool,
							Removed:  "This field is removed as of 3.0.0. If previously set to true, remove it from your config. If false, remove it.",
							Computed: true,
							Optional: true,
						},

						"create_subnetwork": {
							Type:     schema.TypeBool,
							Removed:  "This field is removed as of 3.0.0. Define an explicit google_compute_subnetwork and use subnetwork instead.",
							Computed: true,
							Optional: true,
						},

						"subnetwork_name": {
							Type:     schema.TypeString,
							Removed:  "This field is removed as of 3.0.0. Define an explicit google_compute_subnetwork and use subnetwork instead.",
							Computed: true,
							Optional: true,
						},

						"node_ipv4_cidr_block": {
							Type:     schema.TypeString,
							Removed:  "This field is removed as of 3.0.0. Define an explicit google_compute_subnetwork and use subnetwork instead.",
							Computed: true,
							Optional: true,
						},
					},
				},
			},

Terraform Configuration Files

resource "google_container_cluster" "test" {
  name               = "tf-test-cc"
  location           = "us-central1-a"
  initial_node_count = 1
  ip_allocation_policy {
  }
}

Debug Output

Expected Behavior

Actual Behavior

terraform apply wants to populate every field including the removed ones:

      + ip_allocation_policy {

			...

          + create_subnetwork             = (known after apply)
          + node_ipv4_cidr_block          = (known after apply)
          + subnetwork_name               = (known after apply)
          + use_ip_aliases                = (known after apply)
        }

The state shows the two boolean fields and not the string fields:

    ip_allocation_policy {

			...			

        create_subnetwork             = false
        use_ip_aliases                = false
    }

Steps to Reproduce

  1. terraform init
  2. terraform apply
  3. terraform show

References

Seems like the solution proposed in #285 (comment) would stop this behavior.

hashicorp/terraform-provider-google#5586

@paultyng
Copy link
Contributor

I think until the fix for #285 lands, you could potentially address this by not flagging these as computed?

@paultyng paultyng added the waiting-response Issues or pull requests waiting for an external response label Feb 12, 2020
@paultyng paultyng self-assigned this Feb 12, 2020
@c2thorn
Copy link
Author

c2thorn commented Feb 12, 2020

Just tried that out, and unfortunately they still show in state. I think it might have something to do with the attributes being within a list? Other removed booleans that appear in the schema root are not showing in state.

I also tried setting the booleans to nil manually before setting them:

d.Set("ip_allocation_policy", flattenIPAllocationPolicy(cluster, d, config))
func flattenIPAllocationPolicy(c *containerBeta.Cluster, d *schema.ResourceData, config *Config) []map[string]interface{} {
		...
	return []map[string]interface{}{
		{
 				... 
			"use_ip_aliases":                nil,
			"create_subnetwork":             nil,
		},
	}
}

But this just makes them default to false. If I set these two to true they show up as true.

I'm unable to do something like

d.Set("ip_allocation_policy.0.use_ip_aliases", nil)

because this returns

Error: ip_allocation_policy.0.use_ip_aliases: can only set full list

@ghost ghost removed waiting-response Issues or pull requests waiting for an external response labels Feb 12, 2020
@paultyng
Copy link
Contributor

Once we land v2 of the SDK we may be able to do some work on this. Unfortunately there is a bit of nuance in the Removed flag. Core doesn't actually have that concept, so the SDK is just emulating this by telling core these fields are part of the schema, but then erroring if they are supplied, so as far as core knows they are just normal schema fields, which is why it looks like this. Until we only have to support the new protocol though it will be hard to clean up, if its possible.

I think long term we will most likely remove this functionality (keeping deprecation though, so you can provide warnings up until the point of removal).

@paultyng paultyng added enhancement New feature or request and removed bug Something isn't working protocol-5-only labels Apr 27, 2020
@paultyng paultyng added this to the v2.0.0 milestone Apr 27, 2020
@paultyng paultyng removed their assignment Apr 27, 2020
@paultyng paultyng changed the title Removed fields still show in state Drop "Removed" functionality from schema.Schema Apr 27, 2020
@paultyng
Copy link
Contributor

Just a minor data point, I confirmed extra fields in state (not mentioned in schema) are silently ignored, so no need to track what the removed names should be.

@ghost
Copy link

ghost commented May 30, 2020

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 May 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants