Merge 7c0660f into backport/gh-17642-watch-checks-tag/suitably-dynami…

…c-owl
hashicorp · Jun 30, 2023 · 6b82460 · 6b82460
2 parents 35eda8b + 7c0660f
commit 6b82460
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 87 deletions.
diff --git a/agent/health_endpoint.go b/agent/health_endpoint.go
@@ -1,7 +1,6 @@
 package agent
 
 import (
-	"fmt"
 	"net/http"
 	"net/url"
 	"strconv"
@@ -35,18 +34,6 @@ func (s *HTTPHandlers) HealthChecksInState(resp http.ResponseWriter, req *http.R
 		return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing check state"}
 	}
 
-	// build tag filter
-	params := req.URL.Query()
-	if tags, ok := params["tag"]; ok {
-		for i, tag := range tags {
-			expr := fmt.Sprintf(`%s in ServiceTags`, tag)
-			if i < len(tags)-1 {
-				expr += " and "
-			}
-			args.Filter += expr
-		}
-	}
-
 	// Make the RPC request
 	var out structs.IndexedHealthChecks
 	defer setMeta(resp, &out.QueryMeta)
@@ -137,18 +124,6 @@ func (s *HTTPHandlers) HealthServiceChecks(resp http.ResponseWriter, req *http.R
 		return nil, HTTPError{StatusCode: http.StatusBadRequest, Reason: "Missing service name"}
 	}
 
-	// build tag filter
-	params := req.URL.Query()
-	if tags, ok := params["tag"]; ok {
-		for i, tag := range tags {
-			expr := fmt.Sprintf(`%s in ServiceTags`, tag)
-			if i < len(tags)-1 {
-				expr += " and "
-			}
-			args.Filter += expr
-		}
-	}
-
 	// Make the RPC request
 	var out structs.IndexedHealthChecks
 	defer setMeta(resp, &out.QueryMeta)

diff --git a/api/health.go b/api/health.go
@@ -258,17 +258,7 @@ func (h *Health) Node(node string, q *QueryOptions) (HealthChecks, *QueryMeta, e
 
 // Checks is used to return the checks associated with a service
 func (h *Health) Checks(service string, q *QueryOptions) (HealthChecks, *QueryMeta, error) {
-	return h.ChecksTags(service, nil, q)
-}
-
-// ChecksTags is used to return the checks associated with a service filtered by tags
-func (h *Health) ChecksTags(service string, tags []string, q *QueryOptions) (HealthChecks, *QueryMeta, error) {
 	r := h.c.newRequest("GET", "/v1/health/checks/"+service)
-	if len(tags) > 0 {
-		for _, tag := range tags {
-			r.params.Add("tag", tag)
-		}
-	}
 	r.setQueryOptions(q)
 	rtt, resp, err := h.c.doRequest(r)
 	if err != nil {
@@ -373,12 +363,6 @@ func (h *Health) service(service string, tags []string, passingOnly bool, q *Que
 // State is used to retrieve all the checks in a given state.
 // The wildcard "any" state can also be used for all checks.
 func (h *Health) State(state string, q *QueryOptions) (HealthChecks, *QueryMeta, error) {
-	return h.StateTags(state, nil, q)
-}
-
-// StateTags is used to retrieve all the checks in a given state and tags.
-// The wildcard "any" state can also be used for all checks.
-func (h *Health) StateTags(state string, tags []string, q *QueryOptions) (HealthChecks, *QueryMeta, error) {
 	switch state {
 	case HealthAny:
 	case HealthWarning:
@@ -389,13 +373,6 @@ func (h *Health) StateTags(state string, tags []string, q *QueryOptions) (Health
 	}
 	r := h.c.newRequest("GET", "/v1/health/state/"+state)
 	r.setQueryOptions(q)
-
-	if len(tags) > 0 {
-		for _, tag := range tags {
-			r.params.Add("tag", tag)
-		}
-	}
-
 	rtt, resp, err := h.c.doRequest(r)
 	if err != nil {
 		return nil, nil, err

diff --git a/api/watch/funcs.go b/api/watch/funcs.go
@@ -172,43 +172,41 @@ func checksWatch(params map[string]interface{}) (WatcherFunc, error) {
 		return nil, err
 	}
 
-	var service, state string
+	var service, state, filter string
 	if err := assignValue(params, "service", &service); err != nil {
 		return nil, err
 	}
 	if err := assignValue(params, "state", &state); err != nil {
 		return nil, err
 	}
+	if err := assignValue(params, "filter", &filter); err != nil {
+		return nil, err
+	}
 	if service != "" && state != "" {
 		return nil, fmt.Errorf("Cannot specify service and state")
 	}
 	if service == "" && state == "" {
 		state = "any"
 	}
 
-	var (
-		tags []string
-	)
-	if err := assignValueStringSlice(params, "tag", &tags); err != nil {
-		return nil, err
-	}
-
 	fn := func(p *Plan) (BlockingParamVal, interface{}, error) {
 		health := p.client.Health()
 		opts := makeQueryOptionsWithContext(p, stale)
 		defer p.cancelFunc()
 		var checks []*consulapi.HealthCheck
 		var meta *consulapi.QueryMeta
 		var err error
+		if filter != "" {
+			opts.Filter = filter
+		}
 		if state != "" {
-			checks, meta, err = health.StateTags(state, tags, &opts)
+			checks, meta, err = health.State(state, &opts)
 		} else {
-			checks, meta, err = health.ChecksTags(service, tags, &opts)
+			checks, meta, err = health.Checks(service, &opts)
 		}
 		if err != nil {
 			return nil, nil, err
 		}
-
 		return WaitIndexVal(meta.LastIndex), checks, err
 	}
 	return fn, nil

diff --git a/api/watch/funcs_test.go b/api/watch/funcs_test.go
@@ -769,7 +769,7 @@ func TestChecksWatch_Service(t *testing.T) {
 	}
 }
 
-func TestChecksWatch_Service_Tag(t *testing.T) {
+func TestChecksWatch_Service_Filter(t *testing.T) {
 	t.Parallel()
 	c, s := makeClient(t)
 	defer s.Stop()
@@ -781,7 +781,7 @@ func TestChecksWatch_Service_Tag(t *testing.T) {
 		notifyCh = make(chan struct{})
 	)
 
-	plan := mustParse(t, `{"type":"checks", "service":"foobar", "tag":["b", "a"]}`)
+	plan := mustParse(t, `{"type":"checks", "filter":"b in ServiceTags and a in ServiceTags"}`)
 	plan.Handler = func(idx uint64, raw interface{}) {
 		if raw == nil {
 			return // ignore
@@ -808,8 +808,6 @@ func TestChecksWatch_Service_Tag(t *testing.T) {
 	<-notifyCh
 	{
 		catalog := c.Catalog()
-
-		// we want to find this one
 		reg := &api.CatalogRegistration{
 			Node:       "foobar",
 			Address:    "1.1.1.1",
@@ -830,28 +828,6 @@ func TestChecksWatch_Service_Tag(t *testing.T) {
 		if _, err := catalog.Register(reg, nil); err != nil {
 			t.Fatalf("err: %v", err)
 		}
-
-		// we don't want to find this one
-		reg = &api.CatalogRegistration{
-			Node:       "bar",
-			Address:    "2.2.2.2",
-			Datacenter: "dc1",
-			Service: &api.AgentService{
-				ID:      "foobar",
-				Service: "foobar",
-				Tags:    []string{"a"},
-			},
-			Check: &api.AgentCheck{
-				Node:      "bar",
-				CheckID:   "foobar",
-				Name:      "foobar",
-				Status:    api.HealthPassing,
-				ServiceID: "foobar",
-			},
-		}
-		if _, err := catalog.Register(reg, nil); err != nil {
-			t.Fatalf("err: %v", err)
-		}
 	}
 
 	// Wait for second wakeup.
@@ -873,7 +849,7 @@ func TestChecksWatch_Service_Tag(t *testing.T) {
 	}
 }
 
-func TestChecksWatch_Tag(t *testing.T) {
+func TestChecksWatch_Filter(t *testing.T) {
 	t.Parallel()
 	c, s := makeClient(t)
 	defer s.Stop()
@@ -885,7 +861,7 @@ func TestChecksWatch_Tag(t *testing.T) {
 		notifyCh = make(chan struct{})
 	)
 
-	plan := mustParse(t, `{"type":"checks", "tag":["b", "a"]}`)
+	plan := mustParse(t, `{"type":"checks", "filter":"b in ServiceTags and a in ServiceTags"}`)
 	plan.Handler = func(idx uint64, raw interface{}) {
 		if raw == nil {
 			return // ignore

diff --git a/command/watch/watch.go b/command/watch/watch.go
@@ -42,6 +42,7 @@ type cmd struct {
 	state       string
 	name        string
 	shell       bool
+	filter      string
 }
 
 func (c *cmd) init() {
@@ -68,6 +69,7 @@ func (c *cmd) init() {
 		"Specifies the states to watch. Optional for 'checks' type.")
 	c.flags.StringVar(&c.name, "name", "",
 		"Specifies an event name to watch. Only for 'event' type.")
+	c.flags.StringVar(&c.filter, "filter", "", "Filter to use with the request")
 
 	c.http = &flags.HTTPFlags{}
 	flags.Merge(c.flags, c.http.ClientFlags())
@@ -125,6 +127,9 @@ func (c *cmd) Run(args []string) int {
 	if c.service != "" {
 		params["service"] = c.service
 	}
+	if c.filter != "" {
+		params["filter"] = c.filter
+	}
 	if len(c.tag) > 0 {
 		params["tag"] = c.tag
 	}