salt-cloud's map with "saltify" seeds the master with private key, but master still retains it if deployment fails
Nobody has claimed this yet.
- 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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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