WIP - transparent attributes struct/wire conversion

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()
+}

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)
+	})
+}

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) {

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)
 	})
 }

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

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
-}

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()