Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use require instead of t.Fatal(err) in tests/e2e package #18821

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 51 additions & 85 deletions tests/e2e/ctl_v3_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,73 +131,61 @@ func authTestCertCN(cx ctlCtx) {
}

func authTestFromKeyPerm(cx ctlCtx) {
if err := authEnable(cx); err != nil {
cx.t.Fatal(err)
}
err := authEnable(cx)
require.NoError(cx.t, err)

cx.user, cx.pass = "root", "root"
authSetupTestUser(cx)

// grant keys after z to test-user
cx.user, cx.pass = "root", "root"
if err := ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "z", "\x00", false}); err != nil {
cx.t.Fatal(err)
}
err = ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "z", "\x00", false})
require.NoError(cx.t, err)

// try the granted open ended permission
cx.user, cx.pass = "test-user", "pass"
for i := 0; i < 10; i++ {
key := fmt.Sprintf("z%d", i)
if err := ctlV3Put(cx, key, "val", ""); err != nil {
cx.t.Fatal(err)
}
require.NoError(cx.t, ctlV3Put(cx, key, "val", ""))
}
largeKey := ""
for i := 0; i < 10; i++ {
largeKey += "\xff"
if err := ctlV3Put(cx, largeKey, "val", ""); err != nil {
cx.t.Fatal(err)
}
require.NoError(cx.t, ctlV3Put(cx, largeKey, "val", ""))
}

// try a non granted key
err := ctlV3PutFailPerm(cx, "x", "baz")
err = ctlV3PutFailPerm(cx, "x", "baz")
require.ErrorContains(cx.t, err, "permission denied")

// revoke the open ended permission
cx.user, cx.pass = "root", "root"
if err := ctlV3RoleRevokePermission(cx, "test-role", "z", "", true); err != nil {
cx.t.Fatal(err)
}
err = ctlV3RoleRevokePermission(cx, "test-role", "z", "", true)
require.NoError(cx.t, err)

// try the revoked open ended permission
cx.user, cx.pass = "test-user", "pass"
for i := 0; i < 10; i++ {
key := fmt.Sprintf("z%d", i)
err := ctlV3PutFailPerm(cx, key, "val")
require.ErrorContains(cx.t, err, "permission denied")
require.ErrorContains(cx.t, ctlV3PutFailPerm(cx, key, "val"), "permission denied")
}

// grant the entire keys
cx.user, cx.pass = "root", "root"
if err := ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "", "\x00", false}); err != nil {
cx.t.Fatal(err)
}
err = ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "", "\x00", false})
require.NoError(cx.t, err)

// try keys, of course it must be allowed because test-role has a permission of the entire keys
cx.user, cx.pass = "test-user", "pass"
for i := 0; i < 10; i++ {
key := fmt.Sprintf("z%d", i)
if err := ctlV3Put(cx, key, "val", ""); err != nil {
cx.t.Fatal(err)
}
require.NoError(cx.t, ctlV3Put(cx, key, "val", ""))
}

// revoke the entire keys
cx.user, cx.pass = "root", "root"
if err := ctlV3RoleRevokePermission(cx, "test-role", "", "", true); err != nil {
cx.t.Fatal(err)
}
err = ctlV3RoleRevokePermission(cx, "test-role", "", "", true)
require.NoError(cx.t, err)

// try the revoked entire key permission
cx.user, cx.pass = "test-user", "pass"
Expand All @@ -209,17 +197,15 @@ func authTestFromKeyPerm(cx ctlCtx) {
}

func authTestWatch(cx ctlCtx) {
if err := authEnable(cx); err != nil {
cx.t.Fatal(err)
}
err := authEnable(cx)
require.NoError(cx.t, err)

cx.user, cx.pass = "root", "root"
authSetupTestUser(cx)

// grant a key range
if err := ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "key", "key4", false}); err != nil {
cx.t.Fatal(err)
}
err = ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "key", "key4", false})
require.NoError(cx.t, err)

