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

Clear only operates on computed keys - secondary_ip_range is not one #3477

Closed
pdecat opened this issue Apr 23, 2019 · 12 comments · Fixed by GoogleCloudPlatform/magic-modules#1686
Assignees
Labels
bug forward/review In review; remove label to forward service/compute-vpc

Comments

@pdecat
Copy link
Contributor

pdecat commented Apr 23, 2019

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment
  • If an issue is assigned to the "modular-magician" user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If an issue is assigned to a user, that user is claiming responsibility for the issue. If an issue is assigned to "hashibot", a community member has claimed the issue already.

Terraform Version

# terraform version
Terraform v0.11.13
+ provider.google v2.5.1
+ provider.google-beta v2.5.1

Same issue with google and google-beta 2.5.0.

Downgrading to 2.4.0 works.

Affected Resource(s)

  • google_compute_subnetwork

Terraform Configuration Files

resource "google_compute_subnetwork" "subnet0" {
  name = "myproject-preprod-europe-west1-subnet0"

  network       = "https://www.googleapis.com/compute/v1/projects/myproject-preprod/global/networks/myproject-preprod-net0"
  ip_cidr_range = "10.118.128.160/28"

  secondary_ip_range {
    range_name    = "myproject-preprod-europe-west1-subnet0-secondary-range-0"
    ip_cidr_range = "10.132.0.0/14"
  }

  secondary_ip_range {
    range_name    = "myproject-preprod-europe-west1-subnet0-secondary-range-1"
    ip_cidr_range = "10.129.0.0/16"
  }

  // Enable Private Google Access to allow pods to access Google APIs without public IP addresses
  // cf. https://cloud.google.com/vpc/docs/private-access-options#pga-supported
  private_ip_google_access = "true"
}

resource "google_compute_subnetwork" "subnet1" {
  name = "myproject-preprod-europe-west1-subnet1"

  network       = "https://www.googleapis.com/compute/v1/projects/myproject-preprod/global/networks/myproject-preprod-net0"
  ip_cidr_range = "172.16.16.0/20"

  secondary_ip_range {
    range_name    = "myproject-preprod-europe-west1-subnet1-secondary-range-0"
    ip_cidr_range = "10.140.0.0/14"
  }

  secondary_ip_range {
    range_name    = "myproject-preprod-europe-west1-subnet1-secondary-range-1"
    ip_cidr_range = "10.137.0.0/16"
  }

  // Enable Private Google Access to allow pods to access Google APIs without public IP addresses
  // cf. https://cloud.google.com/vpc/docs/private-access-options#pga-supported
  private_ip_google_access = "true"
}

Debug Output

https://gist.github.com/pdecat/673f5e7ffb290322b6fe4182469a62f5

Panic Output

N/A

Expected Behavior

terraform plan -target module.net0.google_compute_subnetwork.subnet1 should detect no change.

Actual Behavior

The following error occurs:

* module.net0.google_compute_subnetwork.subnet1: 1 error occurred:
        * Clear only operates on computed keys - secondary_ip_range is not one

Steps to Reproduce

  1. terraform plan -target module.net0.google_compute_subnetwork.subnet1

Important Factoids

That's maybe new behavior introduced by using the terraform 0.12 SDK with terraform 0.11.13.

In the same module, there's a similarly configured subnet0 resource that's not affected:

# terraform plan -target module.net0.google_compute_subnetwork.subnet0
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

google_compute_network.net0: Refreshing state... (ID: myproject-preprod-net0)
google_compute_subnetwork.subnet0: Refreshing state... (ID: europe-west1/myproject-preprod-europe-west1-subnet0)

------------------------------------------------------------------------

No changes. Infrastructure is up-to-date.

This means that Terraform did not detect any differences between your
configuration and real physical resources that exist. As a result, no
actions need to be performed.

References

@ghost ghost added the bug label Apr 23, 2019
@pdecat
Copy link
Contributor Author

pdecat commented Apr 23, 2019

The order of the secondary ranges returned by the Google API is reversed for subnet1:

# gcloud --project=myproject-preprod compute networks subnets describe myproject-preprod-europe-west1-subnet1 --region europe-west1
creationTimestamp: '2016-11-04T07:10:41.813-07:00'
fingerprint: AVPZ0iZ03Ks=
gatewayAddress: 172.16.16.1
id: '123456789'
ipCidrRange: 172.16.16.0/20
kind: compute#subnetwork
name: myproject-preprod-europe-west1-subnet1
network: https://www.googleapis.com/compute/v1/projects/myproject-preprod/global/networks/myproject-preprod-net0
privateIpGoogleAccess: true
region: https://www.googleapis.com/compute/v1/projects/myproject-preprod/regions/europe-west1
secondaryIpRanges:
- ipCidrRange: 10.137.0.0/16
  rangeName: myproject-preprod-europe-west1-subnet1-secondary-range-1
- ipCidrRange: 10.140.0.0/14
  rangeName: myproject-preprod-europe-west1-subnet1-secondary-range-0
