From 6aac79e194be271e11a949852ec42ce8cd0b0265 Mon Sep 17 00:00:00 2001 From: James Nugent Date: Tue, 22 Mar 2016 14:22:33 +0000 Subject: [PATCH] state: Add support for outputs of multiple types This commit adds the groundwork for supporting module outputs of types other than string. In order to do so, the state version is increased from 1 to 2 (though the "public-facing" state version is actually as the first state file was binary). Tests are added to ensure that V2 (1) state is upgraded to V3 (2) state, though no separate read path is required since the V2 JSON will unmarshal correctly into the V3 structure. Outputs in a ModuleState are now of type map[string]interface{}, and a test covers round-tripping string, []string and map[string]string, which should cover all of the types in question. Type switches have been added where necessary to deal with the interface{} value, but they currently default to panicking when the input is not a string. --- builtin/providers/terraform/resource_state.go | 2 +- .../tls/resource_cert_request_test.go | 8 +- .../tls/resource_locally_signed_cert_test.go | 6 +- .../tls/resource_private_key_test.go | 57 ++++++++++-- .../tls/resource_self_signed_cert_test.go | 7 +- command/output.go | 8 +- command/output_test.go | 16 ++-- state/remote/atlas_test.go | 4 +- state/testing.go | 6 +- terraform/context_apply_test.go | 4 +- terraform/context_refresh_test.go | 6 +- terraform/interpolate.go | 35 +++++--- terraform/interpolate_test.go | 2 +- terraform/state.go | 19 ++-- terraform/state_test.go | 90 +++++++++++++++++++ terraform/transform_output_test.go | 2 +- 16 files changed, 221 insertions(+), 51 deletions(-) diff --git a/builtin/providers/terraform/resource_state.go b/builtin/providers/terraform/resource_state.go index fb0e85ee2..8f5855573 100644 --- a/builtin/providers/terraform/resource_state.go +++ b/builtin/providers/terraform/resource_state.go @@ -60,7 +60,7 @@ func resourceRemoteStateRead(d *schema.ResourceData, meta interface{}) error { return err } - var outputs map[string]string + var outputs map[string]interface{} if !state.State().Empty() { outputs = state.State().RootModule().Outputs } diff --git a/builtin/providers/tls/resource_cert_request_test.go b/builtin/providers/tls/resource_cert_request_test.go index 5ddad805c..2c2c4f5d4 100644 --- a/builtin/providers/tls/resource_cert_request_test.go +++ b/builtin/providers/tls/resource_cert_request_test.go @@ -50,7 +50,13 @@ EOT } `, testPrivateKey), Check: func(s *terraform.State) error { - got := s.RootModule().Outputs["key_pem"] + gotUntyped := s.RootModule().Outputs["key_pem"] + + got, ok := gotUntyped.(string) + if !ok { + return fmt.Errorf("output for \"key_pem\" is not a string") + } + if !strings.HasPrefix(got, "-----BEGIN CERTIFICATE REQUEST----") { return fmt.Errorf("key is missing CSR PEM preamble") } diff --git a/builtin/providers/tls/resource_locally_signed_cert_test.go b/builtin/providers/tls/resource_locally_signed_cert_test.go index 7e9688d12..aa705ece8 100644 --- a/builtin/providers/tls/resource_locally_signed_cert_test.go +++ b/builtin/providers/tls/resource_locally_signed_cert_test.go @@ -47,7 +47,11 @@ EOT } `, testCertRequest, testCACert, testCAPrivateKey), Check: func(s *terraform.State) error { - got := s.RootModule().Outputs["cert_pem"] + gotUntyped := s.RootModule().Outputs["cert_pem"] + got, ok := gotUntyped.(string) + if !ok { + return fmt.Errorf("output for \"cert_pem\" is not a string") + } if !strings.HasPrefix(got, "-----BEGIN CERTIFICATE----") { return fmt.Errorf("key is missing cert PEM preamble") } diff --git a/builtin/providers/tls/resource_private_key_test.go b/builtin/providers/tls/resource_private_key_test.go index 00fc8abbd..cec3a8198 100644 --- a/builtin/providers/tls/resource_private_key_test.go +++ b/builtin/providers/tls/resource_private_key_test.go @@ -29,7 +29,12 @@ func TestPrivateKeyRSA(t *testing.T) { } `, Check: func(s *terraform.State) error { - gotPrivate := s.RootModule().Outputs["private_key_pem"] + gotPrivateUntyped := s.RootModule().Outputs["private_key_pem"] + gotPrivate, ok := gotPrivateUntyped.(string) + if !ok { + return fmt.Errorf("output for \"private_key_pem\" is not a string") + } + if !strings.HasPrefix(gotPrivate, "-----BEGIN RSA PRIVATE KEY----") { return fmt.Errorf("private key is missing RSA key PEM preamble") } @@ -37,12 +42,20 @@ func TestPrivateKeyRSA(t *testing.T) { return fmt.Errorf("private key PEM looks too long for a 2048-bit key (got %v characters)", len(gotPrivate)) } - gotPublic := s.RootModule().Outputs["public_key_pem"] + gotPublicUntyped := s.RootModule().Outputs["public_key_pem"] + gotPublic, ok := gotPublicUntyped.(string) + if !ok { + return fmt.Errorf("output for \"public_key_pem\" is not a string") + } if !strings.HasPrefix(gotPublic, "-----BEGIN PUBLIC KEY----") { return fmt.Errorf("public key is missing public key PEM preamble") } - gotPublicSSH := s.RootModule().Outputs["public_key_openssh"] + gotPublicSSHUntyped := s.RootModule().Outputs["public_key_openssh"] + gotPublicSSH, ok := gotPublicSSHUntyped.(string) + if !ok { + return fmt.Errorf("output for \"public_key_openssh\" is not a string") + } if !strings.HasPrefix(gotPublicSSH, "ssh-rsa ") { return fmt.Errorf("SSH public key is missing ssh-rsa prefix") } @@ -61,7 +74,11 @@ func TestPrivateKeyRSA(t *testing.T) { } `, Check: func(s *terraform.State) error { - got := s.RootModule().Outputs["key_pem"] + gotUntyped := s.RootModule().Outputs["key_pem"] + got, ok := gotUntyped.(string) + if !ok { + return fmt.Errorf("output for \"key_pem\" is not a string") + } if !strings.HasPrefix(got, "-----BEGIN RSA PRIVATE KEY----") { return fmt.Errorf("key is missing RSA key PEM preamble") } @@ -95,12 +112,22 @@ func TestPrivateKeyECDSA(t *testing.T) { } `, Check: func(s *terraform.State) error { - gotPrivate := s.RootModule().Outputs["private_key_pem"] + gotPrivateUntyped := s.RootModule().Outputs["private_key_pem"] + gotPrivate, ok := gotPrivateUntyped.(string) + if !ok { + return fmt.Errorf("output for \"private_key_pem\" is not a string") + } + if !strings.HasPrefix(gotPrivate, "-----BEGIN EC PRIVATE KEY----") { return fmt.Errorf("Private key is missing EC key PEM preamble") } - gotPublic := s.RootModule().Outputs["public_key_pem"] + gotPublicUntyped := s.RootModule().Outputs["public_key_pem"] + gotPublic, ok := gotPublicUntyped.(string) + if !ok { + return fmt.Errorf("output for \"public_key_pem\" is not a string") + } + if !strings.HasPrefix(gotPublic, "-----BEGIN PUBLIC KEY----") { return fmt.Errorf("public key is missing public key PEM preamble") } @@ -130,17 +157,29 @@ func TestPrivateKeyECDSA(t *testing.T) { } `, Check: func(s *terraform.State) error { - gotPrivate := s.RootModule().Outputs["private_key_pem"] + gotPrivateUntyped := s.RootModule().Outputs["private_key_pem"] + gotPrivate, ok := gotPrivateUntyped.(string) + if !ok { + return fmt.Errorf("output for \"private_key_pem\" is not a string") + } if !strings.HasPrefix(gotPrivate, "-----BEGIN EC PRIVATE KEY----") { return fmt.Errorf("Private key is missing EC key PEM preamble") } - gotPublic := s.RootModule().Outputs["public_key_pem"] + gotPublicUntyped := s.RootModule().Outputs["public_key_pem"] + gotPublic, ok := gotPublicUntyped.(string) + if !ok { + return fmt.Errorf("output for \"public_key_pem\" is not a string") + } if !strings.HasPrefix(gotPublic, "-----BEGIN PUBLIC KEY----") { return fmt.Errorf("public key is missing public key PEM preamble") } - gotPublicSSH := s.RootModule().Outputs["public_key_openssh"] + gotPublicSSHUntyped := s.RootModule().Outputs["public_key_openssh"] + gotPublicSSH, ok := gotPublicSSHUntyped.(string) + if !ok { + return fmt.Errorf("output for \"public_key_openssh\" is not a string") + } if !strings.HasPrefix(gotPublicSSH, "ecdsa-sha2-nistp256 ") { return fmt.Errorf("P256 SSH public key is missing ecdsa prefix") } diff --git a/builtin/providers/tls/resource_self_signed_cert_test.go b/builtin/providers/tls/resource_self_signed_cert_test.go index 2ba3b2939..b403956f4 100644 --- a/builtin/providers/tls/resource_self_signed_cert_test.go +++ b/builtin/providers/tls/resource_self_signed_cert_test.go @@ -60,7 +60,12 @@ EOT } `, testPrivateKey), Check: func(s *terraform.State) error { - got := s.RootModule().Outputs["key_pem"] + gotUntyped := s.RootModule().Outputs["key_pem"] + got, ok := gotUntyped.(string) + if !ok { + return fmt.Errorf("output for \"public_key_openssh\" is not a string") + } + if !strings.HasPrefix(got, "-----BEGIN CERTIFICATE----") { return fmt.Errorf("key is missing cert PEM preamble") } diff --git a/command/output.go b/command/output.go index 7c2324b41..420dd438b 100644 --- a/command/output.go +++ b/command/output.go @@ -98,7 +98,13 @@ func (c *OutputCommand) Run(args []string) int { return 1 } - c.Ui.Output(v) + switch output := v.(type) { + case string: + c.Ui.Output(output) + default: + panic(fmt.Errorf("Unknown output type: %T", output)) + } + return 0 } diff --git a/command/output_test.go b/command/output_test.go index e8d469029..9c79f82ca 100644 --- a/command/output_test.go +++ b/command/output_test.go @@ -16,7 +16,7 @@ func TestOutput(t *testing.T) { Modules: []*terraform.ModuleState{ &terraform.ModuleState{ Path: []string{"root"}, - Outputs: map[string]string{ + Outputs: map[string]interface{}{ "foo": "bar", }, }, @@ -52,13 +52,13 @@ func TestModuleOutput(t *testing.T) { Modules: []*terraform.ModuleState{ &terraform.ModuleState{ Path: []string{"root"}, - Outputs: map[string]string{ + Outputs: map[string]interface{}{ "foo": "bar", }, }, &terraform.ModuleState{ Path: []string{"root", "my_module"}, - Outputs: map[string]string{ + Outputs: map[string]interface{}{ "blah": "tastatur", }, }, @@ -96,7 +96,7 @@ func TestMissingModuleOutput(t *testing.T) { Modules: []*terraform.ModuleState{ &terraform.ModuleState{ Path: []string{"root"}, - Outputs: map[string]string{ + Outputs: map[string]interface{}{ "foo": "bar", }, }, @@ -129,7 +129,7 @@ func TestOutput_badVar(t *testing.T) { Modules: []*terraform.ModuleState{ &terraform.ModuleState{ Path: []string{"root"}, - Outputs: map[string]string{ + Outputs: map[string]interface{}{ "foo": "bar", }, }, @@ -160,7 +160,7 @@ func TestOutput_blank(t *testing.T) { Modules: []*terraform.ModuleState{ &terraform.ModuleState{ Path: []string{"root"}, - Outputs: map[string]string{ + Outputs: map[string]interface{}{ "foo": "bar", "name": "john-doe", }, @@ -253,7 +253,7 @@ func TestOutput_noVars(t *testing.T) { Modules: []*terraform.ModuleState{ &terraform.ModuleState{ Path: []string{"root"}, - Outputs: map[string]string{}, + Outputs: map[string]interface{}{}, }, }, } @@ -282,7 +282,7 @@ func TestOutput_stateDefault(t *testing.T) { Modules: []*terraform.ModuleState{ &terraform.ModuleState{ Path: []string{"root"}, - Outputs: map[string]string{ + Outputs: map[string]interface{}{ "foo": "bar", }, }, diff --git a/state/remote/atlas_test.go b/state/remote/atlas_test.go index 847fb39cb..4deea7a3f 100644 --- a/state/remote/atlas_test.go +++ b/state/remote/atlas_test.go @@ -245,7 +245,7 @@ func (f *fakeAtlas) handler(resp http.ResponseWriter, req *http.Request) { // loads the state. var testStateModuleOrderChange = []byte( `{ - "version": 1, + "version": 2, "serial": 1, "modules": [ { @@ -276,7 +276,7 @@ var testStateModuleOrderChange = []byte( var testStateSimple = []byte( `{ - "version": 1, + "version": 2, "serial": 1, "modules": [ { diff --git a/state/testing.go b/state/testing.go index 6a4a88ad0..c5305ecef 100644 --- a/state/testing.go +++ b/state/testing.go @@ -36,7 +36,7 @@ func TestState(t *testing.T, s interface{}) { if ws, ok := s.(StateWriter); ok { current.Modules = append(current.Modules, &terraform.ModuleState{ Path: []string{"root"}, - Outputs: map[string]string{ + Outputs: map[string]interface{}{ "bar": "baz", }, }) @@ -94,7 +94,7 @@ func TestState(t *testing.T, s interface{}) { current.Modules = []*terraform.ModuleState{ &terraform.ModuleState{ Path: []string{"root", "somewhere"}, - Outputs: map[string]string{"serialCheck": "true"}, + Outputs: map[string]interface{}{"serialCheck": "true"}, }, } if err := writer.WriteState(current); err != nil { @@ -123,7 +123,7 @@ func TestStateInitial() *terraform.State { Modules: []*terraform.ModuleState{ &terraform.ModuleState{ Path: []string{"root", "child"}, - Outputs: map[string]string{ + Outputs: map[string]interface{}{ "foo": "bar", }, }, diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 7348dd0dd..3485b13bf 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -969,7 +969,7 @@ func TestContext2Apply_moduleDestroyOrder(t *testing.T) { }, }, }, - Outputs: map[string]string{ + Outputs: map[string]interface{}{ "a_output": "a", }, }, @@ -1438,7 +1438,7 @@ func TestContext2Apply_outputOrphan(t *testing.T) { Modules: []*ModuleState{ &ModuleState{ Path: rootModulePath, - Outputs: map[string]string{ + Outputs: map[string]interface{}{ "foo": "bar", "bar": "baz", }, diff --git a/terraform/context_refresh_test.go b/terraform/context_refresh_test.go index dbab70255..3d46c27c2 100644 --- a/terraform/context_refresh_test.go +++ b/terraform/context_refresh_test.go @@ -452,7 +452,7 @@ func TestContext2Refresh_output(t *testing.T) { }, }, - Outputs: map[string]string{ + Outputs: map[string]interface{}{ "foo": "foo", }, }, @@ -738,7 +738,7 @@ func TestContext2Refresh_orphanModule(t *testing.T) { }, }, }, - Outputs: map[string]string{ + Outputs: map[string]interface{}{ "id": "i-bcd234", "grandchild_id": "i-cde345", }, @@ -752,7 +752,7 @@ func TestContext2Refresh_orphanModule(t *testing.T) { }, }, }, - Outputs: map[string]string{ + Outputs: map[string]interface{}{ "id": "i-cde345", }, }, diff --git a/terraform/interpolate.go b/terraform/interpolate.go index 7ec549c93..2dec59adc 100644 --- a/terraform/interpolate.go +++ b/terraform/interpolate.go @@ -110,6 +110,25 @@ func (i *Interpolater) valueCountVar( } } +func interfaceToHILVariable(input interface{}) ast.Variable { + switch v := input.(type) { + case string: + return ast.Variable{ + Type: ast.TypeString, + Value: v, + } + default: + panic(fmt.Errorf("Unknown interface type %T in interfaceToHILVariable", v)) + } +} + +func unknownVariable() ast.Variable { + return ast.Variable{ + Type: ast.TypeString, + Value: config.UnknownVariableValue, + } +} + func (i *Interpolater) valueModuleVar( scope *InterpolationScope, n string, @@ -136,7 +155,6 @@ func (i *Interpolater) valueModuleVar( defer i.StateLock.RUnlock() // Get the module where we're looking for the value - var value string mod := i.State.ModuleByPath(path) if mod == nil { // If the module doesn't exist, then we can return an empty string. @@ -145,21 +163,18 @@ func (i *Interpolater) valueModuleVar( // modules reference other modules, and graph ordering should // ensure that the module is in the state, so if we reach this // point otherwise it really is a panic. - value = config.UnknownVariableValue + result[n] = unknownVariable() } else { // Get the value from the outputs - var ok bool - value, ok = mod.Outputs[v.Field] - if !ok { + if value, ok := mod.Outputs[v.Field]; ok { + result[n] = interfaceToHILVariable(value) + } else { // Same reasons as the comment above. - value = config.UnknownVariableValue + result[n] = unknownVariable() + } } - result[n] = ast.Variable{ - Value: value, - Type: ast.TypeString, - } return nil } diff --git a/terraform/interpolate_test.go b/terraform/interpolate_test.go index 31d066ba9..afdbd5f37 100644 --- a/terraform/interpolate_test.go +++ b/terraform/interpolate_test.go @@ -67,7 +67,7 @@ func TestInterpolater_moduleVariable(t *testing.T) { }, &ModuleState{ Path: []string{RootModuleName, "child"}, - Outputs: map[string]string{ + Outputs: map[string]interface{}{ "foo": "bar", }, }, diff --git a/terraform/state.go b/terraform/state.go index ace3b36f1..f7c7f9c5a 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -18,7 +18,7 @@ import ( const ( // StateVersion is the current version for our state file - StateVersion = 1 + StateVersion = 2 ) // rootModulePath is the path of the root module @@ -540,7 +540,7 @@ type ModuleState struct { // Outputs declared by the module and maintained for each module // even though only the root module technically needs to be kept. // This allows operators to inspect values at the boundaries. - Outputs map[string]string `json:"outputs"` + Outputs map[string]interface{} `json:"outputs"` // Resources is a mapping of the logically named resource to // the state of the resource. Each resource may actually have @@ -665,7 +665,7 @@ func (m *ModuleState) View(id string) *ModuleState { func (m *ModuleState) init() { if m.Outputs == nil { - m.Outputs = make(map[string]string) + m.Outputs = make(map[string]interface{}) } if m.Resources == nil { m.Resources = make(map[string]*ResourceState) @@ -678,7 +678,7 @@ func (m *ModuleState) deepcopy() *ModuleState { } n := &ModuleState{ Path: make([]string, len(m.Path)), - Outputs: make(map[string]string, len(m.Outputs)), + Outputs: make(map[string]interface{}, len(m.Outputs)), Resources: make(map[string]*ResourceState, len(m.Resources)), } copy(n.Path, m.Path) @@ -1338,7 +1338,8 @@ func ReadState(src io.Reader) (*State, error) { return upgradeV0State(old) } - // Otherwise, must be V2 + // Otherwise, must be V2 or V3 - V2 reads as V3 however so we need take + // no special action here - new state will be written as V3. dec := json.NewDecoder(buf) state := &State{} if err := dec.Decode(state); err != nil { @@ -1419,8 +1420,12 @@ func upgradeV0State(old *StateV0) (*State, error) { // directly into the root module. root := s.RootModule() - // Copy the outputs - root.Outputs = old.Outputs + // Copy the outputs, first converting them to map[string]interface{} + oldOutputs := make(map[string]interface{}, len(old.Outputs)) + for key, value := range old.Outputs { + oldOutputs[key] = value + } + root.Outputs = oldOutputs // Upgrade the resources for id, rs := range old.Resources { diff --git a/terraform/state_test.go b/terraform/state_test.go index 9d19cb234..a51b670b7 100644 --- a/terraform/state_test.go +++ b/terraform/state_test.go @@ -76,6 +76,38 @@ func TestStateAddModule(t *testing.T) { } } +func TestStateOutputTypeRoundTrip(t *testing.T) { + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: RootModulePath, + Outputs: map[string]interface{}{ + "string_output": "String Value", + "list_output": []interface{}{"List", "Value"}, + "map_output": map[string]interface{}{ + "key1": "Map", + "key2": "Value", + }, + }, + }, + }, + } + + buf := new(bytes.Buffer) + if err := WriteState(state, buf); err != nil { + t.Fatalf("err: %s", err) + } + + roundTripped, err := ReadState(buf) + if err != nil { + t.Fatalf("err: %s", err) + } + + if !reflect.DeepEqual(state, roundTripped) { + t.Fatalf("bad: %#v", roundTripped) + } +} + func TestStateModuleOrphans(t *testing.T) { state := &State{ Modules: []*ModuleState{ @@ -1162,6 +1194,33 @@ func TestInstanceState_MergeDiff_nilDiff(t *testing.T) { } } +func TestReadUpgradeStateV1toV2(t *testing.T) { + // ReadState should transparently detect the old version but will upgrade + // it on Write. + actual, err := ReadState(strings.NewReader(testV1State)) + if err != nil { + t.Fatalf("err: %s", err) + } + + buf := new(bytes.Buffer) + if err := WriteState(actual, buf); err != nil { + t.Fatalf("err: %s", err) + } + + if actual.Version != 2 { + t.Fatalf("bad: State version not incremented; is %d", actual.Version) + } + + roundTripped, err := ReadState(buf) + if err != nil { + t.Fatalf("err: %s", err) + } + + if !reflect.DeepEqual(actual, roundTripped) { + t.Fatalf("bad: %#v", actual) + } +} + func TestReadUpgradeState(t *testing.T) { state := &StateV0{ Resources: map[string]*ResourceStateV0{ @@ -1486,3 +1545,34 @@ func TestParseResourceStateKey(t *testing.T) { } } } + +const testV1State = `{ + "version": 1, + "serial": 9, + "remote": { + "type": "http", + "config": { + "url": "http://my-cool-server.com/" + } + }, + "modules": [ + { + "path": [ + "root" + ], + "outputs": null, + "resources": { + "foo": { + "type": "", + "primary": { + "id": "bar" + } + } + }, + "depends_on": [ + "aws_instance.bar" + ] + } + ] +} +` diff --git a/terraform/transform_output_test.go b/terraform/transform_output_test.go index dc9ea0a76..6ba2150dc 100644 --- a/terraform/transform_output_test.go +++ b/terraform/transform_output_test.go @@ -11,7 +11,7 @@ func TestAddOutputOrphanTransformer(t *testing.T) { Modules: []*ModuleState{ &ModuleState{ Path: RootModulePath, - Outputs: map[string]string{ + Outputs: map[string]interface{}{ "foo": "bar", "bar": "baz", },