jsonformat: Tolerate incorrect paths in plan relevant_attributes

The code for matching relevant_attributes against resource_drift entries
(a part of the heuristic for deciding whether to show "changes outside of
OpenTofu" in the human-oriented plan UI) was previously assuming that paths
in resource_drift would always be valid for the associated resource
instance object values because in most cases the language runtime will
detect invalid references and so fail to generate a plan at all.

However, when the reference is to something within a dynamically-typed
argument (such as the manifest in kubernetes_manifest) and when it appears
only as an argument to either the "try" or "can" functions (so the dynamic
error is intentionally suppressed) the language runtime can't catch it
and so the incorrect reference will leak out into relevant_attributes,
thereby violating assumptions made by the path matcher.

Instead then, we'll continue the existing precedent that this "relevant
attributes" mechanism is a best-effort heuristic that prefers to succeed
with an incomplete result rather than to fail, extending that to the
traversals in the plan renderer which will now treat incorrectly-typed
steps as not matching rather than causing OpenTofu to crash completely.

Since a reference to something that doesn't exist cannot succeed it also
cannot possibly _actually_ contribute directly to the final result of the
expression it appeared in, so in practice it should be okay to disregard
these invalid references for the purposes of deciding which changes outside
of OpenTofu seem likely to have caused the actions that OpenTofu is
proposing to make during the apply phase.

Signed-off-by: Martin Atkins <mart@degeneration.co.uk>

internal/command/jsonformat/structured/attribute_path/matcher.go

@@ -7,7 +7,6 @@ package attribute_path
 
 import (
 	"encoding/json"
-	"fmt"
 	"strconv"
 )
 
@@ -153,7 +152,14 @@ func (p *PathMatcher) GetChildWithKey(key string) Matcher {
 			continue
 		}
 
-		if path[0].(string) == key {
+		// The next step should be a string that equals the given key.
+		// This must tolerate the next step being something other than a string
+		// because the paths we are matching against are not guaranteed to
+		// conform to the schema of the item they apply to. For example, the
+		// path might have been extracted by the lang/globalref reference
+		// analyzer from an argument to the "try" function and so would've been
+		// allowed to pass through without causing a validation error.
+		if gotKey, ok := path[0].(string); ok && gotKey == key {
 			child.Paths = append(child.Paths, path[1:])
 		}
 	}
@@ -185,10 +191,15 @@ func (p *PathMatcher) GetChildWithIndex(index int) Matcher {
 		//    - test_resource.resource.list[0].attribute
 		//    - test_resource.resource.list["0"].attribute
 		//
-		// Note, that OpenTofu will raise a validation error if the string
-		// can't be coerced into a number, so we will panic here if anything
-		// goes wrong safe in the knowledge the validation should stop this from
-		// happening.
+		// In most cases a string that can't be converted to a number will
+		// get caught by static checks in the language runtime, but that isn't
+		// true if test_resource.resource.list above is dynamically-typed,
+		// so we must tolerate and ignore strings that can't be converted.
+		// (In practice we'd get here in that case only if the invalid
+		// expression were used inside the "try" or "can" functions where a
+		// dynamic type check failure is suppressed, so it's okay to just
+		// silently ignore those here since that's the likely intention of
+		// using those functions.)
 
 		switch val := path[0].(type) {
 		case float64:
@@ -197,14 +208,9 @@ func (p *PathMatcher) GetChildWithIndex(index int) Matcher {
 			}
 		case string:
 			f, err := strconv.ParseFloat(val, 64)
-			if err != nil {
-				panic(fmt.Errorf("found invalid type within path (%v:%T), the validation shouldn't have allowed this to happen; this is a bug in OpenTofu, please report it", val, val))
-			}
-			if int(f) == index {
+			if err == nil && int(f) == index {
 				child.Paths = append(child.Paths, path[1:])
 			}
-		default:
-			panic(fmt.Errorf("found invalid type within path (%v:%T), the validation shouldn't have allowed this to happen; this is a bug in OpenTofu, please report it", val, val))
 		}
 	}
 	return child

internal/command/jsonformat/structured/attribute_path/matcher_test.go

@@ -11,7 +11,7 @@ func TestPathMatcher_FollowsPath(t *testing.T) {
 	var matcher Matcher
 
 	matcher = &PathMatcher{
-		Paths: [][]interface{}{
+		Paths: [][]any{
 			{
 				float64(0),
 				"key",
@@ -27,6 +27,10 @@ func TestPathMatcher_FollowsPath(t *testing.T) {
 		t.Errorf("should have partial matched at base level")
 	}
 
+	if matcher := matcher.GetChildWithKey("key"); matcher.MatchesPartial() {
+		t.Errorf("should not have matched string key in first step (it's actually an index step)")
+	}
+
 	matcher = matcher.GetChildWithIndex(0)
 
 	if matcher.Matches() {
@@ -54,11 +58,69 @@ func TestPathMatcher_FollowsPath(t *testing.T) {
 		t.Errorf("should have partial matched at leaf level")
 	}
 }
+
+func TestPathMatcher_HandlesListIndexWithString(t *testing.T) {
+	t.Run("convertible", func(t *testing.T) {
+		var matcher Matcher
+		matcher = &PathMatcher{
+			Paths: [][]any{
+				{
+					"0",
+					"key",
+				},
+			},
+		}
+
+		matcher = matcher.GetChildWithIndex(0)
+		if matcher.Matches() {
+			t.Errorf("should not have exact matched at first level")
+		}
+		if !matcher.MatchesPartial() {
+			t.Errorf("should have partial matched at first level")
+		}
+
+		matcher = matcher.GetChildWithKey("key")
+		if !matcher.Matches() {
+			t.Errorf("should have exact matched at second level")
+		}
+		if !matcher.MatchesPartial() {
+			t.Errorf("should have partial matched at second level")
+		}
+
+	})
+	t.Run("not convertible", func(t *testing.T) {
+		var matcher Matcher
+		matcher = &PathMatcher{
+			Paths: [][]any{
+				{
+					"not a number",
+					"key",
+				},
+			},
+		}
+
+		// The main thing we're testing here is actually that this
+		// succeeds without crashing, since we should treat the non-number
+		// step as a non-match. This is to allow for situations where there
+		// is an invalid reference to something that can only be
+		// dynamically-typechecked from in a "try" or "can" function call,
+		// which can cause invalid paths to appear in a matcher even though
+		// planning otherwise succeeded.
+		matcher = matcher.GetChildWithIndex(0)
+		if matcher.Matches() {
+			t.Errorf("should not have exact matched at first level")
+		}
+		if matcher.MatchesPartial() {
+			t.Errorf("should not have partial matched at first level")
+		}
+	})
+}
+
 func TestPathMatcher_Propagates(t *testing.T) {
 	var matcher Matcher
 
 	matcher = &PathMatcher{
-		Paths: [][]interface{}{
+		Paths: [][]any{
 			{
 				float64(0),
 				"key",
@@ -74,6 +136,10 @@ func TestPathMatcher_Propagates(t *testing.T) {
 		t.Errorf("should have partial matched at base level")
 	}
 
+	if matcher := matcher.GetChildWithKey("key"); matcher.MatchesPartial() {
+		t.Errorf("should not have matched string key in first step (it's actually an index step)")
+	}
+
 	matcher = matcher.GetChildWithIndex(0)
 
 	if matcher.Matches() {