| *** mhen_ is now known as mhen | 02:30 | |
| opendevreview | Artem Goncharov proposed openstack/keystone master: Prevent MFA bypass https://review.opendev.org/c/openstack/keystone/+/945429 | 08:17 |
|---|---|---|
| *** ykarel_ is now known as ykarel | 11:12 | |
| opendevreview | Merged openstack/keystone master: Remove leftovers for SQLAlchemy < 2 https://review.opendev.org/c/openstack/keystone/+/929387 | 15:55 |
| stephenfin | gtema: fyi https://github.com/gophercloud/gophercloud/issues/3337 | 17:17 |
| gtema | how cool | 17:18 |
| stephenfin | before we file a keystone bug and work on a fix, that's not an expected change, right? I don't see anything in the release notes (and would expect this to be behind a microversion if so) | 17:18 |
| gtema | I think I recently found those absolutely undocumented query parameters | 17:19 |
| gtema | honestly I am not 100% sure how to deal with that. Technically Keystone would allow XXX_contains and some other query params, but that is not documented anywhere | 17:20 |
| gtema | so is it a documentation bug, jsonschema introduced incpmpatibility or it is a user issue of using undocumented things | 17:21 |
| stephenfin | IMO, we need additionalProperties=True right now at a minimum | 17:21 |
| stephenfin | going forward, we can either (a) document these in the schema/api-ref and tighten up comparator validation, or (b) decide we no longer want to support them and drop additionalProperties=True in a microversion | 17:22 |
| gtema | yeah, but we tried to prevent doing them since we were able to catch so many issues by restricting it | 17:22 |
| gtema | Keystone does not support microversions | 17:22 |
| stephenfin | oh, then s/decide we no longer want to support them and drop additionalProperties=True in a microversion/or (b) decide we no longer want to support them and drop additionalProperties=True in a release/ | 17:23 |
| stephenfin | at the moment we're kind of stuck in limbo though, as the clients are using these filters and keystone doesn't say it doesn't support them/no longer supports them, but it also doesn't say it does | 17:24 |
| gtema | how is contains used at the moment? What uses it? | 17:26 |
| stephenfin | does you mean how is it implemented, or what clients are using it? | 17:27 |
| gtema | which clients | 17:28 |
| stephenfin | Oh, in real life, I have no idea. Obviously Gophercloud is using it in tests which is how we spotted the issue | 17:28 |
| gtema | since it is till now not documented I hope nobody uses it | 17:30 |
| gtema | and so far we got no other reports, but maybe after release we will have some | 17:30 |
| stephenfin | Okay, if you're inclined to keep this, can we at least document it? | 17:33 |
| stephenfin | and it seems like keystone doesn't do real microversions, but they do "fake" ones? https://docs.openstack.org/api-ref/identity/v3/ Can we introduce 3.15? | 17:33 |
| gtema | those are not microversions, this is simply the api version | 17:34 |
| stephenfin | okay, then can we bump the API version? | 17:34 |
| stephenfin | ...to signal something has changed | 17:34 |
| gtema | I said some lines above that keystone does not support MV at all | 17:34 |
| gtema | we can bump it, but I would prefer if we could do this once we finish the jsonschema works | 17:35 |
| stephenfin | yeah, but I saw 3.14 and thought "that looks like a microversion but gtema said they don't support microversions so that must be a microversion-lookalike" | 17:35 |
| stephenfin | :) | 17:35 |
| gtema | cause we eventually will land other similar things | 17:35 |
| stephenfin | okay, then in that case can we set additionalProperties=True? | 17:36 |
| stephenfin | and remove it when we land the other similar things? | 17:36 |
| gtema | temporary - yes | 17:36 |
| stephenfin | okay, I'll send the patch | 17:36 |
| gtema | thks. The point is that it is a general thing that is applied to all apis, so the patch is also going to be big | 17:37 |
| opendevreview | Stephen Finucane proposed openstack/keystone master: api: Don't restrict unknown querystring parameters yet https://review.opendev.org/c/openstack/keystone/+/945504 | 18:11 |
| opendevreview | Stephen Finucane proposed openstack/keystone master: api: Correct query string schema for access rules API https://review.opendev.org/c/openstack/keystone/+/945505 | 18:11 |
| stephenfin | gtema: ^ Just running unit tests locally to ensure I didn't break anything | 18:11 |
| gtema | thks | 18:12 |
| stephenfin | It should go without saying, but I think that should be backported to stable/2025.1 :D I'll propose that pre-emptively shortly | 18:15 |
| gtema | agreed | 18:16 |
| opendevreview | Stephen Finucane proposed openstack/keystone master: api: Don't restrict unknown querystring parameters yet https://review.opendev.org/c/openstack/keystone/+/945504 | 18:25 |
| opendevreview | Stephen Finucane proposed openstack/keystone master: api: Correct query string schema for access rules API https://review.opendev.org/c/openstack/keystone/+/945505 | 18:25 |
| stephenfin | Weird: I proposed backports but they didn't get logged here... | 18:33 |
| gtema | weird, but np | 18:34 |
| stephenfin | https://review.opendev.org/c/openstack/project-config/+/945512 | 18:35 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!