tests := []struct {
puts []kv
Expand Down Expand Up @@ -286,9 +272,8 @@ func authTestWatch(cx ctlCtx) {
func authTestSnapshot(cx ctlCtx) {
maintenanceInitKeys(cx)

if err := authEnable(cx); err != nil {
cx.t.Fatal(err)
}
err := authEnable(cx)
require.NoError(cx.t, err)

cx.user, cx.pass = "root", "root"
authSetupTestUser(cx)
Expand All @@ -298,20 +283,16 @@ func authTestSnapshot(cx ctlCtx) {

// ordinary user cannot save a snapshot
cx.user, cx.pass = "test-user", "pass"
if err := ctlV3SnapshotSave(cx, fpath); err == nil {
cx.t.Fatal("ordinary user should not be able to save a snapshot")
}
err = ctlV3SnapshotSave(cx, fpath)
require.Errorf(cx.t, err, "ordinary user should not be able to save a snapshot")

// root can save a snapshot
cx.user, cx.pass = "root", "root"
if err := ctlV3SnapshotSave(cx, fpath); err != nil {
cx.t.Fatalf("snapshotTest ctlV3SnapshotSave error (%v)", err)
}
err = ctlV3SnapshotSave(cx, fpath)
require.NoErrorf(cx.t, err, "snapshotTest ctlV3SnapshotSave error")

st, err := getSnapshotStatus(cx, fpath)
if err != nil {
cx.t.Fatalf("snapshotTest getSnapshotStatus error (%v)", err)
}
require.NoErrorf(cx.t, err, "snapshotTest getSnapshotStatus error")
if st.Revision != 4 {
cx.t.Fatalf("expected 4, got %d", st.Revision)
}
Expand All @@ -321,83 +302,68 @@ func authTestSnapshot(cx ctlCtx) {
}

func authTestEndpointHealth(cx ctlCtx) {
if err := authEnable(cx); err != nil {
cx.t.Fatal(err)
}
err := authEnable(cx)
require.NoError(cx.t, err)

cx.user, cx.pass = "root", "root"
authSetupTestUser(cx)

if err := ctlV3EndpointHealth(cx); err != nil {
cx.t.Fatalf("endpointStatusTest ctlV3EndpointHealth error (%v)", err)
}
err = ctlV3EndpointHealth(cx)
require.NoErrorf(cx.t, err, "endpointStatusTest ctlV3EndpointHealth error")

// health checking with an ordinary user "succeeds" since permission denial goes through consensus
cx.user, cx.pass = "test-user", "pass"
if err := ctlV3EndpointHealth(cx); err != nil {
cx.t.Fatalf("endpointStatusTest ctlV3EndpointHealth error (%v)", err)
}
err = ctlV3EndpointHealth(cx)
require.NoErrorf(cx.t, err, "endpointStatusTest ctlV3EndpointHealth error")

// succeed if permissions granted for ordinary user
cx.user, cx.pass = "root", "root"
if err := ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "health", "", false}); err != nil {
cx.t.Fatal(err)
}
err = ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "health", "", false})
require.NoError(cx.t, err)
cx.user, cx.pass = "test-user", "pass"
if err := ctlV3EndpointHealth(cx); err != nil {
cx.t.Fatalf("endpointStatusTest ctlV3EndpointHealth error (%v)", err)
}
}

func certCNAndUsername(cx ctlCtx, noPassword bool) {
if err := authEnable(cx); err != nil {
cx.t.Fatal(err)
}
err := authEnable(cx)
require.NoError(cx.t, err)

cx.user, cx.pass = "root", "root"
authSetupTestUser(cx)

if noPassword {
if err := ctlV3User(cx, []string{"add", "example.com", "--no-password"}, "User example.com created", []string{""}); err != nil {
cx.t.Fatal(err)
}
require.NoError(cx.t, ctlV3User(cx, []string{"add", "example.com", "--no-password"}, "User example.com created", []string{""}))
} else {
if err := ctlV3User(cx, []string{"add", "example.com", "--interactive=false"}, "User example.com created", []string{""}); err != nil {
cx.t.Fatal(err)
}
}
if err := e2e.SpawnWithExpectWithEnv(append(cx.PrefixArgs(), "role", "add", "test-role-cn"), cx.envMap, expect.ExpectedResponse{Value: "Role test-role-cn created"}); err != nil {
cx.t.Fatal(err)
}
if err := ctlV3User(cx, []string{"grant-role", "example.com", "test-role-cn"}, "Role test-role-cn is granted to user example.com", nil); err != nil {
cx.t.Fatal(err)
require.NoError(cx.t, ctlV3User(cx, []string{"add", "example.com", "--interactive=false"}, "User example.com created", []string{""}))
}
err = e2e.SpawnWithExpectWithEnv(append(cx.PrefixArgs(), "role", "add", "test-role-cn"), cx.envMap, expect.ExpectedResponse{Value: "Role test-role-cn created"})
require.NoError(cx.t, err)
err = ctlV3User(cx, []string{"grant-role", "example.com", "test-role-cn"}, "Role test-role-cn is granted to user example.com", nil)
require.NoError(cx.t, err)

// grant a new key for CN based user
if err := ctlV3RoleGrantPermission(cx, "test-role-cn", grantingPerm{true, true, "hoo", "", false}); err != nil {
cx.t.Fatal(err)
}
err = ctlV3RoleGrantPermission(cx, "test-role-cn", grantingPerm{true, true, "hoo", "", false})
require.NoError(cx.t, err)

// grant a new key for username based user
if err := ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "bar", "", false}); err != nil {
cx.t.Fatal(err)
}
err = ctlV3RoleGrantPermission(cx, "test-role", grantingPerm{true, true, "bar", "", false})
require.NoError(cx.t, err)

// try a granted key for CN based user
cx.user, cx.pass = "", ""
if err := ctlV3Put(cx, "hoo", "bar", ""); err != nil {
cx.t.Error(err)
}
err = ctlV3Put(cx, "hoo", "bar", "")
require.NoError(cx.t, err)

