cloud-init-dev team mailing list archive

[Scott Moser <ssmoser2+ubuntu@xxxxxxxxx>]

It is not the 'ubuntu' user that gets keys.
It is the system configured default user.

The proposal here changes behavior for all Ubuntu instances launched on GCE.
While some users may expect Ubuntu to act consistently with other platforms
when running on GCE other users expect Ubuntu to act consistently across
all platforms.

So some more thought needs to be done here on what happens by default.

There are some specific nits inline below.  Definitely bug 1707037 is
the tough portion of this.


Diff comments:

> diff --git a/cloudinit/sources/DataSourceGCE.py b/cloudinit/sources/DataSourceGCE.py
> index ccae420..faf2727 100644
> --- a/cloudinit/sources/DataSourceGCE.py
> +++ b/cloudinit/sources/DataSourceGCE.py
> @@ -89,15 +94,54 @@ class DataSourceGCE(sources.DataSource):
>          return self.availability_zone.rsplit('-', 1)[0]
>  
>  
> -def _trim_key(public_key):
> -    # GCE takes sshKeys attribute in the format of '<user>:<public_key>'
> -    # so we have to trim each key to remove the username part
> +def _has_expired(public_key):
> +    # Check whether an SSH key is expired using GCE specific key format.

Can you give an example of what that format is  here please.

> +    try:
> +        # Check for the Google-specific schema identifier.
> +        schema, json_str = public_key.split(None, 3)[2:]
> +    except (ValueError, AttributeError):
> +        return False
> +
> +    # Do not expire keys if they do not have the expected schema identifier.
> +    if schema != 'google-ssh':
> +        return False
> +
> +    try:
> +        json_obj = json.loads(json_str)
> +    except ValueError:
> +        return False
> +
> +    # Do not expire keys if there is no expriation timestamp.
> +    if 'expireOn' not in json_obj:
> +        return False
> +
> +    expire_str = json_obj['expireOn']
> +    format_str = '%Y-%m-%dT%H:%M:%S+0000'
>      try:
> -        index = public_key.index(':')
> -        if index > 0:
> -            return public_key[(index + 1):]
> -    except Exception:
> -        return public_key
> +        expire_time = datetime.datetime.strptime(expire_str, format_str)
> +    except ValueError:
> +        return False
> +
> +    # Expire the key if and only if we have exceeded the expiration timestamp.
> +    return datetime.datetime.utcnow() > expire_time
> +
> +
> +def _parse_public_keys(public_keys_data):
> +    # Parse the SSH key data for the Ubuntu user account.

example of what input is would be good.

> +    public_keys = []
> +    if not public_keys_data:
> +        return public_keys
> +    lines = [line for line in public_keys_data.splitlines() if line]
> +    for line in lines:
> +        if not all(ord(c) < 128 for c in line):
> +            continue
> +        split_line = line.split(':', 1)
> +        if len(split_line) != 2:
> +            continue
> +        user, key = split_line
> +        if user == 'ubuntu' and not _has_expired(key):

This needs to somehow manage to not use the static key 'ubuntu', but rather use the system configured default user.  That default user can be changed from user-data, which makes this a bit tricky.

> +            public_keys.append(key)
> +    return public_keys
>  
>  
>  def read_md(address=None, platform_check=True):
> @@ -146,17 +187,25 @@ def read_md(address=None, platform_check=True):
>              return ret
>          md[mkey] = value
>  
> -    if md['public-keys']:
> -        lines = md['public-keys'].splitlines()
> -        md['public-keys'] = [_trim_key(k) for k in lines]
> +    print('Instance: %s' % md['instance-data'])
> +    print('Instance Type: %s' % type(md['instance-data']))
> +    instance_data = json.loads(md['instance-data'] or '{}')
> +    project_data = json.loads(md['project-data'] or '{}')
> +    valid_keys = [instance_data.get('sshKeys'), instance_data.get('ssh-keys')]
> +    block_project = instance_data.get('block-project-ssh-keys', '').lower()
> +    if block_project != 'true' and not instance_data.get('sshKeys'):
> +        valid_keys.append(project_data.get('ssh-keys'))
> +        valid_keys.append(project_data.get('sshKeys'))
> +    public_keys_data = '\n'.join([key for key in valid_keys if key])

I'm pretty sure that this just creates public_keys_data as a string of lines
just so that _parse_public_keys can split it by newline.

Right?
Ie, just pass 'valid_keys' to _parse_public_keys have have parse_public_keys expect a list.

> +    md['public-keys'] = _parse_public_keys(public_keys_data)
>  
>      if md['availability-zone']:
>          md['availability-zone'] = md['availability-zone'].split('/')[-1]
>  
> -    encoding = md.get('user-data-encoding')
> +    encoding = instance_data.get('user-data-encoding')
>      if encoding:
>          if encoding == 'base64':
> -            md['user-data'] = b64decode(md['user-data'])
> +            md['user-data'] = b64decode(instance_data.get('user-data'))
>          else:
>              LOG.warning('unknown user-data-encoding: %s, ignoring', encoding)
>  


-- 
https://code.launchpad.net/~illfelder/cloud-init/+git/cloud-init/+merge/334777
Your team cloud-init commiters is requested to review the proposed merge of ~illfelder/cloud-init:master into cloud-init:master.