Skip to content

Comments

proto: use the proto defined in data-plane-api#39

Merged
junr03 merged 4 commits intomasterfrom
data-plane-api-proto
Jun 11, 2018
Merged

proto: use the proto defined in data-plane-api#39
junr03 merged 4 commits intomasterfrom
data-plane-api-proto

Conversation

@junr03
Copy link
Member

@junr03 junr03 commented Jun 4, 2018

Fixes #14

In order to not break any clients using the old proto, Ratelimit is going to support both handlers: the one for the data-plane-api service definition and the legacy service definition.

The legacy handler will accept a request, translate it into a data-plane-api proto request and use the rest of the codebase as if the new handler had been called. This will incur a perf penalty for clients using the legacy proto.

@junr03 junr03 force-pushed the data-plane-api-proto branch from 1934479 to 273e8e3 Compare June 8, 2018 18:02
@junr03 junr03 force-pushed the data-plane-api-proto branch from ea1b4d5 to fb67c15 Compare June 8, 2018 20:48
@junr03 junr03 changed the title [WIP] proto: use the proto defined in data-plane-api proto: use the proto defined in data-plane-api Jun 8, 2018
@junr03
Copy link
Member Author

junr03 commented Jun 8, 2018

@lyft/network-team @rodaine ptal

@junr03
Copy link
Member Author

junr03 commented Jun 8, 2018

@mattklein123 given that we don't have a deprecation policy in place for ratelimit I think what we should do is the following.

  1. Tag current master as v1.0
  2. When we merge this PR tag as v1.1. Announce in envoy-users.
  3. After a couple months remove the legacy shim and tag as v2.0 The reason I propose leaving the shim on master for a couple months is in case people are running master instead of pinning to a version.

wdyt?

@@ -0,0 +1,143 @@
package ratelimit
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why maintain this code at all? Can't you just convert to YAML and back?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the reason I went with this route was to potentially squeeze some perf out of manipulating only golang structs, and not having to deal with marshal/unmarshal oddities which I have had to deal with in envoymanager.

But, the approach you suggest is less code to maintain, and less surface for developer error. I pushed that approach below.

@mattklein123
Copy link
Member

@junr03 yes that plan sounds fine to me.

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go++ would make this less heinous, but LGTM. Nice work. :)

I would probably get @rodaine to give it a skim also.

Copy link
Member

@rodaine rodaine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Could use clearer docs about how this will evolve after this release, but that can be a separate PR.

defined in this repo) requests to the data-plane-api
definitions. Therefore, the ratelimit service will upgrade the requests, process them internally as it would
process a data-plane-api ratelimit request, and then downgrade the response to send back to the client. This means that,
for a slight performance hit for clients using the legacy proto, ratelimit is backwards compatible with the legacy proto.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we supporting it long-term, or just for this release? IIRC, the discussions around this were to yank out the old proto support with next release, correct? If so, I'd recommend adding a note to that effect.