// try a granted key for username based user
cx.user, cx.pass = "test-user", "pass"
if err := ctlV3Put(cx, "bar", "bar", ""); err != nil {
cx.t.Error(err)
}
err = ctlV3Put(cx, "bar", "bar", "")
require.NoError(cx.t, err)

// try a non-granted key for both of them
cx.user, cx.pass = "", ""
err := ctlV3PutFailPerm(cx, "baz", "bar")
err = ctlV3PutFailPerm(cx, "baz", "bar")
require.ErrorContains(cx.t, err, "permission denied")

cx.user, cx.pass = "test-user", "pass"
Expand Down
7 changes: 4 additions & 3 deletions tests/e2e/ctl_v3_defrag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package e2e
import (
"testing"

"github.com/stretchr/testify/require"

"go.etcd.io/etcd/pkg/v3/expect"
"go.etcd.io/etcd/tests/v3/framework/e2e"
)
Expand All @@ -28,9 +30,8 @@ func TestCtlV3DefragOffline(t *testing.T) {
func maintenanceInitKeys(cx ctlCtx) {
var kvs = []kv{{"key", "val1"}, {"key", "val2"}, {"key", "val3"}}
for i := range kvs {
if err := ctlV3Put(cx, kvs[i].key, kvs[i].val, ""); err != nil {
cx.t.Fatal(err)
}
err := ctlV3Put(cx, kvs[i].key, kvs[i].val, "")
require.NoError(cx.t, err)
}
}

Expand Down
27 changes: 9 additions & 18 deletions tests/e2e/ctl_v3_elect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ func testElect(cx ctlCtx) {
name := "a"

holder, ch, err := ctlV3Elect(cx, name, "p1", false)
if err != nil {
cx.t.Fatal(err)
}
require.NoError(cx.t, err)

l1 := ""
select {
Expand All @@ -51,9 +49,7 @@ func testElect(cx ctlCtx) {

// blocked process that won't win the election
blocked, ch, err := ctlV3Elect(cx, name, "p2", true)
if err != nil {
cx.t.Fatal(err)
}
require.NoError(cx.t, err)
select {
case <-time.After(100 * time.Millisecond):
case <-ch:
Expand All @@ -62,9 +58,7 @@ func testElect(cx ctlCtx) {

// overlap with a blocker that will win the election
blockAcquire, ch, err := ctlV3Elect(cx, name, "p2", false)
if err != nil {
cx.t.Fatal(err)
}
require.NoError(cx.t, err)
defer func(blockAcquire *expect.ExpectProcess) {
err = blockAcquire.Stop()
require.NoError(cx.t, err)
Expand All @@ -78,22 +72,19 @@ func testElect(cx ctlCtx) {
}

// kill blocked process with clean shutdown
if err = blocked.Signal(os.Interrupt); err != nil {
cx.t.Fatal(err)
}
err = blocked.Signal(os.Interrupt)
require.NoError(cx.t, err)
err = e2e.CloseWithTimeout(blocked, time.Second)
if err != nil {
// due to being blocked, this can potentially get killed and thus exit non-zero sometimes
require.ErrorContains(cx.t, err, "unexpected exit code")
}

// kill the holder with clean shutdown
if err = holder.Signal(os.Interrupt); err != nil {
cx.t.Fatal(err)
}
if err = e2e.CloseWithTimeout(holder, time.Second); err != nil {
cx.t.Fatal(err)
}
err = holder.Signal(os.Interrupt)
require.NoError(cx.t, err)
err = e2e.CloseWithTimeout(holder, time.Second)
require.NoError(cx.t, err)

// blockAcquire should win the election
select {
Expand Down
9 changes: 3 additions & 6 deletions tests/e2e/ctl_v3_grpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.etcd.io/etcd/pkg/v3/expect"
"go.etcd.io/etcd/tests/v3/framework/config"
Expand Down Expand Up @@ -135,9 +136,7 @@ func TestAuthority(t *testing.T) {
assert.NoError(t, err)
for i := 0; i < 100; i++ {
err = client.Put(ctx, "foo", "bar", config.PutOptions{})
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
}

testutils.ExecuteWithTimeout(t, 5*time.Second, func() {
Expand Down Expand Up @@ -166,9 +165,7 @@ func assertAuthority(t *testing.T, expectAuthorityPattern string, clus *e2e.Etcd
line = strings.TrimSuffix(line, "\r")

u, err := url.Parse(clus.Procs[i].EndpointsGRPC()[0])
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
expectAuthority := strings.ReplaceAll(expectAuthorityPattern, "${MEMBER_PORT}", u.Port())
expectLine := fmt.Sprintf(`http2: decoded hpack field header field ":authority" = %q`, expectAuthority)
assert.Truef(t, strings.HasSuffix(line, expectLine), "Got %q expected suffix %q", line, expectLine)
Expand Down
Loading