From cda884ac819783b9c148de019444e45a5bab6669 Mon Sep 17 00:00:00 2001 From: wangxinyi7 <121973291+wangxinyi7@users.noreply.github.com> Date: Fri, 11 Aug 2023 14:11:11 -0700 Subject: [PATCH] read endpoint (#18268) implement http read endpoint to expose resource grpc service read method --- internal/resource/http/http.go | 96 +++++++++++++++------- internal/resource/http/http_test.go | 119 ++++++++++++++++++---------- 2 files changed, 145 insertions(+), 70 deletions(-) diff --git a/internal/resource/http/http.go b/internal/resource/http/http.go index b6a018c405..569f143e2f 100644 --- a/internal/resource/http/http.go +++ b/internal/resource/http/http.go @@ -59,6 +59,8 @@ func (h *resourceHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { switch r.Method { case http.MethodPut: h.handleWrite(w, r, ctx) + case http.MethodGet: + h.handleRead(w, r, ctx) case http.MethodDelete: h.handleDelete(w, r, ctx) default: @@ -88,17 +90,17 @@ func (h *resourceHandler) handleWrite(w http.ResponseWriter, r *http.Request, ct return } - tenancyInfo, resourceName, version := parseParams(r) + tenancyInfo, params := parseParams(r) rsp, err := h.client.Write(ctx, &pbresource.WriteRequest{ Resource: &pbresource.Resource{ Id: &pbresource.ID{ Type: h.reg.Type, Tenancy: tenancyInfo, - Name: resourceName, + Name: params["resourceName"], }, Owner: req.Owner, - Version: version, + Version: params["version"], Metadata: req.Metadata, Data: anyProtoMsg, }, @@ -117,18 +119,71 @@ func (h *resourceHandler) handleWrite(w http.ResponseWriter, r *http.Request, ct w.Write(output) } -func parseParams(r *http.Request) (tenancy *pbresource.Tenancy, resourceName string, version string) { - params := r.URL.Query() - tenancy = &pbresource.Tenancy{ - Partition: params.Get("partition"), - PeerName: params.Get("peer_name"), - Namespace: params.Get("namespace"), +func (h *resourceHandler) handleRead(w http.ResponseWriter, r *http.Request, ctx context.Context) { + tenancyInfo, params := parseParams(r) + if params["consistent"] != "" { + ctx = metadata.AppendToOutgoingContext(ctx, "x-consul-consistency-mode", "consistent") } - resourceName = path.Base(r.URL.Path) + + rsp, err := h.client.Read(ctx, &pbresource.ReadRequest{ + Id: &pbresource.ID{ + Type: h.reg.Type, + Tenancy: tenancyInfo, + Name: params["resourceName"], + }, + }) + if err != nil { + handleResponseError(err, w, h) + return + } + + output, err := jsonMarshal(rsp.Resource) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + h.logger.Error("Failed to unmarshal GRPC resource response", "error", err) + return + } + w.Write(output) +} + +// Note: The HTTP endpoints do not accept UID since it is quite unlikely that the user will have access to it +func (h *resourceHandler) handleDelete(w http.ResponseWriter, r *http.Request, ctx context.Context) { + tenancyInfo, params := parseParams(r) + _, err := h.client.Delete(ctx, &pbresource.DeleteRequest{ + Id: &pbresource.ID{ + Type: h.reg.Type, + Tenancy: tenancyInfo, + Name: params["resourceName"], + }, + Version: params["version"], + }) + if err != nil { + handleResponseError(err, w, h) + return + } + w.WriteHeader(http.StatusNoContent) + w.Write([]byte("{}")) +} + +func parseParams(r *http.Request) (tenancy *pbresource.Tenancy, params map[string]string) { + query := r.URL.Query() + tenancy = &pbresource.Tenancy{ + Partition: query.Get("partition"), + PeerName: query.Get("peer_name"), + Namespace: query.Get("namespace"), + } + + resourceName := path.Base(r.URL.Path) if resourceName == "." || resourceName == "/" { resourceName = "" } - version = params.Get("version") + + params = make(map[string]string) + params["resourceName"] = resourceName + params["version"] = query.Get("version") + if _, ok := query["consistent"]; ok { + params["consistent"] = "true" + } return } @@ -173,22 +228,3 @@ func handleResponseError(err error, w http.ResponseWriter, h *resourceHandler) { } w.Write([]byte(err.Error())) } - -// Note: The HTTP endpoints do not accept UID since it is quite unlikely that the user will have access to it -func (h *resourceHandler) handleDelete(w http.ResponseWriter, r *http.Request, ctx context.Context) { - tenancyInfo, resourceName, version := parseParams(r) - _, err := h.client.Delete(ctx, &pbresource.DeleteRequest{ - Id: &pbresource.ID{ - Type: h.reg.Type, - Tenancy: tenancyInfo, - Name: resourceName, - }, - Version: version, - }) - if err != nil { - handleResponseError(err, w, h) - return - } - w.WriteHeader(http.StatusNoContent) - w.Write([]byte("{}")) -} diff --git a/internal/resource/http/http_test.go b/internal/resource/http/http_test.go index 6747fb251a..07c2b9dbc8 100644 --- a/internal/resource/http/http_test.go +++ b/internal/resource/http/http_test.go @@ -27,6 +27,7 @@ import ( const testACLTokenArtistReadPolicy = "00000000-0000-0000-0000-000000000001" const testACLTokenArtistWritePolicy = "00000000-0000-0000-0000-000000000002" +const fakeToken = "fake-token" func parseToken(req *http.Request, token *string) { *token = req.Header.Get("x-consul-token") @@ -109,25 +110,9 @@ func TestResourceWriteHandler(t *testing.T) { client := svctest.RunResourceServiceWithACL(t, aclResolver, demo.RegisterTypes) - v1ArtistHandler := resourceHandler{ - resource.Registration{ - Type: demo.TypeV1Artist, - Proto: &pbdemov1.Artist{}, - }, - client, - parseToken, - hclog.NewNullLogger(), - } - - v2ArtistHandler := resourceHandler{ - resource.Registration{ - Type: demo.TypeV2Artist, - Proto: &pbdemov2.Artist{}, - }, - client, - parseToken, - hclog.NewNullLogger(), - } + r := resource.NewRegistry() + demo.RegisterTypes(r) + handler := NewHandler(client, r, parseToken, hclog.NewNullLogger()) t.Run("should be blocked if the token is not authorized", func(t *testing.T) { rsp := httptest.NewRecorder() @@ -145,7 +130,7 @@ func TestResourceWriteHandler(t *testing.T) { req.Header.Add("x-consul-token", testACLTokenArtistReadPolicy) - v2ArtistHandler.ServeHTTP(rsp, req) + handler.ServeHTTP(rsp, req) require.Equal(t, http.StatusForbidden, rsp.Result().StatusCode) }) @@ -166,7 +151,7 @@ func TestResourceWriteHandler(t *testing.T) { req.Header.Add("x-consul-token", testACLTokenArtistWritePolicy) - v2ArtistHandler.ServeHTTP(rsp, req) + handler.ServeHTTP(rsp, req) require.Equal(t, http.StatusOK, rsp.Result().StatusCode) @@ -206,7 +191,7 @@ func TestResourceWriteHandler(t *testing.T) { req.Header.Add("x-consul-token", testACLTokenArtistWritePolicy) - v2ArtistHandler.ServeHTTP(rsp, req) + handler.ServeHTTP(rsp, req) require.Equal(t, http.StatusOK, rsp.Result().StatusCode) var result map[string]any @@ -231,7 +216,7 @@ func TestResourceWriteHandler(t *testing.T) { req.Header.Add("x-consul-token", testACLTokenArtistWritePolicy) - v2ArtistHandler.ServeHTTP(rsp, req) + handler.ServeHTTP(rsp, req) require.Equal(t, http.StatusConflict, rsp.Result().StatusCode) }) @@ -265,7 +250,7 @@ func TestResourceWriteHandler(t *testing.T) { req.Header.Add("x-consul-token", testACLTokenArtistWritePolicy) - v1ArtistHandler.ServeHTTP(rsp, req) + handler.ServeHTTP(rsp, req) require.Equal(t, http.StatusOK, rsp.Result().StatusCode) @@ -291,7 +276,7 @@ func TestResourceWriteHandler(t *testing.T) { }) } -func createResource(t *testing.T, artistHandler resourceHandler) { +func createResource(t *testing.T, artistHandler http.Handler) map[string]any { rsp := httptest.NewRecorder() req := httptest.NewRequest("PUT", "/demo/v2/artist/keith-urban?partition=default&peer_name=local&namespace=default", strings.NewReader(` { @@ -309,6 +294,65 @@ func createResource(t *testing.T, artistHandler resourceHandler) { artistHandler.ServeHTTP(rsp, req) require.Equal(t, http.StatusOK, rsp.Result().StatusCode) + + var result map[string]any + require.NoError(t, json.NewDecoder(rsp.Body).Decode(&result)) + return result +} + +func TestResourceReadHandler(t *testing.T) { + aclResolver := &resourceSvc.MockACLResolver{} + aclResolver.On("ResolveTokenAndDefaultMeta", testACLTokenArtistReadPolicy, mock.Anything, mock.Anything). + Return(svctest.AuthorizerFrom(t, demo.ArtistV1ReadPolicy, demo.ArtistV2ReadPolicy), nil) + aclResolver.On("ResolveTokenAndDefaultMeta", testACLTokenArtistWritePolicy, mock.Anything, mock.Anything). + Return(svctest.AuthorizerFrom(t, demo.ArtistV1WritePolicy, demo.ArtistV2WritePolicy), nil) + aclResolver.On("ResolveTokenAndDefaultMeta", fakeToken, mock.Anything, mock.Anything). + Return(svctest.AuthorizerFrom(t, ""), nil) + + client := svctest.RunResourceServiceWithACL(t, aclResolver, demo.RegisterTypes) + + r := resource.NewRegistry() + demo.RegisterTypes(r) + handler := NewHandler(client, r, parseToken, hclog.NewNullLogger()) + + createdResource := createResource(t, handler) + + t.Run("Read resource", func(t *testing.T) { + rsp := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/demo/v2/artist/keith-urban?partition=default&peer_name=local&namespace=default&consistent", nil) + + req.Header.Add("x-consul-token", testACLTokenArtistReadPolicy) + + handler.ServeHTTP(rsp, req) + + require.Equal(t, http.StatusOK, rsp.Result().StatusCode) + + var result map[string]any + require.NoError(t, json.NewDecoder(rsp.Body).Decode(&result)) + require.Equal(t, result, createdResource) + }) + + t.Run("should not be found if resource not exist", func(t *testing.T) { + rsp := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/demo/v2/artist/keith-not-exist?partition=default&peer_name=local&namespace=default&consistent", nil) + + req.Header.Add("x-consul-token", testACLTokenArtistReadPolicy) + + handler.ServeHTTP(rsp, req) + + require.Equal(t, http.StatusNotFound, rsp.Result().StatusCode) + }) + + t.Run("should be blocked if the token is not authorized", func(t *testing.T) { + rsp := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/demo/v2/artist/keith-urban?partition=default&peer_name=local&namespace=default&consistent", nil) + + req.Header.Add("x-consul-token", fakeToken) + + handler.ServeHTTP(rsp, req) + + require.Equal(t, http.StatusForbidden, rsp.Result().StatusCode) + }) } func TestResourceDeleteHandler(t *testing.T) { @@ -320,38 +364,33 @@ func TestResourceDeleteHandler(t *testing.T) { client := svctest.RunResourceServiceWithACL(t, aclResolver, demo.RegisterTypes) - v2ArtistHandler := resourceHandler{ - resource.Registration{ - Type: demo.TypeV2Artist, - Proto: &pbdemov2.Artist{}, - }, - client, - parseToken, - hclog.NewNullLogger(), - } + r := resource.NewRegistry() + demo.RegisterTypes(r) + + handler := NewHandler(client, r, parseToken, hclog.NewNullLogger()) t.Run("should surface PermissionDenied error from resource service", func(t *testing.T) { - createResource(t, v2ArtistHandler) + createResource(t, handler) deleteRsp := httptest.NewRecorder() deletReq := httptest.NewRequest("DELETE", "/demo/v2/artist/keith-urban?partition=default&peer_name=local&namespace=default", strings.NewReader("")) deletReq.Header.Add("x-consul-token", testACLTokenArtistReadPolicy) - v2ArtistHandler.ServeHTTP(deleteRsp, deletReq) + handler.ServeHTTP(deleteRsp, deletReq) require.Equal(t, http.StatusForbidden, deleteRsp.Result().StatusCode) }) t.Run("should delete a resource without version", func(t *testing.T) { - createResource(t, v2ArtistHandler) + createResource(t, handler) deleteRsp := httptest.NewRecorder() deletReq := httptest.NewRequest("DELETE", "/demo/v2/artist/keith-urban?partition=default&peer_name=local&namespace=default", strings.NewReader("")) deletReq.Header.Add("x-consul-token", testACLTokenArtistWritePolicy) - v2ArtistHandler.ServeHTTP(deleteRsp, deletReq) + handler.ServeHTTP(deleteRsp, deletReq) require.Equal(t, http.StatusNoContent, deleteRsp.Result().StatusCode) @@ -370,14 +409,14 @@ func TestResourceDeleteHandler(t *testing.T) { }) t.Run("should delete a resource with version", func(t *testing.T) { - createResource(t, v2ArtistHandler) + createResource(t, handler) rsp := httptest.NewRecorder() req := httptest.NewRequest("DELETE", "/demo/v2/artist/keith-urban?partition=default&peer_name=local&namespace=default&version=1", strings.NewReader("")) req.Header.Add("x-consul-token", testACLTokenArtistWritePolicy) - v2ArtistHandler.ServeHTTP(rsp, req) + handler.ServeHTTP(rsp, req) require.Equal(t, http.StatusNoContent, rsp.Result().StatusCode)