Skip to content

Commit 4c18071

Browse files
dylandreimerinkdylandreimerink
committed
internal/testutils: Change RunWithToken to a subtest API style
This commit changes `RunWithToken` to use a subtest API style. This is more intuitive to use, over the old style where the caller had to do an early return if `RunWithToken` returned true. Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
1 parent 2dd93ba commit 4c18071

File tree

5 files changed

+68
-108
lines changed

5 files changed

+68
-108
lines changed

‎btf/handle_test.go‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -95,29 +95,21 @@ func TestNewHandleFromBTFWithToken(t *testing.T) {
9595
buf, err := b.Marshal(nil, nil)
9696
qt.Assert(t, qt.IsNil(err))
9797

98-
t.Run("no-cmd", func(t *testing.T) {
99-
if testutils.RunWithToken(t, testutils.Delegated{
100-
Cmds: []sys.Cmd{},
101-
// We need to delegate at least one permission, so picking a random map type that we don't use in this test.
102-
Maps: []sys.MapType{sys.BPF_MAP_TYPE_ARRAY},
103-
}) {
104-
return
105-
}
106-
98+
testutils.RunWithToken(t, "no-cmd", testutils.Delegated{
99+
Cmds: []sys.Cmd{},
100+
// We need to delegate at least one permission, so picking a random map type that we don't use in this test.
101+
Maps: []sys.MapType{sys.BPF_MAP_TYPE_ARRAY},
102+
}, func(t *testing.T) {
107103
h, err := btf.NewHandleFromRawBTF(buf)
108104
testutils.SkipIfNotSupported(t, err)
109105
qt.Assert(t, qt.ErrorIs(err, unix.EPERM))
110106
h.Close()
111107
})
112108

113-
t.Run("success", func(t *testing.T) {
114-
if testutils.RunWithToken(t, testutils.Delegated{
115-
Cmds: []sys.Cmd{sys.BPF_BTF_LOAD},
116-
Maps: []sys.MapType{},
117-
}) {
118-
return
119-
}
120-
109+
testutils.RunWithToken(t, "success", testutils.Delegated{
110+
Cmds: []sys.Cmd{sys.BPF_BTF_LOAD},
111+
Maps: []sys.MapType{},
112+
}, func(t *testing.T) {
121113
h, err := btf.NewHandleFromRawBTF(buf)
122114
testutils.SkipIfNotSupported(t, err)
123115
qt.Assert(t, qt.IsNil(err))

‎internal/testutils/token_linux.go‎

Lines changed: 25 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -37,38 +37,37 @@ func init() {
3737
}
3838
}
3939

40-
// RunWithToken sets up an environment with a BPFFS with the provided delegated permissions.
41-
//
42-
// The calling test is effectively forked, this function returns true in the parent process, and false in the child
43-
// process. Only the child process with have access to the token, so the parent process should do an early return.
40+
// RunWithToken runs the provided `fn` in an environment where a BFPFS is setup to provide tokens with the specified
41+
// delegated permissions. The `name` parameter is used to name the subtest that will run `fn`, and the `delegated`
42+
// parameter specifies the permissions that will be delegated to the BPF tokens.
4443
//
4544
// Example usage:
4645
//
47-
// if testutils.RunWithToken(t, testutils.Delegated{
46+
// if testutils.RunWithToken(t, "happy-path-example", testutils.Delegated{
4847
// Cmds: []sys.Cmd{sys.BPF_MAP_CREATE},
4948
// Maps: []sys.MapType{sys.BPF_MAP_TYPE_HASH},
50-
// }) {
51-
// return
52-
// }
53-
// _, err := newMap(t, hashMapSpec, nil)
54-
// qt.Assert(t, qt.IsNil(err))
55-
func RunWithToken(tb testing.TB, delegated Delegated) bool {
56-
tb.Helper()
49+
// }, func(t *testing.T) {
50+
// _, err := newMap(t, hashMapSpec, nil)
51+
// qt.Assert(t, qt.IsNil(err))
52+
// })
53+
func RunWithToken(t *testing.T, name string, delegated Delegated, fn func(t *testing.T)) {
54+
t.Helper()
5755

5856
if !platform.IsLinux {
59-
tb.Skip("BPF tokens only work on Linux")
57+
t.Skip("BPF tokens only work on Linux")
6058
}
6159

62-
SkipOnOldKernel(tb, "6.9", "BPF_TOKEN_CREATE")
60+
SkipOnOldKernel(t, "6.9", "BPF_TOKEN_CREATE")
6361

64-
// Detect when we are running the the child process and bail out.
62+
// Detect when we are running the the child process and call fn.
6563
if _, ok := os.LookupEnv(TOKEN_SUBTEST); ok {
66-
return false
64+
t.Run(name, fn)
65+
return
6766
}
6867

6968
// Run just the current test in a subprocess
7069
args := []string{
71-
"-test.run=^" + tb.Name() + "$",
70+
"-test.run=^" + t.Name() + "/" + name + "$",
7271
}
7372

7473
// If we are running in verbose mode, pass the flag to the subtest as well.
@@ -80,10 +79,10 @@ func RunWithToken(tb testing.TB, delegated Delegated) bool {
8079
// started with a relative path or just the name of the binary, and we want an absolute path to re-exec ourselves.
8180
exe, err := os.Executable()
8281
if err != nil {
83-
tb.Fatal(err)
82+
t.Fatal(err)
8483
}
8584

86-
cmd := exec.CommandContext(tb.Context(), exe, args...)
85+
cmd := exec.CommandContext(t.Context(), exe, args...)
8786

8887
// Pass an environment variable to the subtest to indicate that it should run, and not skip.
8988
cmd.Env = append(cmd.Environ(), TOKEN_SUBTEST+"=1")
@@ -95,7 +94,7 @@ func RunWithToken(tb testing.TB, delegated Delegated) bool {
9594
// Create a socket pair for communication between the parent and child process.
9695
pair, err := unix.Socketpair(unix.AF_UNIX, unix.SOCK_STREAM, 0)
9796
if err != nil {
98-
tb.Fatal(err)
97+
t.Fatal(err)
9998
}
10099
parent, child := pair[0], pair[1]
101100
defer unix.Close(parent)
@@ -104,7 +103,7 @@ func RunWithToken(tb testing.TB, delegated Delegated) bool {
104103
// So find out all UIDs and GIDs of users on the system and map them into the user namespace of the child process.
105104
uidMappings, gidMappings, err := parseUIDsGIDs()
106105
if err != nil {
107-
tb.Fatal(err)
106+
t.Fatal(err)
108107
}
109108

110109
// Let the child process inherit the child's end of the socket pair.
@@ -125,35 +124,33 @@ func RunWithToken(tb testing.TB, delegated Delegated) bool {
125124
err = cmd.Start()
126125
if err != nil {
127126
unix.Close(child)
128-
tb.Fatal(err)
127+
t.Fatal(err)
129128
}
130129
unix.Close(child)
131130

132131
// Receive the bpffs context fd from the child process.
133132
fds, err := recvFDs(1, parent)
134133
if err != nil {
135-
tb.Fatal(err)
134+
t.Fatal(err)
136135
}
137136
bpffs := fds[0]
138137

139138
err = configureDelegated(bpffs, delegated)
140139
if err != nil {
141-
tb.Fatal(err)
140+
t.Fatal(err)
142141
}
143142

144143
// Send back a message, we don't care about the content, just a signal to tell the child process that the bpffs is
145144
// configured and it can proceed with the test.
146145
err = unix.Sendmsg(parent, []byte("done"), nil, nil, 0)
147146
if err != nil {
148-
tb.Fatal(err)
147+
t.Fatal(err)
149148
}
150149

151150
err = cmd.Wait()
152151
if err != nil {
153-
tb.Fatal(err)
152+
t.Fatal(err)
154153
}
155-
156-
return true
157154
}
158155

159156
func parseUIDsGIDs() (uids, gids []syscall.SysProcIDMap, err error) {

‎internal/testutils/token_other.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"github.com/cilium/ebpf/internal"
99
)
1010

11-
func RunWithToken(tb testing.TB, delegated Delegated) bool {
12-
tb.Skip(internal.ErrNotSupportedOnOS)
13-
return false
11+
func RunWithToken(t *testing.T, name string, delegated Delegated, fn func(t *testing.T)) {
12+
t.Skip(internal.ErrNotSupportedOnOS)
1413
}

‎map_test.go‎

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1729,38 +1729,26 @@ func TestLoadWrongPin(t *testing.T) {
17291729
}
17301730

17311731
func TestMapWithToken(t *testing.T) {
1732-
t.Run("no-cmd", func(t *testing.T) {
1733-
if testutils.RunWithToken(t, testutils.Delegated{
1734-
Cmds: []sys.Cmd{},
1735-
Maps: []sys.MapType{sys.BPF_MAP_TYPE_HASH},
1736-
}) {
1737-
return
1738-
}
1739-
1732+
testutils.RunWithToken(t, "no-cmd", testutils.Delegated{
1733+
Cmds: []sys.Cmd{},
1734+
Maps: []sys.MapType{sys.BPF_MAP_TYPE_HASH},
1735+
}, func(t *testing.T) {
17401736
_, err := newMap(t, hashMapSpec, nil)
17411737
qt.Assert(t, qt.ErrorIs(err, unix.EPERM))
17421738
})
17431739

1744-
t.Run("no-map", func(t *testing.T) {
1745-
if testutils.RunWithToken(t, testutils.Delegated{
1746-
Cmds: []sys.Cmd{sys.BPF_MAP_CREATE},
1747-
Maps: []sys.MapType{},
1748-
}) {
1749-
return
1750-
}
1751-
1740+
testutils.RunWithToken(t, "no-map", testutils.Delegated{
1741+
Cmds: []sys.Cmd{sys.BPF_MAP_CREATE},
1742+
Maps: []sys.MapType{},
1743+
}, func(t *testing.T) {
17521744
_, err := newMap(t, hashMapSpec, nil)
17531745
qt.Assert(t, qt.ErrorIs(err, unix.EPERM))
17541746
})
17551747

1756-
t.Run("success", func(t *testing.T) {
1757-
if testutils.RunWithToken(t, testutils.Delegated{
1758-
Cmds: []sys.Cmd{sys.BPF_MAP_CREATE},
1759-
Maps: []sys.MapType{sys.BPF_MAP_TYPE_HASH},
1760-
}) {
1761-
return
1762-
}
1763-
1748+
testutils.RunWithToken(t, "success", testutils.Delegated{
1749+
Cmds: []sys.Cmd{sys.BPF_MAP_CREATE},
1750+
Maps: []sys.MapType{sys.BPF_MAP_TYPE_HASH},
1751+
}, func(t *testing.T) {
17641752
_, err := newMap(t, hashMapSpec, nil)
17651753
qt.Assert(t, qt.IsNil(err))
17661754
})

‎prog_test.go‎

Lines changed: 20 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -917,54 +917,38 @@ func TestProgramLoadBoundToDevice(t *testing.T) {
917917
}
918918

919919
func TestProgramWithToken(t *testing.T) {
920-
t.Run("no-cmd", func(t *testing.T) {
921-
if testutils.RunWithToken(t, testutils.Delegated{
922-
Cmds: []sys.Cmd{},
923-
Progs: []sys.ProgType{sys.BPF_PROG_TYPE_SOCKET_FILTER},
924-
AttachTypes: []sys.AttachType{sys.BPF_CGROUP_INET_INGRESS},
925-
}) {
926-
return
927-
}
928-
920+
testutils.RunWithToken(t, "no-cmd", testutils.Delegated{
921+
Cmds: []sys.Cmd{},
922+
Progs: []sys.ProgType{sys.BPF_PROG_TYPE_SOCKET_FILTER},
923+
AttachTypes: []sys.AttachType{sys.BPF_CGROUP_INET_INGRESS},
924+
}, func(t *testing.T) {
929925
_, err := newProgram(t, basicProgramSpec, nil)
930926
qt.Assert(t, qt.ErrorIs(err, unix.EPERM))
931927
})
932928

933-
t.Run("no-prog", func(t *testing.T) {
934-
if testutils.RunWithToken(t, testutils.Delegated{
935-
Cmds: []sys.Cmd{sys.BPF_PROG_LOAD},
936-
Progs: []sys.ProgType{},
937-
AttachTypes: []sys.AttachType{sys.BPF_CGROUP_INET_INGRESS},
938-
}) {
939-
return
940-
}
941-
929+
testutils.RunWithToken(t, "no-prog", testutils.Delegated{
930+
Cmds: []sys.Cmd{sys.BPF_PROG_LOAD},
931+
Progs: []sys.ProgType{},
932+
AttachTypes: []sys.AttachType{sys.BPF_CGROUP_INET_INGRESS},
933+
}, func(t *testing.T) {
942934
_, err := newProgram(t, basicProgramSpec, nil)
943935
qt.Assert(t, qt.ErrorIs(err, unix.EPERM))
944936
})
945937

946-
t.Run("no-attach-type", func(t *testing.T) {
947-
if testutils.RunWithToken(t, testutils.Delegated{
948-
Cmds: []sys.Cmd{sys.BPF_PROG_LOAD},
949-
Progs: []sys.ProgType{sys.BPF_PROG_TYPE_SOCKET_FILTER},
950-
AttachTypes: []sys.AttachType{},
951-
}) {
952-
return
953-
}
954-
938+
testutils.RunWithToken(t, "no-attach-type", testutils.Delegated{
939+
Cmds: []sys.Cmd{sys.BPF_PROG_LOAD},
940+
Progs: []sys.ProgType{sys.BPF_PROG_TYPE_SOCKET_FILTER},
941+
AttachTypes: []sys.AttachType{},
942+
}, func(t *testing.T) {
955943
_, err := newProgram(t, basicProgramSpec, nil)
956944
qt.Assert(t, qt.ErrorIs(err, unix.EPERM))
957945
})
958946

959-
t.Run("success", func(t *testing.T) {
960-
if testutils.RunWithToken(t, testutils.Delegated{
961-
Cmds: []sys.Cmd{sys.BPF_PROG_LOAD},
962-
Progs: []sys.ProgType{sys.BPF_PROG_TYPE_SOCKET_FILTER},
963-
AttachTypes: []sys.AttachType{sys.BPF_CGROUP_INET_INGRESS},
964-
}) {
965-
return
966-
}
967-
947+
testutils.RunWithToken(t, "success", testutils.Delegated{
948+
Cmds: []sys.Cmd{sys.BPF_PROG_LOAD},
949+
Progs: []sys.ProgType{sys.BPF_PROG_TYPE_SOCKET_FILTER},
950+
AttachTypes: []sys.AttachType{sys.BPF_CGROUP_INET_INGRESS},
951+
}, func(t *testing.T) {
968952
_, err := newProgram(t, basicProgramSpec, nil)
969953
qt.Assert(t, qt.IsNil(err))
970954
})

0 commit comments

Comments
 (0)