diff --git a/internal/host/plugin/host_catalog_secret.go b/internal/host/plugin/host_catalog_secret.go index 7daf0abf3..84eda4867 100644 --- a/internal/host/plugin/host_catalog_secret.go +++ b/internal/host/plugin/host_catalog_secret.go @@ -90,7 +90,11 @@ func (c *HostCatalogSecret) decrypt(ctx context.Context, cipher wrapping.Wrapper return nil } -func (c *HostCatalogSecret) insertQuery() (query string, queryValues []interface{}) { +// 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), @@ -118,6 +122,7 @@ func (c *HostCatalogSecret) oplogMessage(opType db.OpType) *oplog.Message { 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 } diff --git a/internal/host/plugin/host_catalog_secret_test.go b/internal/host/plugin/host_catalog_secret_test.go index 9a24404b1..09bb7af02 100644 --- a/internal/host/plugin/host_catalog_secret_test.go +++ b/internal/host/plugin/host_catalog_secret_test.go @@ -135,7 +135,7 @@ func TestHostCatalogSecret_Custom_Queries(t *testing.T) { &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.insertQuery() + q, v := hcs.upsertQuery() _, err = rw.Exec(ctx, q, v) assert.NoError(t, err) @@ -153,7 +153,7 @@ func TestHostCatalogSecret_Custom_Queries(t *testing.T) { &structpb.Struct{Fields: map[string]*structpb.Value{"updated": structpb.NewStringValue("value")}}) require.NoError(t, err) assert.NoError(t, updated.encrypt(ctx, databaseWrapper)) - q, v = updated.insertQuery() + q, v = updated.upsertQuery() _, err = rw.Exec(ctx, q, v) assert.NoError(t, err) diff --git a/internal/host/plugin/repository_host_catalog.go b/internal/host/plugin/repository_host_catalog.go index 6007106d3..dda662693 100644 --- a/internal/host/plugin/repository_host_catalog.go +++ b/internal/host/plugin/repository_host_catalog.go @@ -3,11 +3,13 @@ package plugin import ( "context" "fmt" + "strings" "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/host" "github.com/hashicorp/boundary/internal/kms" + "github.com/hashicorp/boundary/internal/libs/patchstruct" "github.com/hashicorp/boundary/internal/observability/event" "github.com/hashicorp/boundary/internal/oplog" hostplugin "github.com/hashicorp/boundary/internal/plugin/host" @@ -18,6 +20,7 @@ import ( "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/structpb" + "google.golang.org/protobuf/types/known/wrapperspb" ) // CreateCatalog inserts c into the repository and returns a new @@ -113,7 +116,7 @@ func (r *Repository) CreateCatalog(ctx context.Context, c *HostCatalog, _ ...Opt if hcSecret != nil { newSecret := hcSecret.clone() - q, v := newSecret.insertQuery() + q, v := newSecret.upsertQuery() rows, err := w.Exec(ctx, q, v) if err != nil { return errors.Wrap(ctx, err, op) @@ -145,6 +148,299 @@ func (r *Repository) CreateCatalog(ctx context.Context, c *HostCatalog, _ ...Opt return newHostCatalog, plg, nil } +// UpdateCatalog updates the repository entry for c.PublicId with the +// values in c for the fields listed in fieldMask. It returns a new +// HostCatalog containing the updated values and a count of the number of +// records updated. c is not changed. +// +// c must contain a valid PublicId. c.Name, c.Description, and +// c.Attributes can be updated; if c.Secrets is present, its contents +// are sent to the plugin (along with any other changes, see below) +// before the update is sent to the database. +// +// An attribute of c will be set to NULL in the database if the +// attribute in c is the zero value and it is included in fieldMask. +// Note that this does not apply to c.Attributes - a null +// c.Attributes is a no-op for modifications. Rather, if fields need +// to be reset, its field in c.Attributes should individually set to +// null. +// +// Updates are sent to OnUpdateCatalog with a full copy of both the +// current catalog, and the state of the new catalog should it be +// updated, along with any secrets included in the new request. This +// request may alter the returned persisted state. Update of the +// record in the database is aborted if this call fails. +func (r *Repository) UpdateCatalog(ctx context.Context, c *HostCatalog, version uint32, fieldMask []string, _ ...Option) (*HostCatalog, *hostplugin.Plugin, int, error) { + const op = "plugin.(Repository).UpdateCatalog" + if c == nil { + return nil, nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "nil HostCatalog") + } + if c.HostCatalog == nil { + return nil, nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "nil embedded HostCatalog") + } + if c.PublicId == "" { + return nil, nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "no public id") + } + if c.ScopeId == "" { + return nil, nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "no scope id") + } + if len(fieldMask) == 0 { + return nil, nil, db.NoRowsAffected, errors.New(ctx, errors.EmptyFieldMask, op, "empty field mask") + } + + // Get the old catalog first. We patch the record first before + // sending it to the DB for updating so that we can run on + // OnUpdateCatalog. Note that the field masks are still used for + // updating. + currentCatalog, currentCatalogPersisted, err := r.getCatalog(ctx, c.PublicId) + if err != nil { + return nil, nil, db.NoRowsAffected, errors.Wrap(ctx, err, op, errors.WithMsg(fmt.Sprintf("error looking up catalog with id %q", c.PublicId))) + } + + if currentCatalog == nil { + return nil, nil, db.NoRowsAffected, errors.New(ctx, errors.RecordNotFound, op, fmt.Sprintf("catalog with id %q not found", c.PublicId)) + } + + // Assert the version of the current catalog to make sure we're + // updating the correct one. + if currentCatalog.GetVersion() != version { + return nil, nil, db.NoRowsAffected, errors.New(ctx, errors.VersionMismatch, op, fmt.Sprintf("catalog version mismatch, want=%d, got=%d", currentCatalog.GetVersion(), version)) + } + + // Clone the catalog so that we can set fields. + newCatalog := currentCatalog.clone() + var updateAttributes bool + var dbMask, nullFields []string + for _, f := range fieldMask { + switch { + case strings.EqualFold("name", f) && c.Name == "": + nullFields = append(nullFields, "name") + newCatalog.Name = c.Name + case strings.EqualFold("name", f) && c.Name != "": + dbMask = append(dbMask, "name") + newCatalog.Name = c.Name + case strings.EqualFold("description", f) && c.Description == "": + nullFields = append(nullFields, "description") + newCatalog.Description = c.Description + case strings.EqualFold("description", f) && c.Description != "": + dbMask = append(dbMask, "description") + newCatalog.Description = c.Description + case strings.EqualFold("attributes", strings.Split(f, ".")[0]): + // Flag attributes for updating. While multiple masks may be + // sent, we only need to do this once. + updateAttributes = true + case strings.EqualFold("secrets", strings.Split(f, ".")[0]): + // While in a similar format, secrets are passed along + // wholesale (for the time being). Don't append this mask + // field, as secrets do not have a database entry. Clear the + // secrets out of the working set after. + newCatalog.Secrets = c.Secrets + c.Secrets = nil + + default: + return nil, nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidFieldMask, op, fmt.Sprintf("invalid field mask: %s", f)) + } + } + + if updateAttributes && c.Attributes != nil { + dbMask = append(dbMask, "attributes") + newCatalog.Attributes, err = patchstruct.PatchBytes(newCatalog.Attributes, c.Attributes) + if err != nil { + return nil, nil, db.NoRowsAffected, errors.Wrap(ctx, err, op, errors.WithMsg("error in catalog attribute JSON")) + } + } + + // Get the plugin for the host catalog - this is to return it back + // after the update is complete. We don't actually do anything with + // the record otherwise. Fetch it here so that if there's an + // integrity error, we don't call the plugin. + plg, err := r.getPlugin(ctx, currentCatalog.GetPluginId()) + if err != nil { + return nil, nil, db.NoRowsAffected, errors.Wrap(ctx, err, op) + } + + // Get the plugin client. + plgClient, ok := r.plugins[currentCatalog.GetPluginId()] + if !ok || plgClient == nil { + return nil, nil, db.NoRowsAffected, errors.New(ctx, errors.Internal, op, fmt.Sprintf("plugin %q not available", currentCatalog.GetPluginId())) + } + + // Convert the catalog values to API protobuf values, which is what + // we use for the plugin hook calls. + currPlgHc, err := toPluginCatalog(ctx, currentCatalog) + if err != nil { + return nil, nil, db.NoRowsAffected, errors.Wrap(ctx, err, op) + } + newPlgHc, err := toPluginCatalog(ctx, newCatalog) + if err != nil { + return nil, nil, db.NoRowsAffected, errors.Wrap(ctx, err, op) + } + + plgResp, err := plgClient.OnUpdateCatalog(ctx, &plgpb.OnUpdateCatalogRequest{ + CurrentCatalog: currPlgHc, + NewCatalog: newPlgHc, + Persisted: currentCatalogPersisted, + }) + if err != nil { + if status.Code(err) != codes.Unimplemented { + return nil, nil, db.NoRowsAffected, errors.Wrap(ctx, err, op) + } + } + + // Success for OnUpdateCatalog. This means we can start updating. + // First, check for returned persisted data and encrypt it. + var hcSecret *HostCatalogSecret + var deleteSecrets bool + if plgResp != nil && plgResp.GetPersisted().GetSecrets() != nil { + if len(plgResp.GetPersisted().GetSecrets().GetFields()) == 0 { + // Flag the secret to be deleted. + hcSecret, err = newHostCatalogSecret(ctx, currentCatalog.GetPublicId(), plgResp.GetPersisted().GetSecrets()) + if err != nil { + return nil, nil, db.NoRowsAffected, errors.Wrap(ctx, err, op) + } + + deleteSecrets = true + } else { + hcSecret, err = newHostCatalogSecret(ctx, currentCatalog.GetPublicId(), plgResp.GetPersisted().GetSecrets()) + if err != nil { + return nil, nil, db.NoRowsAffected, errors.Wrap(ctx, err, op) + } + dbWrapper, err := r.kms.GetWrapper(ctx, newCatalog.ScopeId, kms.KeyPurposeDatabase) + if err != nil { + return nil, nil, db.NoRowsAffected, errors.Wrap(ctx, err, op, errors.WithMsg("unable to get db wrapper")) + } + if err := hcSecret.encrypt(ctx, dbWrapper); err != nil { + return nil, nil, db.NoRowsAffected, errors.Wrap(ctx, err, op) + } + } + } + + // Get the oplog. + oplogWrapper, err := r.kms.GetWrapper(ctx, newCatalog.ScopeId, kms.KeyPurposeOplog) + if err != nil { + return nil, nil, db.NoRowsAffected, errors.Wrap(ctx, err, op, errors.WithMsg("unable to get oplog wrapper")) + } + + var recordUpdated bool + var returnedCatalog *HostCatalog + _, err = r.writer.DoTx( + ctx, + db.StdRetryCnt, + db.ExpBackoff{}, + func(_ db.Reader, w db.Writer) error { + msgs := make([]*oplog.Message, 0, 3) + ticket, err := w.GetTicket(newCatalog) + if err != nil { + return errors.Wrap(ctx, err, op, errors.WithMsg("unable to get ticket")) + } + + if len(dbMask) != 0 || len(nullFields) != 0 { + returnedCatalog = newCatalog.clone() + var cOplogMsg oplog.Message + catalogsUpdated, err := w.Update( + ctx, + returnedCatalog, + dbMask, + nullFields, + db.NewOplogMsg(&cOplogMsg), + db.WithVersion(&version), + ) + if err != nil { + return errors.Wrap(ctx, err, op) + } + if catalogsUpdated != 1 { + return errors.New(ctx, errors.MultipleRecords, op, fmt.Sprintf("expected 1 catalog to be deleted, got %d", catalogsUpdated)) + } + msgs = append(msgs, &cOplogMsg) + recordUpdated = true + } else { + // Returned catalog needs to be the old copy, as no fields in the + // catalog itself are being updated (note: secrets may still be + // updated). + returnedCatalog = currentCatalog.clone() + } + + if hcSecret != nil { + if deleteSecrets { + // We didn't set/encrypt the persisted data because there was + // none returned. Just delete the entry. + deletedSecret := hcSecret.clone() + q, v := deletedSecret.deleteQuery() + var err error + secretsUpdated, err := w.Exec(ctx, q, v) + 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)) + } + msgs = append(msgs, deletedSecret.oplogMessage(db.DeleteOp)) + } else { + // Update the secrets. + updatedSecret := hcSecret.clone() + q, v := updatedSecret.upsertQuery() + var err error + secretsUpdated, err := w.Exec(ctx, q, v) + 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 updated, got %d", secretsUpdated)) + } + msgs = append(msgs, updatedSecret.oplogMessage(db.UpdateOp)) + } + + if !recordUpdated { + // we only updated secrets, so we need to increment the + // version of the host catalog manually. + returnedCatalog = newCatalog.clone() + returnedCatalog.Version = uint32(version) + 1 + var cOplogMsg oplog.Message + catalogsUpdated, err := w.Update( + ctx, + returnedCatalog, + []string{"version"}, + []string{}, + db.NewOplogMsg(&cOplogMsg), + db.WithVersion(&version), + ) + if err != nil { + return errors.Wrap(ctx, err, op) + } + if catalogsUpdated != 1 { + return errors.New(ctx, errors.MultipleRecords, op, fmt.Sprintf("expected 1 catalog to be updated, got %d", catalogsUpdated)) + } + msgs = append(msgs, &cOplogMsg) + recordUpdated = true + } + } + + if len(msgs) != 0 { + metadata := newCatalog.oplog(oplog.OpType_OP_TYPE_UPDATE) + if err := w.WriteOplogEntryWith(ctx, oplogWrapper, ticket, metadata, msgs); err != nil { + return errors.Wrap(ctx, err, op, errors.WithMsg("unable to write oplog")) + } + } + + return nil + }, + ) + + if err != nil { + if errors.IsUniqueError(err) { + return nil, nil, db.NoRowsAffected, errors.Wrap(ctx, err, op, errors.WithMsg(fmt.Sprintf("in %s: name %s already exists", newCatalog.PublicId, newCatalog.Name))) + } + return nil, nil, db.NoRowsAffected, errors.Wrap(ctx, err, op, errors.WithMsg(fmt.Sprintf("in %s", newCatalog.PublicId))) + } + + var numUpdated int + if recordUpdated { + numUpdated = 1 + } + + return returnedCatalog, plg, numUpdated, nil +} + // LookupCatalog returns the HostCatalog for id. Returns nil, nil if no // HostCatalog is found for id. func (r *Repository) LookupCatalog(ctx context.Context, id string, _ ...Option) (*HostCatalog, *hostplugin.Plugin, error) { @@ -322,9 +618,19 @@ func toPluginCatalog(ctx context.Context, in *HostCatalog) (*pb.HostCatalog, err if in == nil { return nil, errors.New(ctx, errors.InvalidParameter, op, "nil storage plugin") } + var name, description *wrapperspb.StringValue + if inName := in.GetName(); inName != "" { + name = wrapperspb.String(inName) + } + if inDescription := in.GetDescription(); inDescription != "" { + description = wrapperspb.String(inDescription) + } + hc := &pb.HostCatalog{ - Id: in.GetPublicId(), - ScopeId: in.GetScopeId(), + Id: in.GetPublicId(), + ScopeId: in.GetScopeId(), + Name: name, + Description: description, } if in.GetAttributes() != nil { attrs := &structpb.Struct{} diff --git a/internal/host/plugin/repository_host_catalog_test.go b/internal/host/plugin/repository_host_catalog_test.go index e8ac5db4d..e0333af1b 100644 --- a/internal/host/plugin/repository_host_catalog_test.go +++ b/internal/host/plugin/repository_host_catalog_test.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" hostplg "github.com/hashicorp/boundary/internal/plugin/host" + hostplugin "github.com/hashicorp/boundary/internal/plugin/host" "github.com/hashicorp/boundary/internal/scheduler" plgpb "github.com/hashicorp/boundary/sdk/pbs/plugin" "github.com/stretchr/testify/assert" @@ -22,6 +23,7 @@ import ( "google.golang.org/protobuf/proto" "google.golang.org/protobuf/testing/protocmp" "google.golang.org/protobuf/types/known/structpb" + "google.golang.org/protobuf/types/known/wrapperspb" ) func TestRepository_CreateCatalog(t *testing.T) { @@ -368,12 +370,719 @@ func TestRepository_CreateCatalog(t *testing.T) { }) } -func assertPluginBasedPublicId(t *testing.T, prefix, actual string) { - t.Helper() - assert.NotEmpty(t, actual) - parts := strings.Split(actual, "_") - assert.Equalf(t, 2, len(parts), "want 1 '_' in PublicId, got %d in %q", len(parts), actual) - assert.Equalf(t, prefix, parts[0], "PublicId want prefix: %q, got: %q in %q", prefix, parts[0], actual) +func TestRepository_UpdateCatalog(t *testing.T) { + ctx := context.Background() + dbConn, _ := db.TestSetup(t, "postgres") + dbRW := db.New(dbConn) + dbWrapper := db.TestWrapper(t) + sched := scheduler.TestScheduler(t, dbConn, dbWrapper) + dbKmsCache := kms.TestKms(t, dbConn, dbWrapper) + _, projectScope := iam.TestScopes(t, iam.TestRepo(t, dbConn, dbWrapper)) + _, projectScopeAlt := iam.TestScopes(t, iam.TestRepo(t, dbConn, dbWrapper)) + + // Define a plugin "manager", basically just a map with a mock + // plugin in it. This also includes functionality to capture the + // state, and set an error and the returned secrets to nil. Note + // that the way that this is set up means that the tests cannot run + // in parallel, but there could be other factors affecting that as + // well. + var setRespSecretsNil bool + var gotOnUpdateCatalogRequest *plgpb.OnUpdateCatalogRequest + var pluginError error + testPlugin := hostplg.TestPlugin(t, dbConn, "test") + testPluginMap := map[string]plgpb.HostPluginServiceClient{ + testPlugin.GetPublicId(): &WrappingPluginClient{ + Server: &TestPluginServer{ + OnUpdateCatalogFn: func(_ context.Context, req *plgpb.OnUpdateCatalogRequest) (*plgpb.OnUpdateCatalogResponse, error) { + gotOnUpdateCatalogRequest = req + respSecrets := req.GetNewCatalog().GetSecrets() + if setRespSecretsNil { + respSecrets = nil + setRespSecretsNil = false + } + + return &plgpb.OnUpdateCatalogResponse{Persisted: &plgpb.HostCatalogPersisted{Secrets: respSecrets}}, pluginError + }, + }, + }, + } + + // Set up two existing catalogs for duplicate tests, one in the + // same scope that we are working in mostly, and another in a + // different one. Both are project scopes. + const ( + testDuplicateCatalogNameProjectScope = "duplicate-catalog-name-project-scope" + testDuplicateCatalogNameAltScope = "duplicate-catalog-name-alt-scope" + ) + + // Project scope + existingProjectScopeCatalog := TestCatalog(t, dbConn, projectScope.PublicId, testPlugin.GetPublicId()) + existingProjectScopeCatalog.Name = testDuplicateCatalogNameProjectScope + numCatUpdated, err := dbRW.Update(ctx, existingProjectScopeCatalog, []string{"name"}, []string{}) + require.NoError(t, err) + require.Equal(t, 1, numCatUpdated) + + // Alternate scope + existingAltScopeCatalog := TestCatalog(t, dbConn, projectScopeAlt.PublicId, testPlugin.GetPublicId()) + existingAltScopeCatalog.Name = testDuplicateCatalogNameAltScope + numCatUpdated, err = dbRW.Update(ctx, existingAltScopeCatalog, []string{"name"}, []string{}) + require.NoError(t, err) + require.Equal(t, 1, numCatUpdated) + + // Define some helpers here to make the test table more readable. + type changeHostCatalogFunc func(c *HostCatalog) *HostCatalog + + changePublicId := func(s string) changeHostCatalogFunc { + return func(c *HostCatalog) *HostCatalog { + c.PublicId = s + return c + } + } + + changeScopeId := func(s string) changeHostCatalogFunc { + return func(c *HostCatalog) *HostCatalog { + c.ScopeId = s + return c + } + } + + changeName := func(s string) changeHostCatalogFunc { + return func(c *HostCatalog) *HostCatalog { + c.Name = s + return c + } + } + + changeDescription := func(s string) changeHostCatalogFunc { + return func(c *HostCatalog) *HostCatalog { + c.Description = s + return c + } + } + + changeAttributes := func(m map[string]interface{}) changeHostCatalogFunc { + return func(c *HostCatalog) *HostCatalog { + c.Attributes = mustMarshal(m) + return c + } + } + + changeSecrets := func(m map[string]interface{}) changeHostCatalogFunc { + return func(c *HostCatalog) *HostCatalog { + c.Secrets = mustStruct(m) + return c + } + } + + changeCatalogToNil := func() changeHostCatalogFunc { + return func(_ *HostCatalog) *HostCatalog { + return nil + } + } + + changeEmbeddedCatalogToNil := func() changeHostCatalogFunc { + return func(c *HostCatalog) *HostCatalog { + c.HostCatalog = nil + return c + } + } + + // Define some checks that will be used in the below tests. Some of + // these are re-used, so we define them here. Most of these are + // assertions and no particular one is non-fatal in that they will + // stop execution. Note that these are executed after wantIsErr, so + // if that is set in an individual table test, these will not be + // executed. + // + // Note that we define some state here, similar to how we + // previously defined gotOnUpdateCatalogRequest above next the + // plugin map. + type checkFunc func(t *testing.T, ctx context.Context) + var ( + gotCatalog *HostCatalog + gotNumUpdated int + ) + + checkName := func(want string) checkFunc { + return func(t *testing.T, ctx context.Context) { + t.Helper() + assert := assert.New(t) + assert.Equal(want, gotCatalog.Name) + } + } + + checkDescription := func(want string) checkFunc { + return func(t *testing.T, ctx context.Context) { + t.Helper() + assert := assert.New(t) + assert.Equal(want, gotCatalog.Description) + } + } + + checkVersion := func(want uint32) checkFunc { + return func(t *testing.T, ctx context.Context) { + t.Helper() + assert := assert.New(t) + assert.Equal(want, gotCatalog.Version) + } + } + + checkAttributes := func(want map[string]interface{}) checkFunc { + 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())) + } + } + + checkSecrets := func(want map[string]interface{}) checkFunc { + return func(t *testing.T, ctx context.Context) { + t.Helper() + assert := assert.New(t) + require := require.New(t) + + cSecret := allocHostCatalogSecret() + err := dbRW.LookupWhere(ctx, &cSecret, "catalog_id=?", gotCatalog.GetPublicId()) + require.NoError(err) + require.Empty(cSecret.Secret) + require.NotEmpty(cSecret.CtSecret) + + dbWrapper, err := dbKmsCache.GetWrapper(ctx, gotCatalog.GetScopeId(), kms.KeyPurposeDatabase) + require.NoError(err) + require.NoError(cSecret.decrypt(ctx, dbWrapper)) + + st := &structpb.Struct{} + require.NoError(proto.Unmarshal(cSecret.Secret, st)) + assert.Empty(cmp.Diff(mustStruct(want), st, protocmp.Transform())) + } + } + + checkSecretsDeleted := func() checkFunc { + return func(t *testing.T, ctx context.Context) { + t.Helper() + assert := assert.New(t) + + cSecret := allocHostCatalogSecret() + err := dbRW.LookupWhere(ctx, &cSecret, "catalog_id=?", gotCatalog.GetPublicId()) + assert.Error(err) + assert.True(errors.IsNotFoundError(err)) + } + } + + checkUpdateCatalogRequestCurrentNameNil := func() checkFunc { + return func(t *testing.T, ctx context.Context) { + t.Helper() + assert := assert.New(t) + assert.Nil(gotOnUpdateCatalogRequest.CurrentCatalog.Name) + } + } + + checkUpdateCatalogRequestNewName := func(want string) checkFunc { + return func(t *testing.T, ctx context.Context) { + t.Helper() + assert := assert.New(t) + assert.Equal(wrapperspb.String(want), gotOnUpdateCatalogRequest.NewCatalog.Name) + } + } + + checkUpdateCatalogRequestNewNameNil := func() checkFunc { + return func(t *testing.T, ctx context.Context) { + t.Helper() + assert := assert.New(t) + assert.Nil(gotOnUpdateCatalogRequest.NewCatalog.Name) + } + } + + checkUpdateCatalogRequestCurrentDescriptionNil := func() checkFunc { + return func(t *testing.T, ctx context.Context) { + t.Helper() + assert := assert.New(t) + assert.Nil(gotOnUpdateCatalogRequest.CurrentCatalog.Description) + } + } + + checkUpdateCatalogRequestNewDescription := func(want string) checkFunc { + return func(t *testing.T, ctx context.Context) { + t.Helper() + assert := assert.New(t) + assert.Equal(wrapperspb.String(want), gotOnUpdateCatalogRequest.NewCatalog.Description) + } + } + + checkUpdateCatalogRequestCurrentAttributes := func(want map[string]interface{}) checkFunc { + return func(t *testing.T, ctx context.Context) { + t.Helper() + assert := assert.New(t) + assert.Empty(cmp.Diff(mustStruct(want), gotOnUpdateCatalogRequest.CurrentCatalog.Attributes, protocmp.Transform())) + } + } + + checkUpdateCatalogRequestNewAttributes := func(want map[string]interface{}) checkFunc { + return func(t *testing.T, ctx context.Context) { + t.Helper() + assert := assert.New(t) + assert.Empty(cmp.Diff(mustStruct(want), gotOnUpdateCatalogRequest.NewCatalog.Attributes, protocmp.Transform())) + } + } + + checkUpdateCatalogRequestPersistedSecrets := func(want map[string]interface{}) checkFunc { + return func(t *testing.T, ctx context.Context) { + t.Helper() + assert := assert.New(t) + assert.Empty(cmp.Diff(mustStruct(want), gotOnUpdateCatalogRequest.Persisted.Secrets, protocmp.Transform())) + } + } + + checkUpdateCatalogRequestSecrets := func(want map[string]interface{}) checkFunc { + return func(t *testing.T, ctx context.Context) { + t.Helper() + assert := assert.New(t) + assert.Empty(cmp.Diff(mustStruct(want), gotOnUpdateCatalogRequest.NewCatalog.Secrets, protocmp.Transform())) + // Ensure that the current catalog's secrets value is always zero + assert.Zero(gotOnUpdateCatalogRequest.CurrentCatalog.Secrets) + } + } + + checkNumUpdated := func(want int) checkFunc { + return func(t *testing.T, ctx context.Context) { + t.Helper() + assert := assert.New(t) + assert.Equal(want, gotNumUpdated) + } + } + + checkVerifyCatalogOplog := func(op oplog.OpType) checkFunc { + return func(t *testing.T, ctx context.Context) { + t.Helper() + assert := assert.New(t) + assert.NoError( + db.TestVerifyOplog( + t, + dbRW, + gotCatalog.PublicId, + db.WithOperation(op), + db.WithCreateNotBefore(10*time.Second), + ), + ) + } + } + + tests := []struct { + name string + withEmptyPluginMap bool + withRespSecretsNil bool + withPluginError error + changeFuncs []changeHostCatalogFunc + version uint32 + fieldMask []string + wantCheckFuncs []checkFunc + wantIsErr errors.Code + }{ + { + name: "nil catalog", + changeFuncs: []changeHostCatalogFunc{changeCatalogToNil()}, + wantIsErr: errors.InvalidParameter, + }, + { + name: "nil embedded catalog", + changeFuncs: []changeHostCatalogFunc{changeEmbeddedCatalogToNil()}, + wantIsErr: errors.InvalidParameter, + }, + { + name: "missing public id", + changeFuncs: []changeHostCatalogFunc{changePublicId("")}, + wantIsErr: errors.InvalidParameter, + }, + { + name: "missing scope id", + changeFuncs: []changeHostCatalogFunc{changeScopeId("")}, + wantIsErr: errors.InvalidParameter, + }, + { + name: "empty field mask", + fieldMask: nil, // Should be testing on len + wantIsErr: errors.EmptyFieldMask, + }, + { + name: "bad catalog id", + changeFuncs: []changeHostCatalogFunc{changePublicId("badid")}, + fieldMask: []string{"name"}, + wantIsErr: errors.RecordNotFound, + }, + { + name: "version mismatch", + changeFuncs: []changeHostCatalogFunc{changeName("foo")}, + version: 1, + fieldMask: []string{"name"}, + wantIsErr: errors.VersionMismatch, + }, + { + name: "plugin lookup error", + withEmptyPluginMap: true, + version: 2, + fieldMask: []string{"name"}, + wantIsErr: errors.Internal, + }, + { + name: "plugin invocation error", + withPluginError: errors.New(context.Background(), errors.Internal, "TestRepository_UpdateCatalog/plugin_invocation_error", "test plugin error"), + version: 2, + fieldMask: []string{"name"}, + wantIsErr: errors.Internal, + }, + { + name: "update name (duplicate, same scope)", + changeFuncs: []changeHostCatalogFunc{changeName(testDuplicateCatalogNameProjectScope)}, + version: 2, + fieldMask: []string{"name"}, + wantIsErr: errors.NotUnique, + }, + { + name: "update name", + changeFuncs: []changeHostCatalogFunc{changeName("foo")}, + version: 2, + fieldMask: []string{"name"}, + wantCheckFuncs: []checkFunc{ + checkVersion(3), + checkUpdateCatalogRequestCurrentNameNil(), + checkUpdateCatalogRequestNewName("foo"), + checkName("foo"), + checkSecrets(map[string]interface{}{ + "one": "two", + }), + checkNumUpdated(1), + checkVerifyCatalogOplog(oplog.OpType_OP_TYPE_UPDATE), + }, + }, + { + name: "update name to same", + changeFuncs: []changeHostCatalogFunc{changeName("")}, + version: 2, + fieldMask: []string{"name"}, + wantCheckFuncs: []checkFunc{ + checkVersion(2), // Version remains same even though row is updated + checkUpdateCatalogRequestCurrentNameNil(), + checkUpdateCatalogRequestNewNameNil(), + checkName(""), + checkSecrets(map[string]interface{}{ + "one": "two", + }), + checkNumUpdated(1), + checkVerifyCatalogOplog(oplog.OpType_OP_TYPE_UPDATE), + }, + }, + { + name: "update name (duplicate, different scope)", + changeFuncs: []changeHostCatalogFunc{changeName(testDuplicateCatalogNameAltScope)}, + version: 2, + fieldMask: []string{"name"}, + wantCheckFuncs: []checkFunc{ + checkVersion(3), + checkUpdateCatalogRequestCurrentNameNil(), + checkUpdateCatalogRequestNewName(testDuplicateCatalogNameAltScope), + checkName(testDuplicateCatalogNameAltScope), + checkSecrets(map[string]interface{}{ + "one": "two", + }), + checkNumUpdated(1), + checkVerifyCatalogOplog(oplog.OpType_OP_TYPE_UPDATE), + }, + }, + { + name: "update description", + changeFuncs: []changeHostCatalogFunc{changeDescription("foo")}, + version: 2, + fieldMask: []string{"description"}, + wantCheckFuncs: []checkFunc{ + checkVersion(3), + checkUpdateCatalogRequestCurrentDescriptionNil(), + checkUpdateCatalogRequestNewDescription("foo"), + checkDescription("foo"), + checkSecrets(map[string]interface{}{ + "one": "two", + }), + checkNumUpdated(1), + checkVerifyCatalogOplog(oplog.OpType_OP_TYPE_UPDATE), + }, + }, + { + name: "update attributes (add)", + changeFuncs: []changeHostCatalogFunc{changeAttributes(map[string]interface{}{ + "baz": "qux", + })}, + version: 2, + fieldMask: []string{"attributes"}, + wantCheckFuncs: []checkFunc{ + checkVersion(3), + checkUpdateCatalogRequestCurrentAttributes(map[string]interface{}{ + "foo": "bar", + }), + checkUpdateCatalogRequestNewAttributes(map[string]interface{}{ + "foo": "bar", + "baz": "qux", + }), + checkAttributes(map[string]interface{}{ + "foo": "bar", + "baz": "qux", + }), + checkSecrets(map[string]interface{}{ + "one": "two", + }), + checkNumUpdated(1), + checkVerifyCatalogOplog(oplog.OpType_OP_TYPE_UPDATE), + }, + }, + { + name: "update attributes (overwrite)", + changeFuncs: []changeHostCatalogFunc{changeAttributes(map[string]interface{}{ + "foo": "baz", + })}, + version: 2, + fieldMask: []string{"attributes"}, + wantCheckFuncs: []checkFunc{ + checkVersion(3), + checkUpdateCatalogRequestCurrentAttributes(map[string]interface{}{ + "foo": "bar", + }), + checkUpdateCatalogRequestNewAttributes(map[string]interface{}{ + "foo": "baz", + }), + checkAttributes(map[string]interface{}{ + "foo": "baz", + }), + checkSecrets(map[string]interface{}{ + "one": "two", + }), + checkNumUpdated(1), + checkVerifyCatalogOplog(oplog.OpType_OP_TYPE_UPDATE), + }, + }, + { + name: "update attributes (null)", + changeFuncs: []changeHostCatalogFunc{changeAttributes(map[string]interface{}{ + "foo": nil, + })}, + version: 2, + fieldMask: []string{"attributes"}, + wantCheckFuncs: []checkFunc{ + checkVersion(3), + checkUpdateCatalogRequestCurrentAttributes(map[string]interface{}{ + "foo": "bar", + }), + checkUpdateCatalogRequestNewAttributes(map[string]interface{}{}), + checkAttributes(map[string]interface{}{}), + checkSecrets(map[string]interface{}{ + "one": "two", + }), + checkNumUpdated(1), + checkVerifyCatalogOplog(oplog.OpType_OP_TYPE_UPDATE), + }, + }, + { + name: "update attributes (combined)", + changeFuncs: []changeHostCatalogFunc{changeAttributes(map[string]interface{}{ + "a": "b", + "foo": "baz", + })}, + version: 2, + fieldMask: []string{"attributes.a", "attributes.foo"}, + wantCheckFuncs: []checkFunc{ + checkVersion(3), + checkUpdateCatalogRequestCurrentAttributes(map[string]interface{}{ + "foo": "bar", + }), + checkUpdateCatalogRequestNewAttributes(map[string]interface{}{ + "a": "b", + "foo": "baz", + }), + checkAttributes(map[string]interface{}{ + "a": "b", + "foo": "baz", + }), + checkSecrets(map[string]interface{}{ + "one": "two", + }), + checkNumUpdated(1), + checkVerifyCatalogOplog(oplog.OpType_OP_TYPE_UPDATE), + }, + }, + { + name: "update secrets", + changeFuncs: []changeHostCatalogFunc{changeSecrets(map[string]interface{}{ + "three": "four", + })}, + version: 2, + fieldMask: []string{"secrets.three"}, + wantCheckFuncs: []checkFunc{ + checkVersion(3), + checkUpdateCatalogRequestPersistedSecrets(map[string]interface{}{ + "one": "two", + }), + checkUpdateCatalogRequestSecrets(map[string]interface{}{ + "three": "four", + }), + checkSecrets(map[string]interface{}{ + "three": "four", + }), + checkNumUpdated(1), + checkVerifyCatalogOplog(oplog.OpType_OP_TYPE_UPDATE), + }, + }, + { + name: "update secrets, return nil secrets from plugin", + withRespSecretsNil: true, + changeFuncs: []changeHostCatalogFunc{changeSecrets(map[string]interface{}{ + "three": "four", + })}, + version: 2, + fieldMask: []string{"secrets.three"}, + wantCheckFuncs: []checkFunc{ + checkVersion(2), // no-op + checkUpdateCatalogRequestPersistedSecrets(map[string]interface{}{ + "one": "two", + }), + checkUpdateCatalogRequestSecrets(map[string]interface{}{ + "three": "four", + }), + checkSecrets(map[string]interface{}{ + "one": "two", + }), + checkNumUpdated(db.NoRowsAffected), + }, + }, + { + name: "delete secrets", + changeFuncs: []changeHostCatalogFunc{changeSecrets(map[string]interface{}{})}, + version: 2, + fieldMask: []string{"secrets"}, + wantCheckFuncs: []checkFunc{ + checkVersion(3), + checkUpdateCatalogRequestPersistedSecrets(map[string]interface{}{ + "one": "two", + }), + checkUpdateCatalogRequestSecrets(map[string]interface{}{}), + checkSecretsDeleted(), + checkNumUpdated(1), + checkVerifyCatalogOplog(oplog.OpType_OP_TYPE_UPDATE), + }, + }, + { + name: "update name and secrets", + changeFuncs: []changeHostCatalogFunc{ + changeName("foo"), + changeSecrets(map[string]interface{}{ + "three": "four", + }), + }, + version: 2, + fieldMask: []string{"name", "secrets.three"}, + wantCheckFuncs: []checkFunc{ + checkVersion(3), + checkUpdateCatalogRequestCurrentNameNil(), + checkUpdateCatalogRequestNewName("foo"), + checkUpdateCatalogRequestSecrets(map[string]interface{}{ + "three": "four", + }), + checkName("foo"), + checkSecrets(map[string]interface{}{ + "three": "four", + }), + checkNumUpdated(1), + checkVerifyCatalogOplog(oplog.OpType_OP_TYPE_UPDATE), + }, + }, + } + + // Finally define a function for bringing the test subject catalog. + // This function also returns a function to clean up the catalog + // afterwards. + setupHostCatalog := func(t *testing.T, ctx context.Context) *HostCatalog { + t.Helper() + require := require.New(t) + + cat := TestCatalog(t, dbConn, projectScope.PublicId, testPlugin.GetPublicId()) + // Set some (default) attributes on our test catalog + cat.Attributes = mustMarshal(map[string]interface{}{ + "foo": "bar", + }) + + numCatUpdated, err := dbRW.Update(ctx, cat, []string{"attributes"}, []string{}) + require.NoError(err) + require.Equal(1, numCatUpdated) + + // Set up some secrets + cSecretProto := mustStruct(map[string]interface{}{ + "one": "two", + }) + cSecret, err := newHostCatalogSecret(ctx, cat.GetPublicId(), cSecretProto) + require.NoError(err) + scopeWrapper, err := dbKmsCache.GetWrapper(ctx, cat.GetScopeId(), kms.KeyPurposeDatabase) + require.NoError(err) + require.NoError(cSecret.encrypt(ctx, scopeWrapper)) + cSecretQ, cSecretV := cSecret.upsertQuery() + secretsUpdated, err := dbRW.Exec(ctx, cSecretQ, cSecretV) + require.NoError(err) + require.Equal(1, secretsUpdated) + + t.Cleanup(func() { + t.Helper() + assert := assert.New(t) + n, err := dbRW.Delete(ctx, cat) + assert.NoError(err) + assert.Equal(1, n) + }) + + return cat + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + origCat := setupHostCatalog(t, ctx) + + pluginMap := testPluginMap + if tt.withEmptyPluginMap { + pluginMap = make(map[string]plgpb.HostPluginServiceClient) + } + pluginError = tt.withPluginError + t.Cleanup(func() { pluginError = nil }) + repo, err := NewRepository(dbRW, dbRW, dbKmsCache, sched, pluginMap) + require.NoError(err) + require.NotNil(repo) + + workingCat := origCat.clone() + for _, cf := range tt.changeFuncs { + workingCat = cf(workingCat) + } + + setRespSecretsNil = tt.withRespSecretsNil + var gotPlugin *hostplugin.Plugin + gotCatalog, gotPlugin, gotNumUpdated, err = repo.UpdateCatalog(ctx, workingCat, tt.version, tt.fieldMask) + if tt.wantIsErr != 0 { + require.Equal(db.NoRowsAffected, gotNumUpdated) + require.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) + return + } + require.NoError(err) + t.Cleanup(func() { gotOnUpdateCatalogRequest = nil }) + + // Quick assertion that the catalog is not nil and that the plugin ID in + // the catalog and the plugin ID in the returned plugin match. Use assert + // as that's what our checks use in the table test's defined checks. + assert.NotNil(gotCatalog) + assert.NotNil(gotPlugin) + assert.Equal(gotCatalog.PluginId, gotPlugin.PublicId) + // Perform checks + for _, check := range tt.wantCheckFuncs { + check(t, ctx) + } + }) + } } func TestRepository_LookupCatalog(t *testing.T) { @@ -628,3 +1337,33 @@ func TestRepository_DeleteCatalogX(t *testing.T) { }) } } + +func assertPluginBasedPublicId(t *testing.T, prefix, actual string) { + t.Helper() + assert.NotEmpty(t, actual) + parts := strings.Split(actual, "_") + assert.Equalf(t, 2, len(parts), "want 1 '_' in PublicId, got %d in %q", len(parts), actual) + assert.Equalf(t, prefix, parts[0], "PublicId want prefix: %q, got: %q in %q", prefix, parts[0], actual) +} + +// mustStruct creates a structpb.Struct, and panics if there is an +// error. +func mustStruct(in map[string]interface{}) *structpb.Struct { + out, err := structpb.NewStruct(in) + if err != nil { + panic(err) + } + + 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 +}