From 05d48f6a52d634edfb5c385c30e904b94252e855 Mon Sep 17 00:00:00 2001 From: Scott Fleener Date: Tue, 11 Mar 2025 15:15:10 -0400 Subject: [PATCH] fix(destination): Do not send admin traffic over opaque transport (#13758) Traffic that is meant for the destination workload can be sent over the opaque transport without issue. However, traffic intended for the proxy itself (metrics scraping, tap) need to be sent directly to the corresponding proxy port to prevent them from being forwarded to the workflow. This adds in special cases for the admin and control ports, read directly from the environment variables on the pods, that excludes them from being sent over opaque transport. Signed-off-by: Scott Fleener --- .../api/destination/endpoint_translator.go | 58 ++++++- .../destination/endpoint_translator_test.go | 148 +++++++++++++++++- controller/api/destination/server_test.go | 8 + controller/api/destination/test_util.go | 48 +++++- 4 files changed, 253 insertions(+), 9 deletions(-) diff --git a/controller/api/destination/endpoint_translator.go b/controller/api/destination/endpoint_translator.go index b88fc8fe3..2c9dac894 100644 --- a/controller/api/destination/endpoint_translator.go +++ b/controller/api/destination/endpoint_translator.go @@ -24,6 +24,8 @@ const ( // inboundListenAddr is the environment variable holding the inbound // listening address for the proxy container. envInboundListenAddr = "LINKERD2_PROXY_INBOUND_LISTEN_ADDR" + envAdminListenAddr = "LINKERD2_PROXY_ADMIN_LISTEN_ADDR" + envControlListenAddr = "LINKERD2_PROXY_CONTROL_LISTEN_ADDR" updateQueueCapacity = 100 ) @@ -651,10 +653,16 @@ func createWeightedAddr( weightedAddr.Http2 = meshedHttp2 weightedAddr.ProtocolHint = &pb.ProtocolHint{} + metaPorts, err := getPodMetaPorts(&address.Pod.Spec) + if err != nil { + return nil, fmt.Errorf("failed to read pod meta ports: %w", err) + } + _, opaquePort := opaquePorts[address.Port] opaquePort = opaquePort || address.OpaqueProtocol + _, isMetaPort := metaPorts[address.Port] - if forceOpaqueTransport || opaquePort { + if !isMetaPort && (forceOpaqueTransport || opaquePort) { port, err := getInboundPort(&address.Pod.Spec) if err != nil { return nil, fmt.Errorf("failed to read inbound port: %w", err) @@ -723,24 +731,64 @@ func newEmptyAddressSet() watcher.AddressSet { // getInboundPort gets the inbound port from the proxy container's environment // variable. func getInboundPort(podSpec *corev1.PodSpec) (uint32, error) { + ports, err := getPodPorts(podSpec, map[string]struct{}{envInboundListenAddr: {}}) + if err != nil { + return 0, err + } + port := ports[envInboundListenAddr] + if port == 0 { + return 0, fmt.Errorf("failed to find inbound port in %s", envInboundListenAddr) + } + return port, nil +} + +func getPodMetaPorts(podSpec *corev1.PodSpec) (map[uint32]struct{}, error) { + ports, err := getPodPorts(podSpec, map[string]struct{}{ + envAdminListenAddr: {}, + envControlListenAddr: {}, + }) + if err != nil { + return nil, err + } + invertedPorts := map[uint32]struct{}{} + for _, port := range ports { + invertedPorts[port] = struct{}{} + } + return invertedPorts, nil +} + +func getPodPorts(podSpec *corev1.PodSpec, addrEnvNames map[string]struct{}) (map[string]uint32, error) { containers := append(podSpec.InitContainers, podSpec.Containers...) for _, containerSpec := range containers { + ports := map[string]uint32{} if containerSpec.Name != pkgK8s.ProxyContainerName { continue } for _, envVar := range containerSpec.Env { - if envVar.Name != envInboundListenAddr { + _, hasEnv := addrEnvNames[envVar.Name] + if !hasEnv { continue } addrPort, err := netip.ParseAddrPort(envVar.Value) if err != nil { - return 0, fmt.Errorf("failed to parse inbound port for proxy container: %w", err) + return map[string]uint32{}, fmt.Errorf("failed to parse inbound port for proxy container: %w", err) } - return uint32(addrPort.Port()), nil + ports[envVar.Name] = uint32(addrPort.Port()) } + if len(ports) != len(addrEnvNames) { + missingEnv := []string{} + for env := range ports { + _, hasEnv := addrEnvNames[env] + if !hasEnv { + missingEnv = append(missingEnv, env) + } + } + return map[string]uint32{}, fmt.Errorf("failed to find %s environment variables in proxy container", missingEnv) + } + return ports, nil } - return 0, fmt.Errorf("failed to find %s environment variable in any container for given pod spec", envInboundListenAddr) + return map[string]uint32{}, fmt.Errorf("failed to find %s environment variables in any container for given pod spec", addrEnvNames) } // getInboundPortFromExternalWorkload gets the inbound port from the ExternalWorkload spec diff --git a/controller/api/destination/endpoint_translator_test.go b/controller/api/destination/endpoint_translator_test.go index 57a637f7d..967a171ae 100644 --- a/controller/api/destination/endpoint_translator_test.go +++ b/controller/api/destination/endpoint_translator_test.go @@ -44,6 +44,14 @@ var ( Name: envInboundListenAddr, Value: "0.0.0.0:4143", }, + { + Name: envAdminListenAddr, + Value: "0.0.0.0:4191", + }, + { + Name: envControlListenAddr, + Value: "0.0.0.0:4190", + }, }, }, }, @@ -73,7 +81,15 @@ var ( Env: []corev1.EnvVar{ { Name: envInboundListenAddr, - Value: "[::1]:4143", + Value: "[::]:4143", + }, + { + Name: envAdminListenAddr, + Value: "[::]:4191", + }, + { + Name: envControlListenAddr, + Value: "[::]:4190", }, }, }, @@ -105,6 +121,14 @@ var ( Name: envInboundListenAddr, Value: "0.0.0.0:4143", }, + { + Name: envAdminListenAddr, + Value: "0.0.0.0:4191", + }, + { + Name: envControlListenAddr, + Value: "0.0.0.0:4190", + }, }, }, }, @@ -133,6 +157,14 @@ var ( Name: envInboundListenAddr, Value: "[::1]:4143", }, + { + Name: envAdminListenAddr, + Value: "0.0.0.0:4191", + }, + { + Name: envControlListenAddr, + Value: "0.0.0.0:4190", + }, }, }, }, @@ -164,6 +196,14 @@ var ( Name: envInboundListenAddr, Value: "0.0.0.0:4143", }, + { + Name: envAdminListenAddr, + Value: "0.0.0.0:4191", + }, + { + Name: envControlListenAddr, + Value: "0.0.0.0:4190", + }, }, }, }, @@ -172,6 +212,84 @@ var ( OpaqueProtocol: true, } + podAdmin = watcher.Address{ + IP: "1.1.1.5", + Port: 4191, + Pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "podAdmin", + Namespace: "ns", + Labels: map[string]string{ + k8s.ControllerNSLabel: "linkerd", + k8s.ProxyDeploymentLabel: "deployment-name", + }, + }, + Spec: corev1.PodSpec{ + ServiceAccountName: "serviceaccount-name", + Containers: []corev1.Container{ + { + Name: k8s.ProxyContainerName, + Env: []corev1.EnvVar{ + { + Name: envInboundListenAddr, + Value: "0.0.0.0:4143", + }, + { + Name: envAdminListenAddr, + Value: "0.0.0.0:4191", + }, + { + Name: envControlListenAddr, + Value: "0.0.0.0:4190", + }, + }, + }, + }, + }, + }, + OwnerKind: "replicationcontroller", + OwnerName: "rc-name", + } + + podControl = watcher.Address{ + IP: "1.1.1.6", + Port: 4190, + Pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "podControl", + Namespace: "ns", + Labels: map[string]string{ + k8s.ControllerNSLabel: "linkerd", + k8s.ProxyDeploymentLabel: "deployment-name", + }, + }, + Spec: corev1.PodSpec{ + ServiceAccountName: "serviceaccount-name", + Containers: []corev1.Container{ + { + Name: k8s.ProxyContainerName, + Env: []corev1.EnvVar{ + { + Name: envInboundListenAddr, + Value: "0.0.0.0:4143", + }, + { + Name: envAdminListenAddr, + Value: "0.0.0.0:4191", + }, + { + Name: envControlListenAddr, + Value: "0.0.0.0:4190", + }, + }, + }, + }, + }, + }, + OwnerKind: "replicationcontroller", + OwnerName: "rc-name", + } + ew1 = watcher.Address{ IP: "1.1.1.1", Port: 1, @@ -542,6 +660,34 @@ func TestEndpointTranslatorForPods(t *testing.T) { } }) + t.Run("Sends admin addresses without opaque transport", func(t *testing.T) { + expectedProtocolHint := &pb.ProtocolHint{ + Protocol: &pb.ProtocolHint_H2_{ + H2: &pb.ProtocolHint_H2{}, + }, + } + + mockGetServer, translator := makeEndpointTranslatorWithOpaqueTransport(t, true) + translator.Start() + defer translator.Stop() + + translator.Add(mkAddressSetForPods(t, podAdmin, podControl)) + + addressesAdded := (<-mockGetServer.updatesReceived).GetAdd().Addrs + actualNumberOfAdded := len(addressesAdded) + expectedNumberOfAdded := 2 + if actualNumberOfAdded != expectedNumberOfAdded { + t.Fatalf("Expecting [%d] addresses to be added, got [%d]: %v", expectedNumberOfAdded, actualNumberOfAdded, addressesAdded) + } + + for _, addressAdded := range addressesAdded { + actualProtocolHint := addressAdded.GetProtocolHint() + if diff := deep.Equal(actualProtocolHint, expectedProtocolHint); diff != nil { + t.Fatalf("ProtocolHint: %v", diff) + } + } + }) + t.Run("Sends metric labels with added addresses", func(t *testing.T) { mockGetServer, translator := makeEndpointTranslator(t) translator.Start() diff --git a/controller/api/destination/server_test.go b/controller/api/destination/server_test.go index 0e7ead1af..1b5ccd1ed 100644 --- a/controller/api/destination/server_test.go +++ b/controller/api/destination/server_test.go @@ -1018,6 +1018,14 @@ func TestGetProfiles(t *testing.T) { Name: "LINKERD2_PROXY_INBOUND_LISTEN_ADDR", Value: "0.0.0.0:4143", }, + { + Name: "LINKERD2_PROXY_ADMIN_LISTEN_ADDR", + Value: "0.0.0.0:4191", + }, + { + Name: "LINKERD2_PROXY_CONTROL_LISTEN_ADDR", + Value: "0.0.0.0:4190", + }, }, }, { diff --git a/controller/api/destination/test_util.go b/controller/api/destination/test_util.go index 1a6328a13..fefe26512 100644 --- a/controller/api/destination/test_util.go +++ b/controller/api/destination/test_util.go @@ -79,6 +79,10 @@ spec: - env: - name: LINKERD2_PROXY_INBOUND_LISTEN_ADDR value: 0.0.0.0:4143 + - name: LINKERD2_PROXY_ADMIN_LISTEN_ADDR + value: 0.0.0.0:4191 + - name: LINKERD2_PROXY_CONTROL_LISTEN_ADDR + value: 0.0.0.0:4190 name: linkerd-proxy`, ` apiVersion: v1 @@ -264,6 +268,10 @@ spec: - env: - name: LINKERD2_PROXY_INBOUND_LISTEN_ADDR value: 0.0.0.0:4143 + - name: LINKERD2_PROXY_ADMIN_LISTEN_ADDR + value: 0.0.0.0:4191 + - name: LINKERD2_PROXY_CONTROL_LISTEN_ADDR + value: 0.0.0.0:4190 name: linkerd-proxy`, } @@ -332,6 +340,10 @@ spec: - env: - name: LINKERD2_PROXY_INBOUND_LISTEN_ADDR value: 0.0.0.0:4143 + - name: LINKERD2_PROXY_ADMIN_LISTEN_ADDR + value: 0.0.0.0:4191 + - name: LINKERD2_PROXY_CONTROL_LISTEN_ADDR + value: 0.0.0.0:4190 name: linkerd-proxy`, } @@ -384,7 +396,17 @@ status: status: "True" podIP: 172.17.13.15 podIPs: - - ip: 172.17.13.15`, + - ip: 172.17.13.15 +spec: + containers: + - env: + - name: LINKERD2_PROXY_INBOUND_LISTEN_ADDR + value: 0.0.0.0:4143 + - name: LINKERD2_PROXY_ADMIN_LISTEN_ADDR + value: 0.0.0.0:4191 + - name: LINKERD2_PROXY_CONTROL_LISTEN_ADDR + value: 0.0.0.0:4190 + name: linkerd-proxy`, } policyResources := []string{ @@ -441,6 +463,10 @@ spec: env: - name: LINKERD2_PROXY_INBOUND_LISTEN_ADDR value: 0.0.0.0:4143 + - name: LINKERD2_PROXY_ADMIN_LISTEN_ADDR + value: 0.0.0.0:4191 + - name: LINKERD2_PROXY_CONTROL_LISTEN_ADDR + value: 0.0.0.0:4190 - name: app image: nginx ports: @@ -527,6 +553,10 @@ spec: env: - name: LINKERD2_PROXY_INBOUND_LISTEN_ADDR value: 0.0.0.0:4143 + - name: LINKERD2_PROXY_ADMIN_LISTEN_ADDR + value: 0.0.0.0:4191 + - name: LINKERD2_PROXY_CONTROL_LISTEN_ADDR + value: 0.0.0.0:4190 - name: app image: nginx ports: @@ -625,10 +655,14 @@ status: - ip: 172.17.55.1 spec: containers: - - env: + - name: linkerd-proxy + env: - name: LINKERD2_PROXY_INBOUND_LISTEN_ADDR value: 0.0.0.0:4143 - name: linkerd-proxy`, + - name: LINKERD2_PROXY_ADMIN_LISTEN_ADDR + value: 0.0.0.0:4191 + - name: LINKERD2_PROXY_CONTROL_LISTEN_ADDR + value: 0.0.0.0:4190`, } destinationCredentialsResources := []string{` @@ -804,6 +838,10 @@ spec: - env: - name: LINKERD2_PROXY_INBOUND_LISTEN_ADDR value: 0.0.0.0:4143 + - name: LINKERD2_PROXY_ADMIN_LISTEN_ADDR + value: 0.0.0.0:4191 + - name: LINKERD2_PROXY_CONTROL_LISTEN_ADDR + value: 0.0.0.0:4190 name: linkerd-proxy`, } @@ -885,6 +923,10 @@ spec: - env: - name: LINKERD2_PROXY_INBOUND_LISTEN_ADDR value: 0.0.0.0:4143 + - name: LINKERD2_PROXY_ADMIN_LISTEN_ADDR + value: 0.0.0.0:4191 + - name: LINKERD2_PROXY_CONTROL_LISTEN_ADDR + value: 0.0.0.0:4190 name: linkerd-proxy`, ` apiVersion: linkerd.io/v1alpha2