From dfb7f5d7e11feee3b5e6f090294efa946a634a0a Mon Sep 17 00:00:00 2001 From: Ilya Lobkov Date: Tue, 11 Jun 2024 13:24:01 +0200 Subject: [PATCH] fix(kuma-cp): mistakenly setting 'kuma.io/display-name' as label (#10430) fix(kuma-cp): mistakenly setting 'kuma.io/display-name' as label while it should an annotation due to the length limit Signed-off-by: Ilya Lobkov --- pkg/plugins/resources/k8s/store.go | 12 +++- pkg/plugins/runtime/k8s/webhooks/defaulter.go | 13 ++++ .../runtime/k8s/webhooks/defaulter_test.go | 69 +++++++++++++++++++ 3 files changed, 92 insertions(+), 2 deletions(-) diff --git a/pkg/plugins/resources/k8s/store.go b/pkg/plugins/resources/k8s/store.go index af1d6496957d..054decc3d394 100644 --- a/pkg/plugins/resources/k8s/store.go +++ b/pkg/plugins/resources/k8s/store.go @@ -58,7 +58,7 @@ func (s *KubernetesStore) Create(ctx context.Context, r core_model.Resource, fs return err } - labels, annotations := splitLabelsAndAnnotations(opts.Labels, obj.GetAnnotations()) + labels, annotations := SplitLabelsAndAnnotations(opts.Labels, obj.GetAnnotations()) obj.GetObjectMeta().SetLabels(labels) obj.GetObjectMeta().SetAnnotations(annotations) obj.SetMesh(opts.Mesh) @@ -99,7 +99,15 @@ func (s *KubernetesStore) Update(ctx context.Context, r core_model.Resource, fs return errors.Wrapf(err, "failed to convert core model of type %s into k8s counterpart", r.Descriptor().Name) } +<<<<<<< HEAD labels, annotations := splitLabelsAndAnnotations(opts.Labels, obj.GetAnnotations()) +======= + updateLabels := r.GetMeta().GetLabels() + if opts.ModifyLabels { + updateLabels = opts.Labels + } + labels, annotations := SplitLabelsAndAnnotations(updateLabels, obj.GetAnnotations()) +>>>>>>> da824ce57 (fix(kuma-cp): mistakenly setting 'kuma.io/display-name' as label (#10430)) obj.GetObjectMeta().SetLabels(labels) obj.GetObjectMeta().SetAnnotations(annotations) obj.SetMesh(r.GetMeta().GetMesh()) @@ -237,7 +245,7 @@ func k8sNameNamespace(coreName string, scope k8s_model.Scope) (string, string, e // Kuma resource labels are generally stored on Kubernetes as labels, except "kuma.io/display-name". // We store it as an annotation because the resource name on k8s is limited by 253 and the label value is limited by 63. -func splitLabelsAndAnnotations(coreLabels map[string]string, currentAnnotations map[string]string) (map[string]string, map[string]string) { +func SplitLabelsAndAnnotations(coreLabels map[string]string, currentAnnotations map[string]string) (map[string]string, map[string]string) { labels := maps.Clone(coreLabels) annotations := maps.Clone(currentAnnotations) if annotations == nil { diff --git a/pkg/plugins/runtime/k8s/webhooks/defaulter.go b/pkg/plugins/runtime/k8s/webhooks/defaulter.go index 5701ea8c28e2..3421de7fe353 100644 --- a/pkg/plugins/runtime/k8s/webhooks/defaulter.go +++ b/pkg/plugins/runtime/k8s/webhooks/defaulter.go @@ -13,7 +13,11 @@ import ( core_model "github.com/kumahq/kuma/pkg/core/resources/model" "github.com/kumahq/kuma/pkg/core/resources/registry" k8s_common "github.com/kumahq/kuma/pkg/plugins/common/k8s" +<<<<<<< HEAD "github.com/kumahq/kuma/pkg/plugins/runtime/k8s/metadata" +======= + "github.com/kumahq/kuma/pkg/plugins/resources/k8s" +>>>>>>> da824ce57 (fix(kuma-cp): mistakenly setting 'kuma.io/display-name' as label (#10430)) ) type Defaulter interface { @@ -72,6 +76,7 @@ func (h *defaultingHandler) Handle(ctx context.Context, req admission.Request) a if resp := h.IsOperationAllowed(req.UserInfo, resource); !resp.Allowed { return resp } +<<<<<<< HEAD if resource.Descriptor().Scope == core_model.ScopeMesh { labels := obj.GetLabels() @@ -94,6 +99,14 @@ func (h *defaultingHandler) Handle(ctx context.Context, req admission.Request) a obj.SetLabels(labels) } } +======= + labels, annotations := k8s.SplitLabelsAndAnnotations( + core_model.ComputeLabels(resource, h.Mode, true, h.SystemNamespace), + obj.GetAnnotations(), + ) + obj.SetLabels(labels) + obj.SetAnnotations(annotations) +>>>>>>> da824ce57 (fix(kuma-cp): mistakenly setting 'kuma.io/display-name' as label (#10430)) marshaled, err := json.Marshal(obj) if err != nil { diff --git a/pkg/plugins/runtime/k8s/webhooks/defaulter_test.go b/pkg/plugins/runtime/k8s/webhooks/defaulter_test.go index c9a30d064cb5..81f03481f34f 100644 --- a/pkg/plugins/runtime/k8s/webhooks/defaulter_test.go +++ b/pkg/plugins/runtime/k8s/webhooks/defaulter_test.go @@ -118,7 +118,14 @@ var _ = Describe("Defaulter", func() { "kind": "Mesh", "metadata": { "name": "empty", +<<<<<<< HEAD "creationTimestamp": null +======= + "creationTimestamp": null, + "annotations": { + "kuma.io/display-name": "empty" + } +>>>>>>> da824ce57 (fix(kuma-cp): mistakenly setting 'kuma.io/display-name' as label (#10430)) }, "spec": { "metrics": { @@ -171,7 +178,14 @@ var _ = Describe("Defaulter", func() { "kind": "Mesh", "metadata": { "name": "empty", +<<<<<<< HEAD "creationTimestamp": null +======= + "creationTimestamp": null, + "annotations": { + "kuma.io/display-name": "empty" + } +>>>>>>> da824ce57 (fix(kuma-cp): mistakenly setting 'kuma.io/display-name' as label (#10430)) }, "spec": { "metrics": { @@ -220,7 +234,15 @@ var _ = Describe("Defaulter", func() { "name": "empty", "creationTimestamp": null, "labels": { +<<<<<<< HEAD "kuma.io/mesh": "my-mesh-1" +======= + "kuma.io/mesh": "my-mesh-1", + "k8s.kuma.io/namespace": "example" + }, + "annotations": { + "kuma.io/display-name": "empty" +>>>>>>> da824ce57 (fix(kuma-cp): mistakenly setting 'kuma.io/display-name' as label (#10430)) } }, "spec": {} @@ -257,7 +279,16 @@ var _ = Describe("Defaulter", func() { "creationTimestamp": null, "labels": { "kuma.io/origin": "zone", +<<<<<<< HEAD "kuma.io/mesh": "default" +======= + "kuma.io/mesh": "default", + "kuma.io/policy-role": "workload-owner", + "k8s.kuma.io/namespace": "example" + }, + "annotations": { + "kuma.io/display-name": "empty" +>>>>>>> da824ce57 (fix(kuma-cp): mistakenly setting 'kuma.io/display-name' as label (#10430)) } }, "spec": { @@ -292,8 +323,18 @@ var _ = Describe("Defaulter", func() { "name": "empty", "creationTimestamp": null, "labels": { +<<<<<<< HEAD "kuma.io/origin": "zone", "kuma.io/mesh": "default" +======= + "k8s.kuma.io/namespace": "example", + "kuma.io/mesh": "default", + "kuma.io/origin": "zone", + "kuma.io/policy-role": "workload-owner" + }, + "annotations": { + "kuma.io/display-name": "empty" +>>>>>>> da824ce57 (fix(kuma-cp): mistakenly setting 'kuma.io/display-name' as label (#10430)) } }, "spec": { @@ -328,8 +369,18 @@ var _ = Describe("Defaulter", func() { "name": "empty", "creationTimestamp": null, "labels": { +<<<<<<< HEAD "kuma.io/origin": "zone", "kuma.io/mesh": "default" +======= + "k8s.kuma.io/namespace": "example", + "kuma.io/mesh": "default", + "kuma.io/origin": "zone", + "kuma.io/policy-role": "workload-owner" + }, + "annotations": { + "kuma.io/display-name": "empty" +>>>>>>> da824ce57 (fix(kuma-cp): mistakenly setting 'kuma.io/display-name' as label (#10430)) } }, "spec": { @@ -375,8 +426,17 @@ var _ = Describe("Defaulter", func() { "name":"empty", "creationTimestamp":null, "labels": { +<<<<<<< HEAD "kuma.io/origin": "zone", "kuma.io/mesh": "default" +======= + "k8s.kuma.io/namespace": "example", + "kuma.io/mesh": "default", + "kuma.io/origin": "zone" + }, + "annotations": { + "kuma.io/display-name": "empty" +>>>>>>> da824ce57 (fix(kuma-cp): mistakenly setting 'kuma.io/display-name' as label (#10430)) } }, "spec":{ @@ -420,7 +480,16 @@ var _ = Describe("Defaulter", func() { "name": "empty", "creationTimestamp": null, "labels": { +<<<<<<< HEAD "kuma.io/mesh": "default" +======= + "k8s.kuma.io/namespace": "example", + "kuma.io/mesh": "default", + "kuma.io/policy-role": "workload-owner" + }, + "annotations": { + "kuma.io/display-name": "empty" +>>>>>>> da824ce57 (fix(kuma-cp): mistakenly setting 'kuma.io/display-name' as label (#10430)) } }, "spec": {