| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2022-05-09 | |||
| 09:03:38 | opendevreview | Arx Cruz proposed openstack/ansible-collections-openstack master: Update port info https://review.opendev.org/c/openstack/ansible-collections-openstack/+/833083 | |
| 09:07:18 | opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Update identity_role_info for latest openstacksdk release https://review.opendev.org/c/openstack/ansible-collections-openstack/+/837751 | |
| 09:32:33 | opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack stable/1.0.0: Synchronize updates to identity_domain_info from master branch https://review.opendev.org/c/openstack/ansible-collections-openstack/+/841085 | |
| 09:46:26 | opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack stable/1.0.0: [DNM] Backported changes to identity_domain_info from master branch https://review.opendev.org/c/openstack/ansible-collections-openstack/+/841085 | |
| 09:47:07 | opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack stable/1.0.0: [DNM] Backported changes to identity_role_info from master branch https://review.opendev.org/c/openstack/ansible-collections-openstack/+/841091 | |
| 10:04:16 | opendevreview | Arx Cruz proposed openstack/ansible-collections-openstack master: Update identity_group_info to new sdk https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839365 | |
| 10:12:12 | opendevreview | Merged openstack/ansible-collections-openstack master: Refactored identity_domain_info https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839445 | |
| 10:22:50 | opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack stable/1.0.0: Backported changes to identity_domain_info from master branch https://review.opendev.org/c/openstack/ansible-collections-openstack/+/841085 | |
| 10:31:50 | opendevreview | Arx Cruz proposed openstack/ansible-collections-openstack master: Update identity_group_info to new sdk https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839365 | |
| 10:38:00 | opendevreview | Arx Cruz proposed openstack/ansible-collections-openstack master: Update identity_group_info to new sdk https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839365 | |
| 11:18:57 | opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Update identity_group_info to new sdk https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839365 | |
| 11:25:37 | opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack stable/1.0.0: Backported changes to identity_domain_info from master branch https://review.opendev.org/c/openstack/ansible-collections-openstack/+/841085 | |
| 12:03:21 | opendevreview | Merged openstack/ansible-collections-openstack master: Reverted identity_user_info from identity.users() to search_users() https://review.opendev.org/c/openstack/ansible-collections-openstack/+/841082 | |
| 12:59:50 | opendevreview | Arx Cruz proposed openstack/ansible-collections-openstack master: Update identity_group_info to new sdk https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839365 | |
| 13:10:34 | opendevreview | Hervé Beraud proposed openstack/microversion-parse master: Drop python3.6/3.7 support in testing runtime https://review.opendev.org/c/openstack/microversion-parse/+/840621 | |
| 13:25:22 | opendevreview | Merged openstack/ansible-collections-openstack master: Update identity_role_info for latest openstacksdk release https://review.opendev.org/c/openstack/ansible-collections-openstack/+/837751 | |
| 15:05:57 | opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack stable/1.0.0: Backported changes to identity_role_info from master branch https://review.opendev.org/c/openstack/ansible-collections-openstack/+/841091 | |
| 15:23:48 | opendevreview | Ananya proposed openstack/ansible-collections-openstack master: Moves image_info from cloud to proxy object https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828158 | |
| 15:52:51 | opendevreview | Ananya proposed openstack/ansible-collections-openstack master: Moves image_info from cloud to proxy object https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828158 | |
| 16:14:32 | opendevreview | Merged openstack/ansible-collections-openstack stable/1.0.0: Backported changes to identity_domain_info from master branch https://review.opendev.org/c/openstack/ansible-collections-openstack/+/841085 | |
| 16:17:40 | opendevreview | Ananya proposed openstack/ansible-collections-openstack master: Moves image_info from cloud to proxy object https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828158 | |
| 16:54:25 | opendevreview | ribaudr proposed openstack/python-openstackclient master: Microversion 2.91: Support specifying destination host to unshelve https://review.opendev.org/c/openstack/python-openstackclient/+/831902 | |
| 16:56:04 | opendevreview | ribaudr proposed openstack/python-openstackclient master: Microversion 2.91: Support specifying destination host to unshelve https://review.opendev.org/c/openstack/python-openstackclient/+/831902 | |
| 17:57:15 | opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Add script to test return values based off of doc string https://review.opendev.org/c/openstack/ansible-collections-openstack/+/840713 | |
| 18:02:00 | opendevreview | Ananya proposed openstack/ansible-collections-openstack master: Moves image_info from cloud to proxy object https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828158 | |
| 18:29:03 | TheJulia | Oh hai everybody! | |
| 18:29:09 | dtantsur | o/ | |
| 18:29:35 | TheJulia | I believe we've found an issue when using the sdk. Specifically when the SDK gets a 307 Redirect to a new host, it turns around and starts to duplicate the arguments | |
| 18:29:45 | dtantsur | likely even keystoneauth | |
| 18:30:00 | TheJulia | quite possible as it seem sto have redirect handling there | |
| 18:30:02 | TheJulia | https://storyboard.openstack.org/#!/story/2010029 | |
| 18:31:11 | dtantsur | I was able to reproduce it locally by inserting a redirect in front of ironic: https://paste.opendev.org/show/b7NUZbgk2HNMinwqOTik/ | |
| 18:32:02 | TheJulia | dtantsur: any chance you can see what the actual http response body is on the redirect? | |
| 18:32:09 | dtantsur | let me hack it | |
| 18:32:38 | TheJulia | k, because the redirect takes the new location and all the prior args | |
| 18:33:08 | TheJulia | and possibly cats... and whiskey | |
| 18:34:06 | dtantsur | we silence keystoneauth logging, don't we... | |
| 18:35:24 | TheJulia | I think the default is things >= warning are suppressed | |
| 18:35:29 | TheJulia | err <= warning | |
| 18:35:41 | dtantsur | hmmmmmm, I definitely use a warning | |
| 18:36:00 | dtantsur | hold on, I'm stupid | |
| 18:36:33 | dtantsur | moral: don't debug stuff with cold | |
| 18:37:43 | TheJulia | ++ | |
| 18:37:50 | TheJulia | I was doing that last week... it was not fun | |
| 18:37:58 | TheJulia | still am a little bit | |
| 18:39:09 | dtantsur | Redirecting to http://127.0.0.1:6385/ | |
| 18:39:11 | dtantsur | Redirecting to http://127.0.0.1:6385/v1/ | |
| 18:39:13 | dtantsur | Redirecting to http://127.0.0.1:6385/v1/nodes?provision_state=active | |
| 18:39:22 | dtantsur | the last one is probably relevant | |
| 18:40:31 | TheJulia | so the server, as it should, responds with the full location | |
| 18:41:22 | dtantsur | ending GET to http://127.0.0.1:6385/v1/nodes?provision_state=active with {'headers': {...}, 'params': {'provision_state': 'active'}, 'verify': True, 'allow_redirects': False} | |
| 18:41:28 | dtantsur | yep yep | |
| 18:41:39 | dtantsur | we have params both in the URL (from the redirect) and in kwargs | |
| 18:41:52 | TheJulia | and yeah, the code is just not *really* redirect safe if there are parameters | |
| 18:41:58 | dtantsur | the bug is somewhere around Session._send_request | |
| 18:42:14 | TheJulia | yup | |
| 18:42:47 | TheJulia | maybe strip params out if they are already in the url? | |
| 18:43:01 | dtantsur | is it safe to just drop params on redirect? | |
| 18:43:21 | dtantsur | I feel like it is, but maybe my brain is missing anything? | |
| 18:43:23 | TheJulia | I think it is going to depend on the server response | |
| 18:43:36 | dtantsur | well, the server knows nothing about our split between the URL and params | |
| 18:43:43 | dtantsur | it receives http://server?foo=bar | |
| 18:43:51 | dtantsur | if it drops foo=bar from the redirect, I guess it means that? | |
| 18:47:44 | TheJulia | the RFC suggests a final end location for the request | |
| 18:47:54 | TheJulia | but that may of course be dynamic | |
| 18:47:54 | dtantsur | I don't see the same problem in pure requests | |
| 18:48:12 | dtantsur | I wonder if it gets rid of params pretty early | |
| 18:49:17 | dtantsur | yeah, it merges params into the URL: https://github.com/psf/requests/blob/40956723f27daf5e0d9759208ca69cef236ab339/requests/models.py#L474-L481 | |
| 18:49:26 | dtantsur | and thus it does not have issues like this | |
| 18:49:28 | TheJulia | It that seems like an okay-ish thing to do on redirect since the response body is supposed to have the uri | |
| 18:50:28 | dtantsur | yep. and it effectively drops the params on redirect since it takes the new URL | |
| 18:51:03 | dtantsur | TheJulia: I don't think we haver HTTP->HTTPS redirects | |
| 18:51:12 | TheJulia | at least in our jobs/config | |
| 18:51:13 | dtantsur | that would involve having two ports for API? | |
| 18:51:17 | TheJulia | well | |
| 18:51:27 | dtantsur | it's purely a browser case where there is a default port (80) | |
| 18:51:30 | TheJulia | if you were doing a "connect to this with http" and redirect it to https, then I could see it | |
| 18:51:44 | dtantsur | that's not how things work, unfortunately | |
| 18:51:45 | TheJulia | but I also thing that would happen with the version detection, and then it might continue talking to the same server | |
| 18:51:55 | dtantsur | the best thing you get is HTTP 400 from nginx with an HTML | |
| 18:52:02 | TheJulia | well, how we work, but ops wise, it could be configured that way | |
| 18:52:21 | dtantsur | TheJulia: https://paste.opendev.org/show/bykSBnlqUoJsfVV9iRf5/ | |
| 18:52:35 | TheJulia | dtantsur: two separate servers | |
| 18:52:35 | dtantsur | yeah, probably. dunno if anyone ever bothers | |
| 18:52:46 | TheJulia | in this day and age... unlikely I guess | |
| 18:53:35 | dtantsur | it's nearly 9pm, I'll have to drop soon. if you have anything by tomorrow, I can test it in the morning. | |
| 18:53:46 | TheJulia | dtantsur: ack, have a wonderful evening | |
| 18:54:07 | dtantsur | you too! | |
| 19:09:09 | opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Moves image_info from cloud to proxy object https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828158 | |
| 19:18:29 | opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Add module to test return values based off of doc string https://review.opendev.org/c/openstack/ansible-collections-openstack/+/840713 | |
| 19:19:26 | opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Add module to test return values based off of doc string https://review.opendev.org/c/openstack/ansible-collections-openstack/+/840713 | |
| 21:09:01 | opendevreview | Merged openstack/ansible-collections-openstack stable/1.0.0: Backported changes to identity_role_info from master branch https://review.opendev.org/c/openstack/ansible-collections-openstack/+/841091 | |
| 21:09:19 | opendevreview | Julia Kreger proposed openstack/keystoneauth master: WIP: Only include parameters not present upon redirect https://review.opendev.org/c/openstack/keystoneauth/+/841169 | |
| 21:10:04 | TheJulia | dtantsur: ^^ I'm having python resolver issues, so if your able to \o/, if not I can retry tomorrow. | |
| #openstack-sdks - 2022-05-10 | |||
| 07:45:22 | opendevreview | Merged openstack/ansible-collections-openstack master: Moves image_info from cloud to proxy object https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828158 | |
| 08:02:50 | opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack stable/1.0.0: Backported changes to image_info from master branch https://review.opendev.org/c/openstack/ansible-collections-openstack/+/841224 | |
| 08:04:18 | opendevreview | Arx Cruz proposed openstack/ansible-collections-openstack master: Update identity_group_info to new sdk https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839365 | |
| 08:19:00 | opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Update identity_group_info to new sdk https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839365 | |
| 09:27:42 | opendevreview | Arx Cruz proposed openstack/ansible-collections-openstack master: Update identity_group_info to new sdk https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839365 | |
| 10:14:01 | opendevreview | Merged openstack/ansible-collections-openstack stable/1.0.0: Backported changes to image_info from master branch https://review.opendev.org/c/openstack/ansible-collections-openstack/+/841224 | |