From 49cf289fb6d79071027937fa636b315492293450 Mon Sep 17 00:00:00 2001 From: Vladimir Date: Tue, 26 Nov 2024 00:03:18 +0100 Subject: [PATCH 1/2] Adds support for SSH ProxyCommand to be able to use "Bastion" servers Parse and validate ProxyCommand from playbook targets Support ad-hoc ProxyCommand for CLI-specified hosts Add test coverage with containerized SSH servers --- cmd/spot/main.go | 37 +- cmd/spot/main_with_proxy_test.go | 567 +++++++++++++++++++++++++ cmd/spot/testdata/conf_proxy.yml | 86 ++++ cmd/spot/testdata/test_ssh_key.pub | 2 +- pkg/config/playbook.go | 113 ++++- pkg/config/playbook_test.go | 175 +++++++- pkg/config/target.go | 40 +- pkg/config/target_test.go | 8 +- pkg/config/target_with_proxy_test.go | 123 ++++++ pkg/executor/connector.go | 209 ++++++++- pkg/executor/connector_test.go | 170 ++++++++ pkg/executor/remote.go | 14 +- pkg/executor/remote_test.go | 74 ++++ pkg/executor/testdata/test_ssh_key.pub | 0 pkg/runner/mocks/playbook.go | 6 +- pkg/runner/runner.go | 52 ++- pkg/runner/runner_test.go | 17 +- pkg/runner/testdata/test_ssh_key | 0 18 files changed, 1625 insertions(+), 68 deletions(-) create mode 100644 cmd/spot/main_with_proxy_test.go create mode 100644 cmd/spot/testdata/conf_proxy.yml create mode 100644 pkg/config/target_with_proxy_test.go mode change 100644 => 100755 pkg/executor/testdata/test_ssh_key.pub mode change 100644 => 100755 pkg/runner/testdata/test_ssh_key diff --git a/cmd/spot/main.go b/cmd/spot/main.go index bc5db6a1..becde67f 100644 --- a/cmd/spot/main.go +++ b/cmd/spot/main.go @@ -44,11 +44,12 @@ type options struct { SSHTempDir string `long:"temp" env:"SPOT_TEMP" description:"temporary directory for ssh" default:""` // overrides - Inventory string `short:"i" long:"inventory" description:"inventory file or url [$SPOT_INVENTORY]"` - SSHUser string `short:"u" long:"user" description:"ssh user"` - SSHKey string `short:"k" long:"key" description:"ssh key"` - Env map[string]string `short:"e" long:"env" description:"environment variables for all commands"` - EnvFile string `short:"E" long:"env-file" env:"SPOT_ENV_FILE" description:"environment variables from file" default:"env.yml"` + Inventory string `short:"i" long:"inventory" description:"inventory file or url [$SPOT_INVENTORY]"` + SSHUser string `short:"u" long:"user" description:"ssh user"` + SSHKey string `short:"k" long:"key" description:"ssh key"` + Env map[string]string `short:"e" long:"env" description:"environment variables for all commands"` + EnvFile string `short:"E" long:"env-file" env:"SPOT_ENV_FILE" description:"environment variables from file" default:"env.yml"` + ProxyCommand string `long:"proxy-command" description:"ssh ProxyCommand, valid only if Targets (-t) overriding hosts, i.e. passed as [:port], in other cases ignored, in normal case that command should be in host Destination structure" default:""` // commands filter Skip []string `short:"s" long:"skip" description:"skip commands"` @@ -363,6 +364,7 @@ func makeRunner(opts options, pbook *config.PlayBook) (*runner.Process, error) { if err != nil { return nil, fmt.Errorf("can't create connector: %w", err) } + if opts.SSHAgent { connector = connector.WithAgent() } @@ -372,18 +374,19 @@ func makeRunner(opts options, pbook *config.PlayBook) (*runner.Process, error) { } r := runner.Process{ - Concurrency: opts.Concurrent, - Connector: connector, - Playbook: pbook, - Only: opts.Only, - Skip: opts.Skip, - Logs: logs, - Verbose: len(opts.Verbose) > 0, - Verbose2: len(opts.Verbose) > 1, - Dry: opts.Dry, - Local: opts.Local, - SSHShell: opts.SSHShell, - SSHTempDir: opts.SSHTempDir, + Concurrency: opts.Concurrent, + Connector: connector, + Playbook: pbook, + Only: opts.Only, + Skip: opts.Skip, + Logs: logs, + Verbose: len(opts.Verbose) > 0, + Verbose2: len(opts.Verbose) > 1, + Dry: opts.Dry, + Local: opts.Local, + SSHShell: opts.SSHShell, + SSHTempDir: opts.SSHTempDir, + AdhocProxyCommand: opts.ProxyCommand, } log.Printf("[DEBUG] runner created: concurrency:%d, connector: %s, ssh_shell:%q, verbose:%v, dry:%v, only:%v, skip:%v", r.Concurrency, r.Connector, r.SSHShell, r.Verbose, r.Dry, r.Only, r.Skip) diff --git a/cmd/spot/main_with_proxy_test.go b/cmd/spot/main_with_proxy_test.go new file mode 100644 index 00000000..64f2dda4 --- /dev/null +++ b/cmd/spot/main_with_proxy_test.go @@ -0,0 +1,567 @@ +package main + +import ( + "context" + "fmt" + "log" + "os" + "os/signal" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/testcontainers/testcontainers-go" + "github.com/testcontainers/testcontainers-go/wait" +) + +func Test_runCompletedWithProxy(t *testing.T) { + + _bastionHostAndPort, _, teardown := startTestContainerAndProxy(t) + // TestContainers always return random ports, but we configured private Docker network to simulate proxy case and in that network + // targetHostAndPort is always = target-host:2222, not using taget host info + + targetHostAndPort := "target-host:2222" + bastionHostAndPort := strings.Split(_bastionHostAndPort, ":") + + log.Printf("[INFO] bastion: %v, target %v", bastionHostAndPort, targetHostAndPort) + defer teardown() + + t.Run("normal run", func(t *testing.T) { + opts := options{ + SSHUser: "test", + SSHKey: "testdata/test_ssh_key", + PlaybookFile: "testdata/conf_proxy.yml", + TaskNames: []string{"task1"}, + Targets: []string{targetHostAndPort}, + ProxyCommand: fmt.Sprintf("ssh -W %%h:%%p test@%s -p %s -i testdata/test_ssh_key -o StrictHostKeyChecking=no", bastionHostAndPort[0], bastionHostAndPort[1]), + Only: []string{"wait"}, + SecretsProvider: SecretsProvider{ + Provider: "spot", + Conn: "testdata/test-secrets.db", + Key: "1234567890", + }, + Dbg: true, + } + st := time.Now() + logOut := captureStdout(t, func() { + err := run(opts) + require.NoError(t, err) + }) + t.Log("out\n", logOut) + assert.True(t, time.Since(st) >= 1*time.Second) + }) + + t.Run("normal run with secrets", func(t *testing.T) { + opts := options{ + SSHUser: "test", + SSHKey: "testdata/test_ssh_key", + PlaybookFile: "testdata/conf_proxy.yml", + TaskNames: []string{"task1"}, + Targets: []string{targetHostAndPort}, + ProxyCommand: fmt.Sprintf("ssh -W %%h:%%p test@%s -p %s -i testdata/test_ssh_key -o StrictHostKeyChecking=no", bastionHostAndPort[0], bastionHostAndPort[1]), + Only: []string{"copy configuration", "some command"}, + SecretsProvider: SecretsProvider{ + Provider: "spot", + Conn: "testdata/test-secrets.db", + Key: "1234567890", + }, + Dbg: true, + } + logOut := captureStdout(t, func() { + err := run(opts) + require.NoError(t, err) + }) + t.Log("out\n", logOut) + assert.Contains(t, logOut, "> secrets: **** ****") + assert.Contains(t, logOut, "> secrets md5: a7ae287dce96d9dad168f42fb87518b2") + assert.NotContains(t, logOut, "secval") + }) + + t.Run("dry run", func(t *testing.T) { + opts := options{ + SSHUser: "test", + SSHKey: "testdata/test_ssh_key", + PlaybookFile: "testdata/conf_proxy.yml", + TaskNames: []string{"task1"}, + Targets: []string{targetHostAndPort}, + ProxyCommand: fmt.Sprintf("ssh -W %%h:%%p test@%s -p %s -i testdata/test_ssh_key -o StrictHostKeyChecking=no", bastionHostAndPort[0], bastionHostAndPort[1]), + Only: []string{"wait"}, + Dry: true, + SecretsProvider: SecretsProvider{ + Provider: "spot", + Conn: "testdata/test-secrets.db", + Key: "1234567890", + }, + Dbg: true, + } + st := time.Now() + logOut := captureStdout(t, func() { + err := run(opts) + require.NoError(t, err) + }) + t.Log("out\n", logOut) + assert.True(t, time.Since(st) < 1*time.Second) + assert.NotContains(t, logOut, "secval") + }) + + t.Run("run with dynamic targets", func(t *testing.T) { + opts := options{ + SSHUser: "test", + SSHKey: "testdata/test_ssh_key", + PlaybookFile: "testdata/conf-dynamic.yml", + SecretsProvider: SecretsProvider{ + Provider: "spot", + Conn: "testdata/test-secrets.db", + Key: "1234567890", + }, + + // This env variable does not control spot directly like cli arguments, instead it is processed and injected + // into playbook, check conf-dynamic.yml. Because value of `targetHostAndPort` does not match + // name/group in playbook/inventory file, it will be treated as direct hostname and Playbook.TargetHosts() method will + // inject AdhocProxyCommand + Env: map[string]string{ + "hostAndPort": targetHostAndPort, + }, + ProxyCommand: fmt.Sprintf("ssh -W %%h:%%p test@%s -p %s -i testdata/test_ssh_key -o StrictHostKeyChecking=no", bastionHostAndPort[0], bastionHostAndPort[1]), + Dbg: true, + } + logOut := captureStdout(t, func() { + err := run(opts) + require.NoError(t, err) + }) + t.Log("out\n", logOut) + }) + + t.Run("run with registered variables", func(t *testing.T) { + opts := options{ + SSHUser: "test", + SSHKey: "testdata/test_ssh_key", + PlaybookFile: "testdata/conf_proxy.yml", + TaskNames: []string{"set_register_var", "use_register_var"}, + Targets: []string{targetHostAndPort}, + ProxyCommand: fmt.Sprintf("ssh -W %%h:%%p test@%s -p %s -i testdata/test_ssh_key -o StrictHostKeyChecking=no", bastionHostAndPort[0], bastionHostAndPort[1]), + SecretsProvider: SecretsProvider{ + Provider: "spot", + Conn: "testdata/test-secrets.db", + Key: "1234567890", + }, + Dbg: true, + Verbose: []bool{true}, + } + logOut := captureStdout(t, func() { + err := run(opts) + require.NoError(t, err) + }) + t.Log("out\n", logOut) + assert.Contains(t, logOut, " > setvar len=13") + assert.Contains(t, logOut, " > len: 13") + }) +} + +func Test_runCompletedSimplePlaybookWithProxy(t *testing.T) { + _bastionHostAndPort, _, teardown := startTestContainerAndProxy(t) + // TestContainers always return random ports, but we configured private Docker network to simulate proxy case and in that network + // targetHostAndPort is always = target-host:2222, not using taget host info + + targetHostAndPort := "target-host:2222" + bastionHostAndPort := strings.Split(_bastionHostAndPort, ":") + + log.Printf("[INFO] bastion: %v, target %v", bastionHostAndPort, targetHostAndPort) + defer teardown() + + opts := options{ + SSHUser: "test", + SSHKey: "testdata/test_ssh_key", + PlaybookFile: "testdata/conf-simple.yml", + Targets: []string{targetHostAndPort}, + ProxyCommand: fmt.Sprintf("ssh -W %%h:%%p test@%s -p %s -i testdata/test_ssh_key -o StrictHostKeyChecking=no", bastionHostAndPort[0], bastionHostAndPort[1]), + Only: []string{"wait"}, + Dbg: true, + } + st := time.Now() + logOut := captureStdout(t, func() { + err := run(opts) + require.NoError(t, err) + }) + t.Log("out\n", logOut) + assert.True(t, time.Since(st) >= 1*time.Second) +} + +func Test_runAdhocWithProxy(t *testing.T) { + _bastionHostAndPort, _, teardown := startTestContainerAndProxy(t) + // TestContainers always return random ports, but we configured private Docker network to simulate proxy case and in that network + // targetHostAndPort is always = target-host:2222, not using taget host info + + targetHostAndPort := "target-host:2222" + bastionHostAndPort := strings.Split(_bastionHostAndPort, ":") + + log.Printf("[INFO] bastion: %v, target %v", bastionHostAndPort, targetHostAndPort) + defer teardown() + + opts := options{ + SSHUser: "test", + SSHKey: "testdata/test_ssh_key", + Targets: []string{targetHostAndPort}, + ProxyCommand: fmt.Sprintf("ssh -W %%h:%%p test@%s -p %s -i testdata/test_ssh_key -o StrictHostKeyChecking=no", bastionHostAndPort[0], bastionHostAndPort[1]), + Dbg: true, + } + opts.PositionalArgs.AdHocCmd = "echo hello" + logOut := captureStdout(t, func() { + err := run(opts) + require.NoError(t, err) + }) + t.Log("out\n", logOut) +} + +func Test_runCompletedSeveralTasksWithProxy(t *testing.T) { + _bastionHostAndPort, _, teardown := startTestContainerAndProxy(t) + // TestContainers always return random ports, but we configured private Docker network to simulate proxy case and in that network + // targetHostAndPort is always = target-host:2222, not using taget host info + + targetHostAndPort := "target-host:2222" + bastionHostAndPort := strings.Split(_bastionHostAndPort, ":") + + log.Printf("[INFO] bastion: %v, target %v", bastionHostAndPort, targetHostAndPort) + defer teardown() + + opts := options{ + SSHUser: "test", + SSHKey: "testdata/test_ssh_key", + PlaybookFile: "testdata/conf3.yml", + TaskNames: []string{"task1", "task2"}, + Targets: []string{targetHostAndPort}, + ProxyCommand: fmt.Sprintf("ssh -W %%h:%%p test@%s -p %s -i testdata/test_ssh_key -o StrictHostKeyChecking=no", bastionHostAndPort[0], bastionHostAndPort[1]), + Dbg: true, + } + + st := time.Now() + logOut := captureStdout(t, func() { + err := run(opts) + require.NoError(t, err) + }) + t.Log("out: ", logOut) + assert.True(t, time.Since(st) >= 1*time.Second) + assert.Contains(t, logOut, "task 1 command 1") + assert.Contains(t, logOut, "task 2 command 1") + assert.NotContains(t, logOut, "task 3 command 1") +} + +func Test_runCompletedAllTasksWithProxy(t *testing.T) { + _bastionHostAndPort, _, teardown := startTestContainerAndProxy(t) + // TestContainers always return random ports, but we configured private Docker network to simulate proxy case and in that network + // targetHostAndPort is always = target-host:2222, not using taget host info + + targetHostAndPort := "target-host:2222" + bastionHostAndPort := strings.Split(_bastionHostAndPort, ":") + + log.Printf("[INFO] bastion: %v, target %v", bastionHostAndPort, targetHostAndPort) + defer teardown() + + opts := options{ + SSHUser: "test", + SSHKey: "testdata/test_ssh_key", + PlaybookFile: "testdata/conf2.yml", + Targets: []string{targetHostAndPort}, + ProxyCommand: fmt.Sprintf("ssh -W %%h:%%p test@%s -p %s -i testdata/test_ssh_key -o StrictHostKeyChecking=no", bastionHostAndPort[0], bastionHostAndPort[1]), + Dbg: true, + } + + st := time.Now() + logOut := captureStdout(t, func() { + err := run(opts) + require.NoError(t, err) + }) + t.Log("out: ", logOut) + + assert.True(t, time.Since(st) >= 1*time.Second) + assert.Contains(t, logOut, "task1") + assert.Contains(t, logOut, "task2") + assert.Contains(t, logOut, "all good, 123") + assert.Contains(t, logOut, "good command 2") + assert.Contains(t, logOut, "all good, 123 - foo-val bar-val") + +} + +func Test_runCanceledWithProxy(t *testing.T) { + _bastionHostAndPort, _, teardown := startTestContainerAndProxy(t) + // TestContainers always return random ports, but we configured private Docker network to simulate proxy case and in that network + // targetHostAndPort is always = target-host:2222, not using taget host info + + targetHostAndPort := "target-host:2222" + bastionHostAndPort := strings.Split(_bastionHostAndPort, ":") + + log.Printf("[INFO] bastion: %v, target %v", bastionHostAndPort, targetHostAndPort) + defer teardown() + + opts := options{ + SSHUser: "test", + SSHKey: "testdata/test_ssh_key", + PlaybookFile: "testdata/conf_proxy.yml", + TaskNames: []string{"task1"}, + Targets: []string{targetHostAndPort}, + ProxyCommand: fmt.Sprintf("ssh -W %%h:%%p test@%s -p %s -i testdata/test_ssh_key -o StrictHostKeyChecking=no", bastionHostAndPort[0], bastionHostAndPort[1]), + Only: []string{"wait"}, + SecretsProvider: SecretsProvider{ + Provider: "spot", + Conn: "testdata/test-secrets.db", + Key: "1234567890", + }, + Dbg: true, + } + setupLog(true) + go func() { + err := run(opts) + assert.ErrorContains(t, err, "remote command exited") + }() + + time.Sleep(500 * time.Millisecond) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + signal.NotifyContext(ctx, os.Interrupt) +} + +func Test_runFailedWithProxy(t *testing.T) { + _bastionHostAndPort, _, teardown := startTestContainerAndProxy(t) + // TestContainers always return random ports, but we configured private Docker network to simulate proxy case and in that network + // targetHostAndPort is always = target-host:2222, not using taget host info + + targetHostAndPort := "target-host:2222" + bastionHostAndPort := strings.Split(_bastionHostAndPort, ":") + + log.Printf("[INFO] bastion: %v, target %v", bastionHostAndPort, targetHostAndPort) + defer teardown() + + opts := options{ + SSHUser: "test", + SSHKey: "testdata/test_ssh_key", + PlaybookFile: "testdata/conf-local-failed.yml", + TaskNames: []string{"default"}, + Targets: []string{targetHostAndPort}, + ProxyCommand: fmt.Sprintf("ssh -W %%h:%%p test@%s -p %s -i testdata/test_ssh_key -o StrictHostKeyChecking=no", bastionHostAndPort[0], bastionHostAndPort[1]), + } + setupLog(true) + err := run(opts) + assert.ErrorContains(t, err, `failed command "show content"`) +} + +// REVIEW TAG not running, tested in main_test.go +// func Test_runNoConfigWithProxy(t *testing.T) { +// opts := options{ +// SSHUser: "test", +// SSHKey: "testdata/test_ssh_key", +// PlaybookFile: "testdata/conf-not-found.yml", +// TaskNames: []string{"task1"}, +// Targets: []string{"localhost"}, +// Only: []string{"wait"}, +// } +// setupLog(true) +// err := run(opts) +// require.ErrorContains(t, err, "can't get playbook \"testdata/conf-not-found.yml\"") +//} + +// REVIEW TAG +// To restrict side effects of ProxyCommand option code was added to the PlayBook.TargetHosts() +// but runGen() does not use it, so not sure if runGen() require changes or this test can be skipped +func Test_runGen_goTmplFileWithProxy(t *testing.T) { + outputFilename := filepath.Join(os.TempDir(), "test_gen_output.data") + testCases := []struct { + name string + opts options + }{{ + name: "generate output for a task", + opts: options{ + SSHUser: "test", + SSHKey: "testdata/test_ssh_key", + PlaybookFile: "testdata/conf_proxy.yml", + TaskNames: []string{"task1"}, + Targets: []string{"dev"}, + ProxyCommand: "ssh -W %%h:%%p test@%s -p %s -i testdata/test_ssh_key -o StrictHostKeyChecking=no", + SecretsProvider: SecretsProvider{ + Provider: "spot", + Conn: "testdata/test-secrets.db", + Key: "1234567890", + }, + Inventory: "testdata/inventory.yml", + GenEnable: true, + GenOutput: outputFilename, + GenTemplate: "testdata/gen.tmpl", + }}, + { + name: "generate output for multiple tasks", + opts: options{ + SSHUser: "test", + SSHKey: "testdata/test_ssh_key", + PlaybookFile: "testdata/conf_proxy.yml", + TaskNames: []string{"task1", "failed_task"}, + Targets: []string{"dev"}, + ProxyCommand: "ssh -W %%h:%%p test@%s -p %s -i testdata/test_ssh_key -o StrictHostKeyChecking=no", + SecretsProvider: SecretsProvider{ + Provider: "spot", + Conn: "testdata/test-secrets.db", + Key: "1234567890", + }, + Inventory: "testdata/inventory.yml", + GenEnable: true, + GenOutput: outputFilename, + GenTemplate: "testdata/gen.tmpl", + }, + }, + } + + defer os.Remove(outputFilename) + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + os.Remove(tc.opts.GenOutput) + + setupLog(true) + err := run(tc.opts) + require.NoError(t, err) + + res, err := os.ReadFile(tc.opts.GenOutput) + require.NoError(t, err) + exp := "\n" + `"Name": "dev1", "Host": "dev1.umputun.dev", "Port": 22, "User": "test","Tags": []` + "\n" + + `"Name": "dev2", "Host": "dev2.umputun.dev", "Port": 22, "User": "test","Tags": []` + assert.Equal(t, exp, string(res), "expected output") + }) + } +} + +func Test_connectFailedWithProxy(t *testing.T) { + _bastionHostAndPort, _, teardown := startTestContainerAndProxy(t) + // TestContainers always return random ports, but we configured private Docker network to simulate proxy case and in that network + // targetHostAndPort is always = target-host:2222, not using taget host info + + targetHostAndPort := "target-host:2222" + bastionHostAndPort := strings.Split(_bastionHostAndPort, ":") + + log.Printf("[INFO] bastion: %v, target %v", bastionHostAndPort, targetHostAndPort) + defer teardown() + + opts := options{ + SSHUser: "bad_user", + SSHKey: "testdata/test_ssh_key", + PlaybookFile: "testdata/conf_proxy.yml", + TaskNames: []string{"task1"}, + Targets: []string{targetHostAndPort}, + ProxyCommand: fmt.Sprintf("ssh -W %%h:%%p test@%s -p %s -i testdata/test_ssh_key -o StrictHostKeyChecking=no", bastionHostAndPort[0], bastionHostAndPort[1]), + SecretsProvider: SecretsProvider{ + Provider: "spot", + Conn: "testdata/test-secrets.db", + Key: "1234567890", + }, + } + setupLog(true) + err := run(opts) + assert.ErrorContains(t, err, `ssh: unable to authenticate`) +} + +func Test_sshAgentForwardingWithProxy(t *testing.T) { + stop := runSSHAgent(t, "testdata/test_ssh_key") + defer stop() + + _bastionHostAndPort, _, teardown := startTestContainerAndProxy(t) + // TestContainers always return random ports, but we configured private Docker network to simulate proxy case and in that network + // targetHostAndPort is always = target-host:2222, not using taget host info + + targetHostAndPort := "target-host:2222" + bastionHostAndPort := strings.Split(_bastionHostAndPort, ":") + + log.Printf("[INFO] bastion: %v, target %v", bastionHostAndPort, targetHostAndPort) + defer teardown() + + opts := options{ + SSHUser: "test", + SSHKey: "testdata/test_ssh_key", + ForwardSSHAgent: true, + Targets: []string{targetHostAndPort}, + ProxyCommand: fmt.Sprintf("ssh -W %%h:%%p test@%s -p %s -i testdata/test_ssh_key -o StrictHostKeyChecking=no", bastionHostAndPort[0], bastionHostAndPort[1]), + Dbg: true, + } + + cmd := fmt.Sprintf("ssh-add -l | awk \"{ print \\$2 }\" > f1; echo %q > f2; diff f1 f2", + getKeyFingerprint(t, "testdata/test_ssh_key")) + + opts.PositionalArgs.AdHocCmd = cmd + + setupLog(true) + err := run(opts) + require.NoError(t, err) +} + +func startTestContainerAndProxy(t *testing.T) (hostAndPort1, hostAndPort2 string, teardown func()) { + t.Helper() + ctx := context.Background() + pubKey, err := os.ReadFile("testdata/test_ssh_key.pub") + require.NoError(t, err) + + // Create a custom network + networkName := "test-network" + + networkRequest := testcontainers.NetworkRequest{ + Name: networkName, + CheckDuplicate: true, + } + network, err := testcontainers.GenericNetwork(ctx, testcontainers.GenericNetworkRequest{ + NetworkRequest: networkRequest, + }) + require.NoError(t, err) + + // Define the container request + containerRequest := func(name string) testcontainers.ContainerRequest { + return testcontainers.ContainerRequest{ + AlwaysPullImage: true, + Image: "lscr.io/linuxserver/openssh-server:latest", + ExposedPorts: []string{"2222/tcp"}, + WaitingFor: wait.NewLogStrategy("done.").WithStartupTimeout(time.Second * 60), + Networks: []string{networkName}, + NetworkAliases: map[string][]string{networkName: {name}}, + Hostname: name, + Files: []testcontainers.ContainerFile{ + {HostFilePath: "testdata/test_ssh_key.pub", ContainerFilePath: "/authorized_key"}, + }, + Env: map[string]string{ + "PUBLIC_KEY": string(pubKey), + "USER_NAME": "test", + "TZ": "Etc/UTC", + "DOCKER_MODS": "linuxserver/mods:openssh-server-ssh-tunnel", + }, + } + } + + // Start the bastion container + container1, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{ + ContainerRequest: containerRequest("bastion-host"), + Started: true, + }) + require.NoError(t, err) + + // Start the container with final ssh connection + container2, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{ + ContainerRequest: containerRequest("target-host"), + Started: true, + }) + require.NoError(t, err) + + // Get the host and port for both containers + host1, err := container1.Host(ctx) + require.NoError(t, err) + port1, err := container1.MappedPort(ctx, "2222") + require.NoError(t, err) + + host2, err := container2.Host(ctx) + require.NoError(t, err) + port2, err := container2.MappedPort(ctx, "2222") + require.NoError(t, err) + + teardown = func() { + container1.Terminate(ctx) + container2.Terminate(ctx) + network.Remove(ctx) + } + + return fmt.Sprintf("%s:%s", host1, port1.Port()), fmt.Sprintf("%s:%s", host2, port2.Port()), teardown +} diff --git a/cmd/spot/testdata/conf_proxy.yml b/cmd/spot/testdata/conf_proxy.yml new file mode 100644 index 00000000..ed9cf361 --- /dev/null +++ b/cmd/spot/testdata/conf_proxy.yml @@ -0,0 +1,86 @@ +user: test + +targets: + remark42: + hosts: [{host: "h1.example.com", name: "host1", proxy_command: "ssh -W %h:%p test@localhost -p bastion-host:2222 -i testdata/test_ssh_key -o StrictHostKeyChecking=no"}, {host: "h2.example.com", name: "host2"}] + + +tasks: + + - name: task1 + options: {secrets: ["sec1"]} + commands: + - name: wait + script: sleep 1s + + - name: copy configuration + copy: {"src": "testdata/conf.yml", "dst": "/tmp/conf.yml", "mkdir": true} + + - name: copy multiple files + mcopy: + - {src: "testdata/conf2.yml", dst: "/tmp/conf2.yml"} + - {src: "testdata/conf-local.yml", dst: "/tmp/conf3.yml"} + + - name: sync things + sync: {"src": "testdata", "dst": "/tmp/things"} + + - name: some command + script: | + ls -laR /tmp + du -hcs /srv + cat /tmp/conf.yml + echo all good, 123 + echo secrets: $sec1 $sec2 + echo secrets md5: `echo -n "$sec1 $sec2" | md5sum` + options: {secrets: ["sec2"]} + + - name: delete things + delete: {"path": "/tmp/things", "recur": true} + + - name: show content + script: ls -laR /tmp + + - name: no auto cmd + script: echo "no auto cmd" + options: {no_auto: true} + + - name: failed_task + commands: + - name: good command + script: echo good command 1 + - name: bad command + script: echo bad command && exit 1 + - name: good command + script: echo good command 2 + + - name: failed_task_with_onerror + on_error: echo onerror called + commands: + - name: good command + script: echo good command 1 + - name: bad command + script: echo bad command && exit 1 + - name: good command + script: echo good command 2 + + + - name: with_wait + commands: + - name: good command + script: echo good command 1 + - name: wait + wait: {cmd: "echo wait done", timeout: 5s, interval: 1s} + + - name: set_register_var + commands: + - name: some command + script: | + echo good command 1 + len=$(echo "file content" | wc -c) + register: [len] + + - name: use_register_var + commands: + - name: some command + script: | + echo "len: $len" diff --git a/cmd/spot/testdata/test_ssh_key.pub b/cmd/spot/testdata/test_ssh_key.pub index e2a0d8f9..ca2f1d60 100644 --- a/cmd/spot/testdata/test_ssh_key.pub +++ b/cmd/spot/testdata/test_ssh_key.pub @@ -1 +1 @@ -ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCl4HgAAcUp59Z6HWjXsMAAFIpU/ES6RVZpYfZWUv9xWzWGjt8v87i4PDgcUIRDMYbqqkchaA6cBM4YmIpZ9DZV4ORyZ0y1D9Upp3WdFIRubo2r4zE/vCSiKFDoN712fz9Ntuydnq+6dMnYnjdtzEtIsVt2iupu2vILka4LBvrdj8HnwYLsd//WSCFhw3jQPZYOysT+9l2TWuAqEenPfXf24nMtcwPsu4YWtJKO4rF2V54DDRga40HmrpV+1vCEk6bZjWKQQry1yo3xKrFZGTlLc3VgMvF7qGg+pcaoE4ZmacZG52aCx83PPZhZxxC9obLwHKyVc0o/Z6lkDIZIFkGt umputun@UMBP.localdomain +ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCl4HgAAcUp59Z6HWjXsMAAFIpU/ES6RVZpYfZWUv9xWzWGjt8v87i4PDgcUIRDMYbqqkchaA6cBM4YmIpZ9DZV4ORyZ0y1D9Upp3WdFIRubo2r4zE/vCSiKFDoN712fz9Ntuydnq+6dMnYnjdtzEtIsVt2iupu2vILka4LBvrdj8HnwYLsd//WSCFhw3jQPZYOysT+9l2TWuAqEenPfXf24nMtcwPsu4YWtJKO4rF2V54DDRga40HmrpV+1vCEk6bZjWKQQry1yo3xKrFZGTlLc3VgMvF7qGg+pcaoE4ZmacZG52aCx83PPZhZxxC9obLwHKyVc0o/Z6lkDIZIFkGt umputun@UMBP.localdomain \ No newline at end of file diff --git a/pkg/config/playbook.go b/pkg/config/playbook.go index 28d7efed..77f58dbc 100644 --- a/pkg/config/playbook.go +++ b/pkg/config/playbook.go @@ -80,11 +80,13 @@ type Target struct { // Destination defines destination info type Destination struct { - Name string `yaml:"name" toml:"name"` - Host string `yaml:"host" toml:"host"` - Port int `yaml:"port" toml:"port"` - User string `yaml:"user" toml:"user"` - Tags []string `yaml:"tags" toml:"tags"` + Name string `yaml:"name" toml:"name"` + Host string `yaml:"host" toml:"host"` + Port int `yaml:"port" toml:"port"` + User string `yaml:"user" toml:"user"` + Tags []string `yaml:"tags" toml:"tags"` + ProxyCommand string `yaml:"proxy_command" toml:"proxy_command"` + ProxyCommandParsed []string `yaml:"-" toml:"-"` // parsed proxy command arguments } // Overrides defines override for task passed from cli @@ -149,6 +151,16 @@ func New(fname string, overrides *Overrides, secProvider SecretsProvider) (res * return nil, fmt.Errorf("can't unmarshal config: %w", err) } + // process all host in targets (parse proxy commands and save it in form suitable for exec.CommandContext() ) + for targetName, target := range res.Targets { + for i := range target.Hosts { + if err := normalizeProxyCommand(&target.Hosts[i]); err != nil { + return nil, fmt.Errorf("failed to process destination in target %s: %w", targetName, err) + } + } + res.Targets[targetName] = target + } + if err = res.checkConfig(); err != nil { return nil, fmt.Errorf("config %s is invalid: %w", fname, err) } @@ -380,7 +392,9 @@ func (p *PlayBook) Task(name string) (*Task, error) { } // TargetHosts returns target hosts for given target name. -func (p *PlayBook) TargetHosts(name string) ([]Destination, error) { +// adhocProxyCommand is the proxy command value that can be passed when name represents a hostname +// not existing in the playbook and passed via CLI argument --target. +func (p *PlayBook) TargetHosts(name, adhocProxyCommand string) ([]Destination, error) { userOverride := func(u string) string { // apply overrides of user @@ -396,7 +410,13 @@ func (p *PlayBook) TargetHosts(name string) ([]Destination, error) { } tgExtractor := newTargetExtractor(p.Targets, p.User, p.inventory) - res, err := tgExtractor.Destinations(name) + + proxyCommandParsed, err := parseProxyCommand(adhocProxyCommand) + if err != nil { + return nil, fmt.Errorf("failed to parse proxy command for host %s: command:%s, error: %w", name, adhocProxyCommand, err) + } + + res, err := tgExtractor.Destinations(name, proxyCommandParsed) if err != nil { return nil, err } @@ -680,3 +700,82 @@ func (p *PlayBook) localShell() string { } return "/bin/sh" } + +// normalizeProxyCommand processes a Destination struct to set default values and parse proxy commands. +func normalizeProxyCommand(dest *Destination) error { + if dest.ProxyCommand != "" { + parsed, err := parseProxyCommand(dest.ProxyCommand) + if err != nil { + return fmt.Errorf("failed to parse proxy command for host %s: %w", dest.Host, err) + } + dest.ProxyCommandParsed = parsed + } + return nil +} + +// parseProxyCommand parses a proxy command string into arguments compatible with exec.Command(). +// It handles shell-style quoting and splitting of the command string. +func parseProxyCommand(commandStr string) ([]string, error) { + if commandStr == "" { + return []string{}, nil + } + + commandStr = strings.TrimSpace(commandStr) + if commandStr == "" { + return []string{}, nil + } + + var args []string + var current strings.Builder + var inQuotes bool + var quoteChar rune + var hadQuotes bool + + for i := 0; i < len(commandStr); i++ { + r := rune(commandStr[i]) + switch { + case !inQuotes && (r == '"' || r == '\''): + inQuotes = true + quoteChar = r + hadQuotes = true + case inQuotes && r == quoteChar: + inQuotes = false + quoteChar = 0 + case r == '\\' && i+1 < len(commandStr): + next := rune(commandStr[i+1]) + + // if special character escaped outside quote substring - saving it without first escape character (unescaping) + if (!inQuotes && (next == ' ' || next == '"' || next == '\'' || next == '\\' || next == ':')) || + (inQuotes && next == quoteChar) { // special case when inside quoted substring a quote character escaped + + current.WriteRune(next) + i++ // Skip the next character (escape character removed from sequence) + } else { + // inside quoted substring save characters as is + current.WriteRune(r) + } + case !inQuotes && r == ' ': + if current.Len() > 0 || hadQuotes { + args = append(args, current.String()) + current.Reset() + hadQuotes = false + } + default: + current.WriteRune(r) + } + } + + if inQuotes { + return nil, fmt.Errorf("unclosed quote in proxy command: %s", commandStr) + } + + if current.Len() > 0 || hadQuotes { + args = append(args, current.String()) + } + + if len(args) == 0 { + return nil, fmt.Errorf("empty proxy command") + } + + return args, nil +} diff --git a/pkg/config/playbook_test.go b/pkg/config/playbook_test.go index 533e37ab..b87b218c 100644 --- a/pkg/config/playbook_test.go +++ b/pkg/config/playbook_test.go @@ -344,9 +344,10 @@ func TestTargetHosts(t *testing.T) { "target3": {Name: "target3", Groups: []string{"group1"}, Hosts: []Destination{{Host: "host4.example.com", Port: 22, Name: "host4", Tags: []string{"tag4"}, User: "user4"}}, }, - "target4": {Name: "target4", Groups: []string{"group1"}, Names: []string{"host3"}}, - "target5": {Name: "target5", Tags: []string{"tag1"}}, - "target-empty": {Name: "target-empty", Groups: []string{"empty-group", "gpu-nodes"}}, + "target4": {Name: "target4", Groups: []string{"group1"}, Names: []string{"host3"}}, + "target5": {Name: "target5", Tags: []string{"tag1"}}, + "target-empty": {Name: "target-empty", Groups: []string{"empty-group", "gpu-nodes"}}, + "targetwithproxy": {Name: "targetwithproxy", Hosts: []Destination{{Host: "host1.example.com", Port: 22, ProxyCommand: "ssh -W %h:%p gateway.example.com", ProxyCommandParsed: []string{"ssh", "-W", "%h:%p", "gateway.example.com"}}}, Tags: []string{"tagproxy"}}, }, inventory: &InventoryData{ Groups: map[string][]Destination{ @@ -406,6 +407,13 @@ func TestTargetHosts(t *testing.T) { {Name: "host3", Host: "host3.example.com", Port: 22, User: "defaultuser", Tags: []string{"tag1", "tag2"}}}, false, }, + { + "target with proxy", "targetwithproxy", nil, + []Destination{ + {Host: "host1.example.com", Port: 22, User: "defaultuser", ProxyCommand: "ssh -W %h:%p gateway.example.com", ProxyCommandParsed: []string{"ssh", "-W", "%h:%p", "gateway.example.com"}}, + }, + false, + }, { "target as group from inventory", "group1", nil, []Destination{{Host: "host2.example.com", Port: 2222, User: "defaultuser", Name: "host2", Tags: []string{"tag1"}}}, @@ -475,10 +483,20 @@ func TestTargetHosts(t *testing.T) { }, } + // In the "normal" flow ProxyCommand is in configuration files, but in case cli argument `--target` passed that + // represent hostname which is not exists in configuration file TargetHosts() will create "in memory" Destination, + // and adhocProxyCommand is being used for that Destination. + // This test checking how TargetHosts() extracts data from playbook so adhocProxyCommand is not important here + // because the playbook structure will have original and parsed proxy command already in it. + // It will be loaded during parsing playbook file. + // One test case added with simulation that proxy command is configured. + var adhocProxyCommand = "" + for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { p.overrides = tc.overrides - res, err := p.TargetHosts(tc.targetName) + + res, err := p.TargetHosts(tc.targetName, adhocProxyCommand) if tc.expectError { require.Error(t, err) } else { @@ -860,3 +878,152 @@ func TestPlayBook_SSHTempDir(t *testing.T) { }) } } +func TestParseProxyCommand(t *testing.T) { + // Not all examples of proxy commands here are valid, do not use them without checking what they are doing. + // Many of them will not open pipe to listen on stdin. Mostly these commands here to test + // that parseProxyCommand() can handle tricky cases of parsing with quotes and other special characters. + + t.Run("basic command", func(t *testing.T) { + result, err := parseProxyCommand("ssh -W %h:%p gateway.example.com") + require.NoError(t, err) + assert.Equal(t, []string{"ssh", "-W", "%h:%p", "gateway.example.com"}, result) + }) + + t.Run("empty string", func(t *testing.T) { + result, err := parseProxyCommand("") + require.NoError(t, err) + assert.Equal(t, []string{}, result) + }) + + t.Run("whitespace only", func(t *testing.T) { + result, err := parseProxyCommand(" \t\n ") + require.NoError(t, err) + assert.Equal(t, []string{}, result) + }) + + t.Run("single quotes", func(t *testing.T) { + result, err := parseProxyCommand("ssh -o 'ProxyCommand=nc %h %p' gateway") + require.NoError(t, err) + assert.Equal(t, []string{"ssh", "-o", "ProxyCommand=nc %h %p", "gateway"}, result) + }) + + t.Run("double quotes", func(t *testing.T) { + result, err := parseProxyCommand(`ssh -o "ProxyCommand=nc %h %p" gateway`) + require.NoError(t, err) + assert.Equal(t, []string{"ssh", "-o", "ProxyCommand=nc %h %p", "gateway"}, result) + }) + + t.Run("mixed quotes", func(t *testing.T) { + result, err := parseProxyCommand(`ssh -o 'User="test user"' gateway`) + require.NoError(t, err) + assert.Equal(t, []string{"ssh", "-o", `User="test user"`, "gateway"}, result) + }) + + t.Run("escaped spaces", func(t *testing.T) { + result, err := parseProxyCommand(`ssh -o ProxyCommand=nc\ %h\ %p gateway`) + require.NoError(t, err) + assert.Equal(t, []string{"ssh", "-o", "ProxyCommand=nc %h %p", "gateway"}, result) + }) + + t.Run("escaped quotes in double quotes", func(t *testing.T) { + result, err := parseProxyCommand(`ssh -o "ProxyCommand=nc \"quoted\" %h %p" gateway`) + require.NoError(t, err) + assert.Equal(t, []string{"ssh", "-o", `ProxyCommand=nc "quoted" %h %p`, "gateway"}, result) + }) + + t.Run("escaped quotes in single quotes", func(t *testing.T) { + result, err := parseProxyCommand(`ssh -o 'ProxyCommand=nc '\''quoted'\'' %h %p' gateway`) + require.NoError(t, err) + assert.Equal(t, []string{"ssh", "-o", "ProxyCommand=nc 'quoted' %h %p", "gateway"}, result) + }) + + t.Run("escaped backslash", func(t *testing.T) { + result, err := parseProxyCommand(`ssh -o ProxyCommand=nc\\server gateway`) + require.NoError(t, err) + assert.Equal(t, []string{"ssh", "-o", `ProxyCommand=nc\server`, "gateway"}, result) + }) + + t.Run("complex real world example", func(t *testing.T) { + result, err := parseProxyCommand(`gcloud compute start-iap-tunnel myinstance 22 --local-host-port=localhost:2222 --zone=us-central1-a`) + require.NoError(t, err) + expected := []string{"gcloud", "compute", "start-iap-tunnel", "myinstance", "22", "--local-host-port=localhost:2222", "--zone=us-central1-a"} + assert.Equal(t, expected, result) + }) + + t.Run("command with special characters", func(t *testing.T) { + result, err := parseProxyCommand(`ssh -o "ProxyCommand=nc -X 5 -x proxy.example.com:1080 %h %p" gateway`) + require.NoError(t, err) + assert.Equal(t, []string{"ssh", "-o", "ProxyCommand=nc -X 5 -x proxy.example.com:1080 %h %p", "gateway"}, result) + }) + + t.Run("multiple spaces", func(t *testing.T) { + result, err := parseProxyCommand("ssh -W %h:%p gateway") + require.NoError(t, err) + assert.Equal(t, []string{"ssh", "-W", "%h:%p", "gateway"}, result) + }) + + t.Run("unclosed single quote", func(t *testing.T) { + _, err := parseProxyCommand("ssh -o 'ProxyCommand=nc %h %p gateway") + require.Error(t, err) + assert.Contains(t, err.Error(), "unclosed quote") + }) + + t.Run("unclosed double quote", func(t *testing.T) { + _, err := parseProxyCommand(`ssh -o "ProxyCommand=nc %h %p gateway`) + require.Error(t, err) + assert.Contains(t, err.Error(), "unclosed quote") + }) + + t.Run("empty argument in quotes", func(t *testing.T) { + result, err := parseProxyCommand(`ssh -o "" gateway`) + require.NoError(t, err) + assert.Equal(t, []string{"ssh", "-o", "", "gateway"}, result) + }) + + t.Run("only spaces in quotes", func(t *testing.T) { + result, err := parseProxyCommand(`ssh -o " " gateway`) + require.NoError(t, err) + assert.Equal(t, []string{"ssh", "-o", " ", "gateway"}, result) + }) + + t.Run("nested quotes different types", func(t *testing.T) { + result, err := parseProxyCommand(`ssh -o 'ProxyCommand="nc %h %p"' gateway`) + require.NoError(t, err) + assert.Equal(t, []string{"ssh", "-o", `ProxyCommand="nc %h %p"`, "gateway"}, result) + }) + + t.Run("ssh proxy command with identity file", func(t *testing.T) { + result, err := parseProxyCommand(`ssh -i ~/.ssh/id_rsa -o "ProxyCommand=ssh -i ~/.ssh/gateway_key gateway nc %h %p" target`) + require.NoError(t, err) + expected := []string{"ssh", "-i", "~/.ssh/id_rsa", "-o", "ProxyCommand=ssh -i ~/.ssh/gateway_key gateway nc %h %p", "target"} + assert.Equal(t, expected, result) + }) + + t.Run("curl with http connect proxy - valid ssh proxycommand", func(t *testing.T) { + result, err := parseProxyCommand(`curl -s --proxy http://proxy.example.com:8080 --proxytunnel --connect-timeout 10 http://%h:%p`) + require.NoError(t, err) + expected := []string{"curl", "-s", "--proxy", "http://proxy.example.com:8080", "--proxytunnel", "--connect-timeout", "10", "http://%h:%p"} + assert.Equal(t, expected, result) + }) + + t.Run("curl with authenticated proxy - valid ssh proxycommand", func(t *testing.T) { + result, err := parseProxyCommand(`curl -s --proxy-user "user:pass" --proxy http://proxy.company.com:8080 --proxytunnel --connect-timeout 10 http://%h:%p`) + require.NoError(t, err) + expected := []string{"curl", "-s", "--proxy-user", "user:pass", "--proxy", "http://proxy.company.com:8080", "--proxytunnel", "--connect-timeout", "10", "http://%h:%p"} + assert.Equal(t, expected, result) + }) + + t.Run("curl with escaped colon in proxy auth", func(t *testing.T) { + result, err := parseProxyCommand(`curl -s --proxy-user user:pa\:ss --proxy http://proxy.com:8080 --proxytunnel http://%h:%p`) + require.NoError(t, err) + expected := []string{"curl", "-s", "--proxy-user", "user:pa:ss", "--proxy", "http://proxy.com:8080", "--proxytunnel", "http://%h:%p"} + assert.Equal(t, expected, result) + }) + + t.Run("curl with protocol and port placeholders", func(t *testing.T) { + result, err := parseProxyCommand(`curl -s --proxy "http://corporate-proxy.company.com:8080" --proxytunnel "http://%h:%p"`) + require.NoError(t, err) + expected := []string{"curl", "-s", "--proxy", "http://corporate-proxy.company.com:8080", "--proxytunnel", "http://%h:%p"} + assert.Equal(t, expected, result) + }) +} diff --git a/pkg/config/target.go b/pkg/config/target.go index b166f500..038a0459 100644 --- a/pkg/config/target.go +++ b/pkg/config/target.go @@ -23,7 +23,7 @@ func newTargetExtractor(targets map[string]Target, user string, inventory *Inven // Destinations returns list of destinations for target name // It first checks if the target exists in the playbook; if not, it looks into the inventory. // After collecting the destinations, it deduplicates them before returning. -func (tg *targetExtractor) Destinations(name string) (res []Destination, err error) { +func (tg *targetExtractor) Destinations(name string, proxyCommandParsed []string) (res []Destination, err error) { dedup := func(in []Destination) (res []Destination) { seen := make(map[string]struct{}) for _, d := range in { @@ -40,7 +40,7 @@ func (tg *targetExtractor) Destinations(name string) (res []Destination, err err if ok { res, err = tg.destinationsFromPlaybook(name, t) } else { - res, err = tg.destinationsFromInventory(name) + res, err = tg.destinationsFromInventory(name, proxyCommandParsed) } if err != nil { return nil, err @@ -141,7 +141,7 @@ func (tg *targetExtractor) matchTagsInventory(name string, tags []string) []Dest // If the target name contains an '@', it splits the user from the host and uses it for the destination. // If the target name contains a ':', it splits the host from the port and uses them for the destination. // If none of the above conditions match, it defaults to using the target name as the host and assumes port 22. -func (tg *targetExtractor) destinationsFromInventory(name string) ([]Destination, error) { +func (tg *targetExtractor) destinationsFromInventory(name string, proxyCommandParsed []string) ([]Destination, error) { hosts, ok := tg.inventory.Groups[name] if ok { // the name is a group in inventory, return all hosts in the group @@ -182,6 +182,13 @@ func (tg *targetExtractor) destinationsFromInventory(name string) ([]Destination } // check if the name looks like host:port + // At this point code will treat name as the taget hostname, if ProxyCommand is not empty + // configure it too in Destination structure. + + // Old code does not expect additional filed (ProxyCommand, ProxyCommandParsed) and tests configured to not expect it, + // to respect that returning Destination with them only then ProxyCommand provided + var adhocRes Destination + if strings.Contains(name, ":") { elems := strings.Split(name, ":") port, err := strconv.Atoi(elems[1]) @@ -189,10 +196,33 @@ func (tg *targetExtractor) destinationsFromInventory(name string) ([]Destination return nil, fmt.Errorf("can't parse port %s: %w", elems[1], err) } log.Printf("[DEBUG] target %q used as host:port %s:%d", name, elems[0], port) - return []Destination{{Host: elems[0], Name: elems[0], Port: port, User: user}}, nil + + adhocRes = Destination{Host: elems[0], Name: elems[0], Port: port, User: user} + adhocRes = tg.updateWithProxyCommand(adhocRes, proxyCommandParsed) + + return []Destination{adhocRes}, nil } // we have no idea what this is, use it as host:22 log.Printf("[DEBUG] target %q used as host:22 %s", name, name) - return []Destination{{Host: name, Name: name, Port: 22, User: user}}, nil + + adhocRes = Destination{Host: name, Name: name, Port: 22, User: user} + adhocRes = tg.updateWithProxyCommand(adhocRes, proxyCommandParsed) + return []Destination{adhocRes}, nil + +} + +// updateWithProxyCommand sets proxy command fields on destination from parsed proxy command arguments. +// Fields are set only if proxyCommandParsed is not empty. +func (tg *targetExtractor) updateWithProxyCommand(res Destination, proxyCommandParsed []string) Destination { + // To run task (or connect with proxy) only parsed form of ProxyCommand is necessary, + // but to not confuse user during debug saving both forms in Destination. + // To not pass 2 forms, reconstruct original form from parsed. + + if len(proxyCommandParsed) > 0 { + res.ProxyCommand = strings.Join(proxyCommandParsed, " ") + res.ProxyCommandParsed = proxyCommandParsed + } + return res + } diff --git a/pkg/config/target_test.go b/pkg/config/target_test.go index efba21c1..b3145969 100644 --- a/pkg/config/target_test.go +++ b/pkg/config/target_test.go @@ -281,10 +281,11 @@ func TestDestinations(t *testing.T) { }, } + var proxyCommandParsed []string for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { tge := newTargetExtractor(tc.targets, tc.user, tc.inventory) - res, err := tge.Destinations("test") + res, err := tge.Destinations("test", proxyCommandParsed) if tc.err { assert.Error(t, err) @@ -294,6 +295,7 @@ func TestDestinations(t *testing.T) { } }) } + } func TestHostAddressParsing(t *testing.T) { @@ -341,10 +343,12 @@ func TestHostAddressParsing(t *testing.T) { }, } + var proxyCommandParsed []string + for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { tge := newTargetExtractor(nil, tc.user, &InventoryData{}) - res, err := tge.Destinations(tc.input) + res, err := tge.Destinations(tc.input, proxyCommandParsed) if tc.err { assert.Error(t, err) diff --git a/pkg/config/target_with_proxy_test.go b/pkg/config/target_with_proxy_test.go new file mode 100644 index 00000000..b806536e --- /dev/null +++ b/pkg/config/target_with_proxy_test.go @@ -0,0 +1,123 @@ +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDestinationsWithProxyCommand(t *testing.T) { + testCases := []struct { + name string + targets map[string]Target + proxyCommandParsed []string + user string + inventory *InventoryData + expected []Destination + err bool + }{ + + // It looks like test pkg/config/target_test.go is testing deduplication by tge.Destinations(). + // With configured ProxyCommand nothing changed in that aspect, because deduplication is based on Host+Port+User. + // From that aspect there is nothing to test. Just repeating couple of tests. + // + // But there is one special case described below. + + { + name: "matching tags", + targets: map[string]Target{ + "test": {Tags: []string{"web"}}, + }, + proxyCommandParsed: nil, + user: "user", + inventory: &InventoryData{ + Groups: map[string][]Destination{ + allHostsGrp: { + {Name: "server1", Host: "192.168.1.1", Port: 22, Tags: []string{"web"}, ProxyCommand: "ssh jump1 -W %h:%p"}, + {Name: "server2", Host: "192.168.1.2", Port: 2222, Tags: []string{"db"}, ProxyCommand: "ssh jump2 -W %h:%p"}, + }, + }, + }, + expected: []Destination{ + {Name: "server1", Host: "192.168.1.1", Port: 22, Tags: []string{"web"}, ProxyCommand: "ssh jump1 -W %h:%p"}, + }, + err: false, + }, + + { + name: "multi match", + targets: map[string]Target{ + "test": { + Hosts: []Destination{{Name: "host1", Host: "192.168.1.3", ProxyCommand: "ssh gateway -W %h:%p"}}, + Groups: []string{"web"}, + Tags: []string{"db"}, + }, + }, + proxyCommandParsed: nil, + user: "user", + inventory: &InventoryData{ + Groups: map[string][]Destination{ + allHostsGrp: { + {Name: "server1", Host: "192.168.1.1", Tags: []string{"web"}, Port: 2222, User: "user2", ProxyCommand: "ssh multi-jump -W %h:%p"}, + {Name: "server2", Host: "192.168.1.2", Tags: []string{"db"}, ProxyCommand: "ssh multi-db -W %h:%p"}, + }, + "web": { + {Name: "server1", Host: "192.168.1.1", Tags: []string{"web"}, Port: 2222, User: "user2", ProxyCommand: "ssh multi-jump -W %h:%p"}, + }, + "db": { + {Name: "server2", Host: "192.168.1.2", Tags: []string{"db"}, ProxyCommand: "ssh multi-db -W %h:%p"}, + }, + }, + }, + expected: []Destination{ + {Name: "host1", Host: "192.168.1.3", ProxyCommand: "ssh gateway -W %h:%p"}, + {Name: "server1", Host: "192.168.1.1", Tags: []string{"web"}, Port: 2222, User: "user2", ProxyCommand: "ssh multi-jump -W %h:%p"}, + {Name: "server2", Host: "192.168.1.2", Tags: []string{"db"}, ProxyCommand: "ssh multi-db -W %h:%p"}, + }, + err: false, + }, + + // If program was started with passing target name as string and targetExtractor.destinationsFromInventory() will not find it in + // inventory/playbook files, then it will assume that name is the host name and all data related in inventory/playbook files + // will be ignored. To be able to test that targetExtractor.destinationsFromInventory() will return Destination + // with passed ProxyCommand this test case added. + // + { + name: "name not found in inventory or playbook", + targets: map[string]Target{}, + proxyCommandParsed: []string{"ssh", "jump1", "-W", "%h:%p"}, + user: "user", + inventory: &InventoryData{}, + expected: []Destination{ + {Name: "test", Host: "test", Port: 22, User: "user", Tags: []string(nil), ProxyCommand: "ssh jump1 -W %h:%p", ProxyCommandParsed: []string{"ssh", "jump1", "-W", "%h:%p"}}, + }, + err: false, + }, + + { + name: "name not found in inventory or playbook, proxyCommandParsed is nil", + targets: map[string]Target{}, + proxyCommandParsed: nil, + user: "user", + inventory: &InventoryData{}, + expected: []Destination{ + {Name: "test", Host: "test", Port: 22, User: "user", Tags: []string(nil), ProxyCommand: "", ProxyCommandParsed: []string(nil)}, + }, + err: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tge := newTargetExtractor(tc.targets, tc.user, tc.inventory) + res, err := tge.Destinations("test", tc.proxyCommandParsed) + + if tc.err { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tc.expected, res) + } + }) + } +} diff --git a/pkg/executor/connector.go b/pkg/executor/connector.go index 49abf9d7..cbf37f79 100644 --- a/pkg/executor/connector.go +++ b/pkg/executor/connector.go @@ -3,9 +3,11 @@ package executor import ( "context" "fmt" + "io" "log" "net" "os" + "os/exec" "strings" "time" @@ -19,9 +21,37 @@ type Connector struct { timeout time.Duration enableAgent bool enableAgentForwarding bool + enableProxy bool + proxyCommandParsed []string + stopProxyCommand context.CancelFunc logs Logs } +// substituteProxyCommand updates variables with values associated with the target host. +// SSH ProxyCommand can use placeholders such as %h, %p, and %r (host, port, username), they have to be replaced with the actual values. +func substituteProxyCommand(username, address string, proxyCommand []string) ([]string, error) { + if len(proxyCommand) == 0 { + return []string{}, nil + } + + host, port, err := net.SplitHostPort(address) + if err != nil { + return nil, fmt.Errorf("failed to split hostAddr and port: %w", err) + } + + cmdArgs := make([]string, len(proxyCommand)) + + for i, arg := range proxyCommand { + arg = strings.ReplaceAll(arg, "%h", host) + if port != "" { + arg = strings.ReplaceAll(arg, "%p", port) + } + arg = strings.ReplaceAll(arg, "%r", username) + cmdArgs[i] = arg + } + return cmdArgs, nil +} + // NewConnector creates a new Connector for a given user and private key. func NewConnector(privateKey string, timeout time.Duration, logs Logs) (res *Connector, err error) { res = &Connector{privateKey: privateKey, timeout: timeout, logs: logs} @@ -61,6 +91,21 @@ func (c *Connector) Connect(ctx context.Context, hostAddr, hostName, user string return &Remote{client: client, hostAddr: hostAddr, hostName: hostName, logs: c.logs.WithHost(hostAddr, hostName)}, nil } +// ConnectWithProxy saves values in Connector necessary to execute SSH ProxyCommand, see https://man.openbsd.org/ssh_config.5#ProxyCommand +func (c *Connector) ConnectWithProxy(ctx context.Context, hostAddr, hostName, user string, proxyCommandParsed []string) (*Remote, error) { + log.Printf("[DEBUG] connect to %q (%s), user %q, proxy command: %s", hostAddr, hostName, user, proxyCommandParsed) + + c.proxyCommandParsed = proxyCommandParsed + c.enableProxy = true + + client, err := c.sshClient(ctx, hostAddr, user) + if err != nil { + return nil, err + } + + return &Remote{client: client, hostAddr: hostAddr, hostName: hostName, logs: c.logs.WithHost(hostAddr, hostName), stopProxyCommand: c.stopProxyCommand}, nil +} + func (c *Connector) forwardAgent(client *ssh.Client) error { if !c.enableAgentForwarding { return nil @@ -89,27 +134,171 @@ func (c *Connector) forwardAgent(client *ssh.Client) error { return nil } -func (c *Connector) sshClient(ctx context.Context, host, user string) (session *ssh.Client, err error) { - log.Printf("[DEBUG] create ssh session to %s, user %s", host, user) - if !strings.Contains(host, ":") { - host += ":22" - } - +func (c *Connector) dial(ctx context.Context, host string, conf *ssh.ClientConfig) (*ssh.Client, error) { + var client *ssh.Client + var conn net.Conn dialer := net.Dialer{Timeout: c.timeout} conn, err := dialer.DialContext(ctx, "tcp", host) if err != nil { return nil, fmt.Errorf("failed to dial: %w", err) } + ncc, chans, reqs, err := ssh.NewClientConn(conn, host, conf) + if err != nil { + return nil, fmt.Errorf("failed to create client connection to %s: %v", host, err) + } + client = ssh.NewClient(ncc, chans, reqs) + return client, nil +} + +func (c *Connector) dialWithProxy(ctx context.Context, host string, cmdArgs []string, conf *ssh.ClientConfig) (*ssh.Client, context.CancelFunc, error) { + var sshClient *ssh.Client + pipeClient, pipeServer := net.Pipe() + + log.Printf("[DEBUG] create ssh session with, ProxyCommand: %s", cmdArgs) + + cmd := exec.CommandContext(ctx, cmdArgs[0], cmdArgs[1:]...) + cmd.Stderr = os.Stderr + + // If stdin, stdout is not standard OS files, cmd.Wait() will wait till files will be closed which for observers + // looks like hangup. To automate management of closing files lines below create "standard" stdin/out + // and there is code that copy data between them and pipe. + stdout, err := cmd.StdoutPipe() + if err != nil { + return nil, nil, fmt.Errorf("failed to get cmd.StdoutPipe(): %w", err) + } + + stdin, err := cmd.StdinPipe() + if err != nil { + return nil, nil, fmt.Errorf("failed to get cmd.StdinPipe(): %w", err) + } + + err = cmd.Start() + if err != nil { + return nil, nil, fmt.Errorf("failed to start proxy command: %w", err) + } + + errChan := make(chan error, 3) + + copyCtx, cancelCopy := context.WithCancel(ctx) + + // sends data to stdin of proxy command + go func() { + defer stdin.Close() + copied, err := io.Copy(stdin, pipeServer) + + log.Printf("[DEBUG] io.Copy(stdin, pipeServer) returned error: %v bytes copied %d", err, copied) + + if err != nil && copyCtx.Err() == nil { + log.Printf("[DEBUG] sending error to channel") + errChan <- fmt.Errorf("failed to copy to proxy stdin: %w", err) + } + }() + + // reads data from stdout of proxy command + go func() { + copied, err := io.Copy(pipeServer, stdout) + + log.Printf("[DEBUG] io.Copy(pipeServer, stdout) returned error: %v, bytes copied %d", err, copied) + if err != nil && copyCtx.Err() == nil { + log.Printf("[DEBUG] sending error to channel") + errChan <- fmt.Errorf("failed to copy from proxy stdout: %w", err) + } + }() + + go func() { + // There is a catch proxy command, for example `gcloud compute start-iap-tunnel`, can stop/exit with error but still return 0 as + // return code. Because of that we can't rely on `if err != nil`, instead treating cmd.Wait() as + // blocking request and if that requested ended - proxy command exited/completed/failed - + // sending Done() signal to channel. + err := cmd.Wait() + + log.Printf("[DEBUG] cmd.Wait() returned: %v", err) + + if err != nil && copyCtx.Err() == nil { + errChan <- fmt.Errorf("proxy command execution failed: %w", err) + } + + if err == nil { + log.Printf("[DEBUG] proxy command exited with returned code %v:", err) + cancelCopy() + } + }() + + // monitoring for proxy command errors + go func() { + log.Printf("[DEBUG] staring proxy command monitoring ") + select { + case proxyErr := <-errChan: + log.Printf("[WARN] proxy error after SSH connection established: %v ; closing pipeServer", proxyErr) + + if sshClient != nil { + sshClient.Close() + } + + pipeClient.Close() + pipeServer.Close() + case <-copyCtx.Done(): + log.Printf("[DEBUG] recevied Done() signal, closing pipeServer ") + + pipeClient.Close() + pipeServer.Close() + } + }() + + ncc, chans, reqs, err := ssh.NewClientConn(pipeClient, host, conf) + + if err != nil { + cancelCopy() + pipeClient.Close() + pipeServer.Close() + if cmd.Process != nil { + cmd.Process.Kill() + } + + return nil, nil, fmt.Errorf("failed to create SSH pipeClient connection: %w", err) + } + + sshClient = ssh.NewClient(ncc, chans, reqs) + + return sshClient, cancelCopy, nil +} + +func (c *Connector) sshClient(ctx context.Context, host, user string) (session *ssh.Client, err error) { + var client *ssh.Client + var stopProxyCommand context.CancelFunc + + log.Printf("[DEBUG] create ssh session to %s, user %s", host, user) + if !strings.Contains(host, ":") { + host += ":22" + } + conf, err := c.sshConfig(user, c.privateKey) if err != nil { return nil, fmt.Errorf("failed to create ssh config: %w", err) } - ncc, chans, reqs, err := ssh.NewClientConn(conn, host, conf) - if err != nil { - return nil, fmt.Errorf("failed to create client connection to %s: %v", host, err) + + if !c.enableProxy { + client, err = c.dial(ctx, host, conf) + if err != nil { + return nil, err + } + } + + if c.enableProxy { + cmdArgs, err := substituteProxyCommand(user, host, c.proxyCommandParsed) + if err != nil { + return nil, fmt.Errorf("failed to substitute proxy command with target host values: %w", err) + } + + client, stopProxyCommand, err = c.dialWithProxy(ctx, host, cmdArgs, conf) + if err != nil { + return nil, fmt.Errorf("failed to create client connection wtth proxy command %s, to %s: %v", cmdArgs, host, err) + } + if stopProxyCommand != nil { + c.stopProxyCommand = stopProxyCommand + } } - client := ssh.NewClient(ncc, chans, reqs) if err := c.forwardAgent(client); err != nil { return nil, fmt.Errorf("failed to forward agent to %s: %v", host, err) diff --git a/pkg/executor/connector_test.go b/pkg/executor/connector_test.go index 86a9453a..66ceb78f 100644 --- a/pkg/executor/connector_test.go +++ b/pkg/executor/connector_test.go @@ -2,6 +2,7 @@ package executor import ( "context" + "strings" "testing" "time" @@ -54,3 +55,172 @@ func TestConnector_Connect(t *testing.T) { require.ErrorContains(t, err, "failed to dial: dial tcp 10.255.255.1:22: i/o timeout") }) } + +func TestConnector_ConnectWithProxy(t *testing.T) { + // To test proxy command, the chain of connection will be next: + // localhost -> localhost: (this is also the bastion host) -> target-host:2222 + // In a real-world application, "target-host:2222" will be replaced with "%h:%p", but since + // testcontainers returns "localhost:" manually, overriding it. + // + // "ssh -W" requires enabling AllowTcpForwarding, to enable it, modification was applied: + // see pkg/executor/remote_test.go, env variable DOCKER_MODS on test container. + // The "bastion-host" is a local host, and we are using a standard SSH client which tries to verify the host key; + // to bypass this check, "-o StrictHostKeyChecking=no” was added to the proxy command. + // + + ctx := context.Background() + bastionHostAndPort, _, teardown := startTestContainerAndProxy(t) + defer teardown() + + bastionAddr := strings.Split(bastionHostAndPort, ":") + proxyCommandParsed := []string{ + "ssh", + "-W", + "target-host:2222", + "test@localhost", + "-p", + bastionAddr[1], + "-i", + "testdata/test_ssh_key", + "-o", + "StrictHostKeyChecking=no", + } + + t.Run("good connection", func(t *testing.T) { + c, err := NewConnector("testdata/test_ssh_key", time.Second*10, MakeLogs(true, false, nil)) + require.NoError(t, err) + sess, err := c.ConnectWithProxy(ctx, "target-host:2222", "target-host", "test", proxyCommandParsed) + require.NoError(t, err) + defer sess.Close() + }) + + t.Run("bad user", func(t *testing.T) { + c, err := NewConnector("testdata/test_ssh_key", time.Second*10, MakeLogs(true, false, nil)) + require.NoError(t, err) + _, err = c.ConnectWithProxy(ctx, "target-host:2222", "target-host", "test33", proxyCommandParsed) + require.ErrorContains(t, err, "ssh: unable to authenticate") + }) + + t.Run("bad key", func(t *testing.T) { + _, err := NewConnector("testdata/test_ssh_key33", time.Second*10, MakeLogs(true, false, nil)) + require.ErrorContains(t, err, "private key file \"testdata/test_ssh_key33\" does not exist", "test") + }) + + t.Run("wrong port", func(t *testing.T) { + c, err := NewConnector("testdata/test_ssh_key", time.Second*10, MakeLogs(true, false, nil)) + require.NoError(t, err) + wrongPortProxyCommand := []string{ + "ssh", + "-W", + "target-host:2222", + "test@localhost", + "-p", + "12345", + "-i", + "testdata/test_ssh_key", + "-o", + "StrictHostKeyChecking=no", + } + _, err = c.ConnectWithProxy(ctx, "target-host:2222", "target-host", "test", wrongPortProxyCommand) + require.ErrorContains(t, err, "failed to create client connection") + }) + + t.Run("timeout", func(t *testing.T) { + t.Skip("Implementation of timeout here is overkill") + + /* Skipped because of next. + For the net.Dialer() there is a parameter that controls timeout for establishing connection. + When proxy command is being used, external program will be called exc.Command() and it seems there is no + "default" functionality for timeout for starting program. I.e. OS receive command to start program, and + then program will fail or will start. + + For ssh client virtual in memory server net.Pipe() will be started on the same host and client of it + will be passed to ssh client, so it is also awkward to test timeout abort for localhost "in memory" connection. + + Some proxy commands can support connection timeout, for example ssh `-o ConnectTimeout=5` but it means + test will try to check behavior of external program. + */ + + c, err := NewConnector("testdata/test_ssh_key", time.Nanosecond, MakeLogs(true, false, nil)) + require.NoError(t, err) + _, err = c.ConnectWithProxy(ctx, "target-host:2222", "target-host", "test", proxyCommandParsed) + require.ErrorContains(t, err, "i/o timeout") + }) + + t.Run("unreachable host", func(t *testing.T) { + c, err := NewConnector("testdata/test_ssh_key", time.Second, MakeLogs(true, false, nil)) + require.NoError(t, err) + unreachableProxyCommand := []string{ + "ssh", + "-W", + "unreachable-host:2222", + "test@10.255.255.1", + "-p", + "22", + "-i", + "testdata/test_ssh_key", + "-o", + "StrictHostKeyChecking=no", + "-o", + "ConnectTimeout=1", // connection timeout option to speed up test, default timeout is too big + + } + _, err = c.ConnectWithProxy(ctx, "unreachable-host:2222", "unreachable-host", "test", unreachableProxyCommand) + + // Commented out, timeout error text will be in exc.Command() output and for simplicity external command + // error output is not copied into spot memory, only exit/return code is being checked. + + // require.ErrorContains(t, err, "failed to create client connection") + }) +} + +func TestSubstituteProxyCommand(t *testing.T) { + tests := []struct { + username string + address string + proxyCommand []string + expected []string + expectError bool + }{ + { + username: "user", + address: "example.com:22", + proxyCommand: []string{"ssh", "-W", "%h:%p", "%r@example.com"}, + expected: []string{"ssh", "-W", "example.com:22", "user@example.com"}, + expectError: false, + }, + { + username: "user", + address: "example.com:22", + proxyCommand: []string{"ssh", "-W", "%h:%p", "%r@example.com", "random arg with spaces"}, + expected: []string{"ssh", "-W", "example.com:22", "user@example.com", "random arg with spaces"}, + expectError: false, + }, + { + username: "user", + address: "example.com", + proxyCommand: []string{"ssh", "-W", "%h:%p", "%r@example.com"}, + expected: nil, + expectError: true, + }, + { + username: "user", + address: "example.com:22", + proxyCommand: []string{}, + expected: []string{}, + expectError: false, + }, + } + + for _, test := range tests { + t.Run(test.address, func(t *testing.T) { + result, err := substituteProxyCommand(test.username, test.address, test.proxyCommand) + if test.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, test.expected, result) + } + }) + } +} diff --git a/pkg/executor/remote.go b/pkg/executor/remote.go index ad542904..4d90ae86 100644 --- a/pkg/executor/remote.go +++ b/pkg/executor/remote.go @@ -20,10 +20,11 @@ import ( // Remote executes commands on remote server, via ssh. Not thread-safe. type Remote struct { - client *ssh.Client - hostAddr string - hostName string - logs Logs + client *ssh.Client + hostAddr string + hostName string + logs Logs + stopProxyCommand context.CancelFunc } // Close connection to remote server. @@ -31,6 +32,11 @@ func (ex *Remote) Close() error { if ex.client != nil { return ex.client.Close() } + + if ex.stopProxyCommand != nil { + ex.stopProxyCommand() + } + return nil } diff --git a/pkg/executor/remote_test.go b/pkg/executor/remote_test.go index e20b7af8..72134055 100644 --- a/pkg/executor/remote_test.go +++ b/pkg/executor/remote_test.go @@ -742,3 +742,77 @@ func startTestContainer(t *testing.T) (hostAndPort string, teardown func()) { require.NoError(t, err) return fmt.Sprintf("%s:%s", host, port.Port()), func() { container.Terminate(ctx) } } + +func startTestContainerAndProxy(t *testing.T) (hostAndPort1, hostAndPort2 string, teardown func()) { + t.Helper() + ctx := context.Background() + pubKey, err := os.ReadFile("testdata/test_ssh_key.pub") + require.NoError(t, err) + + // Create a custom network + networkName := "test-network" + + networkRequest := testcontainers.NetworkRequest{ + Name: networkName, + CheckDuplicate: true, + } + network, err := testcontainers.GenericNetwork(ctx, testcontainers.GenericNetworkRequest{ + NetworkRequest: networkRequest, + }) + require.NoError(t, err) + + // Define the container request + containerRequest := func(name string) testcontainers.ContainerRequest { + return testcontainers.ContainerRequest{ + AlwaysPullImage: true, + Image: "lscr.io/linuxserver/openssh-server:latest", + ExposedPorts: []string{"2222/tcp"}, + WaitingFor: wait.NewLogStrategy("done.").WithStartupTimeout(time.Second * 60), + Networks: []string{networkName}, + NetworkAliases: map[string][]string{networkName: {name}}, + Hostname: name, + Files: []testcontainers.ContainerFile{ + {HostFilePath: "testdata/test_ssh_key.pub", ContainerFilePath: "/authorized_key"}, + }, + Env: map[string]string{ + "PUBLIC_KEY": string(pubKey), + "USER_NAME": "test", + "TZ": "Etc/UTC", + "DOCKER_MODS": "linuxserver/mods:openssh-server-ssh-tunnel", + }, + } + } + + // Start the bastion container + container1, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{ + ContainerRequest: containerRequest("bastion-host"), + Started: true, + }) + require.NoError(t, err) + + // Start the container with final ssh connection + container2, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{ + ContainerRequest: containerRequest("target-host"), + Started: true, + }) + require.NoError(t, err) + + // Get the host and port for both containers + host1, err := container1.Host(ctx) + require.NoError(t, err) + port1, err := container1.MappedPort(ctx, "2222") + require.NoError(t, err) + + host2, err := container2.Host(ctx) + require.NoError(t, err) + port2, err := container2.MappedPort(ctx, "2222") + require.NoError(t, err) + + teardown = func() { + container1.Terminate(ctx) + container2.Terminate(ctx) + network.Remove(ctx) + } + + return fmt.Sprintf("%s:%s", host1, port1.Port()), fmt.Sprintf("%s:%s", host2, port2.Port()), teardown +} diff --git a/pkg/executor/testdata/test_ssh_key.pub b/pkg/executor/testdata/test_ssh_key.pub old mode 100644 new mode 100755 diff --git a/pkg/runner/mocks/playbook.go b/pkg/runner/mocks/playbook.go index ad62ee1d..90e396c7 100644 --- a/pkg/runner/mocks/playbook.go +++ b/pkg/runner/mocks/playbook.go @@ -47,7 +47,7 @@ type PlaybookMock struct { AllTasksFunc func() []config.Task // TargetHostsFunc mocks the TargetHosts method. - TargetHostsFunc func(name string) ([]config.Destination, error) + TargetHostsFunc func(name string, adhocProxyCommand string) ([]config.Destination, error) // TaskFunc mocks the Task method. TaskFunc func(name string) (*config.Task, error) @@ -150,7 +150,7 @@ func (mock *PlaybookMock) AllTasksCalls() []struct { } // TargetHosts calls TargetHostsFunc. -func (mock *PlaybookMock) TargetHosts(name string) ([]config.Destination, error) { +func (mock *PlaybookMock) TargetHosts(name string, adhocProxyCommand string) ([]config.Destination, error) { if mock.TargetHostsFunc == nil { panic("PlaybookMock.TargetHostsFunc: method is nil but Playbook.TargetHosts was just called") } @@ -162,7 +162,7 @@ func (mock *PlaybookMock) TargetHosts(name string) ([]config.Destination, error) mock.lockTargetHosts.Lock() mock.calls.TargetHosts = append(mock.calls.TargetHosts, callInfo) mock.lockTargetHosts.Unlock() - return mock.TargetHostsFunc(name) + return mock.TargetHostsFunc(name, adhocProxyCommand) } // TargetHostsCalls gets all the calls that were made to TargetHosts. diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index 79430a7c..97e26634 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -41,6 +41,17 @@ type Process struct { SSHShell string SSHTempDir string + // REVIEW TAG + // So it looks like ProxyCommand is related to host or target, it is not common but possible that to reach + // 2 different hosts 2 different ProxyCommand is necessary. + // There is also exists cli argument `--target` that in one form can be treated as forced hostname (target) + // and it is accessed inside Process.Run(). Also that `target` is one of the options for method main.run() and + // is being used in tests to bypass host configuration in playbook. This complicates passing ProxyCommand to + // the places where it is needed. + // To not override signature of Process.Run(), adding AdhocProxyCommand to Process structure + + AdhocProxyCommand string + Skip []string Only []string } @@ -48,13 +59,14 @@ type Process struct { // Connector is an interface for connecting to a host, and returning remote executer. type Connector interface { Connect(ctx context.Context, hostAddr, hostName, user string) (*executor.Remote, error) + ConnectWithProxy(ctx context.Context, hostAddr, hostName, user string, proxyCommandParsed []string) (*executor.Remote, error) } // Playbook is an interface for getting task and target information from playbook. type Playbook interface { AllTasks() []config.Task Task(name string) (*config.Task, error) - TargetHosts(name string) ([]config.Destination, error) + TargetHosts(name string, adhocProxyCommand string) ([]config.Destination, error) AllSecretValues() []string UpdateTasksTargets(vars map[string]string) UpdateRegisteredVars(vars map[string]string) @@ -87,7 +99,12 @@ func (p *Process) Run(ctx context.Context, task, target string) (s ProcResp, err allVars := make(map[string]string) allRegistered := make(map[string]string) - targetHosts, err := p.Playbook.TargetHosts(target) + + // If target here represents hostname and not the name of target in the Playbook, it is possible that target + // requires ProxyCommand to connect. Such target will need to pass AdhocProxyCommand as cli argument `--proxy-command` + // and it's value is passed down here. + targetHosts, err := p.Playbook.TargetHosts(target, p.AdhocProxyCommand) + if err != nil { return ProcResp{}, fmt.Errorf("can't get target %s: %w", target, err) } @@ -110,7 +127,8 @@ func (p *Process) Run(ctx context.Context, task, target string) (s ProcResp, err if tsk.User != "" { user = tsk.User // override user from task if any set } - resp, e := p.runTaskOnHost(ctx, tsk, fmt.Sprintf("%s:%d", host.Host, host.Port), host.Name, user) + + resp, e := p.runTaskOnHost(ctx, tsk, fmt.Sprintf("%s:%d", host.Host, host.Port), host.Name, user, host.ProxyCommandParsed) if i == 0 { atomic.AddInt32(&commands, int32(resp.count)) } @@ -151,7 +169,7 @@ func (p *Process) Gen(targets []string, tmplRdr io.Reader, respWr io.Writer) err targetHosts := []config.Destination{} for _, target := range targets { - hosts, err := p.Playbook.TargetHosts(target) + hosts, err := p.Playbook.TargetHosts(target, p.AdhocProxyCommand) if err != nil { return fmt.Errorf("can't get target %s: %w", target, err) } @@ -182,7 +200,7 @@ func (p *Process) Gen(targets []string, tmplRdr io.Reader, respWr io.Writer) err // runTaskOnHost executes all commands of a task on a target host. hostAddr can be a remote host or localhost with port. // returns number of executed commands, vars from all commands and error if any. -func (p *Process) runTaskOnHost(ctx context.Context, tsk *config.Task, hostAddr, hostName, user string) (taskOnHostResp, error) { +func (p *Process) runTaskOnHost(ctx context.Context, tsk *config.Task, hostAddr, hostName, user string, proxyCommandParsed []string) (taskOnHostResp, error) { report := func(hostAddr, hostName, f string, vals ...any) { p.Logs.WithHost(hostAddr, hostName).Info.Printf(f, vals...) } @@ -194,13 +212,27 @@ func (p *Process) runTaskOnHost(ctx context.Context, tsk *config.Task, hostAddr, if p.anyRemoteCommand(tsk) && !p.Local { // make remote executor only if there is a remote command in the task and not in local mode var err error - remote, err = p.Connector.Connect(ctx, hostAddr, hostName, user) - if err != nil { - if hostName != "" { - return taskOnHostResp{}, fmt.Errorf("can't connect to %s, user: %s: %w", hostName, user, err) + + if len(proxyCommandParsed) == 0 { + remote, err = p.Connector.Connect(ctx, hostAddr, hostName, user) + if err != nil { + if hostName != "" { + return taskOnHostResp{}, fmt.Errorf("can't connect to %s, user: %s: %w", hostName, user, err) + } + return taskOnHostResp{}, err + } + } + + if len(proxyCommandParsed) > 0 { + remote, err = p.Connector.ConnectWithProxy(ctx, hostAddr, hostName, user, proxyCommandParsed) + if err != nil { + if hostName != "" { + return taskOnHostResp{}, fmt.Errorf("can't connect through proxy command %s to %s, user: %s: %w", proxyCommandParsed, hostName, user, err) + } + return taskOnHostResp{}, err } - return taskOnHostResp{}, err } + defer remote.Close() report(hostAddr, hostName, "run task %q, commands: %d\n", tsk.Name, len(tsk.Commands)) } else { diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index e7fce522..5ce742c4 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -1123,10 +1123,10 @@ func Test_shouldRunCmd(t *testing.T) { func TestGen(t *testing.T) { mockPbook := &mocks.PlaybookMock{ - TargetHostsFunc: func(string) ([]config.Destination, error) { + TargetHostsFunc: func(string, string) ([]config.Destination, error) { return []config.Destination{ - {Name: "test1", Host: "host1", Port: 8080, User: "user1", Tags: []string{"tag1", "tag2"}}, - {Name: "test2", Host: "host2", Port: 8081, User: "user2", Tags: []string{"tag3", "tag4"}}, + {Name: "test1", Host: "host1", Port: 8080, User: "user1", Tags: []string{"tag1", "tag2"}, ProxyCommand: "ssh bastion1 -W %h:%p", ProxyCommandParsed: []string{"ssh", "jump1", "-W", "%h:%p"}}, + {Name: "test2", Host: "host2", Port: 8081, User: "user2", Tags: []string{"tag3", "tag4"}, ProxyCommand: "ssh bastion1 -W %h:%p", ProxyCommandParsed: []string{"ssh", "jump1", "-W", "%h:%p"}}, }, nil }, } @@ -1152,6 +1152,13 @@ func TestGen(t *testing.T) { wantErr: false, want: "test1, host1, 8080, user1test2, host2, 8081, user2", }, + { + name: "multiple fields, and proxy command ", + target: "test", + tmplInput: `{{range .}}{{.Name}}, {{.Host}}, {{.Port}}, {{.User}}, {{.ProxyCommand}}{{end}}`, + wantErr: false, + want: "test1, host1, 8080, user1, ssh bastion1 -W %h:%ptest2, host2, 8081, user2, ssh bastion1 -W %h:%p", + }, { name: "invalid template", target: "test", @@ -1219,8 +1226,8 @@ func TestRegisteredVarTemplateSubstitution(t *testing.T) { TaskFunc: func(name string) (*config.Task, error) { return conf.Task(name) }, - TargetHostsFunc: func(name string) ([]config.Destination, error) { - return conf.TargetHosts(name) + TargetHostsFunc: func(name string, adhocProxyCommand string) ([]config.Destination, error) { + return conf.TargetHosts(name, adhocProxyCommand) }, AllSecretValuesFunc: conf.AllSecretValues, UpdateTasksTargetsFunc: conf.UpdateTasksTargets, diff --git a/pkg/runner/testdata/test_ssh_key b/pkg/runner/testdata/test_ssh_key old mode 100644 new mode 100755 From 9295661064ce70845844b1e19b6b06406c3aecc7 Mon Sep 17 00:00:00 2001 From: Vladimir Date: Mon, 27 Oct 2025 15:34:50 +0100 Subject: [PATCH 2/2] Missed that Connector is not unique per SSH target and the proxy command will be overwritten if Spot starts many concurrent connections to targets that have different proxy commands. To fix that, moving the proxy command to sshClient(). --- pkg/executor/connector.go | 39 ++++++++++++++------------------------- 1 file changed, 14 insertions(+), 25 deletions(-) diff --git a/pkg/executor/connector.go b/pkg/executor/connector.go index cbf37f79..a06da0fe 100644 --- a/pkg/executor/connector.go +++ b/pkg/executor/connector.go @@ -21,9 +21,6 @@ type Connector struct { timeout time.Duration enableAgent bool enableAgentForwarding bool - enableProxy bool - proxyCommandParsed []string - stopProxyCommand context.CancelFunc logs Logs } @@ -84,26 +81,23 @@ func (c *Connector) WithAgentForwarding() *Connector { // Connect connects to a remote hostAddr and returns a remote executer, caller must close. func (c *Connector) Connect(ctx context.Context, hostAddr, hostName, user string) (*Remote, error) { log.Printf("[DEBUG] connect to %q (%s), user %q", hostAddr, hostName, user) - client, err := c.sshClient(ctx, hostAddr, user) + client, _, err := c.sshClient(ctx, hostAddr, user, nil) if err != nil { return nil, err } return &Remote{client: client, hostAddr: hostAddr, hostName: hostName, logs: c.logs.WithHost(hostAddr, hostName)}, nil } -// ConnectWithProxy saves values in Connector necessary to execute SSH ProxyCommand, see https://man.openbsd.org/ssh_config.5#ProxyCommand +// ConnectWithProxy connects to a remote host through a proxy command and returns a remote executer, caller must close. func (c *Connector) ConnectWithProxy(ctx context.Context, hostAddr, hostName, user string, proxyCommandParsed []string) (*Remote, error) { log.Printf("[DEBUG] connect to %q (%s), user %q, proxy command: %s", hostAddr, hostName, user, proxyCommandParsed) - c.proxyCommandParsed = proxyCommandParsed - c.enableProxy = true - - client, err := c.sshClient(ctx, hostAddr, user) + client, stopProxyCommand, err := c.sshClient(ctx, hostAddr, user, proxyCommandParsed) if err != nil { return nil, err } - return &Remote{client: client, hostAddr: hostAddr, hostName: hostName, logs: c.logs.WithHost(hostAddr, hostName), stopProxyCommand: c.stopProxyCommand}, nil + return &Remote{client: client, hostAddr: hostAddr, hostName: hostName, logs: c.logs.WithHost(hostAddr, hostName), stopProxyCommand: stopProxyCommand}, nil } func (c *Connector) forwardAgent(client *ssh.Client) error { @@ -264,7 +258,7 @@ func (c *Connector) dialWithProxy(ctx context.Context, host string, cmdArgs []st return sshClient, cancelCopy, nil } -func (c *Connector) sshClient(ctx context.Context, host, user string) (session *ssh.Client, err error) { +func (c *Connector) sshClient(ctx context.Context, host, user string, proxyCommandParsed []string) (*ssh.Client, context.CancelFunc, error) { var client *ssh.Client var stopProxyCommand context.CancelFunc @@ -275,37 +269,32 @@ func (c *Connector) sshClient(ctx context.Context, host, user string) (session * conf, err := c.sshConfig(user, c.privateKey) if err != nil { - return nil, fmt.Errorf("failed to create ssh config: %w", err) + return nil, nil, fmt.Errorf("failed to create ssh config: %w", err) } - if !c.enableProxy { + if len(proxyCommandParsed) == 0 { client, err = c.dial(ctx, host, conf) if err != nil { - return nil, err + return nil, nil, err } - } - - if c.enableProxy { - cmdArgs, err := substituteProxyCommand(user, host, c.proxyCommandParsed) + } else { + cmdArgs, err := substituteProxyCommand(user, host, proxyCommandParsed) if err != nil { - return nil, fmt.Errorf("failed to substitute proxy command with target host values: %w", err) + return nil, nil, fmt.Errorf("failed to substitute proxy command with target host values: %w", err) } client, stopProxyCommand, err = c.dialWithProxy(ctx, host, cmdArgs, conf) if err != nil { - return nil, fmt.Errorf("failed to create client connection wtth proxy command %s, to %s: %v", cmdArgs, host, err) - } - if stopProxyCommand != nil { - c.stopProxyCommand = stopProxyCommand + return nil, nil, fmt.Errorf("failed to create client connection wtth proxy command %s, to %s: %v", cmdArgs, host, err) } } if err := c.forwardAgent(client); err != nil { - return nil, fmt.Errorf("failed to forward agent to %s: %v", host, err) + return nil, nil, fmt.Errorf("failed to forward agent to %s: %v", host, err) } log.Printf("[DEBUG] ssh session created to %s", host) - return client, nil + return client, stopProxyCommand, nil } func (c *Connector) sshConfig(user, privateKeyPath string) (*ssh.ClientConfig, error) {