internal/host/plugin: remove custom queries for secrets (#1676)

* internal/host/plugin: remove custom queries for secrets

This removes the custom queries for host catalog secrets, now that #1658
has been merged.

It also does a lot of cleanup surrounding the related oplog entries,
using db.WithOplog natively.

* Simplify conflict actions by replacing SetColumnValues with SetColumns

* Remove unused oplog method

internal/host/plugin/host_catalog_secret.go

@@ -2,12 +2,9 @@ package plugin
 
 import (
 	"context"
-	"database/sql"
 
-	"github.com/hashicorp/boundary/internal/db"
 	"github.com/hashicorp/boundary/internal/errors"
 	"github.com/hashicorp/boundary/internal/host/plugin/store"
-	"github.com/hashicorp/boundary/internal/oplog"
 	wrapping "github.com/hashicorp/go-kms-wrapping"
 	"github.com/hashicorp/go-kms-wrapping/structwrapping"
 	"google.golang.org/protobuf/proto"
@@ -89,42 +86,3 @@ func (c *HostCatalogSecret) decrypt(ctx context.Context, cipher wrapping.Wrapper
 	c.CtSecret = nil
 	return nil
 }
-
-// TODO(ICU-2835): this is necessary as update logic requires a static
-// public_id or private_id field. Once post-lookup updates can
-// support alternate primary key fields, we can port this to native
-// Update and upserts w/OnConflict.
-func (c *HostCatalogSecret) upsertQuery() (query string, queryValues []interface{}) {
-	query = upsertHostCatalogSecretQuery
-	queryValues = []interface{}{
-		sql.Named("catalog_id", c.CatalogId),
-		sql.Named("secret", c.CtSecret),
-		sql.Named("key_id", c.KeyId),
-	}
-	return
-}
-
-func (c *HostCatalogSecret) deleteQuery() (query string, queryValues []interface{}) {
-	query = deleteHostCatalogSecretQuery
-	queryValues = []interface{}{
-		sql.Named("catalog_id", c.CatalogId),
-	}
-	return
-}
-
-func (c *HostCatalogSecret) oplogMessage(opType db.OpType) *oplog.Message {
-	msg := oplog.Message{
-		Message:  c.clone(),
-		TypeName: c.TableName(),
-	}
-	switch opType {
-	case db.CreateOp:
-		msg.OpType = oplog.OpType_OP_TYPE_CREATE
-	case db.UpdateOp:
-		msg.OpType = oplog.OpType_OP_TYPE_UPDATE
-		msg.FieldMaskPaths = []string{"secret", "key_id"}
-	case db.DeleteOp:
-		msg.OpType = oplog.OpType_OP_TYPE_DELETE
-	}
-	return &msg
-}

internal/host/plugin/host_catalog_secret_test.go

@@ -118,57 +118,82 @@ func TestHostCatalogSecret_New(t *testing.T) {
 	}
 }
 
-func TestHostCatalogSecret_Custom_Queries(t *testing.T) {
-	ctx := context.Background()
+func TestHostCatalogSecret_Create_Upsert_Update_Delete(t *testing.T) {
 	conn, _ := db.TestSetup(t, "postgres")
-	rw := db.New(conn)
 	wrapper := db.TestWrapper(t)
 	kkms := kms.TestKms(t, conn, wrapper)
 
 	_, prj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper))
 	plg := host.TestPlugin(t, conn, "test")
 	cat := TestCatalog(t, conn, prj.GetPublicId(), plg.GetPublicId())
+	ctx := context.Background()
+
+	secret, err := newHostCatalogSecret(ctx, cat.GetPublicId(), mustStruct(map[string]interface{}{
+		"foo": "bar",
+	}))
+	require.NoError(t, err)
+	require.NotNil(t, secret)
+	require.Empty(t, secret.CtSecret)
+
 	databaseWrapper, err := kkms.GetWrapper(ctx, prj.PublicId, kms.KeyPurposeDatabase)
 	require.NoError(t, err)
+	require.NotNil(t, databaseWrapper)
+	require.NoError(t, secret.encrypt(ctx, databaseWrapper))
 
-	hcs, err := newHostCatalogSecret(ctx, cat.GetPublicId(),
-		&structpb.Struct{Fields: map[string]*structpb.Value{"foo": structpb.NewStringValue("bar")}})
-	require.NoError(t, err)
-	assert.NoError(t, hcs.encrypt(ctx, databaseWrapper))
-	q, v := hcs.upsertQuery()
-	_, err = rw.Exec(ctx, q, v)
-	assert.NoError(t, err)
+	// Create
+	w := db.New(conn)
+	require.NoError(t, w.Create(ctx, secret))
 
-	found, err := newHostCatalogSecret(ctx, cat.GetPublicId(), nil)
-	require.NoError(t, err)
-	assert.NoError(t, rw.LookupWhere(ctx, found, "catalog_id=?", found.GetCatalogId()))
+	// Upsert
+	newStructUpsert := mustMarshal(map[string]interface{}{
+		"baz": "qux",
+	})
+	newSecretUpsert := secret.clone()
+	newSecretUpsert.Secret = newStructUpsert
+	require.NoError(t, newSecretUpsert.encrypt(ctx, databaseWrapper))
+	require.NoError(t, w.Create(ctx, newSecretUpsert, db.WithOnConflict(&db.OnConflict{
+		Target: db.Columns{"catalog_id"},
+		Action: db.SetColumns([]string{"secret", "key_id"}),
+	})))
+	found := &HostCatalogSecret{
+		HostCatalogSecret: &store.HostCatalogSecret{
+			CatalogId: cat.GetPublicId(),
+		},
+	}
+	require.NoError(t, w.LookupById(ctx, found))
+	assert.Empty(t, cmp.Diff(newSecretUpsert.HostCatalogSecret, found.HostCatalogSecret, protocmp.Transform()))
 	require.NoError(t, found.decrypt(ctx, databaseWrapper))
-	require.NoError(t, hcs.decrypt(ctx, databaseWrapper))
-	// update the created/updated time from the original
-	hcs.CreateTime, hcs.UpdateTime = found.CreateTime, found.UpdateTime
-	assert.Empty(t, cmp.Diff(hcs, found, protocmp.Transform()))
+	assert.Empty(t, cmp.Diff(newStructUpsert, found.Secret, protocmp.Transform()))
 
-	// Update the secret and see the value updated.
-	updated, err := newHostCatalogSecret(ctx, cat.GetPublicId(),
-		&structpb.Struct{Fields: map[string]*structpb.Value{"updated": structpb.NewStringValue("value")}})
+	// Update
+	newStructUpdate := mustMarshal(map[string]interface{}{
+		"one": "two",
+	})
+	newSecretUpdate := newSecretUpsert.clone()
+	newSecretUpdate.Secret = newStructUpdate
+	require.NoError(t, newSecretUpdate.encrypt(ctx, databaseWrapper))
+	rowsUpdated, err := w.Update(ctx, newSecretUpdate, []string{"CtSecret"}, []string{})
 	require.NoError(t, err)
-	assert.NoError(t, updated.encrypt(ctx, databaseWrapper))
-	q, v = updated.upsertQuery()
-	_, err = rw.Exec(ctx, q, v)
-	assert.NoError(t, err)
-
-	assert.NoError(t, rw.LookupWhere(ctx, found, "catalog_id=?", found.GetCatalogId()))
+	require.Equal(t, 1, rowsUpdated)
+	found = &HostCatalogSecret{
+		HostCatalogSecret: &store.HostCatalogSecret{
+			CatalogId: cat.GetPublicId(),
+		},
+	}
+	require.NoError(t, w.LookupById(ctx, found))
+	assert.Empty(t, cmp.Diff(newSecretUpdate.HostCatalogSecret, found.HostCatalogSecret, protocmp.Transform()))
 	require.NoError(t, found.decrypt(ctx, databaseWrapper))
-	require.NoError(t, updated.decrypt(ctx, databaseWrapper))
-	// set the created time to the first and update time from the newly found
-	updated.CreateTime, updated.UpdateTime = hcs.CreateTime, found.UpdateTime
-	assert.Empty(t, cmp.Diff(updated, found, protocmp.Transform()))
+	assert.Empty(t, cmp.Diff(newStructUpdate, found.Secret, protocmp.Transform()))
 
-	// Try to delete this secret.
-	q, v = updated.deleteQuery()
-	_, err = rw.Exec(ctx, q, v)
-	assert.NoError(t, err)
-	err = rw.LookupWhere(ctx, found, "catalog_id=?", found.GetCatalogId())
-	assert.Error(t, err)
-	assert.True(t, errors.IsNotFoundError(err))
+	// Delete
+	rowsDeleted, err := w.Delete(ctx, found)
+	require.NoError(t, err)
+	require.Equal(t, 1, rowsDeleted)
+	err = w.LookupById(ctx, &HostCatalogSecret{
+		HostCatalogSecret: &store.HostCatalogSecret{
+			CatalogId: cat.GetPublicId(),
+		},
+	})
+	require.Error(t, err)
+	require.True(t, errors.IsNotFoundError(err))
 }

internal/host/plugin/repository_host_catalog.go

@@ -142,15 +142,11 @@ func (r *Repository) CreateCatalog(ctx context.Context, c *HostCatalog, _ ...Opt
 				}
 				if hcSecret != nil {
 					newSecret := hcSecret.clone()
-					q, v := newSecret.upsertQuery()
-					rows, err := w.Exec(ctx, q, v)
-					if err != nil {
+					var sOplogMsg oplog.Message
+					if err := w.Create(ctx, newSecret, db.NewOplogMsg(&sOplogMsg)); err != nil {
 						return errors.Wrap(ctx, err, op)
 					}
-					if rows > 1 {
-						return errors.New(ctx, errors.MultipleRecords, op, "more than 1 catalog secret would have been created")
-					}
-					msgs = append(msgs, newSecret.oplogMessage(db.CreateOp))
+					msgs = append(msgs, &sOplogMsg)
 				}
 			}
 
@@ -400,16 +396,16 @@ func (r *Repository) UpdateCatalog(ctx context.Context, c *HostCatalog, version
 					// We didn't set/encrypt the persisted data because there was
 					// none returned. Just delete the entry.
 					deletedSecret := hcSecret.clone()
-					q, v := deletedSecret.deleteQuery()
-					secretsUpdated, err := w.Exec(ctx, q, v)
+					var sOplogMsg oplog.Message
+					secretsDeleted, err := w.Delete(ctx, deletedSecret, db.NewOplogMsg(&sOplogMsg))
 					if err != nil {
 						return errors.Wrap(ctx, err, op)
 					}
-					if secretsUpdated != 1 {
-						return errors.New(ctx, errors.MultipleRecords, op, fmt.Sprintf("expected 1 catalog secret to be deleted, got %d", secretsUpdated))
+					if secretsDeleted != 1 {
+						return errors.New(ctx, errors.MultipleRecords, op, fmt.Sprintf("expected 1 catalog secret to be deleted, got %d", secretsDeleted))
 					}
 					updatedPersisted = true
-					msgs = append(msgs, deletedSecret.oplogMessage(db.DeleteOp))
+					msgs = append(msgs, &sOplogMsg)
 				} else {
 					hcSecret, err := newHostCatalogSecret(ctx, currentCatalog.GetPublicId(), plgResp.GetPersisted().GetSecrets())
 					if err != nil {
@@ -421,16 +417,20 @@ func (r *Repository) UpdateCatalog(ctx context.Context, c *HostCatalog, version
 
 					// Update the secrets.
 					updatedSecret := hcSecret.clone()
-					q, v := updatedSecret.upsertQuery()
-					secretsUpdated, err := w.Exec(ctx, q, v)
-					if err != nil {
+					var sOplogMsg oplog.Message
+					if err := w.Create(
+						ctx,
+						updatedSecret,
+						db.WithOnConflict(&db.OnConflict{
+							Target: db.Columns{"catalog_id"},
+							Action: db.SetColumns([]string{"secret", "key_id"}),
+						}),
+						db.NewOplogMsg(&sOplogMsg),
+					); err != nil {
 						return errors.Wrap(ctx, err, op)
 					}
-					if secretsUpdated != 1 {
-						return errors.New(ctx, errors.MultipleRecords, op, fmt.Sprintf("expected 1 catalog secret to be updated, got %d", secretsUpdated))
-					}
 					updatedPersisted = true
-					msgs = append(msgs, updatedSecret.oplogMessage(db.UpdateOp))
+					msgs = append(msgs, &sOplogMsg)
 				}
 			}
 

internal/host/plugin/repository_host_catalog_test.go

@@ -1119,10 +1119,8 @@ func TestRepository_UpdateCatalog(t *testing.T) {
 		cSecret, err := newHostCatalogSecret(ctx, cat.GetPublicId(), cat.Secrets)
 		require.NoError(err)
 		require.NoError(cSecret.encrypt(ctx, scopeWrapper))
-		cSecretQ, cSecretV := cSecret.upsertQuery()
-		secretsUpdated, err := dbRW.Exec(ctx, cSecretQ, cSecretV)
+		err = dbRW.Create(ctx, cSecret)
 		require.NoError(err)
-		require.Equal(1, secretsUpdated)
 
 		// Set some (default) attributes on our test catalog and update SecretsHmac at the same time
 		cat.Attributes = mustMarshal(map[string]interface{}{

internal/host/plugin/repository_host_set_test.go

@@ -424,10 +424,8 @@ func TestRepository_UpdateSet(t *testing.T) {
 	scopeWrapper, err := dbKmsCache.GetWrapper(ctx, testCatalog.GetScopeId(), kms.KeyPurposeDatabase)
 	require.NoError(t, err)
 	require.NoError(t, testCatalogSecret.encrypt(ctx, scopeWrapper))
-	testCatalogSecretQ, testCatalogSecretV := testCatalogSecret.upsertQuery()
-	secretsUpdated, err := dbRW.Exec(ctx, testCatalogSecretQ, testCatalogSecretV)
+	err = dbRW.Create(ctx, testCatalogSecret)
 	require.NoError(t, err)
-	require.Equal(t, 1, secretsUpdated)
 
 	// Create a test duplicate set. We don't use this set, it just
 	// exists to ensure that we can test for conflicts when setting