From ff7c0edd64aab6cf0cfd8b92c933fbe8b1fd2ed3 Mon Sep 17 00:00:00 2001 From: Georges Chaudy Date: Sat, 13 Jul 2024 01:01:24 +0200 Subject: [PATCH] ResourceServer: Update twice should return an ErrOptimisticLockingFailed (#90378) Update twice should return an ErrOptimisticLockingFailed --- pkg/storage/unified/resource/server.go | 4 ++ pkg/storage/unified/resource/server_test.go | 53 +++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/pkg/storage/unified/resource/server.go b/pkg/storage/unified/resource/server.go index 0f455ca68a5..a151b57bdf4 100644 --- a/pkg/storage/unified/resource/server.go +++ b/pkg/storage/unified/resource/server.go @@ -338,6 +338,10 @@ func (s *server) Update(ctx context.Context, req *UpdateRequest) (*UpdateRespons return nil, apierrors.NewBadRequest("current value does not exist") } + if req.ResourceVersion > 0 && latest.ResourceVersion != req.ResourceVersion { + return nil, ErrOptimisticLockingFailed + } + builder, err := s.newEventBuilder(ctx, req.Key, req.Value, latest.Value) if err != nil { rsp.Status, err = errToStatus(err) diff --git a/pkg/storage/unified/resource/server_test.go b/pkg/storage/unified/resource/server_test.go index e3bfd646e08..5de4342c06a 100644 --- a/pkg/storage/unified/resource/server_test.go +++ b/pkg/storage/unified/resource/server_test.go @@ -164,4 +164,57 @@ func TestSimpleServer(t *testing.T) { require.NoError(t, err) require.Len(t, all.Items, 0) // empty }) + + t.Run("playlist update optimistic concurrency check", func(t *testing.T) { + raw := []byte(`{ + "apiVersion": "playlist.grafana.app/v0alpha1", + "kind": "Playlist", + "metadata": { + "name": "fdgsv37qslr0ga", + "namespace": "default", + "annotations": { + "grafana.app/originName": "elsewhere", + "grafana.app/originPath": "path/to/item", + "grafana.app/originTimestamp": "2024-02-02T00:00:00Z" + } + }, + "spec": { + "title": "hello", + "interval": "5m", + "items": [ + { + "type": "dashboard_by_uid", + "value": "vmie2cmWz" + } + ] + } + }`) + + key := &ResourceKey{ + Group: "playlist.grafana.app", + Resource: "rrrr", // can be anything :( + Namespace: "default", + Name: "fdgsv37qslr0ga", + } + + created, err := server.Create(ctx, &CreateRequest{ + Value: raw, + Key: key, + }) + require.NoError(t, err) + + // Update should return an ErrOptimisticLockingFailed the second time + + _, err = server.Update(ctx, &UpdateRequest{ + Key: key, + Value: raw, + ResourceVersion: created.ResourceVersion}) + require.NoError(t, err) + + _, err = server.Update(ctx, &UpdateRequest{ + Key: key, + Value: raw, + ResourceVersion: created.ResourceVersion}) + require.ErrorIs(t, err, ErrOptimisticLockingFailed) + }) }