I'd prefer it if tenant_id and user_id weren't in the whitelist config option but were always allowed. These are values set by keystone and not from the config file, so the help text is kind of confusing (it says the values are in the config file) and a user might think they can remove those values, but it's probably going to break just about every deployment if they do.
Some minor suggestions on the patches:
1) It would be nice if the list in the default value for endpoint_substitution_whitelist was alphabetized.
2) The help text for the option should say that it's deprecated since config replacement will be removed in Juno.
3) I'd assume that WhiteListedFormatter does formatting, but it doesn't, so rename to FilterWhiteListedItems or something.
4) The sample config file should be updated.
- test
- passed, no endpoint with admin token.
icehouse patch in comment #16:
- tox -e py27,pep8
- I had to add oslo.utils to my venv for this to work... maybe we have to wait for a change to stable/icehouse for this?
- check backport
- looks good.
- test
- passed... get a 501 error now.
havana patch in comment #17:
- tox -e py27,pep8
- fails, right at the beginning: None ERROR 0.00
======================================================================
ERROR: Failure: ImportError (cannot import name fixture)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/opt/stack/keystone/.tox/py27/local/lib/python2.7/site-packages/nose/loader.py", line 414, in loadTestsFromName
addr.filename, addr.module)
File "/opt/stack/keystone/.tox/py27/local/lib/python2.7/site-packages/nose/importer.py", line 47, in importFromPath
return self.importFromDir(dir_path, fqname)
File "/opt/stack/keystone/.tox/py27/local/lib/python2.7/site-packages/nose/importer.py", line 94, in importFromDir
mod = load_module(part_fqname, fh, filename, desc)
File "/opt/stack/keystone/keystone/tests/unit/catalog/test_core.py", line 13, in <module>
from oslo.config import fixture as config_fixture
ImportError: cannot import name fixture
-- this change (and the icehouse one?) should use keystone/openstack/common/fixture/config.py instead.
- check backport (generally looks correct, but the tests failed so didn't look too closely).
master patch in comment #13:
- tox -e py27,pep8
- passed
- visual inspection
I'd prefer it if tenant_id and user_id weren't in the whitelist config option but were always allowed. These are values set by keystone and not from the config file, so the help text is kind of confusing (it says the values are in the config file) and a user might think they can remove those values, but it's probably going to break just about every deployment if they do.
Some minor suggestions on the patches: substitution_ whitelist was alphabetized. atter does formatting, but it doesn't, so rename to FilterWhiteList edItems or something.
1) It would be nice if the list in the default value for endpoint_
2) The help text for the option should say that it's deprecated since config replacement will be removed in Juno.
3) I'd assume that WhiteListedForm
4) The sample config file should be updated.
- test
- passed, no endpoint with admin token.
icehouse patch in comment #16:
- tox -e py27,pep8
- I had to add oslo.utils to my venv for this to work... maybe we have to wait for a change to stable/icehouse for this?
- check backport
- looks good.
- test
- passed... get a 501 error now.
havana patch in comment #17:
- tox -e py27,pep8
- fails, right at the beginning: None ERROR 0.00
======= ======= ======= ======= ======= ======= ======= ======= ======= ======= ------- ------- ------- ------- ------- ------- ------- ------- ------- keystone/ .tox/py27/ local/lib/ python2. 7/site- packages/ nose/loader. py", line 414, in loadTestsFromName keystone/ .tox/py27/ local/lib/ python2. 7/site- packages/ nose/importer. py", line 47, in importFromPath Dir(dir_ path, fqname) keystone/ .tox/py27/ local/lib/ python2. 7/site- packages/ nose/importer. py", line 94, in importFromDir part_fqname, fh, filename, desc) keystone/ keystone/ tests/unit/ catalog/ test_core. py", line 13, in <module>
ERROR: Failure: ImportError (cannot import name fixture)
-------
Traceback (most recent call last):
File "/opt/stack/
addr.filename, addr.module)
File "/opt/stack/
return self.importFrom
File "/opt/stack/
mod = load_module(
File "/opt/stack/
from oslo.config import fixture as config_fixture
ImportError: cannot import name fixture
-- this change (and the icehouse one?) should use keystone/ openstack/ common/ fixture/ config. py instead.
- check backport (generally looks correct, but the tests failed so didn't look too closely).
- test
- passed, get a 501 now