| opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Update identity_domain_info to use proxy layer https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839445 | 07:34 |
|---|---|---|
| opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Refactored identity_domain_info https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839445 | 08:00 |
| opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Reverted identity_role_info from identity.users() to search_users() https://review.opendev.org/c/openstack/ansible-collections-openstack/+/841082 | 08:34 |
| opendevreview | Jakob Meng proposed 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 | 08:43 |
| opendevreview | Arx Cruz proposed openstack/ansible-collections-openstack master: Update catalog service for the new sdk https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839352 | 08:53 |
| opendevreview | Arx Cruz proposed openstack/ansible-collections-openstack master: Update routers_info for the new SDK https://review.opendev.org/c/openstack/ansible-collections-openstack/+/838489 | 09:01 |
| opendevreview | Arx Cruz proposed openstack/ansible-collections-openstack master: Update port info https://review.opendev.org/c/openstack/ansible-collections-openstack/+/833083 | 09:03 |
| 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:07 |
| 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:32 |
| 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:46 |
| 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 | 09:47 |
| 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:04 |
| opendevreview | Merged openstack/ansible-collections-openstack master: Refactored identity_domain_info https://review.opendev.org/c/openstack/ansible-collections-openstack/+/839445 | 10:12 |
| 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:22 |
| 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:31 |
| 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 |
| 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:18 |
| 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 | 11:25 |
| 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:03 |
| 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 | 12:59 |
| 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:10 |
| 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 | 13:25 |
| 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:05 |
| 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:23 |
| 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 |
| 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:14 |
| 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:17 |
| 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:54 |
| 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 |
| 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 | 17:57 |
| 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:02 |
| TheJulia | Oh hai everybody! | 18:29 |
| dtantsur | o/ | 18:29 |
| 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 |
| dtantsur | likely even keystoneauth | 18:29 |
| * dtantsur is staring out the session code | 18:29 | |
| TheJulia | quite possible as it seem sto have redirect handling there | 18:30 |
| TheJulia | https://storyboard.openstack.org/#!/story/2010029 | 18:30 |
| dtantsur | I was able to reproduce it locally by inserting a redirect in front of ironic: https://paste.opendev.org/show/b7NUZbgk2HNMinwqOTik/ | 18:31 |
| TheJulia | dtantsur: any chance you can see what the actual http response body is on the redirect? | 18:32 |
| dtantsur | let me hack it | 18:32 |
| * dtantsur loves bifrost: no containers, no troubles, only git, vim and you | 18:32 | |
| TheJulia | k, because the redirect takes the new location and all the prior args | 18:32 |
| TheJulia | and possibly cats... and whiskey | 18:33 |
| dtantsur | we silence keystoneauth logging, don't we... | 18:34 |
| TheJulia | I think the default is things >= warning are suppressed | 18:35 |
| TheJulia | err <= warning | 18:35 |
| dtantsur | hmmmmmm, I definitely use a warning | 18:35 |
| dtantsur | hold on, I'm stupid | 18:36 |
| * dtantsur is changing the server side..... | 18:36 | |
| dtantsur | moral: don't debug stuff with cold | 18:36 |
| TheJulia | ++ | 18:37 |
| TheJulia | I was doing that last week... it was not fun | 18:37 |
| TheJulia | still am a little bit | 18:37 |
| dtantsur | Redirecting to http://127.0.0.1:6385/ | 18:39 |
| dtantsur | Redirecting to http://127.0.0.1:6385/v1/ | 18:39 |
| dtantsur | Redirecting to http://127.0.0.1:6385/v1/nodes?provision_state=active | 18:39 |
| dtantsur | the last one is probably relevant | 18:39 |
| TheJulia | so the server, as it should, responds with the full location | 18:40 |
| 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 |
| dtantsur | yep yep | 18:41 |
| dtantsur | we have params both in the URL (from the redirect) and in kwargs | 18:41 |
| TheJulia | and yeah, the code is just not *really* redirect safe if there are parameters | 18:41 |
| dtantsur | the bug is somewhere around Session._send_request | 18:41 |
| TheJulia | yup | 18:42 |
| TheJulia | maybe strip params out if they are already in the url? | 18:42 |
| dtantsur | is it safe to just drop params on redirect? | 18:43 |
| dtantsur | I feel like it is, but maybe my brain is missing anything? | 18:43 |
| TheJulia | I think it is going to depend on the server response | 18:43 |
| dtantsur | well, the server knows nothing about our split between the URL and params | 18:43 |
| dtantsur | it receives http://server?foo=bar | 18:43 |
| * TheJulia looks up the redirect RFCs | 18:43 | |
| dtantsur | if it drops foo=bar from the redirect, I guess it means that? | 18:43 |
| * dtantsur is not sure | 18:43 | |
| TheJulia | the RFC suggests a final end location for the request | 18:47 |
| dtantsur | I don't see the same problem in pure requests | 18:47 |
| TheJulia | but that may of course be dynamic | 18:47 |
| dtantsur | I wonder if it gets rid of params pretty early | 18:48 |
| dtantsur | yeah, it merges params into the URL: https://github.com/psf/requests/blob/40956723f27daf5e0d9759208ca69cef236ab339/requests/models.py#L474-L481 | 18:49 |
| dtantsur | and thus it does not have issues like this | 18:49 |
| TheJulia | It that seems like an okay-ish thing to do on redirect since the response body is supposed to have the uri | 18:49 |
| * TheJulia wonders how this didn't get noticed previously... say with HTTP->HTTPS redirects | 18:50 | |
| dtantsur | yep. and it effectively drops the params on redirect since it takes the new URL | 18:50 |
| dtantsur | TheJulia: I don't think we haver HTTP->HTTPS redirects | 18:51 |
| TheJulia | at least in our jobs/config | 18:51 |
| dtantsur | that would involve having two ports for API? | 18:51 |
| TheJulia | well | 18:51 |
| dtantsur | it's purely a browser case where there is a default port (80) | 18:51 |
| TheJulia | if you were doing a "connect to this with http" and redirect it to https, then I could see it | 18:51 |
| dtantsur | that's not how things work, unfortunately | 18:51 |
| TheJulia | but I also thing that would happen with the version detection, and then it might continue talking to the same server | 18:51 |
| dtantsur | the best thing you get is HTTP 400 from nginx with an HTML | 18:51 |
| TheJulia | well, how we work, but ops wise, it could be configured that way | 18:52 |
| dtantsur | TheJulia: https://paste.opendev.org/show/bykSBnlqUoJsfVV9iRf5/ | 18:52 |
| dtantsur | yeah, probably. dunno if anyone ever bothers | 18:52 |
| TheJulia | dtantsur: two separate servers | 18:52 |
| TheJulia | in this day and age... unlikely I guess | 18:52 |
| 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 |
| TheJulia | dtantsur: ack, have a wonderful evening | 18:53 |
| dtantsur | you too! | 18:54 |
| 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:09 |
| 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:18 |
| 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 |
| 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 |
| opendevreview | Julia Kreger proposed openstack/keystoneauth master: WIP: Only include parameters not present upon redirect https://review.opendev.org/c/openstack/keystoneauth/+/841169 | 21:09 |
| TheJulia | dtantsur: ^^ I'm having python resolver issues, so if your able to \o/, if not I can retry tomorrow. | 21:10 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!