diff --git a/internal/host/plugin/host_catalog_secret.go b/internal/host/plugin/host_catalog_secret.go index 84eda4867d..d618b2bbff 100644 --- a/internal/host/plugin/host_catalog_secret.go +++ b/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 -} diff --git a/internal/host/plugin/host_catalog_secret_test.go b/internal/host/plugin/host_catalog_secret_test.go index 09bb7af022..146af909b9 100644 --- a/internal/host/plugin/host_catalog_secret_test.go +++ b/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()) - databaseWrapper, err := kkms.GetWrapper(ctx, prj.PublicId, kms.KeyPurposeDatabase) - require.NoError(t, err) + ctx := context.Background() - hcs, err := newHostCatalogSecret(ctx, cat.GetPublicId(), - &structpb.Struct{Fields: map[string]*structpb.Value{"foo": structpb.NewStringValue("bar")}}) + secret, err := newHostCatalogSecret(ctx, cat.GetPublicId(), mustStruct(map[string]interface{}{ + "foo": "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) + require.NotNil(t, secret) + require.Empty(t, secret.CtSecret) - found, err := newHostCatalogSecret(ctx, cat.GetPublicId(), nil) + databaseWrapper, err := kkms.GetWrapper(ctx, prj.PublicId, kms.KeyPurposeDatabase) require.NoError(t, err) - assert.NoError(t, rw.LookupWhere(ctx, found, "catalog_id=?", found.GetCatalogId())) + require.NotNil(t, databaseWrapper) + require.NoError(t, secret.encrypt(ctx, databaseWrapper)) + + // Create + w := db.New(conn) + require.NoError(t, w.Create(ctx, secret)) + + // 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())) - - // 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")}}) + assert.Empty(t, cmp.Diff(newStructUpsert, found.Secret, protocmp.Transform())) + + // 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())) - - // 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)) + assert.Empty(t, cmp.Diff(newStructUpdate, found.Secret, protocmp.Transform())) + + // 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)) } diff --git a/internal/host/plugin/repository_host_catalog.go b/internal/host/plugin/repository_host_catalog.go index 2bf89a2c38..3b6f6469bf 100644 --- a/internal/host/plugin/repository_host_catalog.go +++ b/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) } } diff --git a/internal/host/plugin/repository_host_catalog_test.go b/internal/host/plugin/repository_host_catalog_test.go index 3125a81acb..32e3716399 100644 --- a/internal/host/plugin/repository_host_catalog_test.go +++ b/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{}{ diff --git a/internal/host/plugin/repository_host_set_test.go b/internal/host/plugin/repository_host_set_test.go index 581fb0757a..31fa3c55e2 100644 --- a/internal/host/plugin/repository_host_set_test.go +++ b/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