From 03afe6471bdbf6462b7035fdaae5aa0dd9545396 Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Tue, 10 Jun 2025 23:08:41 -0400 Subject: [PATCH] Add a replacement for cmp.Diff using json+go-difflib Co-authored-by: Jordan Liggitt Signed-off-by: Davanum Srinivas --- cmd/kubeadm/app/cmd/upgrade/plan_test.go | 2 +- hack/golangci-hints.yaml | 2 + hack/golangci.yaml | 2 + hack/golangci.yaml.in | 2 + .../certificates/validation/validation.go | 4 +- pkg/apis/core/validation/validation.go | 10 +- .../certificates/authority/authority_test.go | 43 ++- .../certificates/signer/signer_test.go | 45 ++- .../device_taint_eviction.go | 9 +- .../persistentvolume/testing/testing.go | 2 +- pkg/kubelet/status/status_manager.go | 4 +- pkg/registry/flowcontrol/ensurer/strategy.go | 6 +- .../dynamicresources/dynamicresources.go | 5 +- .../framework/plugins/noderesources/fit.go | 5 +- pkg/scheduler/profile/profile.go | 5 +- .../admission/noderestriction/admission.go | 5 +- staging/src/k8s.io/apimachinery/go.mod | 2 +- .../api/apitesting/roundtrip/compatibility.go | 2 +- .../pkg/api/apitesting/roundtrip/roundtrip.go | 2 +- .../api/apitesting/roundtrip/unstructured.go | 2 +- .../k8s.io/apimachinery/pkg/util/diff/cmp.go | 31 ++ .../pkg/util/diff/complex_test.go | 356 ++++++++++++++++++ .../k8s.io/apimachinery/pkg/util/diff/diff.go | 140 ++----- .../apimachinery/pkg/util/diff/legacy_diff.go | 67 ++++ .../pkg/admission/testing/helpers.go | 2 +- .../endpoints/discovery/aggregated/fake.go | 4 +- .../pkg/storage/testing/store_tests.go | 2 +- .../apiserver/pkg/storage/testing/utils.go | 2 +- .../pkg/util/flowcontrol/apf_controller.go | 4 +- .../data_consistency_detector.go | 8 +- .../src/k8s.io/code-generator/examples/go.mod | 1 + .../component-base/config/testing/helpers.go | 2 +- .../config/testing/roundtrip.go | 2 +- .../component-base/logs/api/v1/options.go | 4 +- .../resourceslice/resourceslicecontroller.go | 5 +- .../resourceslice/tracker/tracker.go | 11 +- staging/src/k8s.io/kube-proxy/go.mod | 2 + staging/src/k8s.io/sample-controller/go.mod | 1 + 38 files changed, 631 insertions(+), 172 deletions(-) create mode 100644 staging/src/k8s.io/apimachinery/pkg/util/diff/cmp.go create mode 100644 staging/src/k8s.io/apimachinery/pkg/util/diff/complex_test.go create mode 100644 staging/src/k8s.io/apimachinery/pkg/util/diff/legacy_diff.go diff --git a/cmd/kubeadm/app/cmd/upgrade/plan_test.go b/cmd/kubeadm/app/cmd/upgrade/plan_test.go index 74efa709717..8530551de30 100644 --- a/cmd/kubeadm/app/cmd/upgrade/plan_test.go +++ b/cmd/kubeadm/app/cmd/upgrade/plan_test.go @@ -823,7 +823,7 @@ _____________________________________________________________________ actual := rt.buf.String() if actual != rt.expected { - t.Errorf("failed PrintUpgradePlan:\n\nexpected:\n%s\n\nactual:\n%s\n\ndiff:\n%s", rt.expected, actual, diff.StringDiff(actual, rt.expected)) + t.Errorf("failed PrintUpgradePlan:\n\nexpected:\n%s\n\nactual:\n%s\n\ndiff:\n%s", rt.expected, actual, diff.Diff(actual, rt.expected)) } }) } diff --git a/hack/golangci-hints.yaml b/hack/golangci-hints.yaml index 81e5e95e753..63e52b1b1e6 100644 --- a/hack/golangci-hints.yaml +++ b/hack/golangci-hints.yaml @@ -251,6 +251,8 @@ linters: - $all - "!$test" - "!**/test/**" + - "!**/testing/**" + - "!**/apitesting/**" deny: - pkg: "github.com/google/go-cmp/cmp" desc: "cmp is allowed only in test files" diff --git a/hack/golangci.yaml b/hack/golangci.yaml index 2306878a6bf..4c38473f584 100644 --- a/hack/golangci.yaml +++ b/hack/golangci.yaml @@ -265,6 +265,8 @@ linters: - $all - "!$test" - "!**/test/**" + - "!**/testing/**" + - "!**/apitesting/**" deny: - pkg: "github.com/google/go-cmp/cmp" desc: "cmp is allowed only in test files" diff --git a/hack/golangci.yaml.in b/hack/golangci.yaml.in index cebfd06fd5e..0e2a57daf78 100644 --- a/hack/golangci.yaml.in +++ b/hack/golangci.yaml.in @@ -195,6 +195,8 @@ linters: - $all - "!$test" - "!**/test/**" + - "!**/testing/**" + - "!**/apitesting/**" deny: - pkg: "github.com/google/go-cmp/cmp" desc: "cmp is allowed only in test files" diff --git a/pkg/apis/certificates/validation/validation.go b/pkg/apis/certificates/validation/validation.go index f0d6b167af8..4f19106cd63 100644 --- a/pkg/apis/certificates/validation/validation.go +++ b/pkg/apis/certificates/validation/validation.go @@ -22,9 +22,9 @@ import ( "encoding/pem" "fmt" - "github.com/google/go-cmp/cmp" //nolint:depguard v1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" utilcert "k8s.io/client-go/util/cert" @@ -304,7 +304,7 @@ func validateCertificateSigningRequestUpdate(newCSR, oldCSR *certificates.Certif case len(newConditions) > len(oldConditions): validationErrorList = append(validationErrorList, field.Forbidden(field.NewPath("status", "conditions"), fmt.Sprintf("updates may not add a condition of type %q", t))) case !apiequality.Semantic.DeepEqual(oldConditions, newConditions): - conditionDiff := cmp.Diff(oldConditions, newConditions) + conditionDiff := diff.Diff(oldConditions, newConditions) validationErrorList = append(validationErrorList, field.Forbidden(field.NewPath("status", "conditions"), fmt.Sprintf("updates may not modify a condition of type %q\n%v", t, conditionDiff))) } } diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 9053dc6d58f..770fed10b14 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -31,7 +31,6 @@ import ( "unicode" "unicode/utf8" - "github.com/google/go-cmp/cmp" //nolint:depguard netutils "k8s.io/utils/net" v1 "k8s.io/api/core/v1" @@ -42,6 +41,7 @@ import ( unversionedvalidation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/conversion" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" @@ -2141,7 +2141,7 @@ func ValidatePersistentVolumeUpdate(newPv, oldPv *core.PersistentVolume, opts Pe // PersistentVolumeSource should be immutable after creation. if !apiequality.Semantic.DeepEqual(newPv.Spec.PersistentVolumeSource, oldPv.Spec.PersistentVolumeSource) { - pvcSourceDiff := cmp.Diff(oldPv.Spec.PersistentVolumeSource, newPv.Spec.PersistentVolumeSource) + pvcSourceDiff := diff.Diff(oldPv.Spec.PersistentVolumeSource, newPv.Spec.PersistentVolumeSource) allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "persistentvolumesource"), fmt.Sprintf("spec.persistentvolumesource is immutable after creation\n%v", pvcSourceDiff))) } allErrs = append(allErrs, ValidateImmutableField(newPv.Spec.VolumeMode, oldPv.Spec.VolumeMode, field.NewPath("volumeMode"))...) @@ -2436,7 +2436,7 @@ func ValidatePersistentVolumeClaimUpdate(newPvc, oldPvc *core.PersistentVolumeCl statusSize := oldPvc.Status.Capacity["storage"] if !apiequality.Semantic.DeepEqual(newPvcClone.Spec, oldPvcClone.Spec) { - specDiff := cmp.Diff(oldPvcClone.Spec, newPvcClone.Spec) + specDiff := diff.Diff(oldPvcClone.Spec, newPvcClone.Spec) allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), fmt.Sprintf("spec is immutable after creation except resources.requests and volumeAttributesClassName for bound claims\n%v", specDiff))) } if newSize.Cmp(oldSize) < 0 { @@ -5416,7 +5416,7 @@ func ValidatePodUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel if !apiequality.Semantic.DeepEqual(mungedPodSpec, oldPod.Spec) { // This diff isn't perfect, but it's a helluva lot better an "I'm not going to tell you what the difference is". // TODO: Pinpoint the specific field that causes the invalid error after we have strategic merge diff - specDiff := cmp.Diff(oldPod.Spec, mungedPodSpec) + specDiff := diff.Diff(oldPod.Spec, mungedPodSpec) errs := field.Forbidden(specPath, fmt.Sprintf("pod updates may not change fields other than %s\n%v", strings.Join(updatablePodSpecFields, ","), specDiff)) allErrs = append(allErrs, errs) } @@ -5629,7 +5629,7 @@ func ValidatePodEphemeralContainersUpdate(newPod, oldPod *core.Pod, opts PodVali if new, ok := newContainerIndex[old.Name]; !ok { allErrs = append(allErrs, field.Forbidden(specPath, fmt.Sprintf("existing ephemeral containers %q may not be removed\n", old.Name))) } else if !apiequality.Semantic.DeepEqual(old, *new) { - specDiff := cmp.Diff(old, *new) + specDiff := diff.Diff(old, *new) allErrs = append(allErrs, field.Forbidden(specPath, fmt.Sprintf("existing ephemeral containers %q may not be changed\n%v", old.Name, specDiff))) } } diff --git a/pkg/controller/certificates/authority/authority_test.go b/pkg/controller/certificates/authority/authority_test.go index 7ae13ec3823..4d38fdd6088 100644 --- a/pkg/controller/certificates/authority/authority_test.go +++ b/pkg/controller/certificates/authority/authority_test.go @@ -22,8 +22,10 @@ import ( "crypto/rand" "crypto/x509" "crypto/x509/pkix" + "fmt" "math/big" "net/url" + "reflect" "testing" "time" @@ -31,7 +33,6 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" capi "k8s.io/api/certificates/v1" - "k8s.io/apimachinery/pkg/util/diff" ) func TestCertificateAuthority(t *testing.T) { @@ -237,7 +238,7 @@ func TestCertificateAuthority(t *testing.T) { "Version", "MaxPathLen", ), - diff.IgnoreUnset(), + ignoreUnset(), cmp.Transformer("RoundTime", func(x time.Time) time.Time { return x.Truncate(time.Second) }), @@ -252,6 +253,44 @@ func TestCertificateAuthority(t *testing.T) { } } +// ignoreUnset is an option that ignores fields that are unset on the right +// hand side of a comparison. This is useful in testing to assert that an +// object is a derivative. +func ignoreUnset() cmp.Option { + return cmp.Options{ + // ignore unset fields in v2 + cmp.FilterPath(func(path cmp.Path) bool { + _, v2 := path.Last().Values() + switch v2.Kind() { + case reflect.Slice, reflect.Map: + if v2.IsNil() || v2.Len() == 0 { + return true + } + case reflect.String: + if v2.Len() == 0 { + return true + } + case reflect.Interface, reflect.Pointer: + if v2.IsNil() { + return true + } + } + return false + }, cmp.Ignore()), + // ignore map entries that aren't set in v2 + cmp.FilterPath(func(path cmp.Path) bool { + switch i := path.Last().(type) { + case cmp.MapIndex: + if _, v2 := i.Values(); !v2.IsValid() { + fmt.Println("E") + return true + } + } + return false + }, cmp.Ignore()), + } +} + func errString(err error) string { if err == nil { return "" diff --git a/pkg/controller/certificates/signer/signer_test.go b/pkg/controller/certificates/signer/signer_test.go index 4366fe7ea70..56ff04c9bbf 100644 --- a/pkg/controller/certificates/signer/signer_test.go +++ b/pkg/controller/certificates/signer/signer_test.go @@ -23,15 +23,16 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/pem" + "fmt" "io/ioutil" "math/rand" + "reflect" "testing" "time" "github.com/google/go-cmp/cmp" capi "k8s.io/api/certificates/v1" - "k8s.io/apimachinery/pkg/util/diff" "k8s.io/client-go/kubernetes/fake" testclient "k8s.io/client-go/testing" "k8s.io/client-go/util/cert" @@ -41,6 +42,44 @@ import ( testingclock "k8s.io/utils/clock/testing" ) +// ignoreUnset is an option that ignores fields that are unset on the right +// hand side of a comparison. This is useful in testing to assert that an +// object is a derivative. +func ignoreUnset() cmp.Option { + return cmp.Options{ + // ignore unset fields in v2 + cmp.FilterPath(func(path cmp.Path) bool { + _, v2 := path.Last().Values() + switch v2.Kind() { + case reflect.Slice, reflect.Map: + if v2.IsNil() || v2.Len() == 0 { + return true + } + case reflect.String: + if v2.Len() == 0 { + return true + } + case reflect.Interface, reflect.Pointer: + if v2.IsNil() { + return true + } + } + return false + }, cmp.Ignore()), + // ignore map entries that aren't set in v2 + cmp.FilterPath(func(path cmp.Path) bool { + switch i := path.Last().(type) { + case cmp.MapIndex: + if _, v2 := i.Values(); !v2.IsValid() { + fmt.Println("E") + return true + } + } + return false + }, cmp.Ignore()), + } +} + func TestSigner(t *testing.T) { fakeClock := testingclock.FakeClock{} @@ -99,8 +138,8 @@ func TestSigner(t *testing.T) { MaxPathLen: -1, } - if !cmp.Equal(*certs[0], want, diff.IgnoreUnset()) { - t.Errorf("unexpected diff: %v", cmp.Diff(certs[0], want, diff.IgnoreUnset())) + if !cmp.Equal(*certs[0], want, ignoreUnset()) { + t.Errorf("unexpected diff: %v", cmp.Diff(certs[0], want, ignoreUnset())) } } diff --git a/pkg/controller/devicetainteviction/device_taint_eviction.go b/pkg/controller/devicetainteviction/device_taint_eviction.go index 3e2abb194d7..a1838e6c040 100644 --- a/pkg/controller/devicetainteviction/device_taint_eviction.go +++ b/pkg/controller/devicetainteviction/device_taint_eviction.go @@ -27,14 +27,13 @@ import ( "sync/atomic" "time" - "github.com/google/go-cmp/cmp" //nolint:depguard // Discouraged for production use (https://github.com/kubernetes/kubernetes/issues/104821) but has no good alternative for logging. - v1 "k8s.io/api/core/v1" resourceapi "k8s.io/api/resource/v1beta1" apiequality "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/diff" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/sets" coreinformers "k8s.io/client-go/informers/core/v1" @@ -546,7 +545,7 @@ func (tc *Controller) handleClaimChange(oldClaim, newClaim *resourceapi.Resource name := newNamespacedName(claim) if tc.eventLogger != nil { // This is intentionally very verbose for debugging. - tc.eventLogger.Info("ResourceClaim changed", "claimObject", name, "oldClaim", klog.Format(oldClaim), "newClaim", klog.Format(newClaim), "diff", cmp.Diff(oldClaim, newClaim)) + tc.eventLogger.Info("ResourceClaim changed", "claimObject", name, "oldClaim", klog.Format(oldClaim), "newClaim", klog.Format(newClaim), "diff", diff.Diff(oldClaim, newClaim)) } // Deleted? @@ -686,7 +685,7 @@ func (tc *Controller) handleSliceChange(oldSlice, newSlice *resourceapi.Resource } if tc.eventLogger != nil { // This is intentionally very verbose for debugging. - tc.eventLogger.Info("ResourceSlice changed", "pool", poolID, "oldSlice", klog.Format(oldSlice), "newSlice", klog.Format(newSlice), "diff", cmp.Diff(oldSlice, newSlice)) + tc.eventLogger.Info("ResourceSlice changed", "pool", poolID, "oldSlice", klog.Format(oldSlice), "newSlice", klog.Format(newSlice), "diff", diff.Diff(oldSlice, newSlice)) } // Determine old and new device taints. Only devices @@ -784,7 +783,7 @@ func (tc *Controller) handlePodChange(oldPod, newPod *v1.Pod) { } if tc.eventLogger != nil { // This is intentionally very verbose for debugging. - tc.eventLogger.Info("Pod changed", "pod", klog.KObj(pod), "oldPod", klog.Format(oldPod), "newPod", klog.Format(newPod), "diff", cmp.Diff(oldPod, newPod)) + tc.eventLogger.Info("Pod changed", "pod", klog.KObj(pod), "oldPod", klog.Format(oldPod), "newPod", klog.Format(newPod), "diff", diff.Diff(oldPod, newPod)) } if newPod == nil { // Nothing left to do for it. No need to emit an event here, it's gone. diff --git a/pkg/controller/volume/persistentvolume/testing/testing.go b/pkg/controller/volume/persistentvolume/testing/testing.go index 4a7e9b53026..c37a18aac47 100644 --- a/pkg/controller/volume/persistentvolume/testing/testing.go +++ b/pkg/controller/volume/persistentvolume/testing/testing.go @@ -24,7 +24,7 @@ import ( "strconv" "sync" - "github.com/google/go-cmp/cmp" //nolint:depguard + "github.com/google/go-cmp/cmp" "k8s.io/klog/v2" v1 "k8s.io/api/core/v1" diff --git a/pkg/kubelet/status/status_manager.go b/pkg/kubelet/status/status_manager.go index 67f7076b4e6..05b8bb56ffa 100644 --- a/pkg/kubelet/status/status_manager.go +++ b/pkg/kubelet/status/status_manager.go @@ -25,7 +25,6 @@ import ( "sync" "time" - "github.com/google/go-cmp/cmp" //nolint:depguard clientset "k8s.io/client-go/kubernetes" v1 "k8s.io/api/core/v1" @@ -33,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/wait" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" @@ -1027,7 +1027,7 @@ func (m *manager) needsReconcile(logger klog.Logger, uid types.UID, status v1.Po } logger.V(3).Info("Pod status is inconsistent with cached status for pod, a reconciliation should be triggered", "pod", klog.KObj(pod), - "statusDiff", cmp.Diff(podStatus, &status)) + "statusDiff", diff.Diff(podStatus, &status)) return true } diff --git a/pkg/registry/flowcontrol/ensurer/strategy.go b/pkg/registry/flowcontrol/ensurer/strategy.go index 9b025424237..5b7211dbd3a 100644 --- a/pkg/registry/flowcontrol/ensurer/strategy.go +++ b/pkg/registry/flowcontrol/ensurer/strategy.go @@ -21,12 +21,12 @@ import ( "fmt" "strconv" - "github.com/google/go-cmp/cmp" //nolint:depguard flowcontrolv1 "k8s.io/api/flowcontrol/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog/v2" ) @@ -285,13 +285,13 @@ func EnsureConfiguration[ObjectType configurationObjectType](ctx context.Context if !update { if klogV := klog.V(5); klogV.Enabled() { klogV.InfoS("No update required", "wrapper", bootstrap.GetObjectKind().GroupVersionKind().Kind, "type", configurationType, "name", name, - "diff", cmp.Diff(current, bootstrap)) + "diff", diff.Diff(current, bootstrap)) } return nil } if _, err = ops.Update(ctx, newObject, metav1.UpdateOptions{FieldManager: fieldManager}); err == nil { - klog.V(2).Infof("Updated the %T type=%s name=%q diff: %s", bootstrap, configurationType, name, cmp.Diff(current, bootstrap)) + klog.V(2).Infof("Updated the %T type=%s name=%q diff: %s", bootstrap, configurationType, name, diff.Diff(current, bootstrap)) return nil } diff --git a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go index 8e54769554c..22d350b1a91 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go @@ -24,8 +24,6 @@ import ( "strings" "sync" - "github.com/google/go-cmp/cmp" //nolint:depguard - v1 "k8s.io/api/core/v1" resourceapi "k8s.io/api/resource/v1beta1" apiequality "k8s.io/apimachinery/pkg/api/equality" @@ -33,6 +31,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/kubernetes" "k8s.io/client-go/util/retry" @@ -258,7 +257,7 @@ func (pl *DynamicResources) isSchedulableAfterClaimChange(logger klog.Logger, po if apiequality.Semantic.DeepEqual(&originalClaim.Status, &modifiedClaim.Status) { if loggerV := logger.V(7); loggerV.Enabled() { // Log more information. - loggerV.Info("claim for pod got modified where the pod doesn't care", "pod", klog.KObj(pod), "claim", klog.KObj(modifiedClaim), "diff", cmp.Diff(originalClaim, modifiedClaim)) + loggerV.Info("claim for pod got modified where the pod doesn't care", "pod", klog.KObj(pod), "claim", klog.KObj(modifiedClaim), "diff", diff.Diff(originalClaim, modifiedClaim)) } else { logger.V(6).Info("claim for pod got modified where the pod doesn't care", "pod", klog.KObj(pod), "claim", klog.KObj(modifiedClaim)) } diff --git a/pkg/scheduler/framework/plugins/noderesources/fit.go b/pkg/scheduler/framework/plugins/noderesources/fit.go index 17cb2fe0c7f..ba779694cd9 100644 --- a/pkg/scheduler/framework/plugins/noderesources/fit.go +++ b/pkg/scheduler/framework/plugins/noderesources/fit.go @@ -21,10 +21,9 @@ import ( "fmt" "strings" - "github.com/google/go-cmp/cmp" //nolint:depguard - v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/component-helpers/resource" "k8s.io/klog/v2" @@ -314,7 +313,7 @@ func (f *Fit) isSchedulableAfterPodEvent(logger klog.Logger, pod *v1.Pod, oldObj if !f.isSchedulableAfterPodScaleDown(pod, originalPod, modifiedPod) { if loggerV := logger.V(10); loggerV.Enabled() { // Log more information. - loggerV.Info("pod got scaled down, but the modification isn't related to the resource requests of the target pod", "pod", klog.KObj(pod), "modifiedPod", klog.KObj(modifiedPod), "diff", cmp.Diff(originalPod, modifiedPod)) + loggerV.Info("pod got scaled down, but the modification isn't related to the resource requests of the target pod", "pod", klog.KObj(pod), "modifiedPod", klog.KObj(modifiedPod), "diff", diff.Diff(originalPod, modifiedPod)) } else { logger.V(5).Info("pod got scaled down, but the modification isn't related to the resource requests of the target pod", "pod", klog.KObj(pod), "modifiedPod", klog.KObj(modifiedPod)) } diff --git a/pkg/scheduler/profile/profile.go b/pkg/scheduler/profile/profile.go index 9cd510d35ee..3ac39035ee4 100644 --- a/pkg/scheduler/profile/profile.go +++ b/pkg/scheduler/profile/profile.go @@ -22,9 +22,8 @@ import ( "errors" "fmt" - "github.com/google/go-cmp/cmp" //nolint:depguard - "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/diff" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/events" "k8s.io/kubernetes/pkg/scheduler/apis/config" @@ -123,7 +122,7 @@ func (v *cfgValidator) validate(cfg config.KubeSchedulerProfile, f framework.Fra if v.queueSort != queueSort { return fmt.Errorf("different queue sort plugins for profile %q: %q, first: %q", cfg.SchedulerName, queueSort, v.queueSort) } - if !cmp.Equal(v.queueSortArgs, queueSortArgs) { + if diff.Diff(v.queueSortArgs, queueSortArgs) != "" { return fmt.Errorf("different queue sort plugin args for profile %q", cfg.SchedulerName) } return nil diff --git a/plugin/pkg/admission/noderestriction/admission.go b/plugin/pkg/admission/noderestriction/admission.go index b85989dbd28..c36c7f7d4b9 100644 --- a/plugin/pkg/admission/noderestriction/admission.go +++ b/plugin/pkg/admission/noderestriction/admission.go @@ -23,13 +23,12 @@ import ( "io" "strings" - "github.com/google/go-cmp/cmp" //nolint:depguard - v1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/diff" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/admission" @@ -476,7 +475,7 @@ func (p *Plugin) admitPVCStatus(nodeName string, a admission.Attributes) error { // ensure no metadata changed. nodes should not be able to relabel, add finalizers/owners, etc if !apiequality.Semantic.DeepEqual(oldPVC, newPVC) { - return admission.NewForbidden(a, fmt.Errorf("node %q is not allowed to update fields other than status.quantity and status.conditions: %v", nodeName, cmp.Diff(oldPVC, newPVC))) + return admission.NewForbidden(a, fmt.Errorf("node %q is not allowed to update fields other than status.quantity and status.conditions: %v", nodeName, diff.Diff(oldPVC, newPVC))) } return nil diff --git a/staging/src/k8s.io/apimachinery/go.mod b/staging/src/k8s.io/apimachinery/go.mod index e6ab82d8430..740a6ec7ab3 100644 --- a/staging/src/k8s.io/apimachinery/go.mod +++ b/staging/src/k8s.io/apimachinery/go.mod @@ -16,6 +16,7 @@ require ( github.com/google/uuid v1.6.0 github.com/moby/spdystream v0.5.0 github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f + github.com/pmezard/go-difflib v1.0.0 github.com/spf13/pflag v1.0.6 github.com/stretchr/testify v1.10.0 golang.org/x/net v0.38.0 @@ -44,7 +45,6 @@ require ( github.com/onsi/ginkgo/v2 v2.21.0 // indirect github.com/onsi/gomega v1.35.1 // indirect github.com/pkg/errors v0.9.1 // indirect - github.com/pmezard/go-difflib v1.0.0 // indirect github.com/rogpeppe/go-internal v1.13.1 // indirect github.com/x448/float16 v0.8.4 // indirect golang.org/x/text v0.23.0 // indirect diff --git a/staging/src/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/compatibility.go b/staging/src/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/compatibility.go index 33565c2568c..deba9b456b7 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/compatibility.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/compatibility.go @@ -29,7 +29,7 @@ import ( "strings" "testing" - "github.com/google/go-cmp/cmp" //nolint:depguard + "github.com/google/go-cmp/cmp" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/runtime" diff --git a/staging/src/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/roundtrip.go b/staging/src/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/roundtrip.go index 51f1a350f83..d9d177c46b0 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/roundtrip.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/roundtrip.go @@ -24,7 +24,7 @@ import ( "strings" "testing" - "github.com/google/go-cmp/cmp" //nolint:depguard + "github.com/google/go-cmp/cmp" flag "github.com/spf13/pflag" "sigs.k8s.io/randfill" diff --git a/staging/src/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/unstructured.go b/staging/src/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/unstructured.go index d5ab664d31d..f1794943d2e 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/unstructured.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/unstructured.go @@ -37,7 +37,7 @@ import ( jsonserializer "k8s.io/apimachinery/pkg/runtime/serializer/json" "k8s.io/apimachinery/pkg/util/sets" - "github.com/google/go-cmp/cmp" //nolint:depguard + "github.com/google/go-cmp/cmp" ) // RoundtripToUnstructured verifies the roundtrip faithfulness of all external types in a scheme diff --git a/staging/src/k8s.io/apimachinery/pkg/util/diff/cmp.go b/staging/src/k8s.io/apimachinery/pkg/util/diff/cmp.go new file mode 100644 index 00000000000..0a8100d0639 --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/util/diff/cmp.go @@ -0,0 +1,31 @@ +//go:build usegocmp +// +build usegocmp + +/* +Copyright 2025 The Kubernetes 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. +*/ + +package diff + +import ( + "github.com/google/go-cmp/cmp" //nolint:depguard +) + +// Diff returns a string representation of the difference between two objects. +// When built with the usegocmp tag, it uses go-cmp/cmp to generate a diff +// between the objects. +func Diff(a, b any) string { + return cmp.Diff(a, b) +} diff --git a/staging/src/k8s.io/apimachinery/pkg/util/diff/complex_test.go b/staging/src/k8s.io/apimachinery/pkg/util/diff/complex_test.go new file mode 100644 index 00000000000..fb31513f090 --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/util/diff/complex_test.go @@ -0,0 +1,356 @@ +//go:build !usegocmp +// +build !usegocmp + +/* +Copyright 2025 The Kubernetes 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. +*/ + +package diff + +import ( + "strings" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" +) + +// TestDiffWithRealGoCmp is a comprehensive test that compares our Diff with the actual go-cmp Diff +// across a variety of data structures and types to ensure compatibility. +func TestDiffWithRealGoCmp(t *testing.T) { + // Test with simple types + t.Run("SimpleTypes", func(t *testing.T) { + testCases := []struct { + name string + a interface{} + b interface{} + }{ + {name: "Integers", a: 42, b: 43}, + {name: "Strings", a: "hello", b: "world"}, + {name: "Booleans", a: true, b: false}, + {name: "Floats", a: 3.14, b: 2.71}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ourDiff := Diff(tc.a, tc.b) + goCmpDiff := cmp.Diff(tc.a, tc.b) + + t.Logf("Our diff:\n%s\n\nGo-cmp diff:\n%s", ourDiff, goCmpDiff) + + // Verify both diffs are non-empty + if ourDiff == "" || goCmpDiff == "" { + t.Errorf("Expected non-empty diffs, got ourDiff: %v, goCmpDiff: %v", + ourDiff == "", goCmpDiff == "") + } + }) + } + }) + + // Test with a simple struct + t.Run("SimpleStruct", func(t *testing.T) { + type TestStruct struct { + Name string + Age int + Tags []string + Map map[string]int + } + + a := TestStruct{ + Name: "Alice", + Age: 30, + Tags: []string{"tag1", "tag2"}, + Map: map[string]int{"a": 1, "b": 2}, + } + + b := TestStruct{ + Name: "Bob", + Age: 25, + Tags: []string{"tag1", "tag3"}, + Map: map[string]int{"a": 1, "c": 3}, + } + + ourDiff := Diff(a, b) + goCmpDiff := cmp.Diff(a, b) + + t.Logf("Our diff:\n%s\n\nGo-cmp diff:\n%s", ourDiff, goCmpDiff) + + // Check that our diff contains key differences + keyDifferences := []string{ + "Name", "Alice", "Bob", + "Age", "30", "25", + "Tags", "tag2", "tag3", + "Map", "b", "c", + } + + for _, key := range keyDifferences { + if !strings.Contains(ourDiff, key) { + t.Errorf("Our diff doesn't contain expected key difference: %q", key) + } + } + }) + + // Test with a complex nested struct + t.Run("ComplexNestedStruct", func(t *testing.T) { + type Address struct { + Street string + City string + State string + PostalCode string + Country string + } + + type Contact struct { + Type string + Value string + } + + type Person struct { + ID int + FirstName string + LastName string + Age int + Addresses map[string]Address + Contacts []Contact + Metadata map[string]interface{} + CreatedAt time.Time + } + + now := time.Now() + later := now.Add(24 * time.Hour) + + person1 := Person{ + ID: 1, + FirstName: "John", + LastName: "Doe", + Age: 30, + Addresses: map[string]Address{ + "home": { + Street: "123 Main St", + City: "Anytown", + State: "CA", + PostalCode: "12345", + Country: "USA", + }, + "work": { + Street: "456 Market St", + City: "Worktown", + State: "CA", + PostalCode: "54321", + Country: "USA", + }, + }, + Contacts: []Contact{ + {Type: "email", Value: "john.doe@example.com"}, + {Type: "phone", Value: "555-1234"}, + }, + Metadata: map[string]interface{}{ + "created": "2023-01-01", + "loginCount": 42, + "settings": map[string]bool{ + "notifications": true, + "darkMode": false, + }, + }, + CreatedAt: now, + } + + person2 := Person{ + ID: 1, + FirstName: "John", + LastName: "Smith", // Different + Age: 31, // Different + Addresses: map[string]Address{ + "home": { + Street: "123 Main St", + City: "Anytown", + State: "CA", + PostalCode: "12345", + Country: "USA", + }, + // "work" address is missing + }, + Contacts: []Contact{ + {Type: "email", Value: "john.smith@example.com"}, // Different + {Type: "phone", Value: "555-1234"}, + {Type: "fax", Value: "555-5678"}, // Additional + }, + Metadata: map[string]interface{}{ + "created": "2023-01-01", + "loginCount": 43, // Different + "settings": map[string]bool{ + "notifications": false, // Different + "darkMode": true, // Different + }, + "newField": "new value", // New field + }, + CreatedAt: later, // Different + } + + ourDiff := Diff(person1, person2) + goCmpDiff := cmp.Diff(person1, person2) + + t.Logf("Our diff:\n%s\n\nGo-cmp diff:\n%s", ourDiff, goCmpDiff) + + // Check that our diff contains key differences + keyDifferences := []string{ + "LastName", "Smith", + "Age", + "Addresses", "work", + "Contacts", "john.smith@example.com", "fax", + "Metadata", "loginCount", "settings", "notifications", "darkMode", "newField", + } + + for _, key := range keyDifferences { + if !strings.Contains(ourDiff, key) { + t.Errorf("Our diff doesn't contain expected key difference: %q", key) + } + } + + // Check that both diffs are non-empty + if ourDiff == "" || goCmpDiff == "" { + t.Errorf("Expected non-empty diffs, got ourDiff: %v, goCmpDiff: %v", + ourDiff == "", goCmpDiff == "") + } + }) + + // Test with slices and maps + t.Run("SlicesAndMaps", func(t *testing.T) { + // Test with slices + a1 := []int{1, 2, 3, 4, 5} + b1 := []int{1, 2, 6, 4, 7} + + ourDiff1 := Diff(a1, b1) + goCmpDiff1 := cmp.Diff(a1, b1) + + t.Logf("Our diff (slices):\n%s\n\nGo-cmp diff (slices):\n%s", ourDiff1, goCmpDiff1) + + // Check that our diff contains the differences + if !strings.Contains(ourDiff1, "3") || !strings.Contains(ourDiff1, "6") || + !strings.Contains(ourDiff1, "5") || !strings.Contains(ourDiff1, "7") { + t.Errorf("Our diff doesn't contain all expected differences for slices") + } + + // Test with maps + a2 := map[string]int{"a": 1, "b": 2, "c": 3} + b2 := map[string]int{"a": 1, "b": 5, "d": 4} + + ourDiff2 := Diff(a2, b2) + goCmpDiff2 := cmp.Diff(a2, b2) + + t.Logf("Our diff (maps):\n%s\n\nGo-cmp diff (maps):\n%s", ourDiff2, goCmpDiff2) + + // Check that our diff contains the differences + if !strings.Contains(ourDiff2, "b") || !strings.Contains(ourDiff2, "5") || + !strings.Contains(ourDiff2, "c") || !strings.Contains(ourDiff2, "d") { + t.Errorf("Our diff doesn't contain all expected differences for maps") + } + }) + + // Test with unexported fields + t.Run("UnexportedFields", func(t *testing.T) { + type WithUnexported struct { + Exported int + unexported int + } + + a := WithUnexported{Exported: 1, unexported: 2} + b := WithUnexported{Exported: 3, unexported: 4} + + ourDiff := Diff(a, b) + // Use cmpopts.IgnoreUnexported to ignore unexported fields in go-cmp + goCmpDiff := cmp.Diff(a, b, cmpopts.IgnoreUnexported(WithUnexported{})) + + t.Logf("Our diff (unexported):\n%s\n\nGo-cmp diff (unexported):\n%s", ourDiff, goCmpDiff) + + // Check that our diff contains only the exported field difference + if !strings.Contains(ourDiff, "Exported") || !strings.Contains(ourDiff, "1") || !strings.Contains(ourDiff, "3") { + t.Errorf("Our diff doesn't contain the exported field difference") + } + }) + + // Test with embedded structs + t.Run("EmbeddedStructs", func(t *testing.T) { + type Embedded struct { + Value int + } + + type Container struct { + Embedded + Extra string + } + + a := Container{Embedded: Embedded{Value: 1}, Extra: "a"} + b := Container{Embedded: Embedded{Value: 2}, Extra: "b"} + + ourDiff := Diff(a, b) + goCmpDiff := cmp.Diff(a, b) + + t.Logf("Our diff (embedded):\n%s\n\nGo-cmp diff (embedded):\n%s", ourDiff, goCmpDiff) + + // Check that our diff contains the container field difference + if !strings.Contains(ourDiff, "Extra") || !strings.Contains(ourDiff, "a") || !strings.Contains(ourDiff, "b") { + t.Errorf("Our diff doesn't contain the container field difference") + } + }) + + // Test with interface values of same type + t.Run("InterfaceValues", func(t *testing.T) { + type Container struct { + Value interface{} + } + + // Test with same type in interface + c := Container{Value: 42} + d := Container{Value: 43} + + ourDiff := Diff(c, d) + goCmpDiff := cmp.Diff(c, d) + + t.Logf("Our diff (interface same type):\n%s\n\nGo-cmp diff (interface same type):\n%s", ourDiff, goCmpDiff) + + // Check that our diff contains the value difference + if !strings.Contains(ourDiff, "42") || !strings.Contains(ourDiff, "43") { + t.Errorf("Our diff doesn't contain the value difference for interface values of same type") + } + }) + + // Test with objects that cannot be marshaled to JSON + t.Run("UnmarshalableObjects", func(t *testing.T) { + // Test with a circular reference, which cannot be marshaled to JSON + type Node struct { + Value int + Next *Node + } + + // Create a circular reference + nodeA := &Node{Value: 1} + nodeA.Next = nodeA // Points to itself + + nodeB := &Node{Value: 2} + nodeB.Next = nodeB // Points to itself + + // This should fall back to using dump.Pretty + circularDiff := Diff(nodeA, nodeB) + + t.Logf("Diff for circular references:\n%s", circularDiff) + + // Verify the diff contains the values + if !strings.Contains(circularDiff, "1") || !strings.Contains(circularDiff, "2") { + t.Errorf("Diff doesn't contain expected value differences for circular references") + } + }) +} diff --git a/staging/src/k8s.io/apimachinery/pkg/util/diff/diff.go b/staging/src/k8s.io/apimachinery/pkg/util/diff/diff.go index 38b666ef59d..aed04524b65 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/diff/diff.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/diff/diff.go @@ -1,5 +1,8 @@ +//go:build !usegocmp +// +build !usegocmp + /* -Copyright 2014 The Kubernetes Authors. +Copyright 2025 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -17,122 +20,43 @@ limitations under the License. package diff import ( - "bytes" + "encoding/json" "fmt" - "reflect" - "strings" - "text/tabwriter" - "github.com/google/go-cmp/cmp" //nolint:depguard + "github.com/pmezard/go-difflib/difflib" + "k8s.io/apimachinery/pkg/util/dump" ) -func legacyDiff(a, b interface{}) string { - return cmp.Diff(a, b) -} +// Diff returns a string representation of the difference between two objects. +// When built without the usegocmp tag, it uses go-difflib/difflib to generate a +// unified diff of the objects. It attempts to use JSON serialization first, +// falling back to an object dump via the dump package if JSON marshaling fails. +func Diff(a, b any) string { -// StringDiff diffs a and b and returns a human readable diff. -// DEPRECATED: use github.com/google/go-cmp/cmp.Diff -func StringDiff(a, b string) string { - return legacyDiff(a, b) -} - -// ObjectDiff prints the diff of two go objects and fails if the objects -// contain unhandled unexported fields. -// DEPRECATED: use github.com/google/go-cmp/cmp.Diff -func ObjectDiff(a, b interface{}) string { - return legacyDiff(a, b) -} - -// ObjectGoPrintDiff prints the diff of two go objects and fails if the objects -// contain unhandled unexported fields. -// DEPRECATED: use github.com/google/go-cmp/cmp.Diff -func ObjectGoPrintDiff(a, b interface{}) string { - return legacyDiff(a, b) -} - -// ObjectReflectDiff prints the diff of two go objects and fails if the objects -// contain unhandled unexported fields. -// DEPRECATED: use github.com/google/go-cmp/cmp.Diff -func ObjectReflectDiff(a, b interface{}) string { - return legacyDiff(a, b) -} - -// ObjectGoPrintSideBySide prints a and b as textual dumps side by side, -// enabling easy visual scanning for mismatches. -func ObjectGoPrintSideBySide(a, b interface{}) string { - sA := dump.Pretty(a) - sB := dump.Pretty(b) - - linesA := strings.Split(sA, "\n") - linesB := strings.Split(sB, "\n") - width := 0 - for _, s := range linesA { - l := len(s) - if l > width { - width = l - } + aStr, aErr := toPrettyJSON(a) + bStr, bErr := toPrettyJSON(b) + if aErr != nil || bErr != nil { + aStr = dump.Pretty(a) + bStr = dump.Pretty(b) } - for _, s := range linesB { - l := len(s) - if l > width { - width = l - } + + diff := difflib.UnifiedDiff{ + A: difflib.SplitLines(aStr), + B: difflib.SplitLines(bStr), + Context: 3, } - buf := &bytes.Buffer{} - w := tabwriter.NewWriter(buf, width, 0, 1, ' ', 0) - max := len(linesA) - if len(linesB) > max { - max = len(linesB) + + diffstr, err := difflib.GetUnifiedDiffString(diff) + if err != nil { + return fmt.Sprintf("error generating diff: %v", err) } - for i := 0; i < max; i++ { - var a, b string - if i < len(linesA) { - a = linesA[i] - } - if i < len(linesB) { - b = linesB[i] - } - fmt.Fprintf(w, "%s\t%s\n", a, b) - } - w.Flush() - return buf.String() + + return diffstr } -// IgnoreUnset is an option that ignores fields that are unset on the right -// hand side of a comparison. This is useful in testing to assert that an -// object is a derivative. -func IgnoreUnset() cmp.Option { - return cmp.Options{ - // ignore unset fields in v2 - cmp.FilterPath(func(path cmp.Path) bool { - _, v2 := path.Last().Values() - switch v2.Kind() { - case reflect.Slice, reflect.Map: - if v2.IsNil() || v2.Len() == 0 { - return true - } - case reflect.String: - if v2.Len() == 0 { - return true - } - case reflect.Interface, reflect.Pointer: - if v2.IsNil() { - return true - } - } - return false - }, cmp.Ignore()), - // ignore map entries that aren't set in v2 - cmp.FilterPath(func(path cmp.Path) bool { - switch i := path.Last().(type) { - case cmp.MapIndex: - if _, v2 := i.Values(); !v2.IsValid() { - fmt.Println("E") - return true - } - } - return false - }, cmp.Ignore()), - } +// toPrettyJSON converts an object to a pretty-printed JSON string. +func toPrettyJSON(data any) (string, error) { + jsonData, err := json.MarshalIndent(data, "", " ") + return string(jsonData), err } diff --git a/staging/src/k8s.io/apimachinery/pkg/util/diff/legacy_diff.go b/staging/src/k8s.io/apimachinery/pkg/util/diff/legacy_diff.go new file mode 100644 index 00000000000..4d32d36adb3 --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/util/diff/legacy_diff.go @@ -0,0 +1,67 @@ +/* +Copyright 2014 The Kubernetes 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. +*/ + +package diff + +import ( + "bytes" + "fmt" + "strings" + "text/tabwriter" + + "k8s.io/apimachinery/pkg/util/dump" +) + +// ObjectGoPrintSideBySide prints a and b as textual dumps side by side, +// enabling easy visual scanning for mismatches. +func ObjectGoPrintSideBySide(a, b interface{}) string { + sA := dump.Pretty(a) + sB := dump.Pretty(b) + + linesA := strings.Split(sA, "\n") + linesB := strings.Split(sB, "\n") + width := 0 + for _, s := range linesA { + l := len(s) + if l > width { + width = l + } + } + for _, s := range linesB { + l := len(s) + if l > width { + width = l + } + } + buf := &bytes.Buffer{} + w := tabwriter.NewWriter(buf, width, 0, 1, ' ', 0) + max := len(linesA) + if len(linesB) > max { + max = len(linesB) + } + for i := 0; i < max; i++ { + var a, b string + if i < len(linesA) { + a = linesA[i] + } + if i < len(linesB) { + b = linesB[i] + } + _, _ = fmt.Fprintf(w, "%s\t%s\n", a, b) + } + _ = w.Flush() + return buf.String() +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/testing/helpers.go b/staging/src/k8s.io/apiserver/pkg/admission/testing/helpers.go index cd1a165281c..76dea6d5c61 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/testing/helpers.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/testing/helpers.go @@ -21,7 +21,7 @@ import ( "reflect" "testing" - "github.com/google/go-cmp/cmp" //nolint:depguard + "github.com/google/go-cmp/cmp" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/admission" diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/fake.go b/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/fake.go index 4146dc01164..8cf0bb0b0ef 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/fake.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/fake.go @@ -25,9 +25,9 @@ import ( "time" "github.com/emicklei/go-restful/v3" - "github.com/google/go-cmp/cmp" //nolint:depguard apidiscoveryv2 "k8s.io/api/apidiscovery/v2" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/wait" ) @@ -91,7 +91,7 @@ func (f *fakeResourceManager) Validate() error { defer f.expect.lock.RUnlock() if !reflect.DeepEqual(f.expect.Actions, f.Actions) { - return errors.New(cmp.Diff(f.expect.Actions, f.Actions)) + return errors.New(diff.Diff(f.expect.Actions, f.Actions)) } return nil } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go b/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go index 3eccde4a524..7aa5838ba89 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go @@ -29,7 +29,7 @@ import ( "sync" "testing" - "github.com/google/go-cmp/cmp" //nolint:depguard + "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" apierrors "k8s.io/apimachinery/pkg/api/errors" diff --git a/staging/src/k8s.io/apiserver/pkg/storage/testing/utils.go b/staging/src/k8s.io/apiserver/pkg/storage/testing/utils.go index 3f2dd71524d..ad6b36e7863 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/testing/utils.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/testing/utils.go @@ -28,7 +28,7 @@ import ( "testing" "time" - "github.com/google/go-cmp/cmp" //nolint:depguard + "github.com/google/go-cmp/cmp" "k8s.io/apimachinery/pkg/api/meta" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" diff --git a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/apf_controller.go b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/apf_controller.go index 652ef57abb8..43765c34252 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/apf_controller.go +++ b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/apf_controller.go @@ -28,11 +28,11 @@ import ( "sync" "time" - "github.com/google/go-cmp/cmp" //nolint:depguard apiequality "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/diff" utilerrors "k8s.io/apimachinery/pkg/util/errors" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/sets" @@ -607,7 +607,7 @@ func (cfgCtlr *configController) digestConfigObjects(newPLs []*flowcontrol.Prior currResult.updatedItems.Insert(fsu.flowSchema.Name) if klogV := klog.V(4); klogV.Enabled() { klogV.Infof("%s writing Condition %s to FlowSchema %s, which had ResourceVersion=%s, because its previous value was %s, diff: %s", - cfgCtlr.name, fsu.condition, fsu.flowSchema.Name, fsu.flowSchema.ResourceVersion, fcfmt.Fmt(fsu.oldValue), cmp.Diff(fsu.oldValue, fsu.condition)) + cfgCtlr.name, fsu.condition, fsu.flowSchema.Name, fsu.flowSchema.ResourceVersion, fcfmt.Fmt(fsu.oldValue), diff.Diff(fsu.oldValue, fsu.condition)) } if err := apply(cfgCtlr.flowcontrolClient.FlowSchemas(), fsu, cfgCtlr.asFieldManager); err != nil { diff --git a/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go b/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go index 72a871d434b..32ee2c76066 100644 --- a/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go +++ b/staging/src/k8s.io/client-go/util/consistencydetector/data_consistency_detector.go @@ -19,14 +19,14 @@ package consistencydetector import ( "context" "fmt" + "reflect" "sort" "time" - "github.com/google/go-cmp/cmp" //nolint:depguard - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/klog/v2" ) @@ -75,8 +75,8 @@ func CheckDataConsistency[T runtime.Object, U any](ctx context.Context, identity sort.Sort(byUID(listItems)) sort.Sort(byUID(retrievedItems)) - if !cmp.Equal(listItems, retrievedItems) { - klog.Infof("previously received data for %s is different than received by the standard list api call against etcd, diff: %v", identity, cmp.Diff(listItems, retrievedItems)) + if !reflect.DeepEqual(listItems, retrievedItems) { + klog.Infof("previously received data for %s is different than received by the standard list api call against etcd, diff: %v", identity, diff.Diff(listItems, retrievedItems)) msg := fmt.Sprintf("data inconsistency detected for %s, panicking!", identity) panic(msg) } diff --git a/staging/src/k8s.io/code-generator/examples/go.mod b/staging/src/k8s.io/code-generator/examples/go.mod index 959a9156aa7..df919bd625f 100644 --- a/staging/src/k8s.io/code-generator/examples/go.mod +++ b/staging/src/k8s.io/code-generator/examples/go.mod @@ -34,6 +34,7 @@ require ( github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/x448/float16 v0.8.4 // indirect golang.org/x/net v0.38.0 // indirect golang.org/x/oauth2 v0.27.0 // indirect diff --git a/staging/src/k8s.io/component-base/config/testing/helpers.go b/staging/src/k8s.io/component-base/config/testing/helpers.go index 1b923335567..8f1fcc8fb9b 100644 --- a/staging/src/k8s.io/component-base/config/testing/helpers.go +++ b/staging/src/k8s.io/component-base/config/testing/helpers.go @@ -23,7 +23,7 @@ import ( "path/filepath" "testing" - "github.com/google/go-cmp/cmp" //nolint:depguard + "github.com/google/go-cmp/cmp" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" diff --git a/staging/src/k8s.io/component-base/config/testing/roundtrip.go b/staging/src/k8s.io/component-base/config/testing/roundtrip.go index fa0b8757323..b40efee82e0 100644 --- a/staging/src/k8s.io/component-base/config/testing/roundtrip.go +++ b/staging/src/k8s.io/component-base/config/testing/roundtrip.go @@ -22,7 +22,7 @@ import ( "path/filepath" "testing" - "github.com/google/go-cmp/cmp" //nolint:depguard + "github.com/google/go-cmp/cmp" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" diff --git a/staging/src/k8s.io/component-base/logs/api/v1/options.go b/staging/src/k8s.io/component-base/logs/api/v1/options.go index 95c0a2cba98..4c8a0d2c53f 100644 --- a/staging/src/k8s.io/component-base/logs/api/v1/options.go +++ b/staging/src/k8s.io/component-base/logs/api/v1/options.go @@ -27,13 +27,13 @@ import ( "sync/atomic" "time" - "github.com/google/go-cmp/cmp" //nolint:depguard "github.com/spf13/pflag" "k8s.io/klog/v2" "k8s.io/klog/v2/textlogger" "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/validation/field" cliflag "k8s.io/component-base/cli/flag" "k8s.io/component-base/featuregate" @@ -240,7 +240,7 @@ func apply(c *LoggingConfiguration, options *LoggingOptions, featureGate feature case ReapplyHandlingError: return errors.New("logging configuration was already applied earlier, changing it is not allowed") case ReapplyHandlingIgnoreUnchanged: - if diff := cmp.Diff(oldP, p); diff != "" { + if diff := diff.Diff(oldP, p); diff != "" { return fmt.Errorf("the logging configuration should not be changed after setting it once (- old setting, + new setting):\n%s", diff) } return nil diff --git a/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/resourceslicecontroller.go b/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/resourceslicecontroller.go index 72f46639b75..a9d213a0d38 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/resourceslicecontroller.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/resourceslicecontroller.go @@ -26,8 +26,6 @@ import ( "sync/atomic" "time" - "github.com/google/go-cmp/cmp" //nolint:depguard - v1 "k8s.io/api/core/v1" resourceapi "k8s.io/api/resource/v1beta2" apiequality "k8s.io/apimachinery/pkg/api/equality" @@ -37,6 +35,7 @@ import ( "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/diff" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/sets" watch "k8s.io/apimachinery/pkg/watch" @@ -456,7 +455,7 @@ func (c *Controller) initInformer(ctx context.Context) error { return } if loggerV := logger.V(6); loggerV.Enabled() { - loggerV.Info("ResourceSlice update", "slice", klog.KObj(newSlice), "diff", cmp.Diff(oldSlice, newSlice)) + loggerV.Info("ResourceSlice update", "slice", klog.KObj(newSlice), "diff", diff.Diff(oldSlice, newSlice)) } else { logger.V(5).Info("ResourceSlice update", "slice", klog.KObj(newSlice)) } diff --git a/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker.go b/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker.go index 8a51409ff97..0d6f7dde1a2 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker.go @@ -23,12 +23,11 @@ import ( "slices" "sync" - "github.com/google/go-cmp/cmp" //nolint:depguard - v1 "k8s.io/api/core/v1" resourcealphaapi "k8s.io/api/resource/v1alpha3" resourceapi "k8s.io/api/resource/v1beta1" labels "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/diff" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/sets" resourcealphainformers "k8s.io/client-go/informers/resource/v1alpha3" @@ -405,7 +404,7 @@ func (t *Tracker) resourceSliceUpdate(ctx context.Context) func(oldObj, newObj a if loggerV := logger.V(6); loggerV.Enabled() { // While debugging, one needs a full dump of the objects for context *and* // a diff because otherwise small changes would be hard to spot. - loggerV.Info("ResourceSlice update", "slice", klog.Format(oldSlice), "oldSlice", klog.Format(newSlice), "diff", cmp.Diff(oldSlice, newSlice)) + loggerV.Info("ResourceSlice update", "slice", klog.Format(oldSlice), "oldSlice", klog.Format(newSlice), "diff", diff.Diff(oldSlice, newSlice)) } else { logger.V(5).Info("ResourceSlice update", "slice", klog.KObj(newSlice)) } @@ -454,7 +453,7 @@ func (t *Tracker) deviceTaintUpdate(ctx context.Context) func(oldObj, newObj any return } if loggerV := logger.V(6); loggerV.Enabled() { - loggerV.Info("DeviceTaintRule update", "patch", klog.KObj(newPatch), "diff", cmp.Diff(oldPatch, newPatch)) + loggerV.Info("DeviceTaintRule update", "patch", klog.KObj(newPatch), "diff", diff.Diff(oldPatch, newPatch)) } else { logger.V(5).Info("DeviceTaintRule update", "patch", klog.KObj(newPatch)) } @@ -514,7 +513,7 @@ func (t *Tracker) deviceClassUpdate(ctx context.Context) func(oldObj, newObj any return } if loggerV := logger.V(6); loggerV.Enabled() { - loggerV.Info("DeviceClass update", "class", klog.KObj(newClass), "diff", cmp.Diff(oldClass, newClass)) + loggerV.Info("DeviceClass update", "class", klog.KObj(newClass), "diff", diff.Diff(oldClass, newClass)) } else { logger.V(5).Info("DeviceClass update", "class", klog.KObj(newClass)) } @@ -618,7 +617,7 @@ func (t *Tracker) syncSlice(ctx context.Context, name string, sendEvent bool) { } if loggerV := logger.V(6); loggerV.Enabled() { - loggerV.Info("ResourceSlice synced", "diff", cmp.Diff(oldPatchedObj, patchedSlice)) + loggerV.Info("ResourceSlice synced", "diff", diff.Diff(oldPatchedObj, patchedSlice)) } else { logger.V(5).Info("ResourceSlice synced") } diff --git a/staging/src/k8s.io/kube-proxy/go.mod b/staging/src/k8s.io/kube-proxy/go.mod index ade38e35d54..f286393c420 100644 --- a/staging/src/k8s.io/kube-proxy/go.mod +++ b/staging/src/k8s.io/kube-proxy/go.mod @@ -15,6 +15,7 @@ require ( github.com/beorn7/perks v1.0.1 // indirect github.com/blang/semver/v4 v4.0.0 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect github.com/fxamacker/cbor/v2 v2.8.0 // indirect github.com/go-logr/logr v1.4.2 // indirect github.com/gogo/protobuf v1.3.2 // indirect @@ -24,6 +25,7 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_golang v1.22.0 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.62.0 // indirect diff --git a/staging/src/k8s.io/sample-controller/go.mod b/staging/src/k8s.io/sample-controller/go.mod index 4825a45172a..996103aec94 100644 --- a/staging/src/k8s.io/sample-controller/go.mod +++ b/staging/src/k8s.io/sample-controller/go.mod @@ -34,6 +34,7 @@ require ( github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/spf13/pflag v1.0.6 // indirect github.com/x448/float16 v0.8.4 // indirect golang.org/x/mod v0.21.0 // indirect