policy cleanup

- remove ReplaceDefinedSet and ReplaceStatement APIs; not intutive and
  should create a new one instead of modifying the existing.

- Rename ReplacePolicyAssignment to SetPolicyAssignment API; we use
  internally SetPolicy() name from the beginning.

- Rename UpdatePolicy() to SetPolicies() API; It doesn't update
  anything so the name is confusing. It discards the all policies and
  create policies from the argument.

- Changes some member names in structures for policy.

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

3 files changed, 83 insertions, 107 deletions

diff --git a/pkg/server/grpc_server.go b/pkg/server/grpc_server.go
index d2173925..009c0fd2 100644
--- a/pkg/server/grpc_server.go
+++ b/pkg/server/grpc_server.go
@@ -131,12 +131,12 @@ func NewAfiSafiConfigFromConfigStruct(c *config.AfiSafi) *api.AfiSafiConfig {
 func NewApplyPolicyFromConfigStruct(c *config.ApplyPolicy) *api.ApplyPolicy {
 	applyPolicy := &api.ApplyPolicy{
 		ImportPolicy: &api.PolicyAssignment{
-			Type:    api.PolicyDirection_IMPORT,
-			Default: api.RouteAction(c.Config.DefaultImportPolicy.ToInt()),
+			Direction:     api.PolicyDirection_IMPORT,
+			DefaultAction: api.RouteAction(c.Config.DefaultImportPolicy.ToInt()),
 		},
 		ExportPolicy: &api.PolicyAssignment{
-			Type:    api.PolicyDirection_EXPORT,
-			Default: api.RouteAction(c.Config.DefaultExportPolicy.ToInt()),
+			Direction:     api.PolicyDirection_EXPORT,
+			DefaultAction: api.RouteAction(c.Config.DefaultExportPolicy.ToInt()),
 		},
 	}
 
@@ -639,8 +639,8 @@ func (s *Server) DisablePeer(ctx context.Context, r *api.DisablePeerRequest) (*e
 	return &empty.Empty{}, s.bgpServer.DisableNeighbor(ctx, r)
 }
 
-func (s *Server) UpdatePolicy(ctx context.Context, r *api.UpdatePolicyRequest) (*empty.Empty, error) {
-	return &empty.Empty{}, s.bgpServer.UpdatePolicy(ctx, r)
+func (s *Server) SetPolicies(ctx context.Context, r *api.SetPoliciesRequest) (*empty.Empty, error) {
+	return &empty.Empty{}, s.bgpServer.SetPolicies(ctx, r)
 }
 
 func NewAPIRoutingPolicyFromConfigStruct(c *config.RoutingPolicy) (*api.RoutingPolicy, error) {
@@ -654,12 +654,12 @@ func NewAPIRoutingPolicyFromConfigStruct(c *config.RoutingPolicy) (*api.RoutingP
 	}
 
 	return &api.RoutingPolicy{
-		DefinedSet:       definedSets,
-		PolicyDefinition: policies,
+		DefinedSets: definedSets,
+		Policies:    policies,
 	}, nil
 }
 
-func NewRoutingPolicyFromApiStruct(arg *api.UpdatePolicyRequest) (*config.RoutingPolicy, error) {
+func NewRoutingPolicyFromApiStruct(arg *api.SetPoliciesRequest) (*config.RoutingPolicy, error) {
 	policyDefinitions := make([]config.PolicyDefinition, 0, len(arg.Policies))
 	for _, p := range arg.Policies {
 		pd, err := NewConfigPolicyFromApiStruct(p)
@@ -669,7 +669,7 @@ func NewRoutingPolicyFromApiStruct(arg *api.UpdatePolicyRequest) (*config.Routin
 		policyDefinitions = append(policyDefinitions, *pd)
 	}
 
-	definedSets, err := NewConfigDefinedSetsFromApiStruct(arg.Sets)
+	definedSets, err := NewConfigDefinedSetsFromApiStruct(arg.DefinedSets)
 	if err != nil {
 		return nil, err
 	}
@@ -910,19 +910,19 @@ func ReadApplyPolicyFromAPIStruct(c *config.ApplyPolicy, a *api.ApplyPolicy) {
 		return
 	}
 	if a.ImportPolicy != nil {
-		c.Config.DefaultImportPolicy = config.IntToDefaultPolicyTypeMap[int(a.ImportPolicy.Default)]
+		c.Config.DefaultImportPolicy = config.IntToDefaultPolicyTypeMap[int(a.ImportPolicy.DefaultAction)]
 		for _, p := range a.ImportPolicy.Policies {
 			c.Config.ImportPolicyList = append(c.Config.ImportPolicyList, p.Name)
 		}
 	}
 	if a.ExportPolicy != nil {
-		c.Config.DefaultExportPolicy = config.IntToDefaultPolicyTypeMap[int(a.ExportPolicy.Default)]
+		c.Config.DefaultExportPolicy = config.IntToDefaultPolicyTypeMap[int(a.ExportPolicy.DefaultAction)]
 		for _, p := range a.ExportPolicy.Policies {
 			c.Config.ExportPolicyList = append(c.Config.ExportPolicyList, p.Name)
 		}
 	}
 	if a.InPolicy != nil {
-		c.Config.DefaultInPolicy = config.IntToDefaultPolicyTypeMap[int(a.InPolicy.Default)]
+		c.Config.DefaultInPolicy = config.IntToDefaultPolicyTypeMap[int(a.InPolicy.DefaultAction)]
 		for _, p := range a.InPolicy.Policies {
 			c.Config.InPolicyList = append(c.Config.InPolicyList, p.Name)
 		}
@@ -1582,7 +1582,7 @@ func (s *Server) ListDefinedSet(r *api.ListDefinedSetRequest, stream api.GobgpAp
 		return err
 	}
 	for _, set := range sets {
-		if err := stream.Send(&api.ListDefinedSetResponse{Set: set}); err != nil {
+		if err := stream.Send(&api.ListDefinedSetResponse{DefinedSet: set}); err != nil {
 			return err
 		}
 	}
@@ -1597,10 +1597,6 @@ func (s *Server) DeleteDefinedSet(ctx context.Context, r *api.DeleteDefinedSetRe
 	return &empty.Empty{}, s.bgpServer.DeleteDefinedSet(ctx, r)
 }
 
-func (s *Server) ReplaceDefinedSet(ctx context.Context, r *api.ReplaceDefinedSetRequest) (*empty.Empty, error) {
-	return &empty.Empty{}, s.bgpServer.ReplaceDefinedSet(ctx, r)
-}
-
 func NewAPIStatementFromTableStruct(t *table.Statement) *api.Statement {
 	return toStatementApi(t.ToConfig())
 }
@@ -2157,10 +2153,6 @@ func (s *Server) DeleteStatement(ctx context.Context, r *api.DeleteStatementRequ
 	return &empty.Empty{}, s.bgpServer.DeleteStatement(ctx, r)
 }
 
-func (s *Server) ReplaceStatement(ctx context.Context, r *api.ReplaceStatementRequest) (*empty.Empty, error) {
-	return &empty.Empty{}, s.bgpServer.ReplaceStatement(ctx, r)
-}
-
 func NewAPIPolicyFromTableStruct(p *table.Policy) *api.Policy {
 	return toPolicyApi(p.ToConfig())
 }
@@ -2180,7 +2172,7 @@ func toPolicyApi(p *config.PolicyDefinition) *api.Policy {
 
 func NewAPIPolicyAssignmentFromTableStruct(t *table.PolicyAssignment) *api.PolicyAssignment {
 	return &api.PolicyAssignment{
-		Type: func() api.PolicyDirection {
+		Direction: func() api.PolicyDirection {
 			switch t.Type {
 			case table.POLICY_DIRECTION_IMPORT:
 				return api.PolicyDirection_IMPORT
@@ -2190,7 +2182,7 @@ func NewAPIPolicyAssignmentFromTableStruct(t *table.PolicyAssignment) *api.Polic
 			log.Errorf("invalid policy-type: %s", t.Type)
 			return api.PolicyDirection_UNKNOWN
 		}(),
-		Default: func() api.RouteAction {
+		DefaultAction: func() api.RouteAction {
 			switch t.Default {
 			case table.ROUTE_TYPE_ACCEPT:
 				return api.RouteAction_ACCEPT
@@ -2200,12 +2192,6 @@ func NewAPIPolicyAssignmentFromTableStruct(t *table.PolicyAssignment) *api.Polic
 			return api.RouteAction_NONE
 		}(),
 		Name: t.Name,
-		Resource: func() api.Resource {
-			if t.Name != "" {
-				return api.Resource_LOCAL
-			}
-			return api.Resource_GLOBAL
-		}(),
 		Policies: func() []*api.Policy {
 			l := make([]*api.Policy, 0)
 			for _, p := range t.Policies {
@@ -2296,10 +2282,6 @@ func (s *Server) DeletePolicy(ctx context.Context, r *api.DeletePolicyRequest) (
 	return &empty.Empty{}, s.bgpServer.DeletePolicy(ctx, r)
 }
 
-func (s *Server) ReplacePolicy(ctx context.Context, r *api.ReplacePolicyRequest) (*empty.Empty, error) {
-	return &empty.Empty{}, s.bgpServer.ReplacePolicy(ctx, r)
-}
-
 func (s *Server) ListPolicyAssignment(r *api.ListPolicyAssignmentRequest, stream api.GobgpApi_ListPolicyAssignmentServer) error {
 	l, err := s.bgpServer.ListPolicyAssignment(context.Background(), r)
 	if err == nil {
@@ -2339,8 +2321,8 @@ func (s *Server) DeletePolicyAssignment(ctx context.Context, r *api.DeletePolicy
 	return &empty.Empty{}, s.bgpServer.DeletePolicyAssignment(ctx, r)
 }
 
-func (s *Server) ReplacePolicyAssignment(ctx context.Context, r *api.ReplacePolicyAssignmentRequest) (*empty.Empty, error) {
-	return &empty.Empty{}, s.bgpServer.ReplacePolicyAssignment(ctx, r)
+func (s *Server) SetPolicyAssignment(ctx context.Context, r *api.SetPolicyAssignmentRequest) (*empty.Empty, error) {
+	return &empty.Empty{}, s.bgpServer.SetPolicyAssignment(ctx, r)
 }
 
 func (s *Server) GetBgp(ctx context.Context, r *api.GetBgpRequest) (*api.GetBgpResponse, error) {
diff --git a/pkg/server/server.go b/pkg/server/server.go
index c69ebbfb..c76ec45d 100644
--- a/pkg/server/server.go
+++ b/pkg/server/server.go
@@ -1613,22 +1613,63 @@ func (s *BgpServer) StopBgp(ctx context.Context, r *api.StopBgpRequest) error {
 	return nil
 }
 
-func (s *BgpServer) UpdatePolicy(ctx context.Context, r *api.UpdatePolicyRequest) error {
+func (s *BgpServer) SetPolicies(ctx context.Context, r *api.SetPoliciesRequest) error {
 	rp, err := NewRoutingPolicyFromApiStruct(r)
 	if err != nil {
 		return err
 	}
 
+	getConfig := func(id string) (*config.ApplyPolicy, error) {
+		f := func(id string, dir table.PolicyDirection) (config.DefaultPolicyType, []string, error) {
+			rt, policies, err := s.policy.GetPolicyAssignment(id, dir)
+			if err != nil {
+				return config.DEFAULT_POLICY_TYPE_REJECT_ROUTE, nil, err
+			}
+			names := make([]string, 0, len(policies))
+			for _, p := range policies {
+				names = append(names, p.Name)
+			}
+			t := config.DEFAULT_POLICY_TYPE_ACCEPT_ROUTE
+			if rt == table.ROUTE_TYPE_REJECT {
+				t = config.DEFAULT_POLICY_TYPE_REJECT_ROUTE
+			}
+			return t, names, nil
+		}
+
+		c := &config.ApplyPolicy{}
+		rt, policies, err := f(id, table.POLICY_DIRECTION_IMPORT)
+		if err != nil {
+			return nil, err
+		}
+		c.Config.ImportPolicyList = policies
+		c.Config.DefaultImportPolicy = rt
+		rt, policies, err = f(id, table.POLICY_DIRECTION_EXPORT)
+		if err != nil {
+			return nil, err
+		}
+		c.Config.ExportPolicyList = policies
+		c.Config.DefaultExportPolicy = rt
+		return c, nil
+	}
+
 	return s.mgmtOperation(func() error {
 		ap := make(map[string]config.ApplyPolicy, len(s.neighborMap)+1)
-		ap[table.GLOBAL_RIB_NAME] = s.bgpConfig.Global.ApplyPolicy
+		a, err := getConfig(table.GLOBAL_RIB_NAME)
+		if err != nil {
+			return err
+		}
+		ap[table.GLOBAL_RIB_NAME] = *a
 		for _, peer := range s.neighborMap {
 			peer.fsm.lock.RLock()
 			log.WithFields(log.Fields{
 				"Topic": "Peer",
 				"Key":   peer.fsm.pConf.State.NeighborAddress,
 			}).Info("call set policy")
-			ap[peer.ID()] = peer.fsm.pConf.ApplyPolicy
+			a, err := getConfig(peer.ID())
+			if err != nil {
+				return err
+			}
+			ap[peer.ID()] = *a
 			peer.fsm.lock.RUnlock()
 		}
 		return s.policy.Reset(rp, ap)
@@ -2970,10 +3011,10 @@ func (s *BgpServer) ListDefinedSet(ctx context.Context, r *api.ListDefinedSetReq
 
 func (s *BgpServer) AddDefinedSet(ctx context.Context, r *api.AddDefinedSetRequest) error {
 	return s.mgmtOperation(func() error {
-		if r == nil || r.Set == nil {
+		if r == nil || r.DefinedSet == nil {
 			return fmt.Errorf("invalid request")
 		}
-		set, err := NewDefinedSetFromApiStruct(r.Set)
+		set, err := NewDefinedSetFromApiStruct(r.DefinedSet)
 		if err != nil {
 			return err
 		}
@@ -2983,10 +3024,10 @@ func (s *BgpServer) AddDefinedSet(ctx context.Context, r *api.AddDefinedSetReque
 
 func (s *BgpServer) DeleteDefinedSet(ctx context.Context, r *api.DeleteDefinedSetRequest) error {
 	return s.mgmtOperation(func() error {
-		if r == nil || r.Set == nil {
+		if r == nil || r.DefinedSet == nil {
 			return fmt.Errorf("invalid request")
 		}
-		set, err := NewDefinedSetFromApiStruct(r.Set)
+		set, err := NewDefinedSetFromApiStruct(r.DefinedSet)
 		if err != nil {
 			return err
 		}
@@ -2994,19 +3035,6 @@ func (s *BgpServer) DeleteDefinedSet(ctx context.Context, r *api.DeleteDefinedSe
 	}, false)
 }
 
-func (s *BgpServer) ReplaceDefinedSet(ctx context.Context, r *api.ReplaceDefinedSetRequest) error {
-	return s.mgmtOperation(func() error {
-		if r == nil || r.Set == nil {
-			return fmt.Errorf("invalid request")
-		}
-		set, err := NewDefinedSetFromApiStruct(r.Set)
-		if err != nil {
-			return err
-		}
-		return s.policy.ReplaceDefinedSet(set)
-	}, false)
-}
-
 func (s *BgpServer) ListStatement(ctx context.Context, r *api.ListStatementRequest) ([]*api.Statement, error) {
 	l := make([]*api.Statement, 0)
 	s.mgmtOperation(func() error {
@@ -3044,19 +3072,6 @@ func (s *BgpServer) DeleteStatement(ctx context.Context, r *api.DeleteStatementR
 	}, false)
 }
 
-func (s *BgpServer) ReplaceStatement(ctx context.Context, r *api.ReplaceStatementRequest) error {
-	return s.mgmtOperation(func() error {
-		if r == nil || r.Statement == nil {
-			return fmt.Errorf("invalid request")
-		}
-		st, err := NewStatementFromApiStruct(r.Statement)
-		if err == nil {
-			err = s.policy.ReplaceStatement(st)
-		}
-		return err
-	}, false)
-}
-
 func (s *BgpServer) ListPolicy(ctx context.Context, r *api.ListPolicyRequest) ([]*api.Policy, error) {
 	l := make([]*api.Policy, 0)
 	s.mgmtOperation(func() error {
@@ -3101,19 +3116,6 @@ func (s *BgpServer) DeletePolicy(ctx context.Context, r *api.DeletePolicyRequest
 	}, false)
 }
 
-func (s *BgpServer) ReplacePolicy(ctx context.Context, r *api.ReplacePolicyRequest) error {
-	return s.mgmtOperation(func() error {
-		if r == nil || r.Policy == nil {
-			return fmt.Errorf("invalid request")
-		}
-		p, err := NewPolicyFromApiStruct(r.Policy)
-		if err == nil {
-			err = s.policy.ReplacePolicy(p, r.ReferExistingStatements, r.PreserveStatements)
-		}
-		return err
-	}, false)
-}
-
 func (s *BgpServer) toPolicyInfo(name string, dir api.PolicyDirection) (string, table.PolicyDirection, error) {
 	if name == "" {
 		return "", table.POLICY_DIRECTION_NONE, fmt.Errorf("empty table name")
@@ -3169,21 +3171,13 @@ func (s *BgpServer) ListPolicyAssignment(ctx context.Context, r *api.ListPolicyA
 				if err != nil {
 					return err
 				}
-				rt, l, err := s.policy.GetPolicyAssignment(id, dir)
+				rt, policies, err := s.policy.GetPolicyAssignment(id, dir)
 				if err != nil {
 					return err
 				}
-				if len(l) == 0 {
+				if len(policies) == 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,
@@ -3203,11 +3197,11 @@ func (s *BgpServer) AddPolicyAssignment(ctx context.Context, r *api.AddPolicyAss
 		if r == nil || r.Assignment == nil {
 			return fmt.Errorf("invalid request")
 		}
-		id, dir, err := s.toPolicyInfo(r.Assignment.Name, r.Assignment.Type)
+		id, dir, err := s.toPolicyInfo(r.Assignment.Name, r.Assignment.Direction)
 		if err != nil {
 			return err
 		}
-		return s.policy.AddPolicyAssignment(id, dir, toPolicyDefinition(r.Assignment.Policies), defaultRouteType(r.Assignment.Default))
+		return s.policy.AddPolicyAssignment(id, dir, toPolicyDefinition(r.Assignment.Policies), defaultRouteType(r.Assignment.DefaultAction))
 	}, false)
 }
 
@@ -3216,7 +3210,7 @@ func (s *BgpServer) DeletePolicyAssignment(ctx context.Context, r *api.DeletePol
 		if r == nil || r.Assignment == nil {
 			return fmt.Errorf("invalid request")
 		}
-		id, dir, err := s.toPolicyInfo(r.Assignment.Name, r.Assignment.Type)
+		id, dir, err := s.toPolicyInfo(r.Assignment.Name, r.Assignment.Direction)
 		if err != nil {
 			return err
 		}
@@ -3224,16 +3218,16 @@ func (s *BgpServer) DeletePolicyAssignment(ctx context.Context, r *api.DeletePol
 	}, false)
 }
 
-func (s *BgpServer) ReplacePolicyAssignment(ctx context.Context, r *api.ReplacePolicyAssignmentRequest) error {
+func (s *BgpServer) SetPolicyAssignment(ctx context.Context, r *api.SetPolicyAssignmentRequest) error {
 	return s.mgmtOperation(func() error {
 		if r == nil || r.Assignment == nil {
 			return fmt.Errorf("invalid request")
 		}
-		id, dir, err := s.toPolicyInfo(r.Assignment.Name, r.Assignment.Type)
+		id, dir, err := s.toPolicyInfo(r.Assignment.Name, r.Assignment.Direction)
 		if err != nil {
 			return err
 		}
-		return s.policy.ReplacePolicyAssignment(id, dir, toPolicyDefinition(r.Assignment.Policies), defaultRouteType(r.Assignment.Default))
+		return s.policy.SetPolicyAssignment(id, dir, toPolicyDefinition(r.Assignment.Policies), defaultRouteType(r.Assignment.DefaultAction))
 	}, false)
 }
 
diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go
index b12481bd..e56c6302 100644
--- a/pkg/server/server_test.go
+++ b/pkg/server/server_test.go
@@ -68,13 +68,13 @@ 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.Direction = api.PolicyDirection_IMPORT
+	r.DefaultAction = api.RouteAction_ACCEPT
 	r.Name = table.GLOBAL_RIB_NAME
 	err = s.AddPolicyAssignment(context.Background(), &api.AddPolicyAssignmentRequest{Assignment: r})
 	assert.Nil(err)
 
-	r.Type = api.PolicyDirection_EXPORT
+	r.Direction = api.PolicyDirection_EXPORT
 	err = s.AddPolicyAssignment(context.Background(), &api.AddPolicyAssignmentRequest{Assignment: r})
 	assert.Nil(err)
 
@@ -85,8 +85,8 @@ func TestModPolicyAssign(t *testing.T) {
 	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.Direction = api.PolicyDirection_IMPORT
+	r.DefaultAction = api.RouteAction_ACCEPT
 	r.Name = table.GLOBAL_RIB_NAME
 	err = s.DeletePolicyAssignment(context.Background(), &api.DeletePolicyAssignmentRequest{Assignment: r})
 	assert.Nil(err)
@@ -135,10 +135,10 @@ func TestListPolicyAssignment(t *testing.T) {
 		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)}},
+			Direction:     api.PolicyDirection_IMPORT,
+			DefaultAction: 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)