← Back to team overview

cloud-init-dev team mailing list archive

Re: [Merge] ~pzakha/cloud-init:userdata into cloud-init:master

 


Diff comments:

> diff --git a/cloudinit/config/cc_ssh.py b/cloudinit/config/cc_ssh.py
> index f8f7cb3..e1542ee 100755
> --- a/cloudinit/config/cc_ssh.py
> +++ b/cloudinit/config/cc_ssh.py
> @@ -182,8 +182,19 @@ def handle(_name, cfg, cloud, log, _args):
>          disable_root = util.get_cfg_option_bool(cfg, "disable_root", True)
>          disable_root_opts = util.get_cfg_option_str(cfg, "disable_root_opts",
>                                                      ssh_util.DISABLE_USER_OPTS)
> +        if 'allow_public_ssh_keys' in cfg:
> +            allow_public_ssh_keys = cfg['allow_public_ssh_keys']
> +        else:
> +            allow_public_ssh_keys = True

IIUC, this is meant to be a boolean, so please use util.get_cfg_option_bool(cfg, "allow_public_ssh_keys", True)
instead of the if/else clause.

> +
> +        if allow_public_ssh_keys:
> +            log.debug('allow_public_ssh_keys = True: Public ssh keys consumed')
> +            keys = cloud.get_public_ssh_keys() or []
> +        else:
> +            log.debug('allow_public_ssh_keys = False: Public ssh keys '
> +                      'discarded')

The default is to accept public keys, so let's only log.debug if they are disabled.  Otherwise it's just noise in the log.

Also, cloud.get_public_keys() already returns [] if there's no data, if it's not valid data, etc. So I think we can just:

keys = [] if not allow_public_ssh_keys else cloud.get_public_ssh_keys()

> +            keys = []
>  
> -        keys = cloud.get_public_ssh_keys() or []
>          if "ssh_authorized_keys" in cfg:
>              cfgkeys = cfg["ssh_authorized_keys"]
>              keys.extend(cfgkeys)


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


References