Skip to content

Commit

Permalink
fix(kuma-cp): avoid concurrent access on resource meta (#11997)
Browse files Browse the repository at this point in the history
There were some places where labels on meta weren't not cloned when
duplicating the meta. Walked the code for odd usages of `GetLabels()`.

See also #11886

Signed-off-by: Charly Molter <[email protected]>
Signed-off-by: Mike Beaumont <[email protected]>
Co-authored-by: Mike Beaumont <[email protected]>
  • Loading branch information
2 people authored and kumahq[bot] committed Nov 12, 2024
1 parent cc04363 commit 82d25dc
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 0 deletions.
64 changes: 64 additions & 0 deletions pkg/kds/util/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,77 @@ func CloneResourceMetaWithNewName(meta model.ResourceMeta, name string) model.Re
}
}

<<<<<<< HEAD

Check failure on line 31 in pkg/kds/util/meta.go

View workflow job for this annotation

GitHub Actions / lint

syntax error: non-declaration statement outside function body
func kumaResourceMetaToResourceMeta(meta *mesh_proto.KumaResource_Meta) model.ResourceMeta {
return &resourceMeta{
name: meta.Name,
mesh: meta.Mesh,
}
}

=======

Check failure on line 39 in pkg/kds/util/meta.go

View workflow job for this annotation

GitHub Actions / lint

syntax error: non-declaration statement outside function body
// PopulateNamespaceLabelFromNameExtension on Kubernetes zones adds 'k8s.kuma.io/namespace' label to the resources
// before syncing them to Global.
//
// In 2.7.x method 'GetMeta().GetLabels()' on Kubernetes returned a label map with 'k8s.kuma.io/namespace' added
// dynamically. This behavior was changed in 2.9.x by https://github.com/kumahq/kuma/pull/11020, the namespace label is now
// supposed to be set in ComputeLabels function. But this functions is called only on Create/Update of the resources.
// This means policies that were created on 2.7.x won't have 'k8s.kuma.io/namespace' label when synced to Global.
// Even though the lack of namespace labels affects only how resource looks in GUI on Global it's still worth setting it.
func PopulateNamespaceLabelFromNameExtension() CloneResourceMetaOpt {
return func(m *resourceMeta) {
namespace := m.nameExtensions[model.K8sNamespaceComponent]
if _, ok := m.labels[mesh_proto.KubeNamespaceTag]; !ok && namespace != "" {
m.labels[mesh_proto.KubeNamespaceTag] = namespace
}
}
}

func WithoutLabel(key string) CloneResourceMetaOpt {
return func(m *resourceMeta) {
delete(m.labels, key)
}
}

func If(condition func(resource model.ResourceMeta) bool, fn CloneResourceMetaOpt) CloneResourceMetaOpt {
return func(meta *resourceMeta) {
if condition(meta) {
fn(meta)
}
}
}

func IsKubernetes(storeType config_store.StoreType) func(model.ResourceMeta) bool {
return func(_ model.ResourceMeta) bool {
return storeType == config_store.KubernetesStore
}
}

func CloneResourceMeta(m model.ResourceMeta, fs ...CloneResourceMetaOpt) model.ResourceMeta {
labels := maps.Clone(m.GetLabels())
if labels == nil {
labels = map[string]string{}
}
ne := maps.Clone(m.GetNameExtensions())
if ne == nil {
ne = model.ResourceNameExtensions{}
}
meta := &resourceMeta{
name: m.GetName(),
mesh: m.GetMesh(),
labels: labels,
nameExtensions: ne,
}
for _, f := range fs {
f(meta)
}
if len(meta.labels) == 0 {
meta.labels = nil
}
return meta
}

