diff --git a/changes/issue-3579-return-not-found-query-policy b/changes/issue-3579-return-not-found-query-policy new file mode 100644 index 0000000000..8fe1ecefe5 --- /dev/null +++ b/changes/issue-3579-return-not-found-query-policy @@ -0,0 +1 @@ +* Properly return not found for queries and policies diff --git a/server/datastore/mysql/policies.go b/server/datastore/mysql/policies.go index 2b791e6f3a..7fce567b00 100644 --- a/server/datastore/mysql/policies.go +++ b/server/datastore/mysql/policies.go @@ -2,6 +2,7 @@ package mysql import ( "context" + "database/sql" "fmt" "sort" "strings" @@ -67,6 +68,9 @@ func policyDB(ctx context.Context, q sqlx.QueryerContext, id uint, teamID *uint) WHERE p.id=? AND %s`, teamWhere), args...) if err != nil { + if err == sql.ErrNoRows { + return nil, ctxerr.Wrap(ctx, notFound("Policy").WithID(id)) + } return nil, ctxerr.Wrap(ctx, err, "getting policy") } return &policy, nil diff --git a/server/datastore/mysql/queries.go b/server/datastore/mysql/queries.go index 51e787cc04..51620011ae 100644 --- a/server/datastore/mysql/queries.go +++ b/server/datastore/mysql/queries.go @@ -149,7 +149,7 @@ func (d *Datastore) DeleteQueries(ctx context.Context, ids []uint) (uint, error) // Query returns a single Query identified by id, if such exists. func (d *Datastore) Query(ctx context.Context, id uint) (*fleet.Query, error) { - sql := ` + sqlQuery := ` SELECT q.*, COALESCE(NULLIF(u.name, ''), u.email, '') AS author_name, COALESCE(u.email, '') AS author_email FROM queries q LEFT JOIN users u @@ -157,7 +157,10 @@ func (d *Datastore) Query(ctx context.Context, id uint) (*fleet.Query, error) { WHERE q.id = ? ` query := &fleet.Query{} - if err := sqlx.GetContext(ctx, d.reader, query, sql, id); err != nil { + if err := sqlx.GetContext(ctx, d.reader, query, sqlQuery, id); err != nil { + if err == sql.ErrNoRows { + return nil, ctxerr.Wrap(ctx, notFound("Query").WithID(id)) + } return nil, ctxerr.Wrap(ctx, err, "selecting query") } diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index c63c8c446a..242621c366 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -455,6 +455,9 @@ func (s *integrationTestSuite) TestGlobalPolicies() { assert.Equal(t, qr.Query, policiesResponse.Policies[0].Query) assert.Equal(t, qr.Description, policiesResponse.Policies[0].Description) + // Get an unexistent policy + s.Do("GET", fmt.Sprintf("/api/v1/fleet/global/policies/%d", 9999), nil, http.StatusNotFound) + singlePolicyResponse := getPolicyByIDResponse{} singlePolicyURL := fmt.Sprintf("/api/v1/fleet/global/policies/%d", policiesResponse.Policies[0].ID) s.DoJSON("GET", singlePolicyURL, nil, http.StatusOK, &singlePolicyResponse) @@ -1476,6 +1479,9 @@ func (s *integrationTestSuite) TestScheduledQueries() { s.DoJSON("POST", "/api/v1/fleet/packs", reqPack, http.StatusOK, &createPackResp) pack := createPackResp.Pack.Pack + // try a non existent query + s.Do("GET", fmt.Sprintf("/api/v1/fleet/queries/%d", 9999), nil, http.StatusNotFound) + // create a query var createQueryResp createQueryResponse reqQuery := &fleet.QueryPayload{ @@ -1520,7 +1526,7 @@ func (s *integrationTestSuite) TestScheduledQueries() { QueryID: query.ID + 1, Interval: 3, } - s.DoJSON("POST", "/api/v1/fleet/schedule", reqSQ, http.StatusInternalServerError, &createResp) + s.DoJSON("POST", "/api/v1/fleet/schedule", reqSQ, http.StatusNotFound, &createResp) // list scheduled queries in pack s.DoJSON("GET", fmt.Sprintf("/api/v1/fleet/packs/%d/scheduled", pack.ID), nil, http.StatusOK, &getInPackResp) diff --git a/server/service/integration_live_queries_test.go b/server/service/integration_live_queries_test.go index 40791de9ec..502f26180b 100644 --- a/server/service/integration_live_queries_test.go +++ b/server/service/integration_live_queries_test.go @@ -318,8 +318,7 @@ func (s *liveQueriesTestSuite) TestLiveQueriesRestFailsToCreateCampaign() { require.Len(t, liveQueryResp.Results, 1) assert.Equal(t, 0, liveQueryResp.Summary.RespondedHostCount) require.NotNil(t, liveQueryResp.Results[0].Error) - assert.Contains(t, *liveQueryResp.Results[0].Error, "selecting query: ") - assert.Contains(t, *liveQueryResp.Results[0].Error, "sql: no rows in result set") + assert.Contains(t, *liveQueryResp.Results[0].Error, "Query 999 was not found in the datastore") } func (s *liveQueriesTestSuite) TestLiveQueriesRestFailsOnSomeHost() {