Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add go-bexpr filters to evals and deployment list endpoints #12034

Merged
merged 5 commits into from
Feb 16, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/12034.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
api: filter values of evaluation and deployment list api endpoints
```
7 changes: 7 additions & 0 deletions api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ type QueryOptions struct {
// AuthToken is the secret ID of an ACL token
AuthToken string

// Filter specifies the go-bexpr filter expression to be used for
// filtering the data prior to returning a response
Filter string

// PerPage is the number of entries to be returned in queries that support
// paginated lists.
PerPage int32
Expand Down Expand Up @@ -586,6 +590,9 @@ func (r *request) setQueryOptions(q *QueryOptions) {
if q.Prefix != "" {
r.params.Set("prefix", q.Prefix)
}
if q.Filter != "" {
r.params.Set("filter", q.Filter)
}
if q.PerPage != 0 {
r.params.Set("per_page", fmt.Sprint(q.PerPage))
}
Expand Down
8 changes: 8 additions & 0 deletions api/evaluations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,14 @@ func TestEvaluations_List(t *testing.T) {
if len(result) != 1 {
t.Fatalf("expected no evals after last one but got %v", result[0])
}

// Query evaluations using a filter.
results, _, err = e.List(&QueryOptions{
Filter: `TriggeredBy == "job-register"`,
})
if len(result) != 1 {
t.Fatalf("expected 1 eval, got %d", len(result))
}
}

func TestEvaluations_PrefixList(t *testing.T) {
Expand Down
12 changes: 12 additions & 0 deletions command/agent/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,9 @@ func (s *HTTPServer) wrap(handler func(resp http.ResponseWriter, req *http.Reque
} else if strings.HasSuffix(errMsg, structs.ErrJobRegistrationDisabled.Error()) {
errMsg = structs.ErrJobRegistrationDisabled.Error()
code = 403
} else if strings.HasSuffix(errMsg, structs.ErrIncompatibleFiltering.Error()) {
errMsg = structs.ErrIncompatibleFiltering.Error()
code = 400
Comment on lines +540 to +542
Copy link
Contributor Author

@lgfa29 lgfa29 Feb 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this is the right place to handle this, but it would be good to have it in a code path that is common to all endpoints.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is absolutely a weird place to jam it and not the API (HasSuffix?!) any of us would prefer but... it's idiomatic for now. 👍

}
}

Expand Down Expand Up @@ -784,6 +787,7 @@ func (s *HTTPServer) parse(resp http.ResponseWriter, req *http.Request, r *strin
parsePrefix(req, b)
parseNamespace(req, &b.Namespace)
parsePagination(req, b)
parseFilter(req, b)
return parseWait(resp, req, b)
}

Expand All @@ -801,6 +805,14 @@ func parsePagination(req *http.Request, b *structs.QueryOptions) {
b.NextToken = query.Get("next_token")
}

// parseFilter parses the filter query parameter for QueryOptions
func parseFilter(req *http.Request, b *structs.QueryOptions) {
query := req.URL.Query()
if filter := query.Get("filter"); filter != "" {
b.Filter = filter
}
}

// parseWriteRequest is a convenience method for endpoints that need to parse a
// write request.
func (s *HTTPServer) parseWriteRequest(req *http.Request, w *structs.WriteRequest) {
Expand Down
13 changes: 11 additions & 2 deletions command/deployment_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ List Options:
-json
Output the deployments in a JSON format.

-filter
Specifies the expression used to filter the queries results prior to
returning the data.

-t
Format and display the deployments using a Go template.

Expand All @@ -43,6 +47,7 @@ func (c *DeploymentListCommand) AutocompleteFlags() complete.Flags {
return mergeAutocompleteFlags(c.Meta.AutocompleteFlags(FlagSetClient),
complete.Flags{
"-json": complete.PredictNothing,
"-filter": complete.PredictAnything,
"-t": complete.PredictAnything,
"-verbose": complete.PredictNothing,
})
Expand All @@ -60,12 +65,13 @@ func (c *DeploymentListCommand) Name() string { return "deployment list" }

func (c *DeploymentListCommand) Run(args []string) int {
var json, verbose bool
var tmpl string
var filter, tmpl string

flags := c.Meta.FlagSet(c.Name(), FlagSetClient)
flags.Usage = func() { c.Ui.Output(c.Help()) }
flags.BoolVar(&verbose, "verbose", false, "")
flags.BoolVar(&json, "json", false, "")
flags.StringVar(&filter, "filter", "", "")
flags.StringVar(&tmpl, "t", "", "")

if err := flags.Parse(args); err != nil {
Expand Down Expand Up @@ -93,7 +99,10 @@ func (c *DeploymentListCommand) Run(args []string) int {
return 1
}

deploys, _, err := client.Deployments().List(nil)
opts := &api.QueryOptions{
Filter: filter,
}
deploys, _, err := client.Deployments().List(opts)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error retrieving deployments: %s", err))
return 1
Expand Down
9 changes: 8 additions & 1 deletion command/eval_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ Eval List Options:
-page-token
Where to start pagination.

-filter
Specifies the expression used to filter the queries results prior to
returning the data.

-job
Only show evaluations for this job ID.

Expand All @@ -61,6 +65,7 @@ func (c *EvalListCommand) AutocompleteFlags() complete.Flags {
"-json": complete.PredictNothing,
"-t": complete.PredictAnything,
"-verbose": complete.PredictNothing,
"-filter": complete.PredictAnything,
"-job": complete.PredictAnything,
"-status": complete.PredictAnything,
"-per-page": complete.PredictAnything,
Expand Down Expand Up @@ -88,7 +93,7 @@ func (c *EvalListCommand) Name() string { return "eval list" }
func (c *EvalListCommand) Run(args []string) int {
var monitor, verbose, json bool
var perPage int
var tmpl, pageToken, filterJobID, filterStatus string
var tmpl, pageToken, filter, filterJobID, filterStatus string

flags := c.Meta.FlagSet(c.Name(), FlagSetClient)
flags.Usage = func() { c.Ui.Output(c.Help()) }
Expand All @@ -98,6 +103,7 @@ func (c *EvalListCommand) Run(args []string) int {
flags.StringVar(&tmpl, "t", "", "")
flags.IntVar(&perPage, "per-page", 0, "")
flags.StringVar(&pageToken, "page-token", "", "")
flags.StringVar(&filter, "filter", "", "")
flags.StringVar(&filterJobID, "job", "", "")
flags.StringVar(&filterStatus, "status", "", "")

Expand All @@ -120,6 +126,7 @@ func (c *EvalListCommand) Run(args []string) int {
}

opts := &api.QueryOptions{
Filter: filter,
PerPage: int32(perPage),
NextToken: pageToken,
Params: map[string]string{},
Expand Down
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ require (
github.com/hashicorp/consul/api v1.9.1
github.com/hashicorp/consul/sdk v0.8.0
github.com/hashicorp/cronexpr v1.1.1
github.com/hashicorp/go-bexpr v0.1.11
github.com/hashicorp/go-checkpoint v0.0.0-20171009173528-1545e56e46de
github.com/hashicorp/go-cleanhttp v0.5.2
github.com/hashicorp/go-connlimit v0.3.0
Expand Down Expand Up @@ -209,6 +210,7 @@ require (
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
github.com/mitchellh/go-homedir v1.1.0 // indirect
github.com/mitchellh/go-wordwrap v1.0.1 // indirect
github.com/mitchellh/pointerstructure v1.2.1 // indirect
github.com/moby/term v0.0.0-20210619224110-3f7ff695adc6 // indirect
github.com/morikuni/aec v1.0.0 // indirect
github.com/mrunalp/fileutils v0.5.0 // indirect
Expand Down
5 changes: 5 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,8 @@ github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brv
github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I=
github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
github.com/hashicorp/go-bexpr v0.1.2/go.mod h1:ANbpTX1oAql27TZkKVeW8p1w8NTdnyzPe/0qqPCKohU=
github.com/hashicorp/go-bexpr v0.1.11 h1:6DqdA/KBjurGby9yTY0bmkathya0lfwF2SeuubCI7dY=
github.com/hashicorp/go-bexpr v0.1.11/go.mod h1:f03lAo0duBlDIUMGCuad8oLcgejw4m7U+N8T+6Kz1AE=
github.com/hashicorp/go-checkpoint v0.0.0-20171009173528-1545e56e46de h1:XDCSythtg8aWSRSO29uwhgh7b127fWr+m5SemqjSUL8=
github.com/hashicorp/go-checkpoint v0.0.0-20171009173528-1545e56e46de/go.mod h1:xIwEieBHERyEvaeKF/TcHh1Hu+lxPM+n2vT1+g9I4m4=
github.com/hashicorp/go-cleanhttp v0.5.0/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
Expand Down Expand Up @@ -938,9 +940,12 @@ github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh
github.com/mitchellh/mapstructure v1.2.3/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/mitchellh/mapstructure v1.3.2/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/mitchellh/mapstructure v1.3.3/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/mitchellh/mapstructure v1.4.1/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/mitchellh/mapstructure v1.4.3 h1:OVowDSCllw/YjdLkam3/sm7wEtOy59d8ndGgCcyj8cs=
github.com/mitchellh/mapstructure v1.4.3/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/mitchellh/osext v0.0.0-20151018003038-5e2d6d41470f/go.mod h1:OkQIRizQZAeMln+1tSwduZz7+Af5oFlKirV/MSYes2A=
github.com/mitchellh/pointerstructure v1.2.1 h1:ZhBBeX8tSlRpu/FFhXH4RC4OJzFlqsQhoHZAz4x7TIw=
github.com/mitchellh/pointerstructure v1.2.1/go.mod h1:BRAsLI5zgXmw97Lf6s25bs8ohIXc3tViBH44KcwB2g4=
github.com/mitchellh/reflectwalk v1.0.0/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
github.com/mitchellh/reflectwalk v1.0.1/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zxSIeXaQ=
Expand Down
14 changes: 12 additions & 2 deletions nomad/deployment_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package nomad

import (
"fmt"
"net/http"
"time"

metrics "github.com/armon/go-metrics"
Expand Down Expand Up @@ -421,13 +422,22 @@ func (d *Deployment) List(args *structs.DeploymentListRequest, reply *structs.De
}

var deploys []*structs.Deployment
paginator := state.NewPaginator(iter, args.QueryOptions,
paginator, err := state.NewPaginator(iter, args.QueryOptions,
func(raw interface{}) {
deploy := raw.(*structs.Deployment)
deploys = append(deploys, deploy)
})
if err != nil {
return structs.NewErrRPCCodedf(
http.StatusBadRequest, "failed to create result paginator: %v", err)
}

nextToken, err := paginator.Page()
if err != nil {
return structs.NewErrRPCCodedf(
http.StatusBadRequest, "failed to read result page: %v", err)
}

nextToken := paginator.Page()
reply.QueryMeta.NextToken = nextToken
reply.Deployments = deploys

Expand Down
62 changes: 59 additions & 3 deletions nomad/deployment_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1288,17 +1288,19 @@ func TestDeploymentEndpoint_List_Pagination(t *testing.T) {
}

aclToken := mock.CreatePolicyAndToken(t, state, 1100, "test-valid-read",
mock.NamespacePolicy(structs.DefaultNamespace, "read", nil)).
mock.NamespacePolicy("*", "read", nil)).
SecretID

cases := []struct {
name string
namespace string
prefix string
filter string
nextToken string
pageSize int32
expectedNextToken string
expectedIDs []string
expectedError string
}{
{
name: "test01 size-2 page-1 default NS",
Expand Down Expand Up @@ -1341,12 +1343,57 @@ func TestDeploymentEndpoint_List_Pagination(t *testing.T) {
},
},
{
name: "test5 no valid results with filters and prefix",
name: "test05 no valid results with filters and prefix",
prefix: "cccc",
pageSize: 2,
nextToken: "",
expectedIDs: []string{},
},
{
name: "test06 go-bexpr filter",
namespace: "*",
filter: `ID matches "^a+[123]"`,
expectedIDs: []string{
"aaaa1111-3350-4b4b-d185-0e1992ed43e9",
"aaaaaa22-3350-4b4b-d185-0e1992ed43e9",
"aaaaaa33-3350-4b4b-d185-0e1992ed43e9",
},
},
{
name: "test07 go-bexpr filter with pagination",
namespace: "*",
filter: `ID matches "^a+[123]"`,
pageSize: 2,
expectedNextToken: "aaaaaa33-3350-4b4b-d185-0e1992ed43e9",
expectedIDs: []string{
"aaaa1111-3350-4b4b-d185-0e1992ed43e9",
"aaaaaa22-3350-4b4b-d185-0e1992ed43e9",
},
},
{
name: "test08 go-bexpr filter in namespace",
namespace: "non-default",
filter: `Status == "cancelled"`,
expectedIDs: []string{
"aaaaaa33-3350-4b4b-d185-0e1992ed43e9",
},
},
{
name: "test09 go-bexpr wrong namespace",
namespace: "default",
filter: `Namespace == "non-default"`,
expectedIDs: []string{},
},
{
name: "test10 go-bexpr invalid expression",
filter: `NotValid`,
expectedError: "failed to read filter expression",
},
{
name: "test11 go-bexpr invalid field",
filter: `InvalidField == "value"`,
expectedError: "error finding value in datum",
},
}

for _, tc := range cases {
Expand All @@ -1357,13 +1404,22 @@ func TestDeploymentEndpoint_List_Pagination(t *testing.T) {
Region: "global",
Namespace: tc.namespace,
Prefix: tc.prefix,
Filter: tc.filter,
PerPage: tc.pageSize,
NextToken: tc.nextToken,
},
}
req.AuthToken = aclToken
var resp structs.DeploymentListResponse
require.NoError(t, msgpackrpc.CallWithCodec(codec, "Deployment.List", req, &resp))
err := msgpackrpc.CallWithCodec(codec, "Deployment.List", req, &resp)
if tc.expectedError == "" {
require.NoError(t, err)
} else {
require.Error(t, err)
require.Contains(t, err.Error(), tc.expectedError)
return
}

gotIDs := []string{}
for _, deployment := range resp.Deployments {
gotIDs = append(gotIDs, deployment.ID)
Expand Down
22 changes: 20 additions & 2 deletions nomad/eval_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package nomad

import (
"fmt"
"net/http"
"time"

metrics "github.com/armon/go-metrics"
Expand Down Expand Up @@ -397,6 +398,14 @@ func (e *Eval) List(args *structs.EvalListRequest, reply *structs.EvalListRespon
return structs.ErrPermissionDenied
}

if args.Filter != "" {
// Check for incompatible filtering.
hasLegacyFilter := args.FilterJobID != "" || args.FilterEvalStatus != ""
if hasLegacyFilter {
return structs.ErrIncompatibleFiltering
}
}

// Setup the blocking query
opts := blockingOptions{
queryOpts: &args.QueryOptions,
Expand Down Expand Up @@ -425,13 +434,22 @@ func (e *Eval) List(args *structs.EvalListRequest, reply *structs.EvalListRespon
})

var evals []*structs.Evaluation
paginator := state.NewPaginator(iter, args.QueryOptions,
paginator, err := state.NewPaginator(iter, args.QueryOptions,
func(raw interface{}) {
eval := raw.(*structs.Evaluation)
evals = append(evals, eval)
})
if err != nil {
return structs.NewErrRPCCodedf(
http.StatusBadRequest, "failed to create result paginator: %v", err)
}

nextToken, err := paginator.Page()
if err != nil {
return structs.NewErrRPCCodedf(
http.StatusBadRequest, "failed to read result page: %v", err)
}

nextToken := paginator.Page()
reply.QueryMeta.NextToken = nextToken
reply.Evaluations = evals

Expand Down
Loading