From dfdf15936093f82dfa6e5a12fda15f2e9e0f15f2 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Fri, 31 May 2024 21:25:25 -0400 Subject: [PATCH] Handle unstructured objects correctly in IgnoreManagedFieldsTimestampsTransformer Kubernetes-commit: c942ab6900ddb7b6e3e7c550c521409693180968 --- .../handlers/fieldmanager/equality.go | 39 +++- .../handlers/fieldmanager/equality_test.go | 173 ++++++++++++++++++ 2 files changed, 211 insertions(+), 1 deletion(-) create mode 100644 pkg/endpoints/handlers/fieldmanager/equality_test.go diff --git a/pkg/endpoints/handlers/fieldmanager/equality.go b/pkg/endpoints/handlers/fieldmanager/equality.go index 2f0bb4e48..09afc2609 100644 --- a/pkg/endpoints/handlers/fieldmanager/equality.go +++ b/pkg/endpoints/handlers/fieldmanager/equality.go @@ -52,7 +52,7 @@ func getAvoidTimestampEqualities() conversion.Equalities { } var eqs = equality.Semantic.Copy() - err := eqs.AddFunc( + err := eqs.AddFuncs( func(a, b metav1.ManagedFieldsEntry) bool { // Two objects' managed fields are equivalent if, ignoring timestamp, // the objects are deeply equal. @@ -60,6 +60,14 @@ func getAvoidTimestampEqualities() conversion.Equalities { b.Time = nil return reflect.DeepEqual(a, b) }, + func(a, b unstructured.Unstructured) bool { + // Check if the managed fields are equal by converting to structured types and leveraging the above + // function, then, ignoring the managed fields, equality check the rest of the unstructured data. + if !avoidTimestampEqualities.DeepEqual(a.GetManagedFields(), b.GetManagedFields()) { + return false + } + return equalIgnoringValueAtPath(a.Object, b.Object, []string{"metadata", "managedFields"}) + }, ) if err != nil { @@ -71,6 +79,35 @@ func getAvoidTimestampEqualities() conversion.Equalities { return avoidTimestampEqualities } +func equalIgnoringValueAtPath(a, b any, path []string) bool { + if len(path) == 0 { // found the value to ignore + return true + } + aMap, aOk := a.(map[string]any) + bMap, bOk := b.(map[string]any) + if !aOk || !bOk { + return !avoidTimestampEqualities.DeepEqual(a, b) + } + if len(aMap) != len(bMap) { + return false + } + pathHead := path[0] + for k, aVal := range aMap { + bVal, ok := bMap[k] + if !ok { + return false + } + if k == pathHead { + if !equalIgnoringValueAtPath(aVal, bVal, path[1:]) { + return false + } + } else if !avoidTimestampEqualities.DeepEqual(aVal, bVal) { + return false + } + } + return true +} + // IgnoreManagedFieldsTimestampsTransformer reverts timestamp updates // if the non-managed parts of the object are equivalent func IgnoreManagedFieldsTimestampsTransformer( diff --git a/pkg/endpoints/handlers/fieldmanager/equality_test.go b/pkg/endpoints/handlers/fieldmanager/equality_test.go new file mode 100644 index 000000000..04c328f64 --- /dev/null +++ b/pkg/endpoints/handlers/fieldmanager/equality_test.go @@ -0,0 +1,173 @@ +/* +Copyright 20214The 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 fieldmanager + +import "testing" + +func TestEqualIgnoringFieldValueAtPath(t *testing.T) { + cases := []struct { + name string + a, b map[string]any + want bool + }{ + { + name: "identical objects", + a: map[string]any{ + "metadata": map[string]any{ + "labels": map[string]any{"env": "dev"}, + "managedFields": []any{}, + }, + "spec": map[string]any{ + "field": "value", + }, + }, + b: map[string]any{ + "metadata": map[string]any{ + "labels": map[string]any{"env": "dev"}, + "managedFields": []any{}, + }, + "spec": map[string]any{ + "field": "value", + }, + }, + want: true, + }, + { + name: "different metadata label value", + a: map[string]any{ + "metadata": map[string]any{ + "labels": map[string]any{"env": "dev"}, + "managedFields": []any{}, + }, + "spec": map[string]any{ + "field": "value", + }, + }, + b: map[string]any{ + "metadata": map[string]any{ + "labels": map[string]any{"env": "prod"}, + "managedFields": []any{}, + }, + "spec": map[string]any{ + "field": "value", + }, + }, + want: false, + }, + { + name: "different spec value", + a: map[string]any{ + "metadata": map[string]any{ + "labels": map[string]any{"env": "dev"}, + "managedFields": []any{}, + }, + "spec": map[string]any{ + "field": "value1", + }, + }, + b: map[string]any{ + "metadata": map[string]any{ + "labels": map[string]any{"env": "dev"}, + "managedFields": []any{}, + }, + "spec": map[string]any{ + "field": "value2", + }, + }, + want: false, + }, + { + name: "extra spec fields in object a", + a: map[string]any{ + "metadata": map[string]any{ + "labels": map[string]any{"env": "dev"}, + "managedFields": []any{}, + }, + "spec": map[string]any{ + "field": "value1", + "otherField": "other", + }, + }, + b: map[string]any{ + "metadata": map[string]any{ + "labels": map[string]any{"env": "dev"}, + "managedFields": []any{}, + }, + "spec": map[string]any{ + "field": "value1", + }, + }, + want: false, + }, + { + name: "different spec field in object b", + a: map[string]any{ + "metadata": map[string]any{ + "labels": map[string]any{"env": "dev"}, + "managedFields": []any{}, + }, + "spec": map[string]any{ + "field": "value1", + }, + }, + b: map[string]any{ + "metadata": map[string]any{ + "labels": map[string]any{"env": "dev"}, + "managedFields": []any{}, + }, + "spec": map[string]any{ + "field": "value1", + "otherField": "other", + }, + }, + want: false, + }, + { + name: "different managed fields should be ignored", + a: map[string]any{ + "metadata": map[string]any{ + "labels": map[string]any{"env": "dev"}, + "managedFields": []any{map[string]any{"manager": "client1"}}, + }, + "spec": map[string]any{ + "field": "value", + }, + }, + b: map[string]any{ + "metadata": map[string]any{ + "labels": map[string]any{"env": "dev"}, + "managedFields": []any{map[string]any{"manager": "client2"}}, + }, + "spec": map[string]any{ + "field": "value", + }, + }, + want: true, + }, + } + + path := []string{"metadata", "managedFields"} + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + actual := equalIgnoringValueAtPath(c.a, c.b, path) + if actual != c.want { + t.Error("Expected equality check to return ", c.want, ", but got ", actual) + } + }) + } +}