diff --git a/.changelog/5329.txt b/.changelog/5329.txt new file mode 100644 index 0000000000..ad159ad6be --- /dev/null +++ b/.changelog/5329.txt @@ -0,0 +1,3 @@ +```release-note:breaking-change +compute: changed the import / drift detection behaviours for `metadata_startup_script`, `metadata.startup-script` in `google_compute_instance`. Now, `metadata.startup-script` will be set by default, and `metadata_startup_script` will only be set if present. +``` diff --git a/google-beta/resource_compute_instance.go b/google-beta/resource_compute_instance.go index 836a257270..8379d49e0c 100644 --- a/google-beta/resource_compute_instance.go +++ b/google-beta/resource_compute_instance.go @@ -1141,19 +1141,15 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error } md := flattenMetadataBeta(instance.Metadata) - existingMetadata := d.Get("metadata").(map[string]interface{}) - // If the existing config specifies "metadata.startup-script" instead of "metadata_startup_script", - // we shouldn't move the remote metadata.startup-script to metadata_startup_script. Otherwise, - // we should. - if _, ok := existingMetadata["startup-script"]; !ok { + // If the existing state contains "metadata_startup_script" instead of "metadata.startup-script", + // we should move the remote metadata.startup-script to metadata_startup_script to avoid + // specifying it in two places. + if _, ok := d.GetOk("metadata_startup_script"); ok { if err := d.Set("metadata_startup_script", md["startup-script"]); err != nil { return fmt.Errorf("Error setting metadata_startup_script: %s", err) } - // Note that here we delete startup-script from our metadata list. This is to prevent storing the startup-script - // as a value in the metadata since the config specifically tracks it under 'metadata_startup_script' - delete(md, "startup-script") - } else if _, ok := d.GetOk("metadata_startup_script"); ok { + delete(md, "startup-script") } diff --git a/google-beta/resource_compute_instance_template.go b/google-beta/resource_compute_instance_template.go index 07e22f7e99..fd474c8c17 100644 --- a/google-beta/resource_compute_instance_template.go +++ b/google-beta/resource_compute_instance_template.go @@ -1313,8 +1313,10 @@ func resourceComputeInstanceTemplateRead(d *schema.ResourceData, meta interface{ if err = d.Set("metadata_startup_script", script); err != nil { return fmt.Errorf("Error setting metadata_startup_script: %s", err) } + delete(_md, "startup-script") } + if err = d.Set("metadata", _md); err != nil { return fmt.Errorf("Error setting metadata: %s", err) } diff --git a/google-beta/resource_compute_instance_test.go b/google-beta/resource_compute_instance_test.go index 5fef0ae082..b14d9d407c 100644 --- a/google-beta/resource_compute_instance_test.go +++ b/google-beta/resource_compute_instance_test.go @@ -76,8 +76,8 @@ func testSweepComputeInstance(region string) error { func computeInstanceImportStep(zone, instanceName string, additionalImportIgnores []string) resource.TestStep { // metadata is only read into state if set in the config - // since importing doesn't know whether metadata.startup_script vs metadata_startup_script is set in the config, - // it guesses metadata_startup_script + // importing doesn't know whether metadata.startup_script vs metadata_startup_script is set in the config, + // it always takes metadata.startup-script ignores := []string{"metadata.%", "metadata.startup-script", "metadata_startup_script"} return resource.TestStep{ diff --git a/google-beta/resource_dataproc_cluster_test.go b/google-beta/resource_dataproc_cluster_test.go index 798461eee3..e3c9d32ff6 100644 --- a/google-beta/resource_dataproc_cluster_test.go +++ b/google-beta/resource_dataproc_cluster_test.go @@ -13,9 +13,8 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" - "google.golang.org/api/googleapi" - dataproc "google.golang.org/api/dataproc/v1beta2" + "google.golang.org/api/googleapi" ) func TestDataprocExtractInitTimeout(t *testing.T) { diff --git a/website/docs/guides/version_4_upgrade.html.markdown b/website/docs/guides/version_4_upgrade.html.markdown index 022dbdf387..3eeeffe742 100644 --- a/website/docs/guides/version_4_upgrade.html.markdown +++ b/website/docs/guides/version_4_upgrade.html.markdown @@ -277,6 +277,28 @@ The provider will now enforce at plan time that one of these fields be set. Previously, if all of these fields were left empty, the firewall defaulted to allowing traffic from 0.0.0.0/0, which is a suboptimal default. +## Resource: `google_compute_instance` + +### `metadata_startup_script` is no longer set on import + +Earlier versions of the provider set the `metadata_startup_script` value on +import, omitting the value of `metadata.startup-script` for historical backwards +compatibility. This was dangerous in practice, as `metadata_startup_script` +would flag an instance for recreation if the values differed rather than for +just an update. + +In `4.0.0` the behaviour has been flipped, and `metadata.startup-script` is the +default value that gets written. Users who want `metadata_startup_script` set +on an imported instance will need to modify their state manually. This is more +consistent with our expectations for the field, that a user who manages an +instance **only** through Terraform uses it but that most users should prefer +the `metadata` block. + +No action is required for user configs with instances already imported. If you +have a config or module where neither is specified- where `import` will be run, +or an old config that is not reconciled with the API- the value that gets set +will change. + ## Resource: `google_compute_instance_group_manager` ### `update_policy.min_ready_sec` is removed from the GA provider diff --git a/website/docs/r/compute_instance.html.markdown b/website/docs/r/compute_instance.html.markdown index 0ef367216a..95d40c677d 100644 --- a/website/docs/r/compute_instance.html.markdown +++ b/website/docs/r/compute_instance.html.markdown @@ -132,16 +132,16 @@ The following arguments are supported: we provide a special attribute, `metadata_startup_script`, which is documented below. * `metadata_startup_script` - (Optional) An alternative to using the - startup-script metadata key, except this one forces the instance to be - recreated (thus re-running the script) if it is changed. This replaces the - startup-script metadata key on the created instance and thus the two - mechanisms are not allowed to be used simultaneously. Users are free to use - either mechanism - the only distinction is that this separate attribute - will cause a recreate on modification. On import, `metadata_startup_script` - will be set, but `metadata.startup-script` will not - if you choose to use the - other mechanism, you will see a diff immediately after import, which will cause a - destroy/recreate operation. You may want to modify your state file manually - using `terraform state` commands, depending on your use case. +startup-script metadata key, except this one forces the instance to be recreated +(thus re-running the script) if it is changed. This replaces the startup-script +metadata key on the created instance and thus the two mechanisms are not +allowed to be used simultaneously. Users are free to use either mechanism - the +only distinction is that this separate attribute will cause a recreate on +modification. On import, `metadata_startup_script` will not be set - if you +choose to specify it you will see a diff immediately after import causing a +destroy/recreate operation. If importing an instance and specifying this value +is desired, you will need to modify your state file manually using +`terraform state` commands. * `min_cpu_platform` - (Optional) Specifies a minimum CPU platform for the VM instance. Applicable values are the friendly names of CPU platforms, such as `Intel Haswell` or `Intel Skylake`. See the complete list [here](https://cloud.google.com/compute/docs/instances/specify-min-cpu-platform).