diff --git a/.changelog/18322.txt b/.changelog/18322.txt new file mode 100644 index 000000000..9be8c2cfa --- /dev/null +++ b/.changelog/18322.txt @@ -0,0 +1,3 @@ +```release-note:bug +catalog api: fixes a bug with catalog api where filter query parameter was not working correctly for the `/v1/catalog/services` endpoint +``` \ No newline at end of file diff --git a/agent/consul/catalog_endpoint.go b/agent/consul/catalog_endpoint.go index 446037a31..c53c41356 100644 --- a/agent/consul/catalog_endpoint.go +++ b/agent/consul/catalog_endpoint.go @@ -596,7 +596,7 @@ func (c *Catalog) ListServices(args *structs.DCSpecificRequest, reply *structs.I if len(args.NodeMetaFilters) > 0 { reply.Index, serviceNodes, err = state.ServicesByNodeMeta(ws, args.NodeMetaFilters, &args.EnterpriseMeta, args.PeerName) } else { - reply.Index, serviceNodes, err = state.Services(ws, &args.EnterpriseMeta, args.PeerName) + reply.Index, serviceNodes, err = state.Services(ws, &args.EnterpriseMeta, args.PeerName, args.Filter != "") } if err != nil { return err diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index 4c2770a37..ceb63fab3 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -1218,7 +1218,7 @@ func terminatingGatewayVirtualIPsSupported(tx ReadTxn, ws memdb.WatchSet) (bool, } // Services returns all services along with a list of associated tags. -func (s *Store) Services(ws memdb.WatchSet, entMeta *acl.EnterpriseMeta, peerName string) (uint64, []*structs.ServiceNode, error) { +func (s *Store) Services(ws memdb.WatchSet, entMeta *acl.EnterpriseMeta, peerName string, joinServiceNodes bool) (uint64, structs.ServiceNodes, error) { tx := s.db.Txn(false) defer tx.Abort() @@ -1236,6 +1236,13 @@ func (s *Store) Services(ws memdb.WatchSet, entMeta *acl.EnterpriseMeta, peerNam for service := services.Next(); service != nil; service = services.Next() { result = append(result, service.(*structs.ServiceNode)) } + if joinServiceNodes { + parsedResult, err := parseServiceNodes(tx, ws, result, entMeta, peerName) + if err != nil { + return 0, nil, fmt.Errorf("failed querying and parsing services :%s", err) + } + return idx, parsedResult, nil + } return idx, result, nil } @@ -1311,6 +1318,7 @@ func (s *Store) ServicesByNodeMeta(ws memdb.WatchSet, filters map[string]string, EnterpriseMeta: *entMeta, PeerName: peerName, }) + if err != nil { return 0, nil, fmt.Errorf("failed nodes lookup: %s", err) } @@ -1330,7 +1338,6 @@ func (s *Store) ServicesByNodeMeta(ws memdb.WatchSet, filters map[string]string, if len(filters) > 1 && !structs.SatisfiesMetaFilters(n.Meta, filters) { continue } - // List all the services on the node services, err := catalogServiceListByNode(tx, n.Node, entMeta, n.PeerName, false) if err != nil { diff --git a/agent/consul/state/catalog_test.go b/agent/consul/state/catalog_test.go index 763d29c8c..7ca578307 100644 --- a/agent/consul/state/catalog_test.go +++ b/agent/consul/state/catalog_test.go @@ -2178,7 +2178,7 @@ func TestStateStore_Services(t *testing.T) { // Listing with no results returns an empty list. ws := memdb.NewWatchSet() - idx, services, err := s.Services(ws, nil, "") + idx, services, err := s.Services(ws, nil, "", false) if err != nil { t.Fatalf("err: %s", err) } @@ -2223,7 +2223,7 @@ func TestStateStore_Services(t *testing.T) { // Pull all the services. ws = memdb.NewWatchSet() - idx, services, err = s.Services(ws, nil, "") + idx, services, err = s.Services(ws, nil, "", false) if err != nil { t.Fatalf("err: %s", err) } @@ -2232,7 +2232,7 @@ func TestStateStore_Services(t *testing.T) { } // Verify the result. - expected := []*structs.ServiceNode{ + expected := structs.ServiceNodes{ ns1Dogs.ToServiceNode("node1"), ns1.ToServiceNode("node1"), ns2.ToServiceNode("node2"), diff --git a/api/catalog_test.go b/api/catalog_test.go index 2b0a4097b..708392c42 100644 --- a/api/catalog_test.go +++ b/api/catalog_test.go @@ -220,6 +220,73 @@ func TestAPI_CatalogServices_NodeMetaFilter(t *testing.T) { }) } +func TestAPI_CatalogServices_FilterExpr_NodeMeta(t *testing.T) { + t.Parallel() + meta := map[string]string{"somekey": "somevalue", "synthetic": "true"} + c, s := makeClientWithConfig(t, nil, func(conf *testutil.TestServerConfig) { + conf.NodeMeta = meta + }) + defer s.Stop() + + catalog := c.Catalog() + // Make sure we get the service back when filtering by filter expression + retry.Run(t, func(r *retry.R) { + services, meta, err := catalog.Services(&QueryOptions{Filter: "NodeMeta[\"synthetic\"] == true and NodeMeta[\"somekey\"] == somevalue"}) + if err != nil { + r.Fatal(err) + } + + if meta.LastIndex == 0 { + r.Fatalf("Bad: %v", meta) + } + if len(services) == 0 { + r.Fatalf("Bad: %v", services) + } + }) + retry.Run(t, func(r *retry.R) { + services, meta, err := catalog.Services(&QueryOptions{Filter: "NodeMeta.synthetic == true"}) + if err != nil { + r.Fatal(err) + } + + if meta.LastIndex == 0 { + r.Fatalf("Bad: %v", meta) + } + + if len(services) == 0 { + r.Fatalf("Bad: %v", services) + } + }) + retry.Run(t, func(r *retry.R) { + services, meta, err := catalog.Services(&QueryOptions{Filter: "NodeMeta.somekey == somevalue"}) + if err != nil { + r.Fatal(err) + } + + if meta.LastIndex == 0 { + r.Fatalf("Bad: %v", meta) + } + + if len(services) == 0 { + r.Fatalf("Bad: %v", services) + } + }) + retry.Run(t, func(r *retry.R) { + services, meta, err := catalog.Services(&QueryOptions{Filter: "NodeMeta.nope == nope"}) + if err != nil { + r.Fatal(err) + } + + if meta.LastIndex == 0 { + r.Fatalf("Bad: %v", meta) + } + + if len(services) != 0 { + r.Fatalf("Bad: %v", services) + } + }) +} + func TestAPI_CatalogService(t *testing.T) { t.Parallel() c, s := makeClient(t)