Rough SSH Bastion Host Support (relates to #2144) #2150

lattwood · 2015-10-30T16:45:56Z

Added ParamikoSSHBastionClient as a subclass of ParamikoSSHClient, with overridden connect and close methods for connection bouncing.
Use ParamikoSSHBastionClient if a ! exists in the host. The left side of the ! is the bastion host, right side is the target host.
All connection parameters for the target host are used for connecting to the bastion host. See https://github.com/lattwood/st2/blob/7cf7ba0938d23ddba29ce6c55d39ac6e49f7aaea/st2actions/st2actions/runners/ssh/paramiko_ssh_bastion.py#L43-L51

Rudimentary testing was done with https://gist.github.com/lattwood/4c52ba7b8583ae4565eb, I'll be doing more extensive testing using st2workroom today. This touches on #2144

This is the most python I've written so I apologize in advance if anything isn't done the python way.

Is a CLA required or would Apache licensing suffice?

lattwood · 2015-10-30T16:54:07Z

st2actions/st2actions/runners/ssh/paramiko_ssh_bastion.py

+        transport = self.bastion_client.get_transport()
+        real_addr = (self.hostname, self.port)
+        # no attempt is made to bind local_addr, so something very "wrong" is used
+        local_addr = ('256.256.256.256', 65536)


I wasn't 100% sure on what values to use here. Originally I had 127.0.0.1:1234, but since it doesn't actually bind the socket, I thought it made sense to use something that was completely outside of spec.

manasdk · 2015-10-30T17:51:45Z

I am taking a look at this PR but we should wait for @lakshmi-kannan and @Kami.

manasdk · 2015-10-30T17:59:32Z

st2actions/st2actions/runners/ssh/parallel_ssh.py

@@ -216,6 +218,12 @@ def _execute_in_pool(self, execute_method, **kwargs):
        return results

    def _connect(self, host, results, raise_on_any_error=False):
+        bastion_hostname, bastion_port = None, None
+
+        if "!" in host:


Typically breaking out magic string as a constant helps.

Probably place it somewhere https://github.com/StackStorm/st2/blob/master/st2common/st2common/constants/runners.py

The is the most python I've written, ever.

manasdk · 2015-10-30T18:39:48Z

st2actions/st2actions/runners/ssh/paramiko_ssh_bastion.py

+        local_addr = ('256.256.256.256', 65536)
+        channel = transport.open_channel("direct-tcpip", real_addr, local_addr)
+
+        conninfo = {'hostname': '256.256.256.256',


can use local_addr[0] and local_addr[1]

That makes sense from a logging perspective.

This made me realize the AutoAddPolicy the ParamikoSSH client uses will end up erroring if you have two hosts with the same IP behind the bastion host- I'll change the policy to WarningPolicy.

Update: nevermind, it appears as though it's not saved on disk as no host_keys_file is specified on the clients.

manasdk · 2015-10-30T18:57:07Z

High level this is great. most of my comments are really cosmetic things.

Few thoughts -

Instead of overriding the hosts property perhaps a bastion_host property would make sense on the runner?
Perhaps a special runner make more sense and keep things explicit for users. This could be a consumption hinderance so maybe this is not such a good idea.

manasdk · 2015-10-30T18:59:22Z

Also, perhaps a good idea to add some units -

https://github.com/StackStorm/st2/blob/master/st2actions/tests/unit/test_parallel_ssh.py

lattwood · 2015-10-30T19:08:12Z

I was thinking about a bastion_host property, but my understanding is that it would require the modification of any actions using it to add the bastion_host(s) parameter. I would like to make it explicit, but that's a large drawback.

If there is a way to implicitly add the bastion_host(s) parameter to all actions supporting the remote-shell-cmd, how do you run a single command on mixed groups of hosts? (ie, how do you indicate hosts a,b,c are behind bastion1, d,e,f are behind bastion2, and g,h are directly connectable)

manasdk · 2015-10-30T19:20:43Z

but my understanding is that it would require the modification of any actions using it to add the bastion_host(s) parameter.

Actually if we make bastion_host(s) an optional runner_parameter then all actions get it for free.

Concretely adding another property like -
https://github.com/StackStorm/st2/blob/master/st2common/st2common/bootstrap/runnersregistrar.py#L117
https://github.com/StackStorm/st2/blob/master/st2common/st2common/bootstrap/runnersregistrar.py#L203

how do you run a single command on mixed groups of hosts?

This is a tricky one. Unless we come up with a host_group concept this will not work very nicely. For the near term I would suggest the for each execution we only allow 1 bastion host i.e. all hosts for that execution would be assumed to be behind the same bastion.

To run against multiple groups would require multiple executions. If practically it turns out to be the common case then we can look at options as the next step.

lattwood · 2015-11-01T00:41:49Z

@manasdk much cleaner this way, just need to figure out unit tests

lakshmi-kannan · 2015-11-02T11:45:20Z

st2actions/st2actions/runners/ssh/paramiko_ssh.py

@@ -111,6 +111,11 @@ def __init__(self, hostname, port=22, username=None, password=None,
        self.client.set_missing_host_key_policy(paramiko.AutoAddPolicy())
        self.logger = logging.getLogger(__name__)
        self.sftp = None
+        self.bastion_host = bastion_host


self._bastion_host. Can you also use the _ convention everywhere consistently?

I didn't use _bastion_host as the other properties on ParamikoSSHClient aren't prefixed.

The SSH connection is established when connect is called this requires mocking SSHClient for those methods

lakshmi-kannan · 2015-11-02T17:12:50Z

st2actions/st2actions/runners/ssh/paramiko_ssh.py

+            local_addr = ('', 0)
+            self.bastion_socket = transport.open_channel('direct-tcpip', real_addr, local_addr)
+
+        self.client = self._connect(self.hostname, self.bastion_socket)


self._connect(host=self.hostname, socket=self.bastion_socket) would be better.

I'll do the same on line 127 as well.

lakshmi-kannan · 2015-11-02T17:18:08Z

st2actions/st2actions/runners/ssh/paramiko_ssh.py

+            self.bastion_client = self._connect(self.bastion_host)
+            transport = self.bastion_client.get_transport()
+            real_addr = (self.hostname, self.port)
+            local_addr = ('', 0)


If it's not too much hassle, could you please include the link to the fabric commit as a link? This might throw off other people when they read the code. Some helpful hinting would be nice IMO.

lakshmi-kannan · 2015-11-02T17:22:19Z

It's looking much better now in terms of code and tests.

Docs - Please add some info about bastion host in ssh docs. https://github.com/StackStorm/st2/blob/master/docs/source/troubleshooting/ssh.rst
Changelog - Add an entry to CHANGELOG with your name in the "in development " section. https://github.com/StackStorm/st2/blob/master/CHANGELOG.rst

One other thing I thought was if bastion host had a non standard SSH port, we'll need to add ability to handle that. But we'll make the person requiring the feature to add that ;).

Thanks for being patient. We'll merge this soon!

lattwood · 2015-11-02T19:19:07Z

@lakshmi-kannan something something security something obscurity :)

lakshmi-kannan · 2015-11-02T19:35:01Z

LGTM. Merging now.

Rough SSH Bastion Host Support (relates to #2144)

saisandeepachanta · 2020-06-12T14:42:28Z

@lakshmi-kannan or @lattwood I has started using stack storm recently. My environment is accessed by only one jump box. My stack storm is installed on a new box where i have passwordless connectivity to jump server. i have to use my jump server as bastion host and then trigger my scripts on target hosts. can you please post me some examples how to use or configure the bastion host details in stackstorm. i desperately need your help.

lattwood reviewed Oct 30, 2015
View reviewed changes

manasdk reviewed Oct 30, 2015
View reviewed changes

lattwood force-pushed the add_ssh_bastion branch from 968b7ac to cec5dbe Compare October 30, 2015 18:00

lattwood added 7 commits October 30, 2015 15:02

Initial Paramiko SSH Bastion client

58b2af6

The is the most python I've written, ever.

Formatting

4619d58

None can't be multiply assigned like that

6fef399

Use correct bastion host configuration parameters

42538de

Add notes around the non-usage of local_addr

d8567c7

Add Apache License header

fadf997

Line length, 103->100

84d3268

lattwood force-pushed the add_ssh_bastion branch from cec5dbe to 84d3268 Compare October 30, 2015 18:02

Extract bastion host separator (!) into a st2common constant

a55dfe4

manasdk reviewed Oct 30, 2015
View reviewed changes

Use single source of truth for values (local_addr)

8f9ff88

lattwood added 2 commits October 31, 2015 21:06

Add bastion_host to runnersregistrar

164a1ce

Move Bastion Host support into ParamikoSSHClient

a87d8c4

lakshmi-kannan reviewed Nov 2, 2015
View reviewed changes

Ensure bastion_client and bastion_socket are always defined

eb479ee

lattwood added 4 commits November 2, 2015 11:55

DRY connect methods

ae657fa

Add mock statements to tests

a44b1c9

The SSH connection is established when connect is called this requires mocking SSHClient for those methods

Add missing mock statement

fbf7426

Add bastion_host connect test

b164e8a

lakshmi-kannan reviewed Nov 2, 2015
View reviewed changes

Remove unused method

aad808a

lakshmi-kannan reviewed Nov 2, 2015
View reviewed changes

lattwood added 3 commits November 2, 2015 13:46

Explicitly specify the named parameters for _connect

1cb7e09

Add note about bastion host to docs

7494f13

Add feature to changelog

27b0116

lakshmi-kannan added a commit that referenced this pull request Nov 2, 2015

Merge pull request #2150 from lattwood/add_ssh_bastion

a75b103

Rough SSH Bastion Host Support (relates to #2144)

lakshmi-kannan merged commit a75b103 into StackStorm:master Nov 2, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rough SSH Bastion Host Support (relates to #2144) #2150

Rough SSH Bastion Host Support (relates to #2144) #2150

lattwood commented Oct 30, 2015

lattwood Oct 30, 2015

manasdk commented Oct 30, 2015

manasdk Oct 30, 2015

manasdk Oct 30, 2015

lattwood Oct 30, 2015

lattwood Oct 30, 2015

manasdk commented Oct 30, 2015

manasdk commented Oct 30, 2015

lattwood commented Oct 30, 2015

manasdk commented Oct 30, 2015

lattwood commented Nov 1, 2015

lakshmi-kannan Nov 2, 2015

lattwood Nov 2, 2015

lakshmi-kannan Nov 2, 2015

lattwood Nov 2, 2015

lakshmi-kannan Nov 2, 2015

lakshmi-kannan commented Nov 2, 2015

lattwood commented Nov 2, 2015

lakshmi-kannan commented Nov 2, 2015

saisandeepachanta commented Jun 12, 2020

Rough SSH Bastion Host Support (relates to #2144) #2150

Rough SSH Bastion Host Support (relates to #2144) #2150

Conversation

lattwood commented Oct 30, 2015

Choose a reason for hiding this comment

manasdk commented Oct 30, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manasdk commented Oct 30, 2015

manasdk commented Oct 30, 2015

lattwood commented Oct 30, 2015

manasdk commented Oct 30, 2015

lattwood commented Nov 1, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lakshmi-kannan commented Nov 2, 2015

lattwood commented Nov 2, 2015

lakshmi-kannan commented Nov 2, 2015

saisandeepachanta commented Jun 12, 2020