Comment 20 for bug 1206254

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (master)

Reviewed: https://review.openstack.org/43768
Committed: http://github.com/openstack/keystone/commit/4e6cf36697eeb02a420101e5e0be67072a5c9583
Submitter: Jenkins
Branch: master

commit 4e6cf36697eeb02a420101e5e0be67072a5c9583
Author: John Dennis <email address hidden>
Date: Mon Aug 12 14:53:52 2013 -0400

    Utilities to create directores, set ownership & permissions

    Add the get_unix_user(), get_unix_group(), set_permissions(),
    make_dirs() utilities to keystone/common/utils.py. These functions are
    much more robust and flexible. Ownership can be specifed either with
    symbolic names or explicit id's. Any value you do not want to override
    can be passed as None and it will be skipped.

    The BaseCertificateConfigure class in openssl.py had it's own code to
    create directories, set ownership, set permissions, etc. which was
    used for setting up certificates, key files, etc. Properly handling
    file system objects should not be a property of openssl, rather these
    should be general utilities which can be used in a variety of
    locations. Plus the openssl versions were not fully general and had
    some pecular limitations. The new utilies will be used in a subsequent
    patch which also needs this functionality but isn't in openssl code.

    Previously the openssl code was inconsistent as to whether it set the
    ownership and permissions, sometimes ownership and permission was set
    only if the file/dir did not exist, other times the ownership and
    permission were set irregardless of prior existence. The modified code
    now always sets ownerhip and permissions (if provided).

    The prior code assumed an id (uid/gid) was being passed for ownership,
    but this was not documented. The code appears to take a symbolic
    name. The cli code converted symbolic names passed on the command line
    to id's before instantiating a class which accepts ownership
    information. If you instantiated one of the class without first
    converting a name to an id it would throw an exception. The new
    utilities accept either symbolic names or id's and since these
    utilities are now used internally you can pass either a name or id and
    one is no longer dependent upon special handling inside cli argument
    parsing code to get correct behavior with these classes.

    There were several places with code like this:

        os.chown(file_name, self.use_keystone_user or -1,
                 self.use_keystone_group or -1)

    The intent would appear to be to avoid setting the uid or gid if the
    attribute had a value of None. Except this fails completely if the
    uid or gid belongs to root because those id's are zero, thus the code
    would never work with root id's. The new utilities explicitly test for
    a value of None and do not rely on the weak "truth test".

    Note: these new utilities are intended to be moved into oslo-incubator
    in the near future.

    Fixes: bug #1206254

    Change-Id: I34b811513ebfd48d91600051724949c0a96c0734