Net-2708/delete resource endpoint (#18420)

* feat: add http delete endpoint for resource service

* refactor: clean up

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("{}"))
+}

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")
+	})
+}