>>>>>>> c3d7187c7 (fix(kuma-cp): avoid concurrent access on resource meta (#11997))

Check failure on line 101 in pkg/kds/util/meta.go

View workflow job for this annotation

GitHub Actions / lint

syntax error: non-declaration statement outside function body

Check failure on line 101 in pkg/kds/util/meta.go

View workflow job for this annotation

GitHub Actions / lint

invalid character U+0023 '#'
func (r *resourceMeta) GetName() string {
return r.name
}
Expand Down
18 changes: 18 additions & 0 deletions pkg/kds/util/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package util

import (
"fmt"
"maps"
"strings"

envoy_sd "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3"
Expand Down Expand Up @@ -53,6 +54,10 @@ func ToEnvoyResources(rlist model.ResourceList) ([]envoy_types.Resource, error)
Meta: &mesh_proto.KumaResource_Meta{
Name: r.GetMeta().GetName(),
Mesh: r.GetMeta().GetMesh(),
<<<<<<< HEAD

Check failure on line 57 in pkg/kds/util/resource.go

View workflow job for this annotation

GitHub Actions / lint

syntax error: unexpected <<, expected expression
=======
Labels: maps.Clone(r.GetMeta().GetLabels()),

Check failure on line 59 in pkg/kds/util/resource.go

View workflow job for this annotation

GitHub Actions / lint

syntax error: unexpected ) in composite literal; possibly missing comma or }
>>>>>>> c3d7187c7 (fix(kuma-cp): avoid concurrent access on resource meta (#11997))

Check failure on line 60 in pkg/kds/util/resource.go

View workflow job for this annotation

GitHub Actions / lint

invalid character U+0023 '#'
Version: "",
},

Check failure on line 62 in pkg/kds/util/resource.go

View workflow job for this annotation

GitHub Actions / lint

syntax error: unexpected }, expected expression
Spec: pbany,
Expand Down Expand Up @@ -140,7 +145,20 @@ func toResources(resourceType model.ResourceType, krs []*mesh_proto.KumaResource
if err = model.FromAny(kr.Spec, obj.GetSpec()); err != nil {
return nil, err
}
<<<<<<< HEAD
obj.SetMeta(kumaResourceMetaToResourceMeta(kr.Meta))
=======
if obj.Descriptor().HasStatus && kr.Status != nil {
if err = model.FromAny(kr.Status, obj.GetStatus()); err != nil {
return nil, err
}
}
obj.SetMeta(&resourceMeta{
name: kr.GetMeta().GetName(),
mesh: kr.GetMeta().GetMesh(),
labels: maps.Clone(kr.GetMeta().GetLabels()),
})
>>>>>>> c3d7187c7 (fix(kuma-cp): avoid concurrent access on resource meta (#11997))
if err := list.AddItem(obj); err != nil {
return nil, err
}
Expand Down
11 changes: 11 additions & 0 deletions pkg/plugins/resources/memory/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package memory
import (
"context"
"fmt"
"maps"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -112,6 +113,10 @@ func (c *memoryStore) Create(_ context.Context, r core_model.Resource, fs ...sto
Version: initialVersion(),
CreationTime: opts.CreationTime,
ModificationTime: opts.CreationTime,
<<<<<<< HEAD
=======
Labels: maps.Clone(opts.Labels),
>>>>>>> c3d7187c7 (fix(kuma-cp): avoid concurrent access on resource meta (#11997))
}

// fill the meta
Expand Down Expand Up @@ -167,6 +172,12 @@ func (c *memoryStore) Update(_ context.Context, r core_model.Resource, fs ...sto
}
meta.Version = meta.Version.Next()
meta.ModificationTime = opts.ModificationTime
<<<<<<< HEAD
=======
if opts.ModifyLabels {
meta.Labels = maps.Clone(opts.Labels)
}
>>>>>>> c3d7187c7 (fix(kuma-cp): avoid concurrent access on resource meta (#11997))
r.SetMeta(meta)

record.Version = meta.Version
Expand Down
15 changes: 15 additions & 0 deletions pkg/plugins/resources/remote/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"encoding/json"
"io"
"maps"
"net/http"
"strconv"

Expand Down Expand Up @@ -38,9 +39,16 @@ type remoteStore struct {
func (s *remoteStore) Create(ctx context.Context, res model.Resource, fs ...store.CreateOptionsFunc) error {
opts := store.NewCreateOptions(fs...)
meta := rest_v1alpha1.ResourceMeta{
<<<<<<< HEAD
Type: string(res.Descriptor().Name),
Name: opts.Name,
Mesh: opts.Mesh,
=======
Type: string(res.Descriptor().Name),
Name: opts.Name,
Mesh: opts.Mesh,
Labels: maps.Clone(opts.Labels),
>>>>>>> c3d7187c7 (fix(kuma-cp): avoid concurrent access on resource meta (#11997))
}
if err := s.upsert(ctx, res, meta); err != nil {
return err
Expand All @@ -50,9 +58,16 @@ func (s *remoteStore) Create(ctx context.Context, res model.Resource, fs ...stor

func (s *remoteStore) Update(ctx context.Context, res model.Resource, fs ...store.UpdateOptionsFunc) error {
meta := rest_v1alpha1.ResourceMeta{
<<<<<<< HEAD
Type: string(res.Descriptor().Name),
Name: res.GetMeta().GetName(),
Mesh: res.GetMeta().GetMesh(),
=======
Type: string(res.Descriptor().Name),
Name: res.GetMeta().GetName(),
Mesh: res.GetMeta().GetMesh(),
Labels: maps.Clone(opts.Labels),
>>>>>>> c3d7187c7 (fix(kuma-cp): avoid concurrent access on resource meta (#11997))
}
if err := s.upsert(ctx, res, meta); err != nil {
return err
Expand Down
17 changes: 17 additions & 0 deletions pkg/plugins/secrets/k8s/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,23 @@ func (s *KubernetesStore) Update(ctx context.Context, r core_model.Resource, fs
return nil
}

<<<<<<< HEAD
=======
func setLabelsAnnotationsAndMesh(s *kube_core.Secret, mesh string, labels map[string]string) {
labels = maps.Clone(labels)
if labels == nil {
labels = map[string]string{}
}
if mesh != "" {
labels[metadata.KumaMeshLabel] = mesh
}

labels, annotations := k8s.SplitLabelsAndAnnotations(labels, s.GetAnnotations())
s.GetObjectMeta().SetLabels(labels)
s.GetObjectMeta().SetAnnotations(annotations)
}

>>>>>>> c3d7187c7 (fix(kuma-cp): avoid concurrent access on resource meta (#11997))
func (s *KubernetesStore) Delete(ctx context.Context, r core_model.Resource, fs ...core_store.DeleteOptionsFunc) error {
opts := core_store.NewDeleteOptions(fs...)
if err := s.Get(ctx, r, core_store.GetByKey(opts.Name, opts.Mesh)); err != nil {
Expand Down

0 comments on commit 82d25dc

Please sign in to comment.