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: iam: add all http args to req_info #31124

Merged
merged 1 commit into from Dec 20, 2019

Conversation

theanalyst
Copy link
Member

parsing a iam post request arguments were broken after
a143534 as all the key values weren't in http
args anymore. Since the if conditional were specifically to url_decode POST args
which are by default url encoded anyway, just add all the key value pairs to the
req_info args as these are used later by the respective process requests

Fixes: https://tracker.ceph.com/issues/42470
Signed-off-by: Abhishek Lekshmanan abhishek@suse.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 crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard backend
  • jenkins test docs
  • jenkins render docs

parsing a iam post request arguments were broken after
a143534 as all the key values weren't in http
args anymore. Since the if conditional were specifically to url_decode POST args
which are by default url encoded anyway, just add all the key value pairs to the
req_info args as these are used later by the respective process requests

Fixes: https://tracker.ceph.com/issues/42470
Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>
@theanalyst
Copy link
Member Author

@yuvalif not sure how this affects pubsub, if all the POST arguments are in the req_info arguments and url decoded, but user policy later looks for these arguments while processing

@yuvalif
Copy link
Contributor

yuvalif commented Oct 24, 2019

@yuvalif not sure how this affects pubsub, if all the POST arguments are in the req_info arguments and url decoded, but user policy later looks for these arguments while processing

@theanalyst testing it now.
BTW, do you have a test suite for iam (or other aws service APIs)?

@theanalyst
Copy link
Member Author

@yuvalif not sure how this affects pubsub, if all the POST arguments are in the req_info arguments and url decoded, but user policy later looks for these arguments while processing

@theanalyst testing it now.
BTW, do you have a test suite for iam (or other aws service APIs)?

I don't see any yet, if that is the case, I'm planning to write a basic put-user-policy tests so that something basic gets tested

@theanalyst
Copy link
Member Author

Added a basic smoke test at ceph/s3-tests#314 depends on #31127

@theanalyst
Copy link
Member Author

jenkins test this please (test_objectstore_memstore.sh which should be unrelated)

Copy link
Contributor

@yuvalif yuvalif left a comment

Choose a reason for hiding this comment

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

MON=1 OSD=1 MGR=0 MSD=0 RGW_MULTI_TEST_CONF=./test_multi.conf nosetests -s --verbosity=3  ../src/test/rgw/test_multi.py:test_ps_s3_topic_on_master

test is passing

@theanalyst
Copy link
Member Author

jenkins test this please

@theanalyst
Copy link
Member Author

QA suite at http://pulpito.ceph.com/abhi-2019-12-04_17:41:25-rgw-wip-abhi-testing-2019-12-04-1433-distro-basic-smithi/, after fixing the user-policy qa pr, the tests were rerun at http://pulpito.ceph.com/abhi-2019-12-06_16:18:07-rgw-wip-abhi-testing-2019-12-04-1433-distro-basic-smithi/

S3tests have passed, but seeing (unrelated?) failures in swift & java tests

@theanalyst theanalyst merged commit 7933cd1 into ceph:master Dec 20, 2019
@theanalyst theanalyst deleted the rgw/user-policy-args branch December 20, 2019 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants