From cb1a98dffafa8393be9961ef6b53bd6afbd4a9af Mon Sep 17 00:00:00 2001 From: Jonathan Gramain Date: Tue, 24 Sep 2024 10:12:47 -0700 Subject: [PATCH] ft: BKTCLT-21 implement CreateBucket(), GetBucketAttributes() CreateBucket takes two options: - one to specify the raft session ID (by simply adding the query string param 'raftsession=') - the other to detect if a bucket already exists with the same UID and then succeed silently (making the request idempotent). This is a convenience layer on top of the bucketd API. --- go/bucketclientrequest.go | 2 +- go/bucketclientrequest_test.go | 2 +- go/createbucket.go | 109 ++++++++++++++++++++++++++++++ go/createbucket_test.go | 118 +++++++++++++++++++++++++++++++++ go/getbucketattributes.go | 13 ++++ go/getbucketattributes_test.go | 33 +++++++++ 6 files changed, 275 insertions(+), 2 deletions(-) create mode 100644 go/createbucket.go create mode 100644 go/createbucket_test.go create mode 100644 go/getbucketattributes.go create mode 100644 go/getbucketattributes_test.go diff --git a/go/bucketclientrequest.go b/go/bucketclientrequest.go index d0f8604..0d43426 100644 --- a/go/bucketclientrequest.go +++ b/go/bucketclientrequest.go @@ -29,7 +29,7 @@ func RequestBodyContentTypeOption(contentType string) RequestOption { } } -func Idempotent(ros *requestOptionSet) { +func RequestIdempotent(ros *requestOptionSet) { ros.idempotent = true } diff --git a/go/bucketclientrequest_test.go b/go/bucketclientrequest_test.go index ba827da..6de3f56 100644 --- a/go/bucketclientrequest_test.go +++ b/go/bucketclientrequest_test.go @@ -89,7 +89,7 @@ var _ = Describe("BucketClient.Request()", func() { "PostIdempotent", "POST", "/idempotent/url", bucketclient.RequestBodyContentTypeOption("text/plain"), bucketclient.RequestBodyOption([]byte("post body")), - bucketclient.Idempotent, + bucketclient.RequestIdempotent, )).To(Equal([]byte("got it"))) }) It("fails with a 400 response on GET request", func(ctx SpecContext) { diff --git a/go/createbucket.go b/go/createbucket.go new file mode 100644 index 0000000..e2953d0 --- /dev/null +++ b/go/createbucket.go @@ -0,0 +1,109 @@ +package bucketclient + +import ( + "context" + "encoding/json" + "fmt" + "net/http" +) + +type createBucketOptionSet struct { + sessionId int + makeIdempotent bool +} + +type CreateBucketOption func(*createBucketOptionSet) + +func CreateBucketSessionIdOption(sessionId int) CreateBucketOption { + return func(options *createBucketOptionSet) { + options.sessionId = sessionId + } +} + +func CreateBucketMakeIdempotent(options *createBucketOptionSet) { + options.makeIdempotent = true +} + +// CreateBucket creates a bucket in metadata. +// bucketAttributes is a JSON blob of bucket attributes +// opts is a set of options: +// +// CreateBucketSessionIdOption forces the session ID where the bucket to be +// created will land +// +// CreateBucketMakeIdempotent makes the request return a success if a bucket +// with the same UID already exists (otherwise returns 409 Conflict, as +// if the option is not passed) +func (client *BucketClient) CreateBucket(ctx context.Context, + bucketName string, bucketAttributes []byte, opts ...CreateBucketOption) error { + parsedOpts := createBucketOptionSet{ + sessionId: 0, + makeIdempotent: false, + } + for _, opt := range opts { + opt(&parsedOpts) + } + resource := fmt.Sprintf("/default/bucket/%s", bucketName) + if parsedOpts.sessionId > 0 { + resource += fmt.Sprintf("?raftsession=%d", parsedOpts.sessionId) + } + requestOptions := []RequestOption{ + RequestBodyOption(bucketAttributes), + RequestBodyContentTypeOption("application/json"), + } + if parsedOpts.makeIdempotent { + // since we will make the request idempotent, it's + // okay to retry it (it may return 409 Conflict at the + // first retry if it initially succeeded, but it will + // then be considered a success) + requestOptions = append(requestOptions, RequestIdempotent) + } + _, err := client.Request(ctx, "CreateBucket", "POST", resource, requestOptions...) + if err == nil { + return nil + } + if parsedOpts.makeIdempotent { + // If the Idempotent option is set, Accept "409 Conflict" as a success iff + // the UIDs match between the existing and the new metadata, to detect and + // return an error if there is an existing bucket that was not created by us + + bcErr := err.(*BucketClientError) + if bcErr.StatusCode != http.StatusConflict { + return err + } + existingBucketAttributes, err := client.GetBucketAttributes(ctx, bucketName) + if err != nil { + return err + } + attributesMatch, cmpErr := compareBucketAttributesUIDs( + bucketAttributes, existingBucketAttributes) + if cmpErr != nil { + // FIXME return BucketClientError + return cmpErr + } + if attributesMatch { + // return silent success without updating the existing metadata + return nil + } + } + return err +} + +func compareBucketAttributesUIDs(attributes1 []byte, attributes2 []byte) (bool, error) { + var parsedAttr1, parsedAttr2 map[string]interface{} + + err := json.Unmarshal(attributes1, &parsedAttr1) + if err != nil { + return false, fmt.Errorf("json.Unmarshal failed: %w", err) + } + err = json.Unmarshal(attributes2, &parsedAttr2) + if err != nil { + return false, fmt.Errorf("json.Unmarshal failed: %w", err) + } + uid1, ok1 := parsedAttr1["uid"] + uid2, ok2 := parsedAttr2["uid"] + if !ok1 || !ok2 { + return false, nil + } + return uid1 == uid2, nil +} diff --git a/go/createbucket_test.go b/go/createbucket_test.go new file mode 100644 index 0000000..a67f6a1 --- /dev/null +++ b/go/createbucket_test.go @@ -0,0 +1,118 @@ +package bucketclient_test + +import ( + "io" + "net/http" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/jarcoal/httpmock" + + "github.com/scality/bucketclient/go" +) + +var _ = Describe("CreateBucket()", func() { + It("creates a bucket on an available raft session", func(ctx SpecContext) { + httpmock.RegisterResponder( + "POST", "/default/bucket/my-new-bucket", + func(req *http.Request) (*http.Response, error) { + defer req.Body.Close() + Expect(io.ReadAll(req.Body)).To(Equal([]byte(`{"foo":"bar"}`))) + return httpmock.NewStringResponse(200, ""), nil + }, + ) + Expect(client.CreateBucket(ctx, "my-new-bucket", []byte(`{"foo":"bar"}`))).To(Succeed()) + }) + + It("creates a bucket on a chosen raft session", func(ctx SpecContext) { + httpmock.RegisterResponder( + "POST", "/default/bucket/my-new-bucket?raftsession=12", + func(req *http.Request) (*http.Response, error) { + defer req.Body.Close() + Expect(io.ReadAll(req.Body)).To(Equal([]byte(`{"foo":"bar"}`))) + return httpmock.NewStringResponse(200, ""), nil + }, + ) + Expect(client.CreateBucket(ctx, "my-new-bucket", []byte(`{"foo":"bar"}`), + bucketclient.CreateBucketSessionIdOption(12))).To(Succeed()) + }) + + It("forwards request error", func(ctx SpecContext) { + httpmock.RegisterResponder( + "POST", "/default/bucket/my-new-bucket", + httpmock.NewStringResponder(http.StatusInternalServerError, "I'm afraid I can't do this"), + ) + err := client.CreateBucket(ctx, "my-new-bucket", []byte(`{"foo":"bar"}`), + bucketclient.CreateBucketMakeIdempotent) + Expect(err).To(HaveOccurred()) + bcErr, ok := err.(*bucketclient.BucketClientError) + Expect(ok).To(BeTrue()) + Expect(bcErr.StatusCode).To(Equal(http.StatusInternalServerError)) + }) + + It("returns 409 Conflict without MakeIdempotent option if bucket with same UID exists", func(ctx SpecContext) { + httpmock.RegisterResponder( + "POST", "/default/bucket/my-new-bucket", + httpmock.NewStringResponder(http.StatusConflict, ""), + ) + // normally unused, but set to match the following tests + httpmock.RegisterResponder( + "GET", "/default/attributes/my-new-bucket", + httpmock.NewStringResponder(200, `{"foo":"bar","uid":"4242"}`), + ) + err := client.CreateBucket(ctx, "my-new-bucket", []byte(`{"foo":"bar","uid":"4242"}`)) + Expect(err).To(HaveOccurred()) + bcErr, ok := err.(*bucketclient.BucketClientError) + Expect(ok).To(BeTrue()) + Expect(bcErr.StatusCode).To(Equal(http.StatusConflict)) + }) + + It("succeeds to create bucket with MakeIdempotent option if bucket with same UID exists", func(ctx SpecContext) { + httpmock.RegisterResponder( + "POST", "/default/bucket/my-new-bucket", + httpmock.NewStringResponder(http.StatusConflict, ""), + ) + httpmock.RegisterResponder( + "GET", "/default/attributes/my-new-bucket", + httpmock.NewStringResponder(200, `{"foo":"bar","uid":"4242"}`), + ) + Expect(client.CreateBucket(ctx, "my-new-bucket", []byte(`{"foo":"bar","uid":"4242"}`), + bucketclient.CreateBucketMakeIdempotent)).To(Succeed()) + }) + + It("returns 409 Conflict with MakeIdempotent option if bucket with different UID exists", func(ctx SpecContext) { + httpmock.RegisterResponder( + "POST", "/default/bucket/my-new-bucket", + httpmock.NewStringResponder(http.StatusConflict, ""), + ) + httpmock.RegisterResponder( + "GET", "/default/attributes/my-new-bucket", + httpmock.NewStringResponder(200, `{"foo":"bar","uid":"OLDUID"}`), + ) + err := client.CreateBucket(ctx, "my-new-bucket", []byte(`{"foo":"bar","uid":"NEWUID"}`), + bucketclient.CreateBucketMakeIdempotent) + Expect(err).To(HaveOccurred()) + bcErr, ok := err.(*bucketclient.BucketClientError) + Expect(ok).To(BeTrue()) + Expect(bcErr.StatusCode).To(Equal(http.StatusConflict)) + }) + + It("returns 409 Conflict with MakeIdempotent option if bucket exists without an \"uid\" attribute", func(ctx SpecContext) { + httpmock.RegisterResponder( + "POST", "/default/bucket/my-new-bucket", + httpmock.NewStringResponder(http.StatusConflict, ""), + ) + httpmock.RegisterResponder( + "GET", "/default/attributes/my-new-bucket", + httpmock.NewStringResponder(200, `{"foo":"bar"}`), + ) + err := client.CreateBucket(ctx, "my-new-bucket", []byte(`{"foo":"bar"}`), + bucketclient.CreateBucketMakeIdempotent) + Expect(err).To(HaveOccurred()) + bcErr, ok := err.(*bucketclient.BucketClientError) + Expect(ok).To(BeTrue()) + Expect(bcErr.StatusCode).To(Equal(http.StatusConflict)) + }) + +}) diff --git a/go/getbucketattributes.go b/go/getbucketattributes.go new file mode 100644 index 0000000..58ace16 --- /dev/null +++ b/go/getbucketattributes.go @@ -0,0 +1,13 @@ +package bucketclient + +import ( + "context" + "fmt" +) + +// GetBucketAttributes retrieves the JSON blob containing the bucket +// attributes attached to a bucket. +func (client *BucketClient) GetBucketAttributes(ctx context.Context, bucketName string) ([]byte, error) { + resource := fmt.Sprintf("/default/attributes/%s", bucketName) + return client.Request(ctx, "GetBucketAttributes", "GET", resource) +} diff --git a/go/getbucketattributes_test.go b/go/getbucketattributes_test.go new file mode 100644 index 0000000..4b49a94 --- /dev/null +++ b/go/getbucketattributes_test.go @@ -0,0 +1,33 @@ +package bucketclient_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/jarcoal/httpmock" + + "github.com/scality/bucketclient/go" +) + +var _ = Describe("GetBucketAttributes()", func() { + It("retrieves the bucket attributes of an existing bucket", func(ctx SpecContext) { + httpmock.RegisterResponder( + "GET", "/default/attributes/my-bucket", + httpmock.NewStringResponder(200, `{"foo":"bar"}`), + ) + Expect(client.GetBucketAttributes(ctx, "my-bucket")).To( + Equal([]byte(`{"foo":"bar"}`))) + }) + + It("returns a 404 error if the bucket does not exist", func(ctx SpecContext) { + httpmock.RegisterResponder( + "GET", "/default/attributes/my-bucket", + httpmock.NewStringResponder(404, ""), + ) + _, err := client.GetBucketAttributes(ctx, "my-bucket") + Expect(err).To(HaveOccurred()) + bcErr, ok := err.(*bucketclient.BucketClientError) + Expect(ok).To(BeTrue()) + Expect(bcErr.StatusCode).To(Equal(404)) + }) +})