diff --git a/server/datastore/datastore_packs_test.go b/server/datastore/datastore_packs_test.go index 49b0d34039..44d0389db9 100644 --- a/server/datastore/datastore_packs_test.go +++ b/server/datastore/datastore_packs_test.go @@ -105,3 +105,22 @@ func testGetHostsInPack(t *testing.T, ds kolide.Datastore) { require.Nil(t, err) require.Len(t, hostsInPack, 2) } + +func testAddLabelToPackTwice(t *testing.T, ds kolide.Datastore) { + l1 := test.NewLabel(t, ds, "l1", "select 1;") + p1 := test.NewPack(t, ds, "p1") + + err := ds.AddLabelToPack(l1.ID, p1.ID) + assert.Nil(t, err) + + labels, err := ds.ListLabelsForPack(p1.ID) + assert.Nil(t, err) + assert.Len(t, labels, 1) + + err = ds.AddLabelToPack(l1.ID, p1.ID) + assert.Nil(t, err) + + labels, err = ds.ListLabelsForPack(p1.ID) + assert.Nil(t, err) + assert.Len(t, labels, 1) +} diff --git a/server/datastore/datastore_test.go b/server/datastore/datastore_test.go index cd5840992c..f5825142e4 100644 --- a/server/datastore/datastore_test.go +++ b/server/datastore/datastore_test.go @@ -59,4 +59,5 @@ var testFunctions = [...]func(*testing.T, kolide.Datastore){ testOptions, testNewScheduledQuery, testOptionsToConfig, + testAddLabelToPackTwice, } diff --git a/server/datastore/inmem/packs.go b/server/datastore/inmem/packs.go index 998601e71d..c85c0d358f 100644 --- a/server/datastore/inmem/packs.go +++ b/server/datastore/inmem/packs.go @@ -16,9 +16,9 @@ func (d *Datastore) NewPack(pack *kolide.Pack) (*kolide.Pack, error) { } d.mtx.Lock() + defer d.mtx.Unlock() newPack.ID = d.nextID(pack) d.packs[newPack.ID] = &newPack - d.mtx.Unlock() pack.ID = newPack.ID @@ -26,33 +26,36 @@ func (d *Datastore) NewPack(pack *kolide.Pack) (*kolide.Pack, error) { } func (d *Datastore) SavePack(pack *kolide.Pack) error { + d.mtx.Lock() + defer d.mtx.Unlock() + if _, ok := d.packs[pack.ID]; !ok { return notFound("Pack").WithID(pack.ID) } - d.mtx.Lock() d.packs[pack.ID] = pack - d.mtx.Unlock() return nil } func (d *Datastore) DeletePack(pid uint) error { + d.mtx.Lock() + defer d.mtx.Unlock() + if _, ok := d.packs[pid]; !ok { return notFound("Pack").WithID(pid) } - d.mtx.Lock() delete(d.packs, pid) - d.mtx.Unlock() return nil } func (d *Datastore) Pack(id uint) (*kolide.Pack, error) { d.mtx.Lock() + defer d.mtx.Unlock() + pack, ok := d.packs[id] - d.mtx.Unlock() if !ok { return nil, notFound("Pack").WithID(id) } @@ -61,9 +64,11 @@ func (d *Datastore) Pack(id uint) (*kolide.Pack, error) { } func (d *Datastore) ListPacks(opt kolide.ListOptions) ([]*kolide.Pack, error) { + d.mtx.Lock() + defer d.mtx.Unlock() + // We need to sort by keys to provide reliable ordering keys := []int{} - d.mtx.Lock() for k, _ := range d.packs { keys = append(keys, int(k)) } @@ -73,7 +78,6 @@ func (d *Datastore) ListPacks(opt kolide.ListOptions) ([]*kolide.Pack, error) { for _, k := range keys { packs = append(packs, d.packs[uint(k)]) } - d.mtx.Unlock() // Apply ordering if opt.OrderKey != "" { @@ -96,7 +100,15 @@ func (d *Datastore) ListPacks(opt kolide.ListOptions) ([]*kolide.Pack, error) { return packs, nil } -func (d *Datastore) AddLabelToPack(lid uint, pid uint) error { +func (d *Datastore) AddLabelToPack(lid, pid uint) error { + d.mtx.Lock() + defer d.mtx.Unlock() + + for _, pt := range d.packTargets { + if pt.PackID == pid && pt.Target.Type == kolide.TargetLabel && pt.Target.TargetID == lid { + return nil + } + } pt := &kolide.PackTarget{ PackID: pid, Target: kolide.Target{ @@ -104,35 +116,56 @@ func (d *Datastore) AddLabelToPack(lid uint, pid uint) error { TargetID: lid, }, } - - d.mtx.Lock() pt.ID = d.nextID(pt) d.packTargets[pt.ID] = pt - d.mtx.Unlock() + + return nil +} + +func (d *Datastore) AddHostToPack(hid, pid uint) error { + d.mtx.Lock() + defer d.mtx.Unlock() + + for _, pt := range d.packTargets { + if pt.PackID == pid && pt.Target.Type == kolide.TargetHost && pt.Target.TargetID == hid { + d.mtx.Unlock() + return nil + } + } + pt := &kolide.PackTarget{ + PackID: pid, + Target: kolide.Target{ + Type: kolide.TargetHost, + TargetID: hid, + }, + } + pt.ID = d.nextID(pt) + d.packTargets[pt.ID] = pt return nil } func (d *Datastore) ListLabelsForPack(pid uint) ([]*kolide.Label, error) { - var labels []*kolide.Label - d.mtx.Lock() + defer d.mtx.Unlock() + var labels []*kolide.Label for _, pt := range d.packTargets { if pt.Type == kolide.TargetLabel && pt.PackID == pid { labels = append(labels, d.labels[pt.TargetID]) } } - d.mtx.Unlock() return labels, nil } -func (d *Datastore) RemoveLabelFromPack(label *kolide.Label, pack *kolide.Pack) error { +func (d *Datastore) RemoveLabelFromPack(lid, pid uint) error { + d.mtx.Lock() + defer d.mtx.Unlock() + var labelsToDelete []uint - d.mtx.Lock() for _, pt := range d.packTargets { - if pt.Type == kolide.TargetLabel && pt.TargetID == label.ID && pt.PackID == pack.ID { + if pt.Type == kolide.TargetLabel && pt.TargetID == lid && pt.PackID == pid { labelsToDelete = append(labelsToDelete, pt.ID) } } @@ -140,16 +173,36 @@ func (d *Datastore) RemoveLabelFromPack(label *kolide.Label, pack *kolide.Pack) for _, id := range labelsToDelete { delete(d.packTargets, id) } + + return nil +} + +func (d *Datastore) RemoveHostFromPack(hid, pid uint) error { + d.mtx.Lock() d.mtx.Unlock() + var hostsToDelete []uint + + for _, pt := range d.packTargets { + if pt.Type == kolide.TargetHost && pt.TargetID == hid && pt.PackID == pid { + hostsToDelete = append(hostsToDelete, pt.ID) + } + } + + for _, id := range hostsToDelete { + delete(d.packTargets, id) + } + return nil } func (d *Datastore) ListHostsInPack(pid uint, opt kolide.ListOptions) ([]*kolide.Host, error) { + d.mtx.Lock() + defer d.mtx.Unlock() + hosts := []*kolide.Host{} hostLookup := map[uint]bool{} - d.mtx.Lock() for _, pt := range d.packTargets { if pt.PackID != pid { continue @@ -170,7 +223,6 @@ func (d *Datastore) ListHostsInPack(pid uint, opt kolide.ListOptions) ([]*kolide } } } - d.mtx.Unlock() // Apply ordering if opt.OrderKey != "" { diff --git a/server/datastore/mysql/migrations/20161230162221_AddUniqueIndexToPackTargets.go b/server/datastore/mysql/migrations/20161230162221_AddUniqueIndexToPackTargets.go new file mode 100644 index 0000000000..4c50934586 --- /dev/null +++ b/server/datastore/mysql/migrations/20161230162221_AddUniqueIndexToPackTargets.go @@ -0,0 +1,28 @@ +package migration + +import ( + "database/sql" + + "github.com/pressly/goose" +) + +func init() { + goose.AddMigration(Up_20161230162221, Down_20161230162221) +} + +func Up_20161230162221(tx *sql.Tx) error { + _, err := tx.Exec( + "ALTER TABLE `pack_targets` " + + "ADD CONSTRAINT `constraint_pack_target_unique` " + + "UNIQUE (`pack_id`, `target_id`, `type`);", + ) + return err +} + +func Down_20161230162221(tx *sql.Tx) error { + _, err := tx.Exec( + "ALTER TABLE `pack_targets` " + + "DROP INDEX `constraint_pack_target_unique`;", + ) + return err +} diff --git a/server/datastore/mysql/packs.go b/server/datastore/mysql/packs.go index 4901d60998..03822495d2 100644 --- a/server/datastore/mysql/packs.go +++ b/server/datastore/mysql/packs.go @@ -82,6 +82,7 @@ func (d *Datastore) AddLabelToPack(lid uint, pid uint) error { sql := ` INSERT INTO pack_targets ( pack_id, type, target_id ) VALUES ( ?, ?, ? ) + ON DUPLICATE KEY UPDATE id=id ` _, err := d.db.Exec(sql, pid, kolide.TargetLabel, lid) if err != nil { @@ -91,6 +92,21 @@ func (d *Datastore) AddLabelToPack(lid uint, pid uint) error { return nil } +// AddHostToPack associates a kolide.Host with a kolide.Pack +func (d *Datastore) AddHostToPack(hid, pid uint) error { + sql := ` + INSERT INTO pack_targets ( pack_id, type, target_id ) + VALUES ( ?, ?, ? ) + ON DUPLICATE KEY UPDATE id=id + ` + _, err := d.db.Exec(sql, pid, kolide.TargetHost, hid) + if err != nil { + return errors.DatabaseError(err) + } + + return nil +} + // ListLabelsForPack will return a list of kolide.Label records associated with kolide.Pack func (d *Datastore) ListLabelsForPack(pid uint) ([]*kolide.Label, error) { sql := ` @@ -123,18 +139,33 @@ func (d *Datastore) ListLabelsForPack(pid uint) ([]*kolide.Label, error) { // RemoreLabelFromPack will remove the association between a kolide.Label and // a kolide.Pack -func (d *Datastore) RemoveLabelFromPack(label *kolide.Label, pack *kolide.Pack) error { +func (d *Datastore) RemoveLabelFromPack(lid, pid uint) error { sql := ` - DELETE FROM pack_labels - WHERE target_id = ? AND pack_id = ? + DELETE FROM pack_targets + WHERE target_id = ? AND pack_id = ? AND type = ? ` - if _, err := d.db.Exec(sql, label.ID, pack.ID); err != nil { + if _, err := d.db.Exec(sql, lid, pid, kolide.TargetLabel); err != nil { return errors.DatabaseError(err) } return nil } +// RemoveHostFromPack will remove the association between a kolide.Host and a +// kolide.Pack +func (d *Datastore) RemoveHostFromPack(hid, pid uint) error { + sql := ` + DELETE FROM pack_targets + WHERE target_id = ? AND pack_id = ? AND type = ? + ` + if _, err := d.db.Exec(sql, hid, pid, kolide.TargetHost); err != nil { + return errors.DatabaseError(err) + } + + return nil + +} + func (d *Datastore) ListHostsInPack(pid uint, opt kolide.ListOptions) ([]*kolide.Host, error) { sql := ` SELECT DISTINCT h.* diff --git a/server/kolide/packs.go b/server/kolide/packs.go index 1ebf9b52bb..68cc0df433 100644 --- a/server/kolide/packs.go +++ b/server/kolide/packs.go @@ -13,24 +13,32 @@ type PackStore interface { ListPacks(opt ListOptions) ([]*Pack, error) // Modifying the labels for packs - AddLabelToPack(lid uint, pid uint) error + AddLabelToPack(lid, pid uint) error + RemoveLabelFromPack(lid, pid uint) error ListLabelsForPack(pid uint) ([]*Label, error) - RemoveLabelFromPack(label *Label, pack *Pack) error + // Modifying the hosts for packs + AddHostToPack(hid uint, pid uint) error + RemoveHostFromPack(hid uint, pid uint) error ListHostsInPack(pid uint, opt ListOptions) ([]*Host, error) } type PackService interface { + // Pack methods ListPacks(ctx context.Context, opt ListOptions) ([]*Pack, error) GetPack(ctx context.Context, id uint) (*Pack, error) NewPack(ctx context.Context, p PackPayload) (*Pack, error) ModifyPack(ctx context.Context, id uint, p PackPayload) (*Pack, error) DeletePack(ctx context.Context, id uint) error + // Modifying the labels for packs AddLabelToPack(ctx context.Context, lid, pid uint) error - ListLabelsForPack(ctx context.Context, pid uint) ([]*Label, error) RemoveLabelFromPack(ctx context.Context, lid, pid uint) error + ListLabelsForPack(ctx context.Context, pid uint) ([]*Label, error) + // Modifying the hosts for packs + AddHostToPack(ctx context.Context, hid, pid uint) error + RemoveHostFromPack(ctx context.Context, hid, pid uint) error ListPacksForHost(ctx context.Context, hid uint) ([]*Pack, error) ListHostsInPack(ctx context.Context, pid uint, opt ListOptions) ([]*Host, error) } @@ -47,10 +55,12 @@ type Pack struct { } type PackPayload struct { - Name *string - Description *string - Platform *string - Disabled *bool + Name *string `json:"name"` + Description *string `json:"description"` + Platform *string `json:"platform"` + Disabled *bool `json:"disabled"` + HostIDs *[]uint `json:"host_ids"` + LabelIDs *[]uint `json:"label_ids"` } type PackTarget struct { diff --git a/server/service/endpoint_packs.go b/server/service/endpoint_packs.go index e1056d9cc3..beeed019f6 100644 --- a/server/service/endpoint_packs.go +++ b/server/service/endpoint_packs.go @@ -6,6 +6,44 @@ import ( "golang.org/x/net/context" ) +type packResponse struct { + kolide.Pack + QueryCount uint `json:"query_count"` + TotalHostsCount uint `json:"total_hosts_count"` + HostIDs []uint `json:"host_ids"` + LabelIDs []uint `json:"label_ids"` +} + +func packResponseForPack(ctx context.Context, svc kolide.Service, pack kolide.Pack) (*packResponse, error) { + queries, err := svc.GetScheduledQueriesInPack(ctx, pack.ID, kolide.ListOptions{}) + if err != nil { + return nil, err + } + hosts, err := svc.ListHostsInPack(ctx, pack.ID, kolide.ListOptions{}) + if err != nil { + return nil, err + } + hostIDs := make([]uint, len(hosts), len(hosts)) + for _, host := range hosts { + hostIDs = append(hostIDs, host.ID) + } + labels, err := svc.ListLabelsForPack(ctx, pack.ID) + labelIDs := make([]uint, len(labels), len(labels)) + for _, label := range labels { + labelIDs = append(labelIDs, label.ID) + } + if err != nil { + return nil, err + } + return &packResponse{ + Pack: pack, + QueryCount: uint(len(queries)), + TotalHostsCount: uint(len(hosts)), + HostIDs: hostIDs, + LabelIDs: labelIDs, + }, nil +} + //////////////////////////////////////////////////////////////////////////////// // Get Pack //////////////////////////////////////////////////////////////////////////////// @@ -14,12 +52,6 @@ type getPackRequest struct { ID uint } -type packResponse struct { - kolide.Pack - QueryCount uint `json:"query_count"` - TotalHostsCount uint `json:"total_hosts_count"` -} - type getPackResponse struct { Pack packResponse `json:"pack,omitempty"` Err error `json:"error,omitempty"` @@ -36,22 +68,13 @@ func makeGetPackEndpoint(svc kolide.Service) endpoint.Endpoint { return getPackResponse{Err: err}, nil } - queries, err := svc.GetScheduledQueriesInPack(ctx, pack.ID, kolide.ListOptions{}) - if err != nil { - return getPackResponse{Err: err}, nil - } - - hosts, err := svc.ListHostsInPack(ctx, pack.ID, kolide.ListOptions{}) + resp, err := packResponseForPack(ctx, svc, *pack) if err != nil { return getPackResponse{Err: err}, nil } return getPackResponse{ - Pack: packResponse{ - Pack: *pack, - QueryCount: uint(len(queries)), - TotalHostsCount: uint(len(hosts)), - }, + Pack: *resp, }, nil } } @@ -79,21 +102,13 @@ func makeListPacksEndpoint(svc kolide.Service) endpoint.Endpoint { return getPackResponse{Err: err}, nil } - resp := listPacksResponse{Packs: []packResponse{}} + resp := listPacksResponse{Packs: make([]packResponse, len(packs), len(packs))} for _, pack := range packs { - queries, err := svc.GetScheduledQueriesInPack(ctx, pack.ID, kolide.ListOptions{}) + packResp, err := packResponseForPack(ctx, svc, *pack) if err != nil { return getPackResponse{Err: err}, nil } - hosts, err := svc.ListHostsInPack(ctx, pack.ID, kolide.ListOptions{}) - if err != nil { - return getPackResponse{Err: err}, nil - } - resp.Packs = append(resp.Packs, packResponse{ - Pack: *pack, - QueryCount: uint(len(queries)), - TotalHostsCount: uint(len(hosts)), - }) + resp.Packs = append(resp.Packs, *packResp) } return resp, nil } @@ -122,23 +137,13 @@ func makeCreatePackEndpoint(svc kolide.Service) endpoint.Endpoint { return createPackResponse{Err: err}, nil } - queries, err := svc.GetScheduledQueriesInPack(ctx, pack.ID, kolide.ListOptions{}) - if err != nil { - return createPackResponse{Err: err}, nil - } - - hosts, err := svc.ListHostsInPack(ctx, pack.ID, kolide.ListOptions{}) + resp, err := packResponseForPack(ctx, svc, *pack) if err != nil { return createPackResponse{Err: err}, nil } return createPackResponse{ - Pack: packResponse{ - Pack: *pack, - QueryCount: uint(len(queries)), - TotalHostsCount: uint(len(hosts)), - }, - Err: nil, + Pack: *resp, }, nil } } @@ -167,23 +172,13 @@ func makeModifyPackEndpoint(svc kolide.Service) endpoint.Endpoint { return modifyPackResponse{Err: err}, nil } - queries, err := svc.GetScheduledQueriesInPack(ctx, pack.ID, kolide.ListOptions{}) - if err != nil { - return modifyPackResponse{Err: err}, nil - } - - hosts, err := svc.ListHostsInPack(ctx, pack.ID, kolide.ListOptions{}) + resp, err := packResponseForPack(ctx, svc, *pack) if err != nil { return modifyPackResponse{Err: err}, nil } return modifyPackResponse{ - Pack: packResponse{ - Pack: *pack, - QueryCount: uint(len(queries)), - TotalHostsCount: uint(len(hosts)), - }, - Err: nil, + Pack: *resp, }, nil } } @@ -212,86 +207,3 @@ func makeDeletePackEndpoint(svc kolide.Service) endpoint.Endpoint { return deletePackResponse{}, nil } } - -//////////////////////////////////////////////////////////////////////////////// -// Add Label To Pack -//////////////////////////////////////////////////////////////////////////////// - -type addLabelToPackRequest struct { - PackID uint - LabelID uint -} - -type addLabelToPackResponse struct { - Err error `json:"error,omitempty"` -} - -func (r addLabelToPackResponse) error() error { return r.Err } - -func makeAddLabelToPackEndpoint(svc kolide.Service) endpoint.Endpoint { - return func(ctx context.Context, request interface{}) (interface{}, error) { - req := request.(addLabelToPackRequest) - err := svc.AddLabelToPack(ctx, req.LabelID, req.PackID) - if err != nil { - return addLabelToPackResponse{Err: err}, nil - } - return addLabelToPackResponse{}, nil - } -} - -//////////////////////////////////////////////////////////////////////////////// -// Get Labels For Pack -//////////////////////////////////////////////////////////////////////////////// - -type getLabelsForPackRequest struct { - PackID uint -} - -type getLabelsForPackResponse struct { - Labels []kolide.Label `json:"labels"` - Err error `json:"error,omitempty"` -} - -func (r getLabelsForPackResponse) error() error { return r.Err } - -func makeGetLabelsForPackEndpoint(svc kolide.Service) endpoint.Endpoint { - return func(ctx context.Context, request interface{}) (interface{}, error) { - req := request.(getLabelsForPackRequest) - labels, err := svc.ListLabelsForPack(ctx, req.PackID) - if err != nil { - return getLabelsForPackResponse{Err: err}, nil - } - - var resp getLabelsForPackResponse - for _, label := range labels { - resp.Labels = append(resp.Labels, *label) - } - return resp, nil - } -} - -//////////////////////////////////////////////////////////////////////////////// -// Delete Label From Pack -//////////////////////////////////////////////////////////////////////////////// - -type deleteLabelFromPackRequest struct { - LabelID uint - PackID uint -} - -type deleteLabelFromPackResponse struct { - Err error `json:"error,omitempty"` -} - -func (r deleteLabelFromPackResponse) error() error { return r.Err } - -func makeDeleteLabelFromPackEndpoint(svc kolide.Service) endpoint.Endpoint { - return func(ctx context.Context, request interface{}) (interface{}, error) { - req := request.(deleteLabelFromPackRequest) - err := svc.RemoveLabelFromPack(ctx, req.LabelID, req.PackID) - if err != nil { - return deleteLabelFromPackResponse{Err: err}, nil - } - return deleteLabelFromPackResponse{}, nil - } -} diff --git a/server/service/handler.go b/server/service/handler.go index 94d5b6a30c..3b54d26e27 100644 --- a/server/service/handler.go +++ b/server/service/handler.go @@ -60,9 +60,6 @@ type KolideEndpoints struct { ListLabels endpoint.Endpoint CreateLabel endpoint.Endpoint DeleteLabel endpoint.Endpoint - AddLabelToPack endpoint.Endpoint - GetLabelsForPack endpoint.Endpoint - DeleteLabelFromPack endpoint.Endpoint GetHost endpoint.Endpoint DeleteHost endpoint.Endpoint ListHosts endpoint.Endpoint @@ -126,9 +123,6 @@ func MakeKolideServerEndpoints(svc kolide.Service, jwtKey string) KolideEndpoint ListLabels: authenticatedUser(jwtKey, svc, makeListLabelsEndpoint(svc)), CreateLabel: authenticatedUser(jwtKey, svc, makeCreateLabelEndpoint(svc)), DeleteLabel: authenticatedUser(jwtKey, svc, makeDeleteLabelEndpoint(svc)), - AddLabelToPack: authenticatedUser(jwtKey, svc, makeAddLabelToPackEndpoint(svc)), - GetLabelsForPack: authenticatedUser(jwtKey, svc, makeGetLabelsForPackEndpoint(svc)), - DeleteLabelFromPack: authenticatedUser(jwtKey, svc, makeDeleteLabelFromPackEndpoint(svc)), SearchTargets: authenticatedUser(jwtKey, svc, makeSearchTargetsEndpoint(svc)), GetOptions: authenticatedUser(jwtKey, svc, mustBeAdmin(makeGetOptionsEndpoint(svc))), ModifyOptions: authenticatedUser(jwtKey, svc, mustBeAdmin(makeModifyOptionsEndpoint(svc))), @@ -189,9 +183,6 @@ type kolideHandlers struct { ListLabels http.Handler CreateLabel http.Handler DeleteLabel http.Handler - AddLabelToPack http.Handler - GetLabelsForPack http.Handler - DeleteLabelFromPack http.Handler GetHost http.Handler DeleteHost http.Handler ListHosts http.Handler @@ -251,9 +242,6 @@ func makeKolideKitHandlers(ctx context.Context, e KolideEndpoints, opts []kithtt ListLabels: newServer(e.ListLabels, decodeListLabelsRequest), CreateLabel: newServer(e.CreateLabel, decodeCreateLabelRequest), DeleteLabel: newServer(e.DeleteLabel, decodeDeleteLabelRequest), - AddLabelToPack: newServer(e.AddLabelToPack, decodeAddLabelToPackRequest), - GetLabelsForPack: newServer(e.GetLabelsForPack, decodeGetLabelsForPackRequest), - DeleteLabelFromPack: newServer(e.DeleteLabelFromPack, decodeDeleteLabelFromPackRequest), GetHost: newServer(e.GetHost, decodeGetHostRequest), DeleteHost: newServer(e.DeleteHost, decodeDeleteHostRequest), ListHosts: newServer(e.ListHosts, decodeListHostsRequest), @@ -347,9 +335,6 @@ func attachKolideAPIRoutes(r *mux.Router, h *kolideHandlers) { r.Handle("/api/v1/kolide/labels", h.ListLabels).Methods("GET").Name("list_labels") r.Handle("/api/v1/kolide/labels", h.CreateLabel).Methods("POST").Name("create_label") r.Handle("/api/v1/kolide/labels/{id}", h.DeleteLabel).Methods("DELETE").Name("delete_label") - r.Handle("/api/v1/kolide/packs/{pid}/labels/{lid}", h.AddLabelToPack).Methods("POST").Name("add_label_to_pack") - r.Handle("/api/v1/kolide/packs/{pid}/labels", h.GetLabelsForPack).Methods("GET").Name("get_labels_for_pack") - r.Handle("/api/v1/kolide/packs/{pid}/labels/{lid}", h.DeleteLabelFromPack).Methods("DELETE").Name("delete_label_from_pack") r.Handle("/api/v1/kolide/hosts", h.ListHosts).Methods("GET").Name("list_hosts") r.Handle("/api/v1/kolide/hosts/{id}", h.GetHost).Methods("GET").Name("get_host") diff --git a/server/service/handler_test.go b/server/service/handler_test.go index f0dd9c4750..32a95aa216 100644 --- a/server/service/handler_test.go +++ b/server/service/handler_test.go @@ -184,18 +184,6 @@ func TestAPIRoutes(t *testing.T) { verb: "DELETE", uri: "/api/v1/kolide/labels/1", }, - { - verb: "POST", - uri: "/api/v1/kolide/packs/1/labels/2", - }, - { - verb: "GET", - uri: "/api/v1/kolide/packs/1/labels", - }, - { - verb: "DELETE", - uri: "/api/v1/kolide/packs/1/labels/2", - }, { verb: "GET", uri: "/api/v1/kolide/hosts/1", diff --git a/server/service/service_packs.go b/server/service/service_packs.go index 10aeb1d783..4efc564b6e 100644 --- a/server/service/service_packs.go +++ b/server/service/service_packs.go @@ -44,6 +44,25 @@ func (svc service) NewPack(ctx context.Context, p kolide.PackPayload) (*kolide.P if err != nil { return nil, err } + + if p.HostIDs != nil { + for _, hostID := range *p.HostIDs { + err = svc.AddHostToPack(ctx, hostID, pack.ID) + if err != nil { + return nil, err + } + } + } + + if p.LabelIDs != nil { + for _, labelID := range *p.LabelIDs { + err = svc.AddLabelToPack(ctx, labelID, pack.ID) + if err != nil { + return nil, err + } + } + } + return &pack, nil } @@ -74,6 +93,102 @@ func (svc service) ModifyPack(ctx context.Context, id uint, p kolide.PackPayload return nil, err } + // we must determine what hosts are attached to this pack. then, given + // our new set of host_ids, we will mutate the database to reflect the + // desired state. + if p.HostIDs != nil { + + // first, let's retrieve the total set of hosts + hosts, err := svc.ListHostsInPack(ctx, pack.ID, kolide.ListOptions{}) + if err != nil { + return nil, err + } + + // it will be efficient to create a data structure with constant time + // lookups to determine whether or not a host is already added + existingHosts := map[uint]bool{} + for _, host := range hosts { + existingHosts[host.ID] = true + } + + // we will also make a constant time lookup map for the desired set of + // hosts as well. + desiredHosts := map[uint]bool{} + for _, hostID := range *p.HostIDs { + desiredHosts[hostID] = true + } + + // if the request declares a host ID but the host is not already + // associated with the pack, we add it + for _, hostID := range *p.HostIDs { + if !existingHosts[hostID] { + err = svc.AddHostToPack(ctx, hostID, pack.ID) + if err != nil { + return nil, err + } + } + } + + // if the request does not declare the ID of a host which currently + // exists, we delete the existing relationship + for hostID := range existingHosts { + if !desiredHosts[hostID] { + err = svc.RemoveHostFromPack(ctx, hostID, pack.ID) + if err != nil { + return nil, err + } + } + } + } + + // we must determine what labels are attached to this pack. then, given + // our new set of label_ids, we will mutate the database to reflect the + // desired state. + if p.LabelIDs != nil { + + // first, let's retrieve the total set of labels + labels, err := svc.ListLabelsForPack(ctx, pack.ID) + if err != nil { + return nil, err + } + + // it will be efficient to create a data structure with constant time + // lookups to determine whether or not a label is already added + existingLabels := map[uint]bool{} + for _, label := range labels { + existingLabels[label.ID] = true + } + + // we will also make a constant time lookup map for the desired set of + // labels as well. + desiredLabels := map[uint]bool{} + for _, labelID := range *p.LabelIDs { + desiredLabels[labelID] = true + } + + // if the request declares a label ID but the label is not already + // associated with the pack, we add it + for _, labelID := range *p.LabelIDs { + if !existingLabels[labelID] { + err = svc.AddLabelToPack(ctx, labelID, pack.ID) + if err != nil { + return nil, err + } + } + } + + // if the request does not declare the ID of a label which currently + // exists, we delete the existing relationship + for labelID := range existingLabels { + if !desiredLabels[labelID] { + err = svc.RemoveLabelFromPack(ctx, labelID, pack.ID) + if err != nil { + return nil, err + } + } + } + } + return pack, err } @@ -86,31 +201,19 @@ func (svc service) AddLabelToPack(ctx context.Context, lid, pid uint) error { } func (svc service) ListLabelsForPack(ctx context.Context, pid uint) ([]*kolide.Label, error) { - labels, err := svc.ds.ListLabelsForPack(pid) - if err != nil { - return nil, err - } - - return labels, nil + return svc.ds.ListLabelsForPack(pid) } func (svc service) RemoveLabelFromPack(ctx context.Context, lid, pid uint) error { - pack, err := svc.ds.Pack(pid) - if err != nil { - return err - } + return svc.ds.RemoveLabelFromPack(lid, pid) +} - label, err := svc.ds.Label(lid) - if err != nil { - return err - } +func (svc service) AddHostToPack(ctx context.Context, hid, pid uint) error { + return svc.ds.AddHostToPack(hid, pid) +} - err = svc.ds.RemoveLabelFromPack(label, pack) - if err != nil { - return err - } - - return nil +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) { diff --git a/server/service/service_packs_test.go b/server/service/service_packs_test.go index 0c9b5724e2..483396f14c 100644 --- a/server/service/service_packs_test.go +++ b/server/service/service_packs_test.go @@ -7,6 +7,7 @@ import ( "github.com/kolide/kolide-ose/server/datastore/inmem" "github.com/kolide/kolide-ose/server/kolide" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "golang.org/x/net/context" ) @@ -64,16 +65,31 @@ func TestNewPack(t *testing.T) { ctx := context.Background() - name := "foo" - _, err = svc.NewPack(ctx, kolide.PackPayload{ - Name: &name, + labelName := "label" + labelQuery := "select 1" + label, err := svc.NewLabel(ctx, kolide.LabelPayload{ + Name: &labelName, + Query: &labelQuery, + }) + + packName := "foo" + packLabelIDs := []uint{label.ID} + pack, err := svc.NewPack(ctx, kolide.PackPayload{ + Name: &packName, + LabelIDs: &packLabelIDs, }) assert.Nil(t, err) - queries, err := ds.ListPacks(kolide.ListOptions{}) + packs, err := ds.ListPacks(kolide.ListOptions{}) assert.Nil(t, err) - assert.Len(t, queries, 1) + require.Len(t, packs, 1) + assert.Equal(t, pack.ID, packs[0].ID) + + labels, err := ds.ListLabelsForPack(pack.ID) + assert.Nil(t, err) + require.Len(t, labels, 1) + assert.Equal(t, label.ID, labels[0].ID) } func TestModifyPack(t *testing.T) { @@ -85,21 +101,48 @@ func TestModifyPack(t *testing.T) { ctx := context.Background() + label := &kolide.Label{ + Name: "label", + Query: "select 1", + } + label, err = ds.NewLabel(label) + assert.Nil(t, err) + assert.NotZero(t, label.ID) + pack := &kolide.Pack{ Name: "foo", } - _, err = ds.NewPack(pack) + pack, err = ds.NewPack(pack) assert.Nil(t, err) assert.NotZero(t, pack.ID) newName := "bar" + labelIDs := []uint{label.ID} packVerify, err := svc.ModifyPack(ctx, pack.ID, kolide.PackPayload{ - Name: &newName, + Name: &newName, + LabelIDs: &labelIDs, }) assert.Nil(t, err) assert.Equal(t, pack.ID, packVerify.ID) assert.Equal(t, "bar", packVerify.Name) + + labels, err := ds.ListLabelsForPack(pack.ID) + assert.Nil(t, err) + require.Len(t, labels, 1) + assert.Equal(t, label.ID, labels[0].ID) + + newLabelIDs := []uint{} + packVerify2, err := svc.ModifyPack(ctx, pack.ID, kolide.PackPayload{ + LabelIDs: &newLabelIDs, + }) + assert.Nil(t, err) + + assert.Equal(t, pack.ID, packVerify2.ID) + + labels, err = ds.ListLabelsForPack(pack.ID) + assert.Nil(t, err) + require.Len(t, labels, 0) } func TestDeletePack(t *testing.T) { @@ -124,5 +167,4 @@ func TestDeletePack(t *testing.T) { queries, err := ds.ListPacks(kolide.ListOptions{}) assert.Nil(t, err) assert.Len(t, queries, 0) - } diff --git a/server/service/transport_packs.go b/server/service/transport_packs.go index b9ef23b87a..832162de2e 100644 --- a/server/service/transport_packs.go +++ b/server/service/transport_packs.go @@ -56,43 +56,3 @@ func decodeListPacksRequest(ctx context.Context, r *http.Request) (interface{}, } return listPacksRequest{ListOptions: opt}, nil } - -func decodeAddLabelToPackRequest(ctx context.Context, r *http.Request) (interface{}, error) { - lid, err := idFromRequest(r, "lid") - if err != nil { - return nil, err - } - pid, err := idFromRequest(r, "pid") - if err != nil { - return nil, err - } - return addLabelToPackRequest{ - PackID: pid, - LabelID: lid, - }, nil -} - -func decodeGetLabelsForPackRequest(ctx context.Context, r *http.Request) (interface{}, error) { - pid, err := idFromRequest(r, "pid") - if err != nil { - return nil, err - } - var req getLabelsForPackRequest - req.PackID = pid - return req, nil -} - -func decodeDeleteLabelFromPackRequest(ctx context.Context, r *http.Request) (interface{}, error) { - lid, err := idFromRequest(r, "lid") - if err != nil { - return nil, err - } - pid, err := idFromRequest(r, "pid") - if err != nil { - return nil, err - } - var req deleteLabelFromPackRequest - req.PackID = pid - req.LabelID = lid - return req, nil -} diff --git a/server/service/transport_packs_test.go b/server/service/transport_packs_test.go index a00295a3df..c3169eef2e 100644 --- a/server/service/transport_packs_test.go +++ b/server/service/transport_packs_test.go @@ -8,6 +8,7 @@ import ( "github.com/gorilla/mux" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "golang.org/x/net/context" ) @@ -20,12 +21,18 @@ func TestDecodeCreatePackRequest(t *testing.T) { params := r.(createPackRequest) assert.Equal(t, "foo", *params.payload.Name) assert.Equal(t, "bar", *params.payload.Description) + require.NotNil(t, params.payload.HostIDs) + assert.Len(t, *params.payload.HostIDs, 3) + require.NotNil(t, params.payload.LabelIDs) + assert.Len(t, *params.payload.LabelIDs, 2) }).Methods("POST") var body bytes.Buffer body.Write([]byte(`{ "name": "foo", - "description": "bar" + "description": "bar", + "host_ids": [1, 2, 3], + "label_ids": [1, 5] }`)) router.ServeHTTP( @@ -41,15 +48,21 @@ func TestDecodeModifyPackRequest(t *testing.T) { assert.Nil(t, err) params := r.(modifyPackRequest) + assert.Equal(t, uint(1), params.ID) assert.Equal(t, "foo", *params.payload.Name) assert.Equal(t, "bar", *params.payload.Description) - assert.Equal(t, uint(1), params.ID) + require.NotNil(t, params.payload.HostIDs) + assert.Len(t, *params.payload.HostIDs, 3) + require.NotNil(t, params.payload.LabelIDs) + assert.Len(t, *params.payload.LabelIDs, 2) }).Methods("PATCH") var body bytes.Buffer body.Write([]byte(`{ "name": "foo", - "description": "bar" + "description": "bar", + "host_ids": [1, 2, 3], + "label_ids": [1, 5] }`)) router.ServeHTTP(