Merge branch 'fj-remove-terminal-references' into 'master'

Remove redirections from Terminal to Channel

Closes #214

See merge request gitlab-org/gitlab-workhorse!397

channel_test.go

@@ -74,50 +74,6 @@ func TestChannelHappyPath(t *testing.T) {
 	}
 }
 
-func TestChannelHappyPathWithTerminalResponse(t *testing.T) {
-	tests := []struct {
-		name        string
-		channelPath string
-	}{
-		{"environments", envTerminalPath},
-		{"jobs", jobTerminalPath},
-		{"services", servicesProxyWSPath},
-	}
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			serverConns, clientURL, close := wireupTerminal(test.channelPath, nil, "channel.k8s.io")
-			defer close()
-
-			client, _, err := dialWebsocket(clientURL, nil, "terminal.gitlab.com")
-			if err != nil {
-				t.Fatal(err)
-			}
-
-			server := (<-serverConns).conn
-			defer server.Close()
-
-			message := "test message"
-
-			// channel.k8s.io: server writes to channel 1, STDOUT
-			if err := say(server, "\x01"+message); err != nil {
-				t.Fatal(err)
-			}
-			assertReadMessage(t, client, websocket.BinaryMessage, message)
-
-			if err := say(client, message); err != nil {
-				t.Fatal(err)
-			}
-
-			// channel.k8s.io: client writes get put on channel 0, STDIN
-			assertReadMessage(t, server, websocket.BinaryMessage, "\x00"+message)
-
-			// Closing the client should send an EOT signal to the server's STDIN
-			client.Close()
-			assertReadMessage(t, server, websocket.BinaryMessage, "\x00\x04")
-		})
-	}
-}
-
 func TestChannelBadTLS(t *testing.T) {
 	_, clientURL, close := wireupChannel(envTerminalPath, badCA, "channel.k8s.io")
 	defer close()
@@ -211,22 +167,6 @@ func wireupChannel(channelPath string, modifier func(*api.Response), subprotocol
 	}
 }
 
-func wireupTerminal(terminalPath string, modifier func(*api.Response), subprotocols ...string) (chan connWithReq, string, func()) {
-	serverConns, remote := startWebsocketServer(subprotocols...)
-	authResponse := terminalOkBody(remote, nil, subprotocols...)
-	if modifier != nil {
-		modifier(authResponse)
-	}
-	upstream := testAuthServer(nil, 200, authResponse)
-	workhorse := startWorkhorseServer(upstream.URL)
-
-	return serverConns, websocketURL(workhorse.URL, terminalPath), func() {
-		workhorse.Close()
-		upstream.Close()
-		remote.Close()
-	}
-}
-
 func startWebsocketServer(subprotocols ...string) (chan connWithReq, *httptest.Server) {
 	upgrader := &websocket.Upgrader{Subprotocols: subprotocols}
 
@@ -264,25 +204,6 @@ func channelOkBody(remote *httptest.Server, header http.Header, subprotocols ...
 	return out
 }
 
-func terminalOkBody(remote *httptest.Server, header http.Header, subprotocols ...string) *api.Response {
-	out := &api.Response{
-		Terminal: &api.TerminalSettings{
-			Url:            websocketURL(remote.URL),
-			Header:         header,
-			Subprotocols:   subprotocols,
-			MaxSessionTime: 0,
-		},
-	}
-
-	if len(remote.TLS.Certificates) > 0 {
-		data := bytes.NewBuffer(nil)
-		pem.Encode(data, &pem.Block{Type: "CERTIFICATE", Bytes: remote.TLS.Certificates[0].Certificate[0]})
-		out.Terminal.CAPem = data.String()
-	}
-
-	return out
-}
-
 func badCA(authResponse *api.Response) {
 	authResponse.Channel.CAPem = "Bad CA"
 }

internal/api/api.go

@@ -125,9 +125,6 @@ type Response struct {
 	Entry string `json:"entry"`
 	// Used to communicate channel session details
 	Channel *ChannelSettings
-	// Used to communicate terminal session details (Deprecated)
-	// Issue to remove this field https://gitlab.com/gitlab-org/gitlab-workhorse/issues/214
-	Terminal *TerminalSettings
 	// GitalyServer specifies an address and authentication token for a gitaly server we should connect to.
 	GitalyServer gitaly.Server
 	// Repository object for making gRPC requests to Gitaly.

internal/api/terminal_settings.go

-package api
-
-import (
-	"crypto/tls"
-	"crypto/x509"
-	"net/http"
-	"net/url"
-
-	"github.com/gorilla/websocket"
-
-	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
-)
-
-type TerminalSettings struct {
-	// The channel provider may require use of a particular subprotocol. If so,
-	// it must be specified here, and Workhorse must have a matching codec.
-	Subprotocols []string
-
-	// The websocket URL to connect to.
-	Url string
-
-	// Any headers (e.g., Authorization) to send with the websocket request
-	Header http.Header
-
-	// The CA roots to validate the remote endpoint with, for wss:// URLs. The
-	// system-provided CA pool will be used if this is blank. PEM-encoded data.
-	CAPem string
-
-	// The value is specified in seconds. It is converted to time.Duration
-	// later.
-	MaxSessionTime int
-}
-
-func (t *TerminalSettings) URL() (*url.URL, error) {
-	return url.Parse(t.Url)
-}
-
-func (t *TerminalSettings) Dialer() *websocket.Dialer {
-	dialer := &websocket.Dialer{
-		Subprotocols: t.Subprotocols,
-	}
-
-	if len(t.CAPem) > 0 {
-		pool := x509.NewCertPool()
-		pool.AppendCertsFromPEM([]byte(t.CAPem))
-		dialer.TLSClientConfig = &tls.Config{RootCAs: pool}
-	}
-
-	return dialer
-}
-
-func (t *TerminalSettings) Clone() *TerminalSettings {
-	// Doesn't clone the strings, but that's OK as strings are immutable in go
-	cloned := *t
-	cloned.Header = helper.HeaderClone(t.Header)
-	return &cloned
-}
-
-func (t *TerminalSettings) Dial() (*websocket.Conn, *http.Response, error) {
-	return t.Dialer().Dial(t.Url, t.Header)
-}
-
-func (t *TerminalSettings) Channel() *ChannelSettings {
-	return &ChannelSettings{
-		Subprotocols:   t.Subprotocols,
-		Url:            t.Url,
-		CAPem:          t.CAPem,
-		MaxSessionTime: t.MaxSessionTime,
-	}
-}

internal/channel/channel.go

@@ -22,14 +22,6 @@ var (
 
 func Handler(myAPI *api.API) http.Handler {
 	return myAPI.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) {
-		// Used during the transition from Terminal to Channel
-		// Once we remove the TerminalSettings object we can remove
-		// this condition
-		// https://gitlab.com/gitlab-org/gitlab-workhorse/issues/214
-		if a.Terminal != nil {
-			a.Channel = a.Terminal.Channel()
-		}
-
 		if err := a.Channel.Validate(); err != nil {
 			helper.Fail500(w, r, err)
 			return