From 31af1b1b7e535119aa23bd0aa936da1a80eb5aa2 Mon Sep 17 00:00:00 2001 From: Chris Marchesi Date: Fri, 29 Oct 2021 16:44:05 -0700 Subject: [PATCH] WIP - transparent attributes struct/wire conversion --- internal/host/plugin/host_catalog.go | 128 ++++++++-- internal/host/plugin/host_catalog_test.go | 236 ++++++++++++++++-- internal/host/plugin/options.go | 8 +- internal/host/plugin/options_test.go | 32 +-- .../host/plugin/repository_host_catalog.go | 10 +- .../plugin/repository_host_catalog_test.go | 146 ++++------- internal/host/plugin/testing.go | 3 +- 7 files changed, 395 insertions(+), 168 deletions(-) diff --git a/internal/host/plugin/host_catalog.go b/internal/host/plugin/host_catalog.go index df151fef0d..2efd6151c2 100644 --- a/internal/host/plugin/host_catalog.go +++ b/internal/host/plugin/host_catalog.go @@ -6,11 +6,11 @@ package plugin import ( "context" - "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/host/plugin/store" "github.com/hashicorp/boundary/internal/oplog" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/structpb" + "gorm.io/gorm" ) // A HostCatalog contains plugin host sets. It is owned by @@ -19,32 +19,28 @@ type HostCatalog struct { *store.HostCatalog tableName string `gorm:"-"` - Secrets *structpb.Struct `gorm:"-"` + Attributes *structpb.Struct `gorm:"-"` + Secrets *structpb.Struct `gorm:"-"` } // NewHostCatalog creates a new in memory HostCatalog assigned to a scopeId // and pluginId. Name and description are the only valid options. All other // options are ignored. -func NewHostCatalog(ctx context.Context, scopeId, pluginId string, opt ...Option) (*HostCatalog, error) { - const op = "plugin.NewHostCatalog" +func NewHostCatalog(ctx context.Context, scopeId, pluginId string, opt ...Option) *HostCatalog { opts := getOpts(opt...) - attrs, err := proto.Marshal(opts.withAttributes) - if err != nil { - return nil, errors.Wrap(ctx, err, op, errors.WithCode(errors.InvalidParameter)) - } - hc := &HostCatalog{ HostCatalog: &store.HostCatalog{ ScopeId: scopeId, PluginId: pluginId, Name: opts.withName, Description: opts.withDescription, - Attributes: attrs, + Attributes: make([]byte, 0), }, - Secrets: opts.withSecrets, + Attributes: opts.withAttributes, + Secrets: opts.withSecrets, } - return hc, nil + return hc } func allocHostCatalog() *HostCatalog { @@ -57,16 +53,18 @@ func allocHostCatalog() *HostCatalog { // secret. The secret shallow copied. func (c *HostCatalog) clone() *HostCatalog { cp := proto.Clone(c.HostCatalog) + newAttributes := proto.Clone(c.Attributes) newSecret := proto.Clone(c.Secrets) hc := &HostCatalog{ HostCatalog: cp.(*store.HostCatalog), + Attributes: newAttributes.(*structpb.Struct), Secrets: newSecret.(*structpb.Struct), } // proto.Clone will convert slices with length and capacity of 0 to nil. // Fix this since gorm treats empty slices differently than nil. - if c.Attributes != nil && len(c.Attributes) == 0 && hc.Attributes == nil { - hc.Attributes = []byte{} + if c.HostCatalog.Attributes != nil && len(c.HostCatalog.Attributes) == 0 && hc.HostCatalog.Attributes == nil { + hc.HostCatalog.Attributes = []byte{} } return hc } @@ -96,3 +94,105 @@ func (s *HostCatalog) oplog(op oplog.OpType) oplog.Metadata { } return metadata } + +// unmarshalAttributes marshals the attributes contained within the +// embedded store.HostCatalog wire format into the higher-level +// structpb.Struct Attribute field. It also sets the embedded store +// field to nil to prevent its direct use. +func (c *HostCatalog) unmarshalAttributes() error { + if c.HostCatalog == nil { + // no-op + return nil + } + + c.Attributes = new(structpb.Struct) + if err := proto.Unmarshal(c.HostCatalog.Attributes, c.Attributes); err != nil { + return err + } + + c.HostCatalog.Attributes = nil + return nil +} + +// AfterFind implements callbacks.AfterFind for HostCatalog for +// transparent unmarshaling of the binary attributes field into the +// higher-level struct field. +func (c *HostCatalog) AfterFind(_ *gorm.DB) error { + return c.unmarshalAttributes() +} + +// marshalAttributes marshals the attributes contained within the +// higher-level Attributes field into the embedded wire-format +// Attributes field within the store data structure. +// +// Note that unlike unmarshalAttributes, it does *not* clear out the +// higher-level attributes field afterwards. +func (c *HostCatalog) marshalAttributes() error { + if c.HostCatalog == nil { + // no-op + return nil + } + + var b []byte + if c.Attributes == nil { + b = make([]byte, 0) + } else { + var err error + b, err = proto.Marshal(c.Attributes) + if err != nil { + return err + } + } + + c.HostCatalog.Attributes = b + return nil +} + +// BeforeSave implements callbacks.BeforeSave for HostCatalog for +// transparent marshaling of the binary attributes field into the +// higher-level struct field. +func (c *HostCatalog) BeforeSave(tx *gorm.DB) error { + // For consistency, update the underlying binary attributes field. + if err := c.marshalAttributes(); err != nil { + return err + } + + // If we're updating the attributes field, we now also need to sync the + // in-flight value with the newly marshaled field. + if tx.Statement.Changed("Attributes") { + tx.Statement.SetColumn("Attributes", c.getEmbeddedAttributes()) + } + + return nil +} + +// AfterSave implements callbacks.AfterSave for HostCatalog to nil +// out the attributes field after saving the record to the database. +func (c *HostCatalog) AfterSave(_ *gorm.DB) error { + if c.HostCatalog == nil { + // no-op + return nil + } + + c.HostCatalog.Attributes = nil + return nil +} + +// GetAttributes overrides the field getter for the embedded []uint8 +// attributes field, and returns the top-level *structpb.Struct +// instead. +func (c *HostCatalog) GetAttributes() *structpb.Struct { + if c == nil { + return nil + } + + return c.Attributes +} + +func (c *HostCatalog) getEmbeddedAttributes() []byte { + if c == nil { + return nil + } + + return c.HostCatalog.GetAttributes() +} diff --git a/internal/host/plugin/host_catalog_test.go b/internal/host/plugin/host_catalog_test.go index cd7d90578f..cd035cefa7 100644 --- a/internal/host/plugin/host_catalog_test.go +++ b/internal/host/plugin/host_catalog_test.go @@ -124,29 +124,24 @@ func TestHostCatalog_Create(t *testing.T) { WithAttributes(&structpb.Struct{Fields: map[string]*structpb.Value{"foo": structpb.NewStringValue("bar")}}), }, }, - want: func() *HostCatalog { - hc := &HostCatalog{ - HostCatalog: &store.HostCatalog{ - PluginId: plg.GetPublicId(), - ScopeId: prj.GetPublicId(), - }, - } - var err error - hc.Attributes, err = proto.Marshal(&structpb.Struct{Fields: map[string]*structpb.Value{"foo": structpb.NewStringValue("bar")}}) - require.NoError(t, err) - return hc - }(), + want: &HostCatalog{ + HostCatalog: &store.HostCatalog{ + PluginId: plg.GetPublicId(), + ScopeId: prj.GetPublicId(), + Attributes: []byte{}, + }, + Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{"foo": structpb.NewStringValue("bar")}}, + }, }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - got, err := NewHostCatalog(ctx, tt.args.scopeId, tt.args.pluginId, tt.args.opts...) - require.NoError(t, err) + got := NewHostCatalog(ctx, tt.args.scopeId, tt.args.pluginId, tt.args.opts...) require.NotNil(t, got) - assert.Emptyf(t, got.PublicId, "PublicId set") + assert.Empty(t, got.PublicId, "PublicId should not be set") assert.Equal(t, tt.want, got) id, err := newHostCatalogId(ctx) @@ -167,7 +162,7 @@ func TestHostCatalog_Create(t *testing.T) { }, } require.NoError(t, w.LookupById(ctx, found)) - assert.Empty(t, cmp.Diff(got.HostCatalog, found.HostCatalog, protocmp.Transform()), "%q compared to %q", got.Attributes, found.Attributes) + assert.Empty(t, cmp.Diff(got.HostCatalog, found.HostCatalog, protocmp.Transform()), "%q compared to %q", got.HostCatalog.Attributes, found.HostCatalog.Attributes) } }) } @@ -183,8 +178,9 @@ func TestHostCatalog_Create_DuplicateNames(t *testing.T) { plg := host.TestPlugin(t, conn, "test1") plg2 := host.TestPlugin(t, conn, "test2") - got, err := NewHostCatalog(ctx, prj.GetPublicId(), plg.GetPublicId(), WithName("duplicate")) - require.NoError(t, err) + got := NewHostCatalog(ctx, prj.GetPublicId(), plg.GetPublicId(), WithName("duplicate")) + require.NotNil(t, got) + var err error got.PublicId, err = newHostCatalogId(ctx) require.NoError(t, err) require.NoError(t, w.Create(ctx, got)) @@ -195,15 +191,15 @@ func TestHostCatalog_Create_DuplicateNames(t *testing.T) { assert.Error(t, w.Create(ctx, got)) // Can't create another resource with same name in same scope even for different plugin - got, err = NewHostCatalog(ctx, prj.GetPublicId(), plg2.GetPublicId(), WithName("duplicate")) - require.NoError(t, err) + got = NewHostCatalog(ctx, prj.GetPublicId(), plg2.GetPublicId(), WithName("duplicate")) + require.NotNil(t, got) got.PublicId, err = newHostCatalogId(ctx) require.NoError(t, err) assert.Error(t, w.Create(ctx, got)) // Can create another resource with same name in different scope even for same plugin - got, err = NewHostCatalog(ctx, prj2.GetPublicId(), plg.GetPublicId(), WithName("duplicate")) - require.NoError(t, err) + got = NewHostCatalog(ctx, prj2.GetPublicId(), plg.GetPublicId(), WithName("duplicate")) + require.NotNil(t, got) got.PublicId, err = newHostCatalogId(ctx) require.NoError(t, err) assert.NoError(t, w.Create(ctx, got)) @@ -314,8 +310,8 @@ func TestHostCatalog_SetTableName(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - def, err := NewHostCatalog(context.Background(), "", "") - require.NoError(t, err) + def := NewHostCatalog(context.Background(), "", "") + require.NotNil(t, def) require.Equal(t, defaultTableName, def.TableName()) s := &HostCatalog{ HostCatalog: &store.HostCatalog{}, @@ -326,3 +322,195 @@ func TestHostCatalog_SetTableName(t *testing.T) { }) } } + +// testStoreHostCatalogBase is a base store.HostCatalog struct that +// embeds store.HostCatalog without any other interfaces implemented +// other than Tabler to give the correct table name. This ensures +// that a record can be written as-is without callbacks, etc, for +// point-in-time testing of various methods. +type testStoreHostCatalog struct { + *store.HostCatalog +} + +func (c *testStoreHostCatalog) TableName() string { return "host_plugin_catalog" } + +func TestHostCatalog_AttributesUnmarsahlOnLookup(t *testing.T) { + require := require.New(t) + ctx := context.Background() + conn, _ := db.TestSetup(t, "postgres") + wrapper := db.TestWrapper(t) + _, prj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) + plg := host.TestPlugin(t, conn, "test") + + expected := mustStruct(map[string]interface{}{ + "foo": "bar", + }) + attrsB, err := proto.Marshal(expected) + require.NoError(err) + require.NotNil(attrsB) + + id, err := newHostCatalogId(ctx) + require.NoError(err) + + // Create a host catalog record using store.HostCatalog first, + // without higher-level logic + shc := &testStoreHostCatalog{ + HostCatalog: &store.HostCatalog{ + PublicId: id, + ScopeId: prj.PublicId, + PluginId: plg.PublicId, + Attributes: attrsB, + }, + } + + w := db.New(conn) + err = w.Create(ctx, shc) + require.NoError(err) + + found := &HostCatalog{ + HostCatalog: &store.HostCatalog{ + PublicId: id, + }, + } + + require.NoError(w.LookupById(ctx, found)) + require.NotNil(found.Attributes) + require.Empty(cmp.Diff(expected, found.Attributes, protocmp.Transform())) + require.Nil(found.HostCatalog.Attributes) +} + +func TestHostCatalog_AttributesMarsahlOnSave(t *testing.T) { + ctx := context.Background() + conn, _ := db.TestSetup(t, "postgres") + wrapper := db.TestWrapper(t) + _, prj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) + plg := host.TestPlugin(t, conn, "test") + + attrsCreate := mustStruct(map[string]interface{}{ + "foo": "bar", + }) + attrsUpdate := mustStruct(map[string]interface{}{ + "baz": "qux", + }) + // Create using the higher-level HostCatalog object first without + // helper/validation methods. + hc := NewHostCatalog(ctx, prj.GetPublicId(), plg.GetPublicId(), WithAttributes(attrsCreate)) + id, err := newHostCatalogId(ctx) + require.NoError(t, err) + hc.PublicId = id + + w := db.New(conn) + + t.Run("create", func(t *testing.T) { + err = w.Create(ctx, hc) + require.NoError(t, err) + require.Nil(t, hc.HostCatalog.Attributes, "wire data should be niled out after creation") + + // Lookup the host catalog record using store.HostCatalog only, + // ensuring that no callbacks interfere. + found := &testStoreHostCatalog{ + HostCatalog: &store.HostCatalog{ + PublicId: id, + }, + } + + require.NoError(t, w.LookupById(ctx, found)) + require.NotNil(t, found.HostCatalog.Attributes) + + expected, err := proto.Marshal(attrsCreate) + require.NoError(t, err) + require.NotNil(t, expected) + require.Equal(t, expected, found.HostCatalog.Attributes) + }) + + t.Run("update", func(t *testing.T) { + hc.Attributes = attrsUpdate + numUpdated, err := w.Update(ctx, hc, []string{"attributes"}, []string{}) + require.NoError(t, err) + require.Equal(t, 1, numUpdated) + require.Nil(t, hc.HostCatalog.Attributes, "wire data should remain nil after update") + + // Lookup the host catalog record using store.HostCatalog only, + // ensuring that no callbacks interfere. + found := &testStoreHostCatalog{ + HostCatalog: &store.HostCatalog{ + PublicId: id, + }, + } + + require.NoError(t, w.LookupById(ctx, found)) + require.NotNil(t, found.HostCatalog.Attributes) + + expected, err := proto.Marshal(attrsUpdate) + require.NoError(t, err) + require.NotNil(t, expected) + require.Equal(t, expected, found.HostCatalog.Attributes) + }) +} + +func TestHostCatalog_marshalAttributes_empty(t *testing.T) { + t.Run("nil", func(t *testing.T) { + hc := &HostCatalog{HostCatalog: &store.HostCatalog{}} + hc.marshalAttributes() + assert.Equal(t, []byte{}, hc.HostCatalog.Attributes) + }) + t.Run("empty struct", func(t *testing.T) { + hc := &HostCatalog{ + HostCatalog: &store.HostCatalog{}, + Attributes: &structpb.Struct{}, + } + hc.marshalAttributes() + assert.Equal(t, []byte{}, hc.HostCatalog.Attributes) + }) +} + +func TestHostCatalog_GetAttributes(t *testing.T) { + t.Run("type", func(t *testing.T) { + require.IsType(t, &structpb.Struct{}, (&HostCatalog{}).GetAttributes()) + }) + + t.Run("nil", func(t *testing.T) { + require.Nil(t, (*HostCatalog)(nil).GetAttributes()) + }) + + t.Run("not nil", func(t *testing.T) { + expected := mustStruct(map[string]interface{}{"foo": "bar"}) + actual := (&HostCatalog{Attributes: mustStruct(map[string]interface{}{"foo": "bar"})}).GetAttributes() + require.Empty(t, cmp.Diff(expected, actual, protocmp.Transform())) + }) +} + +func TestHostCatalog_Clone_EmptyAttributes(t *testing.T) { + t.Run("embedded", func(t *testing.T) { + c := &HostCatalog{ + HostCatalog: &store.HostCatalog{ + Attributes: make([]byte, 0), + }, + } + actual := c.clone() + assert.NotNil(t, actual.HostCatalog.Attributes) + assert.Nil(t, actual.Attributes) + }) + + t.Run("top", func(t *testing.T) { + c := &HostCatalog{ + HostCatalog: &store.HostCatalog{}, + Attributes: &structpb.Struct{}, + } + actual := c.clone() + assert.Nil(t, actual.HostCatalog.Attributes) + assert.NotNil(t, actual.Attributes) + }) + + t.Run("both", func(t *testing.T) { + c := &HostCatalog{ + HostCatalog: &store.HostCatalog{ + Attributes: make([]byte, 0), + }, + Attributes: &structpb.Struct{}, + } + actual := c.clone() + assert.NotNil(t, actual.HostCatalog.Attributes) + assert.NotNil(t, actual.Attributes) + }) +} diff --git a/internal/host/plugin/options.go b/internal/host/plugin/options.go index 0467bfa822..f5352f4eec 100644 --- a/internal/host/plugin/options.go +++ b/internal/host/plugin/options.go @@ -4,7 +4,7 @@ import "google.golang.org/protobuf/types/known/structpb" // getOpts - iterate the inbound Options and return a struct func getOpts(opt ...Option) options { - opts := getDefaultOptions() + opts := options{} for _, o := range opt { o(&opts) } @@ -27,12 +27,6 @@ type options struct { withLimit int } -func getDefaultOptions() options { - return options{ - withAttributes: &structpb.Struct{}, - } -} - // WithPluginId provides an optional plugin id. func withPluginId(with string) Option { return func(o *options) { diff --git a/internal/host/plugin/options_test.go b/internal/host/plugin/options_test.go index 758e3d619e..cbeb59845e 100644 --- a/internal/host/plugin/options_test.go +++ b/internal/host/plugin/options_test.go @@ -10,44 +10,30 @@ func Test_GetOpts(t *testing.T) { t.Parallel() t.Run("WithName", func(t *testing.T) { opts := getOpts(WithName("test")) - testOpts := getDefaultOptions() - testOpts.withName = "test" - assert.Equal(t, opts, testOpts) + assert.Equal(t, "test", opts.withName) }) t.Run("WithPluginId", func(t *testing.T) { opts := getOpts(withPluginId("test")) - testOpts := getDefaultOptions() - testOpts.withPluginId = "test" - assert.Equal(t, opts, testOpts) + assert.Equal(t, "test", opts.withPluginId) }) t.Run("WithDescription", func(t *testing.T) { opts := getOpts(WithDescription("test desc")) - testOpts := getDefaultOptions() - testOpts.withDescription = "test desc" - assert.Equal(t, opts, testOpts) + assert.Equal(t, "test desc", opts.withDescription) }) t.Run("WithLimit", func(t *testing.T) { opts := getOpts(WithLimit(5)) - testOpts := getDefaultOptions() - testOpts.withLimit = 5 - assert.Equal(t, opts, testOpts) + assert.Equal(t, 5, opts.withLimit) }) t.Run("WithPreferredEndpoints", func(t *testing.T) { opts := getOpts(WithPreferredEndpoints([]string{"foo"})) - testOpts := getDefaultOptions() - testOpts.withPreferredEndpoints = []string{"foo"} - assert.EqualValues(t, opts, testOpts) + assert.EqualValues(t, []string{"foo"}, opts.withPreferredEndpoints) }) - t.Run("withDnsNames", func(t *testing.T) { + t.Run("WithDnsNames", func(t *testing.T) { opts := getOpts(withDnsNames([]string{"foo"})) - testOpts := getDefaultOptions() - testOpts.withDnsNames = []string{"foo"} - assert.EqualValues(t, opts, testOpts) + assert.EqualValues(t, []string{"foo"}, opts.withDnsNames) }) - t.Run("withIpAddresses", func(t *testing.T) { + t.Run("WithIpAddresses", func(t *testing.T) { opts := getOpts(withIpAddresses([]string{"foo"})) - testOpts := getDefaultOptions() - testOpts.withIpAddresses = []string{"foo"} - assert.EqualValues(t, opts, testOpts) + assert.EqualValues(t, []string{"foo"}, opts.withIpAddresses) }) } diff --git a/internal/host/plugin/repository_host_catalog.go b/internal/host/plugin/repository_host_catalog.go index d3e69ca593..2f97d9b125 100644 --- a/internal/host/plugin/repository_host_catalog.go +++ b/internal/host/plugin/repository_host_catalog.go @@ -233,7 +233,7 @@ func (r *Repository) UpdateCatalog(ctx context.Context, c *HostCatalog, version // Attributes are patched from the JSON included in the mask to // the attributes that exist in the record. dbMask = append(dbMask, "attributes") - newCatalog.Attributes, err = patchstruct.PatchBytes(newCatalog.Attributes, c.Attributes) + newCatalog.Attributes = patchstruct.PatchStruct(newCatalog.Attributes, c.Attributes) if err != nil { return nil, db.NoRowsAffected, db.NoRowsAffected, errors.Wrap(ctx, err, op, errors.WithMsg("error in catalog attribute JSON")) } @@ -608,12 +608,8 @@ func toPluginCatalog(ctx context.Context, in *HostCatalog) (*pb.HostCatalog, err Name: wrapperspb.String(in.GetName()), Description: wrapperspb.String(in.GetDescription()), } - if in.GetAttributes() != nil { - attrs := &structpb.Struct{} - if err := proto.Unmarshal(in.GetAttributes(), attrs); err != nil { - return nil, errors.Wrap(ctx, err, op, errors.WithMsg("unable to unmarshal attributes")) - } - hc.Attributes = attrs + if in.Attributes != nil { + hc.Attributes = in.Attributes } if in.Secrets != nil { hc.Secrets = in.Secrets diff --git a/internal/host/plugin/repository_host_catalog_test.go b/internal/host/plugin/repository_host_catalog_test.go index 4108ba2e51..f410424010 100644 --- a/internal/host/plugin/repository_host_catalog_test.go +++ b/internal/host/plugin/repository_host_catalog_test.go @@ -69,9 +69,9 @@ func TestRepository_CreateCatalog(t *testing.T) { name: "no-scope", in: &HostCatalog{ HostCatalog: &store.HostCatalog{ - PluginId: plg.GetPublicId(), - Attributes: []byte{}, + PluginId: plg.GetPublicId(), }, + Attributes: &structpb.Struct{}, }, wantIsErr: errors.InvalidParameter, }, @@ -79,9 +79,9 @@ func TestRepository_CreateCatalog(t *testing.T) { name: "no-plugin", in: &HostCatalog{ HostCatalog: &store.HostCatalog{ - ScopeId: prj.GetPublicId(), - Attributes: []byte{}, + ScopeId: prj.GetPublicId(), }, + Attributes: &structpb.Struct{}, }, wantIsErr: errors.InvalidParameter, }, @@ -99,44 +99,44 @@ func TestRepository_CreateCatalog(t *testing.T) { name: "valid-no-options", in: &HostCatalog{ HostCatalog: &store.HostCatalog{ - ScopeId: prj.GetPublicId(), - PluginId: plg.GetPublicId(), - Attributes: []byte{}, + ScopeId: prj.GetPublicId(), + PluginId: plg.GetPublicId(), }, + Attributes: &structpb.Struct{}, }, want: &HostCatalog{ HostCatalog: &store.HostCatalog{ - ScopeId: prj.GetPublicId(), - PluginId: plg.GetPublicId(), - Attributes: []byte{}, + ScopeId: prj.GetPublicId(), + PluginId: plg.GetPublicId(), }, + Attributes: &structpb.Struct{}, }, }, { name: "valid-unimplemented-plugin", in: &HostCatalog{ HostCatalog: &store.HostCatalog{ - ScopeId: prj.GetPublicId(), - PluginId: unimplementedPlugin.GetPublicId(), - Attributes: []byte{}, + ScopeId: prj.GetPublicId(), + PluginId: unimplementedPlugin.GetPublicId(), }, + Attributes: &structpb.Struct{}, }, want: &HostCatalog{ HostCatalog: &store.HostCatalog{ - ScopeId: prj.GetPublicId(), - PluginId: unimplementedPlugin.GetPublicId(), - Attributes: []byte{}, + ScopeId: prj.GetPublicId(), + PluginId: unimplementedPlugin.GetPublicId(), }, + Attributes: &structpb.Struct{}, }, }, { name: "not-found-plugin", in: &HostCatalog{ HostCatalog: &store.HostCatalog{ - ScopeId: prj.GetPublicId(), - PluginId: "unknown_plugin", - Attributes: []byte{}, + ScopeId: prj.GetPublicId(), + PluginId: "unknown_plugin", }, + Attributes: &structpb.Struct{}, }, wantIsErr: errors.InvalidParameter, }, @@ -144,19 +144,19 @@ func TestRepository_CreateCatalog(t *testing.T) { name: "valid-with-name", in: &HostCatalog{ HostCatalog: &store.HostCatalog{ - Name: "test-name-repo", - ScopeId: prj.GetPublicId(), - PluginId: plg.GetPublicId(), - Attributes: []byte{}, + Name: "test-name-repo", + ScopeId: prj.GetPublicId(), + PluginId: plg.GetPublicId(), }, + Attributes: &structpb.Struct{}, }, want: &HostCatalog{ HostCatalog: &store.HostCatalog{ - Name: "test-name-repo", - ScopeId: prj.GetPublicId(), - PluginId: plg.GetPublicId(), - Attributes: []byte{}, + Name: "test-name-repo", + ScopeId: prj.GetPublicId(), + PluginId: plg.GetPublicId(), }, + Attributes: &structpb.Struct{}, }, }, { @@ -166,16 +166,16 @@ func TestRepository_CreateCatalog(t *testing.T) { Description: "test-description-repo", ScopeId: prj.GetPublicId(), PluginId: plg.GetPublicId(), - Attributes: []byte{}, }, + Attributes: &structpb.Struct{}, }, want: &HostCatalog{ HostCatalog: &store.HostCatalog{ Description: "test-description-repo", ScopeId: prj.GetPublicId(), PluginId: plg.GetPublicId(), - Attributes: []byte{}, }, + Attributes: &structpb.Struct{}, }, }, { @@ -184,27 +184,15 @@ func TestRepository_CreateCatalog(t *testing.T) { HostCatalog: &store.HostCatalog{ ScopeId: prj.GetPublicId(), PluginId: plg.GetPublicId(), - Attributes: func() []byte { - st, err := structpb.NewStruct(map[string]interface{}{"k1": "foo"}) - require.NoError(t, err) - b, err := proto.Marshal(st) - require.NoError(t, err) - return b - }(), }, + Attributes: mustStruct(map[string]interface{}{"k1": "foo"}), }, want: &HostCatalog{ HostCatalog: &store.HostCatalog{ ScopeId: prj.GetPublicId(), PluginId: plg.GetPublicId(), - Attributes: func() []byte { - st, err := structpb.NewStruct(map[string]interface{}{"k1": "foo"}) - require.NoError(t, err) - b, err := proto.Marshal(st) - require.NoError(t, err) - return b - }(), }, + Attributes: mustStruct(map[string]interface{}{"k1": "foo"}), }, }, { @@ -214,17 +202,13 @@ func TestRepository_CreateCatalog(t *testing.T) { Description: "test-description-repo", ScopeId: prj.GetPublicId(), PluginId: plg.GetPublicId(), - Attributes: []byte{}, }, - Secrets: func() *structpb.Struct { - st, err := structpb.NewStruct(map[string]interface{}{ - "k1": "v1", - "k2": 2, - "k3": nil, - }) - require.NoError(t, err) - return st - }(), + Attributes: &structpb.Struct{}, + Secrets: mustStruct(map[string]interface{}{ + "k1": "v1", + "k2": 2, + "k3": nil, + }), }, want: &HostCatalog{ HostCatalog: &store.HostCatalog{ @@ -233,16 +217,13 @@ func TestRepository_CreateCatalog(t *testing.T) { PluginId: plg.GetPublicId(), Attributes: []byte{}, }, + Attributes: &structpb.Struct{}, }, - wantSecret: func() *structpb.Struct { - st, err := structpb.NewStruct(map[string]interface{}{ - "k1": "v1", - "k2": 2, - "k3": nil, - }) - require.NoError(t, err) - return st - }(), + wantSecret: mustStruct(map[string]interface{}{ + "k1": "v1", + "k2": 2, + "k3": nil, + }), }, } @@ -269,11 +250,8 @@ func TestRepository_CreateCatalog(t *testing.T) { assert.Equal(tt.want.Description, got.Description) assert.Equal(got.CreateTime, got.UpdateTime) - // wantedPluginAttributes := &structpb.Struct{} - // require.NoError(t, proto.Unmarshal(tt.want.GetAttributes(), wantedPluginAttributes)) - gotB, err := proto.Marshal(gotPluginAttrs) - require.NoError(t, err) - assert.Equal(tt.want.GetAttributes(), gotB) + // Compare attributes from request with attributes in expected + assert.Empty(cmp.Diff(tt.want.GetAttributes(), gotPluginAttrs, protocmp.Transform())) assert.NoError(db.TestVerifyOplog(t, rw, got.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_CREATE), db.WithCreateNotBefore(10*time.Second))) @@ -308,11 +286,11 @@ func TestRepository_CreateCatalog(t *testing.T) { _, prj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) in := &HostCatalog{ HostCatalog: &store.HostCatalog{ - ScopeId: prj.GetPublicId(), - Name: "test-name-repo", - PluginId: plg.GetPublicId(), - Attributes: []byte{}, + ScopeId: prj.GetPublicId(), + Name: "test-name-repo", + PluginId: plg.GetPublicId(), }, + Attributes: &structpb.Struct{}, } got, _, err := repo.CreateCatalog(context.Background(), in) @@ -338,10 +316,10 @@ func TestRepository_CreateCatalog(t *testing.T) { org, prj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) in := &HostCatalog{ HostCatalog: &store.HostCatalog{ - Name: "test-name-repo", - PluginId: plg.GetPublicId(), - Attributes: []byte{}, + Name: "test-name-repo", + PluginId: plg.GetPublicId(), }, + Attributes: &structpb.Struct{}, } in2 := in.clone() @@ -449,7 +427,7 @@ func TestRepository_UpdateCatalog(t *testing.T) { changeAttributes := func(m map[string]interface{}) changeHostCatalogFunc { return func(c *HostCatalog) *HostCatalog { - c.Attributes = mustMarshal(m) + c.Attributes = mustStruct(m) return c } } @@ -519,10 +497,7 @@ func TestRepository_UpdateCatalog(t *testing.T) { return func(t *testing.T, ctx context.Context) { t.Helper() assert := assert.New(t) - require := require.New(t) - st := &structpb.Struct{} - require.NoError(proto.Unmarshal(gotCatalog.Attributes, st)) - assert.Empty(cmp.Diff(mustStruct(want), st, protocmp.Transform())) + assert.Empty(cmp.Diff(mustStruct(want), gotCatalog.Attributes, protocmp.Transform())) } } @@ -899,7 +874,7 @@ func TestRepository_UpdateCatalog(t *testing.T) { cat := TestCatalog(t, dbConn, projectScope.PublicId, testPlugin.GetPublicId()) // Set some (default) attributes on our test catalog - cat.Attributes = mustMarshal(map[string]interface{}{ + cat.Attributes = mustStruct(map[string]interface{}{ "foo": "bar", }) @@ -1237,14 +1212,3 @@ func mustStruct(in map[string]interface{}) *structpb.Struct { return out } - -// mustMarshal behaves like mustStruct but also converts the Struct -// to wire-format data. -func mustMarshal(in map[string]interface{}) []byte { - b, err := proto.Marshal(mustStruct(in)) - if err != nil { - panic(err) - } - - return b -} diff --git a/internal/host/plugin/testing.go b/internal/host/plugin/testing.go index 45d38c42e0..7fee03baaa 100644 --- a/internal/host/plugin/testing.go +++ b/internal/host/plugin/testing.go @@ -26,8 +26,7 @@ func TestCatalog(t *testing.T, conn *db.DB, scopeId, pluginId string, opt ...Opt ctx := context.Background() w := db.New(conn) - cat, err := NewHostCatalog(ctx, scopeId, pluginId, opt...) - require.NoError(t, err) + cat := NewHostCatalog(ctx, scopeId, pluginId, opt...) assert.NotNil(t, cat) plg := host.NewPlugin()