gyee | what we get back are DNs and attributes, those are independent of search scope | 00:00 |
---|---|---|
ayoung | if self.LDAP_SCOPE == ldap.SCOPE_ONELEVEL: | 00:00 |
ayoung | return self._id_to_dn_string(object_id | 00:00 |
ayoung | DNs are, but ID attribute is not | 00:01 |
gyee | I am not using the DN at all | 00:01 |
gyee | just straight attribute map | 00:01 |
ayoung | if self.LDAP_SCOPE == ldap.SCOPE_ONELEVEL: | 00:01 |
ayoung | if self.LDAP_SCOPE == ldap.SCOPE_ONELEVEL: | 00:01 |
ayoung | return self._id_to_dn_string(object_id | 00:01 |
ayoung | er... | 00:01 |
ayoung | if self.LDAP_SCOPE == ldap.SCOPE_ONELEVEL: | 00:02 |
ayoung | obj = self.model(id=self._dn_to_id(res[0])) | 00:02 |
ayoung | not | 00:02 |
gyee | that won't fix the attribute map problem as we are still making assumptions on the DN | 00:02 |
ayoung | if any([self.allow_create, self.allow_update, self.allow_delete]): | 00:02 |
ayoung | the assumptions are based on if self.LDAP_SCOPE == ldap.SCOPE_ONELEVEL: not read only, though | 00:03 |
gyee | how do we know its read-only? | 00:03 |
ayoung | gyee, it does not matter | 00:03 |
gyee | ayoung, I tested it with live LDAP and it works fine | 00:04 |
ayoung | if it is read only, and self.LDAP_SCOPE == ldap.SCOPE_ONELEVEL tjhe current logic is correct. If it is read-write...I don't think you can write to LDAP if there is SUBtree set | 00:04 |
ayoung | it doesn't know where to write the value | 00:04 |
gyee | what? we use scope to determine read-only? | 00:04 |
ayoung | no | 00:05 |
ayoung | use scope to determine if the id is in a field or in part of the DN | 00:06 |
ayoung | your logic is wrong | 00:06 |
ayoung | id is not in the id field | 00:06 |
gyee | search scope and DN are unrelated? | 00:06 |
gyee | DN ain't gonna change because of search scope | 00:06 |
*** marcoemorais has quit IRC | 00:07 | |
gyee | ayoung, look at the tests and tell me what I break | 00:07 |
*** marcoemorais has joined #openstack-keystone | 00:07 | |
ayoung | I think that you break the case where self.LDAP_SCOPE == ldap.SCOPE_ONELEVEL | 00:07 |
*** mfainberg_phone has joined #openstack-keystone | 00:08 | |
gyee | oneleve is the default config | 00:08 |
ayoung | did you run this against a live server? | 00:08 |
gyee | oh yeah | 00:08 |
ayoung | don't trust the unit tests. they use fake ldap | 00:08 |
gyee | did both one and sub | 00:08 |
gyee | I have OpenLDAP running on my dev vm | 00:08 |
gyee | that's how I confirmed the bug | 00:09 |
ayoung | you might have had a false negative then. If the id_filed matches what is in the attrbitue, but it is really using the DN section, you won't see a failure | 00:09 |
gyee | I have both configurations setup | 00:09 |
bknudson | ayoung: I think gyee is saying that the user ID isn't in the DN. | 00:10 |
openstackgerrit | Jamie Lennox proposed a change to openstack/keystonemiddleware: Always add auth URI to unauthorized requests https://review.openstack.org/119261 | 00:11 |
gyee | bknudson, right, and we can't make that assumption for read-only ldap | 00:11 |
ayoung | bknudson, user ID is in the DN if ONELEVEL | 00:11 |
*** mfainberg_phone has quit IRC | 00:11 | |
ayoung | and it is not in the DN if sub | 00:11 |
gyee | ayoung, no, that's the assumption we are making | 00:12 |
ayoung | bknudson, and if it happens to be that the DN section matches the attribute in the LDAP object, you won't know that you broke anything | 00:12 |
bknudson | ayoung: the dn is like cn=Brant,ou=IBM -- attrs are user_id: bknudson | 00:12 |
bknudson | and you want to use user_id as the user ID attribute. | 00:12 |
ayoung | bknudson, then do sub | 00:12 |
gyee | bknudson, amen brother | 00:12 |
bknudson | ayoung: does that work already? | 00:13 |
gyee | ayoung, how does sub help? your DN still the same | 00:13 |
ayoung | bknudson, yes | 00:13 |
ayoung | gyee, I'll link | 00:13 |
*** zzzeek has quit IRC | 00:13 | |
morganfainberg | bah just missed zzzeek | 00:14 |
morganfainberg | dang it | 00:14 |
ayoung | http://git.openstack.org/cgit/openstack/keystone/tree/keystone/common/ldap/core.py#n1255 bknudson gyee | 00:14 |
*** ncoghlan__ has joined #openstack-keystone | 00:15 | |
ayoung | bknudson, that code is based on a config file option. | 00:15 |
*** amcrn has quit IRC | 00:16 | |
gyee | ayoung, but we don't use that logic for lookup | 00:16 |
ayoung | If SCOPE_ONELEVEL then the id to dn conversion is done in process with not call to ldap | 00:16 |
ayoung | that code is called from | 00:17 |
gyee | ayoung, I follow the rabbit hole from here http://git.openstack.org/cgit/openstack/keystone/tree/keystone/common/ldap/core.py#n1427 | 00:17 |
*** gokrokve has quit IRC | 00:17 | |
ayoung | create...update...delete | 00:17 |
ayoung | enabled.... | 00:18 |
*** gokrokve has joined #openstack-keystone | 00:18 | |
bknudson | gyee: so you're saying the id in the result isn't the id that it's looking up in http://git.openstack.org/cgit/openstack/keystone/tree/keystone/common/ldap/core.py#n1255 | 00:18 |
gyee | for read-only LDAP they are all disabled | 00:18 |
*** ncoghlan_ has quit IRC | 00:18 | |
bknudson | it's more like line 1259 | 00:19 |
bknudson | it does a search with %(id_attr)s=%(id)s which uses user_id_attr | 00:19 |
bknudson | but then when it builds the user dict to return it's not going to use id_attr anymore, it uses the first DN value | 00:19 |
gyee | bknudson, http://git.openstack.org/cgit/openstack/keystone/tree/keystone/common/ldap/core.py#n1255 | 00:19 |
gyee | the get logic doesn't use id_to_dn map | 00:20 |
gyee | sorry I mean line 1427 | 00:20 |
*** marcoemorais has quit IRC | 00:21 | |
*** marcoemorais has joined #openstack-keystone | 00:21 | |
bknudson | _ldap_get still does %(id_attr)s=%(id)s | 00:21 |
bknudson | so it should find the right entry | 00:21 |
gyee | that logic is used for writable LDAP only | 00:21 |
gyee | right | 00:22 |
ayoung | bknudson, self._ldap_get(object_id, ldap_filter) should be called with the DN | 00:22 |
gyee | that's why I was explicitly checking for create, delete, and update | 00:22 |
gyee | like I said, I am not touching writable LDAP | 00:22 |
*** gokrokve has quit IRC | 00:22 | |
bknudson | keep it simple and don't mess with writable ldap | 00:22 |
gyee | right | 00:23 |
ayoung | this is a monster | 00:23 |
gyee | ayoung, I think there's room for improvement in our LDAP code, but one at a time man :) | 00:23 |
bknudson | gyee: if you're not touching writable ldap then why does https://review.openstack.org/#/c/117658/5/keystone/common/ldap/core.py have a change in create()? | 00:23 |
ayoung | no, I think this is broken and your code is just succeeding out of pure luck | 00:24 |
gyee | bknudson, because create() ignores the user_id_attribute map | 00:24 |
gyee | bknudson, if I don't do that, fakeldap's not happy | 00:24 |
gyee | we need that attribute in there | 00:24 |
gyee | ayoung, look at the tests and let me know which one is broken | 00:25 |
bknudson | gyee: I'm worried that someone might have the same attr for id_attr and another attr and then create() wouldn't work right. | 00:25 |
ayoung | gyee, here is what concerns me. And it is not your code | 00:26 |
ayoung | get takes in an ID | 00:26 |
ayoung | it does no transform and does an ldap lookup | 00:26 |
ayoung | query = (u'(&(%(id_attr)s=%(id)s)' | 00:26 |
gyee | bknudson, I don't see how that'll break stuff, we are using the user_id_attribute in the filter | 00:26 |
ayoung | however, all of the code that does actual morphs of the database uses a different piece of logic | 00:27 |
gyee | ayoung, I am only fixing read-only LDAP | 00:27 |
ayoung | conn.add_s(self._id_to_dn(values['id']), attrs) | 00:27 |
gyee | ayoung, if we don't have enough tests (unit or gate), than that's a different story | 00:27 |
ayoung | gyee, I understand that | 00:27 |
gyee | if I break stuff, the tests would've scream by now | 00:28 |
ayoung | but I don't think that Read/Write LDAP can function with the code as written | 00:28 |
ayoung | get uses different logic than create | 00:28 |
bknudson | gyee: what tests? we don't test enough | 00:28 |
bknudson | there's no read-only gate test or r/w gate test. | 00:28 |
gyee | bkudson, that's my point, we keep saying my changes break stuff, gimme evident | 00:28 |
ayoung | gyee, it is not your changes | 00:29 |
*** ncoghlan has joined #openstack-keystone | 00:29 | |
ayoung | I think the underlying code is broken, your changes just expose that | 00:29 |
ayoung | well, me looking at your code makes me suspect that | 00:29 |
*** ncoghlan_ has joined #openstack-keystone | 00:29 | |
gyee | without that change, we can't effectively support read-only LDAP, which is 90% of the use case if not more | 00:29 |
ayoung | gyee, its the fact that you two both read the code and say "read only is different from read write" | 00:29 |
ayoung | there was never logic in the code base that made that distinction | 00:29 |
gyee | ayoung, only reason I touch create was because of fakeldap | 00:30 |
ayoung | the only logic we have had is onelevel versus subtree, with onelevel being a requirement of read-write ldap | 00:30 |
gyee | create didn't take the user_id_attribute map into consideration | 00:30 |
ayoung | with good reason | 00:30 |
ayoung | user_id_attribute is where all this logic hinges | 00:31 |
gyee | onelevel and subtree are all we care, base doesn't get use at all | 00:31 |
bknudson | create builds the DN and doesn't do a search to figure out the DN | 00:31 |
ayoung | bknudson, right | 00:31 |
gyee | base are for schema lookup mostly | 00:31 |
bknudson | create doesn't work with subtree or onelevel | 00:31 |
bknudson | it just puts it where it wants it | 00:31 |
ayoung | bknudson, yes it does | 00:31 |
ayoung | create calss id_to_dn | 00:31 |
ayoung | http://git.openstack.org/cgit/openstack/keystone/tree/keystone/common/ldap/core.py#n1364 | 00:32 |
ayoung | http://git.openstack.org/cgit/openstack/keystone/tree/keystone/common/ldap/core.py#n1254 then id_to_dn checks | 00:32 |
bknudson | if it's created with cn=<id>,whatever , that will work with this code. | 00:32 |
bknudson | because the entry will get a cn: <id> attribute. | 00:32 |
*** ncoghlan__ has quit IRC | 00:32 | |
bknudson | although it might wind up being multivalued and then it would break | 00:32 |
gyee | huh? | 00:33 |
bknudson | but since this code is only used with read-only ldap you don't have to worry about create | 00:33 |
*** ncoghlan has quit IRC | 00:33 | |
gyee | bknudson, exactly | 00:33 |
ayoung | return self.conn.add_s(dn, modlist) | 00:33 |
ayoung | bknudson, my concern is that read only ldap is already broken | 00:34 |
ayoung | not that gyee is breaking it | 00:34 |
bknudson | ayoung: that's what gyee is fixing | 00:34 |
gyee | ayoung, only that part, we've been hammering it like crazy :) | 00:34 |
ayoung | er,, read-write ldap is broken | 00:34 |
gyee | testing it against both AD and OpenLDAP | 00:34 |
bknudson | read-write ldap is broken, agreed. | 00:34 |
ayoung | bknudson, my concern is that read-write ldap is broken | 00:34 |
gyee | that's how we discovered the mapping issue in the first place | 00:35 |
bknudson | it works for some extremely limited use | 00:35 |
gyee | our read-write logic make assumptions, which is fine as we have control | 00:35 |
gyee | but for read-only LDAP, we must honor the mappings | 00:35 |
ayoung | but I think that this code also is wrong for certain read-onluy cases | 00:36 |
ayoung | bknudson, if I have cn=ayoung,ou=cloud,cn=redhat,cn=com as my dn, but my object has CN=Adam it is broken | 00:36 |
gyee | ayoung, bknudson, let me know which test case to add, I'll be glad to do that | 00:36 |
ayoung | and that is a common case | 00:36 |
bknudson | ayoung: what's your user ID attr? cn? | 00:37 |
ayoung | CN in the DN is a different value than the CN attribute | 00:37 |
ayoung | bknudson, uid | 00:37 |
gyee | ayoung, right, we can't use multivalued attribute as IDs | 00:37 |
gyee | I check for that as well | 00:37 |
ayoung | sorry | 00:37 |
ayoung | bknudson, yes, my ID attr is CN and scope == onelevel | 00:37 |
bknudson | ayoung: as gyee mentioned, you wouldn't be able to use cn with his change because it's multivalued. | 00:38 |
bknudson | your entry would actually have 2 CNs | 00:38 |
ayoung | bknudson no | 00:38 |
gyee | for multivalued attributes, we just can't determine the ID | 00:38 |
bknudson | CN: Adam and cn: ayoung | 00:38 |
ayoung | bknudson, the CN=ayoung would be from the DN | 00:38 |
ayoung | DN is not really an attribute | 00:38 |
bknudson | the server will add it. | 00:39 |
ayoung | but it would be DN: CN=ayoung,.... | 00:39 |
morganfainberg | ayoung, so you'd use DN as the id attr then, and dn2str would be called? | 00:39 |
ayoung | morganfainberg, if only it were so simple | 00:39 |
bknudson | morganfainberg: DN isn't an attribute. | 00:39 |
morganfainberg | bknudson, oh.. right derp | 00:39 |
ayoung | morganfainberg, its a flipping mess | 00:40 |
bknudson | gyee: how about if the attribute is multivalued then use the first RDN? | 00:40 |
morganfainberg | we could just force DN to be an attr in the translation layer. :P | 00:40 |
gyee | bknudson, that's dangerous assumption | 00:40 |
bknudson | gyee: you could check if the RDN attr is the ID attr | 00:40 |
bknudson | gyee: then that would be the preferred value | 00:41 |
gyee | bknudson, only if RDN is returned in order | 00:41 |
ayoung | gyee, I think you need to replace the check in your code with a check for ONELEVEL | 00:41 |
gyee | which I am not sure if that's guaranteed | 00:41 |
bknudson | gyee: you could have a multi-valued RDN, but that's just weird. | 00:41 |
ayoung | that will fix part of the problem, but not all | 00:41 |
ayoung | but you don't get an attribute for ONELEVEL, so you need additional code to populate that | 00:41 |
gyee | bknudson, I think we should error out for multivalued ID map | 00:42 |
ayoung | don't do your logic based on read-only, do it based on the query strategy | 00:42 |
gyee | ayoung, no, I need to be able to support sub search | 00:42 |
*** stevemar has joined #openstack-keystone | 00:42 | |
ayoung | gyee, that is the easy one | 00:42 |
gyee | I think think my changes have any dependency on one vs sub | 00:42 |
gyee | sub search is very important for read-only LDAP | 00:43 |
ayoung | its the other case I'm worried about | 00:43 |
ayoung | your code is correct for SUB | 00:43 |
ayoung | it is not correct for ONELEVEL | 00:43 |
ayoung | but I think that get(obejct_id) is also not correct for one level | 00:43 |
ayoung | and that really worries me | 00:43 |
gyee | it works for onelevel as well, I verified both configuration with OpenLDAP | 00:44 |
ayoung | what calls get(object_id)... | 00:44 |
gyee | GET /users/user_id and GET /users | 00:44 |
ayoung | by luck, I think, gyee, not by design | 00:45 |
gyee | ayoung, I don't believe in luck :) | 00:45 |
ayoung | gyee, ok, than due to the fact that your DN matched the cn field in your ldap | 00:45 |
gyee | sheeeit, I loose money everytime I go to casino | 00:46 |
*** Ugallu has quit IRC | 00:46 | |
bknudson | gyee: it all goes to topol | 00:46 |
gyee | ayoung, I have cn=Guang Yee,ou=users,dc=acme,dc=com, and user_id_attribute=uid | 00:47 |
ayoung | gyee, in a read-write setup? | 00:47 |
gyee | ayoung, no, I don't use read-write | 00:47 |
gyee | just read-only | 00:47 |
ayoung | gyee, ok, so in that case it will work if you look up the user by LDAP uid as the user_id | 00:48 |
ayoung | even if it is set to onelevel | 00:48 |
ayoung | and it is not supposed to | 00:48 |
ayoung | it is supposed to use cn=Guang Yee asthe user id | 00:49 |
gyee | but the uid is not part of DN | 00:49 |
ayoung | gyee, right. it should be | 00:49 |
ayoung | looking for | 00:49 |
morganfainberg | ayoung, bknudson, i *thought* that if cn is part of the DN it can't be multi-value | 00:49 |
ayoung | uid=gyee,ou=users,dc=acme,dc=com, | 00:49 |
morganfainberg | bknudson, ayoung, for example. | 00:49 |
morganfainberg | multi RDN always sound like it would be broken. | 00:50 |
bknudson | morganfainberg: no, you can have multiple CNs and have one of them in the DN | 00:50 |
gyee | morganfainberg, it can be, we don't have control over how read-only LDAPs are deployed | 00:50 |
ayoung | morganfainberg, when I wrote this code, I thought that cn=ayoung,ou=users,dc=acme,dc=com implied that the cn=ayoung attribute would be set on the user | 00:50 |
gyee | ayoung, you want to add a test case for me, because I don't think I get what the issue is | 00:50 |
ayoung | and my guess is that the reason this works at all is because everyone runnning with read-write ldap is doing that | 00:50 |
ayoung | which is probably like three organizations | 00:50 |
morganfainberg | bknudson, hm. there was something similar i tried ... but the attr for the DN wasn't multi-able. but... i might be misremebering the use case | 00:51 |
ayoung | gyee, you've exposed a bug | 00:51 |
morganfainberg | or it was alreadya non-multi attr. | 00:51 |
ayoung | but not one that I can see how to fix | 00:51 |
gyee | ayoung, yay, I earned karma points | 00:51 |
ayoung | gyee++ | 00:51 |
*** marcoemorais has quit IRC | 00:51 | |
ayoung | we don't have a karma bot, sorry | 00:51 |
gyee | heh | 00:51 |
ayoung | I'm going home. | 00:51 |
gyee | ayoung, uh, the -1? | 00:52 |
ayoung | gyee, I don't know what to do about your patch | 00:52 |
morganfainberg | ayoung, make our ldap core check if one of the items from the attr is in the DN (we can do that) and if it is in the DN use it as the RDN instead of the non-RDN one | 00:52 |
ayoung | I need to think about it | 00:52 |
morganfainberg | ayoung, otherwise bail on multi-attrs | 00:52 |
gyee | ayoung, trust me, the shit works | 00:52 |
ayoung | morganfainberg, yes.... | 00:52 |
morganfainberg | ayoung, shouldn't be awful to do that. | 00:52 |
bknudson | I'll mark gyee's karma point on my whiteboard. | 00:52 |
gyee | ha | 00:53 |
ayoung | morganfainberg, I'm not certain we know that information when we do the transform, though | 00:53 |
morganfainberg | ayoung, it might take reworking, but we can ensure that info is available, ldap core *can* be made smart enough, just would be lowlevel smarts - we do have the DN, it's a question of using it and making sure it propagates up | 00:54 |
ayoung | morganfainberg, I think I would rather make the RDN case an explicit config value | 00:54 |
morganfainberg | ayoung, not opposed to that either tbh, that would work as well. | 00:54 |
ayoung | LDAP.user_id_in_dn or something | 00:54 |
ayoung | morganfainberg, I think it is the only way out of the mess... | 00:54 |
morganfainberg | gyee, moved my score to +1 for the mapping, but down from +2 due to this convo | 00:54 |
morganfainberg | ayoung, i think either impl would work tbh, it's a question of which knob we want deployers to need to know to turn | 00:55 |
bknudson | morganfainberg: ayoung: I'm not sure what situation you're worried about breaking. | 00:55 |
gyee | morganfainberg, I am not convinced anything is broken, we need test cases | 00:55 |
bknudson | is it just the multi-valued? | 00:55 |
ayoung | morganfainberg, lets ask marekd and the CERN folks. I know of very few other people running the LDAP writable code | 00:55 |
morganfainberg | bknudson, i think so | 00:55 |
bknudson | we're not changing the writeable case. | 00:55 |
gyee | if anything breaks, show me a test case | 00:56 |
morganfainberg | bknudson, and if we resolve this convo that we're not concerned i'll re up to +2. just don't want to make it look like it can merge | 00:56 |
ayoung | bknudson, we are making logic based on writeable versus not-writable. That distinction did not exist before | 00:56 |
ayoung | at least not explicitly | 00:56 |
bknudson | we should have. | 00:56 |
ayoung | if nothing else, that check is exposing a bug | 00:56 |
gyee | ayoung, can you add a test case in that patch so I know what's broken? | 00:56 |
ayoung | gyee, no I can't | 00:57 |
gyee | then I don't know what to fix | 00:57 |
ayoung | gyee, I thought I could, but I am too drainbead to do so | 00:57 |
gyee | first rule of fixing a bug is to reproduce the bug | 00:57 |
gyee | ayoung, that's fine, whenever you have time, appreciated | 00:58 |
ayoung | gyee, when you look at code and say "that should not work" and you know it does, it implies something is wrong. That is what happened here. I believe that, empircally, you showed your code works for the cases you tested. I also know that the logic between to parts of ldap is different that should be the same | 00:58 |
ayoung | I think your code just avoids the problem cases | 00:59 |
ayoung | but it might be that the problem cases are dealt with through pure,dumb luck on my part | 00:59 |
gyee | ayoung, then I think that should be a separate fix | 00:59 |
ayoung | gyee, but the logic in your code based on "writable" should not be there | 00:59 |
ayoung | I understand it is you being cautious | 00:59 |
*** dims has joined #openstack-keystone | 00:59 | |
ayoung | what happens if you remove that check and that block? | 01:00 |
gyee | ayoung, I bet if I remove that check and the tests still happy | 01:00 |
ayoung | the fact that you had to code around it in the create case indicates that something is wrong | 01:00 |
ayoung | and that is why, right now, I can't change it away from -1 | 01:00 |
bknudson | gyee: in the writable case we're saying we have control of the entries, so maybe you don't have to worry about it. | 01:01 |
gyee | ayoung, anyway, show me a test case, I'll fix whatever | 01:01 |
gyee | bknudson, right, I am not touching the writable part | 01:01 |
ayoung | bknudson, I think, actually, that I might force the user_id field to be set to the same value as the RDN value | 01:01 |
bknudson | ayoung: the LDAP server does that already | 01:02 |
bknudson | except maybe there's some old OpenLDAP that doesn't | 01:02 |
ayoung | bknudson, no, I mean when we write the value from Keystone. But no, an LDAP server would let you create aa DN with CN=ayoung,cn=redhat but an attribute of cn=Adam | 01:02 |
bknudson | ayoung: yes, but it would add the CN=ayoung to the entry. | 01:03 |
ayoung | its just that the writable LDAP code makes an assumption that just happens to work | 01:03 |
bknudson | so you'd have 2 CNs | 01:03 |
ayoung | bknudson, and that would be fine for search | 01:03 |
*** dims has quit IRC | 01:03 | |
ayoung | because it would be searching for the one that had CN=ayoung ... | 01:03 |
bknudson | not with gyee's code. | 01:03 |
ayoung | right | 01:03 |
ayoung | gyee, I really do apologize for this code | 01:04 |
*** dims has joined #openstack-keystone | 01:04 | |
bknudson | the problem is we provided a r/w ldap. should have stopped at read-only | 01:04 |
ayoung | bknudson, wasn't an option | 01:04 |
ayoung | bknudson, identity used to contain tenants | 01:04 |
gyee | I haven't come across any requirement for writable LDAP yet | 01:04 |
ayoung | gyee, cuz I fixed that | 01:05 |
gyee | maybe just CERN | 01:05 |
ayoung | remember splitting identity | 01:05 |
gyee | definitely not enterprise customers | 01:05 |
*** david-lyle has joined #openstack-keystone | 01:05 | |
ayoung | gyee, without writable LDAP in the past, you could not have new projects | 01:05 |
ayoung | but...water under the washed out bridge | 01:05 |
*** dims_ has joined #openstack-keystone | 01:06 | |
bknudson | the LDAP admin can create entries for projects | 01:06 |
ayoung | gyee, anyway...I'll take another look at it tomorrow with a fresh set of eyes, and I'll help you come to a conclusion | 01:06 |
bknudson | or just use groups or something | 01:06 |
ayoung | bknudson, I did use groups to begin with | 01:06 |
gyee | ayoung, thanks | 01:06 |
ayoung | the horror, the horror | 01:06 |
bknudson | the role assignments are a real horror | 01:06 |
ayoung | gyee, I think that you can just remove the if and the top block. Can you start by testing just that? | 01:07 |
gyee | remove the create, delete, and update check? | 01:07 |
ayoung | yes, and the code below it | 01:07 |
*** dims has quit IRC | 01:08 | |
gyee | k, lemme give it a shot | 01:08 |
ayoung | gyee, just test it and let me know what you find. I'm goingto head home. | 01:09 |
gyee | ayoung, will do | 01:10 |
*** ayoung has quit IRC | 01:13 | |
*** dims_ has quit IRC | 01:14 | |
openstackgerrit | A change was merged to openstack/keystone: use one indentation style https://review.openstack.org/118894 | 01:14 |
*** dims has joined #openstack-keystone | 01:14 | |
*** david-lyle has quit IRC | 01:16 | |
gyee | bknduson, morganfainberg, ayoung, yeah, we suck, we use multivalued attributes in our tests | 01:18 |
gyee | for cn | 01:18 |
*** dims has quit IRC | 01:19 | |
*** gokrokve has joined #openstack-keystone | 01:19 | |
gyee | cn=Doe\\5c, John,ou=Users,cn=example,cn=com | 01:20 |
*** dims has joined #openstack-keystone | 01:20 | |
bknudson | gyee: if it's multivalued then use the value from the DN like it was before. | 01:20 |
gyee | if I change it to cn=Doe\\5c, John,ou=Users,dc=example,dc=com, everything's happy | 01:20 |
gyee | bknudson, yeah, I don't think I have much choice | 01:20 |
gyee | either that or changes the tests | 01:21 |
bknudson | if the tests are wrong then fix the tests | 01:21 |
gyee | bknudson, I don't want to touch the tests, we are not wrong, but I doubt anybody would deploy the LDAP that way | 01:21 |
gyee | for domains, usually dc= | 01:22 |
gyee | not cn= | 01:22 |
gyee | that's what AD and OpenLDAP uses by default | 01:22 |
bknudson | gyee: I don't understand why changing it to cn from dc would make a difference for the tests | 01:23 |
gyee | bknudson, because dn would not longer multivalued | 01:23 |
bknudson | ?? | 01:23 |
gyee | right now, if DN is cn=Doe\\5c, John,ou=Users,cn=example,cn=com, the cn attribute contains ['Doe\\5c, John', 'example', 'com'] | 01:23 |
bknudson | really? | 01:24 |
gyee | cn returned from the search | 01:24 |
gyee | which is expected | 01:24 |
bknudson | that's f'ed up. | 01:24 |
stevemar | jamielennox, ping | 01:24 |
bknudson | why is that expected? | 01:24 |
gyee | sorry, expected from our fakeldap logic | 01:24 |
bknudson | that's just broken | 01:24 |
stevemar | jamielennox, actually... give me a few minutes, need to get my links all together | 01:24 |
bknudson | it's fakeldap it's not supposed to be brokenldap | 01:25 |
gyee | hehe | 01:25 |
gyee | yeah man, lemme see if I can go through that hairball | 01:25 |
* gyee feels like Alice in Wonderland | 01:26 | |
bknudson | you're down the rabbit hole now | 01:26 |
gyee | heh | 01:26 |
jamielennox | stevemar: ready when you are | 01:26 |
gyee | ah, fuggit, I'll look after dinner | 01:27 |
*** tqtran has quit IRC | 01:39 | |
*** ncoghlan__ has joined #openstack-keystone | 01:41 | |
*** yasukun has joined #openstack-keystone | 01:43 | |
*** ncoghlan_ has quit IRC | 01:45 | |
*** dims has quit IRC | 01:46 | |
*** dims has joined #openstack-keystone | 01:47 | |
*** dims_ has joined #openstack-keystone | 01:49 | |
*** dims has quit IRC | 01:52 | |
*** ayoung has joined #openstack-keystone | 01:54 | |
*** dguitarbite has joined #openstack-keystone | 01:55 | |
*** diegows has quit IRC | 01:56 | |
*** david-lyle has joined #openstack-keystone | 01:56 | |
*** diegows has joined #openstack-keystone | 01:58 | |
*** ocho has quit IRC | 02:02 | |
*** david-lyle has quit IRC | 02:02 | |
stevemar | jamielennox, so i was gonna ping you about the horizon / kerberos stuff that was on the ML | 02:10 |
jamielennox | stevemar: yea, i got to the end of that email and nearly didn't send it | 02:11 |
jamielennox | i think ayoung was talking much more about the token fetching process than the how to fetch a token | 02:11 |
stevemar | jamielennox, haha, don't worry, i just wanted a short sounding board. | 02:11 |
stevemar | short discussion / sounding board* | 02:11 |
jamielennox | ok - wasn't sure how shortening it helped | 02:12 |
stevemar | jamielennox, so from my (selfish) PoV, i just want the federation stuff to work, kerberos is an afterthought | 02:12 |
stevemar | jamielennox, i think you totally nailed it wrt a function to list IdPs | 02:13 |
jamielennox | so this has come up internally a few times how we want this to work, and my suggestion is just list what's available and let other people do the smarts | 02:13 |
jamielennox | kerberos is a bit of a problem child | 02:13 |
stevemar | and AFAIK, once the user inputs her username / password, it sends off a request to this function in DOA: https://github.com/openstack/django_openstack_auth/blob/0a0db68468a5a9a56c0f57b9b96eaa008b4b0d10/openstack_auth/backend.py#L86-L98 | 02:13 |
jamielennox | phh, obviously something stupid | 02:14 |
*** diegows has quit IRC | 02:14 | |
jamielennox | uses v3 though, that's kinda good | 02:14 |
stevemar | jamielennox, i was thinking that if the user ends on selecting an idp, then we could use the samlunscopedtoken plugin we already have | 02:15 |
stevemar | hopefully meaning that we don't need any new html (aside from maybe a drop down box) | 02:15 |
stevemar | yep, v3 is good | 02:15 |
jamielennox | lol - i'm not much of a designer but we can probably do better than a drop down box | 02:15 |
jamielennox | so if we have a way of listing idps then we should have a way to determine from that which plugin would be required for that idp | 02:16 |
jamielennox | some sort of hints, not sure exactly what | 02:16 |
jamielennox | same as if we have a way to query available 'method': ['xxx'] values | 02:17 |
stevemar | jamielennox, agreed, i'm just trying to connect the dots | 02:18 |
jamielennox | from that i *think* most of what horizon has to do is provide a nice way of fetching user details or redirecting to places and match up the plugin to the idp | 02:18 |
stevemar | jamielennox, so i'm wondering if i'm missing any other dots here? | 02:18 |
ayoung | stevemar, I'm here now | 02:19 |
jamielennox | right - there are obstacles, saml2 assumes ECP but none of that is a real problem | 02:19 |
stevemar | ayoung, i gotta check out your reviews for DOA | 02:20 |
jamielennox | stevemar: unrelated https://etherpad.openstack.org/p/token2saml have a look at dolphm's comments | 02:20 |
ayoung | stevemar, don't rush. I've got to rework them basd on what I learned/discussed today | 02:21 |
stevemar | ayoung, i was thinking plugins would have been used | 02:21 |
dstanek | bknudson: what is the point of _LI vs. _? | 02:21 |
ayoung | stevemar, sort of. I think that the short solution is that if the Horizon URL is kerberized, we'll assume we need to do kerberos to Keystone as well | 02:22 |
ayoung | stevemar, mod_auth_kerb sets a bunch of vars | 02:22 |
ayoung | stevemar, this ugly code you can see checks for https://review.openstack.org/#/c/115463/1/openstack_auth/backend.py,cm https://review.openstack.org/#/c/115463/1/openstack_auth/backend.py,cm | 02:23 |
ayoung | os.environ['KRB5CCNAME'] = request.META['KRB5CCNAME'] | 02:23 |
ayoung | stevemar, so I don't think DOA will have plug ins so much as it will have logic to handle the auth mechansim it determines needs to be used | 02:24 |
ayoung | the _auth_ mechanism in Django is supposed to be the plugin arch itself | 02:24 |
ayoung | jamielennox, where would the "supported mechanisms" url live in your proposal? | 02:25 |
jamielennox | ayoung: can we expose plugins to a higher level that that? | 02:25 |
*** dims_ has quit IRC | 02:25 | |
ayoung | jamielennox, ah, I see, under /auth | 02:25 |
jamielennox | not sure, i think one part of GET /auth | 02:25 |
ayoung | GET /auth would list them.... | 02:25 |
*** dims has joined #openstack-keystone | 02:25 | |
jamielennox | there are other things i think would want to be there as well | 02:26 |
ayoung | and then you would see /auth/kerb and /auth/password? | 02:26 |
ayoung | or a map that implies that same info | 02:26 |
ayoung | jamielennox, " can we expose plugins to a higher level that that?" what do you mean? | 02:26 |
jamielennox | ayoung: i'm not sure yet, having /auth/password is a fairly large departure from our current | 02:26 |
*** harlowja is now known as harlowja_away | 02:27 | |
jamielennox | however we need to be able to support having /kerb and others on a different URL that straight /auth/tokens | 02:27 |
ayoung | jamielennox, right....I feel like we are doing stuff specific to Keystone that is already handled by the HTTP protocol. THis is Negotiate type stuff. | 02:27 |
jamielennox | i don't know when negotiate is used outside of kerberos | 02:28 |
ayoung | /kerb really is due to shortcomings of the Apache auth model: doesn't allow fallback | 02:28 |
ayoung | jamielennox, if you hit a page with no auth you get a list of support auth mechanisms | 02:28 |
jamielennox | it's a problem of the clients as well, requests-kerberos at least has no way to send a ticket opportunistically - ie without getting a 401 first | 02:29 |
ayoung | 401 Unauthorized " The response must include a WWW-Authenticate header field containing a challenge applicable to the requested resource." | 02:29 |
jamielennox | ayoung: well timed then: https://review.openstack.org/#/c/119261/ | 02:30 |
*** dims_ has joined #openstack-keystone | 02:30 | |
*** dims has quit IRC | 02:30 | |
ayoung | jamielennox, I think I like that one... | 02:31 |
ayoung | yep, I'm sure of it | 02:31 |
jamielennox | ok, so for keystone though we need a way to discover this stuff and probably not just on 401 | 02:32 |
ayoung | right | 02:32 |
jamielennox | this is another reason for the unscoped catalog as well | 02:32 |
ayoung | the thing is, for federation, doesn't it depend on the IdP | 02:32 |
ayoung | No, unscoped catalog is after authentication, too late for this | 02:33 |
jamielennox | i can only think that GET /auth would include a link to GET /OS-FEDERATION/idp if federation is enabled | 02:33 |
ayoung | jamielennox, what if we treated keystones internal IdP as just another federated IdP? | 02:33 |
jamielennox | ... what if keystone didn't have an internal idp .. | 02:34 |
jamielennox | i know - done to death | 02:34 |
ayoung | jamielennox, 3 classes of users: employees, partners, customers | 02:34 |
jamielennox | so you want to actually mount it behind /OS-FEDERATION or redeisgn /auth | 02:34 |
ayoung | for customer (people that pay us to use our cloud) we need a way to create them | 02:34 |
ayoung | I think /OS-FEDERATION needs to be core, so /auth | 02:35 |
jamielennox | ayoung: right - that doesn't need to be *internal* though | 02:35 |
ayoung | jamielennox, there are other issues, too | 02:35 |
ayoung | Federation for partners and customer | 02:36 |
ayoung | you don't want to advertise to everyone who is using your cloud | 02:36 |
ayoung | Pepsi should not look at the customer list and see Coke in there | 02:36 |
jamielennox | i think we've mixed up ideas here | 02:37 |
jamielennox | back on topic then - so how would we list out available auths | 02:37 |
jamielennox | or more rightly how would we manage them | 02:37 |
jamielennox | so if /auth gave you /auth/password /auth/kerb etc | 02:38 |
jamielennox | federation has create idp, create protocol, fix up apache based on all this | 02:38 |
jamielennox | brb | 02:38 |
ayoung | jamielennox, the question is how do we list available auths for a given IdP | 02:39 |
ayoung | for employees that is LDAP | 02:39 |
ayoung | for Partners it is Federation | 02:39 |
ayoung | for customers it is SQL in Keystone | 02:39 |
ayoung | now, I'ld like to convert SQL also to LDAP | 02:39 |
ayoung | but they are both fronted by the existing Keystone APIs | 02:39 |
ayoung | So /auth really needs to list the mechanism that keystone the token-generator accepts. | 02:40 |
ayoung | that would be /kerb /password and /x509 | 02:40 |
ayoung | and then /SAML or maybe just /Fed | 02:41 |
stevemar | jamielennox, see i was thinking /auth would just list the auth methods in keystone.conf | 02:46 |
*** richm has quit IRC | 02:46 | |
stevemar | ayoung, what exactly does 'kerberizing' a url mean? | 02:47 |
stevemar | it makes me think that horizon is going to have a dependency on kerberos and that makes me sad | 02:47 |
ayoung | stevemar, not a dependency | 02:47 |
ayoung | just a potential to support | 02:47 |
ayoung | the Kerberos config is done at the Apache level | 02:47 |
ayoung | stevemar, I (of course) have a blog post about it | 02:48 |
ayoung | let me find it | 02:48 |
ayoung | stevemar, http://adam.younglogic.com/2014/07/kerberos-for-horizon-and-keystone/ | 02:48 |
ayoung | about 2/3rds of the way through that blog post I have an sample apache config | 02:49 |
ayoung | look for <Location "/dashboard/auth/login/"> | 02:49 |
jamielennox | stevemar: kind of like federation you need an apache module so you can't just use /auth/token | 02:49 |
stevemar | hmmm, ok | 02:50 |
stevemar | ayoung, jamielennox so how does this fit together with your keystone cops stuff? | 02:51 |
stevemar | i guess that question is more ayoung than jamielennox | 02:51 |
ayoung | stevemar, cops will be a follow on approach | 02:51 |
ayoung | it needs CORS support and I think that is going to be non-trivial | 02:51 |
stevemar | gotcha | 02:51 |
ayoung | cops will be the long term solution though | 02:51 |
ayoung | more correctly, Horizon will be rewritten as a single page app and make requests direct to the services out of the browser | 02:52 |
stevemar | so this current approach - just playing around with the DOA backend.py file, and loading /auth /idps, we should call it something | 02:52 |
ayoung | DOAK | 02:52 |
jamielennox | so do we expect for the stnadard password authentication exchange people would GET /auth find the url for password and then go there? | 02:52 |
ayoung | pronounces DOAK | 02:52 |
stevemar | cause I want it to clearly distinguish itself from that webSSO spec we had a while ago | 02:52 |
ayoung | stevemar, I think the COPS apporach will incorporate webSSO | 02:53 |
stevemar | but i'm thinking K release, not O or P (or whenever horizon switches to CORS) | 02:53 |
ayoung | stevemar, I think we can do CORS in Kilo | 02:54 |
stevemar | i thought you said it depends on horizon being re-written? | 02:54 |
ayoung | stevemar, for CORS we can do magic in Auth token middleware that allows all service/endpoints to support OPTIONS | 02:54 |
ayoung | no, other way around | 02:54 |
ayoung | rewriting Horizon requires CORS. We have to be first | 02:55 |
stevemar | ayoung, is OPTIONS something specific, or did you just capitalize for fun? | 02:56 |
jamielennox | so i only know the basics here but CORS is like ~50 lines of middleware in keystone right? | 02:56 |
ayoung | OPTIONS is an HTTP verb that all of the services need to support for CORS | 02:56 |
ayoung | jamielennox, about that | 02:56 |
stevemar | gotcha | 02:56 |
jamielennox | possibly even a general middleware so that horizon can do the same thing with other services | 02:56 |
stevemar | ayoung, agree or disagree, we would need to change the horizon login screen? to show the idps a user can connect to? | 02:57 |
ayoung | stevemar, not sure | 02:57 |
ayoung | stevemar, for Federation...proba bly? | 02:57 |
stevemar | ayoung, why would you be opposed? | 02:57 |
jamielennox | ayoung: going beyond kerberos to federation though - surely | 02:57 |
stevemar | yeah | 02:58 |
ayoung | yeah, and I think that for Federation is should be something like: | 02:58 |
ayoung | here are list of known supported Idps, or enter your own URL here: | 02:58 |
ayoung | not sure if that is the Horizon login screen or what, but, yeah, we need a page flow for it | 02:58 |
jamielennox | surely we want something like https://sites.google.com/site/oauthgoog/UXFedLogin/summary | 02:58 |
jamielennox | option 1 | 02:59 |
stevemar | jamielennox, what would the buttons be? | 02:59 |
ayoung | jamielennox, something like.... | 03:00 |
jamielennox | ah, right this is what you meant by we can't necessarily publicly list our partner idps | 03:00 |
ayoung | not sure if the list of supported Federated buttons needs to be there, but yeah | 03:00 |
ayoung | right | 03:00 |
ayoung | so we need a "provide your own link" field and then we can say "sorry, not supported" | 03:01 |
jamielennox | ayoung: well that was part of the idea, to explicity say which one do you want to log in with | 03:01 |
ayoung | I've seen that in other openid based work flows | 03:01 |
ayoung | yep....and I don;t know that this is just our problem to solve....cross project meeting with Horizon at the summit for sure | 03:01 |
jamielennox | i don't want to have idp urls as part of a login flow | 03:01 |
ayoung | http://www.joyoftech.com/joyoftech/index.html | 03:02 |
ayoung | ah, cloud security | 03:02 |
openstackgerrit | wanghong proposed a change to openstack/keystone: remove default check keys in assertValidEntity https://review.openstack.org/112573 | 03:03 |
ayoung | jamielennox, I think it needs to be there, but once the user enters it once, we can remember in a cookie | 03:03 |
*** ncoghlan__ is now known as ncoghlan_afk | 03:03 | |
jamielennox | maybe we need to scope a horizon instance to a domain | 03:04 |
*** dims_ has quit IRC | 03:04 | |
jamielennox | ugh - that doesn't mean anything here - ignore that | 03:04 |
stevemar | jamielennox, yeah, i'm not liking the urls there either | 03:05 |
*** dims has joined #openstack-keystone | 03:05 | |
jamielennox | so there are two very different use cases here | 03:05 |
jamielennox | listing idps is a normal activity and should be open | 03:05 |
stevemar | jamielennox, ayoung i was hoping to have a drop down that listed idps + localauth (for service users) | 03:06 |
jamielennox | listing idps is very secret | 03:06 |
jamielennox | stevemar: ++ | 03:06 |
stevemar | why would it be very secret? | 03:06 |
ayoung | jamielennox, so we have a list of public IdPs and a list of not so public IdPs | 03:06 |
jamielennox | stevemar: learning which companies are partners of your public cloud | 03:06 |
*** wanghong has quit IRC | 03:07 | |
stevemar | jamielennox, well, we could have a new idp field that makes it a partner or not, so it doesn't appear, but how would the user select it then? | 03:07 |
jamielennox | stevemar: this is the problem | 03:07 |
ayoung | stevemar, for a partner, we assume that the company passes out the URL to use over an internal email | 03:07 |
jamielennox | so we have that new google apps trial and if i want to login to that i enter my @redhat.com email address and get bounced to the saml server | 03:08 |
ayoung | and they cut and paste it in | 03:08 |
jamielennox | that's nice and easy because my username contains my domain name | 03:08 |
jamielennox | how is horizon expecting to deal with multiple idps in different domains anyway? | 03:09 |
jamielennox | username is no longer a sufficient login | 03:09 |
*** dims has quit IRC | 03:10 | |
jamielennox | this is kind of what i meant earlier by scope horizon to a domain | 03:10 |
ayoung | if you don't specify domain, you get the default domain | 03:11 |
ayoung | heh | 03:11 |
jamielennox | what does that look like, i haven't seen juno horizon | 03:12 |
stevemar | jamielennox, i tink it's the same thing | 03:13 |
ayoung | no idea...didn't get V3 Horizon working | 03:13 |
stevemar | they just always assume default domain | 03:13 |
jamielennox | so our problem here is just an extension of that right? | 03:13 |
jamielennox | you should be able to list the idps available for your domain | 03:14 |
stevemar | thats what it looked like in the code that i saw | 03:14 |
stevemar | but you can't login without knowing your idp or domain | 03:14 |
jamielennox | you should be able to list the idps available for your domain | 03:15 |
jamielennox | oops, wrong screen for up + enter | 03:16 |
openstackgerrit | wanghong proposed a change to openstack/keystone: remove default check keys in assertValidEntity https://review.openstack.org/112573 | 03:17 |
openstackgerrit | Jamie Lennox proposed a change to openstack/keystonemiddleware: Always add auth URI to unauthorized requests https://review.openstack.org/119261 | 03:18 |
*** wanghong has joined #openstack-keystone | 03:19 | |
jamielennox | ayoung: that review https://review.openstack.org/#/c/119261/ failed the py33 gate so had to re-upload | 03:21 |
ayoung | ok. I'll wait to see if it passes, and, if so, I think it is safe to reapply the +A | 03:22 |
*** radez is now known as radez_g0n3 | 03:27 | |
stevemar | oh neat, devstack now creates another domain just for heat users and projects | 03:27 |
jamielennox | stevemar: i think that's a heat requirement rather than a devstack specific thing | 03:28 |
stevemar | jamielennox, heres v3 horizon http://imgur.com/HgUzJYH | 03:30 |
stevemar | so you can switch projects, and get a project scoped token | 03:30 |
stevemar | but theres no knowledge of domains yet | 03:31 |
stevemar | AFAIK... | 03:31 |
jamielennox | i dont think horizon needs to know much about domains | 03:31 |
stevemar | jamielennox, should have some knowledge, at least to list them | 03:31 |
jamielennox | domain admin is a rare enough thing it can remain cli | 03:32 |
jamielennox | stevemar: it has no way to know which users have the priviledges to list domains | 03:32 |
jamielennox | and its kind of a bad UX to provide a button that only your super admins can use | 03:32 |
stevemar | use the same logic that the cli uses | 03:32 |
stevemar | just list none if you can't access any | 03:33 |
stevemar | meh | 03:33 |
jamielennox | yea, we have /auth/domains now - so that's essentially the same thing | 03:33 |
stevemar | jamielennox, blah, we are going to run into a problem in DOA, it relies on users having a domain | 03:35 |
jamielennox | stevemar: inputted you mean | 03:35 |
stevemar | jamielennox, i'm thinking of federation issued tokens, they don't have one | 03:35 |
stevemar | we really should just put a damn dummy id in there | 03:36 |
jamielennox | stevemar: i don't think that matters user_domain_id is just for auth lookup | 03:36 |
jamielennox | once you've got the token it shouldn't matter | 03:37 |
stevemar | jamielennox, i'm glad we're all at least - relatively - on the same page | 03:37 |
jamielennox | :) | 03:37 |
jamielennox | stevemar: did you have a look at that etherpad | 03:39 |
stevemar | jamielennox, i'm actually looking at that now | 03:40 |
stevemar | and then i looked at zuul, and saw my patch at the top of the queue! yay! | 03:40 |
stevemar | after 5 rechecks it's finally there! | 03:40 |
*** jorge_munoz has left #openstack-keystone | 03:43 | |
jamielennox | https://kantarainitiative.org/SWITCHwayf/WAYF.php?entityID=https%3A%2F%2Fkantarainitiative.org%2Fshibboleth-sp&return=https%3A%2F%2Fkantarainitiative.org%2FShibboleth.sso%2FWAYF%3FSAMLDS%3D1%26target%3Dcookie%253A1409888557_e9a7 | 03:43 |
jamielennox | so drop down boxes might still be a thing | 03:43 |
jamielennox | better link http://kantarainitiative.org/confluence/display/ulx/Home | 03:44 |
stevemar | jamielennox, yep, i recall the UKent folks had drop downs too | 03:47 |
openstackgerrit | A change was merged to openstack/keystone: Create SAML generation route and controller https://review.openstack.org/114138 | 03:47 |
stevemar | thats pretty neat | 03:47 |
jamielennox | http://www.slideshare.net/CloudIDSummit/02-dawes-relying-party | 03:47 |
jamielennox | still heavily relies on have an @domain though | 03:48 |
jamielennox | and maybe that is the answer, to login via horizon you need to do username@domainname and we start pushing people towards email addresses | 03:49 |
jamielennox | then associate domains with idps | 03:49 |
stevemar | jamielennox, that works from a UX side | 03:50 |
bknudson | dstanek: they go in different catalogs and it gives a hint to the translator which ones are more important | 03:51 |
openstackgerrit | wanghong proposed a change to openstack/keystone: Make the the assertValid<Entity> calls internal. https://review.openstack.org/119284 | 03:51 |
dstanek | bknudson: what about cases where the same message is used multiple times like http://paste.openstack.org/show/106234/ ? | 03:54 |
*** yasukun has quit IRC | 03:56 | |
*** yasukun has joined #openstack-keystone | 03:57 | |
*** yasukun has quit IRC | 03:58 | |
*** yasukun has joined #openstack-keystone | 03:59 | |
*** ncoghlan has joined #openstack-keystone | 04:04 | |
*** ncoghlan_afk has quit IRC | 04:08 | |
*** ncoghlan_ has joined #openstack-keystone | 04:10 | |
*** ncoghlan__ has joined #openstack-keystone | 04:10 | |
*** ncoghlan__ is now known as ncoghlan_afk | 04:10 | |
jamielennox | stevemar: so what do we do about region urls ? | 04:11 |
stevemar | jamielennox, right, i agree with what dolphm said in his first and longest response | 04:12 |
stevemar | thats the flow we envisioned at the mid cycle meet up | 04:12 |
stevemar | i agree that it is completely meaningless for a non-federated case | 04:12 |
stevemar | jamielennox, your statement "given this token, what other clouds can i access?" | 04:13 |
stevemar | i don't think thats something we thought of, or want to support, it's a bit too dynamic i think (for our setup right now) | 04:14 |
*** ncoghlan has quit IRC | 04:14 | |
stevemar | authN with your local keystone, then ask for saml assertion for another cloud, it's scoped to one cloud | 04:14 |
*** ncoghlan_ has quit IRC | 04:14 | |
jamielennox | stevemar: how is that different to listing regions and seeing all the available URLs | 04:15 |
jamielennox | ? | 04:15 |
jamielennox | also it's not so much that i disagree with the case i just don't think that region is the right concept to use | 04:15 |
stevemar | oh, i thought you meant that the token can be used at another cloud | 04:15 |
jamielennox | no, you shouldn't be able to use your token on another cloud - your token is relative to the keystone that issued it | 04:16 |
stevemar | correct | 04:16 |
jamielennox | i just mean a listing of known "partner" (for lack of a better word) clouds | 04:17 |
*** miqui has quit IRC | 04:23 | |
openstackgerrit | guang-yee proposed a change to openstack/keystone: Use id attribute map for read-only LDAP https://review.openstack.org/117658 | 04:34 |
dtroyer_zz | jamielennox: are you familiar with novaclient's timing facility to time each API request? | 04:40 |
jamielennox | dtroyer_zz: i am | 04:40 |
dtroyer_zz | how would you feel about adding that to the ksc session? | 04:41 |
jamielennox | dtroyer_zz: i've been trying to avoid it, it seems to be a CLI only thing | 04:41 |
dtroyer_zz | ok. I'm going to see if I can figure out a hook to do it somehow then | 04:42 |
jamielennox | however would reconsider if there is other need | 04:42 |
jamielennox | dtroyer_zz: for OSC it's simple | 04:42 |
jamielennox | subclass Session, override request() with a time.start, super(), time.finish | 04:42 |
dtroyer_zz | right….some people don't like my subclasses ;) that's the way I'm probably headed | 04:43 |
jamielennox | i was doing that in some messing around i was doing with novaclient CLI | 04:43 |
jamielennox | dtroyer_zz: so everything goes through request() on purpose to allow for stuff like that | 04:44 |
jamielennox | the other option is you could pass your own requests.Session to ksc.Session which did timing | 04:44 |
jamielennox | if you trust there interfaces more than mine | 04:44 |
dtroyer_zz | that's how I have to actually use my subclass, right? | 04:44 |
jamielennox | no | 04:45 |
jamielennox | class TimedSession(Session): | 04:45 |
jamielennox | def request(self, *args, **kwargs): | 04:45 |
jamielennox | time_start = time.now() | 04:45 |
jamielennox | resp = super(TimedSession, self).request(*args, **kwargs) | 04:46 |
jamielennox | time_stop = time.now() | 04:46 |
jamielennox | # add to timings list | 04:46 |
jamielennox | return resp | 04:46 |
jamielennox | then just use that instead of ksc.Session | 04:46 |
dtroyer_zz | I get that part…oh, I see the difference…I'm already creating my own session in OSC in the ksc.session review, so it'll just work | 04:47 |
*** rushiagr_away is now known as rushiagr | 04:47 | |
jamielennox | yea. OSC is an application so somewhere you must be creating the session | 04:48 |
jamielennox | it should be relatively easy to override the Session.get_options() as well if you are using Session.load_from_cli_arguments() | 04:50 |
dtroyer_zz | I'm not using that (yet)…first step is to just duplicate the existing functionality, and I'm having a hard enough time to string together enough minutes to get this far | 04:51 |
stevemar | dtroyer_zz, can we expect a new patch soon-ish? with plugins being used :D | 04:52 |
stevemar | dtroyer_zz, just read your last sentence.... when you reference time in minutes, you know you're short on time | 04:52 |
jamielennox | dtroyer_zz: if you want me to take a look or have a go at anything let me know | 04:52 |
dtroyer_zz | stevemar: if I don't fall asleep first, maybe even tonight. but I've been playing with oscquintette and replacing client libs directly…its getting fun | 04:52 |
dtroyer_zz | jamielennox: will do, thanks. maybe once I get something that passes some tests | 04:53 |
*** yasukun has quit IRC | 04:58 | |
*** yasukun has joined #openstack-keystone | 05:00 | |
*** kevinbenton has quit IRC | 05:03 | |
*** ncoghlan_afk is now known as ncoghlan__ | 05:04 | |
*** kevinbenton has joined #openstack-keystone | 05:06 | |
*** gokrokve_ has joined #openstack-keystone | 05:09 | |
*** yasukun has quit IRC | 05:10 | |
*** gokrokve has quit IRC | 05:11 | |
stevemar | dtroyer_zz, bahhhh on swift | 05:19 |
mfisch | bknudson: I sent you and morgan some more data on the utf8 decode issue | 05:19 |
stevemar | i have no idea how swiftclient downloads an object | 05:20 |
mfisch | it does not like my msExhangeSecuritySomethingOrOther field from AD | 05:20 |
mfisch | stevemar: under the hood it uses zmodem | 05:20 |
stevemar | mfisch, i don't even know if that a joke | 05:20 |
dtroyer_zz | stevemar: don't listen to mfisch, it's really xmodem. 1k blocks are for wimps | 05:21 |
mfisch | maybe stevemar is not old like us | 05:21 |
stevemar | upon googling i now know that it is infact, a joke | 05:21 |
mfisch | pretty sure swift uuencodes files first though | 05:21 |
dtroyer_zz | ya, he doesn't even know how much of an improvement it was for procomm to support ymodem-batch | 05:21 |
stevemar | dtroyer_zz, just for you: https://pypi.python.org/pypi/xmodem | 05:22 |
dtroyer_zz | that's going into devstack tonight! | 05:22 |
dtroyer_zz | I'm sure ironic's ipmi needs it or something | 05:22 |
mfisch | one day canada will get modems | 05:22 |
stevemar | lol | 05:22 |
stevemar | i'm just tethering right now, phones are a pretty recent phenomena here | 05:23 |
mfisch | also bedtime for me, its been good, and bad to be back looking at keystone this week | 05:23 |
stevemar | welcome back to the dark side | 05:23 |
mfisch | stevemar: moose antler antenna to improve reception | 05:23 |
stevemar | i think thats only for where anteaya lives | 05:24 |
mfisch | to be fair, lots of moose in my state | 05:24 |
mfisch | seriously bedtime now | 05:24 |
stevemar | see ya | 05:25 |
*** ncoghlan__ has quit IRC | 05:27 | |
*** ncoghlan__ has joined #openstack-keystone | 05:27 | |
jamielennox | does pressing ctrl+c not kill the s-proxy service in devstack or is it just me | 05:29 |
jamielennox | ctrl+z then kill %1 takes down the screen process but doesn't kill the workers | 05:31 |
dtroyer_zz | jamielennox: I don't mess with swift enough to know about the proxy, but unstack.sh should get everything cleaned up | 05:31 |
jamielennox | probably, i just need to mess with swift specifically | 05:32 |
stevemar | never had to kill the proxy | 05:40 |
stevemar | lookup the process and kill it? | 05:40 |
stevemar | dtroyer_zz, might have object save done for ya | 05:40 |
jamielennox | yea, but you kill the script, then you can kill the workers - but something gets screwed up that it doesn't just die nicely | 05:40 |
stevemar | dtroyer_zz, so for glance, we don't return anything upon a save, keep it that way i assume? | 05:41 |
dtroyer_zz | stevemar: you mean visibly? correct, success==no output without -v | 05:42 |
stevemar | dtroyer_zz, m'alright | 05:42 |
stevemar | gonna toss something up as soon as pep8 passes | 05:43 |
dtroyer_zz | I'll race ya | 05:43 |
stevemar | i just nuked .tox, waiting for requirements | 05:44 |
stevemar | i think you'll win this one | 05:44 |
dtroyer_zz | that was too easy…. rm -rf .tox; tox | 05:44 |
stevemar | but my moose connection is slow | 05:44 |
stevemar | dtroyer_zz, up | 05:46 |
openstackgerrit | guang-yee proposed a change to openstack/keystone: Use id attribute map for read-only LDAP https://review.openstack.org/117658 | 05:46 |
stevemar | dtroyer_zz, https://review.openstack.org/119293 | 05:46 |
stevemar | yay | 05:46 |
dtroyer_zz | stevemar: https://review.openstack.org/106178 | 05:47 |
stevemar | dtroyer_zz, ohhh the same minute | 05:47 |
dtroyer_zz | also, I'm pushing in https://review.openstack.org/113046, it's baked long enough | 05:47 |
stevemar | dtroyer_zz, i think so | 05:47 |
stevemar | i was going to, just waiting on gate nonsense | 05:47 |
dtroyer_zz | gate is down to normal busy levels, I'll pass through the rest of them in toe morning | 05:48 |
stevemar | dtroyer_zz, great, now you're going to make me rebase :) | 05:48 |
*** yasukun has joined #openstack-keystone | 05:49 | |
dtroyer_zz | the thrill of victory! | 05:49 |
ekarlso- | jamielennox: gotten that patch in for ksclient ? ;P | 05:49 |
stevemar | i'll have at least 1 successful build | 05:49 |
jamielennox | ekarlso-: heh, ksclient has barely moved in the last week or so, hoping once the freeze is finished | 05:49 |
ekarlso- | k | 05:50 |
stevemar | dtroyer_zz, i'll talk to you later about downloading an entire container, not sure what the default behaviour of that should be | 05:50 |
jamielennox | need to jam through about 10 patches next week cause then i'm away for 4 | 05:50 |
stevemar | s/later/tomorrow | 05:50 |
ekarlso- | jamielennox: 4 weeks ? | 05:50 |
jamielennox | yep | 05:50 |
ekarlso- | jamielennox: dang | 05:50 |
stevemar | jamielennox, damn thats a sweet vacation | 05:50 |
ekarlso- | hope u get that auth one in also! | 05:50 |
ekarlso- | it's a really niceness for clients that need to keep legacy support in the constructor | 05:50 |
jamielennox | honeymoon :) | 05:50 |
ekarlso- | oh, grats | 05:51 |
stevemar | jamielennox, !! congrats dude :) | 05:51 |
ekarlso- | jamielennox: think u will make it in with that before u gotta go ? | 05:51 |
jamielennox | i'm always surprised by the people who aren't aware of this, guess i never advertised it or anythign | 05:51 |
jamielennox | ekarlso-: gyee is pretty keen for that one as well, so hoping so | 05:52 |
*** nsaje has joined #openstack-keystone | 06:02 | |
*** stevemar has quit IRC | 06:03 | |
openstackgerrit | OpenStack Proposal Bot proposed a change to openstack/keystone: Imported Translations from Transifex https://review.openstack.org/111920 | 06:03 |
*** oomichi has quit IRC | 06:05 | |
*** ajayaa has joined #openstack-keystone | 06:10 | |
gyee | jamielennox, which patch? Didn't I +2 everythong already? :) | 06:15 |
gyee | everything | 06:15 |
jamielennox | gyee: definetly not - i would have noticed | 06:15 |
jamielennox | https://review.openstack.org/#/c/81147/ | 06:15 |
jamielennox | gyee: and if you need a few more let me know | 06:17 |
gyee | jamielennox, you not sharing code with the existing plugins? | 06:20 |
jamielennox | gyee: what do you mean? | 06:20 |
gyee | we have existing plugins in keystoneclient.identity.auth | 06:21 |
jamielennox | yea | 06:21 |
gyee | they are implementing stuff in identity base | 06:21 |
gyee | so these are brand new ones with their own base class | 06:21 |
jamielennox | identity/generic/base subclasses identity/base | 06:22 |
gyee | jamielennox, nevermind, I see what you did there | 06:22 |
gyee | lgtm! | 06:22 |
*** yasukun has quit IRC | 06:23 | |
*** k4n0 has joined #openstack-keystone | 06:25 | |
gyee | jamielennox, I'll find more time for code review later | 06:25 |
*** jaosorior has joined #openstack-keystone | 06:25 | |
gyee | I am way passed bed time | 06:25 |
*** gyee has quit IRC | 06:25 | |
jamielennox | gyee: i'm going to try and force people to do them next week as we are passed FF, if you can have a go before then it'd be appreciated | 06:25 |
*** yasukun has joined #openstack-keystone | 06:31 | |
openstackgerrit | wanghong proposed a change to openstack/keystone: Make the the assertValid<Entity> calls internal. https://review.openstack.org/119284 | 06:34 |
openstackgerrit | Marcos Fermín Lobo proposed a change to openstack/keystone: Keystone part of a PoC for Horizon/Keystone WebSSO https://review.openstack.org/106096 | 06:53 |
*** amirosh has joined #openstack-keystone | 06:54 | |
*** bvandenh has joined #openstack-keystone | 06:58 | |
openstackgerrit | wanghong proposed a change to openstack/keystonemiddleware: convert the conf value into correct type https://review.openstack.org/113191 | 07:00 |
jamielennox | wanghong: comment on ^ | 07:05 |
*** nsaje_ has joined #openstack-keystone | 07:12 | |
*** nsaje has quit IRC | 07:13 | |
openstackgerrit | Jamie Lennox proposed a change to openstack/python-keystoneclient: Add auth manager https://review.openstack.org/118531 | 07:18 |
openstackgerrit | Jamie Lennox proposed a change to openstack/python-keystoneclient: Make keystoneclient use an adapter https://review.openstack.org/97681 | 07:18 |
openstackgerrit | Jamie Lennox proposed a change to openstack/python-keystoneclient: Make tests run against original client and session https://review.openstack.org/117089 | 07:18 |
openstackgerrit | Jamie Lennox proposed a change to openstack/python-keystoneclient: Allow retrying some failed requests https://review.openstack.org/118004 | 07:18 |
openstackgerrit | wanghong proposed a change to openstack/keystone: wrong logic in assertValidRoleAssignmentListResponse method https://review.openstack.org/119303 | 07:18 |
* jamielennox weekend | 07:26 | |
wanghong | jamielennox, are you here? | 07:27 |
*** KanagarajM has joined #openstack-keystone | 07:34 | |
openstackgerrit | wanghong proposed a change to openstack/keystonemiddleware: convert the conf value into correct type https://review.openstack.org/113191 | 07:44 |
*** afazekas has joined #openstack-keystone | 08:09 | |
*** jimbaker has quit IRC | 08:16 | |
openstackgerrit | Marcos Fermín Lobo proposed a change to openstack/keystone: Keystone part of a PoC for Horizon/Keystone WebSSO https://review.openstack.org/106096 | 08:28 |
*** jimbaker has joined #openstack-keystone | 08:28 | |
*** jimbaker has quit IRC | 08:29 | |
*** jimbaker has joined #openstack-keystone | 08:29 | |
*** ncoghlan__ has quit IRC | 08:45 | |
*** KanagarajM has quit IRC | 08:46 | |
*** gokrokve has joined #openstack-keystone | 08:49 | |
*** gokrokve_ has quit IRC | 08:53 | |
*** gokrokve has quit IRC | 08:53 | |
*** BAKfr has joined #openstack-keystone | 08:56 | |
*** k4n0 has quit IRC | 09:16 | |
*** andreaf has joined #openstack-keystone | 09:19 | |
*** gokrokve has joined #openstack-keystone | 09:25 | |
openstackgerrit | wanghong proposed a change to openstack/keystonemiddleware: convert the conf value into correct type https://review.openstack.org/113191 | 09:29 |
*** ajayaa has quit IRC | 09:30 | |
*** gokrokve has quit IRC | 09:30 | |
*** KanagarajM has joined #openstack-keystone | 09:33 | |
*** yasukun has quit IRC | 09:34 | |
*** KanagarajM2 has joined #openstack-keystone | 09:36 | |
*** KanagarajM has quit IRC | 09:38 | |
*** ajayaa has joined #openstack-keystone | 09:41 | |
*** vhoward has joined #openstack-keystone | 10:02 | |
*** yasukun has joined #openstack-keystone | 10:05 | |
*** yasukun has quit IRC | 10:13 | |
*** andreaf has quit IRC | 10:23 | |
*** andreaf has joined #openstack-keystone | 10:23 | |
*** gokrokve has joined #openstack-keystone | 10:25 | |
*** bvandenh has quit IRC | 10:28 | |
*** bvandenh has joined #openstack-keystone | 10:29 | |
*** gokrokve has quit IRC | 10:30 | |
*** aix has joined #openstack-keystone | 10:38 | |
*** andreaf_ has quit IRC | 10:39 | |
*** andreaf_ has joined #openstack-keystone | 10:40 | |
*** dims has joined #openstack-keystone | 10:54 | |
*** dims has quit IRC | 10:56 | |
*** dims_ has joined #openstack-keystone | 10:57 | |
openstackgerrit | Andreas Jaeger proposed a change to openstack/keystonemiddleware: Improve help strings https://review.openstack.org/118048 | 10:57 |
*** dims has joined #openstack-keystone | 10:57 | |
*** KanagarajM2 has quit IRC | 10:57 | |
openstackgerrit | Sasikanth Eda proposed a change to openstack/keystone: Fix create and user-role-add in LDAP backend https://review.openstack.org/119345 | 11:01 |
*** dims_ has quit IRC | 11:01 | |
*** andreaf has quit IRC | 11:09 | |
*** andreaf has joined #openstack-keystone | 11:10 | |
*** samuelmz_ has joined #openstack-keystone | 11:15 | |
*** ajayaa has quit IRC | 11:22 | |
*** gokrokve has joined #openstack-keystone | 11:25 | |
*** gokrokve has quit IRC | 11:30 | |
*** diegows has joined #openstack-keystone | 11:31 | |
*** samuelmz_ has quit IRC | 11:36 | |
*** ajayaa has joined #openstack-keystone | 11:48 | |
*** k4n0 has joined #openstack-keystone | 11:55 | |
*** openstackgerrit has quit IRC | 12:01 | |
*** openstackgerrit has joined #openstack-keystone | 12:02 | |
*** amirosh has quit IRC | 12:04 | |
*** amirosh has joined #openstack-keystone | 12:04 | |
*** andreaf has quit IRC | 12:07 | |
*** andreaf has joined #openstack-keystone | 12:07 | |
openstackgerrit | David Stanek proposed a change to openstack/keystone: Fixes a mock cleanup issue caused by oslotest https://review.openstack.org/119224 | 12:13 |
*** dims has quit IRC | 12:19 | |
*** dims has joined #openstack-keystone | 12:20 | |
*** gokrokve has joined #openstack-keystone | 12:25 | |
*** samuelmz_ has joined #openstack-keystone | 12:29 | |
*** gokrokve has quit IRC | 12:30 | |
openstackgerrit | Andreas Jaeger proposed a change to openstack/keystonemiddleware: Improve help strings https://review.openstack.org/118048 | 12:35 |
*** dims has quit IRC | 12:39 | |
*** dims has joined #openstack-keystone | 12:40 | |
*** lbragstad has quit IRC | 12:43 | |
*** lbragstad has joined #openstack-keystone | 12:43 | |
*** d34dh0r53 has quit IRC | 12:44 | |
*** d34dh0r53 has joined #openstack-keystone | 12:44 | |
*** pabelanger has quit IRC | 12:45 | |
*** dims has quit IRC | 12:45 | |
openstackgerrit | Juan Antonio Osorio Robles proposed a change to openstack/keystone: Refactor assignment expansion related functions https://review.openstack.org/119363 | 12:45 |
*** dims_ has joined #openstack-keystone | 12:46 | |
*** pabelanger has joined #openstack-keystone | 12:46 | |
*** dims_ has quit IRC | 12:49 | |
*** dims has joined #openstack-keystone | 12:50 | |
*** morganfainberg has quit IRC | 12:54 | |
*** morganfainberg has joined #openstack-keystone | 12:57 | |
*** richm has joined #openstack-keystone | 13:10 | |
ayoung | bknudson, so I was right....the _id_to_dn code does get called at a point in the identity code | 13:11 |
*** amirosh has quit IRC | 13:11 | |
*** amirosh has joined #openstack-keystone | 13:12 | |
*** andreaf has quit IRC | 13:14 | |
*** andreaf has joined #openstack-keystone | 13:14 | |
*** amirosh has quit IRC | 13:16 | |
*** amirosh has joined #openstack-keystone | 13:16 | |
*** joesavak has joined #openstack-keystone | 13:21 | |
*** morganfainberg has quit IRC | 13:25 | |
*** gokrokve has joined #openstack-keystone | 13:25 | |
*** morganfainberg has joined #openstack-keystone | 13:27 | |
*** gokrokve has quit IRC | 13:30 | |
*** bknudson has quit IRC | 13:30 | |
ayoung | bknudson, dolphm nkinder https://bugs.launchpad.net/keystone/+bug/1366020 I don't know if this is a real problem or not, but it affects the bug gyee was working on that is a blocker | 13:32 |
uvirtbot | Launchpad bug 1366020 in keystone "LDAP Identity does not convert ID to DN for lookup" [Undecided,New] | 13:32 |
*** bvandenh has quit IRC | 13:33 | |
ajayaa | dolphm, ayoung, Is the multi domain identity backend being reverted? | 13:33 |
ayoung | ajayaa, not that I know of. Why? | 13:33 |
ajayaa | https://blueprints.launchpad.net/keystone/+spec/revert-multiple-ldap-servers | 13:33 |
ajayaa | ayoung, I see a blueprint registered by doplhm. | 13:34 |
*** d34dh0r53 has quit IRC | 13:34 | |
*** d34dh0r53 has joined #openstack-keystone | 13:35 | |
*** topol has joined #openstack-keystone | 13:37 | |
ayoung | ajayaa, no that is old and we've fixed them instead | 13:40 |
*** dhellmann is now known as dhellmann_ | 13:40 | |
ajayaa | ayoung, Do I need to create service users in sql backend if I am using a read only ldap server? | 13:40 |
ayoung | ajayaa, yep | 13:41 |
ayoung | ajayaa, or mount a second LDAP server for service users | 13:41 |
ayoung | ajayaa, http://adam.younglogic.com/2014/08/getting-service-users-out-of-ldap/ | 13:41 |
ajayaa | ayoung, I assume the service users can be contained in any domain. | 13:43 |
ayoung | yep | 13:43 |
ajayaa | ayoung, okay thanks. | 13:44 |
openstackgerrit | A change was merged to openstack/keystone: Add test for single app loaded version response https://review.openstack.org/118902 | 13:44 |
openstackgerrit | A change was merged to openstack/keystone: Fix admin server doesn't report v2 support in Apache httpd https://review.openstack.org/118757 | 13:45 |
*** bknudson has joined #openstack-keystone | 13:51 | |
*** r-daneel has joined #openstack-keystone | 13:53 | |
*** jaosorior has quit IRC | 14:02 | |
*** gokrokve has joined #openstack-keystone | 14:02 | |
*** zzzeek has joined #openstack-keystone | 14:05 | |
*** dims is now known as dimsum_ | 14:12 | |
*** bklei has joined #openstack-keystone | 14:15 | |
*** bklei has left #openstack-keystone | 14:15 | |
*** morganfainberg has quit IRC | 14:18 | |
*** luvshines has joined #openstack-keystone | 14:19 | |
*** luvshines is now known as viral_mutant | 14:20 | |
*** david-lyle has joined #openstack-keystone | 14:26 | |
*** morganfainberg has joined #openstack-keystone | 14:27 | |
*** sigmavirus24_awa is now known as sigmavirus24 | 14:28 | |
*** amirosh has quit IRC | 14:29 | |
*** amirosh has joined #openstack-keystone | 14:30 | |
*** cjellick has quit IRC | 14:30 | |
*** viral_mutant has quit IRC | 14:30 | |
*** cjellick has joined #openstack-keystone | 14:31 | |
*** andreaf has quit IRC | 14:33 | |
*** andreaf has joined #openstack-keystone | 14:33 | |
*** _nonameentername is now known as nonameentername | 14:35 | |
*** amirosh has quit IRC | 14:35 | |
*** radez_g0n3 is now known as radez | 14:35 | |
*** cjellick has quit IRC | 14:36 | |
*** dhellmann_ is now known as dhellmann | 14:43 | |
openstackgerrit | ayoung proposed a change to openstack/python-keystoneclient: Initial kerberos plugin implementation. https://review.openstack.org/74974 | 14:49 |
ajayaa | dolphm, ayoung, We are importing ldappool in keystone/common/ldap/core.py. Should not it be present in requirements.txt? | 14:50 |
ayoung | ajayaa, yes | 14:50 |
ajayaa | It is not there I think. | 14:51 |
ajayaa | https://github.com/openstack/keystone/blob/master/requirements.txt | 14:51 |
ayoung | ajayaa, please fix | 14:51 |
ajayaa | okay. Shall I first file a bug and then fix it? | 14:51 |
ajayaa | ayoung, | 14:51 |
ayoung | yep | 14:52 |
*** jorge_munoz has joined #openstack-keystone | 14:52 | |
*** vhoward has left #openstack-keystone | 14:54 | |
*** k4n0 has quit IRC | 14:57 | |
*** stevemar has joined #openstack-keystone | 14:57 | |
openstackgerrit | Ajaya Agrawal proposed a change to openstack/keystone: Added ldappool to requirements.txt https://review.openstack.org/119392 | 15:00 |
r1chardj0n3s | ayoung: dolphm has nixed your keystonemiddleware plan, but I have a new plan involving olso now | 15:01 |
ayoung | r1chardj0n3s, of course he has. He's a purist. He's also just voicing his opinion. Lets discuss it at the next Keystone team meeting. | 15:02 |
r1chardj0n3s | ayoung: he makes a compelling argument | 15:02 |
ayoung | r1chardj0n3s, was it in here? I'll check evesdrop | 15:03 |
r1chardj0n3s | ayoung: no, was IRL just now | 15:03 |
ayoung | r1chardj0n3s, then it didn't happen | 15:03 |
ayoung | IRL does not exist | 15:03 |
r1chardj0n3s | well, I could submit the spec and have it rejected if you like :) | 15:04 |
*** joesavak has quit IRC | 15:04 | |
ayoung | r1chardj0n3s, here's the deal: if we do anything other than auth_token.py it will require an additional middleware config in all servers. If that is the case, it doesn 't matter where it lives | 15:04 |
ayoung | But this is Keystone specific logic, and should be in keystonemiddleware, even if not auth_token.py | 15:05 |
r1chardj0n3s | ayoung: there's an abandoned change in oslo to add cors that I'm going to look at resurrecting | 15:05 |
ayoung | We need to know "who" can do cors and that means the service catalog | 15:05 |
r1chardj0n3s | ayoung: the config issue is solved by people's existing configuration management tools (aka salt, etc) | 15:05 |
r1chardj0n3s | the "who" is handled by each API in its own config | 15:06 |
ayoung | r1chardj0n3s, if it helps you to sleep at night to think that, please go right ahead | 15:06 |
ayoung | nope | 15:06 |
r1chardj0n3s | ayoung: well, it'll help me move forward; whereas the keystonemiddleware proposal is going to be rejected | 15:06 |
ayoung | r1chardj0n3s, dolphm and I often go head to head on these things...its part of the process that makes Keystone better | 15:06 |
r1chardj0n3s | ayoung: if you would like me to I can commit the spec | 15:07 |
r1chardj0n3s | ayoung: go through the formal process | 15:07 |
ayoung | r1chardj0n3s, oslo is not for new ideas. oslo is for things that are already vetter in another project | 15:07 |
ayoung | CORS support in Keystone is going to be different from the other services anyway. Lets solve that one first | 15:07 |
ayoung | Keystone does not do auth_token.py | 15:07 |
r1chardj0n3s | ayoung: but CORS isn't tied to auth_token | 15:07 |
ayoung | r1chardj0n3s, of course it is | 15:08 |
ayoung | cuz auth-token extracts the service catalog | 15:08 |
r1chardj0n3s | ayoung: I don't see how | 15:08 |
ayoung | and CORS will need that service catalog | 15:08 |
ayoung | it can be a follow on component | 15:08 |
r1chardj0n3s | the service catalog is not needed for CORS, unless I'm missing something | 15:08 |
ayoung | you are missing something | 15:08 |
ayoung | how do we know what set of services can respond yes to CORS if not from the service catalog? | 15:09 |
ayoung | and please don't say "config management tools" cuz that is not an answer | 15:09 |
r1chardj0n3s | each service configures itself | 15:09 |
r1chardj0n3s | but that is the answer | 15:09 |
r1chardj0n3s | sorry, better: why isn't that the answer? :) | 15:09 |
ayoung | r1chardj0n3s, hmmm....the short answer is that CORS evaluation needs to be on a request by request basis | 15:11 |
ayoung | just because one user can do CORS from keystone1 to nova1 doesn't mean that every request is valid | 15:11 |
dolphm | shouldn't the CORS header just contain Horizon's domain - which isn't in the service catalog? | 15:12 |
r1chardj0n3s | what aspect of the request is needed apart from the host? that's basically all CORS cares about - it's not up to CORS to provide any level of protection beyond host, and possibly filter some headers | 15:12 |
r1chardj0n3s | dolphm: correct | 15:12 |
dolphm | so, nova would respond with Access-Control-Allow-Origin: https://horizon.example.com/ | 15:12 |
r1chardj0n3s | dolphm: yes, though there's some ancillary stuff around that | 15:12 |
r1chardj0n3s | (OPTIONS handling, maybe some header controls as well) | 15:12 |
r1chardj0n3s | ayoung: security is not CORS' concern. | 15:13 |
ayoung | dolphm, once again, I am thinking MOC and don't trust anyone.... | 15:13 |
r1chardj0n3s | ayoung: er, user-based security that is | 15:13 |
dolphm | r1chardj0n3s: sure, but there's no configuration i'm aware of beyond "nova, glance, keystone, etc need to know that horizon belongs in [filter:oslo.cors.whatever] allowed_origins" | 15:14 |
r1chardj0n3s | dolphm: oh, yes | 15:14 |
ayoung | r1chardj0n3s, the goal is to prevent cross site request forgeries | 15:14 |
dolphm | ayoung: how is CORS related to XSRF? | 15:15 |
*** cjellick has joined #openstack-keystone | 15:15 | |
ayoung | r1chardj0n3s, there is also the problem that so many people deploy with IP addresses, but lets punt on that for a moment | 15:15 |
dolphm | http://stackoverflow.com/questions/19793695/does-a-proper-cors-setup-prevent-xsrf | 15:15 |
ayoung | dolphm, those answers are cursory and don't explain the whole thing. Let me see if I can talk it through | 15:16 |
bknudson | CORS implies that your application is written correctly. | 15:16 |
ayoung | dolphm, you might be right, though, that with CORS we can be more forgiving, since the token itself will restrict access | 15:16 |
r1chardj0n3s | yes, it's up to the token to provide that protection | 15:17 |
ayoung | dolphm, bascially, the XSRF attack is "I think this user might have gone to that site before and have a cookie that allows unfettered access. Let me submit a second response." | 15:17 |
ayoung | The normal way to protect against XSRF is by writing a header that is not a cookie. In this case, it would be the Token | 15:18 |
ayoung | now...I think actually even with CORS we are ok | 15:18 |
ayoung | cus the origin would be the attackers site, not horizon | 15:18 |
ayoung | 0k...I think maybe using the service catalog for the OPTIONS response might be more than we need, and it means that we need to put horizon in there... | 15:19 |
ayoung | hmmm..... | 15:19 |
ayoung | r1chardj0n3s, let me look at the Oslo spec... | 15:19 |
ayoung | I'm not 100% on this. I kindof suspect that we need the service catalog as part of "is CORS legal for this request" | 15:21 |
*** pabelanger has left #openstack-keystone | 15:21 | |
r1chardj0n3s | ayoung: CORS should be independent of request payload *except* perhaps blocking headers *but only* based on header name, not contents | 15:22 |
r1chardj0n3s | (except the specific CORS headers, natch) | 15:22 |
ayoung | r1chardj0n3s, OK...lets to a thought experiment here. To keep things simple, we will limit it to two keystone servers, one horizon server and two nova servers | 15:23 |
ayoung | horizon is purely static javascript | 15:24 |
ayoung | user "logs in" to Horizon by going to keystone and getting a token. | 15:24 |
ayoung | that requires CORS supportin Keystone, accepting the horizon server | 15:24 |
*** gokrokve_ has joined #openstack-keystone | 15:24 | |
ayoung | now that token has a service catalog that says Nova1 | 15:25 |
ayoung | OPTIONS to NOVA1 says CORS OK | 15:25 |
ajayaa | bknudson, while using ldap identity backend through devstack I encountered this error. | 15:25 |
ayoung | and the "create vm" call goes through | 15:25 |
ayoung | auth token is still processed. | 15:25 |
bknudson | ajayaa: sounds like a bug in devstack | 15:25 |
ayoung | now...somehow something malicious got into, oh, say horizon's javascript. Cuz I'm paranoid | 15:26 |
ajayaa | bknudson, I think they don't consolidate test-requirements.txt. | 15:26 |
ayoung | at this point...anything could happen, and the damage is limited by auth_token | 15:26 |
ayoung | it says, send your token to nova2 | 15:26 |
ajayaa | bknudson, But if something is critical to starting of keystone server itself, should not it be present in requirements.txt? | 15:27 |
bknudson | ajayaa: still sounds like a bug in devstack | 15:27 |
ayoung | nova2 is an attack site, but OPTIONS comes back and says "Nova2 OK" | 15:27 |
ajayaa | instead of test-requirements.txt | 15:27 |
ayoung | and now your token is sent to an attack site. | 15:27 |
ayoung | But...I don't think there is any defense against that | 15:27 |
ayoung | and service catalog is irrelevant | 15:27 |
r1chardj0n3s | CORS is really not going to help, yeah ;) | 15:27 |
ayoung | hmmm | 15:27 |
*** gokrokve has quit IRC | 15:28 | |
ayoung | and if I got to hooorizon instead of horizon, keystone CORS is going to reject it and no token will be available | 15:28 |
r1chardj0n3s | yep | 15:29 |
* ayoung assumes hooorizon is an attack site | 15:29 | |
*** gokrokve_ has quit IRC | 15:29 | |
ayoung | and if your DNS gets poisoned...CORS doesn't protect you at all anyway | 15:29 |
r1chardj0n3s | CORS is only handling a very specific thing, which is cross-site javascript. It doesn't pretend to offer protection for anything else. | 15:30 |
r1chardj0n3s | trying to overload it to handle anything else is a mistake as well, imo | 15:31 |
ajayaa | bknudson, we are importing ldappool in "keystone/common/ldap/core.py" | 15:31 |
bknudson | ajayaa: that part isn't imported unless keystone is configured to use ldap. | 15:32 |
ayoung | r1chardj0n3s, I'm aware. I was just trying to limit the amount that we open things up via CORS. | 15:32 |
ayoung | CORS says "relax the browser protections for these set of sites" | 15:32 |
ayoung | and I want to keep that set as small as possible | 15:32 |
r1chardj0n3s | yep, we have to ensure that people don't just "allow_origin: *" without being duly warned | 15:32 |
ajayaa | bknudson, Okay. Thanks | 15:33 |
ayoung | HTTPS is a requirement for this to work | 15:33 |
ayoung | I mean, strict certificate checking | 15:33 |
*** ajayaa has quit IRC | 15:33 | |
r1chardj0n3s | hmm, that makes sense, but will probably be far too high a barrier | 15:34 |
r1chardj0n3s | note that the APIs are already exposed, just not to Javascript | 15:34 |
r1chardj0n3s | (though not with the potential availability of an auth token, I suppose) | 15:35 |
r1chardj0n3s | but they're already open, and most likely not over HTTPS | 15:35 |
r1chardj0n3s | I gotta run for a moment, brb | 15:36 |
*** r1chardj0n3s is now known as r1chardj0n3s_afk | 15:36 | |
*** packet has joined #openstack-keystone | 15:38 | |
*** gokrokve has joined #openstack-keystone | 15:38 | |
morganfainberg | mornin | 15:41 |
ayoung | morning morganfainberg, | 15:42 |
ayoung | Guten morgen | 15:42 |
morganfainberg | lol | 15:42 |
ayoung | morganfainberg, read up on out conversation about CORS if you get the chance. | 15:43 |
morganfainberg | yeah reading the scrollback now | 15:43 |
*** r1chardj0n3s_afk is now known as r1chardj0n3s | 15:43 | |
r1chardj0n3s | back | 15:43 |
ayoung | r1chardj0n3s, http://adam.younglogic.com/2014/08/phishing/ is why we really shouldn't trust DNS | 15:43 |
*** samuelmz_ has quit IRC | 15:44 | |
ayoung | r1chardj0n3s, but that attack could be used against Horizon itself. HTTPS is a baseline requirement for OpenStack security. | 15:44 |
r1chardj0n3s | yep | 15:44 |
morganfainberg | interesting | 15:45 |
*** gokrokve has quit IRC | 15:49 | |
*** gyee has joined #openstack-keystone | 15:49 | |
*** gokrokve has joined #openstack-keystone | 15:50 | |
r1chardj0n3s | ayoung: FYI the new point of work for CORS will start from https://bugs.launchpad.net/oslo.middleware/+bug/987044 | 15:51 |
uvirtbot | Launchpad bug 987044 in oslo.middleware "OpenStack APIs should support CORS to be usable from Javascript" [Wishlist,Triaged] | 15:51 |
nkinder | ayoung, jdennis: what do you think the right behavior is if ldap2py() encounters a decoding error due to a binary value? | 15:51 |
dolphm | morganfainberg: if a callable wrapped by dogpile's cache-on-arguments raises an exception, that exception isn't cached for that method signature is it? | 15:52 |
nkinder | ayoung, jdennis: skip that attribute and log an error, but continue on? | 15:52 |
morganfainberg | dolphm, negative. exceptions are not cached | 15:52 |
dolphm | yay | 15:52 |
nkinder | it is unlikely that the binary value is actually needed by keystone for anything (as we'd blow up elsewhere) | 15:52 |
morganfainberg | dolphm, also if the SHOULD_CACHE_FN check fails it wont be cached. | 15:52 |
ayoung | nkinder, right | 15:52 |
nkinder | ayoung: ok, cool. I have a patch coming up | 15:53 |
ayoung | nkinder, I don;t think I can foresee all possible uses | 15:53 |
morganfainberg | so we can add a lot of extra logic magic to that if needed / wanted | 15:53 |
ayoung | but this is keystone specific, right? | 15:53 |
nkinder | ayoung: yes | 15:53 |
ayoung | then, yes, that is the right behavior | 15:53 |
ayoung | nkinder, did you see the bug I filed, though? | 15:53 |
nkinder | ayoung: LOG.warn? | 15:53 |
nkinder | ayoung: yes, but that's different | 15:53 |
ayoung | no logging | 15:53 |
ayoung | it will spam the hell out of the logs | 15:54 |
nkinder | ayoung: well, LOG.debug | 15:54 |
ayoung | log.DEBUG yea | 15:54 |
jdennis | nkinder: In IPA we never get into that situation (well, we shouldn't) because we know the type of every attribute given the schema, therefore we only decode UTF8 strings | 15:54 |
ayoung | would be nice to have "log.ONCE | 15:54 |
nkinder | jdennis: what I found last night was that AD will return objectSID, which blows us up | 15:54 |
jdennis | but OpenStack doesn't have enough information, so I would just skip it and use the binary value | 15:54 |
ayoung | jdennis, and others have this same problem | 15:54 |
nkinder | jdennis: we shouldn't be getting that attribute back in the first place though | 15:54 |
*** gokrokve has quit IRC | 15:54 | |
ayoung | the issue is that a get without any attribute filter will return all attributes | 15:55 |
nkinder | jdennis: so just copy the raw value, or eliminate the attribute completely? | 15:55 |
ayoung | even an attribute filter of [] will do that | 15:55 |
nkinder | I suppose the former is better, though something else may barf on it later if it tries to use that value | 15:55 |
jdennis | nkinder: I don't know the answer to your question because I don't know the context, what attributes are you expecting? | 15:56 |
ayoung | nkinder, I'm almost tempted to say we should only fetch the attributes explicitly required | 15:56 |
dstanek | nkinder: could the AD issue be what Brent is seeing here? https://review.openstack.org/#/c/118430/ | 15:56 |
nkinder | ayoung: I already fixed that too | 15:56 |
nkinder | ayoung: but we should make the code tolerant in case something slips in later | 15:56 |
ayoung | nkinder, what I was thinking was maybe we should fail catastophically instead | 15:56 |
morganfainberg | dolphm, i'm thinking as a K1 target doing the functional test setup like swift/neutron does for keystone so we can start migrating/updating the relevant tests from RESTFUL / tempest over. | 15:57 |
ayoung | if someone said that user_id was a binary field.... | 15:57 |
nkinder | jdennis: so keystone does a search in some cases where it doesn't ask for any attribute to be returned | 15:57 |
nkinder | jdennis: this results in all attributes being returned | 15:57 |
ayoung | ID to DN conversion: just look for existence | 15:57 |
nkinder | jdennis: when we convert that entry with ldap2py, it blows up on any binary values | 15:57 |
bknudson | nkinder: mfisch has this same problem | 15:57 |
jdennis | nkinder: I think it's a problem to get back attributes you don't know anything about | 15:58 |
nkinder | jdennis: the assumption (and my experience) is that any attribute with a binary value isn't an attribute that Keystone actually will use | 15:58 |
nkinder | jdennis: it is, and I'm fixing that | 15:58 |
bknudson | y, keystone has no use for binary attrs | 15:58 |
nkinder | So do we silently drop the whole attribute? | 15:58 |
bknudson | nkinder: we shouldn't have requested it | 15:58 |
nkinder | I'm OK with that if we log it at LOG.debug | 15:58 |
bknudson | we should know what attrs we need. | 15:58 |
nkinder | bknudson: I know, and I fixed that | 15:58 |
nkinder | bknudson: but I'd like to protect against crashing if someone adds a search back in that gets all attrs | 15:59 |
jdennis | nkinder: ok, then you know which are strings, right? | 15:59 |
bknudson | nkinder: add a test. | 15:59 |
bknudson | put it in fakeldap... you must specify attrs. | 15:59 |
nkinder | jdennis: well, I would catch UnicodeDecodeError | 16:00 |
*** wwriverrat has joined #openstack-keystone | 16:00 | |
morganfainberg | bknudson, but i want to use the picture of the person in my AD setup as their ID /snark | 16:00 |
bknudson | openstack user list should display their jpeg. | 16:00 |
morganfainberg | ++ totally! | 16:01 |
nkinder | bknudson: I still think tolerant code is the correct thing to do though | 16:01 |
nkinder | tests are great, but the code should catch the exception | 16:01 |
jdennis | nkinder: but catching a UnicodeDecodeError is a weak test | 16:01 |
bknudson | I have no problem with also dropping binary attrs on the result. | 16:01 |
bknudson | maybe a debug log. | 16:01 |
morganfainberg | sounds reasonable. | 16:01 |
jdennis | nkinder: what happens when you successfully UTF8 decode a binary values that's not a string? Or for that matter a string that's supposed to be UTF8 encoded but can't be decoded? | 16:02 |
wwriverrat | I'm having trouble with outputting the value to the logs. Our Active Directory impl has a "Thumbnail" jpeg attribute that would likely munge the logs every time that attribute was hit | 16:03 |
dstanek | jdennis: in the first case you'll get garbage test and in the second case you'll get an exception | 16:03 |
nkinder | jdennis: gotta jump on a call. bbiab | 16:03 |
dstanek | nkinder: jdennis bknudson: https://review.openstack.org/#/c/118430/4/keystone/common/ldap/core.py,cm | 16:04 |
nkinder | jdennis: but I would hope that the ldap server is enforcing syntax correctness | 16:04 |
jdennis | dstanek: yes I know that, I'm just saying it's not robust logic | 16:04 |
bknudson | dstanek: can the caller handle getting None back? | 16:05 |
nkinder | jdennis: I agree about the logic being weak, but keystone shouldn't be using binary attrs unless it's misconfigured | 16:05 |
dstanek | doesn't the ldap library tell use what types we're getting back | 16:05 |
bknudson | dstanek: maybe it should return a string indicating the ldap value was invalid. | 16:05 |
bknudson | dstanek: ldap library doesn't know the type | 16:05 |
jdennis | nkinder: the ldap server enforcing syntax isn't the issue, the issue is we don't utilize the schema to know what the syntax is | 16:06 |
bknudson | dstanek: the server's schema can define what's a valid value (binary or not) | 16:06 |
dstanek | bknudson: could be i commented on the review that i thought they were solving the wrong problem | 16:06 |
dstanek | bknudson: that's unfortunate | 16:06 |
wwriverrat | imho: It would be nice if every call to retrieve data from ldap specified the attrlist it should retrieve. We know that if not specified (or empty list) it will return them all (including the binary attrs). | 16:06 |
*** joesavak has joined #openstack-keystone | 16:07 | |
jdennis | wwriverrat: but at some point someone is going to need a binary attriibute, an x509 cert is a perfect example | 16:08 |
wwriverrat | yep... so not fully comfortable with the UnicodeDecodeError catch either | 16:08 |
*** BAKfr has quit IRC | 16:09 | |
*** packet has quit IRC | 16:09 | |
*** dims_ has joined #openstack-keystone | 16:09 | |
*** dims_ has quit IRC | 16:09 | |
*** dims_ has joined #openstack-keystone | 16:10 | |
*** david-lyle has quit IRC | 16:11 | |
dstanek | i say for right now just specify the attribute | 16:11 |
*** dims has joined #openstack-keystone | 16:11 | |
dstanek | at some point in the future Keystone will just have to know what attributes are text and what are not | 16:12 |
dstanek | think in terms of defining a SQLAlchemy model | 16:12 |
*** dims_ has joined #openstack-keystone | 16:13 | |
jdennis | FWIW lessons learned the hard way during IPA development is your LDAP code must know the schema otherwise you're flying blind when it comes to data types | 16:13 |
*** dimsum_ has quit IRC | 16:14 | |
*** dims_ is now known as dimsum_ | 16:14 | |
wwriverrat | There is a couple of identified locations where ldap being called in this bug. There is also one fix. https://bugs.launchpad.net/keystone/+bug/1355489 | 16:14 |
uvirtbot | Launchpad bug 1355489 in keystone "authenticate ldap binary fields fail when converting fields to utf8" [Medium,Triaged] | 16:14 |
*** dimsum_ has quit IRC | 16:14 | |
*** dolphm changes topic to "Countdown to RC1 https://launchpad.net/keystone/+milestone/juno-rc1" | 16:15 | |
*** dimsum_ has joined #openstack-keystone | 16:16 | |
*** dimsum_ has quit IRC | 16:17 | |
*** packet has joined #openstack-keystone | 16:19 | |
*** marcoemorais has joined #openstack-keystone | 16:20 | |
*** dims has joined #openstack-keystone | 16:24 | |
*** dims has quit IRC | 16:24 | |
ayoung | dstanek, http://logs.openstack.org/74/74974/15/check/gate-python-keystoneclient-python33/4eee72e/console.html seems the problem with Kerberos in Python is that it imports commands which is a python27 thing. Is that right? | 16:28 |
dstanek | ayoung: looking...but i thought commands was old school | 16:28 |
ayoung | dstanek, it looks like python-kerberos is old school, too | 16:29 |
dstanek | yeah, commands has been deprecated since 2.6 :-( | 16:29 |
dstanek | i have bigger fish to fry with the 33 tests, so i can get this fixed too if you don't have the time | 16:30 |
*** marcoemorais has quit IRC | 16:32 | |
*** marcoemorais has joined #openstack-keystone | 16:32 | |
*** dims has joined #openstack-keystone | 16:33 | |
*** packet has quit IRC | 16:33 | |
ayoung | dstanek, that would be great | 16:34 |
ayoung | dstanek, I am not clear what is happening | 16:34 |
*** packet has joined #openstack-keystone | 16:34 | |
ayoung | is it from the build process, or is it something in the kerberos package we fetch from upstream? | 16:34 |
nkinder | wwriverrat: I have a newer patch that fixes all of the areas where we don't specify attributes that I could fine (in the identity and assignment drivers) | 16:36 |
wwriverrat | cool | 16:36 |
nkinder | wwriverrat, jdennis, dstanek: so was the decision to just specify the attributes to return and leave the decoding code as is? | 16:38 |
nkinder | I'll add that this issue was tough to track down with the current logging, so perhaps we should catch the UnicodeDecodeError and at least log a debug message | 16:39 |
jdennis | nkinder: I believe so, but down the road we really need to address the need to know the data type of attributes, see my comment in https://bugs.launchpad.net/keystone/+bug/1355489 | 16:39 |
uvirtbot | Launchpad bug 1355489 in keystone "authenticate ldap binary fields fail when converting fields to utf8" [Medium,Triaged] | 16:39 |
dstanek | nkinder: i'd be ok with just specifying the attributes and letting the decoding bomb | 16:40 |
nkinder | jdennis: I agree. I'm looking for a short term solution. This hoses up Keystone->AD pretty badly at the moment | 16:40 |
dstanek | nkinder: that other patch logs it, but allows the request to keep going after if fails | 16:40 |
nkinder | dstanek: which patch? | 16:41 |
jdennis | nkinder: yup I think your approach is a reasonable short term work around | 16:41 |
dstanek | i asumg that it works for the patch auther because if was trying to decode a non-necessary binary field | 16:41 |
dstanek | nkinder: https://review.openstack.org/#/c/118430/4/keystone/common/ldap/core.py,cm | 16:41 |
wwriverrat | for the logging the attempted value my opinion wavers: 1) would be nice to see what failed, but 2) I really dont want to clutter my logs with the binary stuff (ours has jpeg image as attribute). | 16:42 |
*** cjellick_ has joined #openstack-keystone | 16:43 | |
*** cjellick has quit IRC | 16:43 | |
dolphm | anyone interested in a script to give you real time growl notifications when there's any activity on your starred reviews in gerrit? | 16:43 |
dstanek | wwriverrat: if we specified every attribute explicitly the only errors in the logs would be true errors | 16:43 |
dstanek | dolphm: me, me, pick me | 16:43 |
wwriverrat | agree | 16:43 |
dolphm | dstanek: lol - k. just wanted to guage if i'm the only OCD one or not. i'll publish on github later today then | 16:44 |
dstanek | dolphm: nice | 16:44 |
nkinder | dstanek: I'm adding my logging to convert_ldap_result() instead of ldap2py(), as it lets us say what attribute and dn we encountered the problem in | 16:46 |
*** rushiagr is now known as rushiagr_away | 16:47 | |
morganfainberg | dolphm, oh god. that memcache bug. | 16:49 |
morganfainberg | dolphm, it's eventlet. | 16:49 |
morganfainberg | dolphm, just confirmed and it's nasty. massive leak | 16:49 |
dstanek | morganfainberg: memcache bug? | 16:51 |
morganfainberg | dstanek, threadlocal + eventlet = OMG BAD | 16:51 |
morganfainberg | basically we leak memcache clients all over | 16:51 |
dstanek | morganfainberg: that's not cool. do we not patch threading or does patching it now patch threadlocals? | 16:53 |
morganfainberg | dstanek, i think that is an eventletism | 16:54 |
*** r1chardj0n3s has left #openstack-keystone | 16:54 | |
morganfainberg | dstanek, under apache we leak no clients | 16:54 |
*** r1chardj0n3s has joined #openstack-keystone | 16:54 | |
morganfainberg | so eventlet probably patches it | 16:54 |
nkinder | dstanek: I still think we should continue processing and just skip the bad attribute for now honestly. If we encounter a binary attribute that Keystone doesn't need, it just seems better to not blow up | 16:54 |
r1chardj0n3s | ayoung: FYI further CORS conversations will be over in #openstack-oslo, since the keystone involvement is now complete AFAICT | 16:54 |
*** r1chardj0n3s has left #openstack-keystone | 16:55 | |
dstanek | morganfainberg: take a look at keystone-all - from some reason we are not patching threading | 16:55 |
dstanek | if we were this may go away | 16:55 |
morganfainberg | oh hm | 16:55 |
morganfainberg | let me try that in my devstack | 16:55 |
morganfainberg | dstanek, isn't https://github.com/openstack/keystone/blob/master/bin/keystone-all#L131 this supposed to patch thread if not set? | 16:57 |
morganfainberg | dstanek, https://github.com/openstack/keystone/blob/master/bin/keystone-all#L139 | 16:57 |
*** gokrokve has joined #openstack-keystone | 16:58 | |
dstanek | depends on that standard threads stuff | 16:58 |
morganfainberg | right | 16:58 |
morganfainberg | i'm fairly certain i'm not setting standard threads here | 16:58 |
*** david-lyle has joined #openstack-keystone | 16:58 | |
*** rkofman1 has quit IRC | 16:59 | |
*** rkofman has joined #openstack-keystone | 16:59 | |
morganfainberg | i'll bet if standard threads are set, we don't leak | 17:01 |
ayoung | gyee, you ready to talk LDAP yet> | 17:03 |
ayoung | ? | 17:03 |
*** marcoemorais has quit IRC | 17:04 | |
*** openstackgerrit has quit IRC | 17:04 | |
*** marcoemorais has joined #openstack-keystone | 17:04 | |
*** marcoemorais has quit IRC | 17:05 | |
*** marcoemorais has joined #openstack-keystone | 17:05 | |
*** harlowja_away is now known as harlowja | 17:05 | |
*** marcoemorais has quit IRC | 17:07 | |
*** marcoemorais has joined #openstack-keystone | 17:07 | |
jdennis | nkinder: fwiw convert_ldap_result() and the other utf8 changes were modeled after IPA LDAP code, however as I hope is obvious the current implementation of ldap2py is a hack. In IPA the equivalent code performs a schema lookup to know what the data type of the attribute is only then converts it to a Python type. | 17:07 |
gyee | ayoung, yo | 17:08 |
ayoung | gyee, So something is wonky | 17:08 |
gyee | ayoung, so I removed the read-only check logic | 17:08 |
ayoung | gyee, the code that nkinder is working on fixing is for AD in a read only mode, and it used id_to_dn | 17:08 |
gyee | ayoung, won't work for AD either | 17:09 |
ayoung | Now, his fix aside, that shows that id_to_dn is not just supposed to be used in read-write mode | 17:09 |
gyee | AD uses SMAccountName for username | 17:09 |
jdennis | nkinder: the attempt to convert to an int and if that failed convert to a string was the existing (flawed) logic that was merely copied because adding schema lookups was beyond the scope of the patch I made. | 17:09 |
ayoung | gyee, what won't work? | 17:09 |
gyee | ayoung, AD uses SAAccountName for username | 17:10 |
ayoung | sAMAccount name, yes, I've become friendly with it | 17:10 |
gyee | CN is not unique | 17:10 |
ayoung | I think username is not an issue here, it is the user_id only | 17:10 |
ayoung | and if you wanted to use sAMAccounName for that, you would need to use subtree | 17:10 |
ayoung | here's the problem | 17:11 |
ayoung | and, once again, it IS NOT YOUR PATCH that iis the problem. It is the existing code | 17:11 |
ayoung | your patch just showed the problem...and I'm trying to figure out how to expose it | 17:11 |
gyee | ayoung, bottom line, we need to honor the attribute maps for read-only LDAP | 17:12 |
ayoung | I think that your patch, as currrently written, will be OK. | 17:12 |
*** HenryG is now known as HenryThe8th | 17:12 | |
ayoung | I'm not 100% certain, however. Just pretty-sure | 17:12 |
gyee | ayoung, I think we need to optimized for read-only LDAP going forward | 17:12 |
ayoung | If I understand what is happening, in the read-write case, there is an assumption that the RDN will match the user_id_attribute field | 17:13 |
gyee | from what we've been hearing, seem like read-only is more urgent | 17:13 |
ayoung | It used to be part of the read-only case, too | 17:13 |
ayoung | People are using Read-Write, too. We can't break that | 17:13 |
*** cjellick_ has quit IRC | 17:14 | |
*** cjellick has joined #openstack-keystone | 17:14 | |
ayoung | Even if it is a smaller use case. With Henrynash's change to multi-backends we'll see more writable LDAP | 17:14 |
morganfainberg | dstanek, this is likely going to require monkeypatching memcache backend. | 17:14 |
morganfainberg | dstanek, like the memcache python lib. | 17:14 |
ayoung | but lets get a solution that makes everything work | 17:14 |
morganfainberg | ugh. i'm very non-plussed :( | 17:14 |
gyee | ayoung, k | 17:14 |
gyee | ayoung, I think we should use attribute map for everything | 17:15 |
ayoung | gyee, so there are 3 use cases: read-only/one read-only/sub read-write/sub | 17:15 |
gyee | but we shouldn't make any distinction between one and sub | 17:15 |
ayoung | I think we can convert the read-only/one case to be the same as read-only/sub | 17:15 |
gyee | they are just search scopes, should not impact the way we formulate the IDs | 17:16 |
ayoung | we need to get rid of the logic that says "id comes from the DN" | 17:16 |
gyee | ayoung, ++++++++ | 17:16 |
ayoung | gyee, I'm just concerned that removing that will break someone | 17:16 |
dstanek | morganfainberg: bummer. does that mean we'll have to maintain monkey patching code of the supported memcached libs? | 17:16 |
morganfainberg | talking w/ zzzeek and YoriSar in oslo channel about how we're going to handle this going forward. but for icehouse and juno it's going to be non-fun | 17:17 |
gyee | ayoung, we'll deal with them one at a time, don't think we have much choices | 17:17 |
gyee | ayoung, the Jenkins gates are all we have to work with right now | 17:17 |
gyee | we can add more test coverages as we discover more use cases | 17:18 |
ayoung | gyee, if we make any changes here, I think we are going to break things for someone | 17:20 |
gyee | I am all for adding more tests, but we can only test the knowns | 17:20 |
ayoung | gyee, why do you need this patch? | 17:20 |
gyee | ayoung, we need to deal with read-only LDAPs | 17:20 |
ayoung | gyee, that is not an answer | 17:21 |
ayoung | this code has been run for a long while now | 17:21 |
ayoung | what is the actual problem | 17:21 |
gyee | ayoung, probelm is we don't have control over the customer's DIT | 17:21 |
dhellmann | dstanek: the new release of oslotest with your fix is on pypi | 17:22 |
ayoung | So | 17:22 |
gyee | and we are not honoring the id attribute mpas | 17:22 |
gyee | maps | 17:22 |
ayoung | gyee, only for ID. and that is a known exception | 17:22 |
ayoung | Are you seeing an actual problem? | 17:22 |
gyee | ayoung, that's a bug, not exception | 17:22 |
ayoung | Npo | 17:22 |
ayoung | No | 17:22 |
ayoung | the SUB apporach (where ID is an attribute) came second | 17:22 |
*** dhellmann is now known as dhellmann_ | 17:23 | |
ayoung | the ID is part of the DN came first | 17:23 |
dstanek | dhellmann: great thanks! | 17:23 |
ayoung | so you can't call it a bug if it was the initial design | 17:23 |
gyee | ayoung, what is user_id_attribute for? | 17:23 |
ayoung | so let me rephrase the question. What problem does not honoring user_id_attribute cause you? | 17:23 |
ayoung | Your bug is not a bug. It is a feature request | 17:24 |
gyee | ayoung, everything, role assignment, lookup | 17:24 |
ayoung | Provide an example. | 17:24 |
gyee | ayoung, you telling me not honoring user_id_attribute is by design? | 17:24 |
gyee | you joking right | 17:24 |
ayoung | gyee, I'm telling you that the original design was that the RDN was the user id, not an attribute | 17:25 |
ayoung | now, I will admit this is a poor design | 17:25 |
ayoung | but there are people who have deployed around this | 17:25 |
ayoung | so, if we are going to break it, I need to be able to justify that breakage | 17:25 |
gyee | how do I assign role to user if user_id map is not correct? | 17:25 |
ayoung | and we are going to break it | 17:25 |
gyee | we ain't breaking anything | 17:25 |
ayoung | gyee, use the actual value of the user_id | 17:26 |
gyee | you keep saying we break something, offer me a test to show it | 17:26 |
ayoung | not what you "think" it should be | 17:26 |
ayoung | offer me a test to show that something is broken right now | 17:26 |
gyee | ayoung, its not what I think, its what user_id_attribute is documented forr | 17:26 |
gyee | ayoung, take a look at the bug | 17:27 |
ayoung | gyee, I've read it. I understand what you are saying. But the problem is that you are trying to pre-calculate the user_ids. For read only LDAP, that seems like it should work, but it is surprising. | 17:27 |
ayoung | Of anything else (SQL) you don't know the UserID until it is created anyway | 17:28 |
ayoung | so, before we change this, and break it for someone, show me why you need this change | 17:28 |
ayoung | I don't disagree that it varies from the docs | 17:28 |
gyee | ayoung, problem is I can't even assign role if user_id map is broken | 17:30 |
ayoung | yes you can | 17:30 |
gyee | how? | 17:30 |
ayoung | you can't assign it "a-priori" | 17:30 |
ayoung | you do an actual query, that is how | 17:30 |
ayoung | look up user by username, like the SQL backend forces you to | 17:30 |
gyee | we lookup users by user_id at the backend | 17:31 |
gyee | project role assignment for example, PUT /projects/project_id/users/user_id/roles/role_id, username is not used | 17:34 |
*** rushiagr_away is now known as rushiagr | 17:44 | |
*** andreaf has quit IRC | 17:47 | |
*** andreaf has joined #openstack-keystone | 17:48 | |
nkinder | ayoung: gyee's problem is that cn is not unique. Really, cn is not an ID, but is a user name. | 17:48 |
*** david-lyle is now known as david-lyle_afk | 17:49 | |
nkinder | ideally, the user_id wodl be sAMAccoutnName (or the userPrincipalName), and user_name would be cn | 17:49 |
ayoung | nkinder, except the sAMAccountName is not in the RFC schemas. | 17:49 |
nkinder | RFCs don't matter here | 17:50 |
ayoung | And the LDAP code was written with defaults as simplisitic as possible | 17:50 |
nkinder | ayoung: the RFCs dont' state what attributes should be in the DN | 17:50 |
ayoung | nkinder, yeah...I was young and dumb. | 17:50 |
ayoung | actually, I'm still Young.... | 17:50 |
ayoung | you can draw conclusions about the other | 17:50 |
nkinder | I could be using inetOrgPerson/POSIX and have 'dn: cn=<name>,...' instead of uid being the RDN | 17:50 |
*** radez is now known as radez_g0n3 | 17:51 | |
ayoung | the mistake I made was based on how FreeIPA works: the rdn must match the id field | 17:51 |
nkinder | AD just popularized the cn as RDN approach | 17:51 |
ayoung | I now know that assumption is not true everywhere | 17:51 |
*** stevelle_ has joined #openstack-keystone | 17:52 | |
gyee | for read-only LDAP, we have zero control over customer's directory, we can only rely on the attribute maps | 17:52 |
nkinder | many AD deployments would end up with the user_id in Keystone being something like "Kinder, Nathan" | 17:52 |
nkinder | which is stupid | 17:52 |
gyee | nkinder, heh, stupid, but customer writes the check | 17:53 |
nkinder | gyee: oh, I'm not saying that it's stupid for that to be the RDN | 17:53 |
nkinder | gyee: it's just not what I woudl consider an ID for a user in Keystone | 17:53 |
nkinder | gyee: I would want the ID to be sAMAccountName (nkinder in my case) | 17:53 |
gyee | nkinder, no, but attribute map is the most effective way to support the stuff | 17:54 |
nkinder | but, that's not what's used in the DN | 17:54 |
ayoung | nkinder, I agree that what we have now is bad. I just don't think we can blindly change it without breaking someone's deployment somewhere | 17:54 |
gyee | that give us most flexibility | 17:54 |
nkinder | gyee: I think you're right, but I haven't had a change to really look into your code yet :( | 17:54 |
ayoung | nkinder, some one is going to have user_id_attribute=cn and the CN RDN is not going to match what is in the object | 17:54 |
nkinder | ayoung: define object.. the LDAP entry? | 17:55 |
gyee | ayoung, we can't worry about someone till we have a test in the gates | 17:55 |
gyee | that's the only thing we have control over | 17:55 |
ayoung | nkinder, I still don't know how the code you looked at triggered an id_to_dn lookup, as the code paths I looked at all used the user.get method which does not do that fetch | 17:55 |
ayoung | and that code predates Icehouse by a good bit | 17:55 |
nkinder | ayoung: not sure, but it did... | 17:56 |
*** wwriverrat has left #openstack-keystone | 17:56 | |
nkinder | ayoung: so what is the concern about breaking someone with what gyee is proposing? Writing to LDAP during new user creation? | 17:57 |
nkinder | and not knowing what attribute to use as the RDN when forming the DN? | 17:57 |
ayoung | nkinder, no, even in a read-only case, if the attributes don't line up, and we start honoring user_id_attribute | 17:57 |
ayoung | but...I can't help but think that it is broken already. | 17:58 |
nkinder | ayoung: I'm not sure I follow. Can you elaborate with an example? | 17:58 |
ayoung | nkinder, user_id_attribute=cn | 17:58 |
ayoung | and the cn part of the RDN does not match the CN attribute in the object | 17:59 |
nkinder | ok, and "dn: uid=foo,..."? | 17:59 |
nkinder | what do you mean cn part of the RDN | 17:59 |
ayoung | nkinder, no | 17:59 |
ayoung | dn: cn=foo,...." | 17:59 |
ayoung | but CN= Adam | 17:59 |
nkinder | ayoung: multi-valued cn's? | 17:59 |
nkinder | well, cn=foo has to exist in the entry if it's in the DN | 17:59 |
ayoung | it turns out the CN segment of the DN doesn't have to match even a single value CN attribute | 18:00 |
nkinder | it is a multi-valued attribute, but the RDN value must be present in the entry | 18:00 |
nkinder | ayoung: no, that's not true | 18:00 |
ayoung | define "must" cus I've seen cases where they don't match | 18:00 |
nkinder | it MUST be in there | 18:00 |
ayoung | it was in the AD != LDAP presentation IIRC | 18:00 |
gyee | nkinder's correct, cn must be the same in the attribute list | 18:00 |
nkinder | even in AD AFAIK | 18:00 |
nkinder | ayoung: here's what you might have seen... | 18:00 |
gyee | nkinder, correct, AD enforce that | 18:00 |
nkinder | ayoung: if you add an LDPA entry and leave out the CN in the entry that you used in the DN, the client (or server) adds it in automatically | 18:01 |
morganfainberg | dolphm, dstanek, actually this eventlet memcache thing is worse than just for keystone, once we have some kind of fix in keystone i'll look at getting it over in keystonemiddleware as well. | 18:01 |
ayoung | That was my assumption when I wrote the code origianlly, and people pointed out places where that got messed up. Sorry, its been too long to remember exactly when. | 18:01 |
gyee | we've been testing against both OpenLDAP and AD | 18:01 |
nkinder | I've written server code that does that before | 18:01 |
ayoung | I'll defer to you guys on what is "right" | 18:01 |
dolphm | morganfainberg: that's what i was afraid to assume :-/ | 18:02 |
nkinder | There are old server versions of OpenLDAP that said "go away stupid user", and newer ones that nicely add the missing attribute for you | 18:02 |
gyee | AD will throw you a very cryptic error if the attribute don't mat | 18:02 |
gyee | match | 18:02 |
*** dhellmann_ is now known as dhellmann | 18:02 | |
morganfainberg | dolphm, YorikSar is going to propose a fix to keystone today. | 18:02 |
ayoung | Let's see. I have a free IPA instance | 18:02 |
gyee | server unwilling to perform - my favor error :) | 18:02 |
nkinder | ayoung: I know what FreeIPA will do... I wrote some of the underlying 389 code that deals with this | 18:03 |
nkinder | gyee: nothing wrong with good ol' err=53 :) | 18:03 |
morganfainberg | dolphm, it's not pretty but it'll get us to K when we can rev dogpile versions and get upstream stuff all fixed / in place. | 18:03 |
gyee | haha | 18:03 |
ayoung | nkinder, I'd like to drop the ID to DN code that does not do a search first | 18:03 |
nkinder | gyee: it's the DSID errors that AD throws that drive me crazy (and no good access logging) | 18:03 |
ayoung | the one that assumes the id_attribute is a portion of the DN | 18:03 |
morganfainberg | dolphm, but short of new deps that aren't even in pip yet / releases of libs, we wont be able to do a clean fix (unless we re-implement python-memcache w/o thread local in-tree for the projectS) | 18:04 |
nkinder | ayoung: yes, we should make no assumption about the DN | 18:04 |
ayoung | and always search against LDAP. It will be slower for the ONE case, but it means that everything is consistant | 18:04 |
nkinder | ayoung: the only time where we really need to care about what is in the DN is if we are adding a user through keystone | 18:04 |
ayoung | however...that was what the AD != LDAP presentation called out. | 18:04 |
nkinder | ayoung: and we might need to add a config setting for "naming_attribute" to offer flexibility there | 18:04 |
morganfainberg | dolphm, i'll tag that bug to keystonemiddleware... do we want to fix python-keystoneclient auth_token in this case too? it's not security fix but it's a bad bug. | 18:05 |
ayoung | nkinder, this is why I think we've needed a specific LDAP meetup at the summit. | 18:05 |
gyee | yeah, sub search have a performance cost, we usually avoid it if we can | 18:05 |
ayoung | Not just in the developers side, but in the real summit... | 18:05 |
dolphm | morganfainberg: let's see the fix in middleware first, but probably | 18:05 |
morganfainberg | dolphm, ++ | 18:05 |
ayoung | gyee, yeah, and the only reason that code is there is for the assignments backend now | 18:05 |
ayoung | well, group membership, too | 18:05 |
*** bobt has joined #openstack-keystone | 18:06 | |
nkinder | ayoung: id_to_dn should only search (unless we've cached a DN in the object) | 18:06 |
nkinder | ayoung: we should never construct a DN unless it's a user add operation | 18:06 |
ayoung | yep | 18:07 |
ayoung | agreed | 18:07 |
ayoung | nkinder, just worried about making changes like this without feedback from the community | 18:07 |
ayoung | marekd, you around? | 18:07 |
gyee | nkinder, there's a patch pending to do caching at the identity manager level, which should help out with performance | 18:07 |
ayoung | We have an LDAP question, and you CERN folks might have the answer | 18:07 |
gyee | nkinder, https://review.openstack.org/#/c/110575/ | 18:08 |
morganfainberg | dolphm, ok targeted icehouse / keystonemiddleware and updated title to reflect the real issue(s). | 18:09 |
dolphm | morganfainberg: thanks! | 18:10 |
*** openstackgerrit has joined #openstack-keystone | 18:10 | |
*** aix has quit IRC | 18:10 | |
ayoung | cjellick, read up when you have a chance | 18:12 |
ayoung | cjellick, we are addressing the thing you commented on during your AD != LDAP talk | 18:13 |
cjellick | kk..will catch up. thanks for the heads up | 18:13 |
*** stevelle_ has quit IRC | 18:17 | |
ayoung | cjellick, remember when we talked about the userid in the DN? | 18:18 |
nkinder | ayoung: though I don't think that this is a violation of LDAP at all | 18:19 |
ayoung | cjellick, if we were to change that...would it break your deployment? | 18:19 |
ayoung | I'm guessing, no. | 18:19 |
ayoung | You just got tripped up on the fact that we were composing the dns, right cjellick ? | 18:20 |
*** YorikSar has joined #openstack-keystone | 18:21 | |
YorikSar | morganfainberg: I'm back. | 18:21 |
YorikSar | morganfainberg: I guess we can continue here. | 18:22 |
YorikSar | morganfainberg: Looking through logs I see you're thinking about doing this thing in middleware instead? | 18:22 |
cjellick | right, composing the dn and assuming the user_id was part of it was/is the issue | 18:22 |
nkinder | ok, we have 3 bugs for the same LDAP binary values issue...https://bugs.launchpad.net/keystone/+bug/1364521 | 18:23 |
uvirtbot | Launchpad bug 1364521 in keystone "LDAP integration with Active Directory backend can throw: UnicodeDecodeError" [Medium,In progress] | 18:23 |
nkinder | https://bugs.launchpad.net/keystone/+bug/1358330 | 18:24 |
uvirtbot | Launchpad bug 1358330 in keystone "Error on _ldap_get_list without attrlist value" [Medium,In progress] | 18:24 |
nkinder | https://bugs.launchpad.net/keystone/+bug/1355489 | 18:24 |
uvirtbot | Launchpad bug 1355489 in keystone "authenticate ldap binary fields fail when converting fields to utf8" [Medium,Triaged] | 18:24 |
nkinder | Which one should "win" here? | 18:24 |
nkinder | I have a proposed fix, but I'm not sure which bug to reference | 18:25 |
nkinder | perhaps the oldest one, then we can abandon the others (assuming my approach is accepted)? | 18:25 |
morganfainberg | YorikSar, welcome back | 18:26 |
morganfainberg | YorikSar, i only have ~30more mins of battery | 18:26 |
morganfainberg | YorikSar, no we need to do this in middleware as well | 18:26 |
morganfainberg | YorikSar, since we deploy middleware under eventlet and it can use memcache to cache tokens | 18:27 |
morganfainberg | YorikSar, it's not just dogpile specific. | 18:27 |
morganfainberg | YorikSar, so once we have the fix for keystone i'll work on implementing that for middleware (should be a lot easier) | 18:28 |
YorikSar | morganfainberg: But where should we put that pool then?.. It'd be sad to copypaste it around. | 18:28 |
morganfainberg | YorikSar, again due to the fact we can't add new deps at the moment we likely will need to copy it around some. we *might* be able to put it in middleware and import it to keystone... but long term it should go to a library as we discussed in -oslo channel | 18:29 |
YorikSar | morganfainberg: Yes, sure. | 18:30 |
gyee | nkinder, I think the first two have patches in review right now, you may want to ping the authors to consolidate | 18:30 |
YorikSar | morganfainberg: Btw, does middleware use dogpile? | 18:30 |
morganfainberg | YorikSar, no, it uses a separate pool implementation | 18:30 |
morganfainberg | YorikSar, https://github.com/openstack/keystonemiddleware/blob/master/keystonemiddleware/auth_token.py#L1085-L1109 | 18:30 |
morganfainberg | YorikSar but everything in middleware is effectively private, so updating it will be easier than keystone, it's doesn't have many public interfaces to worry about | 18:31 |
YorikSar | morganfainberg: Just found out that I've been testing Icehouse version using devstack's master. Keystone stability is cool :) | 18:33 |
morganfainberg | YorikSar, we've worked *really* hard on that | 18:33 |
morganfainberg | YorikSar, :) | 18:34 |
morganfainberg | YorikSar, and i'm super happy to hear that. means we're doing something right :) | 18:34 |
YorikSar | morganfainberg: although then my patch got tested on our 'scale' lab, with all Icehouse :) | 18:34 |
morganfainberg | there has been a lot of shuffling of stuff in juno within keystone, but the eventlet and common.cache and common.kvs have stayed fairly stable | 18:35 |
YorikSar | morganfainberg: Oh, when I got appointed to "handle Keystone issues for our OpenStack distro" my first reaction was: "what issues?" :) | 18:35 |
YorikSar | morganfainberg: And it seems that memcached thing is the only issue we found | 18:36 |
YorikSar | morganfainberg: Yeah, my patch rebased cleanly on them :) | 18:36 |
morganfainberg | dolphm, ayoung, gyee, bknudson, lbragstad, dstanek, stevemar, topol, ^ See that compliment from YorikSar! :) good stuff! | 18:37 |
bknudson | he he. | 18:37 |
ayoung | morganfainberg, looks like I need to break more things then | 18:37 |
morganfainberg | ayoung, hehehe | 18:37 |
bknudson | can I send some your way? | 18:38 |
morganfainberg | if i haven't said it recently, i'm very happy with what we've accomplished with Keystone. and it's a good team to be working with :) | 18:38 |
stevemar | that is pretty awesome to hear YorikSar and morganfainberg | 18:39 |
morganfainberg | nkinder, ayoung, https://bugs.launchpad.net/keystone/+bug/1360446 this is potential dos effect, do we need an OSSN on this? | 18:39 |
uvirtbot | Launchpad bug 1360446 in keystonemiddleware "client connection leak to memcached under eventlet due to threadlocal" [High,Triaged] | 18:39 |
ayoung | morganfainberg, I think that bug title should be "Eventlet considered harmful. Flee" | 18:40 |
gyee | ha | 18:40 |
morganfainberg | ayoung, don't disagree.. | 18:40 |
ayoung | morganfainberg, let me read a bit more in depth | 18:40 |
*** hrybacki has joined #openstack-keystone | 18:40 | |
YorikSar | morganfainberg: Ok. It looks like I'll be working on keystonemiddleware as well. | 18:40 |
bknudson | morganfainberg: if we're providing a fix then it's generally OSSA | 18:41 |
morganfainberg | bknudson, ok OSSA | 18:41 |
morganfainberg | but same question applies | 18:41 |
morganfainberg | YorikSar, cool. let me know if you need any help. | 18:41 |
YorikSar | morganfainberg: (that guy who reported it is from our team) | 18:41 |
ayoung | morganfainberg, standard-thread under Eventlet is even an option? | 18:41 |
morganfainberg | ayoung, i don't think so not for production | 18:42 |
ayoung | morganfainberg, are there tuning parameters, or does it require a code fix? | 18:42 |
morganfainberg | ayoung, this requires a code fix. nothing we can tune | 18:42 |
ayoung | OK | 18:42 |
bknudson | morganfainberg: I thought we had a change in middleware to avoid thread local issues. | 18:42 |
ayoung | morganfainberg, and its not just memcached backend, but using it for caching. Is it all dogpile, or just memcached? | 18:42 |
bknudson | morganfainberg: we have a pool of memcached clients. | 18:43 |
morganfainberg | bknudson, it doesn't look like it works, dogpile does pooling as well | 18:43 |
bknudson | morganfainberg: dolphm provided that fix | 18:43 |
morganfainberg | bknudson, memcache library explicitly uses unavoidable threadlocal | 18:43 |
morganfainberg | bknudson, and wrapping it in the pool doesn't "fix" the issue. | 18:43 |
morganfainberg | it just makes some of the racyness go away | 18:44 |
*** andreaf has quit IRC | 18:44 | |
morganfainberg | we still run into issues with running out of FDs due to leaking connections | 18:44 |
bknudson | morganfainberg: memcache lib needs eventlet monkeypatching | 18:44 |
morganfainberg | bknudson, yes. :( | 18:44 |
*** andreaf has joined #openstack-keystone | 18:45 | |
morganfainberg | it's *basically* what YorikSar is doing. | 18:45 |
dolphm | bknudson: that just fixed a security vulnerability... or seemed to | 18:45 |
YorikSar | morganfainberg: Do you know if smth changed with parsing backend_argument in [cache]? | 18:45 |
morganfainberg | YorikSar, shouldn't have. | 18:45 |
morganfainberg | dolphm, which iirc was race-related | 18:46 |
dolphm | morganfainberg: yes | 18:46 |
YorikSar | morganfainberg, bknudson: Nooo, it's eventlet-friendly. It's so eventlet-friendly that we basically get new memcached Client instance for every green thread. | 18:46 |
morganfainberg | LOL | 18:46 |
morganfainberg | that is a good description | 18:46 |
morganfainberg | it works *too well* with eventlet | 18:47 |
YorikSar | And if we wrap it in pool, we'll get new connection for every green thread for every Client in pool | 18:47 |
dolphm | morganfainberg: talking about my unbounded poolish thing? | 18:48 |
morganfainberg | bknudson, if the deployer doesn't use memcache in middleware/keystone it wont cause issues though. | 18:48 |
morganfainberg | dolphm, yeah | 18:48 |
topol | did I get pinged? | 18:48 |
morganfainberg | bknudson, or in the case of keystone, use apache, again no issue. | 18:48 |
dolphm | under load, yep! | 18:48 |
dolphm | morganfainberg: ++ use not eventlet many problem is fixed | 18:49 |
morganfainberg | topol, yes was pointing a nice compliment out to the people who live eat sleep and breath keystone, scroll up :) | 18:49 |
bknudson | eventlet is unsafe at any speed. | 18:49 |
bknudson | convenient, though | 18:49 |
morganfainberg | bknudson, +∞ | 18:50 |
YorikSar | morganfainberg: I'm lost... How do I provide 'url' backend_argument to cache backend?.. | 18:50 |
topol | morganfainberg, I see that. From YorikSar. Been a while since we have heard from him :-) | 18:50 |
morganfainberg | YorikSar, hm. ok let me see. | 18:50 |
morganfainberg | YorikSar in kvs or cache? | 18:50 |
topol | YorikSar still loving the LDAP keystone integration world? | 18:50 |
YorikSar | morganfainberg: It used to be like backend_argument=url:localhost:11211,localhost:11212,localhost:11213 | 18:51 |
morganfainberg | unfortunately it's two different implementations **to be fixed soon* | 18:51 |
YorikSar | morganfainberg: cache, kvs don't have arguments (or didn't?) | 18:51 |
morganfainberg | YorikSark kvs does but it's super awkward | 18:51 |
YorikSar | topol: Yeah.. I even have intern working on LDAP :) | 18:51 |
morganfainberg | yeah that looks right for cache | 18:51 |
YorikSar | topol: https://review.openstack.org/#/q/owner:%22Peter+Razumovsky%22+status:open,n,z | 18:52 |
morganfainberg | topol, yes, backend_argument=url:localhost:11211.... | 18:52 |
morganfainberg | erm YorikSar ,^ not topol | 18:52 |
morganfainberg | YorikSar, that should still be the syntax | 18:52 |
YorikSar | morganfainberg: But... It doesn't work anymore. There used to be special branch for 'url' | 18:52 |
topol | easy to confuse. Im a better looking version of YorikSar :-) | 18:53 |
morganfainberg | YorikSar, it should still come from build_cache_config in keystone.common.cache.core | 18:53 |
YorikSar | morganfainberg: around here: https://github.com/openstack/keystone/blob/master/keystone/common/cache/core.py#L93 | 18:53 |
morganfainberg | YorikSar, yesh | 18:54 |
YorikSar | morganfainberg: Yes, but it isn't split into list anymore... | 18:54 |
morganfainberg | oh url needs to be an explicit list? | 18:54 |
YorikSar | morganfainberg: Yep. | 18:54 |
morganfainberg | oh wait | 18:54 |
YorikSar | morganfainberg: It might happily handle string like '127.0.0.1:11211', but not comma-separated list | 18:55 |
morganfainberg | YorikSar, hmm.. ah that must be it | 18:55 |
morganfainberg | doh! | 18:55 |
morganfainberg | ok associated bug | 18:55 |
morganfainberg | :( | 18:55 |
morganfainberg | ok i'm out of battery and need lunch. | 18:55 |
morganfainberg | sorry | 18:55 |
morganfainberg | be back in a bit. | 18:55 |
morganfainberg | (have someone waiting for me for lunch) | 18:55 |
* YorikSar found a bug while fixing two other bugs (and might be adding forth one to the list) | 18:56 | |
morganfainberg | YorikSar, hehe | 18:56 |
YorikSar | morganfainberg: I'll try to overcome that and test my pool on master in the mean time. | 18:56 |
*** arosen has joined #openstack-keystone | 18:57 | |
ayoung | bknudson, on https://bugs.launchpad.net/keystone/+bug/1366062 if ldap is in requirements, shouldn't ldappool be there, too? | 19:00 |
uvirtbot | Launchpad bug 1366062 in keystone "ldappool is not present in requirements.txt" [Undecided,Won't fix] | 19:00 |
bknudson | ayoung: http://git.openstack.org/cgit/openstack/keystone/tree/requirements.txt | 19:01 |
*** openstackgerrit has quit IRC | 19:01 | |
bknudson | there's no ldap in there | 19:01 |
*** openstackgerrit has joined #openstack-keystone | 19:01 | |
bknudson | it's in http://git.openstack.org/cgit/openstack/keystone/tree/test-requirements.txt | 19:02 |
bknudson | (both ldap and ldappool are in there) | 19:02 |
ayoung | bknudson, hmmm, I thought we had agreed to move ldap. Must be misrememberising | 19:02 |
bknudson | I guess it's just because it's optional to configure keystone for ldap | 19:03 |
bknudson | or it's not the default configuration | 19:03 |
*** packet has quit IRC | 19:03 | |
ayoung | dstanek, morganfainberg I need to debug a tox issue. THere is something in /opt/stack/python-keystoneclient/.tox/py33/build/kerberos/setup.py that is messing things up, but that file does not exist post tox run...what am I missing? | 19:04 |
ayoung | bknudson, yeah, I remember the discussion. Might mess up the packagers, though ,as I know they look at requirements.txt | 19:05 |
*** rushiagr is now known as rushiagr_away | 19:05 | |
dstanek | ayoung: build files are deleted after tox creates the venv - you'll have to get a local checkout of the code | 19:05 |
bknudson | ayoung: I just got a defect from someone because we don't have an ldappool rpm. | 19:05 |
bknudson | ayoung: so it will mess up packagers. | 19:06 |
ayoung | bknudson, I wonder if we should do that as an optional import? | 19:06 |
ayoung | IE, not fail if it doesn't exist? | 19:06 |
ayoung | or is ldappool going to be the norm, do you think? | 19:06 |
*** joesavak has quit IRC | 19:07 | |
bknudson | ayoung: considering the performance increase that gyee mentioned I think it should be used. | 19:07 |
ayoung | ok...lets see the packagin status... | 19:07 |
bknudson | I'm surprised that ldap admins don't kill us for what we do to their servers. | 19:08 |
ayoung | http://koji.fedoraproject.org/koji/packageinfo?packageID=13647 | 19:08 |
ayoung | that is upstream Fedora... | 19:08 |
ayoung | 1.0-3.fc20 getting installed | 19:09 |
*** packet has joined #openstack-keystone | 19:09 | |
*** samuelmz_ has joined #openstack-keystone | 19:12 | |
YorikSar | morganfainberg: Ok, that parsing was added in our internal patch... I wonder why didn't it land to upstream already. | 19:12 |
dstanek | ayoung: kerberos looks like it may take a bit of work to be py33 compatible, but the good news is that all of the code is right there in *subversion* | 19:13 |
ayoung | dstanek, rapture | 19:13 |
bknudson | there's a git-subversion tool | 19:14 |
*** david-lyle_afk is now known as david-lyle | 19:14 | |
dstanek | bknudson: been there done that - it still sucks | 19:14 |
dstanek | i had to do that were i used to work - you couldn't take advantage of the best git feature -- merges | 19:15 |
dstanek | on the other hand the project hasn't been released in 3 years or seen a commit in 4 months so likely there would be no need for merging | 19:15 |
samuelmz_ | Hi, I'd like to know your thoughts on using functional programming on keystone | 19:16 |
dstanek | samuelmz_: in what context? | 19:16 |
samuelmz_ | Sometimes I get advises to use it on my patches | 19:16 |
YorikSar | bknudson: Unfortunately those who actually use LDAP backend are enterprises that can easily throw more resources to LDAP server. | 19:16 |
samuelmz_ | like https://review.openstack.org/#/c/117787/6/keystone/assignment/backends/sql.py | 19:16 |
YorikSar | bknudson: And they are far-far from upstream. | 19:16 |
YorikSar | bknudson: That's why LDAP backend lacks behind... | 19:17 |
samuelmz_ | dstanek, in those cases, I don't think we have a better performance nor readability | 19:17 |
dstanek | samuelmz_: what wu is talking about isn't really functional programming - if you give me a few to finish up what i was working on i can take a deeper look | 19:17 |
samuelmz_ | dstanek, yes sure | 19:18 |
*** broskos has joined #openstack-keystone | 19:18 | |
dstanek | samuelmz_: to me what Alexander suggested is really, really not readable | 19:18 |
samuelmz_ | dstanek, +1 | 19:18 |
ayoung | dstanek, are you dealing with he Kerberos packaging? | 19:20 |
dstanek | ayoung: no i just took a look at the code - i can't be packaged by py3 | 19:21 |
ayoung | dstanek, I don;t think anything can package you | 19:21 |
dstanek | i work fine in py2 :-) | 19:22 |
*** stevemar has quit IRC | 19:23 | |
*** joesavak has joined #openstack-keystone | 19:23 | |
*** stevemar has joined #openstack-keystone | 19:24 | |
broskos | what does the workflow -1 mean? | 19:24 |
broskos | https://review.openstack.org/#/c/118430/ | 19:24 |
bknudson | broskos: work in progress | 19:24 |
broskos | ok | 19:25 |
broskos | ty | 19:25 |
broskos | bknudson: did you have a particular length in mind for your truncate suggestion? | 19:26 |
bknudson | broskos: actually I don't want to see the value because I think it will be useless. | 19:27 |
bknudson | 10 chars should be enough | 19:27 |
bknudson | but as I said I don't see what the point is of logging the value | 19:27 |
ayoung | dstanek, is this a show stopper? Is there no way to do commands in python 3? | 19:27 |
broskos | sure.. yep, it is pretty useless. I’ll chop it to 10. | 19:27 |
broskos | bknudson: I origninally submitted it without logging :) | 19:28 |
bknudson | broskos: logging at debug seems adequate. | 19:29 |
broskos | broskos: and have done 3 or so updates on polishing the output that really doesn’t tell you which dn throws it. | 19:29 |
broskos | bknudson: good idea | 19:29 |
dstanek | ayoung: we can't deploy to py3 anyway - so i wouldn't even worry about it | 19:29 |
ayoung | dstanek, its client code | 19:29 |
bknudson | broskos: we've got so many problems with logging and error reporting that I can't make a big deal out of this one. | 19:29 |
dstanek | oh, hmmm.. that's not good then | 19:29 |
ayoung | dstanek, right | 19:29 |
dstanek | ayoung: i wonder if it would be easy to port... | 19:30 |
ayoung | dstanek, I'm going to try now | 19:30 |
broskos | bknudson: cool.. I’m just happy to get something in that works for whatever crap AD is spewing back at us | 19:30 |
broskos | There are other fixes in progress to limit the attributes returned, I believe | 19:31 |
*** amirosh has joined #openstack-keystone | 19:31 | |
dstanek | samuelmz_: i commented on that review | 19:32 |
samuelmz_ | dstanek, ok i'm gonna take a look at it, thanks | 19:32 |
dstanek | ayoung: there's good amount of C in there :-( | 19:34 |
ayoung | dstanek, you say that like that is a bad thing | 19:34 |
dstanek | ayoung: for porting it's either been really easy or really hard for me | 19:35 |
openstackgerrit | Aaron Rosen proposed a change to openstack/python-keystoneclient: Sync with latest oslo-incubator https://review.openstack.org/119451 | 19:35 |
dstanek | since there is almost no code besides C it may end up being really easy | 19:35 |
openstackgerrit | Yuriy Taraday proposed a change to openstack/keystone: Add a pool of memcached clients https://review.openstack.org/119452 | 19:36 |
dstanek | ayoung: it may be as easy as replacing commands with subprocess | 19:36 |
YorikSar | morganfainberg, ayoung, bknudson, dolphm, whoever else interested in memcached bug: ^^^ | 19:36 |
ayoung | dstanek, I'm going to get it into Git first | 19:36 |
dolphm | dstanek: any chance you have terminal-notifier installed? (brew) | 19:38 |
dstanek | dolphm: i don't, but that's easy to change | 19:39 |
dolphm | dstanek: i'm looking for a better option | 19:40 |
dstanek | dolphm: i have used a python lib for this for other stuff like weechat | 19:40 |
dstanek | i think the one i currectly use is pygrowl, but i'd have to doublecheck | 19:41 |
dolphm | dstanek: i'm about to try this https://wiki.python.org/moin/MacPython/Growl/AppleScriptSupport | 19:43 |
dolphm | dstanek: but right now i'm using subprocess to call terminal-notifier | 19:43 |
bknudson | is that like VBScript? | 19:43 |
dstanek | dolphm: i think i just 'pip install growl' to get my weechat plugin to work | 19:44 |
dolphm | dstanek: https://github.com/dolph/gerrit-growler | 19:46 |
dstanek | dolphm: nice, i'll give is a try momentarily | 19:47 |
dolphm | dstanek: run it with --debug and --verbose the first time ... otherwise it authenticates similarly to next-review | 19:50 |
openstackgerrit | Nathan Kinder proposed a change to openstack/keystone: Avoid conversion of binary LDAP values https://review.openstack.org/119457 | 19:55 |
bknudson | nkinder: you and broskos should make sure your fixes work together! | 19:55 |
nkinder | gyee, ayoung, dstanek, bknudson: ^^^ that should address this binary value issue | 19:55 |
ayoung | nkinder, looking | 19:55 |
nkinder | bknudson: they should, though I don't know that his is necessary the way I implemented mine | 19:56 |
ayoung | nkinder, I've hit a bit of a blocker on Kerberos. It won't build under python33, and python33 is required for keystoneclient | 19:56 |
bknudson | nkinder: it's got kind of the same code. | 19:56 |
nkinder | bknudson: that is a good topic for discussion though | 19:56 |
nkinder | bknudson: yes, just at different levels | 19:56 |
dolphm | stevemar: see you in 12 more hours | 19:57 |
ayoung | nkinder, what is the ['1.1'] in there? | 19:57 |
stevemar | dolphm, ugh | 19:57 |
stevemar | we really need recheck rubber stamps | 19:58 |
nkinder | ayoung: 1.1 means 'return no values' | 19:58 |
bknudson | stevemar: just ninja it at some point | 19:58 |
nkinder | ayoung: only the dn will be returned | 19:58 |
nkinder | ayoung: it's an LDAP standard | 19:58 |
ayoung | nkinder, MAJICK! | 19:58 |
broskos | :) | 19:59 |
ayoung | code looks good. but wonder if that should be commented | 19:59 |
bknudson | ayoung: http://tools.ietf.org/html/rfc4511#section-4.5.1.8 | 19:59 |
ayoung | I mean, really, no one know LDAP | 19:59 |
nkinder | broskos: so, didn't want to step on your patch either | 19:59 |
bknudson | it's a special OID | 19:59 |
dolphm | stevemar: i *think* when jenkins says "This change depends on a change that failed to merge." that you don't need to recheck. when the failed underlying patch re-gates, the subsequent patch will go with it | 19:59 |
nkinder | ayoung: we already use 1.1 throughout the assignment code | 19:59 |
bknudson | nkinder: ayoung: I think I did comment it at one place... | 19:59 |
ayoung | good enough | 19:59 |
broskos | nkinder: I’d rather fix it before it get’s to ldap2py as you have done | 19:59 |
nkinder | broskos: yeah, we can log the attr name that way | 20:00 |
stevemar | dolphm, yeah i also think that too, but i don't want to miss out on it | 20:00 |
ayoung | nkinder, +2 from me...the change looks right | 20:00 |
dolphm | stevemar: https://github.com/dolph/gerrit-growler/ | 20:00 |
dolphm | stevemar: fair enough | 20:00 |
*** topol has quit IRC | 20:01 | |
ayoung | http://ksylvest.github.io/jquery-growl/ | 20:01 |
broskos | nkinder: do we want to keep a try around the utf8 encode where I have it? The main reason that I ask, is that if something else falls through, all we get to go on is user/password error | 20:01 |
ayoung | very different from http://cdnjs.com/libraries/bootstrap-growl | 20:01 |
* ayoung going to name his next project growl | 20:01 | |
broskos | nkinder: otherwise I can just abandon | 20:01 |
openstackgerrit | Aaron Rosen proposed a change to openstack/python-keystoneclient: Sync with latest oslo-incubator https://review.openstack.org/119451 | 20:02 |
ayoung | broskos, so long as why gets logged in debug I think that is correct behavior | 20:02 |
nkinder | broskos: I added a log statement that will catch it too, though I left it at DEBUG | 20:02 |
ayoung | broskos, here's my rationale | 20:02 |
ayoung | we've seen this from people "trying out" new versions of Keystone | 20:02 |
ayoung | they are going to start getting problems, and need to be able to debug it | 20:02 |
ayoung | we don't want to spam a log for a live deployment | 20:03 |
ayoung | so, we really need to focus on providing the tools for identifying the problem in debug mode first and foremoist | 20:03 |
ayoung | most | 20:03 |
broskos | ayoung: makes sense - I’m not a fan of useless logging. Previously it was a silient failure, which had nothing to do with user/pw specifically | 20:04 |
broskos | and it took several hours of digging to find the real issue | 20:04 |
ayoung | broskos, and twice I've spent a lot of time tracingthrough ... and nkinder picked that up from me yesterday...this is a pain to debug | 20:04 |
broskos | I’m happy to abandon at this point, it looks like this a better approach. | 20:05 |
ayoung | Keystone LDAP in general needs better debugability | 20:05 |
broskos | fingers crossed for an icehouse backport | 20:05 |
ayoung | yeah, I think this should be safe for backport | 20:05 |
* broskos dances | 20:05 | |
ayoung | broskos, If your really want it, can you submit the backported patch> | 20:05 |
ayoung | ? | 20:05 |
broskos | sure | 20:06 |
broskos | just submit via review to stable/icehouse ? | 20:06 |
nkinder | broskos: yep, as soon as I get approval on the master fix, I'll propose the backport | 20:07 |
nkinder | broskos: I think it's really really needed for icehouse | 20:08 |
nkinder | broskos: the fact that there are 3 bugs on it already is a testament to that | 20:08 |
broskos | +1 | 20:08 |
broskos | cool, thanks guys | 20:08 |
nkinder | broskos: if my fix looks good for you, please +1 it too | 20:08 |
broskos | k | 20:09 |
broskos | Im going to take a few to test it locally on my broke ad integration, then I will +1 | 20:09 |
openstackgerrit | David Stanek proposed a change to openstack/keystone: Adds missing log hints for level E/I/W https://review.openstack.org/118883 | 20:09 |
openstackgerrit | David Stanek proposed a change to openstack/keystone: Extends hacking check for logging to verify i18n hints https://review.openstack.org/118884 | 20:09 |
openstackgerrit | David Stanek proposed a change to openstack/keystone: Fixes a spelling error in hacking tests https://review.openstack.org/119461 | 20:09 |
*** raildo has quit IRC | 20:11 | |
*** amirosh has quit IRC | 20:12 | |
openstackgerrit | Andreas Jaeger proposed a change to openstack/keystonemiddleware: Improve help strings https://review.openstack.org/118048 | 20:12 |
*** amirosh has joined #openstack-keystone | 20:13 | |
dolphm | dstanek: so bug 1364521 is a dupe of bug 1355489? | 20:14 |
uvirtbot | Launchpad bug 1364521 in keystone "LDAP integration with Active Directory backend can throw: UnicodeDecodeError" [Medium,In progress] https://launchpad.net/bugs/1364521 | 20:14 |
uvirtbot | Launchpad bug 1355489 in keystone "authenticate ldap binary fields fail when converting fields to utf8" [Medium,In progress] https://launchpad.net/bugs/1355489 | 20:14 |
*** amirosh has quit IRC | 20:14 | |
*** amirosh has joined #openstack-keystone | 20:14 | |
*** rodrigods has quit IRC | 20:15 | |
dolphm | broskos: nkinder: ayoung: ^ | 20:16 |
nkinder | dolphm: yes, there are 2 dups (grabbing #'s) | 20:17 |
nkinder | dolphm: 1358330 and 1364521 are dups of 1355489 | 20:19 |
nkinder | dolphm: 3 bugs in one fell swoop! ;) | 20:19 |
nkinder | broskos: that testing would be much appreciated! | 20:20 |
dstanek | dolphm: it looks like we got 2 birds with 1 stone | 20:22 |
dolphm | nkinder: dstanek: awesome. lbragstad and i were wondering about those yesterday :) | 20:23 |
lbragstad | oh, the unicode ones with ldap | 20:23 |
dolphm | dstanek: nkinder: so this should be abandoned as well? https://review.openstack.org/#/c/114986/ | 20:24 |
dstanek | dolphm: nkinder: i have no idea what that is doing | 20:26 |
*** arborism has joined #openstack-keystone | 20:26 | |
dstanek | dolphm: nkinder: based on the TB here: https://bugs.launchpad.net/keystone/+bug/1358330 i think it's the same issue - not sure why the fix looks so different | 20:27 |
uvirtbot | Launchpad bug 1358330 in keystone "Error on _ldap_get_list without attrlist value (dup-of: 1355489)" [Medium,In progress] | 20:27 |
uvirtbot | Launchpad bug 1355489 in keystone "authenticate ldap binary fields fail when converting fields to utf8" [Medium,In progress] | 20:27 |
*** amirosh has quit IRC | 20:27 | |
openstackgerrit | Steve Martinelli proposed a change to openstack/keystone: Use oslo_debug_helper and remove our own version https://review.openstack.org/119422 | 20:27 |
*** amirosh has joined #openstack-keystone | 20:28 | |
* dolphm is off to beer for now. | 20:30 | |
openstackgerrit | Samuel de Medeiros Queiroz proposed a change to openstack/keystone: Add test for getting a token with inherited role https://review.openstack.org/119206 | 20:31 |
*** amirosh has quit IRC | 20:32 | |
dstanek | stevemar: how is that helper installed? | 20:34 |
stevemar | dstanek, i added it to oslotest | 20:35 |
stevemar | dstanek, and oslotest exports it in setup.py/cfg | 20:35 |
stevemar | dstanek, https://github.com/openstack/oslotest/blob/master/setup.cfg#L26 | 20:36 |
dstanek | is that is the version they released this morning? that doesn't get installed for me | 20:37 |
*** hrybacki has quit IRC | 20:37 | |
stevemar | dstanek, yep, it's based on the version released this morning | 20:37 |
stevemar | dstanek, thats why i upgraded test-requirements | 20:37 |
dstanek | i didn't know you could install shell scripts like that | 20:38 |
stevemar | dstanek, i didn't either | 20:38 |
stevemar | dstanek, bnemec and dhellmann helped :) | 20:38 |
*** jsavak has joined #openstack-keystone | 20:39 | |
stevemar | dstanek, we have to wait until global requirements is actually updated, but i wanted to make sure it worked | 20:40 |
stevemar | dstanek, and it means other projects can use it more easily now :D | 20:41 |
*** joesavak has quit IRC | 20:41 | |
dstanek | nice | 20:41 |
dstanek | i say just don't use testr for development :-) i can't anymore because it's too slow | 20:41 |
stevemar | i think a few other projects like pycadf/ksc/osc were carrying their own versions, but now it's just a one line change | 20:42 |
bknudson | it's not testr that's the problem it's our unit tests | 20:43 |
dstanek | no, it's definitely testr - i shouldn't have to wait for it to build a list of all possible tests when I want to run tests in a single file | 20:45 |
dstanek | it throws off my testing mojo | 20:45 |
lbragstad | my testing mojo is hard to find... so when it's thrown off it *really* messes with me | 20:46 |
dstanek | it's great for automated testing when you don't care how slow it is, but when you want a 1 second response you need to look elsewhere | 20:46 |
dstanek | so with FF we can't add features, config options and new string to master? | 20:48 |
*** cjellick has quit IRC | 20:48 | |
*** cjellick has joined #openstack-keystone | 20:53 | |
*** hrybacki has joined #openstack-keystone | 20:54 | |
bknudson | dstanek: you can request a feature freeze exemption | 20:55 |
dstanek | bknudson: but that's to merge into master right? | 20:56 |
*** packet has quit IRC | 20:56 | |
bknudson | dstanek: yes, the feature freeze is for juno | 20:57 |
bknudson | dstanek: where else are you going to merge things? | 20:57 |
*** joesavak has joined #openstack-keystone | 20:57 | |
nkinder | dstanek: yeah, I wasn't sure about that fix proposal either (None vs []) | 20:57 |
bknudson | feature branch? | 20:57 |
dstanek | bknudson: is was hoping that they would cut a tag or something so we can continue working on new stuff | 20:59 |
*** jsavak has quit IRC | 21:00 | |
nkinder | dolphm, dstanek: I just replied to your review comments. I did code it to skip the entire attribute if we encounter a binary value on purpose. | 21:01 |
nkinder | I'm OK with continuing to process through the values though | 21:01 |
nkinder | It we encounter an attribute with binary values, it seems likely that it's an attribute that Keystone shouldn't even be using in the first place (like the objectSID occurrence that I saw). | 21:02 |
dstanek | nkinder: ah, i see... that list of values is for a single attribute? | 21:03 |
nkinder | dstanek: yes | 21:03 |
nkinder | dstanek: so we will process additional attributes | 21:04 |
*** andreaf has quit IRC | 21:05 | |
*** gokrokve has quit IRC | 21:05 | |
ayoung | dstanek, nkinder https://github.com/admiyo/PyKerberos | 21:07 |
dstanek | nkinder: that makes sense to me | 21:07 |
ayoung | lets hear it for the github importer | 21:07 |
*** dims has quit IRC | 21:08 | |
broskos | nkinder: tests fine on my icehouse / ad integration - good stuf | 21:09 |
nkinder | broskos: great, thanks! | 21:10 |
openstackgerrit | Brant Knudson proposed a change to openstack/keystone: Fix dn_startswith https://review.openstack.org/119478 | 21:10 |
openstackgerrit | Brant Knudson proposed a change to openstack/keystone: Add characterization test for group role assignment listing https://review.openstack.org/119479 | 21:10 |
openstackgerrit | Brant Knudson proposed a change to openstack/keystone: Fix LDAP group role assignment listing https://review.openstack.org/119480 | 21:10 |
bknudson | more LDAP fixins. | 21:10 |
broskos | nkinder: for backport fyi you will need to bring _ldap_get_list along in common/ldap/core | 21:10 |
nkinder | dolphm: are you good with it? | 21:10 |
broskos | nkinder: once I did that it worked fine | 21:10 |
*** r-daneel has quit IRC | 21:12 | |
*** dims has joined #openstack-keystone | 21:12 | |
*** gokrokve has joined #openstack-keystone | 21:14 | |
nkinder | broskos: ok, cool. I'm going to head out here in a bit, but I'll backport tonight or tomorrow | 21:16 |
*** joesavak has quit IRC | 21:16 | |
dstanek | ayoung: nice | 21:21 |
*** jasondotstar has quit IRC | 21:24 | |
*** jorge_munoz has left #openstack-keystone | 21:24 | |
*** nkinder has quit IRC | 21:26 | |
*** openstackgerrit has quit IRC | 21:31 | |
*** openstackgerrit has joined #openstack-keystone | 21:32 | |
dstanek | ayoung: have you seen this https://pypi.python.org/pypi/python-krbV/1.0.90 ? | 21:37 |
*** samuelmz_ has quit IRC | 21:38 | |
ayoung | dstanek, doesn't surprise me, but in this case, I want to go with the module we've been coding to. | 21:41 |
*** zzzeek has quit IRC | 21:41 | |
*** broskos has left #openstack-keystone | 21:41 | |
ayoung | dstanek, there were several python kerberos implementations. I need the one that works with requests-kerberos | 21:42 |
*** dims_ has joined #openstack-keystone | 21:44 | |
*** dims_ has quit IRC | 21:46 | |
ayoung | dstanek, doesn't look too bad | 21:46 |
ayoung | http://paste.openstack.org/show/106812/ | 21:46 |
ayoung | dstanek, that is with import subprocess as commands | 21:47 |
*** dims has quit IRC | 21:47 | |
*** dims_ has joined #openstack-keystone | 21:47 | |
ayoung | I think I'll need 6 or something to get it to work for both, but shouldn't be that bad | 21:48 |
openstackgerrit | Brant Knudson proposed a change to openstack/keystone: Remove unused variable TIME_FORMAT https://review.openstack.org/119490 | 21:52 |
*** saipandi has joined #openstack-keystone | 21:55 | |
ayoung | dstanek, Ok, here is the state: if I am on python2 I want commands, if python3, I want subprocess. I really don;t want to pull in six just to do the build. | 21:56 |
dstanek | python2 has subprocess | 21:56 |
*** dims_ has quit IRC | 21:57 | |
*** dims has joined #openstack-keystone | 21:58 | |
ayoung | dstanek, yeah, but it does not have the right functions (getoutput) | 21:59 |
ayoung | http://paste.fedoraproject.org/131428/09954298/ | 21:59 |
ayoung | dstanek, ^^ works ok | 21:59 |
ayoung | is that horrible? | 21:59 |
*** bobt has quit IRC | 22:00 | |
*** marcoemorais has quit IRC | 22:01 | |
bknudson | Deprecated since version 2.6: The commands module has been removed in Python 3. Use the subprocess module instead. | 22:01 |
bknudson | https://docs.python.org/2/library/commands.html | 22:01 |
dstanek | ayoung: you just need subprocess.check_output right? | 22:01 |
*** marcoemorais has joined #openstack-keystone | 22:02 | |
ayoung | dstanek, um, probably | 22:02 |
ayoung | here is the full code | 22:02 |
ayoung | http://paste.fedoraproject.org/131429/95451514 | 22:02 |
*** marcoemorais has quit IRC | 22:02 | |
dstanek | there isn't anything you can do with commands that youy can't do with subprocess in some way | 22:02 |
*** marcoemorais has joined #openstack-keystone | 22:02 | |
*** marcoemorais has quit IRC | 22:03 | |
dstanek | actually check_output is 2.7 so you may have to use Popen directly to get the same effect | 22:03 |
ayoung | it actually needs to parse the output, so more than just check_output | 22:03 |
*** marcoemorais has joined #openstack-keystone | 22:03 | |
ayoung | not worth it | 22:03 |
*** marcoemorais has quit IRC | 22:03 | |
ayoung | this is build code | 22:03 |
*** marcoemorais has joined #openstack-keystone | 22:03 | |
ayoung | python 3 subprocess does what the old commands code does | 22:04 |
ayoung | I'd be more prone to change the pre 3 version to | 22:04 |
ayoung | import commands as subprocess | 22:04 |
ayoung | and be done with it | 22:04 |
*** ayoung is now known as ayoung-afk | 22:06 | |
*** dims has quit IRC | 22:06 | |
*** sigmavirus24 is now known as sigmavirus24_awa | 22:08 | |
*** saipandi has quit IRC | 22:13 | |
*** stevemar has quit IRC | 22:28 | |
*** bknudson has quit IRC | 22:30 | |
*** saipandi has joined #openstack-keystone | 22:36 | |
*** hrybacki has quit IRC | 22:41 | |
*** arborism is now known as amcrn | 22:46 | |
*** harlowja is now known as harlowja_away | 23:00 | |
*** richm has quit IRC | 23:17 | |
*** sigmavirus24_awa is now known as sigmavirus24 | 23:22 | |
*** marcoemorais has quit IRC | 23:26 | |
*** marcoemorais has joined #openstack-keystone | 23:26 | |
*** sigmavirus24 is now known as sigmavirus24_awa | 23:26 | |
*** marcoemorais has quit IRC | 23:26 | |
*** marcoemorais has joined #openstack-keystone | 23:26 | |
*** marcoemorais has quit IRC | 23:27 | |
*** marcoemorais has joined #openstack-keystone | 23:27 | |
*** larsks has quit IRC | 23:30 | |
*** harlowja_away is now known as harlowja | 23:31 | |
*** david-lyle has quit IRC | 23:49 | |
*** gokrokve has quit IRC | 23:51 | |
*** marcoemorais has quit IRC | 23:55 | |
*** marcoemorais has joined #openstack-keystone | 23:56 | |
*** gyee has quit IRC | 23:58 | |
*** amerine has joined #openstack-keystone | 23:59 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!