Fix bug in import 'to' parsing in json configurations (#1665)

Signed-off-by: Ronny Orot <ronny.orot@gmail.com>

internal/configs/hcl2shim/util.go

@@ -66,3 +66,29 @@ func schemaWithDynamic(schema *hcl.BodySchema) *hcl.BodySchema {
 
 	return ret
 }
+
+// ConvertJSONExpressionToHCL is used to convert HCL *json.expression into
+// regular hcl syntax.
+// Sometimes, we manually parse an expression instead of using the hcl library
+// for parsing. In this case we need to handle json configs specially, as the
+// values will be json strings rather than hcl.
+func ConvertJSONExpressionToHCL(expr hcl.Expression) (hcl.Expression, hcl.Diagnostics) {
+	var diags hcl.Diagnostics
+	// We can abuse the hcl json api and rely on the fact that calling
+	// Value on a json expression with no EvalContext will return the
+	// raw string. We can then parse that as normal hcl syntax, and
+	// continue with the decoding.
+	value, ds := expr.Value(nil)
+	diags = append(diags, ds...)
+	if diags.HasErrors() {
+		return nil, diags
+	}
+
+	expr, ds = hclsyntax.ParseExpression([]byte(value.AsString()), expr.Range().Filename, expr.Range().Start)
+	diags = append(diags, ds...)
+	if diags.HasErrors() {
+		return nil, diags
+	}
+
+	return expr, diags
+}

internal/configs/hcl2shim/util_test.go

@@ -0,0 +1,55 @@
+package hcl2shim
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/hashicorp/hcl/v2"
+	"github.com/hashicorp/hcl/v2/hclsyntax"
+	hclJSON "github.com/hashicorp/hcl/v2/json"
+)
+
+func TestConvertJSONExpressionToHCL(t *testing.T) {
+	tests := []struct {
+		Input string
+	}{
+		{
+			Input: "hello",
+		},
+		{
+			Input: "resource.test_resource[0]",
+		},
+	}
+
+	for _, test := range tests {
+		JSONExpr, diags := hclJSON.ParseExpression([]byte(`"`+test.Input+`"`), "")
+		if diags.HasErrors() {
+			t.Errorf("got %d diagnostics; want 0", len(diags))
+			for _, d := range diags {
+				t.Logf("  - %s", d.Error())
+			}
+		}
+
+		want, diags := hclsyntax.ParseExpression([]byte(test.Input), "", hcl.Pos{Line: 1, Column: 1})
+		if diags.HasErrors() {
+			t.Errorf("got %d diagnostics; want 0", len(diags))
+			for _, d := range diags {
+				t.Logf("  - %s", d.Error())
+			}
+		}
+
+		t.Run(test.Input, func(t *testing.T) {
+			resultExpr, diags := ConvertJSONExpressionToHCL(JSONExpr)
+			if diags.HasErrors() {
+				t.Errorf("got %d diagnostics; want 0", len(diags))
+				for _, d := range diags {
+					t.Logf("  - %s", d.Error())
+				}
+			}
+
+			if !reflect.DeepEqual(resultExpr, want) {
+				t.Errorf("got %s, but want %s", resultExpr, want)
+			}
+		})
+	}
+}

internal/configs/import.go

@@ -8,7 +8,9 @@ package configs
 import (
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/hclsyntax"
+	hcljson "github.com/hashicorp/hcl/v2/json"
 	"github.com/opentofu/opentofu/internal/addrs"
+	"github.com/opentofu/opentofu/internal/configs/hcl2shim"
 	"github.com/opentofu/opentofu/internal/tfdiags"
 )
 
