From ef94e28fa080541a5d7f27554d51ce4685f03697 Mon Sep 17 00:00:00 2001 From: VenelinMartinov Date: Tue, 29 Oct 2024 18:31:12 +0000 Subject: [PATCH] Move property path code to separate file (#2497) This is a pure refactor and just moves the propertyPath-related code to a new file. This is a separate PR to make reviewing easier. Stacked on https://github.com/pulumi/pulumi-terraform-bridge/pull/2515, https://github.com/pulumi/pulumi-terraform-bridge/pull/2516 and https://github.com/pulumi/pulumi-terraform-bridge/pull/2496 --- pkg/tfbridge/detailed_diff.go | 103 +------------------------------- pkg/tfbridge/property_path.go | 107 ++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 102 deletions(-) create mode 100644 pkg/tfbridge/property_path.go diff --git a/pkg/tfbridge/detailed_diff.go b/pkg/tfbridge/detailed_diff.go index 5e87e39f8..f58b7ec13 100644 --- a/pkg/tfbridge/detailed_diff.go +++ b/pkg/tfbridge/detailed_diff.go @@ -9,10 +9,8 @@ import ( "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" pulumirpc "github.com/pulumi/pulumi/sdk/v3/proto/go" - "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info" shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/walk" - "github.com/pulumi/pulumi-terraform-bridge/v3/unstable/propertyvalue" ) func isPresent(val resource.PropertyValue) bool { @@ -21,11 +19,6 @@ func isPresent(val resource.PropertyValue) bool { !(val.IsObject() && val.ObjectValue() == nil) } -func isForceNew(tfs shim.Schema, ps *SchemaInfo) bool { - return (tfs != nil && tfs.ForceNew()) || - (ps != nil && ps.ForceNew != nil && *ps.ForceNew) -} - func sortedMergedKeys[K cmp.Ordered, V any, M ~map[K]V](a, b M) []K { keys := make(map[K]struct{}) for k := range a { @@ -62,67 +55,6 @@ func isTypeShapeMismatched(val resource.PropertyValue, propType shim.ValueType) } } -func lookupSchemas( - path propertyPath, tfs shim.SchemaMap, ps map[string]*info.Schema, -) (shim.Schema, *info.Schema, error) { - schemaPath := PropertyPathToSchemaPath(resource.PropertyPath(path), tfs, ps) - return LookupSchemas(schemaPath, tfs, ps) -} - -func propertyPathTriggersReplacement( - path propertyPath, rootTFSchema shim.SchemaMap, rootPulumiSchema map[string]*info.Schema, -) bool { - // A change on a property might trigger a replacement if: - // - The property itself is marked as ForceNew - // - The direct parent property is a collection (list, set, map) and is marked as ForceNew - // See pkg/cross-tests/diff_cross_test.go - // TestAttributeCollectionForceNew, TestBlockCollectionForceNew, TestBlockCollectionElementForceNew - // for a full case study of replacements in TF - tfs, ps, err := lookupSchemas(path, rootTFSchema, rootPulumiSchema) - if err != nil { - return false - } - if isForceNew(tfs, ps) { - return true - } - - if len(path) == 1 { - return false - } - - parent := path[:len(path)-1] - tfs, ps, err = lookupSchemas(parent, rootTFSchema, rootPulumiSchema) - if err != nil { - return false - } - // Note this is mimicking the TF behaviour, so the effective type is not considered here. - if tfs.Type() != shim.TypeList && tfs.Type() != shim.TypeSet && tfs.Type() != shim.TypeMap { - return false - } - return isForceNew(tfs, ps) -} - -func propertyValueTriggersReplacement( - path propertyPath, value resource.PropertyValue, tfs shim.SchemaMap, ps map[string]*info.Schema, -) bool { - replacement := false - visitor := func(subpath resource.PropertyPath, val resource.PropertyValue) (resource.PropertyValue, error) { - if propertyPathTriggersReplacement(propertyPath(subpath), tfs, ps) { - replacement = true - } - return val, nil - } - - _, err := propertyvalue.TransformPropertyValue( - resource.PropertyPath(path), - visitor, - value, - ) - contract.AssertNoErrorf(err, "TransformPropertyValue should not return an error") - - return replacement -} - func promoteToReplace(diff *pulumirpc.PropertyDiff) *pulumirpc.PropertyDiff { if diff == nil { return nil @@ -191,39 +123,7 @@ func makeBaseDiff(old, new resource.PropertyValue) baseDiff { return undecidedDiff } -type ( - detailedDiffKey string - propertyPath resource.PropertyPath -) - -func newPropertyPath(root resource.PropertyKey) propertyPath { - return propertyPath{string(root)} -} - -func (k propertyPath) String() string { - return resource.PropertyPath(k).String() -} - -func (k propertyPath) Key() detailedDiffKey { - return detailedDiffKey(k.String()) -} - -func (k propertyPath) append(subkey interface{}) propertyPath { - return append(k, subkey) -} - -func (k propertyPath) Subpath(subkey string) propertyPath { - return k.append(subkey) -} - -func (k propertyPath) Index(i int) propertyPath { - return k.append(i) -} - -func (k propertyPath) IsReservedKey() bool { - leaf := k[len(k)-1] - return leaf == "__meta" || leaf == "__defaults" -} +type detailedDiffKey string type detailedDiffer struct { tfs shim.SchemaMap @@ -310,7 +210,6 @@ func (differ detailedDiffer) makePropDiff( return nil } propType := differ.getEffectiveType(differ.propertyPathToSchemaPath(path)) - if isTypeShapeMismatched(old, propType) || isTypeShapeMismatched(new, propType) { return differ.makePlainPropDiff(path, old, new) } diff --git a/pkg/tfbridge/property_path.go b/pkg/tfbridge/property_path.go new file mode 100644 index 000000000..4c88323c9 --- /dev/null +++ b/pkg/tfbridge/property_path.go @@ -0,0 +1,107 @@ +package tfbridge + +import ( + "github.com/pulumi/pulumi/sdk/v3/go/common/resource" + "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" + + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info" + shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" + "github.com/pulumi/pulumi-terraform-bridge/v3/unstable/propertyvalue" +) + +type propertyPath resource.PropertyPath + +func isForceNew(tfs shim.Schema, ps *SchemaInfo) bool { + return (tfs != nil && tfs.ForceNew()) || + (ps != nil && ps.ForceNew != nil && *ps.ForceNew) +} + +func newPropertyPath(root resource.PropertyKey) propertyPath { + return propertyPath{string(root)} +} + +func (k propertyPath) String() string { + return resource.PropertyPath(k).String() +} + +func (k propertyPath) Key() detailedDiffKey { + return detailedDiffKey(k.String()) +} + +func (k propertyPath) append(subkey interface{}) propertyPath { + return append(k, subkey) +} + +func (k propertyPath) Subpath(subkey string) propertyPath { + return k.append(subkey) +} + +func (k propertyPath) Index(i int) propertyPath { + return k.append(i) +} + +func (k propertyPath) IsReservedKey() bool { + leaf := k[len(k)-1] + return leaf == "__meta" || leaf == "__defaults" +} + +func lookupSchemas( + path propertyPath, tfs shim.SchemaMap, ps map[string]*info.Schema, +) (shim.Schema, *info.Schema, error) { + schemaPath := PropertyPathToSchemaPath(resource.PropertyPath(path), tfs, ps) + return LookupSchemas(schemaPath, tfs, ps) +} + +func propertyPathTriggersReplacement( + path propertyPath, rootTFSchema shim.SchemaMap, rootPulumiSchema map[string]*info.Schema, +) bool { + // A change on a property might trigger a replacement if: + // - The property itself is marked as ForceNew + // - The direct parent property is a collection (list, set, map) and is marked as ForceNew + // See pkg/cross-tests/diff_cross_test.go + // TestAttributeCollectionForceNew, TestBlockCollectionForceNew, TestBlockCollectionElementForceNew + // for a full case study of replacements in TF + tfs, ps, err := lookupSchemas(path, rootTFSchema, rootPulumiSchema) + if err != nil { + return false + } + if isForceNew(tfs, ps) { + return true + } + + if len(path) == 1 { + return false + } + + parent := path[:len(path)-1] + tfs, ps, err = lookupSchemas(parent, rootTFSchema, rootPulumiSchema) + if err != nil { + return false + } + // Note this is mimicking the TF behaviour, so the effective type is not considered here. + if tfs.Type() != shim.TypeList && tfs.Type() != shim.TypeSet && tfs.Type() != shim.TypeMap { + return false + } + return isForceNew(tfs, ps) +} + +func propertyValueTriggersReplacement( + path propertyPath, value resource.PropertyValue, tfs shim.SchemaMap, ps map[string]*info.Schema, +) bool { + replacement := false + visitor := func(subpath resource.PropertyPath, val resource.PropertyValue) (resource.PropertyValue, error) { + if propertyPathTriggersReplacement(propertyPath(subpath), tfs, ps) { + replacement = true + } + return val, nil + } + + _, err := propertyvalue.TransformPropertyValue( + resource.PropertyPath(path), + visitor, + value, + ) + contract.AssertNoErrorf(err, "TransformPropertyValue should not return an error") + + return replacement +}