Add gRPC proto for rate-limit xDS Config communication

[renuka-fernando]

Related Issue

#201

[mattklein123]

I think it would probably be better to put this in https://github.com/cncf/xds. cc @envoyproxy/api-shepherds any thoughts? This is a new xDS based API for serving rate limit configurations to a rate limit server. Possibly related to the parallel work the Google folks are doing on the new quota based rate limit API.

[markdroth]

Instead of building this based on the old RLS protocol, It may make sense do it based on the new RLQS protocol introduced in envoyproxy/envoy#19793. This is currently being implemented in Envoy by @tyxia (CC @yanavlasov @sergiitk). I think in the long run, we will want to move away from RLS and instead recommend using RLQS, which is much more scalable, so it may not make sense to invest more in RLS at this point.

In terms of what repo it should go in, I don't think I have a strong feeling. As I mentioned below, I think we're trying to move the xDS ecosystem to a model where everything uses ADS, which can handle any arbitrary resource type, and there's not really any requirement that all xDS resource type protos are defined in a single repo, as long as there are no namespace collisions (which there generally aren't, since people tend to use the name of the repo as the top-level proto package name). So it seems fine to keep it here, but I also wouldn't object to having it in the cncf/xds repo if there's a desire to do that.

[markdroth]

I don't think we need a new RPC service for this. I believe we've agreed that as a general strategy for the xDS ecosystem, we're trying to move away from having an individual RPC service for every resource type. Instead, we should just use ADS for whatever resource types we want.

[mattklein123]

Yeah using ADS sounds like the right path to me, then we don't need much extra work here.

[mattklein123]

Instead of building this based on the old RLS protocol, It may make sense do it based on the new RLQS protocol introduced in envoyproxy/envoy#19793. This is currently being implemented in Envoy by @tyxia (CC @yanavlasov @sergiitk). I think in the long run, we will want to move away from RLS and instead recommend using RLQS, which is much more scalable, so it may not make sense to invest more in RLS at this point.

Lots of people use the existing RLS and it works fine for many cases. I think it's fine to have work still done on that.

[renuka-fernando]

Hi @markdroth, @mattklein123,

Could you please guide me adding this on the cncf/xds repo?
I would like to know the proper location that the proto files should be included and other changes that should be made.

I drafted a PR with removing the service RateLimitConfigDiscoveryService, which is included in this PR, since we are going to use ADS. Hence it is just the RLS config proto messages in the draft PR: cncf/xds#51.

[mattklein123]

As we said above, I don't think we have to add anything to either cncf/xds or go-control-plane. You can use ADS and define the protos wherever you want, including here. I'm going to close this PR for now so feel free to open a new simpler one.

[renuka-fernando]

As we said above, I don't think we have to add anything to either cncf/xds or go-control-plane. You can use ADS and define the protos wherever you want, including here. I'm going to close this PR for now so feel free to open a new simpler one.

Hi @mattklein123, thank you for your quick response.

If we add RLS config type as a known type in go-control-plane as in the PR: envoyproxy/go-control-plane#598, we can use the existing Snapshot in go-control-plane to easily implement a server.

import (
	"github.com/envoyproxy/go-control-plane/pkg/cache/types"
	"github.com/envoyproxy/go-control-plane/pkg/cache/v3"
	"github.com/envoyproxy/go-control-plane/pkg/resource/v3"
	"github.com/envoyproxy/go-control-plane/pkg/server/v3"
)
...
	snapshot, _ := cache.NewSnapshot("1",
		map[resource.Type][]types.Resource{
			resource.RateLimitConfigType: {
				&rls_config.RateLimitConfig{
					Domain: "foo",
					Descriptors: []*rls_config.RateLimitDescriptor{
						{
							Key:   "k1",
							Value: "v1",
							RateLimit: &rls_config.RateLimitPolicy{
								Unit:            "minute",
								RequestsPerUnit: 3,
							},
						},
					},
				},
			},
		},
	)

	if err := snapshot.Consistent(); err != nil {
		log.Printf("snapshot inconsistency: %+v\n%+v", snapshot, err)
	}

	snapCache = cache.NewSnapshotCache(false, cache.IDHash{}, nil)
	if err := snapCache.SetSnapshot(context.Background(), "test-node", snapshot); err != nil {
		log.Printf("snapshot error %q for %+v", err, snapshot)
	}

	grpcServer = grpc.NewServer(grpcOptions...)

	lis, err := net.Listen("tcp", fmt.Sprintf(":%d", port))
	if err != nil {
		log.Fatal(err)
	}

	discoverygrpc.RegisterAggregatedDiscoveryServiceServer(grpcServer, srv)
	if err = grpcServer.Serve(lis); err != nil {
		log.Println(err)
	}

Otherwise, we can implement our own Snapshot in the ratelimit repo and use the SnapshotCache in go-control-plane to implement an xDS server for ratelimit service.

I think adding known types is required even if we use ADS. Please correct me if I am wrong.
Please share your thoughts on this.

cc @alecholmez