osrg / gobgp

BGP implemented in the Go Programming Language
https://osrg.github.io/gobgp/
Apache License 2.0
3.65k stars 697 forks source link

v3 API ignores Global ApplyPolicy configuration #2814

Open dswaffordcw opened 5 months ago

dswaffordcw commented 5 months ago

Hi,

I am working with an existing GoBGP integration that uses the "v3 API", via a direct Go import. I am attempting to configure a default import policy over the API similar to the example shown in https://github.com/osrg/gobgp/blob/master/docs/sources/policy.md#42-attach-policy-to-route-server-client.

I am not seeing the expected result. This configuration should reject all announced BGP paths toward the GoBGP instance, yet they continue to be accepted and imported.

In https://github.com/osrg/gobgp/issues/1768#issuecomment-1839399167, I see another person reported the same issue with the V3 API on an old, closed, issue.

My code is as follows:


    gobgp "github.com/osrg/gobgp/v3/api"
    "github.com/osrg/gobgp/v3/pkg/server"

    s := server.NewBgpServer(server.LoggerOption(logger))
    go s.Serve()

    startReq := &gobgp.StartBgpRequest{
        Global: &gobgp.Global{
            Asn:        params.Global.ASN,
            RouterId:   params.Global.RouterID,
            ListenPort: params.Global.ListenPort,
            ApplyPolicy: &gobgp.ApplyPolicy{
                ImportPolicy: &gobgp.PolicyAssignment{
                    DefaultAction: gobgp.RouteAction_REJECT,
                },
            },
        },
    }

    if err := s.StartBgp(ctx, startReq); err != nil {
        return nil, fmt.Errorf("failed starting BGP server: %w", err)
    }

In the output below, I am observing the accepted/imported routes from the GoBGP instance using a custom CLI tool. This is a route that was announced to a remote FRR instance and then advertised into this GoBGP instance. It should have been rejected.

$  cilium bgp routes
(Defaulting to `available ipv4 unicast` routes, please see help for more options)

Node                              VRouter   Prefix           NextHop    Age      Attrs
bgp-cplane-dev-v4-control-plane   65001     100.64.12.1/32   10.0.1.1   30m51s   [{Origin: i} {AsPath: 65000 65002} {Nexthop: 10.0.1.1} {Communities: 65002:100} {LargeCommunity: [ 65002:100:1]}]
fujita commented 5 months ago

Policy in gobgp.StartBgpRequest will be ignored if I remember correctly.

You could work around like the following:


s.SetPolicyAssignment(context.Background(), &api.SetPolicyAssignmentRequest{
    Assignment: &api.PolicyAssignment{
        Name:          "global",
        Direction:     api.PolicyDirection_IMPORT,
        DefaultAction: api.RouteAction_REJECT,
    },
})
dswaffordcw commented 5 months ago

@fujita Thanks! SetPolicyAssignment() works well.

If configuring a policy is not supported during startup, would you be open to a pull request to delete ApplyPolicy from message Global on https://github.com/osrg/gobgp/blob/master/api/gobgp.proto#L1109C1-L1109C33?

Or adding an error on startup if it's populated?

fujita commented 5 months ago

I prefer to add an error because I don't want to change the API without the major version updated.