| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2020-07-27 | |||
| 12:23:57 | dtantsur | mordred: hey! so, we use get_api_major_version in our service discovery, which does not work without endpoint_override, right?\ | |
| 12:24:02 | dtantsur | which sounds.. wrong to me | |
| 12:29:26 | mordred | that sounds wrong to me too | |
| 12:30:06 | mordred | oh - I think it's possible that we set endpoint_override as part of proxy creation | |
| 12:30:30 | mordred | like - iirc, we do a round of discovery to find the endpoint and then create an adapter with that endpoint set via endpoint_override | |
| 12:30:43 | mordred | maybe | |
| 12:42:20 | dtantsur | well, I'm trying to use http_basic auth and get_api_major_version is broken in two palces | |
| 12:42:47 | dtantsur | first, get_endpoint_data always returns None if endpoint_override is None (ignoring self.endpoint) | |
| 12:42:54 | dtantsur | I have no clue how it works for no-auth now, it should not | |
| 12:43:13 | mordred | hrm | |
| 12:43:29 | dtantsur | seconds, this condition is hit for ironic https://opendev.org/openstack/keystoneauth/src/branch/master/keystoneauth1/discover.py#L1115-L1119 | |
| 12:43:51 | dtantsur | again, no clue why it works for no-auth, but here it ends up with EndpointData populated with None | |
| 12:44:00 | dtantsur | because we don't run any discovery whatsoever | |
| 12:44:45 | mordred | well - you need to run some discovery to get microversion info - even with an endpoint_override | |
| 12:45:05 | dtantsur | even to get major version info | |
| 12:45:39 | dtantsur | killing the code in the 2nd link with fire and fixing http_basic.py to implement get_endpoint_data allows me to progress | |
| 12:45:46 | mordred | yeah. because otherwise how would it know | |
| 12:45:47 | dtantsur | but I wonder what I'm missing since it works for no-auth.. | |
| 12:46:06 | mordred | it's a good question | |
| 12:47:06 | dtantsur | it HAS to hit all the same problems. I don't understand why it does not. | |
| 13:16:43 | mordred | dtantsur: I agree with you - I cannot see any difference between the two | |
| 13:19:08 | dtantsur | :( | |
| 13:19:08 | KeithMnemonic | is there anyone here he wolud be kind enough to do a few review on openstackclient that passed a while back but are stuck waiting on reviews? thank you https://review.opendev.org/#/q/project:openstack/python-openstackclient+owner:%22Keith+Berger+%253Ckeith.berger%2540suse.com%253E%22+status:open | |
| 13:19:37 | dtantsur | I'm close to giving up on this, honestly.. but that would be a big loss if we cannot use authentication in bifrost without keystone | |
| 13:21:09 | mordred | dtantsur: there's a couple of unit tests we have for noauth but not for httpbasic - let me see what adding similar ones for basic doe | |
| 13:21:17 | dtantsur | +++ | |
| 13:22:01 | mordred | dtantsur: oh - when you get back ... | |
| 13:22:10 | mordred | no, nevermind | |
| 13:26:50 | openstackgerrit | Monty Taylor proposed openstack/keystoneauth master: Add unit test for httpbasicauth and discovery https://review.opendev.org/743227 | |
| 13:27:13 | mordred | dtantsur: remote: https://review.opendev.org/743227 Add unit test for httpbasicauth and discovery | |
| 13:27:13 | mordred | dtantsur: when you get back, can you point me to an example anywhere of this not working? unit test for the auth plugin calls get_api_major_version just fine on an http basic auth plugin | |
| 13:35:49 | openstackgerrit | jayaditya gupta proposed openstack/python-openstackclient master: openstack CLI migration list support https://review.opendev.org/742210 | |
| 13:36:39 | nightmare_unreal | hello can someone review this patch and also leave a response to my comment regarding it. https://review.opendev.org/#/c/742210/ | |
| 15:07:37 | dtantsur | mordred: if you have a spare VM, I can show you how to reproduce the bifrost failure | |
| 15:07:48 | dtantsur | I don't have a simple reproducer (which is another reason why I'm so frustrated) | |
| 15:12:25 | dtantsur | mordred: may I take over https://review.opendev.org/#/c/743227 and add a fix things there that might be a partial fix for my situation? | |
| 15:21:07 | openstackgerrit | Dmitry Tantsur proposed openstack/keystoneauth master: Fix get_endpoint_data for non-keystone plugins https://review.opendev.org/743227 | |
| 15:21:20 | dtantsur | mordred: ^^^ | |
| 15:26:49 | mordred | dtantsur: neat | |
| 15:27:21 | mordred | dtantsur: oh - that looks good | |
| 15:27:30 | mordred | dtantsur: still no clue why it works for one and not the other :) | |
| 15:28:03 | mordred | dtantsur: I had a thought - is it possible that ironic is http-auth protecting the discovery endpoit and ksa is just not sending auth when it talks to it? | |
| 15:28:33 | mordred | so ksa is trying to do discovery on the endpoint and failing? | |
| 15:28:49 | mordred | (which would explain why noauth works) | |
| 15:28:57 | dtantsur | mordred: checked that already | |
| 15:29:04 | mordred | nod | |
| 15:29:05 | dtantsur | curl works correctly | |
| 15:29:23 | dtantsur | (we had this problem with ironic-inspector, but we seem to be past it ) | |
| 16:04:37 | dtantsur | AttributeError: 'EntryPoint' object has no attribute 'module_name' | |
| 16:04:46 | dtantsur | hasn't it been fixed? I see it in keystoneauth on `tox -edocs` | |
| 16:06:43 | mordred | I thougth so? | |
| 16:07:00 | mordred | I thought it was a version something something | |
| 16:07:58 | mordred | dtantsur: I agree - I see that same issue | |
| 16:08:10 | dtantsur | yep :( | |
| 16:08:17 | mordred | dtantsur: maybe we fixed it in sdk but not ksa | |
| 16:12:14 | mordred | dtantsur: fix coming | |
| 16:12:45 | dtantsur | nice! if you also guess why this bloody discovery does not work, you'll be my saviour today :) | |
| 16:13:18 | openstackgerrit | Monty Taylor proposed openstack/keystoneauth master: Fix docs builds for modern stevedors https://review.opendev.org/743274 | |
| 16:13:23 | mordred | dtantsur: ^^ that | |
| 16:13:34 | mordred | dtantsur: I like being saviors :) | |
| 16:17:46 | dtantsur | but I think that get_endpoint_data patch is a part of the puzzle | |
| 16:21:25 | dtantsur | mordred: mmm, look. the adapter calls get_endpoint_data with service_type:baremetal, version:1 and so on, but it all goes to /dev/null | |
| 16:21:49 | dtantsur | because **kwargs are ignored in BaseAuthPlugin.get_endpoint_data | |
| 16:21:57 | dtantsur | is it intended? | |
| 16:22:17 | dtantsur | okay, I guess ignoring service_type is fine, but ignoring version seems to inhibit the discovery? | |
| 16:22:20 | mordred | not to my knowlege - probably just oversight. does passing the **kwargs on help? | |
| 16:22:23 | mordred | yeah | |
| 16:22:39 | mordred | (still confused why noauth works) | |
| 16:22:40 | dtantsur | which brings me back to the question why we inhibit discovery when a version is not requested.. | |
| 16:22:46 | dtantsur | yep. me too | |
| 16:26:45 | dtantsur | quite a few tests break if I remove the no_version handling from discover.py | |
| 16:26:55 | dtantsur | maybe I should go the other way and set max_version=min_version=kwargs.get('version')? | |
| 16:27:56 | dtantsur | wait, it's much easier! | |
| 16:28:01 | mordred | dtantsur: I think we inhibit discovery when a version isn't provided because we just assume that whatever is the default in the catalog is fine | |
| 16:28:02 | mordred | oh? | |
| 16:28:04 | mordred | cool! | |
| 16:28:35 | dtantsur | mordred: do you have a clue why inside get_api_major_version we call get_endpoint_data with discover_version=False? | |
| 16:28:41 | dtantsur | it seems like a version is what we need to discover | |
| 16:28:50 | dtantsur | if I change that to true, things start to work for me | |
| 16:29:20 | mordred | hrm. I think it's because we're trying to avoid a full discovery call if we don't need one | |
| 16:29:33 | mordred | but honestly - I kind of feel like perhaps that's effort we should stop caring about | |
| 16:29:59 | dtantsur | yeah, but then we short-circuit discovery if no versions are requested | |
| 16:30:09 | dtantsur | resulting in api_version being None | |
| 16:30:26 | mordred | does is break things to set it to true? :) | |
| 16:30:32 | dtantsur | lemme try! | |
| 16:30:56 | mordred | is there any chance that noauth vs httpauth we're using versioned vs unversioned endpoints as input? | |
| 16:31:13 | mordred | like - because we'll infer major api version from the /v1 in the endpoint if its there | |
| 16:31:24 | mordred | so maybe we've just accidentally worked with noauth based on input data? | |
| 16:31:40 | dtantsur | mordred: in case of bifrost the endpoints should be the same (unversioned) | |
| 16:31:49 | mordred | nod | |
| 16:31:55 | mordred | so yeah - no clue here | |
| 16:32:22 | dtantsur | if I change it, the no-auth/http_basic tests fail with: No mock address: GET http://keystone.example.com:5000/v3 | |
| 16:32:25 | dtantsur | which is pretty expected | |
| 16:32:47 | dtantsur | I guess what we need is "please do discover the major version but don't discovery microversions" | |
| 16:33:30 | mordred | or just add in mocks for mv discovery to those tests | |
| 16:35:22 | dtantsur | yeah, I'm still trying to avoid discovering microversions.. but it seems non-trivial with the code we have | |
| 16:37:26 | dtantsur | mordred: and this is why it works for keystone ;) https://opendev.org/openstack/keystoneauth/src/branch/master/keystoneauth1/identity/base.py#L501-L506 | |
| 16:38:14 | mordred | dtantsur: hahahahah | |
| 16:38:56 | dtantsur | okay, I'll update the same patch and hope I don't break anything else :D | |
| 16:39:44 | mordred | ++ | |