mirror of
https://github.com/fleetdm/fleet
synced 2026-04-21 13:37:30 +00:00
Fix get/create/update query response (#41966)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #41603 # Details Fixes an issue where the nested `query` key in the get, create and update query API responses, which is the literal SQL query, was getting duplicated into a `report` key with the SQL. This was happening because our JSON field duplicator which adds the renamed version of deprecated keys to responses is intentionally naive; it doesn't account for cases where the parent and child structs have the same key with different meanings because that is... not ideal. In Fleet 5 we won't have this problem since it'll just be `report.query`, but for now the solution is to just hard-code a `Report` field onto the response structs, rather than over-complicate the duplicator code to account for different nesting levels. # Checklist for submitter If some of the following don't apply, delete the relevant line. - [X] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. ## Testing - [X] Added/updated automated tests - [X] QA'd all new/changed functionality manually - [X] verified no `query.report` or `report.report` present in get query response - [X] verified no `query.report` or `report.report` present in create query response - [X] verified no `query.report` or `report.report` present in modify query response
This commit is contained in:
parent
3c73610d4e
commit
f093406e04
3 changed files with 34 additions and 10 deletions
1
changes/41603-fix-query-responses
Normal file
1
changes/41603-fix-query-responses
Normal file
|
|
@ -0,0 +1 @@
|
|||
- Removed incorrect `report` key from get/create/modify API responses.
|
||||
|
|
@ -3757,13 +3757,16 @@ func (s *integrationTestSuite) TestScheduledQueries() {
|
|||
assert.Equal(t, 0, listQryResp.InheritedQueryCount)
|
||||
|
||||
// create a query
|
||||
sql := "select * from time;"
|
||||
var createQueryResp createQueryResponse
|
||||
reqQuery := &fleet.QueryPayload{
|
||||
Name: ptr.String(strings.ReplaceAll(t.Name(), "/", "_")),
|
||||
Query: ptr.String("select * from time;"),
|
||||
Query: ptr.String(sql),
|
||||
}
|
||||
s.DoJSON("POST", "/api/latest/fleet/queries", reqQuery, http.StatusOK, &createQueryResp)
|
||||
query := createQueryResp.Query
|
||||
assert.Equal(t, query.Query, sql)
|
||||
assert.Equal(t, createQueryResp.Report.Query, sql)
|
||||
|
||||
// listing returns that query
|
||||
s.DoJSON("GET", "/api/latest/fleet/queries", nil, http.StatusOK, &listQryResp)
|
||||
|
|
@ -3803,6 +3806,9 @@ func (s *integrationTestSuite) TestScheduledQueries() {
|
|||
var getQryResp getQueryResponse
|
||||
s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/queries/%d", query.ID), nil, http.StatusOK, &getQryResp)
|
||||
assert.Equal(t, query.ID, getQryResp.Query.ID)
|
||||
assert.Equal(t, query.ID, getQryResp.Report.ID)
|
||||
assert.Equal(t, sql, getQryResp.Query.Query)
|
||||
assert.Equal(t, sql, getQryResp.Report.Query)
|
||||
|
||||
// list scheduled queries in pack, none yet
|
||||
var getInPackResp getScheduledQueriesInPackResponse
|
||||
|
|
@ -3888,6 +3894,8 @@ func (s *integrationTestSuite) TestScheduledQueries() {
|
|||
var modQryResp modifyQueryResponse
|
||||
s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/queries/%d", query.ID), fleet.QueryPayload{Description: ptr.String("updated")}, http.StatusOK, &modQryResp)
|
||||
assert.Equal(t, "updated", modQryResp.Query.Description)
|
||||
assert.Equal(t, sql, modQryResp.Query.Query)
|
||||
assert.Equal(t, sql, modQryResp.Report.Query)
|
||||
|
||||
// TODO(jahziel): check that the query results were deleted
|
||||
|
||||
|
|
|
|||
|
|
@ -24,8 +24,13 @@ type getQueryRequest struct {
|
|||
}
|
||||
|
||||
type getQueryResponse struct {
|
||||
Query *fleet.Query `json:"query,omitempty" renameto:"report"`
|
||||
Err error `json:"error,omitempty"`
|
||||
// Because `fleet.Query` has a `query` field that we don't want to rename,
|
||||
// it's simpler to just duplicate the query in the response struct rather than
|
||||
// relying on the `renameto` tag here.
|
||||
// TODO - In Fleet 5, remove the extra field.
|
||||
Query *fleet.Query `json:"query,omitempty"`
|
||||
Report *fleet.Query `json:"report,omitempty"`
|
||||
Err error `json:"error,omitempty"`
|
||||
}
|
||||
|
||||
func (r getQueryResponse) Error() error { return r.Err }
|
||||
|
|
@ -36,7 +41,7 @@ func getQueryEndpoint(ctx context.Context, request interface{}, svc fleet.Servic
|
|||
if err != nil {
|
||||
return getQueryResponse{Err: err}, nil
|
||||
}
|
||||
return getQueryResponse{query, nil}, nil
|
||||
return getQueryResponse{Query: query, Report: query}, nil
|
||||
}
|
||||
|
||||
func (svc *Service) GetQuery(ctx context.Context, id uint) (*fleet.Query, error) {
|
||||
|
|
@ -251,8 +256,13 @@ type createQueryRequest struct {
|
|||
}
|
||||
|
||||
type createQueryResponse struct {
|
||||
Query *fleet.Query `json:"query,omitempty" renameto:"report"`
|
||||
Err error `json:"error,omitempty"`
|
||||
// Because `fleet.Query` has a `query` field that we don't want to rename,
|
||||
// it's simpler to just duplicate the query in the response struct rather than
|
||||
// relying on the `renameto` tag here.
|
||||
// TODO - In Fleet 5, remove the extra field.
|
||||
Query *fleet.Query `json:"query,omitempty"`
|
||||
Report *fleet.Query `json:"report,omitempty"`
|
||||
Err error `json:"error,omitempty"`
|
||||
}
|
||||
|
||||
func (r createQueryResponse) Error() error { return r.Err }
|
||||
|
|
@ -263,7 +273,7 @@ func createQueryEndpoint(ctx context.Context, request interface{}, svc fleet.Ser
|
|||
if err != nil {
|
||||
return createQueryResponse{Err: err}, nil
|
||||
}
|
||||
return createQueryResponse{query, nil}, nil
|
||||
return createQueryResponse{Query: query, Report: query}, nil
|
||||
}
|
||||
|
||||
func (svc *Service) NewQuery(ctx context.Context, p fleet.QueryPayload) (*fleet.Query, error) {
|
||||
|
|
@ -389,8 +399,13 @@ type modifyQueryRequest struct {
|
|||
}
|
||||
|
||||
type modifyQueryResponse struct {
|
||||
Query *fleet.Query `json:"query,omitempty" renameto:"report"`
|
||||
Err error `json:"error,omitempty"`
|
||||
// Because `fleet.Query` has a `query` field that we don't want to rename,
|
||||
// it's simpler to just duplicate the query in the response struct rather than
|
||||
// relying on the `renameto` tag here.
|
||||
// TODO - In Fleet 5, remove the extra field.
|
||||
Query *fleet.Query `json:"query,omitempty"`
|
||||
Report *fleet.Query `json:"report,omitempty"`
|
||||
Err error `json:"error,omitempty"`
|
||||
}
|
||||
|
||||
func (r modifyQueryResponse) Error() error { return r.Err }
|
||||
|
|
@ -401,7 +416,7 @@ func modifyQueryEndpoint(ctx context.Context, request interface{}, svc fleet.Ser
|
|||
if err != nil {
|
||||
return modifyQueryResponse{Err: err}, nil
|
||||
}
|
||||
return modifyQueryResponse{query, nil}, nil
|
||||
return modifyQueryResponse{Query: query, Report: query}, nil
|
||||
}
|
||||
|
||||
func (svc *Service) ModifyQuery(ctx context.Context, id uint, p fleet.QueryPayload) (*fleet.Query, error) {
|
||||
|
|
|
|||
Loading…
Reference in a new issue