CHANGELOG.md

@@ -1,3 +1,10 @@
+## 1.0.8 (September 29, 2021)
+
+BUG FIXES:
+
+* cli: Check `required_version` as early as possibly during `init` so that version incompatibility can be reported before errors about new syntax ([#29665](https://github.com/hashicorp/terraform/issues/29665))
+* core: Don't plan to remove orphaned resource instances in refresh-only plans ([#29640](https://github.com/hashicorp/terraform/issues/29640))
+
 ## 1.0.7 (September 15, 2021)
 
 BUG FIXES:

internal/command/init.go

@@ -150,19 +150,7 @@ func (c *InitCommand) Run(args []string) int {
 	// initialization functionality remains built around "earlyconfig" and
 	// so we need to still load the module via that mechanism anyway until we
 	// can do some more invasive refactoring here.
-	rootMod, confDiags := c.loadSingleModule(path)
 	rootModEarly, earlyConfDiags := c.loadSingleModuleEarly(path)
-	if confDiags.HasErrors() {
-		c.Ui.Error(c.Colorize().Color(strings.TrimSpace(errInitConfigError)))
-		// TODO: It would be nice to check the version constraints in
-		// rootModEarly.RequiredCore and print out a hint if the module is
-		// declaring that it's not compatible with this version of Terraform,
-		// though we're deferring that for now because we're intending to
-		// refactor our use of "earlyconfig" here anyway and so whatever we
-		// might do here right now would likely be invalidated by that.
-		c.showDiagnostics(confDiags)
-		return 1
-	}
 	// If _only_ the early loader encountered errors then that's unusual
 	// (it should generally be a superset of the normal loader) but we'll
 	// return those errors anyway since otherwise we'll probably get
@@ -172,7 +160,12 @@ func (c *InitCommand) Run(args []string) int {
 		c.Ui.Error(c.Colorize().Color(strings.TrimSpace(errInitConfigError)))
 		// Errors from the early loader are generally not as high-quality since
 		// it has less context to work with.
-		diags = diags.Append(confDiags)
+
+		// TODO: It would be nice to check the version constraints in
+		// rootModEarly.RequiredCore and print out a hint if the module is
+		// declaring that it's not compatible with this version of Terraform,
+		// and that may be what caused earlyconfig to fail.
+		diags = diags.Append(earlyConfDiags)
 		c.showDiagnostics(diags)
 		return 1
 	}
@@ -192,28 +185,31 @@ func (c *InitCommand) Run(args []string) int {
 	// With all of the modules (hopefully) installed, we can now try to load the
 	// whole configuration tree.
 	config, confDiags := c.loadConfig(path)
-	diags = diags.Append(confDiags)
-	if confDiags.HasErrors() {
-		c.Ui.Error(strings.TrimSpace(errInitConfigError))
-		c.showDiagnostics(diags)
-		return 1
-	}
+	// configDiags will be handled after the version constraint check, since an
+	// incorrect version of terraform may be producing errors for configuration
+	// constructs added in later versions.
 
-	// Before we go further, we'll check to make sure none of the modules in the
-	// configuration declare that they don't support this Terraform version, so
-	// we can produce a version-related error message rather than
+	// Before we go further, we'll check to make sure none of the modules in
+	// the configuration declare that they don't support this Terraform
+	// version, so we can produce a version-related error message rather than
 	// potentially-confusing downstream errors.
 	versionDiags := terraform.CheckCoreVersionRequirements(config)
-	diags = diags.Append(versionDiags)
 	if versionDiags.HasErrors() {
+		c.showDiagnostics(versionDiags)
+		return 1
+	}
+
+	diags = diags.Append(confDiags)
+	if confDiags.HasErrors() {
+		c.Ui.Error(strings.TrimSpace(errInitConfigError))
 		c.showDiagnostics(diags)
 		return 1
 	}
 
 	var back backend.Backend
 	if flagBackend {
 
-		be, backendOutput, backendDiags := c.initBackend(rootMod, flagConfigExtra)
+		be, backendOutput, backendDiags := c.initBackend(config.Module, flagConfigExtra)
 		diags = diags.Append(backendDiags)
 		if backendDiags.HasErrors() {
 			c.showDiagnostics(diags)

internal/command/init_test.go

@@ -1565,6 +1565,61 @@ func TestInit_checkRequiredVersion(t *testing.T) {
 	}
 }
 
+// Verify that init will error out with an invalid version constraint, even if
+// there are other invalid configuration constructs.
+func TestInit_checkRequiredVersionFirst(t *testing.T) {
+	t.Run("root_module", func(t *testing.T) {
+		td := tempDir(t)
+		testCopyDir(t, testFixturePath("init-check-required-version-first"), td)
+		defer os.RemoveAll(td)
+		defer testChdir(t, td)()
+
+		ui := cli.NewMockUi()
+		view, _ := testView(t)
+		c := &InitCommand{
+			Meta: Meta{
+				testingOverrides: metaOverridesForProvider(testProvider()),
+				Ui:               ui,
+				View:             view,
+			},
+		}
+
+		args := []string{}
+		if code := c.Run(args); code != 1 {
+			t.Fatalf("got exit status %d; want 1\nstderr:\n%s\n\nstdout:\n%s", code, ui.ErrorWriter.String(), ui.OutputWriter.String())
+		}
+		errStr := ui.ErrorWriter.String()
+		if !strings.Contains(errStr, `Unsupported Terraform Core version`) {
+			t.Fatalf("output should point to unmet version constraint, but is:\n\n%s", errStr)
+		}
+	})
+	t.Run("sub_module", func(t *testing.T) {
+		td := tempDir(t)
+		testCopyDir(t, testFixturePath("init-check-required-version-first-module"), td)
+		defer os.RemoveAll(td)
+		defer testChdir(t, td)()
+
+		ui := cli.NewMockUi()
+		view, _ := testView(t)
+		c := &InitCommand{
+			Meta: Meta{
+				testingOverrides: metaOverridesForProvider(testProvider()),
+				Ui:               ui,
+				View:             view,
+			},
+		}
+
+		args := []string{}
+		if code := c.Run(args); code != 1 {
+			t.Fatalf("got exit status %d; want 1\nstderr:\n%s\n\nstdout:\n%s", code, ui.ErrorWriter.String(), ui.OutputWriter.String())
+		}
+		errStr := ui.ErrorWriter.String()
+		if !strings.Contains(errStr, `Unsupported Terraform Core version`) {
+			t.Fatalf("output should point to unmet version constraint, but is:\n\n%s", errStr)
+		}
+	})
+}
+
 func TestInit_providerLockFile(t *testing.T) {
 	// Create a temporary working directory that is empty
 	td := tempDir(t)

internal/command/show_test.go

@@ -106,8 +106,6 @@ func TestShow_aliasedProvider(t *testing.T) {
 		},
 	}
 
-	fmt.Println(os.Getwd())
-
 	// the statefile created by testStateFile is named state.tfstate
 	args := []string{"state.tfstate"}
 	if code := c.Run(args); code != 0 {

internal/command/testdata/init-check-required-version-first-module/main.tf

@@ -0,0 +1,3 @@
+module "mod" {
+  source = "./mod"
+}

internal/command/testdata/init-check-required-version-first-module/mod/main.tf

@@ -0,0 +1,17 @@
+terraform {
+  required_version = ">200.0.0"
+
+  bad {
+    block = "false"
+  }
+
+  required_providers {
+    bang = {
+      oops = "boom"
+    }
+  }
+}
+
+nope {
+  boom {}
+}

internal/command/testdata/init-check-required-version-first/main.tf

@@ -0,0 +1,17 @@
+terraform {
+  required_version = ">200.0.0"
+
+  bad {
+    block = "false"
+  }
+
+  required_providers {
+    bang = {
+      oops = "boom"
+    }
+  }
+}
+
+nope {
+  boom {}
+}

internal/configs/configload/loader_load.go

@@ -21,7 +21,13 @@ import (
 func (l *Loader) LoadConfig(rootDir string) (*configs.Config, hcl.Diagnostics) {
 	rootMod, diags := l.parser.LoadConfigDir(rootDir)
 	if rootMod == nil || diags.HasErrors() {
-		return nil, diags
+		// Ensure we return any parsed modules here so that required_version
+		// constraints can be verified even when encountering errors.
+		cfg := &configs.Config{
+			Module: rootMod,
+		}
+
+		return cfg, diags
 	}
 
 	cfg, cDiags := configs.BuildConfig(rootMod, configs.ModuleWalkerFunc(l.moduleWalkerLoad))

internal/configs/configload/loader_load_test.go

@@ -91,8 +91,16 @@ func TestLoaderLoadConfig_loadDiags(t *testing.T) {
 		t.Fatalf("unexpected error from NewLoader: %s", err)
 	}
 
-	_, diags := loader.LoadConfig(fixtureDir)
+	cfg, diags := loader.LoadConfig(fixtureDir)
 	if !diags.HasErrors() {
-		t.Fatalf("success; want error")
+		t.Fatal("success; want error")
+	}
+
+	if cfg == nil {
+		t.Fatal("partial config not returned with diagnostics")
+	}
+
+	if cfg.Module == nil {
+		t.Fatal("expected config module")
 	}
 }

internal/terraform/context_plan2_test.go

@@ -998,6 +998,149 @@ func TestContext2Plan_refreshOnlyMode_deposed(t *testing.T) {
 	}
 }
 
+func TestContext2Plan_refreshOnlyMode_orphan(t *testing.T) {
+	addr := mustAbsResourceAddr("test_object.a")
+
+	// The configuration, the prior state, and the refresh result intentionally
+	// have different values for "test_string" so we can observe that the
+	// refresh took effect but the configuration change wasn't considered.
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+			resource "test_object" "a" {
+				arg = "after"
+				count = 1
+			}
+
+			output "out" {
+				value = test_object.a.*.arg
+			}
+		`,
+	})
+	state := states.BuildState(func(s *states.SyncState) {
+		s.SetResourceInstanceCurrent(addr.Instance(addrs.IntKey(0)), &states.ResourceInstanceObjectSrc{
+			AttrsJSON: []byte(`{"arg":"before"}`),
+			Status:    states.ObjectReady,
+		}, mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`))
+		s.SetResourceInstanceCurrent(addr.Instance(addrs.IntKey(1)), &states.ResourceInstanceObjectSrc{
+			AttrsJSON: []byte(`{"arg":"before"}`),
+			Status:    states.ObjectReady,
+		}, mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`))
+	})
+
+	p := simpleMockProvider()
+	p.GetProviderSchemaResponse = &providers.GetProviderSchemaResponse{
+		Provider: providers.Schema{Block: simpleTestSchema()},
+		ResourceTypes: map[string]providers.Schema{
+			"test_object": {
+				Block: &configschema.Block{
+					Attributes: map[string]*configschema.Attribute{
+						"arg": {Type: cty.String, Optional: true},
+					},
+				},
+			},
+		},
+	}
+	p.ReadResourceFn = func(req providers.ReadResourceRequest) providers.ReadResourceResponse {
+		newVal, err := cty.Transform(req.PriorState, func(path cty.Path, v cty.Value) (cty.Value, error) {
+			if len(path) == 1 && path[0] == (cty.GetAttrStep{Name: "arg"}) {
+				return cty.StringVal("current"), nil
+			}
+			return v, nil
+		})
+		if err != nil {
+			// shouldn't get here
+			t.Fatalf("ReadResourceFn transform failed")
+			return providers.ReadResourceResponse{}
+		}
+		return providers.ReadResourceResponse{
+			NewState: newVal,
+		}
+	}
+	p.UpgradeResourceStateFn = func(req providers.UpgradeResourceStateRequest) (resp providers.UpgradeResourceStateResponse) {
+		// We should've been given the prior state JSON as our input to upgrade.
+		if !bytes.Contains(req.RawStateJSON, []byte("before")) {
+			t.Fatalf("UpgradeResourceState request doesn't contain the 'before' object\n%s", req.RawStateJSON)
+		}
+
+		// We'll put something different in "arg" as part of upgrading, just
+		// so that we can verify below that PrevRunState contains the upgraded
+		// (but NOT refreshed) version of the object.
+		resp.UpgradedState = cty.ObjectVal(map[string]cty.Value{
+			"arg": cty.StringVal("upgraded"),
+		})
+		return resp
+	}
+
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		State:  state,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+		PlanMode: plans.RefreshOnlyMode,
+	})
+
+	plan, diags := ctx.Plan()
+	if diags.HasErrors() {
+		t.Fatalf("unexpected errors\n%s", diags.Err().Error())
+	}
+
+	if !p.UpgradeResourceStateCalled {
+		t.Errorf("Provider's UpgradeResourceState wasn't called; should've been")
+	}
+	if !p.ReadResourceCalled {
+		t.Errorf("Provider's ReadResource wasn't called; should've been")
+	}
+
+	if got, want := len(plan.Changes.Resources), 0; got != want {
+		t.Errorf("plan contains resource changes; want none\n%s", spew.Sdump(plan.Changes.Resources))
+	}
+
+	if rState := plan.PriorState.Resource(addr); rState == nil {
+		t.Errorf("%s has no prior state at all after plan", addr)
+	} else {
+		for i := 0; i < 2; i++ {
+			instKey := addrs.IntKey(i)
+			if obj := rState.Instance(instKey).Current; obj == nil {
+				t.Errorf("%s%s has no object after plan", addr, instKey)
+			} else if got, want := obj.AttrsJSON, `"current"`; !bytes.Contains(got, []byte(want)) {
+				// Should've saved the result of refreshing
+				t.Errorf("%s%s has wrong prior state after plan\ngot:\n%s\n\nwant substring: %s", addr, instKey, got, want)
+			}
+		}
+	}
+	if rState := plan.PrevRunState.Resource(addr); rState == nil {
+		t.Errorf("%s has no prior state at all after plan", addr)
+	} else {
+		for i := 0; i < 2; i++ {
+			instKey := addrs.IntKey(i)
+			if obj := rState.Instance(instKey).Current; obj == nil {
+				t.Errorf("%s%s has no object after plan", addr, instKey)
+			} else if got, want := obj.AttrsJSON, `"upgraded"`; !bytes.Contains(got, []byte(want)) {
+				// Should've saved the result of upgrading
+				t.Errorf("%s%s has wrong prior state after plan\ngot:\n%s\n\nwant substring: %s", addr, instKey, got, want)
+			}
+		}
+	}
+
+	// The output value should also have updated. If not, it's likely that we
+	// skipped updating the working state to match the refreshed state when we
+	// were evaluating the resource.
+	if outChangeSrc := plan.Changes.OutputValue(addrs.RootModuleInstance.OutputValue("out")); outChangeSrc == nil {
+		t.Errorf("no change planned for output value 'out'")
+	} else {
+		outChange, err := outChangeSrc.Decode()
+		if err != nil {
+			t.Fatalf("failed to decode output value 'out': %s", err)
+		}
+		got := outChange.After
+		want := cty.TupleVal([]cty.Value{cty.StringVal("current"), cty.StringVal("current")})
+		if !want.RawEquals(got) {
+			t.Errorf("wrong value for output value 'out'\ngot:  %#v\nwant: %#v", got, want)
+		}
+	}
+}
+
 func TestContext2Plan_invalidSensitiveModuleOutput(t *testing.T) {
 	m := testModuleInline(t, map[string]string{
 		"child/main.tf": `

internal/terraform/node_resource_plan.go

@@ -132,6 +132,8 @@ func (n *nodeExpandPlannableResource) DynamicExpand(ctx EvalContext) (*Graph, er
 
 		return &NodePlannableResourceInstanceOrphan{
 			NodeAbstractResourceInstance: a,
+			skipRefresh:                  n.skipRefresh,
+			skipPlanChanges:              n.skipPlanChanges,
 		}
 	}
 
@@ -355,6 +357,7 @@ func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) {
 		return &NodePlannableResourceInstanceOrphan{
 			NodeAbstractResourceInstance: a,
 			skipRefresh:                  n.skipRefresh,
+			skipPlanChanges:              n.skipPlanChanges,
 		}
 	}