From 7c8f2383d6303a97257ad5034f755b7ba5197526 Mon Sep 17 00:00:00 2001 From: Tianchu Zhao Date: Fri, 22 Jul 2022 03:52:26 +0000 Subject: [PATCH 1/3] feat: apiratelimit headers Signed-off-by: Tianchu Zhao --- server/apiserver/argoserver.go | 31 +++++++------------------------ test/e2e/argo_server_test.go | 13 +++++++++++++ 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/server/apiserver/argoserver.go b/server/apiserver/argoserver.go index 1d8b9ff446f6..643e81bdd00f 100644 --- a/server/apiserver/argoserver.go +++ b/server/apiserver/argoserver.go @@ -64,6 +64,7 @@ import ( "github.com/argoproj/argo-workflows/v3/workflow/hydrator" limiter "github.com/sethvargo/go-limiter" + "github.com/sethvargo/go-limiter/httplimit" "github.com/sethvargo/go-limiter/memorystore" ) @@ -318,10 +319,15 @@ func (as *argoServer) newGRPCServer(instanceIDService instanceid.Service, offloa func (as *argoServer) newHTTPServer(ctx context.Context, port int, artifactServer *artifacts.ArtifactServer) *http.Server { endpoint := fmt.Sprintf("localhost:%d", port) + ratelimit_middleware, err := httplimit.NewMiddleware(as.apiRateLimiter, httplimit.IPKeyFunc()) + if err != nil { + log.Fatal(err) + } + mux := http.NewServeMux() httpServer := http.Server{ Addr: endpoint, - Handler: as.httpLimit(accesslog.Interceptor(mux)), + Handler: ratelimit_middleware.Handle(accesslog.Interceptor(mux)), TLSConfig: as.tlsConfig, } dialOpts := []grpc.DialOption{ @@ -413,26 +419,3 @@ func (as *argoServer) checkServeErr(name string, err error) { log.Infof("graceful shutdown %s", name) } } - -func (as *argoServer) httpLimit(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Get the IP address for the current user. - ip, _, err := net.SplitHostPort(r.RemoteAddr) - if err != nil { - http.Error(w, "Internal Server Error", http.StatusInternalServerError) - return - } - ctx := r.Context() - _, _, _, ok, err := as.apiRateLimiter.Take(ctx, ip) - if err != nil { - http.Error(w, "Internal Server Error", http.StatusInternalServerError) - return - } - if !ok { - http.Error(w, http.StatusText(429), http.StatusTooManyRequests) - return - } - - next.ServeHTTP(w, r) - }) -} diff --git a/test/e2e/argo_server_test.go b/test/e2e/argo_server_test.go index 4b63f3a49a2c..93883de20ca9 100644 --- a/test/e2e/argo_server_test.go +++ b/test/e2e/argo_server_test.go @@ -1979,6 +1979,19 @@ func (s *ArgoServerSuite) TestSensorService() { }) } +func (s *ArgoServerSuite) TestRateLimitHeader() { + s.Run("GetRateLimit", func() { + resp := s.e().GET("/api/v1/version"). + Expect(). + Status(200) + + resp.Header("X-RateLimit-Limit").NotEmpty() + resp.Header("X-RateLimit-Remaining").NotEmpty() + resp.Header("X-RateLimit-Reset").NotEmpty() + resp.Header("Retry-After").Empty() + }) +} + func TestArgoServerSuite(t *testing.T) { suite.Run(t, new(ArgoServerSuite)) } From 4be645229591c5e1d8251dc4c9ad4dd547e45937 Mon Sep 17 00:00:00 2001 From: Tianchu Zhao Date: Fri, 22 Jul 2022 03:52:49 +0000 Subject: [PATCH 2/3] docs: argoserver apiratelimit Signed-off-by: Tianchu Zhao --- docs/argo-server.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/argo-server.md b/docs/argo-server.md index d12ab4c8cc74..82f515341edd 100644 --- a/docs/argo-server.md +++ b/docs/argo-server.md @@ -164,3 +164,13 @@ Argo Server does not perform authentication directly. It delegates this to eithe ### IP Address Logging Argo Server does not log the IP addresses of API requests. We recommend you put the Argo Server behind a load balancer, and that load balancer is configured to log the IP addresses of requests that return authentication or authorization errors. + +### Rate Limiting + +> v3.4 and after + +Argo Server by default rate limits to 1000 per IP per minute, you can configure it through `--api-rate-limit`. You can access additional information through the following headers. +* `X-Rate-Limit-Limit` - the rate limit ceiling that is applicable for the current request. +* `X-Rate-Limit-Remaining` - the number of requests left for the current rate-limit window. +* `X-Rate-Limit-Reset` - the time at which the rate limit resets, specified in UTC time. +* `Retry-After` - indicate when a client should retry requests (when the rate limit expires), in UTC time. From b73a4b2ef1cc3db462566ef80ce80451bb5b443f Mon Sep 17 00:00:00 2001 From: Tianchu Zhao Date: Fri, 22 Jul 2022 04:23:41 +0000 Subject: [PATCH 3/3] fix: lint Signed-off-by: Tianchu Zhao --- docs/argo-server.md | 1 + server/apiserver/argoserver.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/argo-server.md b/docs/argo-server.md index 82f515341edd..eb53d1b294e0 100644 --- a/docs/argo-server.md +++ b/docs/argo-server.md @@ -170,6 +170,7 @@ Argo Server does not log the IP addresses of API requests. We recommend you put > v3.4 and after Argo Server by default rate limits to 1000 per IP per minute, you can configure it through `--api-rate-limit`. You can access additional information through the following headers. + * `X-Rate-Limit-Limit` - the rate limit ceiling that is applicable for the current request. * `X-Rate-Limit-Remaining` - the number of requests left for the current rate-limit window. * `X-Rate-Limit-Reset` - the time at which the rate limit resets, specified in UTC time. diff --git a/server/apiserver/argoserver.go b/server/apiserver/argoserver.go index 643e81bdd00f..2abeaed13f0c 100644 --- a/server/apiserver/argoserver.go +++ b/server/apiserver/argoserver.go @@ -321,7 +321,7 @@ func (as *argoServer) newHTTPServer(ctx context.Context, port int, artifactServe ratelimit_middleware, err := httplimit.NewMiddleware(as.apiRateLimiter, httplimit.IPKeyFunc()) if err != nil { - log.Fatal(err) + log.Fatal(err) } mux := http.NewServeMux()