From 8bb1b89554f9f4354e94ae44609d98cba8460776 Mon Sep 17 00:00:00 2001
From: Dan Upton <daniel@floppy.co>
Date: Fri, 3 Dec 2021 20:51:02 +0000
Subject: [PATCH] coordinate: support `ResultsFilteredByACLs` flag/header
 (#11617)

---
 agent/consul/acl.go                      |  9 ++-
 agent/consul/acl_test.go                 | 70 ++++++++++++++++--------
 agent/consul/coordinate_endpoint_test.go |  3 +
 3 files changed, 57 insertions(+), 25 deletions(-)

diff --git a/agent/consul/acl.go b/agent/consul/acl.go
index 078dc4026..0805561ae 100644
--- a/agent/consul/acl.go
+++ b/agent/consul/acl.go
@@ -1405,10 +1405,11 @@ func (f *aclFilter) filterSessions(sessions *structs.Sessions) bool {
 }
 
 // filterCoordinates is used to filter nodes in a coordinate dump based on ACL
-// rules.
-func (f *aclFilter) filterCoordinates(coords *structs.Coordinates) {
+// rules. Returns true if any elements were removed.
+func (f *aclFilter) filterCoordinates(coords *structs.Coordinates) bool {
 	c := *coords
 	var authzContext acl.AuthorizerContext
+	var removed bool
 
 	for i := 0; i < len(c); i++ {
 		c[i].FillAuthzContext(&authzContext)
@@ -1417,10 +1418,12 @@ func (f *aclFilter) filterCoordinates(coords *structs.Coordinates) {
 			continue
 		}
 		f.logger.Debug("dropping node from result due to ACLs", "node", structs.NodeNameString(node, c[i].GetEnterpriseMeta()))
+		removed = true
 		c = append(c[:i], c[i+1:]...)
 		i--
 	}
 	*coords = c
+	return removed
 }
 
 // filterIntentions is used to filter intentions based on ACL rules.
@@ -1827,7 +1830,7 @@ func filterACLWithAuthorizer(logger hclog.Logger, authorizer acl.Authorizer, sub
 		filt.filterDatacenterCheckServiceNodes(&v.DatacenterNodes)
 
 	case *structs.IndexedCoordinates:
-		filt.filterCoordinates(&v.Coordinates)
+		v.QueryMeta.ResultsFilteredByACLs = filt.filterCoordinates(&v.Coordinates)
 
 	case *structs.IndexedHealthChecks:
 		v.QueryMeta.ResultsFilteredByACLs = filt.filterHealthChecks(&v.HealthChecks)
diff --git a/agent/consul/acl_test.go b/agent/consul/acl_test.go
index eaf43690e..45b82d23d 100644
--- a/agent/consul/acl_test.go
+++ b/agent/consul/acl_test.go
@@ -2767,31 +2767,57 @@ service "bar" {
 
 func TestACL_filterCoordinates(t *testing.T) {
 	t.Parallel()
-	// Create some coordinates.
-	coords := structs.Coordinates{
-		&structs.Coordinate{
-			Node:  "node1",
-			Coord: generateRandomCoordinate(),
-		},
-		&structs.Coordinate{
-			Node:  "node2",
-			Coord: generateRandomCoordinate(),
-		},
+
+	logger := hclog.NewNullLogger()
+
+	makeList := func() *structs.IndexedCoordinates {
+		return &structs.IndexedCoordinates{
+			Coordinates: structs.Coordinates{
+				{Node: "node1", Coord: generateRandomCoordinate()},
+				{Node: "node2", Coord: generateRandomCoordinate()},
+			},
+		}
 	}
 
-	// Try permissive filtering.
-	filt := newACLFilter(acl.AllowAll(), nil)
-	filt.filterCoordinates(&coords)
-	if len(coords) != 2 {
-		t.Fatalf("bad: %#v", coords)
-	}
+	t.Run("allowed", func(t *testing.T) {
+		require := require.New(t)
 
-	// Try restrictive filtering
-	filt = newACLFilter(acl.DenyAll(), nil)
-	filt.filterCoordinates(&coords)
-	if len(coords) != 0 {
-		t.Fatalf("bad: %#v", coords)
-	}
+		list := makeList()
+		filterACLWithAuthorizer(logger, acl.AllowAll(), list)
+
+		require.Len(list.Coordinates, 2)
+		require.False(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
+	})
+
+	t.Run("allowed to read one node", func(t *testing.T) {
+		require := require.New(t)
+
+		policy, err := acl.NewPolicyFromSource(`
+			node "node1" {
+			  policy = "read"
+			}
+		`, acl.SyntaxLegacy, nil, nil)
+		require.NoError(err)
+
+		authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
+		require.NoError(err)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, authz, list)
+
+		require.Len(list.Coordinates, 1)
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
+
+	t.Run("denied", func(t *testing.T) {
+		require := require.New(t)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, acl.DenyAll(), list)
+
+		require.Empty(list.Coordinates)
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
 }
 
 func TestACL_filterSessions(t *testing.T) {
diff --git a/agent/consul/coordinate_endpoint_test.go b/agent/consul/coordinate_endpoint_test.go
index f40ee102d..df9ac8330 100644
--- a/agent/consul/coordinate_endpoint_test.go
+++ b/agent/consul/coordinate_endpoint_test.go
@@ -452,6 +452,9 @@ func TestCoordinate_ListNodes_ACLFilter(t *testing.T) {
 	if len(resp.Coordinates) != 1 || resp.Coordinates[0].Node != "foo" {
 		t.Fatalf("bad: %#v", resp.Coordinates)
 	}
+	if !resp.QueryMeta.ResultsFilteredByACLs {
+		t.Fatal("ResultsFilteredByACLs should be true")
+	}
 }
 
 func TestCoordinate_Node(t *testing.T) {