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 046613c commit 139c9c6
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 13 deletions.
8 changes: 0 additions & 8 deletions pkg/kds/util/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,6 @@ func CloneResourceMeta(m model.ResourceMeta, fs ...CloneResourceMetaOpt) model.R
return meta
}

func kumaResourceMetaToResourceMeta(meta *mesh_proto.KumaResource_Meta) model.ResourceMeta {
return &resourceMeta{
name: meta.Name,
mesh: meta.Mesh,
labels: meta.GetLabels(),
}
}

func (r *resourceMeta) GetName() string {
return r.name
}
Expand Down
16 changes: 15 additions & 1 deletion 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 @@ -56,7 +57,7 @@ func ToEnvoyResources(rlist model.ResourceList) ([]envoy_types.Resource, error)
Meta: &mesh_proto.KumaResource_Meta{
Name: r.GetMeta().GetName(),
Mesh: r.GetMeta().GetMesh(),
Labels: r.GetMeta().GetLabels(),
Labels: maps.Clone(r.GetMeta().GetLabels()),
Version: "",
},
Spec: pbany,
Expand Down Expand Up @@ -136,7 +137,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
5 changes: 3 additions & 2 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 @@ -131,7 +132,7 @@ func (c *memoryStore) Create(_ context.Context, r core_model.Resource, fs ...sto
Version: initialVersion(),
CreationTime: opts.CreationTime,
ModificationTime: opts.CreationTime,
Labels: opts.Labels,
Labels: maps.Clone(opts.Labels),
}

// fill the meta
Expand Down Expand Up @@ -185,7 +186,7 @@ func (c *memoryStore) Update(_ context.Context, r core_model.Resource, fs ...sto
meta.Version = meta.Version.Next()
meta.ModificationTime = opts.ModificationTime
if opts.ModifyLabels {
meta.Labels = opts.Labels
meta.Labels = maps.Clone(opts.Labels)
}
r.SetMeta(meta)

Expand Down
5 changes: 3 additions & 2 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 @@ -42,7 +43,7 @@ func (s *remoteStore) Create(ctx context.Context, res model.Resource, fs ...stor
Type: string(res.Descriptor().Name),
Name: opts.Name,
Mesh: opts.Mesh,
Labels: opts.Labels,
Labels: maps.Clone(opts.Labels),
}
if err := s.upsert(ctx, res, meta); err != nil {
return err
Expand All @@ -56,7 +57,7 @@ func (s *remoteStore) Update(ctx context.Context, res model.Resource, fs ...stor
Type: string(res.Descriptor().Name),
Name: res.GetMeta().GetName(),
Mesh: res.GetMeta().GetMesh(),
Labels: opts.Labels,
Labels: maps.Clone(opts.Labels),
}
if err := s.upsert(ctx, res, meta); err != nil {
return err
Expand Down
1 change: 1 addition & 0 deletions pkg/plugins/secrets/k8s/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ func (s *KubernetesStore) Update(ctx context.Context, r core_model.Resource, fs
}

func setLabelsAnnotationsAndMesh(s *kube_core.Secret, mesh string, labels map[string]string) {
labels = maps.Clone(labels)
if labels == nil {
labels = map[string]string{}
}
Expand Down

0 comments on commit 139c9c6

Please sign in to comment.