From f583f68cfcb4b6477694dc91109f160d68b9db34 Mon Sep 17 00:00:00 2001 From: Poonam Jadhav Date: Wed, 9 Aug 2023 15:46:25 -0400 Subject: [PATCH] feat: list resources endpoint --- internal/resource/http/http.go | 115 +++++++++++++++---- internal/resource/http/http_test.go | 171 +++++++++++++++++++++------- 2 files changed, 225 insertions(+), 61 deletions(-) diff --git a/internal/resource/http/http.go b/internal/resource/http/http.go index b6a018c40578..5e2215102669 100644 --- a/internal/resource/http/http.go +++ b/internal/resource/http/http.go @@ -23,6 +23,11 @@ import ( "github.com/hashicorp/consul/proto-public/pbresource" ) +const ( + HeaderConsulToken = "x-consul-token" + HeaderConsistencyMode = "x-consul-consistency-mode" +) + func NewHandler( client pbresource.ResourceServiceClient, registry resource.Registry, @@ -30,8 +35,12 @@ func NewHandler( logger hclog.Logger) http.Handler { mux := http.NewServeMux() for _, t := range registry.Types() { - // Individual Resource Endpoints. - prefix := strings.ToLower(fmt.Sprintf("/%s/%s/%s/", t.Type.Group, t.Type.GroupVersion, t.Type.Kind)) + // List Endpoint + base := strings.ToLower(fmt.Sprintf("/%s/%s/%s", t.Type.Group, t.Type.GroupVersion, t.Type.Kind)) + mux.Handle(base, http.StripPrefix(base, &listHandler{t, client, parseToken, logger})) + + // Individual Resource Endpoints + prefix := strings.ToLower(fmt.Sprintf("%s/", base)) logger.Info("Registered resource endpoint", "endpoint", prefix) mux.Handle(prefix, http.StripPrefix(prefix, &resourceHandler{t, client, parseToken, logger})) } @@ -55,7 +64,7 @@ type resourceHandler struct { func (h *resourceHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { var token string h.parseToken(r, &token) - ctx := metadata.AppendToOutgoingContext(r.Context(), "x-consul-token", token) + ctx := metadata.AppendToOutgoingContext(r.Context(), HeaderConsulToken, token) switch r.Method { case http.MethodPut: h.handleWrite(w, r, ctx) @@ -88,23 +97,23 @@ func (h *resourceHandler) handleWrite(w http.ResponseWriter, r *http.Request, ct return } - tenancyInfo, resourceName, version := parseParams(r) + tenancyInfo, queryParams := 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: queryParams["resourceName"], }, Owner: req.Owner, - Version: version, + Version: queryParams["version"], Metadata: req.Metadata, Data: anyProtoMsg, }, }) if err != nil { - handleResponseError(err, w, h) + handleResponseError(err, w, h.logger) return } @@ -117,20 +126,28 @@ 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) { +func parseParams(r *http.Request) (tenancy *pbresource.Tenancy, queryParams map[string]string) { params := r.URL.Query() tenancy = &pbresource.Tenancy{ Partition: params.Get("partition"), PeerName: params.Get("peer_name"), Namespace: params.Get("namespace"), } - resourceName = path.Base(r.URL.Path) + resourceName := path.Base(r.URL.Path) if resourceName == "." || resourceName == "/" { resourceName = "" } - version = params.Get("version") - return + queryParams = make(map[string]string) + queryParams["resourceName"] = resourceName + queryParams["version"] = params.Get("version") + queryParams["namePrefix"] = params.Get("name_prefix") + + if params.Has("consistent") { + queryParams["consistent"] = "true" + } + + return tenancy, queryParams } func jsonMarshal(res *pbresource.Resource) ([]byte, error) { @@ -148,47 +165,101 @@ func jsonMarshal(res *pbresource.Resource) ([]byte, error) { return json.MarshalIndent(stuff, "", " ") } -func handleResponseError(err error, w http.ResponseWriter, h *resourceHandler) { +func handleResponseError(err error, w http.ResponseWriter, logger hclog.Logger) { if e, ok := status.FromError(err); ok { switch e.Code() { case codes.InvalidArgument: w.WriteHeader(http.StatusBadRequest) - h.logger.Info("User has mal-formed request", "error", err) + logger.Info("User has mal-formed request", "error", err) case codes.NotFound: w.WriteHeader(http.StatusNotFound) - h.logger.Info("Received error from resource service: Not found", "error", err) + logger.Info("Received error from resource service: Not found", "error", err) case codes.PermissionDenied: w.WriteHeader(http.StatusForbidden) - h.logger.Info("Received error from resource service: User not authenticated", "error", err) + logger.Info("Received error from resource service: User not authenticated", "error", err) case codes.Aborted: w.WriteHeader(http.StatusConflict) - h.logger.Info("Received error from resource service: the request conflict with the current state of the target resource", "error", err) + logger.Info("Received error from resource service: the request conflict with the current state of the target resource", "error", err) default: w.WriteHeader(http.StatusInternalServerError) - h.logger.Error("Received error from resource service", "error", err) + logger.Error("Received error from resource service", "error", err) } } else { w.WriteHeader(http.StatusInternalServerError) - h.logger.Error("Received error from resource service: not able to parse error returned", "error", err) + logger.Error("Received error from resource service: not able to parse error returned", "error", err) } 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) + tenancyInfo, queryParams := parseParams(r) _, err := h.client.Delete(ctx, &pbresource.DeleteRequest{ Id: &pbresource.ID{ Type: h.reg.Type, Tenancy: tenancyInfo, - Name: resourceName, + Name: queryParams["resourceName"], }, - Version: version, + Version: queryParams["version"], }) if err != nil { - handleResponseError(err, w, h) + handleResponseError(err, w, h.logger) return } w.WriteHeader(http.StatusNoContent) w.Write([]byte("{}")) } + +type listHandler struct { + reg resource.Registration + client pbresource.ResourceServiceClient + parseToken func(req *http.Request, token *string) + logger hclog.Logger +} + +func (h *listHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + w.WriteHeader(http.StatusMethodNotAllowed) + return + } + + var token string + h.parseToken(r, &token) + ctx := metadata.AppendToOutgoingContext(r.Context(), HeaderConsulToken, token) + + tenancyInfo, queryParams := parseParams(r) + if queryParams["consistent"] == "true" { + ctx = metadata.AppendToOutgoingContext(ctx, HeaderConsistencyMode, "consistent") + } + + rsp, err := h.client.List(ctx, &pbresource.ListRequest{ + Type: h.reg.Type, + Tenancy: tenancyInfo, + NamePrefix: queryParams["namePrefix"], + }) + if err != nil { + handleResponseError(err, w, h.logger) + return + } + + output := make([]json.RawMessage, len(rsp.Resources)) + for idx, res := range rsp.Resources { + b, err := jsonMarshal(res) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + h.logger.Error("Failed to unmarshal GRPC resource response", "error", err) + return + } + output[idx] = b + } + + b, err := json.MarshalIndent(struct { + Resources []json.RawMessage `json:"resources"` + }{output}, "", " ") + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + h.logger.Error("Failed to correcty format the list response", "error", err) + return + } + w.Write(b) +} diff --git a/internal/resource/http/http_test.go b/internal/resource/http/http_test.go index 6747fb251a5a..faa5f396d4e7 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 testACLTokenArtistListPolicy = "00000000-0000-0000-0000-000000000003" func parseToken(req *http.Request, token *string) { *token = req.Header.Get("x-consul-token") @@ -109,25 +110,10 @@ 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(), - } + r := resource.NewRegistry() + demo.RegisterTypes(r) - v2ArtistHandler := resourceHandler{ - resource.Registration{ - Type: demo.TypeV2Artist, - Proto: &pbdemov2.Artist{}, - }, - client, - parseToken, - hclog.NewNullLogger(), - } + 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 +131,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 +152,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 +192,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 +217,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 +251,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 +277,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 +295,20 @@ 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 deleteResource(t *testing.T, artistHandler http.Handler) { + rsp := httptest.NewRecorder() + req := httptest.NewRequest("DELETE", "/demo/v2/artist/keith-urban?partition=default&peer_name=local&namespace=default", strings.NewReader("")) + + req.Header.Add("x-consul-token", testACLTokenArtistWritePolicy) + + artistHandler.ServeHTTP(rsp, req) + require.Equal(t, http.StatusNoContent, rsp.Result().StatusCode) } func TestResourceDeleteHandler(t *testing.T) { @@ -320,38 +320,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 +365,15 @@ 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) + req.Header.Add("x-consul-token", testACLTokenArtistListPolicy) - v2ArtistHandler.ServeHTTP(rsp, req) + handler.ServeHTTP(rsp, req) require.Equal(t, http.StatusNoContent, rsp.Result().StatusCode) @@ -391,3 +387,100 @@ func TestResourceDeleteHandler(t *testing.T) { require.ErrorContains(t, err, "resource not found") }) } + +func TestResourceListHandler(t *testing.T) { + aclResolver := &resourceSvc.MockACLResolver{} + aclResolver.On("ResolveTokenAndDefaultMeta", testACLTokenArtistListPolicy, mock.Anything, mock.Anything). + Return(svctest.AuthorizerFrom(t, demo.ArtistV2ListPolicy), nil) + aclResolver.On("ResolveTokenAndDefaultMeta", testACLTokenArtistWritePolicy, mock.Anything, mock.Anything). + Return(svctest.AuthorizerFrom(t, demo.ArtistV2WritePolicy), nil) + + client := svctest.RunResourceServiceWithACL(t, aclResolver, demo.RegisterTypes) + + r := resource.NewRegistry() + demo.RegisterTypes(r) + + handler := NewHandler(client, r, parseToken, hclog.NewNullLogger()) + + t.Run("should return MethodNotAllowed", func(t *testing.T) { + rsp := httptest.NewRecorder() + req := httptest.NewRequest("PUT", "/demo/v2/artist?partition=default&peer_name=local&namespace=default", strings.NewReader("")) + + req.Header.Add("x-consul-token", testACLTokenArtistListPolicy) + + handler.ServeHTTP(rsp, req) + + require.Equal(t, http.StatusMethodNotAllowed, 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?partition=default&peer_name=local&namespace=default", strings.NewReader("")) + + req.Header.Add("x-consul-token", testACLTokenArtistWritePolicy) + + handler.ServeHTTP(rsp, req) + + require.Equal(t, http.StatusForbidden, rsp.Result().StatusCode) + }) + + t.Run("should return list of resources", func(t *testing.T) { + resource := createResource(t, handler) + + rsp := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/demo/v2/artist?partition=default&peer_name=local&namespace=default", strings.NewReader("")) + + req.Header.Add("x-consul-token", testACLTokenArtistListPolicy) + + 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)) + + resources, _ := result["resources"].([]any) + require.Len(t, resources, 1) + + require.Equal(t, resource, resources[0]) + + // clean up + deleteResource(t, handler) + }) + + t.Run("should return empty list when no resources are found", func(t *testing.T) { + rsp := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/demo/v2/artist?partition=default&peer_name=local&namespace=default", strings.NewReader("")) + + req.Header.Add("x-consul-token", testACLTokenArtistListPolicy) + + 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)) + + resources, _ := result["resources"].([]any) + require.Len(t, resources, 0) + }) + + t.Run("should return empty list when name prefix matches don't match", func(t *testing.T) { + _ = createResource(t, handler) + + rsp := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/demo/v2/artist?partition=default&peer_name=local&namespace=default&name_prefix=noname", strings.NewReader("")) + + req.Header.Add("x-consul-token", testACLTokenArtistListPolicy) + + 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)) + + resources, _ := result["resources"].([]any) + require.Len(t, resources, 0) + }) +}