forked from hashicorp/terraform
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
consul: Fix several problems w/ consul_keys update
Implementation notes: * The hash implementation was not considering key value, causing "diffs did not match" errors when a value was updated. Switching to default HashResource implementation fixes this * Using HashResource as a default exposed a bug in helper/schema that needed to be fixed so the Set function is picked up properly during Read * Stop writing back values into the `key` attribute; it triggers extra diffs when `default` is used. Computed values all just go into `var`. * Includes a state migration to prevent unnecessary diffs based on "key" attribute hashcodes changing. In the tests: * Switch from leaning on the public demo Consul instance to requiring a CONSUL_HTTP_ADDR variable be set pointing to a `consul agent -dev` instance to be used only for testing. * Add a test that exposes the updating issues and covers the fixes Fixes hashicorp#774 Fixes hashicorp#1866 Fixes hashicorp#3023
- Loading branch information
Showing
6 changed files
with
256 additions
and
74 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| package consul | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "log" | ||
| "strings" | ||
|
|
||
| "github.com/hashicorp/terraform/helper/schema" | ||
| "github.com/hashicorp/terraform/terraform" | ||
| ) | ||
|
|
||
| func resourceConsulKeysMigrateState( | ||
| v int, is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) { | ||
| switch v { | ||
| case 0: | ||
| log.Println("[INFO] Found consul_keys State v0; migrating to v1") | ||
| return resourceConsulKeysMigrateStateV0toV1(is) | ||
| default: | ||
| return is, fmt.Errorf("Unexpected schema version: %d", v) | ||
| } | ||
| } | ||
|
|
||
| func resourceConsulKeysMigrateStateV0toV1(is *terraform.InstanceState) (*terraform.InstanceState, error) { | ||
| if is.Empty() || is.Attributes == nil { | ||
| log.Println("[DEBUG] Empty InstanceState; nothing to migrate.") | ||
| return is, nil | ||
| } | ||
|
|
||
| log.Printf("[DEBUG] Attributes before migration: %#v", is.Attributes) | ||
|
|
||
| res := resourceConsulKeys() | ||
| keys, err := readV0Keys(is, res) | ||
| if err != nil { | ||
| return is, err | ||
| } | ||
| if err := clearV0Keys(is); err != nil { | ||
| return is, err | ||
| } | ||
| if err := writeV1Keys(is, res, keys); err != nil { | ||
| return is, err | ||
| } | ||
|
|
||
| log.Printf("[DEBUG] Attributes after migration: %#v", is.Attributes) | ||
| return is, nil | ||
| } | ||
|
|
||
| func readV0Keys( | ||
| is *terraform.InstanceState, | ||
| res *schema.Resource, | ||
| ) (*schema.Set, error) { | ||
| reader := &schema.MapFieldReader{ | ||
| Schema: res.Schema, | ||
| Map: schema.BasicMapReader(is.Attributes), | ||
| } | ||
| result, err := reader.ReadField([]string{"key"}) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| oldKeys, ok := result.Value.(*schema.Set) | ||
| if !ok { | ||
| return nil, fmt.Errorf("Got unexpected value from state: %#v", result.Value) | ||
| } | ||
| return oldKeys, nil | ||
| } | ||
|
|
||
| func clearV0Keys(is *terraform.InstanceState) error { | ||
| for k := range is.Attributes { | ||
| if strings.HasPrefix(k, "key.") { | ||
| delete(is.Attributes, k) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func writeV1Keys( | ||
| is *terraform.InstanceState, | ||
| res *schema.Resource, | ||
| keys *schema.Set, | ||
| ) error { | ||
| writer := schema.MapFieldWriter{ | ||
| Schema: res.Schema, | ||
| } | ||
| if err := writer.WriteField([]string{"key"}, keys); err != nil { | ||
| return err | ||
| } | ||
| for k, v := range writer.Map() { | ||
| is.Attributes[k] = v | ||
| } | ||
|
|
||
| return nil | ||
| } |
90 changes: 90 additions & 0 deletions
90
builtin/providers/consul/resource_consul_keys_migrate_test.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| package consul | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/hashicorp/terraform/terraform" | ||
| ) | ||
|
|
||
| func TestConsulKeysMigrateState(t *testing.T) { | ||
| cases := map[string]struct { | ||
| StateVersion int | ||
| Attributes map[string]string | ||
| Expected map[string]string | ||
| Meta interface{} | ||
| }{ | ||
| "v0.6.9 and earlier, with old values hash function": { | ||
| StateVersion: 0, | ||
| Attributes: map[string]string{ | ||
| "key.#": "2", | ||
| "key.12345.name": "hello", | ||
| "key.12345.path": "foo/bar", | ||
| "key.12345.value": "world", | ||
| "key.12345.default": "", | ||
| "key.12345.delete": "false", | ||
| "key.6789.name": "temp", | ||
| "key.6789.path": "foo/foo", | ||
| "key.6789.value": "", | ||
| "key.6789.default": "", | ||
| "key.6789.delete": "true", | ||
| }, | ||
| Expected: map[string]string{ | ||
| "key.#": "2", | ||
| "key.2401383718.default": "", | ||
| "key.2401383718.delete": "true", | ||
| "key.2401383718.name": "temp", | ||
| "key.2401383718.path": "foo/foo", | ||
| "key.2401383718.value": "", | ||
| "key.3116955509.path": "foo/bar", | ||
| "key.3116955509.default": "", | ||
| "key.3116955509.delete": "false", | ||
| "key.3116955509.name": "hello", | ||
| "key.3116955509.value": "world", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| for tn, tc := range cases { | ||
| is := &terraform.InstanceState{ | ||
| ID: "consul", | ||
| Attributes: tc.Attributes, | ||
| } | ||
| is, err := resourceConsulKeys().MigrateState( | ||
| 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 TestConsulKeysMigrateState_empty(t *testing.T) { | ||
| var is *terraform.InstanceState | ||
| var meta interface{} | ||
|
|
||
| // should handle nil | ||
| is, err := resourceConsulKeys().MigrateState(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 = resourceConsulKeys().MigrateState(0, is, meta) | ||
|
|
||
| if err != nil { | ||
| t.Fatalf("err: %#v", err) | ||
| } | ||
| } |
Oops, something went wrong.