cloud-init-dev team mailing list archive

[Jon Grimm <jon.grimm@xxxxxxxxxxxxx>]

Quick look just because I wanted to know more about the impl. 

Diff comments:

> diff --git a/cloudinit/config/cc_snap_config.py b/cloudinit/config/cc_snap_config.py
> new file mode 100644
> index 0000000..667b9c6
> --- /dev/null
> +++ b/cloudinit/config/cc_snap_config.py
> @@ -0,0 +1,177 @@
> +# vi: ts=4 expandtab
> +#
> +#    Copyright (C) 2016 Canonical Ltd.
> +#
> +#    Author: Ryan Harper <ryan.harper@xxxxxxxxxxxxx>
> +#
> +#    This program is free software: you can redistribute it and/or modify
> +#    it under the terms of the GNU General Public License version 3, as
> +#    published by the Free Software Foundation.
> +#
> +#    This program is distributed in the hope that it will be useful,
> +#    but WITHOUT ANY WARRANTY; without even the implied warranty of
> +#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +#    GNU General Public License for more details.
> +#
> +#    You should have received a copy of the GNU General Public License
> +#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +"""
> +Snappy
> +------
> +**Summary:** snap_config modules allows configuration of snapd.
> +
> +This module uses the same ``snappy`` namespace for configuration but
> +acts only only a subset of the configuration.
> +
> +If ``assertions`` is set and the user has included a list of assertions
> +then cloud-init will collect the assertions into a single assertion file
> +and invoke ``snap ack <path to file with assertions>`` which will attempt
> +to load the provided assertions into the snapd assertion database.
> +
> +If ``email`` is set, this value is used to create an authorized user for
> +contacting and installing snaps from the Ubuntu Store.  This is done by
> +calling ``snap create-user`` command.
> +
> +If ``known`` is set to True, then it is expected the user also included
> +an assertion of type ``system-user``.  When ``snap create-user`` is called
> +cloud-init will append '--known' flag which instructs snapd to look for
> +a system-user assertion with the details.  If ``known`` is not set, then
> +``snap create-user`` will contact the Ubuntu SSO for validating and importing
> +a system-user for the instance.
> +
> +.. note::
> +    If the system is already managed, then cloud-init will not attempt to
> +    create a system-user.
> +
> +**Internal name:** ``cc_snap_config``
> +
> +**Module frequency:** per instance
> +
> +**Supported distros:** ubuntu
> +
> +**Config keys**::
> +
> +    #cloud-config
> +    snappy:
> +        assertions:
> +        - |
> +        <assertion 1>
> +        - |
> +        <assertion 2>
> +        email: user@xxxxxxxx
> +        known: true
> +
> +"""
> +
> +from cloudinit import log as logging
> +from cloudinit.settings import PER_INSTANCE
> +from cloudinit import util
> +
> +LOG = logging.getLogger(__name__)

I guess its not clear to me when this should be used vs util.logexc... config modules seem to have almost arbitrary use?  just a question.

> +
> +frequency = PER_INSTANCE
> +SNAPPY_CMD = "snap"
> +ASSERTIONS_FILE = "/var/lib/cloud/instance/snapd.assertions"
> +
> +distros = ['ubuntu']
> +
> +
> +def set_snappy_command():
> +    global SNAPPY_CMD
> +    if util.which("snappy-go"):
> +        SNAPPY_CMD = "snappy-go"
> +    elif util.which("snappy"):
> +        SNAPPY_CMD = "snappy"
> +    else:
> +        SNAPPY_CMD = "snap"
> +    LOG.debug("snappy command is '%s'", SNAPPY_CMD)
> +
> +
> +"""
> +snappy:
> +  assertions:
> +  - |
> +  <snap assertion 1>
> +  - |
> +  <snap assertion 2>
> +  email: foo@xxxxxx
> +  known: true
> +"""
> +
> +
> +def add_assertions(assertions):
> +    """ Import list of assertions by concatenating each
> +        assertion into a string separated by a '\n'.
> +        Write this string to a instance file and
> +        then invoke `snap ack /path/to/file`
> +        and check for errors.
> +
> +        If snap exits 0, then all assertions are imported.
> +    """
> +    if not assertions:

afaict add_assertions() is helper only within module (for that matter maybe should be named _add_assertions if so),  correspondingly the error checks here on 118 and 121 are sort a bit overprotective of yourself, that is you also have a check in handle for len(assertions) > 0, etc in handle().  nit of course.

> +        assertions = []
> +
> +    if not isinstance(assertions, list):
> +        raise ValueError('assertion parameter was not a list: %s', assertions)
> +
> +    snap_cmd = [SNAPPY_CMD, 'ack']
> +    combined = "\n".join(assertions)
> +    if len(combined) == 0:
> +        raise ValueError("Assertion list is empty")
> +
> +    for asrt in assertions:
> +        LOG.debug('Acking: %s', asrt.split('\n')[0:2])
> +
> +    util.write_file(ASSERTIONS_FILE, combined.encode('utf-8'))

any useful errors to deal with or log at least around write_file, or the 'snap ack'?

> +    util.subp(snap_cmd + [ASSERTIONS_FILE], capture=True)
> +
> +
> +def handle(name, cfg, cloud, log, args):
> +    cfgin = cfg.get('snappy')
> +    if not cfgin:
> +        LOG.debug('No snappy config provided, skipping')
> +        return
> +
> +    if not(util.system_is_snappy()):
> +        LOG.debug("%s: system not snappy", name)
> +        return
> +
> +    set_snappy_command()
> +
> +    assertions = cfgin.get('assertions', [])
> +    if len(assertions) > 0:
> +        LOG.debug('Importing user-provided snap assertions')
> +        add_assertions(assertions)
> +
> +    # Create a snap user if requested.
> +    # Snap systems contact the store with a user's email
> +    # and extract information needed to create a local user.
> +    # A user may provide a 'system-user' assertion which includes
> +    # the required information. Using such an assertion to create
> +    # a local user requires specifying 'known: true' in the supplied
> +    # user-data.
> +    snapuser = cfgin.get('email', None)
> +    if snapuser:
> +        usercfg = {
> +            'snapuser': snapuser,
> +            'known': cfgin.get('known', False),
> +        }
> +
> +        # query if we're already registered
> +        out, _ = util.subp([SNAPPY_CMD, 'managed'], capture=True)
> +        if out.strip() == "true":
> +            LOG.warning('This device is already managed. '
> +                        'Skipping system-user creation')
> +            return
> +
> +        if usercfg.get('known'):
> +            # Check that we imported a system-user assertion
> +            out, _ = util.subp([SNAPPY_CMD, 'known', 'system-user'],
> +                               capture=True)
> +            if len(out) == 0:
> +                LOG.error('Missing "system-user" assertion. '
> +                          'Check "snappy" user-data assertions.')
> +                return
> +
> +        cloud.distro.create_user(snapuser, **usercfg)
> diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py
> index b1192e8..101c7ec 100644
> --- a/cloudinit/distros/__init__.py
> +++ b/cloudinit/distros/__init__.py
> @@ -445,6 +448,32 @@ class Distro(object):
>              util.logexc(LOG, "Failed to create user %s", name)
>              raise e
>  
> +    def add_snap_user(self, name, **kwargs):
> +        """
> +        Add a snappy user to the system using snappy tools
> +        """
> +
> +        snapuser = kwargs.get('snapuser')
> +        known = kwargs.get('known', False)
> +        adduser_cmd = ["snap", "create-user", "--sudoer", "--json"]
> +        if known:
> +            adduser_cmd.append("--known")
> +        adduser_cmd.append(snapuser)
> +
> +        # Run the command
> +        LOG.debug("Adding snap user %s", name)
> +        try:
> +            (out, err) = util.subp(adduser_cmd, logstring=adduser_cmd,

why parens around out,err? nit

> +                                   capture=True)
> +            LOG.debug("snap create-user returned: %s:%s", out, err)
> +            jobj = util.load_json(out)
> +            username = jobj.get('username', None)
> +        except Exception as e:
> +            util.logexc(LOG, "Failed to create snap user %s", name)
> +            raise e
> +
> +        return username
> +
>      def create_user(self, name, **kwargs):
>          """
>          Creates users for the system using the GNU passwd tools. This


-- 
https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/304700
Your team cloud init development team is requested to review the proposed merge of ~raharper/cloud-init:snapuser-create into cloud-init:master.