Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v3 API ignores Global ApplyPolicy configuration #2814

Open
dswaffordcw opened this issue Jun 11, 2024 · 3 comments
Open

v3 API ignores Global ApplyPolicy configuration #2814

dswaffordcw opened this issue Jun 11, 2024 · 3 comments

Comments

@dswaffordcw
Copy link

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 #1768 (comment), 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]}]
@dswaffordcw dswaffordcw changed the title v3 API ignores ApplyPolicy configuration v3 API ignores Global.ApplyPolicy configuration Jun 11, 2024
@dswaffordcw dswaffordcw changed the title v3 API ignores Global.ApplyPolicy configuration v3 API ignores Global ApplyPolicy Jun 11, 2024
@dswaffordcw dswaffordcw changed the title v3 API ignores Global ApplyPolicy v3 API ignores Global ApplyPolicy configuration Jun 11, 2024
@fujita
Copy link
Member

fujita commented Jun 14, 2024

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
Copy link
Author

@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
Copy link
Member

fujita commented Jun 15, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants