nautilus: mgr/dashboard: change bucket owner between owners from same tenant by alfonsomthd · Pull Request #29485 · ceph/ceph

alfonsomthd · 2019-08-05T06:38:13Z

Since #28813 is in master, the bucket operations (with tenanted owners) behaviour has changed regarding nautilus, so this fix is applied directly.

Fixes: https://tracker.ceph.com/issues/41067
Signed-off-by: alfonsomthd almartin@redhat.com

Checklist

References tracker ticket
Updates documentation if necessary
Includes tests for new functionality or reproducer for bug

Show available Jenkins commands

jenkins retest this please
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test docs
jenkins render docs

tspmelo · 2019-08-09T16:12:01Z

jenkins test dashboard

votdev

LGTM

votdev · 2019-08-12T07:27:08Z

src/pybind/mgr/dashboard/controllers/rgw.py

            raise DashboardException(e, http_status_code=500, component='rgw')

    def set(self, bucket, bucket_id, uid):
+        bucket_tenant = bucket[:bucket.find('/')] if bucket.find('/') >= 0 else None


Can you please add a comment here why it is necessary to strip the tenant in a special case. In some months nobody is remembering this.

Second, wouldn't it make sense to relocate the logic into strip_tenant_from_bucket_name, too? This would make the set method more cleaner.

smithfarm

Please update the commit message so it explains why the fix cannot be cherry-picked from master.

alfonsomthd · 2019-08-20T06:54:25Z

Please update the commit message so it explains why the fix cannot be cherry-picked from master.

@smithfarm Done.

smithfarm · 2019-08-20T07:34:31Z

Please update the commit message so it explains why the fix cannot be cherry-picked from master.

@smithfarm Done.

Hm, maybe I'm missing something, but the commit message seems to still be the same, i.e.:

mgr/dashboard: change bucket owner between owners from same tenant

Fixes: https://tracker.ceph.com/issues/41067
Signed-off-by: alfonsomthd <almartin@redhat.com>

Having the explanation in the PR is better than nothing, but having it in the git history is best. Therefore, I was hoping the commit message could be changed so it includes this nice explanation that you added to the PR description:

Since https://github.com/ceph/ceph/pull/28813 is in master, the bucket operations
(with tenanted owners) behaviour has changed regarding nautilus, so this fix is
applied to nautilus directly.

alfonsomthd · 2019-08-20T09:05:33Z

Hm, maybe I'm missing something, but the commit message seems to still be the same, i.e.:

@smithfarm sorry, I updated the commit message accordingly.

LenzGr

LGTM - thank you

alfonsomthd · 2019-08-21T14:05:20Z

jenkins test dashboard

Since ceph#28813 is in master, the bucket operations (with tenanted owners) behaviour has changed regarding nautilus, so this fix is applied to nautilus directly. Fixes: https://tracker.ceph.com/issues/41067 Signed-off-by: Alfonso Martínez <almartin@redhat.com>

bk201 · 2019-09-18T08:27:34Z

Thanks! I tested the changes on the latest Nautilus branch (462e659).
Got an error when changing bucket's owner to a user in a different tenant.

e.g.

Two users: foo$bar and xxx$yyy
Create a bucket and owned by foo$bar
Error when editing the bucket and change the owner to xxx$yyy

Also, I saw different behaviors between master and nautilus. When changing the owner for a bucket, the tenant prefix will be changed accordingly in master but not in nautilus. Is this a bug?

commit message looks good now

alfonsomthd · 2019-09-18T08:38:51Z

Thanks! I tested the changes on the latest Nautilus branch (462e659).

Got an error when changing bucket's owner to a user in a different tenant.

e.g.
* Two users: foo$bar and xxx$yyy

* Create a bucket and owned by `foo$bar`

* Error when editing the bucket and change the owner to `xxx$yyy`
Also, I saw different behaviors between master and nautilus. When changing the owner for a bucket, the tenant prefix will be changed accordingly in master but not in nautilus. Is this a bug?

The error is expected as currently it is not possible to access another tenant’s buckets:
https://docs.ceph.com/docs/nautilus/radosgw/multitenancy/#accessing-buckets-with-explicit-tenants

Thus a complete compatibility is maintained with previous releases, as long as the referred
buckets and referring user belong to the same tenant. In other words, anything unusual occurs
when accessing another tenant’s buckets only.

The behavior in master has evolved regarding nautilus.

alfonsomthd · 2019-09-25T08:41:45Z

jenkins test dashboard backend

alfonsomthd · 2019-09-25T14:25:29Z

jenkins test dashboard backend

bk201 · 2019-09-26T07:52:24Z

LGTM. Also run qa test mgr.dashboard.test_rgw.RgwBucketTest locally.

smithfarm · 2019-10-01T16:08:41Z

@LenzGr I see you added the nautilus-batch-1 label. Does that mean we can disregard the failed ceph dashboard backend API tests Jenkins PR CI test?

(Ordinarily, nautilus-batch-1 is not added until all relevant Jenkins PR tests are passing.)

