Respect [HTTP(S)|NO]_PROXY envs when dialing directly to Kube (#30583…

…) (#30615) * Respect `[HTTP(S)|NO]_PROXY` envs when dialing directly to Kube PR #11257 disabled support of `HTTP_PROXY`, `HTTPS_PROXY` and `NO_PROXY` environement flags for Kubernetes Access. The desired behavior was expected to be respected only by the Kubernetes Proxy and Kubernetes Legacy Proxy when dialing over reverse tunnel but ended up applied to all outbound connections from Kube Access flow. This PR enables support for proxy env's when dialing directly to the Kubernetes Cluster - `kubernetes_service` and `legacy_proxy` when the cluster is local. Fixes #30550 * fix func name * fix comment --------- Signed-off-by: Tiago Silva <[email protected]>
gravitational · Aug 22, 2023 · c1bdcd0 · c1bdcd0
1 parent a11dfe6
commit c1bdcd0
Show file tree

Hide file tree

Showing 2 changed files with 94 additions and 50 deletions.
diff --git a/lib/kube/proxy/auth.go b/lib/kube/proxy/auth.go
@@ -26,6 +26,7 @@ import (
 	"github.com/sirupsen/logrus"
 	authzapi "k8s.io/api/authorization/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	utilnet "k8s.io/apimachinery/pkg/util/net"
 	"k8s.io/client-go/kubernetes"
 	authztypes "k8s.io/client-go/kubernetes/typed/authorization/v1"
 	// Load kubeconfig auth plugins for gcp and azure.
@@ -185,9 +186,12 @@ func extractKubeCreds(ctx context.Context, component string, cluster string, cli
 }
 
 // newDirectTransports creates a new http.Transport that will be used to connect to the Kubernetes API server.
-// It is a direct connection, not going through a proxy.
+// It is a direct connection, not going through a Teleport proxy.
+// The transport used respects HTTP_PROXY, HTTPS_PROXY, and NO_PROXY environment variables.
 func newDirectTransports(component string, tlsConfig *tls.Config, transportConfig *transport.Config) (httpTransport, error) {
-	h1Transport, err := wrapTransport(newH1Transport(tlsConfig, nil), transportConfig)
+	h1Transport, err := wrapTransport(
+		utilnet.SetTransportDefaults(newH1Transport(tlsConfig, nil)),
+		transportConfig)
 	if err != nil {
 		return httpTransport{}, trace.Wrap(err)
 	}
@@ -196,6 +200,9 @@ func newDirectTransports(component string, tlsConfig *tls.Config, transportConfi
 	if err != nil {
 		return httpTransport{}, trace.Wrap(err)
 	}
+	// SetTransportDefaults sets the default values for the transport including
+	// support for HTTP_PROXY, HTTPS_PROXY, NO_PROXY, and the default user agent.
+	h2HTTPTransport = utilnet.SetTransportDefaults(h2HTTPTransport)
 	h2Transport, err := wrapTransport(h2HTTPTransport, transportConfig)
 	if err != nil {
 		return httpTransport{}, trace.Wrap(err)

diff --git a/lib/kube/proxy/forwarder_test.go b/lib/kube/proxy/forwarder_test.go
@@ -1162,8 +1162,9 @@ func TestClusterSessionDial(t *testing.T) {
 	require.Equal(t, "addr1", sess.kubeAddress)
 }
 
-// TestKubeFwdHTTPProxyEnv ensures that kube forwarder doesn't respect HTTPS_PROXY env
-// and Kubernetes API is called directly.
+// TestKubeFwdHTTPProxyEnv ensures that Teleport only respects the `[HTTP(S)|NO]_PROXY`
+// env variables when dialing directly to the EKS cluster and doesn't respect
+// them when dialing via reverse tunnel to other Teleport services.
 func TestKubeFwdHTTPProxyEnv(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	t.Cleanup(cancel)
@@ -1192,67 +1193,103 @@ func TestKubeFwdHTTPProxyEnv(t *testing.T) {
 		return new(net.Dialer).DialContext(ctx, mockKubeAPI.Listener.Addr().Network(), mockKubeAPI.Listener.Addr().String())
 	}
 
-	checkTransportProxy := func(rt http.RoundTripper) http.RoundTripper {
+	checkTransportProxyDirectDial := func(rt http.RoundTripper) http.RoundTripper {
 		tr, ok := rt.(*http.Transport)
 		require.True(t, ok)
-		require.Nil(t, tr.Proxy, "kube forwarder should not take into account HTTPS_PROXY env")
+		require.NotNil(t, tr.Proxy, "kube forwarder should take into account HTTPS_PROXY env when dialing to kubernetes API")
 		return rt
 	}
 
-	h2Transport, err := newH2Transport(&tls.Config{
-		InsecureSkipVerify: true,
-	}, nil)
-	require.NoError(t, err)
-	f.clusterDetails = map[string]*kubeDetails{
-		"local": {
-			kubeCreds: &staticKubeCreds{
-				targetAddr: mockKubeAPI.URL,
-				tlsConfig:  mockKubeAPI.TLS,
-				transportConfig: &transport.Config{
-					WrapTransport: checkTransportProxy,
+	checkTransportProxIndirectDialer := func(rt http.RoundTripper) http.RoundTripper {
+		tr, ok := rt.(*http.Transport)
+		require.True(t, ok)
+		require.Nil(t, tr.Proxy, "kube forwarder should not take into account HTTPS_PROXY env when dialing over tunnel")
+		return rt
+	}
+
+	t.Setenv("HTTP_PROXY", "example.com:9999")
+	t.Setenv("HTTPS_PROXY", "example.com:9999")
+
+	for _, test := range []struct {
+		name      string
+		rtBuilder func(t *testing.T) httpTransport
+		checkFunc func(t *testing.T, req *http.Request)
+	}{
+		{
+			name: "newDirectTransports",
+			rtBuilder: func(t *testing.T) httpTransport {
+				rts, err := newDirectTransports("test", &tls.Config{
+					InsecureSkipVerify: true,
 				},
-				transport: httpTransport{
-					h1Transport: newH1Transport(&tls.Config{
-						InsecureSkipVerify: true,
-					}, nil),
+					&transport.Config{
+						WrapTransport: checkTransportProxyDirectDial,
+					})
+				require.NoError(t, err)
+				return rts
+			},
+		},
+		{
+			name: "newTransport",
+			rtBuilder: func(t *testing.T) httpTransport {
+				h1Transport, err := wrapTransport(newH1Transport(&tls.Config{
+					InsecureSkipVerify: true,
+				}, nil), &transport.Config{
+					WrapTransport: checkTransportProxIndirectDialer,
+				})
+				require.NoError(t, err)
+				h2HTTPTransport, err := newH2Transport(&tls.Config{
+					InsecureSkipVerify: true,
+				}, nil)
+				require.NoError(t, err)
+				h2Transport, err := wrapTransport(h2HTTPTransport, &transport.Config{
+					WrapTransport: checkTransportProxIndirectDialer,
+				})
+				require.NoError(t, err)
+				return httpTransport{
 					h2Transport: h2Transport,
-				},
+					h1Transport: h1Transport,
+				}
 			},
 		},
-	}
-
-	authCtx.kubeClusterName = "local"
-	sess, err := f.newClusterSession(ctx, authCtx)
-	require.NoError(t, err)
-	t.Cleanup(sess.close)
-	require.Equal(t, []kubeClusterEndpoint{{addr: f.clusterDetails["local"].getTargetAddr()}}, sess.kubeClusterEndpoints)
+	} {
+
+		f.clusterDetails = map[string]*kubeDetails{
+			"local": {
+				kubeCreds: &staticKubeCreds{
+					targetAddr: mockKubeAPI.URL,
+					tlsConfig:  mockKubeAPI.TLS,
+					transport:  test.rtBuilder(t),
+				},
+			},
+		}
 
-	sess.tlsConfig.InsecureSkipVerify = true
+		authCtx.kubeClusterName = "local"
+		sess, err := f.newClusterSession(ctx, authCtx)
+		require.NoError(t, err)
+		t.Cleanup(sess.close)
 
-	t.Setenv("HTTP_PROXY", "example.com:9999")
-	t.Setenv("HTTPS_PROXY", "example.com:9999")
+		// Set upgradeToHTTP2 to trigger h2 transport upgrade logic.
+		sess.upgradeToHTTP2 = true
+		fwd, err := f.makeSessionForwarder(sess)
+		require.NoError(t, err)
 
-	// Set upgradeToHTTP2 to trigger h2 transport upgrade logic.
-	sess.upgradeToHTTP2 = true
-	fwd, err := f.makeSessionForwarder(sess)
-	require.NoError(t, err)
+		// Create KubeProxy that uses fwd and forward incoming request to kubernetes API.
+		kubeProxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			r.URL, err = url.Parse(mockKubeAPI.URL)
+			require.NoError(t, err)
+			fwd.ServeHTTP(w, r)
+		}))
+		t.Cleanup(kubeProxy.Close)
 
-	// Create KubeProxy that uses fwd and forward incoming request to kubernetes API.
-	kubeProxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		r.URL, err = url.Parse(mockKubeAPI.URL)
+		req, err := http.NewRequest("GET", kubeProxy.URL, nil)
 		require.NoError(t, err)
-		fwd.ServeHTTP(w, r)
-	}))
-	t.Cleanup(kubeProxy.Close)
 
-	req, err := http.NewRequest("GET", kubeProxy.URL, nil)
-	require.NoError(t, err)
-
-	resp, err := http.DefaultClient.Do(req)
-	require.NoError(t, err)
-	require.Equal(t, http.StatusOK, resp.StatusCode)
-	require.Equal(t, uint32(1), atomic.LoadUint32(&kubeAPICallCount))
-	require.NoError(t, resp.Body.Close())
+		resp, err := http.DefaultClient.Do(req)
+		require.NoError(t, err)
+		require.Equal(t, http.StatusOK, resp.StatusCode)
+		require.NoError(t, resp.Body.Close())
+	}
+	require.Equal(t, uint32(2), atomic.LoadUint32(&kubeAPICallCount))
 }
 
 func newMockForwader(ctx context.Context, t *testing.T) *Forwarder {