@junr03 junr03 merged commit 2e0387b into master Jun 11, 2018
@junr03 junr03 deleted the data-plane-api-proto branch July 5, 2018 17:28
mjallday referenced this pull request in verygoodsecurity/ratelimit Jan 26, 2021
* Add Docker Compose File (#27)

* docs: match envoy docs for remote_address ratelimiting (#29)

* docs: document dependency on gostats (#30)

* test and document whitelist behavior (#31)

Signed-off-by: Daniel Hochman <danielhochman@users.noreply.github.com>

* update dependencies (#35)

* update dependencies

* proto: use the proto defined in data-plane-api (#39)

* proto: check in protos to allow importing ratelimit as a library (#40)

* docs: update contact info (#42)

* redis: add the option to use a separate redis pool for per second limits (#41)

* fix duplicate mv (#43)

* docker: upgrade docker-compose setup (#46)

* Add gRPC health check (#47)

* logging: set log level (#50)

* go version: update to 1.11 (#53)

* docker compose: expose gRPC port on docker compose setup (#55)

* Configuration to ignore dotfiles. (#52)

This allows ratelimit to run on Kubernetes with configuration from a configmap.

* Add Dockerfile to enable builds (#58)

Signed-off-by: Steve Sloka <steves@heptio.com>

* ci: fix build (#73)

Fixes envoyproxy#71

Signed-off-by: Matt Klein <mklein@lyft.com>

* Run unit and integration tests with race detector enabled (#65)

* deps: update several of ratelimit's dependencies (#76)

* add stalebot (#78)

Signed-off-by: Matt Klein <mklein@lyft.com>

* docs: fix example 4 sample config (#79)

* fix redis-server binary name (#88)

* Fix build Dockerfile (envoyproxy#98)

Fix problem:
src/service_cmd/runner/runner.go:10:2: cannot find package "github.com/lyft/ratelimit/proto/ratelimit" in any of:
        /usr/local/go/src/github.com/lyft/ratelimit/vendor/github.com/lyft/ratelimit/proto/ratelimit (vendor tree)
        /usr/local/go/src/vendor/github.com/lyft/ratelimit/proto/ratelimit
        /usr/local/go/src/github.com/lyft/ratelimit/proto/ratelimit (from $GOROOT)
        /go/src/github.com/lyft/ratelimit/proto/ratelimit (from $GOPATH)
The command '/bin/sh -c go build -o /usr/local/bin/ratelimit src/service_cmd/main.go' returned a non-zero code: 1

* Redis TLS and Auth support (envoyproxy#96)

This adds support for TLS connections to redis as well as support for authentication.
Somewhat related to issue #61

* healthcheck: allow customizable healthcheck name (envoyproxy#102)

Description: this patch allows a consumer of the server package to customize the name of the healthchecker.

Signed-off-by: Jose Nino <jnino@lyft.com>

* health: make a few more types public (envoyproxy#104)

Description: envoyproxy#102 allowed for some customization. This PR makes the types public so that other servers can use this implementation.

Signed-off-by: Jose Nino <jnino@lyft.com>

* Add local cache to store whether it is over the limit (envoyproxy#111)

* Plugin statstore into runner (envoyproxy#115)

* fix: support auth without tls (envoyproxy#116)

Signed-off-by: tangxinfa <tangxinfa@gmail.com>

* add local cache stats (envoyproxy#114)

* Move license to templated Apache-2.0 (envoyproxy#123)

Signed-off-by: Derek Schaller <d_a_schaller@yahoo.com>

* Enable go modules (envoyproxy#124)

Signed-off-by: Steve Sloka <slokas@vmware.com>

* CI: Github Actions (envoyproxy#127)

Signed-off-by: Steve Sloka <slokas@vmware.com>

* community: update contributing guide (envoyproxy#139)

Fixes envoyproxy#138

Signed-off-by: Matt Klein <mklein@lyft.com>

* add http 1 `/json` endpoint (envoyproxy#136)

Signed-off-by: David Black <david.black@autodesk.com>

* Use mockgen version from go.mod instead of from "make bootstrap" (envoyproxy#143)

Even though the Makefile wants to encourage using mockgen@1.4.1, it
seems like the mocks have been generated using a pre-1.0 version of
mockgen. Using "go run github.com/golang/mock/mockgen" as a go:generate
command instead of just "mockgen" avoids the need to pre-install into
the developer's $PATH and uses the go.mod-specified version

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Upgrade gostats dependency from 0.2.6 to 0.4.0 (envoyproxy#141)

My interest is the UDP protocol support which appeared in gotstats 0.3.10

There's a breaking change as of https://github.com/lyft/gostats/releases/tag/v0.3.0
which is that gostats no longer publishes stats as expvars.

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Upgrade radix (envoyproxy#137)

Signed-off-by: Tong Cai <caitong93@gmail.com>

* cache_impl_test.go: fix failing test with ipv6 (envoyproxy#144)

A newly-added test in envoyproxy#137 checks the exact text of an error message which
seems to vary when the network is tcp4 vs tcp6. This change relaxes the
assertion to look for "connection refused" in a panic without making
assumptions about what an IP address looks like.

Example failure:

--- FAIL: TestNewClientImpl (0.00s)
    --- FAIL: TestNewClientImpl/connection_refused (0.00s)
        cache_impl_test.go:442:
                Error Trace:    cache_impl_test.go:442
                Error:          func (assert.PanicTestFunc)(0x1724110) should panic with error message: "dial tcp 127.0.0.1:12345: connect: connection refused"
                                        Panic value:    "dial tcp [::1]:12345: connect: connection refused"
                                        Panic stack:    goroutine 27 [running]:

The testify assert package doesn't seem to support inexact matching on error messages, so the code gets a bit uglier than before.

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Split redis-specific logic from generic key-value store logic (envoyproxy#142)

This is a pure refactoring with no behavior changes. It's a step toward being able
to add memcache as a backend (see envoyproxy#140).

This PR moves RateLimitCache from the redis package to a new "limiter" package, along with
code for time/jitter, local cache stats,  and constructing cache keys. All that can be reused
with memcache.

After this PR, the redis package is imported in exactly two places:
- in service_cmd/runner/runner.go to call redis.NewRateLimiterCacheImplFromSettings()
- in service/ratelimit.go in ShouldRateLimit to identify if a recovered panic is a redis.RedisError. If so, a stat is incremented and the panic() propagation is ended and in favor of returning the error as a the function result.

The PR also includes changes by goimports to test/service/ratelimit_test.go
so that the difference between package name vs file path name is explicit
instead of implicit.

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* json handler: return full ratelimit service response as json (envoyproxy#148)

Previously an HTTP POST to /json would only return an HTTP status code,
not all the other details supported by grpc ratelimit responses.

With this change an HTTP POST to /json receives the full proto3 response
encoded as json by jsonpb.

It seems unlikely that anyone would be parsing the text "over limit" from
the HTTP body instead of just reading the 429 response code, but for anyone
doing that this would be a breaking change.

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Update goruntime to latest, 0.2.5.  Add new config for watching changes in runtime config folder directly instead of the runtime root dir. (envoyproxy#151)

Signed-off-by: Yuki Sawa <yukisawa@gmail.com>

* Drop support for legacy ratelimit.proto and upgrade to v3 rls.proto (envoyproxy#153)

Signed-off-by: Petr Pchelko <ppchelko@wikimedia.org>

* Followups to v3 upgrade (envoyproxy#155)

- Regenerate mocks based on new default protocol
- Manually transform v2 messages to v3 messages - some of the fields
were renamed thus json Marshal/Unmarshal does not work anymore
- Added tests that verify conversion v2<->v3 works for headers fields
- Update tests to use proto.Equal - simple assert.Equals might not
work correctly for protobuf messages.

Signed-off-by: Petr Pchelko <ppchelko@wikimedia.org>

* Introduce a Dockerfile for running integration tests (envoyproxy#156)

This diff creates Dockerfile.integration for running integration tests with clearly-defined dependencies. Previously the dependencies of the integration tests were defined within the github actions config.

The new "make docker_tests" target should work for any developer with Docker installed.

Previously there was no single command that would run integration tests across platforms, which makes development and onboarding harder. Even copying the command from github actions wouldn't have worked before, since that command quietly assumed that redis was already running on port 6379.

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Add support for rate limit overrides. (envoyproxy#158)

Fixes envoyproxy#154

Signed-off-by: Petr Pchelko <ppchelko@wikimedia.org>

* redis client: default to use explicit pipelining (envoyproxy#163)

Signed-off-by: Tong Cai <caitong93@gmail.com>

* Clean go.mod file and update logrus to latest (envoyproxy#166)

Signed-off-by: Yuki Sawa <yukisawa@gmail.com>

* Add full test environment example. Fix bug in existing docker-compose. (envoyproxy#170)

Signed-off-by: Yuki Sawa <yukisawa@gmail.com>

* Implement LOG_FORMAT=json (envoyproxy#173)

Centralized log collection system works better with logs in json format.
E.g. DataDog strongly encourage setting up your logging library to produce your logs in JSON format to avoid the need for custom parsing rules.
So, the next small fix is all we need to get json logs.

Signed-off-by: Sergey Belyaev <sbelyaev@setronica.com>

* ci: Update github action to push docker image tagged with sha for each merge to master branch (envoyproxy#176)

Updates the github action to also push a tagged image based upon the git sha. The tag also
includes the current version of the release.

Example tag: envoyproxy/ratelimit:f1758150b6dfed3e5c0ae13fb7bb6b8f6ae00b0e

Fixes envoyproxy#174

Signed-off-by: Steve Sloka <slokas@vmware.com>

* Update README.md to refer to existing files (envoyproxy#178)

Signed-off-by: Margaret Gorguissian <margaret.gorguissian@tufts.edu>

* Add redis cluster and sentinel support (envoyproxy#179)

Signed-off-by: Diego Erdody <diego@medallia.com>

* Add support for x-ratelimit-reset header (envoyproxy#182)

Signed-off-by: Clara Andrew-Wani <candrewwani@gmail.com>

* Create repokitteh.star (envoyproxy#187)

Signed-off-by: Itay Donanhirsh <itay@bazoo.org>

* refactor NearLimitRatio to environment variable (envoyproxy#186)

Signed-off-by: zufardhiyaulhaq <zufardhiyaulhaq@gmail.com>

* Fix flakey tests with DurationUntilReset. Update docker example to V3 config. (envoyproxy#192)

Signed-off-by: Yuki Sawa <yukisawa@gmail.com>

* Separate Redis cache and driver implementation (envoyproxy#194)

Signed-off-by: William Albertus Dembo <w.albertusd@gmail.com>

* Set ratelimit filter to v3 api (envoyproxy#196)

Signed-off-by: Yuki Sawa <yukisawa@gmail.com>

* Add debug logging to indicate descriptor and limit (envoyproxy#197)

Signed-off-by: Sasha Kulbii <okulbii@wayfair.com>

* Implement BACKEND_TYPE=memcache as an alternative k/v store to redis (envoyproxy#172)

MEMCACHE_HOST_PORT=host:port must be set with BACKEND_TYPE=memcache

To minimize roundtrips when getting multiple keys, the memcache implementation
does a GetMulti to fetch the existing rate limit usage and does increments
asynchronously in background goroutines, since the memcache API doesn't offer
multi-increment.

Resolves envoyproxy#140

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Refactoring of duplicated code across backend types (envoyproxy#202)

Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>

* Small typo fix in README (envoyproxy#204)

Signed-off-by: cpaika <paika.christopher@gmail.com>

* Circle CI (#1)

* Add Circle Config

* Touch to build

* Move to expected path

* Use Docker Command

* Do it all here then

* Env

* Add Readme

* Actual README

* Add Docker Compose (#2)

* VAULT-893 Fix docker-compose.yml

* VAULT-893 Fix docker-compose.yml

Co-authored-by: Marshall Jones <marshall@offby3.com>
Co-authored-by: Jose Ulises Nino Rivera <junr03@users.noreply.github.com>
Co-authored-by: Daniel Hochman <danielhochman@users.noreply.github.com>
Co-authored-by: Martien Verbruggen <martien.verbruggen@gmail.com>
Co-authored-by: Ben Pope <BenPope@users.noreply.github.com>
Co-authored-by: Steve Sloka <steve@stevesloka.com>
Co-authored-by: Matt Klein <mattklein123@gmail.com>
Co-authored-by: Charlie Vieth <charlie.vieth@gmail.com>
Co-authored-by: Adil Hafeez <ahafeez@lyft.com>
Co-authored-by: Kartograf <kartogrof@gmail.com>
Co-authored-by: repl-david-winiarski <33431229+repl-david-winiarski@users.noreply.github.com>
Co-authored-by: Junchao Lyu <6963707+freedomljc@users.noreply.github.com>
Co-authored-by: tangxinfa <tangxinfa@gmail.com>
Co-authored-by: Steve Sloka <slokas@vmware.com>
Co-authored-by: Matt Klein <mklein@lyft.com>
Co-authored-by: dblackdblack <github@dhb.is>
Co-authored-by: David Weitzman <dweitzman@pinterest.com>
Co-authored-by: Tong Cai <caitong@caicloud.io>
Co-authored-by: Yuki Sawa <yukisawa@gmail.com>
Co-authored-by: Petr Pchelko <petrpchelko@gmail.com>
Co-authored-by: Petr Pchelko <ppchelko@wikimedia.org>
Co-authored-by: Sergey Belyaev <svdba@users.noreply.github.com>
Co-authored-by: Margaret G <Margaret.Gorguissian@tufts.edu>
Co-authored-by: Diego Erdody <erdody@gmail.com>
Co-authored-by: Clara <candrewwani@gmail.com>
Co-authored-by: Itay Donanhirsh <itay@bazoo.org>
Co-authored-by: Zufar Dhiyaulhaq <zufardhiyaulhaq@gmail.com>
Co-authored-by: William Albertus Dembo <w.albertusd@gmail.com>
Co-authored-by: Alex Kulbii <jncneo@gmail.com>
Co-authored-by: Kateryna Nezdolii <nezdolik@spotify.com>
Co-authored-by: Christopher <paika.christopher@gmail.com>
zdmytriv referenced this pull request in verygoodsecurity/ratelimit Jul 23, 2021
* Add Docker Compose File (#27)

* docs: match envoy docs for remote_address ratelimiting (#29)

* docs: document dependency on gostats (#30)

* test and document whitelist behavior (#31)

Signed-off-by: Daniel Hochman <danielhochman@users.noreply.github.com>

* update dependencies (#35)

* update dependencies

* proto: use the proto defined in data-plane-api (#39)

* proto: check in protos to allow importing ratelimit as a library (#40)

* docs: update contact info (#42)

* redis: add the option to use a separate redis pool for per second limits (#41)

* fix duplicate mv (#43)

* docker: upgrade docker-compose setup (#46)

* Add gRPC health check (#47)

* logging: set log level (#50)

* go version: update to 1.11 (#53)

* docker compose: expose gRPC port on docker compose setup (#55)

* Configuration to ignore dotfiles. (#52)

This allows ratelimit to run on Kubernetes with configuration from a configmap.

* Add Dockerfile to enable builds (#58)

Signed-off-by: Steve Sloka <steves@heptio.com>

* ci: fix build (#73)

Fixes envoyproxy#71

Signed-off-by: Matt Klein <mklein@lyft.com>

* Run unit and integration tests with race detector enabled (#65)

* deps: update several of ratelimit's dependencies (#76)

* add stalebot (#78)

Signed-off-by: Matt Klein <mklein@lyft.com>

* docs: fix example 4 sample config (#79)

* fix redis-server binary name (#88)

* Fix build Dockerfile (envoyproxy#98)

Fix problem:
src/service_cmd/runner/runner.go:10:2: cannot find package "github.com/lyft/ratelimit/proto/ratelimit" in any of:
        /usr/local/go/src/github.com/lyft/ratelimit/vendor/github.com/lyft/ratelimit/proto/ratelimit (vendor tree)
        /usr/local/go/src/vendor/github.com/lyft/ratelimit/proto/ratelimit
        /usr/local/go/src/github.com/lyft/ratelimit/proto/ratelimit (from $GOROOT)
        /go/src/github.com/lyft/ratelimit/proto/ratelimit (from $GOPATH)
The command '/bin/sh -c go build -o /usr/local/bin/ratelimit src/service_cmd/main.go' returned a non-zero code: 1

* Redis TLS and Auth support (envoyproxy#96)

This adds support for TLS connections to redis as well as support for authentication.
Somewhat related to issue #61

* healthcheck: allow customizable healthcheck name (envoyproxy#102)

Description: this patch allows a consumer of the server package to customize the name of the healthchecker.

Signed-off-by: Jose Nino <jnino@lyft.com>

* health: make a few more types public (envoyproxy#104)

Description: envoyproxy#102 allowed for some customization. This PR makes the types public so that other servers can use this implementation.

Signed-off-by: Jose Nino <jnino@lyft.com>

* Add local cache to store whether it is over the limit (envoyproxy#111)

* Plugin statstore into runner (envoyproxy#115)

* fix: support auth without tls (envoyproxy#116)

Signed-off-by: tangxinfa <tangxinfa@gmail.com>

* add local cache stats (envoyproxy#114)

* Move license to templated Apache-2.0 (envoyproxy#123)

Signed-off-by: Derek Schaller <d_a_schaller@yahoo.com>

* Enable go modules (envoyproxy#124)

Signed-off-by: Steve Sloka <slokas@vmware.com>

* CI: Github Actions (envoyproxy#127)

Signed-off-by: Steve Sloka <slokas@vmware.com>

* community: update contributing guide (envoyproxy#139)

Fixes envoyproxy#138

Signed-off-by: Matt Klein <mklein@lyft.com>

* add http 1 `/json` endpoint (envoyproxy#136)

Signed-off-by: David Black <david.black@autodesk.com>

* Use mockgen version from go.mod instead of from "make bootstrap" (envoyproxy#143)

Even though the Makefile wants to encourage using mockgen@1.4.1, it
seems like the mocks have been generated using a pre-1.0 version of
mockgen. Using "go run github.com/golang/mock/mockgen" as a go:generate
command instead of just "mockgen" avoids the need to pre-install into
the developer's $PATH and uses the go.mod-specified version

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Upgrade gostats dependency from 0.2.6 to 0.4.0 (envoyproxy#141)

My interest is the UDP protocol support which appeared in gotstats 0.3.10

There's a breaking change as of https://github.com/lyft/gostats/releases/tag/v0.3.0
which is that gostats no longer publishes stats as expvars.

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Upgrade radix (envoyproxy#137)

Signed-off-by: Tong Cai <caitong93@gmail.com>

* cache_impl_test.go: fix failing test with ipv6 (envoyproxy#144)

A newly-added test in envoyproxy#137 checks the exact text of an error message which
seems to vary when the network is tcp4 vs tcp6. This change relaxes the
assertion to look for "connection refused" in a panic without making
assumptions about what an IP address looks like.

Example failure:

--- FAIL: TestNewClientImpl (0.00s)
    --- FAIL: TestNewClientImpl/connection_refused (0.00s)
        cache_impl_test.go:442:
                Error Trace:    cache_impl_test.go:442
                Error:          func (assert.PanicTestFunc)(0x1724110) should panic with error message: "dial tcp 127.0.0.1:12345: connect: connection refused"
                                        Panic value:    "dial tcp [::1]:12345: connect: connection refused"
                                        Panic stack:    goroutine 27 [running]:

The testify assert package doesn't seem to support inexact matching on error messages, so the code gets a bit uglier than before.

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Split redis-specific logic from generic key-value store logic (envoyproxy#142)

This is a pure refactoring with no behavior changes. It's a step toward being able
to add memcache as a backend (see envoyproxy#140).

This PR moves RateLimitCache from the redis package to a new "limiter" package, along with
code for time/jitter, local cache stats,  and constructing cache keys. All that can be reused
with memcache.

After this PR, the redis package is imported in exactly two places:
- in service_cmd/runner/runner.go to call redis.NewRateLimiterCacheImplFromSettings()
- in service/ratelimit.go in ShouldRateLimit to identify if a recovered panic is a redis.RedisError. If so, a stat is incremented and the panic() propagation is ended and in favor of returning the error as a the function result.

The PR also includes changes by goimports to test/service/ratelimit_test.go
so that the difference between package name vs file path name is explicit
instead of implicit.

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* json handler: return full ratelimit service response as json (envoyproxy#148)

Previously an HTTP POST to /json would only return an HTTP status code,
not all the other details supported by grpc ratelimit responses.

With this change an HTTP POST to /json receives the full proto3 response
encoded as json by jsonpb.

It seems unlikely that anyone would be parsing the text "over limit" from
the HTTP body instead of just reading the 429 response code, but for anyone
doing that this would be a breaking change.

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Update goruntime to latest, 0.2.5.  Add new config for watching changes in runtime config folder directly instead of the runtime root dir. (envoyproxy#151)

Signed-off-by: Yuki Sawa <yukisawa@gmail.com>

* Drop support for legacy ratelimit.proto and upgrade to v3 rls.proto (envoyproxy#153)

Signed-off-by: Petr Pchelko <ppchelko@wikimedia.org>

* Followups to v3 upgrade (envoyproxy#155)

- Regenerate mocks based on new default protocol
- Manually transform v2 messages to v3 messages - some of the fields
were renamed thus json Marshal/Unmarshal does not work anymore
- Added tests that verify conversion v2<->v3 works for headers fields
- Update tests to use proto.Equal - simple assert.Equals might not
work correctly for protobuf messages.

Signed-off-by: Petr Pchelko <ppchelko@wikimedia.org>

* Introduce a Dockerfile for running integration tests (envoyproxy#156)

This diff creates Dockerfile.integration for running integration tests with clearly-defined dependencies. Previously the dependencies of the integration tests were defined within the github actions config.

The new "make docker_tests" target should work for any developer with Docker installed.

Previously there was no single command that would run integration tests across platforms, which makes development and onboarding harder. Even copying the command from github actions wouldn't have worked before, since that command quietly assumed that redis was already running on port 6379.

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Add support for rate limit overrides. (envoyproxy#158)

Fixes envoyproxy#154

Signed-off-by: Petr Pchelko <ppchelko@wikimedia.org>

* redis client: default to use explicit pipelining (envoyproxy#163)

Signed-off-by: Tong Cai <caitong93@gmail.com>

* Clean go.mod file and update logrus to latest (envoyproxy#166)

Signed-off-by: Yuki Sawa <yukisawa@gmail.com>

* Add full test environment example. Fix bug in existing docker-compose. (envoyproxy#170)

Signed-off-by: Yuki Sawa <yukisawa@gmail.com>

* Implement LOG_FORMAT=json (envoyproxy#173)

Centralized log collection system works better with logs in json format.
E.g. DataDog strongly encourage setting up your logging library to produce your logs in JSON format to avoid the need for custom parsing rules.
So, the next small fix is all we need to get json logs.

Signed-off-by: Sergey Belyaev <sbelyaev@setronica.com>

* ci: Update github action to push docker image tagged with sha for each merge to master branch (envoyproxy#176)

Updates the github action to also push a tagged image based upon the git sha. The tag also
includes the current version of the release.

Example tag: envoyproxy/ratelimit:f1758150b6dfed3e5c0ae13fb7bb6b8f6ae00b0e

Fixes envoyproxy#174

Signed-off-by: Steve Sloka <slokas@vmware.com>

* Update README.md to refer to existing files (envoyproxy#178)

Signed-off-by: Margaret Gorguissian <margaret.gorguissian@tufts.edu>

* Add redis cluster and sentinel support (envoyproxy#179)

Signed-off-by: Diego Erdody <diego@medallia.com>

* Add support for x-ratelimit-reset header (envoyproxy#182)

Signed-off-by: Clara Andrew-Wani <candrewwani@gmail.com>

* Create repokitteh.star (envoyproxy#187)

Signed-off-by: Itay Donanhirsh <itay@bazoo.org>

* refactor NearLimitRatio to environment variable (envoyproxy#186)

Signed-off-by: zufardhiyaulhaq <zufardhiyaulhaq@gmail.com>

* Fix flakey tests with DurationUntilReset. Update docker example to V3 config. (envoyproxy#192)

Signed-off-by: Yuki Sawa <yukisawa@gmail.com>

* Separate Redis cache and driver implementation (envoyproxy#194)

Signed-off-by: William Albertus Dembo <w.albertusd@gmail.com>

* Set ratelimit filter to v3 api (envoyproxy#196)

Signed-off-by: Yuki Sawa <yukisawa@gmail.com>

* Add debug logging to indicate descriptor and limit (envoyproxy#197)

Signed-off-by: Sasha Kulbii <okulbii@wayfair.com>

* Implement BACKEND_TYPE=memcache as an alternative k/v store to redis (envoyproxy#172)

MEMCACHE_HOST_PORT=host:port must be set with BACKEND_TYPE=memcache

To minimize roundtrips when getting multiple keys, the memcache implementation
does a GetMulti to fetch the existing rate limit usage and does increments
asynchronously in background goroutines, since the memcache API doesn't offer
multi-increment.

Resolves envoyproxy#140

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Refactoring of duplicated code across backend types (envoyproxy#202)

Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>

* Small typo fix in README (envoyproxy#204)

Signed-off-by: cpaika <paika.christopher@gmail.com>

* Circle CI (#1)

* Add Circle Config

* Touch to build

* Move to expected path

* Use Docker Command

* Do it all here then

* Env

* Add Readme

* Actual README

* Add Docker Compose (#2)

* VAULT-893 Fix docker-compose.yml

* VAULT-893 Fix docker-compose.yml

Co-authored-by: Marshall Jones <marshall@offby3.com>
Co-authored-by: Jose Ulises Nino Rivera <junr03@users.noreply.github.com>
Co-authored-by: Daniel Hochman <danielhochman@users.noreply.github.com>
Co-authored-by: Martien Verbruggen <martien.verbruggen@gmail.com>
Co-authored-by: Ben Pope <BenPope@users.noreply.github.com>
Co-authored-by: Steve Sloka <steve@stevesloka.com>
Co-authored-by: Matt Klein <mattklein123@gmail.com>
Co-authored-by: Charlie Vieth <charlie.vieth@gmail.com>
Co-authored-by: Adil Hafeez <ahafeez@lyft.com>
Co-authored-by: Kartograf <kartogrof@gmail.com>
Co-authored-by: repl-david-winiarski <33431229+repl-david-winiarski@users.noreply.github.com>
Co-authored-by: Junchao Lyu <6963707+freedomljc@users.noreply.github.com>
Co-authored-by: tangxinfa <tangxinfa@gmail.com>
Co-authored-by: Steve Sloka <slokas@vmware.com>
Co-authored-by: Matt Klein <mklein@lyft.com>
Co-authored-by: dblackdblack <github@dhb.is>
Co-authored-by: David Weitzman <dweitzman@pinterest.com>
Co-authored-by: Tong Cai <caitong@caicloud.io>
Co-authored-by: Yuki Sawa <yukisawa@gmail.com>
Co-authored-by: Petr Pchelko <petrpchelko@gmail.com>
Co-authored-by: Petr Pchelko <ppchelko@wikimedia.org>
Co-authored-by: Sergey Belyaev <svdba@users.noreply.github.com>
Co-authored-by: Margaret G <Margaret.Gorguissian@tufts.edu>
Co-authored-by: Diego Erdody <erdody@gmail.com>
Co-authored-by: Clara <candrewwani@gmail.com>
Co-authored-by: Itay Donanhirsh <itay@bazoo.org>
Co-authored-by: Zufar Dhiyaulhaq <zufardhiyaulhaq@gmail.com>
Co-authored-by: William Albertus Dembo <w.albertusd@gmail.com>
Co-authored-by: Alex Kulbii <jncneo@gmail.com>
Co-authored-by: Kateryna Nezdolii <nezdolik@spotify.com>
Co-authored-by: Christopher <paika.christopher@gmail.com>
zdmytriv referenced this pull request in verygoodsecurity/ratelimit Aug 2, 2021
* Add Docker Compose File (#27)

* docs: match envoy docs for remote_address ratelimiting (#29)

* docs: document dependency on gostats (#30)

* test and document whitelist behavior (#31)

Signed-off-by: Daniel Hochman <danielhochman@users.noreply.github.com>

* update dependencies (#35)

* update dependencies

* proto: use the proto defined in data-plane-api (#39)

* proto: check in protos to allow importing ratelimit as a library (#40)

* docs: update contact info (#42)

* redis: add the option to use a separate redis pool for per second limits (#41)

* fix duplicate mv (#43)

* docker: upgrade docker-compose setup (#46)

* Add gRPC health check (#47)

* logging: set log level (#50)

* go version: update to 1.11 (#53)

* docker compose: expose gRPC port on docker compose setup (#55)

* Configuration to ignore dotfiles. (#52)

This allows ratelimit to run on Kubernetes with configuration from a configmap.

* Add Dockerfile to enable builds (#58)

Signed-off-by: Steve Sloka <steves@heptio.com>

* ci: fix build (#73)

Fixes envoyproxy#71

Signed-off-by: Matt Klein <mklein@lyft.com>

* Run unit and integration tests with race detector enabled (#65)

* deps: update several of ratelimit's dependencies (#76)

* add stalebot (#78)

Signed-off-by: Matt Klein <mklein@lyft.com>

* docs: fix example 4 sample config (#79)

* fix redis-server binary name (#88)

* Fix build Dockerfile (envoyproxy#98)

Fix problem:
src/service_cmd/runner/runner.go:10:2: cannot find package "github.com/lyft/ratelimit/proto/ratelimit" in any of:
        /usr/local/go/src/github.com/lyft/ratelimit/vendor/github.com/lyft/ratelimit/proto/ratelimit (vendor tree)
        /usr/local/go/src/vendor/github.com/lyft/ratelimit/proto/ratelimit
        /usr/local/go/src/github.com/lyft/ratelimit/proto/ratelimit (from $GOROOT)
        /go/src/github.com/lyft/ratelimit/proto/ratelimit (from $GOPATH)
The command '/bin/sh -c go build -o /usr/local/bin/ratelimit src/service_cmd/main.go' returned a non-zero code: 1

* Redis TLS and Auth support (envoyproxy#96)

This adds support for TLS connections to redis as well as support for authentication.
Somewhat related to issue #61

* healthcheck: allow customizable healthcheck name (envoyproxy#102)

Description: this patch allows a consumer of the server package to customize the name of the healthchecker.

Signed-off-by: Jose Nino <jnino@lyft.com>

* health: make a few more types public (envoyproxy#104)

Description: envoyproxy#102 allowed for some customization. This PR makes the types public so that other servers can use this implementation.

Signed-off-by: Jose Nino <jnino@lyft.com>

* Add local cache to store whether it is over the limit (envoyproxy#111)

* Plugin statstore into runner (envoyproxy#115)

* fix: support auth without tls (envoyproxy#116)

Signed-off-by: tangxinfa <tangxinfa@gmail.com>

* add local cache stats (envoyproxy#114)

* Move license to templated Apache-2.0 (envoyproxy#123)

Signed-off-by: Derek Schaller <d_a_schaller@yahoo.com>

* Enable go modules (envoyproxy#124)

Signed-off-by: Steve Sloka <slokas@vmware.com>

* CI: Github Actions (envoyproxy#127)

Signed-off-by: Steve Sloka <slokas@vmware.com>

* community: update contributing guide (envoyproxy#139)

Fixes envoyproxy#138

Signed-off-by: Matt Klein <mklein@lyft.com>

* add http 1 `/json` endpoint (envoyproxy#136)

Signed-off-by: David Black <david.black@autodesk.com>

* Use mockgen version from go.mod instead of from "make bootstrap" (envoyproxy#143)

Even though the Makefile wants to encourage using mockgen@1.4.1, it
seems like the mocks have been generated using a pre-1.0 version of
mockgen. Using "go run github.com/golang/mock/mockgen" as a go:generate
command instead of just "mockgen" avoids the need to pre-install into
the developer's $PATH and uses the go.mod-specified version

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Upgrade gostats dependency from 0.2.6 to 0.4.0 (envoyproxy#141)

My interest is the UDP protocol support which appeared in gotstats 0.3.10

There's a breaking change as of https://github.com/lyft/gostats/releases/tag/v0.3.0
which is that gostats no longer publishes stats as expvars.

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Upgrade radix (envoyproxy#137)

Signed-off-by: Tong Cai <caitong93@gmail.com>

* cache_impl_test.go: fix failing test with ipv6 (envoyproxy#144)

A newly-added test in envoyproxy#137 checks the exact text of an error message which
seems to vary when the network is tcp4 vs tcp6. This change relaxes the
assertion to look for "connection refused" in a panic without making
assumptions about what an IP address looks like.

Example failure:

--- FAIL: TestNewClientImpl (0.00s)
    --- FAIL: TestNewClientImpl/connection_refused (0.00s)
        cache_impl_test.go:442:
                Error Trace:    cache_impl_test.go:442
                Error:          func (assert.PanicTestFunc)(0x1724110) should panic with error message: "dial tcp 127.0.0.1:12345: connect: connection refused"
                                        Panic value:    "dial tcp [::1]:12345: connect: connection refused"
                                        Panic stack:    goroutine 27 [running]:

The testify assert package doesn't seem to support inexact matching on error messages, so the code gets a bit uglier than before.

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Split redis-specific logic from generic key-value store logic (envoyproxy#142)

This is a pure refactoring with no behavior changes. It's a step toward being able
to add memcache as a backend (see envoyproxy#140).

This PR moves RateLimitCache from the redis package to a new "limiter" package, along with
code for time/jitter, local cache stats,  and constructing cache keys. All that can be reused
with memcache.

After this PR, the redis package is imported in exactly two places:
- in service_cmd/runner/runner.go to call redis.NewRateLimiterCacheImplFromSettings()
- in service/ratelimit.go in ShouldRateLimit to identify if a recovered panic is a redis.RedisError. If so, a stat is incremented and the panic() propagation is ended and in favor of returning the error as a the function result.

The PR also includes changes by goimports to test/service/ratelimit_test.go
so that the difference between package name vs file path name is explicit
instead of implicit.

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* json handler: return full ratelimit service response as json (envoyproxy#148)

Previously an HTTP POST to /json would only return an HTTP status code,
not all the other details supported by grpc ratelimit responses.

With this change an HTTP POST to /json receives the full proto3 response
encoded as json by jsonpb.

It seems unlikely that anyone would be parsing the text "over limit" from
the HTTP body instead of just reading the 429 response code, but for anyone
doing that this would be a breaking change.

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Update goruntime to latest, 0.2.5.  Add new config for watching changes in runtime config folder directly instead of the runtime root dir. (envoyproxy#151)

Signed-off-by: Yuki Sawa <yukisawa@gmail.com>

* Drop support for legacy ratelimit.proto and upgrade to v3 rls.proto (envoyproxy#153)

Signed-off-by: Petr Pchelko <ppchelko@wikimedia.org>

* Followups to v3 upgrade (envoyproxy#155)

- Regenerate mocks based on new default protocol
- Manually transform v2 messages to v3 messages - some of the fields
were renamed thus json Marshal/Unmarshal does not work anymore
- Added tests that verify conversion v2<->v3 works for headers fields
- Update tests to use proto.Equal - simple assert.Equals might not
work correctly for protobuf messages.

Signed-off-by: Petr Pchelko <ppchelko@wikimedia.org>

* Introduce a Dockerfile for running integration tests (envoyproxy#156)

This diff creates Dockerfile.integration for running integration tests with clearly-defined dependencies. Previously the dependencies of the integration tests were defined within the github actions config.

The new "make docker_tests" target should work for any developer with Docker installed.

Previously there was no single command that would run integration tests across platforms, which makes development and onboarding harder. Even copying the command from github actions wouldn't have worked before, since that command quietly assumed that redis was already running on port 6379.

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Add support for rate limit overrides. (envoyproxy#158)

Fixes envoyproxy#154

Signed-off-by: Petr Pchelko <ppchelko@wikimedia.org>

* redis client: default to use explicit pipelining (envoyproxy#163)

Signed-off-by: Tong Cai <caitong93@gmail.com>

* Clean go.mod file and update logrus to latest (envoyproxy#166)

Signed-off-by: Yuki Sawa <yukisawa@gmail.com>

* Add full test environment example. Fix bug in existing docker-compose. (envoyproxy#170)

Signed-off-by: Yuki Sawa <yukisawa@gmail.com>

* Implement LOG_FORMAT=json (envoyproxy#173)

Centralized log collection system works better with logs in json format.
E.g. DataDog strongly encourage setting up your logging library to produce your logs in JSON format to avoid the need for custom parsing rules.
So, the next small fix is all we need to get json logs.

Signed-off-by: Sergey Belyaev <sbelyaev@setronica.com>

* ci: Update github action to push docker image tagged with sha for each merge to master branch (envoyproxy#176)

Updates the github action to also push a tagged image based upon the git sha. The tag also
includes the current version of the release.

Example tag: envoyproxy/ratelimit:f1758150b6dfed3e5c0ae13fb7bb6b8f6ae00b0e

Fixes envoyproxy#174

Signed-off-by: Steve Sloka <slokas@vmware.com>

* Update README.md to refer to existing files (envoyproxy#178)

Signed-off-by: Margaret Gorguissian <margaret.gorguissian@tufts.edu>

* Add redis cluster and sentinel support (envoyproxy#179)

Signed-off-by: Diego Erdody <diego@medallia.com>

* Add support for x-ratelimit-reset header (envoyproxy#182)

Signed-off-by: Clara Andrew-Wani <candrewwani@gmail.com>

* Create repokitteh.star (envoyproxy#187)

Signed-off-by: Itay Donanhirsh <itay@bazoo.org>

* refactor NearLimitRatio to environment variable (envoyproxy#186)

Signed-off-by: zufardhiyaulhaq <zufardhiyaulhaq@gmail.com>

* Fix flakey tests with DurationUntilReset. Update docker example to V3 config. (envoyproxy#192)

Signed-off-by: Yuki Sawa <yukisawa@gmail.com>

* Separate Redis cache and driver implementation (envoyproxy#194)

Signed-off-by: William Albertus Dembo <w.albertusd@gmail.com>

* Set ratelimit filter to v3 api (envoyproxy#196)

Signed-off-by: Yuki Sawa <yukisawa@gmail.com>

* Add debug logging to indicate descriptor and limit (envoyproxy#197)

Signed-off-by: Sasha Kulbii <okulbii@wayfair.com>

* Implement BACKEND_TYPE=memcache as an alternative k/v store to redis (envoyproxy#172)

MEMCACHE_HOST_PORT=host:port must be set with BACKEND_TYPE=memcache

To minimize roundtrips when getting multiple keys, the memcache implementation
does a GetMulti to fetch the existing rate limit usage and does increments
asynchronously in background goroutines, since the memcache API doesn't offer
multi-increment.

Resolves envoyproxy#140

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

* Refactoring of duplicated code across backend types (envoyproxy#202)

Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>

* Small typo fix in README (envoyproxy#204)

Signed-off-by: cpaika <paika.christopher@gmail.com>

* Circle CI (#1)

* Add Circle Config

* Touch to build

* Move to expected path

* Use Docker Command

* Do it all here then

* Env

* Add Readme

* Actual README

* Add Docker Compose (#2)

* VAULT-893 Fix docker-compose.yml

* VAULT-893 Fix docker-compose.yml

Co-authored-by: Marshall Jones <marshall@offby3.com>
Co-authored-by: Jose Ulises Nino Rivera <junr03@users.noreply.github.com>
Co-authored-by: Daniel Hochman <danielhochman@users.noreply.github.com>
Co-authored-by: Martien Verbruggen <martien.verbruggen@gmail.com>
Co-authored-by: Ben Pope <BenPope@users.noreply.github.com>
Co-authored-by: Steve Sloka <steve@stevesloka.com>
Co-authored-by: Matt Klein <mattklein123@gmail.com>
Co-authored-by: Charlie Vieth <charlie.vieth@gmail.com>
Co-authored-by: Adil Hafeez <ahafeez@lyft.com>
Co-authored-by: Kartograf <kartogrof@gmail.com>
Co-authored-by: repl-david-winiarski <33431229+repl-david-winiarski@users.noreply.github.com>
Co-authored-by: Junchao Lyu <6963707+freedomljc@users.noreply.github.com>
Co-authored-by: tangxinfa <tangxinfa@gmail.com>
Co-authored-by: Steve Sloka <slokas@vmware.com>
Co-authored-by: Matt Klein <mklein@lyft.com>
Co-authored-by: dblackdblack <github@dhb.is>
Co-authored-by: David Weitzman <dweitzman@pinterest.com>
Co-authored-by: Tong Cai <caitong@caicloud.io>
Co-authored-by: Yuki Sawa <yukisawa@gmail.com>
Co-authored-by: Petr Pchelko <petrpchelko@gmail.com>
Co-authored-by: Petr Pchelko <ppchelko@wikimedia.org>
Co-authored-by: Sergey Belyaev <svdba@users.noreply.github.com>
Co-authored-by: Margaret G <Margaret.Gorguissian@tufts.edu>
Co-authored-by: Diego Erdody <erdody@gmail.com>
Co-authored-by: Clara <candrewwani@gmail.com>
Co-authored-by: Itay Donanhirsh <itay@bazoo.org>
Co-authored-by: Zufar Dhiyaulhaq <zufardhiyaulhaq@gmail.com>
Co-authored-by: William Albertus Dembo <w.albertusd@gmail.com>
Co-authored-by: Alex Kulbii <jncneo@gmail.com>
Co-authored-by: Kateryna Nezdolii <nezdolik@spotify.com>
Co-authored-by: Christopher <paika.christopher@gmail.com>
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

Successfully merging this pull request may close these issues.

3 participants