Skip to content

Commit

Permalink
use walkTwoPropertyValues recursion
Browse files Browse the repository at this point in the history
  • Loading branch information
@VenelinMartinov
VenelinMartinov committed Dec 19, 2024
1 parent 42ba75a commit 9b46547
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 163 deletions.
170 changes: 22 additions & 148 deletions pkg/tfbridge/property_path.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@ func (k propertyPath) Index(i int) propertyPath {
return k.append(i)
}

func (k propertyPath) Get(v resource.PropertyValue) (resource.PropertyValue, bool) {
path := resource.PropertyPath(k)
return path.Get(v)
}

func (k propertyPath) IsReservedKey() bool {
leaf := k[len(k)-1]
return leaf == "__meta" || leaf == "__defaults"
Expand All @@ -60,16 +55,6 @@ func (k propertyPath) GetFromMap(v resource.PropertyMap) (resource.PropertyValue
return path.Get(resource.NewProperty(v))
}

func (k propertyPath) GetPathRelativeTo(other propertyPath) (propertyPath, error) {
contains := resource.PropertyPath(other).Contains(resource.PropertyPath(k))
if !contains {
return propertyPath{}, errors.New("other path is not a subpath of k")
}

relativePath := resource.PropertyPath(k)[len(resource.PropertyPath(other)):]
return propertyPath(relativePath), nil
}

func lookupSchemas(
path propertyPath, tfs shim.SchemaMap, ps map[string]*info.Schema,
) (shim.Schema, *info.Schema, error) {
Expand Down Expand Up @@ -244,9 +229,11 @@ func propertyValueTriggersReplacement(
return replacement
}

// isPlanCompatibleWithInputs returns true if all values in the walkedValue are also in the comparedValue,
// bar any computed properties.
func isPlanCompatibleWithInputs(
// validInputsFromPlan returns true if the given plan property value could originate from the given inputs.
// Under the hood, it walks the plan and the inputs and checks that all differences stem from computed properties.
// Any differences coming from properties which are not computed will be rejected.
// Note that we are relying on the fact that the inputs will have defaults already applied.
func validInputsFromPlan(
path propertyPath,
inputs resource.PropertyValue,
plan resource.PropertyValue,
Expand All @@ -255,166 +242,53 @@ func isPlanCompatibleWithInputs(
) bool {
abortErr := errors.New("abort")
visitor := func(
subpath resource.PropertyPath, planSubVal resource.PropertyValue, entering bool,
) (resource.PropertyValue, error) {
if !entering {
return planSubVal, nil
}

subpath propertyPath, inputsSubVal, planSubVal resource.PropertyValue,
) error {
// Do not compare and do not descend into internal properties.
{
lastPathStep := subpath[len(subpath)-1]
if stepStr, ok := lastPathStep.(string); ok {
if resource.IsInternalPropertyKey(resource.PropertyKey(stepStr)) {
return planSubVal, propertyvalue.SkipChildrenError{}
}
}
if subpath.IsReservedKey() {
return SkipChildrenError{}
}

tfs, _, err := lookupSchemas(propertyPath(subpath), tfs, ps)
tfs, _, err := lookupSchemas(subpath, tfs, ps)
if err != nil {
// TODO log
return resource.NewNullProperty(), abortErr
}

relativePath, err := propertyPath(subpath).GetPathRelativeTo(path)
if err != nil {
return resource.NewNullProperty(), abortErr
}

inputsSubVal, ok := relativePath.Get(inputs)
if !ok || inputsSubVal.IsNull() {
if tfs.Computed() {
return planSubVal, nil
}
return resource.NewNullProperty(), abortErr
}

if tfs.Type() == shim.TypeList || tfs.Type() == shim.TypeMap {
// We only need to check the leaf values, so we skip any collection types.
return planSubVal, nil
}

// We can not descend into nested sets as planning re-orders the elements
if tfs.Type() == shim.TypeSet {
// TODO: more sophisticated comparison of nested sets
if planSubVal.DeepEquals(inputsSubVal) {
return planSubVal, propertyvalue.SkipChildrenError{}
}
return resource.NewNullProperty(), abortErr
}

if planSubVal.DeepEquals(inputsSubVal) {
return planSubVal, nil
}

return resource.NewNullProperty(), abortErr
}
_, err := propertyvalue.TransformPropertyValueDirectional(
resource.PropertyPath(path),
visitor,
plan,
)
if err == abortErr {
return false
}
contract.AssertNoErrorf(err, "TransformPropertyValue should only return an abort error")
return true
}

func isInputCompatibleWithPlan(
path propertyPath,
plan resource.PropertyValue,
inputs resource.PropertyValue,
tfs shim.SchemaMap,
ps map[string]*info.Schema,
) bool {
abortErr := errors.New("abort")
visitor := func(
subpath resource.PropertyPath, inputsSubVal resource.PropertyValue, entering bool,
) (resource.PropertyValue, error) {
if !entering {
return inputsSubVal, nil
}
// Do not compare and do not descend into internal properties.
{
lastPathStep := subpath[len(subpath)-1]
if stepStr, ok := lastPathStep.(string); ok {
if resource.IsInternalPropertyKey(resource.PropertyKey(stepStr)) {
return inputsSubVal, propertyvalue.SkipChildrenError{}
}
}
}

tfs, _, err := lookupSchemas(propertyPath(subpath), tfs, ps)
if err != nil {
// TODO log
return resource.NewNullProperty(), abortErr
}

relativePath, err := propertyPath(subpath).GetPathRelativeTo(path)
if err != nil {
return resource.NewNullProperty(), abortErr
}

planSubVal, ok := relativePath.Get(plan)
if !ok || planSubVal.IsNull() {
return resource.NewNullProperty(), abortErr
return abortErr
}

if tfs.Computed() && inputsSubVal.IsNull() {
return planSubVal, nil
// This is a computed property populated by the plan. We should not recurse into it.
return SkipChildrenError{}
}

if tfs.Type() == shim.TypeList || tfs.Type() == shim.TypeMap {
// We only need to check the leaf values, so we skip any collection types.
return inputsSubVal, nil
return nil
}

// We can not descend into nested sets as planning re-orders the elements
if tfs.Type() == shim.TypeSet {
// TODO: more sophisticated comparison of nested sets
if inputsSubVal.DeepEquals(planSubVal) {
return inputsSubVal, propertyvalue.SkipChildrenError{}
return SkipChildrenError{}
}
return resource.NewNullProperty(), abortErr
return abortErr
}

if inputsSubVal.DeepEquals(planSubVal) {
return inputsSubVal, nil
return nil
}

return resource.NewNullProperty(), abortErr
return abortErr
}
_, err := propertyvalue.TransformPropertyValueDirectional(
resource.PropertyPath(path),
visitor,
err := walkTwoPropertyValues(
path,
inputs,
plan,
visitor,
)
if err == abortErr {
return false
}
contract.AssertNoErrorf(err, "TransformPropertyValue should only return an abort error")
return true
}

// validInputsFromPlan returns true if the given plan property value could originate from the given inputs.
// Under the hood, it walks the plan and the inputs and checks that all differences stem from computed properties.
// Any differences coming from properties which are not computed will be rejected.
// Note that we are relying on the fact that the inputs will have defaults already applied.
func validInputsFromPlan(
path propertyPath,
inputs resource.PropertyValue,
plan resource.PropertyValue,
tfs shim.SchemaMap,
ps map[string]*info.Schema,
) bool {
// We walk both the plan and the inputs and check that all differences stem from computed properties.
// We first walk over the plan.
if !isPlanCompatibleWithInputs(path, inputs, plan, tfs, ps) {
return false
}

// We then walk over the inputs.
return isInputCompatibleWithPlan(path, plan, inputs, tfs, ps)
}
15 changes: 0 additions & 15 deletions pkg/tfbridge/property_path_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/pkg/errors"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim"
Expand Down Expand Up @@ -310,20 +309,6 @@ func TestPropertyPath(t *testing.T) {
require.Equal(t, newPropertyPath("foo").Subpath("bar").Key(), detailedDiffKey("foo.bar"))
}

func TestGetPathRelativeTo(t *testing.T) {
t.Parallel()
relativePath, err := newPropertyPath("foo").Subpath("bar").GetPathRelativeTo(newPropertyPath("foo"))
require.NoError(t, err)
assert.Equal(t, relativePath, newPropertyPath("bar"))

relativePath, err = newPropertyPath("foo").Subpath("bar").Index(0).GetPathRelativeTo(newPropertyPath("foo"))
require.NoError(t, err)
assert.Equal(t, relativePath, newPropertyPath("bar").Index(0))

_, err = newPropertyPath("foo").GetPathRelativeTo(newPropertyPath("bar"))
require.Error(t, err)
}

func TestLookupSchemasPropertyPath(t *testing.T) {
t.Parallel()

Expand Down

0 comments on commit 9b46547

Please sign in to comment.