From 92048385c24fd1297b336b7b2fb3cad04c43e47d Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Thu, 18 Jun 2026 07:37:25 +0000 Subject: [PATCH] servertest: cover API error translation for missing and invalid input Asserts not-found nodes/users/keys return 404 and invalid CIDR/policy return 400. These caught the node-500 and pre-auth-key silent-success bugs fixed alongside. --- docs/v1-ogen/CHANGES.md | 13 ++++ hscontrol/servertest/apiv1_errors_test.go | 95 +++++++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 hscontrol/servertest/apiv1_errors_test.go diff --git a/docs/v1-ogen/CHANGES.md b/docs/v1-ogen/CHANGES.md index 580c6c29..fe80abf8 100644 --- a/docs/v1-ogen/CHANGES.md +++ b/docs/v1-ogen/CHANGES.md @@ -99,6 +99,19 @@ an empty object. **Client impact:** scripts parsing the empty `{}` should read the `result` field (machine-readable output) or rely on the exit code. +### Missing resources return a consistent `404` + +**What:** renaming or expiring an unknown node, and expiring or deleting an +unknown pre-auth key, now return `404 Not Found`. Previously the node +operations surfaced as `500` and the pre-auth key operations reported success +without changing anything. + +**Why:** a missing resource is a client error, not a server error, and an +expire or delete that matched no row should not report success. + +**Client impact:** code that treated these as `500` or as a silent success +should handle `404`. + ## Delivery note (not a shipped behaviour change) The grpc-gateway HTTP facade is replaced wholesale at `/api/v1` by the ogen diff --git a/hscontrol/servertest/apiv1_errors_test.go b/hscontrol/servertest/apiv1_errors_test.go new file mode 100644 index 00000000..e6e20613 --- /dev/null +++ b/hscontrol/servertest/apiv1_errors_test.go @@ -0,0 +1,95 @@ +package servertest_test + +import ( + "context" + "net/http" + "testing" + + apiv1 "github.com/juanfont/headscale/gen/api/v1" +) + +// The not-found cases below exercise the gRPC->HTTP error translation: a state +// "record not found" must surface as an RFC 7807 404, not a 500 or a success. + +func TestAPIv1_Nodes_NotFound(t *testing.T) { + _, client := apiClient(t) + ctx := context.Background() + + const missing = uint64(99999) + + requireProblem(t, client.DeleteNode(ctx, apiv1.DeleteNodeParams{NodeID: missing}), http.StatusNotFound) + + _, err := client.RenameNode(ctx, apiv1.RenameNodeParams{NodeID: missing, NewName: "x"}) + requireProblem(t, err, http.StatusNotFound) + + _, err = client.ExpireNode(ctx, apiv1.ExpireNodeParams{NodeID: missing}) + requireProblem(t, err, http.StatusNotFound) +} + +func TestAPIv1_Users_NotFound(t *testing.T) { + _, client := apiClient(t) + ctx := context.Background() + + const missing = uint64(99999) + + requireProblem(t, client.DeleteUser(ctx, apiv1.DeleteUserParams{ID: missing}), http.StatusNotFound) + + _, err := client.RenameUser(ctx, apiv1.RenameUserParams{OldID: missing, NewName: "x"}) + requireProblem(t, err, http.StatusNotFound) +} + +func TestAPIv1_ApiKeys_NotFound(t *testing.T) { + _, client := apiClient(t) + ctx := context.Background() + + requireProblem(t, client.ExpireApiKey(ctx, &apiv1.ExpireApiKeyReq{ + ID: apiv1.NewOptUint64(99999), + }), http.StatusNotFound) + + requireProblem(t, client.DeleteApiKey(ctx, apiv1.DeleteApiKeyParams{ + Prefix: "nonexistent", + }), http.StatusNotFound) +} + +func TestAPIv1_PreAuthKeys_Errors(t *testing.T) { + _, client := apiClient(t) + ctx := context.Background() + + requireProblem(t, client.ExpirePreAuthKey(ctx, &apiv1.ExpirePreAuthKeyReq{ + ID: apiv1.NewOptUint64(99999), + }), http.StatusNotFound) + + requireProblem(t, client.DeletePreAuthKey(ctx, apiv1.DeletePreAuthKeyParams{ + ID: apiv1.NewOptUint64(99999), + }), http.StatusNotFound) + + // Creating a key for a user that does not exist is a 404. + _, err := client.CreatePreAuthKey(ctx, &apiv1.CreatePreAuthKeyReq{ + User: apiv1.NewOptUint64(99999), + }) + requireProblem(t, err, http.StatusNotFound) +} + +func TestAPIv1_SetApprovedRoutes_InvalidCIDR(t *testing.T) { + srv, client := apiClient(t) + ctx := context.Background() + + user := srv.CreateUser(t, "route-user") + node := srv.CreateNode(t, user, "route-node") + + _, err := client.SetApprovedRoutes(ctx, + &apiv1.SetApprovedRoutesReq{Routes: []string{"not-a-cidr"}}, + apiv1.SetApprovedRoutesParams{NodeID: uint64(node.ID)}, + ) + requireProblem(t, err, http.StatusBadRequest) +} + +func TestAPIv1_SetPolicy_Invalid(t *testing.T) { + _, client := apiClient(t) + ctx := context.Background() + + _, err := client.SetPolicy(ctx, &apiv1.SetPolicyReq{ + Policy: apiv1.NewOptString("{ this is not valid hujson"), + }) + requireProblem(t, err, http.StatusBadRequest) +}