Skip to content
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.

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

Elide uninteresting fields in object diffs. #498

Merged
merged 2 commits into from
Feb 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion pkg/test/step.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,8 @@ func (s *Step) CheckResource(expected runtime.Object, namespace string) []error
tmpTestErrors := []error{}

if err := testutils.IsSubset(expectedObj, actual.UnstructuredContent()); err != nil {
diff, diffErr := testutils.PrettyDiff(expected, &actual)
diff, diffErr := testutils.PrettyDiff(
&unstructured.Unstructured{Object: expectedObj}, &actual)
if diffErr == nil {
tmpTestErrors = append(tmpTestErrors, fmt.Errorf(diff))
} else {
Expand Down
61 changes: 59 additions & 2 deletions pkg/test/utils/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,16 +372,73 @@ func Namespaced(dClient discovery.DiscoveryInterface, obj runtime.Object, namesp
return m.GetName(), namespace, nil
}

func pruneLargeAdditions(expected *unstructured.Unstructured, actual *unstructured.Unstructured) runtime.Object {
pruned := actual.DeepCopy()
prune(expected.Object, pruned.Object)
return pruned
}

// prune replaces some fields in the actual tree to make it smaller for display.
//
// The goal is to make diffs on large objects much less verbose but not any less useful,
// by omitting these fields in the object which are not specified in the assertion and are at least
// moderately long when serialized.
//
// This way, for example when asserting on status.availableReplicas of a Deployment
// (which is missing if zero replicas are available) will still show the status.unavailableReplicas
// for example, but will omit spec completely unless the assertion also mentions it.
//
// This saves hundreds to thousands of lines of logs to scroll when debugging failures of some operator tests.
func prune(expected map[string]interface{}, actual map[string]interface{}) {
porridge marked this conversation as resolved.
Show resolved Hide resolved
// This value was chosen so that it is low enough to hide huge fields like `metadata.managedFields`,
// but large enough such that for example a typical `metadata.labels` still shows,
// since it might be useful for identifying reported objects like pods.
// This could potentially be turned into a knob in the future.
const maxLines = 10
porridge marked this conversation as resolved.
Show resolved Hide resolved
var toRemove []string
for k, v := range actual {
if _, inExpected := expected[k]; inExpected {
expectedMap, isExpectedMap := expected[k].(map[string]interface{})
actualMap, isActualMap := actual[k].(map[string]interface{})
if isActualMap && isExpectedMap {
prune(expectedMap, actualMap)
}
continue
}
numLines, err := countLines(k, v)
if err != nil || numLines < maxLines {
continue
}
toRemove = append(toRemove, k)
}
for _, s := range toRemove {
actual[s] = fmt.Sprintf("[... elided field over %d lines long ...]", maxLines)
}
}

func countLines(k string, v interface{}) (int, error) {
buf := strings.Builder{}
dummyObj := &unstructured.Unstructured{
Object: map[string]interface{}{k: v}}
err := MarshalObject(dummyObj, &buf)
if err != nil {
return 0, fmt.Errorf("cannot marshal field %s to compute its length in lines: %w", k, err)
}
return strings.Count(buf.String(), "\n"), nil
}

// PrettyDiff creates a unified diff highlighting the differences between two Kubernetes resources
func PrettyDiff(expected runtime.Object, actual runtime.Object) (string, error) {
func PrettyDiff(expected *unstructured.Unstructured, actual *unstructured.Unstructured) (string, error) {
actualPruned := pruneLargeAdditions(expected, actual)

expectedBuf := &bytes.Buffer{}
actualBuf := &bytes.Buffer{}

if err := MarshalObject(expected, expectedBuf); err != nil {
return "", err
}

if err := MarshalObject(actual, actualBuf); err != nil {
if err := MarshalObject(actualPruned, actualBuf); err != nil {
return "", err
}

Expand Down
52 changes: 52 additions & 0 deletions pkg/test/utils/kubernetes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -515,3 +515,55 @@ func TestRunScript(t *testing.T) {
})
}
}

func TestPrettyDiff(t *testing.T) {
actual, err := LoadYAMLFromFile("test_data/prettydiff-actual.yaml")
assert.NoError(t, err)
assert.Len(t, actual, 1)
expected, err := LoadYAMLFromFile("test_data/prettydiff-expected.yaml")
assert.NoError(t, err)
assert.Len(t, expected, 1)

result, err := PrettyDiff(expected[0].(*unstructured.Unstructured), actual[0].(*unstructured.Unstructured))
assert.NoError(t, err)
assert.Equal(t, `--- Deployment:/central
+++ Deployment:kuttl-test-thorough-hermit/central
@@ -1,7 +1,35 @@
apiVersion: apps/v1
kind: Deployment
metadata:
+ annotations:
+ email: support@stackrox.com
+ meta.helm.sh/release-name: stackrox-central-services
+ meta.helm.sh/release-namespace: kuttl-test-thorough-hermit
+ owner: stackrox
+ labels:
+ app: central
+ app.kubernetes.io/component: central
+ app.kubernetes.io/instance: stackrox-central-services
+ app.kubernetes.io/managed-by: Helm
+ app.kubernetes.io/name: stackrox
+ app.kubernetes.io/part-of: stackrox-central-services
+ app.kubernetes.io/version: 4.3.x-160-g465d734c11
+ helm.sh/chart: stackrox-central-services-400.3.0-160-g465d734c11
+ managedFields: '[... elided field over 10 lines long ...]'
name: central
+ namespace: kuttl-test-thorough-hermit
+ ownerReferences:
+ - apiVersion: platform.stackrox.io/v1alpha1
+ blockOwnerDeletion: true
+ controller: true
+ kind: Central
+ name: stackrox-central-services
+ uid: ff834d91-0853-42b3-9460-7ebf1c659f8a
+spec: '[... elided field over 10 lines long ...]'
status:
- availableReplicas: 1
+ conditions: '[... elided field over 10 lines long ...]'
+ observedGeneration: 2
+ replicas: 1
+ unavailableReplicas: 1
+ updatedReplicas: 1

`, result)
}
Loading
Loading