Skip to content

Commit

Permalink
fix: :db/:rp -> :dbrp, fixed create bucket, update bucket (#23898) (#…
Browse files Browse the repository at this point in the history 
…23900)

Fix URL processing for V2 buckets API, add error checking.

(cherry picked from commit 636fbb3)

Co-authored-by: Vlasta Hajek <vlastimil.hajek@bonitoo.io>
  • Loading branch information
@davidby-influx @vlastahajek
davidby-influx and vlastahajek committed Nov 11, 2022
1 parent 9e9f1be commit be9a3d4
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 50 deletions.
100 changes: 72 additions & 28 deletions services/httpd/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,55 +215,55 @@ func NewHandler(c Config) *Handler {
},
Route{
"delete-bucket",
"DELETE", "/api/v2/buckets/:db/:rp", false, true, h.serveDeleteBucketV2,
"DELETE", "/api/v2/buckets/:dbrp", false, true, h.serveDeleteBucketV2,
},
Route{
"retrieve-bucket",
"GET", "/api/v2/buckets/:db/:rp", false, true, h.serveRetrieveBucketV2,
"GET", "/api/v2/buckets/:dbrp", false, true, h.serveRetrieveBucketV2,
},
Route{
"list-buckets",
"GET", "/api/v2/buckets", false, true, h.serveListBucketsV2,
},
Route{
"update-bucket",
"PATCH", "/api/v2/buckets/:db/:rp", false, true, h.serveUpdateBucketV2,
"PATCH", "/api/v2/buckets/:dbrp", false, true, h.serveUpdateBucketV2,
},
Route{
"bucket-labels",
"GET", "/api/v2/buckets/:db/:rp/labels", false, true, h.serveLabelsNotAllowedV2,
"GET", "/api/v2/buckets/:dbrp/labels", false, true, h.serveLabelsNotAllowedV2,
},
Route{
"add-bucket-label",
"POST", "/api/v2/buckets/:db/:rp/labels", false, true, h.serveLabelsNotAllowedV2,
"POST", "/api/v2/buckets/:dbrp/labels", false, true, h.serveLabelsNotAllowedV2,
},
Route{
"delete-bucket-label",
"DELETE", "/api/v2/buckets/:db/:rp/labels/:labelID", false, true, h.serveLabelsNotAllowedV2,
"DELETE", "/api/v2/buckets/:dbrp/labels/:labelID", false, true, h.serveLabelsNotAllowedV2,
},
Route{
"bucket-members",
"GET", "/api/v2/buckets/:db/:rp/members", false, true, h.serveBucketMembersNotAllowedV2,
"GET", "/api/v2/buckets/:dbrp/members", false, true, h.serveBucketMembersNotAllowedV2,
},
Route{
"add-bucket-member",
"POST", "/api/v2/buckets/:db/:rp/members", false, true, h.serveBucketMembersNotAllowedV2,
"POST", "/api/v2/buckets/:dbrp/members", false, true, h.serveBucketMembersNotAllowedV2,
},
Route{
"delete-bucket-member",
"DELETE", "/api/v2/buckets/:db/:rp/members/:userID", false, true, h.serveBucketMembersNotAllowedV2,
"DELETE", "/api/v2/buckets/:dbrp/members/:userID", false, true, h.serveBucketMembersNotAllowedV2,
},
Route{
"bucket-owners",
"GET", "/api/v2/buckets/:db/:rp/owners", false, true, h.serveBucketOwnersNotAllowedV2,
"GET", "/api/v2/buckets/:dbrp/owners", false, true, h.serveBucketOwnersNotAllowedV2,
},
Route{
"add-bucket-owner",
"POST", "/api/v2/buckets/:db/:rp/owners", false, true, h.serveBucketOwnersNotAllowedV2,
"POST", "/api/v2/buckets/:dbrp/owners", false, true, h.serveBucketOwnersNotAllowedV2,
},
Route{
"delete-bucket-owner",
"DELETE", "/api/v2/buckets/:db/:rp/owners/:userID", false, true, h.serveBucketOwnersNotAllowedV2,
"DELETE", "/api/v2/buckets/:dbrp/owners/:userID", false, true, h.serveBucketOwnersNotAllowedV2,
},
Route{
"write", // Data-ingest route.
Expand Down Expand Up @@ -1166,27 +1166,52 @@ func (h *Handler) servePostCreateBucketV2(w http.ResponseWriter, r *http.Request
}
}
}

