pkg/server: ListPolicyAssignment() returns all assignments by default

Like other ListPolicy*() functions, if no conditions are specified,
all assignments are returned.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

6 files changed, 146 insertions, 85 deletions

diff --git a/cmd/gobgp/cmd/common.go b/cmd/gobgp/cmd/common.go
index a1104836..f4a72b1e 100644
--- a/cmd/gobgp/cmd/common.go
+++ b/cmd/gobgp/cmd/common.go
@@ -32,6 +32,8 @@ import (
 	"github.com/osrg/gobgp/pkg/packet/bgp"
 )
 
+const GLOBAL_RIB_NAME = "global"
+
 const (
 	CMD_GLOBAL         = "global"
 	CMD_NEIGHBOR       = "neighbor"
diff --git a/cmd/gobgp/cmd/neighbor.go b/cmd/gobgp/cmd/neighbor.go
index 6bd531a6..e863003a 100644
--- a/cmd/gobgp/cmd/neighbor.go
+++ b/cmd/gobgp/cmd/neighbor.go
@@ -1072,6 +1072,8 @@ func modNeighborPolicy(remoteIP, policyType, cmdType string, args []string) erro
 	resource := api.Resource_GLOBAL
 	if remoteIP != "" {
 		resource = api.Resource_LOCAL
+	} else {
+		remoteIP = GLOBAL_RIB_NAME
 	}
 
 	assign := &api.PolicyAssignment{
diff --git a/cmd/gobgpd/main.go b/cmd/gobgpd/main.go
index 146facad..463387b9 100644
--- a/cmd/gobgpd/main.go
+++ b/cmd/gobgpd/main.go
@@ -334,7 +334,7 @@ func main() {
 						ps := toPolicies(a.ImportPolicyList)
 						apiServer.ReplacePolicyAssignment(context.Background(), &api.ReplacePolicyAssignmentRequest{
 							Assignment: server.NewAPIPolicyAssignmentFromTableStruct(&table.PolicyAssignment{
-								Name:     "",
+								Name:     table.GLOBAL_RIB_NAME,
 								Type:     table.POLICY_DIRECTION_IMPORT,
 								Policies: ps,
 								Default:  def,
@@ -345,7 +345,7 @@ func main() {
 						ps = toPolicies(a.ExportPolicyList)
 						apiServer.ReplacePolicyAssignment(context.Background(), &api.ReplacePolicyAssignmentRequest{
 							Assignment: server.NewAPIPolicyAssignmentFromTableStruct(&table.PolicyAssignment{
-								Name:     "",
+								Name:     table.GLOBAL_RIB_NAME,
 								Type:     table.POLICY_DIRECTION_EXPORT,
 								Policies: ps,
 								Default:  def,
diff --git a/pkg/server/grpc_server.go b/pkg/server/grpc_server.go
index c325ed41..d2173925 100644
--- a/pkg/server/grpc_server.go
+++ b/pkg/server/grpc_server.go
@@ -2300,32 +2300,6 @@ func (s *Server) ReplacePolicy(ctx context.Context, r *api.ReplacePolicyRequest)
 	return &empty.Empty{}, s.bgpServer.ReplacePolicy(ctx, r)
 }
 
-func toPolicyAssignmentName(a *api.PolicyAssignment) (string, table.PolicyDirection, error) {
-	switch a.Resource {
-	case api.Resource_GLOBAL:
-		switch a.Type {
-		case api.PolicyDirection_IMPORT:
-			return "", table.POLICY_DIRECTION_IMPORT, nil
-		case api.PolicyDirection_EXPORT:
-			return "", table.POLICY_DIRECTION_EXPORT, nil
-		default:
-			return "", table.POLICY_DIRECTION_NONE, fmt.Errorf("invalid policy type")
-		}
-	case api.Resource_LOCAL:
-		switch a.Type {
-		case api.PolicyDirection_IMPORT:
-			return a.Name, table.POLICY_DIRECTION_IMPORT, nil
-		case api.PolicyDirection_EXPORT:
-			return a.Name, table.POLICY_DIRECTION_EXPORT, nil
-		default:
-			return "", table.POLICY_DIRECTION_NONE, fmt.Errorf("invalid policy type")
-		}
-	default:
-		return "", table.POLICY_DIRECTION_NONE, fmt.Errorf("invalid resource type")
-	}
-
-}
-
 func (s *Server) ListPolicyAssignment(r *api.ListPolicyAssignmentRequest, stream api.GobgpApi_ListPolicyAssignmentServer) error {
 	l, err := s.bgpServer.ListPolicyAssignment(context.Background(), r)
 	if err == nil {
diff --git a/pkg/server/server.go b/pkg/server/server.go
index 1ac5845b..c69ebbfb 100644
--- a/pkg/server/server.go
+++ b/pkg/server/server.go
@@ -3114,63 +3114,86 @@ func (s *BgpServer) ReplacePolicy(ctx context.Context, r *api.ReplacePolicyReque
 	}, false)
 }
 
-func (server *BgpServer) toPolicyInfo(name string, dir table.PolicyDirection) (string, error) {
+func (s *BgpServer) toPolicyInfo(name string, dir api.PolicyDirection) (string, table.PolicyDirection, error) {
 	if name == "" {
-		switch dir {
-		case table.POLICY_DIRECTION_IMPORT, table.POLICY_DIRECTION_EXPORT:
-			return table.GLOBAL_RIB_NAME, nil
-		}
-		return "", fmt.Errorf("invalid policy type")
+		return "", table.POLICY_DIRECTION_NONE, fmt.Errorf("empty table name")
+	}
+
+	if name == table.GLOBAL_RIB_NAME {
+		name = table.GLOBAL_RIB_NAME
 	} else {
-		peer, ok := server.neighborMap[name]
+		peer, ok := s.neighborMap[name]
 		if !ok {
-			return "", fmt.Errorf("not found peer %s", name)
+			return "", table.POLICY_DIRECTION_NONE, fmt.Errorf("not found peer %s", name)
 		}
 		if !peer.isRouteServerClient() {
-			return "", fmt.Errorf("non-rs-client peer %s doesn't have per peer policy", name)
+			return "", table.POLICY_DIRECTION_NONE, fmt.Errorf("non-rs-client peer %s doesn't have per peer policy", name)
 		}
-		return peer.ID(), nil
+		name = peer.ID()
 	}
+	switch dir {
+	case api.PolicyDirection_IMPORT:
+		return name, table.POLICY_DIRECTION_IMPORT, nil
+	case api.PolicyDirection_EXPORT:
+		return name, table.POLICY_DIRECTION_EXPORT, nil
+	}
+	return "", table.POLICY_DIRECTION_NONE, fmt.Errorf("invalid policy type")
 }
 
 func (s *BgpServer) ListPolicyAssignment(ctx context.Context, r *api.ListPolicyAssignmentRequest) ([]*api.PolicyAssignment, error) {
 	a := make([]*api.PolicyAssignment, 0)
 	err := s.mgmtOperation(func() error {
-		if r == nil || r.Direction == api.PolicyDirection_UNKNOWN {
+		if r == nil {
 			return fmt.Errorf("invalid request")
 		}
-		dir := func() table.PolicyDirection {
-			if r.Direction == api.PolicyDirection_EXPORT {
-				return table.POLICY_DIRECTION_EXPORT
-			}
-			return table.POLICY_DIRECTION_IMPORT
-		}()
 
-		id, err := s.toPolicyInfo(r.Name, dir)
-		if err != nil {
-			return err
+		names := make([]string, 0, len(s.neighborMap)+1)
+		if r.Name == "" {
+			names = append(names, table.GLOBAL_RIB_NAME)
+			for name := range s.neighborMap {
+				names = append(names, name)
+			}
+		} else {
+			names = append(names, r.Name)
 		}
-		rt, l, err := s.policy.GetPolicyAssignment(id, dir)
-		if err != nil {
-			return err
+		dirs := make([]api.PolicyDirection, 0, 2)
+		if r.Direction == api.PolicyDirection_UNKNOWN {
+			dirs = []api.PolicyDirection{api.PolicyDirection_EXPORT, api.PolicyDirection_IMPORT}
+		} else {
+			dirs = append(dirs, r.Direction)
 		}
 
-		policies := make([]*table.Policy, 0, len(l))
-		for _, p := range l {
-			t, err := table.NewPolicy(*p)
-			if err != nil {
-				return err
+		for _, name := range names {
+			for _, dir := range dirs {
+				id, dir, err := s.toPolicyInfo(name, dir)
+				if err != nil {
+					return err
+				}
+				rt, l, err := s.policy.GetPolicyAssignment(id, dir)
+				if err != nil {
+					return err
+				}
+				if len(l) == 0 {
+					continue
+				}
+				policies := make([]*table.Policy, 0, len(l))
+				for _, p := range l {
+					np, err := table.NewPolicy(*p)
+					if err != nil {
+						return err
+					}
+					policies = append(policies, np)
+				}
+				t := &table.PolicyAssignment{
+					Name:     name,
+					Type:     dir,
+					Default:  rt,
+					Policies: policies,
+				}
+				a = append(a, NewAPIPolicyAssignmentFromTableStruct(t))
 			}
-			policies = append(policies, t)
-		}
-		t := &table.PolicyAssignment{
-			Name:     r.Name,
-			Type:     dir,
-			Default:  rt,
-			Policies: policies,
 		}
-		a = append(a, NewAPIPolicyAssignmentFromTableStruct(t))
-		return err
+		return nil
 	}, false)
 	return a, err
 }
@@ -3180,12 +3203,7 @@ func (s *BgpServer) AddPolicyAssignment(ctx context.Context, r *api.AddPolicyAss
 		if r == nil || r.Assignment == nil {
 			return fmt.Errorf("invalid request")
 		}
-		name, dir, err := toPolicyAssignmentName(r.Assignment)
-		if err != nil {
-			return err
-		}
-
-		id, err := s.toPolicyInfo(name, dir)
+		id, dir, err := s.toPolicyInfo(r.Assignment.Name, r.Assignment.Type)
 		if err != nil {
 			return err
 		}
@@ -3198,12 +3216,7 @@ func (s *BgpServer) DeletePolicyAssignment(ctx context.Context, r *api.DeletePol
 		if r == nil || r.Assignment == nil {
 			return fmt.Errorf("invalid request")
 		}
-		name, dir, err := toPolicyAssignmentName(r.Assignment)
-		if err != nil {
-			return err
-		}
-
-		id, err := s.toPolicyInfo(name, dir)
+		id, dir, err := s.toPolicyInfo(r.Assignment.Name, r.Assignment.Type)
 		if err != nil {
 			return err
 		}
@@ -3211,17 +3224,12 @@ func (s *BgpServer) DeletePolicyAssignment(ctx context.Context, r *api.DeletePol
 	}, false)
 }
 
-//func (s *BgpServer) ReplacePolicyAssignment(name string, dir table.PolicyDirection, policies []*config.PolicyDefinition, def table.RouteType) error {
 func (s *BgpServer) ReplacePolicyAssignment(ctx context.Context, r *api.ReplacePolicyAssignmentRequest) error {
 	return s.mgmtOperation(func() error {
 		if r == nil || r.Assignment == nil {
 			return fmt.Errorf("invalid request")
 		}
-		name, dir, err := toPolicyAssignmentName(r.Assignment)
-		if err != nil {
-			return err
-		}
-		id, err := s.toPolicyInfo(name, dir)
+		id, dir, err := s.toPolicyInfo(r.Assignment.Name, r.Assignment.Type)
 		if err != nil {
 			return err
 		}
diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go
index 742d9b93..b12481bd 100644
--- a/pkg/server/server_test.go
+++ b/pkg/server/server_test.go
@@ -17,6 +17,7 @@ package server
 
 import (
 	"context"
+	"fmt"
 	"net"
 	"runtime"
 	"testing"
@@ -69,21 +70,95 @@ func TestModPolicyAssign(t *testing.T) {
 	r := f([]*config.PolicyDefinition{&config.PolicyDefinition{Name: "p1"}, &config.PolicyDefinition{Name: "p2"}, &config.PolicyDefinition{Name: "p3"}})
 	r.Type = api.PolicyDirection_IMPORT
 	r.Default = api.RouteAction_ACCEPT
+	r.Name = table.GLOBAL_RIB_NAME
 	err = s.AddPolicyAssignment(context.Background(), &api.AddPolicyAssignmentRequest{Assignment: r})
 	assert.Nil(err)
 
-	ps, err := s.ListPolicyAssignment(context.Background(), &api.ListPolicyAssignmentRequest{Direction: api.PolicyDirection_IMPORT})
+	r.Type = api.PolicyDirection_EXPORT
+	err = s.AddPolicyAssignment(context.Background(), &api.AddPolicyAssignmentRequest{Assignment: r})
+	assert.Nil(err)
+
+	ps, err := s.ListPolicyAssignment(context.Background(), &api.ListPolicyAssignmentRequest{
+		Name:      table.GLOBAL_RIB_NAME,
+		Direction: api.PolicyDirection_IMPORT})
 	assert.Nil(err)
 	assert.Equal(len(ps[0].Policies), 3)
 
 	r = f([]*config.PolicyDefinition{&config.PolicyDefinition{Name: "p1"}})
 	r.Type = api.PolicyDirection_IMPORT
 	r.Default = api.RouteAction_ACCEPT
+	r.Name = table.GLOBAL_RIB_NAME
 	err = s.DeletePolicyAssignment(context.Background(), &api.DeletePolicyAssignmentRequest{Assignment: r})
 	assert.Nil(err)
 
-	ps, _ = s.ListPolicyAssignment(context.Background(), &api.ListPolicyAssignmentRequest{Direction: api.PolicyDirection_IMPORT})
+	ps, _ = s.ListPolicyAssignment(context.Background(), &api.ListPolicyAssignmentRequest{
+		Name:      table.GLOBAL_RIB_NAME,
+		Direction: api.PolicyDirection_IMPORT})
 	assert.Equal(len(ps[0].Policies), 2)
+
+	ps, _ = s.ListPolicyAssignment(context.Background(), &api.ListPolicyAssignmentRequest{
+		Name: table.GLOBAL_RIB_NAME,
+	})
+	assert.Equal(len(ps), 2)
+}
+
+func TestListPolicyAssignment(t *testing.T) {
+	assert := assert.New(t)
+
+	s := NewBgpServer()
+	go s.Serve()
+	err := s.StartBgp(context.Background(), &api.StartBgpRequest{
+		Global: &api.Global{
+			As:         1,
+			RouterId:   "1.1.1.1",
+			ListenPort: -1,
+		},
+	})
+	assert.Nil(err)
+
+	for i := 1; i < 4; i++ {
+		addr := fmt.Sprintf("127.0.0.%d", i)
+		p := &api.Peer{
+			Conf: &api.PeerConf{
+				NeighborAddress: addr,
+				PeerAs:          uint32(i + 1),
+			},
+			RouteServer: &api.RouteServer{
+				RouteServerClient: true,
+			},
+		}
+		err = s.AddPeer(context.Background(), &api.AddPeerRequest{Peer: p})
+		assert.Nil(err)
+
+		err = s.AddPolicy(context.Background(),
+			&api.AddPolicyRequest{Policy: NewAPIPolicyFromTableStruct(&table.Policy{Name: fmt.Sprintf("p%d", i)})})
+		assert.Nil(err)
+
+		pa := &api.PolicyAssignment{
+			Type:     api.PolicyDirection_IMPORT,
+			Default:  api.RouteAction_ACCEPT,
+			Name:     addr,
+			Policies: []*api.Policy{&api.Policy{Name: fmt.Sprintf("p%d", i)}},
+		}
+		err = s.AddPolicyAssignment(context.Background(), &api.AddPolicyAssignmentRequest{Assignment: pa})
+		assert.Nil(err)
+	}
+
+	ps, err := s.ListPolicyAssignment(context.Background(), &api.ListPolicyAssignmentRequest{
+		Name: table.GLOBAL_RIB_NAME,
+	})
+	assert.Nil(err)
+	assert.Equal(len(ps), 0)
+
+	ps, err = s.ListPolicyAssignment(context.Background(), &api.ListPolicyAssignmentRequest{})
+	assert.Nil(err)
+	assert.Equal(len(ps), 3)
+
+	ps, err = s.ListPolicyAssignment(context.Background(), &api.ListPolicyAssignmentRequest{
+		Direction: api.PolicyDirection_EXPORT,
+	})
+	assert.Nil(err)
+	assert.Equal(len(ps), 0)
 }
 
 func TestMonitor(test *testing.T) {