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 a889d98 commit 6730c4f
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

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

View workflow job for this annotation

GitHub Actions / check

syntax error: unexpected <<, expected }

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

View workflow job for this annotation

GitHub Actions / check

expected statement, found '<<' (typecheck)

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

View workflow job for this annotation

GitHub Actions / check

syntax error: unexpected <<, expected }

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

View workflow job for this annotation

GitHub Actions / test

syntax error: unexpected <<, expected }

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

View workflow job for this annotation

GitHub Actions / test_e2e_env (kubernetes, v1.28.1-k3s1, amd64, 1, flannel, false) / e2e (0)

syntax error: unexpected <<, expected }

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

View workflow job for this annotation

GitHub Actions / test_e2e_env (universal, kind, amd64, 1, flannel, false) / e2e (0)

syntax error: unexpected <<, expected }

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

View workflow job for this annotation

GitHub Actions / test_e2e_env (multizone, v1.28.1-k3s1, amd64, 1, flannel, false) / e2e (0)

syntax error: unexpected <<, expected }
obj.SetMeta(kumaResourceMetaToResourceMeta(kr.Meta))
=======

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

View workflow job for this annotation

GitHub Actions / check

syntax error: unexpected ==, expected }

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

View workflow job for this annotation

GitHub Actions / check

syntax error: unexpected ==, expected }

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

View workflow job for this annotation

GitHub Actions / test

syntax error: unexpected ==, expected }

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

View workflow job for this annotation

GitHub Actions / test_e2e_env (kubernetes, v1.28.1-k3s1, amd64, 1, flannel, false) / e2e (0)

syntax error: unexpected ==, expected }

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

View workflow job for this annotation

GitHub Actions / test_e2e_env (universal, kind, amd64, 1, flannel, false) / e2e (0)

syntax error: unexpected ==, expected }

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

View workflow job for this annotation

GitHub Actions / test_e2e_env (multizone, v1.28.1-k3s1, amd64, 1, flannel, false) / e2e (0)

syntax error: unexpected ==, expected }
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))

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

View workflow job for this annotation

GitHub Actions / check

invalid character U+0023 '#' (typecheck)

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

View workflow job for this annotation

GitHub Actions / check

expected statement, found '>>' (typecheck)

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

View workflow job for this annotation

GitHub Actions / check

invalid character U+0023 '#') (typecheck)

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

View workflow job for this annotation

GitHub Actions / test

invalid character U+0023 '#'

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

View workflow job for this annotation

GitHub Actions / test_e2e_env (kubernetes, v1.28.1-k3s1, amd64, 1, flannel, false) / e2e (0)

invalid character U+0023 '#'

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

View workflow job for this annotation

GitHub Actions / test_e2e_env (universal, kind, amd64, 1, flannel, false) / e2e (0)

invalid character U+0023 '#'

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

View workflow job for this annotation

GitHub Actions / test_e2e_env (multizone, v1.28.1-k3s1, amd64, 1, flannel, false) / e2e (0)

invalid character U+0023 '#'
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 @@ -123,7 +124,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 @@ -180,7 +181,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 @@ -41,7 +42,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 @@ -55,7 +56,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 6730c4f

Please sign in to comment.