Fix bugs with packs created in UI (#1843)

- Delete duplicate queries in packs created by the UI (because the duplicates
  were causing undefined behavior). Now it is not possible to schedule
  duplicates in the UI (but is in fleetctl).
- Fix bug in which packs created in UI could not be loaded by fleetctl.
This commit is contained in:
Zachary Wasserman 2018-06-21 17:06:44 -07:00 committed by GitHub
parent 4ade65b4c1
commit 08a2dc73cd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 117 additions and 18 deletions

View file

@ -194,6 +194,7 @@ func setupPackSpecsTest(t *testing.T, ds kolide.Datastore) []*kolide.PackSpec {
Queries: []kolide.PackSpecQuery{
kolide.PackSpecQuery{
QueryName: queries[0].Name,
Name: "q0",
Description: "test_foo",
Interval: 42,
},
@ -204,6 +205,7 @@ func setupPackSpecsTest(t *testing.T, ds kolide.Datastore) []*kolide.PackSpec {
Snapshot: boolPtr(true),
},
kolide.PackSpecQuery{
Name: "q2",
QueryName: queries[1].Name,
Interval: 600,
Removed: boolPtr(false),

View file

@ -227,6 +227,7 @@ func testLoadPacksForQueries(t *testing.T, ds kolide.Datastore) {
Name: "p2",
Queries: []kolide.PackSpecQuery{
kolide.PackSpecQuery{
Name: "q0",
QueryName: queries[0].Name,
Interval: 60,
},
@ -288,10 +289,12 @@ func testLoadPacksForQueries(t *testing.T, ds kolide.Datastore) {
Name: "p3",
Queries: []kolide.PackSpecQuery{
kolide.PackSpecQuery{
Name: "q0",
QueryName: queries[0].Name,
Interval: 60,
},
kolide.PackSpecQuery{
Name: "q1",
QueryName: queries[1].Name,
Interval: 60,
},

View file

@ -0,0 +1,81 @@
package tables
import (
"database/sql"
"github.com/jmoiron/sqlx"
"github.com/jmoiron/sqlx/reflectx"
"github.com/pkg/errors"
)
func init() {
MigrationClient.AddMigration(Up_20180620175054, Down_20180620175054)
}
func Up_20180620175054(tx *sql.Tx) error {
// Make sure all names are non-empty
query := `
UPDATE scheduled_queries
SET name = COALESCE(name, query_name),
description = COALESCE(description, ''),
platform = COALESCE(platform, ''),
version = COALESCE(version, '')
`
if _, err := tx.Exec(query); err != nil {
return errors.Wrap(err, "set name for all queries")
}
// Dedupe (name, pack_id)
query = `
SELECT name, pack_id
FROM scheduled_queries
GROUP BY name, pack_id
HAVING count(pack_id) > 1
`
txx := sqlx.Tx{Tx: tx, Mapper: reflectx.NewMapperFunc("db", sqlx.NameMapper)}
var dupes []struct {
Name string `db:"name"`
PackID uint `db:"pack_id"`
}
if err := txx.Select(&dupes, query); err != nil && err != sql.ErrNoRows {
return errors.Wrap(err, "get duplicate query names")
}
for _, dupe := range dupes {
// Yes you really need to SELECT id FROM ( SELECT id FROM...
// Otherwise MySQL errors
query = `
DELETE FROM scheduled_queries
WHERE id IN
( SELECT id FROM (
SELECT id from scheduled_queries
WHERE name = ? AND pack_id = ?
ORDER BY id DESC
LIMIT 9999 OFFSET 1
) AS t
)
`
if _, err := tx.Exec(query, dupe.Name, dupe.PackID); err != nil {
return errors.Wrapf(err, "delete dupe %s", dupe.Name)
}
}
// Enforce not-null, uniqueness and add column defaults
query = `
ALTER TABLE scheduled_queries
MODIFY name varchar(255) NOT NULL,
MODIFY description varchar(1023) DEFAULT '',
MODIFY platform varchar(255) DEFAULT '',
MODIFY version varchar(255) DEFAULT '',
ADD UNIQUE KEY unique_names_in_packs (name, pack_id)
`
if _, err := tx.Exec(query); err != nil {
return errors.Wrapf(err, "altering table")
}
return nil
}
func Down_20180620175054(tx *sql.Tx) error {
return nil
}

View file

@ -12,13 +12,13 @@ func (d *Datastore) ListScheduledQueriesInPack(id uint, opts kolide.ListOptions)
SELECT
sq.id,
sq.pack_id,
COALESCE(sq.name, q.name) AS name,
sq.name,
sq.query_name,
COALESCE(sq.description, '') AS description,
sq.description,
sq.interval,
sq.snapshot,
sq.removed,
COALESCE(sq.platform, '') AS platform,
sq.platform,
sq.version,
sq.shard,
q.query,
@ -47,6 +47,7 @@ func (d *Datastore) NewScheduledQuery(sq *kolide.ScheduledQuery, opts ...kolide.
query := `
INSERT INTO scheduled_queries (
query_name,
name,
pack_id,
snapshot,
removed,
@ -55,11 +56,11 @@ func (d *Datastore) NewScheduledQuery(sq *kolide.ScheduledQuery, opts ...kolide.
version,
shard
)
SELECT name, ?, ?, ?, ?, ?, ?, ?
SELECT name, ?, ?, ?, ?, ?, ?, ?, ?
FROM queries
WHERE id = ?
`
result, err := db.Exec(query, sq.PackID, sq.Snapshot, sq.Removed, sq.Interval, sq.Platform, sq.Version, sq.Shard, sq.QueryID)
result, err := db.Exec(query, sq.Name, sq.PackID, sq.Snapshot, sq.Removed, sq.Interval, sq.Platform, sq.Version, sq.Shard, sq.QueryID)
if err != nil {
return nil, errors.Wrap(err, "inserting scheduled query")
}
@ -128,7 +129,7 @@ func (d *Datastore) ScheduledQuery(id uint) (*kolide.ScheduledQuery, error) {
sq.version,
sq.shard,
sq.query_name,
COALESCE(sq.description, '') AS description,
sq.description,
q.query,
q.name,
q.id AS query_id

View file

@ -29,12 +29,12 @@ type ScheduledQuery struct {
QueryID uint `json:"query_id" db:"query_id"`
QueryName string `json:"query_name" db:"query_name"`
Query string `json:"query"` // populated via a join on queries
Description string `json:"description"`
Description string `json:"description,omitempty"`
Interval uint `json:"interval"`
Snapshot *bool `json:"snapshot"`
Removed *bool `json:"removed"`
Platform *string `json:"platform"`
Version *string `json:"version"`
Platform *string `json:"platform,omitempty"`
Version *string `json:"version,omitempty"`
Shard *uint `json:"shard"`
}

View file

@ -56,8 +56,8 @@ type getScheduledQueryRequest struct {
}
type getScheduledQueryResponse struct {
Scheduled scheduledQueryResponse `json:"scheduled,omitempty"`
Err error `json:"error,omitempty"`
Scheduled *scheduledQueryResponse `json:"scheduled,omitempty"`
Err error `json:"error,omitempty"`
}
func (r getScheduledQueryResponse) error() error { return r.Err }
@ -72,7 +72,7 @@ func makeGetScheduledQueryEndpoint(svc kolide.Service) endpoint.Endpoint {
}
return getScheduledQueryResponse{
Scheduled: scheduledQueryResponse{
Scheduled: &scheduledQueryResponse{
ScheduledQuery: *sq,
},
}, nil
@ -95,10 +95,12 @@ type scheduleQueryRequest struct {
}
type scheduleQueryResponse struct {
Scheduled scheduledQueryResponse `json:"scheduled"`
Err error `json:"error,omitempty"`
Scheduled *scheduledQueryResponse `json:"scheduled,omitempty"`
Err error `json:"error,omitempty"`
}
func (r scheduleQueryResponse) error() error { return r.Err }
func makeScheduleQueryEndpoint(svc kolide.Service) endpoint.Endpoint {
return func(ctx context.Context, request interface{}) (interface{}, error) {
req := request.(scheduleQueryRequest)
@ -116,7 +118,7 @@ func makeScheduleQueryEndpoint(svc kolide.Service) endpoint.Endpoint {
if err != nil {
return scheduleQueryResponse{Err: err}, nil
}
return scheduleQueryResponse{Scheduled: scheduledQueryResponse{
return scheduleQueryResponse{Scheduled: &scheduledQueryResponse{
ScheduledQuery: *scheduled,
}}, nil
}
@ -132,8 +134,8 @@ type modifyScheduledQueryRequest struct {
}
type modifyScheduledQueryResponse struct {
Scheduled scheduledQueryResponse `json:"scheduled,omitempty"`
Err error `json:"error,omitempty"`
Scheduled *scheduledQueryResponse `json:"scheduled,omitempty"`
Err error `json:"error,omitempty"`
}
func (r modifyScheduledQueryResponse) error() error { return r.Err }
@ -148,7 +150,7 @@ func makeModifyScheduledQueryEndpoint(svc kolide.Service) endpoint.Endpoint {
}
return modifyScheduledQueryResponse{
Scheduled: scheduledQueryResponse{
Scheduled: &scheduledQueryResponse{
ScheduledQuery: *sq,
},
}, nil

View file

@ -16,6 +16,16 @@ func (svc service) GetScheduledQuery(ctx context.Context, id uint) (*kolide.Sche
}
func (svc service) ScheduleQuery(ctx context.Context, sq *kolide.ScheduledQuery) (*kolide.ScheduledQuery, error) {
// Fill in the name with query name if it is unset (because the UI
// doesn't provide a way to set it)
if sq.Name == "" {
query, err := svc.ds.Query(sq.QueryID)
if err != nil {
return nil, errors.Wrap(err, "lookup name for query")
}
sq.Name = query.Name
sq.QueryName = query.Name
}
return svc.ds.NewScheduledQuery(sq)
}