Custom logic for 3-way-merging null values

pkg/lib/update/common.go

@@ -37,6 +37,10 @@ func PkgHasUpdatedUpstream(local, origin string) (bool, error) {
 		return false, errors.E(op, types.UniquePath(local), err)
 	}
 
+	if originKf.Upstream == nil {
+		return false, nil
+	}
+
 	// If the upstream information in local has changed from origin, it
 	// means the user had updated the package independently and we don't
 	// want to override it.

pkg/lib/update/merge3/merge3_test.go

@@ -158,3 +158,17 @@ func (t *Merge3TestSuite) TestInferAssocList() {
 		t.commonTest(name, tc)
 	}
 }
+
+func (t *Merge3TestSuite) TestNonAssocList() {
+	testCases := map[string]testCase{
+		"non-assoc-crd": {
+			dir:     "non-assoc-crd",
+			crds:    []string{},
+			checkFn: nonAssocListMergeCheck,
+		},
+	}
+
+	for name, tc := range testCases {
+		t.commonTest(name, tc)
+	}
+}

pkg/lib/update/merge3/merge3_util_test.go

@@ -164,6 +164,14 @@ func assocListMergeCheck(t *Merge3TestSuite, kos fn.KubeObjects) {
 	})(t, kos)
 }
 
+func nonAssocListMergeCheck(t *Merge3TestSuite, kos fn.KubeObjects) {
+	makeFruitCheckFunc(10, map[string]int{
+		"apple": 20,
+		"grape": 5,
+		"pear":  30,
+	})(t, kos)
+}
+
 //nolint:unparam
 func makeFruitCheckFunc(expectedTemp int, expectedFruits map[string]int) func(*Merge3TestSuite, fn.KubeObjects) {
 	return func(t *Merge3TestSuite, kos fn.KubeObjects) {

pkg/lib/update/merge3/testdata/non-assoc-crd/destination.yaml

@@ -0,0 +1,32 @@
+apiVersion: infer.kpt.dev/v1alpha1
+kind: MergeTestResources
+metadata:
+  name: "test-resources"
+spec:
+  packageName: "test-package"
+  workspaceName: "v2"
+  revision: 2
+  repositoryName: "test-repo"
+  resources:
+    Kptfile: |
+      apiVersion: kpt.dev/v1
+      kind: Kptfile
+      metadata:
+        name: test-package
+      info:
+        description: test package
+    fruitstore.yaml: |
+      apiVersion: test.group/v1
+      kind: FruitStore
+      metadata:
+        name: test-fruit-store
+      spec:
+        airConditioned: false
+        preferredTemperature: 20
+        fruits:
+        - name: grape
+          amount: 5
+        - name: apple
+          amount: 25
+        - name: banana
+          amount: 3

pkg/lib/update/merge3/testdata/non-assoc-crd/original.yaml

@@ -0,0 +1,30 @@
+apiVersion: infer.kpt.dev/v1alpha1
+kind: MergeTestResources
+metadata:
+  name: "test-resources"
+spec:
+  packageName: "test-package"
+  workspaceName: "v1"
+  revision: 1
+  repositoryName: "test-repo"
+  resources:
+    Kptfile: |
+      apiVersion: kpt.dev/v1
+      kind: Kptfile
+      metadata:
+        name: test-package
+      info:
+        description: test package
+    fruitstore.yaml: |
+      apiVersion: test.group/v1
+      kind: FruitStore
+      metadata:
+        name: test-fruit-store
+      spec:
+        airConditioned: false
+        preferredTemperature: 15
+        fruits:
+        - name: apple
+          amount: 10
+        - name: grape
+          amount: 5

pkg/lib/update/merge3/testdata/non-assoc-crd/updated.yaml

@@ -0,0 +1,32 @@
+apiVersion: infer.kpt.dev/v1alpha1
+kind: MergeTestResources
+metadata:
+  name: "test-resources"
+spec:
+  packageName: "test-package"
+  workspaceName: "v3"
+  revision: 3
+  repositoryName: "test-repo"
+  resources:
+    Kptfile: |
+      apiVersion: kpt.dev/v1
+      kind: Kptfile
+      metadata:
+        name: test-package
+      info:
+        description: test package
+    fruitstore.yaml: |
+      apiVersion: test.group/v1
+      kind: FruitStore
+      metadata:
+        name: test-fruit-store
+      spec:
+        airConditioned: false
+        preferredTemperature: 10
+        fruits:
+        - name: pear
+          amount: 30
+        - name: apple
+          amount: 20
+        - name: grape
+          amount: 5

pkg/lib/update/merge3/tuple.go

@@ -20,7 +20,6 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/kio/filters"
 	"sigs.k8s.io/kustomize/kyaml/kio/kioutil"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
-	"sigs.k8s.io/kustomize/kyaml/yaml/merge3"
 	"sigs.k8s.io/kustomize/kyaml/yaml/walk"
 )
 
@@ -34,8 +33,10 @@ type tuple struct {
 // merge performs a 3-way merge on the tuple
 func (t *tuple) merge() (*yaml.RNode, error) {
 	return walk.Walker{
+		// modified Visitor
+		Visitor: &Visitor{},
+
 		// same as in merge3.Merge()
-		Visitor:            merge3.Visitor{},
 		VisitKeysAsScalars: true,
 		Sources:            []*yaml.RNode{t.dest, t.original, t.updated},
 

pkg/lib/update/merge3/visitor.go

@@ -0,0 +1,145 @@
+// Copyright 2019 The Kubernetes Authors.
+// Copyright 2026 The kpt Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// copied from sigs.k8s.io/kustomize/kyaml@v0.21.1/yaml/merge3/visitor.go with modifications
+
+package merge3
+
+import (
+	"sigs.k8s.io/kustomize/kyaml/openapi"
+	"sigs.k8s.io/kustomize/kyaml/yaml"
+	"sigs.k8s.io/kustomize/kyaml/yaml/walk"
+)
+
+type Visitor struct{}
+
+func (m *Visitor) VisitMap(nodes walk.Sources, _ *openapi.ResourceSchema) (*yaml.RNode, error) {
+	if m.isCleared(nodes.Origin(), nodes.Updated()) || m.isCleared(nodes.Origin(), nodes.Dest()) { // MODIFIED
+		// explicitly cleared from either dest or update
+		return walk.ClearNode, nil
+	}
+	if nodes.Dest() == nil && nodes.Updated() == nil {
+		// implicitly cleared missing from both dest and update
+		return walk.ClearNode, nil
+	}
+
+	if nodes.Dest() == nil {
+		// not cleared, but missing from the dest
+		// initialize a new value that can be recursively merged
+		return yaml.NewRNode(&yaml.Node{Kind: yaml.MappingNode}), nil
+	}
+
+	// recursively merge the dest with the original and updated
+	return nodes.Dest(), nil
+}
+
+func (m *Visitor) visitAList(nodes walk.Sources, _ *openapi.ResourceSchema) (*yaml.RNode, error) {
+	if yaml.IsMissingOrNull(nodes.Updated()) && !yaml.IsMissingOrNull(nodes.Origin()) {
+		// implicitly cleared from update -- element was deleted
+		return walk.ClearNode, nil
+	}
+	if yaml.IsMissingOrNull(nodes.Dest()) {
+		// not cleared, but missing from the dest
+		// initialize a new value that can be recursively merged
+		return yaml.NewRNode(&yaml.Node{Kind: yaml.SequenceNode}), nil
+	}
+
+	// recursively merge the dest with the original and updated
+	return nodes.Dest(), nil
+}
+
+func (m *Visitor) VisitScalar(nodes walk.Sources, _ *openapi.ResourceSchema) (*yaml.RNode, error) {
+	if m.isCleared(nodes.Origin(), nodes.Updated()) || m.isCleared(nodes.Origin(), nodes.Dest()) { // MODIFIED
+		// explicitly cleared from either dest or update
+		return nil, nil
+	}
+	if yaml.IsMissingOrNull(nodes.Updated()) != yaml.IsMissingOrNull(nodes.Origin()) {
+		// value added or removed in update
+		return nodes.Updated(), nil
+	}
+	if yaml.IsMissingOrNull(nodes.Updated()) && yaml.IsMissingOrNull(nodes.Origin()) {
+		// value absent in both origin and update
+		return nodes.Dest(), nil
+	}
+
+	if nodes.Updated().YNode().Value != nodes.Origin().YNode().Value {
+		// value changed in update
+		return nodes.Updated(), nil
+	}
+
+	// unchanged between origin and update, keep the dest
+	return nodes.Dest(), nil
+}
+
+func (m *Visitor) visitNAList(nodes walk.Sources) (*yaml.RNode, error) {
+	if m.isCleared(nodes.Origin(), nodes.Updated()) || m.isCleared(nodes.Origin(), nodes.Dest()) { // MODIFIED
+		// explicitly cleared from either dest or update
+		return walk.ClearNode, nil
+	}
+
+	if yaml.IsMissingOrNull(nodes.Updated()) != yaml.IsMissingOrNull(nodes.Origin()) {
+		// value added or removed in update
+		return nodes.Updated(), nil
+	}
+	if yaml.IsMissingOrNull(nodes.Updated()) && yaml.IsMissingOrNull(nodes.Origin()) {
+		// value not present in source or dest
+		return nodes.Dest(), nil
+	}
+
+	if !m.isNodeContentEqual(nodes.Origin().YNode(), nodes.Updated().YNode()) {
+		// value changed in update
+		return nodes.Updated(), nil
+	}
+
+	// unchanged between origin and update, keep the dest
+	return nodes.Dest(), nil
+}
+
+// NEW
+// isCleared returns if the node has not tagged null in `left` but explicitly removed in `right`
+func (*Visitor) isCleared(left, right *yaml.RNode) bool {
+	return !left.IsTaggedNull() && right.IsTaggedNull()
+}
+
+func (m *Visitor) VisitList(nodes walk.Sources, s *openapi.ResourceSchema, kind walk.ListKind) (*yaml.RNode, error) {
+	if kind == walk.AssociativeList {
+		return m.visitAList(nodes, s)
+	}
+	// non-associative list
+	return m.visitNAList(nodes)
+}
+
+// SIMPLIFIED
+// isNodeContentEqual compares the nodes structurally (kind, tag, value and children),
+// avoiding YAML serialization. Presentation style and comments are ignored.
+func (m *Visitor) isNodeContentEqual(a, b *yaml.Node) bool {
+	if a == nil || b == nil {
+		return a == b
+	}
+	if a.Kind != b.Kind || a.Tag != b.Tag || a.Value != b.Value {
+		return false
+	}
+	if len(a.Content) != len(b.Content) {
+		return false
+	}
+	for i := range a.Content {
+		if !m.isNodeContentEqual(a.Content[i], b.Content[i]) {
+			return false
+		}
+	}
+	return true
+}
+
+var _ walk.Visitor = &Visitor{}

pkg/lib/update/resource-merge_test.go

@@ -16,14 +16,17 @@
 package update_test
 
 import (
+	"os"
 	"path/filepath"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/kptdev/kpt/internal/testutil"
 	"github.com/kptdev/kpt/internal/testutil/pkgbuilder"
 	"github.com/kptdev/kpt/pkg/lib/update"
 	"github.com/kptdev/kpt/pkg/lib/update/updatetypes"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestUpdate_ResourceMerge(t *testing.T) {
@@ -301,3 +304,43 @@ func TestUpdate_ResourceMerge(t *testing.T) {
 		})
 	}
 }
+
+func TestKeepTaggedNull(t *testing.T) {
+	updater := update.ResourceMergeUpdater{}
+
+	testdata, err := filepath.Abs(filepath.Join(".", "testdata", "keep-tagged-null"))
+	require.NoError(t, err)
+
+	tmpDir := t.TempDir()
+	newDest := filepath.Join(tmpDir, "dest")
+	err = os.MkdirAll(newDest, 0755)
+	require.NoError(t, err)
+
+	oldDestFiles, err := os.ReadDir(filepath.Join(testdata, "dest"))
+	require.NoError(t, err)
+	for _, file := range oldDestFiles {
+		if file.Type().IsRegular() {
+			content, err := os.ReadFile(filepath.Join(testdata, "dest", file.Name()))
+			require.NoError(t, err)
+			err = os.WriteFile(filepath.Join(newDest, file.Name()), content, 0644)
+			require.NoError(t, err)
+		}
+	}
+
+	options := updatetypes.Options{
+		OriginPath:  filepath.Join(testdata, "origin"),
+		UpdatedPath: filepath.Join(testdata, "updated"),
+		LocalPath:   newDest,
+	}
+	err = updater.Update(options)
+	require.NoError(t, err)
+
+	destBytes, err := os.ReadFile(filepath.Join(newDest, "configmap.yaml"))
+	require.NoError(t, err)
+	expBytes, err := os.ReadFile(filepath.Join(testdata, "expected", "configmap.yaml"))
+	require.NoError(t, err)
+
+	if diff := cmp.Diff(expBytes, destBytes); diff != "" {
+		t.Errorf("unexpected result (-want, +got): %s", diff)
+	}
+}

pkg/lib/update/testdata/keep-tagged-null/dest/Kptfile

@@ -0,0 +1,21 @@
+apiVersion: kpt.dev/v1
+kind: Kptfile
+metadata:
+  name: test
+  annotations:
+    config.kubernetes.io/local-config: "true"
+upstream:
+  type: git
+  git:
+    repo: https://github.com/droot/pkg-catalog.git
+    directory: basens
+    ref: basens/v0
+upstreamLock:
+  type: git
+  git:
+    repo: https://github.com/droot/pkg-catalog.git
+    directory: basens
+    ref: basens/v0
+    commit: b3e1d439516a5e8d49adc0c82d3e95578570dbfa
+info:
+  description: Test package.