Comment 22 for bug 1354208

Revision history for this message
Brant Knudson (blk-u) wrote : Re: Catalog replacement allows reading config

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:
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).

- test
  - passed, get a 501 now