-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
migrate auth tests to common #7 #15578
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -629,6 +629,99 @@ func TestAuthTestInvalidMgmt(t *testing.T) { | |
}) | ||
} | ||
|
||
func TestAuthLeaseRevoke(t *testing.T) { | ||
testRunner.BeforeTest(t) | ||
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | ||
defer cancel() | ||
clus := testRunner.NewCluster(ctx, t, config.WithClusterConfig(config.ClusterConfig{ClusterSize: 1})) | ||
defer clus.Close() | ||
cc := testutils.MustClient(clus.Client()) | ||
testutils.ExecuteUntil(ctx, t, func() { | ||
require.NoErrorf(t, setupAuth(cc, []authRole{testRole}, []authUser{rootUser, testUser}), "failed to enable auth") | ||
rootAuthClient := testutils.MustClient(clus.Client(WithAuth(rootUserName, rootPassword))) | ||
|
||
lresp, err := rootAuthClient.Grant(ctx, 10) | ||
require.NoError(t, err) | ||
err = rootAuthClient.Put(ctx, "key", "value", config.PutOptions{LeaseID: lresp.ID}) | ||
require.NoError(t, err) | ||
|
||
_, err = rootAuthClient.Revoke(ctx, lresp.ID) | ||
require.NoError(t, err) | ||
|
||
_, err = rootAuthClient.Get(ctx, "key", config.GetOptions{}) | ||
require.NoError(t, err) | ||
}) | ||
} | ||
|
||
func TestAuthRoleGet(t *testing.T) { | ||
testRunner.BeforeTest(t) | ||
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | ||
defer cancel() | ||
clus := testRunner.NewCluster(ctx, t, config.WithClusterConfig(config.ClusterConfig{ClusterSize: 1})) | ||
defer clus.Close() | ||
cc := testutils.MustClient(clus.Client()) | ||
testutils.ExecuteUntil(ctx, t, func() { | ||
require.NoErrorf(t, setupAuth(cc, []authRole{testRole}, []authUser{rootUser, testUser}), "failed to enable auth") | ||
rootAuthClient := testutils.MustClient(clus.Client(WithAuth(rootUserName, rootPassword))) | ||
testUserAuthClient := testutils.MustClient(clus.Client(WithAuth(testUserName, testPassword))) | ||
|
||
resp, err := rootAuthClient.RoleGet(ctx, testRoleName) | ||
require.NoError(t, err) | ||
requireRolePermissionEqual(t, testRole, resp.Perm) | ||
|
||
// test-user can get the information of test-role because it belongs to the role | ||
resp, err = testUserAuthClient.RoleGet(ctx, testRoleName) | ||
require.NoError(t, err) | ||
requireRolePermissionEqual(t, testRole, resp.Perm) | ||
// test-user cannot get the information of root because it doesn't belong to the role | ||
_, err = testUserAuthClient.RoleGet(ctx, rootRoleName) | ||
require.ErrorContains(t, err, PermissionDenied) | ||
}) | ||
} | ||
|
||
func TestAuthUserGet(t *testing.T) { | ||
testRunner.BeforeTest(t) | ||
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | ||
defer cancel() | ||
clus := testRunner.NewCluster(ctx, t, config.WithClusterConfig(config.ClusterConfig{ClusterSize: 1})) | ||
defer clus.Close() | ||
cc := testutils.MustClient(clus.Client()) | ||
testutils.ExecuteUntil(ctx, t, func() { | ||
require.NoErrorf(t, setupAuth(cc, []authRole{testRole}, []authUser{rootUser, testUser}), "failed to enable auth") | ||
rootAuthClient := testutils.MustClient(clus.Client(WithAuth(rootUserName, rootPassword))) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Logically this are 2 subtests, so ideally we should run them in tested t.run(...). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please point to which 2 tests? I guess root and test user, respectively? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems not a big deal to me. |
||
testUserAuthClient := testutils.MustClient(clus.Client(WithAuth(testUserName, testPassword))) | ||
|
||
resp, err := rootAuthClient.UserGet(ctx, testUserName) | ||
require.NoError(t, err) | ||
requireUserRolesEqual(t, testUser, resp.Roles) | ||
|
||
// test-user can get the information of test-user itself | ||
resp, err = testUserAuthClient.UserGet(ctx, testUserName) | ||
require.NoError(t, err) | ||
requireUserRolesEqual(t, testUser, resp.Roles) | ||
// test-user cannot get the information of root | ||
_, err = testUserAuthClient.UserGet(ctx, rootUserName) | ||
require.ErrorContains(t, err, PermissionDenied) | ||
}) | ||
} | ||
|
||
func TestAuthRoleList(t *testing.T) { | ||
testRunner.BeforeTest(t) | ||
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | ||
defer cancel() | ||
clus := testRunner.NewCluster(ctx, t, config.WithClusterConfig(config.ClusterConfig{ClusterSize: 1})) | ||
defer clus.Close() | ||
cc := testutils.MustClient(clus.Client()) | ||
testutils.ExecuteUntil(ctx, t, func() { | ||
require.NoErrorf(t, setupAuth(cc, []authRole{testRole}, []authUser{rootUser, testUser}), "failed to enable auth") | ||
rootAuthClient := testutils.MustClient(clus.Client(WithAuth(rootUserName, rootPassword))) | ||
|
||
resp, err := rootAuthClient.RoleList(ctx) | ||
require.NoError(t, err) | ||
requireUserRolesEqual(t, testUser, resp.Roles) | ||
}) | ||
} | ||
|
||
func mustAbsPath(path string) string { | ||
abs, err := filepath.Abs(path) | ||
if err != nil { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we have a test targeting this method specifically, it would be good to assert what's returning actually.
[I know that it was not tested previously... ]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clientv3 Lease revoke response is the type alias of the proto buf generated one. It only contains
ResponseHeader
.I was blindly migrate the old logic to this one. How about assert the revoked lease is no longer in LeaseList response and get response is empty?