Fix YAML replacement affecting wrong fields with same value

internal/files/yaml.go

@@ -82,8 +82,11 @@ func UpdateYAMLFile(cfg VersionFileConfig, currentVersion, newVersion string) er
 		newValue = newVersionStr
 	}
 
-	// Perform surgical replacement - find and replace only the value
-	newData, err := surgicalReplace(data, oldValue, newValue)
+	// Extract the key name from the path for targeted replacement
+	key := extractKeyFromPath(cfg.Path)
+
+	// Perform surgical replacement - find and replace only the value for this specific key
+	newData, err := surgicalReplace(data, key, oldValue, newValue)
 	if err != nil {
 		return fmt.Errorf("replacing value at path %s: %w", cfg.Path, err)
 	}
@@ -96,84 +99,111 @@ func UpdateYAMLFile(cfg VersionFileConfig, currentVersion, newVersion string) er
 	return nil
 }
 
+// extractKeyFromPath extracts the final key name from a dot-notation path.
+// Examples:
+//
+//	"version" -> "version"
+//	"metadata.version" -> "version"
+//	"spec.containers[0].image" -> "image"
+//	"operator.image" -> "image"
+func extractKeyFromPath(path string) string {
+	// Remove any array indices from the path
+	re := regexp.MustCompile(`\[\d+\]`)
+	cleanPath := re.ReplaceAllString(path, "")
+
+	// Get the last component after splitting by dots
+	parts := strings.Split(cleanPath, ".")
+	if len(parts) == 0 {
+		return path
+	}
+	return parts[len(parts)-1]
+}
+
 // replacementRule defines a pattern-replacement pair for surgical YAML value replacement.
 // Each rule targets a specific quote style or value format in YAML files.
 type replacementRule struct {
 	// name describes what this rule handles (for debugging/documentation)
 	name string
-	// pattern returns the regex pattern to match for the given old value
-	pattern func(oldValue string) string
-	// replacement returns the replacement string for the given new value
-	replacement func(newValue string) string
+	// pattern returns the regex pattern to match for the given key and old value.
+	// The key is included to ensure we only match the specific YAML key we're updating.
+	pattern func(key, oldValue string) string
+	// replacement returns the replacement string for the given key and new value
+	replacement func(key, newValue string) string
 }
 
 // replacementRules defines all the rules for surgical YAML value replacement.
 // Rules are tried in order; the first matching rule is applied.
