From cef8e3d27b2bab062d44e8d55f40a2e97c8efe3b Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Fri, 28 Jul 2023 17:58:23 +0530 Subject: [PATCH 01/27] logs for debugging --- agent/consul/catalog_endpoint.go | 4 ++++ agent/consul/state/catalog.go | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/agent/consul/catalog_endpoint.go b/agent/consul/catalog_endpoint.go index 32b8067e5a1f..65c77e19f656 100644 --- a/agent/consul/catalog_endpoint.go +++ b/agent/consul/catalog_endpoint.go @@ -561,6 +561,8 @@ func (c *Catalog) ListServices(args *structs.DCSpecificRequest, reply *structs.I return err } + fmt.Println("in list services") + // Supporting querying by PeerName in this API would require modifying the return type or the ACL // filtering logic so that it can be made aware that the data queried is coming from a peer. // Currently the ACL filter will receive plain name strings with no awareness of the peer name, @@ -594,8 +596,10 @@ func (c *Catalog) ListServices(args *structs.DCSpecificRequest, reply *structs.I var err error var serviceNodes structs.ServiceNodes if len(args.NodeMetaFilters) > 0 { + fmt.Println("in node meta filters > 0") reply.Index, serviceNodes, err = state.ServicesByNodeMeta(ws, args.NodeMetaFilters, &args.EnterpriseMeta, args.PeerName) } else { + fmt.Println("in node meta filters == 0") reply.Index, serviceNodes, err = state.Services(ws, &args.EnterpriseMeta, args.PeerName) } if err != nil { diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index 4e9fcf716c47..f298442246ae 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -1311,6 +1311,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) } @@ -1327,10 +1328,11 @@ func (s *Store) ServicesByNodeMeta(ws memdb.WatchSet, filters map[string]string, var result structs.ServiceNodes for node := nodes.Next(); node != nil; node = nodes.Next() { n := node.(*structs.Node) + fmt.Println("fetched nodes = ", n.Node) if len(filters) > 1 && !structs.SatisfiesMetaFilters(n.Meta, filters) { continue } - + fmt.Println("after filter > 1 continue") // List all the services on the node services, err := catalogServiceListByNode(tx, n.Node, entMeta, n.PeerName, false) if err != nil { @@ -1339,6 +1341,7 @@ func (s *Store) ServicesByNodeMeta(ws memdb.WatchSet, filters map[string]string, ws.AddWithLimit(watchLimit, services.WatchCh(), allServicesCh) for service := services.Next(); service != nil; service = services.Next() { + fmt.Println("got services", service.(*structs.ServiceNode).ServiceID) result = append(result, service.(*structs.ServiceNode)) } } From 79fdf19ce319cd28e685e7f2963dc7d9e1f1ac4d Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Fri, 28 Jul 2023 23:25:22 +0530 Subject: [PATCH 02/27] Init --- agent/catalog_endpoint.go | 4 ++++ agent/consul/catalog_endpoint.go | 5 ----- agent/consul/state/catalog.go | 11 ++++++----- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/agent/catalog_endpoint.go b/agent/catalog_endpoint.go index ad72c4b47f35..51b8d0a993a9 100644 --- a/agent/catalog_endpoint.go +++ b/agent/catalog_endpoint.go @@ -277,7 +277,11 @@ func (s *HTTPHandlers) CatalogServices(resp http.ResponseWriter, req *http.Reque return nil, err } + var filterExpression string + s.parseFilter(req, &filterExpression) + args.NodeMetaFilters = s.parseMetaFilter(req) + args.Filter = filterExpression if done := s.parse(resp, req, &args.Datacenter, &args.QueryOptions); done { return nil, nil } diff --git a/agent/consul/catalog_endpoint.go b/agent/consul/catalog_endpoint.go index 65c77e19f656..02a936eea1a6 100644 --- a/agent/consul/catalog_endpoint.go +++ b/agent/consul/catalog_endpoint.go @@ -561,8 +561,6 @@ func (c *Catalog) ListServices(args *structs.DCSpecificRequest, reply *structs.I return err } - fmt.Println("in list services") - // Supporting querying by PeerName in this API would require modifying the return type or the ACL // filtering logic so that it can be made aware that the data queried is coming from a peer. // Currently the ACL filter will receive plain name strings with no awareness of the peer name, @@ -596,10 +594,8 @@ func (c *Catalog) ListServices(args *structs.DCSpecificRequest, reply *structs.I var err error var serviceNodes structs.ServiceNodes if len(args.NodeMetaFilters) > 0 { - fmt.Println("in node meta filters > 0") reply.Index, serviceNodes, err = state.ServicesByNodeMeta(ws, args.NodeMetaFilters, &args.EnterpriseMeta, args.PeerName) } else { - fmt.Println("in node meta filters == 0") reply.Index, serviceNodes, err = state.Services(ws, &args.EnterpriseMeta, args.PeerName) } if err != nil { @@ -610,7 +606,6 @@ func (c *Catalog) ListServices(args *structs.DCSpecificRequest, reply *structs.I reply.QueryMeta.NotModified = true return nil } - raw, err := filter.Execute(serviceNodes) if err != nil { return err diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index f298442246ae..b4d1787025d1 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) (uint64, structs.ServiceNodes, error) { tx := s.db.Txn(false) defer tx.Abort() @@ -1236,7 +1236,11 @@ 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)) } - return idx, result, nil + 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 } func (s *Store) ServiceList(ws memdb.WatchSet, entMeta *acl.EnterpriseMeta, peerName string) (uint64, structs.ServiceList, error) { @@ -1328,11 +1332,9 @@ func (s *Store) ServicesByNodeMeta(ws memdb.WatchSet, filters map[string]string, var result structs.ServiceNodes for node := nodes.Next(); node != nil; node = nodes.Next() { n := node.(*structs.Node) - fmt.Println("fetched nodes = ", n.Node) if len(filters) > 1 && !structs.SatisfiesMetaFilters(n.Meta, filters) { continue } - fmt.Println("after filter > 1 continue") // List all the services on the node services, err := catalogServiceListByNode(tx, n.Node, entMeta, n.PeerName, false) if err != nil { @@ -1341,7 +1343,6 @@ func (s *Store) ServicesByNodeMeta(ws memdb.WatchSet, filters map[string]string, ws.AddWithLimit(watchLimit, services.WatchCh(), allServicesCh) for service := services.Next(); service != nil; service = services.Next() { - fmt.Println("got services", service.(*structs.ServiceNode).ServiceID) result = append(result, service.(*structs.ServiceNode)) } } From 4fa91a699d68a75415a85a6fc8db97428bd3b509 Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Fri, 28 Jul 2023 23:31:44 +0530 Subject: [PATCH 03/27] white spaces fix --- agent/consul/catalog_endpoint.go | 1 + agent/consul/state/catalog.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/agent/consul/catalog_endpoint.go b/agent/consul/catalog_endpoint.go index 02a936eea1a6..32b8067e5a1f 100644 --- a/agent/consul/catalog_endpoint.go +++ b/agent/consul/catalog_endpoint.go @@ -606,6 +606,7 @@ func (c *Catalog) ListServices(args *structs.DCSpecificRequest, reply *structs.I reply.QueryMeta.NotModified = true return nil } + raw, err := filter.Execute(serviceNodes) if err != nil { return err diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index b4d1787025d1..d24523986807 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -1315,7 +1315,6 @@ 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) } @@ -1335,6 +1334,7 @@ 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 { From 92ed62db85100836ce0a662603370693095f9cf9 Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Fri, 28 Jul 2023 23:35:07 +0530 Subject: [PATCH 04/27] added change log --- .changelog/18322.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 .changelog/18322.txt diff --git a/.changelog/18322.txt b/.changelog/18322.txt new file mode 100644 index 000000000000..545e6ecff13e --- /dev/null +++ b/.changelog/18322.txt @@ -0,0 +1,4 @@ +```release-note:bug +catalog api: fix a bug with catalog api where filter query parameter was not working correctly. endpoint fixed - +/v1/catalog/services +``` \ No newline at end of file From 1a5394468a2c54a6d00be7b016bd436d8532df2f Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Sat, 29 Jul 2023 00:09:31 +0530 Subject: [PATCH 05/27] Fix tests --- agent/consul/state/catalog_test.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/agent/consul/state/catalog_test.go b/agent/consul/state/catalog_test.go index e6b279580b03..f62c45b46be2 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, &acl.EnterpriseMeta{}, "") if err != nil { t.Fatalf("err: %s", err) } @@ -2199,10 +2199,14 @@ func TestStateStore_Services(t *testing.T) { Port: 1111, } ns1.EnterpriseMeta.Normalize() + ns1.Tags = []string{} + ns1.Meta = map[string]string{} if err := s.EnsureService(2, "node1", ns1); err != nil { t.Fatalf("err: %s", err) } ns1Dogs := testRegisterService(t, s, 3, "node1", "dogs") + ns1Dogs.Tags = []string{} + ns1Dogs.Meta = map[string]string{} ns1Dogs.EnterpriseMeta.Normalize() testRegisterNode(t, s, 4, "node2") @@ -2213,7 +2217,9 @@ func TestStateStore_Services(t *testing.T) { Address: "1.1.1.1", Port: 1111, } + ns2.Tags = []string{} ns2.EnterpriseMeta.Normalize() + ns2t.Meta = map[string]string{} if err := s.EnsureService(5, "node2", ns2); err != nil { t.Fatalf("err: %s", err) } @@ -2223,7 +2229,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, &acl.EnterpriseMeta{}, "") if err != nil { t.Fatalf("err: %s", err) } @@ -2232,7 +2238,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"), From c7c5876001bd0d3c66e011f8d829fbed5fa99dc3 Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Sat, 29 Jul 2023 00:09:50 +0530 Subject: [PATCH 06/27] fix typo --- agent/consul/state/catalog_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/consul/state/catalog_test.go b/agent/consul/state/catalog_test.go index f62c45b46be2..453beeb05ec8 100644 --- a/agent/consul/state/catalog_test.go +++ b/agent/consul/state/catalog_test.go @@ -2219,7 +2219,7 @@ func TestStateStore_Services(t *testing.T) { } ns2.Tags = []string{} ns2.EnterpriseMeta.Normalize() - ns2t.Meta = map[string]string{} + ns2.Meta = map[string]string{} if err := s.EnsureService(5, "node2", ns2); err != nil { t.Fatalf("err: %s", err) } From db1039a84f8e3e5cc7643851777073acf0e05597 Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Sat, 29 Jul 2023 08:03:51 +0530 Subject: [PATCH 07/27] using queryoptionfilter to populate args.filter --- agent/catalog_endpoint.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/agent/catalog_endpoint.go b/agent/catalog_endpoint.go index 51b8d0a993a9..4c05892c7514 100644 --- a/agent/catalog_endpoint.go +++ b/agent/catalog_endpoint.go @@ -277,14 +277,11 @@ func (s *HTTPHandlers) CatalogServices(resp http.ResponseWriter, req *http.Reque return nil, err } - var filterExpression string - s.parseFilter(req, &filterExpression) - args.NodeMetaFilters = s.parseMetaFilter(req) - args.Filter = filterExpression if done := s.parse(resp, req, &args.Datacenter, &args.QueryOptions); done { return nil, nil } + args.Filter = args.QueryOptions.Filter var out structs.IndexedServices defer setMeta(resp, &out.QueryMeta) From ecc8dbf0625ec1b99ca71e59f731135e2c1a240f Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Sat, 16 Sep 2023 13:10:27 +0530 Subject: [PATCH 08/27] tests --- api/catalog_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/api/catalog_test.go b/api/catalog_test.go index 2b0a4097b332..6028b6f4dd73 100644 --- a/api/catalog_test.go +++ b/api/catalog_test.go @@ -220,6 +220,49 @@ func TestAPI_CatalogServices_NodeMetaFilter(t *testing.T) { }) } +func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { + t.Parallel() + meta := map[string]string{"somekey": "somevalue"} + c, s := makeClientWithConfig(t, nil, func(conf *testutil.TestServerConfig) { + conf.NodeMeta = meta + conf.NodeName = "foobar" + }) + defer s.Stop() + + // Register service and proxy instances to test against. + service := &AgentService{ + ID: "redis1", + Service: "redis", + Port: 8000, + Connect: &AgentServiceConnect{Native: true}, + } + + reg := &CatalogRegistration{ + Datacenter: "dc1", + Node: "foobar", + Address: "192.168.10.10", + Service: service, + } + + catalog := c.Catalog() + retry.Run(t, func(r *retry.R) { + if _, err := catalog.Register(reg, nil); err != nil { + r.Fatal(err) + } + services, meta, err := catalog.Services(&QueryOptions{NodeMeta: meta}) + if err != nil { + r.Fatal(err) + } + + if meta.LastIndex == 0 { + r.Fatalf("Bad: %v", meta) + } + + if len(services) == 1 { + r.Fatalf("Bad: %v", services) + } + }) +} func TestAPI_CatalogService(t *testing.T) { t.Parallel() c, s := makeClient(t) From b8dfd8714105cdd16ea310d364e2c703fc47a911 Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Sat, 16 Sep 2023 13:49:25 +0530 Subject: [PATCH 09/27] fix test --- api/catalog_test.go | 82 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 74 insertions(+), 8 deletions(-) diff --git a/api/catalog_test.go b/api/catalog_test.go index 6028b6f4dd73..68e4a8bb61e9 100644 --- a/api/catalog_test.go +++ b/api/catalog_test.go @@ -222,7 +222,7 @@ func TestAPI_CatalogServices_NodeMetaFilter(t *testing.T) { func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { t.Parallel() - meta := map[string]string{"somekey": "somevalue"} + meta := map[string]string{"somekey": "somevalue", "synthetic-node": "true"} c, s := makeClientWithConfig(t, nil, func(conf *testutil.TestServerConfig) { conf.NodeMeta = meta conf.NodeName = "foobar" @@ -232,37 +232,103 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { // Register service and proxy instances to test against. service := &AgentService{ ID: "redis1", - Service: "redis", + Service: "redis1", Port: 8000, Connect: &AgentServiceConnect{Native: true}, } - reg := &CatalogRegistration{ + reg1 := &CatalogRegistration{ + Datacenter: "dc1", + Node: "foobar1", + Service: service, + SkipNodeUpdate: true, + } + + service2 := &AgentService{ + ID: "redis2", + Service: "redis2", + Port: 8000, + Connect: &AgentServiceConnect{Native: true}, + } + + reg2 := &CatalogRegistration{ Datacenter: "dc1", - Node: "foobar", + Node: "foobar2", Address: "192.168.10.10", - Service: service, + Service: service2, + NodeMeta: map[string]string{"somekey": "somevalue"}, + } + + service3 := &AgentService{ + ID: "redis3", + Service: "redis3", + Port: 8000, + Connect: &AgentServiceConnect{Native: true}, + } + + reg3 := &CatalogRegistration{ + Datacenter: "dc1", + Node: "foobar3", + Address: "192.168.10.10", + Service: service3, + NodeMeta: map[string]string{"synthetic-node": "true"}, } + proxyReg := testUnmanagedProxyRegistration(t) + catalog := c.Catalog() retry.Run(t, func(r *retry.R) { - if _, err := catalog.Register(reg, nil); err != nil { + _, err := catalog.Register(proxyReg, nil) + r.Check(err) + + if _, err := catalog.Register(reg1, nil); err != nil { r.Fatal(err) } - services, meta, err := catalog.Services(&QueryOptions{NodeMeta: meta}) + if _, err := catalog.Register(reg2, nil); err != nil { + r.Fatal(err) + } + if _, err := catalog.Register(reg3, nil); err != nil { + r.Fatal(err) + } + services, meta, err := catalog.Services(&QueryOptions{NodeMeta: map[string]string{"somekey": "somevalue"}}) if err != nil { r.Fatal(err) } + if meta.LastIndex == 0 { + r.Fatalf("Bad: %v", meta) + } + if len(services) != 1 { + r.Fatalf("Bad: %v", services) + } + }) + retry.Run(t, func(r *retry.R) { + _, err := catalog.Register(proxyReg, nil) + r.Check(err) + + if _, err := catalog.Register(reg1, nil); err != nil { + r.Fatal(err) + } + if _, err := catalog.Register(reg2, nil); err != nil { + r.Fatal(err) + } + if _, err := catalog.Register(reg3, nil); err != nil { + r.Fatal(err) + } + services, meta, err := catalog.Services(&QueryOptions{NodeMeta: map[string]string{"synthetic-node": "true"}}) + if err != nil { + r.Fatal(err) + } if meta.LastIndex == 0 { r.Fatalf("Bad: %v", meta) } - if len(services) == 1 { + if len(services) != 2 { r.Fatalf("Bad: %v", services) } }) } + func TestAPI_CatalogService(t *testing.T) { t.Parallel() c, s := makeClient(t) From 174f8b0c28f3851a3664233f9ebd93760c51fdfa Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Sat, 16 Sep 2023 13:56:35 +0530 Subject: [PATCH 10/27] fix tests --- api/catalog_test.go | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/api/catalog_test.go b/api/catalog_test.go index 68e4a8bb61e9..c1605b2c05d7 100644 --- a/api/catalog_test.go +++ b/api/catalog_test.go @@ -225,7 +225,6 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { meta := map[string]string{"somekey": "somevalue", "synthetic-node": "true"} c, s := makeClientWithConfig(t, nil, func(conf *testutil.TestServerConfig) { conf.NodeMeta = meta - conf.NodeName = "foobar" }) defer s.Stop() @@ -242,6 +241,7 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { Node: "foobar1", Service: service, SkipNodeUpdate: true, + NodeMeta: map[string]string{"key": "value"}, } service2 := &AgentService{ @@ -271,7 +271,7 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { Node: "foobar3", Address: "192.168.10.10", Service: service3, - NodeMeta: map[string]string{"synthetic-node": "true"}, + NodeMeta: map[string]string{"synthetic-node": "true", "somekey": "somevalue"}, } proxyReg := testUnmanagedProxyRegistration(t) @@ -294,11 +294,12 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { if err != nil { r.Fatal(err) } + delete(services, "consul") if meta.LastIndex == 0 { r.Fatalf("Bad: %v", meta) } - if len(services) != 1 { + if len(services) != 2 { r.Fatalf("Bad: %v", services) } }) @@ -319,11 +320,38 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { if err != nil { r.Fatal(err) } + delete(services, "consul") if meta.LastIndex == 0 { r.Fatalf("Bad: %v", meta) } - if len(services) != 2 { + if len(services) != 1 { + r.Fatalf("Bad: %v", services) + } + }) + retry.Run(t, func(r *retry.R) { + _, err := catalog.Register(proxyReg, nil) + r.Check(err) + + if _, err := catalog.Register(reg1, nil); err != nil { + r.Fatal(err) + } + if _, err := catalog.Register(reg2, nil); err != nil { + r.Fatal(err) + } + if _, err := catalog.Register(reg3, nil); err != nil { + r.Fatal(err) + } + services, meta, err := catalog.Services(&QueryOptions{NodeMeta: map[string]string{"key": "value"}}) + delete(services, "consul") + if err != nil { + r.Fatal(err) + } + if meta.LastIndex == 0 { + r.Fatalf("Bad: %v", meta) + } + + if len(services) != 1 { r.Fatalf("Bad: %v", services) } }) From 6d1cee7c69cad4b41da04ef5f6c55e99a0e1aca3 Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Sat, 16 Sep 2023 14:14:59 +0530 Subject: [PATCH 11/27] fix tests --- api/catalog_test.go | 61 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 9 deletions(-) diff --git a/api/catalog_test.go b/api/catalog_test.go index c1605b2c05d7..b169250bcaa0 100644 --- a/api/catalog_test.go +++ b/api/catalog_test.go @@ -222,13 +222,25 @@ func TestAPI_CatalogServices_NodeMetaFilter(t *testing.T) { func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { t.Parallel() - meta := map[string]string{"somekey": "somevalue", "synthetic-node": "true"} - c, s := makeClientWithConfig(t, nil, func(conf *testutil.TestServerConfig) { - conf.NodeMeta = meta - }) + c, s := makeClientWithConfig(t, nil, nil) defer s.Stop() // Register service and proxy instances to test against. + service0 := &AgentService{ + ID: "redis0", + Service: "redis0", + Port: 8001, + Connect: &AgentServiceConnect{Native: true}, + } + + reg0 := &CatalogRegistration{ + Datacenter: "dc1", + Node: "foobar0", + Service: service0, + SkipNodeUpdate: true, + NodeMeta: map[string]string{"somekey": "somevalue", "synthetic-node": "true"}, + } + service := &AgentService{ ID: "redis1", Service: "redis1", @@ -276,9 +288,35 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { proxyReg := testUnmanagedProxyRegistration(t) + proxyService := &AgentService{ + ID: proxyReg.Service.Proxy.DestinationServiceID, + Service: proxyReg.Service.Proxy.DestinationServiceName, + Port: 8000, + } + + check := &AgentCheck{ + Node: "foobar", + CheckID: "service:" + proxyService.ID, + Name: "Redis health check", + Notes: "Script based health check", + Status: HealthPassing, + ServiceID: proxyService.ID, + } + + proxyCatalogReg := &CatalogRegistration{ + Datacenter: "dc1", + Node: "foobar", + Address: "192.168.10.10", + Service: proxyService, + Check: check, + } + catalog := c.Catalog() retry.Run(t, func(r *retry.R) { - _, err := catalog.Register(proxyReg, nil) + _, err := catalog.Register(proxyCatalogReg, nil) + r.Check(err) + + _, err = catalog.Register(reg0, nil) r.Check(err) if _, err := catalog.Register(reg1, nil); err != nil { @@ -295,16 +333,19 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { r.Fatal(err) } delete(services, "consul") + if meta.LastIndex == 0 { r.Fatalf("Bad: %v", meta) } - if len(services) != 2 { + if len(services) != 3 { r.Fatalf("Bad: %v", services) } }) retry.Run(t, func(r *retry.R) { - _, err := catalog.Register(proxyReg, nil) + _, err := catalog.Register(proxyCatalogReg, nil) + r.Check(err) + _, err = catalog.Register(reg0, nil) r.Check(err) if _, err := catalog.Register(reg1, nil); err != nil { @@ -325,12 +366,14 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { r.Fatalf("Bad: %v", meta) } - if len(services) != 1 { + if len(services) != 2 { r.Fatalf("Bad: %v", services) } }) retry.Run(t, func(r *retry.R) { - _, err := catalog.Register(proxyReg, nil) + _, err := catalog.Register(proxyCatalogReg, nil) + r.Check(err) + _, err = catalog.Register(reg0, nil) r.Check(err) if _, err := catalog.Register(reg1, nil); err != nil { From 8b21150b695984a33dca5ca3ddbff48adec4d9ff Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Sat, 16 Sep 2023 14:18:18 +0530 Subject: [PATCH 12/27] fix tests --- api/catalog_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/catalog_test.go b/api/catalog_test.go index b169250bcaa0..dee91eb3d423 100644 --- a/api/catalog_test.go +++ b/api/catalog_test.go @@ -309,6 +309,7 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { Address: "192.168.10.10", Service: proxyService, Check: check, + NodeMeta: map[string]string{"synthetic-node": "true"}, } catalog := c.Catalog() @@ -366,7 +367,7 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { r.Fatalf("Bad: %v", meta) } - if len(services) != 2 { + if len(services) != 3 { r.Fatalf("Bad: %v", services) } }) From 6d8d39617a8ffc219fe9c17d66fce2e72cada2db Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Sat, 16 Sep 2023 14:22:45 +0530 Subject: [PATCH 13/27] fix tests --- api/catalog_test.go | 34 ++++++---------------------------- 1 file changed, 6 insertions(+), 28 deletions(-) diff --git a/api/catalog_test.go b/api/catalog_test.go index dee91eb3d423..6de7c7e9782d 100644 --- a/api/catalog_test.go +++ b/api/catalog_test.go @@ -287,34 +287,12 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { } proxyReg := testUnmanagedProxyRegistration(t) - - proxyService := &AgentService{ - ID: proxyReg.Service.Proxy.DestinationServiceID, - Service: proxyReg.Service.Proxy.DestinationServiceName, - Port: 8000, - } - - check := &AgentCheck{ - Node: "foobar", - CheckID: "service:" + proxyService.ID, - Name: "Redis health check", - Notes: "Script based health check", - Status: HealthPassing, - ServiceID: proxyService.ID, - } - - proxyCatalogReg := &CatalogRegistration{ - Datacenter: "dc1", - Node: "foobar", - Address: "192.168.10.10", - Service: proxyService, - Check: check, - NodeMeta: map[string]string{"synthetic-node": "true"}, - } + proxyReg.Node = "foobar4" + proxyReg.SkipNodeUpdate = true catalog := c.Catalog() retry.Run(t, func(r *retry.R) { - _, err := catalog.Register(proxyCatalogReg, nil) + _, err := catalog.Register(proxyReg, nil) r.Check(err) _, err = catalog.Register(reg0, nil) @@ -344,7 +322,7 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { } }) retry.Run(t, func(r *retry.R) { - _, err := catalog.Register(proxyCatalogReg, nil) + _, err := catalog.Register(proxyReg, nil) r.Check(err) _, err = catalog.Register(reg0, nil) r.Check(err) @@ -367,12 +345,12 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { r.Fatalf("Bad: %v", meta) } - if len(services) != 3 { + if len(services) != 2 { r.Fatalf("Bad: %v", services) } }) retry.Run(t, func(r *retry.R) { - _, err := catalog.Register(proxyCatalogReg, nil) + _, err := catalog.Register(proxyReg, nil) r.Check(err) _, err = catalog.Register(reg0, nil) r.Check(err) From f63c251576a2786162ad459718f15665be347598 Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Mon, 18 Sep 2023 11:32:35 +0530 Subject: [PATCH 14/27] fix variable name --- api/catalog_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/catalog_test.go b/api/catalog_test.go index 6de7c7e9782d..e5fcd41626c3 100644 --- a/api/catalog_test.go +++ b/api/catalog_test.go @@ -241,7 +241,7 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { NodeMeta: map[string]string{"somekey": "somevalue", "synthetic-node": "true"}, } - service := &AgentService{ + service1 := &AgentService{ ID: "redis1", Service: "redis1", Port: 8000, @@ -251,7 +251,7 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { reg1 := &CatalogRegistration{ Datacenter: "dc1", Node: "foobar1", - Service: service, + Service: service1, SkipNodeUpdate: true, NodeMeta: map[string]string{"key": "value"}, } From 7f6c53786270e87a9f0112e0fc184c23011ebfdd Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Mon, 18 Sep 2023 12:33:26 +0530 Subject: [PATCH 15/27] fix tests --- api/catalog_test.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/api/catalog_test.go b/api/catalog_test.go index e5fcd41626c3..d8e1758a87ad 100644 --- a/api/catalog_test.go +++ b/api/catalog_test.go @@ -4,6 +4,7 @@ package api import ( + "fmt" "reflect" "testing" "time" @@ -220,7 +221,7 @@ func TestAPI_CatalogServices_NodeMetaFilter(t *testing.T) { }) } -func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { +func TestAPI_CatalogServices_NodeMetaViaFilter(t *testing.T) { t.Parallel() c, s := makeClientWithConfig(t, nil, nil) defer s.Stop() @@ -238,7 +239,7 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { Node: "foobar0", Service: service0, SkipNodeUpdate: true, - NodeMeta: map[string]string{"somekey": "somevalue", "synthetic-node": "true"}, + NodeMeta: map[string]string{"somekey": "somevalue", "syntheticnode": "true"}, } service1 := &AgentService{ @@ -283,7 +284,7 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { Node: "foobar3", Address: "192.168.10.10", Service: service3, - NodeMeta: map[string]string{"synthetic-node": "true", "somekey": "somevalue"}, + NodeMeta: map[string]string{"syntheticnode": "true", "somekey": "somevalue"}, } proxyReg := testUnmanagedProxyRegistration(t) @@ -307,17 +308,18 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { if _, err := catalog.Register(reg3, nil); err != nil { r.Fatal(err) } - services, meta, err := catalog.Services(&QueryOptions{NodeMeta: map[string]string{"somekey": "somevalue"}}) + services, meta, err := catalog.Services(&QueryOptions{Filter: "NodeMeta.syntheticnode == true and NodeMeta.somekey == somevalue"}) if err != nil { r.Fatal(err) } delete(services, "consul") + fmt.Println(services) if meta.LastIndex == 0 { r.Fatalf("Bad: %v", meta) } - if len(services) != 3 { + if len(services) != 2 { r.Fatalf("Bad: %v", services) } }) @@ -336,7 +338,7 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { if _, err := catalog.Register(reg3, nil); err != nil { r.Fatal(err) } - services, meta, err := catalog.Services(&QueryOptions{NodeMeta: map[string]string{"synthetic-node": "true"}}) + services, meta, err := catalog.Services(&QueryOptions{Filter: "NodeMeta.syntheticnode == true"}) if err != nil { r.Fatal(err) } @@ -364,7 +366,7 @@ func TestAPI_CatalogServices_NodeMetaFilterFix(t *testing.T) { if _, err := catalog.Register(reg3, nil); err != nil { r.Fatal(err) } - services, meta, err := catalog.Services(&QueryOptions{NodeMeta: map[string]string{"key": "value"}}) + services, meta, err := catalog.Services(&QueryOptions{Filter: "NodeMeta.key == value"}) delete(services, "consul") if err != nil { r.Fatal(err) From 6c08f9c135871df9cc904fe20aa7d22a7f5ddadb Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Mon, 18 Sep 2023 13:06:25 +0530 Subject: [PATCH 16/27] fix tests --- api/catalog_test.go | 130 ++++---------------------------------------- 1 file changed, 11 insertions(+), 119 deletions(-) diff --git a/api/catalog_test.go b/api/catalog_test.go index d8e1758a87ad..3bf98e58e769 100644 --- a/api/catalog_test.go +++ b/api/catalog_test.go @@ -4,7 +4,6 @@ package api import ( - "fmt" "reflect" "testing" "time" @@ -223,159 +222,52 @@ func TestAPI_CatalogServices_NodeMetaFilter(t *testing.T) { func TestAPI_CatalogServices_NodeMetaViaFilter(t *testing.T) { t.Parallel() - c, s := makeClientWithConfig(t, nil, nil) + meta := map[string]string{"somekey": "somevalue", "syntheticnode": "true"} + c, s := makeClientWithConfig(t, nil, func(conf *testutil.TestServerConfig) { + conf.NodeMeta = meta + conf.NodeName = "foobar" + }) defer s.Stop() - // Register service and proxy instances to test against. - service0 := &AgentService{ - ID: "redis0", - Service: "redis0", - Port: 8001, - Connect: &AgentServiceConnect{Native: true}, - } - - reg0 := &CatalogRegistration{ - Datacenter: "dc1", - Node: "foobar0", - Service: service0, - SkipNodeUpdate: true, - NodeMeta: map[string]string{"somekey": "somevalue", "syntheticnode": "true"}, - } - - service1 := &AgentService{ - ID: "redis1", - Service: "redis1", - Port: 8000, - Connect: &AgentServiceConnect{Native: true}, - } - - reg1 := &CatalogRegistration{ - Datacenter: "dc1", - Node: "foobar1", - Service: service1, - SkipNodeUpdate: true, - NodeMeta: map[string]string{"key": "value"}, - } - - service2 := &AgentService{ - ID: "redis2", - Service: "redis2", - Port: 8000, - Connect: &AgentServiceConnect{Native: true}, - } - - reg2 := &CatalogRegistration{ - Datacenter: "dc1", - Node: "foobar2", - Address: "192.168.10.10", - Service: service2, - NodeMeta: map[string]string{"somekey": "somevalue"}, - } - - service3 := &AgentService{ - ID: "redis3", - Service: "redis3", - Port: 8000, - Connect: &AgentServiceConnect{Native: true}, - } - - reg3 := &CatalogRegistration{ - Datacenter: "dc1", - Node: "foobar3", - Address: "192.168.10.10", - Service: service3, - NodeMeta: map[string]string{"syntheticnode": "true", "somekey": "somevalue"}, - } - - proxyReg := testUnmanagedProxyRegistration(t) - proxyReg.Node = "foobar4" - proxyReg.SkipNodeUpdate = true - catalog := c.Catalog() retry.Run(t, func(r *retry.R) { - _, err := catalog.Register(proxyReg, nil) - r.Check(err) - - _, err = catalog.Register(reg0, nil) - r.Check(err) - - if _, err := catalog.Register(reg1, nil); err != nil { - r.Fatal(err) - } - if _, err := catalog.Register(reg2, nil); err != nil { - r.Fatal(err) - } - if _, err := catalog.Register(reg3, nil); err != nil { - r.Fatal(err) - } services, meta, err := catalog.Services(&QueryOptions{Filter: "NodeMeta.syntheticnode == true and NodeMeta.somekey == somevalue"}) if err != nil { r.Fatal(err) } - delete(services, "consul") - fmt.Println(services) if meta.LastIndex == 0 { r.Fatalf("Bad: %v", meta) } - - if len(services) != 2 { + if len(services) == 0 { r.Fatalf("Bad: %v", services) } }) retry.Run(t, func(r *retry.R) { - _, err := catalog.Register(proxyReg, nil) - r.Check(err) - _, err = catalog.Register(reg0, nil) - r.Check(err) - - if _, err := catalog.Register(reg1, nil); err != nil { - r.Fatal(err) - } - if _, err := catalog.Register(reg2, nil); err != nil { - r.Fatal(err) - } - if _, err := catalog.Register(reg3, nil); err != nil { - r.Fatal(err) - } services, meta, err := catalog.Services(&QueryOptions{Filter: "NodeMeta.syntheticnode == true"}) if err != nil { r.Fatal(err) } - delete(services, "consul") + if meta.LastIndex == 0 { r.Fatalf("Bad: %v", meta) } - if len(services) != 2 { + if len(services) == 0 { r.Fatalf("Bad: %v", services) } }) retry.Run(t, func(r *retry.R) { - _, err := catalog.Register(proxyReg, nil) - r.Check(err) - _, err = catalog.Register(reg0, nil) - r.Check(err) - - if _, err := catalog.Register(reg1, nil); err != nil { - r.Fatal(err) - } - if _, err := catalog.Register(reg2, nil); err != nil { - r.Fatal(err) - } - if _, err := catalog.Register(reg3, nil); err != nil { - r.Fatal(err) - } - services, meta, err := catalog.Services(&QueryOptions{Filter: "NodeMeta.key == value"}) - delete(services, "consul") + 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) != 1 { + if len(services) == 0 { r.Fatalf("Bad: %v", services) } }) From 44f366b4ad3d223b8c761cda12a825008b6c4cbb Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Mon, 18 Sep 2023 13:25:03 +0530 Subject: [PATCH 17/27] fix tests --- api/catalog_test.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/api/catalog_test.go b/api/catalog_test.go index 3bf98e58e769..a2665fd92f5b 100644 --- a/api/catalog_test.go +++ b/api/catalog_test.go @@ -222,7 +222,7 @@ func TestAPI_CatalogServices_NodeMetaFilter(t *testing.T) { func TestAPI_CatalogServices_NodeMetaViaFilter(t *testing.T) { t.Parallel() - meta := map[string]string{"somekey": "somevalue", "syntheticnode": "true"} + meta := map[string]string{"somekey": "somevalue", "synthetic": "true"} c, s := makeClientWithConfig(t, nil, func(conf *testutil.TestServerConfig) { conf.NodeMeta = meta conf.NodeName = "foobar" @@ -231,7 +231,7 @@ func TestAPI_CatalogServices_NodeMetaViaFilter(t *testing.T) { catalog := c.Catalog() retry.Run(t, func(r *retry.R) { - services, meta, err := catalog.Services(&QueryOptions{Filter: "NodeMeta.syntheticnode == true and NodeMeta.somekey == somevalue"}) + services, meta, err := catalog.Services(&QueryOptions{Filter: "NodeMeta[\"synthetic\"] == true and NodeMeta[\"somekey\"] == somevalue"}) if err != nil { r.Fatal(err) } @@ -244,7 +244,7 @@ func TestAPI_CatalogServices_NodeMetaViaFilter(t *testing.T) { } }) retry.Run(t, func(r *retry.R) { - services, meta, err := catalog.Services(&QueryOptions{Filter: "NodeMeta.syntheticnode == true"}) + services, meta, err := catalog.Services(&QueryOptions{Filter: "NodeMeta.synthetic == true"}) if err != nil { r.Fatal(err) } @@ -271,6 +271,20 @@ func TestAPI_CatalogServices_NodeMetaViaFilter(t *testing.T) { 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) { From 961eeef0fd499a4168e06ac7841677e6ce7cdea7 Mon Sep 17 00:00:00 2001 From: Ashesh Vidyut <134911583+absolutelightning@users.noreply.github.com> Date: Mon, 18 Sep 2023 14:50:09 +0530 Subject: [PATCH 18/27] Update .changelog/18322.txt Co-authored-by: Ganesh S --- .changelog/18322.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/18322.txt b/.changelog/18322.txt index 545e6ecff13e..0e4d5c7b8f20 100644 --- a/.changelog/18322.txt +++ b/.changelog/18322.txt @@ -1,4 +1,4 @@ ```release-note:bug -catalog api: fix a bug with catalog api where filter query parameter was not working correctly. endpoint fixed - +catalog api: fixes a bug with catalog api where filter query parameter was not working correctly for the `/v1/catalog/services` endpoint /v1/catalog/services ``` \ No newline at end of file From 5af1b1cf98fb743df365aaeef39e4dd7c0e02637 Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Mon, 18 Sep 2023 14:53:34 +0530 Subject: [PATCH 19/27] fix change log --- .changelog/18322.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/.changelog/18322.txt b/.changelog/18322.txt index 0e4d5c7b8f20..9be8c2cfa3e2 100644 --- a/.changelog/18322.txt +++ b/.changelog/18322.txt @@ -1,4 +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 -/v1/catalog/services ``` \ No newline at end of file From 1763589f555660c351a4e886b6009cc68e5cecd0 Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Mon, 18 Sep 2023 19:57:23 +0530 Subject: [PATCH 20/27] address nits --- api/catalog_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/catalog_test.go b/api/catalog_test.go index a2665fd92f5b..708392c42b57 100644 --- a/api/catalog_test.go +++ b/api/catalog_test.go @@ -220,16 +220,16 @@ func TestAPI_CatalogServices_NodeMetaFilter(t *testing.T) { }) } -func TestAPI_CatalogServices_NodeMetaViaFilter(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 - conf.NodeName = "foobar" }) 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 { From f475356bfc29848c4949412aabb5e94e2c044575 Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Mon, 18 Sep 2023 20:27:18 +0530 Subject: [PATCH 21/27] removed unused line --- agent/catalog_endpoint.go | 1 - 1 file changed, 1 deletion(-) diff --git a/agent/catalog_endpoint.go b/agent/catalog_endpoint.go index 08283ee97f4e..1dac61befa47 100644 --- a/agent/catalog_endpoint.go +++ b/agent/catalog_endpoint.go @@ -281,7 +281,6 @@ func (s *HTTPHandlers) CatalogServices(resp http.ResponseWriter, req *http.Reque if done := s.parse(resp, req, &args.Datacenter, &args.QueryOptions); done { return nil, nil } - args.Filter = args.QueryOptions.Filter var out structs.IndexedServices defer setMeta(resp, &out.QueryMeta) From 4ed56cc018586bb6afd213e541def117733dfc0b Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Wed, 20 Sep 2023 06:47:10 +0530 Subject: [PATCH 22/27] doing join only when filter has nodemeta --- agent/consul/catalog_endpoint.go | 3 ++- agent/consul/state/catalog.go | 13 ++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/agent/consul/catalog_endpoint.go b/agent/consul/catalog_endpoint.go index 0f1527c7e13d..dcefb29266f1 100644 --- a/agent/consul/catalog_endpoint.go +++ b/agent/consul/catalog_endpoint.go @@ -596,7 +596,8 @@ 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, + strings.Contains(strings.ToLower(args.Filter), "nodemeta")) } if err != nil { return err diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index 97d9cf9842c5..fd5d4ab25bd5 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.ServiceNodes, 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,11 +1236,14 @@ 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)) } - parsedResult, err := parseServiceNodes(tx, ws, result, entMeta, peerName) - if err != nil { - return 0, nil, fmt.Errorf("failed querying and parsing services :%s", err) + 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, parsedResult, nil + return idx, result, nil } func (s *Store) ServiceList(ws memdb.WatchSet, entMeta *acl.EnterpriseMeta, peerName string) (uint64, structs.ServiceList, error) { From 7a95115bfcbebe0cddb01aa22614cef48ecb4b28 Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Wed, 20 Sep 2023 07:12:47 +0530 Subject: [PATCH 23/27] fix tests --- agent/consul/state/catalog_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/consul/state/catalog_test.go b/agent/consul/state/catalog_test.go index c15007e76e9a..7d55731a88d2 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, &acl.EnterpriseMeta{}, "") + idx, services, err := s.Services(ws, &acl.EnterpriseMeta{}, "", false) if err != nil { t.Fatalf("err: %s", err) } @@ -2229,7 +2229,7 @@ func TestStateStore_Services(t *testing.T) { // Pull all the services. ws = memdb.NewWatchSet() - idx, services, err = s.Services(ws, &acl.EnterpriseMeta{}, "") + idx, services, err = s.Services(ws, &acl.EnterpriseMeta{}, "", false) if err != nil { t.Fatalf("err: %s", err) } From 94e73db70d9bf2490b1cbb1e1d5a2c61b4ef88a0 Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Wed, 20 Sep 2023 07:39:20 +0530 Subject: [PATCH 24/27] fix tests --- agent/consul/state/catalog_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/consul/state/catalog_test.go b/agent/consul/state/catalog_test.go index 7d55731a88d2..105a99015873 100644 --- a/agent/consul/state/catalog_test.go +++ b/agent/consul/state/catalog_test.go @@ -2205,7 +2205,7 @@ func TestStateStore_Services(t *testing.T) { t.Fatalf("err: %s", err) } ns1Dogs := testRegisterService(t, s, 3, "node1", "dogs") - ns1Dogs.Tags = []string{} + ns1Dogs.Tags = nil ns1Dogs.Meta = map[string]string{} ns1Dogs.EnterpriseMeta.Normalize() From b7aa37e40329cfb65ee8ba5a987b6614f49face2 Mon Sep 17 00:00:00 2001 From: Ashesh Vidyut <134911583+absolutelightning@users.noreply.github.com> Date: Tue, 26 Sep 2023 08:02:19 +0530 Subject: [PATCH 25/27] Update agent/consul/catalog_endpoint.go Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com> --- agent/consul/catalog_endpoint.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/agent/consul/catalog_endpoint.go b/agent/consul/catalog_endpoint.go index dcefb29266f1..36426afe253a 100644 --- a/agent/consul/catalog_endpoint.go +++ b/agent/consul/catalog_endpoint.go @@ -596,8 +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, - strings.Contains(strings.ToLower(args.Filter), "nodemeta")) + reply.Index, serviceNodes, err = state.Services(ws, &args.EnterpriseMeta, args.PeerName, args.Filter != "") } if err != nil { return err From 89ed77281454365708da4d0d39b9307736a04b52 Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Tue, 26 Sep 2023 08:04:57 +0530 Subject: [PATCH 26/27] fix tests --- agent/consul/state/catalog_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/consul/state/catalog_test.go b/agent/consul/state/catalog_test.go index 105a99015873..c04fa8d914aa 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, &acl.EnterpriseMeta{}, "", false) + idx, services, err := s.Services(ws, nil, "", false) if err != nil { t.Fatalf("err: %s", err) } @@ -2229,7 +2229,7 @@ func TestStateStore_Services(t *testing.T) { // Pull all the services. ws = memdb.NewWatchSet() - idx, services, err = s.Services(ws, &acl.EnterpriseMeta{}, "", false) + idx, services, err = s.Services(ws, nil, "", false) if err != nil { t.Fatalf("err: %s", err) } From 11414443e04e83895d1f83376b352d3a244f2bc8 Mon Sep 17 00:00:00 2001 From: absolutelightning Date: Tue, 26 Sep 2023 09:54:03 +0530 Subject: [PATCH 27/27] removed unwanted code --- agent/consul/state/catalog_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/agent/consul/state/catalog_test.go b/agent/consul/state/catalog_test.go index c04fa8d914aa..f18b9beae843 100644 --- a/agent/consul/state/catalog_test.go +++ b/agent/consul/state/catalog_test.go @@ -2199,14 +2199,10 @@ func TestStateStore_Services(t *testing.T) { Port: 1111, } ns1.EnterpriseMeta.Normalize() - ns1.Tags = []string{} - ns1.Meta = map[string]string{} if err := s.EnsureService(2, "node1", ns1); err != nil { t.Fatalf("err: %s", err) } ns1Dogs := testRegisterService(t, s, 3, "node1", "dogs") - ns1Dogs.Tags = nil - ns1Dogs.Meta = map[string]string{} ns1Dogs.EnterpriseMeta.Normalize() testRegisterNode(t, s, 4, "node2") @@ -2217,9 +2213,7 @@ func TestStateStore_Services(t *testing.T) { Address: "1.1.1.1", Port: 1111, } - ns2.Tags = []string{} ns2.EnterpriseMeta.Normalize() - ns2.Meta = map[string]string{} if err := s.EnsureService(5, "node2", ns2); err != nil { t.Fatalf("err: %s", err) }