@@ -66,8 +68,25 @@ func decodeImportBlock(block *hcl.Block) (*Import, hcl.Diagnostics) {
 	}
 
 	if attr, exists := content.Attributes["to"]; exists {
-		imp.To = attr.Expr
-		staticAddress, addressDiags := staticImportAddress(attr.Expr)
+		toExpr := attr.Expr
+		// Since we are manually parsing the 'to' argument, we need to specially
+		// handle json configs, in which case the values will be json strings
+		// rather than hcl
+		isJSON := hcljson.IsJSONExpression(attr.Expr)
+
+		if isJSON {
+			convertedExpr, convertDiags := hcl2shim.ConvertJSONExpressionToHCL(toExpr)
+			diags = append(diags, convertDiags...)
+
+			if diags.HasErrors() {
+				return imp, diags
+			}
+
+			toExpr = convertedExpr
+		}
+
+		imp.To = toExpr
+		staticAddress, addressDiags := staticImportAddress(toExpr)
 		diags = append(diags, addressDiags.ToHCL()...)
 
 		// Exit early if there are issues resolving the static address part. We wouldn't be able to validate the provider in such a case

internal/configs/resource.go

@@ -12,8 +12,8 @@ import (
 	"github.com/hashicorp/hcl/v2/gohcl"
 	"github.com/hashicorp/hcl/v2/hclsyntax"
 	hcljson "github.com/hashicorp/hcl/v2/json"
-
 	"github.com/opentofu/opentofu/internal/addrs"
+	"github.com/opentofu/opentofu/internal/configs/hcl2shim"
 	"github.com/opentofu/opentofu/internal/lang"
 	"github.com/opentofu/opentofu/internal/tfdiags"
 )
@@ -551,18 +551,9 @@ func decodeReplaceTriggeredBy(expr hcl.Expression) ([]hcl.Expression, hcl.Diagno
 
 	for i, expr := range exprs {
 		if isJSON {
-			// We can abuse the hcl json api and rely on the fact that calling
-			// Value on a json expression with no EvalContext will return the
-			// raw string. We can then parse that as normal hcl syntax, and
-			// continue with the decoding.
-			v, ds := expr.Value(nil)
-			diags = diags.Extend(ds)
-			if diags.HasErrors() {
-				continue
-			}
-
-			expr, ds = hclsyntax.ParseExpression([]byte(v.AsString()), "", expr.Range().Start)
-			diags = diags.Extend(ds)
+			var convertDiags hcl.Diagnostics
+			expr, convertDiags = hcl2shim.ConvertJSONExpressionToHCL(expr)
+			diags = diags.Extend(convertDiags)
 			if diags.HasErrors() {
 				continue
 			}

internal/tofu/context_plan2_test.go

@@ -3583,8 +3583,15 @@ output "a" {
 }
 
 func TestContext2Plan_triggeredBy(t *testing.T) {
-	m := testModuleInline(t, map[string]string{
-		"main.tf": `
+	type TestConfiguration struct {
+		Description         string
+		inlineConfiguration map[string]string
+	}
+	configurations := []TestConfiguration{
+		{
+			Description: "TF configuration",
+			inlineConfiguration: map[string]string{
+				"main.tf": `
 resource "test_object" "a" {
   count = 1
   test_string = "new"
@@ -3598,7 +3605,41 @@ resource "test_object" "b" {
   }
 }
 `,
-	})
+			},
+		},
+		{
+			Description: "Json configuration",
+			inlineConfiguration: map[string]string{
+				"main.tf.json": `
+{
+    "resource": {
+        "test_object": {
+            "a": [
+                {
+                    "count": 1,
+                    "test_string": "new"
+                }
+            ],
+            "b": [
+                {
+                    "count": 1,
+                    "lifecycle": [
+                        {
+                            "replace_triggered_by": [
+                                "test_object.a[count.index].test_string"
+                            ]
+                        }
+                    ],
+                    "test_string": "test_object.a[count.index].test_string"
+                }
+            ]
+        }
+    }
+}
+`,
+			},
+		},
+	}
 
 	p := simpleMockProvider()
 
@@ -3626,28 +3667,31 @@ resource "test_object" "b" {
 			mustProviderConfig(`provider["registry.opentofu.org/hashicorp/test"]`),
 		)
 	})
+	for _, configuration := range configurations {
+		m := testModuleInline(t, configuration.inlineConfiguration)
 
-	plan, diags := ctx.Plan(m, state, &PlanOpts{
-		Mode: plans.NormalMode,
-	})
-	if diags.HasErrors() {
-		t.Fatalf("unexpected errors\n%s", diags.Err().Error())
-	}
-	for _, c := range plan.Changes.Resources {
-		switch c.Addr.String() {
-		case "test_object.a[0]":
-			if c.Action != plans.Update {
-				t.Fatalf("unexpected %s change for %s\n", c.Action, c.Addr)
+		plan, diags := ctx.Plan(m, state, &PlanOpts{
+			Mode: plans.NormalMode,
+		})
+		if diags.HasErrors() {
+			t.Fatalf("unexpected errors\n%s", diags.Err().Error())
+		}
+		for _, c := range plan.Changes.Resources {
+			switch c.Addr.String() {
+			case "test_object.a[0]":
+				if c.Action != plans.Update {
+					t.Fatalf("unexpected %s change for %s\n", c.Action, c.Addr)
+				}
+			case "test_object.b[0]":
+				if c.Action != plans.DeleteThenCreate {
+					t.Fatalf("unexpected %s change for %s\n", c.Action, c.Addr)
+				}
+				if c.ActionReason != plans.ResourceInstanceReplaceByTriggers {
+					t.Fatalf("incorrect reason for change: %s\n", c.ActionReason)
+				}
+			default:
+				t.Fatal("unexpected change", c.Addr, c.Action)
 			}
-		case "test_object.b[0]":
-			if c.Action != plans.DeleteThenCreate {
-				t.Fatalf("unexpected %s change for %s\n", c.Action, c.Addr)
-			}
-			if c.ActionReason != plans.ResourceInstanceReplaceByTriggers {
-				t.Fatalf("incorrect reason for change: %s\n", c.ActionReason)
-			}
-		default:
-			t.Fatal("unexpected change", c.Addr, c.Action)
 		}
 	}
 }
@@ -4207,8 +4251,16 @@ resource "test_object" "a" {
 
 func TestContext2Plan_importResourceBasic(t *testing.T) {
 	addr := mustResourceInstanceAddr("test_object.a")
-	m := testModuleInline(t, map[string]string{
-		"main.tf": `
+
+	type TestConfiguration struct {
+		Description         string
+		inlineConfiguration map[string]string
+	}
+	configurations := []TestConfiguration{
+		{
+			Description: "TF configuration",
+			inlineConfiguration: map[string]string{
+				"main.tf": `
 resource "test_object" "a" {
   test_string = "foo"
 }
@@ -4218,7 +4270,33 @@ import {
   id   = "123"
 }
 `,
-	})
+			},
+		},
+		{
+			Description: "Json configuration",
+			inlineConfiguration: map[string]string{
+				"main.tf.json": `
+{
+    "import": [
+        {
+            "id": "123",
+            "to": "test_object.a"
+        }
+    ],
+    "resource": {
+        "test_object": {
+            "a": [
+		        {
+		            "test_string": "foo"
+		        }
+            ]
+        }
+    }
+}
+`,
+			},
+		},
+	}
 
 	p := simpleMockProvider()
 	hook := new(MockHook)
@@ -4244,47 +4322,51 @@ import {
 		},
 	}
 
-	plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts)
-	if diags.HasErrors() {
-		t.Fatalf("unexpected errors\n%s", diags.Err().Error())
+	for _, configuration := range configurations {
+		m := testModuleInline(t, configuration.inlineConfiguration)
+
+		plan, diags := ctx.Plan(m, states.NewState(), DefaultPlanOpts)
+		if diags.HasErrors() {
+			t.Fatalf("unexpected errors\n%s", diags.Err().Error())
+		}
+
+		t.Run(configuration.Description, func(t *testing.T) {
+			instPlan := plan.Changes.ResourceInstance(addr)
+			if instPlan == nil {
+				t.Fatalf("no plan for %s at all", addr)
+			}
+
+			if got, want := instPlan.Addr, addr; !got.Equal(want) {
+				t.Errorf("wrong current address\ngot:  %s\nwant: %s", got, want)
+			}
+			if got, want := instPlan.PrevRunAddr, addr; !got.Equal(want) {
+				t.Errorf("wrong previous run address\ngot:  %s\nwant: %s", got, want)
+			}
+			if got, want := instPlan.Action, plans.NoOp; got != want {
+				t.Errorf("wrong planned action\ngot:  %s\nwant: %s", got, want)
+			}
+			if got, want := instPlan.ActionReason, plans.ResourceInstanceChangeNoReason; got != want {
+				t.Errorf("wrong action reason\ngot:  %s\nwant: %s", got, want)
+			}
+			if instPlan.Importing.ID != "123" {
+				t.Errorf("expected import change from \"123\", got non-import change")
+			}
+
+			if !hook.PrePlanImportCalled {
+				t.Fatalf("PostPlanImport hook not called")
+			}
+			if addr, wantAddr := hook.PrePlanImportAddr, instPlan.Addr; !addr.Equal(wantAddr) {
+				t.Errorf("expected addr to be %s, but was %s", wantAddr, addr)
+			}
+
+			if !hook.PostPlanImportCalled {
+				t.Fatalf("PostPlanImport hook not called")
+			}
+			if addr, wantAddr := hook.PostPlanImportAddr, instPlan.Addr; !addr.Equal(wantAddr) {
+				t.Errorf("expected addr to be %s, but was %s", wantAddr, addr)
+			}
+		})
 	}
-
-	t.Run(addr.String(), func(t *testing.T) {
-		instPlan := plan.Changes.ResourceInstance(addr)
-		if instPlan == nil {
-			t.Fatalf("no plan for %s at all", addr)
-		}
-
-		if got, want := instPlan.Addr, addr; !got.Equal(want) {
-			t.Errorf("wrong current address\ngot:  %s\nwant: %s", got, want)
-		}
-		if got, want := instPlan.PrevRunAddr, addr; !got.Equal(want) {
-			t.Errorf("wrong previous run address\ngot:  %s\nwant: %s", got, want)
-		}
-		if got, want := instPlan.Action, plans.NoOp; got != want {
-			t.Errorf("wrong planned action\ngot:  %s\nwant: %s", got, want)
-		}
-		if got, want := instPlan.ActionReason, plans.ResourceInstanceChangeNoReason; got != want {
-			t.Errorf("wrong action reason\ngot:  %s\nwant: %s", got, want)
-		}
-		if instPlan.Importing.ID != "123" {
-			t.Errorf("expected import change from \"123\", got non-import change")
-		}
-
-		if !hook.PrePlanImportCalled {
-			t.Fatalf("PostPlanImport hook not called")
-		}
-		if addr, wantAddr := hook.PrePlanImportAddr, instPlan.Addr; !addr.Equal(wantAddr) {
-			t.Errorf("expected addr to be %s, but was %s", wantAddr, addr)
-		}
-
-		if !hook.PostPlanImportCalled {
-			t.Fatalf("PostPlanImport hook not called")
-		}
-		if addr, wantAddr := hook.PostPlanImportAddr, instPlan.Addr; !addr.Equal(wantAddr) {
-			t.Errorf("expected addr to be %s, but was %s", wantAddr, addr)
-		}
-	})
 }
 
 func TestContext2Plan_importToDynamicAddress(t *testing.T) {
@@ -4295,7 +4377,7 @@ func TestContext2Plan_importToDynamicAddress(t *testing.T) {
 	}
 	configurations := []TestConfiguration{
 		{
-			Description:     "To address includes a variable as index",
+			Description:     "To address includes a variable as index in TF configuration",
 			ResolvedAddress: "test_object.a[0]",
 			inlineConfiguration: map[string]string{
 				"main.tf": `
@@ -4312,6 +4394,37 @@ import {
   to   = test_object.a[var.index]
   id   = "%d"
 }
+`,
+			},
+		},
+		{
+			Description:     "To address includes a variable as index in JSON configuration",
+			ResolvedAddress: "test_object.a[0]",
+			inlineConfiguration: map[string]string{
+				"main.tf.json": `
+{
+     "locals": [
+        {
+            "index": 0
+        }
+    ],
+    "import": [
+        {
+            "id": "%d",
+            "to": "test_object.a[local.index]"
+        }
+    ],
+    "resource": {
+        "test_object": {
+            "a": [
+		        {
+                    "count": 1,
+		            "test_string": "foo"
+		        }
+            ]
+        }
+    }
+}
 `,
 			},
 		},