ayoung | Well...most of it is. Its not the cleanest chunk of code in Keystone | 00:00 |
---|---|---|
kmalloc | which is previous controller code. it is what does all the logic of processing the auth request and passing through the auth plugins | 00:00 |
kmalloc | really, it is in the right spot. it is ugly code, but it is "process a request", it isn't something the manager has real insight into | 00:00 |
ayoung | I'd probably have taken a smaller step in general, and only put the absolute minimum under api/ so that the other files didn't change as much, making them easier to review, and then cleanup as a second one, but I am not the one doing the work. | 00:00 |
ayoung | and I do agree that it is now in the right spot | 00:01 |
kmalloc | unfortunatly, this is all very webob code | 00:01 |
ayoung | this is kinda like Trusts. I'm not sure who else really can review it | 00:01 |
ayoung | maybe Guang if he returned from the ether | 00:01 |
kmalloc | so it would need to be totally reworked for flask anyway | 00:01 |
kmalloc | yeah gyee did some of the work here | 00:01 |
kmalloc | but mostly auth flow has been you, me, steve, and previously dolph and henry (@protected) | 00:02 |
ayoung | A good deal more than "some" and, while he is a good thinker, his code organization alwasy begged for followon cleanup | 00:02 |
ayoung | anyway, I'm going through it, and it looks ok. I think the test changes are small enough that I am ok with them | 00:02 |
kmalloc | yeah the test changes annoyed me, but this is one of those cases where it's... well so tightly coupled | 00:03 |
ayoung | yep | 00:03 |
kmalloc | i *could* split 2-3 things out but it's not a lot | 00:03 |
kmalloc | probably wouild have saved ~100 lines of change total | 00:03 |
ayoung | it was not until later that dolph really started pushing the "test things through the web interface" that got us out of these kinds of tests | 00:03 |
ayoung | 3600 line changes are painful | 00:04 |
kmalloc | i *hate* this change but i don't see a way of making it smaller without working in a model like a pull request where you stack multiple commits in a single merge | 00:04 |
kmalloc | this is where gerrit tips over. | 00:04 |
kmalloc | unless i did it all without testing and then supplied a merge commit | 00:04 |
kmalloc | but i don't think that is a lot better | 00:04 |
kmalloc | (e.g. feature branch) | 00:05 |
ayoung | probably could have continued to pass a request object around | 00:05 |
kmalloc | the merge commit would have been hell. | 00:05 |
ayoung | but those changes are trivial to review | 00:05 |
kmalloc | right. and all the rest of the code had to be reworked or craft a request and translate flask->webob->flask which was ugly enough in os-federation | 00:05 |
kmalloc | tl;dr: I'm sorry, but thanks for reviewing it | 00:06 |
kmalloc | on the plus side, everything (inc. federation functional) is green except that one elastic-recheck caught thing. | 00:06 |
ayoung | I'm ok with this. I think it is easier as one big commit, TBH | 00:06 |
kmalloc | projects and users next | 00:06 |
kmalloc | super light compared to this | 00:06 |
kmalloc | and then collapse all our internal middleware into "before-request" functions | 00:07 |
kmalloc | and i can rip out all the ugly webob / old wsgi stuff. | 00:07 |
kmalloc | there is cleanup to do, but this has been really good imo overall | 00:07 |
ayoung | It has been a long time coming | 00:08 |
ayoung | I remember the early spikes looking in to pecan etc | 00:08 |
ayoung | dstank and jamielennox both wanted a cleanup like this back then | 00:08 |
ayoung | heh | 00:08 |
ayoung | dstank | 00:08 |
*** spartakos has quit IRC | 00:19 | |
*** tristanC has quit IRC | 00:33 | |
*** tristanC has joined #openstack-keystone | 00:33 | |
*** dave-mccowan has quit IRC | 00:52 | |
*** adriant has quit IRC | 00:54 | |
*** gyee has quit IRC | 00:54 | |
*** felipemonteiro has quit IRC | 00:55 | |
*** adriant has joined #openstack-keystone | 01:00 | |
*** felipemonteiro has joined #openstack-keystone | 01:13 | |
*** felipemonteiro has quit IRC | 01:18 | |
*** felipemonteiro has joined #openstack-keystone | 01:49 | |
*** Dinesh_Bhor has joined #openstack-keystone | 01:57 | |
*** Dinesh_Bhor has quit IRC | 02:05 | |
*** cfriesen has quit IRC | 02:09 | |
*** Dinesh_Bhor has joined #openstack-keystone | 02:19 | |
*** pooja-jadhav has joined #openstack-keystone | 02:42 | |
*** pooja_jadhav has quit IRC | 02:46 | |
*** Dinesh_Bhor has quit IRC | 03:30 | |
*** sapd1_ has quit IRC | 03:42 | |
*** sapd1 has joined #openstack-keystone | 03:42 | |
*** shyamb has joined #openstack-keystone | 03:45 | |
*** felipemonteiro has quit IRC | 03:45 | |
*** pooja-jadhav has quit IRC | 04:14 | |
*** kukacz has quit IRC | 04:14 | |
*** kukacz has joined #openstack-keystone | 04:16 | |
*** pooja_jadhav has joined #openstack-keystone | 04:16 | |
*** shyamb has quit IRC | 04:22 | |
*** shyamb has joined #openstack-keystone | 04:22 | |
vishakha | @cmurphy , kmalloc I have a question regarding ec2 APIs. | 04:38 |
*** Dinesh_Bhor has joined #openstack-keystone | 04:38 | |
kmalloc | Sure. | 04:38 |
kmalloc | Ask away. | 04:38 |
vishakha | kmalloc: As per https://docs.openstack.org/releasenotes/keystone/mitaka.html -"EC2 APIs are indefinitely deprecated and will not be removed in the ‘Q’ release. | 04:38 |
vishakha | kmalloc: But in this commit https://review.openstack.org/#/c/572846/2/keystone/contrib/ec2/controllers.py EC2 v2 controller is removed and EC2 v3 controller stay there. | 04:39 |
vishakha | kmalloc: Does it mean that ec2 v3 APIs are not deprecated and will stay.? | 04:39 |
kmalloc | V3 is not deprecated | 04:40 |
kmalloc | V2 was missed in a deprecation notice, so they were kept another cycle | 04:40 |
kmalloc | That note was meant to only impact v2 | 04:41 |
kmalloc | Keystone v2* | 04:41 |
vishakha | kmalloc: is there any plan to deprecate ec2 v3 also? or they will reamin as it is? | 04:42 |
vishakha | *remain | 04:42 |
kmalloc | No plans to deprecate | 04:43 |
vishakha | kmalloc: thanks for the response | 04:43 |
kmalloc | NP :) | 04:43 |
*** felipemonteiro has joined #openstack-keystone | 04:50 | |
*** shyamb has quit IRC | 05:11 | |
*** shyamb has joined #openstack-keystone | 05:11 | |
*** shyamb has quit IRC | 05:26 | |
*** shyamb has joined #openstack-keystone | 05:53 | |
openstackgerrit | Merged openstack/keystone master: Purge soft-deleted trusts https://review.openstack.org/604970 | 06:20 |
*** rcernin has quit IRC | 06:38 | |
*** rcernin has joined #openstack-keystone | 06:38 | |
*** shyamb has quit IRC | 06:38 | |
*** pcaruana has joined #openstack-keystone | 06:40 | |
*** shyamb has joined #openstack-keystone | 06:44 | |
*** felipemonteiro has quit IRC | 06:53 | |
*** rcernin has quit IRC | 07:10 | |
openstackgerrit | Vishakha Agarwal proposed openstack/keystone master: Implement scope_type checking for ec2 credentials https://review.openstack.org/607820 | 07:15 |
openstackgerrit | Vishakha Agarwal proposed openstack/keystone master: [WIP] Implement scope_type checking for ec2 credentials https://review.openstack.org/607820 | 07:17 |
openstackgerrit | Vishakha Agarwal proposed openstack/keystone master: [WIP] Implement scope_type checking for ec2 credentials https://review.openstack.org/607820 | 07:17 |
vishakha | kmalloc : I am working on this patch https://review.openstack.org/#/c/607820. and I did not find where ec2 policies are enforced in keystone.? | 07:37 |
*** shyamb has quit IRC | 07:38 | |
*** shyamb has joined #openstack-keystone | 07:38 | |
vishakha | cmurphy, ^^ | 07:38 |
*** pjrusak has joined #openstack-keystone | 07:40 | |
cmurphy | vishakha: it's enforced by the @controller.protected decorator in the controller, for example here http://git.openstack.org/cgit/openstack/keystone/tree/keystone/contrib/ec2/controllers.py#n302 | 07:43 |
vishakha | cmurphy: I understood that with the help of this func , it enforces https://github.com/openstack/keystone/blob/master/keystone/common/authorization.py#L130 | 07:53 |
vishakha | cmurphy: But I would like to know that credentials policies does not enforces with the help of this decorator. Is this is not consistent all over? | 07:54 |
*** mvkr has quit IRC | 07:55 | |
*** pcaruana has quit IRC | 07:55 | |
*** mvkr has joined #openstack-keystone | 07:57 | |
*** pcaruana has joined #openstack-keystone | 07:57 | |
*** shyamb has quit IRC | 07:59 | |
*** jdennis has quit IRC | 08:03 | |
cmurphy | vishakha: with the flask work we're in the middle of transitioning from using the @controller.protected decorator to using ENFORCER.enforce_call which is defined here http://git.openstack.org/cgit/openstack/keystone/tree/keystone/common/rbac_enforcer/enforcer.py#n246 | 08:05 |
cmurphy | so you're right it's not consistent | 08:05 |
*** Dinesh_Bhor has quit IRC | 08:05 | |
vishakha | cmurphy: Thanks for the information. | 08:08 |
*** Dinesh_Bhor has joined #openstack-keystone | 08:10 | |
*** jdennis has joined #openstack-keystone | 08:20 | |
*** Dinesh_Bhor has quit IRC | 08:21 | |
*** Dinesh_Bhor has joined #openstack-keystone | 08:26 | |
*** Dinesh_Bhor has quit IRC | 08:31 | |
*** ayoung has quit IRC | 08:36 | |
*** Dinesh_Bhor has joined #openstack-keystone | 08:38 | |
*** shyamb has joined #openstack-keystone | 08:46 | |
*** sapd1 has quit IRC | 09:02 | |
*** sapd1 has joined #openstack-keystone | 09:03 | |
*** shyamb has quit IRC | 09:29 | |
*** shyamb has joined #openstack-keystone | 09:30 | |
*** shyamb has quit IRC | 09:40 | |
*** Dinesh_Bhor has quit IRC | 10:08 | |
*** Dinesh_Bhor has joined #openstack-keystone | 10:21 | |
*** shyamb has joined #openstack-keystone | 10:27 | |
openstackgerrit | Vishakha Agarwal proposed openstack/keystone master: Adding 'date' for trust_flush https://review.openstack.org/607897 | 10:28 |
*** shyamb has quit IRC | 10:38 | |
*** shyamb has joined #openstack-keystone | 10:38 | |
*** dave-mccowan has joined #openstack-keystone | 11:02 | |
*** pcaruana has quit IRC | 11:02 | |
*** pcaruana has joined #openstack-keystone | 11:02 | |
*** Dinesh_Bhor has quit IRC | 11:17 | |
*** raildo has joined #openstack-keystone | 11:41 | |
*** shyamb has quit IRC | 11:44 | |
*** shyam89 has joined #openstack-keystone | 11:45 | |
*** mvkr has quit IRC | 13:02 | |
openstackgerrit | Merged openstack/keystone master: Organize project tag api-ref by route https://review.openstack.org/606874 | 13:14 |
*** shyam89 has quit IRC | 13:23 | |
*** mvkr has joined #openstack-keystone | 13:30 | |
*** Emine has joined #openstack-keystone | 13:34 | |
*** Emine has quit IRC | 13:44 | |
*** mchlumsky has joined #openstack-keystone | 13:45 | |
*** pcaruana has quit IRC | 13:50 | |
*** jaosorior has quit IRC | 14:16 | |
openstackgerrit | Colleen Murphy proposed openstack/keystone master: Clarify group-mapping example in docs https://review.openstack.org/607967 | 14:17 |
*** cfriesen has joined #openstack-keystone | 14:30 | |
*** pooja_jadhav has quit IRC | 14:30 | |
*** kukacz has quit IRC | 14:30 | |
*** kukacz has joined #openstack-keystone | 14:30 | |
*** pooja_jadhav has joined #openstack-keystone | 14:31 | |
*** shyamb has joined #openstack-keystone | 14:43 | |
*** itlinux has quit IRC | 14:48 | |
*** shyamb has quit IRC | 15:23 | |
*** itlinux has joined #openstack-keystone | 15:57 | |
*** gyee has joined #openstack-keystone | 16:00 | |
gyee | kmalloc, ayoung, sorry I missed your conversation yesterday. What do you need from me with regarding to federation? | 16:02 |
openstackgerrit | Merged openstack/keystone master: Update auto-provisioning example to use reader https://review.openstack.org/605496 | 16:13 |
*** pcaruana has joined #openstack-keystone | 17:17 | |
*** mvkr has quit IRC | 17:20 | |
kmalloc | gyee: not federation | 17:42 |
kmalloc | gyee: conversion of keystone auth from webob to flask | 17:42 |
kmalloc | gyee: it's a massive change, but that is because how auth touched everything and how even tests were baked into using the auth controllers | 17:43 |
kmalloc | https://review.openstack.org/#/c/603461/ | 17:43 |
kmalloc | >3000 lines, but needed since most everything really is tightly coupled | 17:43 |
kmalloc | the rest of the flask changes have been a good deal more isolated. | 17:43 |
*** Emine has joined #openstack-keystone | 17:51 | |
*** pcaruana has quit IRC | 18:01 | |
*** mvkr has joined #openstack-keystone | 18:09 | |
*** imacdonn has quit IRC | 18:22 | |
*** imacdonn has joined #openstack-keystone | 18:22 | |
*** aojea has joined #openstack-keystone | 18:36 | |
openstackgerrit | ayoung proposed openstack/keystone master: Route based Management Interface for AppCreds https://review.openstack.org/401808 | 20:44 |
*** mchlumsky has quit IRC | 20:58 | |
kmalloc | vishakha: within the next <short time period> @protected will be gone. | 20:59 |
kmalloc | vishakha: we only have 2-3 more APIs to port to flask and the old webob code will be ripped out, so it should be a bit more straightforward to see what is going on at that point | 21:00 |
openstackgerrit | ayoung proposed openstack/keystone master: Replace UUID with id_generator for Federated users https://review.openstack.org/605169 | 21:06 |
*** itlinux has quit IRC | 21:14 | |
*** itlinux has joined #openstack-keystone | 21:17 | |
*** itlinux has quit IRC | 22:05 | |
*** itlinux has joined #openstack-keystone | 22:22 | |
*** itlinux has quit IRC | 22:25 | |
*** rcernin has joined #openstack-keystone | 22:27 | |
*** aojea has quit IRC | 22:46 | |
*** edmondsw has quit IRC | 23:38 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!