consul: better tests for acl filtering

consul/catalog_endpoint_test.go

@@ -778,18 +778,14 @@ func TestCatalogRegister_FailedCase1(t *testing.T) {
 	}
 }
 
-func TestCatalog_filterACL(t *testing.T) {
-	dir, srv := testServerWithConfig(t, func(c *Config) {
+func testACLFilterServer(t *testing.T) (dir, token string, srv *Server, client *rpc.Client) {
+	dir, srv = testServerWithConfig(t, func(c *Config) {
 		c.ACLDatacenter = "dc1"
 		c.ACLMasterToken = "root"
 		c.ACLDefaultPolicy = "deny"
 	})
-	defer os.RemoveAll(dir)
-	defer srv.Shutdown()
-
-	client := rpcClient(t, srv)
-	defer client.Close()
 
+	client = rpcClient(t, srv)
 	testutil.WaitForLeader(t, client.Call, "dc1")
 
 	// Create a new token
@@ -803,8 +799,7 @@ func TestCatalog_filterACL(t *testing.T) {
 		},
 		WriteRequest: structs.WriteRequest{Token: "root"},
 	}
-	var id string
-	if err := client.Call("ACL.Apply", &arg, &id); err != nil {
+	if err := client.Call("ACL.Apply", &arg, &token); err != nil {
 		t.Fatalf("err: %v", err)
 	}
 
@@ -847,90 +842,101 @@ func TestCatalog_filterACL(t *testing.T) {
 	if err := client.Call("Catalog.Register", &regArg, nil); err != nil {
 		t.Fatalf("err: %s", err)
 	}
+	return
+}
 
-	// Catalog.ListServices properly filters and returns services
-	{
-		opt := structs.DCSpecificRequest{
-			Datacenter:   "dc1",
-			QueryOptions: structs.QueryOptions{Token: id},
-		}
-		reply := structs.IndexedServices{}
-		if err := client.Call("Catalog.ListServices", &opt, &reply); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		if _, ok := reply.Services["foo"]; !ok {
-			t.Fatalf("bad: %#v", reply.Services)
-		}
-		if _, ok := reply.Services["bar"]; ok {
-			t.Fatalf("bad: %#v", reply.Services)
+func TestCatalog_ListServices_FilterACL(t *testing.T) {
+	dir, token, srv, client := testACLFilterServer(t)
+	defer os.RemoveAll(dir)
+	defer srv.Shutdown()
+	defer client.Close()
+
+	opt := structs.DCSpecificRequest{
+		Datacenter:   "dc1",
+		QueryOptions: structs.QueryOptions{Token: token},
+	}
+	reply := structs.IndexedServices{}
+	if err := client.Call("Catalog.ListServices", &opt, &reply); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if _, ok := reply.Services["foo"]; !ok {
+		t.Fatalf("bad: %#v", reply.Services)
+	}
+	if _, ok := reply.Services["bar"]; ok {
+		t.Fatalf("bad: %#v", reply.Services)
+	}
+}
+
+func TestCatalog_ServiceNodes_FilterACL(t *testing.T) {
+	dir, token, srv, client := testACLFilterServer(t)
+	defer os.RemoveAll(dir)
+	defer srv.Shutdown()
+	defer client.Close()
+
+	opt := structs.ServiceSpecificRequest{
+		Datacenter:   "dc1",
+		ServiceName:  "foo",
+		QueryOptions: structs.QueryOptions{Token: token},
+	}
+	reply := structs.IndexedServiceNodes{}
+	if err := client.Call("Catalog.ServiceNodes", &opt, &reply); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	found := false
+	for _, sn := range reply.ServiceNodes {
+		if sn.ServiceID == "foo" {
+			found = true
+			break
 		}
 	}
+	if !found {
+		t.Fatalf("bad: %#v", reply.ServiceNodes)
+	}
 
-	// Catalog.ServiceNodes returns services we have access to
-	{
-		opt := structs.ServiceSpecificRequest{
-			Datacenter:   "dc1",
-			ServiceName:  "foo",
-			QueryOptions: structs.QueryOptions{Token: id},
-		}
-		reply := structs.IndexedServiceNodes{}
-		if err := client.Call("Catalog.ServiceNodes", &opt, &reply); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		found := false
-		for _, sn := range reply.ServiceNodes {
-			if sn.ServiceID == "foo" {
-				found = true
-				break
-			}
-		}
-		if !found {
+	// Filters services we can't access
+	opt = structs.ServiceSpecificRequest{
+		Datacenter:   "dc1",
+		ServiceName:  "bar",
+		QueryOptions: structs.QueryOptions{Token: token},
+	}
+	reply = structs.IndexedServiceNodes{}
+	if err := client.Call("Catalog.ServiceNodes", &opt, &reply); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	for _, sn := range reply.ServiceNodes {
+		if sn.ServiceID == "bar" {
 			t.Fatalf("bad: %#v", reply.ServiceNodes)
 		}
 	}
+}
 
-	// Catalog.ServiceNodes filters services we can't access
-	{
-		opt := structs.ServiceSpecificRequest{
-			Datacenter:   "dc1",
-			ServiceName:  "bar",
-			QueryOptions: structs.QueryOptions{Token: id},
+func TestCatalog_NodeServices_FilterACL(t *testing.T) {
+	dir, token, srv, client := testACLFilterServer(t)
+	defer os.RemoveAll(dir)
+	defer srv.Shutdown()
+	defer client.Close()
+
+	opt := structs.NodeSpecificRequest{
+		Datacenter:   "dc1",
+		Node:         srv.config.NodeName,
+		QueryOptions: structs.QueryOptions{Token: token},
+	}
+	reply := structs.IndexedNodeServices{}
+	if err := client.Call("Catalog.NodeServices", &opt, &reply); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	found := false
+	for _, svc := range reply.NodeServices.Services {
+		if svc.ID == "bar" {
+			t.Fatalf("bad: %#v", reply.NodeServices.Services)
 		}
-		reply := structs.IndexedServiceNodes{}
-		if err := client.Call("Catalog.ServiceNodes", &opt, &reply); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		for _, sn := range reply.ServiceNodes {
-			if sn.ServiceID == "bar" {
-				t.Fatalf("bad: %#v", reply.ServiceNodes)
-			}
+		if svc.ID == "foo" {
+			found = true
+			break
 		}
 	}
-
-	// Catalog.NodeServices filters and returns services properly
-	{
-		opt := structs.NodeSpecificRequest{
-			Datacenter:   "dc1",
-			Node:         srv.config.NodeName,
-			QueryOptions: structs.QueryOptions{Token: id},
-		}
-		reply := structs.IndexedNodeServices{}
-		if err := client.Call("Catalog.NodeServices", &opt, &reply); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		found := false
-		for _, svc := range reply.NodeServices.Services {
-			if svc.ID == "bar" {
-				t.Fatalf("bad: %#v", reply.NodeServices.Services)
-			}
-			if svc.ID == "foo" {
-				found = true
-				break
-			}
-		}
-		if !found {
-			t.Fatalf("bad: %#v", reply.NodeServices)
-		}
+	if !found {
+		t.Fatalf("bad: %#v", reply.NodeServices)
 	}
 }
 

consul/health_endpoint_test.go

@@ -223,155 +223,96 @@ func TestHealth_ServiceNodes(t *testing.T) {
 	}
 }
 
-func TestHealth_filterACL(t *testing.T) {
-	dir, srv := testServerWithConfig(t, func(c *Config) {
-		c.ACLDatacenter = "dc1"
-		c.ACLMasterToken = "root"
-		c.ACLDefaultPolicy = "deny"
-	})
+func TestHealth_NodeChecks_FilterACL(t *testing.T) {
+	dir, token, srv, client := testACLFilterServer(t)
 	defer os.RemoveAll(dir)
 	defer srv.Shutdown()
-
-	client := rpcClient(t, srv)
 	defer client.Close()
 
-	testutil.WaitForLeader(t, client.Call, "dc1")
-
-	// Create a new token
-	arg := structs.ACLRequest{
-		Datacenter: "dc1",
-		Op:         structs.ACLSet,
-		ACL: structs.ACL{
-			Name:  "User token",
-			Type:  structs.ACLTypeClient,
-			Rules: testRegisterRules,
-		},
-		WriteRequest: structs.WriteRequest{Token: "root"},
+	opt := structs.NodeSpecificRequest{
+		Datacenter:   "dc1",
+		Node:         srv.config.NodeName,
+		QueryOptions: structs.QueryOptions{Token: token},
 	}
-	var id string
-	if err := client.Call("ACL.Apply", &arg, &id); err != nil {
-		t.Fatalf("err: %v", err)
-	}
-
-	// Register a service we have access to
-	regArg := structs.RegisterRequest{
-		Datacenter: "dc1",
-		Node:       srv.config.NodeName,
-		Address:    "127.0.0.1",
-		Service: &structs.NodeService{
-			ID:      "foo",
-			Service: "foo",
-		},
-		Check: &structs.HealthCheck{
-			CheckID:   "service:foo",
-			Name:      "service:foo",
-			ServiceID: "foo",
-		},
-		WriteRequest: structs.WriteRequest{Token: "root"},
-	}
-	if err := client.Call("Catalog.Register", &regArg, nil); err != nil {
+	reply := structs.IndexedHealthChecks{}
+	if err := client.Call("Health.NodeChecks", &opt, &reply); err != nil {
 		t.Fatalf("err: %s", err)
 	}
-
-	// Register a service we don't have access to
-	regArg = structs.RegisterRequest{
-		Datacenter: "dc1",
-		Node:       srv.config.NodeName,
-		Address:    "127.0.0.1",
-		Service: &structs.NodeService{
-			ID:      "bar",
-			Service: "bar",
-		},
-		Check: &structs.HealthCheck{
-			CheckID:   "service:bar",
-			Name:      "service:bar",
-			ServiceID: "bar",
-		},
-		WriteRequest: structs.WriteRequest{Token: "root"},
-	}
-	if err := client.Call("Catalog.Register", &regArg, nil); err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	// Health.NodeChecks properly filters and returns services
-	{
-		opt := structs.NodeSpecificRequest{
-			Datacenter:   "dc1",
-			Node:         srv.config.NodeName,
-			QueryOptions: structs.QueryOptions{Token: id},
-		}
-		reply := structs.IndexedHealthChecks{}
-		if err := client.Call("Health.NodeChecks", &opt, &reply); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		found := false
-		for _, chk := range reply.HealthChecks {
-			switch chk.ServiceName {
-			case "foo":
-				found = true
-			case "bar":
-				t.Fatalf("bad: %#v", reply.HealthChecks)
-			}
-		}
-		if !found {
+	found := false
+	for _, chk := range reply.HealthChecks {
+		switch chk.ServiceName {
+		case "foo":
+			found = true
+		case "bar":
 			t.Fatalf("bad: %#v", reply.HealthChecks)
 		}
 	}
-
-	// Health.ServiceChecks properly filters and returns services
-	{
-		opt := structs.ServiceSpecificRequest{
-			Datacenter:   "dc1",
-			ServiceName:  "foo",
-			QueryOptions: structs.QueryOptions{Token: id},
-		}
-		reply := structs.IndexedHealthChecks{}
-		if err := client.Call("Health.ServiceChecks", &opt, &reply); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		found := false
-		for _, chk := range reply.HealthChecks {
-			if chk.ServiceName == "foo" {
-				found = true
-				break
-			}
-		}
-		if !found {
-			t.Fatalf("bad: %#v", reply.HealthChecks)
-		}
-
-		opt.ServiceName = "bar"
-		reply = structs.IndexedHealthChecks{}
-		if err := client.Call("Health.ServiceChecks", &opt, &reply); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		if len(reply.HealthChecks) != 0 {
-			t.Fatalf("bad: %#v", reply.HealthChecks)
-		}
-	}
-
-	// Health.ServiceNodes properly filters and returns services
-	{
-		opt := structs.ServiceSpecificRequest{
-			Datacenter:   "dc1",
-			ServiceName:  "foo",
-			QueryOptions: structs.QueryOptions{Token: id},
-		}
-		reply := structs.IndexedCheckServiceNodes{}
-		if err := client.Call("Health.ServiceNodes", &opt, &reply); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		if len(reply.Nodes) != 1 {
-			t.Fatalf("bad: %#v", reply.Nodes)
-		}
-
-		opt.ServiceName = "bar"
-		reply = structs.IndexedCheckServiceNodes{}
-		if err := client.Call("Health.ServiceNodes", &opt, &reply); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		if len(reply.Nodes) != 0 {
-			t.Fatalf("bad: %#v", reply.Nodes)
-		}
+	if !found {
+		t.Fatalf("bad: %#v", reply.HealthChecks)
+	}
+}
+
+func TestHealth_ServiceChecks_FilterACL(t *testing.T) {
+	dir, token, srv, client := testACLFilterServer(t)
+	defer os.RemoveAll(dir)
+	defer srv.Shutdown()
+	defer client.Close()
+
+	opt := structs.ServiceSpecificRequest{
+		Datacenter:   "dc1",
+		ServiceName:  "foo",
+		QueryOptions: structs.QueryOptions{Token: token},
+	}
+	reply := structs.IndexedHealthChecks{}
+	if err := client.Call("Health.ServiceChecks", &opt, &reply); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	found := false
+	for _, chk := range reply.HealthChecks {
+		if chk.ServiceName == "foo" {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Fatalf("bad: %#v", reply.HealthChecks)
+	}
+
+	opt.ServiceName = "bar"
+	reply = structs.IndexedHealthChecks{}
+	if err := client.Call("Health.ServiceChecks", &opt, &reply); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if len(reply.HealthChecks) != 0 {
+		t.Fatalf("bad: %#v", reply.HealthChecks)
+	}
+}
+
+func TestHealth_ServiceNodes_FilterACL(t *testing.T) {
+	dir, token, srv, client := testACLFilterServer(t)
+	defer os.RemoveAll(dir)
+	defer srv.Shutdown()
+	defer client.Close()
+
+	opt := structs.ServiceSpecificRequest{
+		Datacenter:   "dc1",
+		ServiceName:  "foo",
+		QueryOptions: structs.QueryOptions{Token: token},
+	}
+	reply := structs.IndexedCheckServiceNodes{}
+	if err := client.Call("Health.ServiceNodes", &opt, &reply); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if len(reply.Nodes) != 1 {
+		t.Fatalf("bad: %#v", reply.Nodes)
+	}
+
+	opt.ServiceName = "bar"
+	reply = structs.IndexedCheckServiceNodes{}
+	if err := client.Call("Health.ServiceNodes", &opt, &reply); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if len(reply.Nodes) != 0 {
+		t.Fatalf("bad: %#v", reply.Nodes)
 	}
 }

consul/internal_endpoint_test.go

@@ -239,152 +239,89 @@ func TestInternal_KeyringOperation(t *testing.T) {
 	}
 }
 
-func TestInternal_filterACL(t *testing.T) {
-	dir, srv := testServerWithConfig(t, func(c *Config) {
-		c.ACLDatacenter = "dc1"
-		c.ACLMasterToken = "root"
-		c.ACLDefaultPolicy = "deny"
-	})
+func TestInternal_NodeInfo_FilterACL(t *testing.T) {
+	dir, token, srv, client := testACLFilterServer(t)
 	defer os.RemoveAll(dir)
 	defer srv.Shutdown()
-
-	client := rpcClient(t, srv)
 	defer client.Close()
 
-	testutil.WaitForLeader(t, client.Call, "dc1")
-
-	// Create a new token
-	arg := structs.ACLRequest{
-		Datacenter: "dc1",
-		Op:         structs.ACLSet,
-		ACL: structs.ACL{
-			Name:  "User token",
-			Type:  structs.ACLTypeClient,
-			Rules: testRegisterRules,
-		},
-		WriteRequest: structs.WriteRequest{Token: "root"},
+	opt := structs.NodeSpecificRequest{
+		Datacenter:   "dc1",
+		Node:         srv.config.NodeName,
+		QueryOptions: structs.QueryOptions{Token: token},
 	}
-	var id string
-	if err := client.Call("ACL.Apply", &arg, &id); err != nil {
-		t.Fatalf("err: %v", err)
-	}
-
-	// Register a service we have access to
-	regArg := structs.RegisterRequest{
-		Datacenter: "dc1",
-		Node:       srv.config.NodeName,
-		Address:    "127.0.0.1",
-		Service: &structs.NodeService{
-			ID:      "foo",
-			Service: "foo",
-		},
-		Check: &structs.HealthCheck{
-			CheckID:   "service:foo",
-			Name:      "service:foo",
-			ServiceID: "foo",
-		},
-		WriteRequest: structs.WriteRequest{Token: "root"},
-	}
-	if err := client.Call("Catalog.Register", &regArg, nil); err != nil {
+	reply := structs.IndexedNodeDump{}
+	if err := client.Call("Health.NodeChecks", &opt, &reply); err != nil {
 		t.Fatalf("err: %s", err)
 	}
-
-	// Register a service we don't have access to
-	regArg = structs.RegisterRequest{
-		Datacenter: "dc1",
-		Node:       srv.config.NodeName,
-		Address:    "127.0.0.1",
-		Service: &structs.NodeService{
-			ID:      "bar",
-			Service: "bar",
-		},
-		Check: &structs.HealthCheck{
-			CheckID:   "service:bar",
-			Name:      "service:bar",
-			ServiceID: "bar",
-		},
-		WriteRequest: structs.WriteRequest{Token: "root"},
-	}
-	if err := client.Call("Catalog.Register", &regArg, nil); err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	// Internal.NodeInfo filters services properly
-	{
-		opt := structs.NodeSpecificRequest{
-			Datacenter:   "dc1",
-			Node:         srv.config.NodeName,
-			QueryOptions: structs.QueryOptions{Token: id},
-		}
-		reply := structs.IndexedNodeDump{}
-		if err := client.Call("Health.NodeChecks", &opt, &reply); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		for _, info := range reply.Dump {
-			found := false
-			for _, chk := range info.Checks {
-				if chk.ServiceName == "foo" {
-					found = true
-				}
-				if chk.ServiceName == "bar" {
-					t.Fatalf("bad: %#v", info.Checks)
-				}
+	for _, info := range reply.Dump {
+		found := false
+		for _, chk := range info.Checks {
+			if chk.ServiceName == "foo" {
+				found = true
 			}
-			if !found {
+			if chk.ServiceName == "bar" {
 				t.Fatalf("bad: %#v", info.Checks)
 			}
+		}
+		if !found {
+			t.Fatalf("bad: %#v", info.Checks)
+		}
 
-			found = false
-			for _, svc := range info.Services {
-				if svc.Service == "foo" {
-					found = true
-				}
-				if svc.Service == "bar" {
-					t.Fatalf("bad: %#v", info.Services)
-				}
+		found = false
+		for _, svc := range info.Services {
+			if svc.Service == "foo" {
+				found = true
 			}
-			if !found {
+			if svc.Service == "bar" {
 				t.Fatalf("bad: %#v", info.Services)
 			}
 		}
-	}
-
-	// Internal.NodeDump filters services properly
-	{
-		opt := structs.DCSpecificRequest{
-			Datacenter:   "dc1",
-			QueryOptions: structs.QueryOptions{Token: id},
-		}
-		reply := structs.IndexedNodeDump{}
-		if err := client.Call("Health.NodeChecks", &opt, &reply); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		for _, info := range reply.Dump {
-			found := false
-			for _, chk := range info.Checks {
-				if chk.ServiceName == "foo" {
-					found = true
-				}
-				if chk.ServiceName == "bar" {
-					t.Fatalf("bad: %#v", info.Checks)
-				}
-			}
-			if !found {
-				t.Fatalf("bad: %#v", info.Checks)
-			}
-
-			found = false
-			for _, svc := range info.Services {
-				if svc.Service == "foo" {
-					found = true
-				}
-				if svc.Service == "bar" {
-					t.Fatalf("bad: %#v", info.Services)
-				}
-			}
-			if !found {
-				t.Fatalf("bad: %#v", info.Services)
-			}
+		if !found {
+			t.Fatalf("bad: %#v", info.Services)
+		}
+	}
+}
+
+func TestInternal_NodeDump_FilterACL(t *testing.T) {
+	dir, token, srv, client := testACLFilterServer(t)
+	defer os.RemoveAll(dir)
+	defer srv.Shutdown()
+	defer client.Close()
+
+	opt := structs.DCSpecificRequest{
+		Datacenter:   "dc1",
+		QueryOptions: structs.QueryOptions{Token: token},
+	}
+	reply := structs.IndexedNodeDump{}
+	if err := client.Call("Health.NodeChecks", &opt, &reply); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	for _, info := range reply.Dump {
+		found := false
+		for _, chk := range info.Checks {
+			if chk.ServiceName == "foo" {
+				found = true
+			}
+			if chk.ServiceName == "bar" {
+				t.Fatalf("bad: %#v", info.Checks)
+			}
+		}
+		if !found {
+			t.Fatalf("bad: %#v", info.Checks)
+		}
+
+		found = false
+		for _, svc := range info.Services {
+			if svc.Service == "foo" {
+				found = true
+			}
+			if svc.Service == "bar" {
+				t.Fatalf("bad: %#v", info.Services)
+			}
+		}
+		if !found {
+			t.Fatalf("bad: %#v", info.Services)
 		}
 	}
 }