addrs: Be explicit about checkable object address kinds

Previously we were attempting to infer the checkable object address kind
of a given address by whether it included "output" in the position where
a resource type name would otherwise go.

That was already potentially risky because we've historically not
prevented a resource type named "output", and it's also a
forward-compatibility hazard in case we introduce additional object kinds
with entirely-new addressing schemes in future.

Given that, we'll instead always be explicit about what kind of address
we're storing in a wire or file format, so that we can make sure to always
use the intended parser when reading an address back into memory, or
return an error if we encounter a kind we're not familiar with.

internal/addrs/check.go

@@ -20,7 +20,8 @@ import (
 //
 // Note also that the check address is only relevant within the scope of a run,
 // as reordering check blocks between runs will result in their addresses
-// changing.
+// changing. Check is therefore for internal use only and should not be exposed
+// in durable artifacts such as state snapshots.
 type Check struct {
 	Container Checkable
 	Type      CheckType
@@ -66,6 +67,41 @@ type checkKey struct {
 
 func (k checkKey) uniqueKeySigil() {}
 
+// CheckType describes a category of check. We use this only to establish
+// uniqueness for Check values, and do not expose this concept of "check types"
+// (which is subject to change in future) in any durable artifacts such as
+// state snapshots.
+//
+// (See [CheckableKind] for an enumeration that we _do_ use externally, to
+// describe the type of object being checked rather than the type of the check
+// itself.)
+type CheckType int
+
+//go:generate go run golang.org/x/tools/cmd/stringer -type=CheckType check.go
+
+const (
+	InvalidCondition      CheckType = 0
+	ResourcePrecondition  CheckType = 1
+	ResourcePostcondition CheckType = 2
+	OutputPrecondition    CheckType = 3
+)
+
+// Description returns a human-readable description of the check type. This is
+// presented in the user interface through a diagnostic summary.
+func (c CheckType) Description() string {
+	switch c {
+	case ResourcePrecondition:
+		return "Resource precondition"
+	case ResourcePostcondition:
+		return "Resource postcondition"
+	case OutputPrecondition:
+		return "Module output value precondition"
+	default:
+		// This should not happen
+		return "Condition"
+	}
+}
+
 // Checkable is an interface implemented by all address types that can contain
 // condition blocks.
 type Checkable interface {
@@ -88,6 +124,7 @@ type Checkable interface {
 	// purposes.
 	ConfigCheckable() ConfigCheckable
 
+	CheckableKind() CheckableKind
 	String() string
 }
 
@@ -96,34 +133,17 @@ var (
 	_ Checkable = AbsOutputValue{}
 )
 
-// CheckType describes the category of check.
-type CheckType int
+// CheckableKind describes the different kinds of checkable objects.
+type CheckableKind rune
 
-//go:generate go run golang.org/x/tools/cmd/stringer -type=CheckType check.go
+//go:generate go run golang.org/x/tools/cmd/stringer -type=CheckableKind check.go
 
 const (
-	InvalidCondition      CheckType = 0
-	ResourcePrecondition  CheckType = 1
-	ResourcePostcondition CheckType = 2
-	OutputPrecondition    CheckType = 3
+	CheckableKindInvalid CheckableKind = 0
+	CheckableResource    CheckableKind = 'R'
+	CheckableOutputValue CheckableKind = 'O'
 )
 
-// Description returns a human-readable description of the check type. This is
-// presented in the user interface through a diagnostic summary.
-func (c CheckType) Description() string {
-	switch c {
-	case ResourcePrecondition:
-		return "Resource precondition"
-	case ResourcePostcondition:
-		return "Resource postcondition"
-	case OutputPrecondition:
-		return "Module output value precondition"
-	default:
-		// This should not happen
-		return "Condition"
-	}
-}
-
 // ConfigCheckable is an interfaces implemented by address types that represent
 // configuration constructs that can have Checkable addresses associated with
 // them.
@@ -137,6 +157,7 @@ type ConfigCheckable interface {
 
 	configCheckableSigil()
 
+	CheckableKind() CheckableKind
 	String() string
 }
 
@@ -145,15 +166,17 @@ var (
 	_ ConfigCheckable = ConfigOutputValue{}
 )
 
-// ParseCheckableStr attempts to parse the given string as a Checkable address.
+// ParseCheckableStr attempts to parse the given string as a Checkable address
+// of the given kind.
 //
 // This should be the opposite of Checkable.String for any Checkable address
-// type.
+// type, as long as "kind" is set to the value returned by the address's
+// CheckableKind method.
 //
 // We do not typically expect users to write out checkable addresses as input,
 // but we use them as part of some of our wire formats for persisting check
 // results between runs.
-func ParseCheckableStr(src string) (Checkable, tfdiags.Diagnostics) {
+func ParseCheckableStr(kind CheckableKind, src string) (Checkable, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 
 	traversal, parseDiags := hclsyntax.ParseTraversalAbs([]byte(src), "", hcl.InitialPos)
@@ -178,8 +201,20 @@ func ParseCheckableStr(src string) (Checkable, tfdiags.Diagnostics) {
 		return nil, diags
 	}
 
-	switch remain.RootName() {
-	case "output":
+	// We use "kind" to disambiguate here because unfortunately we've
+	// historically never reserved "output" as a possible resource type name
+	// and so it is in principle possible -- albeit unlikely -- that there
+	// might be a resource whose type is literally "output".
+	switch kind {
+	case CheckableResource:
+		riAddr, moreDiags := parseResourceInstanceUnderModule(path, remain)
+		diags = diags.Append(moreDiags)
+		if diags.HasErrors() {
+			return nil, diags
+		}
+		return riAddr, diags
+
+	case CheckableOutputValue:
 		if len(remain) != 2 {
 			diags = diags.Append(&hcl.Diagnostic{
 				Severity: hcl.DiagError,
@@ -189,6 +224,15 @@ func ParseCheckableStr(src string) (Checkable, tfdiags.Diagnostics) {
 			})
 			return nil, diags
 		}
+		if remain.RootName() != "output" {
+			diags = diags.Append(&hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  "Invalid checkable address",
+				Detail:   "Output address must follow the module address with the keyword 'output'.",
+				Subject:  remain.SourceRange().Ptr(),
+			})
+			return nil, diags
+		}
 		if step, ok := remain[1].(hcl.TraverseAttr); !ok {
 			diags = diags.Append(&hcl.Diagnostic{
 				Severity: hcl.DiagError,
@@ -200,12 +244,8 @@ func ParseCheckableStr(src string) (Checkable, tfdiags.Diagnostics) {
 		} else {
 			return OutputValue{Name: step.Name}.Absolute(path), diags
 		}
+
 	default:
-		riAddr, moreDiags := parseResourceInstanceUnderModule(path, remain)
-		diags = diags.Append(moreDiags)
-		if diags.HasErrors() {
-			return nil, diags
-		}
-		return riAddr, diags
+		panic(fmt.Sprintf("unsupported CheckableKind %s", kind))
 	}
 }

internal/addrs/output_value.go

@@ -98,6 +98,10 @@ func (v AbsOutputValue) ConfigCheckable() ConfigCheckable {
 	return v.ConfigOutputValue()
 }
 
+func (v AbsOutputValue) CheckableKind() CheckableKind {
+	return CheckableOutputValue
+}
+
 func (v AbsOutputValue) UniqueKey() UniqueKey {
 	return absOutputValueUniqueKey(v.String())
 }
@@ -206,6 +210,10 @@ func (v ConfigOutputValue) configCheckableSigil() {
 	// ConfigOutputValue is the ConfigCheckable for AbsOutputValue.
 }
 
+func (v ConfigOutputValue) CheckableKind() CheckableKind {
+	return CheckableOutputValue
+}
+
 func (v ConfigOutputValue) UniqueKey() UniqueKey {
 	return configOutputValueUniqueKey(v.String())
 }

internal/addrs/resource.go

@@ -297,6 +297,10 @@ func (r AbsResourceInstance) Check(t CheckType, i int) Check {
 	}
 }
 
+func (v AbsResourceInstance) CheckableKind() CheckableKind {
+	return CheckableResource
+}
+
 func (r AbsResourceInstance) Equal(o AbsResourceInstance) bool {
 	return r.Module.Equal(o.Module) && r.Resource.Equal(o.Resource)
 }
@@ -421,6 +425,10 @@ func (r ConfigResource) configCheckableSigil() {
 	// ConfigResource represents a configuration object that declares checkable objects
 }
 
+func (v ConfigResource) CheckableKind() CheckableKind {
+	return CheckableResource
+}
+
 type configResourceKey string
 
 func (k configResourceKey) uniqueKeySigil() {}

internal/command/jsonchecks/objects.go

@@ -15,6 +15,11 @@ func makeStaticObjectAddr(addr addrs.ConfigCheckable) staticObjectAddr {
 
 	switch addr := addr.(type) {
 	case addrs.ConfigResource:
+		if kind := addr.CheckableKind(); kind != addrs.CheckableResource {
+			// Something has gone very wrong
+			panic(fmt.Sprintf("%T has CheckableKind %s", addr, kind))
+		}
+
 		ret["kind"] = "resource"
 		switch addr.Resource.Mode {
 		case addrs.ManagedResourceMode:
@@ -30,6 +35,11 @@ func makeStaticObjectAddr(addr addrs.ConfigCheckable) staticObjectAddr {
 			ret["module"] = addr.Module.String()
 		}
 	case addrs.ConfigOutputValue:
+		if kind := addr.CheckableKind(); kind != addrs.CheckableOutputValue {
+			// Something has gone very wrong
+			panic(fmt.Sprintf("%T has CheckableKind %s", addr, kind))
+		}
+
 		ret["kind"] = "output_value"
 		ret["name"] = addr.OutputValue.Name
 		if !addr.Module.IsRoot() {