Unmarshal with k8s.io/apimachinery/pkg/util/yaml

- apimachinery yaml.Unmarshal fixes number types to int64 and float64
- Fix inaccurate error message in mutator
- Move YamlStringer to object pkg for reuse by other pkgs
- Fix jsonpath tests parsing y as a bool
- Fix kstatus example_test.go to actually be a test

pkg/apply/mutator/apply_time_mutator.go

@@ -17,8 +17,8 @@ import (
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/cli-utils/pkg/apply/cache"
 	"sigs.k8s.io/cli-utils/pkg/jsonpath"
+	"sigs.k8s.io/cli-utils/pkg/object"
 	"sigs.k8s.io/cli-utils/pkg/object/mutation"
-	"sigs.k8s.io/yaml"
 )
 
 // ApplyTimeMutator mutates a resource by injecting values specified by the
@@ -52,10 +52,10 @@ func (atm *ApplyTimeMutator) Mutate(ctx context.Context, obj *unstructured.Unstr
 
 	subs, err := mutation.ReadAnnotation(obj)
 	if err != nil {
-		return mutated, reason, fmt.Errorf("failed to read jsonpath field in target resource (%s): %w", targetRef, err)
+		return mutated, reason, fmt.Errorf("failed to read annotation in resource (%s): %w", targetRef, err)
 	}
 
-	klog.V(4).Infof("target resource (%v):\n%s", targetRef, yamlStringer{obj})
+	klog.V(4).Infof("target resource (%s):\n%s", targetRef, object.YamlStringer{O: obj})
 
 	// validate no self-references
 	// Early validation to avoid GETs, but won't catch sources with implicit namespace.
@@ -91,7 +91,7 @@ func (atm *ApplyTimeMutator) Mutate(ctx context.Context, obj *unstructured.Unstr
 			return mutated, reason, fmt.Errorf("failed to get source resource (%s): %w", sourceRef, err)
 		}
 
-		klog.V(4).Infof("source resource (%s):\n%s", targetRef, yamlStringer{sourceObj})
+		klog.V(4).Infof("source resource (%s):\n%s", sourceRef, object.YamlStringer{O: sourceObj})
 
 		// lookup target field in target resource
 		targetValue, _, err := readFieldValue(obj, sub.TargetPath)
@@ -129,8 +129,8 @@ func (atm *ApplyTimeMutator) Mutate(ctx context.Context, obj *unstructured.Unstr
 			newValue = strings.ReplaceAll(targetValueString, sub.Token, sourceValueString)
 		}
 
-		klog.V(5).Infof("substitution on (%v): source=(%s), token=(%s), old=(%s), new=(%s)",
-			targetRef, sourceValue, sub.Token, targetValue, newValue)
+		klog.V(5).Infof("substitution: targetRef=(%s), sourceRef=(%s): sourceValue=(%v), token=(%s), oldTargetValue=(%v), newTargetValue=(%v)",
+			targetRef, sourceRef, sourceValue, sub.Token, targetValue, newValue)
 
 		// update target field in target resource
 		err = writeFieldValue(obj, sub.TargetPath, newValue)
@@ -143,7 +143,7 @@ func (atm *ApplyTimeMutator) Mutate(ctx context.Context, obj *unstructured.Unstr
 	}
 
 	if mutated {
-		klog.V(4).Infof("mutated target resource (%s):\n%s", targetRef, yamlStringer{obj})
+		klog.V(4).Infof("mutated target resource (%s):\n%s", targetRef, object.YamlStringer{O: obj})
 	}
 
 	return mutated, reason, nil
@@ -245,19 +245,3 @@ func valueToString(value interface{}) (string, error) {
 	}
 	return valueString, nil
 }
-
-// yamlStringer delays YAML marshalling for logging until String() is called.
-type yamlStringer struct {
-	obj *unstructured.Unstructured
-}
-
-// String marshals the wrapped object to a YAML string. If serializing errors,
-// the error string will be returned instead. This is primarily for use with
-// verbose multi-line logging.
-func (ys yamlStringer) String() string {
-	yamlBytes, err := yaml.Marshal(ys.obj.Object)
-	if err != nil {
-		return fmt.Sprintf("failed to serialize as yaml: %s", err)
-	}
-	return string(yamlBytes)
-}

pkg/apply/mutator/apply_time_mutator_test.go

