[virt-tools-list] [virt-bootstrap] [PATCH v4 05/13] Add remapping ownership of files in rootfs

Cedric Bosdonnat cbosdonnat at suse.com
Fri Jul 21 13:34:00 UTC 2017


On Fri, 2017-07-21 at 13:13 +0100, Radostin Stoyanov wrote:
> When Libvirt creates LXC container with enabled user namespace the
> ownership of files in the container should be mapped to the specified
> target UID/GID.
> 
> The implementation of the mapping is inspired by the tool uidmapshift:
> http://bazaar.launchpad.net/%7Eserge-hallyn/+junk/nsexec/view/head:/uidmapshift.c

This mention should also go above the corresponding code with a
note mentioning the original GPLv2 license.

> Mapping values can be specified with the flags:
> 
>     --idmap    Map both UIDs/GIDs
>     --uidmap   Map UIDs
>     --gidmap   Map GIDs
> 
> Each of these flags can be specified multiple times.
> 
> Example:
> 
>     virt-bootstrap docker://fedora /tmp/foo --uidmap 0:1000:10 --gidmap 0:1000:10
> 
> Will map the ownership of files with UIDs/GIDs: 0-9 to 1000-1009
> 
> The same result can be achived with:
> 
>     virt-bootstrap docker://fedora /tmp/foo --idmap 0:1000:10
> 
> Multiple mapping values can be specified as follows:
> 
>     virt_bootstrap.py docker://ubuntu /tmp/foo --idmap 0:1000:10 --idmap 500:1500:10
> 
> This will map the UID/GIDs: 0-9 to 1000-1009 and 500-509 to 1500-1509

Nice a complete commit message: thanks!

> ---
>  src/virtBootstrap/virt_bootstrap.py | 119 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 114 insertions(+), 5 deletions(-)
> 
> diff --git a/src/virtBootstrap/virt_bootstrap.py b/src/virtBootstrap/virt_bootstrap.py
> index b444b32..9ad35ce 100755
> --- a/src/virtBootstrap/virt_bootstrap.py
> +++ b/src/virtBootstrap/virt_bootstrap.py
> @@ -69,12 +69,100 @@ def get_source(source_type):
>          raise Exception("Invalid image URL scheme: '%s'" % source_type)
>  
>  

Add inspiration + license notice here.

> +def get_map_id(old_id, opts):
> +    """
> +    Calculate new map_id.
> +    """
> +    if old_id >= opts['first'] and old_id < opts['last']:
> +        return old_id + opts['offset']
> +    return -1
> +
> +
> +def parse_idmap(idmap):
> +    """
> +    Parse user input to 'start', 'target' and 'count' values.
> +
> +    Example:
> +        ['0:1000:10', '500:500:10']
> +    is converted to:
> +        [[0, 1000, 10], [500, 500, 10]]
> +    """
> +    if not isinstance(idmap, list) or idmap == []:
> +        return None
> +    try:
> +        idmap_list = []
> +        for x in idmap:
> +            start, target, count = x.split(':')
> +            idmap_list.append([int(start), int(target), int(count)])
> +
> +        return idmap_list
> +    except Exception:
> +        raise ValueError("Invalid UID/GID mapping value: %s" % idmap)
> +
> +
> +def get_mapping_opts(mapping):
> +    """
> +    Get range options from UID/GID mapping
> +    """
> +    start = mapping[0] if mapping[0] > -1 else 0
> +    target = mapping[1] if mapping[1] > -1 else 0
> +    count = mapping[2] if mapping[2] > -1 else 1
> +
> +    opts = {}
> +    opts['first'] = start
> +    opts['last'] = start + count
> +    opts['offset'] = target - start

Wouldn't this be more readable / beautiful?

       opts = {
           'first' : start,
           'last'  : start + count,
           'offset': target - start
       }

