Replace exec with pkg ssh #635

JoshVanL · 2018-11-20T18:32:21Z

Use a ssh package to replace exec for all ssh connections including tunnel.

Has fully featured ssh shell.
Tunneling for commands and interactive shell.
New hidden sub-command tarmak tunnel that opens a tunnel to a specified destination+port and local port that exits after 10 mins.
Persistent tunnel through an exec of tarmak tunnel which is orphaned.

fixes #634
rebased on top of #664

Use in package ssh rather than exec

JoshVanL · 2018-11-23T14:05:23Z

/unassign
/assign @simonswine
Seems to be working quite well. Think this should be ready for some review now

simonswine

In general I mentioned my concerns with replacing ssh through a command in the catch-up call:

tarmak cluster ssh might be used in ways we never planned for. It is actually quite debug-able by the average Linux admin (-vvv, change config,. ..). While if a library fails you are lost. I think using golang's library is fine and fair enough for the terraform provider, tunneling use cases.

I am suggesting:

We offer an option to run tarmak cluster ssh via golang package, but by default we still are using ssh passthrough
All programmatic access/use of ssh is used through golang package
We have to write a proposal how we fix the public key issue. Not checking at all is worse what we used to do
We should keep the bastion connetion open all the time and use a connection multiple to tunnel to Kube/Vault APIs and SSH of nodes

/assign @JoshVanL
/unassign

pkg/tarmak/ssh/ssh.go

simonswine · 2019-01-28T14:07:09Z

I struggle to build a working cluster from this:

I am getting those errors:

WARN[0349] failed to create tunnel to remote connection: ssh: rejected: connect failed (Connection refused)  app=tarmak destination=vault-3.tarmak.local
WARN[0349] failed to create tunnel to remote connection: ssh: rejected: connect failed (Connection refused)  app=tarmak destination=vault-1.tarmak.local
WARN[0349] failed to create tunnel to remote connection: ssh: rejected: connect failed (Connection refused)  app=tarmak destination=vault-2.tarmak.local
WARN[0352] failed to create tunnel to remote connection: ssh: rejected: connect failed (Connection refused)  app=tarmak destination=vault-3.tarmak.local
WARN[0352] failed to create tunnel to remote connection: ssh: rejected: connect failed (Connection refused)  app=tarmak destination=vault-1.tarmak.local
WARN[0352] failed to create tunnel to remote connection: ssh: rejected: connect failed (Connection refused)  app=tarmak destination=vault-2.tarmak.local
WARN[0355] failed to create tunnel to remote connection: ssh: rejected: connect failed (Connection refused)  app=tarmak destination=vault-3.tarmak.local
WARN[0355] failed to create tunnel to remote connection: ssh: rejected: connect failed (Connection refused)  app=tarmak destination=vault-1.tarmak.local
WARN[0356] failed to create tunnel to remote connection: ssh: rejected: connect failed (Connection refused)  app=tarmak destination=vault-2.tarmak.local
WARN[0358] failed to create tunnel to remote connection: ssh: rejected: connect failed (Connection refused)  app=tarmak destination=vault-1.tarmak.local
WARN[0359] failed to create tunnel to remote connection: ssh: rejected: connect failed (Connection refused)  app=tarmak destination=vault-2.tarmak.local

And apply gets stuck here:

DEBU[1009] module.kubernetes.tarmak_vault_cluster.vault: Still creating... (11m0s elapsed)  app=tarmak module=terraform std=out
DEBU[1019] module.kubernetes.tarmak_vault_cluster.vault: Still creating... (11m10s elapsed)  app=tarmak module=terraform std=out
DEBU[1029] module.kubernetes.tarmak_vault_cluster.vault: Still creating... (11m20s elapsed)  app=tarmak module=terraform std=out
DEBU[1039] module.kubernetes.tarmak_vault_cluster.vault: Still creating... (11m30s elapsed)  app=tarmak module=terraform std=out
DEBU[1049] module.kubernetes.tarmak_vault_cluster.vault: Still creating... (11m40s elapsed)  app=tarmak module=terraform std=out
DEBU[1059] module.kubernetes.tarmak_vault_cluster.vault: Still creating... (11m50s elapsed)  app=tarmak module=terraform std=out

My ssh-known-hosts was empty before and now looks like that:

8.202.74.208 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBOZHQeWBspyzgPH5ogfvXTZH2WZKbI5uSeb+kEinDKcnkOrIrnbz2vtJs5v+e5cDHHgMLozT3H/tW1ao+XC40uQ=
10.99.32.10 ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIFxR/GnZg1xfFb077yBTKieoIe5dPtwojfoc5pvSvspW
10.99.64.10 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBCH3wrOtgH/bcZHQLukE056BmbTS77FgDWMx/QGfDVhBvk4p02kpFoU568NBvIJyZ+CUvdmaeiR83LrLl6pH7E4=
10.99.96.10 ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDEYzZTMZSCB4u4H6ihVDR5D3dORWPvCVFx9CA7i4vw6JDpMqXQAs44o5c35Ok6SpVxnekYTgKRurffk9ydXtc9J1WfR6M6tMPs8NxnZkYYkYvZsr/J8zXVmYt+JxBrxJ/T2tlHkUVaz10RyU+vNsOC1xn3fnSLuFNI6trRNHJq+dJV5LWF/ZO+n9mhbp1gCCzaWGPKWcHHvjVtRbAilacYZF2mjnVe7tuKWQpUry+ydkgvW+aEG5AC9QcGUcpPzo+SrYPrMYQNDS3kU8DIaYM1qZC7L0GXOqOUsHxmUI7DLIvXw3IEq8a+5am47nlkCXIgehjdXjPLvk8FickyxUZ3

simonswine · 2019-01-28T14:11:57Z

cmd/tarmak/cmd/cluster_ssh.go

 	Run: func(cmd *cobra.Command, args []string) {
 		t := tarmak.New(globalFlags)
-		defer t.Cleanup()
-		t.SSHPassThrough(args)
+		t.Perform(t.SSHPassThrough(args[0], args[1:]))


This is still ssh CLI powered instead of golang native SSH package.

The following seems to break (Which is really useful for debugging):

tarmak-0.5.2 cluster ssh bastion -vvv OpenSSH_7.6p1 Ubuntu-4ubuntu0.1, OpenSSL 1.0.2n 7 Dec 2017 debug1: Reading configuration data /home/christian/.tarmak-dev/csire-cluster/ssh_config debug2: resolving "bastion" port 22 ssh: Could not resolve hostname bastion: No address associated with hostname FATA[0000] exit status 255

On master this looks like:

./tarmak_linux_amd64 cluster ssh bastion -vvv ssh: Could not resolve hostname bastion: No address associated with hostname FATA[0000] exit status 255

simonswine · 2019-01-28T14:21:58Z

I suspect we want to store all host keys per instance in the known hosts file, instead of only a signle random one. That might also cause my issues

/assign @JoshVanL
/unassign

simonswine · 2019-01-29T10:10:31Z

pkg/tarmak/ssh/ssh.go

 	for _, host := range hosts {
+		// TODO: do the strict checking settings


@JoshVanL can you take a look at that as well

simonswine · 2019-01-29T10:14:31Z

Ok I have pushed my changes now. I think we need to make Vault Create Cluster fail hard once all three vault are not reachable:

Err: vault1/2/3 not reachable through bastion after 5 tries (sorry don't have a real output right now)

And obviously try to connect more often with longer timeouts.

I am also not happy seeing a lot of expected warning and errors for unreachable bastion and failed vault tunnels. We should not show them at all as long as we get the connection at some point

simonswine · 2019-01-29T10:14:45Z

Over to you now @JoshVanL

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Signed-off-by: Christian Simon <simon@swine.de>

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

JoshVanL · 2019-01-30T16:33:12Z

/unassign
/assign @simonswine

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

simonswine · 2019-01-31T10:07:48Z

Thank you, this is quite stable for me

/lgtm

jetstack-bot assigned JoshVanL Nov 20, 2018

jetstack-bot requested a review from simonswine November 20, 2018 18:32

jetstack-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 23, 2018

JoshVanL force-pushed the use-crypto-ssh branch 3 times, most recently from 69fd5ad to b1432ab Compare November 23, 2018 13:45

JoshVanL changed the title ~~WIP: Replace exec with pkg ssh~~ Replace exec with pkg ssh Nov 23, 2018

jetstack-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 23, 2018

jetstack-bot assigned simonswine and unassigned JoshVanL Nov 23, 2018

simonswine added this to the release-0.6 milestone Nov 26, 2018

simonswine added this to Needs review in release-0.6 Nov 26, 2018

JoshVanL force-pushed the use-crypto-ssh branch from b1432ab to 3d389be Compare November 27, 2018 09:33

simonswine reviewed Nov 28, 2018

View reviewed changes

pkg/tarmak/ssh/ssh.go Outdated Show resolved Hide resolved

jetstack-bot assigned JoshVanL and unassigned simonswine Nov 28, 2018

jetstack-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 28, 2018

JoshVanL force-pushed the use-crypto-ssh branch from 3328a04 to c9f3d8a Compare November 29, 2018 16:15

jetstack-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 29, 2018

jetstack-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 7, 2018

JoshVanL force-pushed the use-crypto-ssh branch from 4c487ef to 791d421 Compare January 21, 2019 13:48

jetstack-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 21, 2019

jetstack-bot assigned simonswine and unassigned JoshVanL Jan 24, 2019

simonswine reviewed Jan 28, 2019

View reviewed changes

jetstack-bot assigned JoshVanL and unassigned simonswine Jan 28, 2019

simonswine reviewed Jan 29, 2019

View reviewed changes

JoshVanL force-pushed the use-crypto-ssh branch from 7b236e8 to 6303b9e Compare January 29, 2019 11:47

JoshVanL and others added 5 commits January 29, 2019 11:50

Use in package ssh for all programmatic use cases

3d35c93

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Adds host key call back function

edac567

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Updates docs, Gopkg.lock and fixes tunnel port string

c7afc55

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Use crypto/ssh/knownhosts

f5316fb

Signed-off-by: Christian Simon <simon@swine.de>

Improve host key call back function and reduce ssh warnings

778ec89

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

JoshVanL force-pushed the use-crypto-ssh branch from d9f30ad to 778ec89 Compare January 29, 2019 16:09

Kill failed or no longer used tunnel processes

4d7e191

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

JoshVanL force-pushed the use-crypto-ssh branch from ce177d7 to 4d7e191 Compare January 29, 2019 16:36

Improve vault cluster status backoff

1499e61

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

jetstack-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 30, 2019

jetstack-bot assigned simonswine and unassigned JoshVanL Jan 30, 2019

Share try or cancel func

6e2fa45

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

JoshVanL force-pushed the use-crypto-ssh branch from de016d5 to 6e2fa45 Compare January 30, 2019 16:39

jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 31, 2019

jetstack-bot merged commit 1e3993d into jetstack:master Jan 31, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace exec with pkg ssh #635

Replace exec with pkg ssh #635

JoshVanL commented Nov 20, 2018 •

edited

Loading

JoshVanL commented Nov 23, 2018

simonswine left a comment

simonswine commented Jan 28, 2019

simonswine Jan 28, 2019

simonswine commented Jan 28, 2019

simonswine Jan 29, 2019

simonswine commented Jan 29, 2019

simonswine commented Jan 29, 2019

JoshVanL commented Jan 30, 2019

simonswine commented Jan 31, 2019

		for _, host := range hosts {
		// TODO: do the strict checking settings

Replace exec with pkg ssh #635

Replace exec with pkg ssh #635

Conversation

JoshVanL commented Nov 20, 2018 • edited Loading

JoshVanL commented Nov 23, 2018

simonswine left a comment

Choose a reason for hiding this comment

simonswine commented Jan 28, 2019

simonswine Jan 28, 2019

Choose a reason for hiding this comment

simonswine commented Jan 28, 2019

simonswine Jan 29, 2019

Choose a reason for hiding this comment

simonswine commented Jan 29, 2019

simonswine commented Jan 29, 2019

JoshVanL commented Jan 30, 2019

simonswine commented Jan 31, 2019

JoshVanL commented Nov 20, 2018 •

edited

Loading