dolphm | bknudson: fixed nits in https://review.openstack.org/#/c/56243/ | 00:01 |
---|---|---|
dolphm | bknudson: thanks! | 00:11 |
*** leseb has quit IRC | 00:19 | |
*** rwsu has quit IRC | 00:27 | |
*** rwsu has joined #openstack-keystone | 00:30 | |
*** andreaf has quit IRC | 00:31 | |
*** achampion has joined #openstack-keystone | 00:42 | |
*** richm has quit IRC | 00:52 | |
*** dims has joined #openstack-keystone | 00:54 | |
bknudson | no more v2 deprecated messages! | 01:02 |
bknudson | except for the first one. | 01:02 |
jamielennox | bknudson: from where? | 01:03 |
morganfainberg | bknudson, yay | 01:03 |
bknudson | in the keystone server log | 01:03 |
morganfainberg | dolphm, are we punting on the per-domain backend stuff till J? | 01:03 |
bknudson | "DEBUG stevedore.extension " -- should probably get rid of those. At least it only happens once. | 01:03 |
morganfainberg | i noticed it was moved to "next"? | 01:04 |
dolphm | morganfainberg: i at least don't want to block I on it with as much conversation/disagreement as it's producing. if we settle on a single solution *very* quickly i'd be happy to see something in icehouse | 01:04 |
bknudson | did we deprecate the v2 client api, too? | 01:04 |
morganfainberg | dolphm, ++ sounds good | 01:05 |
dolphm | bknudson: the lib? | 01:05 |
bknudson | dolphm: yes, in python-keystoneclient | 01:05 |
dolphm | bknudson: no | 01:05 |
bknudson | didn't remember seeing it... any reason to not deprecate it? | 01:05 |
jamielennox | bknudson: no we don't deprecate the client | 01:05 |
jamielennox | that requires a major semver | 01:05 |
jamielennox | but i'm not sure we need to be in a rush on the client side | 01:06 |
bknudson | it requires a 1.0 client to deprecate something? I thought that was when it was removed. | 01:06 |
dolphm | the CLI help includes a pending deprecation message | 01:06 |
jamielennox | bknudson: i would like to do proper semver for the clients - i might be aiming to high | 01:06 |
dolphm | i'd like to upgrade that to a proper 'deprecated' warning soon | 01:07 |
dolphm | i want to make sure the UX in python-openstackclient is up to par (the hardcoded /v2.0/ catalog issue is the only pain point there afaict) | 01:07 |
dolphm | bknudson: we need to land this in tempest too https://review.openstack.org/#/c/76949/ | 01:08 |
dolphm | waiting on a recheck :( | 01:08 |
bknudson | dolphm: it's the crappy xml serializer | 01:09 |
bknudson | we should deprecate that. | 01:09 |
dolphm | bknudson: ours? we did | 01:09 |
bknudson | jamielennox: we don't do proper semver for the clients? | 01:10 |
dolphm | bknudson: https://review.openstack.org/#/c/76301/ | 01:10 |
jamielennox | bknudson: i think clients break compatability all over the place | 01:10 |
bknudson | dolphm: can tempest stop testing it now? | 01:10 |
dolphm | bknudson: i think it should be tested as long as it's supported :-/ | 01:11 |
dolphm | bknudson: otherwise that commit would have contained a ton of deletes on our side :D | 01:11 |
bknudson | dolphm: the v3 catalog response is broken -- https://review.openstack.org/#/c/77375/9/keystone/tests/test_v3_auth.py | 01:12 |
bknudson | line 2036 | 01:12 |
morganfainberg | dolphm, DELETE ALL THE THINGS! | 01:12 |
bknudson | I mean the catalog in the v3 auth response. | 01:13 |
dolphm | bknudson: oh wow | 01:13 |
dolphm | bknudson: i hadn't gotten that far in the sequence | 01:13 |
jamielennox | bknudson: lol - in a shoot me kind of way | 01:16 |
*** rwsu has quit IRC | 01:17 | |
dolphm | morganfainberg: jamielennox: bknudson: i manually deleted keystone.openstack.common.crypto and forgot to remove the entry from openstack-common.conf; latest patchset fixes that https://review.openstack.org/#/c/77701/ | 01:22 |
morganfainberg | dolphm, ah | 01:22 |
morganfainberg | ok | 01:22 |
bknudson | that caused the docs to fail? | 01:22 |
dolphm | bknudson: sort of but not ours-- we're building docs for keystone.openstack.common which failed as a result | 01:27 |
bknudson | dolphm: docs built fine for me locally with that change. | 01:27 |
*** stevemar has joined #openstack-keystone | 01:28 | |
*** ChanServ sets mode: +v stevemar | 01:28 | |
dolphm | bknudson: the patchset previous to that failed (where i deleted the pycrypto requirement but didn't delete keystone.common.common.crypto which tried to import it) | 01:28 |
dolphm | bknudson: with common.crypto gone, the docs did build, but i obviously didn't make the change properly | 01:28 |
*** leseb has joined #openstack-keystone | 01:30 | |
*** ayoung_afk is now known as ayoung | 01:30 | |
morganfainberg | dolphm, what is the maximum size user_id derived section we are willing to accept? | 01:34 |
*** leseb has quit IRC | 01:34 | |
morganfainberg | dolphm, mulling over some alternatives | 01:35 |
jamielennox | bknudson: we talked a while ago about replacing auth_host, auth_port etc in auth_token middleware with just one uri | 01:36 |
jamielennox | bknudson: i posted https://review.openstack.org/#/c/77491/ with the config 'admin_uri' | 01:36 |
jamielennox | is admin_uri sufficiently distinguished from auth_uri? | 01:36 |
bknudson | jamielennox: that was a while ago! | 01:37 |
jamielennox | bknudson: yea, something was wrong back then that it couldn't be done very easily | 01:37 |
jamielennox | i tried it again and it worked out well | 01:37 |
jamielennox | or at least fairly easily | 01:37 |
bknudson | jamielennox: maybe we could name it based on how it's used... | 01:39 |
jamielennox | bknudson: you can review when you have time but does admin_uri work? | 01:39 |
bknudson | how it's used by auth_token | 01:39 |
bknudson | doesn't seem like we should say admin endpoint... v3 isn't really an admin endpoint? | 01:40 |
jamielennox | so it appears that we've always advocated that auth_port should be 35357 so we are talking to the admin, rather than auth port | 01:40 |
jamielennox | bknudson: right - but it's weird for v3 | 01:40 |
bknudson | It's called 'request_uri' in the code. | 01:41 |
jamielennox | yea, but i remember doing that purely because i couldn't come up with anything better | 01:41 |
bknudson | could call it 'identity_uri' | 01:42 |
jamielennox | yea, that's ok - i think just uri is too short | 01:42 |
jamielennox | can you validate a UUID token against the public endpoint in v2? | 01:42 |
bknudson | people might think 'uri geller' spoon bending | 01:42 |
jamielennox | do we care about people using the admin port for this any more? | 01:43 |
jamielennox | i'm sure it works to put the public endpoint in v2 | 01:44 |
bknudson | auth_token needs the v2 admin endpoint to get the certs. | 01:44 |
jamielennox | ah, the certs | 01:45 |
bknudson | did that finally get ported to v3? | 01:45 |
jamielennox | yea | 01:45 |
jamielennox | not supported by auth_token yet though | 01:45 |
jamielennox | so we are stuck with having the new 'identity_uri' conf option pointing to the admin port | 01:47 |
bknudson | jamielennox: I'm leaning towards identity_uri , and document that you set it to the v2 admin endpoint ? | 01:47 |
bknudson | in the hopes of eventually changing it to unversioned endpoint. | 01:47 |
jamielennox | bknudson: yea - i would like to doc that it should be the public | 01:47 |
jamielennox | ah, you don't need the v2 endpoint | 01:47 |
jamielennox | auth_token has always worked without the endpoint | 01:48 |
bknudson | provide an example in the doc | 01:48 |
jamielennox | (versioned endpoint) | 01:48 |
bknudson | or a suggested value | 01:48 |
bknudson | I guess you can't set the default because that would override the deprecated values... | 01:49 |
bknudson | jamielennox: how about setting the deprecated options to default=None and have admin_uri have a default? | 01:49 |
jamielennox | bknudson: then i would never be able to tell if the identity_api was unset | 01:50 |
bknudson | you'd use the deprecated options only if they were set. | 01:51 |
bknudson | only if any was set, or something. | 01:51 |
jamielennox | oh, reverse it | 01:51 |
jamielennox | hmm, i see the point in documenting a default but it does make it kind of the wrong way round | 01:52 |
jamielennox | you'd be saying if auth_host is set it would override identity_uri | 01:52 |
bknudson | jamielennox: right, if it was set explicitly | 01:52 |
jamielennox | in the case of a misconfigure though where someone sets auth_host, auth_port and identity_uri i want to take identity_uri and ignore the rest | 01:53 |
jamielennox | also because of the defaults of eg auth_schema, if you set auth_host and nothing else it will build a correct url | 01:53 |
bknudson | jamielennox: ok, was just thinking it would be nice to be able to have a default... I guess conf doesn't let us check if it was set. | 01:54 |
jamielennox | bknudson: yea, i agree but we can't tell between default and set | 01:54 |
*** amcrn has quit IRC | 01:55 | |
jamielennox | i'll put an eg in the doc and that should suffice for now | 01:55 |
bknudson | jamielennox: an egg in the doc works for me. | 01:55 |
*** devlaps has quit IRC | 02:01 | |
ayoung | bknudson, whenever I build docs I get a slew of errors like File "/opt/stack/keystone/.tox/venv/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 321, in import_object | 02:11 |
ayoung | __import__(self.modname) | 02:11 |
ayoung | ImportError: No module named rpc.zmq_receive | 02:11 |
ayoung | any advice? | 02:11 |
ayoung | WARNING: autodoc can't import/find module 'keystone.openstack.common.notifier', it reported error: "No module named notifier", please check your spelling and sys.path | 02:11 |
ayoung | morganfainberg, I can't help but think that there is some clear solution right under out noses with the id mapping thing. | 02:12 |
morganfainberg | ayoung, i'm looking into some alternatives now | 02:12 |
ayoung | cool | 02:13 |
morganfainberg | ayoung, i _might_ have something usable. | 02:13 |
morganfainberg | ayoung, but trying it out first | 02:13 |
*** browne has quit IRC | 02:13 | |
ayoung | morganfainberg, dumb idea, just to generate a smart one...what if we used PKI | 02:15 |
morganfainberg | ayoung, not far off the idea i had | 02:15 |
ayoung | so the user id is .... encrypted with the private key | 02:15 |
ayoung | hashed or something | 02:15 |
ayoung | nah... | 02:15 |
ayoung | it needs to be reversable | 02:16 |
morganfainberg | ayoung, that is what i'm aiming for | 02:16 |
ayoung | but...yeah, a key encrypted should be short, no? | 02:16 |
morganfainberg | ayoung, except the padding of encrypt will likely overflow our small dataset (64bytes) | 02:16 |
ayoung | PKI is expensive, though | 02:16 |
ayoung | what if it was not asym | 02:17 |
morganfainberg | ayoung, i'm looking into some b64 encode magic see if i can do some stuff with mangling up the format of the uuids etc. | 02:17 |
morganfainberg | ayoung, right now i can consistently build 31byte user_names and a domain uuid in 64bytes urlsafe reversible | 02:17 |
ayoung | morganfainberg, I owe you beer just for getting that far | 02:18 |
morganfainberg | trying to see if i can increase the user_name element size some way | 02:18 |
ayoung | chop one byte off the domain id | 02:18 |
morganfainberg | ayoung, hehe i'm only using 16bytes for the domain id | 02:18 |
morganfainberg | ayoung, uuid.uuid4().bytes | 02:18 |
ayoung | ah | 02:19 |
morganfainberg | well, 17, an explicit \0 for the delimiter between the two elements | 02:19 |
ayoung | leave that off | 02:19 |
morganfainberg | how do you split between then? | 02:19 |
ayoung | fixed length | 02:19 |
morganfainberg | hm.. | 02:19 |
ayoung | just a thought...if every byte counrs | 02:19 |
morganfainberg | was having an issue with slicing getting confused w/ bytes | 02:19 |
ayoung | counts | 02:19 |
morganfainberg | let me try it again | 02:20 |
morganfainberg | also if it's a uuid user_id i compress it down to 16 bytes as well | 02:20 |
ayoung | acha | 02:21 |
ayoung | We won't be mixing old UUIDs and new ones together this way, though | 02:21 |
morganfainberg | ayoung, but if we can keep the string smaller, why not? | 02:22 |
ayoung | ah | 02:22 |
ayoung | unles we go by length | 02:22 |
morganfainberg | and i can eliminate the \0 | 02:22 |
morganfainberg | i was typoing before | 02:22 |
ayoung | 32byte = straight UUID. 64byte = new style | 02:23 |
ayoung | Existing LDAP is going to be problematic, have to be grandfathered in | 02:23 |
morganfainberg | ayoung, http://paste.openstack.org/show/71982/ | 02:25 |
morganfainberg | ayoung, simple test code, but something like that | 02:25 |
*** lbragstad has joined #openstack-keystone | 02:25 | |
morganfainberg | we could remove the extra magic uuid thing to shorten the user_id | 02:26 |
morganfainberg | ayoung, all user_ids would be grandfathered in | 02:28 |
morganfainberg | we can detect if it's a "new-style" id | 02:28 |
ayoung | morganfainberg, if the userid < 64 its a grandfathered value? | 02:28 |
morganfainberg | nah, if not b64encode | 02:28 |
ayoung | UUID4.hex won't accidentally show up as b64? | 02:29 |
morganfainberg | don't think so. | 02:29 |
dolphm | jamielennox: bknudson: morganfainberg: jenkins-shaped +1 on https://review.openstack.org/#/c/77701/ | 02:29 |
morganfainberg | ayoung, doh it will | 02:29 |
dolphm | also there's a storm going through here and our power keeps going out | 02:29 |
*** gokrokve has quit IRC | 02:30 | |
morganfainberg | dolphm, LGTM +2 | 02:30 |
lbragstad | nice, I was +1 on that before | 02:30 |
jamielennox | dolphm: i guess it's only fitting i do the +A | 02:31 |
jamielennox | sniff, i hope we see you again KDS | 02:31 |
dolphm | jamielennox: i'll +A the delete all of keystone in the branch then :P | 02:31 |
ayoung | dolphm, thanks for keeping up with the revoke patch. I was keeping the Fake backend around as an escape hatch, but we don't need it anymore. Using the events removed most of the dependencies from the revoke code, leaving only the token provider. I made it optional there, and now if you don't enable the revoke backend, there should be no trace of it.... | 02:32 |
ayoung | which leads to the question of whether we should disable it or enable it by default | 02:32 |
ayoung | If disabling it makes people feel more comfortable....and increases the chance of it going it, I am more than willing to do so | 02:32 |
morganfainberg | ayoung, hmm. trying to figure out how to differentiate uuid from "new" id | 02:32 |
ayoung | morganfainberg, length | 02:32 |
jamielennox | dolphm: i want https://review.openstack.org/#/c/77525/ before any client release - just in case you were getting ideas | 02:33 |
morganfainberg | ayoung, i guess we could pad everything up to 64 always. | 02:33 |
ayoung | jamielennox, ++ on that | 02:33 |
dolphm | ayoung: there's a recommendation to enable it in the config help, right? | 02:33 |
ayoung | morganfainberg, huh? Old uuids are out there, recorded on other systems. | 02:33 |
jamielennox | it's not major - i'm just learning that some things shouldn't be public straight away | 02:34 |
dolphm | jamielennox: target it to a bug / bp and point it at 0.6.x | 02:34 |
ayoung | dolphm, um...no, just to not disable "revoke by id" without also enabling the events | 02:34 |
morganfainberg | ayoung, if you look at the example i gave, a short name (e.g. "adam" as the ldap bit) could net less than 32 bytes in the id: #SHORT: c2hvcnTFkKCA4xVGhqHao2G41agj 28 | 02:34 |
ayoung | ah...yeah, pad | 02:34 |
ayoung | COBOL! | 02:34 |
ayoung | CCCCCCCCCCCDDDDDDDDD | 02:35 |
morganfainberg | ayoung, user_id - pad to 32 then encode? | 02:35 |
morganfainberg | hm... | 02:35 |
ayoung | Pad with spaces, I think | 02:35 |
morganfainberg | ayoung, yeah | 02:35 |
morganfainberg | checking that | 02:35 |
* dolphm bonus points for dishwashers that resume after power outages | 02:35 | |
ayoung | then trim is part of the decode process | 02:36 |
morganfainberg | ayoung, yeah that works | 02:37 |
morganfainberg | dolphm woo! | 02:37 |
jamielennox | dolphm: 0.6.1 or 0.7? | 02:37 |
jamielennox | didn't we do 0.6.1? | 02:37 |
dolphm | jamielennox: https://pypi.python.org/pypi/python-keystoneclient/ | 02:37 |
jamielennox | yea, i checked that immediately after typing... | 02:38 |
dolphm | https://launchpad.net/python-keystoneclient/+milestone/0.6.1 | 02:38 |
dolphm | jamielennox: was auth plugins introduced in 0.6.0 or since? | 02:39 |
jamielennox | 0.6 | 02:39 |
jamielennox | ahh, since 0.6 | 02:39 |
jamielennox | we haven't had a release since | 02:39 |
dolphm | jamielennox: is there a bp for it? | 02:39 |
jamielennox | let me try again - there was some base plugin stuff in 0.6.0, the actual implementations have been since then | 02:40 |
jamielennox | hmm, there was | 02:40 |
jamielennox | i guess we can close that now | 02:40 |
dolphm | jamielennox: you can point it to 0.7.0 and we can make that the next release? | 02:41 |
jamielennox | ok | 02:41 |
dolphm | jamielennox: delete 0.6.1 | 02:42 |
dolphm | deleted* | 02:42 |
dolphm | ayoung: did you have a patch for this? https://bugs.launchpad.net/cinder/+bug/1285833 | 02:45 |
ayoung | dolphm, yep | 02:45 |
ayoung | https://review.openstack.org/#/c/77215/ dolphm I think that is the same issue | 02:45 |
dolphm | ayoung: your patch is referencing the wrong bug, use bug 1285833 | 02:46 |
ayoung | OK | 02:46 |
stevemar | dolphm, yay limited trusts! | 02:48 |
ayoung | dolphm, BTW, the KDS removal misses all these keystone/locale/kn/LC_MESSAGES/keystone.po:#: keystone/contrib/kds/common/exception.py | 02:48 |
ayoung | is it in? | 02:48 |
dolphm | stevemar: did it land? | 02:49 |
dolphm | https://review.openstack.org/#/c/56243/ | 02:49 |
dolphm | gating | 02:49 |
stevemar | it's going through | 02:49 |
*** marcoemorais has quit IRC | 02:49 | |
ayoung | dolphm, before I repost.... bknudson had this comment: (unrelated) how is an AssertionError or KeyError going to happen?? I am now cleaning up that code...but am reluctant to change the exception handling | 02:51 |
ayoung | https://review.openstack.org/#/c/77215/1/keystoneclient/middleware/auth_token.py | 02:51 |
bknudson | ayoung: it was broken before so don't need to fix it. | 02:51 |
ayoung | bknudson, kindof my feeling, but I refactored the two calls into a single function | 02:52 |
ayoung | I guess I'll post and see what you think | 02:52 |
dolphm | ayoung: it's only your new code in the try block now -- you should feel comfortable changing it | 02:52 |
stevemar | dolphm, oh yeah - https://review.openstack.org/#/c/74531/ | 02:52 |
ayoung | dolphm, I don't remeber why those two exceptions, except that there was a lot of trial and error. | 02:53 |
dolphm | stevemar: i thought that landed forever ago | 02:53 |
stevemar | dolphm, there were a few | 02:53 |
stevemar | dolphm, this one slipped through | 02:53 |
dolphm | ayoung: trial and error without tests? | 02:53 |
morganfainberg | ayoung, so is 32 bytes too little for the user_id derived element? | 02:53 |
morganfainberg | ayoung, http://paste.openstack.org/show/71990/ | 02:53 |
ayoung | dolphm, for fetching the certs? Maybe. It landed a long while back...2 years now? | 02:54 |
dolphm | stevemar: +A | 02:54 |
stevemar | dolphm, ty | 02:54 |
jamielennox | ayoung: yea i think that code has caught errors that don't exist for ages | 02:54 |
jamielennox | there was a review a while ago where they were removed and it got negatively commented on and reinstated | 02:55 |
morganfainberg | ayoung, still seeing if i can figure out a better mechanism. | 02:55 |
ayoung | morganfainberg, 32chars for username should be OK. Even with email addresses, that is pretty forgiving | 02:56 |
*** joesavak has joined #openstack-keystone | 02:56 | |
dolphm | ayoung: both except blocks should be removed from the calling methods... if os.rename fails, a 500 wouldn't be so bad (it's a blatant misconfiguration) | 02:56 |
ayoung | dolphm, true | 02:56 |
dolphm | and getting a named temp file should always succeed | 02:57 |
ayoung | Key error seems wonlky | 02:57 |
ayoung | OK...it was moved over from server code...so the reason has been lost in antiquity | 02:58 |
ayoung | AssertionError is, I think, coming from the token parsing done earlier, probably to make the call to fetch the cert | 03:00 |
ayoung | Key error, too | 03:00 |
ayoung | which means that the block is useless | 03:00 |
dolphm | jamielennox: bah, unset the approval on https://review.openstack.org/#/c/77701/ | 03:07 |
dolphm | jamielennox: the successful check was a lie... | 03:07 |
dolphm | morganfainberg: we're not running the config check_uptodate.sh on check jobs? :( | 03:07 |
morganfainberg | we should be | 03:07 |
dolphm | morganfainberg: https://review.openstack.org/#/c/77701/ passed check, and is now failing gate | 03:07 |
dolphm | https://jenkins07.openstack.org/job/gate-keystone-pep8/371/console | 03:07 |
*** amcrn has joined #openstack-keystone | 03:08 | |
*** joesavak has quit IRC | 03:11 | |
ayoung | -pep8 probably due to config file merge | 03:13 |
ayoung | can we drop that check? | 03:13 |
ayoung | no, it just probably merged with something else. check_uptodate.sh is too strict | 03:14 |
*** dstanek has quit IRC | 03:24 | |
*** dstanek_afk has joined #openstack-keystone | 03:24 | |
*** ChanServ sets mode: +v dstanek_afk | 03:24 | |
*** derek_c has joined #openstack-keystone | 03:26 | |
*** gokrokve has joined #openstack-keystone | 03:27 | |
*** dstanek_afk is now known as dstanek | 03:27 | |
dstanek | it looks like there is only 1 review left for i3? | 03:27 |
ayoung | mine? | 03:29 |
dstanek | looks like it - walking through it again now | 03:30 |
ayoung | thanks | 03:30 |
dolphm | someone double check me on this... gate failed https://review.openstack.org/#/c/77701/ on the new config check job, which tried to import keystone.openstack.common.sslutils for config opts https://jenkins07.openstack.org/job/gate-keystone-pep8/371/consoleFull | 03:32 |
dolphm | absolutely nothing uses that module (not even other oslo code) | 03:32 |
dolphm | it's not referenced in openstack-common.conf | 03:32 |
dolphm | git blame shows it was added in an irrelevant (i think?) patch, and hasn't been touched since https://review.openstack.org/#/c/41487/ | 03:33 |
dolphm | morganfainberg: ^ and i still don't understand why the check job would have passed | 03:34 |
dolphm | it was easy to repro offline | 03:34 |
ayoung | find keystone/ -name \*py | xargs grep sslutil finds nothing | 03:34 |
ayoung | dolphm, config/generator.py" | 03:35 |
dolphm | morganfainberg: (check_uptodate.sh ran and exited successfully in the check job, but failed in gate) | 03:35 |
ayoung | could that be called in by check_uptodate.sh | 03:35 |
ayoung | like an old one? | 03:35 |
dolphm | tools/config/generator.py? | 03:36 |
morganfainberg | dolphm, hm | 03:36 |
ayoung | .tox/py27/lib/python2.7/site-packages/oslo/messaging/_drivers/impl_rabbit.py:from oslo.messaging.openstack.common import sslutils | 03:36 |
dolphm | ayoung: tools/config/generate_sample.sh ? | 03:36 |
morganfainberg | dolphm, the check_sample stuff is dumb and catches .pyc files | 03:36 |
dolphm | morganfainberg: but, in zuul?! | 03:37 |
morganfainberg | dolphm, it looks like something tried to push an empty sample_config | 03:37 |
morganfainberg | what.... | 03:37 |
*** chandan_kumar has joined #openstack-keystone | 03:37 | |
morganfainberg | sslutils error could do a number on it | 03:37 |
dolphm | morganfainberg: sslutils bailed with stderr, while stdout from the generator was being redirected to keystone.conf.sample maybe? | 03:38 |
morganfainberg | dolphm, possibly | 03:38 |
morganfainberg | dolphm, therefore the "sample" it was comparing against was effectively empty | 03:39 |
dolphm | morganfainberg: seems like the order of the diff should be reversed then | 03:39 |
dolphm | morganfainberg: it should be -every line not +every line | 03:39 |
ayoung | can we drop the check for now? I think we've caught up with the config.py file | 03:39 |
morganfainberg | dolphm, checking right now before we drop that check | 03:40 |
dolphm | ayoung: morganfainberg: why do we want to drop the check? | 03:40 |
ayoung | we just for the next couple of days | 03:40 |
ayoung | dolphm, it causes a lot of rebase errors | 03:40 |
morganfainberg | dolphm, if we can't solve it :) | 03:41 |
morganfainberg | dolphm, i'd rather not | 03:41 |
dolphm | ayoung: rebase errors? | 03:41 |
ayoung | if two patches run the tool, the resultant merge doesn't pass the check | 03:41 |
morganfainberg | dolphm, so i see a legitimate error w/ the sslutils options | 03:41 |
dolphm | morganfainberg: ? | 03:42 |
morganfainberg | iif i checkout your patchset | 03:42 |
ayoung | so if something merges after I submit, even if my patch passed check the first time, the auto-rebase on gerrit fails the second | 03:42 |
morganfainberg | dolphm, http://paste.openstack.org/show/72004/ | 03:42 |
dolphm | ayoung: that just sounds like bad luck... have an example? | 03:42 |
ayoung | dolphm, well, my patch has gone through 20 reivsions in about 4 dyas...several because of that | 03:43 |
morganfainberg | hrm | 03:43 |
morganfainberg | let me try a quick rebase | 03:43 |
dolphm | morganfainberg: is that the latest patchset? or the one that failed gate? | 03:43 |
morganfainberg | yeah | 03:43 |
morganfainberg | dolphm, give me a sec trying a rebase | 03:43 |
ayoung | we can re-enable it once we get the tool stable | 03:44 |
morganfainberg | dolphm, yep, looks like the keystone.openstack.common.sslutils stuff is causing issues | 03:44 |
dolphm | ayoung: better yet, let's stop landing features for the next couple weeks so no new config options will be introduced :D | 03:45 |
morganfainberg | dolphm, ah because you removed them without regenerating the sample | 03:45 |
ayoung | dolphm, heh. OK. | 03:45 |
dolphm | morganfainberg: am i regenerating the sample wrong?? | 03:45 |
ayoung | dolphm, maybe the tool picked up stray .pyc files | 03:45 |
ayoung | do a git clean | 03:45 |
morganfainberg | dolphm, your patch doesn't have a sample config in it | 03:45 |
ayoung | git clean -xdf keystone | 03:46 |
morganfainberg | dolphm, keystone.conf.sample | 03:46 |
dolphm | morganfainberg: your docs don't say how to actually generate the sample :P http://docs.openstack.org/developer/keystone/developing.html#generating-updated-sample-config-file | 03:46 |
morganfainberg | tox -esample_config -r should do it | 03:46 |
morganfainberg | but if you have .pyc files | 03:46 |
morganfainberg | yeah should say you need to do a git clean -fdx | 03:46 |
morganfainberg | or at least remove all .pyc files to be 100% sure | 03:46 |
morganfainberg | even files not being tracked by git, if they provide options, will be caught | 03:47 |
morganfainberg | :( | 03:47 |
morganfainberg | sorry :( | 03:47 |
dolphm | morganfainberg: i remove pyc files, actually... | 03:47 |
ayoung | what are we doing now to activate things in paste.ini for tests that are not in etc/keystone-paste.ini | 03:48 |
dolphm | ayoung: there's a config generator function that can insert things into the pipeline | 03:49 |
morganfainberg | ayoung, i think the oauth and endpoint_filtering stuff has those semantics in it | 03:49 |
dolphm | ayoung: it basically does a find/replace on the default pipeline | 03:49 |
morganfainberg | yep | 03:49 |
dolphm | ayoung: look in keystone.tests.core for 'generate' | 03:49 |
ayoung | only in extension code, though? | 03:49 |
dolphm | morganfainberg: okay, trying to regenerate conf again... did an ls to ensure no *.pyc first | 03:50 |
ayoung | hmmm...I might have trouble with that if I remove the revoke one. | 03:50 |
ayoung | I would need to pass in multiple extensions | 03:50 |
dolphm | ayoung: like what? | 03:51 |
morganfainberg | dolphm, you should see the sslutils opts disappear | 03:51 |
ayoung | dolphm, oauth events | 03:51 |
ayoung | maybe not... | 03:53 |
ayoung | nah, those are just testing for generation of events | 03:53 |
morganfainberg | dolphm, looks like some people went through and "fixed" a bunch of strings as well | 03:53 |
morganfainberg | with periods at the end of them (maybe oslo.messaging) | 03:53 |
morganfainberg | dolphm, i have a | 03:54 |
morganfainberg | "fixed" patchset if you want me to try | 03:54 |
morganfainberg | if not i can let you publish | 03:54 |
dolphm | morganfainberg: for every code review, i git checkout $BRANCH && git pull && git checkout $BRANCH~0 && find . -name "*.pyc" -delete && git review [-x|-d] $REVIEW_NUMBER where $BRANCH is usually 'master' | 03:54 |
morganfainberg | dolphm, that should be fine | 03:54 |
morganfainberg | dolphm, no reason it should cause issues | 03:55 |
jamielennox | dolphm: :O, you have that as a script or something i take it | 03:55 |
jamielennox | http://www.jamielennox.net/blog/2014/02/18/dealing-with-pyc/ | 03:56 |
dolphm | morganfainberg: no success http://pasteraw.com/a3lj19yywkwh8dfx0ybjlx0lqrllvqo | 03:56 |
dolphm | jamielennox: script is in there ^ | 03:56 |
jamielennox | doesn't cover everything of that | 03:56 |
morganfainberg | dolphm, so clean venv, git clean -fdx and nothing lingering around | 03:57 |
morganfainberg | dolphm, http://pasteraw.com/h1e5capcn76y8dyaa61t5pakz7858ad | 03:58 |
lbragstad | ayoung: getting down the nitty gritty here, https://review.openstack.org/#/c/55908/ | 03:59 |
dolphm | morganfainberg: i ran rm_pyc again and then http://pasteraw.com/bthcfj4t85bd55yc7afl78ck2r1cb55 | 03:59 |
morganfainberg | dolphm, and i ran tox -esample_config -r to generate that | 03:59 |
morganfainberg | yeah that should be fine. | 03:59 |
morganfainberg | dolphm, WTF. | 03:59 |
lbragstad | ayoung: you mind if I checkout that patch and go through all the indentation and comment '#<space>' fixes? | 03:59 |
ayoung | lbragstad, dagnabit, I know I changed those return codes in an earlier patch. | 03:59 |
dolphm | lbragstad: can you get that into openstack/hacking :D | 04:00 |
ayoung | lbragstad, yeah, but let me get an updated review | 04:00 |
morganfainberg | dolphm, lbragstad ++ in hacking! | 04:00 |
lbragstad | oh, the consistent | 04:00 |
lbragstad | # comment check? | 04:00 |
ayoung | I'm testing out disabling the extension by default, but need to update a few tests first | 04:00 |
lbragstad | hell yeah, I'll check it out... that would be really... really nice | 04:00 |
morganfainberg | dolphm, not sure why we're getting wildly different results here | 04:01 |
dolphm | lbragstad: i started coding it myself but didn't get very far http://pasteraw.com/rsi2frll7ojzr1b5bhj6si6mkbf7e8y | 04:01 |
dolphm | lbragstad: i'm not sure what's useful in that patch | 04:02 |
lbragstad | dolphm: cool, I'll check it out. | 04:02 |
* lbragstad bookmarks | 04:02 | |
dolphm | morganfainberg: go ahead and post a revised patchset | 04:04 |
morganfainberg | dolphm, ok | 04:04 |
dolphm | morganfainberg: wow... you weren't kidding about the periods | 04:05 |
morganfainberg | dolphm, ok i think we need to change how the sample config is done. | 04:05 |
morganfainberg | lets do it like we do man pages | 04:05 |
morganfainberg | disable it from pep8, and just do it once every X period | 04:06 |
morganfainberg | like before we cut an rc. | 04:06 |
lbragstad | ayoung: you're posting another token revocation patch right? | 04:06 |
morganfainberg | make it a bug ticket that needs to be the last one to land until it gets more automated | 04:06 |
morganfainberg | s/rc/milestone | 04:06 |
dolphm | morganfainberg: i'd rather man pages be done more like this... is nova approaching this any differently? | 04:06 |
ayoung | lbragstad, I have a bunch of nits....and I am also looking to see how hard it would be to get the tests running if we disable the extension by default | 04:06 |
lbragstad | ayoung: because I can go through the nits I made | 04:07 |
morganfainberg | dolphm, they are supposed to be automating it for deployment | 04:07 |
lbragstad | while you work on other things, if that helps | 04:07 |
morganfainberg | dolphm, e.g. when you cut a release, this is done | 04:07 |
morganfainberg | dolphm, but no idea where that is or how hard it is to do atm | 04:07 |
ayoung | lbragstad, if you have a cleaned copy of the patch, you can post it and I can merge, or you can post comments....I'm guessing you have a pretty extensive review? | 04:07 |
ayoung | Oh, wait, I confused you with bknudson , nevermind | 04:08 |
lbragstad | lol | 04:08 |
lbragstad | damn... i was going to take credit! :) | 04:08 |
morganfainberg | dolphm, maybe we should get a jenkins task to do the update? same as translations/requirements? | 04:08 |
morganfainberg | dolphm, same w/ man page? | 04:08 |
ayoung | so...to disable, is it OK to leave the filter definition in, and just remove the filter from the pipeline? | 04:09 |
dolphm | morganfainberg: i'd be down with that | 04:09 |
dolphm | morganfainberg: on both counts | 04:09 |
morganfainberg | dolphm, i'll get something whipped up and proposed to infra for us (hey maybe others will want it too) | 04:09 |
dolphm | morganfainberg: as long as it's automated in some fashion -- bonus points if jenkins proposes commits on top of the commits that are causing the changes rather than waiting for master :P | 04:09 |
morganfainberg | dolphm, was going to do it like the translations, a periodic that is just run. | 04:10 |
morganfainberg | dolphm, if there is a change, propose it | 04:10 |
dolphm | morganfainberg: ah, i thought translations was triggered | 04:10 |
morganfainberg | dolphm, nope, don't think so. | 04:10 |
dolphm | morganfainberg: either way | 04:10 |
morganfainberg | dolphm, but they only run once a week or so, requirements is triggered afaik | 04:10 |
* dolphm need tips on horizontally scaling sqlite in production thx | 04:11 | |
morganfainberg | dolphm, OH! i stopped using SQLite so i could use mongodb, it's webscale | 04:11 |
dolphm | morganfainberg: but is mongodb lite? this needs to fast | 04:12 |
morganfainberg | dolphm, i don't know if it's lite...but i hear it's ACID compliant | 04:13 |
dolphm | lol | 04:13 |
dolphm | morganfainberg: can you back mongodb to existing sqlite deployment? | 04:14 |
morganfainberg | dolphm, I love it! | 04:14 |
morganfainberg | dolphm, we need to do that. | 04:14 |
dolphm | morganfainberg: is it problem if sqlite db is 6.8GB? how do you enable sharding | 04:18 |
dolphm | -rw-r--r-- 1 keystone keystone 6.8G Mar 4 04:17 /var/lib/keystone/keystone.db | 04:18 |
morganfainberg | dolphm, LOL | 04:18 |
morganfainberg | dolphm, thats an awesome SQLite db | 04:19 |
morganfainberg | dolphm, wow. | 04:19 |
ayoung | the sad thing is that people wouldn't know you were joking without the LOL | 04:20 |
morganfainberg | ayoung yeah | 04:20 |
morganfainberg | =/ | 04:21 |
lbragstad | ayoung: here are the fixes I made, | 04:21 |
lbragstad | http://paste.openstack.org/show/72011/ | 04:21 |
lbragstad | ayoung: ready to push for review if its cool with you | 04:21 |
ayoung | dolphm, OK, so disable revoke_events by default doesn't look too bad...just had to add the extension to a couple tests. Running the whole thing now | 04:21 |
ayoung | lbragstad, looking | 04:21 |
ayoung | lbragstad, do me a favor: git format-patch it and mail me? I'll integrate it into my next version | 04:22 |
ayoung | dolphm, why does building docs fail with sphinx.errors.SphinxWarning: /opt/stack/keystone/doc/source/api/keystone.openstack.common.notifier.rst:7: WARNING: autodoc can't import/find module 'keystone.openstack.common.notifier', it reported error: "No module named notifier", please check your spelling and sys.path | 04:23 |
morganfainberg | ayoung, .pyc files? | 04:23 |
dolphm | ayoung: curl http://paste.openstack.org/raw/72011/ | git apply | 04:23 |
morganfainberg | or sphinx is trying to load / configured to read things from there? | 04:23 |
ayoung | morganfainberg, I've git cleaned...or are those somehow embedded in the venv | 04:23 |
ayoung | dolphm, thanks.... | 04:24 |
morganfainberg | dolphm, oh. | 04:24 |
morganfainberg | dolphm, ick | 04:24 |
morganfainberg | dolphm, eyah | 04:24 |
dolphm | morganfainberg: are you convulsing? | 04:24 |
morganfainberg | dolphm, kfjasfjkskf122131231adf | 04:24 |
morganfainberg | dolphm, maybe | 04:24 |
morganfainberg | dolphm, makes me cry inside. | 04:25 |
ayoung | fatal: corrupt patch at line 185 | 04:26 |
dolphm | ayoung: that's a conflict with your own changes | 04:26 |
ayoung | dolphm, yeah...I can do it as a cherry pick | 04:27 |
*** wchrisj has quit IRC | 04:27 | |
dolphm | ayoung: could also try git apply -C [0,1,2] (reduce context) | 04:28 |
dolphm | -C1 (no space) | 04:28 |
dolphm | ayoung: you can also --exclude files you've already made changes to | 04:29 |
lbragstad | ayoung: did you get it? | 04:29 |
ayoung | lbragstad, not yet | 04:29 |
dolphm | ayoung: and lastly, --recount has fixed other contextual issues for me | 04:29 |
*** harlowja is now known as harlowja_away | 04:30 | |
ayoung | dolphm, used an oldey but gody | 04:31 |
ayoung | patch -p1 < /tmp/lbrag.patch | 04:31 |
ayoung | dolphm, do you want the extension disabled by default? Let me know before I try to merge his with mine | 04:32 |
ayoung | I'm guessing Yes at this point, though | 04:32 |
dolphm | ayoung: yeah :-/ as much as i think we should **strongly** recommend everyone enable it immediately | 04:33 |
ayoung | dolphm, we can always flip the switch on it once it is tested | 04:34 |
ayoung | we need client support for it to be useful anywa | 04:34 |
ayoung | y | 04:34 |
dolphm | ayoung: ++ | 04:34 |
ayoung | OK...I'll submit that way | 04:34 |
*** harlowja_away is now known as harlowja | 04:35 | |
lbragstad | woo vagrant dir ftw | 04:35 |
dolphm | best devstack run ever http://logs.openstack.org/15/77215/2/check/gate-tempest-dsvm-large-ops/953025e/console.html | 04:35 |
*** wchrisj has joined #openstack-keystone | 04:36 | |
ayoung | lbragstad, reposting. Please confirm your changes were made clean | 04:36 |
lbragstad | ayoung: will do | 04:37 |
ayoung | lbragstad, I didn't rebase prior to posting.... | 04:37 |
* ayoung getting tired | 04:37 | |
ayoung | morganfainberg, stevemar BTW, I think I have a new record for number of revisions on a patch. Oauth went to 64. I'm pushing 80 | 04:40 |
* stevemar passed the torch back to ayoung | 04:41 | |
stevemar | passes* | 04:42 |
stevemar | iirc you had it for trusts, at 61? | 04:42 |
ayoung | stevemar, that is not including revisions on the two patches I merged in to this. | 04:42 |
ayoung | trusts was about that...and the tests were separate, but dolphm wrote those | 04:42 |
stevemar | ayoung, details, details | 04:42 |
* ayoung not writing any big security related patchsets ever again | 04:43 | |
dstanek | what was the veridict on removing existing openstack copyrights? | 04:44 |
ayoung | ah...right | 04:45 |
ayoung | dstanek, let me see if I wrote that in the first place, in which case its moot, no? | 04:45 |
ayoung | dstanek, which file? | 04:45 |
dolphm | dstanek: leave existing ones alone -- just don't add new ones for now | 04:45 |
dolphm | dstanek: existing ones need to be removed all at once, and probably by the foundation | 04:46 |
dstanek | ayoung: :-) i don't know anymore...jas let me see if i can find it | 04:46 |
ayoung | patch set 71 | 04:47 |
dstanek | https://review.openstack.org/#/c/55908/77/keystone/tests/test_v3_auth.py | 04:47 |
dstanek | ayoung: ^ | 04:47 |
ayoung | ah, that is likely dolphm 's work | 04:48 |
ayoung | dstanek, OK, think that is the only one I butchered, but I'll double check | 04:49 |
ayoung | ahhh! | 04:50 |
ayoung | patching errors | 04:50 |
ayoung | dstanek, OK, that was the only copyright change in the patch...aside from accidentally adding in keystone/token/controllers.py.orig which will be gone in the next revision | 04:51 |
lbragstad | ayoung: there were a couple things i missed or they were new in your patch | 04:51 |
ayoung | lbragstad, you commented on the erroneous .orig file | 04:51 |
ayoung | https://review.openstack.org/#/c/55908/78/keystone/token/controllers.py.orig | 04:51 |
ayoung | that was an artifact of doing the patch, and some fuzz | 04:52 |
lbragstad | ayoung: ahh... yep, you're right | 04:52 |
ayoung | lbragstad, I think all of the changes made it in to https://review.openstack.org/#/c/55908/78/keystone/token/controllers.py | 04:52 |
dstanek | ayoung: just commented on it | 04:53 |
ayoung | dstanek, integrating your changes now | 04:53 |
lbragstad | ayoung: yeah ^^ that one looks right | 04:53 |
*** chandan_kumar has quit IRC | 04:53 | |
ayoung | dstanek, II'm assuming the loop in downgrade is not essential to change? | 04:55 |
dstanek | no, at least not to me - it's just funny looking | 04:55 |
dstanek | i think the exit (or lack of exit) is what really needs addressed | 04:55 |
ayoung | dstanek, roles = token_data.get('metadata', {}).get('roles', []) | 04:55 |
ayoung | if there is no metadata...will there be roles? | 04:55 |
ayoung | easy to test | 04:56 |
ayoung | OK...I guess roles can't be none | 04:56 |
dstanek | ayoung: with a lack of keys that should give you an [], but is there a case there the dict will actually contain a None? | 04:56 |
ayoung | dstanek, ,metadata will never be None if that is what you mean? | 04:57 |
dolphm | ayoung: "metadata" isn't supposed to be in the token | 04:57 |
ayoung | dolphm, V2? | 04:58 |
dolphm | ayoung: "roles" are in "user", and "metadata" is a bug | 04:58 |
ayoung | ah..... | 04:58 |
ayoung | dolphm, so | 04:58 |
dolphm | essex or folsom leaked data from the backend into the token, and it stuck | 04:58 |
dolphm | which is where "extra" came from as well | 04:58 |
ayoung | roles = token_data.get('user', {}).get('roles', []) | 04:58 |
dolphm | and then gyee added it to v3 *intentionally* | 04:58 |
dolphm | ayoung: that looks right, but it's role names, i think? | 04:59 |
dolphm | ayoung: one is IDs, the other is names | 04:59 |
ayoung | looking | 04:59 |
ayoung | nope roles | 05:00 |
ayoung | https://github.com/openstack/keystone/blob/master/keystone/token/providers/common.py#L50 dolphm | 05:00 |
dolphm | ayoung: i mean the values in that list are names | 05:00 |
ayoung | same data is used for both user and metadata sections | 05:01 |
dolphm | ayoung: they're passed in to that method separately | 05:02 |
dolphm | ayoung: metadata comes out of token_ref, and roles_ref is passed in explicitly | 05:03 |
dolphm | jamielennox: still around? | 05:04 |
jamielennox | dolphm: yea | 05:04 |
jamielennox | haven't been following | 05:04 |
dolphm | jamielennox: morganfainberg's revision to https://review.openstack.org/#/c/77701/ is about to pass jenkins check, needs another +A | 05:04 |
stevemar | dolphm rgr | 05:05 |
jamielennox | dolphm: i'll put a +a on it when jenkins passes | 05:05 |
dolphm | jamielennox: danke | 05:05 |
dolphm | i'm off to bed - night everyone | 05:07 |
lbragstad | later | 05:08 |
morganfainberg | dolphm, https://review.openstack.org/#/c/77789 https://review.openstack.org/#/c/77790/ | 05:11 |
morganfainberg | dolphm, lets see what infra has to say about it | 05:11 |
morganfainberg | ayoung, jamielennox, ^ | 05:12 |
jamielennox | morganfainberg: failed already :( | 05:13 |
morganfainberg | jamielennox, typo in yaml | 05:13 |
morganfainberg | ok typo fixed | 05:17 |
*** wchrisj has quit IRC | 05:17 | |
ayoung | crud...looks like in V2 we only get role names... | 05:18 |
ayoung | we don't have any tests that are specific on the roles being revoked for v2, so it was slipping through. I was pulling the values out of the wrong part of the token | 05:20 |
ayoung | dstanek, nice catch | 05:20 |
ayoung | do I need to add role_name to the revocation API? | 05:21 |
ayoung | dolphm, you still awake by any chance? | 05:22 |
morganfainberg | ayoung, really only role name? | 05:22 |
morganfainberg | ayoung, doh! | 05:22 |
ayoung | yep | 05:22 |
morganfainberg | ayoung, when v2 goes away....... | 05:22 |
ayoung | morganfainberg, in both metadata and in user section | 05:23 |
morganfainberg | ayoung, bleh, prob need to revoke by role name then as well. | 05:23 |
ayoung | wait...maybe | 05:23 |
ayoung | it might be Id in metadata | 05:23 |
morganfainberg | ayoung, that would make life a bit better | 05:23 |
ayoung | yehaa! | 05:23 |
morganfainberg | this is why i ask these questions | 05:23 |
ayoung | I need a test for that, though | 05:24 |
ayoung | should have been caught by existing tests | 05:25 |
morganfainberg | ayoung, ++ | 05:25 |
ayoung | morganfainberg, I'm guessing that revoking a role is doing something much more draconian, and that is masking it | 05:25 |
morganfainberg | ayoung, probably | 05:26 |
morganfainberg | ayoung, actually didn't that do something horrible like revoke all tokens for that user (at least at one point) | 05:26 |
ayoung | yeah | 05:26 |
morganfainberg | ayoung, https://review.openstack.org/#/c/77790/ | 05:28 |
morganfainberg | ayoung, hopefully that gets looked at / in soon | 05:28 |
morganfainberg | then pep8 on sample config goes away | 05:28 |
stevemar | jamielennox, jenkins passed https://review.openstack.org/#/c/77701/ | 05:31 |
stevemar | you can do the honors | 05:31 |
jamielennox | stevemar: done thanks | 05:32 |
dstanek | morganfainberg: that's really nice | 05:33 |
morganfainberg | dstanek, ayoung, stevemar, so an alternative to the user_id stuff: http://paste.openstack.org/show/72032/ I'm not pleased with the result...but it's better than user_id@@domain (no chopping needed to fit everything). it still suffers from 32 bytes being the limit for the user_id derivation | 05:35 |
ayoung | 640k should beenough for anyone | 05:36 |
morganfainberg | dstanek, ayoung, stevemar, after spending a good deal of time talking it over here internally, we [my coworkers and I] feel that 32 bytes is just going to be too restrictive. I'll back off on my "you're wrong" if this is the only way, but honestly, I think we're going to run into issues with data for user_id, and the map table is a better fit | 05:37 |
ayoung | morganfainberg, we have been restricted to 32 bytes thus far | 05:37 |
ayoung | but... | 05:37 |
morganfainberg | ayoung, we haven't been sourcing data from federated users and SAML assertions | 05:37 |
ayoung | send it past henrynash, he's the one with the burning need for multiple LDAP | 05:38 |
morganfainberg | yeah. | 05:38 |
*** gokrokve has quit IRC | 05:38 | |
morganfainberg | ayoung, the real win here is just that we know the fixed fields and get a nice(who am i kidding, b64 is ugly) user_id that is urlsafe | 05:38 |
ayoung | morganfainberg, I'd also be interesting in Chadwicks perspective | 05:39 |
*** gokrokve has joined #openstack-keystone | 05:39 | |
ayoung | morganfainberg, I'm not sure that the effort is worth it | 05:39 |
morganfainberg | ayoung, without regressing or needing to update field lengths everywhere in OS | 05:39 |
ayoung | I'd rather get a longer userid than worry about the urlsafety | 05:39 |
stevemar | ayoung, yeah, chadwicks opinion would be good on this issue | 05:39 |
morganfainberg | ayoung, you're not going to get a longer user_id | 05:39 |
ayoung | I think urlsafety is going to be provided | 05:39 |
ayoung | morganfainberg, yes..if we do a really short domain id | 05:40 |
morganfainberg | ayoung, i do magic to raw bytes to cut down the size of the domain_id | 05:40 |
ayoung | drop the uuencode | 05:40 |
ayoung | userid@@123456 | 05:40 |
morganfainberg | ayoung, can do the same thing w/ this code and maintain url_safety | 05:40 |
ayoung | but ^^ gets you close to 60 bytes | 05:40 |
derek_c | is there a way to log the HTTP requests/responses sent/received by keystoneclient? | 05:40 |
morganfainberg | ayoung, i'm not going to budge on urlsafe. sorry. | 05:41 |
jamielennox | derek_c: using the standard python logging | 05:41 |
jamielennox | it'll print to debug | 05:41 |
jamielennox | --debug if you mean the CLI | 05:41 |
morganfainberg | ayoung, asking everyone who interacts with the API to urlencode when we could sanitize is really dumb | 05:41 |
morganfainberg | ayoung, and terrible ux | 05:41 |
ayoung | morganfainberg, no...you are missing the point | 05:42 |
morganfainberg | and since we don't control what is coming in from SAML or ldap bits... | 05:42 |
ayoung | the IDS that come in *have* to be URL safe already | 05:42 |
ayoung | its not for us to provide an additional layer on top of that | 05:42 |
morganfainberg | ayoung, how are you ever enforcing that? | 05:42 |
derek_c | jamielennox: thanks! that was helpful | 05:42 |
ayoung | morganfainberg, its on the LDAP admins to do it | 05:42 |
morganfainberg | ayoung, "oopse this user can't use OpenStack because they have an unsafe element" | 05:42 |
ayoung | morganfainberg, heh..funny story | 05:43 |
ayoung | so there is a Woman at BU with the username Nova.... | 05:43 |
*** gokrokve has quit IRC | 05:43 | |
morganfainberg | ayoung, you're asking a lot. we can't guarantee someone will change their schema for us, how can we guarantee they wont do something non-url safe? | 05:44 |
morganfainberg | ayoung, let alone whatever falls out of the SAML assertion | 05:44 |
morganfainberg | ayoung, hell, we can't even write to many ldap deployments, - admins loathe to change things for applications | 05:44 |
morganfainberg | ayoung, anyway, we'll run this by henry and dolph tomorrow | 05:45 |
ayoung | morganfainberg, if a user has a non urlsafe username, none of the web apps work. | 05:45 |
ayoung | Its like a rule or something | 05:46 |
*** stevemar has quit IRC | 05:46 | |
*** bvandenh has joined #openstack-keystone | 05:49 | |
ayoung | morganfainberg, but ... url encoding actually won't be that expensive, will it? Its like, what 3 extra charaters for each non url safe char? | 05:51 |
derek_c | looking at the keystoneclient code, does anyone know why some APIs use URLs like /users/%s/OS-KSADM/enabled? What's that OS-KSADM for? | 05:53 |
ayoung | It was an extension that let you do non-core stuff | 05:53 |
jamielennox | derek_c: by convention anything that starts with an OS- is an extension | 05:53 |
ayoung | https://github.com/openstack/keystone/blob/master/keystone/contrib/admin_crud/core.py derek_c | 05:54 |
ayoung | derek_c, it was before the V3 api, and alot of the basic crud operations were missing | 05:54 |
morganfainberg | ayoung, it's 50% total more space or so. | 05:54 |
morganfainberg | ayoung, oh w/o b64 sure | 05:54 |
morganfainberg | ayoung, not sure how friendly urlencoded is but it's a blob userid | 05:55 |
ayoung | morganfainberg, yeah, b64 is only for binary | 05:55 |
ayoung | morganfainberg, so...would that satify your craving for url-safety goodness? | 05:55 |
morganfainberg | ayoung, i'm concerned about going to too short a domain. | 05:55 |
derek_c | interesting. I'm working on a contrib for the V3 api. so should I also use a URL like this? | 05:55 |
morganfainberg | ayoung, but sure urlencode would be fine. | 05:55 |
morganfainberg | ayoung, and as long as we keep enough bytes (how efficient is the SQL for searching on the chopped domain?) i'm fine with that | 05:56 |
ayoung | domain id? Can be short, and we can even record it if we worry about conflicted. 6 chras off the front of the UUID for the domain by deffault | 05:56 |
morganfainberg | ayoung, yeah i figure 12 chars is about right | 05:56 |
morganfainberg | maybe 10 | 05:56 |
morganfainberg | i still think even if we went 10, with 52 bytes we're going to be cutting the id REALLY close to the limits | 05:57 |
morganfainberg | 52 bytes isn't much room. | 05:57 |
morganfainberg | ayoung, i'm still in favor of the map table. but lets see what henry has to say and dolph tomorrow | 05:57 |
ayoung | morganfainberg, if we go with mapping, we do it in the existing user table | 05:58 |
morganfainberg | ayoung, not opposed to something like that | 05:58 |
morganfainberg | ayoung, but i understand why you don't want it. it makes sense. | 05:58 |
ayoung | morganfainberg, no. But I am. | 05:59 |
ayoung | heh...let me get this thing working... | 05:59 |
morganfainberg | ayoung, back to revocations. | 05:59 |
morganfainberg | ayoung, :P | 05:59 |
morganfainberg | ayoung, tomororw we will chat w/ dolph and henry see what makes sense if anything | 05:59 |
ayoung | yeah...so the V2 tests by pass the code that triggers the event... | 05:59 |
ayoung | goes right to the underlying API | 05:59 |
morganfainberg | ayoung, oh joy | 05:59 |
morganfainberg | ayoung, :( | 05:59 |
morganfainberg | ayoung, somehow not surprised | 06:00 |
ayoung | I can fix it...I have the technology | 06:00 |
morganfainberg | ayoung, a text editor and unit test framework? | 06:01 |
ayoung | and the web API | 06:01 |
*** topol has quit IRC | 06:01 | |
*** marcoemorais has joined #openstack-keystone | 06:01 | |
morganfainberg | ayoung, might need scotch to go with that | 06:01 |
ayoung | my dad is a television repairman, he's got the ultimate set of tools. I can fix it. | 06:01 |
ayoung | Heh, I might need that OS_KASDMIN extension for this | 06:02 |
* lbragstad perks at scotch | 06:04 | |
derek_c | does the keystoneclient only support 2.0 APIs? | 06:05 |
jamielennox | derek_c: the CLI - yes | 06:05 |
derek_c | jamielennox: hmmm | 06:06 |
derek_c | jamielennox: the code is there though. why don't we use it? | 06:06 |
jamielennox | we are switching to openstackclient for v3 cli | 06:06 |
lbragstad | derek_c: you can use the openstack-client for some of the other v3 related stuff though | 06:06 |
jamielennox | the keystoneclient is useful for more than just the CLI | 06:06 |
jamielennox | plenty of python applications will want to use the library directly | 06:06 |
lbragstad | osc will forward to keystone-client | 06:07 |
derek_c | ok I see. but if I'm hacking on v3-related stuff, I guess I should add the code to openstackclient? | 06:08 |
jamielennox | derek_c: you will want to add the action code to keystoneclient and then the CLI code to osc | 06:09 |
jamielennox | (although i'm not sure if they are using us at the moment - check first) | 06:09 |
ayoung | OK...so a lot of the role_assignments are going to slip through right now... | 06:10 |
*** bvandenh has quit IRC | 06:11 | |
*** henrynash has joined #openstack-keystone | 06:11 | |
ayoung | we don't emit notifications for role-assignment changes. | 06:11 |
derek_c | jamielennox: I see, thanks! | 06:11 |
morganfainberg | ayoung, hmm. | 06:12 |
morganfainberg | ayoung, v3 do we or all role assignments? | 06:12 |
morganfainberg | ayoung, slip through that is? | 06:12 |
ayoung | nah, all of those were happening in the controllers | 06:12 |
morganfainberg | ayoung, ah ok | 06:13 |
morganfainberg | ayoung, so role assignments (addition or both addition/removal) in v2? | 06:13 |
morganfainberg | because if it's addition, i don't care too much | 06:13 |
ayoung | no, removal | 06:13 |
morganfainberg | ayoung, boo :( | 06:13 |
ayoung | remove role from user and tenant | 06:13 |
*** amcrn has quit IRC | 06:13 | |
morganfainberg | ayoung, hmmmm | 06:13 |
ayoung | self.assignment_api.remove_role_from_user_and_project( | 06:13 |
ayoung | we don't have a notification for that | 06:14 |
* morganfainberg looks | 06:14 | |
ayoung | so I need to call revoke_api directly for now | 06:14 |
morganfainberg | ayoung, i think that is a reasonable alternative. | 06:14 |
morganfainberg | just add it into the manager.remove_role_from_user_and_project ? | 06:15 |
ayoung | um wait... | 06:15 |
*** harlowja is now known as harlowja_away | 06:16 | |
morganfainberg | it's the same place we do revoke_tokens for that. | 06:16 |
ayoung | ah..yeah, trest is now failing for the old API...which I need to make pass again. | 06:16 |
ayoung | nah...too much to get done tonight, I suspect | 06:16 |
*** lbragstad is now known as lbragstad_ | 06:18 | |
derek_c | I'm trying to use the tools/install_venv.py in openstackclient but got a bunch of errors when installing the cryptography package. the errors are basically the same as described here: http://stackoverflow.com/questions/22073516/failed-to-install-pyhton-cryptography-package-with-pip-and-setup-py | 06:19 |
derek_c | does anyone know what could be going on? | 06:19 |
morganfainberg | derek_c, same errors as that SO post? | 06:20 |
morganfainberg | derek_c, or different, that one looks like cffi lib is the wrong version | 06:20 |
morganfainberg | ayoung, i'll do another pass on the revocation stuff before i head home in an hour here | 06:21 |
morganfainberg | ayoung, i don't think i had anything outstanding but i've looked at it a lot | 06:21 |
derek_c | mine also seems to be cffi-related | 06:21 |
derek_c | seems to die at: c/_cffi_backend.c:14:17: fatal error: ffi.h: No such file or directory | 06:21 |
ayoung | morganfainberg, thanks...I'll direct yo uat the V2 specific stuff | 06:21 |
morganfainberg | ayoung, thanks :) | 06:21 |
morganfainberg | derek_c, ah you need the devel libraries for ffi | 06:22 |
derek_c | libffi-dev? | 06:22 |
morganfainberg | derek_c, that sounds right | 06:22 |
derek_c | morganfainberg: cool I will give it a try. thanks! | 06:22 |
morganfainberg | derek_c, sure thing! | 06:22 |
ayoung | morganfainberg, OK, its not so bad...just scary that it got this far, but now I see how I got fooled | 06:26 |
morganfainberg | ayoung, np. | 06:26 |
*** gokrokve has joined #openstack-keystone | 06:27 | |
morganfainberg | ayoung, it happens :P | 06:27 |
morganfainberg | it's why we code review massive changes into the ground. | 06:27 |
morganfainberg | actually, it's why we don't do massive code reviews (not that it was easily avoidable here) | 06:27 |
morganfainberg | ayoung, crud, i was going to add something to the meeting agenda... | 06:28 |
morganfainberg | now i can't remember what it is | 06:28 |
*** gokrokve_ has joined #openstack-keystone | 06:29 | |
ayoung | morganfainberg, there was no choice with this one. | 06:29 |
morganfainberg | ayoung, yep. | 06:29 |
morganfainberg | ayoung, it wasn't exactly bite sized bits being implemented | 06:29 |
*** gokrokve has quit IRC | 06:31 | |
*** henrynash has quit IRC | 06:34 | |
*** gyee has quit IRC | 06:34 | |
*** henrynash has joined #openstack-keystone | 06:56 | |
*** henrynash has quit IRC | 06:58 | |
*** henrynash has joined #openstack-keystone | 07:00 | |
ayoung | morganfainberg, https://review.openstack.org/#/c/77411/ one patch, no rechecks. THat may be a record | 07:08 |
morganfainberg | ayoung, hehe | 07:09 |
morganfainberg | ayoung, no rechecks is the part that makes me go "what happened here"? :P | 07:09 |
ayoung | morganfainberg, just reposted https://review.openstack.org/#/c/55908/ | 07:09 |
morganfainberg | ayoung, ah was actually in the middle of reviewing :P | 07:10 |
ayoung | the part that really needs a deep look is the changes to assignments/core | 07:10 |
morganfainberg | ayoung, ok no worries. i'll pull forward any comments i have | 07:10 |
morganfainberg | nbd | 07:10 |
ayoung | 'salright...diff is pretty clear | 07:10 |
ayoung | I'll look for comments in each review | 07:10 |
morganfainberg | ayoung, nah some were related to dstanek's i'll just pull em all forward only had 1 or two so far | 07:11 |
ayoung | the new test in test_content_types is important. It is what really tests the revoke on v2 tokens | 07:11 |
morganfainberg | k | 07:11 |
morganfainberg | thanks | 07:11 |
ayoung | I'm a crash | 07:12 |
ayoung | its past 2...and we still will havea a7 AM wakeup with the kids | 07:12 |
*** saju_m has joined #openstack-keystone | 07:12 | |
morganfainberg | see ya | 07:12 |
*** ayoung is now known as ayoung-ZZzZzzZz_ | 07:12 | |
*** bvandenh has joined #openstack-keystone | 07:30 | |
*** henrynash has quit IRC | 07:32 | |
*** bvandenh has quit IRC | 07:36 | |
*** morganfainberg is now known as morganfainberg_Z | 08:02 | |
*** henrynash has joined #openstack-keystone | 08:03 | |
*** henrynash has quit IRC | 08:22 | |
*** saju_m has quit IRC | 08:26 | |
*** morganfainberg_Z is now known as morganfainberg | 08:44 | |
*** morganfainberg is now known as morganfainberg_Z | 08:46 | |
*** amichon has joined #openstack-keystone | 08:54 | |
*** leseb has joined #openstack-keystone | 08:57 | |
*** derek_c has quit IRC | 08:57 | |
*** arborism has joined #openstack-keystone | 09:04 | |
*** amichon has quit IRC | 09:06 | |
*** amichon has joined #openstack-keystone | 09:06 | |
*** arborism is now known as amcrn | 09:07 | |
*** marcoemorais has quit IRC | 09:19 | |
*** raies has quit IRC | 09:30 | |
*** saju_m has joined #openstack-keystone | 09:42 | |
*** leseb has quit IRC | 10:07 | |
*** leseb has joined #openstack-keystone | 10:09 | |
*** morganfainberg_Z is now known as morganfainberg | 10:39 | |
* morganfainberg wonders if about the Nova v2 conversation(s) | 10:40 | |
*** amcrn has quit IRC | 11:04 | |
*** amcrn has joined #openstack-keystone | 11:13 | |
*** morganfainberg is now known as morganfainberg_Z | 11:16 | |
*** leseb has quit IRC | 11:18 | |
*** leseb has joined #openstack-keystone | 11:19 | |
*** leseb has quit IRC | 11:23 | |
*** marekd|away is now known as marekd | 11:43 | |
*** amichon has quit IRC | 11:57 | |
*** leseb has joined #openstack-keystone | 12:01 | |
*** leseb has quit IRC | 12:05 | |
*** amichon has joined #openstack-keystone | 12:23 | |
*** leseb has joined #openstack-keystone | 12:30 | |
*** david-lyle has quit IRC | 12:39 | |
*** topol has joined #openstack-keystone | 12:39 | |
*** saju_m has quit IRC | 12:40 | |
*** zhiyan has joined #openstack-keystone | 12:43 | |
*** jamielennox is now known as jamielennox|away | 12:50 | |
*** saju_m has joined #openstack-keystone | 12:52 | |
*** achampion has quit IRC | 12:57 | |
*** saju_m has quit IRC | 13:04 | |
*** bvandenh has joined #openstack-keystone | 13:08 | |
*** bvandenh has quit IRC | 13:18 | |
*** bvandenh has joined #openstack-keystone | 13:18 | |
dstanek | dolphm, morganfainberg_Z, ayoung-ZZzZzzZz_: i just update the token revocation review to fix minor things Morgan and I brought up | 13:53 |
dstanek | there are still a few things on it that may need to be addressede | 13:54 |
*** wchrisj has joined #openstack-keystone | 14:08 | |
*** achampion has joined #openstack-keystone | 14:09 | |
*** lbragstad_ has quit IRC | 14:18 | |
*** amichon has quit IRC | 14:26 | |
dolphm | dstanek: thanks! looking now | 14:27 |
dstanek | dolphm: i thought his comment about optional dependencies may need some discussion | 14:28 |
dolphm | dstanek: where at? | 14:28 |
dstanek | dolphm: he mentions it pretty much everywhere revoke_api is being injected | 14:29 |
*** richm has joined #openstack-keystone | 14:32 | |
*** dims has quit IRC | 14:43 | |
*** nkinder has quit IRC | 14:45 | |
*** lbragstad has joined #openstack-keystone | 14:45 | |
*** dims has joined #openstack-keystone | 14:46 | |
*** huats has quit IRC | 14:46 | |
*** huats has joined #openstack-keystone | 14:47 | |
*** huats has quit IRC | 14:47 | |
*** huats has joined #openstack-keystone | 14:47 | |
*** stevemar has joined #openstack-keystone | 14:52 | |
*** ChanServ sets mode: +v stevemar | 14:52 | |
*** browne has joined #openstack-keystone | 14:57 | |
dolphm | dstanek: left some comments on https://review.openstack.org/#/c/77215/ -- the exception handling obviously isn't tested... but i can't imagine what's there is useful today, either? | 14:58 |
*** leseb has quit IRC | 14:59 | |
stevemar | is there a reason we haven't approved https://review.openstack.org/#/c/59914/12 (from ML) ? | 14:59 |
dolphm | stevemar: lack of review focus on the client? | 14:59 |
stevemar | dolphm, ding ding ding | 15:00 |
dolphm | stevemar: that'd be great to have (and it's calling your code, dstanek) | 15:00 |
stevemar | yeah, i was wondering if it conflicts with dstanek 's code, but i don't think it does | 15:00 |
*** gokrokve_ has quit IRC | 15:01 | |
*** gokrokve has joined #openstack-keystone | 15:01 | |
bknudson | dolphm: looks like auth_token tries to create the directory if it didn't exist. | 15:02 |
dolphm | bknudson: but it can't properly detect the condition, afaict. what raises an IOError? | 15:03 |
bknudson | dolphm: I'd have to try it out. | 15:04 |
dstanek | dolphm: tempfile does raise IOError in some cases | 15:04 |
dstanek | i don't know if it's in the code path we are using, but it's there | 15:04 |
dolphm | bknudson: dstanek: http://pasteraw.com/sonyss4gove3d5n1dvt5ben76obxeq9 | 15:04 |
dolphm | bknudson: dstanek: so maybe it should catch (IOError, OSError) ? | 15:05 |
bknudson | why would IOError be raised? | 15:05 |
bknudson | and if it is, should auth_token try to write again after checking the directory exists? | 15:05 |
*** leseb has joined #openstack-keystone | 15:05 | |
dstanek | bknudson: there are a few places in the tempfile module that potentially raise it - stat-ing a file, looking for the default tempdir, etc. | 15:06 |
*** joesavak has joined #openstack-keystone | 15:07 | |
bknudson | ok. I don't have a problem with catching IOError, OSError. | 15:08 |
dolphm | bknudson: dstanek: i'll rev the patch as such | 15:09 |
dolphm | dstanek: bknudson: any reason not to use tempfile.mkstemp instead of tempfile.NamedTemporaryFile? | 15:15 |
bknudson | NamedTemporary file closes automatically in context. | 15:16 |
dstanek | dolphm: the named version has a filename on disk that you can get at - i don't think the other guarantees that | 15:17 |
dolphm | bknudson: mkstemp doesn't? | 15:19 |
dolphm | dstanek: http://docs.python.org/2/library/tempfile.html#tempfile.mkstemp the last line in the doc says you get back a file name | 15:19 |
bknudson | dolphm: mkstemp() returns a tuple containing an OS-level handle to an open file | 15:19 |
bknudson | so you'd have to close it. | 15:19 |
bknudson | and you'd have to close it even if there was an exception... so would need try/except | 15:20 |
dstanek | dolphm: not entirely sure why, but i remember named as being safer | 15:23 |
dstanek | plus what bknudson said | 15:23 |
*** chandan_kumar has joined #openstack-keystone | 15:24 | |
dstanek | dolphm: oh, wait; i had to look through the module, but i remember :-) | 15:25 |
bknudson | I used to be able to set "disable_service mysql ; enable_service postgresql" in my devstack localrc to run postgres... | 15:25 |
dstanek | you want NamedTemporaryFile over TemporaryFile because you are guaranteed a filename on the filesystem | 15:25 |
dstanek | you want NamedTemporaryFile over using mkstemp directly because of what bknudson said | 15:26 |
dolphm | dstanek: ah, interesting | 15:27 |
*** david-lyle has joined #openstack-keystone | 15:28 | |
dstanek | stevemar: beat me to the changing passwords review! | 15:30 |
*** ayoung-ZZzZzzZz_ is now known as ayoung | 15:30 | |
ayoung | dstanek, you may be my new favorite person. | 15:31 |
*** nkinder has joined #openstack-keystone | 15:32 | |
dstanek | ayoung: ? | 15:32 |
ayoung | "David StanekUploaded patch set 80." | 15:32 |
dstanek | is that a good thing or a bad thing? | 15:32 |
dstanek | ah | 15:32 |
ayoung | I'll let you male that call | 15:32 |
stevemar | dstanek, you can still review it, too :P | 15:32 |
dstanek | i didn't address morganfainberg_Z's quesiton about optional dependencies | 15:32 |
ayoung | looking now | 15:32 |
ayoung | I likwe 30 * 60....Ah, whateve | 15:34 |
dolphm | ayoung: i'm working on revving the atomic write patch, btw | 15:34 |
ayoung | dolphm, sounds good. Nits, or something real? | 15:35 |
dolphm | ayoung: both, i'd say | 15:35 |
dolphm | ayoung: nitty refactor and i think improving exception handling significantly | 15:35 |
ayoung | Ah, that is good | 15:36 |
*** bvandenh has quit IRC | 15:38 | |
dolphm | ayoung: this is where i'm at http://pasteraw.com/z6plh1drrkrh19v14sbqfr5ya3129f | 15:38 |
dolphm | ayoung: centralizes the exception handling into _atomic_write_to_signing_dir() and includes a catch for OSError | 15:39 |
*** bvandenh has joined #openstack-keystone | 15:41 | |
ayoung | dolphm, looks about right. | 15:42 |
ayoung | dolphm, so, revoke events looks pretty close. Found a bug last night in v2 token handling. What I'd like to do is this: | 15:42 |
ayoung | knock out any last things we find on review and get it merged | 15:43 |
ayoung | add in a line in the docs: this is new and experimental | 15:43 |
ayoung | engage my own upstream QA to work on tempest tests while I work on a client piece | 15:43 |
ayoung | \ | 15:44 |
*** thedodd has joined #openstack-keystone | 15:44 | |
ayoung | For Juno, we can work to make it the default mechanism and grow morganfainberg_Z 's ephemeral support on top of it | 15:45 |
dolphm | ayoung: sounds like a reasonable goal; i'm trying to knock out a few smaller things, and then i want to spend the rest of the day (as necessary) on revocation | 15:46 |
ayoung | ++ | 15:46 |
dolphm | ayoung: granted today is meeting day... bad timing! | 15:47 |
ayoung | of course. | 15:47 |
dolphm | bknudson: this is gating https://review.openstack.org/#/c/76949/ - and then we can land your fixes in keystone | 15:48 |
bknudson | dolphm: I rebased since a migration 41 merged on it. | 15:49 |
ayoung | dolphm, is the autoinc for sqlalchemy well supported? morganfainberg_Z suggested I use it in place of uids, and considering the conversation about 10000 speed up, I am tempted to do so | 15:49 |
dolphm | ayoung: yeah you don't have any reason to use UUIDs there | 15:49 |
dolphm | ayoung: well... | 15:49 |
ayoung | dolphm, my thinking was just to avoid contention for the database | 15:49 |
ayoung | we don't select or index on them, but sql does treat it as a pkey | 15:50 |
dolphm | ayoung: yeah... use either | 15:50 |
*** devlaps has joined #openstack-keystone | 15:50 | |
dstanek | stevemar: i think i may just fix it...so we can get rolling | 15:50 |
ayoung | going to try it with pkey...I'll put something in the migration test to check for mysql and postgres | 15:50 |
stevemar | dstanek, sure thing | 15:51 |
dolphm | ayoung: actually hold up, i'd rather not see any unnecessary flux on the patch | 15:52 |
ayoung | dolphm, its OK, I can just test it out. I won't submit it | 15:52 |
dstanek | stevemar: i tired out your test suggestions and they don't work :-( | 15:56 |
dstanek | *tried | 15:56 |
dstanek | stevemar: if you don't set the user_id on client it is set to None | 15:57 |
stevemar | dstanek, hmm, sec | 15:58 |
stevemar | dstanek, could use self.TEST_USER | 15:59 |
dstanek | stevemar: let me give that a try | 15:59 |
stevemar | rather than create a new one | 15:59 |
dstanek | stevemar: the author basically reused this http://git.openstack.org/cgit/openstack/python-keystoneclient/tree/keystoneclient/tests/v2_0/test_users.py#n209 | 15:59 |
stevemar | ahh okay | 15:59 |
stevemar | my second comment was referring to line 215 of your link | 16:00 |
*** devlaps has quit IRC | 16:11 | |
*** devlaps has joined #openstack-keystone | 16:11 | |
dstanek | stevemar: what is the value of specifying the response body and asserting that we got it? just that it was passed through? | 16:12 |
stevemar | dstanek, i take that back .. i wanted to make sure the manager was returning the right thing | 16:13 |
stevemar | the other stuff i don't think is helpful | 16:13 |
dstanek | stevemar: in this case the manager method won't return anything | 16:15 |
dstanek | stevemar: or i should say isn't - looking to see if i messed it up or if it was already messed up | 16:16 |
stevemar | assertIsNone then? | 16:17 |
*** bvandenh has quit IRC | 16:17 | |
dstanek | stevemar: it's actually correct; we get an empty body back from the update | 16:18 |
dstanek | yes, assertIsNone would make it clear that we expect that | 16:18 |
stevemar | cool cool | 16:18 |
stevemar | i'm good with that | 16:19 |
dstanek | probably should be a 204, but it's returning a 200 with no content | 16:19 |
stevemar | yeah, that's what i was wondering ... | 16:24 |
dstanek | stevemar: i was just up too late this morning - don't listen to me | 16:26 |
dstanek | errr...last night | 16:27 |
ayoung | dolphm, http://paste.fedoraproject.org/82230/95071113/ is the diff for chaning over to sql.Integer for the sql backend | 16:32 |
ayoung | including a good bit of testing | 16:33 |
ayoung | half the patch is a refactor moving insert_dict into the base class | 16:33 |
dstanek | i don't see a lot of _() in keystoneclient - do we do something different/special there? | 16:37 |
*** harlowja_away is now known as harlowja | 16:40 | |
*** harlowja has quit IRC | 16:41 | |
dolphm | dstanek: it's just relatively new in the client | 16:41 |
dolphm | dstanek: (if it's there at all?) | 16:41 |
bknudson | dstanek: I'm not sure how _() would work in the client library | 16:41 |
dstanek | dolphm: i don't think it's there at all | 16:42 |
bknudson | they'd need to have the translated strings. | 16:42 |
dstanek | bknudson: yeah, they would need to be in the package that we release to pypi | 16:43 |
bknudson | and you'd need to install the translation domain or something? | 16:43 |
dstanek | bknudson: i think most things look to the environment to see what to use | 16:43 |
bknudson | dstanek: keystone does "gettextutils.install('keystone', lazy=True)" | 16:44 |
bknudson | so it would also have to gettextutils.install('keystoneclient', lazy=True)" ? | 16:44 |
bknudson | or is it keystoneclient would do that with it's _? | 16:44 |
bknudson | keystoneclient would need a different _. | 16:45 |
*** gyee has joined #openstack-keystone | 16:48 | |
dolphm | token revocation has a +1 from jenkins https://review.openstack.org/#/c/55908/ | 16:48 |
dstanek | bknudson: good question...no idea; i've never really had to worry about dealing with non-English languages | 16:50 |
dstanek | and when i did it was using Python's gettext directly | 16:51 |
*** leseb has quit IRC | 16:56 | |
*** harlowja has joined #openstack-keystone | 16:56 | |
*** leseb has joined #openstack-keystone | 16:57 | |
dstanek | stevemar: just pushed an update | 16:57 |
stevemar | dstanek, just reviewed it | 16:57 |
dstanek | stevemar: gracias | 16:58 |
stevemar | dstanek, de nada | 16:58 |
stevemar | dstanek, if you want to stay in the keystoneclient groove: https://review.openstack.org/#/c/60820/ | 17:07 |
*** topol has quit IRC | 17:12 | |
*** rwsu has joined #openstack-keystone | 17:14 | |
*** stevemar has quit IRC | 17:21 | |
*** marcoemorais has joined #openstack-keystone | 17:24 | |
gyee | ayoung, trying to understand your comment here https://review.openstack.org/#/c/55908/80/keystone/common/controller.py | 17:27 |
gyee | I thought validate_token also hit the caching layer, no? | 17:28 |
ayoung | self.token_api.token_provider_api.validate_token( does nt get cached | 17:29 |
gyee | but it calls token_api.get_token() right? | 17:30 |
ayoung | gyee, what I would like to do, though, is have validate token be responsible for unpacking the token in the future | 17:31 |
ayoung | and using the guts of the PKI token that was handed in | 17:31 |
ayoung | gyee, ideally the code will be identical between client and server with the exception of how the revocation events are fetched | 17:31 |
gyee | ayoung, sure, for PKI token, Keystone is essentially its own consumer | 17:32 |
*** leseb has quit IRC | 17:32 | |
ayoung | right, and we want to move to ephemeral in Juno | 17:32 |
*** leseb has joined #openstack-keystone | 17:32 | |
gyee | ++ for ephemeral, we have problem with stale data in general | 17:33 |
*** fabiog has joined #openstack-keystone | 17:36 | |
dstanek | stevemar: i added a couple of comments to that review | 17:36 |
*** leseb has quit IRC | 17:36 | |
*** stevemar has joined #openstack-keystone | 17:44 | |
*** ChanServ sets mode: +v stevemar | 17:44 | |
*** jraim_ has joined #openstack-keystone | 17:45 | |
*** henrynash has joined #openstack-keystone | 17:48 | |
*** henrynash has quit IRC | 17:48 | |
*** browne has quit IRC | 17:50 | |
*** henrynash has joined #openstack-keystone | 17:51 | |
*** topol has joined #openstack-keystone | 17:51 | |
*** chandan_kumar has quit IRC | 17:53 | |
*** jraim has quit IRC | 17:53 | |
*** jraim_ is now known as jraim | 17:53 | |
*** topol has quit IRC | 17:55 | |
*** gokrokve has quit IRC | 17:55 | |
*** gokrokve has joined #openstack-keystone | 17:56 | |
ayoung | gyee, can you make the point on the discussion about the mapping table approach to deconflicting userids | 17:57 |
*** browne has joined #openstack-keystone | 17:57 | |
gyee | ayoung, sure | 17:58 |
ayoung | ++ | 17:58 |
*** topol has joined #openstack-keystone | 17:59 | |
gyee | ayoung, for read-only LDAP deployment, user don't live or managed by Keystone | 17:59 |
ayoung | gyee, yeah. | 17:59 |
ayoung | gyee, same will be true for Federated | 17:59 |
ayoung | and I don't want stale data in the Identity portion of out table | 17:59 |
ayoung | er | 17:59 |
ayoung | database | 18:00 |
gyee | infra services don't use the user ID I don't think | 18:00 |
*** gokrokve has quit IRC | 18:00 | |
*** gokrokve has joined #openstack-keystone | 18:01 | |
gyee | for usage tracking and aggregation, it is whatever ID that Keystone assigns | 18:01 |
lbragstad | FYI, pushed hacking check for spaces after # in comments: https://review.openstack.org/#/c/77950/2 ( morganfainberg_Z dolphm ) | 18:05 |
*** morganfainberg_Z is now known as morganfainberg | 18:08 | |
*** jamielennox|away is now known as jamielennox | 18:10 | |
*** haneef_ has joined #openstack-keystone | 18:11 | |
*** haneef_ has quit IRC | 18:18 | |
*** haneef_ has joined #openstack-keystone | 18:21 | |
*** gokrokve has quit IRC | 18:25 | |
*** gokrokve has joined #openstack-keystone | 18:25 | |
*** henrynash has quit IRC | 18:29 | |
*** gokrokve has quit IRC | 18:29 | |
*** leseb has joined #openstack-keystone | 18:32 | |
*** achampion has quit IRC | 18:42 | |
*** achampion has joined #openstack-keystone | 18:49 | |
*** gokrokve has joined #openstack-keystone | 18:50 | |
*** thedodd has quit IRC | 18:56 | |
*** thedodd has joined #openstack-keystone | 19:00 | |
gyee | dstanek, but encoding is generally slow, no? | 19:00 |
gyee | I mean it adds overhead | 19:01 |
dstanek | gyee: i haven't measured, but probably a little | 19:01 |
dstanek | that won't bypass encoding in Py2 for unicode strings, which need to be encoded | 19:02 |
dolphm | lbragstad: i love that you patch failed the hacking codebase itself | 19:03 |
dolphm | your* | 19:03 |
morganfainberg | dolphm, hehehe | 19:03 |
*** finite has joined #openstack-keystone | 19:04 | |
dolphm | lbragstad: although, looking at the failures, i'm wondering if it should be "whitespace" instead of "a single space" | 19:04 |
dolphm | lbragstad: i'm not fan of leaving commented out / indented code in the codebase (and if you do, it should be in blockquotes), but the license block has a sort of decent use case for "whitespace", i think | 19:05 |
lbragstad | dolphm: yeah I was thinking about htat too | 19:05 |
dolphm | lbragstad: another valid exception :-/ "#!/usr/bin/env python" | 19:05 |
lbragstad | I would say we'd need to add that with a regex | 19:05 |
lbragstad | so, we would need to allow the following cases: '#TODO', '#FIXME', '#NOTE', '#!/usr/bin/env python', | 19:07 |
dolphm | lbragstad: why do you think we should keep #TODO instead of requiring # TODO ? | 19:07 |
dolphm | lbragstad: is there some special handing for #TODO in IDE's or something? | 19:07 |
lbragstad | I agree with switching to # TODO, I was just being consistent with the H101 check | 19:08 |
lbragstad | I would rather have it be #<space>TODO, or NOTE, or FIXME | 19:08 |
lbragstad | but, I was looking through the existing checks and though 'I suppose I better not break H101 | 19:09 |
lbragstad | ' | 19:09 |
lbragstad | dolphm: afaict there is no special handling for TODO in IDEs... I use vim | 19:11 |
dolphm | lbragstad: are there tests or something that require supporting #TODO? | 19:12 |
dolphm | lbragstad: (that you'd have to change) | 19:12 |
*** fabiog has quit IRC | 19:12 | |
lbragstad | checking | 19:12 |
finite | Hey folks, I'm trying to set up SSL on my keystone install, and have been going through this: http://docs.openstack.org/developer/keystone/configuration.html#ssl but coming up short. The service doesn't seem to start back up after making the changes. I am using a cert provided by our internal PKI | 19:13 |
finite | Is there a log other than the system log I can look at for keystone failing to start? | 19:14 |
lbragstad | dolphm: not seeing any tests specifically for ensuring '#TODO' | 19:14 |
dolphm | lbragstad: then kill support for it :) | 19:15 |
lbragstad | :) | 19:15 |
ayoung | finite, turn on debugging: | 19:16 |
*** gokrokve_ has joined #openstack-keystone | 19:17 | |
finite | Thanks ayoung, I'll turn on debugging and see what that gleans me. | 19:18 |
dstanek | lbragstad: ensuring that there is no space? | 19:19 |
lbragstad | https://review.openstack.org/#/c/77950/ | 19:19 |
lbragstad | dstanek: ^ | 19:19 |
lbragstad | dstanek: do you know if that is tested somehow in hacking? | 19:19 |
lbragstad | s/that/the case where #TODO/ | 19:20 |
lbragstad | test: hacking.tests.test_doctest.HackingTestCase.test_pep8(hacking_todo_format-line-4) | 19:20 |
dstanek | lbragstad: ha, i have the same check | 19:20 |
lbragstad | ++ | 19:21 |
*** gokrokve has quit IRC | 19:21 | |
lbragstad | dstanek: yeah, we were talking about it last night when reviewing ayoung 's patch for token revocation, I wanted to go through and fix all the #style comments | 19:21 |
stevemar | anyone know the pluggable auth structure for client well enough? who is not jamie? | 19:22 |
lbragstad | and it was mentioned that we should try and add that check to hacking | 19:22 |
ayoung | stevemar, a bit | 19:22 |
dstanek | lbragstad: mine is more general i think - http://paste.openstack.org/show/72283/ | 19:22 |
stevemar | ayoung, i don't wanna bug you though, you got enough on your plate :) | 19:22 |
dstanek | lbragstad: i fixed our current master to pass that test | 19:23 |
lbragstad | dstanek: yep, yours is... | 19:23 |
ayoung | stevemar, 'salright..I'm mostly waiting for feedback right now | 19:23 |
lbragstad | dstanek: where does that live? | 19:23 |
dstanek | lbragstad: how quickly do they accept stuff like that into hacking? | 19:23 |
lbragstad | dstanek: that I'm not sure... | 19:23 |
dstanek | lbragstad: right now in a local stash - that's a part of the bunch i'm going to submit by the end of the day | 19:24 |
lbragstad | it didn't take long to attact a couple -1's after I pushed it for review though | 19:24 |
stevemar | ayoung, okay, so i'm trying to add auth via oauth tokens, but I don't want to go and mangle all the client and httpclient stuff that's already set up | 19:24 |
dstanek | is hacking an -infra thing? | 19:24 |
*** amcrn has quit IRC | 19:24 | |
lbragstad | dstanek: well, i know when new stuff goes in it can break a bunch of stuff... so I would image the infra guys are usually all over is | 19:24 |
lbragstad | s/is/it | 19:24 |
ayoung | stevemar, I think we still need to figure out how to select the right plugin for the CLI use cases ,but fro calling from pyuthon code...you should be creating a new auth plugin | 19:25 |
dstanek | lbragstad: that's part of the reason i'm putting my checks in out codebase - some other projects may not be able to upgrade to hacking (with my checks) easily | 19:26 |
lbragstad | dstanek: I like the way you do the isspace detection better than the way I did mine | 19:26 |
lbragstad | dstanek: right.. | 19:27 |
dstanek | lbragstad: i'll ping you when i have the review up...i have to chop out the pieces that are not yet working | 19:27 |
lbragstad | dstanek: I modeled the space detection after the existing way in hacking here: https://github.com/openstack-dev/hacking/blob/master/hacking/core.py#L125 | 19:27 |
lbragstad | dstanek: what can I help you with? | 19:28 |
lbragstad | I can work on throwing a couple up if you want me to | 19:28 |
dstanek | lbragstad: that would be great | 19:28 |
lbragstad | dstanek: sure thing, what do you need? | 19:28 |
dstanek | lbragstad: i have been using a mix of ast, logical_line and physical line checks based on what i am looking for | 19:29 |
dstanek | lbragstad: let me get this stuff in order so you can see what i'm doing and then we can talk about the ones on the list that i haven't tackled yet | 19:29 |
lbragstad | dstanek: cool, that sounds like a plan | 19:30 |
dstanek | lbragstad: are you comforable with ast? | 19:30 |
dstanek | lbragstad: i haven't worked on my pet peeve at all: % should not be used in log statements | 19:30 |
stevemar | ayoung, https://review.openstack.org/#/c/77977/ WIP | 19:31 |
ayoung | stevemar, you missed an important step | 19:31 |
ayoung | adding jamielennox to the review | 19:31 |
ayoung | :) | 19:31 |
stevemar | ayoung, i just posted it 2 seconds ago :) | 19:31 |
lbragstad | dstanek: not entirely, | 19:32 |
ayoung | stevemar, so...can you work on a test for it, so I can see how you are planning to call it? | 19:32 |
stevemar | https://review.openstack.org/#/c/77977/1/keystoneclient/tests/v3/test_oauth1.py | 19:32 |
dstanek | lbragstad: the % you can do using logical lines and matching | 19:33 |
dstanek | lbragstad: ast would be used for more complicated stuff that needs context like try-except-pass | 19:33 |
lbragstad | so, tighter control on syntax almost | 19:34 |
stevemar | ayoung, the only data in the post request to /auth/tokens should be an empty oauth1 property, and oauth1 in methods. The other oauth data, such as consumer and access keys, are in the request header | 19:35 |
*** leseb has quit IRC | 19:36 | |
dstanek | lbragstad: yeah, you are looking the the tree structure of the code instead of just text matching | 19:36 |
ayoung | stevemar, this is going to be fun to test, isn't it | 19:37 |
lbragstad | dstanek: nice | 19:37 |
dstanek | lbragstad: here is a snipped using AST that looks for people using None in assertEqual - http://paste.openstack.org/show/72284/ | 19:38 |
stevemar | ayoung, just do the usual mock request / response test, it should match the expected structure | 19:38 |
dstanek | gyee: question about your comment on the atomic write patch - why would you need a retry counter? i think tempfile already has that builtin | 19:40 |
lbragstad | dstanek: makes sense | 19:40 |
dstanek | lbragstad: excited that i have a partner in crime | 19:41 |
gyee | dstanek, really? | 19:41 |
dstanek | gyee: for finding a filename or dirname to use (pretty sure); what case were you thinking of? | 19:41 |
lbragstad | dstanek: hopefully I'm helpful, I think once I see the review I should be able to get a little more context | 19:42 |
gyee | dstanek, but the code is essentially doing a retry on IOError or OSError | 19:42 |
gyee | hence my comment | 19:42 |
ayoung | stevemar, you are assuming my mind has fully groked the test code in the client...I'm still making the transition....my client work has focused on Auth Token thus far, and while I get what jamielennox is doing, I still need to see it written out in black and white. Add to that the fact I've not looked at oauth in a long time, and current work has me in a few directions.... | 19:43 |
*** topol has quit IRC | 19:43 | |
ayoung | write a test...it will be required for commit, and it will make it easier for all to review | 19:43 |
*** joesavak has quit IRC | 19:43 | |
finite | Hey @ayoung, turning on debugging I see this in they keystone.log: 2014-03-04 19:23:42.625 8461 TRACE keystone.common.environment.eventlet_server SSLError: [Errno 336445442] _ssl.c:355: error:140DC002:SSL routines:SSL_CTX_use_certificate_chain_file:system lib | 19:44 |
ayoung | finite, are your certs readable? | 19:44 |
*** stevemar2 has joined #openstack-keystone | 19:44 | |
*** ChanServ sets mode: +v stevemar2 | 19:44 | |
finite | 644 root.root | 19:46 |
*** wchrisj has quit IRC | 19:47 | |
*** arborism has joined #openstack-keystone | 19:47 | |
*** arborism is now known as amcrn | 19:47 | |
finite | it looked like glance had an issue like this at one time? https://bugs.launchpad.net/glance/+bug/1160529 | 19:47 |
*** stevemar has quit IRC | 19:48 | |
*** wchrisj has joined #openstack-keystone | 19:49 | |
*** stevemar has joined #openstack-keystone | 19:49 | |
*** ChanServ sets mode: +v stevemar | 19:49 | |
lbragstad | dolphm: looks like we are going to have to throw https://review.openstack.org/#/c/77950/ in https://github.com/jcrocholl/pep8/blob/master/pep8.py | 19:49 |
stevemar | ayoung, i hear ya | 19:50 |
*** stevemar2 has quit IRC | 19:51 | |
lbragstad | cc dstanek ^ | 19:52 |
finite | ayoung, I found one that wasn't readable. Looks like that might have fixed it. Thanks for helping! | 19:53 |
stevemar | dstanek, qq | 19:53 |
ayoung | finite, so...want a better approach? | 19:54 |
stevemar | dstanek, i think there was a typo in your first comment: https://review.openstack.org/#/c/60820/12/keystoneclient/tests/v3/test_oauth1.py | 19:54 |
*** topol has joined #openstack-keystone | 19:54 | |
finite | For sure | 19:54 |
finite | ayoung, always heh | 19:56 |
ayoung | finite, certmonger | 19:56 |
ayoung | finite, I'm messing around with it right now | 19:56 |
ayoung | but using certmonger to talk to a CA | 19:56 |
ayoung | or even just to track your "selfsigned" if you don';t have a CA | 19:57 |
ayoung | what platform are you on? | 19:57 |
finite | CentOS 6.4 64bit | 19:57 |
finite | ayoung I'll dig into that, thanks. :) | 19:58 |
ayoung | finite, are you going to be getting real CA certs, or just doing selfsigned? | 19:59 |
finite | Company signed PKI, internal. | 19:59 |
ayoung | finite, ah...what CA are you using? | 19:59 |
ayoung | software | 20:00 |
*** leseb has joined #openstack-keystone | 20:01 | |
finite | ayoung, no idea, other it folks generate everything and I can't find any info in the cert | 20:02 |
ayoung | finite, ask them...one nice thing about certmonger is it will do renewals for you | 20:02 |
*** gyee has quit IRC | 20:03 | |
ayoung | I work with one of the certmonger core devs..one issue we have is there are not enough people requesting support for their CAs, so we don;t know whom to support | 20:03 |
finite | ayoung, nice I'll hit them up as I have a few more to request. Thanks for all your help, greatly appreciated | 20:03 |
ayoung | Dogtag and FreeIPA are well supported | 20:03 |
ayoung | as is selfsigned | 20:03 |
*** krsna has joined #openstack-keystone | 20:08 | |
krsna | I have a +2 already and a few +1s. I just need an aproved https://review.openstack.org/#/c/77294/ if anyone cares to take a look ;) | 20:09 |
stevemar | dstanek, i think i addressed your comments, through either reply backs, or new code: https://review.openstack.org/#/c/60820/ | 20:09 |
morganfainberg | bknudson, https://review.openstack.org/#/c/77404/ strings fixed, added a bunch of punctuation | 20:16 |
bknudson | a bunch!!!! | 20:16 |
morganfainberg | yeah, now all our helpstrings end with proper punctuation | 20:16 |
morganfainberg | should have made them all ! or even better ? | 20:17 |
morganfainberg | really confuse people | 20:17 |
*** henrynash has joined #openstack-keystone | 20:19 | |
ayoung | dolphm, so...zuul is getting slow again. I don't plan on posting another revision of the revoke patch | 20:21 |
ayoung | I think that what we have (assuming it passes check and would pass gate) is what we are going to get | 20:21 |
dolphm | ayoung: ack, that's what i'm hoping | 20:26 |
dolphm | ayoung: i'm reviewing now until the next openstack meeting | 20:26 |
dolphm | ayoung: TC is meeting about barbican right now though | 20:26 |
dolphm | if anyone has free review bandwidth: https://review.openstack.org/#/c/75727/ | 20:29 |
dolphm | bknudson: the second patchset in that sequence failed jenkins ^ | 20:29 |
lbragstad | sure, I was going to follow up on that one, and I'm waiting to give dstanek a hand | 20:30 |
bknudson | dolphm: I'll change it to address topol's comment | 20:31 |
dolphm | bknudson: rechecked (bug 1284371) | 20:31 |
dolphm | ayoung: i thought you weren't including the extension by default? | 20:32 |
ayoung | dolphm, its in the paste but not in the pipeline | 20:33 |
dolphm | ayoung: it's in the pipeline | 20:33 |
ayoung | ? | 20:33 |
ayoung | https://review.openstack.org/#/c/55908/82/etc/keystone-paste.ini | 20:35 |
ayoung | only change to the paste is to create the filter | 20:36 |
dolphm | ayoung: wtf, maybe i was reviewing an old patchset | 20:36 |
dolphm | ayoung: i went back to get a link and it was gone | 20:36 |
dolphm | ayoung: yep... i have draft comments on patchset 77 | 20:36 |
* dolphm starts over | 20:36 | |
*** topol has quit IRC | 20:36 | |
ayoung | dolphm, post them as is and I will look | 20:36 |
dolphm | ayoung: no bother, i had only made it through a few files | 20:36 |
ayoung | k | 20:36 |
dolphm | did that object?.maybe_call_me() syntax ever land in java? | 20:37 |
dolphm | dstanek: lbragstad: https://review.openstack.org/#/c/77950/ got a -2 ... not sure i understand | 20:43 |
*** finite has quit IRC | 20:43 | |
lbragstad | dolphm: I think what Joe means here is that since '# This is a comment' is a pep8 style "guideline" it should live in the pep8tool | 20:44 |
lbragstad | and not be carried in the openstack-dev/hacking project | 20:44 |
lbragstad | when it could be made upstream | 20:44 |
*** krsna has quit IRC | 20:45 | |
*** derek_c has joined #openstack-keystone | 20:47 | |
*** derek_c has quit IRC | 20:48 | |
*** derek_c has joined #openstack-keystone | 20:48 | |
ayoung | bknudson, so we are just going to add enabled into the endpoint, right? We won't be changing the API to not return disabled endpoints? | 20:49 |
bknudson | ayoung: the fix to return only enabled endpoints in the catalog is https://review.openstack.org/#/c/77441/ | 20:50 |
ayoung | bknudson, only the SQL backend? | 20:50 |
ayoung | ah..in the earlier patch | 20:51 |
bknudson | ayoung: I looked at the other backends and those ones didn't make sense to have disabled endpoints | 20:51 |
ayoung | yeah, saw the comment | 20:51 |
bknudson | e.g., with the templated one... you just wouldn't put the endpoint there if it was disabled. | 20:51 |
bknudson | ayoung: so the thing that's missing is disabled services. | 20:52 |
ayoung | that is OK. | 20:52 |
bknudson | I wanted to get some feedback on the way that I'm doing endpoints | 20:52 |
ayoung | I would almost argue that a service should not be disabled | 20:52 |
ayoung | just endpoints | 20:52 |
bknudson | the changes to handle services are essentially the same. | 20:52 |
ayoung | bknudson, so, say an endpoint is disabled, what would the workflow be like to reenable it | 20:57 |
dstanek | dolphm, lbragstad: that's silly; i have this check it my current batch | 20:57 |
dstanek | dolphm, lbragstad: we can decide where it goes from there | 20:57 |
ayoung | list endpoints will still show it, it just won't show up in the catalog? | 20:58 |
lbragstad | dstanek: dolphm looks like we will have to carry this in Keystone then? | 20:58 |
bknudson | ayoung: you can enable an endpoint with { 'endpoint': { 'enabled': True } } | 20:58 |
ayoung | so list the endpoiinbt and update | 20:58 |
dolphm | lbragstad: i'm okay with that | 20:58 |
ayoung | yeah, I meant finding it | 20:58 |
bknudson | ayoung: yes, it still shows up in the endpoints list. | 20:58 |
ayoung | cool | 20:58 |
bknudson | it's only the token catalog is affected. | 20:58 |
bknudson | so if you validate your token maybe you'll get a different catalog! | 20:58 |
lbragstad | nova has a bunch of their own check s | 20:59 |
dstanek | lbragstad: maybe; i was putting in keystone because i don't understand what they want | 20:59 |
ayoung | should endpoint = self.get_endpoint(entry.endpoint_id) be endpoint = self.get_endpoint(entry.endpoint_id, enabled=True) in the future? | 20:59 |
lbragstad | they == openstack-dev/hacking or pep8tool? | 20:59 |
ayoung | nah, nbevermind | 20:59 |
bknudson | ayoung: is this in the contrib OS-EP-FILTER? | 21:00 |
ayoung | I was thinking in the list itself | 21:00 |
ayoung | bknudson, yeah | 21:00 |
dstanek | lbragstad: hacking | 21:00 |
ayoung | get_endpoints... | 21:00 |
bknudson | ayoung: I didn't touch it too much since I wasn't really sure what OS-EP-FILTER was doing... | 21:00 |
ayoung | bknudson, no, what you did looks right to me | 21:00 |
bknudson | especially when it turned out that none of that code was tested. | 21:01 |
ayoung | I was just thinking about disable in general, as we have that for many entities now | 21:01 |
bknudson | it would be interesting to see if other entities store enabled in the extras column. | 21:01 |
*** stevemar has quit IRC | 21:02 | |
ayoung | bknudson, that whole patch series has 2 +2s on it | 21:02 |
ayoung | bknudson, we've pulled a few of them out | 21:02 |
ayoung | project and domain are not | 21:03 |
ayoung | but I thik that project was origianlly (tenant) | 21:03 |
ayoung | bknudson, domain, projet an d user all have explicit enabled fields, I just checked | 21:05 |
bknudson | so not service | 21:05 |
ayoung | yeha, neither of the service catalog entities in master | 21:05 |
bknudson | /v3/services says enabled (boolean) , so I assume it can actually be anything. | 21:06 |
lbragstad | dstanek: do you have a check for vim headers? | 21:06 |
bknudson | and disabling a service has no effect. | 21:06 |
dstanek | lbragstad: nope | 21:06 |
dstanek | lbragstad: that would be a great addition though | 21:07 |
lbragstad | dstanek: ok, yeah I think so too | 21:07 |
bknudson | ayoung: I think services is the only one that has enabled in extras now. | 21:08 |
ayoung | Juno for that. | 21:08 |
ayoung | we are going to be doing a bit with the service catalog, to include endpoint binding of tokens and service/endpoint specific roles | 21:09 |
ayoung | RBAC and Catalog should be one summit track | 21:09 |
dolphm | bknudson: ++ | 21:16 |
*** marekd is now known as marekd|away | 21:20 | |
derek_c | when running openstackclient, by that I mean when typing "openstack" in terminal, I get a "ERROR: openstackclient.shell Exception raised: Authorization failed: The resource could not be found. (HTTP 404)". Does anyone have a clue what's going on? | 21:23 |
derek_c | I suspect it has to do with me setting OS_AUTH_URL to a v2.0 API? | 21:23 |
derek_c | I mean URL | 21:23 |
bknudson | derek_c: there's also an option / env var for the identity api version. | 21:24 |
derek_c | bknudson: I have set OS_IDENTITY_API_VERSION to 3 | 21:25 |
bknudson | derek_c: then you'll need to have the OS_AUTH_URL set to a v3 endpoint. | 21:25 |
derek_c | bknudson: how do I find out the correct endpoint? | 21:25 |
derek_c | I tried /v3.0 and /v3 | 21:26 |
bknudson | derek_c: do a GET on the base URL and the result should include the versioned endpoints. | 21:26 |
lbragstad | dstanek: I assume the Keystone specific hacking stuff is going to live in /keystone/hacking/ , right | 21:28 |
lbragstad | ? | 21:28 |
derek_c | bknudson: ah, it worked! | 21:28 |
derek_c | bknudson: thanks a lot :) | 21:28 |
bknudson | derek_c: no problem. | 21:28 |
morganfainberg | lbragstad, keystone.hacking? keystone.tests.hacking? | 21:32 |
*** stevemar has joined #openstack-keystone | 21:34 | |
*** ChanServ sets mode: +v stevemar | 21:34 | |
morganfainberg | bknudson, dolphm, ayoung, ok so infra consensus, no periodic job to update sample.conf, moving to a non-voting check job that will say "sample config is out of date" | 21:38 |
ayoung | morganfainberg, meh | 21:39 |
morganfainberg | dolphm, bknudson, ayoung, next step will be to make it part of doc build and release building and evict it from git | 21:39 |
morganfainberg | no pep8 checks | 21:39 |
lbragstad | morganfainberg: dstanek and I are looking to add hacking checks | 21:39 |
morganfainberg | lbragstad, nod, i was offering other options | 21:39 |
lbragstad | morganfainberg: ahh, right | 21:40 |
morganfainberg | so sample config will never exist in the git tree, will be always built. | 21:40 |
bknudson | morganfainberg: I think that will work better. | 21:40 |
morganfainberg | ayoung, it's the direction the infra team wants to go. and if we can get there it solves the same one. #1 goal - no pep8, | 21:40 |
dstanek | morganfainberg: it'll be in keystone.hacking | 21:40 |
bknudson | I'm worried about the config file changes when the oslo libraries are updated. | 21:40 |
morganfainberg | i would prefer it being in git tree, but that is personal preference. | 21:40 |
dolphm | morganfainberg: hmm, what was the reasoning against a periodic job? | 21:40 |
morganfainberg | dolphm, we control the artifacts. it can be built at install time | 21:41 |
dolphm | morganfainberg: wait, no sample config in tree? | 21:41 |
ayoung | ++ | 21:41 |
morganfainberg | dolphm, make it something we build just like docs...it really is a doc | 21:41 |
morganfainberg | dolphm, yep, no sample config in the git tree, it would be built like docs are built and at package/setup time | 21:41 |
dolphm | morganfainberg: i want it to be published, and i want it to be easy to copy it and modify it to my liking; where is it going to be published? | 21:42 |
morganfainberg | dolphm, longer term, with the documents | 21:42 |
morganfainberg | dolphm, published with each merge and each release | 21:43 |
morganfainberg | i would keep the generator in tox | 21:43 |
morganfainberg | dolphm, i don't see why a specific project can't keep it in tree as well | 21:43 |
*** arunkant has quit IRC | 21:43 | |
morganfainberg | but it wont be gated upon being "up to date" | 21:43 |
morganfainberg | can't do that reasonably with oslo libraries (~20 or so targeted for juno) | 21:44 |
bknudson | maybe oslo libraries will change how they do config | 21:44 |
morganfainberg | but initially (today) we will get a non-voting check job that will tell us "hey this is not up to date" | 21:45 |
morganfainberg | bknudson, ++ i would like that. | 21:45 |
morganfainberg | bknudson, but dunno how likely that will be | 21:45 |
bknudson | morganfainberg: we just need to push the changes. | 21:45 |
morganfainberg | bknudson, yeah, there are some minor ones i want to see first, but not sure what they should be doing for config | 21:46 |
dolphm | morganfainberg: alright i'm confused now... if the sample conf is not in-tree, then what's not up to date? | 21:46 |
morganfainberg | dolphm, initially it will be in tree | 21:46 |
morganfainberg | dolphm, then we will move it to docs and out of tree at that point | 21:46 |
dolphm | morganfainberg: openstack-manuals? or keystone/docs ? | 21:46 |
morganfainberg | dolphm, openstack-manuals | 21:46 |
morganfainberg | dolphm, and packaging time would stick it in keystone/etc | 21:47 |
morganfainberg | so it's there for deployment | 21:47 |
*** achampion has quit IRC | 21:47 | |
morganfainberg | dolphm, 3 phases. 1: check job that says "Hey in-tree is out of date", 2: openstack-manuals publishing and setup/package time building, 3: evict check-job and git (if project desires) | 21:48 |
morganfainberg | the pep8 check would go away in the first phase | 21:49 |
dolphm | morganfainberg: alright, so 1) will fail when i change a config opt, alerting reviewers to the fact that i should include a revised sample conf with my patch? | 21:49 |
morganfainberg | dolphm, correct. it wont be voting, but it will show in the check run from jenkins that says it's out of date | 21:50 |
morganfainberg | just like a py33 fail. sure it failes but it wouldn't block the review | 21:50 |
dolphm | bknudson: speaking of, check_uptodate.sh failed on https://jenkins06.openstack.org/job/gate-keystone-pep8/445/console | 21:51 |
bknudson | "kombu_reconnect_delay=1.0" | 21:52 |
dolphm | bknudson: yep.. | 21:52 |
bknudson | wtf! | 21:52 |
dolphm | morganfainberg: alright i'm in favor. | 21:52 |
bknudson | that was quick | 21:52 |
*** topol has joined #openstack-keystone | 21:54 | |
bknudson | I'll see if I can recreate the pep8 failure. | 21:54 |
bknudson | I would expect this to cause everything to fail | 21:56 |
bknudson | so... how about disabling the pep8 sample config check? | 21:56 |
bknudson | was able to recreate with a tox -r -e pep8 | 21:57 |
*** gyee has joined #openstack-keystone | 21:58 | |
ayoung | morganfainberg, dstanek can you guys give at least a +1 to https://review.openstack.org/#/c/55908/ It feels unloved | 21:58 |
bknudson | https://review.openstack.org/#/c/78024/ -- updated sample config file | 22:00 |
ayoung | bknudson, +2 | 22:01 |
*** marcoemorais has quit IRC | 22:02 | |
*** marcoemorais has joined #openstack-keystone | 22:02 | |
morganfainberg | ayoung, 82! | 22:04 |
ayoung | So long as it doesn't go to 86. There is no return from 86. Don't even try. | 22:05 |
ayoung | dolphm, can we squeeze this one in: https://review.openstack.org/#/c/47441/ | 22:06 |
ayoung | its a nice performace fix...could go in to RC | 22:07 |
dolphm | ayoung: commit message sounds great - but i defer to bknudson | 22:08 |
ayoung | sounds good... | 22:08 |
*** krsna has joined #openstack-keystone | 22:08 | |
morganfainberg | ayoung, and if it hits 88, we're uhm going back in time | 22:09 |
ayoung | We'll actually commit patch 57? | 22:10 |
morganfainberg | 55 | 22:10 |
dolphm | 82 gets denser and denser as you go | 22:10 |
dolphm | i'm like 45% through, all nits so far, making coffee | 22:11 |
dolphm | there's also dead code in the manager | 22:11 |
*** achampion has joined #openstack-keystone | 22:11 | |
*** marcoemorais has quit IRC | 22:15 | |
stevemar | morganfainberg, poke | 22:16 |
morganfainberg | stevemar, ouch! | 22:16 |
morganfainberg | stevemar, my eye | 22:16 |
stevemar | morganfainberg, i'm forwarding a poke request: https://review.openstack.org/#/c/77294/ | 22:16 |
morganfainberg | i saw openstack-dev :P | 22:17 |
stevemar | :) | 22:17 |
ayoung | dolphm, I'll be explaining this code to people for years, I am afraid | 22:18 |
morganfainberg | ayoung, nah, likely dolph steve and i will explain it wrong... then you'll correct us | 22:19 |
morganfainberg | ok ok, stevemar and i will explain it wrong | 22:19 |
* stevemar slowly walks away | 22:20 | |
morganfainberg | stevemar, playing the hulk theme...sadly walking away? | 22:20 |
morganfainberg | stevemar, http://www.youtube.com/watch?v=uq0HTOGtfSw | 22:21 |
stevemar | morganfainberg, as long as you're the one explaining revocation, you can play whatever theme you want | 22:21 |
morganfainberg | stevemar, i can probably explain it now actually | 22:21 |
morganfainberg | stevemar, i think some general code restructuring in Juno will make it easier to follow, but it's not that bad | 22:22 |
morganfainberg | stevemar, it's just a massive code dump, which makes it hard to review | 22:22 |
stevemar | morganfainberg, i can get through most of the stuff in contrib, but after that it gets a bit dicey | 22:22 |
morganfainberg | stevemar, i've been lurking in revocations, tokens, etc for a while now...and changing assignment etc. | 22:23 |
morganfainberg | i think... | 22:23 |
morganfainberg | it's warped my brain | 22:23 |
ayoung | stevemar, something happens, and we immediately tell the contrib code "revoke tokens that have this property" or "these couple properties" | 22:25 |
ayoung | then, when a new token request comes in, check it against those rules to see if it revoked | 22:25 |
ayoung | Ideally , we would do all of the revocations based on events, but adding in the new ones right now is just to invasive | 22:26 |
dstanek | ayoung: i'll walk through it again now | 22:26 |
dstanek | ayoung: did you ever do a test to load a few thousand revoked tokens in memory? | 22:27 |
ayoung | the part that is trickiest is the "revoke by tree" | 22:27 |
ayoung | dstanek, nope... | 22:27 |
ayoung | dstanek, I'd need to somehow keep them from all being related | 22:27 |
ayoung | like 10K different users each with one | 22:27 |
ayoung | or a handful | 22:28 |
ayoung | def something for tempes | 22:28 |
ayoung | t | 22:28 |
*** devlaps1 has joined #openstack-keystone | 22:28 | |
*** devlaps has quit IRC | 22:29 | |
*** leseb has quit IRC | 22:32 | |
* dolphm coffee took forever but is a magnificent success; attempting to apply extraneous success to token revocation | 22:33 | |
bknudson | https://review.openstack.org/#/c/78030/ -- based on change in nova https://review.openstack.org/#/c/78028/ | 22:35 |
*** achampion has quit IRC | 22:39 | |
dolphm | bknudson: +2 | 22:43 |
dolphm | morganfainberg: ^ | 22:43 |
*** lbragstad has quit IRC | 22:43 | |
dolphm | ayoung: ^ | 22:43 |
dolphm | stevemar: where is the oauth manager instantiated? | 22:44 |
stevemar | in the dependency stuff, 1 sec | 22:44 |
ayoung | +2 | 22:48 |
stevemar | dolphm, https://github.com/openstack/keystone/blob/master/keystone/contrib/oauth1/core.py#L144 | 22:49 |
dolphm | stevemar: but what instantiates an instance of that class? | 22:50 |
stevemar | dolphm, magic at the dependency layer | 22:51 |
dolphm | stevemar: something calls oauth1.Manager() so that it's configured and made available... | 22:51 |
stevemar | dolphm, https://github.com/openstack/keystone/blob/master/keystone/common/dependency.py#L190 | 22:52 |
dolphm | stevemar: it's not done where other managers are done to avoid creating a dep on oauth1lib | 22:52 |
stevemar | correct | 22:52 |
*** dims has quit IRC | 22:52 | |
morganfainberg | dolphm, +2 | 22:54 |
stevemar | dolphm, https://review.openstack.org/#/c/64349/ | 22:55 |
dolphm | stevemar: hrm... | 22:56 |
dolphm | stevemar: revoke should ideally do... whatever that's doing.. too. | 22:56 |
dolphm | stevemar: and yeah, that's magic to me | 22:56 |
stevemar | bknudson might be able to shed more light on the magic | 22:56 |
stevemar | dolphm, ^ | 22:56 |
bknudson | something has to call oauth1.Manager() | 22:57 |
bknudson | stevemar: when that happens then the instance will be added to the dependency map. | 22:57 |
bknudson | and it'll be injected into any other objects that require it. | 22:57 |
bknudson | keystone/auth/plugins/oauth1.py: self.oauth_api = oauth1.Manager() | 22:58 |
bknudson | keystone/contrib/oauth1/routers.py: oauth1.Manager() | 22:58 |
bknudson | that's probably where it comes from. | 22:59 |
bknudson | if you add the middleware to the paste pipeline it'll eventually create the router. | 22:59 |
bknudson | the constructor really shouldn't be called multiple times... | 22:59 |
bknudson | e.g., why keystone/contrib/oauth1/validator.py: self.oauth_api = oauth1.Manager() ? | 22:59 |
dolphm | bknudson: i'm now wondering if that's a necessary bug ^ | 23:01 |
dolphm | the "magic" | 23:01 |
bknudson | dolphm: if it is then there should be a comment. | 23:02 |
bknudson | I assume there's no need to an oauthvalidator if the routes aren't there. | 23:02 |
dolphm | bknudson: i bet if you delete that then you can't spin up oauth | 23:03 |
dolphm | (outside of tests) | 23:03 |
bknudson | I have to run to bowling. | 23:03 |
*** marcoemorais has joined #openstack-keystone | 23:04 | |
*** arborism has joined #openstack-keystone | 23:05 | |
*** amcrn has quit IRC | 23:06 | |
*** dims has joined #openstack-keystone | 23:08 | |
*** marcoemorais has quit IRC | 23:09 | |
dolphm | ayoung: +1 for the extra assertions in test_v3_auth -- first time i've seen them | 23:09 |
ayoung | dolphm, need more. | 23:11 |
dolphm | ayoung: they're waaaay stronger than before though | 23:11 |
*** dstanek has quit IRC | 23:12 | |
dolphm | ayoung: more importantly, i could draw up what the response looks like for the most part, although i'd have to guess at the datetime format, etc | 23:12 |
*** marcoemorais has joined #openstack-keystone | 23:12 | |
dolphm | ayoung: on the 'disable a domain' test for example, i'd like to see the domain re-enabled, ensuring that the event still exists, and then re-disabling the domain should revise the event | 23:13 |
ayoung | dolphm, so...the event only handles revoking things that happen in the past | 23:15 |
ayoung | now, tests happen artificially fast | 23:15 |
dolphm | ayoung: you can control time with mock + timeutils | 23:16 |
ayoung | but, re-enable means you get viable tokens again, and the revoke event should not match | 23:16 |
ayoung | and then a second disable should just update the time, | 23:16 |
dolphm | ayoung: exactly - i don't see that tested (?) | 23:16 |
ayoung | so, two events, but still just one path through the tree, and it will catch all of the the events | 23:16 |
dolphm | or i just haven't gotten to it yet | 23:16 |
ayoung | not on domain...its more in testing the tree structure itself | 23:17 |
ayoung | but...yeah, more testing better | 23:17 |
ayoung | but " domain re-enabled, ensuring that the event still exists" you mean "renable and new tokens should get through but old tokens should still be revoked" | 23:18 |
ayoung | that is a good test | 23:18 |
dolphm | ayoung: correct | 23:18 |
ayoung | dolphm, can we do that as a follow on? The more I think about this, the more testing I'd like to do across the board | 23:19 |
dolphm | ayoung: yeah | 23:20 |
dolphm | ayoung: if you're confident it would pass if we wrote it | 23:20 |
ayoung | dolphm, it is accounted for in the design and tested in other ways | 23:23 |
dolphm | ayoung: pop quiz what does revoke_api.check_token() do if the token is invalid? | 23:23 |
ayoung | dolphm, it would probably 500 | 23:24 |
dolphm | ayoung: i mean as in revoked | 23:25 |
ayoung | it takes a map, and assumes the values in there have been popleated | 23:25 |
dolphm | ayoung: i don't care about the impl, what's the interface? | 23:25 |
ayoung | dolphm, if a token is revoked, that means that there is an entry wiuth user and expires at that will match the token, and it will not pass | 23:25 |
ayoung | specifically | 23:25 |
ayoung | model is_revoke returns true | 23:26 |
ayoung | then in core | 23:26 |
ayoung | https://review.openstack.org/#/c/55908/82/keystone/contrib/revoke/core.py rasises TokenNotFound | 23:26 |
dolphm | ayoung: okay you killed me i'm going to go make dinner | 23:26 |
dolphm | ayoung: https://review.openstack.org/#/c/55908/ | 23:27 |
ayoung | thanks | 23:27 |
*** david-lyle has quit IRC | 23:34 | |
morganfainberg | dolphm, ayoung https://review.openstack.org/#/c/78030/ pre-emptively approving | 23:38 |
morganfainberg | dolphm, ayoung, since we know we want it and it should start gating asap | 23:38 |
*** thedodd has quit IRC | 23:39 | |
stevemar | ayoung, it's lookin good for you | 23:53 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!