From 93bad3ea1bcbcf272ebc802b0d55c218b2486dd3 Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Mon, 22 May 2023 10:44:49 -0400 Subject: [PATCH] Allow resource updates to omit an owner refs UID (#17423) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change enables workflows where you are reapplying a resource that should have an owner ref to publish modifications to the resources data without performing a read to figure out the current owner resource incarnations UID. Basically we want workflows similar to `kubectl apply` or `consul config write` to be able to work seamlessly even for owned resources. In these cases the users intention is to have the resource owned by the “current” incarnation of the owner resource. --- .../grpc-external/services/resource/write.go | 9 +++++++++ .../services/resource/write_test.go | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/agent/grpc-external/services/resource/write.go b/agent/grpc-external/services/resource/write.go index 3a365cee37..34799ae8d8 100644 --- a/agent/grpc-external/services/resource/write.go +++ b/agent/grpc-external/services/resource/write.go @@ -179,6 +179,15 @@ func (s *Server) Write(ctx context.Context, req *pbresource.WriteRequest) (*pbre return storage.ErrCASFailure } + // Fill in an empty Owner UID with the existing owner's UID. If other parts + // of the owner ID like the type or name have changed then the subsequent + // EqualID call will still error as you are not allowed to change the owner. + // This is a small UX nicety to repeatedly "apply" a resource that should + // have an owner without having to care about the current owners incarnation. + if input.Owner != nil && existing.Owner != nil && input.Owner.Uid == "" { + input.Owner.Uid = existing.Owner.Uid + } + // Owner can only be set on creation. Enforce immutability. if !resource.EqualID(input.Owner, existing.Owner) { return status.Errorf(codes.InvalidArgument, "owner cannot be changed") diff --git a/agent/grpc-external/services/resource/write_test.go b/agent/grpc-external/services/resource/write_test.go index 9b94ecae8b..3da4ec478a 100644 --- a/agent/grpc-external/services/resource/write_test.go +++ b/agent/grpc-external/services/resource/write_test.go @@ -571,6 +571,25 @@ func TestWrite_Owner_Uid(t *testing.T) { require.NotEmpty(t, rsp2.Resource.Owner.Uid) require.Equal(t, artist.Id.Uid, rsp2.Resource.Owner.Uid) }) + + t.Run("no-uid - update auto resolve", func(t *testing.T) { + artist, err := demo.GenerateV2Artist() + require.NoError(t, err) + + uid := ulid.Make().String() + album, err := demo.GenerateV2Album(artist.Id) + require.NoError(t, err) + album.Owner.Uid = uid + + _, err = client.Write(testContext(t), &pbresource.WriteRequest{Resource: album}) + require.NoError(t, err) + + // unset the uid and rewrite the resource + album.Owner.Uid = "" + rsp, err := client.Write(testContext(t), &pbresource.WriteRequest{Resource: album}) + require.NoError(t, err) + require.Equal(t, uid, rsp.GetResource().GetOwner().GetUid()) + }) } type blockOnceBackend struct {