From 6dc56ad5f0ceccebdb61ee27b7e2192335d7a9e5 Mon Sep 17 00:00:00 2001 From: John Murphy Date: Sun, 12 Feb 2017 12:27:43 +0800 Subject: [PATCH] Add ability to modify labels (#1202) --- server/datastore/datastore_labels_test.go | 22 ++++ server/datastore/datastore_test.go | 1 + server/datastore/inmem/labels.go | 4 + server/datastore/mysql/labels.go | 14 +++ server/kolide/labels.go | 13 +++ server/mock/datastore.go | 3 +- server/mock/datastore_labels.go | 123 ++++++++++++++++++++++ server/service/endpoint_labels.go | 23 ++++ server/service/handler.go | 5 + server/service/logging_labels.go | 18 ++++ server/service/metrics_labels.go | 24 +++++ server/service/service_labels.go | 14 +++ server/service/service_labels_test.go | 28 +++++ server/service/transport_labels.go | 14 +++ 14 files changed, 305 insertions(+), 1 deletion(-) create mode 100644 server/mock/datastore_labels.go create mode 100644 server/service/metrics_labels.go diff --git a/server/datastore/datastore_labels_test.go b/server/datastore/datastore_labels_test.go index 8d4ca3773..af0f544e0 100644 --- a/server/datastore/datastore_labels_test.go +++ b/server/datastore/datastore_labels_test.go @@ -355,3 +355,25 @@ func testListUniqueHostsInLabels(t *testing.T, db kolide.Datastore) { require.Len(t, labels, 2) } + +func testSaveLabel(t *testing.T, db kolide.Datastore) { + if db.Name() == "inmem" { + t.Skip("inmem is being deprecated") + } + label := &kolide.Label{ + Name: "my label", + Description: "a label", + Query: "select 1 from processes;", + Platform: "darwin", + } + label, err := db.NewLabel(label) + require.Nil(t, err) + label.Name = "changed name" + label.Description = "changed description" + _, err = db.SaveLabel(label) + require.Nil(t, err) + saved, err := db.Label(label.ID) + require.Nil(t, err) + assert.Equal(t, label.Name, saved.Name) + assert.Equal(t, label.Description, saved.Description) +} diff --git a/server/datastore/datastore_test.go b/server/datastore/datastore_test.go index c54b96574..1472dc782 100644 --- a/server/datastore/datastore_test.go +++ b/server/datastore/datastore_test.go @@ -71,4 +71,5 @@ var testFunctions = [...]func(*testing.T, kolide.Datastore){ testIdempotentDeleteHost, testChangeEmail, testLicense, + testSaveLabel, } diff --git a/server/datastore/inmem/labels.go b/server/datastore/inmem/labels.go index 51dec2c7a..071cddb0b 100644 --- a/server/datastore/inmem/labels.go +++ b/server/datastore/inmem/labels.go @@ -224,3 +224,7 @@ func (d *Datastore) ListUniqueHostsInLabels(labels []uint) ([]kolide.Host, error return hosts, nil } + +func (d *Datastore) SaveLabel(label *kolide.Label) (*kolide.Label, error) { + panic("inmem is being deprecated") +} diff --git a/server/datastore/mysql/labels.go b/server/datastore/mysql/labels.go index 81e0c40c1..7e32c0348 100644 --- a/server/datastore/mysql/labels.go +++ b/server/datastore/mysql/labels.go @@ -314,3 +314,17 @@ func (d *Datastore) SearchLabels(query string, omit ...uint) ([]kolide.Label, er } return matches, nil } + +func (d *Datastore) SaveLabel(label *kolide.Label) (*kolide.Label, error) { + query := ` + UPDATE labels SET + name = ?, + description = ? + WHERE id = ? + ` + _, err := d.db.Exec(query, label.Name, label.Description, label.ID) + if err != nil { + return nil, errors.Wrap(err, "saving label") + } + return label, nil +} diff --git a/server/kolide/labels.go b/server/kolide/labels.go index 3dec08c1e..a581f0444 100644 --- a/server/kolide/labels.go +++ b/server/kolide/labels.go @@ -33,6 +33,9 @@ type LabelStore interface { ListUniqueHostsInLabels(labels []uint) ([]Host, error) SearchLabels(query string, omit ...uint) ([]Label, error) + + // SaveLabel allows modification of a label's name and/or description + SaveLabel(label *Label) (*Label, error) } type LabelService interface { @@ -40,11 +43,21 @@ type LabelService interface { GetLabel(ctx context.Context, id uint) (label *Label, err error) NewLabel(ctx context.Context, p LabelPayload) (label *Label, err error) DeleteLabel(ctx context.Context, id uint) (err error) + + // ModifyLabel is used to change editable fields belonging to a Label + ModifyLabel(ctx context.Context, id uint, payload ModifyLabelPayload) (*Label, error) + // HostIDsForLabel returns ids of hosts that belong to the label identified // by lid HostIDsForLabel(lid uint) ([]uint, error) } +// ModifyLabelPayload is used to change editable fields for a Label +type ModifyLabelPayload struct { + Name *string `json:"name"` + Description *string `json:"description"` +} + type LabelPayload struct { Name *string `json:"name"` Query *string `json:"query"` diff --git a/server/mock/datastore.go b/server/mock/datastore.go index fa338aaa7..74a52ed4a 100644 --- a/server/mock/datastore.go +++ b/server/mock/datastore.go @@ -4,6 +4,7 @@ package mock //go:generate mockimpl -o datastore_invites.go "s *InviteStore" "kolide.InviteStore" //go:generate mockimpl -o datastore_appconfig.go "s *AppConfigStore" "kolide.AppConfigStore" //go:generate mockimpl -o datastore_licenses.go "s *LicenseStore" "kolide.LicenseStore" +//go:generate mockimpl -o datastore_labels.go "s *LabelStore" "kolide.LabelStore" import "github.com/kolide/kolide/server/kolide" @@ -11,7 +12,6 @@ var _ kolide.Datastore = (*Store)(nil) type Store struct { kolide.HostStore - kolide.LabelStore kolide.PackStore kolide.CampaignStore kolide.SessionStore @@ -26,6 +26,7 @@ type Store struct { InviteStore UserStore AppConfigStore + LabelStore } func (m *Store) Drop() error { diff --git a/server/mock/datastore_labels.go b/server/mock/datastore_labels.go new file mode 100644 index 000000000..48ad50ea6 --- /dev/null +++ b/server/mock/datastore_labels.go @@ -0,0 +1,123 @@ +// Automatically generated by mockimpl. DO NOT EDIT! + +package mock + +import ( + "time" + + "github.com/kolide/kolide/server/kolide" +) + +var _ kolide.LabelStore = (*LabelStore)(nil) + +type NewLabelFunc func(Label *kolide.Label) (*kolide.Label, error) + +type DeleteLabelFunc func(lid uint) error + +type LabelFunc func(lid uint) (*kolide.Label, error) + +type ListLabelsFunc func(opt kolide.ListOptions) ([]*kolide.Label, error) + +type LabelQueriesForHostFunc func(host *kolide.Host, cutoff time.Time) (map[string]string, error) + +type RecordLabelQueryExecutionsFunc func(host *kolide.Host, results map[uint]bool, t time.Time) error + +type ListLabelsForHostFunc func(hid uint) ([]kolide.Label, error) + +type ListHostsInLabelFunc func(lid uint) ([]kolide.Host, error) + +type ListUniqueHostsInLabelsFunc func(labels []uint) ([]kolide.Host, error) + +type SearchLabelsFunc func(query string, omit ...uint) ([]kolide.Label, error) + +type SaveLabelFunc func(label *kolide.Label) (*kolide.Label, error) + +type LabelStore struct { + NewLabelFunc NewLabelFunc + NewLabelFuncInvoked bool + + DeleteLabelFunc DeleteLabelFunc + DeleteLabelFuncInvoked bool + + LabelFunc LabelFunc + LabelFuncInvoked bool + + ListLabelsFunc ListLabelsFunc + ListLabelsFuncInvoked bool + + LabelQueriesForHostFunc LabelQueriesForHostFunc + LabelQueriesForHostFuncInvoked bool + + RecordLabelQueryExecutionsFunc RecordLabelQueryExecutionsFunc + RecordLabelQueryExecutionsFuncInvoked bool + + ListLabelsForHostFunc ListLabelsForHostFunc + ListLabelsForHostFuncInvoked bool + + ListHostsInLabelFunc ListHostsInLabelFunc + ListHostsInLabelFuncInvoked bool + + ListUniqueHostsInLabelsFunc ListUniqueHostsInLabelsFunc + ListUniqueHostsInLabelsFuncInvoked bool + + SearchLabelsFunc SearchLabelsFunc + SearchLabelsFuncInvoked bool + + SaveLabelFunc SaveLabelFunc + SaveLabelFuncInvoked bool +} + +func (s *LabelStore) NewLabel(Label *kolide.Label) (*kolide.Label, error) { + s.NewLabelFuncInvoked = true + return s.NewLabelFunc(Label) +} + +func (s *LabelStore) DeleteLabel(lid uint) error { + s.DeleteLabelFuncInvoked = true + return s.DeleteLabelFunc(lid) +} + +func (s *LabelStore) Label(lid uint) (*kolide.Label, error) { + s.LabelFuncInvoked = true + return s.LabelFunc(lid) +} + +func (s *LabelStore) ListLabels(opt kolide.ListOptions) ([]*kolide.Label, error) { + s.ListLabelsFuncInvoked = true + return s.ListLabelsFunc(opt) +} + +func (s *LabelStore) LabelQueriesForHost(host *kolide.Host, cutoff time.Time) (map[string]string, error) { + s.LabelQueriesForHostFuncInvoked = true + return s.LabelQueriesForHostFunc(host, cutoff) +} + +func (s *LabelStore) RecordLabelQueryExecutions(host *kolide.Host, results map[uint]bool, t time.Time) error { + s.RecordLabelQueryExecutionsFuncInvoked = true + return s.RecordLabelQueryExecutionsFunc(host, results, t) +} + +func (s *LabelStore) ListLabelsForHost(hid uint) ([]kolide.Label, error) { + s.ListLabelsForHostFuncInvoked = true + return s.ListLabelsForHostFunc(hid) +} + +func (s *LabelStore) ListHostsInLabel(lid uint) ([]kolide.Host, error) { + s.ListHostsInLabelFuncInvoked = true + return s.ListHostsInLabelFunc(lid) +} + +func (s *LabelStore) ListUniqueHostsInLabels(labels []uint) ([]kolide.Host, error) { + s.ListUniqueHostsInLabelsFuncInvoked = true + return s.ListUniqueHostsInLabelsFunc(labels) +} + +func (s *LabelStore) SearchLabels(query string, omit ...uint) ([]kolide.Label, error) { + s.SearchLabelsFuncInvoked = true + return s.SearchLabelsFunc(query, omit...) +} + +func (s *LabelStore) SaveLabel(label *kolide.Label) (*kolide.Label, error) { + s.SaveLabelFuncInvoked = true + return s.SaveLabelFunc(label) +} diff --git a/server/service/endpoint_labels.go b/server/service/endpoint_labels.go index ac84942e6..23024bbbb 100644 --- a/server/service/endpoint_labels.go +++ b/server/service/endpoint_labels.go @@ -151,3 +151,26 @@ func makeDeleteLabelEndpoint(svc kolide.Service) endpoint.Endpoint { return deleteLabelResponse{}, nil } } + +type modifyLabelRequest struct { + ID uint + payload kolide.ModifyLabelPayload +} + +type modifyLabelResponse struct { + Label *kolide.Label `json:"label"` + Err error `json:"error,omitempty"` +} + +func (r modifyLabelResponse) error() error { return r.Err } + +func makeModifyLabelEndpoint(svc kolide.Service) endpoint.Endpoint { + return func(ctx context.Context, request interface{}) (interface{}, error) { + req := request.(modifyLabelRequest) + label, err := svc.ModifyLabel(ctx, req.ID, req.payload) + if err != nil { + return modifyLabelResponse{Err: err}, nil + } + return modifyLabelResponse{Label: label}, err + } +} diff --git a/server/service/handler.go b/server/service/handler.go index 6bd97d8df..2b37d29ae 100644 --- a/server/service/handler.go +++ b/server/service/handler.go @@ -65,6 +65,7 @@ type KolideEndpoints struct { ListLabels endpoint.Endpoint CreateLabel endpoint.Endpoint DeleteLabel endpoint.Endpoint + ModifyLabel endpoint.Endpoint GetHost endpoint.Endpoint DeleteHost endpoint.Endpoint ListHosts endpoint.Endpoint @@ -141,6 +142,7 @@ 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)), + ModifyLabel: authenticatedUser(jwtKey, svc, makeModifyLabelEndpoint(svc)), SearchTargets: authenticatedUser(jwtKey, svc, makeSearchTargetsEndpoint(svc)), GetOptions: authenticatedUser(jwtKey, svc, mustBeAdmin(makeGetOptionsEndpoint(svc))), ModifyOptions: authenticatedUser(jwtKey, svc, mustBeAdmin(makeModifyOptionsEndpoint(svc))), @@ -210,6 +212,7 @@ type kolideHandlers struct { ListLabels http.Handler CreateLabel http.Handler DeleteLabel http.Handler + ModifyLabel http.Handler GetHost http.Handler DeleteHost http.Handler ListHosts http.Handler @@ -279,6 +282,7 @@ func makeKolideKitHandlers(ctx context.Context, e KolideEndpoints, opts []kithtt ListLabels: newServer(e.ListLabels, decodeListLabelsRequest), CreateLabel: newServer(e.CreateLabel, decodeCreateLabelRequest), DeleteLabel: newServer(e.DeleteLabel, decodeDeleteLabelRequest), + ModifyLabel: newServer(e.ModifyLabel, decodeModifyLabelRequest), GetHost: newServer(e.GetHost, decodeGetHostRequest), DeleteHost: newServer(e.DeleteHost, decodeDeleteHostRequest), ListHosts: newServer(e.ListHosts, decodeListHostsRequest), @@ -385,6 +389,7 @@ 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/labels/{id}", h.ModifyLabel).Methods("PATCH").Name("modify_label") r.Handle("/api/v1/kolide/hosts", h.ListHosts).Methods("GET").Name("list_hosts") r.Handle("/api/v1/kolide/host_summary", h.GetHostSummary).Methods("GET").Name("get_host_summary") diff --git a/server/service/logging_labels.go b/server/service/logging_labels.go index 649c96bb1..845ed671a 100644 --- a/server/service/logging_labels.go +++ b/server/service/logging_labels.go @@ -7,6 +7,24 @@ import ( "golang.org/x/net/context" ) +func (mw loggingMiddleware) ModifyLabel(ctx context.Context, id uint, p kolide.ModifyLabelPayload) (*kolide.Label, error) { + var ( + label *kolide.Label + err error + ) + + defer func(begin time.Time) { + mw.logger.Log( + "method", "ModifyLabel", + "err", err, + "took", time.Since(begin), + ) + }(time.Now()) + + label, err = mw.Service.ModifyLabel(ctx, id, p) + return label, err +} + func (mw loggingMiddleware) ListLabels(ctx context.Context, opt kolide.ListOptions) ([]*kolide.Label, error) { var ( labels []*kolide.Label diff --git a/server/service/metrics_labels.go b/server/service/metrics_labels.go new file mode 100644 index 000000000..698c7ead3 --- /dev/null +++ b/server/service/metrics_labels.go @@ -0,0 +1,24 @@ +package service + +import ( + "fmt" + "time" + + "github.com/kolide/kolide/server/kolide" + "golang.org/x/net/context" +) + +func (mw metricsMiddleware) ModifyLabel(ctx context.Context, id uint, p kolide.ModifyLabelPayload) (*kolide.Label, error) { + var ( + lic *kolide.Label + err error + ) + defer func(begin time.Time) { + lvs := []string{"method", "ModifyLabel", "error", fmt.Sprint(err != nil)} + mw.requestCount.With(lvs...).Add(1) + mw.requestLatency.With(lvs...).Observe(time.Since(begin).Seconds()) + }(time.Now()) + lic, err = mw.Service.ModifyLabel(ctx, id, p) + return lic, err + +} diff --git a/server/service/service_labels.go b/server/service/service_labels.go index 645d1023d..7883b0d82 100644 --- a/server/service/service_labels.go +++ b/server/service/service_labels.go @@ -56,3 +56,17 @@ func (svc service) HostIDsForLabel(lid uint) ([]uint, error) { } return ids, nil } + +func (svc service) ModifyLabel(ctx context.Context, id uint, payload kolide.ModifyLabelPayload) (*kolide.Label, error) { + label, err := svc.ds.Label(id) + if err != nil { + return nil, err + } + if payload.Name != nil { + label.Name = *payload.Name + } + if payload.Description != nil { + label.Description = *payload.Description + } + return svc.ds.SaveLabel(label) +} diff --git a/server/service/service_labels_test.go b/server/service/service_labels_test.go index 4e52a1643..18796f94a 100644 --- a/server/service/service_labels_test.go +++ b/server/service/service_labels_test.go @@ -6,10 +6,38 @@ import ( "github.com/kolide/kolide/server/config" "github.com/kolide/kolide/server/datastore/inmem" "github.com/kolide/kolide/server/kolide" + "github.com/kolide/kolide/server/mock" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "golang.org/x/net/context" ) +func TestModifyLabel(t *testing.T) { + ds := new(mock.Store) + ds.LabelFunc = func(id uint) (*kolide.Label, error) { + l := &kolide.Label{ + ID: id, + Name: "name", + Description: "desc", + } + return l, nil + } + ds.SaveLabelFunc = func(l *kolide.Label) (*kolide.Label, error) { + return l, nil + } + svc, err := newTestService(ds, nil) + require.Nil(t, err) + lp := kolide.ModifyLabelPayload{ + Name: stringPtr("new name"), + Description: stringPtr("new desc"), + } + l, err := svc.ModifyLabel(context.Background(), uint(1), lp) + assert.Equal(t, "new name", l.Name) + assert.Equal(t, "new desc", l.Description) + assert.True(t, ds.LabelFuncInvoked) + assert.True(t, ds.SaveLabelFuncInvoked) +} + func TestListLabels(t *testing.T) { ds, err := inmem.New(config.TestConfig()) assert.Nil(t, err) diff --git a/server/service/transport_labels.go b/server/service/transport_labels.go index f1162aba5..56209cf98 100644 --- a/server/service/transport_labels.go +++ b/server/service/transport_labels.go @@ -42,3 +42,17 @@ func decodeListLabelsRequest(ctx context.Context, r *http.Request) (interface{}, } return listLabelsRequest{ListOptions: opt}, nil } + +func decodeModifyLabelRequest(ctx context.Context, r *http.Request) (interface{}, error) { + id, err := idFromRequest(r, "id") + if err != nil { + return nil, err + } + var resp modifyLabelRequest + err = json.NewDecoder(r.Body).Decode(&resp.payload) + if err != nil { + return nil, err + } + resp.ID = id + return resp, nil +}