var rpi *meta.RetentionPolicyInfo
if dbi == nil {
if _, err = h.MetaClient.CreateDatabaseWithRetentionPolicy(db, &spec); err != nil {
if dbi, err = h.MetaClient.CreateDatabaseWithRetentionPolicy(db, &spec); err != nil {
h.httpError(w, fmt.Sprintf("buckets - cannot create bucket %q: %s", brd.Name, err.Error()), http.StatusBadRequest)
return
} else {
rpi = &dbi.RetentionPolicies[0]
}

} else {
if _, err = h.MetaClient.CreateRetentionPolicy(db, &spec, false); err != nil {
if rpi, err = h.MetaClient.CreateRetentionPolicy(db, &spec, false); err != nil {
h.httpError(w, fmt.Sprintf("buckets - cannot create bucket %q: %s", brd.Name, err.Error()), http.StatusBadRequest)
return
}
}
bucket := makeBucket(rpi, db)
b, err := json.Marshal(bucket)
if err != nil {
h.httpError(w, fmt.Sprintf("buckets - cannot marshal bucket %q: %s", brd.Name, err.Error()), http.StatusBadRequest)
return
}
w.WriteHeader(http.StatusCreated)
w.Write(b)
}

// checkDbRp checks id for database/retention-policy format
func (h *Handler) checkDbRp(w http.ResponseWriter, id string) (string, string, bool) {
db, rp, err := bucket2dbrp(id)
if err != nil {
h.httpError(w, fmt.Sprintf("bucket %q: %s", id, err.Error()), http.StatusBadRequest)
return "", "", false
} else if rp == "" {
h.httpError(w, fmt.Sprintf("bucket %q: illegal bucket id, empty retention policy", id), http.StatusBadRequest)
return "", "", false
}
return db, rp, true
}

// serveDeleteBucketV2 should do the same thing as coordinator/statement_executor.go/executeDropRetentionPolicyStatement
// i.e., Store.DeleteRetentionPolicy and MetaClient.DropRetentionPolicy
func (h *Handler) serveDeleteBucketV2(w http.ResponseWriter, r *http.Request, user meta.User) {
db := r.URL.Query().Get(":db")
rp := r.URL.Query().Get(":rp")
id := fmt.Sprintf("%s/%s", db, rp)

id := r.URL.Query().Get(":dbrp")
db, rp, ok := h.checkDbRp(w, id)
if !ok {
return
}
if h.Config.AuthEnabled {
if user == nil {
h.httpError(w, fmt.Sprintf("delete bucket - user is required to delete from database %q", db), http.StatusForbidden)
Expand Down Expand Up @@ -1215,17 +1240,19 @@ func (h *Handler) serveDeleteBucketV2(w http.ResponseWriter, r *http.Request, us

func (h *Handler) serveUpdateBucketV2(w http.ResponseWriter, r *http.Request, user meta.User) {
var bs []byte
db := r.URL.Query().Get(":db")
rp := r.URL.Query().Get(":rp")
bucket := fmt.Sprintf("%s/%s", db, rp)
id := r.URL.Query().Get(":dbrp")
db, rp, ok := h.checkDbRp(w, id)
if !ok {
return
}
if h.Config.AuthEnabled {
if user == nil {
h.httpError(w, fmt.Sprintf("update bucket - user is required to update %q", bucket), http.StatusForbidden)
h.httpError(w, fmt.Sprintf("update bucket - user is required to update %q", id), http.StatusForbidden)
return
}
// This is the privilege required in the Enterprise authorization.
if err := h.QueryAuthorizer.AuthorizeCreateDatabase(user); err != nil {
h.httpError(w, fmt.Sprintf("update bucket - %q is not authorized to modify %q: %s", user.ID(), bucket, err.Error()), http.StatusForbidden)
h.httpError(w, fmt.Sprintf("update bucket - %q is not authorized to modify %q: %s", user.ID(), id, err.Error()), http.StatusForbidden)
return
}
}
Expand Down Expand Up @@ -1262,23 +1289,40 @@ func (h *Handler) serveUpdateBucketV2(w http.ResponseWriter, r *http.Request, us
}
rf := meta.DefaultRetentionPolicyReplicaN
m := &meta.RetentionPolicyUpdate{
Name: &bu.Name,
Name: &rp,
Duration: &dur,
ReplicaN: &rf,
ShardGroupDuration: &sgDur,
}

if err := h.MetaClient.UpdateRetentionPolicy(db, rp, m, false); err != nil {
h.httpError(w, fmt.Sprintf("update bucket %q: %s", bucket, err.Error()), http.StatusBadRequest)
h.httpError(w, fmt.Sprintf("update bucket %q: %s", id, err.Error()), http.StatusBadRequest)
return
}
rpi := &meta.RetentionPolicyInfo{
Name: *m.Name,
ReplicaN: *m.ReplicaN,
Duration: *m.Duration,
ShardGroupDuration: *m.ShardGroupDuration,
}
bucket := makeBucket(rpi, db)
b, err := json.Marshal(bucket)
if err != nil {
h.httpError(w, fmt.Sprintf("buckets - cannot marshal bucket %q: %s", bucket.Name, err.Error()), http.StatusBadRequest)
return
}
w.WriteHeader(http.StatusOK)
w.Write(b)
}

func (h *Handler) serveRetrieveBucketV2(w http.ResponseWriter, r *http.Request, user meta.User) {
// This is the API for returning a single bucket
// In V1, BucketID and BucketName are the same: "db/rp"
db := r.URL.Query().Get(":db")
rp := r.URL.Query().Get(":rp")
id := r.URL.Query().Get(":dbrp")
db, rp, ok := h.checkDbRp(w, id)
if !ok {
return
}
if h.Config.AuthEnabled {
if user == nil {
h.httpError(w, fmt.Sprintf("retrieve bucket - user is required for database %q", db), http.StatusForbidden)
Expand Down
49 changes: 27 additions & 22 deletions services/httpd/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1891,7 +1891,7 @@ func TestHandler_CreateDeleteBuckets(t *testing.T) {
errMsg: `buckets - retention policy "/": invalid name`,
},
{
url: "/api/v2/buckets/" + existingDb + "/" + goodRp,
url: "/api/v2/buckets/" + existingDb + "%2f" + goodRp,
method: patchMethod,
body: httpd.BucketsBody{
BucketUpdate: httpd.BucketUpdate{
Expand All @@ -1912,7 +1912,7 @@ func TestHandler_CreateDeleteBuckets(t *testing.T) {
status: http.StatusNotFound,
},
{
url: "/api/v2/buckets/baddb/" + goodRp,
url: "/api/v2/buckets/baddb%2f" + goodRp,
method: deleteMethod,
status: http.StatusNotFound,
errMsg: `delete bucket - not found: "baddb/myrp"`,
Expand All @@ -1933,10 +1933,10 @@ func TestHandler_CreateDeleteBuckets(t *testing.T) {
Rp: goodRp,
SchemaType: "implicit",
},
status: http.StatusOK,
status: http.StatusCreated,
},
{
url: "/api/v2/buckets/" + existingDb + "/badrp",
url: "/api/v2/buckets/" + existingDb + "%2fbadrp",
method: deleteMethod,
status: http.StatusNotFound,
errMsg: `delete bucket - not found: "mydb/badrp"`,
Expand All @@ -1957,10 +1957,10 @@ func TestHandler_CreateDeleteBuckets(t *testing.T) {
Rp: goodRp,
SchemaType: "implicit",
},
status: http.StatusOK,
status: http.StatusCreated,
},
{
url: "/api/v2/buckets/" + existingDb + "/" + goodRp,
url: "/api/v2/buckets/" + existingDb + "%2f" + goodRp,
method: deleteMethod,
status: http.StatusOK,
},
Expand Down Expand Up @@ -2247,22 +2247,27 @@ func TestHandler_RetrieveBucket(t *testing.T) {
status: http.StatusNotFound,
},
{
url: "/api/v2/buckets//rpTwo_1",
status: http.StatusNotFound,
url: "/api/v2/buckets/%2frpTwo_1",
status: http.StatusBadRequest,
errMsg: `bucket "/rpTwo_1": bucket name "/rpTwo_1" is in db/rp form but has an empty database`,
},
{
url: "/api/v2/buckets/dbFive/rpTwo_1",
url: "/api/v2/buckets/dbOne%2f",
status: http.StatusBadRequest,
errMsg: `bucket "dbOne/": illegal bucket id, empty retention policy`,
},
{
url: "/api/v2/buckets/dbFive%2frpTwo_1",
status: http.StatusNotFound,
errMsg: `bucket not found: "dbFive/rpTwo_1"`,
},
{
url: "/api/v2/buckets/dbOne/rpTwo_1",
url: "/api/v2/buckets/dbOne%2frpTwo_1",
status: http.StatusOK,
exp: "dbOne/rpTwo_1",
},
{
url: "/api/v2/buckets/dbOne/rpOne_2",
url: "/api/v2/buckets/dbOne%2frpOne_2",
status: http.StatusNotFound,
errMsg: `bucket not found: "dbOne/rpOne_2"`,
},
Expand Down Expand Up @@ -2291,11 +2296,11 @@ func TestHandler_RetrieveBucket(t *testing.T) {
h.ServeHTTP(w, req)
var errMsg string
if w.Code != ct.status {
t.Fatalf("error, expected %d got %d: %s", ct.status, w.Code, errMsg)
t.Fatalf("error, test %s: expected %d got %d: %s", ct.url, ct.status, w.Code, errMsg)
} else if w.Code != http.StatusOK {
errMsg = w.Header().Get("X-InfluxDB-Error")
if errMsg != ct.errMsg {
t.Fatalf("incorrect error message, expected: %q, got: %q", ct.errMsg, errMsg)
t.Fatalf("incorrect error message, test %s: expected: %q, got: %q", ct.url, ct.errMsg, errMsg)
}
} else {
var got httpd.Bucket
Expand All @@ -2322,53 +2327,53 @@ func TestHandler_UnsupportedV2API(t *testing.T) {
tests := []*test{
{
method: "GET",
url: "/api/v2/buckets/mydb/myrp/labels",
url: "/api/v2/buckets/mydb%2fmyrp/labels",
status: http.StatusMethodNotAllowed,
errMsg: "bucket labels not supported in this version"},
{
method: "POST",
url: "/api/v2/buckets/mydb/myrp/labels",
url: "/api/v2/buckets/mydb%2fmyrp/labels",
status: http.StatusMethodNotAllowed,
errMsg: "bucket labels not supported in this version",
},
{
method: "DELETE",
url: "/api/v2/buckets/mydb/myrp/labels/mylabel",
url: "/api/v2/buckets/mydb%2fmyrp/labels/mylabel",
status: http.StatusMethodNotAllowed,
errMsg: "bucket labels not supported in this version"},
{
method: "GET",
url: "/api/v2/buckets/mydb/myrp/members",
url: "/api/v2/buckets/mydb%2fmyrp/members",
status: http.StatusMethodNotAllowed,
errMsg: "bucket members not supported in this version",
},
{
method: "POST",
url: "/api/v2/buckets/mydb/myrp/members",
url: "/api/v2/buckets/mydb%2fmyrp/members",
status: http.StatusMethodNotAllowed,
errMsg: "bucket members not supported in this version",
},
{
method: "DELETE",
url: "/api/v2/buckets/mydb/myrp/members/amember",
url: "/api/v2/buckets/mydb%2fmyrp/members/amember",
status: http.StatusMethodNotAllowed,
errMsg: "bucket members not supported in this version",
},
{
method: "GET",
url: "/api/v2/buckets/mydb/myrp/owners",
url: "/api/v2/buckets/mydb%2fmyrp/owners",
status: http.StatusMethodNotAllowed,
errMsg: "bucket owners not supported in this version",
},
{
method: "POST",
url: "/api/v2/buckets/mydb/myrp/owners",
url: "/api/v2/buckets/mydb%2fmyrp/owners",
status: http.StatusMethodNotAllowed,
errMsg: "bucket owners not supported in this version",
},
{
method: "DELETE",
url: "/api/v2/buckets/mydb/myrp/owners/anowner",
url: "/api/v2/buckets/mydb%2fmyrp/owners/anowner",
status: http.StatusMethodNotAllowed,
errMsg: "bucket owners not supported in this version",
},
Expand Down

0 comments on commit be9a3d4

Please sign in to comment.