From 3101f895a7375266a76149b2c9d1a02f17295358 Mon Sep 17 00:00:00 2001 From: Mike Poindexter Date: Tue, 3 Sep 2024 00:22:17 -0700 Subject: [PATCH] Fix 764 (#2093) * Fix KeyExpiration when a zero time value has a timezone When a zero time value is loaded from JSON or a DB in a way that assigns it the local timezone, it does not roudtrip in JSON as a value for which IsZero returns true. This causes KeyExpiry to be treated as a far past value instead of a nilish value. See https://github.com/golang/go/issues/57040 * Fix whitespace * Ensure that postgresql is used for all tests when env var is set * Pass through value of HEADSCALE_INTEGRATION_POSTGRES env var * Add option to set timezone on headscale container * Add test for registration with auth key in alternate timezone --- .github/workflows/test-integration.yaml | 1 + CHANGELOG.md | 2 + hscontrol/mapper/tail.go | 4 +- hscontrol/mapper/tail_test.go | 66 +++++++++++++++++++++++++ integration/general_test.go | 10 +++- integration/hsic/hsic.go | 6 +++ integration/run.sh | 1 + integration/scenario.go | 8 +-- 8 files changed, 91 insertions(+), 7 deletions(-) diff --git a/.github/workflows/test-integration.yaml b/.github/workflows/test-integration.yaml index aa220261..d5b362b7 100644 --- a/.github/workflows/test-integration.yaml +++ b/.github/workflows/test-integration.yaml @@ -45,6 +45,7 @@ jobs: - TestPingAllByIPPublicDERP - TestAuthKeyLogoutAndRelogin - TestEphemeral + - TestEphemeralInAlternateTimezone - TestEphemeral2006DeletedTooQuickly - TestPingAllByHostname - TestTaildrop diff --git a/CHANGELOG.md b/CHANGELOG.md index fa5d7f74..bbb837fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,8 @@ after improving the test harness as part of adopting [#1460](https://github.com/ - Make registration page easier to use on mobile devices - Make write-ahead-log default on and configurable for SQLite [#1985](https://github.com/juanfont/headscale/pull/1985) - Add APIs for managing headscale policy. [#1792](https://github.com/juanfont/headscale/pull/1792) +- Fix for registering nodes using preauthkeys when running on a postgres database in a non-UTC timezone. [#764](https://github.com/juanfont/headscale/issues/764) +- Make sure integration tests cover postgres for all scenarios ## 0.22.3 (2023-05-12) diff --git a/hscontrol/mapper/tail.go b/hscontrol/mapper/tail.go index d21e4d8d..b0878d1a 100644 --- a/hscontrol/mapper/tail.go +++ b/hscontrol/mapper/tail.go @@ -93,7 +93,7 @@ func tailNode( User: tailcfg.UserID(node.UserID), Key: node.NodeKey, - KeyExpiry: keyExpiry, + KeyExpiry: keyExpiry.UTC(), Machine: node.MachineKey, DiscoKey: node.DiscoKey, @@ -102,7 +102,7 @@ func tailNode( Endpoints: node.Endpoints, DERP: derp, Hostinfo: node.Hostinfo.View(), - Created: node.CreatedAt, + Created: node.CreatedAt.UTC(), Online: node.IsOnline, diff --git a/hscontrol/mapper/tail_test.go b/hscontrol/mapper/tail_test.go index ac50d5a6..f744c9c6 100644 --- a/hscontrol/mapper/tail_test.go +++ b/hscontrol/mapper/tail_test.go @@ -1,6 +1,7 @@ package mapper import ( + "encoding/json" "net/netip" "testing" "time" @@ -205,3 +206,68 @@ func TestTailNode(t *testing.T) { }) } } + +func TestNodeExpiry(t *testing.T) { + tp := func(t time.Time) *time.Time { + return &t + } + tests := []struct { + name string + exp *time.Time + wantTime time.Time + wantTimeZero bool + }{ + { + name: "no-expiry", + exp: nil, + wantTimeZero: true, + }, + { + name: "zero-expiry", + exp: &time.Time{}, + wantTimeZero: true, + }, + { + name: "localtime", + exp: tp(time.Time{}.Local()), + wantTimeZero: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + node := &types.Node{ + GivenName: "test", + Expiry: tt.exp, + } + tn, err := tailNode( + node, + 0, + &policy.ACLPolicy{}, + &types.Config{}, + ) + if err != nil { + t.Fatalf("nodeExpiry() error = %v", err) + } + + // Round trip the node through JSON to ensure the time is serialized correctly + seri, err := json.Marshal(tn) + if err != nil { + t.Fatalf("nodeExpiry() error = %v", err) + } + var deseri tailcfg.Node + err = json.Unmarshal(seri, &deseri) + if err != nil { + t.Fatalf("nodeExpiry() error = %v", err) + } + + if tt.wantTimeZero { + if !deseri.KeyExpiry.IsZero() { + t.Errorf("nodeExpiry() = %v, want zero", deseri.KeyExpiry) + } + } else if deseri.KeyExpiry != tt.wantTime { + t.Errorf("nodeExpiry() = %v, want %v", deseri.KeyExpiry, tt.wantTime) + } + }) + } +} diff --git a/integration/general_test.go b/integration/general_test.go index 2819edb2..6de00fd2 100644 --- a/integration/general_test.go +++ b/integration/general_test.go @@ -215,6 +215,14 @@ func TestAuthKeyLogoutAndRelogin(t *testing.T) { } func TestEphemeral(t *testing.T) { + testEphemeralWithOptions(t, hsic.WithTestName("ephemeral")) +} + +func TestEphemeralInAlternateTimezone(t *testing.T) { + testEphemeralWithOptions(t, hsic.WithTestName("ephemeral-tz"), hsic.WithTimezone("America/Los_Angeles")) +} + +func testEphemeralWithOptions(t *testing.T, opts ...hsic.Option) { IntegrationSkip(t) t.Parallel() @@ -227,7 +235,7 @@ func TestEphemeral(t *testing.T) { "user2": len(MustTestVersions), } - headscale, err := scenario.Headscale(hsic.WithTestName("ephemeral")) + headscale, err := scenario.Headscale(opts...) assertNoErrHeadscaleEnv(t, err) for userName, clientCount := range spec { diff --git a/integration/hsic/hsic.go b/integration/hsic/hsic.go index bef05818..b9026225 100644 --- a/integration/hsic/hsic.go +++ b/integration/hsic/hsic.go @@ -211,6 +211,12 @@ func WithTuning(batchTimeout time.Duration, mapSessionChanSize int) Option { } } +func WithTimezone(timezone string) Option { + return func(hsic *HeadscaleInContainer) { + hsic.env["TZ"] = timezone + } +} + // New returns a new HeadscaleInContainer instance. func New( pool *dockertest.Pool, diff --git a/integration/run.sh b/integration/run.sh index 8cad3f02..137bcfb7 100755 --- a/integration/run.sh +++ b/integration/run.sh @@ -26,6 +26,7 @@ run_tests() { --volume "$PWD:$PWD" -w "$PWD"/integration \ --volume /var/run/docker.sock:/var/run/docker.sock \ --volume "$PWD"/control_logs:/tmp/control \ + -e "HEADSCALE_INTEGRATION_POSTGRES" \ golang:1 \ go test ./... \ -failfast \ diff --git a/integration/scenario.go b/integration/scenario.go index 6476fd58..075d1fd5 100644 --- a/integration/scenario.go +++ b/integration/scenario.go @@ -249,6 +249,10 @@ func (s *Scenario) Headscale(opts ...hsic.Option) (ControlServer, error) { return headscale, nil } + if usePostgresForTest { + opts = append(opts, hsic.WithPostgres()) + } + headscale, err := hsic.New(s.pool, s.network, opts...) if err != nil { return nil, fmt.Errorf("failed to create headscale container: %w", err) @@ -465,10 +469,6 @@ func (s *Scenario) CreateHeadscaleEnv( tsOpts []tsic.Option, opts ...hsic.Option, ) error { - if usePostgresForTest { - opts = append(opts, hsic.WithPostgres()) - } - headscale, err := s.Headscale(opts...) if err != nil { return err