personal_interest_mirrors/nebula
1
0 Fork 0
mirror of https://github.com/slackhq/nebula.git synced 2026-11-15 15:46:52 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Drop the info API tests that tested the control socket's code twice

Browse Source 
The endpoint is one type under both servers, so the info API's copies of
the rebind, failed-rebind and cancellation tests ran the control socket's
code a second time, and in-place access changes were pinned twice more.

- info API: the three endpoint tests, the mode test and the wire test's mode step go
- the control socket's group hand-over test stands for both servers
- registry: one Resolve test with both the typed and the worded assertions, in registry_test

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Matt RichardsonandClaude Fable 5.1 committed 2026-10-21 12:26:57 -04:00
1 parent aa04cc446a
commit f2a8883563
4 files changed
+15 -134
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

commands/registry_test.go
+12 -7
View File
@@ -87,19 +87,24 @@ func TestWithheldIsNotReportedAsUnknown(t *testing.T) {
assert.Contains(t, out.String(), "did not understand: no-such-command")
}
// TestResolveTypesTheWithheldRefusal: a surface with status codes tells a withheld
// command from an unknown one by the error's type, never by asking the unfiltered
// registry, which is the one thing a filtered surface must not consult.
func TestResolveTypesTheWithheldRefusal(t *testing.T) {
// TestResolveTellsUnknownFromWithheld: the two cases read differently to an operator,
// and a surface with status codes tells them apart by the error's type, never by asking
// the unfiltered registry, which is the one thing a filtered surface must not consult.
func TestResolveTellsUnknownFromWithheld(t *testing.T) {
r := testRegistry().Filter(notWithheld, testReason)
_, err := r.Resolve("withheld")
c, err := r.Resolve("served")
require.NoError(t, err)
assert.Equal(t, Name("served"), c.Name)
_, err = r.Resolve("withheld")
var withheld *WithheldError
require.ErrorAs(t, err, &withheld)
assert.Contains(t, err.Error(), testReason)
require.ErrorContains(t, err, "withheld is not available on this surface")
require.ErrorContains(t, err, testReason)
_, err = r.Resolve("no-such-command")
require.Error(t, err)
require.ErrorContains(t, err, `unknown command "no-such-command"`)
assert.NotErrorAs(t, err, &withheld, "an unknown name is not a withheld one")
}
commands/request_test.go
-16
View File
@@ -68,22 +68,6 @@ func TestParseLineCarriesWhatWasSetInParseableForm(t *testing.T) {
assert.Equal(t, paramsSeen{Verbose: true, Wait: 2 * time.Second}, (*seen)[0])
}
// TestResolveTellsUnknownFromWithheld: the two cases read differently to an operator,
// and Resolve is the edge that decides which one a name from the wire is.
func TestResolveTellsUnknownFromWithheld(t *testing.T) {
r := testRegistry().Filter(notWithheld, testReason)
c, err := r.Resolve("served")
require.NoError(t, err)
assert.Equal(t, Name("served"), c.Name)
_, err = r.Resolve("withheld")
require.ErrorContains(t, err, "withheld is not available on this surface")
_, err = r.Resolve("no-such-command")
require.ErrorContains(t, err, `unknown command "no-such-command"`)
}
// TestLineRunsTheFlagSetItParsed: the ssh path hands Run the FlagSet the line was parsed
// into, with no trip through a Request. A flag.Func has no String worth re-parsing, so a
// round trip would hand Run an empty value and no error; the line's value must arrive.
localapi/access_unix_test.go
+1 -28
View File
@@ -4,7 +4,6 @@ package localapi
import (
"context"
"net/http"
"os"
"os/user"
"strconv"
@@ -86,7 +85,7 @@ func TestControlAccessFromConfig(t *testing.T) {
// TestControlSocketIsHandedToTheGroup: the socket is created with the group's gid and
// 0660, whatever the umask, and a reload that takes the group away tightens the socket
// in place, without moving it.
// in place, without moving it. The endpoint is shared, so this covers the info API too.
func TestControlSocketIsHandedToTheGroup(t *testing.T) {
old := unix.Umask(0077)
defer unix.Umask(old)
@@ -177,32 +176,6 @@ func TestInfoAccessFromConfig(t *testing.T) {
assert.Nil(t, cfg.Access.Group)
}
// TestSocketIsHandedToTheGroup: the socket is created with the group's gid and the
// mode, and a changed group or mode on a reload is applied to the socket in place.
func TestSocketIsHandedToTheGroup(t *testing.T) {
name, gid := ownGroup(t)
group := &Group{Name: name, ID: gid}
want, err := strconv.ParseUint(gid, 10, 32)
require.NoError(t, err)
s, path := serveInfo(t, infoRegistry(), 0600)
require.NoError(t, s.Rebind(context.Background(), path, Access{Mode: 0660, Group: group}))
fi, err := os.Lstat(path)
require.NoError(t, err)
assert.Equal(t, os.FileMode(0660), fi.Mode().Perm())
assert.Equal(t, uint32(want), socketGroup(t, path))
status, _ := test.HTTPGet(t, path, "/v1/whoami")
assert.Equal(t, http.StatusOK, status, "the same listener is still serving")
// The mode is the operator's to widen further.
require.NoError(t, s.Rebind(context.Background(), path, Access{Mode: 0664, Group: group}))
fi, err = os.Lstat(path)
require.NoError(t, err)
assert.Equal(t, os.FileMode(0664), fi.Mode().Perm())
assert.Equal(t, path, s.Path())
}
// TestRemovingTheGroupHandsTheSocketBack: a reload that drops the group key leaves the
// mode the operator set but returns the socket to the daemon's own group, so the old
// group's members are no longer admitted. The socket is first handed to a group this
localapi/info_test.go
+2 -83
View File
@@ -23,7 +23,6 @@ import (
"github.com/slackhq/nebula/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/sys/unix"
)
type whoamiFlags struct {
@@ -218,28 +217,6 @@ func TestInfoPresentationFlags(t *testing.T) {
assert.Equal(t, compact, off)
}
// TestInfoSocketMode: the mode is the operator's, pinned regardless of the umask, and a
// changed mode on a reload is applied in place rather than by rebinding.
func TestInfoSocketMode(t *testing.T) {
old := unix.Umask(0077)
defer unix.Umask(old)
s, path := serveInfo(t, infoRegistry(), 0660)
fi, err := os.Stat(path)
require.NoError(t, err)
assert.Equal(t, os.FileMode(0660), fi.Mode().Perm())
require.NoError(t, s.Rebind(context.Background(), path, Access{Mode: 0640}))
fi, err = os.Stat(path)
require.NoError(t, err)
assert.Equal(t, os.FileMode(0640), fi.Mode().Perm())
assert.Equal(t, path, s.Path())
status, _ := test.HTTPGet(t, path, "/v1/whoami")
assert.Equal(t, http.StatusOK, status, "the same listener is still serving")
}
// TestInfoModeChangeRefusesASwappedSocket: whoever can write the directory can replace
// the node with a link to a file of their choosing; the daemon must not set the
// operator's mode on that file. The node is seen to be gone, the link is refused by the
@@ -261,54 +238,6 @@ func TestInfoModeChangeRefusesASwappedSocket(t *testing.T) {
assert.Empty(t, s.Path(), "nothing of ours answers at the path, so nothing is served")
}
func TestInfoRebindMovesAndStopRemoves(t *testing.T) {
s, first := serveInfo(t, infoRegistry(), 0600)
second := testSocket(t)
require.NoError(t, s.Rebind(context.Background(), second, Access{Mode: 0600}))
assert.Equal(t, second, s.Path())
_, err := os.Stat(first)
assert.True(t, os.IsNotExist(err), "the old socket is removed")
status, body := test.HTTPGet(t, second, "/v1/whoami?name=alice")
assert.Equal(t, http.StatusOK, status)
assert.JSONEq(t, `{"name":"alice"}`, body)
s.Stop()
assert.Empty(t, s.Path())
_, err = os.Stat(second)
assert.True(t, os.IsNotExist(err), "Stop removes the socket")
}
func TestInfoRebindFailureKeepsTheOldSocket(t *testing.T) {
s, first := serveInfo(t, infoRegistry(), 0600)
err := s.Rebind(context.Background(), filepath.Join(privateBase(t), "missing", "n", "s.sock"), Access{Mode: 0600})
require.Error(t, err)
assert.Equal(t, first, s.Path())
status, _ := test.HTTPGet(t, first, "/v1/whoami")
assert.Equal(t, http.StatusOK, status)
}
func TestInfoContextCancellationStops(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
path := testSocket(t)
s := NewInfoServer(test.NewLogger(), infoRegistry())
require.NoError(t, s.Rebind(ctx, path, Access{Mode: 0600}))
t.Cleanup(s.Stop)
cancel()
require.Eventually(t, func() bool { return s.Path() == "" }, time.Second, 10*time.Millisecond)
_, err := os.Stat(path)
assert.True(t, os.IsNotExist(err))
// And a reload that lands during shutdown must not bring it back.
err = s.Rebind(ctx, path, Access{Mode: 0600})
require.Error(t, err)
assert.ErrorContains(t, err, "shutting down")
}
func TestInfoConfig(t *testing.T) {
load := func(t *testing.T, yaml string) (infoConfig, error) {
t.Helper()
@@ -365,11 +294,8 @@ func TestWireInfoFollowsConfig(t *testing.T) {
ctlPath := testSocket(t)
infoPath := testSocket(t)
sectionWithMode := func(enabled bool, socket, mode string) string {
return fmt.Sprintf("local_api:\n control:\n socket: %s\n info:\n enabled: %t\n socket: %s\n socket_mode: \"%s\"\n", ctlPath, enabled, socket, mode)
}
section := func(enabled bool, socket string) string {
return sectionWithMode(enabled, socket, "0600")
return fmt.Sprintf("local_api:\n control:\n socket: %s\n info:\n enabled: %t\n socket: %s\n", ctlPath, enabled, socket)
}
c := config.NewC(test.NewLogger())
@@ -391,18 +317,11 @@ func TestWireInfoFollowsConfig(t *testing.T) {
require.NoError(t, c.ReloadConfigString(section(true, infoPath)).Err)
assert.Equal(t, infoPath, eps.Info.Path())
// A reload that changes only the mode applies it in place.
require.NoError(t, c.ReloadConfigString(sectionWithMode(true, infoPath, "0660")).Err)
assert.Equal(t, infoPath, eps.Info.Path())
fi, err := os.Stat(infoPath)
require.NoError(t, err)
assert.Equal(t, os.FileMode(0660), fi.Mode().Perm())
// A reload that moves the socket moves it, and removes the old one.
moved := testSocket(t)
require.NoError(t, c.ReloadConfigString(section(true, moved)).Err)
assert.Equal(t, moved, eps.Info.Path())
_, err = os.Stat(infoPath)
_, err := os.Stat(infoPath)
assert.True(t, os.IsNotExist(err))
status, body = test.HTTPGet(t, moved, "/v1/whoami?name=bob")
assert.Equal(t, http.StatusOK, status)