health: support `ResultsFilteredByACLs` flag/header (#11602)

agent/consul/acl.go

@@ -1219,10 +1219,12 @@ func (f *aclFilter) allowSession(node string, ent *acl.AuthorizerContext) bool {
 }
 
 // filterHealthChecks is used to filter a set of health checks down based on
-// the configured ACL rules for a token.
+// the configured ACL rules for a token. Returns true if any elements were
-func (f *aclFilter) filterHealthChecks(checks *structs.HealthChecks) {
+// removed.
+func (f *aclFilter) filterHealthChecks(checks *structs.HealthChecks) bool {
 	hc := *checks
 	var authzContext acl.AuthorizerContext
+	var removed bool
 
 	for i := 0; i < len(hc); i++ {
 		check := hc[i]
@@ -1232,10 +1234,12 @@ func (f *aclFilter) filterHealthChecks(checks *structs.HealthChecks) {
 		}
 
 		f.logger.Debug("dropping check from result due to ACLs", "check", check.CheckID)
+		removed = true
 		hc = append(hc[:i], hc[i+1:]...)
 		i--
 	}
 	*checks = hc
+	return removed
 }
 
 // filterServices is used to filter a set of services based on ACLs.
@@ -1332,10 +1336,12 @@ func (f *aclFilter) filterNodeServiceList(services **structs.NodeServiceList) {
 	}
 }
 
-// filterCheckServiceNodes is used to filter nodes based on ACL rules.
+// filterCheckServiceNodes is used to filter nodes based on ACL rules. Returns
-func (f *aclFilter) filterCheckServiceNodes(nodes *structs.CheckServiceNodes) {
+// true if any elements were removed.
+func (f *aclFilter) filterCheckServiceNodes(nodes *structs.CheckServiceNodes) bool {
 	csn := *nodes
 	var authzContext acl.AuthorizerContext
+	var removed bool
 
 	for i := 0; i < len(csn); i++ {
 		node := csn[i]
@@ -1344,22 +1350,20 @@ func (f *aclFilter) filterCheckServiceNodes(nodes *structs.CheckServiceNodes) {
 			continue
 		}
 		f.logger.Debug("dropping node from result due to ACLs", "node", structs.NodeNameString(node.Node.Node, node.Node.GetEnterpriseMeta()))
+		removed = true
 		csn = append(csn[:i], csn[i+1:]...)
 		i--
 	}
 	*nodes = csn
+	return removed
 }
 
 // filterServiceTopology is used to filter upstreams/downstreams based on ACL rules.
 // this filter is unlike others in that it also returns whether the result was filtered by ACLs
 func (f *aclFilter) filterServiceTopology(topology *structs.ServiceTopology) bool {
-	numUp := len(topology.Upstreams)
+	filteredUpstreams := f.filterCheckServiceNodes(&topology.Upstreams)
-	numDown := len(topology.Downstreams)
+	filteredDownstreams := f.filterCheckServiceNodes(&topology.Downstreams)
-
+	return filteredUpstreams || filteredDownstreams
-	f.filterCheckServiceNodes(&topology.Upstreams)
-	f.filterCheckServiceNodes(&topology.Downstreams)
-
-	return numUp != len(topology.Upstreams) || numDown != len(topology.Downstreams)
 }
 
 // filterDatacenterCheckServiceNodes is used to filter nodes based on ACL rules.
@@ -1801,7 +1805,7 @@ func filterACLWithAuthorizer(logger hclog.Logger, authorizer acl.Authorizer, sub
 		filt.filterCheckServiceNodes(v)
 
 	case *structs.IndexedCheckServiceNodes:
-		filt.filterCheckServiceNodes(&v.Nodes)
+		v.QueryMeta.ResultsFilteredByACLs = filt.filterCheckServiceNodes(&v.Nodes)
 
 	case *structs.IndexedServiceTopology:
 		filtered := filt.filterServiceTopology(v.ServiceTopology)
@@ -1817,7 +1821,7 @@ func filterACLWithAuthorizer(logger hclog.Logger, authorizer acl.Authorizer, sub
 		filt.filterCoordinates(&v.Coordinates)
 
 	case *structs.IndexedHealthChecks:
-		filt.filterHealthChecks(&v.HealthChecks)
+		v.QueryMeta.ResultsFilteredByACLs = filt.filterHealthChecks(&v.HealthChecks)
 
 	case *structs.IndexedIntentions:
 		filt.filterIntentions(&v.Intentions)

agent/consul/acl_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/go-uuid"
 	msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc"
 	"github.com/mitchellh/copystructure"
@@ -2151,72 +2152,93 @@ func testACLResolver_variousTokens(t *testing.T, delegate *ACLResolverTestDelega
 
 func TestACL_filterHealthChecks(t *testing.T) {
 	t.Parallel()
-	// Create some health checks.
+
-	fill := func() structs.HealthChecks {
+	logger := hclog.NewNullLogger()
-		return structs.HealthChecks{
+
-			&structs.HealthCheck{
+	makeList := func() *structs.IndexedHealthChecks {
-				Node:        "node1",
+		return &structs.IndexedHealthChecks{
-				CheckID:     "check1",
+			HealthChecks: structs.HealthChecks{
-				ServiceName: "foo",
+				{
+					Node:        "node1",
+					CheckID:     "check1",
+					ServiceName: "foo",
+				},
 			},
 		}
 	}
 
-	{
+	t.Run("allowed", func(t *testing.T) {
-		hc := fill()
+		require := require.New(t)
-		filt := newACLFilter(acl.DenyAll(), nil)
-		filt.filterHealthChecks(&hc)
-		if len(hc) != 0 {
-			t.Fatalf("bad: %#v", hc)
-		}
-	}
 
-	// Allowed to see the service but not the node.
+		policy, err := acl.NewPolicyFromSource(`
-	policy, err := acl.NewPolicyFromSource(`
+			service "foo" {
-service "foo" {
+			  policy = "read"
-  policy = "read"
+			}
-}
+			node "node1" {
-`, acl.SyntaxLegacy, nil, nil)
+			  policy = "read"
-	if err != nil {
+			}
-		t.Fatalf("err %v", err)
+		`, acl.SyntaxLegacy, nil, nil)
-	}
+		require.NoError(err)
-	perms, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
 
-	{
+		authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
-		hc := fill()
+		require.NoError(err)
-		filt := newACLFilter(perms, nil)
-		filt.filterHealthChecks(&hc)
-		if len(hc) != 0 {
-			t.Fatalf("bad: %#v", hc)
-		}
-	}
 
-	// Chain on access to the node.
+		list := makeList()
-	policy, err = acl.NewPolicyFromSource(`
+		filterACLWithAuthorizer(logger, authz, list)
-node "node1" {
-  policy = "read"
-}
-`, acl.SyntaxLegacy, nil, nil)
-	if err != nil {
-		t.Fatalf("err %v", err)
-	}
-	perms, err = acl.NewPolicyAuthorizerWithDefaults(perms, []*acl.Policy{policy}, nil)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
 
-	// Now it should go through.
+		require.Len(list.HealthChecks, 1)
-	{
+		require.False(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
-		hc := fill()
+	})
-		filt := newACLFilter(perms, nil)
+
-		filt.filterHealthChecks(&hc)
+	t.Run("allowed to read the service, but not the node", func(t *testing.T) {
-		if len(hc) != 1 {
+		require := require.New(t)
-			t.Fatalf("bad: %#v", hc)
+
-		}
+		policy, err := acl.NewPolicyFromSource(`
-	}
+			service "foo" {
+			  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.Empty(list.HealthChecks)
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
+
+	t.Run("allowed to read the node, but not the service", 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.Empty(list.HealthChecks)
+		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.HealthChecks)
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
 }
 
 func TestACL_filterIntentions(t *testing.T) {
@@ -2474,100 +2496,104 @@ node "node1" {
 
 func TestACL_filterCheckServiceNodes(t *testing.T) {
 	t.Parallel()
-	// Create some nodes.
+
-	fill := func() structs.CheckServiceNodes {
+	logger := hclog.NewNullLogger()
-		return structs.CheckServiceNodes{
+
-			structs.CheckServiceNode{
+	makeList := func() *structs.IndexedCheckServiceNodes {
-				Node: &structs.Node{
+		return &structs.IndexedCheckServiceNodes{
-					Node: "node1",
+			Nodes: structs.CheckServiceNodes{
-				},
+				{
-				Service: &structs.NodeService{
+					Node: &structs.Node{
-					ID:      "foo",
+						Node: "node1",
-					Service: "foo",
+					},
-				},
+					Service: &structs.NodeService{
-				Checks: structs.HealthChecks{
+						ID:      "foo",
-					&structs.HealthCheck{
+						Service: "foo",
-						Node:        "node1",
+					},
-						CheckID:     "check1",
+					Checks: structs.HealthChecks{
-						ServiceName: "foo",
+						{
+							Node:        "node1",
+							CheckID:     "check1",
+							ServiceName: "foo",
+						},
 					},
 				},
 			},
 		}
 	}
 
-	// Try permissive filtering.
+	t.Run("allowed", func(t *testing.T) {
-	{
+		require := require.New(t)
-		nodes := fill()
-		filt := newACLFilter(acl.AllowAll(), nil)
-		filt.filterCheckServiceNodes(&nodes)
-		if len(nodes) != 1 {
-			t.Fatalf("bad: %#v", nodes)
-		}
-		if len(nodes[0].Checks) != 1 {
-			t.Fatalf("bad: %#v", nodes[0].Checks)
-		}
-	}
 
-	// Try restrictive filtering.
+		policy, err := acl.NewPolicyFromSource(`
-	{
+			service "foo" {
-		nodes := fill()
+			  policy = "read"
-		filt := newACLFilter(acl.DenyAll(), nil)
+			}
-		filt.filterCheckServiceNodes(&nodes)
+			node "node1" {
-		if len(nodes) != 0 {
+			  policy = "read"
-			t.Fatalf("bad: %#v", nodes)
+			}
-		}
+		`, acl.SyntaxLegacy, nil, nil)
-	}
+		require.NoError(err)
 
-	// Allowed to see the service but not the node.
+		authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
-	policy, err := acl.NewPolicyFromSource(`
+		require.NoError(err)
-service "foo" {
-  policy = "read"
-}
-`, acl.SyntaxLegacy, nil, nil)
-	if err != nil {
-		t.Fatalf("err %v", err)
-	}
-	perms, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
 
-	{
+		list := makeList()
-		nodes := fill()
+		filterACLWithAuthorizer(logger, authz, list)
-		filt := newACLFilter(perms, nil)
-		filt.filterCheckServiceNodes(&nodes)
-		if len(nodes) != 0 {
-			t.Fatalf("bad: %#v", nodes)
-		}
-	}
 
-	// Chain on access to the node.
+		require.Len(list.Nodes, 1)
-	policy, err = acl.NewPolicyFromSource(`
+		require.False(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
-node "node1" {
+	})
-  policy = "read"
-}
-`, acl.SyntaxLegacy, nil, nil)
-	if err != nil {
-		t.Fatalf("err %v", err)
-	}
-	perms, err = acl.NewPolicyAuthorizerWithDefaults(perms, []*acl.Policy{policy}, nil)
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
 
-	// Now it should go through.
+	t.Run("allowed to read the service, but not the node", func(t *testing.T) {
-	{
+		require := require.New(t)
-		nodes := fill()
+
-		filt := newACLFilter(perms, nil)
+		policy, err := acl.NewPolicyFromSource(`
-		filt.filterCheckServiceNodes(&nodes)
+			service "foo" {
-		if len(nodes) != 1 {
+			  policy = "read"
-			t.Fatalf("bad: %#v", nodes)
+			}
-		}
+		`, acl.SyntaxLegacy, nil, nil)
-		if len(nodes[0].Checks) != 1 {
+		require.NoError(err)
-			t.Fatalf("bad: %#v", nodes[0].Checks)
+
-		}
+		authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
-	}
+		require.NoError(err)
+
+		list := makeList()
+		filterACLWithAuthorizer(logger, authz, list)
+
+		require.Empty(list.Nodes)
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
+
+	t.Run("allowed to read the node, but not the service", 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.Empty(list.Nodes)
+		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.Nodes)
+		require.True(list.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
 }
 
 func TestACL_filterServiceTopology(t *testing.T) {

agent/consul/catalog_endpoint_test.go

@@ -2742,6 +2742,7 @@ node_prefix "" {
 			CheckID:   "service:bar",
 			Name:      "service:bar",
 			ServiceID: "bar",
+			Status:    api.HealthPassing,
 		},
 		WriteRequest: structs.WriteRequest{Token: "root"},
 	}

agent/consul/health_endpoint.go

@@ -55,9 +55,6 @@ func (h *Health) ChecksInState(args *structs.ChecksInStateRequest,
 				return err
 			}
 			reply.Index, reply.HealthChecks = index, checks
-			if err := h.srv.filterACL(args.Token, reply); err != nil {
-				return err
-			}
 
 			raw, err := filter.Execute(reply.HealthChecks)
 			if err != nil {
@@ -65,6 +62,13 @@ func (h *Health) ChecksInState(args *structs.ChecksInStateRequest,
 			}
 			reply.HealthChecks = raw.(structs.HealthChecks)
 
+			// Note: we filter the results with ACLs *after* applying the user-supplied
+			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
+			// results that would be filtered out even if the user did have permission.
+			if err := h.srv.filterACL(args.Token, reply); err != nil {
+				return err
+			}
+
 			return h.srv.sortNodesByDistanceFrom(args.Source, reply.HealthChecks)
 		})
 }
@@ -99,15 +103,20 @@ func (h *Health) NodeChecks(args *structs.NodeSpecificRequest,
 				return err
 			}
 			reply.Index, reply.HealthChecks = index, checks
-			if err := h.srv.filterACL(args.Token, reply); err != nil {
-				return err
-			}
 
 			raw, err := filter.Execute(reply.HealthChecks)
 			if err != nil {
 				return err
 			}
 			reply.HealthChecks = raw.(structs.HealthChecks)
+
+			// Note: we filter the results with ACLs *after* applying the user-supplied
+			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
+			// results that would be filtered out even if the user did have permission.
+			if err := h.srv.filterACL(args.Token, reply); err != nil {
+				return err
+			}
+
 			return nil
 		})
 }
@@ -156,9 +165,6 @@ func (h *Health) ServiceChecks(args *structs.ServiceSpecificRequest,
 				return err
 			}
 			reply.Index, reply.HealthChecks = index, checks
-			if err := h.srv.filterACL(args.Token, reply); err != nil {
-				return err
-			}
 
 			raw, err := filter.Execute(reply.HealthChecks)
 			if err != nil {
@@ -166,6 +172,13 @@ func (h *Health) ServiceChecks(args *structs.ServiceSpecificRequest,
 			}
 			reply.HealthChecks = raw.(structs.HealthChecks)
 
+			// Note: we filter the results with ACLs *after* applying the user-supplied
+			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
+			// results that would be filtered out even if the user did have permission.
+			if err := h.srv.filterACL(args.Token, reply); err != nil {
+				return err
+			}
+
 			return h.srv.sortNodesByDistanceFrom(args.Source, reply.HealthChecks)
 		})
 }
@@ -232,16 +245,19 @@ func (h *Health) ServiceNodes(args *structs.ServiceSpecificRequest, reply *struc
 				reply.Nodes = nodeMetaFilter(args.NodeMetaFilters, reply.Nodes)
 			}
 
-			if err := h.srv.filterACL(args.Token, reply); err != nil {
-				return err
-			}
-
 			raw, err := filter.Execute(reply.Nodes)
 			if err != nil {
 				return err
 			}
 			reply.Nodes = raw.(structs.CheckServiceNodes)
 
+			// Note: we filter the results with ACLs *after* applying the user-supplied
+			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
+			// results that would be filtered out even if the user did have permission.
+			if err := h.srv.filterACL(args.Token, reply); err != nil {
+				return err
+			}
+
 			return h.srv.sortNodesByDistanceFrom(args.Source, reply.Nodes)
 		})
 

agent/consul/health_endpoint_test.go

@@ -1431,6 +1431,9 @@ func TestHealth_NodeChecks_FilterACL(t *testing.T) {
 	}
 
 	t.Parallel()
+
+	require := require.New(t)
+
 	dir, token, srv, codec := testACLFilterServer(t)
 	defer os.RemoveAll(dir)
 	defer srv.Shutdown()
@@ -1442,9 +1445,9 @@ func TestHealth_NodeChecks_FilterACL(t *testing.T) {
 		QueryOptions: structs.QueryOptions{Token: token},
 	}
 	reply := structs.IndexedHealthChecks{}
-	if err := msgpackrpc.CallWithCodec(codec, "Health.NodeChecks", &opt, &reply); err != nil {
+	err := msgpackrpc.CallWithCodec(codec, "Health.NodeChecks", &opt, &reply)
-		t.Fatalf("err: %s", err)
+	require.NoError(err)
-	}
+
 	found := false
 	for _, chk := range reply.HealthChecks {
 		switch chk.ServiceName {
@@ -1454,9 +1457,8 @@ func TestHealth_NodeChecks_FilterACL(t *testing.T) {
 			t.Fatalf("bad: %#v", reply.HealthChecks)
 		}
 	}
-	if !found {
+	require.True(found, "bad: %#v", reply.HealthChecks)
-		t.Fatalf("bad: %#v", reply.HealthChecks)
+	require.True(reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
-	}
 
 	// We've already proven that we call the ACL filtering function so we
 	// test node filtering down in acl.go for node cases. This also proves
@@ -1471,6 +1473,9 @@ func TestHealth_ServiceChecks_FilterACL(t *testing.T) {
 	}
 
 	t.Parallel()
+
+	require := require.New(t)
+
 	dir, token, srv, codec := testACLFilterServer(t)
 	defer os.RemoveAll(dir)
 	defer srv.Shutdown()
@@ -1482,9 +1487,9 @@ func TestHealth_ServiceChecks_FilterACL(t *testing.T) {
 		QueryOptions: structs.QueryOptions{Token: token},
 	}
 	reply := structs.IndexedHealthChecks{}
-	if err := msgpackrpc.CallWithCodec(codec, "Health.ServiceChecks", &opt, &reply); err != nil {
+	err := msgpackrpc.CallWithCodec(codec, "Health.ServiceChecks", &opt, &reply)
-		t.Fatalf("err: %s", err)
+	require.NoError(err)
-	}
+
 	found := false
 	for _, chk := range reply.HealthChecks {
 		if chk.ServiceName == "foo" {
@@ -1492,18 +1497,14 @@ func TestHealth_ServiceChecks_FilterACL(t *testing.T) {
 			break
 		}
 	}
-	if !found {
+	require.True(found, "bad: %#v", reply.HealthChecks)
-		t.Fatalf("bad: %#v", reply.HealthChecks)
-	}
 
 	opt.ServiceName = "bar"
 	reply = structs.IndexedHealthChecks{}
-	if err := msgpackrpc.CallWithCodec(codec, "Health.ServiceChecks", &opt, &reply); err != nil {
+	err = msgpackrpc.CallWithCodec(codec, "Health.ServiceChecks", &opt, &reply)
-		t.Fatalf("err: %s", err)
+	require.NoError(err)
-	}
+	require.Empty(reply.HealthChecks)
-	if len(reply.HealthChecks) != 0 {
+	require.True(reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
-		t.Fatalf("bad: %#v", reply.HealthChecks)
-	}
 
 	// We've already proven that we call the ACL filtering function so we
 	// test node filtering down in acl.go for node cases. This also proves
@@ -1518,6 +1519,9 @@ func TestHealth_ServiceNodes_FilterACL(t *testing.T) {
 	}
 
 	t.Parallel()
+
+	require := require.New(t)
+
 	dir, token, srv, codec := testACLFilterServer(t)
 	defer os.RemoveAll(dir)
 	defer srv.Shutdown()
@@ -1529,21 +1533,16 @@ func TestHealth_ServiceNodes_FilterACL(t *testing.T) {
 		QueryOptions: structs.QueryOptions{Token: token},
 	}
 	reply := structs.IndexedCheckServiceNodes{}
-	if err := msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &opt, &reply); err != nil {
+	err := msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &opt, &reply)
-		t.Fatalf("err: %s", err)
+	require.NoError(err)
-	}
+	require.Len(reply.Nodes, 1)
-	if len(reply.Nodes) != 1 {
-		t.Fatalf("bad: %#v", reply.Nodes)
-	}
 
 	opt.ServiceName = "bar"
 	reply = structs.IndexedCheckServiceNodes{}
-	if err := msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &opt, &reply); err != nil {
+	err = msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &opt, &reply)
-		t.Fatalf("err: %s", err)
+	require.NoError(err)
-	}
+	require.Empty(reply.Nodes)
-	if len(reply.Nodes) != 0 {
+	require.True(reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
-		t.Fatalf("bad: %#v", reply.Nodes)
-	}
 
 	// We've already proven that we call the ACL filtering function so we
 	// test node filtering down in acl.go for node cases. This also proves
@@ -1558,6 +1557,9 @@ func TestHealth_ChecksInState_FilterACL(t *testing.T) {
 	}
 
 	t.Parallel()
+
+	require := require.New(t)
+
 	dir, token, srv, codec := testACLFilterServer(t)
 	defer os.RemoveAll(dir)
 	defer srv.Shutdown()
@@ -1569,9 +1571,8 @@ func TestHealth_ChecksInState_FilterACL(t *testing.T) {
 		QueryOptions: structs.QueryOptions{Token: token},
 	}
 	reply := structs.IndexedHealthChecks{}
-	if err := msgpackrpc.CallWithCodec(codec, "Health.ChecksInState", &opt, &reply); err != nil {
+	err := msgpackrpc.CallWithCodec(codec, "Health.ChecksInState", &opt, &reply)
-		t.Fatalf("err: %s", err)
+	require.NoError(err)
-	}
 
 	found := false
 	for _, chk := range reply.HealthChecks {
@@ -1582,9 +1583,8 @@ func TestHealth_ChecksInState_FilterACL(t *testing.T) {
 			t.Fatalf("bad service 'bar': %#v", reply.HealthChecks)
 		}
 	}
-	if !found {
+	require.True(found, "missing service 'foo': %#v", reply.HealthChecks)
-		t.Fatalf("missing service 'foo': %#v", reply.HealthChecks)
+	require.True(reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
-	}
 
 	// We've already proven that we call the ACL filtering function so we
 	// test node filtering down in acl.go for node cases. This also proves