cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

This looks good Dan, I have a couple of comments inline.

lOne thing I think this lacks for our ability to test is the ability to adapt ua client configuration to point to contracts.staging.canonical.com

This could be resolved by making sure we provide a #cloud-config like the following, though I expect we'll need to base64 encode the "content" value.


write_files:
-   content: |
        # ua-contracts staging config
        sso_auth_url: 'https://login.ubuntu.com'
        contract_url: 'https://contracts.staging.canonical.com'
        data_dir: /var/lib/ubuntu-advantage
        log_level: info
        log_file: /var/log/ubuntu-advantage.log
    path: /etc/ubuntu-advantage/uaclient.conf

Diff comments:

> diff --git a/cloudinit/config/cc_ubuntu_advantage.py b/cloudinit/config/cc_ubuntu_advantage.py
> index 5e082bd..9732ffa 100644
> --- a/cloudinit/config/cc_ubuntu_advantage.py
> +++ b/cloudinit/config/cc_ubuntu_advantage.py
> @@ -1,150 +1,125 @@
> -# Copyright (C) 2018 Canonical Ltd.
> -#
>  # This file is part of cloud-init. See LICENSE file for license information.
>  
> -"""Ubuntu advantage: manage ubuntu-advantage offerings from Canonical."""
> +"""ubuntu_advantage: Configure Ubuntu Advantage support entitlements"""
>  
> -import sys
>  from textwrap import dedent
>  
> -from cloudinit import log as logging
>  from cloudinit.config.schema import (
>      get_schema_doc, validate_cloudconfig_schema)
> +from cloudinit import log as logging
>  from cloudinit.settings import PER_INSTANCE
> -from cloudinit.subp import prepend_base_command
>  from cloudinit import util
>  
>  
> -distros = ['ubuntu']
> -frequency = PER_INSTANCE
> +UA_URL = 'https://ubuntu.com/advantage'
>  
> -LOG = logging.getLogger(__name__)
> +distros = ['ubuntu']
>  
>  schema = {
>      'id': 'cc_ubuntu_advantage',
>      'name': 'Ubuntu Advantage',
> -    'title': 'Install, configure and manage ubuntu-advantage offerings',
> +    'title': 'Configure Ubuntu Advantage support entitlements',
>      'description': dedent("""\
> -        This module provides configuration options to setup ubuntu-advantage
> -        subscriptions.
> -
> -        .. note::
> -            Both ``commands`` value can be either a dictionary or a list. If
> -            the configuration provided is a dictionary, the keys are only used
> -            to order the execution of the commands and the dictionary is
> -            merged with any vendor-data ubuntu-advantage configuration
> -            provided. If a ``commands`` is provided as a list, any vendor-data
> -            ubuntu-advantage ``commands`` are ignored.
> -
> -        Ubuntu-advantage ``commands`` is a dictionary or list of
> -        ubuntu-advantage commands to run on the deployed machine.
> -        These commands can be used to enable or disable subscriptions to
> -        various ubuntu-advantage products. See 'man ubuntu-advantage' for more
> -        information on supported subcommands.
> -
> -        .. note::
> -           Each command item can be a string or list. If the item is a list,
> -           'ubuntu-advantage' can be omitted and it will automatically be
> -           inserted as part of the command.
> +        Attach machine to an existing Ubuntu Advantage support contract and
> +        enable or disable support entitlements such as livepatch, ESM,

Livepatch is capitalized. I think we probably should mention Common Criteria (doc reference https://docs.ubuntu.com/security-certs/en/cc-16). As far as I know cis-audit may not make it in this release so might want to omit that.

> +        FIPS, FIPS Updates and CIS Audit tools. When attaching a machine to
> +        Ubuntu Advantage, one can also specify entitlements to
> +        enable.  When the 'entitlements' list is present, any named entitlement
> +        will be enabled and all absent entitlements will remain disabled.
> +
> +        Note that when enabling FIPS or FIPS updates you will need to schedule
> +        a reboot to ensure the machine is running the FIPS-compliant kernel.
> +        See :ref:`Power State Change` for information on how to configure
> +        cloud-init to perform this reboot.
>          """),
>      'distros': distros,
>      'examples': [dedent("""\
> -        # Enable Extended Security Maintenance using your service auth token
> +        # Attach the machine to a Ubuntu Advantage support contract with a
> +        # UA user token obtained from %s.
> +        ubuntu_advantage:
> +          token: <ua_user_token>

It is officially called a UA contact token, and currently should be obtained from UA_URL = auth.contracts.canonical.com but I *think* ubuntu.com/advantage should redirect or reference that once the service is public. Anyway it's the text we point uaclient users at as well so it's probably good as is

> +    """ % UA_URL), dedent("""\
> +        # Attach the machine to an Ubuntu Advantage support contract enabling
> +        # only fips and esm entitlements. Entitlements will only be enabled if
> +        # the environment supports said entitlement. Otherwise warnings will
> +        # be logged for incompatible entitlements specified.
>          ubuntu-advantage:
> -            commands:
> -              00: ubuntu-advantage enable-esm <token>
> +          token: <ua_user_token>
> +          entitlements:
> +          - fips
> +          - esm
>      """), dedent("""\
> -        # Enable livepatch by providing your livepatch token

good examples

> +        # Attach the machine to an Ubuntu Advantage support contract and enable
> +        # the FIPS entitlement.  Perform a reboot once cloud-init has
> +        # completed.
> +        power_state:
> +          mode: reboot
>          ubuntu-advantage:
> -            commands:
> -                00: ubuntu-advantage enable-livepatch <livepatch-token>
> -
> -    """), dedent("""\
> -        # Convenience: the ubuntu-advantage command can be omitted when
> -        # specifying commands as a list and 'ubuntu-advantage' will
> -        # automatically be prepended.
> -        # The following commands are equivalent
> -        ubuntu-advantage:
> -            commands:
> -                00: ['enable-livepatch', 'my-token']
> -                01: ['ubuntu-advantage', 'enable-livepatch', 'my-token']
> -                02: ubuntu-advantage enable-livepatch my-token
> -                03: 'ubuntu-advantage enable-livepatch my-token'
> -    """)],
> +          token: <ua_user_token>
> +          entitlements:
> +          - fips
> +        """)],
>      'frequency': PER_INSTANCE,
>      'type': 'object',
>      'properties': {
> -        'ubuntu-advantage': {
> +        'ubuntu_advantage': {
>              'type': 'object',
>              'properties': {
> -                'commands': {
> -                    'type': ['object', 'array'],  # Array of strings or dict
> -                    'items': {
> -                        'oneOf': [
> -                            {'type': 'array', 'items': {'type': 'string'}},
> -                            {'type': 'string'}]
> -                    },
> -                    'additionalItems': False,  # Reject non-string & non-list
> -                    'minItems': 1,
> -                    'minProperties': 1,
> +                'entitlements': {
> +                    'type': 'array',
> +                    'items': {'type': 'string'},

Good here, we want to keep this flexible as more entitlements are added (or dropped)

> +                },
> +                'token': {
> +                    'type': 'string',
> +                    'description': "A user-token obtained from %s." % UA_URL
>                  }
>              },
> -            'additionalProperties': False,  # Reject keys not in schema
> -            'required': ['commands']
> +            'required': ['token'],
> +            'additionalProperties': False
>          }
>      }
>  }
>  
> -# TODO schema for 'assertions' and 'commands' are too permissive at the moment.
> -# Once python-jsonschema supports schema draft 6 add support for arbitrary
> -# object keys with 'patternProperties' constraint to validate string values.
> -
>  __doc__ = get_schema_doc(schema)  # Supplement python help()
>  
> -UA_CMD = "ubuntu-advantage"
> +LOG = logging.getLogger(__name__)
>  
>  
> -def run_commands(commands):
> -    """Run the commands provided in ubuntu-advantage:commands config.
> +def configure_ua(token=None, entitlements=None):
> +    """Call ua commandline client to attach or enable entitlements."""
> +    error = None
> +    if not token:
> +        error = ('ubuntu_advantage: token must be provided')
> +        LOG.error(error)
> +        raise RuntimeError(error)
>  
> -     Commands are run individually. Any errors are collected and reported
> -     after attempting all commands.
> +    entitlement_cmds = []
> +    if entitlements is not None:  # Entitlements explicitly enabled

Probably want an additional RuntimeError check here on isinstance(entitlements, list) here as the #cloud-config could have contained just a string. Even though json schema does print a warning it'd be better in context to fail hard here instead of processing each character of a string value.

> +        entitlement_cmds.extend(
> +            [['ua', 'enable', name] for name in entitlements])
>  
> -     @param commands: A list or dict containing commands to run. Keys of a
> -         dict will be used to order the commands provided as dict values.
> -     """
> -    if not commands:
> -        return
> -    LOG.debug('Running user-provided ubuntu-advantage commands')
> -    if isinstance(commands, dict):
> -        # Sort commands based on dictionary key
> -        commands = [v for _, v in sorted(commands.items())]
> -    elif not isinstance(commands, list):
> -        raise TypeError(
> -            'commands parameter was not a list or dict: {commands}'.format(
> -                commands=commands))
> -
> -    fixed_ua_commands = prepend_base_command('ubuntu-advantage', commands)
> -
> -    cmd_failures = []
> -    for command in fixed_ua_commands:
> -        shell = isinstance(command, str)
> -        try:
> -            util.subp(command, shell=shell, status_cb=sys.stderr.write)
> -        except util.ProcessExecutionError as e:
> -            cmd_failures.append(str(e))
> -    if cmd_failures:
> -        msg = (
> -            'Failures running ubuntu-advantage commands:\n'
> -            '{cmd_failures}'.format(
> -                cmd_failures=cmd_failures))
> +    attach_cmd = ['ua', 'attach', token]
> +    LOG.debug('Attaching to Ubuntu Advantage. %s', ' '.join(attach_cmd))
> +    try:
> +        util.subp(attach_cmd)
> +    except util.ProcessExecutionError as e:
> +        msg = 'Failure attaching ubuntu advantage:\n{error}'.format(
> +            error=str(e))
>          util.logexc(LOG, msg)
>          raise RuntimeError(msg)
> +    for cmd in entitlement_cmds:
> +        try:
> +            util.subp(cmd, capture=True)
> +        except util.ProcessExecutionError as e:
> +            msg = 'Failure enabling ubuntu advantage:\n{error}'.format(
> +                error=str(e))
> +            util.logexc(LOG, msg)
> +            raise RuntimeError(msg)
>  
>  
>  def maybe_install_ua_tools(cloud):
>      """Install ubuntu-advantage-tools if not present."""

So the package version cutoff where new (python) uaclient is introduced is >=19, so we could test that if you wanted. Though, I do feel like your checking on 'ua' is a feature delta between old and new version so that would work.

> -    if util.which('ubuntu-advantage'):
> +    if util.which('ua'):
>          return
>      try:
>          cloud.distro.update_package_sources()
> @@ -159,14 +134,26 @@ def maybe_install_ua_tools(cloud):
>  
>  
>  def handle(name, cfg, cloud, log, args):
> -    cfgin = cfg.get('ubuntu-advantage')
> -    if cfgin is None:
> +    if 'ubuntu-advantage' in cfg:
> +        msg = ('Deprecated configuration key "ubuntu-advantage" provided.'

We actually can't process or adapt the old config to the new config because old config took a set of commands which didn't include the UserToken required to attach the new service. We *should* log a warning about cloud-init ignoring the old config values, but warnings are an indication that someone is providing ignored content. I'd much rather be noisy about the failure so we know for certain what that some configuration provided (which could be security related) didn't apply.

I was going to suggest we should also RuntimeError if 'commands' subkey exists in our new config section, but it looks like you beat me to it.

Normally I'd want cloud-init to get the system up as best as we can, but since livepatching, fips and common criteria may be in the mix, it might be best to stop the line on this so folks can be certain their patches are applied or packages restricted etc.

> +               ' Expected underscore delimited "ubuntu_advantage"')
> +        LOG.error(msg)
> +        raise RuntimeError(msg)
> +    ua_section = cfg.get('ubuntu_advantage')
> +    if ua_section is None:
>          LOG.debug(("Skipping module named %s,"
> -                   " no 'ubuntu-advantage' key in configuration"), name)
> +                   " no 'ubuntu_advantage' configuration found"), name)
>          return
> -
>      validate_cloudconfig_schema(cfg, schema)
> +    if 'commands' in ua_section:
> +        msg = (
> +            'Deprecated configuration "ubuntu_advantage: commands" provided.'
> +            ' Expected "token"')
> +        LOG.error(msg)
> +        raise RuntimeError(msg)
> +
>      maybe_install_ua_tools(cloud)
> -    run_commands(cfgin.get('commands', []))
> +    configure_ua(token=ua_section.get('token'),
> +                 entitlements=ua_section.get('entitlements'))
>  
>  # vi: ts=4 expandtab


-- 
https://code.launchpad.net/~daniel-thewatkins/cloud-init/+git/cloud-init/+merge/365366
Your team cloud-init commiters is requested to review the proposed merge of ~daniel-thewatkins/cloud-init/+git/cloud-init:feature/cc-uaclient into cloud-init:master.