Sort generated properties by their keys to get deterministic output. (#…

…110) * Sort generated properties by the position of their keys to get deterministic output. * review feedback --------- Co-authored-by: Fraser Waters <fraser@pulumi.com>
pulumi · Feb 22, 2024 · 1489e19 · 1489e19
1 parent 41b7d19
commit 1489e19
Show file tree

Hide file tree

Showing 10 changed files with 63 additions and 18 deletions.
diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md
@@ -1,3 +1,5 @@
 ### Improvements
 
+ - Sort generated properties by the position of their keys to get deterministic output.
+
 ### Bug Fixes
diff --git a/pkg/convert/testdata/programs/expressions/main.tf b/pkg/convert/testdata/programs/expressions/main.tf
@@ -33,6 +33,17 @@ output "str_object_out" {
     }
 }
 
+output "sorted_object_out" {
+    value = {
+        nested: {
+            b: 4
+            a: 3
+        }
+        b: 2
+        a: 1
+    }
+}
+
 locals {
     a_key = "hello"
     a_value = -1

diff --git a/pkg/convert/testdata/programs/expressions/pcl/main.pp b/pkg/convert/testdata/programs/expressions/pcl/main.pp
@@ -32,6 +32,17 @@
     goodbye = "ha det"
   }
 }
+
+output "sortedObjectOut" {
+  value = {
+    nested = {
+      b = 4
+      a = 3
+    }
+    b = 2
+    a = 1
+  }
+}
 aKey   = "hello"
 aValue = -1
 aList  = [1, 2, 3]

diff --git a/pkg/convert/testdata/programs/name_conflict/pcl/main.pp b/pkg/convert/testdata/programs/name_conflict/pcl/main.pp
@@ -1,16 +1,16 @@
 config "aThing" {
 }
