diff --git a/docs/api-status.json b/docs/api-status.json index 6a12eb8cc..1733091b1 100644 --- a/docs/api-status.json +++ b/docs/api-status.json @@ -1776,6 +1776,24 @@ "comment": "LinkBucket will link a bucket to a specified user\nunlinking the bucket from any previous user\n PREVIEW\n", "added_in_version": "v0.15.0", "expected_stable_version": "v0.17.0" + }, + { + "name": "API.CreateSubuser", + "comment": "CreateSubuser - https://docs.ceph.com/en/latest/radosgw/adminops/#create-subuser\n PREVIEW\n", + "added_in_version": "v0.15.0", + "expected_stable_version": "v0.17.0" + }, + { + "name": "API.RemoveSubuser", + "comment": "RemoveSubuser - https://docs.ceph.com/en/latest/radosgw/adminops/#remove-subuser\n PREVIEW\n", + "added_in_version": "v0.15.0", + "expected_stable_version": "v0.17.0" + }, + { + "name": "API.ModifySubuser", + "comment": "ModifySubuser - https://docs.ceph.com/en/latest/radosgw/adminops/#modify-subuser\n PREVIEW\n", + "added_in_version": "v0.15.0", + "expected_stable_version": "v0.17.0" } ], "stable_api": [ diff --git a/docs/api-status.md b/docs/api-status.md index c72ceb6a1..11578ddf4 100644 --- a/docs/api-status.md +++ b/docs/api-status.md @@ -59,6 +59,9 @@ Name | Added in Version | Expected Stable Version | ---- | ---------------- | ----------------------- | API.UnlinkBucket | v0.15.0 | v0.17.0 | API.LinkBucket | v0.15.0 | v0.17.0 | +API.CreateSubuser | v0.15.0 | v0.17.0 | +API.RemoveSubuser | v0.15.0 | v0.17.0 | +API.ModifySubuser | v0.15.0 | v0.17.0 | ## Package: common/admin/manager diff --git a/rgw/admin/bucket.go b/rgw/admin/bucket.go index 9fe6fb990..f8125d899 100644 --- a/rgw/admin/bucket.go +++ b/rgw/admin/bucket.go @@ -98,7 +98,7 @@ func (api *API) ListBuckets(ctx context.Context) ([]string, error) { // GetBucketInfo will return various information about a specific token func (api *API) GetBucketInfo(ctx context.Context, bucket Bucket) (Bucket, error) { - body, err := api.call(ctx, http.MethodGet, "/bucket", valueToURLParams(bucket)) + body, err := api.call(ctx, http.MethodGet, "/bucket", valueToURLParams(bucket, []string{"bucket", "uid", "stats"})) if err != nil { return Bucket{}, err } @@ -116,7 +116,9 @@ func (api *API) GetBucketInfo(ctx context.Context, bucket Bucket) (Bucket, error func (api *API) GetBucketPolicy(ctx context.Context, bucket Bucket) (Policy, error) { policy := true bucket.Policy = &policy - body, err := api.call(ctx, http.MethodGet, "/bucket", valueToURLParams(bucket)) + + // valid parameters not supported by go-ceph: object + body, err := api.call(ctx, http.MethodGet, "/bucket", valueToURLParams(bucket, []string{"bucket"})) if err != nil { return Policy{}, err } @@ -132,7 +134,7 @@ func (api *API) GetBucketPolicy(ctx context.Context, bucket Bucket) (Policy, err // RemoveBucket will remove a given token from the object store func (api *API) RemoveBucket(ctx context.Context, bucket Bucket) error { - _, err := api.call(ctx, http.MethodDelete, "/bucket", valueToURLParams(bucket)) + _, err := api.call(ctx, http.MethodDelete, "/bucket", valueToURLParams(bucket, []string{"bucket", "purge-objects"})) if err != nil { return err } diff --git a/rgw/admin/caps.go b/rgw/admin/caps.go index 847ebb47b..a231c1bf5 100644 --- a/rgw/admin/caps.go +++ b/rgw/admin/caps.go @@ -19,7 +19,7 @@ func (api *API) AddUserCap(ctx context.Context, uid, userCap string) ([]UserCapS } user := User{ID: uid, UserCaps: userCap} - body, err := api.call(ctx, http.MethodPut, "/user?caps", valueToURLParams(user)) + body, err := api.call(ctx, http.MethodPut, "/user?caps", valueToURLParams(user, []string{"uid", "user-caps"})) if err != nil { return nil, err } @@ -45,7 +45,7 @@ func (api *API) RemoveUserCap(ctx context.Context, uid, userCap string) ([]UserC } user := User{ID: uid, UserCaps: userCap} - body, err := api.call(ctx, http.MethodDelete, "/user?caps", valueToURLParams(user)) + body, err := api.call(ctx, http.MethodDelete, "/user?caps", valueToURLParams(user, []string{"uid", "user-caps"})) if err != nil { return nil, err } diff --git a/rgw/admin/errors.go b/rgw/admin/errors.go index ce9161a0f..7ce3c644c 100644 --- a/rgw/admin/errors.go +++ b/rgw/admin/errors.go @@ -87,6 +87,7 @@ const ( var ( errMissingUserID = errors.New("missing user ID") + errMissingSubuserID = errors.New("missing subuser ID") errMissingUserAccessKey = errors.New("missing user access key") errMissingUserDisplayName = errors.New("missing user display name") errMissingUserCap = errors.New("missing user capabilities") diff --git a/rgw/admin/link.go b/rgw/admin/link.go index c88f14e77..3a3e57eee 100644 --- a/rgw/admin/link.go +++ b/rgw/admin/link.go @@ -25,7 +25,7 @@ func (api *API) UnlinkBucket(ctx context.Context, link BucketLinkInput) error { if link.Bucket == "" { return errMissingBucket } - _, err := api.call(ctx, http.MethodPost, "/bucket", valueToURLParams(link)) + _, err := api.call(ctx, http.MethodPost, "/bucket", valueToURLParams(link, []string{"uid", "bucket"})) return err } @@ -39,6 +39,7 @@ func (api *API) LinkBucket(ctx context.Context, link BucketLinkInput) error { if link.Bucket == "" { return errMissingBucket } - _, err := api.call(ctx, http.MethodPut, "/bucket", valueToURLParams(link)) + // valid parameters not supported by go-ceph: new-bucket-name + _, err := api.call(ctx, http.MethodPut, "/bucket", valueToURLParams(link, []string{"uid", "bucket-id", "bucket"})) return err } diff --git a/rgw/admin/quota.go b/rgw/admin/quota.go index 912c43a67..5ebac8617 100644 --- a/rgw/admin/quota.go +++ b/rgw/admin/quota.go @@ -28,7 +28,7 @@ func (api *API) GetUserQuota(ctx context.Context, quota QuotaSpec) (QuotaSpec, e return QuotaSpec{}, errMissingUserID } - body, err := api.call(ctx, http.MethodGet, "/user?quota", valueToURLParams(quota)) + body, err := api.call(ctx, http.MethodGet, "/user?quota", valueToURLParams(quota, []string{"uid", "quota-type"})) if err != nil { return QuotaSpec{}, err } @@ -53,7 +53,7 @@ func (api *API) SetUserQuota(ctx context.Context, quota QuotaSpec) error { return errMissingUserID } - _, err := api.call(ctx, http.MethodPut, "/user?quota", valueToURLParams(quota)) + _, err := api.call(ctx, http.MethodPut, "/user?quota", valueToURLParams(quota, []string{"uid", "quota-type", "enabled", "max-size", "max-size-kb", "max-objects"})) if err != nil { return err } diff --git a/rgw/admin/subuser.go b/rgw/admin/subuser.go new file mode 100644 index 000000000..32d5cef24 --- /dev/null +++ b/rgw/admin/subuser.go @@ -0,0 +1,107 @@ +//go:build ceph_preview +// +build ceph_preview + +package admin + +import ( + "context" + "fmt" + "net/http" +) + +// validateSubuserAcess - Return whether the given subuser access value is valid as input parameter +func (s SubuserSpec) validateSubuserAccess() bool { + a := s.Access + return a == "" || + a == SubuserAccessRead || + a == SubuserAccessWrite || + a == SubuserAccessReadWrite || + a == SubuserAccessFull +} + +func makeInvalidSubuserAccessLevelError(spec SubuserSpec) error { + return fmt.Errorf("invalid subuser access level %q", spec.Access) +} + +// The following are the subuser API functions. +// +// We need to explain the omission of ?subuser in the API path common +// to all three functions. +// +// According to the docs, this has to be included to select the +// subuser operation, but we already have subuser as a parameter with +// a value (and make sure it's not empty and thus included by +// validating the SubuserSpec). The presence of this parameter +// triggers the subuser operation. +// +// If we add the subuser with the empty value the API call fails as +// having an invalid signature (and it is semantically wrong as we +// then have *two* values for the subuser name, an empty one an the +// relevant one, the upstream code does not seem to handle that case +// gracefully). + +// CreateSubuser - https://docs.ceph.com/en/latest/radosgw/adminops/#create-subuser +// PREVIEW +func (api *API) CreateSubuser(ctx context.Context, user User, subuser SubuserSpec) error { + if user.ID == "" { + return errMissingUserID + } + if subuser.Name == "" { + return errMissingSubuserID + } + if !subuser.validateSubuserAccess() { + return makeInvalidSubuserAccessLevelError(subuser) + } + // valid parameters not supported by go-ceph: access-key, gen-access-key + v := valueToURLParams(user, []string{"uid"}) + addToURLParams(&v, subuser, []string{"subuser", "access", "secret-key", "generate-secret", "key-type"}) + _, err := api.call(ctx, http.MethodPut, "/user", v) + if err != nil { + return err + } + + return nil +} + +// RemoveSubuser - https://docs.ceph.com/en/latest/radosgw/adminops/#remove-subuser +// PREVIEW +func (api *API) RemoveSubuser(ctx context.Context, user User, subuser SubuserSpec) error { + if user.ID == "" { + return errMissingUserID + } + if subuser.Name == "" { + return errMissingSubuserID + } + + v := valueToURLParams(user, []string{"uid"}) + addToURLParams(&v, subuser, []string{"subuser", "purge-keys"}) + _, err := api.call(ctx, http.MethodDelete, "/user", v) + if err != nil { + return err + } + + return nil +} + +// ModifySubuser - https://docs.ceph.com/en/latest/radosgw/adminops/#modify-subuser +// PREVIEW +func (api *API) ModifySubuser(ctx context.Context, user User, subuser SubuserSpec) error { + if user.ID == "" { + return errMissingUserID + } + if subuser.Name == "" { + return errMissingSubuserID + } + if !subuser.validateSubuserAccess() { + return makeInvalidSubuserAccessLevelError(subuser) + } + + v := valueToURLParams(user, []string{"uid"}) + addToURLParams(&v, subuser, []string{"subuser", "access", "secret", "generate-secret", "key-type"}) + _, err := api.call(ctx, http.MethodPost, "/user", v) + if err != nil { + return err + } + + return nil +} diff --git a/rgw/admin/subuser_test.go b/rgw/admin/subuser_test.go new file mode 100644 index 000000000..b023e15d3 --- /dev/null +++ b/rgw/admin/subuser_test.go @@ -0,0 +1,72 @@ +package admin + +import ( + "bytes" + "context" + "fmt" + "io/ioutil" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestValidateSubuserAccess(t *testing.T) { + assert.True(t, SubuserSpec{Access: SubuserAccessNone}.validateSubuserAccess()) + assert.True(t, SubuserSpec{Access: SubuserAccessRead}.validateSubuserAccess()) + assert.True(t, SubuserSpec{Access: SubuserAccessWrite}.validateSubuserAccess()) + assert.True(t, SubuserSpec{Access: SubuserAccessReadWrite}.validateSubuserAccess()) + assert.True(t, SubuserSpec{Access: SubuserAccessFull}.validateSubuserAccess()) + assert.False(t, SubuserSpec{Access: SubuserAccessReplyFull}.validateSubuserAccess()) + assert.False(t, SubuserSpec{Access: SubuserAccess("bar")}.validateSubuserAccess()) +} + +func TestCreateSubuserMockAPI(t *testing.T) { + t.Run("test create subuser validation: neither is set", func(t *testing.T) { + api, err := New("127.0.0.1", "accessKey", "secretKey", returnMockClientCreateSubuser()) + assert.NoError(t, err) + err = api.CreateSubuser(context.TODO(), User{}, SubuserSpec{}) + assert.Equal(t, err, errMissingUserID) + }) + t.Run("test create subuser validation: no user ID", func(t *testing.T) { + api, err := New("127.0.0.1", "accessKey", "secretKey", returnMockClientCreateSubuser()) + assert.NoError(t, err) + err = api.CreateSubuser(context.TODO(), User{}, SubuserSpec{Name: "foo"}) + assert.Equal(t, err, errMissingUserID) + }) + t.Run("test create subuser validation: no subuser ID", func(t *testing.T) { + api, err := New("127.0.0.1", "accessKey", "secretKey", returnMockClientCreateSubuser()) + assert.NoError(t, err) + err = api.CreateSubuser(context.TODO(), User{ID: "dashboard-admin"}, SubuserSpec{}) + assert.Equal(t, err, errMissingSubuserID) + }) + t.Run("test create subuser validation: valid", func(t *testing.T) { + api, err := New("127.0.0.1", "accessKey", "secretKey", returnMockClientCreateSubuser()) + assert.NoError(t, err) + err = api.CreateSubuser(context.TODO(), User{ID: "dashboard-admin"}, SubuserSpec{Name: "foo"}) + assert.NoError(t, err) + }) + t.Run("test create subuser validation: invalid access", func(t *testing.T) { + api, err := New("127.0.0.1", "accessKey", "secretKey", returnMockClientCreateSubuser()) + assert.NoError(t, err) + err = api.CreateSubuser(context.TODO(), User{ID: "dashboard-admin"}, SubuserSpec{Name: "foo", Access: SubuserAccess("foo")}) + assert.Error(t, err) + assert.EqualError(t, err, `invalid subuser access level "foo"`) + }) +} + +// mockClient is defined in user_test.go +func returnMockClientCreateSubuser() *mockClient { + r := ioutil.NopCloser(bytes.NewReader([]byte(""))) + return &mockClient{ + mockDo: func(req *http.Request) (*http.Response, error) { + if req.Method == http.MethodPut && req.URL.Path == "127.0.0.1/admin/user" { + return &http.Response{ + StatusCode: 201, + Body: r, + }, nil + } + return nil, fmt.Errorf("unexpected request: %q. method %q. path %q", req.URL.RawQuery, req.Method, req.URL.Path) + }, + } +} diff --git a/rgw/admin/usage.go b/rgw/admin/usage.go index 8b33adf5e..d4ecbaa26 100644 --- a/rgw/admin/usage.go +++ b/rgw/admin/usage.go @@ -50,7 +50,8 @@ type Usage struct { // GetUsage request bandwidth usage information on the object store func (api *API) GetUsage(ctx context.Context, usage Usage) (Usage, error) { - body, err := api.call(ctx, http.MethodGet, "/usage", valueToURLParams(usage)) + // valid parameters not supported by go-ceph: category, bucket + body, err := api.call(ctx, http.MethodGet, "/usage", valueToURLParams(usage, []string{"uid", "start", "end", "show-entries", "show-summary"})) if err != nil { return Usage{}, err } @@ -65,6 +66,7 @@ func (api *API) GetUsage(ctx context.Context, usage Usage) (Usage, error) { // TrimUsage removes bandwidth usage information. With no dates specified, removes all usage information. func (api *API) TrimUsage(ctx context.Context, usage Usage) error { - _, err := api.call(ctx, http.MethodDelete, "/usage", valueToURLParams(usage)) + // valid parameters not supported by go-ceph: bucket + _, err := api.call(ctx, http.MethodDelete, "/usage", valueToURLParams(usage, []string{"uid", "start", "end", "remove-all"})) return err } diff --git a/rgw/admin/user.go b/rgw/admin/user.go index 9daf13fed..385075801 100644 --- a/rgw/admin/user.go +++ b/rgw/admin/user.go @@ -9,31 +9,75 @@ import ( // User is GO representation of the json output of a user creation type User struct { - ID string `json:"user_id" url:"uid"` - DisplayName string `json:"display_name" url:"display-name"` - Email string `json:"email" url:"email"` - Suspended *int `json:"suspended" url:"suspended"` - MaxBuckets *int `json:"max_buckets" url:"max-buckets"` - Subusers []interface{} `json:"subusers"` - Keys []UserKeySpec `json:"keys"` - SwiftKeys []interface{} `json:"swift_keys"` - Caps []UserCapSpec `json:"caps"` - OpMask string `json:"op_mask"` - DefaultPlacement string `json:"default_placement"` - DefaultStorageClass string `json:"default_storage_class"` - PlacementTags []interface{} `json:"placement_tags"` - BucketQuota QuotaSpec `json:"bucket_quota"` - UserQuota QuotaSpec `json:"user_quota"` - TempURLKeys []interface{} `json:"temp_url_keys"` - Type string `json:"type"` - MfaIds []interface{} `json:"mfa_ids"` - KeyType string `url:"key-type"` - Tenant string `url:"tenant"` - GenerateKey *bool `url:"generate-key"` - PurgeData *int `url:"purge-data"` - GenerateStat *bool `url:"stats"` - Stat UserStat `json:"stats"` - UserCaps string `url:"user-caps"` + ID string `json:"user_id" url:"uid"` + DisplayName string `json:"display_name" url:"display-name"` + Email string `json:"email" url:"email"` + Suspended *int `json:"suspended" url:"suspended"` + MaxBuckets *int `json:"max_buckets" url:"max-buckets"` + Subusers []SubuserSpec `json:"subusers" url:"-"` + Keys []UserKeySpec `json:"keys"` + SwiftKeys []SwiftKeySpec `json:"swift_keys" url:"-"` + Caps []UserCapSpec `json:"caps"` + OpMask string `json:"op_mask"` + DefaultPlacement string `json:"default_placement"` + DefaultStorageClass string `json:"default_storage_class"` + PlacementTags []interface{} `json:"placement_tags"` + BucketQuota QuotaSpec `json:"bucket_quota"` + UserQuota QuotaSpec `json:"user_quota"` + TempURLKeys []interface{} `json:"temp_url_keys"` + Type string `json:"type"` + MfaIds []interface{} `json:"mfa_ids"` + KeyType string `url:"key-type"` + Tenant string `url:"tenant"` + GenerateKey *bool `url:"generate-key"` + PurgeData *int `url:"purge-data"` + GenerateStat *bool `url:"stats"` + Stat UserStat `json:"stats"` + UserCaps string `url:"user-caps"` +} + +// SubuserSpec represents a subusers of a ceph-rgw user +type SubuserSpec struct { + Name string `json:"id" url:"subuser"` + Access SubuserAccess `json:"permissions" url:"access"` + + // these are always nil in answers, they are only relevant in requests + GenerateKey *bool `json:"-" url:"generate-key"` + SecretKey *string `json:"-" url:"secret-key"` + Secret *string `json:"-" url:"secret"` + PurgeKeys *bool `json:"-" url:"purge-keys"` + KeyType *string `json:"-" url:"key-type"` +} + +// SubuserAccess represents an access level for a subuser +type SubuserAccess string + +// The possible values of SubuserAccess +// +// There are two sets of constants as the API parameters and the +// values returned by the API do not match. The SubuserAccess* values +// must be used when setting access level, the SubuserAccessReply* +// values are the ones that may be returned. This is a design problem +// of the upstream API. We do not feel confident to do the mapping in +// the library. +const ( + SubuserAccessNone SubuserAccess = "" + SubuserAccessRead SubuserAccess = "read" + SubuserAccessWrite SubuserAccess = "write" + SubuserAccessReadWrite SubuserAccess = "readwrite" + SubuserAccessFull SubuserAccess = "full" + + SubuserAccessReplyNone SubuserAccess = "" + SubuserAccessReplyRead SubuserAccess = "read" + SubuserAccessReplyWrite SubuserAccess = "write" + SubuserAccessReplyReadWrite SubuserAccess = "read-write" + SubuserAccessReplyFull SubuserAccess = "full-control" +) + +// SwiftKeySpec represents the secret key associated to a subuser +type SwiftKeySpec struct { + User string `json:"user"` + SecretKey string `json:"secret_key"` } // UserCapSpec represents a user capability which gives access to certain ressources @@ -69,7 +113,9 @@ func (api *API) GetUser(ctx context.Context, user User) (User, error) { } } - body, err := api.call(ctx, http.MethodGet, "/user", valueToURLParams(user)) + // valid parameters not supported by go-ceph: sync + body, err := api.call(ctx, http.MethodGet, "/user", valueToURLParams(user, []string{"uid", "access-key", "stats"})) + if err != nil { return User{}, err } @@ -107,8 +153,8 @@ func (api *API) CreateUser(ctx context.Context, user User) (User, error) { return User{}, errMissingUserDisplayName } - // Send request - body, err := api.call(ctx, http.MethodPut, "/user", valueToURLParams(user)) + // valid parameters not supported by go-ceph: system, exclusive, default-placement, placement-tags + body, err := api.call(ctx, http.MethodPut, "/user", valueToURLParams(user, []string{"uid", "display-name", "email", "key-type", "access-key", "secret-key", "user-caps", "tenant", "generate-key", "max-buckets", "suspended", "op-mask"})) if err != nil { return User{}, err } @@ -129,7 +175,7 @@ func (api *API) RemoveUser(ctx context.Context, user User) error { return errMissingUserID } - _, err := api.call(ctx, http.MethodDelete, "/user", valueToURLParams(user)) + _, err := api.call(ctx, http.MethodDelete, "/user", valueToURLParams(user, []string{"uid", "purge-data"})) if err != nil { return err } @@ -143,7 +189,8 @@ func (api *API) ModifyUser(ctx context.Context, user User) (User, error) { return User{}, errMissingUserID } - body, err := api.call(ctx, http.MethodPost, "/user", valueToURLParams(user)) + // valid parameters not supported by go-ceph: system, default-placement, placement-tags + body, err := api.call(ctx, http.MethodPost, "/user", valueToURLParams(user, []string{"uid", "display-name", "email", "generate-key", "access-key", "secret-key", "key-type", "max-buckets", "suspended", "op-mask"})) if err != nil { return User{}, err } diff --git a/rgw/admin/user_test.go b/rgw/admin/user_test.go index be83497c7..0165cd202 100644 --- a/rgw/admin/user_test.go +++ b/rgw/admin/user_test.go @@ -36,7 +36,12 @@ var ( "email": "", "suspended": 0, "max_buckets": 1000, - "subusers": [], + "subusers": [ + { + "id": "dashboard-admin:swift", + "permissions": "read" + } + ], "keys": [ { "user": "dashboard-admin", @@ -44,7 +49,12 @@ var ( "secret_key": "YSaT5bEcJTjBJCDG5yvr2NhGQ9xzoTIg8B1gQHa3" } ], - "swift_keys": [], + "swift_keys": [ + { + "user": "dashboard-admin:swift", + "secret_key": "VERY_SECRET" + } + ], "caps": [], "op_mask": "read, write, delete", "system": "true", @@ -164,6 +174,42 @@ func (suite *RadosGWTestSuite) TestUser() { assert.NotNil(suite.T(), user.Stat.Size) }) + suite.T().Run("create a subuser", func(t *testing.T) { + err := co.CreateSubuser(context.Background(), User{ID: "leseb"}, SubuserSpec{Name: "foo", Access: SubuserAccessReadWrite}) + assert.NoError(suite.T(), err) + + user, err := co.GetUser(context.Background(), User{ID: "leseb"}) + assert.NoError(suite.T(), err) + if err == nil { + assert.Equal(suite.T(), user.Subusers[0].Name, "leseb:foo") + // Note: the returned values are not equal to the input values ... + assert.Equal(suite.T(), user.Subusers[0].Access, SubuserAccess("read-write")) + } + }) + + suite.T().Run("modify a subuser", func(t *testing.T) { + err := co.ModifySubuser(context.Background(), User{ID: "leseb"}, SubuserSpec{Name: "foo", Access: SubuserAccessRead}) + assert.NoError(suite.T(), err) + + user, err := co.GetUser(context.Background(), User{ID: "leseb"}) + assert.NoError(suite.T(), err) + if err == nil { + assert.Equal(suite.T(), user.Subusers[0].Name, "leseb:foo") + assert.Equal(suite.T(), user.Subusers[0].Access, SubuserAccess("read")) + } + }) + + suite.T().Run("remove a subuser", func(t *testing.T) { + err := co.RemoveSubuser(context.Background(), User{ID: "leseb"}, SubuserSpec{Name: "foo"}) + assert.NoError(suite.T(), err) + + user, err := co.GetUser(context.Background(), User{ID: "leseb"}) + assert.NoError(suite.T(), err) + if err == nil { + assert.Equal(suite.T(), len(user.Subusers), 0) + } + }) + suite.T().Run("remove user", func(t *testing.T) { err = co.RemoveUser(context.Background(), User{ID: "leseb"}) assert.NoError(suite.T(), err) diff --git a/rgw/admin/utils.go b/rgw/admin/utils.go index a36aa77cd..97751fa60 100644 --- a/rgw/admin/utils.go +++ b/rgw/admin/utils.go @@ -27,50 +27,79 @@ func buildQueryPath(endpoint, path, args string) string { } // valueToURLParams encodes structs into URL query parameters. -func valueToURLParams(i interface{}) url.Values { +func valueToURLParams(i interface{}, accpetableFields []string) url.Values { values := url.Values{} // Always return json values.Add("format", "json") - getReflect(i, &values) + getReflect(i, accpetableFields, &values) return values } -func getReflect(i interface{}, values *url.Values) { +func addToURLParams(v *url.Values, i interface{}, acceptableFields []string) { + getReflect(i, acceptableFields, v) +} + +// NOTE: we use linear search here, as none of the API endpoints +// supports more than 10 parameters, in this case asymptotics don't +// matter and we are likely faster this way (even when compared to a +// map). +func contains(tagList []string, tag string) bool { + for _, tag2 := range tagList { + if tag == tag2 { + return true + } + } + return false +} + +func getReflect(i interface{}, acceptableFields []string, values *url.Values) { t := reflect.TypeOf(i) v := reflect.ValueOf(i) for b := 0; b < v.NumField(); b++ { v2 := v.Field(b) - name := t.Field(b).Tag.Get("url") + tag := t.Field(b).Tag.Get("url") + if tag == "-" { + continue + } + tagList := strings.Split(tag, ",") + name := tagList[0] + if len(name) == 0 { + name = t.Field(b).Name + } - for _, name := range strings.Split(name, ",") { - if v2.Kind() == reflect.Struct { - getReflect(v2.Interface(), values) - } + if v2.Kind() == reflect.Struct { + getReflect(v2.Interface(), acceptableFields, values) + continue + } - if v2.Kind() == reflect.Slice { - for i := 0; i < v2.Len(); i++ { - item := v2.Index(i) - getReflect(item.Interface(), values) - } + if v2.Kind() == reflect.Slice { + for i := 0; i < v2.Len(); i++ { + item := v2.Index(i) + getReflect(item.Interface(), acceptableFields, values) } + continue + } - if v2.Kind() == reflect.String || - v2.Kind() == reflect.Bool || - v2.Kind() == reflect.Int { + if v2.Kind() == reflect.String || + v2.Kind() == reflect.Bool || + v2.Kind() == reflect.Int { - _v2 := fmt.Sprint(v2) - if len(_v2) > 0 && len(name) > 0 { - values.Add(name, _v2) - } + _v2 := fmt.Sprint(v2) + if len(_v2) > 0 && contains(acceptableFields, name) { + values.Add(name, _v2) } + continue + } - if v2.Kind() == reflect.Ptr && v2.IsValid() && !v2.IsNil() { - _v2 := fmt.Sprint(v2.Elem()) + if v2.Kind() == reflect.Ptr && v2.IsValid() && !v2.IsNil() { + _v2 := fmt.Sprint(v2.Elem()) + if len(_v2) > 0 && contains(acceptableFields, name) { values.Add(name, _v2) } + continue } } } diff --git a/rgw/admin/utils_test.go b/rgw/admin/utils_test.go index 9f2da1c24..933318305 100644 --- a/rgw/admin/utils_test.go +++ b/rgw/admin/utils_test.go @@ -18,6 +18,9 @@ func getDefaultValue() url.Values { func TestBuildQueryPath(t *testing.T) { queryPath := buildQueryPath("http://192.168.0.1", "/user", getDefaultValue().Encode()) assert.Equal(t, "http://192.168.0.1/admin/user?format=json", queryPath) + + queryPath = buildQueryPath("http://192.168.0.1", "/user?foo", getDefaultValue().Encode()) + assert.Equal(t, "http://192.168.0.1/admin/user?foo&format=json", queryPath) } func TestValueToURLParams(t *testing.T) { @@ -29,11 +32,11 @@ func TestValueToURLParams(t *testing.T) { args args want string }{ - {"default", args{User{ID: "leseb"}}, "format=json&uid=leseb"}, + {"default", args{User{ID: "leseb", Email: "leseb@example.com"}}, "format=json&uid=leseb"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := valueToURLParams(tt.args.i) + got := valueToURLParams(tt.args.i, []string{"uid"}) if !reflect.DeepEqual(got.Encode(), tt.want) { t.Errorf("valueToURLParams() = %v, want %v", got.Encode(), tt.want) }