Upgrade to v1.8.4 (#12)

.golangci.yml

@@ -41,6 +41,11 @@ issues:
       linters:
         - funlen
         - dupl
+        - revive
+    - path: (.+)_test.go
+      text: "ST1003"
+    - path: (.+)_test.go
+      text: "var-naming: don't use underscores in Go names"
 
 linters:
   disable-all: true

CHANGELOG.md

@@ -1,3 +1,26 @@
+## 1.8.5 (unreleased)
+
+## 1.8.4
+
+BUG FIXES:
+
+* `tofu init` will no longer return a supurious "Backend configuration changed" error when re-initializing a working directory with identical settings, backend configuration contains references to variables or local values, and when the `-backend-config` command line option is used. That combination previously caused OpenTofu to incorrectly treat the backend configuration as invalid. ([#2055](https://github.com/opentofu/opentofu/pull/2055))
+* configuration generation should no longer fail when generating sensitive properties
+* Provider defined functions are now supported better in child modules
+* Fixed an issue where X-Terraform-Get was not being read correctly if a custom module registry returns a 200 statuscode instead of 201
+
+## 1.8.3
+
+SECURITY:
+* Added option to enable the sensitive flag for variables used in module sources/versions and backend configurations.
+  * This emits a warning by default to prevent breaking compatability with previous 1.8.x versions.
+  * It is *highly recommended* to set `TOFU_ENABLE_STATIC_SENSITIVE=1` in any environments using this release.
+  * This will be enabled by default as a breaking change in v1.9.0
+
+BUG FIXES:
+* Fixed autoloaded test tfvar files being used in non-test scenarios ([#2039](https://github.com/opentofu/opentofu/pull/2039))
+* Fixed crash when using sensitive values in module sources/versions and backend configurations ([#2046](https://github.com/opentofu/opentofu/pull/2046))
+
 ## 1.8.2
 
 SECURITY:

internal/command/e2etest/testdata/encryption-flow/required.tf.disabled

@@ -1,8 +1,18 @@
+variable "passphrase" {
+  type = string
+  default = "26281afb-83f1-47ec-9b2d-2aebf6417167"
+  sensitive = true
+}
+
+locals {
+  key_length = sensitive(32)
+}
+
 terraform {
   encryption {
     key_provider "pbkdf2" "basic" {
-      passphrase    = "26281afb-83f1-47ec-9b2d-2aebf6417167"
-      key_length    = 32
+      passphrase    = var.passphrase
+      key_length    = local.key_length
       iterations    = 200000
       hash_function = "sha512"
       salt_length   = 12

internal/command/meta_backend.go

@@ -20,7 +20,6 @@ import (
 	"strings"
 
 	"github.com/terramate-io/hcl/v2"
-	"github.com/terramate-io/hcl/v2/hcldec"
 	"github.com/zclconf/go-cty/cty"
 	ctyjson "github.com/zclconf/go-cty/cty/json"
 
@@ -170,7 +169,7 @@ func (m *Meta) Backend(opts *BackendOpts, enc encryption.StateEncryption) (backe
 	// then return that as-is. This works even if b == nil (it will be !ok).
 	if enhanced, ok := b.(backend.Enhanced); ok {
 		log.Printf("[TRACE] Meta.Backend: backend %T supports operations", b)
-		return enhanced, nil
+		return enhanced, diags
 	}
 
 	// We either have a non-enhanced backend or no backend configured at
@@ -211,7 +210,7 @@ func (m *Meta) Backend(opts *BackendOpts, enc encryption.StateEncryption) (backe
 		}
 	}
 
-	return local, nil
+	return local, diags
 }
 
 // selectWorkspace gets a list of existing workspaces and then checks
@@ -1363,8 +1362,7 @@ func (m *Meta) backendConfigNeedsMigration(c *configs.Backend, s *legacy.Backend
 	b := f(nil) // We don't need encryption here as it's only used for config/schema
 
 	schema := b.ConfigSchema()
-	decSpec := schema.NoneRequired().DecoderSpec()
-	givenVal, diags := hcldec.Decode(c.Config, decSpec, nil)
+	givenVal, diags := c.Decode(schema)
 	if diags.HasErrors() {
 		log.Printf("[TRACE] backendConfigNeedsMigration: failed to decode given config; migration codepath must handle problem: %s", diags.Error())
 		return true // let the migration codepath deal with these errors

internal/command/meta_backend_test.go

@@ -15,6 +15,8 @@ import (
 	"testing"
 
 	"github.com/mitchellh/cli"
+	"github.com/terramate-io/hcl/v2"
+	"github.com/terramate-io/hcl/v2/hcltest"
 	"github.com/zclconf/go-cty/cty"
 
 	"github.com/terramate-io/opentofulib/internal/addrs"
@@ -655,6 +657,104 @@ func TestMetaBackend_configuredUnchanged(t *testing.T) {
 	}
 }
 
+// Saved backend state matching config when the configuration uses static eval references
+// and there's an argument overridden on the commandl ine.
+func TestMetaBackend_configuredUnchangedWithStaticEvalVars(t *testing.T) {
+	// This test is covering the fix for the following issue:
+	// https://github.com/opentofu/opentofu/issues/2024
+	//
+	// To match that issue's reproduction case the following must both be true:
+	// - The configuration written in the fixture's .tf file must include either a
+	//   reference to a named value or a function call. Currently we use a reference
+	//   to a variable.
+	// - There must be at least one -backend-config argument on the command line,
+	//   which causes us to go into the trickier comparison codepath that has to
+	//   re-evaluate _just_ the configuration to distinguish from the combined
+	//   configuration plus command-line overrides. Without this the configuration
+	//   doesn't get re-evaluated and so the expressions used to construct it are
+	//   irrelevant.
+	//
+	// Although not strictly required for reproduction at the time of writing this
+	// test, the local-state.tfstate file in the fixture also includes an output
+	// value to ensure that it can't be classified as an "empty state" and thus
+	// have migration skipped, even if the rules for activating that fast path
+	// change in future.
+
+	defer testChdir(t, testFixturePath("backend-unchanged-vars"))()
+
+	// Setup the meta
+	m := testMetaBackend(t, nil)
+	// testMetaBackend normally sets migrateState on, because most of the tests
+	// _want_ to perform migration, but for this one we're behaving as if the
+	// user hasn't set the -migrate-state option and thus it should be an error
+	// if state migration is required.
+	m.migrateState = false
+
+	// Get the backend
+	b, diags := m.Backend(
+		&BackendOpts{
+			Init: true,
+
+			// ConfigOverride is the internal representation of the -backend-config
+			// command line options. In the normal codepath this gets built into
+			// a synthetic hcl.Body so it can be merged with the real hcl.Body
+			// for evaluation. For testing purposes here we're constructing the
+			// synthetic body using the hcltest package instead, but the effect
+			// is the same.
+			ConfigOverride: hcltest.MockBody(&hcl.BodyContent{
+				Attributes: hcl.Attributes{
+					"workspace_dir": {
+						Name: "workspace_dir",
+						// We're using the "default" workspace in this test and so the workspace_dir
+						// isn't actually significant -- we're setting it only to enter the full-evaluation
+						// codepath. The only thing that matters is that the value here matches the
+						// argument value stored in the .terraform/terraform.tfstate file in the
+						// test fixture, meaning that state migration is not required because the
+						// configuration is unchanged.
+						Expr: hcltest.MockExprLiteral(cty.StringVal("doesnt-actually-matter-what-this-is")),
+					},
+				},
+			}),
+		},
+		encryption.StateEncryptionDisabled(),
+	)
+	if diags.HasErrors() {
+		// The original problem reported in https://github.com/opentofu/opentofu/issues/2024
+		// would return an error here: "Backend configuration has changed".
+		t.Fatal(diags.Err())
+	}
+
+	// The remaining checks are not directly related to the issue that this test
+	// is covering, but are included for completeness to check that this situation
+	// also follows the usual invariants for a failed backend init.
+
+	// Check the state
+	s, err := b.StateMgr(backend.DefaultStateName)
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+	if err := s.RefreshState(); err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+	state := s.State()
+	if state == nil {
+		t.Fatal("nil state")
+	}
+	if testStateMgrCurrentLineage(s) != "configuredUnchanged" {
+		t.Fatalf("bad: %#v", state)
+	}
+
+	// Verify the default paths don't exist
+	if _, err := os.Stat(DefaultStateFilename); err == nil {
+		t.Fatal("file should not exist")
+	}
+
+	// Verify a backup doesn't exist
+	if _, err := os.Stat(DefaultStateFilename + DefaultBackupExtension); err == nil {
+		t.Fatal("file should not exist")
+	}
+}
+
 // Changing a configured backend
 func TestMetaBackend_configuredChange(t *testing.T) {
 	// Create a temporary working directory that is empty

internal/command/meta_vars.go

@@ -25,6 +25,15 @@ import (
 // for root module input variables.
 const VarEnvPrefix = "TF_VAR_"
 
+// collectVariableValuesWithTests inspects the same sources of variables as
+// collectVariableValues, but also includes any autoloaded variables from the
+// given tests directory.
+func (m *Meta) collectVariableValuesWithTests(testDir string) (map[string]backend.UnparsedVariableValue, tfdiags.Diagnostics) {
+	values, diags := m.collectVariableValues()
+	diags = diags.Append(m.addVarsFromDir(testDir, values))
+	return values, diags
+}
+
 // collectVariableValues inspects the various places that root module input variable
 // values can come from and constructs a map ready to be passed to the
 // backend as part of a backend.Operation.
@@ -73,7 +82,6 @@ func (m *Meta) collectVariableValues() (map[string]backend.UnparsedVariableValue
 	// There's the original terraform.tfvars (DefaultVarsFilename) along with the later-added
 	// search for all files ending in .auto.tfvars.
 	diags = diags.Append(m.addVarsFromDir(".", ret))
-	diags = diags.Append(m.addVarsFromDir("tests", ret))
 
 	// Finally we process values given explicitly on the command line, either
 	// as individual literal settings or as additional files to read.

internal/command/test.go

@@ -125,7 +125,7 @@ func (c *TestCommand) Run(rawArgs []string) int {
 	}
 	c.variableArgs = rawFlags{items: &items}
 
-	variables, variableDiags := c.collectVariableValues()
+	variables, variableDiags := c.collectVariableValuesWithTests(args.TestDirectory)
 	diags = diags.Append(variableDiags)
 	if variableDiags.HasErrors() {
 		view.Diagnostics(nil, nil, diags)

internal/command/testdata/backend-unchanged-vars/.terraform/terraform.tfstate

@@ -0,0 +1,23 @@
+{
+    "version": 3,
+    "serial": 1,
+    "lineage": "666f9301-7e65-4b19-ae23-71184bb19b03",
+    "backend": {
+        "type": "local",
+        "config": {
+            "path": "local-state.tfstate",
+            "workspace_dir": "doesnt-actually-matter-what-this-is"
+        },
+        "hash": 4282859327
+    },
+    "modules": [
+        {
+            "path": [
+                "root"
+            ],
+            "outputs": {},
+            "resources": {},
+            "depends_on": []
+        }
+    ]
+}

internal/command/testdata/backend-unchanged-vars/local-state.tfstate

@@ -0,0 +1,12 @@
+{
+    "version": 4,
+    "terraform_version": "0.14.0",
+    "serial": 7,
+    "lineage": "configuredUnchanged",
+    "outputs": {
+        "foo": {
+            "type": "string",
+            "value": "This is only here so that the state snapshot isn't 'empty' and so can't enter a no-migration-needed fast path."
+        }
+    }
+}

internal/command/testdata/backend-unchanged-vars/main.tf

@@ -0,0 +1,10 @@
+variable "state_filename" {
+  type    = string
+  default = "local-state.tfstate"
+}
+
+terraform {
+  backend "local" {
+    path = var.state_filename
+  }
+}

internal/configs/backend.go

@@ -13,6 +13,7 @@ import (
 	"github.com/terramate-io/opentofulib/internal/addrs"
 	"github.com/terramate-io/opentofulib/internal/configs/configschema"
 	"github.com/terramate-io/opentofulib/lang"
+	"github.com/terramate-io/opentofulib/lang/marks"
 	"github.com/zclconf/go-cty/cty"
 )
 
@@ -62,6 +63,15 @@ func (b *Backend) Hash(schema *configschema.Block) (int, hcl.Diagnostics) {
 		val = cty.UnknownVal(schema.ImpliedType())
 	}
 
+	if marks.Contains(val, marks.Sensitive) {
+		return -1, diags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Backend config contains sensitive values",
+			Detail:   "The backend configuration is stored in .terraform/terraform.tfstate as well as plan files. It is recommended to instead supply sensitive credentials via backend specific environment variables",
+			Subject:  b.DeclRange.Ptr(),
+		})
+	}
+
 	toHash := cty.TupleVal([]cty.Value{
 		cty.StringVal(b.Type),
 		val,

internal/configs/static_evaluator.go

@@ -6,11 +6,14 @@
 package configs
 
 import (
+	"fmt"
+
 	"github.com/terramate-io/hcl/v2"
 	"github.com/terramate-io/hcl/v2/gohcl"
 	"github.com/terramate-io/hcl/v2/hcldec"
 	"github.com/terramate-io/opentofulib/internal/addrs"
 	"github.com/terramate-io/opentofulib/lang"
+	"github.com/terramate-io/opentofulib/lang/marks"
 	"github.com/zclconf/go-cty/cty"
 )
 
@@ -90,21 +93,21 @@ func (s StaticEvaluator) Evaluate(expr hcl.Expression, ident StaticIdentifier) (
 }
 
 func (s StaticEvaluator) DecodeExpression(expr hcl.Expression, ident StaticIdentifier, val any) hcl.Diagnostics {
-	var diags hcl.Diagnostics
-
-	refs, refsDiags := lang.ReferencesInExpr(addrs.ParseRef, expr)
-	diags = append(diags, refsDiags.ToHCL()...)
+	srcVal, diags := s.Evaluate(expr, ident)
 	if diags.HasErrors() {
 		return diags
 	}
 
-	ctx, ctxDiags := s.scope(ident).EvalContext(refs)
-	diags = append(diags, ctxDiags.ToHCL()...)
-	if diags.HasErrors() {
-		return diags
+	if marks.Contains(srcVal, marks.Sensitive) {
+		return diags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Sensitive value not allowed",
+			Detail:   fmt.Sprintf("Sensitive values, or values derived from sensitive values, cannot be used as %s.", ident.String()),
+			Subject:  expr.Range().Ptr(),
+		})
 	}
 
-	return gohcl.DecodeExpression(expr, ctx, val)
+	return diags.Extend(gohcl.DecodeValue(srcVal, expr.StartRange(), expr.Range(), val))
 }
 
 func (s StaticEvaluator) DecodeBlock(body hcl.Body, spec hcldec.Spec, ident StaticIdentifier) (cty.Value, hcl.Diagnostics) {