Add KubeObject setters that try to keep YAML comments and the order o…

…f fields while updating big chunks of a KubeObject

.gitignore

@@ -17,3 +17,4 @@
 # Tox environments
 .tox/
 *.dic
+/.vscode

krm-functions/lib/kubeobject/kubeobject.go

@@ -18,8 +18,11 @@ package kubeobject
 
 import (
 	"fmt"
+	"reflect"
+	"unsafe"
 
 	"github.com/GoogleContainerTools/kpt-functions-sdk/go/fn"
+	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
 type KubeObjectExt[T1 any] struct {
@@ -104,3 +107,161 @@ func NewFromGoStruct[T1 any](x any) (*KubeObjectExt[T1], error) {
 	}
 	return NewFromKubeObject[T1](o)
 }
+
+// NOTE: the following functions are considered as "methods" of KubeObject,
+// and thus nill checking of `obj` was omitted intentionally:
+// the caller is responsible for ensuring that `obj` is not nil`
+
+// ToStruct converts the KubeObject to a go struct with type `T`
+func ToStruct[T any](obj *fn.KubeObject) (T, error) {
+	var x T
+	err := obj.As(&x)
+	return x, err
+}
+
+// GetSpec returns with the `spec` field of the KubeObject as a go struct with type `T`
+// NOTE: consider using ToStruct() instead
+func GetSpec[T any](obj *fn.KubeObject) (T, error) {
+	var spec T
+	err := obj.UpsertMap("spec").As(&spec)
+	return spec, err
+
+}
+
+// GetStatus returns with the `status` field of the KubeObject as a go struct with type `T`
+// NOTE: consider using ToStruct() instead
+func GetStatus[T any](obj *fn.KubeObject) (T, error) {
+	var status T
+	err := obj.UpsertMap("status").As(&status)
+	return status, err
+
+}
+
+// SetSpec sets the `spec` field of a KubeObject to the value of `newSpec`,
+// while trying to keep as much formatting as possible
+func SetSpec(obj *fn.KubeObject, newSpec interface{}) error {
+	return SetNestedFieldKeepFormatting(&obj.SubObject, newSpec, "spec")
+}
+
+// SetStatus sets the `status` field of a KubeObject to the value of `newStatus`,
+// while trying to keep as much formatting as possible
+func SetStatus(obj *fn.KubeObject, newStatus interface{}) error {
+	return SetNestedFieldKeepFormatting(&obj.SubObject, newStatus, "status")
+}
+
+// SetNestedFieldKeepFormatting is similar to KubeObject.SetNestedField(), but keeps the
+// comments and the order of fields in the YAML wherever it is possible.
+//
+// NOTE: This functionality should be solved in the upstream SDK.
+// Merging the code below to the upstream SDK is in progress and tracked in this issue:
+// https://github.com/GoogleContainerTools/kpt/issues/3923
+func SetNestedFieldKeepFormatting(obj *fn.SubObject, value interface{}, field string) error {
+	oldNode := yamlNodeOf(obj.UpsertMap(field))
+	err := obj.SetNestedField(value, field)
+	if err != nil {
+		return err
+	}
+	newNode := yamlNodeOf(obj.GetMap(field))
+
+	restoreFieldOrder(oldNode, newNode)
+	deepCopyComments(oldNode, newNode)
+	return nil
+}
+
+///////////////// internals
+
+func shallowCopyComments(src, dst *yaml.Node) {
+	dst.HeadComment = src.HeadComment
+	dst.LineComment = src.LineComment
+	dst.FootComment = src.FootComment
+}
+
+func deepCopyComments(src, dst *yaml.Node) {
+	if src.Kind != dst.Kind {
+		return
+	}
+	shallowCopyComments(src, dst)
+	if dst.Kind == yaml.MappingNode {
+		if (len(src.Content)%2 != 0) || (len(dst.Content)%2 != 0) {
+			panic("unexpected number of children for YAML map")
+		}
+		for i := 0; i < len(dst.Content); i += 2 {
+			dstKeyNode := dst.Content[i]
+			key, ok := asString(dstKeyNode)
+			if !ok {
+				continue
+			}
+
+			j, ok := findKey(src, key)
+			if !ok {
+				continue
+			}
+			srcKeyNode, srcValueNode := src.Content[j], src.Content[j+1]
+			dstValueNode := dst.Content[i+1]
+			shallowCopyComments(srcKeyNode, dstKeyNode)
+			deepCopyComments(srcValueNode, dstValueNode)
+		}
+	}
+}
+
+func restoreFieldOrder(src, dst *yaml.Node) {
+	if (src.Kind != dst.Kind) || (dst.Kind != yaml.MappingNode) {
+		return
+	}
+	if (len(src.Content)%2 != 0) || (len(dst.Content)%2 != 0) {
+		panic("unexpected number of children for YAML map")
+	}
+
+	nextInDst := 0
+	for i := 0; i < len(src.Content); i += 2 {
+		key, ok := asString(src.Content[i])
+		if !ok {
+			continue
+		}
+
+		j, ok := findKey(dst, key)
+		if !ok {
+			continue
+		}
+		if j != nextInDst {
+			dst.Content[j], dst.Content[nextInDst] = dst.Content[nextInDst], dst.Content[j]
+			dst.Content[j+1], dst.Content[nextInDst+1] = dst.Content[nextInDst+1], dst.Content[j+1]
+		}
+		nextInDst += 2
+
+		srcValueNode := src.Content[i+1]
+		dstValueNode := dst.Content[nextInDst-1]
+		restoreFieldOrder(srcValueNode, dstValueNode)
+	}
+}
+
+func asString(node *yaml.Node) (string, bool) {
+	if node.Kind == yaml.ScalarNode && (node.Tag == "!!str" || node.Tag == "") {
+		return node.Value, true
+	}
+	return "", false
+}
+
+func findKey(m *yaml.Node, key string) (int, bool) {
+	children := m.Content
+	if len(children)%2 != 0 {
+		panic("unexpected number of children for YAML map")
+	}
+	for i := 0; i < len(children); i += 2 {
+		keyNode := children[i]
+		k, ok := asString(keyNode)
+		if ok && k == key {
+			return i, true
+		}
+	}
+	return 0, false
+}
+
+// This is a temporary workaround until SetNestedFieldKeppFormatting functionality is merged into the upstream SDK
+// The merge process has already started and tracked in this issue: https://github.com/GoogleContainerTools/kpt/issues/3923
+func yamlNodeOf(obj *fn.SubObject) *yaml.Node {
+	internalObj := reflect.ValueOf(*obj).FieldByName("obj")
+	nodePtr := internalObj.Elem().FieldByName("node")
+	nodePtr = reflect.NewAt(nodePtr.Type(), unsafe.Pointer(nodePtr.UnsafeAddr())).Elem()
+	return nodePtr.Interface().(*yaml.Node)
+}

krm-functions/lib/kubeobject/kubeobject_test.go

@@ -17,10 +17,15 @@ limitations under the License.
 package kubeobject
 
 import (
+	"os"
+	"strings"
 	"testing"
 
 	"github.com/GoogleContainerTools/kpt-functions-sdk/go/fn"
-	v1 "k8s.io/api/apps/v1"
+	"github.com/GoogleContainerTools/kpt-functions-sdk/go/fn/testhelpers"
+	testlib "github.com/nephio-project/nephio/krm-functions/lib/test"
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/yaml"
 )
@@ -93,7 +98,7 @@ func TestNewFromKubeObject(t *testing.T) {
 		},
 	}
 	for _, tt := range testItems {
-		deploymentReceived := v1.Deployment{
+		deploymentReceived := appsv1.Deployment{
 			TypeMeta: metav1.TypeMeta{
 				APIVersion: tt.input.gv,
 				Kind:       tt.input.kind,
@@ -102,7 +107,7 @@ func TestNewFromKubeObject(t *testing.T) {
 				Name:      tt.input.name,
 				Namespace: tt.input.namespace,
 			},
-			Spec: v1.DeploymentSpec{
+			Spec: appsv1.DeploymentSpec{
 				Replicas: &tt.input.replicas,
 				Paused:   tt.input.paused,
 				Selector: &metav1.LabelSelector{
@@ -115,7 +120,7 @@ func TestNewFromKubeObject(t *testing.T) {
 			t.Errorf("YAML Marshal error: %s", err)
 		}
 		deploymentKubeObject, _ := fn.ParseKubeObject(b)
-		deploymentKubeObjectParser, _ := NewFromKubeObject[v1.Deployment](deploymentKubeObject)
+		deploymentKubeObjectParser, _ := NewFromKubeObject[appsv1.Deployment](deploymentKubeObject)
 		if deploymentKubeObjectParser.SubObject != deploymentKubeObject.SubObject {
 			t.Errorf("-want%s, +got:\n%s", deploymentKubeObjectParser.String(), deploymentKubeObject.String())
 		}
@@ -198,7 +203,7 @@ func TestNewFromYaml(t *testing.T) {
 		},
 	}
 	for _, tt := range testItems {
-		deploymentReceived := v1.Deployment{
+		deploymentReceived := appsv1.Deployment{
 			TypeMeta: metav1.TypeMeta{
 				APIVersion: tt.input.gv,
 				Kind:       tt.input.kind,
@@ -207,7 +212,7 @@ func TestNewFromYaml(t *testing.T) {
 				Name:      tt.input.name,
 				Namespace: tt.input.namespace,
 			},
-			Spec: v1.DeploymentSpec{
+			Spec: appsv1.DeploymentSpec{
 				Replicas: &tt.input.replicas,
 				Paused:   tt.input.paused,
 				Selector: &metav1.LabelSelector{
@@ -219,7 +224,7 @@ func TestNewFromYaml(t *testing.T) {
 		if err != nil {
 			t.Errorf("YAML Marshal error: %s", err)
 		}
-		deploymentKubeObjectParser, _ := NewFromYaml[v1.Deployment](b)
+		deploymentKubeObjectParser, _ := NewFromYaml[appsv1.Deployment](b)
 
 		if deploymentKubeObjectParser.String() != string(b) {
 			t.Errorf("-want%s, +got:\n%s", string(b), deploymentKubeObjectParser.String())
@@ -303,7 +308,7 @@ func TestNewFromGoStruct(t *testing.T) {
 		},
 	}
 	for _, tt := range testItems {
-		deploymentReceived := v1.Deployment{
+		deploymentReceived := appsv1.Deployment{
 			TypeMeta: metav1.TypeMeta{
 				APIVersion: tt.input.gv,
 				Kind:       tt.input.kind,
@@ -312,15 +317,15 @@ func TestNewFromGoStruct(t *testing.T) {
 				Name:      tt.input.name,
 				Namespace: tt.input.namespace,
 			},
-			Spec: v1.DeploymentSpec{
+			Spec: appsv1.DeploymentSpec{
 				Replicas: &tt.input.replicas,
 				Paused:   tt.input.paused,
 				Selector: &metav1.LabelSelector{
 					MatchLabels: tt.input.selector,
 				},
 			},
 		}
-		deploymentKubeObjectParser, _ := NewFromGoStruct[v1.Deployment](deploymentReceived)
+		deploymentKubeObjectParser, _ := NewFromGoStruct[appsv1.Deployment](deploymentReceived)
 
 		s, _, err := deploymentKubeObjectParser.NestedString([]string{"metadata", "name"}...)
 		if err != nil {
@@ -331,3 +336,88 @@ func TestNewFromGoStruct(t *testing.T) {
 		}
 	}
 }
+
+func compareKubeObjectWithExpectedYaml(t *testing.T, obj *fn.KubeObject, inputFile string) {
+	actualYAML := strings.TrimSpace(obj.String())
+	expectedFile := testlib.InsertBeforeExtension(inputFile, "_expected")
+	expectedYAML := strings.TrimSpace(string(testhelpers.MustReadFile(t, expectedFile)))
+
+	if actualYAML != expectedYAML {
+		t.Errorf(`mismatch in expected and actual KubeObject YAML:
+--- want: -----
+%v
+--- got: ----
+%v
+----------------`, expectedYAML, actualYAML)
+		os.WriteFile(testlib.InsertBeforeExtension(inputFile, "_actual"), []byte(actualYAML), 0666)
+	}
+
+}
+
+func TestSetNestedFieldKeepFormatting(t *testing.T) {
+	testfiles := []string{"testdata/comments.yaml"}
+	for _, inputFile := range testfiles {
+		t.Run(inputFile, func(t *testing.T) {
+			obj := testlib.MustParseKubeObject(t, inputFile)
+
+			deploy, err := ToStruct[appsv1.Deployment](obj)
+			if err != nil {
+				t.Errorf("unexpected error in ToStruct[v1.Deployment]: %v", err)
+			}
+			deploy.Spec.Replicas = nil                                              // delete Replicas field if present
+			deploy.Spec.Template.Spec.RestartPolicy = corev1.RestartPolicyOnFailure // update field value
+			err = SetNestedFieldKeepFormatting(&obj.SubObject, deploy.Spec, "spec")
+			if err != nil {
+				t.Errorf("unexpected error in SetNestedFieldKeepFormatting: %v", err)
+			}
+
+			compareKubeObjectWithExpectedYaml(t, obj, inputFile)
+		})
+	}
+}
+
+func TestSetSpec(t *testing.T) {
+	testfiles := []string{"testdata/comments.yaml"}
+	for _, inputFile := range testfiles {
+		t.Run(inputFile, func(t *testing.T) {
+			obj := testlib.MustParseKubeObject(t, inputFile)
+
+			spec, err := GetSpec[appsv1.DeploymentSpec](obj)
+			if err != nil {
+				t.Errorf("unexpected error in GetSpec: %v", err)
+			}
+			spec.Replicas = nil                                              // delete Replicas field if present
+			spec.Template.Spec.RestartPolicy = corev1.RestartPolicyOnFailure // update field value
+			err = SetSpec(obj, spec)
+			if err != nil {
+				t.Errorf("unexpected error in SetSpec: %v", err)
+			}
+
+			compareKubeObjectWithExpectedYaml(t, obj, inputFile)
+		})
+	}
+}
+
+func TestSetStatus(t *testing.T) {
+	testfiles := []string{
+		"testdata/status_comments.yaml",
+		"testdata/empty_status.yaml",
+	}
+	for _, inputFile := range testfiles {
+		t.Run(inputFile, func(t *testing.T) {
+			obj := testlib.MustParseKubeObject(t, inputFile)
+
+			status, err := GetStatus[appsv1.DeploymentStatus](obj)
+			if err != nil {
+				t.Errorf("unexpected error in GetStatus: %v", err)
+			}
+			status.AvailableReplicas = 0
+			err = SetStatus(obj, status)
+			if err != nil {
+				t.Errorf("unexpected error in SetStatus: %v", err)
+			}
+
+			compareKubeObjectWithExpectedYaml(t, obj, inputFile)
+		})
+	}
+}

krm-functions/lib/kubeobject/testdata/.gitignore

@@ -0,0 +1 @@
+/*_actual.yaml