From d267f42ddc31829f4dc6557774ec810f7e5deb0c Mon Sep 17 00:00:00 2001 From: Mike Beaumont Date: Wed, 30 Oct 2024 17:03:03 +0100 Subject: [PATCH 1/3] fix(k8s): set annotation kuma.io/display-name for Secrets and Configs (#11923) Otherwise we have issues if the Secret name is too long, already handled for other resources in https://github.com/kumahq/kuma/pull/10430 Also this internally fixes `UpdateWithLabels` not being applied for secrets. Signed-off-by: Mike Beaumont --- pkg/plugins/config/k8s/store.go | 32 +++++++++++++++++++++ pkg/plugins/config/k8s/store_test.go | 17 ++++++++++-- pkg/plugins/secrets/k8s/store.go | 40 +++++++++++++++++++++------ pkg/plugins/secrets/k8s/store_test.go | 14 ++++++++-- 4 files changed, 90 insertions(+), 13 deletions(-) diff --git a/pkg/plugins/config/k8s/store.go b/pkg/plugins/config/k8s/store.go index e5e83f5da985..43f9c31e8350 100644 --- a/pkg/plugins/config/k8s/store.go +++ b/pkg/plugins/config/k8s/store.go @@ -2,6 +2,7 @@ package k8s import ( "context" + "maps" "time" "github.com/pkg/errors" @@ -12,11 +13,13 @@ import ( kube_client "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "github.com/kumahq/kuma/api/mesh/v1alpha1" system_proto "github.com/kumahq/kuma/api/system/v1alpha1" config_model "github.com/kumahq/kuma/pkg/core/resources/apis/system" core_model "github.com/kumahq/kuma/pkg/core/resources/model" core_store "github.com/kumahq/kuma/pkg/core/resources/store" common_k8s "github.com/kumahq/kuma/pkg/plugins/common/k8s" + "github.com/kumahq/kuma/pkg/plugins/resources/k8s" ) var _ core_store.ResourceStore = &KubernetesStore{} @@ -62,6 +65,11 @@ func (s *KubernetesStore) Create(ctx context.Context, r core_model.Resource, fs configMapKey: configRes.Spec.Config, }, } + + labels, annotations := k8s.SplitLabelsAndAnnotations(opts.Labels, cm.GetAnnotations()) + cm.GetObjectMeta().SetLabels(labels) + cm.GetObjectMeta().SetAnnotations(annotations) + if opts.Owner != nil { k8sOwner, err := s.converter.ToKubernetesObject(opts.Owner) if err != nil { @@ -83,6 +91,7 @@ func (s *KubernetesStore) Update(ctx context.Context, r core_model.Resource, fs if !ok { return newInvalidTypeError() } + opts := core_store.NewUpdateOptions(fs...) cm := &kube_core.ConfigMap{ TypeMeta: kube_meta.TypeMeta{ Kind: "ConfigMap", @@ -94,6 +103,16 @@ func (s *KubernetesStore) Update(ctx context.Context, r core_model.Resource, fs configMapKey: configRes.Spec.Config, }, } + + updateLabels := cm.GetLabels() + if opts.ModifyLabels { + updateLabels = opts.Labels + } + + labels, annotations := k8s.SplitLabelsAndAnnotations(updateLabels, cm.GetAnnotations()) + cm.GetObjectMeta().SetLabels(labels) + cm.GetObjectMeta().SetAnnotations(annotations) + if err := s.client.Update(ctx, cm); err != nil { if kube_apierrs.IsConflict(err) { return core_store.ErrorResourceConflict(r.Descriptor().Name, r.GetMeta().GetName(), r.GetMeta().GetMesh()) @@ -192,6 +211,19 @@ func (m *KubernetesMetaAdapter) GetModificationTime() time.Time { return m.GetObjectMeta().GetCreationTimestamp().Time } +func (m *KubernetesMetaAdapter) GetLabels() map[string]string { + labels := maps.Clone(m.GetObjectMeta().GetLabels()) + if labels == nil { + labels = map[string]string{} + } + if displayName, ok := m.GetObjectMeta().GetAnnotations()[v1alpha1.DisplayName]; ok { + labels[v1alpha1.DisplayName] = displayName + } else { + labels[v1alpha1.DisplayName] = m.GetObjectMeta().GetName() + } + return labels +} + func newInvalidTypeError() error { return errors.New("resource has a wrong type") } diff --git a/pkg/plugins/config/k8s/store_test.go b/pkg/plugins/config/k8s/store_test.go index cf72ba60dc2c..3af73c72f313 100644 --- a/pkg/plugins/config/k8s/store_test.go +++ b/pkg/plugins/config/k8s/store_test.go @@ -12,6 +12,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/serializer" "sigs.k8s.io/controller-runtime/pkg/client" + mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1" system_proto "github.com/kumahq/kuma/api/system/v1alpha1" "github.com/kumahq/kuma/pkg/core" system_model "github.com/kumahq/kuma/pkg/core/resources/apis/system" @@ -91,13 +92,15 @@ var _ = Describe("KubernetesStore", func() { kind: ConfigMap metadata: name: "kuma-internal-config" - namespace : "kuma-system" + namespace: "kuma-system" data: config: "test" `).(*kube_core.ConfigMap) // when - err := s.Create(context.Background(), config, core_store.CreateByKey("kuma-internal-config", "")) + err := s.Create(context.Background(), config, core_store.CreateByKey("kuma-internal-config", ""), core_store.CreateWithLabels(map[string]string{ + mesh_proto.DisplayName: "kuma-internal-config", + })) // then Expect(err).ToNot(HaveOccurred()) @@ -108,6 +111,8 @@ var _ = Describe("KubernetesStore", func() { // then Expect(actual.Data).To(Equal(expected.Data)) + Expect(actual.GetLabels()).NotTo(HaveKey(mesh_proto.DisplayName)) + Expect(actual.GetAnnotations()).To(HaveKeyWithValue(mesh_proto.DisplayName, "kuma-internal-config")) }) }) @@ -120,6 +125,8 @@ var _ = Describe("KubernetesStore", func() { metadata: name: "kuma-internal-config" namespace : %s + annotations: + kuma.io/display-name: "kuma-internal-config" data: config: "test" `, ns)) @@ -130,7 +137,9 @@ var _ = Describe("KubernetesStore", func() { kind: ConfigMap metadata: name: "kuma-internal-config" - namespace : %s + namespace: %s + annotations: + kuma.io/display-name: "kuma-internal-config" data: config: "next test" `, ns)).(*kube_core.ConfigMap) @@ -156,6 +165,8 @@ var _ = Describe("KubernetesStore", func() { // then Expect(actual.Data).To(Equal(expected.Data)) + Expect(actual.GetLabels()).NotTo(HaveKey(mesh_proto.DisplayName)) + Expect(actual.GetAnnotations()).To(HaveKeyWithValue(mesh_proto.DisplayName, "kuma-internal-config")) }) It("should return error in case of resource conflict", func() { diff --git a/pkg/plugins/secrets/k8s/store.go b/pkg/plugins/secrets/k8s/store.go index 729997410417..d3f0f492af6f 100644 --- a/pkg/plugins/secrets/k8s/store.go +++ b/pkg/plugins/secrets/k8s/store.go @@ -3,6 +3,7 @@ package k8s import ( "context" "fmt" + "maps" "time" "github.com/pkg/errors" @@ -13,6 +14,7 @@ import ( kube_client "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "github.com/kumahq/kuma/api/mesh/v1alpha1" system_proto "github.com/kumahq/kuma/api/system/v1alpha1" secret_model "github.com/kumahq/kuma/pkg/core/resources/apis/system" core_model "github.com/kumahq/kuma/pkg/core/resources/model" @@ -56,9 +58,8 @@ func (s *KubernetesStore) Create(ctx context.Context, r core_model.Resource, fs } secret.Namespace = s.namespace secret.Name = opts.Name - if r.Descriptor().Scope == core_model.ScopeMesh { - setMesh(secret, opts.Mesh, opts.Labels) - } + + setLabelsAnnotationsAndMesh(secret, opts.Mesh, opts.Labels) if opts.Owner != nil { k8sOwner, err := s.resourcesConverter.ToKubernetesObject(opts.Owner) @@ -91,9 +92,14 @@ func (s *KubernetesStore) Update(ctx context.Context, r core_model.Resource, fs return errors.Wrap(err, "failed to convert core Secret into k8s counterpart") } secret.Namespace = s.namespace - if r.Descriptor().Scope == core_model.ScopeMesh { - setMesh(secret, r.GetMeta().GetMesh(), opts.Labels) + + updateLabels := r.GetMeta().GetLabels() + if opts.ModifyLabels { + updateLabels = opts.Labels } + + setLabelsAnnotationsAndMesh(secret, r.GetMeta().GetMesh(), updateLabels) + if err := s.writer.Update(ctx, secret); err != nil { if kube_apierrs.IsConflict(err) { return core_store.ErrorResourceConflict(r.Descriptor().Name, secret.Name, r.GetMeta().GetMesh()) @@ -107,12 +113,17 @@ func (s *KubernetesStore) Update(ctx context.Context, r core_model.Resource, fs return nil } -func setMesh(s *kube_core.Secret, mesh string, labels map[string]string) { +func setLabelsAnnotationsAndMesh(s *kube_core.Secret, mesh string, labels map[string]string) { if labels == nil { labels = map[string]string{} } - labels[metadata.KumaMeshLabel] = mesh - s.SetLabels(labels) + if mesh != "" { + labels[metadata.KumaMeshLabel] = mesh + } + + labels, annotations := k8s.SplitLabelsAndAnnotations(labels, s.GetAnnotations()) + s.GetObjectMeta().SetLabels(labels) + s.GetObjectMeta().SetAnnotations(annotations) } func (s *KubernetesStore) Delete(ctx context.Context, r core_model.Resource, fs ...core_store.DeleteOptionsFunc) error { @@ -236,6 +247,19 @@ func (m *KubernetesMetaAdapter) GetModificationTime() time.Time { return m.GetObjectMeta().GetCreationTimestamp().Time } +func (m *KubernetesMetaAdapter) GetLabels() map[string]string { + labels := maps.Clone(m.GetObjectMeta().GetLabels()) + if labels == nil { + labels = map[string]string{} + } + if displayName, ok := m.GetObjectMeta().GetAnnotations()[v1alpha1.DisplayName]; ok { + labels[v1alpha1.DisplayName] = displayName + } else { + labels[v1alpha1.DisplayName] = m.GetObjectMeta().GetName() + } + return labels +} + type Converter interface { ToKubernetesObject(resource core_model.Resource) (*kube_core.Secret, error) ToCoreResource(secret *kube_core.Secret, out core_model.Resource) error diff --git a/pkg/plugins/secrets/k8s/store_test.go b/pkg/plugins/secrets/k8s/store_test.go index f1273e74fc3a..bdaa9b5c6bc4 100644 --- a/pkg/plugins/secrets/k8s/store_test.go +++ b/pkg/plugins/secrets/k8s/store_test.go @@ -13,6 +13,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/serializer" "sigs.k8s.io/controller-runtime/pkg/client" + mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1" system_proto "github.com/kumahq/kuma/api/system/v1alpha1" "github.com/kumahq/kuma/pkg/core" core_system "github.com/kumahq/kuma/pkg/core/resources/apis/system" @@ -106,7 +107,9 @@ var _ = Describe("KubernetesStore", func() { `).(*kube_core.Secret) // when - err := rs.Create(context.Background(), secret, store.CreateByKey(name, "demo")) + err := rs.Create(context.Background(), secret, store.CreateByKey(name, "demo"), store.CreateWithLabels(map[string]string{ + mesh_proto.DisplayName: name, + })) // then Expect(err).ToNot(HaveOccurred()) @@ -122,6 +125,9 @@ var _ = Describe("KubernetesStore", func() { // then Expect(actual.Data).To(Equal(expected.Data)) Expect(actual.Type).To(Equal(expected.Type)) + Expect(actual.GetObjectMeta().GetLabels()).NotTo(HaveKey(mesh_proto.DisplayName)) + Expect(actual.GetObjectMeta().GetAnnotations()).To(HaveKeyWithValue(mesh_proto.DisplayName, name)) + // and Expect(actual.ObjectMeta.ResourceVersion).To(Equal(secret.Meta.GetVersion())) }) @@ -192,9 +198,11 @@ var _ = Describe("KubernetesStore", func() { name: %s labels: kuma.io/mesh: demo + annotations: + kuma.io/display-name: %s data: value: ZXhhbXBsZQ== # base64(example) -`, ns, name)) +`, ns, name, name)) backend.Create(initial) // and expected := backend.ParseYAML(` @@ -231,6 +239,8 @@ var _ = Describe("KubernetesStore", func() { // then Expect(actual.Data).To(Equal(expected.Data)) Expect(actual.Type).To(Equal(expected.Type)) + Expect(actual.GetObjectMeta().GetLabels()).NotTo(HaveKey(mesh_proto.DisplayName)) + Expect(actual.GetObjectMeta().GetAnnotations()).To(HaveKeyWithValue(mesh_proto.DisplayName, name)) // and Expect(actual.ObjectMeta.ResourceVersion).To(Equal(secret.Meta.GetVersion())) }) From 5c3fa2d630815b73019c5428b7a97ab36a5d15b6 Mon Sep 17 00:00:00 2001 From: Mike Beaumont Date: Wed, 30 Oct 2024 21:10:25 +0100 Subject: [PATCH 2/3] fix: update for 2.6 Signed-off-by: Mike Beaumont --- pkg/plugins/config/k8s/store.go | 8 +------- pkg/plugins/resources/k8s/store.go | 6 +++--- pkg/plugins/secrets/k8s/store.go | 7 +------ 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/pkg/plugins/config/k8s/store.go b/pkg/plugins/config/k8s/store.go index 43f9c31e8350..07445df2b6b1 100644 --- a/pkg/plugins/config/k8s/store.go +++ b/pkg/plugins/config/k8s/store.go @@ -91,7 +91,6 @@ func (s *KubernetesStore) Update(ctx context.Context, r core_model.Resource, fs if !ok { return newInvalidTypeError() } - opts := core_store.NewUpdateOptions(fs...) cm := &kube_core.ConfigMap{ TypeMeta: kube_meta.TypeMeta{ Kind: "ConfigMap", @@ -104,12 +103,7 @@ func (s *KubernetesStore) Update(ctx context.Context, r core_model.Resource, fs }, } - updateLabels := cm.GetLabels() - if opts.ModifyLabels { - updateLabels = opts.Labels - } - - labels, annotations := k8s.SplitLabelsAndAnnotations(updateLabels, cm.GetAnnotations()) + labels, annotations := k8s.SplitLabelsAndAnnotations(cm.GetLabels(), cm.GetAnnotations()) cm.GetObjectMeta().SetLabels(labels) cm.GetObjectMeta().SetAnnotations(annotations) diff --git a/pkg/plugins/resources/k8s/store.go b/pkg/plugins/resources/k8s/store.go index af1d6496957d..4aa6a1070a53 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,7 @@ 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) } - labels, annotations := splitLabelsAndAnnotations(opts.Labels, obj.GetAnnotations()) + labels, annotations := SplitLabelsAndAnnotations(opts.Labels, obj.GetAnnotations()) obj.GetObjectMeta().SetLabels(labels) obj.GetObjectMeta().SetAnnotations(annotations) obj.SetMesh(r.GetMeta().GetMesh()) @@ -237,7 +237,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/secrets/k8s/store.go b/pkg/plugins/secrets/k8s/store.go index d3f0f492af6f..6958ba741f06 100644 --- a/pkg/plugins/secrets/k8s/store.go +++ b/pkg/plugins/secrets/k8s/store.go @@ -93,12 +93,7 @@ func (s *KubernetesStore) Update(ctx context.Context, r core_model.Resource, fs } secret.Namespace = s.namespace - updateLabels := r.GetMeta().GetLabels() - if opts.ModifyLabels { - updateLabels = opts.Labels - } - - setLabelsAnnotationsAndMesh(secret, r.GetMeta().GetMesh(), updateLabels) + setLabelsAnnotationsAndMesh(secret, r.GetMeta().GetMesh(), opts.Labels) if err := s.writer.Update(ctx, secret); err != nil { if kube_apierrs.IsConflict(err) { From b505ea9b1a89e9f0a4d851a0e739218175d33e3a Mon Sep 17 00:00:00 2001 From: Mike Beaumont Date: Thu, 31 Oct 2024 17:28:07 +0100 Subject: [PATCH 3/3] fix: UpdateWithLabels for Secrets and Configs Signed-off-by: Mike Beaumont --- pkg/plugins/config/k8s/store.go | 8 +++++++- pkg/plugins/secrets/k8s/store.go | 6 +++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/pkg/plugins/config/k8s/store.go b/pkg/plugins/config/k8s/store.go index 07445df2b6b1..43f9c31e8350 100644 --- a/pkg/plugins/config/k8s/store.go +++ b/pkg/plugins/config/k8s/store.go @@ -91,6 +91,7 @@ func (s *KubernetesStore) Update(ctx context.Context, r core_model.Resource, fs if !ok { return newInvalidTypeError() } + opts := core_store.NewUpdateOptions(fs...) cm := &kube_core.ConfigMap{ TypeMeta: kube_meta.TypeMeta{ Kind: "ConfigMap", @@ -103,7 +104,12 @@ func (s *KubernetesStore) Update(ctx context.Context, r core_model.Resource, fs }, } - labels, annotations := k8s.SplitLabelsAndAnnotations(cm.GetLabels(), cm.GetAnnotations()) + updateLabels := cm.GetLabels() + if opts.ModifyLabels { + updateLabels = opts.Labels + } + + labels, annotations := k8s.SplitLabelsAndAnnotations(updateLabels, cm.GetAnnotations()) cm.GetObjectMeta().SetLabels(labels) cm.GetObjectMeta().SetAnnotations(annotations) diff --git a/pkg/plugins/secrets/k8s/store.go b/pkg/plugins/secrets/k8s/store.go index 6958ba741f06..fd30e075b72f 100644 --- a/pkg/plugins/secrets/k8s/store.go +++ b/pkg/plugins/secrets/k8s/store.go @@ -93,7 +93,11 @@ func (s *KubernetesStore) Update(ctx context.Context, r core_model.Resource, fs } secret.Namespace = s.namespace - setLabelsAnnotationsAndMesh(secret, r.GetMeta().GetMesh(), opts.Labels) + updateLabels := r.GetMeta().GetLabels() + if opts.ModifyLabels { + updateLabels = opts.Labels + } + setLabelsAnnotationsAndMesh(secret, r.GetMeta().GetMesh(), updateLabels) if err := s.writer.Update(ctx, secret); err != nil { if kube_apierrs.IsConflict(err) {