Merge pull request #70940 from mikedanese/sshcleanup

cleanup pkg/ssh

pkg/master/tunneler/BUILD

@@ -25,7 +25,6 @@ go_library(
         "//pkg/util/file:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/clock:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
-        "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library",
         "//vendor/k8s.io/klog:go_default_library",
     ],
 )

pkg/master/tunneler/ssh.go

@@ -29,11 +29,9 @@ import (
 
 	"k8s.io/apimachinery/pkg/util/clock"
 	"k8s.io/apimachinery/pkg/util/wait"
+	"k8s.io/klog"
 	"k8s.io/kubernetes/pkg/ssh"
 	utilfile "k8s.io/kubernetes/pkg/util/file"
-
-	"github.com/prometheus/client_golang/prometheus"
-	"k8s.io/klog"
 )
 
 type InstallSSHKey func(ctx context.Context, user string, data []byte) error
@@ -83,9 +81,8 @@ type SSHTunneler struct {
 	InstallSSHKey  InstallSSHKey
 	HealthCheckURL *url.URL
 
-	tunnels        *ssh.SSHTunnelList
+	tunnels *ssh.SSHTunnelList
-	lastSyncMetric prometheus.GaugeFunc
+	clock   clock.Clock
-	clock          clock.Clock
 
 	getAddresses AddressFunc
 	stopChan     chan struct{}

pkg/master/tunneler/ssh_test.go

@@ -25,9 +25,9 @@ import (
 	"testing"
 	"time"
 
-	"k8s.io/apimachinery/pkg/util/clock"
-
 	"github.com/stretchr/testify/assert"
+
+	"k8s.io/apimachinery/pkg/util/clock"
 )
 
 // TestSecondsSinceSync verifies that proper results are returned

pkg/ssh/ssh.go

@@ -26,7 +26,6 @@ import (
 	"encoding/pem"
 	"errors"
 	"fmt"
-	"io"
 	"io/ioutil"
 	mathrand "math/rand"
 	"net"
@@ -39,11 +38,11 @@ import (
 
 	"github.com/prometheus/client_golang/prometheus"
 	"golang.org/x/crypto/ssh"
-	"k8s.io/klog"
 
 	utilnet "k8s.io/apimachinery/pkg/util/net"
 	"k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
+	"k8s.io/klog"
 )
 
 var (
@@ -68,51 +67,27 @@ func init() {
 
 // TODO: Unit tests for this code, we can spin up a test SSH server with instructions here:
 // https://godoc.org/golang.org/x/crypto/ssh#ServerConn
-type SSHTunnel struct {
+type sshTunnel struct {
 	Config  *ssh.ClientConfig
 	Host    string
 	SSHPort string
-	running bool
-	sock    net.Listener
 	client  *ssh.Client
 }
 
-func (s *SSHTunnel) copyBytes(out io.Writer, in io.Reader) {
+func makeSSHTunnel(user string, signer ssh.Signer, host string) (*sshTunnel, error) {
-	if _, err := io.Copy(out, in); err != nil {
-		klog.Errorf("Error in SSH tunnel: %v", err)
-	}
-}
-
-func NewSSHTunnel(user, keyfile, host string) (*SSHTunnel, error) {
-	signer, err := MakePrivateKeySignerFromFile(keyfile)
-	if err != nil {
-		return nil, err
-	}
-	return makeSSHTunnel(user, signer, host)
-}
-
-func NewSSHTunnelFromBytes(user string, privateKey []byte, host string) (*SSHTunnel, error) {
-	signer, err := MakePrivateKeySignerFromBytes(privateKey)
-	if err != nil {
-		return nil, err
-	}
-	return makeSSHTunnel(user, signer, host)
-}
-
-func makeSSHTunnel(user string, signer ssh.Signer, host string) (*SSHTunnel, error) {
 	config := ssh.ClientConfig{
 		User:            user,
 		Auth:            []ssh.AuthMethod{ssh.PublicKeys(signer)},
 		HostKeyCallback: ssh.InsecureIgnoreHostKey(),
 	}
-	return &SSHTunnel{
+	return &sshTunnel{
 		Config:  &config,
 		Host:    host,
 		SSHPort: "22",
 	}, nil
 }
 
-func (s *SSHTunnel) Open() error {
+func (s *sshTunnel) Open() error {
 	var err error
 	s.client, err = realTimeoutDialer.Dial("tcp", net.JoinHostPort(s.Host, s.SSHPort), s.Config)
 	tunnelOpenCounter.Inc()
@@ -122,7 +97,7 @@ func (s *SSHTunnel) Open() error {
 	return err
 }
 
-func (s *SSHTunnel) Dial(ctx context.Context, network, address string) (net.Conn, error) {
+func (s *sshTunnel) Dial(ctx context.Context, network, address string) (net.Conn, error) {
 	if s.client == nil {
 		return nil, errors.New("tunnel is not opened.")
 	}
@@ -130,20 +105,7 @@ func (s *SSHTunnel) Dial(ctx context.Context, network, address string) (net.Conn
 	return s.client.Dial(network, address)
 }
 
-func (s *SSHTunnel) tunnel(conn net.Conn, remoteHost, remotePort string) error {
+func (s *sshTunnel) Close() error {
-	if s.client == nil {
-		return errors.New("tunnel is not opened.")
-	}
-	tunnel, err := s.client.Dial("tcp", net.JoinHostPort(remoteHost, remotePort))
-	if err != nil {
-		return err
-	}
-	go s.copyBytes(tunnel, conn)
-	go s.copyBytes(conn, tunnel)
-	return nil
-}
-
-func (s *SSHTunnel) Close() error {
 	if s.client == nil {
 		return errors.New("Cannot close tunnel. Tunnel was not opened.")
 	}
@@ -305,13 +267,17 @@ type sshTunnelEntry struct {
 }
 
 type sshTunnelCreator interface {
-	NewSSHTunnel(user, keyFile, healthCheckURL string) (tunnel, error)
+	newSSHTunnel(user, keyFile, host string) (tunnel, error)
 }
 
 type realTunnelCreator struct{}
 
-func (*realTunnelCreator) NewSSHTunnel(user, keyFile, healthCheckURL string) (tunnel, error) {
+func (*realTunnelCreator) newSSHTunnel(user, keyFile, host string) (tunnel, error) {
-	return NewSSHTunnel(user, keyFile, healthCheckURL)
+	signer, err := MakePrivateKeySignerFromFile(keyFile)
+	if err != nil {
+		return nil, err
+	}
+	return makeSSHTunnel(user, signer, host)
 }
 
 type SSHTunnelList struct {
@@ -481,7 +447,7 @@ func (l *SSHTunnelList) Update(addrs []string) {
 
 func (l *SSHTunnelList) createAndAddTunnel(addr string) {
 	klog.Infof("Trying to add tunnel to %q", addr)
-	tunnel, err := l.tunnelCreator.NewSSHTunnel(l.user, l.keyfile, addr)
+	tunnel, err := l.tunnelCreator.newSSHTunnel(l.user, l.keyfile, addr)
 	if err != nil {
 		klog.Errorf("Failed to create tunnel for %q: %v", addr, err)
 		return

pkg/ssh/ssh_test.go

@@ -27,9 +27,9 @@ import (
 	"testing"
 	"time"
 
-	"k8s.io/apimachinery/pkg/util/wait"
-
 	"golang.org/x/crypto/ssh"
+
+	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/klog"
 )
 
@@ -134,7 +134,7 @@ func TestSSHTunnel(t *testing.T) {
 	}
 
 	privateData := EncodePrivateKey(private)
-	tunnel, err := NewSSHTunnelFromBytes("foo", privateData, server.Host)
+	tunnel, err := newSSHTunnelFromBytes("foo", privateData, server.Host)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 		t.FailNow()
@@ -183,7 +183,7 @@ func (*fakeTunnel) Dial(ctx context.Context, network, address string) (net.Conn,
 
 type fakeTunnelCreator struct{}
 
-func (*fakeTunnelCreator) NewSSHTunnel(string, string, string) (tunnel, error) {
+func (*fakeTunnelCreator) newSSHTunnel(string, string, string) (tunnel, error) {
 	return &fakeTunnel{}, nil
 }
 
@@ -355,3 +355,11 @@ func TestTimeoutDialer(t *testing.T) {
 
 	listener.Close()
 }
+
+func newSSHTunnelFromBytes(user string, privateKey []byte, host string) (*sshTunnel, error) {
+	signer, err := MakePrivateKeySignerFromBytes(privateKey)
+	if err != nil {
+		return nil, err
+	}
+	return makeSSHTunnel(user, signer, host)
+}