diff --git a/tool/tctl/common/auth_command_test.go b/tool/tctl/common/auth_command_test.go index c8707034b76a3..e3252d5afa38c 100644 --- a/tool/tctl/common/auth_command_test.go +++ b/tool/tctl/common/auth_command_test.go @@ -15,13 +15,12 @@ package common import ( - "bytes" "context" "crypto/x509/pkix" "encoding/json" - "net" "net/http" "net/http/httptest" + "net/url" "os" "path/filepath" "testing" @@ -32,6 +31,7 @@ import ( "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/client/webclient" + "github.com/gravitational/teleport/api/fixtures" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/auth" "github.com/gravitational/teleport/lib/client" @@ -45,29 +45,18 @@ import ( ) func TestAuthSignKubeconfig(t *testing.T) { - // create a HTTPS_PROXY endpoint that intercepts the proxy Ping request - // and returns a mock response - // We need to do this because the Ping request is made using a custom - // http.Transport and we can't use a custom dialer to intercept the request. - t.Setenv("HTTPS_PROXY", newHTTPSProxy(t)) - - tmpDir := t.TempDir() + pingTestServer := httptest.NewTLSServer(&pingSrv{}) + t.Cleanup(func() { pingTestServer.Close() }) clusterName, err := services.NewClusterNameWithRandomID(types.ClusterNameSpecV2{ ClusterName: "example.com", }) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) remoteCluster, err := types.NewRemoteCluster("leaf.example.com") - if err != nil { - t.Fatal(err) - } - - _, cert, err := tlsca.GenerateSelfSignedCA(pkix.Name{CommonName: "example.com"}, nil, time.Minute) require.NoError(t, err) + cert := []byte(fixtures.TLSCACertPEM) ca, err := types.NewCertAuthority(types.CertAuthoritySpecV2{ Type: types.HostCA, ClusterName: "example.com", @@ -77,35 +66,74 @@ func TestAuthSignKubeconfig(t *testing.T) { }, }) require.NoError(t, err) - - separatedCluster := &mockClient{ - clusterName: clusterName, - remoteClusters: []types.RemoteCluster{remoteCluster}, - userCerts: &proto.Certs{ - SSH: []byte("SSH cert"), - TLS: cert, - TLSCACerts: [][]byte{ - cert, + // newSeparatedCluster returns a mockClient that simulates a cluster with + // a separate proxy. + // We create a separate cluster per test because it's not safe to use it in + // parallel. + newSeparatedCluster := func() *mockClient { + return &mockClient{ + clusterName: clusterName, + remoteClusters: []types.RemoteCluster{remoteCluster}, + userCerts: &proto.Certs{ + SSH: []byte("SSH cert"), + TLS: cert, + TLSCACerts: [][]byte{ + cert, + }, }, - }, - cas: []types.CertAuthority{ca}, - proxies: []types.Server{ - &types.ServerV2{ - Kind: types.KindNode, - Version: types.V2, - Metadata: types.Metadata{ - Name: "proxy", + cas: []types.CertAuthority{ca}, + proxies: []types.Server{ + &types.ServerV2{ + Kind: types.KindNode, + Version: types.V2, + Metadata: types.Metadata{ + Name: "proxy", + }, + Spec: types.ServerSpecV2{ + // This is the address that will be used by the client to call + // the proxy ping endpoint. This is not the address that will be + // used in the kubeconfig server address. + PublicAddr: mustGetHost(t, pingTestServer.URL), + }, + }, + }, + } + } + // newMultiplexCluster returns a mockClient that simulates a cluster with + // a multiplex proxy. + // We create a separate cluster per test because it's not safe to use it in + // parallel. + newMultiplexCluster := func() *mockClient { + return &mockClient{ + clusterName: clusterName, + remoteClusters: []types.RemoteCluster{remoteCluster}, + networkConfig: &types.ClusterNetworkingConfigV2{ + Spec: types.ClusterNetworkingConfigSpecV2{ + ProxyListenerMode: types.ProxyListenerMode_Multiplex, }, - Spec: types.ServerSpecV2{ - PublicAddrs: []string{"proxy-from-api.example.com:3080"}, + }, + userCerts: &proto.Certs{ + SSH: []byte("SSH cert"), + TLS: cert, + TLSCACerts: [][]byte{ + cert, }, }, - }, + cas: []types.CertAuthority{ca}, + proxies: []types.Server{ + &types.ServerV2{ + Kind: types.KindNode, + Version: types.V2, + Metadata: types.Metadata{ + Name: "proxy", + }, + Spec: types.ServerSpecV2{ + PublicAddr: "proxy-from-api.example.com:3080", + }, + }, + }, + } } - multiplexCluster := *separatedCluster - multiplexCluster.networkConfig = &types.ClusterNetworkingConfigV2{} - multiplexCluster.networkConfig.SetProxyListenerMode(types.ProxyListenerMode_Multiplex) - tests := []struct { desc string ac AuthCommand @@ -116,9 +144,9 @@ func TestAuthSignKubeconfig(t *testing.T) { }{ { desc: "valid --proxy URL with valid URL scheme", - client: separatedCluster, + client: newSeparatedCluster(), ac: AuthCommand{ - output: filepath.Join(tmpDir, "kubeconfig"), + output: filepath.Join(t.TempDir(), "kubeconfig"), outputFormat: identityfile.FormatKubernetes, signOverwrite: true, proxyAddr: "https://proxy-from-flag.example.com", @@ -128,9 +156,9 @@ func TestAuthSignKubeconfig(t *testing.T) { }, { desc: "valid --proxy URL with invalid URL scheme", - client: separatedCluster, + client: newSeparatedCluster(), ac: AuthCommand{ - output: filepath.Join(tmpDir, "kubeconfig"), + output: filepath.Join(t.TempDir(), "kubeconfig"), outputFormat: identityfile.FormatKubernetes, signOverwrite: true, proxyAddr: "file://proxy-from-flag.example.com", @@ -142,9 +170,9 @@ func TestAuthSignKubeconfig(t *testing.T) { }, { desc: "valid --proxy URL without URL scheme", - client: separatedCluster, + client: newSeparatedCluster(), ac: AuthCommand{ - output: filepath.Join(tmpDir, "kubeconfig"), + output: filepath.Join(t.TempDir(), "kubeconfig"), outputFormat: identityfile.FormatKubernetes, signOverwrite: true, proxyAddr: "proxy-from-flag.example.com", @@ -154,9 +182,9 @@ func TestAuthSignKubeconfig(t *testing.T) { }, { desc: "invalid --proxy URL", - client: separatedCluster, + client: newSeparatedCluster(), ac: AuthCommand{ - output: filepath.Join(tmpDir, "kubeconfig"), + output: filepath.Join(t.TempDir(), "kubeconfig"), outputFormat: identityfile.FormatKubernetes, signOverwrite: true, proxyAddr: "1https://proxy-from-flag.example.com", @@ -168,9 +196,9 @@ func TestAuthSignKubeconfig(t *testing.T) { }, { desc: "k8s proxy running locally with public_addr", - client: separatedCluster, + client: newSeparatedCluster(), ac: AuthCommand{ - output: filepath.Join(tmpDir, "kubeconfig"), + output: filepath.Join(t.TempDir(), "kubeconfig"), outputFormat: identityfile.FormatKubernetes, signOverwrite: true, config: &servicecfg.Config{Proxy: servicecfg.ProxyConfig{Kube: servicecfg.KubeProxyConfig{ @@ -183,9 +211,9 @@ func TestAuthSignKubeconfig(t *testing.T) { }, { desc: "k8s proxy running locally without public_addr", - client: separatedCluster, + client: newSeparatedCluster(), ac: AuthCommand{ - output: filepath.Join(tmpDir, "kubeconfig"), + output: filepath.Join(t.TempDir(), "kubeconfig"), outputFormat: identityfile.FormatKubernetes, signOverwrite: true, config: &servicecfg.Config{Proxy: servicecfg.ProxyConfig{ @@ -200,9 +228,9 @@ func TestAuthSignKubeconfig(t *testing.T) { }, { desc: "k8s proxy from cluster info", - client: separatedCluster, + client: newSeparatedCluster(), ac: AuthCommand{ - output: filepath.Join(tmpDir, "kubeconfig"), + output: filepath.Join(t.TempDir(), "kubeconfig"), outputFormat: identityfile.FormatKubernetes, signOverwrite: true, config: &servicecfg.Config{Proxy: servicecfg.ProxyConfig{ @@ -217,9 +245,9 @@ func TestAuthSignKubeconfig(t *testing.T) { }, { desc: "--kube-cluster specified with valid cluster", - client: separatedCluster, + client: newSeparatedCluster(), ac: AuthCommand{ - output: filepath.Join(tmpDir, "kubeconfig"), + output: filepath.Join(t.TempDir(), "kubeconfig"), outputFormat: identityfile.FormatKubernetes, signOverwrite: true, leafCluster: remoteCluster.GetMetadata().Name, @@ -232,12 +260,13 @@ func TestAuthSignKubeconfig(t *testing.T) { }, wantCluster: remoteCluster.GetMetadata().Name, assertErr: require.NoError, + wantAddr: "https://proxy-from-api.example.com:3060", }, { desc: "--kube-cluster specified with invalid cluster", - client: separatedCluster, + client: newSeparatedCluster(), ac: AuthCommand{ - output: filepath.Join(tmpDir, "kubeconfig"), + output: filepath.Join(t.TempDir(), "kubeconfig"), outputFormat: identityfile.FormatKubernetes, signOverwrite: true, leafCluster: "doesnotexist.example.com", @@ -255,9 +284,9 @@ func TestAuthSignKubeconfig(t *testing.T) { }, { desc: "k8s proxy running locally in multiplex mode without public_addr", - client: &multiplexCluster, + client: newMultiplexCluster(), ac: AuthCommand{ - output: filepath.Join(tmpDir, "kubeconfig"), + output: filepath.Join(t.TempDir(), "kubeconfig"), outputFormat: identityfile.FormatKubernetes, signOverwrite: true, config: &servicecfg.Config{ @@ -277,10 +306,10 @@ func TestAuthSignKubeconfig(t *testing.T) { assertErr: require.NoError, }, { - desc: "k8s proxy from cluster info", - client: &multiplexCluster, + desc: "k8s proxy from cluster info with multiplex mode", + client: newMultiplexCluster(), ac: AuthCommand{ - output: filepath.Join(tmpDir, "kubeconfig"), + output: filepath.Join(t.TempDir(), "kubeconfig"), outputFormat: identityfile.FormatKubernetes, signOverwrite: true, config: &servicecfg.Config{Proxy: servicecfg.ProxyConfig{ @@ -295,65 +324,37 @@ func TestAuthSignKubeconfig(t *testing.T) { }, } for _, tt := range tests { + tt := tt t.Run(tt.desc, func(t *testing.T) { + t.Parallel() // Generate kubeconfig. err := tt.ac.generateUserKeys(context.Background(), tt.client) tt.assertErr(t, err) - - // Validate kubeconfig contents. - kc, err := kubeconfig.Load(tt.ac.output) + // error is already asserted, so we can return early. if err != nil { - t.Fatalf("loading generated kubeconfig: %v", err) + return } + // Validate kubeconfig contents. + kc, err := kubeconfig.Load(tt.ac.output) + require.NoError(t, err) currentCtx, ok := kc.Contexts[kc.CurrentContext] - if !ok { - t.Fatalf("currentContext %q not present in kubeconfig", kc.CurrentContext) - } + require.Truef(t, ok, "currentContext %q not present in kubeconfig", kc.CurrentContext) gotCert := kc.AuthInfos[currentCtx.AuthInfo].ClientCertificateData - if !bytes.Equal(gotCert, tt.client.userCerts.TLS) { - t.Errorf("got client cert: %q, want %q", gotCert, tt.client.userCerts.TLS) - } + require.Equal(t, gotCert, tt.client.userCerts.TLS, "client certs not equal") gotCA := kc.Clusters[currentCtx.Cluster].CertificateAuthorityData wantCA := ca.GetActiveKeys().TLS[0].Cert - if !bytes.Equal(gotCA, wantCA) { - t.Errorf("got CA cert: %q, want %q", gotCA, wantCA) - } + require.Equal(t, wantCA, gotCA, "CA certs not equal") gotServerAddr := kc.Clusters[currentCtx.Cluster].Server - if tt.wantAddr != "" && gotServerAddr != tt.wantAddr { - t.Errorf("got server address: %q, want %q", gotServerAddr, tt.wantAddr) - } - if tt.wantCluster != "" && kc.CurrentContext != tt.wantCluster { - t.Errorf("got cluster: %q, want %q", kc.CurrentContext, tt.wantCluster) - } + require.Equal(t, tt.wantAddr, gotServerAddr, "server address not equal") }) } } -// proxyHandler is a simple HTTP handler that proxies all requests to the -// upstreamAddr. -type proxyHandler struct { - upstreamAddr string -} - -func (p *proxyHandler) ServeHTTP(wr http.ResponseWriter, req *http.Request) { - dest_conn, err := net.DialTimeout("tcp", p.upstreamAddr, 10*time.Second) - if err != nil { - http.Error(wr, err.Error(), http.StatusServiceUnavailable) - return - } - - wr.WriteHeader(http.StatusOK) - hijacker, ok := wr.(http.Hijacker) - if !ok { - http.Error(wr, "Hijacking not supported", http.StatusInternalServerError) - return - } - client_conn, _, err := hijacker.Hijack() - if err != nil { - http.Error(wr, err.Error(), http.StatusServiceUnavailable) - } - - utils.ProxyConn(req.Context(), client_conn, dest_conn) +// mustGetHost returns the host from a full URL. +func mustGetHost(t *testing.T, fullURL string) string { + u, err := url.Parse(fullURL) + require.NoError(t, err) + return u.Host } // pingSrv is a simple HTTP handler that returns a PingResponse with a @@ -374,18 +375,6 @@ func (p *pingSrv) ServeHTTP(wr http.ResponseWriter, req *http.Request) { ) } -func newHTTPSProxy(t *testing.T) string { - pingTestServer := httptest.NewTLSServer(&pingSrv{}) - t.Cleanup(func() { pingTestServer.Close() }) - - proxyTestServer := httptest.NewServer(&proxyHandler{ - upstreamAddr: pingTestServer.Listener.Addr().String(), - }) - t.Cleanup(func() { proxyTestServer.Close() }) - - return proxyTestServer.URL -} - type mockClient struct { auth.ClientI