saltstack / saltstack/salt

salt-cloud's map with "saltify" seeds the master with private key, but master still retains it if deployment fails

Open
#55,758 6 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Question Salt-Cloud
Dominant language
Python
Stars
15.7k
Forks
5.6k
Avg merge
2d 44m
Merged PRs (30d)
80

Description

Description of Issue

Is this intended behaviour? After running salt-cloud -m w/ a mapfile, some of the hosts failed to be deployed, but the keys are still created despite that failure.

You can see this in the code under the create() method from the Cloud class in salt.cloud.__init__.py where the following creates the key and then assigns it to a dict.

        if deploy:
            if not make_master and 'master' not in minion_dict:
                log.warning(
                    'There\'s no master defined on the \'%s\' VM settings.',
                    vm_['name']
                )

            if 'pub_key' not in vm_ and 'priv_key' not in vm_:
                log.debug('Generating minion keys for \'%s\'', vm_['name'])
                priv, pub = salt.utils.cloud.gen_keys(
                    salt.config.get_cloud_config_value(
                        'keysize',
                        vm_,
                        self.opts
                    )
                )
                vm_['pub_key'] = pub
                vm_['priv_key'] = priv

A little bit later, it calls create. Note the error handler doesn't result in anything different happening which would imply that output would be checked by the caller, but when you look at any of the caller methods such as the "run_profile()" method, there's no error handling even attempting to remove the key.

        try:
            alias, driver = vm_['provider'].split(':')
            func = '{0}.create'.format(driver)
            with salt.utils.context.func_globals_inject(
                self.clouds[fun],
                __active_provider_name__=':'.join([alias, driver])
            ):
                output = self.clouds[func](vm_)
...
        except KeyError as exc:
            log.exception(
                'Failed to create VM %s. Configuration value %s needs '
                'to be set', vm_['name'], exc
            )
Setup

Setup something simple that uses deploy and can be easily made to fail. You can use the saltify driver with this as your provider.

saltify-linux:
    driver: saltify
    deploy: true
    force_minion_config: true
    remove_config_on_destroy: true
    shutdown_on_destroy: true

Now create some simple profile.

fuzz-linux@softmaker-office:
    ssh_username: doesnt matter
    password: letitdie
    provider: saltify-linux

Now produce a mapfile for the profile pointing to a host that doesn't exist, so that way it cannot possibly succeed.

fuzz-linux@softmaker-office:
 - shouldfail:
     ssh_host: 1.1.1.255
Steps to Reproduce Issue

Simply run salt-cloud -y -m /path/to/mapfile, let it fail, and then check salt-key. You'll see that the key for the host was still created despite the host not possibly succeeding, and salt-cloud emitting a failure. Even if you hit ctrl+c, before it tries to connect to the host, the key is still created which means salt-cloud is not handling errors.

Versions Report

The described logic is from the master branch, but if you really need a versions-report.

Salt Version:
           Salt: 2019.2.2

Dependency Versions:
 Apache Libcloud: Not Installed
            cffi: 1.12.3
        cherrypy: Not Installed
        dateutil: 2.8.0
       docker-py: Not Installed
           gitdb: 2.0.3
       gitpython: 3.0.5
           ioflo: 2.0.0
          Jinja2: 2.10.1
         libgit2: 0.28.2
         libnacl: 1.7
        M2Crypto: Not Installed
            Mako: 1.1.0
    msgpack-pure: Not Installed
  msgpack-python: 0.6.2
    mysql-python: Not Installed
       pycparser: 2.14
        pycrypto: 2.6.1
    pycryptodome: 3.9.4
          pygit2: 0.28.2
          Python: 3.7.5 (default, Dec 15 2019, 17:54:26)
    python-gnupg: Not Installed
          PyYAML: 5.1.2
           PyZMQ: 18.0.2
            RAET: Not Installed
           smmap: 0.9.0
         timelib: Not Installed
         Tornado: 4.5.3
             ZMQ: 4.3.2

System Versions:
           dist: Ubuntu 19.04 disco
         locale: UTF-8
        machine: x86_64
        release: 5.0.0-37-generic
         system: Linux
        version: Ubuntu 19.04 disco

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in salt/cloud/init.py at the Cloud.create() method and trace how run_profile() handles failed deployments after minion keys are generated. Reproduce the failure with the saltify map and check salt-key; done means a failed or interrupted deployment does not leave the host key behind.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
cloud
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.