From 1489e193c75948e909d2e4b1746ba2a524c7c711 Mon Sep 17 00:00:00 2001 From: Zaid Ajaj Date: Thu, 22 Feb 2024 08:44:05 +0100 Subject: [PATCH] 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 --- CHANGELOG_PENDING.md | 2 ++ .../testdata/programs/expressions/main.tf | 11 +++++++ .../testdata/programs/expressions/pcl/main.pp | 11 +++++++ .../programs/name_conflict/pcl/main.pp | 6 ++-- .../pcl/diagnostics.json | 4 +-- .../partial_attribute_lookup/pcl/main.pp | 4 +-- .../pcl/dir_1.0.0/files.pp | 2 +- .../pcl/dir_1.0.2/files.pp | 2 +- pkg/convert/tf.go | 29 ++++++++++++++----- pkg/convert/tf_scopes.go | 10 ++++++- 10 files changed, 63 insertions(+), 18 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 5ecddce..c450264 100644 --- 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 index e5a1c84..4107d5c 100644 --- 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 index c747f5d..9e09676 100644 --- 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 index 614202f..e7b1f1e 100644 --- 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 index 41b4bde..ad460e4 100644 --- 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 index ddb9699..b4a7bf9 100644 --- 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 index e31b8ea..4ed134a 100644 --- 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 index e31b8ea..4ed134a 100644 --- 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 index 46ad8e0..bf8e305 100644 --- 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 index 7918b39..4db8cb7 100644 --- 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 }