diff --git a/internal/resource/http/http.go b/internal/resource/http/http.go index 26ae55a943..b6a018c405 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.MethodDelete: + h.handleDelete(w, r, ctx) default: w.WriteHeader(http.StatusMethodNotAllowed) return @@ -86,7 +88,7 @@ func (h *resourceHandler) handleWrite(w http.ResponseWriter, r *http.Request, ct return } - tenancyInfo, resourceName, version := checkURL(r) + tenancyInfo, resourceName, version := parseParams(r) rsp, err := h.client.Write(ctx, &pbresource.WriteRequest{ Resource: &pbresource.Resource{ @@ -115,7 +117,7 @@ func (h *resourceHandler) handleWrite(w http.ResponseWriter, r *http.Request, ct w.Write(output) } -func checkURL(r *http.Request) (tenancy *pbresource.Tenancy, resourceName string, version string) { +func parseParams(r *http.Request) (tenancy *pbresource.Tenancy, resourceName string, version string) { params := r.URL.Query() tenancy = &pbresource.Tenancy{ Partition: params.Get("partition"), @@ -154,20 +156,39 @@ func handleResponseError(err error, w http.ResponseWriter, h *resourceHandler) { h.logger.Info("User has mal-formed request", "error", err) case codes.NotFound: w.WriteHeader(http.StatusNotFound) - h.logger.Info("Failed to write to GRPC resource: Not found", "error", err) + h.logger.Info("Received error from resource service: Not found", "error", err) case codes.PermissionDenied: w.WriteHeader(http.StatusForbidden) - h.logger.Info("Failed to write to GRPC resource: User not authenticated", "error", err) + h.logger.Info("Received error from resource service: User not authenticated", "error", err) case codes.Aborted: w.WriteHeader(http.StatusConflict) - h.logger.Info("Failed to write to GRPC resource: the request conflict with the current state of the target resource", "error", err) + h.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("Failed to write to GRPC resource", "error", err) + h.logger.Error("Received error from resource service", "error", err) } } else { w.WriteHeader(http.StatusInternalServerError) - h.logger.Error("Failed to write to GRPC resource: not able to parse error returned", "error", err) + h.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) + _, 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 65b9ce6ba9..6747fb251a 100644 --- a/internal/resource/http/http_test.go +++ b/internal/resource/http/http_test.go @@ -29,7 +29,7 @@ const testACLTokenArtistReadPolicy = "00000000-0000-0000-0000-000000000001" const testACLTokenArtistWritePolicy = "00000000-0000-0000-0000-000000000002" func parseToken(req *http.Request, token *string) { - *token = req.Header.Get("X-Consul-Token") + *token = req.Header.Get("x-consul-token") } func TestResourceHandler_InputValidation(t *testing.T) { @@ -83,6 +83,12 @@ func TestResourceHandler_InputValidation(t *testing.T) { response: httptest.NewRecorder(), expectedResponseCode: http.StatusBadRequest, }, + { + description: "no id", + request: httptest.NewRequest("DELETE", "/?partition=default&peer_name=local&namespace=default", strings.NewReader("")), + response: httptest.NewRecorder(), + expectedResponseCode: http.StatusBadRequest, + }, } for _, tc := range testCases { @@ -284,3 +290,104 @@ func TestResourceWriteHandler(t *testing.T) { require.Equal(t, "Keith Urban V1", artist.Name) }) } + +func createResource(t *testing.T, artistHandler resourceHandler) { + rsp := httptest.NewRecorder() + req := httptest.NewRequest("PUT", "/demo/v2/artist/keith-urban?partition=default&peer_name=local&namespace=default", strings.NewReader(` + { + "metadata": { + "foo": "bar" + }, + "data": { + "name": "Keith Urban", + "genre": "GENRE_COUNTRY" + } + } + `)) + + req.Header.Add("x-consul-token", testACLTokenArtistWritePolicy) + + artistHandler.ServeHTTP(rsp, req) + require.Equal(t, http.StatusOK, rsp.Result().StatusCode) +} + +func TestResourceDeleteHandler(t *testing.T) { + aclResolver := &resourceSvc.MockACLResolver{} + aclResolver.On("ResolveTokenAndDefaultMeta", testACLTokenArtistReadPolicy, mock.Anything, mock.Anything). + Return(svctest.AuthorizerFrom(t, demo.ArtistV2ReadPolicy), nil) + aclResolver.On("ResolveTokenAndDefaultMeta", testACLTokenArtistWritePolicy, mock.Anything, mock.Anything). + Return(svctest.AuthorizerFrom(t, demo.ArtistV2WritePolicy), nil) + + client := svctest.RunResourceServiceWithACL(t, aclResolver, demo.RegisterTypes) + + v2ArtistHandler := resourceHandler{ + resource.Registration{ + Type: demo.TypeV2Artist, + Proto: &pbdemov2.Artist{}, + }, + client, + parseToken, + hclog.NewNullLogger(), + } + + t.Run("should surface PermissionDenied error from resource service", func(t *testing.T) { + createResource(t, v2ArtistHandler) + + 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) + + require.Equal(t, http.StatusForbidden, deleteRsp.Result().StatusCode) + }) + + t.Run("should delete a resource without version", func(t *testing.T) { + createResource(t, v2ArtistHandler) + + 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) + + require.Equal(t, http.StatusNoContent, deleteRsp.Result().StatusCode) + + var result map[string]any + require.NoError(t, json.NewDecoder(deleteRsp.Body).Decode(&result)) + require.Empty(t, result) + + _, err := client.Read(testutil.TestContext(t), &pbresource.ReadRequest{ + Id: &pbresource.ID{ + Type: demo.TypeV2Artist, + Tenancy: demo.TenancyDefault, + Name: "keith-urban", + }, + }) + require.ErrorContains(t, err, "resource not found") + }) + + t.Run("should delete a resource with version", func(t *testing.T) { + createResource(t, v2ArtistHandler) + + 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) + + require.Equal(t, http.StatusNoContent, rsp.Result().StatusCode) + + _, err := client.Read(testutil.TestContext(t), &pbresource.ReadRequest{ + Id: &pbresource.ID{ + Type: demo.TypeV2Artist, + Tenancy: demo.TenancyDefault, + Name: "keith-urban", + }, + }) + require.ErrorContains(t, err, "resource not found") + }) +}