Code review comment for lp:~newell-jensen/maas/rack_register_command

Revision history for this message
Gavin Panella (allenap) wrote :

This looks good, but it should extend the install-shared-secret command.

The latter prompts the user for the shared secret and accepts it via
stdin. Accepting passwords and security tokens via command-line
arguments has always been problematic because any user of the system can
see them.

It could call InstallSharedSecretScript.run() like so:

  # In src/provisioningserver/register_command.py:

  """Register rack controller.

  A shared secret required for communications with the region controller
  will be prompted for (or read from stdin in a non-interactive shell)
  if one has not previously been installed. You can find it at
  /var/lib/maas/secret on the region.
  """

  from provisioningserver.security import (
      get_shared_secret_from_filesystem,
      InstallSharedSecretScript,
  )

  def add_arguments(parser):
      """Add this command's options to the `ArgumentParser`.

      Specified by the `ActionScript` interface.
      """
      parser.add_argument(
          '--url', action='store', required=False,
          help=('URL of the region controller where to connect '
                'this rack controller.'))
      InstallSharedSecretScript.add_arguments(parser)

  def run(args):
      """Update configuration settings."""
      if args.url is not None:
          with ClusterConfiguration.open_for_update() as config:
              config.maas_url = args.url
      if get_shared_secret_from_filesystem() is None:
          InstallSharedSecretScript.run(args)

review: Needs Fixing

« Back to merge proposal