Skip to content

Commit

Permalink
Update Dataset API GET routes to POST
Browse files Browse the repository at this point in the history 
Unlike cURL, prominent web clients and interfaces do not permit GET
requests to send request data in the body; largely invalid. On the
contrary, we can't use params in RESTful style because of dataset names
with '/'.

Signed-off-by: Raamsri Kumar <[email protected]>
  • Loading branch information
@raamsri
raamsri committed Jan 27, 2025
1 parent 527a959 commit 97344f8
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 272 deletions.
15 changes: 15 additions & 0 deletions pkg/server/middleware.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
package server

import (
"bytes"
"io"
"log/slog"
"time"

Expand Down Expand Up @@ -50,6 +52,14 @@ func LoggerMiddleware(l logger.Logger) gin.HandlerFunc {
// Store request ID in context for error correlation
c.Set("request_id", requestID)

bodyBytes := []byte{}

// Log request body if present
if c.Request.Body != nil {
bodyBytes, _ = io.ReadAll(c.Request.Body)
c.Request.Body = io.NopCloser(bytes.NewBuffer(bodyBytes))
}

// Process request
c.Next()

Expand All @@ -73,6 +83,11 @@ func LoggerMiddleware(l logger.Logger) gin.HandlerFunc {

// Handle errors if present
if len(c.Errors) > 0 {
// Log request body if present
if len(bodyBytes) > 0 {
attrs = append(attrs, slog.String("body", string(bodyBytes)))
}

for _, err := range c.Errors {
if re, ok := err.Err.(*errors.RodentError); ok {
// Add RodentError fields
Expand Down
10 changes: 5 additions & 5 deletions pkg/zfs/api/dataset_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func TestDatasetAPI(t *testing.T) {
Recursive: false,
}
body, _ = json.Marshal(listReq)
req = httptest.NewRequest("GET", dsURI+"/filesystems", bytes.NewBuffer(body))
req = httptest.NewRequest("POST", dsURI+"/filesystems/list", bytes.NewBuffer(body))
req.Header.Set("Content-Type", "application/json")
w = httptest.NewRecorder()
router.ServeHTTP(w, req)
Expand Down Expand Up @@ -190,7 +190,7 @@ func TestDatasetAPI(t *testing.T) {
}

body, _ = json.Marshal(getReq)
req = httptest.NewRequest("GET", dsURI+"/property", bytes.NewBuffer(body))
req = httptest.NewRequest("POST", dsURI+"/property/fetch", bytes.NewBuffer(body))
req.Header.Set("Content-Type", "application/json")
w = httptest.NewRecorder()
router.ServeHTTP(w, req)
Expand Down Expand Up @@ -241,7 +241,7 @@ func TestDatasetAPI(t *testing.T) {
Type: "volume",
}
body, _ = json.Marshal(listReq)
req = httptest.NewRequest("GET", dsURI+"/volumes", bytes.NewBuffer(body))
req = httptest.NewRequest("POST", dsURI+"/volumes/list", bytes.NewBuffer(body))
req.Header.Set("Content-Type", "application/json")
w = httptest.NewRecorder()
router.ServeHTTP(w, req)
Expand Down Expand Up @@ -283,7 +283,7 @@ func TestDatasetAPI(t *testing.T) {
Name: baseFS,
}
body, _ = json.Marshal(listReq)
req = httptest.NewRequest("GET", dsURI+"/snapshots", bytes.NewBuffer(body))
req = httptest.NewRequest("POST", dsURI+"/snapshots/list", bytes.NewBuffer(body))
req.Header.Set("Content-Type", "application/json")
w = httptest.NewRecorder()
router.ServeHTTP(w, req)
Expand Down Expand Up @@ -633,7 +633,7 @@ func TestDatasetAPI(t *testing.T) {
}

body, _ := json.Marshal(listReq)
req := httptest.NewRequest("GET", dsURI+"/permissions",
req := httptest.NewRequest("POST", dsURI+"/permissions/list",
bytes.NewBuffer(body))
req.Header.Set("Content-Type", "application/json")
w := httptest.NewRecorder()
Expand Down
11 changes: 11 additions & 0 deletions pkg/zfs/api/pool.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,17 @@ func (h *PoolHandler) getPoolStatus(c *gin.Context) {
c.JSON(http.StatusOK, status)
}

func (h *PoolHandler) getProperties(c *gin.Context) {
name := c.Param("name")

result, err := h.manager.GetProperties(c.Request.Context(), name)
if err != nil {
APIError(c, err)
return
}
c.JSON(http.StatusOK, result)
}

func (h *PoolHandler) getProperty(c *gin.Context) {
name := c.Param("name")
property := c.Param("property")
Expand Down
21 changes: 12 additions & 9 deletions pkg/zfs/api/routes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (h *DatasetHandler) RegisterRoutes(router *gin.RouterGroup) {
// TODO: Add appropriate validation middlewares

// Dataset operations
dataset.GET("", h.listDatasets)
dataset.POST("/list", h.listDatasets)

dataset.DELETE("",
ValidateZFSEntityName(common.TypeZFSEntityMask),
Expand All @@ -149,13 +149,13 @@ func (h *DatasetHandler) RegisterRoutes(router *gin.RouterGroup) {
properties := dataset.Group("/properties",
ValidateZFSEntityName(common.TypeZFSEntityMask))
{
properties.GET("", h.listProperties)
properties.POST("/list", h.listProperties)
}

property := dataset.Group("/property",
ValidateZFSEntityName(common.TypeZFSEntityMask))
{
property.GET("",
property.POST("/fetch",
ValidatePropertyName(),
h.getProperty)
property.PUT("",
Expand All @@ -169,7 +169,7 @@ func (h *DatasetHandler) RegisterRoutes(router *gin.RouterGroup) {
// Filesystem operations
filesystems := dataset.Group("/filesystems")
{
filesystems.GET("", h.listFilesystems)
filesystems.POST("/list", h.listFilesystems)
}

filesystem := dataset.Group("/filesystem")
Expand All @@ -194,7 +194,7 @@ func (h *DatasetHandler) RegisterRoutes(router *gin.RouterGroup) {
// Volume operations
volumes := dataset.Group("/volumes")
{
volumes.GET("", h.listVolumes)
volumes.POST("/list", h.listVolumes)
}
volume := dataset.Group("/volume")
{
Expand All @@ -208,7 +208,7 @@ func (h *DatasetHandler) RegisterRoutes(router *gin.RouterGroup) {
// Snapshot operations
snapshots := dataset.Group("/snapshots")
{
snapshots.GET("", h.listSnapshots)
snapshots.POST("/list", h.listSnapshots)
}
snapshot := dataset.Group("/snapshot")
{
Expand Down Expand Up @@ -239,7 +239,7 @@ func (h *DatasetHandler) RegisterRoutes(router *gin.RouterGroup) {
// Bookmark operations
bookmarks := dataset.Group("/bookmarks")
{
bookmarks.GET("", h.listBookmarks)
bookmarks.POST("/list", h.listBookmarks)
}
bookmark := dataset.Group("/bookmark")
{
Expand All @@ -252,7 +252,7 @@ func (h *DatasetHandler) RegisterRoutes(router *gin.RouterGroup) {
permissions := dataset.Group("/permissions",
ValidateZFSEntityName(common.TypeDatasetMask))
{
permissions.GET("", h.listPermissions)
permissions.POST("/list", h.listPermissions)
permissions.POST("",
ValidatePermissionConfig(),
h.allowPermissions)
Expand All @@ -274,7 +274,7 @@ func (h *DatasetHandler) RegisterRoutes(router *gin.RouterGroup) {
transfer.POST("/send",
h.sendDataset)

transfer.GET("/resume-token",
transfer.POST("/resume-token/fetch",
ValidateZFSEntityName(common.TypeFilesystem),
h.getResumeToken)
}
Expand Down Expand Up @@ -369,6 +369,9 @@ func (h *PoolHandler) RegisterRoutes(router *gin.RouterGroup) {

// Status and properties
pools.GET("/:name/status", ValidatePoolName(), h.getPoolStatus)
pools.GET("/:name/properties",
ValidatePoolName(),
h.getProperties)
pools.GET("/:name/properties/:property",
ValidatePoolName(),
ValidatePoolProperty(common.ValidPoolGetPropContext),
Expand Down
Loading

0 comments on commit 97344f8

Please sign in to comment.