+// Each pattern includes the key name to ensure we only replace the value for the
+// specific YAML key being updated, not other keys with the same value.
 var replacementRules = []replacementRule{
 	{
 		// Handles double-quoted values: key: "value"
 		name: "double-quoted",
-		pattern: func(oldValue string) string {
-			return fmt.Sprintf(`"(%s)"`, regexp.QuoteMeta(oldValue))
+		pattern: func(key, oldValue string) string {
+			return fmt.Sprintf(`(%s:\s*)"(%s)"`, regexp.QuoteMeta(key), regexp.QuoteMeta(oldValue))
 		},
-		replacement: func(newValue string) string {
-			return fmt.Sprintf(`"%s"`, newValue)
+		replacement: func(_, newValue string) string {
+			// Use ${1} syntax to avoid ambiguity when newValue starts with a digit
+			return fmt.Sprintf(`${1}"%s"`, newValue)
 		},
 	},
 	{
 		// Handles single-quoted values: key: 'value'
 		name: "single-quoted",
-		pattern: func(oldValue string) string {
-			return fmt.Sprintf(`'(%s)'`, regexp.QuoteMeta(oldValue))
+		pattern: func(key, oldValue string) string {
+			return fmt.Sprintf(`(%s:\s*)'(%s)'`, regexp.QuoteMeta(key), regexp.QuoteMeta(oldValue))
 		},
-		replacement: func(newValue string) string {
-			return fmt.Sprintf(`'%s'`, newValue)
+		replacement: func(_, newValue string) string {
+			return fmt.Sprintf(`${1}'%s'`, newValue)
 		},
 	},
 	{
 		// Handles unquoted values at end of line: key: value\n
 		name: "unquoted-eol",
-		pattern: func(oldValue string) string {
-			return fmt.Sprintf(`: (%s)(\s*)$`, regexp.QuoteMeta(oldValue))
+		pattern: func(key, oldValue string) string {
+			return fmt.Sprintf(`(%s:\s*)(%s)(\s*)$`, regexp.QuoteMeta(key), regexp.QuoteMeta(oldValue))
 		},
-		replacement: func(newValue string) string {
-			return fmt.Sprintf(`: %s$2`, newValue)
+		replacement: func(_, newValue string) string {
+			return fmt.Sprintf(`${1}%s${3}`, newValue)
 		},
 	},
 	{
 		// Handles unquoted values followed by inline comment: key: value # comment
 		name: "unquoted-with-comment",
-		pattern: func(oldValue string) string {
-			return fmt.Sprintf(`: (%s)(\s*#)`, regexp.QuoteMeta(oldValue))
+		pattern: func(key, oldValue string) string {
+			return fmt.Sprintf(`(%s:\s*)(%s)(\s*#)`, regexp.QuoteMeta(key), regexp.QuoteMeta(oldValue))
 		},
-		replacement: func(newValue string) string {
-			return fmt.Sprintf(`: %s$2`, newValue)
+		replacement: func(_, newValue string) string {
+			return fmt.Sprintf(`${1}%s${3}`, newValue)
 		},
 	},
 }
 
 // surgicalReplace performs a targeted replacement of a YAML value while preserving
-// the original formatting (quotes, whitespace, etc.)
-func surgicalReplace(data []byte, oldValue, newValue string) ([]byte, error) {
+// the original formatting (quotes, whitespace, etc.). The key parameter ensures
+// we only replace the value for the specific YAML key being updated.
+func surgicalReplace(data []byte, key, oldValue, newValue string) ([]byte, error) {
 	content := string(data)
 
 	// Try each replacement rule in order; use the first one that matches
 	for _, rule := range replacementRules {
-		pattern := rule.pattern(oldValue)
+		pattern := rule.pattern(key, oldValue)
 		re := regexp.MustCompile(`(?m)` + pattern)
 		if re.MatchString(content) {
-			result := re.ReplaceAllString(content, rule.replacement(newValue))
+			result := re.ReplaceAllString(content, rule.replacement(key, newValue))
 			return []byte(result), nil
 		}
 	}
 
-	// Fallback: simple string replacement if no pattern matched
-	if strings.Contains(content, oldValue) {
-		result := strings.Replace(content, oldValue, newValue, 1)
+	// Fallback: key-aware simple string replacement if no pattern matched
+	// Look for "key: oldValue" or "key:oldValue" patterns
+	keyPattern := regexp.MustCompile(fmt.Sprintf(`(%s:\s*)%s`, regexp.QuoteMeta(key), regexp.QuoteMeta(oldValue)))
+	if keyPattern.MatchString(content) {
+		result := keyPattern.ReplaceAllString(content, fmt.Sprintf(`${1}%s`, newValue))
 		return []byte(result), nil
 	}
 
-	return nil, fmt.Errorf("could not find value %q to replace", oldValue)
+	return nil, fmt.Errorf("could not find value %q for key %q to replace", oldValue, key)
 }
 
 // findEmbeddedVersion looks for a version pattern in the value and returns it if found.

internal/files/yaml_test.go

@@ -251,6 +251,34 @@ func TestConvertToYAMLPath(t *testing.T) {
 	}
 }
 
+func TestExtractKeyFromPath(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		input string
+		want  string
+	}{
+		{"version", "version"},
+		{"appVersion", "appVersion"},
+		{"metadata.version", "version"},
+		{"spec.template.spec.image.tag", "tag"},
+		{"operator.image", "image"},
+		{"containers[0].image", "image"},
+		{"spec.containers[0].image", "image"},
+		{"data[0]", "data"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			t.Parallel()
+			got := extractKeyFromPath(tt.input)
+			if got != tt.want {
+				t.Errorf("extractKeyFromPath(%q) = %q, want %q", tt.input, got, tt.want)
+			}
+		})
+	}
+}
+
 func TestUpdateYAMLFile_InvalidPath(t *testing.T) {
 	t.Parallel()
 
@@ -607,3 +635,115 @@ app:
 		})
 	}
 }
+
+// TestUpdateYAMLFile_SameValueDifferentKeys tests that updating one field does not
+// accidentally modify another field with the same value but different formatting.
+// This was a bug where updating "version: 0.9.0" would also modify "appVersion: "0.9.0""
+// because the replacement was not key-aware.
+func TestUpdateYAMLFile_SameValueDifferentKeys(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name           string
+		input          string
+		config         VersionFileConfig
+		currentVersion string
+		newVersion     string
+		wantContains   []string
+		wantNotContain []string
+	}{
+		{
+			name: "update unquoted version without affecting quoted appVersion",
+			input: `apiVersion: v2
+name: test-chart
+version: 0.9.0
+appVersion: "0.9.0"
+`,
+			config:         VersionFileConfig{Path: "version"},
+			currentVersion: "0.9.0",
+			newVersion:     "0.9.1",
+			wantContains: []string{
+				"version: 0.9.1",
+				`appVersion: "0.9.0"`, // appVersion should NOT be changed
+			},
+			wantNotContain: []string{
+				`appVersion: "0.9.1"`, // should NOT happen
+			},
+		},
+		{
+			name: "update quoted appVersion without affecting unquoted version",
+			input: `apiVersion: v2
+name: test-chart
+version: 0.9.0
+appVersion: "0.9.0"
+`,
+			config:         VersionFileConfig{Path: "appVersion"},
+			currentVersion: "0.9.0",
+			newVersion:     "0.9.1",
+			wantContains: []string{
+				"version: 0.9.0", // version should NOT be changed
+				`appVersion: "0.9.1"`,
+			},
+			wantNotContain: []string{
+				"version: 0.9.1", // should NOT happen
+			},
+		},
+		{
+			name: "sequential updates to different keys with same value",
+			input: `apiVersion: v2
+name: operator-crds
+version: 0.9.0
+appVersion: "0.9.0"
+`,
+			config:         VersionFileConfig{Path: "version"},
+			currentVersion: "0.9.0",
+			newVersion:     "0.9.1",
+			wantContains: []string{
+				"version: 0.9.1",
+				`appVersion: "0.9.0"`,
+			},
+		},
+		{
+			name: "different keys same value - nested paths",
+			input: `chart:
+  version: 0.9.0
+  appVersion: "0.9.0"
+  description: A test chart
+`,
+			config:         VersionFileConfig{Path: "chart.version"},
+			currentVersion: "0.9.0",
+			newVersion:     "0.9.1",
+			wantContains: []string{
+				"version: 0.9.1",
+				`appVersion: "0.9.0"`, // appVersion should NOT be changed
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			tmpPath := createTempFile(t, tt.input, "test*.yaml")
+			cfg := tt.config
+			cfg.File = tmpPath
+
+			err := UpdateYAMLFile(cfg, tt.currentVersion, tt.newVersion)
+			if err != nil {
+				t.Fatalf("UpdateYAMLFile() error = %v", err)
+			}
+
+			got := readTempFile(t, tmpPath)
+			for _, want := range tt.wantContains {
+				if !strings.Contains(got, want) {
+					t.Errorf("UpdateYAMLFile() should contain %q, got:\n%s", want, got)
+				}
+			}
+			for _, notWant := range tt.wantNotContain {
+				if strings.Contains(got, notWant) {
+					t.Errorf("UpdateYAMLFile() should NOT contain %q, got:\n%s", notWant, got)
+				}
+			}
+		})
+	}
+}