Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rgw/admin: fix getbucketpolicy api #886

Merged
merged 1 commit into from
Jun 15, 2023

Conversation

thotz
Copy link
Collaborator

@thotz thotz commented Jun 2, 2023

The request header in GetBucketPolicy(), was not passing policy in its
header, so the response received Bucket than the actual policy.

fixes #882

rgw/admin/bucket.go Outdated Show resolved Hide resolved
@thotz thotz force-pushed the fix-owner-struct-in-policy branch from 831ae65 to cf26535 Compare June 2, 2023 10:28
@anoopcs9
Copy link
Collaborator

anoopcs9 commented Jun 5, 2023

Following diff is shown as the root cause of CI failures:

--- rgw/admin/bucket.go.orig
+++ rgw/admin/bucket.go
@@ -78,9 +78,9 @@
 	} `json:"acl"`
 	Owner struct {
 		ID struct {
-			ID      string `json:"id"`
+			ID     string `json:"id"`
 			Tenant string `json:"tenant"`
-			NS      string `json:"ns"`
+			NS     string `json:"ns"`
 		} `json:"id"`
 		DisplayName string `json:"display_name"`
 	} `json:"owner"`

@thotz thotz force-pushed the fix-owner-struct-in-policy branch from cf26535 to d3485ac Compare June 6, 2023 09:28
@thotz thotz requested a review from anoopcs9 June 6, 2023 14:37
Copy link
Collaborator

@anoopcs9 anoopcs9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized that we are missing a test for GetBucketPolicy. Can we add a very basic test to bucket_test.go? And while we are at it let's fix the following dead link from LINENO:116:

// GetBucketPolicy - http://docs.ceph.com/docs/mimic/radosgw/adminops/#get-bucket-or-object-policy

rgw/admin/bucket.go Outdated Show resolved Hide resolved
@anoopcs9 anoopcs9 added the bug label Jun 8, 2023
@thotz thotz force-pushed the fix-owner-struct-in-policy branch from d3485ac to 30e4fc2 Compare June 12, 2023 16:36
@thotz thotz requested a review from anoopcs9 June 12, 2023 16:41
@thotz thotz force-pushed the fix-owner-struct-in-policy branch 3 times, most recently from dbcd55d to e2ae172 Compare June 13, 2023 09:15
@anoopcs9 anoopcs9 added the API This PR includes a change to the public API of a go-ceph package label Jun 13, 2023
@anoopcs9
Copy link
Collaborator

@Mergifyio rebase

@mergify
Copy link

mergify bot commented Jun 13, 2023

rebase

✅ Branch has been successfully rebased

@anoopcs9 anoopcs9 force-pushed the fix-owner-struct-in-policy branch from e2ae172 to 0e747f5 Compare June 13, 2023 11:14
The request header in GetBucketPolicy(), was not passing policy in its
header, so the response received Bucket than the actual policy.

Signed-off-by: Jiffin Tony Thottan <[email protected]>
@thotz thotz force-pushed the fix-owner-struct-in-policy branch from 0e747f5 to 19749d2 Compare June 14, 2023 09:45
@thotz thotz changed the title rgw/admin: fix owner struct in bucket policy rgw/admin: fix getbucketpolicy api Jun 14, 2023
Copy link
Collaborator

@anoopcs9 anoopcs9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

@anoopcs9 anoopcs9 requested a review from phlogistonjohn June 14, 2023 16:45
Copy link
Collaborator

@phlogistonjohn phlogistonjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM.

One test is failing but it appears to be due to trouble getting to the RPM repos on the ceph infra. I hope that retrying a few time will eventually resolve the issue.

@mergify mergify bot merged commit 3051f22 into ceph:master Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API This PR includes a change to the public API of a go-ceph package bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mismatched struct Policy vs Bucket in https://github.com/ceph/go-ceph/blob/master/rgw/admin/bucket.go
3 participants