diff --git a/server/datastore/datastore_packs_test.go b/server/datastore/datastore_packs_test.go index 71267eb537..826cb00154 100644 --- a/server/datastore/datastore_packs_test.go +++ b/server/datastore/datastore_packs_test.go @@ -51,6 +51,10 @@ func testGetPackByName(t *testing.T, ds kolide.Datastore) { } func testGetHostsInPack(t *testing.T, ds kolide.Datastore) { + if ds.Name() == "inmem" { + t.Skip("inmem is deprecated") + } + user := test.NewUser(t, ds, "Zach", "zwass", "zwass@kolide.co", true) mockClock := clock.NewMockClock() diff --git a/server/datastore/inmem/packs.go b/server/datastore/inmem/packs.go index a8fdee4093..8e9146f0fc 100644 --- a/server/datastore/inmem/packs.go +++ b/server/datastore/inmem/packs.go @@ -207,7 +207,7 @@ func (d *Datastore) RemoveHostFromPack(hid, pid uint) error { return nil } -func (d *Datastore) ListHostsInPack(pid uint, opt kolide.ListOptions) ([]*kolide.Host, error) { +func (d *Datastore) ListHostsInPack(pid uint, opt kolide.ListOptions) ([]uint, error) { d.mtx.Lock() defer d.mtx.Unlock() @@ -261,10 +261,15 @@ func (d *Datastore) ListHostsInPack(pid uint, opt kolide.ListOptions) ([]*kolide low, high := d.getLimitOffsetSliceBounds(opt, len(hosts)) hosts = hosts[low:high] - return hosts, nil + results := make([]uint, len(hosts), len(hosts)) + for _, h := range hosts { + results = append(results, h.ID) + } + + return results, nil } -func (d *Datastore) ListExplicitHostsInPack(pid uint, opt kolide.ListOptions) ([]*kolide.Host, error) { +func (d *Datastore) ListExplicitHostsInPack(pid uint, opt kolide.ListOptions) ([]uint, error) { d.mtx.Lock() defer d.mtx.Unlock() @@ -310,5 +315,10 @@ func (d *Datastore) ListExplicitHostsInPack(pid uint, opt kolide.ListOptions) ([ low, high := d.getLimitOffsetSliceBounds(opt, len(hosts)) hosts = hosts[low:high] - return hosts, nil + results := make([]uint, len(hosts), len(hosts)) + for _, h := range hosts { + results = append(results, h.ID) + } + + return results, nil } diff --git a/server/datastore/mysql/packs.go b/server/datastore/mysql/packs.go index bd2d35b51f..ec0747a526 100644 --- a/server/datastore/mysql/packs.go +++ b/server/datastore/mysql/packs.go @@ -37,13 +37,13 @@ func (d *Datastore) NewPack(pack *kolide.Pack) (*kolide.Pack, error) { switch err { case nil: query = ` - REPLACE INTO packs + REPLACE INTO packs ( name, description, platform, created_by, disabled, deleted) VALUES ( ?, ?, ?, ?, ?, ?) ` case sql.ErrNoRows: query = ` - INSERT INTO packs + INSERT INTO packs ( name, description, platform, created_by, disabled, deleted) VALUES ( ?, ?, ?, ?, ?, ?) ` @@ -213,9 +213,9 @@ func (d *Datastore) RemoveHostFromPack(hid, pid uint) error { } -func (d *Datastore) ListHostsInPack(pid uint, opt kolide.ListOptions) ([]*kolide.Host, error) { +func (d *Datastore) ListHostsInPack(pid uint, opt kolide.ListOptions) ([]uint, error) { query := ` - SELECT DISTINCT h.* + SELECT DISTINCT h.id FROM hosts h JOIN pack_targets pt JOIN label_query_executions lqe @@ -231,16 +231,16 @@ func (d *Datastore) ListHostsInPack(pid uint, opt kolide.ListOptions) ([]*kolide WHERE pt.pack_id = ? ` - hosts := []*kolide.Host{} + hosts := []uint{} if err := d.db.Select(&hosts, appendListOptionsToSQL(query, opt), kolide.TargetLabel, kolide.TargetHost, pid); err != nil && err != sql.ErrNoRows { return nil, errors.Wrap(err, "listing hosts in pack") } return hosts, nil } -func (d *Datastore) ListExplicitHostsInPack(pid uint, opt kolide.ListOptions) ([]*kolide.Host, error) { +func (d *Datastore) ListExplicitHostsInPack(pid uint, opt kolide.ListOptions) ([]uint, error) { query := ` - SELECT DISTINCT h.* + SELECT DISTINCT h.id FROM hosts h JOIN pack_targets pt ON ( @@ -249,7 +249,7 @@ func (d *Datastore) ListExplicitHostsInPack(pid uint, opt kolide.ListOptions) ([ ) WHERE pt.pack_id = ? ` - hosts := []*kolide.Host{} + hosts := []uint{} if err := d.db.Select(&hosts, appendListOptionsToSQL(query, opt), kolide.TargetHost, pid); err != nil && err != sql.ErrNoRows { return nil, errors.Wrap(err, "listing explicit hosts in pack") } diff --git a/server/kolide/packs.go b/server/kolide/packs.go index 9450e764eb..8d41b361a0 100644 --- a/server/kolide/packs.go +++ b/server/kolide/packs.go @@ -41,13 +41,13 @@ type PackStore interface { // an existing pack, both by ID. RemoveHostFromPack(hid uint, pid uint) error - // ListHostsInPack lists all hosts that are associated with a pack, both - // through labels and manual associations. - ListHostsInPack(pid uint, opt ListOptions) ([]*Host, error) + // ListHostsInPack lists the IDs of all hosts that are associated with a pack, + // both through labels and manual associations. + ListHostsInPack(pid uint, opt ListOptions) ([]uint, error) - // ListExplicitHostsInPack lists hosts that have been manually associated - // with a query pack. - ListExplicitHostsInPack(pid uint, opt ListOptions) ([]*Host, error) + // ListExplicitHostsInPack lists the IDs of hosts that have been manually + // associated with a query pack. + ListExplicitHostsInPack(pid uint, opt ListOptions) ([]uint, error) } // PackService is the service interface for managing query packs. @@ -87,13 +87,13 @@ type PackService interface { // ListPacksForHost lists the packs that a host should execute. ListPacksForHost(ctx context.Context, hid uint) (packs []*Pack, err error) - // ListHostsInPack lists all hosts that are associated with a pack, both - // through labels and manual associations. - ListHostsInPack(ctx context.Context, pid uint, opt ListOptions) (hosts []*Host, err error) + // ListHostsInPack lists the IDs of all hosts that are associated with a pack, + // both through labels and manual associations. + ListHostsInPack(ctx context.Context, pid uint, opt ListOptions) (hosts []uint, err error) - // ListExplicitHostsInPack lists hosts that have been manually associated + // ListExplicitHostsInPack lists the IDs of hosts that have been manually associated // with a query pack. - ListExplicitHostsInPack(ctx context.Context, pid uint, opt ListOptions) (hosts []*Host, err error) + ListExplicitHostsInPack(ctx context.Context, pid uint, opt ListOptions) (hosts []uint, err error) } // Pack is the structure which represents an osquery query pack. diff --git a/server/service/endpoint_packs.go b/server/service/endpoint_packs.go index ba2b04c709..c5adb149ee 100644 --- a/server/service/endpoint_packs.go +++ b/server/service/endpoint_packs.go @@ -39,10 +39,6 @@ func packResponseForPack(ctx context.Context, svc kolide.Service, pack kolide.Pa if err != nil { return nil, err } - hostIDs := make([]uint, len(hosts), len(hosts)) - for i, host := range hosts { - hostIDs[i] = host.ID - } labels, err := svc.ListLabelsForPack(ctx, pack.ID) labelIDs := make([]uint, len(labels), len(labels)) for i, label := range labels { @@ -55,7 +51,7 @@ func packResponseForPack(ctx context.Context, svc kolide.Service, pack kolide.Pa Pack: pack, QueryCount: uint(len(queries)), TotalHostsCount: totalHostCount, - HostIDs: hostIDs, + HostIDs: hosts, LabelIDs: labelIDs, }, nil } diff --git a/server/service/logging_packs.go b/server/service/logging_packs.go index 1ccb6da504..30e8ab04a4 100644 --- a/server/service/logging_packs.go +++ b/server/service/logging_packs.go @@ -200,9 +200,9 @@ func (mw loggingMiddleware) ListPacksForHost(ctx context.Context, hid uint) ([]* return packs, err } -func (mw loggingMiddleware) ListHostsInPack(ctx context.Context, pid uint, opt kolide.ListOptions) ([]*kolide.Host, error) { +func (mw loggingMiddleware) ListHostsInPack(ctx context.Context, pid uint, opt kolide.ListOptions) ([]uint, error) { var ( - hosts []*kolide.Host + hosts []uint err error ) diff --git a/server/service/service_packs.go b/server/service/service_packs.go index 0a4ba6f7e1..c6e4328991 100644 --- a/server/service/service_packs.go +++ b/server/service/service_packs.go @@ -108,7 +108,7 @@ func (svc service) ModifyPack(ctx context.Context, id uint, p kolide.PackPayload // lookups to determine whether or not a host is already added existingHosts := map[uint]bool{} for _, host := range hosts { - existingHosts[host.ID] = true + existingHosts[host] = true } // we will also make a constant time lookup map for the desired set of @@ -216,11 +216,11 @@ func (svc service) RemoveHostFromPack(ctx context.Context, hid, pid uint) error return svc.ds.RemoveHostFromPack(hid, pid) } -func (svc service) ListHostsInPack(ctx context.Context, pid uint, opt kolide.ListOptions) ([]*kolide.Host, error) { +func (svc service) ListHostsInPack(ctx context.Context, pid uint, opt kolide.ListOptions) ([]uint, error) { return svc.ds.ListHostsInPack(pid, opt) } -func (svc service) ListExplicitHostsInPack(ctx context.Context, pid uint, opt kolide.ListOptions) ([]*kolide.Host, error) { +func (svc service) ListExplicitHostsInPack(ctx context.Context, pid uint, opt kolide.ListOptions) ([]uint, error) { return svc.ds.ListExplicitHostsInPack(pid, opt) } @@ -278,7 +278,7 @@ func (svc service) ListPacksForHost(ctx context.Context, hid uint) ([]*kolide.Pa // o(n) iteration to determine whether or not a pack is enabled // in this case, n is len(hostsForPack) for _, host := range hostsForPack { - if host.ID == hid { + if host == hid { packs = append(packs, pack) break }