16:35:32 <cmurphy> #startmeeting keystone-office-hours
16:35:32 <openstack> Meeting started Tue Jul  2 16:35:32 2019 UTC and is due to finish in 60 minutes.  The chair is cmurphy. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:35:34 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
16:35:36 <openstack> The meeting name has been set to 'keystone_office_hours'
16:35:46 <kmalloc> i just have to be able to transit between locations and airport/etc
16:35:49 <kmalloc> so, can't do video atm
16:36:12 <cmurphy> irc makes it easier for others to catch up anyways
16:36:29 <bnemec> o/
16:38:46 <lbragstad> oh - good call
16:39:07 <lbragstad> i suppose we should start here https://review.opendev.org/#/c/668523/2
16:39:36 <lbragstad> i should ask - is everyone ready?
16:39:40 * cmurphy ready
16:40:22 <lbragstad> alright - 668523 is trivial
16:40:25 <bnemec> Well, that one's easy. It's approved. :-)
16:40:31 <lbragstad> sweet
16:40:40 <lbragstad> next one is https://review.opendev.org/#/c/666085/9
16:40:57 <lbragstad> i wanted to keep the ksa logic separate from everything else
16:41:10 <lbragstad> nothing in this patch actually uses it, it's just laying the ground work
16:41:29 <lbragstad> one area of improvement i can think of though, is additional testing
16:41:42 <lbragstad> but it would likely have to be mocked pretty heavy in order to work
16:43:04 <bnemec> You mean unit testing?
16:43:19 <lbragstad> right - if we wanted to unit test the functionality of that patch
16:43:37 <lbragstad> i was going to bring this up in a subsequent review, since it's related to that, too
16:43:40 <bnemec> I'm a little inclined not to worry too much about it since we're expecting to iterate on the implementation quite a bit.
16:43:51 <bnemec> Which I realize violates every principle of TDD.
16:44:03 <lbragstad> yeah - i was also thinking about setting up a function test suite for this stuff, too
16:44:20 <bnemec> But I also don't want someone to spend a ton of time writing tests for code that we end up not using.
16:44:24 <lbragstad> (the last patch in the series touches on that a bit)
16:44:41 <bnemec> Yeah, that seems good too.
16:45:06 <bnemec> For now, maybe look at it as a prototype and just write the tests we absolutely need?
16:45:23 * bnemec feels so dirty for suggesting we not test code
16:45:30 <lbragstad> outside of testing, does anyone have questions about the connection logic itself?
16:47:59 <cmurphy> i've been wondering if there's a way around making the connection a global
16:48:04 <cmurphy> no specific suggestion though
16:48:32 <lbragstad> cmurphy do you want it to be a property of the Enforcer object instead?
16:48:36 <lbragstad> or something along those lines?
16:49:47 <cmurphy> maybe, but i suppose you might instantiate multiple enforcers and you wouldn't want to have that many live connections?
16:49:58 <lbragstad> probably not
16:50:42 <lbragstad> I could document it as an optimization
16:52:42 <cmurphy> i think it's probably fine as is
16:52:59 <kmalloc> pass in a connection, if none is passed in, attribute.
16:53:12 <kmalloc> s/attribute/property that is on-demand built
16:53:41 <kmalloc> it allows for the same mechanism as global without explicitly expecting global(s)
16:53:59 <kmalloc> can be iterated on down the line
16:54:05 <kmalloc> no reason to fix that now.
16:54:48 * lbragstad nodes
16:54:57 * lbragstad nods*
16:55:11 <lbragstad> alright - anything else on 666085
16:55:28 <bnemec> I left a comment about the endpoint id.
16:55:47 <lbragstad> psh - working ahead
16:55:48 <cmurphy> bnemec: good point about it not being the keystone service
16:56:35 <lbragstad> ++
16:57:19 <bnemec> I completely agree that region+service would be easier to handle in a deployment tool though.
16:57:45 <cmurphy> maybe worth optimizing in the future
16:58:05 <bnemec> I'd be curious to know what percentage of deployers are actually writing their own configurations vs. using Puppet/Ansible/Chef/etc.
16:58:17 <bnemec> That would inform which use case we should optimize for.
16:58:47 <cmurphy> i am reasonably sure almost no one deploys openstack without some kind of deployment tool
16:59:01 <lbragstad> yeah
16:59:15 <bnemec> I don't disagree, but I've been surprised before. ;-)
16:59:19 <lbragstad> my home lab is built using osa - and i don't even have customers
16:59:27 <cmurphy> lol
16:59:53 <bnemec> My home lab is built using devstack. Yes, I'm that guy. :-P
17:00:43 <lbragstad> tangent: it would be nice to have a baremetal box to pxe boot with a base image for baremetal devstack deployments
17:01:40 <lbragstad> alrighty - anything else on 666085 ?
17:01:52 <cmurphy> lgtm
17:02:03 <lbragstad> cool
17:02:10 <lbragstad> https://review.opendev.org/#/c/666444/7 is where things start to get interesting
17:02:13 <bnemec> FWIW, user survey says "Other Tool" is only 15% of the deployment tooling ecosystem.
17:02:50 <lbragstad> the main goal of 666444 is to just get the basic laid down and wait for actual implementation details of the models for another patch
17:03:30 <lbragstad> what i think would be really helpful in looking at this patch, is the relationship between the Enforcer and the models
17:03:50 <lbragstad> i want to keep the models separate (as their own objects that inherit an interface)
17:04:08 <lbragstad> but i found that there is a lot information you have to share between the enforcer and the models
17:05:11 <lbragstad> Enforcer.enforce() essentially passes a bunch of stuff through or needs to use composition to set various attributes on the model to limit method signatures with lots of arguments
17:07:15 <lbragstad> i tried to capture some of that information in Enforcer's doc strings
17:07:41 <bnemec> At first glance that semes to make sense. They are enforcement models, so it makes sense that they would be pretty tightly tied to the enforcer.
17:08:04 <lbragstad> yeah
17:08:25 <cmurphy> i wonder if there's a way we could reuse the keystoneauth plugin model for this, where it has a plugin class with a method class as an attribute https://opendev.org/openstack/keystoneauth/src/branch/master/keystoneauth1/identity/v3/password.py
17:08:30 <lbragstad> at the same time, i want people to be able to extend an interface if they want to add a new model (as opposed to extending Enforcer)
17:08:43 <lbragstad> cmurphy that's a good idea
17:09:55 <lbragstad> this is all internal stuff at this point
17:10:00 <lbragstad> so we can evolve it
17:10:03 <cmurphy> maybe Enforcer has self.enforcer = _FlatEnforcer and Enforcer.enforce() calls self.enforcer.enforce()
17:10:21 <cmurphy> and then project_id etc wouldn't need to be duplicated?
17:10:49 <cmurphy> i don't think the current way is bad though
17:10:55 <lbragstad> how would _FlatEnforces implementation get the project id in question?
17:11:08 <lbragstad> _FlatEnforcer's*
17:11:47 <cmurphy> hm i guess that doesn't really solve that problem
17:13:48 * lbragstad can document this, too
17:14:36 <bnemec> Like lbragstad said, this is all private stuff right now anyway. If people want to extend it to other enforcement models then maybe we can have a discussion over the best way to expose this then?
17:14:46 <cmurphy> ++
17:14:49 <lbragstad> true
17:14:51 <bnemec> Having a specific use case in mind would probably help with the design too.
17:14:57 <cmurphy> i can't really think of a better way to handle it at the moment
17:15:05 <lbragstad> at the same time - it works
17:15:11 <cmurphy> ++
17:15:13 <bnemec> Ship it!
17:15:22 <lbragstad> FILGTM!
17:15:27 <lbragstad> alright - moving on
17:15:29 <lbragstad> https://review.opendev.org/#/c/667452/3
17:16:01 <cmurphy> would probably be good to have tests for this one somehow
17:16:50 <lbragstad> yeah - so this is what i was kinda talking about ealier
17:16:52 <lbragstad> earlier*
17:17:14 <lbragstad> maybe what we can do is add a super *super* basic functional testing frame work in the ksa patch
17:17:44 <lbragstad> that only tests to make sure we can instantiate an Enforcer without the connection to keystone (in a devstack) deployment without it blowing up
17:18:05 <lbragstad> having that in place would make it a little easier to introduce functional tests here
17:18:12 <cmurphy> ++
17:18:28 <lbragstad> (i tested this logic, but i was using the script in the next patch set)
17:19:29 <bnemec> If a functional test framework could replace the test script that would be good.
17:19:36 <lbragstad> ++
17:19:57 <lbragstad> i feel like the natural evolution of that script is to just make it an official functional test
17:20:06 <bnemec> Yep
17:20:16 <lbragstad> cool
17:20:27 <lbragstad> anything else on this patch, outside of testing?
17:22:03 <cmurphy> it seems like a good first iteration to me, i'm sure down the road we can find ways to improve the enforce logic
17:22:25 * lbragstad nods
17:22:34 <bnemec> I probably just need to take a closer look. I got nothing useful done last week because downstream, so I haven't been through this part of the series yet. :-/
17:23:24 <lbragstad> ok - well it sounds like having functional testing there is useful so we can do that part in the meantime
17:23:29 <lbragstad> finally we have https://review.opendev.org/#/c/667242/7
17:23:35 <lbragstad> which is just documentation and a usage example
17:23:43 <bnemec> But like I've said before, I'm pretty much ready to +2 anything that gets us _an_ implementation. If it's horribly broken, we'll fix it. :-)
17:23:57 <lbragstad> some of that example is going to get pulled into a formal functional test thought
17:23:59 <lbragstad> though*
17:25:21 <bnemec> So to try that I would stand up a basic devstack, pull the series and install it, then run the example.py script?
17:25:40 <lbragstad> yeah
17:25:44 <bnemec> Tweaking the example.conf for my devstack, of course.
17:25:44 <cmurphy> you'd probably need to edit the project ids in the script
17:25:56 <lbragstad> you'll need to create a few limits and the registered limits
17:26:09 <lbragstad> and do what cmurphy said, update the project ids in the script
17:26:24 <bnemec> Makes sense.
17:26:33 <lbragstad> but yeah - it should work
17:26:51 <lbragstad> ok - so to summarize
17:27:17 <lbragstad> 666085 needs a functional testing framework and a bug open to investigate reusable ksa connections
17:27:41 <lbragstad> 666444 needs an open bug to investigate pluggable architectures for enforcement models
17:28:04 <lbragstad> 667452 needs functional testing (built on the action from 666085)
17:28:11 <lbragstad> anything else?
17:28:40 <cmurphy> i think this script will need to be tweaked for the two-level model?
17:28:54 <bnemec> Maybe a bug for 666085 to look at how to configure the endpoint?
17:29:17 <knikolla> cmurphy: sorry for missing the meeting. we're transitioning to the new adjutant based registration system and i've been swamped with work the past few weeks.
17:29:23 <lbragstad> yeah - i developed it to help me write the flat enforcement stuff, i wasn't really thinking about any of the strict-two level properties
17:29:31 <lbragstad> bnemec ++
17:29:40 <cmurphy> no worries knikolla
17:30:47 <lbragstad> outside of those actions, does anyone have anything else to add?
17:30:57 <cmurphy> lbragstad: and i guess next steps after all that would be implementing enforce() for the two-level model
17:31:10 <lbragstad> cmurphy correct
17:31:25 <cmurphy> fun :)
17:31:31 <lbragstad> i was thinking it would be easier to implement that once the functional testing is in place
17:31:40 <cmurphy> for sure
17:31:46 <lbragstad> then we could actually use TTD to get it done
17:31:54 <lbragstad> opposed to a hacktastic script
17:32:21 <bnemec> Yeah, right now we're mostly trying to define the public interface so other projects can start looking at how it would integrate with them.
17:32:35 <lbragstad> true
17:32:39 <bnemec> Additional enforcement models can be done in parallel.
17:32:47 <lbragstad> a lot of this actually came from johnthetubaguy
17:33:28 <lbragstad> i might have to poke someone to help with setting up zuul for the functional testing bits
17:33:40 <lbragstad> (we'll need a basic devstack in order to do that)
17:34:59 <bnemec> We have some functional tests in other oslo projects, so we might be able to help with that.
17:35:06 <lbragstad> sweet
17:35:51 <lbragstad> anything else to touch on for this?
17:35:59 <cmurphy> not from me
17:36:01 <lbragstad> otherwise - that's about all i had
17:36:31 <bnemec> I'm good too. Thanks again for driving this!
17:36:38 <cmurphy> ++ thanks lbragstad
17:36:46 <lbragstad> no problem - thanks for taking the time to go through the patches
17:36:55 <lbragstad> i'll write up a recap and send it to the ML
17:37:00 <cmurphy> sweet
17:37:10 <bnemec> dude
17:37:30 <cmurphy> :'D
17:37:30 <lbragstad> dude!
17:37:43 <cmurphy> #endmeeting