From 01e75e0fc39bc85bfb8716d31fdc187c6c91aa7b Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Mon, 13 Apr 2015 19:04:10 -0500 Subject: [PATCH] google: simplify instance metadata schema It doesn't need to be a List of Maps, it can just be a Map. We're also safe to remove a previous workaround I stuck in there. The config parsing is equivalent between a list of maps and a plain map, so we just need a state migration to make this backwards compatible. --- .../google/resource_compute_instance.go | 38 ++++------ .../resource_compute_instance_migrate.go | 72 ++++++++++++++++++ .../resource_compute_instance_migrate_test.go | 75 +++++++++++++++++++ .../google/resource_compute_instance_test.go | 4 + 4 files changed, 166 insertions(+), 23 deletions(-) create mode 100644 builtin/providers/google/resource_compute_instance_migrate.go create mode 100644 builtin/providers/google/resource_compute_instance_migrate_test.go diff --git a/builtin/providers/google/resource_compute_instance.go b/builtin/providers/google/resource_compute_instance.go index c7f0f8d37..61104b0b6 100644 --- a/builtin/providers/google/resource_compute_instance.go +++ b/builtin/providers/google/resource_compute_instance.go @@ -18,6 +18,9 @@ func resourceComputeInstance() *schema.Resource { Update: resourceComputeInstanceUpdate, Delete: resourceComputeInstanceDelete, + SchemaVersion: 1, + MigrateState: resourceComputeInstanceMigrateState, + Schema: map[string]*schema.Schema{ "name": &schema.Schema{ Type: schema.TypeString, @@ -168,11 +171,9 @@ func resourceComputeInstance() *schema.Resource { }, "metadata": &schema.Schema{ - Type: schema.TypeList, + Type: schema.TypeMap, Optional: true, - Elem: &schema.Schema{ - Type: schema.TypeMap, - }, + Elem: schema.TypeString, }, "service_account": &schema.Schema{ @@ -735,6 +736,7 @@ func resourceComputeInstanceDelete(d *schema.ResourceData, meta interface{}) err config := meta.(*Config) zone := d.Get("zone").(string) + log.Printf("[INFO] Requesting instance deletion: %s", d.Id()) op, err := config.clientCompute.Instances.Delete(config.Project, zone, d.Id()).Do() if err != nil { return fmt.Errorf("Error deleting instance: %s", err) @@ -751,32 +753,22 @@ func resourceComputeInstanceDelete(d *schema.ResourceData, meta interface{}) err } func resourceInstanceMetadata(d *schema.ResourceData) *compute.Metadata { - var metadata *compute.Metadata - if metadataList := d.Get("metadata").([]interface{}); len(metadataList) > 0 { - m := new(compute.Metadata) - m.Items = make([]*compute.MetadataItems, 0, len(metadataList)) - for _, metadataMap := range metadataList { - for key, val := range metadataMap.(map[string]interface{}) { - // TODO: fix https://github.com/hashicorp/terraform/issues/883 - // and remove this workaround <3 phinze - if key == "#" { - continue - } - m.Items = append(m.Items, &compute.MetadataItems{ - Key: key, - Value: val.(string), - }) - } + m := &compute.Metadata{} + if mdMap := d.Get("metadata").(map[string]interface{}); len(mdMap) > 0 { + m.Items = make([]*compute.MetadataItems, 0, len(mdMap)) + for key, val := range mdMap { + m.Items = append(m.Items, &compute.MetadataItems{ + Key: key, + Value: val.(string), + }) } // Set the fingerprint. If the metadata has never been set before // then this will just be blank. m.Fingerprint = d.Get("metadata_fingerprint").(string) - - metadata = m } - return metadata + return m } func resourceInstanceTags(d *schema.ResourceData) *compute.Tags { diff --git a/builtin/providers/google/resource_compute_instance_migrate.go b/builtin/providers/google/resource_compute_instance_migrate.go new file mode 100644 index 000000000..dd883f0f5 --- /dev/null +++ b/builtin/providers/google/resource_compute_instance_migrate.go @@ -0,0 +1,72 @@ +package google + +import ( + "fmt" + "log" + "strconv" + "strings" + + "github.com/hashicorp/terraform/terraform" +) + +func resourceComputeInstanceMigrateState( + v int, is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) { + if is.Empty() { + log.Println("[DEBUG] Empty InstanceState; nothing to migrate.") + return is, nil + } + + switch v { + case 0: + log.Println("[INFO] Found Compute Instance State v0; migrating to v1") + return migrateStateV0toV1(is) + default: + return is, fmt.Errorf("Unexpected schema version: %d", v) + } +} + +func migrateStateV0toV1(is *terraform.InstanceState) (*terraform.InstanceState, error) { + log.Printf("[DEBUG] Attributes before migration: %#v", is.Attributes) + + // Delete old count + delete(is.Attributes, "metadata.#") + + newMetadata := make(map[string]string) + + for k, v := range is.Attributes { + if !strings.HasPrefix(k, "metadata.") { + continue + } + + // We have a key that looks like "metadata.*" and we know it's not + // metadata.# because we deleted it above, so it must be metadata.. + // from the List of Maps. Just need to convert it to a single Map by + // ditching the '' field. + kParts := strings.SplitN(k, ".", 3) + + // Sanity check: all three parts should be there and should be a number + badFormat := false + if len(kParts) != 3 { + badFormat = true + } else if _, err := strconv.Atoi(kParts[1]); err != nil { + badFormat = true + } + + if badFormat { + return is, fmt.Errorf( + "migration error: found metadata key in unexpected format: %s", k) + } + + // Rejoin as "metadata." + newK := strings.Join([]string{kParts[0], kParts[2]}, ".") + newMetadata[newK] = v + delete(is.Attributes, k) + } + + for k, v := range newMetadata { + is.Attributes[k] = v + } + + log.Printf("[DEBUG] Attributes after migration: %#v", is.Attributes) + return is, nil +} diff --git a/builtin/providers/google/resource_compute_instance_migrate_test.go b/builtin/providers/google/resource_compute_instance_migrate_test.go new file mode 100644 index 000000000..2bf01ff67 --- /dev/null +++ b/builtin/providers/google/resource_compute_instance_migrate_test.go @@ -0,0 +1,75 @@ +package google + +import ( + "testing" + + "github.com/hashicorp/terraform/terraform" +) + +func TestComputeInstanceMigrateState(t *testing.T) { + cases := map[string]struct { + StateVersion int + Attributes map[string]string + Expected map[string]string + Meta interface{} + }{ + "v0.4.2 and earlier": { + StateVersion: 0, + Attributes: map[string]string{ + "metadata.#": "2", + "metadata.0.foo": "bar", + "metadata.1.baz": "qux", + "metadata.2.with.dots": "should.work", + }, + Expected: map[string]string{ + "metadata.foo": "bar", + "metadata.baz": "qux", + "metadata.with.dots": "should.work", + }, + }, + } + + for tn, tc := range cases { + is := &terraform.InstanceState{ + ID: "i-abc123", + Attributes: tc.Attributes, + } + is, err := resourceComputeInstanceMigrateState( + tc.StateVersion, is, tc.Meta) + + if err != nil { + t.Fatalf("bad: %s, err: %#v", tn, err) + } + + for k, v := range tc.Expected { + if is.Attributes[k] != v { + t.Fatalf( + "bad: %s\n\n expected: %#v -> %#v\n got: %#v -> %#v\n in: %#v", + tn, k, v, k, is.Attributes[k], is.Attributes) + } + } + } +} + +func TestComputeInstanceMigrateState_empty(t *testing.T) { + var is *terraform.InstanceState + var meta interface{} + + // should handle nil + is, err := resourceComputeInstanceMigrateState(0, is, meta) + + if err != nil { + t.Fatalf("err: %#v", err) + } + if is != nil { + t.Fatalf("expected nil instancestate, got: %#v", is) + } + + // should handle non-nil but empty + is = &terraform.InstanceState{} + is, err = resourceComputeInstanceMigrateState(0, is, meta) + + if err != nil { + t.Fatalf("err: %#v", err) + } +} diff --git a/builtin/providers/google/resource_compute_instance_test.go b/builtin/providers/google/resource_compute_instance_test.go index 9c53fae04..efffd48b5 100644 --- a/builtin/providers/google/resource_compute_instance_test.go +++ b/builtin/providers/google/resource_compute_instance_test.go @@ -47,6 +47,7 @@ func TestAccComputeInstance_basic(t *testing.T) { "google_compute_instance.foobar", &instance), testAccCheckComputeInstanceTag(&instance, "foo"), testAccCheckComputeInstanceMetadata(&instance, "foo", "bar"), + testAccCheckComputeInstanceMetadata(&instance, "baz", "qux"), testAccCheckComputeInstanceDisk(&instance, "terraform-test", true, true), ), }, @@ -387,6 +388,9 @@ resource "google_compute_instance" "foobar" { metadata { foo = "bar" } + metadata { + baz = "qux" + } }` const testAccComputeInstance_basic2 = `