selfLink: https://www.googleapis.com/compute/v1/projects/myproject-preprod/regions/europe-west1/subnetworks/myproject-preprod-europe-west1-subnet1

For subnet0 which does not have the issue:

# gcloud --project=myproject-preprod compute networks subnets describe myproject-preprod-europe-west1-subnet0 --region europe-west1
creationTimestamp: '2016-10-17T06:51:43.384-07:00'
fingerprint: 4NbXjznut0c=
gatewayAddress: 10.118.128.161
id: '123456789'
ipCidrRange: 10.118.128.160/28
kind: compute#subnetwork
name: myproject-preprod-europe-west1-subnet0
network: https://www.googleapis.com/compute/v1/projects/myproject-preprod/global/networks/myproject-preprod-net0
privateIpGoogleAccess: true
region: https://www.googleapis.com/compute/v1/projects/myproject-preprod/regions/europe-west1
secondaryIpRanges:
- ipCidrRange: 10.132.0.0/14
  rangeName: myproject-preprod-europe-west1-subnet0-secondary-range-0
- ipCidrRange: 10.129.0.0/16
  rangeName: myproject-preprod-europe-west1-subnet0-secondary-range-1
selfLink: https://www.googleapis.com/compute/v1/projects/myproject-preprod/regions/europe-west1/subnetworks/myproject-preprod-europe-west1-subnet0

@pdecat
Copy link
Contributor Author

pdecat commented Apr 23, 2019

Removing the resource from the state then reimporting it does not help:

# terraform state rm module.net0.google_compute_subnetwork.subnet1                                                                                                                      
1 items removed.
Item removal successful.
# terraform import module.net0.google_compute_subnetwork.subnet1 https://www.googleapis.com/compute/v1/projects/myproject-preprod/regions/europe-west1/subnetworks/myproject-preprod-europe-west1-subnet1
module.net0.google_compute_subnetwork.subnet1: Importing from ID "https://www.googleapis.com/compute/v1/projects/myproject-preprod/regions/europe-west1/subnetworks/myproject-preprod-europe-west1-subnet1"...
module.net0.google_compute_subnetwork.subnet1: Import complete!
  Imported google_compute_subnetwork (ID: europe-west1/myproject-preprod-europe-west1-subnet1)
module.net0.google_compute_subnetwork.subnet1: Refreshing state... (ID: europe-west1/myproject-preprod-europe-west1-subnet1)

Import successful!

The resources that were imported are shown above. These resources are now in
your Terraform state and will henceforth be managed by Terraform.
# terraform plan -target module.net0.google_compute_subnetwork.subnet1
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

google_compute_network.net0: Refreshing state... (ID: myproject-preprod-net0)
google_compute_subnetwork.subnet1: Refreshing state... (ID: europe-west1/myproject-preprod-europe-west1-subnet1)

------------------------------------------------------------------------

Error: Error running plan: 1 error(s) occurred:

* module.net0.google_compute_subnetwork.subnet1: 1 error(s) occurred:

* module.net0.google_compute_subnetwork.subnet1: 1 error occurred:
        * Clear only operates on computed keys - secondary_ip_range is not one

@pdecat
Copy link
Contributor Author

pdecat commented Apr 23, 2019

Reordering the secondary_ip_range block on the subnet1 configuration eliminates the issue, the plan detects no change.
That's however not a practical work-around as this module is reused for another prod workspace and the issue there is different: both subnet0 and subnet1 exhibit the issue.

@pdecat
Copy link
Contributor Author

pdecat commented Apr 24, 2019

The issue can be reproduced by running the acceptance tests with the following change:

diff --git a/google/resource_compute_subnetwork_test.go b/google/resource_compute_subnetwork_test.go                                                                                    
index f97decbb..6005475d 100644
--- a/google/resource_compute_subnetwork_test.go
+++ b/google/resource_compute_subnetwork_test.go
@@ -159,6 +159,14 @@ func TestAccComputeSubnetwork_secondaryIpRanges(t *testing.T) {
                                        testAccCheckComputeSubnetworkHasSecondaryIpRange(&subnetwork, "tf-test-secondary-range-update2", "192.168.11.0/24"),                            
                                ),
                        },
