From 109989005d414240bbe730ae1d8688dfe90d7e34 Mon Sep 17 00:00:00 2001 From: Nick Date: Fri, 11 Apr 2025 12:39:08 +0200 Subject: [PATCH] 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: https://github.com/juanfont/headscale/issues/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. --- CHANGELOG.md | 6 +++++- hscontrol/mapper/tail_test.go | 26 ++++++++++++++++++++++++++ hscontrol/types/node.go | 2 +- hscontrol/types/node_test.go | 8 ++++---- integration/auth_oidc_test.go | 9 +++++---- integration/dns_test.go | 2 +- integration/dockertestutil/execute.go | 27 +++++++++++++++++++++++++-- 7 files changed, 67 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a322dcf..e4c0fd81 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/hscontrol/mapper/tail_test.go b/hscontrol/mapper/tail_test.go index 9722df2e..1c3c018f 100644 --- a/hscontrol/mapper/tail_test.go +++ b/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 diff --git a/hscontrol/types/node.go b/hscontrol/types/node.go index c333a148..2e6a0eeb 100644 --- a/hscontrol/types/node.go +++ b/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, ) diff --git a/hscontrol/types/node_test.go b/hscontrol/types/node_test.go index d439d483..702fa251 100644 --- a/hscontrol/types/node_test.go +++ b/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.", }, } diff --git a/integration/auth_oidc_test.go b/integration/auth_oidc_test.go index c86138a8..a036fdd0 100644 --- a/integration/auth_oidc_test.go +++ b/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) } diff --git a/integration/dns_test.go b/integration/dns_test.go index 9bd171f9..77b0f639 100644 --- a/integration/dns_test.go +++ b/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", diff --git a/integration/dockertestutil/execute.go b/integration/dockertestutil/execute.go index 078b3bc2..e77b7cb8 100644 --- a/integration/dockertestutil/execute.go +++ b/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,