Repository: osrg/gobgp

Issue:
Wrong as-path-lengh policy implement
I am defining policies below:

```
policy-definitions:

   ...

  # Export: dont export providers/peers only provider+peer routes
  - name: outbound-policy
    statements:

      ...

      # Export self route
      - conditions:
          match-prefix-set:
            prefix-set: self_address
            match-set-options: any
          bgp-conditions:
            # route-type: local
            as-path-length:
              operator: eq
              value: 0
        actions:
          route-disposition: accept-route

      # Don't export routes directly connected to neighbors
      - conditions:
          match-prefix-set:
            prefix-set: connect_address
            match-set-options: any
          bgp-conditions:
            # route-type: local
            as-path-length:
              operator: ge
              value: 2
        actions:
          route-disposition: accept-route

      # Export self generated routes
      - conditions:
          match-prefix-set:
            prefix-set: connect_address
            match-set-options: invert
          bgp-conditions:
            # route-type: local
            as-path-length:
              operator: le
              value: 1
        actions:
          route-disposition: accept-route
```

But `gobgp policy` shows:
```
    StatementName outbound-policy_stmt3:
      Conditions:
        PrefixSet: any self_address 
        AsPathLength: >=0
        RPKI result: UNSPECIFIED
      Actions:
         accept
    StatementName outbound-policy_stmt4:
      Conditions:
        PrefixSet: any connect_address 
        AsPathLength: <=2
        RPKI result: UNSPECIFIED
      Actions:
         accept
    StatementName outbound-policy_stmt5:
      Conditions:
        PrefixSet: invert connect_address 
        AsPathLength: =1
        RPKI result: UNSPECIFIED
      Actions:
```

It seems that "eq", "ge", and "le" are wrongly mapped to ">=", "<=", and "=".

I am using latest commit:
> cb60aef8ef9a66a9732de4a46ff3627dd690d2db

I am trying to fix this and may update this issue later.

Hints from issue comments:
@ponedo: The policies i wrote caused unexpected route export. I feel like this is not just a error in printing.

Solution patch:
diff --git a/pkg/server/grpc_server.go b/pkg/server/grpc_server.go
index 54432f53..6d0b862f 100644
--- a/pkg/server/grpc_server.go
+++ b/pkg/server/grpc_server.go
@@ -1420,6 +1420,19 @@ func (s *server) DeleteDefinedSet(ctx context.Context, r *api.DeleteDefinedSetRe
 
 var _regexpMedActionType = regexp.MustCompile(`([+-]?)(\d+)`)
 
+func toOcAttributeComparison(a api.Comparison) oc.AttributeComparison {
+	switch a {
+	case api.Comparison_COMPARISON_EQ:
+		return oc.ATTRIBUTE_COMPARISON_EQ
+	case api.Comparison_COMPARISON_GE:
+		return oc.ATTRIBUTE_COMPARISON_GE
+	case api.Comparison_COMPARISON_LE:
+		return oc.ATTRIBUTE_COMPARISON_LE
+	default:
+		return oc.ATTRIBUTE_COMPARISON_EQ
+	}
+}
+
 func matchSetOptionsRestrictedTypeToAPI(t oc.MatchSetOptionsRestrictedType) api.MatchSet_Type {
 	t = t.DefaultAsNeeded()
 	switch t {
@@ -1726,7 +1739,7 @@ func newCommunityCountConditionFromApiStruct(a *api.CommunityCount) (*table.Comm
 		return nil, nil
 	}
 	return table.NewCommunityCountCondition(oc.CommunityCount{
-		Operator: oc.IntToAttributeComparisonMap[int(a.Type)],
+		Operator: toOcAttributeComparison(a.Type),
 		Value:    a.Count,
 	})
 }
@@ -1736,7 +1749,7 @@ func newAsPathLengthConditionFromApiStruct(a *api.AsPathLength) (*table.AsPathLe
 		return nil, nil
 	}
 	return table.NewAsPathLengthCondition(oc.AsPathLength{
-		Operator: oc.IntToAttributeComparisonMap[int(a.Type)],
+		Operator: toOcAttributeComparison(a.Type),
 		Value:    a.Length,
 	})
 }



Test patch:
diff --git a/pkg/server/grpc_server_test.go b/pkg/server/grpc_server_test.go
index 6991ea07..62bc9e36 100644
--- a/pkg/server/grpc_server_test.go
+++ b/pkg/server/grpc_server_test.go
@@ -12,6 +12,7 @@ import (
 	"github.com/osrg/gobgp/v4/api"
 	"github.com/osrg/gobgp/v4/internal/pkg/table"
 	"github.com/osrg/gobgp/v4/pkg/apiutil"
+	"github.com/osrg/gobgp/v4/pkg/config/oc"
 	"github.com/osrg/gobgp/v4/pkg/packet/bgp"
 	"github.com/stretchr/testify/assert"
 	"google.golang.org/grpc"
@@ -447,3 +448,69 @@ func TestGRPCWatchEvent(t *testing.T) {
 
 	assert.Equal(2, count)
 }
+
+func TestToOcAttributeComparison(t *testing.T) {
+	tests := []struct {
+		in   api.Comparison
+		want oc.AttributeComparison
+	}{
+		{api.Comparison_COMPARISON_EQ, oc.ATTRIBUTE_COMPARISON_EQ},
+		{api.Comparison_COMPARISON_GE, oc.ATTRIBUTE_COMPARISON_GE},
+		{api.Comparison_COMPARISON_LE, oc.ATTRIBUTE_COMPARISON_LE},
+	}
+	for _, tt := range tests {
+		if got := toOcAttributeComparison(tt.in); got != tt.want {
+			t.Fatalf("toOcAttributeComparison(%v) = %v, want %v", tt.in, got, tt.want)
+		}
+	}
+}
+
+func TestNewAsPathLengthConditionFromApiStruct(t *testing.T) {
+	tests := []struct {
+		inType api.Comparison
+		inVal  uint32
+		wantOp string
+	}{
+		{api.Comparison_COMPARISON_EQ, 1, "="},
+		{api.Comparison_COMPARISON_GE, 2, ">="},
+		{api.Comparison_COMPARISON_LE, 3, "<="},
+	}
+	for _, tt := range tests {
+		cond, err := newAsPathLengthConditionFromApiStruct(&api.AsPathLength{Type: tt.inType, Length: tt.inVal})
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if cond == nil {
+			t.Fatalf("condition is nil")
+		}
+		got := cond.String()
+		if got[:len(tt.wantOp)] != tt.wantOp {
+			t.Fatalf("operator mismatch: got %q want prefix %q", got, tt.wantOp)
+		}
+	}
+}
+
+func TestNewCommunityCountConditionFromApiStruct(t *testing.T) {
+	tests := []struct {
+		inType api.Comparison
+		inVal  uint32
+		wantOp string
+	}{
+		{api.Comparison_COMPARISON_EQ, 10, "="},
+		{api.Comparison_COMPARISON_GE, 20, ">="},
+		{api.Comparison_COMPARISON_LE, 30, "<="},
+	}
+	for _, tt := range tests {
+		cond, err := newCommunityCountConditionFromApiStruct(&api.CommunityCount{Type: tt.inType, Count: tt.inVal})
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if cond == nil {
+			t.Fatalf("condition is nil")
+		}
+		got := cond.String()
+		if got[:len(tt.wantOp)] != tt.wantOp {
+			t.Fatalf("operator mismatch: got %q want prefix %q", got, tt.wantOp)
+		}
+	}
+}