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: fix use of creds in forward_iam_request() #55148

Merged
merged 4 commits into from
Jan 16, 2024
Merged

Conversation

cbodley
Copy link
Contributor

@cbodley cbodley commented Jan 11, 2024

variable creds was moved into the RGWRESTConn constructor before being passed into forward_iam_request(). change forward_iam_request() so it uses the member variable from the constructor instead of taking it as an argument

Fixes: https://tracker.ceph.com/issues/63994

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows
  • jenkins test rook e2e

variable `creds` was moved into the `RGWRESTConn` constructor before being
passed into `forward_iam_request()`. change `forward_iam_request()` so
it uses the member variable from the constructor instead of taking it as
an argument

Fixes: https://tracker.ceph.com/issues/63994

Signed-off-by: Casey Bodley <cbodley@redhat.com>
in testing, i was seeing meta sync checkpoints finish even though sync
hadn't started yet:
```
rgw_multi.tests: DEBUG: current meta sync status={
    "sync_status": {
        "info": {
            "status": "building-full-sync-maps",
```

wait for the global status to reach "sync" before starting to compare
period epochs or sync markers

Signed-off-by: Casey Bodley <cbodley@redhat.com>
fix a regression from commit d3ad0ef
which changed how we parse the response bufferlist:

-  std::string r = response.c_str();
+  std::string r = response.to_str();

when the response contains a trailing null character, this now ends up in
`r` and breaks json parsing in `parser.parse(r.c_str(), r.length(), 1)`

replace `response.to_str()` with `rgw_bl_str(response)` which trims
trailing nulls

Signed-off-by: Casey Bodley <cbodley@redhat.com>
where bl contains multiple buffer segments, c_str() has to
rellocate and copy those segments into a single buffer. use c_str()
instead, which just copies each segment into the resulting string

this allows the function to take the bufferlist argument by const ref

Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
Copy link
Contributor Author

cbodley commented Jan 12, 2024

failed qa in https://pulpito.ceph.com/cbodley-2024-01-11_18:40:08-rgw:multisite-wip-63994-distro-default-smithi/

2024-01-11T20:25:14.661 ERROR:boto:409 Conflict
2024-01-11T20:25:14.661 ERROR:boto:b'<?xml version="1.0" encoding="UTF-8"?><Error><Code>BucketNotEmpty</Code><Message></Message><RequestId>tx00000bd13b2353be5b319-0065a04eaa-6192-test-zone2</RequestId><HostId>6192-test-zone2-test-zonegroup</HostId></Error>'
2024-01-11T20:25:14.831 INFO:tasks.rgw_multisite_tests:rgw_multi.tests.test_role_sync ... ERROR

creation of role2 on the secondary zone gets forwarded to the metadata master zone where it succeeds, but the secondary rgw fails to parse that response:

iam:create_role ERROR: failed to parse response from master zonegroup

that parse failure results in a 500 response which boto retries. when forwarded to the metedata master, the retried CreateRole request fails with "409 EntityAlreadyExists". the secondary rgw maps that 409 error to "409 BucketNotEmpty"

@github-actions github-actions bot added the tests label Jan 12, 2024
@cbodley
Copy link
Contributor Author

cbodley commented Jan 12, 2024

iam:create_role ERROR: failed to parse response from master zonegroup

that was a separate regression from #50599 which changed how trailing null characters were handled. with that fixed, i see test_role_sync passing

@cbodley cbodley requested a review from smanjara January 12, 2024 16:45
@cbodley
Copy link
Contributor Author

cbodley commented Jan 12, 2024

jenkins test api

@cbodley
Copy link
Contributor Author

cbodley commented Jan 14, 2024

https://pulpito.ceph.com/cbodley-2024-01-12_19:40:46-rgw:multisite-wip-63994-distro-default-smithi/

2024-01-13T08:46:19.903 INFO:tasks.rgw_multisite_tests:rgw_multi.tests.test_role_sync ... ok
2024-01-13T09:11:45.479 INFO:tasks.rgw_multisite_tests:rgw_multi.tests.test_role_sync ... ok

@cbodley
Copy link
Contributor Author

cbodley commented Jan 16, 2024

@cbodley cbodley merged commit 5f3ea96 into ceph:main Jan 16, 2024
10 of 11 checks passed
@cbodley cbodley deleted the wip-63994 branch January 16, 2024 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants