Skip to content

fix(tfpluginsdk): fix ignore changes path for converted singleton lists #633

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
castorw wants to merge 2 commits into
base: main
Choose a base branch
from
+21 −2
Open

fix(tfpluginsdk): fix ignore changes path for converted singleton lists #633

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2781a2c
fix(): expand ignore changes path for converted singleton lists
castorw Apr 20, 2026
adc87b2
fix cycle continuation istead of break in singleton list expansions
castorw Apr 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions pkg/controller/external_tfpluginsdk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ func (c *TerraformPluginSDKConnector) Connect(ctx context.Context, mg xpresource
}, nil
}

func filterInitExclusiveDiffs(tr resource.Terraformed, instanceDiff *tf.InstanceDiff) error { //nolint:gocyclo
func filterInitExclusiveDiffs(config *config.Resource, tr resource.Terraformed, instanceDiff *tf.InstanceDiff) error { //nolint:gocyclo
if instanceDiff == nil || instanceDiff.Empty() {
return nil
}
Expand All @@ -338,6 +338,25 @@ func filterInitExclusiveDiffs(tr resource.Terraformed, instanceDiff *tf.Instance
}

initProviderExclusiveParamKeys := getTerraformIgnoreChanges(paramsForProvider, paramsInitProvider)

/* process singleton list expansions */
initProviderExclusiveParamKeysConverted := []string{}
for _, key := range initProviderExclusiveParamKeys {
matched := false
for _, p := range config.TFListConversionPaths() {
matchPrefix := fmt.Sprintf("%s.", p)
if strings.HasPrefix(key, matchPrefix) {
initProviderExclusiveParamKeysConverted = append(initProviderExclusiveParamKeysConverted, strings.Replace(key, matchPrefix, fmt.Sprintf("%s0.", matchPrefix), 1))
matched = true
break
}
}
if !matched {
initProviderExclusiveParamKeysConverted = append(initProviderExclusiveParamKeysConverted, key)
}
}
initProviderExclusiveParamKeys = initProviderExclusiveParamKeysConverted
Comment on lines +342 to +358
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Normalize and apply singleton-list conversions deterministically.

Thanks for fixing the simple scaling_config case. This loop still compares raw TFListConversionPaths() values directly, but those paths may contain [0]/[*]; overlapping singleton paths can also append duplicate partial keys because continue stays inside the inner loop. Please normalize paths and produce one fully converted key per input, ideally applying longer prefixes first.

🐛 Suggested direction 
 import (
 	"context"
 	"fmt"
+	"sort"
 	"strings"
 	"time"
@@
+func normalizeTFListConversionPath(p string) string {
+	p = strings.ReplaceAll(p, "[*]", "")
+	return strings.ReplaceAll(p, "[0]", "")
+}
+
 func filterInitExclusiveDiffs(config *config.Resource, tr resource.Terraformed, instanceDiff *tf.InstanceDiff) error { //nolint:gocyclo
@@
 	/* process singleton list expansions */
-	initProviderExclusiveParamKeysConverted := []string{}
+	conversionPaths := config.TFListConversionPaths()
+	sort.Slice(conversionPaths, func(i, j int) bool {
+		return len(normalizeTFListConversionPath(conversionPaths[i])) > len(normalizeTFListConversionPath(conversionPaths[j]))
+	})
+
+	initProviderExclusiveParamKeysConverted := make([]string, 0, len(initProviderExclusiveParamKeys))
 	for _, key := range initProviderExclusiveParamKeys {
-		matched := false
-		for _, p := range config.TFListConversionPaths() {
+		convertedKey := key
+		for _, p := range conversionPaths {
+			p = normalizeTFListConversionPath(p)
 			matchPrefix := fmt.Sprintf("%s.", p)
-			if strings.HasPrefix(key, matchPrefix) {
-				initProviderExclusiveParamKeysConverted = append(initProviderExclusiveParamKeysConverted, strings.Replace(key, matchPrefix, fmt.Sprintf("%s0.", matchPrefix), 1))
-				matched = true
-				continue
+			if strings.HasPrefix(convertedKey, matchPrefix) {
+				convertedKey = strings.Replace(convertedKey, matchPrefix, fmt.Sprintf("%s0.", matchPrefix), 1)
 			}
 		}
-		if !matched {
-			initProviderExclusiveParamKeysConverted = append(initProviderExclusiveParamKeysConverted, key)
-		}
+		initProviderExclusiveParamKeysConverted = append(initProviderExclusiveParamKeysConverted, convertedKey)
 	}

Please add table-driven coverage for [0], [*], and nested/overlapping conversion paths. As per coding guidelines, All Upjet code must be covered by tests. Do not use Ginkgo tests or third party testing libraries; use Go's standard testing approach. Upjet encourages the use of table driven unit tests.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/controller/external_tfpluginsdk.go` around lines 342 - 358, Normalize
TFListConversionPaths() entries by replacing "[0]" and "[*]" with "0" and
trimming/ensuring dot separators, sort the normalized paths by descending length
so longer prefixes match first, then iterate over those normalizedPaths in the
loop that builds initProviderExclusiveParamKeysConverted (use matchPrefix :=
fmt.Sprintf("%s.", normalizedPath)) and on first match replace the prefix with
fmt.Sprintf("%s0.", matchPrefix) and break the inner loop to avoid duplicate
partial conversions; add table-driven unit tests exercising patterns with "[0]",
"[*]", nested and overlapping paths to ensure one deterministic converted key
per input.


for _, keyToIgnore := range initProviderExclusiveParamKeys {
for attributeKey := range instanceDiff.Attributes {
keyToIgnoreAsPrefix := fmt.Sprintf("%s.", keyToIgnore)
Expand Down Expand Up @@ -451,7 +470,7 @@ func (n *terraformPluginSDKExternal) getResourceDataDiff(tr resource.Terraformed
}

if resourceExists {
if err := filterInitExclusiveDiffs(tr, instanceDiff); err != nil {
if err := filterInitExclusiveDiffs(n.config, tr, instanceDiff); err != nil {
return nil, errors.Wrap(err, "failed to filter the diffs exclusive to spec.initProvider in the terraform.InstanceDiff")
}
}
Expand Down