+                       {
+                               Config: testAccComputeSubnetwork_secondaryIpRanges_update3(cnName, subnetworkName),                                                                     
+                               Check: resource.ComposeTestCheckFunc(
+                                       testAccCheckComputeSubnetworkExists("google_compute_subnetwork.network-with-private-secondary-ip-ranges", &subnetwork),                         
+                                       testAccCheckComputeSubnetworkHasSecondaryIpRange(&subnetwork, "tf-test-secondary-range-update1", "192.168.10.0/24"),                            
+                                       testAccCheckComputeSubnetworkHasSecondaryIpRange(&subnetwork, "tf-test-secondary-range-update2", "192.168.11.0/24"),                            
+                               ),
+                       },
                        {
                                Config: testAccComputeSubnetwork_secondaryIpRanges_update1(cnName, subnetworkName),                                                                     
                                Check: resource.ComposeTestCheckFunc(
@@ -384,6 +392,30 @@ resource "google_compute_subnetwork" "network-with-private-secondary-ip-ranges"                                                                                    
 `, cnName, subnetworkName)
 }

+func testAccComputeSubnetwork_secondaryIpRanges_update3(cnName, subnetworkName string) string {                                                                                        
+       return fmt.Sprintf(`
+resource "google_compute_network" "custom-test" {
+       name = "%s"
+       auto_create_subnetworks = false
+}
+
+resource "google_compute_subnetwork" "network-with-private-secondary-ip-ranges" {
+       name = "%s"
+       ip_cidr_range = "10.2.0.0/16"
+       region = "us-central1"
+       network = "${google_compute_network.custom-test.self_link}"
+       secondary_ip_range {
+               range_name = "tf-test-secondary-range-update2"
+               ip_cidr_range = "192.168.11.0/24"
+       }
+       secondary_ip_range {
+               range_name = "tf-test-secondary-range-update1"
+               ip_cidr_range = "192.168.10.0/24"
+       }
+}
+`, cnName, subnetworkName)
+}
+
 func testAccComputeSubnetwork_flowLogs(cnName, subnetworkName string, enableLogs bool) string {                                                                                        
        return fmt.Sprintf(`
 resource "google_compute_network" "custom-test" {

Output:

# make testacc TEST='./google' TESTARGS='-run=TestAccComputeSubnetwork_secondaryIpRanges -count=1'                                                                                     
==> Checking source code against gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google -v -run=TestAccComputeSubnetwork_secondaryIpRanges -count=1 -timeout 240m -ldflags="-X=github.com/terraform-providers/terraform-provider-google/version.ProviderVersion=acc"
=== RUN   TestAccComputeSubnetwork_secondaryIpRanges
=== PAUSE TestAccComputeSubnetwork_secondaryIpRanges
=== CONT  TestAccComputeSubnetwork_secondaryIpRanges
--- FAIL: TestAccComputeSubnetwork_secondaryIpRanges (175.16s)
    testing.go:568: Step 2 error: errors during plan:
        
        Error: 1 error occurred:
                * Clear only operates on computed keys - secondary_ip_range is not one
        
        
        
          on /tmp/tf-test404995511/main.tf line 7:
          (source code not available)
        
        
FAIL
FAIL    github.com/terraform-providers/terraform-provider-google/google 175.191s
make: *** [GNUmakefile:16: testacc] Error 1

@pdecat
Copy link
Contributor Author

pdecat commented Apr 24, 2019

Simply swapping the secondary_ip_range blocks in testAccComputeSubnetwork_secondaryIpRanges_update2() triggers the issue.

@pdecat
Copy link
Contributor Author

pdecat commented Apr 24, 2019

This is the culprit: 8279ae6#diff-c00013899b923b404c727949952f35ffL120

@pdecat
Copy link
Contributor Author

pdecat commented Apr 24, 2019

Adding Computed: true back to google_compute_subnetwork's secondary_ip_range field resolves the issue.

@pdecat
Copy link
Contributor Author

pdecat commented Apr 24, 2019

Hi @rileykarson, I believe the aforementioned change was unintentional, would you mind having a look at this somewhat blocking regression?

@pdecat
Copy link
Contributor Author

pdecat commented Apr 24, 2019

Oh, the real culprit seems to be GoogleCloudPlatform/magic-modules@19b8ac5

@rileykarson
Copy link
Collaborator

Thanks for filing and for the reproduction @pdecat! For context, we removed the Computed identifier from the field as part of our provider's resolution to hashicorp/terraform#20505, and didn't notice that this broke the CustomizeDiff because no test exercised it. Computed + Optional nested blocks worked by coincidence in Terraform 0.11 and earlier, and were broken by Terraform 0.12, so in any opportunities possible we wanted to move away from using them, and we misidentified this case.

The PR you've highlighted is the one that changed that functionality, unfortunately it doesn't look like the PR's owner added it to the changelog. I'll amend that and note that it's a known issue in 2.5.X releases.

This also impacted some GKE users, for whom GKE was adding secondary ranges to their subnetworks automatically. This was reported in the GCP Community Slack.

For 2.6.0, I'll add back the Computed markup and apply the fixup from hashicorp/terraform#20837 to allow empty secondary ranges to be explicitly specified again.

@rileykarson rileykarson self-assigned this Apr 25, 2019
@pdecat
Copy link
Contributor Author

pdecat commented Apr 25, 2019

Thanks for the feedback @rileykarson.

FWIW, I'm a GKE user with alias IP/container native load balancing enabled and that's why I'm affected too.

@ghost
Copy link

ghost commented May 31, 2019

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators May 31, 2019
@github-actions github-actions bot added forward/review In review; remove label to forward service/compute-vpc labels Jan 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug forward/review In review; remove label to forward service/compute-vpc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants