json handler: return full ratelimit service response as json (#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 <[email protected]>

README.md

@@ -383,7 +383,32 @@ Takes an HTTP POST with a JSON body of the form e.g.
 }
 ```
 The service will return an http 200 if this request is allowed (if no ratelimits exceeded) or 429 if one or more 
-ratelimits were exceeded. Endpoint does not currently return detailed information on which limits were exceeded.
+ratelimits were exceeded.
+
+The response is a RateLimitResponse encoded with
+[proto3-to-json mapping](https://developers.google.com/protocol-buffers/docs/proto3#json):
+```json
+{
+  "overallCode": "OVER_LIMIT",
+  "statuses": [
+    {
+      "code": "OVER_LIMIT",
+      "currentLimit": {
+        "requestsPerUnit": 1,
+        "unit": "MINUTE"
+      }
+    },
+    {
+      "code": "OK",
+      "currentLimit": {
+        "requestsPerUnit": 2,
+        "unit": "MINUTE"
+      },
+      "limitRemaining": 1
+    }
+  ]
+}
+```
 
 # Debug Port
 

src/server/server_impl.go

@@ -1,7 +1,7 @@
 package server
 
 import (
-	"encoding/json"
+	"bytes"
 	"expvar"
 	"fmt"
 	"io"
@@ -19,6 +19,7 @@ import (
 	pb "github.com/envoyproxy/go-control-plane/envoy/service/ratelimit/v2"
 	"github.com/envoyproxy/ratelimit/src/limiter"
 	"github.com/envoyproxy/ratelimit/src/settings"
+	"github.com/golang/protobuf/jsonpb"
 	"github.com/gorilla/mux"
 	reuseport "github.com/kavu/go_reuseport"
 	"github.com/lyft/goruntime/loader"
@@ -53,15 +54,18 @@ func (server *server) AddDebugHttpEndpoint(path string, help string, handler htt
 	server.debugListener.endpoints[path] = help
 }
 
-// add an http/1 handler at the /json endpoint which allows this ratelimit service to work with
+// create an http/1 handler at the /json endpoint which allows this ratelimit service to work with
 // clients that cannot use the gRPC interface (e.g. lua)
 // example usage from cURL with domain "dummy" and descriptor "perday":
 // echo '{"domain": "dummy", "descriptors": [{"entries": [{"key": "perday"}]}]}' | curl -vvvXPOST --data @/dev/stdin localhost:8080/json
-func (server *server) AddJsonHandler(svc pb.RateLimitServiceServer) {
-	handler := func(writer http.ResponseWriter, request *http.Request) {
+func NewJsonHandler(svc pb.RateLimitServiceServer) func(http.ResponseWriter, *http.Request) {
+	// Default options include enums as strings and no identation.
+	m := &jsonpb.Marshaler{}
+
+	return func(writer http.ResponseWriter, request *http.Request) {
 		var req pb.RateLimitRequest
 
-		if err := json.NewDecoder(request.Body).Decode(&req); err != nil {
+		if err := jsonpb.Unmarshal(request.Body, &req); err != nil {
 			logger.Warnf("error: %s", err.Error())
 			http.Error(writer, err.Error(), http.StatusBadRequest)
 			return
@@ -73,15 +77,29 @@ func (server *server) AddJsonHandler(svc pb.RateLimitServiceServer) {
 			http.Error(writer, err.Error(), http.StatusBadRequest)
 			return
 		}
+
 		logger.Debugf("resp:%s", resp)
-		if resp.OverallCode == pb.RateLimitResponse_OVER_LIMIT {
-			http.Error(writer, "over limit", http.StatusTooManyRequests)
-		} else if resp.OverallCode == pb.RateLimitResponse_UNKNOWN {
-			http.Error(writer, "unknown", http.StatusInternalServerError)
+
+		buf := bytes.NewBuffer(nil)
+		err = m.Marshal(buf, resp)
+		if err != nil {
+			logger.Errorf("error marshaling proto3 to json: %s", err.Error())
+			http.Error(writer, "error marshaling proto3 to json: "+err.Error(), http.StatusInternalServerError)
+			return
 		}
 
+		writer.Header().Set("Content-Type", "application/json")
+		if resp == nil || resp.OverallCode == pb.RateLimitResponse_UNKNOWN {
+			writer.WriteHeader(http.StatusInternalServerError)
+		} else if resp.OverallCode == pb.RateLimitResponse_OVER_LIMIT {
+			writer.WriteHeader(http.StatusTooManyRequests)
+		}
+		writer.Write(buf.Bytes())
 	}
-	server.router.HandleFunc("/json", handler)
+}
+
+func (server *server) AddJsonHandler(svc pb.RateLimitServiceServer) {
+	server.router.HandleFunc("/json", NewJsonHandler(svc))
 }
 
 func (server *server) GrpcServer() *grpc.Server {

test/integration/integration_test.go

@@ -5,6 +5,7 @@ package integration_test
 import (
 	"bytes"
 	"fmt"
+	"io/ioutil"
 	"math/rand"
 	"net/http"
 	"os"
@@ -376,11 +377,15 @@ func TestBasicConfigLegacy(t *testing.T) {
 	}`)
 	http_resp, _ := http.Post("http://localhost:8082/json", "application/json", bytes.NewBuffer(json_body))
 	assert.Equal(http_resp.StatusCode, 200)
+	body, _ := ioutil.ReadAll(http_resp.Body)
 	http_resp.Body.Close()
+	assert.Equal(`{"overallCode":"OK","statuses":[{"code":"OK","currentLimit":{"requestsPerUnit":1,"unit":"MINUTE"}}]}`, string(body))
 
 	http_resp, _ = http.Post("http://localhost:8082/json", "application/json", bytes.NewBuffer(json_body))
 	assert.Equal(http_resp.StatusCode, 429)
+	body, _ = ioutil.ReadAll(http_resp.Body)
 	http_resp.Body.Close()
+	assert.Equal(`{"overallCode":"OVER_LIMIT","statuses":[{"code":"OVER_LIMIT","currentLimit":{"requestsPerUnit":1,"unit":"MINUTE"}}]}`, string(body))
 
 	invalid_json := []byte(`{"unclosed quote: []}`)
 	http_resp, _ = http.Post("http://localhost:8082/json", "application/json", bytes.NewBuffer(invalid_json))

test/mocks/mocks.go

@@ -5,3 +5,4 @@ package mocks
 //go:generate go run github.com/golang/mock/mockgen -destination ./config/config.go github.com/envoyproxy/ratelimit/src/config RateLimitConfig,RateLimitConfigLoader
 //go:generate go run github.com/golang/mock/mockgen -destination ./redis/redis.go github.com/envoyproxy/ratelimit/src/redis Client
 //go:generate go run github.com/golang/mock/mockgen -destination ./limiter/limiter.go github.com/envoyproxy/ratelimit/src/limiter RateLimitCache,TimeSource,JitterRandSource
+//go:generate go run github.com/golang/mock/mockgen -destination ./rls/rls.go github.com/envoyproxy/go-control-plane/envoy/service/ratelimit/v2 RateLimitServiceServer

test/server/server_impl_test.go

@@ -0,0 +1,86 @@
+package server_test
+
+import (
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	pb "github.com/envoyproxy/go-control-plane/envoy/service/ratelimit/v2"
+
+	"github.com/envoyproxy/ratelimit/src/server"
+	mock_v2 "github.com/envoyproxy/ratelimit/test/mocks/rls"
+	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/assert"
+)
+
+func assertHttpResponse(t *testing.T,
+	handler http.HandlerFunc,
+	requestBody string,
+	expectedStatusCode int,
+	expectedContentType string,
+	expectedResponseBody string) {
+
+	t.Helper()
+	assert := assert.New(t)
+
+	req := httptest.NewRequest("METHOD_NOT_CHECKED", "/path_not_checked", strings.NewReader(requestBody))
+	w := httptest.NewRecorder()
+	handler(w, req)
+
+	resp := w.Result()
+	actualBody, _ := ioutil.ReadAll(resp.Body)
+	assert.Equal(expectedContentType, resp.Header.Get("Content-Type"))
+	assert.Equal(expectedStatusCode, resp.StatusCode)
+	assert.Equal(expectedResponseBody, string(actualBody))
+}
+
+func TestJsonHandler(t *testing.T) {
+	controller := gomock.NewController(t)
+	defer controller.Finish()
+
+	rls := mock_v2.NewMockRateLimitServiceServer(controller)
+	handler := server.NewJsonHandler(rls)
+
+	// Missing request body
+	assertHttpResponse(t, handler, "", 400, "text/plain; charset=utf-8", "EOF\n")
+
+	// Request body is not valid json
+	assertHttpResponse(t, handler, "}", 400, "text/plain; charset=utf-8", "invalid character '}' looking for beginning of value\n")
+
+	// Unknown response code
+	rls.EXPECT().ShouldRateLimit(nil, &pb.RateLimitRequest{
+		Domain: "foo",
+	}).Return(&pb.RateLimitResponse{}, nil)
+	assertHttpResponse(t, handler, `{"domain": "foo"}`, 500, "application/json", "{}")
+
+	// ratelimit service error
+	rls.EXPECT().ShouldRateLimit(nil, &pb.RateLimitRequest{
+		Domain: "foo",
+	}).Return(nil, fmt.Errorf("some error"))
+	assertHttpResponse(t, handler, `{"domain": "foo"}`, 400, "text/plain; charset=utf-8", "some error\n")
+
+	// json unmarshaling error
+	rls.EXPECT().ShouldRateLimit(nil, &pb.RateLimitRequest{
+		Domain: "foo",
+	}).Return(nil, nil)
+	assertHttpResponse(t, handler, `{"domain": "foo"}`, 500, "text/plain; charset=utf-8", "error marshaling proto3 to json: Marshal called with nil\n")
+
+	// successful request, not rate limited
+	rls.EXPECT().ShouldRateLimit(nil, &pb.RateLimitRequest{
+		Domain: "foo",
+	}).Return(&pb.RateLimitResponse{
+		OverallCode: pb.RateLimitResponse_OK,
+	}, nil)
+	assertHttpResponse(t, handler, `{"domain": "foo"}`, 200, "application/json", `{"overallCode":"OK"}`)
+
+	// successful request, rate limited
+	rls.EXPECT().ShouldRateLimit(nil, &pb.RateLimitRequest{
+		Domain: "foo",
+	}).Return(&pb.RateLimitResponse{
+		OverallCode: pb.RateLimitResponse_OVER_LIMIT,
+	}, nil)
+	assertHttpResponse(t, handler, `{"domain": "foo"}`, 429, "application/json", `{"overallCode":"OVER_LIMIT"}`)
+}