From 8a0c69d6d6ac566a777e51d357a40a8e2960d30a Mon Sep 17 00:00:00 2001 From: Andrew Lytvynov Date: Fri, 15 Mar 2024 06:23:30 -0700 Subject: [PATCH 1/6] clientupdate: do not allow msiexec to reboot the OS (#11409) (#11416) According to https://learn.microsoft.com/en-us/windows/win32/msi/standard-installer-command-line-options#promptrestart, `/promptrestart` is ignored with `/quiet` is set, so msiexec.exe can sometimes silently trigger a reboot. The best we can do to reduce unexpected disruption is to just prevent restarts, until the user chooses to do it. Restarts aren't normally needed for Tailscale updates, but there seem to be some situations where it's triggered. Updates #18254 Signed-off-by: Andrew Lytvynov --- clientupdate/clientupdate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clientupdate/clientupdate.go b/clientupdate/clientupdate.go index 565544e7f451e..2a08d6638bb37 100644 --- a/clientupdate/clientupdate.go +++ b/clientupdate/clientupdate.go @@ -836,7 +836,7 @@ func (up *Updater) switchOutputToFile() (io.Closer, error) { func (up *Updater) installMSI(msi string) error { var err error for tries := 0; tries < 2; tries++ { - cmd := exec.Command("msiexec.exe", "/i", filepath.Base(msi), "/quiet", "/promptrestart", "/qn") + cmd := exec.Command("msiexec.exe", "/i", filepath.Base(msi), "/quiet", "/norestart", "/qn") cmd.Dir = filepath.Dir(msi) cmd.Stdout = up.Stdout cmd.Stderr = up.Stderr From 6953dbcd8fe39a5d05cad6f35871871b73608e8a Mon Sep 17 00:00:00 2001 From: Irbe Krumina Date: Wed, 20 Mar 2024 08:34:09 +0000 Subject: [PATCH 2/6] cmd/k8s-operator,ipn/conf.go: fix --accept-routes for proxies (#11454) Fix a bug where all proxies got configured with --accept-routes set to true. The bug was introduced in https://github.com/tailscale/tailscale/pull/11238. Updates#cleanup Signed-off-by: Irbe Krumina --- cmd/k8s-operator/connector_test.go | 40 +++--- cmd/k8s-operator/ingress_test.go | 50 ++++---- cmd/k8s-operator/operator_test.go | 189 +++++++++++++++++----------- cmd/k8s-operator/proxyclass_test.go | 4 +- cmd/k8s-operator/sts.go | 9 +- cmd/k8s-operator/testutils_test.go | 39 ++++-- ipn/conf.go | 4 +- 7 files changed, 190 insertions(+), 145 deletions(-) diff --git a/cmd/k8s-operator/connector_test.go b/cmd/k8s-operator/connector_test.go index 7e8e9599fbcf1..07f0e2d976708 100644 --- a/cmd/k8s-operator/connector_test.go +++ b/cmd/k8s-operator/connector_test.go @@ -73,38 +73,34 @@ func TestConnector(t *testing.T) { hostname: "test-connector", isExitNode: true, subnetRoutes: "10.40.0.0/14", - confFileHash: "9321660203effb80983eaecc7b5ac5a8c53934926f46e895b9fe295dcfc5a904", } - expectEqual(t, fc, expectedSecret(t, opts)) - expectEqual(t, fc, expectedSTS(t, fc, opts)) + expectEqual(t, fc, expectedSecret(t, opts), nil) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation) // Add another route to be advertised. mustUpdate[tsapi.Connector](t, fc, "", "test", func(conn *tsapi.Connector) { conn.Spec.SubnetRouter.AdvertiseRoutes = []tsapi.Route{"10.40.0.0/14", "10.44.0.0/20"} }) opts.subnetRoutes = "10.40.0.0/14,10.44.0.0/20" - opts.confFileHash = "fb6c4daf67425f983985750cd8d6f2beae77e614fcb34176604571f5623d6862" expectReconciled(t, cr, "", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts)) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation) // Remove a route. mustUpdate[tsapi.Connector](t, fc, "", "test", func(conn *tsapi.Connector) { conn.Spec.SubnetRouter.AdvertiseRoutes = []tsapi.Route{"10.44.0.0/20"} }) opts.subnetRoutes = "10.44.0.0/20" - opts.confFileHash = "bacba177bcfe3849065cf6fee53d658a9bb4144197ac5b861727d69ea99742bb" expectReconciled(t, cr, "", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts)) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation) // Remove the subnet router. mustUpdate[tsapi.Connector](t, fc, "", "test", func(conn *tsapi.Connector) { conn.Spec.SubnetRouter = nil }) opts.subnetRoutes = "" - opts.confFileHash = "7c421a99128eb80e79a285a82702f19f8f720615542a15bd794858a6275d8079" expectReconciled(t, cr, "", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts)) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation) // Re-add the subnet router. mustUpdate[tsapi.Connector](t, fc, "", "test", func(conn *tsapi.Connector) { @@ -113,9 +109,8 @@ func TestConnector(t *testing.T) { } }) opts.subnetRoutes = "10.44.0.0/20" - opts.confFileHash = "bacba177bcfe3849065cf6fee53d658a9bb4144197ac5b861727d69ea99742bb" expectReconciled(t, cr, "", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts)) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation) // Delete the Connector. if err = fc.Delete(context.Background(), cn); err != nil { @@ -156,19 +151,17 @@ func TestConnector(t *testing.T) { parentType: "connector", subnetRoutes: "10.40.0.0/14", hostname: "test-connector", - confFileHash: "57d922331890c9b1c8c6ae664394cb254334c551d9cd9db14537b5d9da9fb17e", } - expectEqual(t, fc, expectedSecret(t, opts)) - expectEqual(t, fc, expectedSTS(t, fc, opts)) + expectEqual(t, fc, expectedSecret(t, opts), nil) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation) // Add an exit node. mustUpdate[tsapi.Connector](t, fc, "", "test", func(conn *tsapi.Connector) { conn.Spec.ExitNode = true }) opts.isExitNode = true - opts.confFileHash = "1499b591fd97a50f0330db6ec09979792c49890cf31f5da5bb6a3f50dba1e77a" expectReconciled(t, cr, "", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts)) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation) // Delete the Connector. if err = fc.Delete(context.Background(), cn); err != nil { @@ -243,10 +236,9 @@ func TestConnectorWithProxyClass(t *testing.T) { hostname: "test-connector", isExitNode: true, subnetRoutes: "10.40.0.0/14", - confFileHash: "9321660203effb80983eaecc7b5ac5a8c53934926f46e895b9fe295dcfc5a904", } - expectEqual(t, fc, expectedSecret(t, opts)) - expectEqual(t, fc, expectedSTS(t, fc, opts)) + expectEqual(t, fc, expectedSecret(t, opts), nil) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation) // 2. Update Connector to specify a ProxyClass. ProxyClass is not yet // ready, so its configuration is NOT applied to the Connector @@ -255,7 +247,7 @@ func TestConnectorWithProxyClass(t *testing.T) { conn.Spec.ProxyClass = "custom-metadata" }) expectReconciled(t, cr, "", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts)) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation) // 3. ProxyClass is set to Ready by proxy-class reconciler. Connector // get reconciled and configuration from the ProxyClass is applied to @@ -269,12 +261,8 @@ func TestConnectorWithProxyClass(t *testing.T) { }}} }) opts.proxyClass = pc.Name - // We lose the auth key on second reconcile, because in code it's set to - // StringData, but is actually read from Data. This works with a real - // API server, but not with our test setup here. - opts.confFileHash = "1499b591fd97a50f0330db6ec09979792c49890cf31f5da5bb6a3f50dba1e77a" expectReconciled(t, cr, "", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts)) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation) // 4. Connector.spec.proxyClass field is unset, Connector gets // reconciled and configuration from the ProxyClass is removed from the @@ -284,5 +272,5 @@ func TestConnectorWithProxyClass(t *testing.T) { }) opts.proxyClass = "" expectReconciled(t, cr, "", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts)) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation) } diff --git a/cmd/k8s-operator/ingress_test.go b/cmd/k8s-operator/ingress_test.go index a7cec1a81df5b..8fc68a8c72cd6 100644 --- a/cmd/k8s-operator/ingress_test.go +++ b/cmd/k8s-operator/ingress_test.go @@ -88,12 +88,11 @@ func TestTailscaleIngress(t *testing.T) { fullName, shortName := findGenName(t, fc, "default", "test", "ingress") opts := configOpts{ - stsName: shortName, - secretName: fullName, - namespace: "default", - parentType: "ingress", - hostname: "default-test", - confFileHash: "6cceb342cd3e1c56cd1bd94c29df63df3653c35fe98a7e7afcdee0dcaa2ad549", + stsName: shortName, + secretName: fullName, + namespace: "default", + parentType: "ingress", + hostname: "default-test", } serveConfig := &ipn.ServeConfig{ TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}}, @@ -101,9 +100,9 @@ func TestTailscaleIngress(t *testing.T) { } opts.serveConfig = serveConfig - expectEqual(t, fc, expectedSecret(t, opts)) - expectEqual(t, fc, expectedHeadlessService(shortName, "ingress")) - expectEqual(t, fc, expectedSTSUserspace(t, fc, opts)) + expectEqual(t, fc, expectedSecret(t, opts), nil) + expectEqual(t, fc, expectedHeadlessService(shortName, "ingress"), nil) + expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation) // 2. Ingress status gets updated with ingress proxy's MagicDNS name // once that becomes available. @@ -118,7 +117,7 @@ func TestTailscaleIngress(t *testing.T) { {Hostname: "foo.tailnetxyz.ts.net", Ports: []networkingv1.IngressPortStatus{{Port: 443, Protocol: "TCP"}}}, }, } - expectEqual(t, fc, ing) + expectEqual(t, fc, ing, nil) // 3. Resources get created for Ingress that should allow forwarding // cluster traffic @@ -126,11 +125,8 @@ func TestTailscaleIngress(t *testing.T) { mak.Set(&ing.ObjectMeta.Annotations, AnnotationExperimentalForwardClusterTrafficViaL7IngresProxy, "true") }) opts.shouldEnableForwardingClusterTrafficViaIngress = true - // configfile hash changed at this point in test env only because we - // lost auth key due to how changes are applied in test client. - opts.confFileHash = "fb9006e30ecda75e88c29dcd0ca2dd28a2ae964d001c66e1be3efe159cc3821d" expectReconciled(t, ingR, "default", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts)) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation) // 4. Resources get cleaned up when Ingress class is unset mustUpdate(t, fc, "default", "test", func(ing *networkingv1.Ingress) { @@ -223,12 +219,11 @@ func TestTailscaleIngressWithProxyClass(t *testing.T) { fullName, shortName := findGenName(t, fc, "default", "test", "ingress") opts := configOpts{ - stsName: shortName, - secretName: fullName, - namespace: "default", - parentType: "ingress", - hostname: "default-test", - confFileHash: "6cceb342cd3e1c56cd1bd94c29df63df3653c35fe98a7e7afcdee0dcaa2ad549", + stsName: shortName, + secretName: fullName, + namespace: "default", + parentType: "ingress", + hostname: "default-test", } serveConfig := &ipn.ServeConfig{ TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}}, @@ -236,9 +231,9 @@ func TestTailscaleIngressWithProxyClass(t *testing.T) { } opts.serveConfig = serveConfig - expectEqual(t, fc, expectedSecret(t, opts)) - expectEqual(t, fc, expectedHeadlessService(shortName, "ingress")) - expectEqual(t, fc, expectedSTSUserspace(t, fc, opts)) + expectEqual(t, fc, expectedSecret(t, opts), nil) + expectEqual(t, fc, expectedHeadlessService(shortName, "ingress"), nil) + expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation) // 2. Ingress is updated to specify a ProxyClass, ProxyClass is not yet // ready, so proxy resource configuration does not change. @@ -246,7 +241,7 @@ func TestTailscaleIngressWithProxyClass(t *testing.T) { mak.Set(&ing.ObjectMeta.Labels, LabelProxyClass, "custom-metadata") }) expectReconciled(t, ingR, "default", "test") - expectEqual(t, fc, expectedSTSUserspace(t, fc, opts)) + expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation) // 3. ProxyClass is set to Ready by proxy-class reconciler. Ingress get // reconciled and configuration from the ProxyClass is applied to the @@ -261,10 +256,7 @@ func TestTailscaleIngressWithProxyClass(t *testing.T) { }) expectReconciled(t, ingR, "default", "test") opts.proxyClass = pc.Name - // configfile hash changed at this point in test env only because we - // lost auth key due to how changes are applied in test client. - opts.confFileHash = "fb9006e30ecda75e88c29dcd0ca2dd28a2ae964d001c66e1be3efe159cc3821d" - expectEqual(t, fc, expectedSTSUserspace(t, fc, opts)) + expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation) // 4. tailscale.com/proxy-class label is removed from the Ingress, the // Ingress gets reconciled and the custom ProxyClass configuration is @@ -274,5 +266,5 @@ func TestTailscaleIngressWithProxyClass(t *testing.T) { }) expectReconciled(t, ingR, "default", "test") opts.proxyClass = "" - expectEqual(t, fc, expectedSTSUserspace(t, fc, opts)) + expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation) } diff --git a/cmd/k8s-operator/operator_test.go b/cmd/k8s-operator/operator_test.go index 3dba7325ff38d..7188ce65bbf5f 100644 --- a/cmd/k8s-operator/operator_test.go +++ b/cmd/k8s-operator/operator_test.go @@ -71,12 +71,11 @@ func TestLoadBalancerClass(t *testing.T) { parentType: "svc", hostname: "default-test", clusterTargetIP: "10.20.30.40", - confFileHash: "6cceb342cd3e1c56cd1bd94c29df63df3653c35fe98a7e7afcdee0dcaa2ad549", } - expectEqual(t, fc, expectedSecret(t, opts)) - expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, opts)) + expectEqual(t, fc, expectedSecret(t, opts), nil) + expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation) // Normally the Tailscale proxy pod would come up here and write its info // into the secret. Simulate that, then verify reconcile again and verify @@ -119,7 +118,7 @@ func TestLoadBalancerClass(t *testing.T) { }, }, } - expectEqual(t, fc, want) + expectEqual(t, fc, want, nil) // Turn the service back into a ClusterIP service, which should make the // operator clean up. @@ -158,7 +157,7 @@ func TestLoadBalancerClass(t *testing.T) { Type: corev1.ServiceTypeClusterIP, }, } - expectEqual(t, fc, want) + expectEqual(t, fc, want, nil) } func TestTailnetTargetFQDNAnnotation(t *testing.T) { @@ -213,12 +212,11 @@ func TestTailnetTargetFQDNAnnotation(t *testing.T) { parentType: "svc", tailnetTargetFQDN: tailnetTargetFQDN, hostname: "default-test", - confFileHash: "6cceb342cd3e1c56cd1bd94c29df63df3653c35fe98a7e7afcdee0dcaa2ad549", } - expectEqual(t, fc, expectedSecret(t, o)) - expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o)) + expectEqual(t, fc, expectedSecret(t, o), nil) + expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil) + expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation) want := &corev1.Service{ TypeMeta: metav1.TypeMeta{ Kind: "Service", @@ -239,10 +237,10 @@ func TestTailnetTargetFQDNAnnotation(t *testing.T) { Selector: nil, }, } - expectEqual(t, fc, want) - expectEqual(t, fc, expectedSecret(t, o)) - expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o)) + expectEqual(t, fc, want, nil) + expectEqual(t, fc, expectedSecret(t, o), nil) + expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil) + expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation) // Change the tailscale-target-fqdn annotation which should update the // StatefulSet @@ -324,12 +322,11 @@ func TestTailnetTargetIPAnnotation(t *testing.T) { parentType: "svc", tailnetTargetIP: tailnetTargetIP, hostname: "default-test", - confFileHash: "6cceb342cd3e1c56cd1bd94c29df63df3653c35fe98a7e7afcdee0dcaa2ad549", } - expectEqual(t, fc, expectedSecret(t, o)) - expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o)) + expectEqual(t, fc, expectedSecret(t, o), nil) + expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil) + expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation) want := &corev1.Service{ TypeMeta: metav1.TypeMeta{ Kind: "Service", @@ -350,10 +347,10 @@ func TestTailnetTargetIPAnnotation(t *testing.T) { Selector: nil, }, } - expectEqual(t, fc, want) - expectEqual(t, fc, expectedSecret(t, o)) - expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o)) + expectEqual(t, fc, want, nil) + expectEqual(t, fc, expectedSecret(t, o), nil) + expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil) + expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation) // Change the tailscale-target-ip annotation which should update the // StatefulSet @@ -432,12 +429,11 @@ func TestAnnotations(t *testing.T) { parentType: "svc", hostname: "default-test", clusterTargetIP: "10.20.30.40", - confFileHash: "6cceb342cd3e1c56cd1bd94c29df63df3653c35fe98a7e7afcdee0dcaa2ad549", } - expectEqual(t, fc, expectedSecret(t, o)) - expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o)) + expectEqual(t, fc, expectedSecret(t, o), nil) + expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil) + expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation) want := &corev1.Service{ TypeMeta: metav1.TypeMeta{ Kind: "Service", @@ -457,7 +453,7 @@ func TestAnnotations(t *testing.T) { Type: corev1.ServiceTypeClusterIP, }, } - expectEqual(t, fc, want) + expectEqual(t, fc, want, nil) // Turn the service back into a ClusterIP service, which should make the // operator clean up. @@ -489,7 +485,7 @@ func TestAnnotations(t *testing.T) { Type: corev1.ServiceTypeClusterIP, }, } - expectEqual(t, fc, want) + expectEqual(t, fc, want, nil) } func TestAnnotationIntoLB(t *testing.T) { @@ -541,12 +537,11 @@ func TestAnnotationIntoLB(t *testing.T) { parentType: "svc", hostname: "default-test", clusterTargetIP: "10.20.30.40", - confFileHash: "6cceb342cd3e1c56cd1bd94c29df63df3653c35fe98a7e7afcdee0dcaa2ad549", } - expectEqual(t, fc, expectedSecret(t, o)) - expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o)) + expectEqual(t, fc, expectedSecret(t, o), nil) + expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil) + expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation) // Normally the Tailscale proxy pod would come up here and write its info // into the secret. Simulate that, since it would have normally happened at @@ -579,7 +574,7 @@ func TestAnnotationIntoLB(t *testing.T) { Type: corev1.ServiceTypeClusterIP, }, } - expectEqual(t, fc, want) + expectEqual(t, fc, want, nil) // Remove Tailscale's annotation, and at the same time convert the service // into a tailscale LoadBalancer. @@ -590,10 +585,8 @@ func TestAnnotationIntoLB(t *testing.T) { }) expectReconciled(t, sr, "default", "test") // None of the proxy machinery should have changed... - // (although configfile hash will change in test env only because we lose auth key due to out test not syncing secret.StringData -> secret.Data) - o.confFileHash = "fb9006e30ecda75e88c29dcd0ca2dd28a2ae964d001c66e1be3efe159cc3821d" - expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o)) + expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil) + expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation) // ... but the service should have a LoadBalancer status. want = &corev1.Service{ @@ -625,7 +618,7 @@ func TestAnnotationIntoLB(t *testing.T) { }, }, } - expectEqual(t, fc, want) + expectEqual(t, fc, want, nil) } func TestLBIntoAnnotation(t *testing.T) { @@ -675,12 +668,11 @@ func TestLBIntoAnnotation(t *testing.T) { parentType: "svc", hostname: "default-test", clusterTargetIP: "10.20.30.40", - confFileHash: "6cceb342cd3e1c56cd1bd94c29df63df3653c35fe98a7e7afcdee0dcaa2ad549", } - expectEqual(t, fc, expectedSecret(t, o)) - expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o)) + expectEqual(t, fc, expectedSecret(t, o), nil) + expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil) + expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation) // Normally the Tailscale proxy pod would come up here and write its info // into the secret. Simulate that, then verify reconcile again and verify @@ -723,7 +715,7 @@ func TestLBIntoAnnotation(t *testing.T) { }, }, } - expectEqual(t, fc, want) + expectEqual(t, fc, want, nil) // Turn the service back into a ClusterIP service, but also add the // tailscale annotation. @@ -742,12 +734,8 @@ func TestLBIntoAnnotation(t *testing.T) { }) expectReconciled(t, sr, "default", "test") - // configfile hash changes on a re-apply in this case in tests only as - // we lose the auth key due to the test apply not syncing - // secret.StringData -> Data. - o.confFileHash = "fb9006e30ecda75e88c29dcd0ca2dd28a2ae964d001c66e1be3efe159cc3821d" - expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o)) + expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil) + expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation) want = &corev1.Service{ TypeMeta: metav1.TypeMeta{ @@ -768,7 +756,7 @@ func TestLBIntoAnnotation(t *testing.T) { Type: corev1.ServiceTypeClusterIP, }, } - expectEqual(t, fc, want) + expectEqual(t, fc, want, nil) } func TestCustomHostname(t *testing.T) { @@ -821,12 +809,11 @@ func TestCustomHostname(t *testing.T) { parentType: "svc", hostname: "reindeer-flotilla", clusterTargetIP: "10.20.30.40", - confFileHash: "42376226c7d76ed6d6318315dc6c402f7d993bc0b01a5b0e6c8a833106b7509e", } - expectEqual(t, fc, expectedSecret(t, o)) - expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o)) + expectEqual(t, fc, expectedSecret(t, o), nil) + expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil) + expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation) want := &corev1.Service{ TypeMeta: metav1.TypeMeta{ Kind: "Service", @@ -847,7 +834,7 @@ func TestCustomHostname(t *testing.T) { Type: corev1.ServiceTypeClusterIP, }, } - expectEqual(t, fc, want) + expectEqual(t, fc, want, nil) // Turn the service back into a ClusterIP service, which should make the // operator clean up. @@ -882,7 +869,7 @@ func TestCustomHostname(t *testing.T) { Type: corev1.ServiceTypeClusterIP, }, } - expectEqual(t, fc, want) + expectEqual(t, fc, want, nil) } func TestCustomPriorityClassName(t *testing.T) { @@ -937,10 +924,9 @@ func TestCustomPriorityClassName(t *testing.T) { hostname: "tailscale-critical", priorityClassName: "custom-priority-class-name", clusterTargetIP: "10.20.30.40", - confFileHash: "13cdef0d5f6f0f2406af028710ea1e0f99f65aba4021e4e70ac75a73cf141fd1", } - expectEqual(t, fc, expectedSTS(t, fc, o)) + expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation) } func TestProxyClassForService(t *testing.T) { @@ -1000,11 +986,10 @@ func TestProxyClassForService(t *testing.T) { parentType: "svc", hostname: "default-test", clusterTargetIP: "10.20.30.40", - confFileHash: "6cceb342cd3e1c56cd1bd94c29df63df3653c35fe98a7e7afcdee0dcaa2ad549", } - expectEqual(t, fc, expectedSecret(t, opts)) - expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, opts)) + expectEqual(t, fc, expectedSecret(t, opts), nil) + expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation) // 2. The Service gets updated with tailscale.com/proxy-class label // pointing at the 'custom-metadata' ProxyClass. The ProxyClass is not @@ -1013,7 +998,7 @@ func TestProxyClassForService(t *testing.T) { mak.Set(&svc.Labels, LabelProxyClass, "custom-metadata") }) expectReconciled(t, sr, "default", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts)) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation) // 3. ProxyClass is set to Ready, the Service gets reconciled by the // services-reconciler and the customization from the ProxyClass is @@ -1027,12 +1012,8 @@ func TestProxyClassForService(t *testing.T) { }}} }) opts.proxyClass = pc.Name - // configfile hash changes on a second apply in test env only because we - // lose auth key due to out test not syncing secret.StringData -> - // secret.Data - opts.confFileHash = "fb9006e30ecda75e88c29dcd0ca2dd28a2ae964d001c66e1be3efe159cc3821d" expectReconciled(t, sr, "default", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts)) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation) // 4. tailscale.com/proxy-class label is removed from the Service, the // configuration from the ProxyClass is removed from the cluster @@ -1042,7 +1023,7 @@ func TestProxyClassForService(t *testing.T) { }) opts.proxyClass = "" expectReconciled(t, sr, "default", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts)) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation) } func TestDefaultLoadBalancer(t *testing.T) { @@ -1086,7 +1067,7 @@ func TestDefaultLoadBalancer(t *testing.T) { fullName, shortName := findGenName(t, fc, "default", "test", "svc") - expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) + expectEqual(t, fc, expectedHeadlessService(shortName, "svc"), nil) o := configOpts{ stsName: shortName, secretName: fullName, @@ -1094,9 +1075,9 @@ func TestDefaultLoadBalancer(t *testing.T) { parentType: "svc", hostname: "default-test", clusterTargetIP: "10.20.30.40", - confFileHash: "6cceb342cd3e1c56cd1bd94c29df63df3653c35fe98a7e7afcdee0dcaa2ad549", } - expectEqual(t, fc, expectedSTS(t, fc, o)) + expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation) + } func TestProxyFirewallMode(t *testing.T) { @@ -1148,10 +1129,70 @@ func TestProxyFirewallMode(t *testing.T) { hostname: "default-test", firewallMode: "nftables", clusterTargetIP: "10.20.30.40", - confFileHash: "6cceb342cd3e1c56cd1bd94c29df63df3653c35fe98a7e7afcdee0dcaa2ad549", } - expectEqual(t, fc, expectedSTS(t, fc, o)) + expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation) +} +func TestTailscaledConfigfileHash(t *testing.T) { + fc := fake.NewFakeClient() + ft := &fakeTSClient{} + zl, err := zap.NewDevelopment() + if err != nil { + t.Fatal(err) + } + sr := &ServiceReconciler{ + Client: fc, + ssr: &tailscaleSTSReconciler{ + Client: fc, + tsClient: ft, + defaultTags: []string{"tag:k8s"}, + operatorNamespace: "operator-ns", + proxyImage: "tailscale/tailscale", + }, + logger: zl.Sugar(), + isDefaultLoadBalancer: true, + } + + // Create a service that we should manage, and check that the initial round + // of objects looks right. + mustCreate(t, fc, &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + // The apiserver is supposed to set the UID, but the fake client + // doesn't. So, set it explicitly because other code later depends + // on it being set. + UID: types.UID("1234-UID"), + }, + Spec: corev1.ServiceSpec{ + ClusterIP: "10.20.30.40", + Type: corev1.ServiceTypeLoadBalancer, + }, + }) + + expectReconciled(t, sr, "default", "test") + + fullName, shortName := findGenName(t, fc, "default", "test", "svc") + o := configOpts{ + stsName: shortName, + secretName: fullName, + namespace: "default", + parentType: "svc", + hostname: "default-test", + clusterTargetIP: "10.20.30.40", + confFileHash: "705e5ffd0bd5326237efdf542c850a65a54101284d5daa30775420fcc64d89c1", + } + expectEqual(t, fc, expectedSTS(t, fc, o), nil) + + // 2. Hostname gets changed, configfile is updated and a new hash value + // is produced. + mustUpdate(t, fc, "default", "test", func(svc *corev1.Service) { + mak.Set(&svc.Annotations, AnnotationHostname, "another-test") + }) + o.hostname = "another-test" + o.confFileHash = "1a087f887825d2b75d3673c7c2b0131f8ec1f0b1cb761d33e236dd28350dfe23" + expectReconciled(t, sr, "default", "test") + expectEqual(t, fc, expectedSTS(t, fc, o), nil) } func Test_isMagicDNSName(t *testing.T) { diff --git a/cmd/k8s-operator/proxyclass_test.go b/cmd/k8s-operator/proxyclass_test.go index aada3b2cb9f29..70b92f29d532f 100644 --- a/cmd/k8s-operator/proxyclass_test.go +++ b/cmd/k8s-operator/proxyclass_test.go @@ -69,7 +69,7 @@ func TestProxyClass(t *testing.T) { LastTransitionTime: &metav1.Time{Time: cl.Now().Truncate(time.Second)}, }) - expectEqual(t, fc, pc) + expectEqual(t, fc, pc, nil) // 2. An invalid ProxyClass resource gets its status updated to Invalid. pc.Spec.StatefulSet.Labels["foo"] = "?!someVal" @@ -79,5 +79,5 @@ func TestProxyClass(t *testing.T) { expectReconciled(t, pcr, "", "test") msg := `ProxyClass is not valid: .spec.statefulSet.labels: Invalid value: "?!someVal": a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')` tsoperator.SetProxyClassCondition(pc, tsapi.ProxyClassready, metav1.ConditionFalse, reasonProxyClassInvalid, msg, 0, cl, zl.Sugar()) - expectEqual(t, fc, pc) + expectEqual(t, fc, pc, nil) } diff --git a/cmd/k8s-operator/sts.go b/cmd/k8s-operator/sts.go index 1c231b1f265f4..8534ef7d24d98 100644 --- a/cmd/k8s-operator/sts.go +++ b/cmd/k8s-operator/sts.go @@ -650,10 +650,11 @@ func applyProxyClassToStatefulSet(pc *tsapi.ProxyClass, ss *appsv1.StatefulSet) // produces returns tailscaled configuration and a hash of that configuration. func tailscaledConfig(stsC *tailscaleSTSConfig, newAuthkey string, oldSecret *corev1.Secret) ([]byte, string, error) { conf := ipn.ConfigVAlpha{ - Version: "alpha0", - AcceptDNS: "false", - Locked: "false", - Hostname: &stsC.Hostname, + Version: "alpha0", + AcceptDNS: "false", + AcceptRoutes: "false", // AcceptRoutes defaults to true + Locked: "false", + Hostname: &stsC.Hostname, } if stsC.Connector != nil { routes, err := netutil.CalcAdvertiseRoutes(stsC.Connector.routes, stsC.Connector.isExitNode) diff --git a/cmd/k8s-operator/testutils_test.go b/cmd/k8s-operator/testutils_test.go index 58c150e5dc5b0..767ab2e0cd18f 100644 --- a/cmd/k8s-operator/testutils_test.go +++ b/cmd/k8s-operator/testutils_test.go @@ -97,7 +97,9 @@ func expectedSTS(t *testing.T, cl client.Client, opts configOpts) *appsv1.Statef ReadOnly: true, MountPath: "/etc/tsconfig", }} - annots["tailscale.com/operator-last-set-config-file-hash"] = opts.confFileHash + if opts.confFileHash != "" { + annots["tailscale.com/operator-last-set-config-file-hash"] = opts.confFileHash + } if opts.firewallMode != "" { tsContainer.Env = append(tsContainer.Env, corev1.EnvVar{ Name: "TS_DEBUG_FIREWALL_MODE", @@ -269,7 +271,6 @@ func expectedSTSUserspace(t *testing.T, cl client.Client, opts configOpts) *apps "tailscale.com/parent-resource-type": opts.parentType, "app": "1234-UID", }, - Annotations: map[string]string{"tailscale.com/operator-last-set-config-file-hash": opts.confFileHash}, }, Spec: corev1.PodSpec{ ServiceAccountName: "proxies", @@ -280,6 +281,10 @@ func expectedSTSUserspace(t *testing.T, cl client.Client, opts configOpts) *apps }, }, } + ss.Spec.Template.Annotations = map[string]string{} + if opts.confFileHash != "" { + ss.Spec.Template.Annotations["tailscale.com/operator-last-set-config-file-hash"] = opts.confFileHash + } // If opts.proxyClass is set, retrieve the ProxyClass and apply // configuration from that to the StatefulSet. if opts.proxyClass != "" { @@ -339,11 +344,12 @@ func expectedSecret(t *testing.T, opts configOpts) *corev1.Secret { mak.Set(&s.StringData, "serve-config", string(serveConfigBs)) } conf := &ipn.ConfigVAlpha{ - Version: "alpha0", - AcceptDNS: "false", - Hostname: &opts.hostname, - Locked: "false", - AuthKey: ptr.To("secret-authkey"), + Version: "alpha0", + AcceptDNS: "false", + Hostname: &opts.hostname, + Locked: "false", + AuthKey: ptr.To("secret-authkey"), + AcceptRoutes: "false", } var routes []netip.Prefix if opts.subnetRoutes != "" || opts.isExitNode { @@ -433,7 +439,13 @@ func mustUpdateStatus[T any, O ptrObject[T]](t *testing.T, client client.Client, } } -func expectEqual[T any, O ptrObject[T]](t *testing.T, client client.Client, want O) { +// expectEqual accepts a Kubernetes object and a Kubernetes client. It tests +// whether an object with equivalent contents can be retrieved by the passed +// client. If you want to NOT test some object fields for equality, ensure that +// they are not present in the passed object and use the modify func to remove +// them from the cluster object. If no such modifications are needed, you can +// pass nil in place of the modify function. +func expectEqual[T any, O ptrObject[T]](t *testing.T, client client.Client, want O, modify func(O)) { t.Helper() got := O(new(T)) if err := client.Get(context.Background(), types.NamespacedName{ @@ -447,6 +459,9 @@ func expectEqual[T any, O ptrObject[T]](t *testing.T, client client.Client, want // so just remove it from both got and want. got.SetResourceVersion("") want.SetResourceVersion("") + if modify != nil { + modify(got) + } if diff := cmp.Diff(got, want); diff != "" { t.Fatalf("unexpected object (-got +want):\n%s", diff) } @@ -543,3 +558,11 @@ func (c *fakeTSClient) Deleted() []string { defer c.Unlock() return c.deleted } + +// removeHashAnnotation can be used to remove declarative tailscaled config hash +// annotation from proxy StatefulSets to make the tests more maintainable (so +// that we don't have to change the annotation in each test case after any +// change to the configfile contents). +func removeHashAnnotation(sts *appsv1.StatefulSet) { + delete(sts.Spec.Template.Annotations, podAnnotationLastSetConfigFileHash) +} diff --git a/ipn/conf.go b/ipn/conf.go index f795e5205d013..ffd4c72693ef3 100644 --- a/ipn/conf.go +++ b/ipn/conf.go @@ -23,8 +23,8 @@ type ConfigVAlpha struct { OperatorUser *string `json:",omitempty"` // local user name who is allowed to operate tailscaled without being root or using sudo Hostname *string `json:",omitempty"` - AcceptDNS opt.Bool `json:"acceptDNS,omitempty"` // --accept-dns - AcceptRoutes opt.Bool `json:"acceptRoutes,omitempty"` + AcceptDNS opt.Bool `json:"acceptDNS,omitempty"` // --accept-dns + AcceptRoutes opt.Bool `json:"acceptRoutes,omitempty"` // --accept-routes defaults to true ExitNode *string `json:"exitNode,omitempty"` // IP, StableID, or MagicDNS base name AllowLANWhileUsingExitNode opt.Bool `json:"allowLANWhileUsingExitNode,omitempty"` From f9cdd9d48861c720b8fcf512bf0d69cb07ca228c Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 20 Mar 2024 06:41:56 -0700 Subject: [PATCH 3/6] control/controlclient: send load balancing hint HTTP request header Updates tailscale/corp#1297 Change-Id: I0b102081e81dfc1261f4b05521ab248a2e4a1298 Signed-off-by: Brad Fitzpatrick (cherry picked from commit 20e9f3369df4cbac167056b4572d1b1fb9ed6f1c) --- control/controlclient/direct.go | 21 +++++++++++++++++---- control/controlclient/noise.go | 5 ++++- control/controlclient/noise_test.go | 2 +- tailcfg/tailcfg.go | 22 ++++++++++++++++++++++ 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 20f51da4eb3de..5eb0254232a87 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -641,6 +641,9 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new if err != nil { return regen, opt.URL, nil, err } + addLBHeader(req, request.OldNodeKey) + addLBHeader(req, request.NodeKey) + res, err := httpc.Do(req) if err != nil { return regen, opt.URL, nil, fmt.Errorf("register request: %w", err) @@ -884,10 +887,11 @@ func (c *Direct) sendMapRequest(ctx context.Context, isStreaming bool, nu Netmap vlogf = c.logf } + nodeKey := persist.PublicNodeKey() request := &tailcfg.MapRequest{ Version: tailcfg.CurrentCapabilityVersion, KeepAlive: true, - NodeKey: persist.PublicNodeKey(), + NodeKey: nodeKey, DiscoKey: c.discoPubKey, Endpoints: eps, EndpointTypes: epTypes, @@ -946,6 +950,7 @@ func (c *Direct) sendMapRequest(ctx context.Context, isStreaming bool, nu Netmap if err != nil { return err } + addLBHeader(req, nodeKey) res, err := httpc.Do(req) if err != nil { @@ -1537,7 +1542,7 @@ func (c *Direct) setDNSNoise(ctx context.Context, req *tailcfg.SetDNSRequest) er if err != nil { return err } - res, err := nc.post(ctx, "/machine/set-dns", &newReq) + res, err := nc.post(ctx, "/machine/set-dns", newReq.NodeKey, &newReq) if err != nil { return err } @@ -1714,8 +1719,10 @@ func (c *Direct) ReportHealthChange(sys health.Subsystem, sysErr error) { // Don't report errors to control if the server doesn't support noise. return } + nodeKey := c.GetPersist().PublicNodeKey() req := &tailcfg.HealthChangeRequest{ - Subsys: string(sys), + Subsys: string(sys), + NodeKey: nodeKey, } if sysErr != nil { req.Error = sysErr.Error() @@ -1724,7 +1731,7 @@ func (c *Direct) ReportHealthChange(sys health.Subsystem, sysErr error) { // Best effort, no logging: ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - res, err := np.post(ctx, "/machine/update-health", req) + res, err := np.post(ctx, "/machine/update-health", nodeKey, req) if err != nil { return } @@ -1768,6 +1775,12 @@ func decodeWrappedAuthkey(key string, logf logger.Logf) (authKey string, isWrapp return authKey, true, sig, priv } +func addLBHeader(req *http.Request, nodeKey key.NodePublic) { + if !nodeKey.IsZero() { + req.Header.Add(tailcfg.LBHeader, nodeKey.String()) + } +} + var ( metricMapRequestsActive = clientmetric.NewGauge("controlclient_map_requests_active") diff --git a/control/controlclient/noise.go b/control/controlclient/noise.go index 660760995157f..f3e5f1bde338b 100644 --- a/control/controlclient/noise.go +++ b/control/controlclient/noise.go @@ -484,7 +484,9 @@ func (nc *NoiseClient) dial(ctx context.Context) (*noiseConn, error) { return ncc, nil } -func (nc *NoiseClient) post(ctx context.Context, path string, body any) (*http.Response, error) { +// post does a POST to the control server at the given path, JSON-encoding body. +// The provided nodeKey is an optional load balancing hint. +func (nc *NoiseClient) post(ctx context.Context, path string, nodeKey key.NodePublic, body any) (*http.Response, error) { jbody, err := json.Marshal(body) if err != nil { return nil, err @@ -493,6 +495,7 @@ func (nc *NoiseClient) post(ctx context.Context, path string, body any) (*http.R if err != nil { return nil, err } + addLBHeader(req, nodeKey) req.Header.Set("Content-Type", "application/json") conn, err := nc.getConn(ctx) diff --git a/control/controlclient/noise_test.go b/control/controlclient/noise_test.go index 9961e3318de45..3ae1fc0e671e3 100644 --- a/control/controlclient/noise_test.go +++ b/control/controlclient/noise_test.go @@ -128,7 +128,7 @@ func (tt noiseClientTest) run(t *testing.T) { checkRes(t, res) // And try using the high-level nc.post API as well. - res, err = nc.post(context.Background(), "/", nil) + res, err = nc.post(context.Background(), "/", key.NodePublic{}, nil) if err != nil { t.Fatal(err) } diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index aeb41ce7e918c..f91403e8644fa 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -2263,6 +2263,10 @@ type SetDNSResponse struct{} type HealthChangeRequest struct { Subsys string // a health.Subsystem value in string form Error string // or empty if cleared + + // NodeKey is the client's current node key. + // In clients <= 1.62.0 it was always the zero value. + NodeKey key.NodePublic } // SSHPolicy is the policy for how to handle incoming SSH connections @@ -2680,3 +2684,21 @@ type EarlyNoise struct { // the client to prove possession of a wireguard private key. NodeKeyChallenge key.ChallengePublic `json:"nodeKeyChallenge"` } + +// LBHeader is the HTTP request header used to provide a load balancer or +// internal reverse proxy with information about the request body without the +// reverse proxy needing to read the body to parse it out. Think of it akin to +// an HTTP Host header or SNI. The value may be absent (notably for old clients) +// but if present, it should match the request. A non-empty value that doesn't +// match the request body's. +// +// The possible values depend on the request path, but for /machine (Noise) +// requests, they'll usually be a node public key (in key.NodePublic.String +// format), matching the Request JSON body's NodeKey. +// +// Note that this is not a security or authentication header; it's strictly +// denormalized redundant data as an optimization. +// +// For some request types, the header may have multiple values. (e.g. OldNodeKey +// vs NodeKey) +const LBHeader = "Ts-Lb" From 7074c49db8bf70bc3516c2b699bc6d3d03a08c98 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 21 Mar 2024 12:19:57 -0700 Subject: [PATCH 4/6] control/controlclient: fix panic regression from earlier load balancer hint header In the recent 20e9f3369 we made HealthChangeRequest machine requests include a NodeKey, as it was the oddball machine request that didn't include one. Unfortunately, that code was sometimes being called (at least in some of our integration tests) without a node key due to its registration with health.RegisterWatcher(direct.ReportHealthChange). Fortunately tests in corp caught this before we cut a release. It's possible this only affects this particular integration test's environment, but still worth fixing. Updates tailscale/corp#1297 Change-Id: I84046779955105763dc1be5121c69fec3c138672 Signed-off-by: Brad Fitzpatrick (cherry picked from commit 8444937c89eb08e41359da38921f90fcae823336) --- control/controlclient/direct.go | 5 ++++- types/persist/persist.go | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 5eb0254232a87..e1a48aad76089 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -1719,7 +1719,10 @@ func (c *Direct) ReportHealthChange(sys health.Subsystem, sysErr error) { // Don't report errors to control if the server doesn't support noise. return } - nodeKey := c.GetPersist().PublicNodeKey() + nodeKey, ok := c.GetPersist().PublicNodeKeyOK() + if !ok { + return + } req := &tailcfg.HealthChangeRequest{ Subsys: string(sys), NodeKey: nodeKey, diff --git a/types/persist/persist.go b/types/persist/persist.go index 19df45dcbd4d3..60c9438e5764c 100644 --- a/types/persist/persist.go +++ b/types/persist/persist.go @@ -51,11 +51,32 @@ func (p *Persist) PublicNodeKey() key.NodePublic { return p.PrivateNodeKey.Public() } +// PublicNodeKeyOK returns the public key for the node key. +// +// Unlike PublicNodeKey, it returns ok=false if there is no node private key +// instead of panicking. +func (p *Persist) PublicNodeKeyOK() (pub key.NodePublic, ok bool) { + if p.PrivateNodeKey.IsZero() { + return + } + return p.PrivateNodeKey.Public(), true +} + // PublicNodeKey returns the public key for the node key. +// +// It panics if there is no node private key. See PublicNodeKeyOK. func (p PersistView) PublicNodeKey() key.NodePublic { return p.ж.PublicNodeKey() } +// PublicNodeKeyOK returns the public key for the node key. +// +// Unlike PublicNodeKey, it returns ok=false if there is no node private key +// instead of panicking. +func (p PersistView) PublicNodeKeyOK() (_ key.NodePublic, ok bool) { + return p.ж.PublicNodeKeyOK() +} + func (p PersistView) Equals(p2 PersistView) bool { return p.ж.Equals(p2.ж) } From 0ad803a3dbbe26b44fbf5b194ba68f882bbf68af Mon Sep 17 00:00:00 2001 From: Irbe Krumina Date: Mon, 25 Mar 2024 19:38:54 +0100 Subject: [PATCH 5/6] util/linuxfw,wgengine/router: enable IPv6 configuration when netfilter is disabled (#11517) Updates #11434 Signed-off-by: James Tucker (cherry picked from commit 3f7313dbdbb86b166b115c59bff444b1dc301e64) Co-authored-by: James Tucker --- util/linuxfw/iptables_runner.go | 2 +- util/linuxfw/linuxfw.go | 2 +- util/linuxfw/nftables_runner.go | 2 +- wgengine/router/router_linux.go | 12 +++++++++--- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/util/linuxfw/iptables_runner.go b/util/linuxfw/iptables_runner.go index 090356798ebe3..196046ce63b05 100644 --- a/util/linuxfw/iptables_runner.go +++ b/util/linuxfw/iptables_runner.go @@ -59,7 +59,7 @@ func newIPTablesRunner(logf logger.Logf) (*iptablesRunner, error) { } supportsV6, supportsV6NAT := false, false - v6err := checkIPv6(logf) + v6err := CheckIPv6(logf) ip6terr := checkIP6TablesExists() var ipt6 *iptables.IPTables switch { diff --git a/util/linuxfw/linuxfw.go b/util/linuxfw/linuxfw.go index 152d2eb07fa84..e4b1929cf4524 100644 --- a/util/linuxfw/linuxfw.go +++ b/util/linuxfw/linuxfw.go @@ -130,7 +130,7 @@ func errCode(err error) int { // missing. It does not check that IPv6 is currently functional or // that there's a global address, just that the system would support // IPv6 if it were on an IPv6 network. -func checkIPv6(logf logger.Logf) error { +func CheckIPv6(logf logger.Logf) error { _, err := os.Stat("/proc/sys/net/ipv6") if os.IsNotExist(err) { return err diff --git a/util/linuxfw/nftables_runner.go b/util/linuxfw/nftables_runner.go index 18d7bd5aee956..e3dc2ec0a1ec3 100644 --- a/util/linuxfw/nftables_runner.go +++ b/util/linuxfw/nftables_runner.go @@ -546,7 +546,7 @@ func newNfTablesRunner(logf logger.Logf) (*nftablesRunner, error) { } nft4 := &nftable{Proto: nftables.TableFamilyIPv4} - v6err := checkIPv6(logf) + v6err := CheckIPv6(logf) if v6err != nil { logf("disabling tunneled IPv6 due to system IPv6 config: %v", v6err) } diff --git a/wgengine/router/router_linux.go b/wgengine/router/router_linux.go index e9c585d98b787..2a1443108a4d4 100644 --- a/wgengine/router/router_linux.go +++ b/wgengine/router/router_linux.go @@ -56,6 +56,7 @@ type linuxRouter struct { // Various feature checks for the network stack. ipRuleAvailable bool // whether kernel was built with IP_MULTIPLE_TABLES + v6Available bool // whether the kernel supports IPv6 fwmaskWorks bool // whether we can use 'ip rule...fwmark /' // ipPolicyPrefBase is the base priority at which ip rules are installed. @@ -142,6 +143,8 @@ func newUserspaceRouterAdvanced(logf logger.Logf, tunname string, netMon *netmon r.logf("mwan3 on openWRT detected, switching policy base priority to 1300") } + r.v6Available = linuxfw.CheckIPv6(r.logf) == nil + r.fixupWSLMTU() return r, nil @@ -416,7 +419,7 @@ func (r *linuxRouter) UpdateMagicsockPort(port uint16, network string) error { case "udp4": magicsockPort = &r.magicsockPortV4 case "udp6": - if !r.nfr.HasIPV6() { + if !r.getV6Available() { return nil } magicsockPort = &r.magicsockPortV6 @@ -523,7 +526,7 @@ func (r *linuxRouter) setNetfilterMode(mode preftype.NetfilterMode) error { return fmt.Errorf("could not add magicsock port rule v4: %w", err) } } - if r.magicsockPortV6 != 0 && r.nfr.HasIPV6() { + if r.magicsockPortV6 != 0 && r.getV6Available() { if err := r.nfr.AddMagicsockPortRule(r.magicsockPortV6, "udp6"); err != nil { return fmt.Errorf("could not add magicsock port rule v6: %w", err) } @@ -563,7 +566,7 @@ func (r *linuxRouter) setNetfilterMode(mode preftype.NetfilterMode) error { return fmt.Errorf("could not add magicsock port rule v4: %w", err) } } - if r.magicsockPortV6 != 0 && r.nfr.HasIPV6() { + if r.magicsockPortV6 != 0 && r.getV6Available() { if err := r.nfr.AddMagicsockPortRule(r.magicsockPortV6, "udp6"); err != nil { return fmt.Errorf("could not add magicsock port rule v6: %w", err) } @@ -602,6 +605,9 @@ func (r *linuxRouter) setNetfilterMode(mode preftype.NetfilterMode) error { } func (r *linuxRouter) getV6Available() bool { + if r.netfilterMode == netfilterOff { + return r.v6Available + } return r.nfr.HasIPV6() } From 2827330c6adacd9a67940621b5f05b589527c550 Mon Sep 17 00:00:00 2001 From: Irbe Krumina Date: Tue, 26 Mar 2024 19:40:57 +0000 Subject: [PATCH 6/6] VERSION.txt: this is v1.62.1 Signed-off-by: Irbe Krumina --- VERSION.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION.txt b/VERSION.txt index 76d0536205683..b77a81dcb12a8 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -1 +1 @@ -1.62.0 +1.62.1