-myaThing = true
+myAThing = true
 
 resource "aThingResource" "simple:index:resource" {
   __logicalName = "a_thing"
   inputOne      = "Hello ${aThing}"
-  inputTwo      = myaThing
+  inputTwo      = myAThing
 }
 
 aThingDataSource = invoke("simple:index:dataSource", {
   inputOne = "Hello ${aThingResource.result}"
-  inputTwo = myaThing
+  inputTwo = myAThing
 })
 
 resource "aThingAnotherResource" "simple:index:anotherResource" {

diff --git a/pkg/convert/testdata/programs/partial_attribute_lookup/pcl/diagnostics.json b/pkg/convert/testdata/programs/partial_attribute_lookup/pcl/diagnostics.json
@@ -1,4 +1,4 @@
 [
-  "warning:main.pp:1,14-42:undefined variable exampledataunknownDataSource:",
-  "warning:main.pp:2,14-40:undefined variable exampleunknownResourceType:"
+  "warning:main.pp:1,14-38:undefined variable exampleUnknownDataSource:",
+  "warning:main.pp:2,14-40:undefined variable exampleUnknownResourceType:"
 ]
diff --git a/pkg/convert/testdata/programs/partial_attribute_lookup/pcl/main.pp b/pkg/convert/testdata/programs/partial_attribute_lookup/pcl/main.pp
@@ -1,4 +1,4 @@
 resource "example" "simple:index:resource" {
-  inputOne = exampledataunknownDataSource.attr
-  inputTwo = exampleunknownResourceType.list[0]
+  inputOne = exampleUnknownDataSource.attr
+  inputTwo = exampleUnknownResourceType.list[0]
 }
diff --git a/pkg/convert/testdata/programs/registry_module_multiple_versions/pcl/dir_1.0.0/files.pp b/pkg/convert/testdata/programs/registry_module_multiple_versions/pcl/dir_1.0.0/files.pp
@@ -10,5 +10,5 @@
 outputFilePaths      = notImplemented("setunion(keys(local.template_file_paths),local.static_file_paths)")
 fileSuffixMatches    = { for p in outputFilePaths : p => notImplemented("regexall(\"\\\\.[^\\\\.]+\\\\z\",p)") }
 fileSuffixes         = { for p, ms in fileSuffixMatches : p => length(ms) > 0 ? ms[0] : "" }
-myfileTypes          = { for p in outputFilePaths : p => notImplemented("lookup(var.file_types,local.file_suffixes[p],var.default_file_type)") }
+myFileTypes          = { for p in outputFilePaths : p => notImplemented("lookup(var.file_types,local.file_suffixes[p],var.default_file_type)") }
 files                = notImplemented("merge(\n{\nforpinkeys(local.template_file_paths):p=>{\ncontent_type=local.file_types[p]\nsource_path=tostring(null)\ncontent=local.template_file_contents[p]\ndigests=tomap({\nmd5=md5(local.template_file_contents[p])\nsha1=sha1(local.template_file_contents[p])\nsha256=sha256(local.template_file_contents[p])\nsha512=sha512(local.template_file_contents[p])\nbase64sha256=base64sha256(local.template_file_contents[p])\nbase64sha512=base64sha512(local.template_file_contents[p])\n})\n}\n},\n{\nforpinlocal.static_file_paths:p=>{\ncontent_type=local.file_types[p]\nsource_path=local.static_file_local_paths[p]\ncontent=tostring(null)\ndigests=tomap({\nmd5=filemd5(local.static_file_local_paths[p])\nsha1=filesha1(local.static_file_local_paths[p])\nsha256=filesha256(local.static_file_local_paths[p])\nsha512=filesha512(local.static_file_local_paths[p])\nbase64sha256=filebase64sha256(local.static_file_local_paths[p])\nbase64sha512=filebase64sha512(local.static_file_local_paths[p])\n})\n}\n},\n)")
diff --git a/pkg/convert/testdata/programs/registry_module_multiple_versions/pcl/dir_1.0.2/files.pp b/pkg/convert/testdata/programs/registry_module_multiple_versions/pcl/dir_1.0.2/files.pp
@@ -10,5 +10,5 @@
 outputFilePaths      = notImplemented("setunion(keys(local.template_file_paths),local.static_file_paths)")
 fileSuffixMatches    = { for p in outputFilePaths : p => notImplemented("regexall(\"\\\\.[^\\\\.]+\\\\z\",p)") }
 fileSuffixes         = { for p, ms in fileSuffixMatches : p => length(ms) > 0 ? ms[0] : "" }
-myfileTypes          = { for p in outputFilePaths : p => notImplemented("lookup(var.file_types,local.file_suffixes[p],var.default_file_type)") }
+myFileTypes          = { for p in outputFilePaths : p => notImplemented("lookup(var.file_types,local.file_suffixes[p],var.default_file_type)") }
 files                = notImplemented("merge(\n{\nforpinkeys(local.template_file_paths):p=>{\ncontent_type=local.file_types[p]\nsource_path=tostring(null)\ncontent=local.template_file_contents[p]\ndigests=tomap({\nmd5=md5(local.template_file_contents[p])\nsha1=sha1(local.template_file_contents[p])\nsha256=sha256(local.template_file_contents[p])\nsha512=sha512(local.template_file_contents[p])\nbase64sha256=base64sha256(local.template_file_contents[p])\nbase64sha512=base64sha512(local.template_file_contents[p])\n})\n}\n},\n{\nforpinlocal.static_file_paths:p=>{\ncontent_type=local.file_types[p]\nsource_path=local.static_file_local_paths[p]\ncontent=tostring(null)\ndigests=tomap({\nmd5=filemd5(local.static_file_local_paths[p])\nsha1=filesha1(local.static_file_local_paths[p])\nsha256=filesha256(local.static_file_local_paths[p])\nsha512=filesha512(local.static_file_local_paths[p])\nbase64sha256=filebase64sha256(local.static_file_local_paths[p])\nbase64sha512=filebase64sha512(local.static_file_local_paths[p])\n})\n}\n},\n)")
diff --git a/pkg/convert/tf.go b/pkg/convert/tf.go
@@ -1193,11 +1193,21 @@ func rewriteTraversal(
 		} else if root.Name == "data" && maybeFirstAttr != nil && maybeSecondAttr != nil {
 			// This is a lookup of a data resources etc, we need to rewrite this traversal such that the root is now the
 			// pulumi invoked value instead.
-			suffix := camelCaseName(maybeFirstAttr.Name)
 			path := "data." + maybeFirstAttr.Name + "." + maybeSecondAttr.Name
-			newName := scopes.getOrAddPulumiName(path, "", "data"+suffix)
-			newTraversal = append(newTraversal, hcl.TraverseRoot{Name: newName})
-			newTraversal = append(newTraversal, rewriteRelativeTraversal(scopes, path, traversal[3:])...)
+			rootName := scopes.lookup(path)
+			if rootName != "" {
+				newName := scopes.getOrAddPulumiName(path, "", "data"+camelCaseName(rootName))
+				newTraversal = append(newTraversal, hcl.TraverseRoot{Name: newName})
+				newTraversal = append(newTraversal, rewriteRelativeTraversal(scopes, path, traversal[3:])...)
+			} else {
+				// unbound data source / invoke usage.
+				// turn data.{data_source_token}.{local_name}.{rest}
+				// into {localName}{DataSourceToken}.{rest}
+				suffix := camelCaseName(maybeFirstAttr.Name)
+				newRootName := scopes.getOrAddPulumiName(path, "", suffix)
+				newTraversal = append(newTraversal, hcl.TraverseRoot{Name: newRootName})
+				newTraversal = append(newTraversal, rewriteRelativeTraversal(scopes, "", traversal[3:])...)
+			}
 		} else if root.Name == "count" && maybeFirstAttr != nil {
 			if maybeFirstAttr.Name == "index" && scopes.countIndex != nil {
 				newTraversal = append(newTraversal, scopes.countIndex...)
@@ -1299,10 +1309,13 @@ func rewriteTraversal(
 					newTraversal = append(newTraversal, hcl.TraverseRoot{Name: newName})
 					newTraversal = append(newTraversal, rewriteRelativeTraversal(scopes, "", traversal[1:])...)
 				} else {
-					// We don't know what this is, so lets assume it's an unknown resource (we shouldn't ever have unknown locals)
-					newName = scopes.getOrAddPulumiName(path, "", camelCaseName(root.Name))
-					newTraversal = append(newTraversal, hcl.TraverseRoot{Name: newName})
-					newTraversal = append(newTraversal, rewriteRelativeTraversal(scopes, path, traversal[2:])...)
+					// We don't know what this is, so let's assume it's an unknown resource (we shouldn't ever have unknown locals)
+					// turn {resource_type}.{resource_name}.{rest}
+					// into {resourceName}{ResourceType}.{rest}
+					suffix := camelCaseName(root.Name)
+					newRootName := scopes.getOrAddPulumiName(path, "", suffix)
+					newTraversal = append(newTraversal, hcl.TraverseRoot{Name: newRootName})
+					newTraversal = append(newTraversal, rewriteRelativeTraversal(scopes, "", traversal[2:])...)
 				}
 			}
 		} else {

diff --git a/pkg/convert/tf_scopes.go b/pkg/convert/tf_scopes.go
@@ -23,6 +23,7 @@ import (
 	"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge"
 	shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim"
 	"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/schema"
+	"github.com/pulumi/pulumi/pkg/v3/codegen/cgstrings"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
 	"github.com/pulumi/terraform/pkg/addrs"
 	"github.com/pulumi/terraform/pkg/lang"
@@ -127,7 +128,14 @@ func (s *scopes) generateUniqueName(name, prefix, suffix string) string {
 		return name
 	}
 	// It's used, so add the prefix and suffix
-	name = prefix + name + suffix
+	if prefix != "" {
+		name = prefix + cgstrings.UppercaseFirst(name)
+	}
+
+	if suffix != "" {
+		name = name + cgstrings.UppercaseFirst(suffix)
+	}
+
 	if !s.isUsed(name) {
 		return name
 	}