[salt-cloud] Protect against passing command line arguments as names for the --destroy command in map files

[arthurzenika]

One can create a subset of a map by using the following command :

salt-cloud -m my_map one_machine another_machine

It seems that adding a --destroy offers to destroy all machines

# salt-cloud -m -d my_map one_machine another_machine
[snip] 
The following virtual machines are set to be destroyed:
  ursa-lxc:
    lxc:
      third_machine
      one_machine
      another_machine
Proceed? [N/y] 

# salt-run --versions-report
                  Salt: 2015.5.0
                Python: 2.7.6 (default, Mar 22 2014, 22:59:56)
                Jinja2: 2.7.2
              M2Crypto: 0.21.1
        msgpack-python: 0.3.0
          msgpack-pure: Not Installed
              pycrypto: 2.6.1
               libnacl: Not Installed
                PyYAML: 3.10
                 ioflo: Not Installed
                 PyZMQ: 14.0.1
                  RAET: Not Installed
                   ZMQ: 4.0.4
                  Mako: 0.9.1
 Debian source package: 2015.5.0+ds-1trusty1

[techhat]

I was unaware that you could select a subset of a map, and it's definitely not intentional.

But I don't understand why you need to specify a map file, then only the machines inside the map which you want to destroy. The map doesn't provide any information that would be useful for destroying. Why not just specify the names and call it good?

[arthurzenika]

@techhat I hadn't thought of it that way, I don't often use the command without a map.

Only thing I'm curious about is the follow use case :

• I have two backends, let's say Amazon for my production, and LXC for my dev on my laptop
• I have machine web.example.org on Amazon which is not connected to my developement salt-master
• I have a provider configured for Amazon so salt-cloud sees web.example.org when it queries
• I've created web.example.org using salt-cloud and a map which indicates the provider to use is LXC

Am I sure that it's not going to delete web.example.org from Amazon if I just use salt-cloud --destroy web.exemple.org without a map ? In this case using -m my_lxc_map gives some information about which provider to query, doesn't it ?

[techhat]

@arthurlogilab, no, there's no guarantee that if both are available, that the wrong one won't be destroyed. But Salt Cloud also wasn't designed to be able to handle duplicate IDs, because it assumes that all IDs belong to the same master.

Recent issues that have been submitted have made it clear that people are regularly using one Salt Cloud instance to manage clouds with multiple salt-masters, or at least with multiple infrastructures. It looks like it's time to do some rethinking on this.

[rallytime]

After discussing this with @techhat, we've decided that we're not going to support this behavior. A warning has been added to the docs declaring this. However, we should help protect against accidentally deleting an entire map file if someone does attempt to pass in a specific argument to the map file. The best thing to do here is to make sure that no arguments being passed in, ever, are ignored. Or, at the very least, raise an invocation error. See the discussion in #9772 for another example.

I'm editing the title of this issue to reflect this change.

[rallytime]

@arthurlogilab I am not sure how are you getting to that point where salt-cloud -d is asking you if you want to delete those machines. I was attempting to add a check to make sure you can't delete your entire map file, but it's stacktracing before I even get to that point:

# salt-cloud -m -d ~/mapfile nt-14
[INFO    ] salt-cloud starting
[ERROR   ] An un-handled exception was caught by salt's global exception handler:
SaltCloudNotFound: The specified map file does not exist: -d

Traceback (most recent call last):
  File "/usr/local/bin/salt-cloud", line 10, in <module>
    salt_cloud()
  File "/root/SaltStack/salt/salt/scripts.py", line 316, in salt_cloud
    client.run()
  File "/root/SaltStack/salt/salt/cloud/cli.py", line 86, in run
    mapper = salt.cloud.Map(self.config)
  File "/root/SaltStack/salt/salt/cloud/__init__.py", line 1558, in __init__
    self.rendered_map = self.read()
  File "/root/SaltStack/salt/salt/cloud/__init__.py", line 1646, in read
    self.opts['map']
SaltCloudNotFound: The specified map file does not exist: -d
Traceback (most recent call last):
  File "/usr/local/bin/salt-cloud", line 10, in <module>
    salt_cloud()
  File "/root/SaltStack/salt/salt/scripts.py", line 316, in salt_cloud
    client.run()
  File "/root/SaltStack/salt/salt/cloud/cli.py", line 86, in run
    mapper = salt.cloud.Map(self.config)
  File "/root/SaltStack/salt/salt/cloud/__init__.py", line 1558, in __init__
    self.rendered_map = self.read()
  File "/root/SaltStack/salt/salt/cloud/__init__.py", line 1646, in read
    self.opts['map']
salt.exceptions.SaltCloudNotFound: The specified map file does not exist: -d

Versions Report:

# salt-cloud --versions
            Salt: 2015.5.0
          Python: 2.7.6 (default, Mar 22 2014, 22:59:56)
          Jinja2: 2.7.3
        M2Crypto: 0.21.1
  msgpack-python: 0.4.4
    msgpack-pure: Not Installed
        pycrypto: 2.6.1
         libnacl: Not Installed
          PyYAML: 3.11
           ioflo: Not Installed
           PyZMQ: 14.0.1
            RAET: Not Installed
             ZMQ: 4.0.4
            Mako: 0.9.1
 Apache Libcloud: 0.14.1

The stacktrace should be cleaned up, but I am not sure how you're getting past that stacktrace. I tried this on 2015.5.0, as well as 2015.5.2.

[techhat]

@rallytime switch the -d and the -m in your command line. I thought we had that cleaned up, but that's what's causing your traceback.