diff --git a/internal/host/plugin/repository_host_catalog.go b/internal/host/plugin/repository_host_catalog.go index 1139109780..8f2114c7ce 100644 --- a/internal/host/plugin/repository_host_catalog.go +++ b/internal/host/plugin/repository_host_catalog.go @@ -286,7 +286,7 @@ func (r *Repository) UpdateCatalog(ctx context.Context, c *HostCatalog, version } } - if updateAttributes && c.Attributes != nil { + if updateAttributes { dbMask = append(dbMask, "attributes") newCatalog.Attributes, err = patchstruct.PatchBytes(newCatalog.Attributes, c.Attributes) if err != nil { diff --git a/internal/libs/patchstruct/patchstruct.go b/internal/libs/patchstruct/patchstruct.go index 313edb821f..6ed6e1eabb 100644 --- a/internal/libs/patchstruct/patchstruct.go +++ b/internal/libs/patchstruct/patchstruct.go @@ -7,15 +7,16 @@ import ( "google.golang.org/protobuf/types/known/structpb" ) -// PatchStruct updates the struct found in dst with the values found in src. -// The intent of this helper is to provide a fallback mechanism for subtype +// PatchStruct updates the struct found in dst with the values found in src. The +// intent of this helper is to provide a fallback mechanism for subtype // attributes when the actual schema of the subtype attributes are unknown. As // such, it's preferred to use other methods (such as mask mapping) when an // actual message for the subtype is known. // // The following rules apply: // -// * The source (src) map is merged into the destination map (dst). +// * The source (src) map is merged into the destination map (dst). If the +// source map is nil, the destination will be a valid, but empty, map. // // * Values are overwritten by the source map if they exist in both. // @@ -32,6 +33,10 @@ import ( // // PatchStruct returns the updated map as a copy, dst and src are not altered. func PatchStruct(dst, src *structpb.Struct) *structpb.Struct { + if src == nil { + ret, _ := structpb.NewStruct(nil) + return ret + } result, err := structpb.NewStruct(patchM(dst.AsMap(), src.AsMap())) if err != nil { // Should never error as values are source from structpb values @@ -41,21 +46,25 @@ func PatchStruct(dst, src *structpb.Struct) *structpb.Struct { return result } -// PatchBytes follows the same rules as above with PatchStruct, but -// instead of patching structpb.Structs, it patches the protobuf -// encoding. An error is returned if there are issues working with -// the data. +// PatchBytes follows the same rules as above with PatchStruct, but instead of +// patching structpb.Structs, it patches the protobuf encoding. An error is +// returned if there are issues working with the data. If src is nil or empty, +// the result is a marshaled, empty struct. func PatchBytes(dst, src []byte) ([]byte, error) { srcpb, dstpb := new(structpb.Struct), new(structpb.Struct) - if err := proto.Unmarshal(dst, dstpb); err != nil { - return nil, fmt.Errorf("error reading destination data: %w", err) - } - - if err := proto.Unmarshal(src, srcpb); err != nil { - return nil, fmt.Errorf("error reading source data: %w", err) + if len(src) != 0 { + if err := proto.Unmarshal(dst, dstpb); err != nil { + return nil, fmt.Errorf("error reading destination data: %w", err) + } + if err := proto.Unmarshal(src, srcpb); err != nil { + return nil, fmt.Errorf("error reading source data: %w", err) + } + dstpb = PatchStruct(dstpb, srcpb) + } else { + dstpb, _ = structpb.NewStruct(nil) } - result, err := proto.Marshal(PatchStruct(dstpb, srcpb)) + result, err := proto.Marshal(dstpb) if err != nil { return nil, fmt.Errorf("error writing result data: %w", err) } diff --git a/internal/libs/patchstruct/patchstruct_test.go b/internal/libs/patchstruct/patchstruct_test.go index efb8f192f8..7eb4118013 100644 --- a/internal/libs/patchstruct/patchstruct_test.go +++ b/internal/libs/patchstruct/patchstruct_test.go @@ -118,10 +118,8 @@ var testCases = []testCase{ dst: map[string]interface{}{ "foo": "bar", }, - src: nil, - expected: map[string]interface{}{ - "foo": "bar", - }, + src: nil, + expected: nil, }, { name: "nil dst", @@ -189,12 +187,6 @@ func TestPatchBytes(t *testing.T) { } func TestPatchBytesErr(t *testing.T) { - t.Run("dst", func(t *testing.T) { - require := require.New(t) - _, err := PatchBytes([]byte("foo"), nil) - require.ErrorIs(err, proto.Error) - require.True(strings.HasPrefix(err.Error(), "error reading destination data: ")) - }) t.Run("src", func(t *testing.T) { require := require.New(t) _, err := PatchBytes(nil, []byte("foo")) diff --git a/internal/servers/controller/handlers/host_catalogs/host_catalog_service_test.go b/internal/servers/controller/handlers/host_catalogs/host_catalog_service_test.go index a8dcf3b078..6e62c68d99 100644 --- a/internal/servers/controller/handlers/host_catalogs/host_catalog_service_test.go +++ b/internal/servers/controller/handlers/host_catalogs/host_catalog_service_test.go @@ -1470,6 +1470,20 @@ func TestUpdate_Plugin(t *testing.T) { assert.Equal(t, map[string]interface{}{"newkey": "newvalue"}, in.GetAttributes().AsMap()) }, }, + { + name: "Update empty attributes for catalog", + masks: []string{"attributes"}, + changes: []updateFn{ + clearReadOnlyFields(), + updateAttrs(func() *structpb.Struct { + ret, _ := structpb.NewStruct(nil) + return ret + }()), + }, + check: func(t *testing.T, in *pb.HostCatalog) { + assert.Equal(t, (*structpb.Struct)(nil), in.GetAttributes()) + }, + }, { name: "Update secrets", masks: []string{"secrets.key1", "secrets.key2"},