Thursday, 2018-10-04

ayoungWell...most of it is.  Its not the cleanest chunk of code in Keystone00:00
kmallocwhich is previous controller code. it is what does all the logic of processing the auth request and passing through the auth plugins00:00
kmallocreally, 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 into00:00
ayoungI'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
ayoungand I do agree that it is now in the right spot00:01
kmallocunfortunatly, this is all very webob code00:01
ayoungthis is kinda like Trusts.  I'm not sure who else really can review it00:01
ayoungmaybe Guang if he returned from the ether00:01
kmallocso it would need to be totally reworked for flask anyway00:01
kmallocyeah gyee did some of the work here00:01
kmallocbut mostly auth flow has been you, me, steve, and previously dolph and henry (@protected)00:02
ayoungA good deal more than "some" and, while he is a good thinker, his code organization alwasy begged for followon cleanup00:02
ayounganyway, I'm going through it, and it looks ok.  I think the test changes are small enough that I am ok with them00:02
kmallocyeah the test changes annoyed me, but this is one of those cases where it's... well so tightly coupled00:03
ayoungyep00:03
kmalloci *could* split 2-3 things out but it's not a lot00:03
kmallocprobably wouild have saved ~100 lines of change total00:03
ayoungit was not until later that dolph really started pushing the "test things through the web interface" that got us out of these kinds of tests00:03
ayoung3600 line changes are painful00:04
kmalloci *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 merge00:04
kmallocthis is where gerrit tips over.00:04
kmallocunless i did it all without testing and then supplied a merge commit00:04
kmallocbut i don't think that is a lot better00:04
kmalloc(e.g. feature branch)00:05
ayoungprobably could have continued to pass a request object around00:05
kmallocthe merge commit would have been hell.00:05
ayoungbut those changes are trivial to review00:05
kmallocright. 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-federation00:05
kmalloctl;dr: I'm sorry, but thanks for reviewing it00:06
kmallocon the plus side, everything (inc. federation functional) is green except that one elastic-recheck caught thing.00:06
ayoungI'm ok with this.  I think it is easier as one big commit, TBH00:06
kmallocprojects and users next00:06
kmallocsuper light compared to this00:06
kmallocand then collapse all our internal middleware into "before-request" functions00:07
kmallocand i can rip out all the ugly webob / old wsgi stuff.00:07
kmallocthere is cleanup to do, but this has been really good imo overall00:07
ayoungIt has been a long time coming00:08
ayoungI remember the early spikes looking in to pecan etc00:08
ayoungdstank and jamielennox both wanted a cleanup like this back then00:08
ayoungheh00:08
ayoungdstank00:08
*** spartakos has quit IRC00:19
*** tristanC has quit IRC00:33
*** tristanC has joined #openstack-keystone00:33
*** dave-mccowan has quit IRC00:52
*** adriant has quit IRC00:54
*** gyee has quit IRC00:54
*** felipemonteiro has quit IRC00:55
*** adriant has joined #openstack-keystone01:00
*** felipemonteiro has joined #openstack-keystone01:13
*** felipemonteiro has quit IRC01:18
*** felipemonteiro has joined #openstack-keystone01:49
*** Dinesh_Bhor has joined #openstack-keystone01:57
*** Dinesh_Bhor has quit IRC02:05
*** cfriesen has quit IRC02:09
*** Dinesh_Bhor has joined #openstack-keystone02:19
*** pooja-jadhav has joined #openstack-keystone02:42
*** pooja_jadhav has quit IRC02:46
*** Dinesh_Bhor has quit IRC03:30
*** sapd1_ has quit IRC03:42
*** sapd1 has joined #openstack-keystone03:42
*** shyamb has joined #openstack-keystone03:45
*** felipemonteiro has quit IRC03:45
*** pooja-jadhav has quit IRC04:14
*** kukacz has quit IRC04:14
*** kukacz has joined #openstack-keystone04:16
*** pooja_jadhav has joined #openstack-keystone04:16
*** shyamb has quit IRC04:22
*** shyamb has joined #openstack-keystone04:22
vishakha@cmurphy , kmalloc I have a question regarding ec2 APIs.04:38
*** Dinesh_Bhor has joined #openstack-keystone04:38
kmallocSure.04:38
kmallocAsk away.04:38
vishakhakmalloc: 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
vishakhakmalloc: 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
vishakhakmalloc:  Does it mean that ec2 v3 APIs are not deprecated and will stay.?04:39
kmallocV3 is not deprecated04:40
kmallocV2 was missed in a deprecation notice, so they were kept another cycle04:40
kmallocThat note was meant to only impact v204:41
kmallocKeystone v2*04:41
vishakhakmalloc:  is there any plan to deprecate ec2 v3 also? or they will reamin as it is?04:42
vishakha*remain04:42
kmallocNo plans to deprecate04:43
vishakhakmalloc: thanks for the response04:43
kmallocNP :)04:43
*** felipemonteiro has joined #openstack-keystone04:50
*** shyamb has quit IRC05:11
*** shyamb has joined #openstack-keystone05:11
*** shyamb has quit IRC05:26
*** shyamb has joined #openstack-keystone05:53
openstackgerritMerged openstack/keystone master: Purge soft-deleted trusts  https://review.openstack.org/60497006:20
*** rcernin has quit IRC06:38
*** rcernin has joined #openstack-keystone06:38
*** shyamb has quit IRC06:38
*** pcaruana has joined #openstack-keystone06:40
*** shyamb has joined #openstack-keystone06:44
*** felipemonteiro has quit IRC06:53
*** rcernin has quit IRC07:10
openstackgerritVishakha Agarwal proposed openstack/keystone master: Implement scope_type checking for ec2 credentials  https://review.openstack.org/60782007:15
openstackgerritVishakha Agarwal proposed openstack/keystone master: [WIP] Implement scope_type checking for ec2 credentials  https://review.openstack.org/60782007:17
openstackgerritVishakha Agarwal proposed openstack/keystone master: [WIP] Implement scope_type checking for ec2 credentials  https://review.openstack.org/60782007:17
vishakhakmalloc : 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 IRC07:38
*** shyamb has joined #openstack-keystone07:38
vishakhacmurphy, ^^07:38
*** pjrusak has joined #openstack-keystone07:40
cmurphyvishakha: 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#n30207:43
vishakhacmurphy: I understood that with the help of this func , it enforces https://github.com/openstack/keystone/blob/master/keystone/common/authorization.py#L13007:53
vishakhacmurphy: 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 IRC07:55
*** pcaruana has quit IRC07:55
*** mvkr has joined #openstack-keystone07:57
*** pcaruana has joined #openstack-keystone07:57
*** shyamb has quit IRC07:59
*** jdennis has quit IRC08:03
cmurphyvishakha: 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#n24608:05
cmurphyso you're right it's not consistent08:05
*** Dinesh_Bhor has quit IRC08:05
vishakhacmurphy: Thanks for the information.08:08
*** Dinesh_Bhor has joined #openstack-keystone08:10
*** jdennis has joined #openstack-keystone08:20
*** Dinesh_Bhor has quit IRC08:21
*** Dinesh_Bhor has joined #openstack-keystone08:26
*** Dinesh_Bhor has quit IRC08:31
*** ayoung has quit IRC08:36
*** Dinesh_Bhor has joined #openstack-keystone08:38
*** shyamb has joined #openstack-keystone08:46
*** sapd1 has quit IRC09:02
*** sapd1 has joined #openstack-keystone09:03
*** shyamb has quit IRC09:29
*** shyamb has joined #openstack-keystone09:30
*** shyamb has quit IRC09:40
*** Dinesh_Bhor has quit IRC10:08
*** Dinesh_Bhor has joined #openstack-keystone10:21
*** shyamb has joined #openstack-keystone10:27
openstackgerritVishakha Agarwal proposed openstack/keystone master: Adding 'date' for trust_flush  https://review.openstack.org/60789710:28
*** shyamb has quit IRC10:38
*** shyamb has joined #openstack-keystone10:38
*** dave-mccowan has joined #openstack-keystone11:02
*** pcaruana has quit IRC11:02
*** pcaruana has joined #openstack-keystone11:02
*** Dinesh_Bhor has quit IRC11:17
*** raildo has joined #openstack-keystone11:41
*** shyamb has quit IRC11:44
*** shyam89 has joined #openstack-keystone11:45
*** mvkr has quit IRC13:02
openstackgerritMerged openstack/keystone master: Organize project tag api-ref by route  https://review.openstack.org/60687413:14
*** shyam89 has quit IRC13:23
*** mvkr has joined #openstack-keystone13:30
*** Emine has joined #openstack-keystone13:34
*** Emine has quit IRC13:44
*** mchlumsky has joined #openstack-keystone13:45
*** pcaruana has quit IRC13:50
*** jaosorior has quit IRC14:16
openstackgerritColleen Murphy proposed openstack/keystone master: Clarify group-mapping example in docs  https://review.openstack.org/60796714:17
*** cfriesen has joined #openstack-keystone14:30
*** pooja_jadhav has quit IRC14:30
*** kukacz has quit IRC14:30
*** kukacz has joined #openstack-keystone14:30
*** pooja_jadhav has joined #openstack-keystone14:31
*** shyamb has joined #openstack-keystone14:43
*** itlinux has quit IRC14:48
*** shyamb has quit IRC15:23
*** itlinux has joined #openstack-keystone15:57
*** gyee has joined #openstack-keystone16:00
gyeekmalloc, ayoung, sorry I missed your conversation yesterday. What do you need from me with regarding to federation?16:02
openstackgerritMerged openstack/keystone master: Update auto-provisioning example to use reader  https://review.openstack.org/60549616:13
*** pcaruana has joined #openstack-keystone17:17
*** mvkr has quit IRC17:20
kmallocgyee: not federation17:42
kmallocgyee: conversion of keystone auth from webob to flask17:42
kmallocgyee: it's a massive change, but that is because how auth touched everything and how even tests were baked into using the auth controllers17:43
kmallochttps://review.openstack.org/#/c/603461/17:43
kmalloc>3000 lines, but needed since most everything really is tightly coupled17:43
kmallocthe rest of the flask changes have been a good deal more isolated.17:43
*** Emine has joined #openstack-keystone17:51
*** pcaruana has quit IRC18:01
*** mvkr has joined #openstack-keystone18:09
*** imacdonn has quit IRC18:22
*** imacdonn has joined #openstack-keystone18:22
*** aojea has joined #openstack-keystone18:36
openstackgerritayoung proposed openstack/keystone master: Route based Management Interface for AppCreds  https://review.openstack.org/40180820:44
*** mchlumsky has quit IRC20:58
kmallocvishakha: within the next <short time period> @protected will be gone.20:59
kmallocvishakha: 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 point21:00
openstackgerritayoung proposed openstack/keystone master: Replace UUID with id_generator for Federated users  https://review.openstack.org/60516921:06
*** itlinux has quit IRC21:14
*** itlinux has joined #openstack-keystone21:17
*** itlinux has quit IRC22:05
*** itlinux has joined #openstack-keystone22:22
*** itlinux has quit IRC22:25
*** rcernin has joined #openstack-keystone22:27
*** aojea has quit IRC22:46
*** edmondsw has quit IRC23:38

Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!