structured run output: impose canonical ordering on jsonstate and jsonplan packages (#32649)

internal/addrs/resource.go

@@ -32,6 +32,22 @@ func (r Resource) Equal(o Resource) bool {
 	return r.Mode == o.Mode && r.Name == o.Name && r.Type == o.Type
 }
 
+func (r Resource) Less(o Resource) bool {
+	switch {
+	case r.Mode != o.Mode:
+		return r.Mode == DataResourceMode
+
+	case r.Type != o.Type:
+		return r.Type < o.Type
+
+	case r.Name != o.Name:
+		return r.Name < o.Name
+
+	default:
+		return false
+	}
+}
+
 func (r Resource) UniqueKey() UniqueKey {
 	return r // A Resource is its own UniqueKey
 }
@@ -100,6 +116,18 @@ func (r ResourceInstance) Equal(o ResourceInstance) bool {
 	return r.Key == o.Key && r.Resource.Equal(o.Resource)
 }
 
+func (r ResourceInstance) Less(o ResourceInstance) bool {
+	if !r.Resource.Equal(o.Resource) {
+		return r.Resource.Less(o.Resource)
+	}
+
+	if r.Key != o.Key {
+		return InstanceKeyLess(r.Key, o.Key)
+	}
+
+	return false
+}
+
 func (r ResourceInstance) UniqueKey() UniqueKey {
 	return r // A ResourceInstance is its own UniqueKey
 }
@@ -195,6 +223,18 @@ func (r AbsResource) Equal(o AbsResource) bool {
 	return r.Module.Equal(o.Module) && r.Resource.Equal(o.Resource)
 }
 
+func (r AbsResource) Less(o AbsResource) bool {
+	if !r.Module.Equal(o.Module) {
+		return r.Module.Less(o.Module)
+	}
+
+	if !r.Resource.Equal(o.Resource) {
+		return r.Resource.Less(o.Resource)
+	}
+
+	return false
+}
+
 func (r AbsResource) absMoveableSigil() {
 	// AbsResource is moveable
 }
@@ -308,30 +348,15 @@ func (r AbsResourceInstance) Equal(o AbsResourceInstance) bool {
 // Less returns true if the receiver should sort before the given other value
 // in a sorted list of addresses.
 func (r AbsResourceInstance) Less(o AbsResourceInstance) bool {
-	switch {
-
-	case len(r.Module) != len(o.Module):
-		return len(r.Module) < len(o.Module)
-
-	case r.Module.String() != o.Module.String():
+	if !r.Module.Equal(o.Module) {
 		return r.Module.Less(o.Module)
-
-	case r.Resource.Resource.Mode != o.Resource.Resource.Mode:
-		return r.Resource.Resource.Mode == DataResourceMode
-
-	case r.Resource.Resource.Type != o.Resource.Resource.Type:
-		return r.Resource.Resource.Type < o.Resource.Resource.Type
-
-	case r.Resource.Resource.Name != o.Resource.Resource.Name:
-		return r.Resource.Resource.Name < o.Resource.Resource.Name
-
-	case r.Resource.Key != o.Resource.Key:
-		return InstanceKeyLess(r.Resource.Key, o.Resource.Key)
-
-	default:
-		return false
-
 	}
+
+	if !r.Resource.Equal(o.Resource) {
+		return r.Resource.Less(o.Resource)
+	}
+
+	return false
 }
 
 // AbsResourceInstance is a Checkable

internal/command/jsonformat/diff.go

@@ -1,13 +1,10 @@
 package jsonformat
 
 import (
-	"sort"
-
 	"github.com/hashicorp/terraform/internal/command/jsonformat/computed"
 	"github.com/hashicorp/terraform/internal/command/jsonformat/differ"
 	"github.com/hashicorp/terraform/internal/command/jsonformat/differ/attribute_path"
 	"github.com/hashicorp/terraform/internal/command/jsonplan"
-	"github.com/hashicorp/terraform/internal/command/jsonstate"
 	"github.com/hashicorp/terraform/internal/plans"
 )
 
@@ -63,31 +60,6 @@ func precomputeDiffs(plan Plan, mode plans.Mode) diffs {
 		diffs.outputs[key] = differ.FromJsonChange(output, attribute_path.AlwaysMatcher()).ComputeDiffForOutput()
 	}
 
-	less := func(drs []diff) func(i, j int) bool {
-		return func(i, j int) bool {
-			left := drs[i].change
-			right := drs[j].change
-
-			if left.ModuleAddress != right.ModuleAddress {
-				return left.ModuleAddress < right.ModuleAddress
-			}
-
-			if left.Mode != right.Mode {
-				return left.Mode == jsonstate.DataResourceMode
-			}
-
-			if left.Address != right.Address {
-				return left.Address < right.Address
-			}
-
-			// Everything else being equal, we'll sort by deposed.
-			return left.Deposed < right.Deposed
-		}
-	}
-
-	sort.Slice(diffs.drift, less(diffs.drift))
-	sort.Slice(diffs.changes, less(diffs.changes))
-
 	return diffs
 }
 

internal/command/jsonformat/state.go

@@ -36,30 +36,11 @@ func (state State) GetSchema(resource jsonstate.Resource) *jsonprovider.Schema {
 }
 
 func (state State) renderHumanStateModule(renderer Renderer, module jsonstate.Module, opts computed.RenderHumanOpts, first bool) {
-	// Sort the resources in the module first, for consistent output.
-	var resources []jsonstate.Resource
-	resources = append(resources, module.Resources...)
-	sort.Slice(resources, func(i, j int) bool {
-		left := resources[i]
-		right := resources[j]
-
-		if left.Mode != right.Mode {
-			return left.Mode == jsonstate.DataResourceMode
-		}
-
-		if left.Address != right.Address {
-			return left.Address < right.Address
-		}
-
-		// Everything else being equal, we'll sort by deposed.
-		return left.DeposedKey < right.DeposedKey
-	})
-
-	if len(resources) > 0 && !first {
+	if len(module.Resources) > 0 && !first {
 		renderer.Streams.Println()
 	}
 
-	for _, resource := range resources {
+	for _, resource := range module.Resources {
 
 		if !first {
 			renderer.Streams.Println()

internal/command/jsonplan/plan.go

@@ -329,7 +329,16 @@ func (p *plan) marshalPlanVariables(vars map[string]plans.DynamicValue, decls ma
 func MarshalResourceChanges(resources []*plans.ResourceInstanceChangeSrc, schemas *terraform.Schemas) ([]ResourceChange, error) {
 	var ret []ResourceChange
 
-	for _, rc := range resources {
+	var sortedResources []*plans.ResourceInstanceChangeSrc
+	sortedResources = append(sortedResources, resources...)
+	sort.Slice(sortedResources, func(i, j int) bool {
+		if !sortedResources[i].Addr.Equal(sortedResources[j].Addr) {
+			return sortedResources[i].Addr.Less(sortedResources[j].Addr)
+		}
+		return sortedResources[i].DeposedKey < sortedResources[j].DeposedKey
+	})
+
+	for _, rc := range sortedResources {
 		var r ResourceChange
 		addr := rc.Addr
 		r.Address = addr.String()
@@ -491,10 +500,6 @@ func MarshalResourceChanges(resources []*plans.ResourceInstanceChangeSrc, schema
 
 	}
 
-	sort.Slice(ret, func(i, j int) bool {
-		return ret[i].Address < ret[j].Address
-	})
-
 	return ret, nil
 }
 

internal/command/jsonstate/state.go

@@ -323,8 +323,27 @@ func marshalModules(
 func marshalResources(resources map[string]*states.Resource, module addrs.ModuleInstance, schemas *terraform.Schemas) ([]Resource, error) {
 	var ret []Resource
 
+	var sortedResources []*states.Resource
 	for _, r := range resources {
-		for k, ri := range r.Instances {
+		sortedResources = append(sortedResources, r)
+	}
+	sort.Slice(sortedResources, func(i, j int) bool {
+		return sortedResources[i].Addr.Less(sortedResources[j].Addr)
+	})
+
+	for _, r := range sortedResources {
+
+		var sortedKeys []addrs.InstanceKey
+		for k := range r.Instances {
+			sortedKeys = append(sortedKeys, k)
+		}
+		sort.Slice(sortedKeys, func(i, j int) bool {
+			return addrs.InstanceKeyLess(sortedKeys[i], sortedKeys[j])
+		})
+
+		for _, k := range sortedKeys {
+			ri := r.Instances[k]
+
 			var err error
 
 			resAddr := r.Addr.Resource
@@ -400,7 +419,15 @@ func marshalResources(resources map[string]*states.Resource, module addrs.Module
 				ret = append(ret, current)
 			}
 
-			for deposedKey, rios := range ri.Deposed {
+			var sortedDeposedKeys []string
+			for k := range ri.Deposed {
+				sortedDeposedKeys = append(sortedDeposedKeys, string(k))
+			}
+			sort.Strings(sortedDeposedKeys)
+
+			for _, deposedKey := range sortedDeposedKeys {
+				rios := ri.Deposed[states.DeposedKey(deposedKey)]
+
 				// copy the base fields from the current instance
 				deposed := Resource{
 					Address:      current.Address,
@@ -436,16 +463,12 @@ func marshalResources(resources map[string]*states.Resource, module addrs.Module
 				if riObj.Status == states.ObjectTainted {
 					deposed.Tainted = true
 				}
-				deposed.DeposedKey = deposedKey.String()
+				deposed.DeposedKey = deposedKey
 				ret = append(ret, deposed)
 			}
 		}
 	}
 
-	sort.Slice(ret, func(i, j int) bool {
-		return ret[i].Address < ret[j].Address
-	})
-
 	return ret, nil
 }
 

internal/command/testdata/show-json/provider-aliasing-conflict/output.json

@@ -39,6 +39,27 @@
     }
   },
   "resource_changes": [
+    {
+      "address": "test_instance.test",
+      "mode": "managed",
+      "type": "test_instance",
+      "name": "test",
+      "provider_name": "registry.terraform.io/hashicorp/test",
+      "change": {
+        "actions": [
+          "create"
+        ],
+        "before": null,
+        "after": {
+          "ami": "foo"
+        },
+        "after_unknown": {
+          "id": true
+        },
+        "before_sensitive": false,
+        "after_sensitive": {}
+      }
+    },
     {
       "address": "module.child.test_instance.test",
       "module_address": "module.child",
@@ -60,27 +81,6 @@
         "before_sensitive": false,
         "after_sensitive": {}
       }
-    },
-    {
-      "address": "test_instance.test",
-      "mode": "managed",
-      "type": "test_instance",
-      "name": "test",
-      "provider_name": "registry.terraform.io/hashicorp/test",
-      "change": {
-        "actions": [
-          "create"
-        ],
-        "before": null,
-        "after": {
-          "ami": "foo"
-        },
-        "after_unknown": {
-          "id": true
-        },
-        "before_sensitive": false,
-        "after_sensitive": {}
-      }
     }
   ],
   "configuration": {

internal/command/testdata/show-json/provider-aliasing-default/output.json

@@ -75,6 +75,49 @@
     }
   },
   "resource_changes": [
+    {
+      "address": "test_instance.test",
+      "mode": "managed",
+      "type": "test_instance",
+      "name": "test",
+      "provider_name": "registry.terraform.io/hashicorp/test",
+      "change": {
+        "actions": [
+          "create"
+        ],
+        "before": null,
+        "after": {
+          "ami": "foo"
+        },
+        "after_unknown": {
+          "id": true
+        },
+        "before_sensitive": false,
+        "after_sensitive": {}
+      }
+    },
+    {
+      "address": "module.child.test_instance.test",
+      "module_address": "module.child",
+      "mode": "managed",
+      "type": "test_instance",
+      "name": "test",
+      "provider_name": "registry.terraform.io/hashicorp/test",
+      "change": {
+        "actions": [
+          "create"
+        ],
+        "before": null,
+        "after": {
+          "ami": "bar"
+        },
+        "after_unknown": {
+          "id": true
+        },
+        "before_sensitive": false,
+        "after_sensitive": {}
+      }
+    },
     {
       "address": "module.child.module.no_requirements.test_instance.test",
       "module_address": "module.child.module.no_requirements",
@@ -118,49 +161,6 @@
         "before_sensitive": false,
         "after_sensitive": {}
       }
-    },
-    {
-      "address": "module.child.test_instance.test",
-      "module_address": "module.child",
-      "mode": "managed",
-      "type": "test_instance",
-      "name": "test",
-      "provider_name": "registry.terraform.io/hashicorp/test",
-      "change": {
-        "actions": [
-          "create"
-        ],
-        "before": null,
-        "after": {
-          "ami": "bar"
-        },
-        "after_unknown": {
-          "id": true
-        },
-        "before_sensitive": false,
-        "after_sensitive": {}
-      }
-    },
-    {
-      "address": "test_instance.test",
-      "mode": "managed",
-      "type": "test_instance",
-      "name": "test",
-      "provider_name": "registry.terraform.io/hashicorp/test",
-      "change": {
-        "actions": [
-          "create"
-        ],
-        "before": null,
-        "after": {
-          "ami": "foo"
-        },
-        "after_unknown": {
-          "id": true
-        },
-        "before_sensitive": false,
-        "after_sensitive": {}
-      }
     }
   ],
   "configuration": {

internal/command/testdata/show-json/provider-aliasing/output.json

@@ -155,11 +155,10 @@
   },
   "resource_changes": [
     {
-      "address": "module.child.module.grandchild.test_instance.test_alternate",
-      "module_address": "module.child.module.grandchild",
+      "address": "test_instance.test",
       "mode": "managed",
       "type": "test_instance",
-      "name": "test_alternate",
+      "name": "test",
       "provider_name": "registry.terraform.io/hashicorp/test",
       "change": {
         "actions": [
@@ -167,7 +166,7 @@
         ],
         "before": null,
         "after": {
-          "ami": "secondary"
+          "ami": "foo"
         },
         "after_unknown": {
           "id": true
@@ -177,11 +176,10 @@
       }
     },
     {
-      "address": "module.child.module.grandchild.test_instance.test_main",
-      "module_address": "module.child.module.grandchild",
+      "address": "test_instance.test_backup",
       "mode": "managed",
       "type": "test_instance",
-      "name": "test_main",
+      "name": "test_backup",
       "provider_name": "registry.terraform.io/hashicorp/test",
       "change": {
         "actions": [
@@ -189,7 +187,7 @@
         ],
         "before": null,
         "after": {
-          "ami": "main"
+          "ami": "foo-backup"
         },
         "after_unknown": {
           "id": true
@@ -242,50 +240,6 @@
         "after_sensitive": {}
       }
     },
-    {
-      "address": "module.sibling.module.grandchild.test_instance.test_alternate",
-      "module_address": "module.sibling.module.grandchild",
-      "mode": "managed",
-      "type": "test_instance",
-      "name": "test_alternate",
-      "provider_name": "registry.terraform.io/hashicorp/test",
-      "change": {
-        "actions": [
-          "create"
-        ],
-        "before": null,
-        "after": {
-          "ami": "secondary"
-        },
-        "after_unknown": {
-          "id": true
-        },
-        "before_sensitive": false,
-        "after_sensitive": {}
-      }
-    },
-    {
-      "address": "module.sibling.module.grandchild.test_instance.test_main",
-      "module_address": "module.sibling.module.grandchild",
-      "mode": "managed",
-      "type": "test_instance",
-      "name": "test_main",
-      "provider_name": "registry.terraform.io/hashicorp/test",
-      "change": {
-        "actions": [
-          "create"
-        ],
-        "before": null,
-        "after": {
-          "ami": "main"
-        },
-        "after_unknown": {
-          "id": true
-        },
-        "before_sensitive": false,
-        "after_sensitive": {}
-      }
-    },
     {
       "address": "module.sibling.test_instance.test_primary",
       "module_address": "module.sibling",
@@ -331,10 +285,11 @@
       }
     },
     {
-      "address": "test_instance.test",
+      "address": "module.child.module.grandchild.test_instance.test_alternate",
+      "module_address": "module.child.module.grandchild",
       "mode": "managed",
       "type": "test_instance",
-      "name": "test",
+      "name": "test_alternate",
       "provider_name": "registry.terraform.io/hashicorp/test",
       "change": {
         "actions": [
@@ -342,7 +297,7 @@
         ],
         "before": null,
         "after": {
-          "ami": "foo"
+          "ami": "secondary"
         },
         "after_unknown": {
           "id": true
@@ -352,10 +307,11 @@
       }
     },
     {
-      "address": "test_instance.test_backup",
+      "address": "module.child.module.grandchild.test_instance.test_main",
+      "module_address": "module.child.module.grandchild",
       "mode": "managed",
       "type": "test_instance",
-      "name": "test_backup",
+      "name": "test_main",
       "provider_name": "registry.terraform.io/hashicorp/test",
       "change": {
         "actions": [
@@ -363,7 +319,51 @@
         ],
         "before": null,
         "after": {
-          "ami": "foo-backup"
+          "ami": "main"
+        },
+        "after_unknown": {
+          "id": true
+        },
+        "before_sensitive": false,
+        "after_sensitive": {}
+      }
+    },
+    {
+      "address": "module.sibling.module.grandchild.test_instance.test_alternate",
+      "module_address": "module.sibling.module.grandchild",
+      "mode": "managed",
+      "type": "test_instance",
+      "name": "test_alternate",
+      "provider_name": "registry.terraform.io/hashicorp/test",
+      "change": {
+        "actions": [
+          "create"
+        ],
+        "before": null,
+        "after": {
+          "ami": "secondary"
+        },
+        "after_unknown": {
+          "id": true
+        },
+        "before_sensitive": false,
+        "after_sensitive": {}
+      }
+    },
+    {
+      "address": "module.sibling.module.grandchild.test_instance.test_main",
+      "module_address": "module.sibling.module.grandchild",
+      "mode": "managed",
+      "type": "test_instance",
+      "name": "test_main",
+      "provider_name": "registry.terraform.io/hashicorp/test",
+      "change": {
+        "actions": [
+          "create"
+        ],
+        "before": null,
+        "after": {
+          "ami": "main"
         },
         "after_unknown": {
           "id": true