diff --git a/internal/host/static/host_set.go b/internal/host/static/host_set.go index 67e43cfe0..9ced6364e 100644 --- a/internal/host/static/host_set.go +++ b/internal/host/static/host_set.go @@ -72,3 +72,12 @@ func (s *HostSet) oplog(op oplog.OpType) oplog.Metadata { } return metadata } + +func newHostSetForMembers(setId string, version uint32) *HostSet { + return &HostSet{ + HostSet: &store.HostSet{ + PublicId: setId, + Version: version + 1, + }, + } +} diff --git a/internal/host/static/host_set_member_test.go b/internal/host/static/host_set_member_test.go index 1d5b71037..2fd203b9c 100644 --- a/internal/host/static/host_set_member_test.go +++ b/internal/host/static/host_set_member_test.go @@ -11,23 +11,19 @@ import ( "github.com/stretchr/testify/require" ) -func TestHostSetMember_New(t *testing.T) { +func TestHostSetMember_Insert(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") - conn.LogMode(false) wrapper := db.TestWrapper(t) _, prj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) cats := TestCatalogs(t, conn, prj.PublicId, 2) blueCat := cats[0] - blueSets := TestSets(t, conn, blueCat.GetPublicId(), 2) - blueHosts := TestHosts(t, conn, blueCat.GetPublicId(), 2) + blueSets := TestSets(t, conn, blueCat.GetPublicId(), 1) + blueHosts := TestHosts(t, conn, blueCat.GetPublicId(), 1) - // TODO(mgaffney) 05/2020: - // these will be needed when the repository code is done - // greenCat := cats[1] - // greenSets := TestSets(t, conn, greenCat.GetPublicId(), 2) - // greenHosts := TestHosts(t, conn, greenCat.GetPublicId(), 2) + greenCat := cats[1] + greenSets := TestSets(t, conn, greenCat.GetPublicId(), 1) var tests = []struct { name string @@ -40,29 +36,27 @@ func TestHostSetMember_New(t *testing.T) { set: blueSets[0], host: blueHosts[0], }, - // { - // name: "invalid-diff-catalogs", - // set: greenSets[0], - // host: blueHosts[0], - // wantErr: true, - // }, + { + name: "invalid-diff-catalogs", + set: greenSets[0], + host: blueHosts[0], + wantErr: true, + }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - assert := assert.New(t) + assert, require := assert.New(t), require.New(t) got, err := NewHostSetMember(tt.set.PublicId, tt.host.PublicId) + require.NoError(err) + require.NotNil(got) + w := db.New(conn) + err2 := w.Create(context.Background(), got) if tt.wantErr { - assert.Error(err) - assert.Nil(got) - } else { - assert.NoError(err) - if assert.NotNil(got) { - w := db.New(conn) - err2 := w.Create(context.Background(), got) - assert.NoError(err2) - } + assert.Error(err2) + return } + assert.NoError(err2) }) } } diff --git a/internal/host/static/query.go b/internal/host/static/query.go new file mode 100644 index 000000000..0aca0a882 --- /dev/null +++ b/internal/host/static/query.go @@ -0,0 +1,46 @@ +package static + +const ( + setChangesQuery = ` +with +final_hosts (host_id) as ( + -- returns the SET list + select public_id + from static_host + where public_id in (%s) +), +current_hosts (host_id) as ( + -- returns the current list + select host_id + from static_host_set_member + where set_id = $1 +), +keep_hosts (host_id) as ( + -- returns the KEEP list + select host_id + from current_hosts + where host_id in (select * from final_hosts) +), +delete_hosts (host_id) as ( + -- returns the DELETE list + select host_id + from current_hosts + where host_id not in (select * from final_hosts) +), +insert_hosts (host_id) as ( + -- returns the ADD list + select host_id + from final_hosts + where host_id not in (select * from keep_hosts) +), +final (action, host_id) as ( + select 'delete', host_id + from delete_hosts + union + select 'add', host_id + from insert_hosts +) +select * from final +order by action, host_id; +` +) diff --git a/internal/host/static/repository_host_set.go b/internal/host/static/repository_host_set.go index f89ba0803..ee26dcbee 100644 --- a/internal/host/static/repository_host_set.go +++ b/internal/host/static/repository_host_set.go @@ -68,8 +68,8 @@ func (r *Repository) CreateSet(ctx context.Context, scopeId string, s *HostSet, // UpdateSet updates the repository entry for s.PublicId with the values in // s for the fields listed in fieldMaskPaths. It returns a new HostSet -// containing the updated values and a count of the number of records -// updated. s is not changed. +// containing the updated values, the hosts assigned to the host set, and a +// count of the number of records updated. s is not changed. // // s must contain a valid PublicId. Only s.Name and s.Description can be // updated. If s.Name is set to a non-empty string, it must be unique @@ -77,21 +77,24 @@ func (r *Repository) CreateSet(ctx context.Context, scopeId string, s *HostSet, // // An attribute of s will be set to NULL in the database if the attribute // in s is the zero value and it is included in fieldMaskPaths. -func (r *Repository) UpdateSet(ctx context.Context, scopeId string, s *HostSet, version uint32, fieldMaskPaths []string, opt ...Option) (*HostSet, int, error) { +// +// The WithLimit option can be used to limit the number of hosts returned. +// All other options are ignored. +func (r *Repository) UpdateSet(ctx context.Context, scopeId string, s *HostSet, version uint32, fieldMaskPaths []string, opt ...Option) (*HostSet, []*Host, int, error) { if s == nil { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host set: %w", db.ErrInvalidParameter) + return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: %w", db.ErrInvalidParameter) } if s.HostSet == nil { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host set: embedded HostSet: %w", db.ErrInvalidParameter) + return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: embedded HostSet: %w", db.ErrInvalidParameter) } if s.PublicId == "" { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host set: missing public id: %w", db.ErrInvalidParameter) + return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: missing public id: %w", db.ErrInvalidParameter) } if version == 0 { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host set: no version supplied: %w", db.ErrInvalidParameter) + return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: no version supplied: %w", db.ErrInvalidParameter) } if scopeId == "" { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host set: no scopeId: %w", db.ErrInvalidParameter) + return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: no scopeId: %w", db.ErrInvalidParameter) } for _, f := range fieldMaskPaths { @@ -99,7 +102,7 @@ func (r *Repository) UpdateSet(ctx context.Context, scopeId string, s *HostSet, case strings.EqualFold("Name", f): case strings.EqualFold("Description", f): default: - return nil, db.NoRowsAffected, fmt.Errorf("update: static host set: field: %s: %w", f, db.ErrInvalidFieldMask) + return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: field: %s: %w", f, db.ErrInvalidFieldMask) } } var dbMask, nullFields []string @@ -111,18 +114,26 @@ func (r *Repository) UpdateSet(ctx context.Context, scopeId string, s *HostSet, fieldMaskPaths, ) if len(dbMask) == 0 && len(nullFields) == 0 { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host set: %w", db.ErrEmptyFieldMask) + return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: %w", db.ErrEmptyFieldMask) + } + + opts := getOpts(opt...) + limit := r.defaultLimit + if opts.withLimit != 0 { + // non-zero signals an override of the default limit for the repo. + limit = opts.withLimit } oplogWrapper, err := r.kms.GetWrapper(ctx, scopeId, kms.KeyPurposeOplog) if err != nil { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host set: unable to get oplog wrapper: %w", err) + return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: unable to get oplog wrapper: %w", err) } var rowsUpdated int var returnedHostSet *HostSet + var hosts []*Host _, err = r.writer.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, - func(_ db.Reader, w db.Writer) error { + func(reader db.Reader, w db.Writer) error { returnedHostSet = s.clone() var err error rowsUpdated, err = w.Update(ctx, returnedHostSet, dbMask, nullFields, @@ -131,36 +142,62 @@ func (r *Repository) UpdateSet(ctx context.Context, scopeId string, s *HostSet, if err == nil && rowsUpdated > 1 { return db.ErrMultipleRecords } + if err != nil { + return err + } + hosts, err = getHosts(ctx, reader, s.PublicId, limit) return err }, ) if err != nil { if db.IsUniqueError(err) { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host set: %s: name %s already exists: %w", + return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: %s: name %s already exists: %w", s.PublicId, s.Name, db.ErrNotUnique) } - return nil, db.NoRowsAffected, fmt.Errorf("update: static host set: %s: %w", s.PublicId, err) + return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: %s: %w", s.PublicId, err) } - return returnedHostSet, rowsUpdated, nil + return returnedHostSet, hosts, rowsUpdated, nil } -// LookupSet will look up a host set in the repository. If the host set is -// not found, it will return nil, nil. All options are ignored. -func (r *Repository) LookupSet(ctx context.Context, publicId string, opt ...Option) (*HostSet, error) { +// LookupSet will look up a host set in the repository and return the host +// set and the hosts assigned to the host set. If the host set is not +// found, it will return nil, nil, nil. The WithLimit option can be used to +// limit the number of hosts returned. All other options are ignored. +func (r *Repository) LookupSet(ctx context.Context, publicId string, opt ...Option) (*HostSet, []*Host, error) { if publicId == "" { - return nil, fmt.Errorf("lookup: static host set: missing public id %w", db.ErrInvalidParameter) + return nil, nil, fmt.Errorf("lookup: static host set: missing public id %w", db.ErrInvalidParameter) } + opts := getOpts(opt...) + limit := r.defaultLimit + if opts.withLimit != 0 { + // non-zero signals an override of the default limit for the repo. + limit = opts.withLimit + } + s := allocHostSet() s.PublicId = publicId - if err := r.reader.LookupByPublicId(ctx, s); err != nil { - if errors.Is(err, db.ErrRecordNotFound) { - return nil, nil + + var hosts []*Host + _, err := r.writer.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, func(reader db.Reader, _ db.Writer) error { + if err := reader.LookupByPublicId(ctx, s); err != nil { + if errors.Is(err, db.ErrRecordNotFound) { + s = nil + return nil + } + return err } - return nil, fmt.Errorf("lookup: static host set: failed %w for %s", err, publicId) + var err error + hosts, err = getHosts(ctx, reader, s.PublicId, limit) + return err + }) + + if err != nil { + return nil, nil, fmt.Errorf("lookup: static host set: failed %w for %s", err, publicId) } - return s, nil + + return s, hosts, nil } // ListSets returns a slice of HostSets for the catalogId. WithLimit is the @@ -220,26 +257,3 @@ func (r *Repository) DeleteSet(ctx context.Context, scopeId string, publicId str return rowsDeleted, nil } - -// AddSetMembers adds hostIds to setId in the repository. It returns a -// slice of all host set members in setId. A host must belong to the same -// catalog as the set to be added. The version must match the current -// version of the setId in the repository. -func (r *Repository) AddSetMembers(ctx context.Context, scopeId string, setId string, version uint32, hostIds []string, opt ...Option) ([]*HostSetMember, error) { - panic("not implemented") -} - -// DeleteSetMembers deletes hostIds from setId in the repository. It -// returns the number of hosts deleted from the set. The version must match -// the current version of the setId in the repository. -func (r *Repository) DeleteSetMembers(ctx context.Context, scopeId string, setId string, version uint32, hostIds []string, opt ...Option) (int, error) { - panic("not implemented") -} - -// SetSetMembers replaces the hosts in setId with hostIds in the -// repository. It returns a slice of all host set members in setId. A host -// must belong to the same catalog as the set to be added. The version must -// match the current version of the setId in the repository. -func (r *Repository) SetSetMembers(ctx context.Context, scopeId string, setId string, version uint32, hostIds []string, opt ...Option) ([]*HostSetMember, error) { - panic("not implemented") -} diff --git a/internal/host/static/repository_host_set_member.go b/internal/host/static/repository_host_set_member.go new file mode 100644 index 000000000..7ae3c86cb --- /dev/null +++ b/internal/host/static/repository_host_set_member.go @@ -0,0 +1,349 @@ +package static + +import ( + "context" + "fmt" + "strings" + + wrapping "github.com/hashicorp/go-kms-wrapping" + + "github.com/hashicorp/boundary/internal/db" + "github.com/hashicorp/boundary/internal/kms" + "github.com/hashicorp/boundary/internal/oplog" +) + +// AddSetMembers adds hostIds to setId in the repository. It returns a +// slice of all hosts in setId. A host must belong to the same catalog as +// the set to be added. The version must match the current version of the +// setId in the repository. +func (r *Repository) AddSetMembers(ctx context.Context, scopeId string, setId string, version uint32, hostIds []string, opt ...Option) ([]*Host, error) { + if scopeId == "" { + return nil, fmt.Errorf("add: static host set members: missing scope id: %w", db.ErrInvalidParameter) + } + if setId == "" { + return nil, fmt.Errorf("add: static host set members: missing set id: %w", db.ErrInvalidParameter) + } + if version == 0 { + return nil, fmt.Errorf("add: static host set members: version is zero: %w", db.ErrInvalidParameter) + } + if len(hostIds) == 0 { + return nil, fmt.Errorf("add: static host set members: empty hostIds: %w", db.ErrInvalidParameter) + } + + // Create in-memory host set members + members, err := r.newMembers(setId, hostIds) + if err != nil { + return nil, fmt.Errorf("add: static host set members: %w", err) + } + + wrapper, err := r.kms.GetWrapper(ctx, scopeId, kms.KeyPurposeOplog) + if err != nil { + return nil, fmt.Errorf("add: static host set members: unable to get oplog wrapper: %w", err) + } + + var hosts []*Host + _, err = r.writer.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, func(reader db.Reader, w db.Writer) error { + set := newHostSetForMembers(setId, version) + metadata := set.oplog(oplog.OpType_OP_TYPE_CREATE) + + // Create host set members + msgs, err := createMembers(ctx, w, members) + if err != nil { + return err + } + // Update host set version + if err := updateVersion(ctx, w, wrapper, metadata, msgs, set, version); err != nil { + return err + } + + hosts, err = getHosts(ctx, reader, setId, unlimited) + return err + }) + if err != nil { + return nil, fmt.Errorf("add: static host set members: %w", err) + } + + return hosts, nil +} + +func (r *Repository) newMembers(setId string, hostIds []string) ([]interface{}, error) { + var members []interface{} + for _, id := range hostIds { + var m *HostSetMember + m, err := NewHostSetMember(setId, id) + if err != nil { + return nil, fmt.Errorf("new members: %w", err) + } + members = append(members, m) + } + return members, nil +} + +func createMembers(ctx context.Context, w db.Writer, members []interface{}) ([]*oplog.Message, error) { + var msgs []*oplog.Message + if err := w.CreateItems(ctx, members, db.NewOplogMsgs(&msgs)); err != nil { + return nil, fmt.Errorf("unable to create host set members: %w", err) + } + return msgs, nil +} + +func updateVersion(ctx context.Context, w db.Writer, wrapper wrapping.Wrapper, metadata oplog.Metadata, msgs []*oplog.Message, set *HostSet, version uint32) error { + setMsg := new(oplog.Message) + rowsUpdated, err := w.Update(ctx, set, []string{"Version"}, nil, db.NewOplogMsg(setMsg), db.WithVersion(&version)) + switch { + case err != nil: + return fmt.Errorf("unable to update host set version: %w", err) + case rowsUpdated > 1: + return fmt.Errorf("unable to update host set version: %w", db.ErrMultipleRecords) + } + msgs = append(msgs, setMsg) + + // Write oplog + ticket, err := w.GetTicket(set) + if err != nil { + return fmt.Errorf("unable to get ticket: %w", err) + } + if err := w.WriteOplogEntryWith(ctx, wrapper, ticket, metadata, msgs); err != nil { + return fmt.Errorf("unable to write oplog: %w", err) + } + return nil +} + +const unlimited = -1 + +func getHosts(ctx context.Context, reader db.Reader, setId string, limit int) ([]*Host, error) { + const whereNoLimit = `public_id in + ( select host_id + from static_host_set_member + where set_id = $1 + )` + + const whereLimit = `public_id in + ( select host_id + from static_host_set_member + where set_id = $1 + limit $2 + )` + + params := []interface{}{setId} + var where string + switch limit { + case unlimited: + where = whereNoLimit + default: + where = whereLimit + params = append(params, limit) + } + + var hosts []*Host + if err := reader.SearchWhere(ctx, &hosts, + where, + params, + db.WithLimit(limit), + ); err != nil { + return nil, fmt.Errorf("get hosts: %w", err) + } + if len(hosts) == 0 { + return nil, nil + } + return hosts, nil +} + +// DeleteSetMembers deletes hostIds from setId in the repository. It +// returns the number of hosts deleted from the set. The version must match +// the current version of the setId in the repository. +func (r *Repository) DeleteSetMembers(ctx context.Context, scopeId string, setId string, version uint32, hostIds []string, opt ...Option) (int, error) { + if scopeId == "" { + return db.NoRowsAffected, fmt.Errorf("delete: static host set members: missing scope id: %w", db.ErrInvalidParameter) + } + if setId == "" { + return db.NoRowsAffected, fmt.Errorf("delete: static host set members: missing set id: %w", db.ErrInvalidParameter) + } + if version == 0 { + return db.NoRowsAffected, fmt.Errorf("delete: static host set members: version is zero: %w", db.ErrInvalidParameter) + } + if len(hostIds) == 0 { + return db.NoRowsAffected, fmt.Errorf("delete: static host set members: empty hostIds: %w", db.ErrInvalidParameter) + } + + // Create in-memory host set members + members, err := r.newMembers(setId, hostIds) + if err != nil { + return db.NoRowsAffected, fmt.Errorf("delete: static host set members: %w", err) + } + + wrapper, err := r.kms.GetWrapper(ctx, scopeId, kms.KeyPurposeOplog) + if err != nil { + return db.NoRowsAffected, fmt.Errorf("delete: static host set members: unable to get oplog wrapper: %w", err) + } + + _, err = r.writer.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, func(_ db.Reader, w db.Writer) error { + set := newHostSetForMembers(setId, version) + metadata := set.oplog(oplog.OpType_OP_TYPE_DELETE) + + // Delete host set members + msgs, err := deleteMembers(ctx, w, members) + if err != nil { + return err + } + + // Update host set version + return updateVersion(ctx, w, wrapper, metadata, msgs, set, version) + }) + + if err != nil { + return db.NoRowsAffected, fmt.Errorf("delete: static host set members: %w", err) + } + return len(hostIds), nil +} + +func deleteMembers(ctx context.Context, w db.Writer, members []interface{}) ([]*oplog.Message, error) { + var msgs []*oplog.Message + rowsDeleted, err := w.DeleteItems(ctx, members, db.NewOplogMsgs(&msgs)) + if err != nil { + return nil, fmt.Errorf("unable to delete host set members: %w", err) + } + if rowsDeleted != len(members) { + return nil, fmt.Errorf("set members deleted %d did not match request for %d", rowsDeleted, len(members)) + } + return msgs, nil +} + +// SetSetMembers replaces the hosts in setId with hostIds in the +// repository. It returns a slice of all hosts in setId and a count of +// hosts added or deleted. A host must belong to the same catalog as the +// set to be added. The version must match the current version of the setId +// in the repository. If hostIds is empty, all hosts will be removed setId. +func (r *Repository) SetSetMembers(ctx context.Context, scopeId string, setId string, version uint32, hostIds []string, opt ...Option) ([]*Host, int, error) { + if scopeId == "" { + return nil, db.NoRowsAffected, fmt.Errorf("set: static host set members: missing scope id: %w", db.ErrInvalidParameter) + } + if setId == "" { + return nil, db.NoRowsAffected, fmt.Errorf("set: static host set members: missing set id: %w", db.ErrInvalidParameter) + } + if version == 0 { + return nil, db.NoRowsAffected, fmt.Errorf("set: static host set members: version is zero: %w", db.ErrInvalidParameter) + } + + // TODO(mgaffney) 08/2020: Oplog does not currently support bulk + // operations. Push these operations to the database once bulk + // operations are added. + + // NOTE(mgaffney) 08/2020: This establishes a new pattern for + // calculating change sets for "SetMembers" methods. The changes are + // calculated by the database using a single query. Existing + // "SetMembers" methods retrieve all of the members of the set and + // calculate the changes outside of the database. Our default moving + // forward is to use SQL for calculations on the data in the database. + + // TODO(mgaffney) 08/2020: Change existing "SetMembers" methods to use + // this pattern. + changes, err := r.changes(ctx, setId, hostIds) + if err != nil { + return nil, db.NoRowsAffected, fmt.Errorf("set: static host set members: %w", err) + } + var deletions, additions []interface{} + for _, c := range changes { + m, err := NewHostSetMember(setId, c.HostId) + if err != nil { + return nil, db.NoRowsAffected, fmt.Errorf("set: static host set members: %w", err) + } + switch c.Action { + case "delete": + deletions = append(deletions, m) + case "add": + additions = append(additions, m) + } + } + + var hosts []*Host + if len(changes) > 0 { + wrapper, err := r.kms.GetWrapper(ctx, scopeId, kms.KeyPurposeOplog) + if err != nil { + return nil, db.NoRowsAffected, fmt.Errorf("set: static host set members: unable to get oplog wrapper: %w", err) + } + + _, err = r.writer.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, func(reader db.Reader, w db.Writer) error { + set := newHostSetForMembers(setId, version) + metadata := set.oplog(oplog.OpType_OP_TYPE_UPDATE) + var msgs []*oplog.Message + + // Delete host set members + if len(deletions) > 0 { + deletedMsgs, err := deleteMembers(ctx, w, deletions) + if err != nil { + return err + } + msgs = append(msgs, deletedMsgs...) + metadata["op-type"] = append(metadata["op-type"], oplog.OpType_OP_TYPE_DELETE.String()) + } + + // Add host set members + if len(additions) > 0 { + createdMsgs, err := createMembers(ctx, w, additions) + if err != nil { + return err + } + msgs = append(msgs, createdMsgs...) + metadata["op-type"] = append(metadata["op-type"], oplog.OpType_OP_TYPE_CREATE.String()) + } + + // Update host set version + if err := updateVersion(ctx, w, wrapper, metadata, msgs, set, version); err != nil { + return err + } + + hosts, err = getHosts(ctx, reader, setId, unlimited) + return err + }) + + if err != nil { + return nil, db.NoRowsAffected, fmt.Errorf("set: static host set members: %w", err) + } + } + return hosts, len(changes), nil +} + +type change struct { + Action string + HostId string +} + +func (r *Repository) changes(ctx context.Context, setId string, hostIds []string) ([]*change, error) { + var inClauseSpots []string + // starts at 2 because there is already a $1 in the query + for i := 2; i < len(hostIds)+2; i++ { + inClauseSpots = append(inClauseSpots, fmt.Sprintf("$%d", i)) + } + inClause := strings.Join(inClauseSpots, ",") + if inClause == "" { + inClause = "''" + } + query := fmt.Sprintf(setChangesQuery, inClause) + + tx, err := r.reader.DB() + if err != nil { + return nil, fmt.Errorf("changes: unable to get DB: %w", err) + } + + var params []interface{} + params = append(params, setId) + for _, v := range hostIds { + params = append(params, v) + } + rows, err := tx.QueryContext(ctx, query, params...) + if err != nil { + return nil, fmt.Errorf("changes: query failed: %w", err) + } + defer rows.Close() + + var changes []*change + for rows.Next() { + var chg change + if err := r.reader.ScanRows(rows, &chg); err != nil { + return nil, fmt.Errorf("changes: scan row failed: %w", err) + } + changes = append(changes, &chg) + } + return changes, nil +} diff --git a/internal/host/static/repository_host_set_member_test.go b/internal/host/static/repository_host_set_member_test.go new file mode 100644 index 000000000..1877abbea --- /dev/null +++ b/internal/host/static/repository_host_set_member_test.go @@ -0,0 +1,715 @@ +package static + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/hashicorp/boundary/internal/db" + "github.com/hashicorp/boundary/internal/iam" + "github.com/hashicorp/boundary/internal/kms" + "github.com/hashicorp/boundary/internal/oplog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/testing/protocmp" +) + +func TestRepository_AddSetMembers_Parameters(t *testing.T) { + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + kms := kms.TestKms(t, conn, wrapper) + iamRepo := iam.TestRepo(t, conn, wrapper) + + _, prj := iam.TestScopes(t, iamRepo) + c := TestCatalogs(t, conn, prj.PublicId, 1)[0] + set := TestSets(t, conn, c.PublicId, 1)[0] + hosts := TestHosts(t, conn, c.PublicId, 5) + var hostIds []string + for _, h := range hosts { + hostIds = append(hostIds, h.PublicId) + } + + badVersion := uint32(12345) + + type args struct { + scopeId string + setId string + version uint32 + hostIds []string + opt []Option + } + + tests := []struct { + name string + args args + want []*Host + wantErr bool + wantIsErr error + }{ + { + name: "empty-scope-id", + args: args{ + setId: set.PublicId, + version: set.Version, + hostIds: hostIds, + }, + wantIsErr: db.ErrInvalidParameter, + }, + { + name: "empty-set-id", + args: args{ + scopeId: prj.PublicId, + version: set.Version, + hostIds: hostIds, + }, + wantIsErr: db.ErrInvalidParameter, + }, + { + name: "zero-version", + args: args{ + scopeId: prj.PublicId, + setId: set.PublicId, + hostIds: hostIds, + }, + wantIsErr: db.ErrInvalidParameter, + }, + { + name: "empty-host-ids", + args: args{ + scopeId: prj.PublicId, + setId: set.PublicId, + version: set.Version, + }, + wantIsErr: db.ErrInvalidParameter, + }, + { + name: "invalid-version", + args: args{ + scopeId: prj.PublicId, + setId: set.PublicId, + version: badVersion, + hostIds: hostIds, + }, + wantErr: true, + }, + { + name: "valid", + args: args{ + scopeId: prj.PublicId, + setId: set.PublicId, + version: set.Version, + hostIds: hostIds, + }, + want: hosts, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + repo, err := NewRepository(rw, rw, kms) + require.NoError(err) + require.NotNil(repo) + got, err := repo.AddSetMembers(context.Background(), tt.args.scopeId, tt.args.setId, tt.args.version, tt.args.hostIds, tt.args.opt...) + if tt.wantIsErr != nil { + assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + assert.Nil(got) + assert.Error(db.TestVerifyOplog(t, rw, tt.args.setId, db.WithOperation(oplog.OpType_OP_TYPE_CREATE), db.WithCreateNotBefore(10*time.Second))) + return + } + if tt.wantErr { + assert.Error(err) + assert.Nil(got) + assert.Error(db.TestVerifyOplog(t, rw, tt.args.setId, db.WithOperation(oplog.OpType_OP_TYPE_CREATE), db.WithCreateNotBefore(10*time.Second))) + return + } + require.NoError(err) + opts := []cmp.Option{ + cmpopts.SortSlices(func(x, y *Host) bool { return x.PublicId < y.PublicId }), + protocmp.Transform(), + } + assert.Len(got, len(tt.want)) + assert.Empty(cmp.Diff(tt.want, got, opts...)) + assert.NoError(db.TestVerifyOplog(t, rw, tt.args.setId, db.WithOperation(oplog.OpType_OP_TYPE_CREATE), db.WithCreateNotBefore(10*time.Second))) + }) + } +} + +func TestRepository_AddSetMembers_Combinations(t *testing.T) { + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + kms := kms.TestKms(t, conn, wrapper) + iamRepo := iam.TestRepo(t, conn, wrapper) + + _, prj := iam.TestScopes(t, iamRepo) + + assert, require := assert.New(t), require.New(t) + c := TestCatalogs(t, conn, prj.PublicId, 1)[0] + set := TestSets(t, conn, c.PublicId, 1)[0] + + repo, err := NewRepository(rw, rw, kms) + require.NoError(err) + require.NotNil(repo) + + hosts := TestHosts(t, conn, c.PublicId, 5) + var hostIds []string + for _, h := range hosts { + hostIds = append(hostIds, h.PublicId) + } + + // first call - add first set of hosts - should succeed + got, err := repo.AddSetMembers(context.Background(), prj.PublicId, set.PublicId, set.Version, hostIds) + require.NoError(err) + require.NotNil(got) + + opts := []cmp.Option{ + cmpopts.SortSlices(func(x, y *Host) bool { return x.PublicId < y.PublicId }), + protocmp.Transform(), + } + assert.Len(got, len(hosts)) + assert.Empty(cmp.Diff(hosts, got, opts...)) + assert.NoError(db.TestVerifyOplog(t, rw, set.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_CREATE), db.WithCreateNotBefore(10*time.Second))) + + // second call - add new set of hosts - should succeed + set.Version = set.Version + 1 + hosts2 := TestHosts(t, conn, c.PublicId, 5) + var hostIds2 []string + for _, h := range hosts2 { + hostIds2 = append(hostIds2, h.PublicId) + } + got2, err2 := repo.AddSetMembers(context.Background(), prj.PublicId, set.PublicId, set.Version, hostIds2) + require.NoError(err2) + require.NotNil(got2) + + hosts = append(hosts, hosts2...) + assert.Len(got2, len(hosts)) + assert.Empty(cmp.Diff(hosts, got2, opts...)) + assert.NoError(db.TestVerifyOplog(t, rw, set.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_CREATE), db.WithCreateNotBefore(10*time.Second))) + + // third call - add new hosts plus a few existing hosts - should fail + set.Version = set.Version + 1 + hosts3 := TestHosts(t, conn, c.PublicId, 5) + var hostIds3 []string + for _, h := range hosts3 { + hostIds3 = append(hostIds2, h.PublicId) + } + hostIds3 = append(hostIds3, hostIds2...) + got3, err3 := repo.AddSetMembers(context.Background(), prj.PublicId, set.PublicId, set.Version, hostIds3) + require.Error(err3) + require.Nil(got3) +} + +func TestRepository_DeleteSetMembers_Parameters(t *testing.T) { + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + kms := kms.TestKms(t, conn, wrapper) + iamRepo := iam.TestRepo(t, conn, wrapper) + + _, prj := iam.TestScopes(t, iamRepo) + c := TestCatalogs(t, conn, prj.PublicId, 1)[0] + set := TestSets(t, conn, c.PublicId, 1)[0] + count := 5 + hosts := TestHosts(t, conn, c.PublicId, count) + TestSetMembers(t, conn, set.PublicId, hosts) + + var hostIds []string + for _, h := range hosts { + hostIds = append(hostIds, h.PublicId) + } + + badVersion := uint32(12345) + + type args struct { + scopeId string + setId string + version uint32 + hostIds []string + opt []Option + } + + tests := []struct { + name string + args args + want int + wantErr bool + wantIsErr error + }{ + { + name: "empty-scope-id", + args: args{ + setId: set.PublicId, + version: set.Version, + hostIds: hostIds, + }, + wantIsErr: db.ErrInvalidParameter, + }, + { + name: "empty-set-id", + args: args{ + scopeId: prj.PublicId, + version: set.Version, + hostIds: hostIds, + }, + wantIsErr: db.ErrInvalidParameter, + }, + { + name: "zero-version", + args: args{ + scopeId: prj.PublicId, + setId: set.PublicId, + hostIds: hostIds, + }, + wantIsErr: db.ErrInvalidParameter, + }, + { + name: "empty-host-ids", + args: args{ + scopeId: prj.PublicId, + setId: set.PublicId, + version: set.Version, + }, + wantIsErr: db.ErrInvalidParameter, + }, + { + name: "invalid-version", + args: args{ + scopeId: prj.PublicId, + setId: set.PublicId, + version: badVersion, + hostIds: hostIds, + }, + wantErr: true, + }, + { + name: "valid", + args: args{ + scopeId: prj.PublicId, + setId: set.PublicId, + version: set.Version, + hostIds: hostIds, + }, + want: count, + wantErr: false, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + repo, err := NewRepository(rw, rw, kms) + require.NoError(err) + require.NotNil(repo) + got, err := repo.DeleteSetMembers(context.Background(), tt.args.scopeId, tt.args.setId, tt.args.version, tt.args.hostIds, tt.args.opt...) + if tt.wantIsErr != nil { + assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + assert.Zero(got) + assert.Error(db.TestVerifyOplog(t, rw, tt.args.setId, db.WithOperation(oplog.OpType_OP_TYPE_DELETE), db.WithCreateNotBefore(10*time.Second))) + return + } + if tt.wantErr { + assert.Error(err) + assert.Zero(got) + assert.Error(db.TestVerifyOplog(t, rw, tt.args.setId, db.WithOperation(oplog.OpType_OP_TYPE_DELETE), db.WithCreateNotBefore(10*time.Second))) + return + } + assert.NoError(err) + assert.Equal(tt.want, got) + assert.NoError(db.TestVerifyOplog(t, rw, tt.args.setId, db.WithOperation(oplog.OpType_OP_TYPE_DELETE), db.WithCreateNotBefore(10*time.Second))) + }) + } +} + +func TestRepository_DeleteSetMembers_Combinations(t *testing.T) { + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + kms := kms.TestKms(t, conn, wrapper) + iamRepo := iam.TestRepo(t, conn, wrapper) + + _, prj := iam.TestScopes(t, iamRepo) + + assert, require := assert.New(t), require.New(t) + c := TestCatalogs(t, conn, prj.PublicId, 1)[0] + set := TestSets(t, conn, c.PublicId, 1)[0] + + repo, err := NewRepository(rw, rw, kms) + require.NoError(err) + require.NotNil(repo) + + count := 10 + hosts := TestHosts(t, conn, c.PublicId, count) + TestSetMembers(t, conn, set.PublicId, hosts) + + var hostIds []string + for _, h := range hosts { + hostIds = append(hostIds, h.PublicId) + } + split := 5 + idsA := hostIds[:split] + hostsB, idsB := hosts[split:], hostIds[split:] + + // first call - delete first half of hosts - should succeed + got, err := repo.DeleteSetMembers(context.Background(), prj.PublicId, set.PublicId, set.Version, idsA) + assert.NoError(err) + require.Equal(len(idsA), got) + assert.NoError(db.TestVerifyOplog(t, rw, set.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_DELETE), db.WithCreateNotBefore(10*time.Second))) + + // verify hostsB are still members + members, err := getHosts(context.Background(), repo.reader, set.PublicId, unlimited) + require.NoError(err) + + opts := []cmp.Option{ + cmpopts.SortSlices(func(x, y *Host) bool { return x.PublicId < y.PublicId }), + protocmp.Transform(), + } + assert.Len(members, len(hostsB)) + assert.Empty(cmp.Diff(hostsB, members, opts...)) + + // second call - delete first half of hosts again - should fail + set.Version = set.Version + 1 + got2, err2 := repo.DeleteSetMembers(context.Background(), prj.PublicId, set.PublicId, set.Version, idsA) + require.Error(err2) + assert.Zero(got2) + + // third call - delete first half and second half - should fail + got3, err3 := repo.DeleteSetMembers(context.Background(), prj.PublicId, set.PublicId, set.Version, hostIds) + require.Error(err3) + assert.Zero(got3) + + // fourth call - delete second half of hosts - should succeed + got4, err4 := repo.DeleteSetMembers(context.Background(), prj.PublicId, set.PublicId, set.Version, idsB) + assert.NoError(err4) + require.Equal(len(idsB), got4) + assert.NoError(db.TestVerifyOplog(t, rw, set.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_DELETE), db.WithCreateNotBefore(10*time.Second))) + + // verify no members remain + members2, err := getHosts(context.Background(), repo.reader, set.PublicId, unlimited) + require.NoError(err) + require.Empty(members2) +} + +func TestRepository_SetSetMembers_Parameters(t *testing.T) { + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + kms := kms.TestKms(t, conn, wrapper) + iamRepo := iam.TestRepo(t, conn, wrapper) + + _, prj := iam.TestScopes(t, iamRepo) + c := TestCatalogs(t, conn, prj.PublicId, 1)[0] + set := TestSets(t, conn, c.PublicId, 1)[0] + + count := 5 + hosts := TestHosts(t, conn, c.PublicId, count) + + // hostsA has the first 3 hosts, hostsB has the last 3 hosts + // the middle host is shared in both. + hostsA, hostsB := hosts[:3], hosts[2:] + //hostsA is the initial set of hosts in the host set + TestSetMembers(t, conn, set.PublicId, hostsA) + + var hostIds []string + for _, h := range hostsB { + hostIds = append(hostIds, h.PublicId) + } + + badVersion := uint32(12345) + + type args struct { + scopeId string + setId string + version uint32 + hostIds []string + opt []Option + } + + tests := []struct { + name string + args args + want []*Host + wantCount int + wantErr bool + wantIsErr error + }{ + { + name: "empty-scope-id", + args: args{ + setId: set.PublicId, + version: set.Version, + hostIds: hostIds, + }, + wantIsErr: db.ErrInvalidParameter, + }, + { + name: "empty-set-id", + args: args{ + scopeId: prj.PublicId, + version: set.Version, + hostIds: hostIds, + }, + wantIsErr: db.ErrInvalidParameter, + }, + { + name: "zero-version", + args: args{ + scopeId: prj.PublicId, + setId: set.PublicId, + hostIds: hostIds, + }, + wantIsErr: db.ErrInvalidParameter, + }, + { + name: "invalid-version", + args: args{ + scopeId: prj.PublicId, + setId: set.PublicId, + version: badVersion, + hostIds: hostIds, + }, + wantErr: true, + }, + { + name: "valid", + args: args{ + scopeId: prj.PublicId, + setId: set.PublicId, + version: set.Version, + hostIds: hostIds, + }, + want: hostsB, + wantCount: 4, // 2 deleted, 2 added + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + repo, err := NewRepository(rw, rw, kms) + require.NoError(err) + require.NotNil(repo) + got, gotCount, err := repo.SetSetMembers(context.Background(), tt.args.scopeId, tt.args.setId, tt.args.version, tt.args.hostIds, tt.args.opt...) + if tt.wantIsErr != nil { + assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + assert.Nil(got) + assert.Equal(tt.wantCount, gotCount) + assert.Error(db.TestVerifyOplog(t, rw, tt.args.setId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) + return + } + if tt.wantErr { + assert.Error(err) + assert.Nil(got) + assert.Equal(tt.wantCount, gotCount) + assert.Error(db.TestVerifyOplog(t, rw, tt.args.setId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) + return + } + require.NoError(err) + opts := []cmp.Option{ + cmpopts.SortSlices(func(x, y *Host) bool { return x.PublicId < y.PublicId }), + protocmp.Transform(), + } + assert.Equal(tt.wantCount, gotCount) + assert.Len(got, len(tt.want)) + assert.Empty(cmp.Diff(tt.want, got, opts...)) + assert.NoError(db.TestVerifyOplog(t, rw, tt.args.setId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) + }) + } +} + +func TestRepository_SetSetMembers_Combinations(t *testing.T) { + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + kms := kms.TestKms(t, conn, wrapper) + iamRepo := iam.TestRepo(t, conn, wrapper) + + _, prj := iam.TestScopes(t, iamRepo) + + assert, require := assert.New(t), require.New(t) + c := TestCatalogs(t, conn, prj.PublicId, 1)[0] + set := TestSets(t, conn, c.PublicId, 1)[0] + + repo, err := NewRepository(rw, rw, kms) + require.NoError(err) + require.NotNil(repo) + + count := 5 + hosts := TestHosts(t, conn, c.PublicId, count) + + var hostIds []string + for _, h := range hosts { + hostIds = append(hostIds, h.PublicId) + } + + // hostsA has the first 3 hosts, hostsB has the last 3 hosts + // the middle host is shared in both. + hostsA, hostsB := hosts[:3], hosts[2:] + hostIdsA, hostIdsB := hostIds[:3], hostIds[2:] + + opts := []cmp.Option{ + cmpopts.SortSlices(func(x, y *Host) bool { return x.PublicId < y.PublicId }), + protocmp.Transform(), + } + + // first call - empty set, empty host Ids - no additions no deletions + got1, gotCount1, err1 := repo.SetSetMembers(context.Background(), prj.PublicId, set.PublicId, set.Version, nil) + assert.NoError(err1) + assert.Empty(got1) + assert.Zero(gotCount1) + assert.Error(db.TestVerifyOplog(t, rw, set.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) + + // second call - all additions + got2, gotCount2, err2 := repo.SetSetMembers(context.Background(), prj.PublicId, set.PublicId, set.Version, hostIdsA) + assert.NoError(err2) + assert.Equal(len(hostsA), gotCount2) + assert.NoError(db.TestVerifyOplog(t, rw, set.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) + assert.Len(got2, len(hostsA)) + assert.Empty(cmp.Diff(hostsA, got2, opts...)) + + // third call - mix of additions and deletions + set.Version = set.Version + 1 + got3, gotCount3, err3 := repo.SetSetMembers(context.Background(), prj.PublicId, set.PublicId, set.Version, hostIdsB) + assert.NoError(err3) + assert.Equal(4, gotCount3) + assert.NoError(db.TestVerifyOplog(t, rw, set.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) + assert.Len(got3, len(hostsB)) + assert.Empty(cmp.Diff(hostsB, got3, opts...)) + + // fourth call - all deletions + set.Version = set.Version + 1 + got4, gotCount4, err4 := repo.SetSetMembers(context.Background(), prj.PublicId, set.PublicId, set.Version, nil) + assert.NoError(err4) + assert.Equal(len(hostsB), gotCount4) + assert.NoError(db.TestVerifyOplog(t, rw, set.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) + assert.Empty(got4) +} + +func TestRepository_changes(t *testing.T) { + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + kms := kms.TestKms(t, conn, wrapper) + iamRepo := iam.TestRepo(t, conn, wrapper) + + _, prj := iam.TestScopes(t, iamRepo) + + t.Run("all-additions", func(t *testing.T) { + c := TestCatalogs(t, conn, prj.PublicId, 1)[0] + set := TestSets(t, conn, c.PublicId, 1)[0] + count := 5 + hosts := TestHosts(t, conn, c.PublicId, count) + + var hostIds []string + for _, h := range hosts { + hostIds = append(hostIds, h.PublicId) + } + + var want []*change + for _, h := range hosts { + chg := &change{ + Action: "add", + HostId: h.PublicId, + } + want = append(want, chg) + } + + assert, require := assert.New(t), require.New(t) + repo, err := NewRepository(rw, rw, kms) + require.NoError(err) + require.NotNil(repo) + + got, err := repo.changes(context.Background(), set.PublicId, hostIds) + assert.NoError(err) + require.NotNil(got) + opts := []cmp.Option{ + cmpopts.SortSlices(func(x, y *change) bool { return x.HostId < y.HostId }), + } + assert.Len(got, len(want)) + assert.Empty(cmp.Diff(want, got, opts...)) + }) + t.Run("all-deletions", func(t *testing.T) { + c := TestCatalogs(t, conn, prj.PublicId, 1)[0] + set := TestSets(t, conn, c.PublicId, 1)[0] + count := 5 + hosts := TestHosts(t, conn, c.PublicId, count) + TestSetMembers(t, conn, set.PublicId, hosts) + + var want []*change + for _, h := range hosts { + chg := &change{ + Action: "delete", + HostId: h.PublicId, + } + want = append(want, chg) + } + + assert, require := assert.New(t), require.New(t) + repo, err := NewRepository(rw, rw, kms) + require.NoError(err) + require.NotNil(repo) + + got, err := repo.changes(context.Background(), set.PublicId, nil) + assert.NoError(err) + require.NotNil(got) + opts := []cmp.Option{ + cmpopts.SortSlices(func(x, y *change) bool { return x.HostId < y.HostId }), + } + assert.Len(got, len(want)) + assert.Empty(cmp.Diff(want, got, opts...)) + }) + t.Run("additions-with-deletions", func(t *testing.T) { + c := TestCatalogs(t, conn, prj.PublicId, 1)[0] + set := TestSets(t, conn, c.PublicId, 1)[0] + + count := 5 + hosts := TestHosts(t, conn, c.PublicId, count) + initialHosts := hosts[:3] + TestSetMembers(t, conn, set.PublicId, initialHosts) + targetHosts := hosts[2:] + deleteHosts := hosts[:2] + insertHosts := hosts[3:] + + var targetHostIds []string + for _, h := range targetHosts { + targetHostIds = append(targetHostIds, h.PublicId) + } + + var want []*change + for _, h := range deleteHosts { + chg := &change{ + Action: "delete", + HostId: h.PublicId, + } + want = append(want, chg) + } + for _, h := range insertHosts { + chg := &change{ + Action: "add", + HostId: h.PublicId, + } + want = append(want, chg) + } + + assert, require := assert.New(t), require.New(t) + repo, err := NewRepository(rw, rw, kms) + require.NoError(err) + require.NotNil(repo) + + got, err := repo.changes(context.Background(), set.PublicId, targetHostIds) + assert.NoError(err) + require.NotNil(got) + opts := []cmp.Option{ + cmpopts.SortSlices(func(x, y *change) bool { return x.HostId < y.HostId }), + } + assert.Len(got, len(want)) + assert.Empty(cmp.Diff(want, got, opts...)) + }) +} diff --git a/internal/host/static/repository_host_set_test.go b/internal/host/static/repository_host_set_test.go index c11129590..ec4cc85f7 100644 --- a/internal/host/static/repository_host_set_test.go +++ b/internal/host/static/repository_host_set_test.go @@ -468,16 +468,18 @@ func TestRepository_UpdateSet(t *testing.T) { _, prj := iam.TestScopes(t, iamRepo) catalog := TestCatalogs(t, conn, prj.PublicId, 1)[0] + hosts := TestHosts(t, conn, catalog.PublicId, 5) tt.orig.CatalogId = catalog.PublicId orig, err := repo.CreateSet(context.Background(), prj.GetPublicId(), tt.orig) assert.NoError(err) require.NotNil(orig) + TestSetMembers(t, conn, orig.PublicId, hosts) if tt.chgFn != nil { orig = tt.chgFn(orig) } - got, gotCount, err := repo.UpdateSet(context.Background(), prj.GetPublicId(), orig, 1, tt.masks) + got, gotHosts, gotCount, err := repo.UpdateSet(context.Background(), prj.GetPublicId(), orig, 1, tt.masks) if tt.wantIsErr != nil { assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) assert.Equal(tt.wantCount, gotCount, "row count") @@ -505,6 +507,12 @@ func TestRepository_UpdateSet(t *testing.T) { if tt.wantCount > 0 { assert.NoError(db.TestVerifyOplog(t, rw, got.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) } + require.Len(gotHosts, len(hosts)) + opts := []cmp.Option{ + cmpopts.SortSlices(func(x, y *Host) bool { return x.PublicId < y.PublicId }), + protocmp.Transform(), + } + assert.Empty(cmp.Diff(hosts, gotHosts, opts...)) }) } @@ -522,21 +530,23 @@ func TestRepository_UpdateSet(t *testing.T) { sA, sB := ss[0], ss[1] sA.Name = name - got1, gotCount1, err := repo.UpdateSet(context.Background(), prj.GetPublicId(), sA, 1, []string{"name"}) + got1, gotHosts1, gotCount1, err := repo.UpdateSet(context.Background(), prj.GetPublicId(), sA, 1, []string{"name"}) assert.NoError(err) require.NotNil(got1) assert.Equal(name, got1.Name) assert.Equal(1, gotCount1, "row count") assert.NoError(db.TestVerifyOplog(t, rw, sA.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) + assert.Empty(gotHosts1) sB.Name = name - got2, gotCount2, err := repo.UpdateSet(context.Background(), prj.GetPublicId(), sB, 1, []string{"name"}) + got2, gotHosts2, gotCount2, err := repo.UpdateSet(context.Background(), prj.GetPublicId(), sB, 1, []string{"name"}) assert.Truef(errors.Is(err, db.ErrNotUnique), "want err: %v got: %v", db.ErrNotUnique, err) assert.Nil(got2) assert.Equal(db.NoRowsAffected, gotCount2, "row count") err = db.TestVerifyOplog(t, rw, sB.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) assert.Error(err) assert.True(errors.Is(db.ErrRecordNotFound, err)) + assert.Empty(gotHosts2) }) t.Run("valid-duplicate-names-diff-Catalogs", func(t *testing.T) { @@ -572,7 +582,7 @@ func TestRepository_UpdateSet(t *testing.T) { assert.NoError(err) require.NotNil(got2) got2.Name = got.Name - got3, gotCount3, err := repo.UpdateSet(context.Background(), prj.GetPublicId(), got2, 1, []string{"name"}) + got3, gotHosts3, gotCount3, err := repo.UpdateSet(context.Background(), prj.GetPublicId(), got2, 1, []string{"name"}) assert.NoError(err) require.NotNil(got3) assert.NotSame(got2, got3) @@ -580,6 +590,7 @@ func TestRepository_UpdateSet(t *testing.T) { assert.Equal(got2.Description, got3.Description) assert.Equal(1, gotCount3, "row count") assert.NoError(db.TestVerifyOplog(t, rw, got2.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) + assert.Empty(gotHosts3) }) t.Run("change-scope-id", func(t *testing.T) { @@ -602,17 +613,18 @@ func TestRepository_UpdateSet(t *testing.T) { sA.CatalogId = sB.CatalogId assert.Equal(sA.CatalogId, sB.CatalogId) - got1, gotCount1, err := repo.UpdateSet(context.Background(), prj.GetPublicId(), sA, 1, []string{"name"}) + got1, gotHosts1, gotCount1, err := repo.UpdateSet(context.Background(), prj.GetPublicId(), sA, 1, []string{"name"}) assert.NoError(err) require.NotNil(got1) assert.Equal(orig.CatalogId, got1.CatalogId) assert.Equal(1, gotCount1, "row count") assert.NoError(db.TestVerifyOplog(t, rw, sA.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) + assert.Empty(gotHosts1) }) } -func TestRepository_LookupSet(t *testing.T) { +func TestRepository_UpdateSet_Limits(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") rw := db.New(conn) wrapper := db.TestWrapper(t) @@ -622,13 +634,96 @@ func TestRepository_LookupSet(t *testing.T) { _, prj := iam.TestScopes(t, iamRepo) catalog := TestCatalogs(t, conn, prj.PublicId, 1)[0] hostSet := TestSets(t, conn, catalog.PublicId, 1)[0] + count := 10 + hosts := TestHosts(t, conn, catalog.PublicId, count) + TestSetMembers(t, conn, hostSet.PublicId, hosts) + + var tests = []struct { + name string + repoOpts []Option + updateOpts []Option + wantLen int + }{ + { + name: "With no limits", + wantLen: count, + }, + { + name: "With repo limit", + repoOpts: []Option{WithLimit(3)}, + wantLen: 3, + }, + { + name: "With negative repo limit", + repoOpts: []Option{WithLimit(-1)}, + wantLen: count, + }, + { + name: "With List limit", + updateOpts: []Option{WithLimit(3)}, + wantLen: 3, + }, + { + name: "With negative List limit", + updateOpts: []Option{WithLimit(-1)}, + wantLen: count, + }, + { + name: "With repo smaller than list limit", + repoOpts: []Option{WithLimit(2)}, + updateOpts: []Option{WithLimit(6)}, + wantLen: 6, + }, + { + name: "With repo larger than list limit", + repoOpts: []Option{WithLimit(6)}, + updateOpts: []Option{WithLimit(2)}, + wantLen: 2, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + repo, err := NewRepository(rw, rw, kms, tt.repoOpts...) + assert.NoError(err) + require.NotNil(repo) + hs := hostSet.clone() + hs.Description = tt.name + got, gotHosts, _, err := repo.UpdateSet(context.Background(), prj.PublicId, hs, hs.Version, []string{"Description"}, tt.updateOpts...) + require.NoError(err) + require.NotNil(got) + assert.Len(gotHosts, tt.wantLen) + require.Greater(got.Version, hs.Version) + hostSet = got + }) + } +} + +func TestRepository_LookupSet(t *testing.T) { + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + kms := kms.TestKms(t, conn, wrapper) + iamRepo := iam.TestRepo(t, conn, wrapper) + _, prj := iam.TestScopes(t, iamRepo) + + catalog := TestCatalogs(t, conn, prj.PublicId, 1)[0] + hostSet := TestSets(t, conn, catalog.PublicId, 1)[0] + hosts := TestHosts(t, conn, catalog.PublicId, 1) + TestSetMembers(t, conn, hostSet.PublicId, hosts) + + emptyHostSet := TestSets(t, conn, catalog.PublicId, 1)[0] hostSetId, err := newHostSetId() require.NoError(t, err) + var tests = []struct { name string in string want *HostSet + wantHosts []*Host wantIsErr error }{ { @@ -640,9 +735,16 @@ func TestRepository_LookupSet(t *testing.T) { in: hostSetId, }, { - name: "with-existing-host-set-id", - in: hostSet.PublicId, - want: hostSet, + name: "with-existing-host-set-id", + in: hostSet.PublicId, + want: hostSet, + wantHosts: hosts, + }, + { + name: "with-existing-host-set-id-empty-hosts", + in: emptyHostSet.PublicId, + want: emptyHostSet, + wantHosts: nil, }, } @@ -653,7 +755,7 @@ func TestRepository_LookupSet(t *testing.T) { repo, err := NewRepository(rw, rw, kms) assert.NoError(err) require.NotNil(repo) - got, err := repo.LookupSet(context.Background(), tt.in) + got, gotHosts, err := repo.LookupSet(context.Background(), tt.in) if tt.wantIsErr != nil { assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) assert.Nil(got) @@ -661,6 +763,85 @@ func TestRepository_LookupSet(t *testing.T) { } require.NoError(err) assert.EqualValues(tt.want, got) + require.Len(gotHosts, len(tt.wantHosts)) + opts := []cmp.Option{ + cmpopts.SortSlices(func(x, y *Host) bool { return x.PublicId < y.PublicId }), + protocmp.Transform(), + } + assert.Empty(cmp.Diff(tt.wantHosts, gotHosts, opts...)) + }) + } +} + +func TestRepository_LookupSet_Limits(t *testing.T) { + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + kms := kms.TestKms(t, conn, wrapper) + iamRepo := iam.TestRepo(t, conn, wrapper) + + _, prj := iam.TestScopes(t, iamRepo) + catalog := TestCatalogs(t, conn, prj.PublicId, 1)[0] + hostSet := TestSets(t, conn, catalog.PublicId, 1)[0] + count := 10 + hosts := TestHosts(t, conn, catalog.PublicId, count) + TestSetMembers(t, conn, hostSet.PublicId, hosts) + + var tests = []struct { + name string + repoOpts []Option + lookupOpts []Option + wantLen int + }{ + { + name: "With no limits", + wantLen: count, + }, + { + name: "With repo limit", + repoOpts: []Option{WithLimit(3)}, + wantLen: 3, + }, + { + name: "With negative repo limit", + repoOpts: []Option{WithLimit(-1)}, + wantLen: count, + }, + { + name: "With List limit", + lookupOpts: []Option{WithLimit(3)}, + wantLen: 3, + }, + { + name: "With negative List limit", + lookupOpts: []Option{WithLimit(-1)}, + wantLen: count, + }, + { + name: "With repo smaller than list limit", + repoOpts: []Option{WithLimit(2)}, + lookupOpts: []Option{WithLimit(6)}, + wantLen: 6, + }, + { + name: "With repo larger than list limit", + repoOpts: []Option{WithLimit(6)}, + lookupOpts: []Option{WithLimit(2)}, + wantLen: 2, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + repo, err := NewRepository(rw, rw, kms, tt.repoOpts...) + assert.NoError(err) + require.NotNil(repo) + got, gotHosts, err := repo.LookupSet(context.Background(), hostSet.PublicId, tt.lookupOpts...) + require.NoError(err) + require.NotNil(got) + assert.Len(gotHosts, tt.wantLen) }) } } diff --git a/internal/host/static/testing.go b/internal/host/static/testing.go index f928b69f1..1cc0c6847 100644 --- a/internal/host/static/testing.go +++ b/internal/host/static/testing.go @@ -84,3 +84,24 @@ func TestSets(t *testing.T, conn *gorm.DB, catalogId string, count int) []*HostS } return sets } + +// TestSetMembers adds hosts to the specified setId in the provided DB. +// The set and hosts must have been created previously and belong to the +// same catalog. The test will fail if any errors are encountered. +func TestSetMembers(t *testing.T, conn *gorm.DB, setId string, hosts []*Host) []*HostSetMember { + t.Helper() + assert := assert.New(t) + + var members []*HostSetMember + for _, host := range hosts { + member, err := NewHostSetMember(setId, host.PublicId) + assert.NoError(err) + assert.NotNil(member) + + w := db.New(conn) + err2 := w.Create(context.Background(), member) + assert.NoError(err2) + members = append(members, member) + } + return members +} diff --git a/internal/host/static/testing_test.go b/internal/host/static/testing_test.go index 36a4786b8..bfbf45b07 100644 --- a/internal/host/static/testing_test.go +++ b/internal/host/static/testing_test.go @@ -64,3 +64,26 @@ func Test_TestSets(t *testing.T) { assert.NotEmpty(s.GetPublicId()) } } + +func Test_TestSetMembers(t *testing.T) { + t.Helper() + assert, require := assert.New(t), require.New(t) + conn, _ := db.TestSetup(t, "postgres") + wrapper := db.TestWrapper(t) + _, prj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) + + require.NotNil(prj) + assert.NotEmpty(prj.GetPublicId()) + + c := TestCatalogs(t, conn, prj.GetPublicId(), 1)[0] + s := TestSets(t, conn, c.GetPublicId(), 1)[0] + + count := 4 + hosts := TestHosts(t, conn, c.GetPublicId(), count) + members := TestSetMembers(t, conn, s.PublicId, hosts) + assert.Len(members, count) + for _, m := range members { + assert.NotEmpty(m.GetSetId()) + assert.NotEmpty(m.GetHostId()) + } +} diff --git a/internal/iam/repository_group.go b/internal/iam/repository_group.go index 1c60aed72..fab6e8777 100644 --- a/internal/iam/repository_group.go +++ b/internal/iam/repository_group.go @@ -381,6 +381,8 @@ func (r *Repository) SetGroupMembers(ctx context.Context, groupId string, groupV return nil, db.NoRowsAffected, fmt.Errorf("set group members: unable to get members %s scope: %w", groupId, err) } + // TODO(mgaffney) 08/2020: Use SQL to calculate changes. + // find existing members (since we're using groupVersion, we can safely do // this here, outside the TxHandler) currentMembers := []*GroupMember{} diff --git a/internal/iam/repository_principal_role.go b/internal/iam/repository_principal_role.go index 7c2ff1916..e7161e509 100644 --- a/internal/iam/repository_principal_role.go +++ b/internal/iam/repository_principal_role.go @@ -408,6 +408,7 @@ type principalSet struct { // TODO: Should this be moved inside the transaction, at this point? func (r *Repository) principalsToSet(ctx context.Context, role *Role, userIds, groupIds []string) (*principalSet, error) { + // TODO(mgaffney) 08/2020: Use SQL to calculate changes. if role == nil { return nil, fmt.Errorf("missing role: %w", db.ErrInvalidParameter) } diff --git a/internal/iam/repository_role_grant.go b/internal/iam/repository_role_grant.go index 9709ddc36..552b6d3c6 100644 --- a/internal/iam/repository_role_grant.go +++ b/internal/iam/repository_role_grant.go @@ -231,6 +231,8 @@ func (r *Repository) SetRoleGrants(ctx context.Context, roleId string, roleVersi role := allocRole() role.PublicId = roleId + // TODO(mgaffney) 08/2020: Use SQL to calculate changes. + // NOTE: Set calculation can safely take place out of the transaction since // we are using roleVersion to ensure that we end up operating on the same // set of data from this query to the final set in the transaction function diff --git a/internal/servers/controller/handlers/host_sets/host_set_service.go b/internal/servers/controller/handlers/host_sets/host_set_service.go index bc73a0568..753381b7c 100644 --- a/internal/servers/controller/handlers/host_sets/host_set_service.go +++ b/internal/servers/controller/handlers/host_sets/host_set_service.go @@ -184,7 +184,7 @@ func (s Service) getFromRepo(ctx context.Context, id string) (*pb.HostSet, error if err != nil { return nil, err } - h, err := repo.LookupSet(ctx, id) + h, _, err := repo.LookupSet(ctx, id) if err != nil { return nil, err } @@ -241,7 +241,7 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, catalogId, id string if err != nil { return nil, err } - out, rowsUpdated, err := repo.UpdateSet(ctx, scopeId, h, item.GetVersion(), dbMask) + out, _, rowsUpdated, err := repo.UpdateSet(ctx, scopeId, h, item.GetVersion(), dbMask) if err != nil { return nil, status.Errorf(codes.Internal, "Unable to update host set: %v.", err) } @@ -288,7 +288,7 @@ func (s Service) addInRepo(ctx context.Context, scopeId, setId string, hostIds [ // if err != nil { // return nil, status.Errorf(codes.Internal, "Unable to add members to group: %v.", err) // } - out, err := repo.LookupSet(ctx, setId) + out, _, err := repo.LookupSet(ctx, setId) if err != nil { return nil, status.Errorf(codes.Internal, "Unable to look up group: %v.", err) } @@ -308,7 +308,7 @@ func (s Service) setInRepo(ctx context.Context, scopeId, setId string, hostIds [ // return nil, status.Errorf(codes.Internal, "Unable to set members on group: %v.", err) // } - out, err := repo.LookupSet(ctx, setId) + out, _, err := repo.LookupSet(ctx, setId) if err != nil { return nil, status.Errorf(codes.Internal, "Unable to look up group: %v.", err) } @@ -327,7 +327,7 @@ func (s Service) removeInRepo(ctx context.Context, scopeId, setId string, hostId // if err != nil { // return nil, status.Errorf(codes.Internal, "Unable to remove members from group: %v.", err) // } - out, err := repo.LookupSet(ctx, setId) + out, _, err := repo.LookupSet(ctx, setId) if err != nil { return nil, status.Errorf(codes.Internal, "Unable to look up group: %v.", err) } diff --git a/internal/servers/controller/handlers/host_sets/host_set_service_test.go b/internal/servers/controller/handlers/host_sets/host_set_service_test.go index a4caa2c77..de97aff1b 100644 --- a/internal/servers/controller/handlers/host_sets/host_set_service_test.go +++ b/internal/servers/controller/handlers/host_sets/host_set_service_test.go @@ -439,7 +439,7 @@ func TestUpdate(t *testing.T) { resetHostSet := func() { version++ - _, _, err = repo.UpdateSet(context.Background(), proj.GetPublicId(), h, version, []string{"Name", "Description"}) + _, _, _, err = repo.UpdateSet(context.Background(), proj.GetPublicId(), h, version, []string{"Name", "Description"}) require.NoError(t, err, "Failed to reset host.") version++ }