@@ -409,7 +409,7 @@ func TestMutate(t *testing.T) {
 			mutated: false,
 			reason:  "",
 			// exact error message isn't very important. Feel free to update if the error text changes.
-			errMsg: `failed to read jsonpath field in target resource (v1/namespaces/map-namespace/ConfigMap/map3-name): ` +
+			errMsg: `failed to read annotation in resource (v1/namespaces/map-namespace/ConfigMap/map3-name): ` +
 				`failed to parse apply-time-mutation annotation: "not a valid substitution list": ` +
 				`error unmarshaling JSON: ` +
 				`while decoding JSON: ` +

pkg/jsonpath/jsonpath_test.go

@@ -36,7 +36,7 @@ entries:
 - name: a
   value: x
 - name: b
-  value: y
+  value: "y"
 - name: c
   value: z
 `
@@ -65,7 +65,7 @@ entries:
 - name: a
   value: x
 - name: b
-  value: y
+  value: "y"
 - name: c
   value: z
 `

pkg/kstatus/polling/testutil/testing.go

@@ -7,9 +7,9 @@ import (
 	"context"
 	"testing"
 
-	"gopkg.in/yaml.v3"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/util/yaml"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 

pkg/kstatus/status/example_test.go

@@ -4,15 +4,16 @@
 package status_test
 
 import (
-	"fmt"
-	"log"
+	"strings"
+	"testing"
 
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	. "sigs.k8s.io/cli-utils/pkg/kstatus/status"
+	"github.com/stretchr/testify/assert"
+	"sigs.k8s.io/cli-utils/pkg/kstatus/polling/testutil"
+	"sigs.k8s.io/cli-utils/pkg/kstatus/status"
 	"sigs.k8s.io/yaml"
 )
 
-func ExampleCompute() {
+func TestExampleCompute(t *testing.T) {
 	deploymentManifest := `
 apiVersion: apps/v1
 kind: Deployment
@@ -27,24 +28,21 @@ status:
    availableReplicas: 1
    replicas: 1
    conditions:
-    - type: Progressing 
+    - type: Progressing
       status: "True"
       reason: NewReplicaSetAvailable
-    - type: Available 
+    - type: Available
       status: "True"
 `
-	deployment := yamlManifestToUnstructured(deploymentManifest)
+	deployment := testutil.YamlToUnstructured(t, deploymentManifest)
 
-	res, err := Compute(deployment)
-	if err != nil {
-		log.Fatal(err)
-	}
-	fmt.Println(res.Status)
-	// Output:
-	// Current
+	res, err := status.Compute(deployment)
+	assert.NoError(t, err)
+
+	assert.Equal(t, status.Status("Current"), res.Status)
 }
 
-func ExampleAugment() {
+func TestExampleAugment(t *testing.T) {
 	deploymentManifest := `
 apiVersion: apps/v1
 kind: Deployment
@@ -59,52 +57,41 @@ status:
    availableReplicas: 1
    replicas: 1
    conditions:
-    - type: Progressing 
+    - type: Progressing
       status: "True"
       reason: NewReplicaSetAvailable
-    - type: Available 
+    - type: Available
       status: "True"
 `
-	deployment := yamlManifestToUnstructured(deploymentManifest)
+	deployment := testutil.YamlToUnstructured(t, deploymentManifest)
+
+	err := status.Augment(deployment)
+	assert.NoError(t, err)
 
-	err := Augment(deployment)
-	if err != nil {
-		log.Fatal(err)
-	}
 	b, err := yaml.Marshal(deployment.Object)
-	if err != nil {
-		log.Fatal(err)
-	}
-	fmt.Println(string(b))
-	// Output:
-	// apiVersion: apps/v1
-	// kind: Deployment
-	// metadata:
-	//   generation: 1
-	//   name: test
-	//   namespace: qual
-	// status:
-	//   availableReplicas: 1
-	//   conditions:
-	//   - reason: NewReplicaSetAvailable
-	//     status: "True"
-	//     type: Progressing
-	//   - status: "True"
-	//     type: Available
-	//   observedGeneration: 1
-	//   readyReplicas: 1
-	//   replicas: 1
-	//   updatedReplicas: 1
-}
+	assert.NoError(t, err)
 
-func yamlManifestToUnstructured(manifest string) *unstructured.Unstructured {
-	jsonManifest, err := yaml.YAMLToJSON([]byte(manifest))
-	if err != nil {
-		log.Fatal(err)
-	}
-	resource, _, err := unstructured.UnstructuredJSONScheme.Decode(jsonManifest, nil, nil)
-	if err != nil {
-		log.Fatal(err)
-	}
-	return resource.(*unstructured.Unstructured)
+	receivedManifest := strings.TrimSpace(string(b))
+	expectedManifest := strings.TrimSpace(`
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  generation: 1
+  name: test
+  namespace: qual
+status:
+  availableReplicas: 1
+  conditions:
+  - reason: NewReplicaSetAvailable
+    status: "True"
+    type: Progressing
+  - status: "True"
+    type: Available
+  observedGeneration: 1
+  readyReplicas: 1
+  replicas: 1
+  updatedReplicas: 1
+`)
+
+	assert.Equal(t, expectedManifest, receivedManifest)
 }

pkg/object/strings.go

@@ -0,0 +1,35 @@
+// Copyright 2020 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package object
+
+import (
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/kubectl/pkg/scheme"
+	"sigs.k8s.io/yaml"
+)
+
+var codec = scheme.Codecs.LegacyCodec(scheme.Scheme.PrioritizedVersionsAllGroups()...)
+
+// YamlStringer delays YAML marshalling for logging until String() is called.
+type YamlStringer struct {
+	O runtime.Object
+}
+
+// String marshals the wrapped object to a YAML string. If serializing errors,
+// the error string will be returned instead. This is primarily for use with
+// verbose logging.
+func (ys YamlStringer) String() string {
+	jsonBytes, err := runtime.Encode(unstructured.NewJSONFallbackEncoder(codec), ys.O)
+	if err != nil {
+		return fmt.Sprintf("<<failed to serialize as json: %s>>", err)
+	}
+	yamlBytes, err := yaml.JSONToYAML(jsonBytes)
+	if err != nil {
+		return fmt.Sprintf("<<failed to convert from json to yaml: %s>>", err)
+	}
+	return string(yamlBytes)
+}

test/e2e/common_test.go

@@ -13,9 +13,9 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/yaml"
 	"sigs.k8s.io/cli-utils/pkg/apply/event"
 	"sigs.k8s.io/cli-utils/pkg/common"
-	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
 func randomString(prefix string) string {