> +    return opts
> +
> +
> +def map_id(path, map_uid, map_gid):
> +    """
> +    Remapping ownership of all files inside a container's rootfs.
> +
> +    map_gid and map_uid: Contain integers in a list of lists with format:
> +        [[<start>, <target>, <count>], ...]

Looks like this comment is wrong. Unless I'm mistaken these contain just 
[<start>, <target>, <count>]

> +    """
> +    if map_uid:
> +        uid_opts = get_mapping_opts(map_uid)
> +    if map_gid:
> +        gid_opts = get_mapping_opts(map_gid)
> +
> +    for root, _ignore, files in os.walk(os.path.realpath(path)):
> +        for name in [root] + files:
> +            file_path = os.path.join(root, name)
> +
> +            stat_info = os.lstat(file_path)
> +            old_uid = stat_info.st_uid
> +            old_gid = stat_info.st_gid
> +
> +            new_uid = get_map_id(old_uid, uid_opts) if map_uid else -1
> +            new_gid = get_map_id(old_gid, gid_opts) if map_gid else -1
> +            os.lchown(file_path, new_uid, new_gid)
> +
> +
> +def mapping_uid_gid(dest, uid_map, gid_map):
> +    """
> +    Mapping ownership for each uid_map
> +    """
> +    len_diff = len(uid_map) - len(gid_map)
> +
> +    if len_diff < 0:
> +        uid_map += [None] * abs(len_diff)
> +    elif len_diff > 0:
> +        gid_map += [None] * len_diff
> +
> +    for uid, gid in zip(uid_map, gid_map):
> +        map_id(dest, uid, gid)
> +
> +
>  # pylint: disable=too-many-arguments
>  def bootstrap(uri, dest,
>                fmt='dir',
>                username=None,
>                password=None,
>                root_password=None,
> +              uid_map=None,
> +              gid_map=None,
>                not_secure=False,
>                no_cache=False,
>                progress_cb=None):
> @@ -104,9 +192,14 @@ def bootstrap(uri, dest,
>             no_cache=no_cache,
>             progress=prog).unpack(dest)
>  
> -    if fmt == "dir" and root_password is not None:
> -        logger.info("Setting password of the root account")
> -        utils.set_root_password(dest, root_password)
> +    if fmt == "dir":
> +        if root_password is not None:
> +            logger.info("Setting password of the root account")
> +            utils.set_root_password(dest, root_password)
> +
> +        if uid_map or gid_map:
> +            logger.info("Mapping UID/GID")
> +            mapping_uid_gid(dest, uid_map, gid_map)
>  
>  
>  def set_logging_conf(loglevel=None):
> @@ -157,6 +250,15 @@ def main():
>                          help=_("Password for accessing the source registry"))
>      parser.add_argument("--root-password", default=None,
>                          help=_("Root password to set in the created rootfs"))
> +    parser.add_argument("--uidmap", default=None, action='append',
> +                        metavar="<start>:<target>:<count>",
> +                        help=_("Map UIDs"))
> +    parser.add_argument("--gidmap", default=None, action='append',
> +                        metavar="<start>:<target>:<count>",
> +                        help=_("Map GIDs"))
> +    parser.add_argument("--idmap", default=None, action='append',
> +                        metavar="<start>:<target>:<count>",
> +                        help=_("Map both UIDs/GIDs"))
>      parser.add_argument("--no-cache", action="store_true",
>                          help=_("Do not store downloaded Docker images"))
>      parser.add_argument("-f", "--format", default='dir',
> @@ -173,8 +275,6 @@ def main():
>                          help=_("Show only the current status and progress"
>                                 "of virt-bootstrap"))
>  
> -    # TODO add UID / GID mapping parameters
> -
>      try:
>          args = parser.parse_args()
>  
> @@ -182,6 +282,13 @@ def main():
>              # Configure logging lovel/format
>              set_logging_conf(args.loglevel)
>  
> +        uid_map = parse_idmap(args.idmap) if args.idmap else []
> +        gid_map = list(uid_map) if args.idmap else []
> +        if args.uidmap:
> +            uid_map += parse_idmap(args.uidmap)
> +        if args.gidmap:
> +            gid_map += parse_idmap(args.gidmap)
> +
>          # do the job here!
>          bootstrap(uri=args.uri,
>                    dest=args.dest,
> @@ -189,6 +296,8 @@ def main():
>                    username=args.username,
>                    password=args.password,
>                    root_password=args.root_password,
> +                  uid_map=uid_map,
> +                  gid_map=gid_map,
>                    not_secure=args.not_secure,
>                    no_cache=args.no_cache,
>                    progress_cb=args.status_only)

ACK with the minor changes mentioned above.
A future commit should work on fixing those for qcow2 format I guess

--
Cedric




More information about the virt-tools-list mailing list