From 050553ffcf37558353853e57d824cb86f185b90c Mon Sep 17 00:00:00 2001 From: Todd Date: Tue, 9 Nov 2021 14:42:48 -0700 Subject: [PATCH] Add update catalog support for plugin based catalogs in handler and go sdk (#1673) * Create catalog service handler for updating plugin. Add tests for handler and run make fmt. Silently drop attributes where the value is nil when creating the catalog or set. --- .../host/plugin/repository_host_catalog.go | 8 + .../plugin/repository_host_catalog_test.go | 5 +- internal/host/plugin/repository_host_set.go | 10 + .../host/plugin/repository_host_set_test.go | 7 +- internal/libs/patchstruct/patchstruct.go | 5 +- internal/libs/patchstruct/patchstruct_test.go | 45 ++- .../host_catalogs/host_catalog_service.go | 59 +++- .../host_catalog_service_test.go | 282 +++++++++++++++++- .../handlers/host_sets/host_set_service.go | 4 +- .../host_sets/host_set_service_test.go | 14 +- .../api/hostcatalogs/host_catalog_test.go | 11 + internal/tests/api/hosts/host_test.go | 10 +- 12 files changed, 437 insertions(+), 23 deletions(-) diff --git a/internal/host/plugin/repository_host_catalog.go b/internal/host/plugin/repository_host_catalog.go index dda662693..c99e433be 100644 --- a/internal/host/plugin/repository_host_catalog.go +++ b/internal/host/plugin/repository_host_catalog.go @@ -60,6 +60,14 @@ func (r *Repository) CreateCatalog(ctx context.Context, c *HostCatalog, _ ...Opt } c.PublicId = id + // Use PatchBytes' functionality that does not add keys where the values + // are nil to the resulting struct since we do not want to store nil valued + // attributes. + c.Attributes, err = patchstruct.PatchBytes([]byte{}, c.Attributes) + if err != nil { + return nil, nil, errors.Wrap(ctx, err, op) + } + plgHc, err := toPluginCatalog(ctx, c) if err != nil { return nil, nil, errors.Wrap(ctx, err, op) diff --git a/internal/host/plugin/repository_host_catalog_test.go b/internal/host/plugin/repository_host_catalog_test.go index 14f96af01..bf9e0c0d2 100644 --- a/internal/host/plugin/repository_host_catalog_test.go +++ b/internal/host/plugin/repository_host_catalog_test.go @@ -188,7 +188,10 @@ func TestRepository_CreateCatalog(t *testing.T) { ScopeId: prj.GetPublicId(), PluginId: plg.GetPublicId(), Attributes: func() []byte { - st, err := structpb.NewStruct(map[string]interface{}{"k1": "foo"}) + st, err := structpb.NewStruct(map[string]interface{}{ + "k1": "foo", + "nilkey": nil, + }) require.NoError(t, err) b, err := proto.Marshal(st) require.NoError(t, err) diff --git a/internal/host/plugin/repository_host_set.go b/internal/host/plugin/repository_host_set.go index 6e3344244..958b557f1 100644 --- a/internal/host/plugin/repository_host_set.go +++ b/internal/host/plugin/repository_host_set.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/boundary/internal/host" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/libs/endpoint" + "github.com/hashicorp/boundary/internal/libs/patchstruct" "github.com/hashicorp/boundary/internal/oplog" hostplugin "github.com/hashicorp/boundary/internal/plugin/host" pb "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/hostsets" @@ -50,6 +51,15 @@ func (r *Repository) CreateSet(ctx context.Context, scopeId string, s *HostSet, } s = s.clone() + // Use PatchBytes' functionality that does not add keys where the values + // are nil to the resulting struct since we do not want to store nil valued + // attributes. + var err error + s.Attributes, err = patchstruct.PatchBytes([]byte{}, s.Attributes) + if err != nil { + return nil, nil, errors.Wrap(ctx, err, op) + } + c, per, err := r.getCatalog(ctx, s.CatalogId) if err != nil { return nil, nil, errors.Wrap(ctx, err, op, errors.WithMsg("looking up catalog")) diff --git a/internal/host/plugin/repository_host_set_test.go b/internal/host/plugin/repository_host_set_test.go index 34fcdfbb8..cc36690d1 100644 --- a/internal/host/plugin/repository_host_set_test.go +++ b/internal/host/plugin/repository_host_set_test.go @@ -186,7 +186,12 @@ func TestRepository_CreateSet(t *testing.T) { CatalogId: catalog.PublicId, Description: ("test-description-repo"), Attributes: func() []byte { - b, err := proto.Marshal(&structpb.Struct{Fields: map[string]*structpb.Value{"k1": structpb.NewStringValue("foo")}}) + st, err := structpb.NewStruct(map[string]interface{}{ + "k1": "foo", + "removed": nil, + }) + require.NoError(t, err) + b, err := proto.Marshal(st) require.NoError(t, err) return b }(), diff --git a/internal/libs/patchstruct/patchstruct.go b/internal/libs/patchstruct/patchstruct.go index 74798006d..313edb821 100644 --- a/internal/libs/patchstruct/patchstruct.go +++ b/internal/libs/patchstruct/patchstruct.go @@ -71,8 +71,9 @@ func patchM(dst, src map[string]interface{}) map[string]interface{} { // If the value in dst a map, continue to patch dst[k] = patchM(y, x) } else { - // Overwrite - dst[k] = x + // Overwrite after stripping out keys to nil values + newX := patchM(make(map[string]interface{}), x) + dst[k] = newX } default: diff --git a/internal/libs/patchstruct/patchstruct_test.go b/internal/libs/patchstruct/patchstruct_test.go index c043be413..c7a972ba2 100644 --- a/internal/libs/patchstruct/patchstruct_test.go +++ b/internal/libs/patchstruct/patchstruct_test.go @@ -1,8 +1,12 @@ package patchstruct import ( + "strings" "testing" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/hashicorp/boundary/internal/host" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/structpb" @@ -75,6 +79,24 @@ var testCases = []testCase{ "foo": "bar", }, }, + { + name: "nested with nil", + dst: nil, + src: map[string]interface{}{ + "a": "b", + "nested": map[string]interface{}{ + "c": "d", + "e": nil, + }, + "f": nil, + }, + expected: map[string]interface{}{ + "a": "b", + "nested": map[string]interface{}{ + "c": "d", + }, + }, + }, { name: "overwrite with map in src", dst: map[string]interface{}{ @@ -111,6 +133,17 @@ var testCases = []testCase{ "foo": "bar", }, }, + { + name: "nil dst with src nil value", + dst: nil, + src: map[string]interface{}{ + "foo": "bar", + "fiz": nil, + }, + expected: map[string]interface{}{ + "foo": "bar", + }, + }, } func TestPatchStruct(t *testing.T) { @@ -130,7 +163,11 @@ func TestPatchStruct(t *testing.T) { } actual := PatchStruct(dst, src) - require.Equal(mustStruct(tc.expected), actual) + require.Empty(cmp.Diff(mustStruct(tc.expected), actual, + cmpopts.IgnoreUnexported(structpb.Struct{}, structpb.Value{}), + cmpopts.SortSlices(func(x, y interface{}) bool { + return x.(*host.IpAddress).Address < y.(*host.IpAddress).Address + }))) require.Equal(dstOrig, dst) require.Equal(srcOrig, src) }) @@ -155,12 +192,14 @@ func TestPatchBytesErr(t *testing.T) { t.Run("dst", func(t *testing.T) { require := require.New(t) _, err := PatchBytes([]byte("foo"), nil) - require.EqualError(err, "error reading destination data: proto: cannot parse invalid wire-format data") + 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")) - require.EqualError(err, "error reading source data: proto: cannot parse invalid wire-format data") + require.ErrorIs(err, proto.Error) + require.True(strings.HasPrefix(err.Error(), "error reading source data: ")) }) } diff --git a/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go b/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go index 59a3bb1b5..a301b1368 100644 --- a/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go +++ b/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go @@ -326,7 +326,7 @@ func (s Service) UpdateHostCatalog(ctx context.Context, req *pbs.UpdateHostCatal if authResults.Error != nil { return nil, authResults.Error } - hc, err := s.updateInRepo(ctx, authResults.Scope.GetId(), req.GetId(), req.GetUpdateMask().GetPaths(), req.GetItem()) + hc, plg, err := s.updateInRepo(ctx, authResults.Scope.GetId(), req) if err != nil { return nil, err } @@ -338,6 +338,9 @@ func (s Service) UpdateHostCatalog(ctx context.Context, req *pbs.UpdateHostCatal outputOpts := make([]handlers.Option, 0, 3) outputOpts = append(outputOpts, handlers.WithOutputFields(&outputFields)) + if plg != nil { + outputOpts = append(outputOpts, handlers.WithPlugin(plg)) + } if outputFields.Has(globals.ScopeField) { outputOpts = append(outputOpts, handlers.WithScope(authResults.Scope)) } @@ -510,8 +513,8 @@ func (s Service) createInRepo(ctx context.Context, projId string, req *pbs.Creat return hc, plg, err } -func (s Service) updateInRepo(ctx context.Context, projId, id string, mask []string, item *pb.HostCatalog) (*static.HostCatalog, error) { - const op = "host_catalogs.(Service).updateInRepo" +func (s Service) updateStaticInRepo(ctx context.Context, projId, id string, mask []string, item *pb.HostCatalog) (*static.HostCatalog, error) { + const op = "host_catalogs.(Service).updateStaticInRepo" h, err := toStorageStaticCatalog(ctx, projId, item) if err != nil { return nil, errors.Wrap(ctx, err, op, errors.WithMsg("unable to build host catalog for update")) @@ -536,6 +539,44 @@ func (s Service) updateInRepo(ctx context.Context, projId, id string, mask []str return out, nil } +func (s Service) updatePluginInRepo(ctx context.Context, projId, id string, mask []string, item *pb.HostCatalog) (*plugin.HostCatalog, *plugins.PluginInfo, error) { + const op = "host_catalogs.(Service).updatePluginInRepo" + h, err := toStoragePluginCatalog(ctx, projId, "", item) + if err != nil { + return nil, nil, errors.Wrap(ctx, err, op, errors.WithMsg("unable to build host catalog for update")) + } + version := item.GetVersion() + h.PublicId = id + dbMask := maskManager.Translate(mask, "attributes", "secrets") + if len(dbMask) == 0 { + return nil, nil, handlers.InvalidArgumentErrorf("No valid fields included in the update mask.", map[string]string{"update_mask": "No valid fields provided in the update mask."}) + } + + repo, err := s.pluginHostRepoFn() + if err != nil { + return nil, nil, errors.Wrap(ctx, err, op) + } + out, plg, rowsUpdated, err := repo.UpdateCatalog(ctx, h, version, dbMask) + if err != nil { + return nil, nil, errors.Wrap(ctx, err, op, errors.WithMsg("unable to update host catalog")) + } + if rowsUpdated == 0 { + return nil, nil, handlers.NotFoundErrorf("Host Catalog %q doesn't exist or incorrect version provided.", id) + } + return out, toPluginInfo(plg), nil +} + +func (s Service) updateInRepo(ctx context.Context, projId string, req *pbs.UpdateHostCatalogRequest) (hc host.Catalog, plg *plugins.PluginInfo, err error) { + const op = "host_catalogs.(Service).updateInRepo" + switch host.SubtypeFromId(req.GetId()) { + case static.Subtype: + hc, err = s.updateStaticInRepo(ctx, projId, req.GetId(), req.GetUpdateMask().GetPaths(), req.GetItem()) + case plugin.Subtype: + hc, plg, err = s.updatePluginInRepo(ctx, projId, req.GetId(), req.GetUpdateMask().GetPaths(), req.GetItem()) + } + return +} + func (s Service) deleteFromRepo(ctx context.Context, id string) (bool, error) { const op = "host_catalogs.(Service).deleteFromRepo" rows := 0 @@ -793,9 +834,19 @@ func validateUpdateRequest(req *pbs.UpdateHostCatalogRequest) error { if req.GetItem().GetType() != "" && host.SubtypeFromType(req.GetItem().GetType()) != static.Subtype { badFields[globals.TypeField] = "Cannot modify resource type." } + if req.GetItem().GetPlugin() != nil { + badFields[globals.PluginField] = "This field is unused for this type of host catalog." + } + case plugin.Subtype: + if req.GetItem().GetType() != "" && host.SubtypeFromType(req.GetItem().GetType()) != plugin.Subtype { + badFields[globals.TypeField] = "Cannot modify resource type." + } + if req.GetItem().GetPlugin() != nil { + badFields[globals.PluginField] = "This is a read only field." + } } return badFields - }, static.HostCatalogPrefix) + }, static.HostCatalogPrefix, plugin.HostCatalogPrefix) } func validateDeleteRequest(req *pbs.DeleteHostCatalogRequest) error { 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 9e8193ed2..dde3262e0 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 @@ -33,6 +33,7 @@ import ( "google.golang.org/protobuf/testing/protocmp" "google.golang.org/protobuf/types/known/structpb" "google.golang.org/protobuf/types/known/timestamppb" + "google.golang.org/protobuf/types/known/wrapperspb" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -972,7 +973,7 @@ func TestCreate_Plugin(t *testing.T) { } } -func TestUpdate(t *testing.T) { +func TestUpdate_Static(t *testing.T) { t.Parallel() conn, _ := db.TestSetup(t, "postgres") wrapper := db.TestWrapper(t) @@ -1339,3 +1340,282 @@ func TestUpdate(t *testing.T) { }) } } + +func TestUpdate_Plugin(t *testing.T) { + t.Parallel() + conn, _ := db.TestSetup(t, "postgres") + wrapper := db.TestWrapper(t) + kms := kms.TestKms(t, conn, wrapper) + sche := scheduler.TestScheduler(t, conn, wrapper) + iamRepo := iam.TestRepo(t, conn, wrapper) + _, proj := iam.TestScopes(t, iamRepo) + rw := db.New(conn) + + name := "test" + plg := host.TestPlugin(t, conn, name) + plgm := map[string]plgpb.HostPluginServiceClient{ + plg.GetPublicId(): plugin.NewWrappingPluginClient(&plugin.TestPluginServer{}), + } + + repoFn := func() (*static.Repository, error) { + return static.NewRepository(rw, rw, kms) + } + pluginHostRepo := func() (*plugin.Repository, error) { + return plugin.NewRepository(rw, rw, kms, sche, plgm) + } + pluginRepo := func() (*host.Repository, error) { + return host.NewRepository(rw, rw, kms) + } + iamRepoFn := func() (*iam.Repository, error) { + return iamRepo, nil + } + tested, err := host_catalogs.NewService(repoFn, pluginHostRepo, pluginRepo, iamRepoFn) + require.NoError(t, err, "Failed to create a new host catalog service.") + + ctx := auth.DisabledAuthTestContext(iamRepoFn, proj.GetPublicId()) + + freshCatalog := func(t *testing.T) *pb.HostCatalog { + attr, err := structpb.NewStruct(map[string]interface{}{ + "foo": "bar", + }) + require.NoError(t, err) + resp, err := tested.CreateHostCatalog(ctx, &pbs.CreateHostCatalogRequest{Item: &pb.HostCatalog{ + ScopeId: proj.GetPublicId(), + PluginId: plg.GetPublicId(), + Name: wrapperspb.String("default"), + Description: wrapperspb.String("default"), + Type: "plugin", + Attributes: attr, + }}) + require.NoError(t, err) + id := resp.GetItem().GetId() + t.Cleanup(func() { + _, err := tested.DeleteHostCatalog(ctx, &pbs.DeleteHostCatalogRequest{Id: id}) + require.NoError(t, err) + }) + return resp.GetItem() + } + + type updateFn func(catalog *pb.HostCatalog) + clearReadOnlyFields := func() updateFn { + return func(c *pb.HostCatalog) { + c.Id = "" + c.Plugin = nil + c.AuthorizedActions = nil + c.AuthorizedCollectionActions = nil + c.CreatedTime = nil + c.UpdatedTime = nil + } + } + updateName := func(i *wrappers.StringValue) updateFn { + return func(c *pb.HostCatalog) { + c.Name = i + } + } + updateDesc := func(i *wrappers.StringValue) updateFn { + return func(c *pb.HostCatalog) { + c.Description = i + } + } + updateAttrs := func(i *structpb.Struct) updateFn { + return func(c *pb.HostCatalog) { + c.Attributes = i + } + } + + cases := []struct { + name string + masks []string + changes []updateFn + check func(*testing.T, *pb.HostCatalog) + err error + }{ + { + name: "Update an Existing HostCatalog", + masks: []string{"name", "description"}, + changes: []updateFn{ + clearReadOnlyFields(), + updateName(wrapperspb.String("new")), + updateDesc(wrapperspb.String("desc")), + }, + check: func(t *testing.T, in *pb.HostCatalog) { + assert.Equal(t, "new", in.Name.GetValue()) + assert.Equal(t, "desc", in.Description.GetValue()) + }, + }, + { + name: "Update arbitrary attribute for catalog", + masks: []string{"attributes.newkey", "attributes.foo"}, + changes: []updateFn{ + clearReadOnlyFields(), + updateAttrs(func() *structpb.Struct { + attr, err := structpb.NewStruct(map[string]interface{}{ + "newkey": "newvalue", + "foo": nil, + }) + require.NoError(t, err) + return attr + }()), + }, + check: func(t *testing.T, in *pb.HostCatalog) { + assert.Equal(t, map[string]interface{}{"newkey": "newvalue"}, in.GetAttributes().AsMap()) + }, + }, + { + name: "Multiple Paths in single string", + masks: []string{"name,description"}, + changes: []updateFn{ + clearReadOnlyFields(), + updateName(wrapperspb.String("new")), + updateDesc(wrapperspb.String("desc")), + }, + check: func(t *testing.T, in *pb.HostCatalog) { + assert.Equal(t, "new", in.Name.GetValue()) + assert.Equal(t, "desc", in.Description.GetValue()) + }, + }, + { + name: "No Update Mask", + changes: []updateFn{ + clearReadOnlyFields(), + updateName(wrapperspb.String("new")), + updateDesc(wrapperspb.String("desc")), + }, + err: handlers.ApiErrorWithCode(codes.InvalidArgument), + }, + { + name: "Empty Path", + masks: []string{}, + changes: []updateFn{ + clearReadOnlyFields(), + updateName(wrapperspb.String("new")), + updateDesc(wrapperspb.String("desc")), + }, + err: handlers.ApiErrorWithCode(codes.InvalidArgument), + }, + { + name: "Only non-existant paths in Mask", + masks: []string{"nonexistant_field"}, + changes: []updateFn{ + clearReadOnlyFields(), + updateName(wrapperspb.String("new")), + updateDesc(wrapperspb.String("desc")), + }, + err: handlers.ApiErrorWithCode(codes.InvalidArgument), + }, + { + name: "Unset Name", + masks: []string{"name"}, + changes: []updateFn{ + clearReadOnlyFields(), + updateName(nil), + updateDesc(wrapperspb.String("ignored")), + }, + check: func(t *testing.T, in *pb.HostCatalog) { + assert.Nil(t, in.Name) + assert.Equal(t, "default", in.Description.GetValue()) + }, + }, + { + name: "Unset Description", + masks: []string{"description"}, + changes: []updateFn{ + clearReadOnlyFields(), + updateName(wrapperspb.String("ignored")), + updateDesc(nil), + }, + check: func(t *testing.T, in *pb.HostCatalog) { + assert.Nil(t, in.Description) + assert.Equal(t, "default", in.Name.GetValue()) + }, + }, + { + name: "Update Only Name", + masks: []string{"name"}, + changes: []updateFn{ + clearReadOnlyFields(), + updateName(wrapperspb.String("updated")), + updateDesc(wrapperspb.String("ignored")), + }, + check: func(t *testing.T, in *pb.HostCatalog) { + assert.Equal(t, "updated", in.Name.GetValue()) + assert.Equal(t, "default", in.Description.GetValue()) + }, + }, + { + name: "Update Only Description", + masks: []string{"description"}, + changes: []updateFn{ + clearReadOnlyFields(), + updateName(wrapperspb.String("ignored")), + updateDesc(wrapperspb.String("updated")), + }, + check: func(t *testing.T, in *pb.HostCatalog) { + assert.Equal(t, "default", in.Name.GetValue()) + assert.Equal(t, "updated", in.Description.GetValue()) + }, + }, + { + name: "Cant change type", + masks: []string{"type"}, + changes: []updateFn{ + clearReadOnlyFields(), + func(c *pb.HostCatalog) { + c.Type = "static" + }, + }, + err: handlers.ApiErrorWithCode(codes.InvalidArgument), + }, + { + name: "Cant specify Updated Time", + masks: []string{"updated_time"}, + changes: []updateFn{ + clearReadOnlyFields(), + func(c *pb.HostCatalog) { + c.UpdatedTime = timestamppb.Now() + }, + }, + err: handlers.ApiErrorWithCode(codes.InvalidArgument), + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + hc := freshCatalog(t) + + id := hc.GetId() + for _, f := range tc.changes { + f(hc) + } + + req := &pbs.UpdateHostCatalogRequest{ + Id: id, + Item: hc, + UpdateMask: &field_mask.FieldMask{Paths: tc.masks}, + } + + // Test some bad versions + req.Item.Version = 2 + _, gErr := tested.UpdateHostCatalog(ctx, req) + require.Error(gErr) + req.Item.Version = 0 + _, gErr = tested.UpdateHostCatalog(ctx, req) + require.Error(gErr) + req.Item.Version = 1 + + got, gErr := tested.UpdateHostCatalog(ctx, req) + if tc.err != nil { + require.Error(gErr) + assert.True(errors.Is(gErr, tc.err), "UpdateHostCatalog(%+v) got error %v, wanted %v", req, gErr, tc.err) + return + } + require.NoError(gErr) + require.NotNil(got) + + item := got.GetItem() + assert.Equal(uint32(2), item.GetVersion()) + assert.Greater(item.GetUpdatedTime().AsTime().UnixNano(), item.GetCreatedTime().AsTime().UnixNano()) + tc.check(t, item) + }) + } +} diff --git a/internal/servers/controller/handlers/host_sets/host_set_service.go b/internal/servers/controller/handlers/host_sets/host_set_service.go index 4d44777e5..1bd93bf6e 100644 --- a/internal/servers/controller/handlers/host_sets/host_set_service.go +++ b/internal/servers/controller/handlers/host_sets/host_set_service.go @@ -37,7 +37,7 @@ var ( // IdActions contains the set of actions that can be performed on // individual resources idActionsTypeMap = map[subtypes.Subtype]action.ActionSet{ - static.Subtype: action.ActionSet{ + static.Subtype: { action.NoOp, action.Read, action.Update, @@ -46,7 +46,7 @@ var ( action.SetHosts, action.RemoveHosts, }, - plugin.Subtype: action.ActionSet{ + plugin.Subtype: { action.NoOp, action.Read, action.Update, diff --git a/internal/servers/controller/handlers/host_sets/host_set_service_test.go b/internal/servers/controller/handlers/host_sets/host_set_service_test.go index 7fac8c530..1122ee407 100644 --- a/internal/servers/controller/handlers/host_sets/host_set_service_test.go +++ b/internal/servers/controller/handlers/host_sets/host_set_service_test.go @@ -815,7 +815,7 @@ func TestCreate_Plugin(t *testing.T) { } hc := plugin.TestCatalog(t, conn, proj.GetPublicId(), plg.GetPublicId()) - testAttrs, err := structpb.NewStruct(map[string]interface{}{ + attrs := map[string]interface{}{ "int": 1, "zero int": 0, "string": "foo", @@ -834,7 +834,13 @@ func TestCreate_Plugin(t *testing.T) { "bool": true, "zero bool": false, }, - }) + } + testInputAttrs, err := structpb.NewStruct(attrs) + require.NoError(t, err) + // The result should clear out all keys with nil values... + delete(attrs, "zero bytes") + delete(attrs["nested"].(map[string]interface{}), "zero bytes") + testOutputAttrs, err := structpb.NewStruct(attrs) require.NoError(t, err) prefEndpoints := []string{"cidr:1.2.3.4", "cidr:2.3.4.5/24"} @@ -879,7 +885,7 @@ func TestCreate_Plugin(t *testing.T) { Name: &wrappers.StringValue{Value: "With Attributes"}, Description: &wrappers.StringValue{Value: "desc"}, Type: plugin.Subtype.String(), - Attributes: testAttrs, + Attributes: testInputAttrs, }}, res: &pbs.CreateHostSetResponse{ Uri: fmt.Sprintf("host-sets/%s_", plugin.HostSetPrefix), @@ -895,7 +901,7 @@ func TestCreate_Plugin(t *testing.T) { Description: &wrappers.StringValue{Value: "desc"}, Type: plugin.Subtype.String(), AuthorizedActions: testAuthorizedActions[plugin.Subtype], - Attributes: testAttrs, + Attributes: testOutputAttrs, }, }, }, diff --git a/internal/tests/api/hostcatalogs/host_catalog_test.go b/internal/tests/api/hostcatalogs/host_catalog_test.go index 5e60d3eeb..d7c74c39d 100644 --- a/internal/tests/api/hostcatalogs/host_catalog_test.go +++ b/internal/tests/api/hostcatalogs/host_catalog_test.go @@ -169,6 +169,17 @@ func TestCrud(t *testing.T) { hostcatalogs.WithAttributes(map[string]interface{}{"foo": "bar"})) require.NoError(err) + c, err = hcClient.Update(tc.Context(), c.Item.Id, c.Item.Version, hostcatalogs.WithName("bar"), + hostcatalogs.WithAttributes(map[string]interface{}{"key": "val", "foo": nil}), + hostcatalogs.WithSecrets(map[string]interface{}{"secretkey": "secretval"})) + checkCatalog("update", c.Item, err, "bar", 2) + assert.Contains(c.Item.Attributes, "key") + assert.NotContains(c.Item.Attributes, "foo") + assert.Empty(c.Item.Secrets) + + c, err = hcClient.Update(tc.Context(), c.Item.Id, c.Item.Version, hostcatalogs.DefaultName()) + checkCatalog("update", c.Item, err, "", 3) + c, err = hcClient.Read(tc.Context(), c.Item.Id) assert.NoError(err) diff --git a/internal/tests/api/hosts/host_test.go b/internal/tests/api/hosts/host_test.go index 0c4391758..688fdee10 100644 --- a/internal/tests/api/hosts/host_test.go +++ b/internal/tests/api/hosts/host_test.go @@ -99,20 +99,20 @@ func TestPluginHosts(t *testing.T) { hset, err := hostsets.NewClient(client).Create(tc.Context(), hc.Item.Id, hostsets.WithAttributes(map[string]interface{}{ "host_info": []interface{}{ map[string]interface{}{ - "external_id": "test1", + "external_id": "test1", "ip_addresses": []string{"10.0.0.1", "192.168.1.1"}, - "dns_names": []string{"foo.hashicorp.com", "boundaryproject.io"}, + "dns_names": []string{"foo.hashicorp.com", "boundaryproject.io"}, }, map[string]interface{}{ - "external_id": "test2", + "external_id": "test2", "ip_addresses": []string{"10.0.0.2", "192.168.1.2"}, - "dns_names": []string{"foo2.hashicorp.com", "boundaryproject2.io"}, + "dns_names": []string{"foo2.hashicorp.com", "boundaryproject2.io"}, }, }, })) require.NoError(err) require.NotNil(hset) - time.Sleep(1*time.Second) + time.Sleep(1 * time.Second) hClient := hosts.NewClient(client) hl, err := hClient.List(tc.Context(), hc.Item.Id)