alfonsomthd · 2019-10-02T06:19:41Z

@LenzGr I see you added the nautilus-batch-1 label. Does that mean we can disregard the failed ceph dashboard backend API tests Jenkins PR CI test?

(Ordinarily, nautilus-batch-1 is not added until all relevant Jenkins PR tests are passing.)

@smithfarm @LenzGr The failure is not related to this PR.
As @bk201 commented before, the related API tests pass locally.

jenkins test dashboard backend is a job that has been tested in master branch (it works) but never in nautilus branch,
I triggered it because "it should work" for nautilus branch, but this subject should be investigated in more detail. There is an issue: https://tracker.ceph.com/issues/40035

smithfarm · 2019-10-02T08:12:12Z

@alfonsomthd Thanks for the explanation!

callithea · 2019-10-28T09:11:27Z

QA run was successful: http://pulpito.ceph.com/laura-2019-10-25_07:05:49-rados:mgr-wip-laura-testing-nautilus-29485-31114-distro-basic-smithi/ (in combination with #31114)

alfonsomthd added bug-fix needs-qa dashboard labels Aug 5, 2019

alfonsomthd requested review from LenzGr, s0nea, smithfarm and votdev August 5, 2019 06:38

tspmelo added this to the nautilus milestone Aug 5, 2019

tspmelo changed the title ~~mgr/dashboard: change bucket owner between owners from same tenant~~ nautilus: mgr/dashboard: change bucket owner between owners from same tenant Aug 5, 2019

votdev requested changes Aug 12, 2019

View reviewed changes

smithfarm previously requested changes Aug 15, 2019

View reviewed changes

smithfarm added needs-review and removed needs-qa labels Aug 15, 2019

alfonsomthd force-pushed the 41067-change-bucket-owner branch from 130f00f to 492c1b3 Compare August 20, 2019 08:44

alfonsomthd requested review from smithfarm and votdev August 20, 2019 09:29

LenzGr approved these changes Aug 21, 2019

View reviewed changes

votdev approved these changes Aug 21, 2019

View reviewed changes

bk201 mentioned this pull request Sep 4, 2019

mgr/dashboard: fix error when linking a bucket to a tenanted u… #30123

Merged

3 tasks

alfonsomthd force-pushed the 41067-change-bucket-owner branch from 492c1b3 to f267225 Compare September 17, 2019 14:04

smithfarm removed their request for review September 18, 2019 08:38

alfonsomthd requested a review from callithea September 23, 2019 13:45

LenzGr added the needs-qa label Sep 25, 2019

LenzGr added the nautilus-batch-1 nautilus point releases label Sep 25, 2019

smithfarm removed the needs-review label Oct 2, 2019

callithea approved these changes Oct 28, 2019

View reviewed changes

callithea added wip-laura-testing and removed wip-laura-testing labels Oct 30, 2019

LenzGr changed the title ~~nautilus: mgr/dashboard: change bucket owner between owners from same tenant~~ nautilus: mgr/dashboard: change bucket owner between owners fr… Nov 7, 2019

LenzGr merged commit 16d86fd into ceph:nautilus Nov 7, 2019

smithfarm changed the title ~~nautilus: mgr/dashboard: change bucket owner between owners fr…~~ nautilus: mgr/dashboard: change bucket owner between owners from same tenant Nov 8, 2019

alfonsomthd deleted the 41067-change-bucket-owner branch April 9, 2020 09:54

Conversation

alfonsomthd commented Aug 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

tspmelo commented Aug 9, 2019

Uh oh!

votdev left a comment

Choose a reason for hiding this comment

Uh oh!

votdev Aug 12, 2019

Choose a reason for hiding this comment

Uh oh!

alfonsomthd Aug 21, 2019

Choose a reason for hiding this comment

Uh oh!

smithfarm left a comment

Choose a reason for hiding this comment

Uh oh!

alfonsomthd commented Aug 20, 2019

Uh oh!

smithfarm commented Aug 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alfonsomthd commented Aug 20, 2019

Uh oh!

LenzGr left a comment

Choose a reason for hiding this comment

Uh oh!

alfonsomthd commented Aug 21, 2019

Uh oh!

bk201 commented Sep 18, 2019

Uh oh!

alfonsomthd commented Sep 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alfonsomthd commented Sep 25, 2019

Uh oh!

alfonsomthd commented Sep 25, 2019

Uh oh!

bk201 commented Sep 26, 2019

Uh oh!

smithfarm commented Oct 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alfonsomthd commented Oct 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smithfarm commented Oct 2, 2019

Uh oh!

callithea commented Oct 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

alfonsomthd commented Aug 5, 2019 •

edited

Loading

smithfarm commented Aug 20, 2019 •

edited

Loading

alfonsomthd commented Sep 18, 2019 •

edited

Loading

smithfarm commented Oct 1, 2019 •

edited

Loading

alfonsomthd commented Oct 2, 2019 •

edited

Loading

callithea commented Oct 28, 2019 •

edited

Loading