From 456f28dcb43cc721d1d1b71db1429e3d925d73ce Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 19 Dec 2018 12:12:24 -0800 Subject: [PATCH 1/2] command: Always normalize config path before operations Previously we were doing this rather inconsistently: some commands would do it and others would not. By doing it here we ensure we always apply the same normalization, regardless of which operation we're running. This normalization is mostly for cosmetic purposes in error messages, but it also ends up being used to populate path.module and path.root and so it's important that we always produce consistent results here so that we don't produce flappy changes as users work with different commands. The fact that thus mutates a data structure as a side-effect is not ideal but this is the best place to ensure it always gets applied without doing any significant refactoring, since everything after this point happens in the backend package where the normalizePath method is not available. --- command/meta.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/command/meta.go b/command/meta.go index 53c59831bc94..7658520e6287 100644 --- a/command/meta.go +++ b/command/meta.go @@ -266,6 +266,10 @@ func (m *Meta) StdinPiped() bool { // operation itself is unsuccessful. Use the "Result" field of the // returned operation object to recognize operation-level failure. func (m *Meta) RunOperation(b backend.Enhanced, opReq *backend.Operation) (*backend.RunningOperation, error) { + if opReq.ConfigDir != "" { + opReq.ConfigDir = m.normalizePath(opReq.ConfigDir) + } + op, err := b.Operation(context.Background(), opReq) if err != nil { return nil, fmt.Errorf("error starting operation: %s", err) From 72f3a0f62e853c85d65088761559dc546238baab Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 19 Dec 2018 12:29:29 -0800 Subject: [PATCH 2/2] core: path.module, path.root, path.cwd use fwd slashes on all platforms Previously we used the native slash type for the host platform, but that leads to issues if the same configuration is applied on both Windows and non-Windows systems. Since Windows supports slashes and backslashes, we can safely return always slashes here and require that users combine the result with subsequent path parts using slashes, like: "${path.module}/foo/bar" Previously the above would lead to an error on Windows if path.module contained any backslashes. This is not really possible to unit test directly right now since we always run our tests on Unix systems and filepath.ToSlash is a no-op on Unix. However, this does include some tests for the basic behavior to verify that it's not regressed as a result of this change. This will need to be reported in the changelog as a potential breaking change, since anyone who was using Terraform _exclusively_ on Windows may have been using expressions like "${path.module}foo\\bar" which they will now need to update. This fixes #14986. --- terraform/evaluate.go | 7 +++--- terraform/evaluate_test.go | 44 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/terraform/evaluate.go b/terraform/evaluate.go index 8e503c3d13aa..38fe8767346d 100644 --- a/terraform/evaluate.go +++ b/terraform/evaluate.go @@ -4,6 +4,7 @@ import ( "fmt" "log" "os" + "path/filepath" "strconv" "sync" @@ -423,7 +424,7 @@ func (d *evaluationStateData) GetPathAttr(addr addrs.PathAttr, rng tfdiags.Sourc }) return cty.DynamicVal, diags } - return cty.StringVal(wd), diags + return cty.StringVal(filepath.ToSlash(wd)), diags case "module": moduleConfig := d.Evaluator.Config.DescendentForInstance(d.ModulePath) @@ -433,11 +434,11 @@ func (d *evaluationStateData) GetPathAttr(addr addrs.PathAttr, rng tfdiags.Sourc panic(fmt.Sprintf("module.path read from module %s, which has no configuration", d.ModulePath)) } sourceDir := moduleConfig.Module.SourceDir - return cty.StringVal(sourceDir), diags + return cty.StringVal(filepath.ToSlash(sourceDir)), diags case "root": sourceDir := d.Evaluator.Config.Module.SourceDir - return cty.StringVal(sourceDir), diags + return cty.StringVal(filepath.ToSlash(sourceDir)), diags default: suggestion := nameSuggestion(addr.Name, []string{"cwd", "module", "root"}) diff --git a/terraform/evaluate_test.go b/terraform/evaluate_test.go index d3f27ec4da03..18b2d65f7d03 100644 --- a/terraform/evaluate_test.go +++ b/terraform/evaluate_test.go @@ -7,6 +7,7 @@ import ( "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/tfdiags" ) @@ -34,3 +35,46 @@ func TestEvaluatorGetTerraformAttr(t *testing.T) { } }) } + +func TestEvaluatorGetPathAttr(t *testing.T) { + evaluator := &Evaluator{ + Meta: &ContextMeta{ + Env: "foo", + }, + Config: &configs.Config{ + Module: &configs.Module{ + SourceDir: "bar/baz", + }, + }, + } + data := &evaluationStateData{ + Evaluator: evaluator, + } + scope := evaluator.Scope(data, nil) + + t.Run("module", func(t *testing.T) { + want := cty.StringVal("bar/baz") + got, diags := scope.Data.GetPathAttr(addrs.PathAttr{ + Name: "module", + }, tfdiags.SourceRange{}) + if len(diags) != 0 { + t.Errorf("unexpected diagnostics %s", spew.Sdump(diags)) + } + if !got.RawEquals(want) { + t.Errorf("wrong result %#v; want %#v", got, want) + } + }) + + t.Run("root", func(t *testing.T) { + want := cty.StringVal("bar/baz") + got, diags := scope.Data.GetPathAttr(addrs.PathAttr{ + Name: "root", + }, tfdiags.SourceRange{}) + if len(diags) != 0 { + t.Errorf("unexpected diagnostics %s", spew.Sdump(diags)) + } + if !got.RawEquals(want) { + t.Errorf("wrong result %#v; want %#v", got, want) + } + }) +}