ensure final dot on node name (#2503)

* ensure final dot on node name

This ensures that nodes which have a base domain set, will have a dot appended to their FQDN.

Resolves: #2501

* improve OIDC TTL expire test

Waiting a bit more than the TTL of the OIDC token seems to remove some flakiness of this test. This furthermore makes use of a go func safe buffer which should avoid race conditions.

Author: thirdeyenick

CHANGELOG.md

@@ -87,7 +87,11 @@ The new policy can be used by setting the environment variable
   [#2493](https://github.com/juanfont/headscale/pull/2493)
   - If a OIDC provider doesn't include the `email_verified` claim in its ID
     tokens, Headscale will attempt to get it from the UserInfo endpoint.
-- Improve performance by only querying relevant nodes from the database for node updates [#2509](https://github.com/juanfont/headscale/pull/2509)
+- Improve performance by only querying relevant nodes from the database for node
+  updates [#2509](https://github.com/juanfont/headscale/pull/2509)
+- node FQDNs in the netmap will now contain a dot (".") at the end. This aligns
+  with behaviour of tailscale.com
+  [#2503](https://github.com/juanfont/headscale/pull/2503)
 
 ## 0.25.1 (2025-02-25)
 

hscontrol/mapper/tail_test.go

@@ -169,6 +169,32 @@ func TestTailNode(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "check-dot-suffix-on-node-name",
+			node: &types.Node{
+				GivenName: "minimal",
+				Hostinfo:  &tailcfg.Hostinfo{},
+			},
+			dnsConfig:  &tailcfg.DNSConfig{},
+			baseDomain: "example.com",
+			want: &tailcfg.Node{
+				// a node name should have a dot appended
+				Name:              "minimal.example.com.",
+				StableID:          "0",
+				HomeDERP:          0,
+				LegacyDERPString:  "127.3.3.40:0",
+				Hostinfo:          hiview(tailcfg.Hostinfo{}),
+				Tags:              []string{},
+				MachineAuthorized: true,
+
+				CapMap: tailcfg.NodeCapMap{
+					tailcfg.CapabilityFileSharing: []tailcfg.RawMessage{},
+					tailcfg.CapabilityAdmin:       []tailcfg.RawMessage{},
+					tailcfg.CapabilitySSH:         []tailcfg.RawMessage{},
+				},
+			},
+			wantErr: false,
+		},
 		// TODO: Add tests to check other aspects of the node conversion:
 		// - With tags and policy
 		// - dnsconfig and basedomain

hscontrol/types/node.go

@@ -364,7 +364,7 @@ func (node *Node) GetFQDN(baseDomain string) (string, error) {
 
 	if baseDomain != "" {
 		hostname = fmt.Sprintf(
-			"%s.%s",
+			"%s.%s.",
 			node.GivenName,
 			baseDomain,
 		)

hscontrol/types/node_test.go

@@ -142,7 +142,7 @@ func TestNodeFQDN(t *testing.T) {
 				},
 			},
 			domain: "example.com",
-			want:   "test.example.com",
+			want:   "test.example.com.",
 		},
 		{
 			name: "all-set",
@@ -153,7 +153,7 @@ func TestNodeFQDN(t *testing.T) {
 				},
 			},
 			domain: "example.com",
-			want:   "test.example.com",
+			want:   "test.example.com.",
 		},
 		{
 			name: "no-given-name",
@@ -171,7 +171,7 @@ func TestNodeFQDN(t *testing.T) {
 				GivenName: strings.Repeat("a", 256),
 			},
 			domain:  "example.com",
-			wantErr: fmt.Sprintf("failed to create valid FQDN (%s.example.com): hostname too long, cannot except 255 ASCII chars", strings.Repeat("a", 256)),
+			wantErr: fmt.Sprintf("failed to create valid FQDN (%s.example.com.): hostname too long, cannot except 255 ASCII chars", strings.Repeat("a", 256)),
 		},
 		{
 			name: "no-dnsconfig",
@@ -182,7 +182,7 @@ func TestNodeFQDN(t *testing.T) {
 				},
 			},
 			domain: "example.com",
-			want:   "test.example.com",
+			want:   "test.example.com.",
 		},
 	}
 

integration/auth_oidc_test.go

@@ -170,10 +170,11 @@ func TestOIDCExpireNodesBasedOnTokenExpiry(t *testing.T) {
 	t.Logf("%d successful pings out of %d (before expiry)", success, len(allClients)*len(allIps))
 
 	// This is not great, but this sadly is a time dependent test, so the
-	// safe thing to do is wait out the whole TTL time before checking if
-	// the clients have logged out. The Wait function can't do it itself
-	// as it has an upper bound of 1 min.
-	time.Sleep(shortAccessTTL)
+	// safe thing to do is wait out the whole TTL time (and a bit more out
+	// of safety reasons) before checking if the clients have logged out.
+	// The Wait function can't do it itself as it has an upper bound of 1
+	// min.
+	time.Sleep(shortAccessTTL + 10*time.Second)
 
 	assertTailscaleNodesLogout(t, allClients)
 }

integration/dns_test.go

@@ -49,7 +49,7 @@ func TestResolveMagicDNS(t *testing.T) {
 			// It is safe to ignore this error as we handled it when caching it
 			peerFQDN, _ := peer.FQDN()
 
-			assert.Equal(t, fmt.Sprintf("%s.headscale.net", peer.Hostname()), peerFQDN)
+			assert.Equal(t, fmt.Sprintf("%s.headscale.net.", peer.Hostname()), peerFQDN)
 
 			command := []string{
 				"tailscale",

integration/dockertestutil/execute.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"sync"
 	"time"
 
 	"github.com/ory/dockertest/v3"
@@ -29,14 +30,36 @@ func ExecuteCommandTimeout(timeout time.Duration) ExecuteCommandOption {
 	})
 }
 
+// buffer is a goroutine safe bytes.buffer
+type buffer struct {
+	store bytes.Buffer
+	mutex sync.Mutex
+}
+
+// Write appends the contents of p to the buffer, growing the buffer as needed. It returns
+// the number of bytes written.
+func (b *buffer) Write(p []byte) (n int, err error) {
+	b.mutex.Lock()
+	defer b.mutex.Unlock()
+	return b.store.Write(p)
+}
+
+// String returns the contents of the unread portion of the buffer
+// as a string.
+func (b *buffer) String() string {
+	b.mutex.Lock()
+	defer b.mutex.Unlock()
+	return b.store.String()
+}
+
 func ExecuteCommand(
 	resource *dockertest.Resource,
 	cmd []string,
 	env []string,
 	options ...ExecuteCommandOption,
 ) (string, string, error) {
-	var stdout bytes.Buffer
-	var stderr bytes.Buffer
+	var stdout = buffer{}
+	var stderr = buffer{}
 
 	execConfig := ExecuteCommandConfig{
 		timeout: dockerExecuteTimeout,