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

Subnetwork: Infers region from zone before using the provider-level region #938

Merged
merged 1 commit into from
Jan 9, 2018

Conversation

rosbo
Copy link
Contributor

@rosbo rosbo commented Jan 9, 2018

Fixes #926

Affects resources with a zone field in its schema and using the getRegion method:

  • google_compute_instance
  • google_compute_disk
  • google_compute_instance_template

This change affects how region is inferred.

The main goal is to change how the region is inferred when a subnetwork is set using the name only (not a self_link).

We used to fallback to the provider-level region first. I changed that to infer the region from the zone in the schema first.

For example, given this config:

resource "google_compute_instance" "default" {
  name = "test"
  zone = "asia-northeast1-b"
  machine_type = "g1-small"

  boot_disk {
    initialize_params {
      image = "${var.instance_image}"
      type = "pd-ssd"
    }
  }

  network_interface {
    subnetwork = "mirror"
    # Before, wrong region 
   # After, region is
  }
}

provider "google" {
  project = "${var.provider_project}"
  region = "us-central1"
} 

Before this change, the `subnetwork` used would be `projects/a-project/regions/us-central1/subnetworks/mirror`. 

After this change, the `subnetwork` used would be `projects/a-project/regions/asia-northeast1/subnetworks/mirror` which is what we want.

This should not break anybody because the region should match the zone for existing resource. It does help when people create new config using only the name for regional resources.

// - provider-level region
// - region extracted from the provider-level zone
func getRegionFromSchema(regionSchemaField, zoneSchemaField string, d TerraformResourceData, config *Config) (string, error) {
if v, ok := d.GetOk(regionSchemaField); ok && regionSchemaField != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: doesn't the "ok" mean that it isn't empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes:
// GetOk returns the data for the given key and whether or not the key
// has been set to a non-zero value at some point.

The check on regionSchemaField is for cases where the field name is set to "" to basically skip checking the schema.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh oh oh I was reading it wrong, I see now

@rosbo rosbo merged commit da3a6f1 into hashicorp:master Jan 9, 2018
@rosbo rosbo deleted the infers-region-zone branch January 9, 2018 21:57
modular-magician added a commit to modular-magician/terraform-provider-google that referenced this pull request Sep 27, 2019
Signed-off-by: Modular Magician <magic-modules@google.com>
@ghost
Copy link

ghost commented Mar 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 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 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

google_compute_instance: Subnetwork name expanded with wrong region
2 participants