From 861dc3631c4337c13d19308d9e0958d030bfcbf3 Mon Sep 17 00:00:00 2001 From: Irbe Krumina Date: Tue, 8 Oct 2024 18:35:23 +0100 Subject: [PATCH] cmd/{k8s-operator,containerboot},kube/egressservices: fix Pod IP check for dual stack clusters (#13721) Currently egress Services for ProxyGroup only work for Pods and Services with IPv4 addresses. Ensure that it works on dual stack clusters by reading proxy Pod's IP from the .status.podIPs list that always contains both IPv4 and IPv6 address (if the Pod has them) rather than .status.podIP that could contain IPv6 only for a dual stack cluster. Updates tailscale/tailscale#13406 Signed-off-by: Irbe Krumina --- cmd/containerboot/main.go | 33 ++-------------- cmd/containerboot/services.go | 10 ++--- cmd/containerboot/settings.go | 57 +++++++++++++++++++++++++++ cmd/k8s-operator/egress-eps.go | 29 ++++++++++++-- cmd/k8s-operator/egress-eps_test.go | 18 +++++++-- cmd/k8s-operator/proxygroup_specs.go | 5 ++- kube/egressservices/egressservices.go | 2 +- 7 files changed, 109 insertions(+), 45 deletions(-) diff --git a/cmd/containerboot/main.go b/cmd/containerboot/main.go index 5ebe22e5f..4c8ba5807 100644 --- a/cmd/containerboot/main.go +++ b/cmd/containerboot/main.go @@ -132,36 +132,9 @@ func newNetfilterRunner(logf logger.Logf) (linuxfw.NetfilterRunner, error) { func main() { log.SetPrefix("boot: ") tailscale.I_Acknowledge_This_API_Is_Unstable = true - cfg := &settings{ - AuthKey: defaultEnvs([]string{"TS_AUTHKEY", "TS_AUTH_KEY"}, ""), - Hostname: defaultEnv("TS_HOSTNAME", ""), - Routes: defaultEnvStringPointer("TS_ROUTES"), - ServeConfigPath: defaultEnv("TS_SERVE_CONFIG", ""), - ProxyTargetIP: defaultEnv("TS_DEST_IP", ""), - ProxyTargetDNSName: defaultEnv("TS_EXPERIMENTAL_DEST_DNS_NAME", ""), - TailnetTargetIP: defaultEnv("TS_TAILNET_TARGET_IP", ""), - TailnetTargetFQDN: defaultEnv("TS_TAILNET_TARGET_FQDN", ""), - DaemonExtraArgs: defaultEnv("TS_TAILSCALED_EXTRA_ARGS", ""), - ExtraArgs: defaultEnv("TS_EXTRA_ARGS", ""), - InKubernetes: os.Getenv("KUBERNETES_SERVICE_HOST") != "", - UserspaceMode: defaultBool("TS_USERSPACE", true), - StateDir: defaultEnv("TS_STATE_DIR", ""), - AcceptDNS: defaultEnvBoolPointer("TS_ACCEPT_DNS"), - KubeSecret: defaultEnv("TS_KUBE_SECRET", "tailscale"), - SOCKSProxyAddr: defaultEnv("TS_SOCKS5_SERVER", ""), - HTTPProxyAddr: defaultEnv("TS_OUTBOUND_HTTP_PROXY_LISTEN", ""), - Socket: defaultEnv("TS_SOCKET", "/tmp/tailscaled.sock"), - AuthOnce: defaultBool("TS_AUTH_ONCE", false), - Root: defaultEnv("TS_TEST_ONLY_ROOT", "/"), - TailscaledConfigFilePath: tailscaledConfigFilePath(), - AllowProxyingClusterTrafficViaIngress: defaultBool("EXPERIMENTAL_ALLOW_PROXYING_CLUSTER_TRAFFIC_VIA_INGRESS", false), - PodIP: defaultEnv("POD_IP", ""), - EnableForwardingOptimizations: defaultBool("TS_EXPERIMENTAL_ENABLE_FORWARDING_OPTIMIZATIONS", false), - HealthCheckAddrPort: defaultEnv("TS_HEALTHCHECK_ADDR_PORT", ""), - EgressSvcsCfgPath: defaultEnv("TS_EGRESS_SERVICES_CONFIG_PATH", ""), - } - if err := cfg.validate(); err != nil { + cfg, err := configFromEnv() + if err != nil { log.Fatalf("invalid configuration: %v", err) } @@ -612,7 +585,7 @@ func main() { kc: kc, stateSecret: cfg.KubeSecret, netmapChan: egressSvcsNotify, - podIP: cfg.PodIP, + podIPv4: cfg.PodIPv4, tailnetAddrs: addrs, } go func() { diff --git a/cmd/containerboot/services.go b/cmd/containerboot/services.go index e46c7c015..4da7286b7 100644 --- a/cmd/containerboot/services.go +++ b/cmd/containerboot/services.go @@ -46,7 +46,7 @@ type egressProxy struct { netmapChan chan ipn.Notify // chan to receive netmap updates on - podIP string // never empty string + podIPv4 string // never empty string, currently only IPv4 is supported // tailnetFQDNs is the egress service FQDN to tailnet IP mappings that // were last used to configure firewall rules for this proxy. @@ -361,7 +361,7 @@ func (ep *egressProxy) getStatus(ctx context.Context) (*egressservices.Status, e if err := json.Unmarshal([]byte(raw), status); err != nil { return nil, fmt.Errorf("error unmarshalling previous config: %w", err) } - if reflect.DeepEqual(status.PodIP, ep.podIP) { + if reflect.DeepEqual(status.PodIPv4, ep.podIPv4) { return status, nil } return nil, nil @@ -374,7 +374,7 @@ func (ep *egressProxy) setStatus(ctx context.Context, status *egressservices.Sta if status == nil { status = &egressservices.Status{} } - status.PodIP = ep.podIP + status.PodIPv4 = ep.podIPv4 secret, err := ep.kc.GetSecret(ctx, ep.stateSecret) if err != nil { return fmt.Errorf("error retrieving state Secret: %w", err) @@ -565,7 +565,7 @@ func servicesStatusIsEqual(st, st1 *egressservices.Status) bool { if st == nil || st1 == nil { return false } - st.PodIP = "" - st1.PodIP = "" + st.PodIPv4 = "" + st1.PodIPv4 = "" return reflect.DeepEqual(*st, *st1) } diff --git a/cmd/containerboot/settings.go b/cmd/containerboot/settings.go index fab4bd2fd..742713e77 100644 --- a/cmd/containerboot/settings.go +++ b/cmd/containerboot/settings.go @@ -14,6 +14,7 @@ "os" "path" "strconv" + "strings" "tailscale.com/ipn/conffile" "tailscale.com/kube/kubeclient" @@ -62,11 +63,67 @@ type settings struct { // PodIP is the IP of the Pod if running in Kubernetes. This is used // when setting up rules to proxy cluster traffic to cluster ingress // target. + // Deprecated: use PodIPv4, PodIPv6 instead to support dual stack clusters PodIP string + PodIPv4 string + PodIPv6 string HealthCheckAddrPort string EgressSvcsCfgPath string } +func configFromEnv() (*settings, error) { + cfg := &settings{ + AuthKey: defaultEnvs([]string{"TS_AUTHKEY", "TS_AUTH_KEY"}, ""), + Hostname: defaultEnv("TS_HOSTNAME", ""), + Routes: defaultEnvStringPointer("TS_ROUTES"), + ServeConfigPath: defaultEnv("TS_SERVE_CONFIG", ""), + ProxyTargetIP: defaultEnv("TS_DEST_IP", ""), + ProxyTargetDNSName: defaultEnv("TS_EXPERIMENTAL_DEST_DNS_NAME", ""), + TailnetTargetIP: defaultEnv("TS_TAILNET_TARGET_IP", ""), + TailnetTargetFQDN: defaultEnv("TS_TAILNET_TARGET_FQDN", ""), + DaemonExtraArgs: defaultEnv("TS_TAILSCALED_EXTRA_ARGS", ""), + ExtraArgs: defaultEnv("TS_EXTRA_ARGS", ""), + InKubernetes: os.Getenv("KUBERNETES_SERVICE_HOST") != "", + UserspaceMode: defaultBool("TS_USERSPACE", true), + StateDir: defaultEnv("TS_STATE_DIR", ""), + AcceptDNS: defaultEnvBoolPointer("TS_ACCEPT_DNS"), + KubeSecret: defaultEnv("TS_KUBE_SECRET", "tailscale"), + SOCKSProxyAddr: defaultEnv("TS_SOCKS5_SERVER", ""), + HTTPProxyAddr: defaultEnv("TS_OUTBOUND_HTTP_PROXY_LISTEN", ""), + Socket: defaultEnv("TS_SOCKET", "/tmp/tailscaled.sock"), + AuthOnce: defaultBool("TS_AUTH_ONCE", false), + Root: defaultEnv("TS_TEST_ONLY_ROOT", "/"), + TailscaledConfigFilePath: tailscaledConfigFilePath(), + AllowProxyingClusterTrafficViaIngress: defaultBool("EXPERIMENTAL_ALLOW_PROXYING_CLUSTER_TRAFFIC_VIA_INGRESS", false), + PodIP: defaultEnv("POD_IP", ""), + EnableForwardingOptimizations: defaultBool("TS_EXPERIMENTAL_ENABLE_FORWARDING_OPTIMIZATIONS", false), + HealthCheckAddrPort: defaultEnv("TS_HEALTHCHECK_ADDR_PORT", ""), + EgressSvcsCfgPath: defaultEnv("TS_EGRESS_SERVICES_CONFIG_PATH", ""), + } + podIPs, ok := os.LookupEnv("POD_IPS") + if ok { + ips := strings.Split(podIPs, ",") + if len(ips) > 2 { + return nil, fmt.Errorf("POD_IPs can contain at most 2 IPs, got %d (%v)", len(ips), ips) + } + for _, ip := range ips { + parsed, err := netip.ParseAddr(ip) + if err != nil { + return nil, fmt.Errorf("error parsing IP address %s: %w", ip, err) + } + if parsed.Is4() { + cfg.PodIPv4 = parsed.String() + continue + } + cfg.PodIPv6 = parsed.String() + } + } + if err := cfg.validate(); err != nil { + return nil, fmt.Errorf("invalid configuration: %v", err) + } + return cfg, nil +} + func (s *settings) validate() error { if s.TailscaledConfigFilePath != "" { dir, file := path.Split(s.TailscaledConfigFilePath) diff --git a/cmd/k8s-operator/egress-eps.go b/cmd/k8s-operator/egress-eps.go index fa13c525f..e8b327263 100644 --- a/cmd/k8s-operator/egress-eps.go +++ b/cmd/k8s-operator/egress-eps.go @@ -9,6 +9,7 @@ "context" "encoding/json" "fmt" + "net/netip" "reflect" "strings" @@ -132,6 +133,19 @@ func (er *egressEpsReconciler) Reconcile(ctx context.Context, req reconcile.Requ return res, nil } +func podIPv4(pod *corev1.Pod) (string, error) { + for _, ip := range pod.Status.PodIPs { + parsed, err := netip.ParseAddr(ip.IP) + if err != nil { + return "", fmt.Errorf("error parsing IP address %s: %w", ip, err) + } + if parsed.Is4() { + return parsed.String(), nil + } + } + return "", nil +} + // podIsReadyToRouteTraffic returns true if it appears that the proxy Pod has configured firewall rules to be able to // route traffic to the given tailnet service. It retrieves the proxy's state Secret and compares the tailnet service // status written there to the desired service configuration. @@ -142,14 +156,21 @@ func (er *egressEpsReconciler) podIsReadyToRouteTraffic(ctx context.Context, pod l.Debugf("proxy Pod is being deleted, ignore") return false, nil } - podIP := pod.Status.PodIP + podIP, err := podIPv4(&pod) + if err != nil { + return false, fmt.Errorf("error determining Pod IP address: %v", err) + } + if podIP == "" { + l.Infof("[unexpected] Pod does not have an IPv4 address, and IPv6 is not currently supported") + return false, nil + } stateS := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: pod.Name, Namespace: pod.Namespace, }, } - err := er.Get(ctx, client.ObjectKeyFromObject(stateS), stateS) + err = er.Get(ctx, client.ObjectKeyFromObject(stateS), stateS) if apierrors.IsNotFound(err) { l.Debugf("proxy does not have a state Secret, waiting...") return false, nil @@ -166,8 +187,8 @@ func (er *egressEpsReconciler) podIsReadyToRouteTraffic(ctx context.Context, pod if err := json.Unmarshal(svcStatusBS, svcStatus); err != nil { return false, fmt.Errorf("error unmarshalling egress service status: %w", err) } - if !strings.EqualFold(podIP, svcStatus.PodIP) { - l.Infof("proxy's egress service status is for Pod IP %s, current proxy's Pod IP %s, waiting for the proxy to reconfigure...", svcStatus.PodIP, podIP) + if !strings.EqualFold(podIP, svcStatus.PodIPv4) { + l.Infof("proxy's egress service status is for Pod IP %s, current proxy's Pod IP %s, waiting for the proxy to reconfigure...", svcStatus.PodIPv4, podIP) return false, nil } st, ok := (*svcStatus).Services[tailnetSvcName] diff --git a/cmd/k8s-operator/egress-eps_test.go b/cmd/k8s-operator/egress-eps_test.go index 00d13b2a7..806f739fd 100644 --- a/cmd/k8s-operator/egress-eps_test.go +++ b/cmd/k8s-operator/egress-eps_test.go @@ -98,7 +98,7 @@ func TestTailscaleEgressEndpointSlices(t *testing.T) { t.Run("pods_are_ready_to_route_traffic", func(t *testing.T) { pod, stateS := podAndSecretForProxyGroup("foo") - stBs := serviceStatusForPodIP(t, svc, pod.Status.PodIP, port) + stBs := serviceStatusForPodIP(t, svc, pod.Status.PodIPs[0].IP, port) mustUpdate(t, fc, "operator-ns", stateS.Name, func(s *corev1.Secret) { mak.Set(&s.Data, egressservices.KeyEgressServices, stBs) }) @@ -114,6 +114,16 @@ func TestTailscaleEgressEndpointSlices(t *testing.T) { }) expectEqual(t, fc, eps, nil) }) + t.Run("status_does_not_match_pod_ip", func(t *testing.T) { + _, stateS := podAndSecretForProxyGroup("foo") // replica Pod has IP 10.0.0.1 + stBs := serviceStatusForPodIP(t, svc, "10.0.0.2", port) // status is for a Pod with IP 10.0.0.2 + mustUpdate(t, fc, "operator-ns", stateS.Name, func(s *corev1.Secret) { + mak.Set(&s.Data, egressservices.KeyEgressServices, stBs) + }) + expectReconciled(t, er, "operator-ns", "foo") + eps.Endpoints = []discoveryv1.Endpoint{} + expectEqual(t, fc, eps, nil) + }) } func configMapForSvc(t *testing.T, svc *corev1.Service, p uint16) *corev1.ConfigMap { @@ -162,7 +172,7 @@ func serviceStatusForPodIP(t *testing.T, svc *corev1.Service, ip string, p uint1 } svcName := tailnetSvcName(svc) st := egressservices.Status{ - PodIP: ip, + PodIPv4: ip, Services: map[string]*egressservices.ServiceStatus{svcName: &svcSt}, } bs, err := json.Marshal(st) @@ -181,7 +191,9 @@ func podAndSecretForProxyGroup(pg string) (*corev1.Pod, *corev1.Secret) { UID: "foo", }, Status: corev1.PodStatus{ - PodIP: "10.0.0.1", + PodIPs: []corev1.PodIP{ + {IP: "10.0.0.1"}, + }, }, } s := &corev1.Secret{ diff --git a/cmd/k8s-operator/proxygroup_specs.go b/cmd/k8s-operator/proxygroup_specs.go index a1ec9ccde..100c0707d 100644 --- a/cmd/k8s-operator/proxygroup_specs.go +++ b/cmd/k8s-operator/proxygroup_specs.go @@ -93,10 +93,11 @@ func pgStatefulSet(pg *tsapi.ProxyGroup, namespace, image, tsFirewallMode, cfgHa Env: func() []corev1.EnvVar { envs := []corev1.EnvVar{ { - Name: "POD_IP", + // TODO(irbekrm): verify that .status.podIPs are always set, else read in .status.podIP as well. + Name: "POD_IPS", // this will be a comma separate list i.e 10.136.0.6,2600:1900:4011:161:0:e:0:6 ValueFrom: &corev1.EnvVarSource{ FieldRef: &corev1.ObjectFieldSelector{ - FieldPath: "status.podIP", + FieldPath: "status.podIPs", }, }, }, diff --git a/kube/egressservices/egressservices.go b/kube/egressservices/egressservices.go index f634458d9..428b476b9 100644 --- a/kube/egressservices/egressservices.go +++ b/kube/egressservices/egressservices.go @@ -86,7 +86,7 @@ func (pm PortMap) MarshalText() ([]byte, error) { // Status represents the currently configured firewall rules for all egress // services for a proxy identified by the PodIP. type Status struct { - PodIP string `json:"podIP"` + PodIPv4 string `json:"podIPv4"` // All egress service status keyed by service name. Services map[string]*ServiceStatus `json:"services"` }