This is an automated email from the ASF dual-hosted git repository.
pbacsko pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/yunikorn-core.git
The following commit(s) were added to refs/heads/master by this push:
new 6c8fd60b [YUNIKORN-2697] Improve usergroup funtion's test coverage
(#901)
6c8fd60b is described below
commit 6c8fd60b82fd0c1973a1c1aa9717fb6bf4440c30
Author: SP12893678 <[email protected]>
AuthorDate: Mon Jul 15 00:49:56 2024 +0200
[YUNIKORN-2697] Improve usergroup funtion's test coverage (#901)
Closes: #901
Signed-off-by: Peter Bacsko <[email protected]>
---
pkg/common/security/usergroup_test.go | 164 ++++++++++++++++++++-----
pkg/common/security/usergroup_test_resolver.go | 25 ++++
2 files changed, 155 insertions(+), 34 deletions(-)
diff --git a/pkg/common/security/usergroup_test.go
b/pkg/common/security/usergroup_test.go
index c1c9873e..0540fe04 100644
--- a/pkg/common/security/usergroup_test.go
+++ b/pkg/common/security/usergroup_test.go
@@ -19,32 +19,76 @@
package security
import (
+ "fmt"
+ "reflect"
"strings"
"testing"
+ "time"
"gotest.tools/v3/assert"
+ "github.com/apache/yunikorn-core/pkg/common"
"github.com/apache/yunikorn-scheduler-interface/lib/go/si"
)
+func (c *UserGroupCache) getUGsize() int {
+ c.lock.RLock()
+ defer c.lock.RUnlock()
+ return len(c.ugs)
+}
+
+func (c *UserGroupCache) getUGGroupSize(user string) int {
+ c.lock.RLock()
+ defer c.lock.RUnlock()
+ ug := c.ugs[user]
+ return len(ug.Groups)
+}
+
+func (c *UserGroupCache) getUGmap() map[string]*UserGroup {
+ c.lock.RLock()
+ defer c.lock.RUnlock()
+ return c.ugs
+}
+
func TestGetUserGroupCache(t *testing.T) {
// get the cache with the test resolver set
testCache := GetUserGroupCache("test")
- if testCache == nil {
- t.Fatal("Cache create failed")
- }
- if len(testCache.ugs) != 0 {
- t.Errorf("Cache not empty: %v", testCache.ugs)
- }
+ assert.Assert(t, testCache != nil, "Cache create failed")
+ assert.Equal(t, 0, testCache.getUGsize(), "Cache is not empty: %v",
testCache.getUGmap())
+
+ testCache.Stop()
+ assert.Assert(t, instance == nil, "instance should be nil")
+ assert.Assert(t, stopped.Load())
+
+ // get the cache with the os resolver set
+ testCache = GetUserGroupCache("os")
+ assert.Assert(t, testCache != nil, "Cache create failed")
+ assert.Equal(t, 0, testCache.getUGsize(), "Cache is not empty: %v",
testCache.getUGmap())
+
+ testCache.Stop()
+ assert.Assert(t, instance == nil, "instance should be nil")
+ assert.Assert(t, stopped.Load())
+
+ // get the cache with the default resolver set
+ testCache = GetUserGroupCache("unknown")
+ assert.Assert(t, testCache != nil, "Cache create failed")
+ assert.Equal(t, 0, testCache.getUGsize(), "Cache is not empty: %v",
testCache.getUGmap())
+
+ testCache.Stop()
+ assert.Assert(t, instance == nil, "instance should be nil")
+ assert.Assert(t, stopped.Load())
+
+ // test for re stop again
+ testCache.Stop()
+ assert.Assert(t, instance == nil, "instance should be nil")
+ assert.Assert(t, stopped.Load())
}
func TestGetUserGroup(t *testing.T) {
testCache := GetUserGroupCache("test")
testCache.resetCache()
// test cache should be empty now
- if len(testCache.ugs) != 0 {
- t.Fatalf("Cache not empty: %v", testCache.ugs)
- }
+ assert.Equal(t, 0, testCache.getUGsize(), "Cache is not empty: %v",
testCache.getUGmap())
ug, err := testCache.GetUserGroup("testuser1")
assert.NilError(t, err, "Lookup should not have failed: testuser1")
@@ -58,12 +102,15 @@ func TestGetUserGroup(t *testing.T) {
if ug.User != "testuser1" || len(ug.Groups) != 2 || ug.resolved == 0 ||
ug.failed {
t.Errorf("User 'testuser1' not resolved correctly: %v", ug)
}
+ testCache.lock.Lock()
cachedUG := testCache.ugs["testuser1"]
if ug.resolved != cachedUG.resolved {
t.Errorf("User 'testuser1' not cached correctly resolution time
differs: %d got %d", ug.resolved, cachedUG.resolved)
}
// click over the clock: if we do not get the cached version the new
time will differ from the cache update
cachedUG.resolved -= 5
+ testCache.lock.Unlock()
+
ug, err = testCache.GetUserGroup("testuser1")
if err != nil || ug.resolved != cachedUG.resolved {
t.Errorf("User 'testuser1' not returned from Cache, resolution
time differs: %d got %d (err = %v)", ug.resolved, cachedUG.resolved, err)
@@ -74,17 +121,14 @@ func TestBrokenUserGroup(t *testing.T) {
testCache := GetUserGroupCache("test")
testCache.resetCache()
// test cache should be empty now
- if len(testCache.ugs) != 0 {
- t.Fatalf("Cache not empty: %v", testCache.ugs)
- }
+ assert.Equal(t, 0, testCache.getUGsize(), "Cache is not empty: %v",
testCache.getUGmap())
ug, err := testCache.GetUserGroup("testuser2")
if err != nil {
t.Error("Lookup should not have failed: testuser2")
}
- if len(testCache.ugs) != 1 {
- t.Errorf("Cache not updated should have 1 entry %d",
len(testCache.ugs))
- }
+
+ assert.Equal(t, 1, testCache.getUGsize(), "Cache not updated should
have 1 entry %d", testCache.getUGmap())
// check returned info: 3 groups etc
if ug.User != "testuser2" || len(ug.Groups) != 3 || ug.resolved == 0 ||
ug.failed {
t.Errorf("User 'testuser2' not resolved correctly: %v", ug)
@@ -98,22 +142,30 @@ func TestBrokenUserGroup(t *testing.T) {
if err != nil {
t.Error("Lookup should not have failed: testuser3")
}
- if len(testCache.ugs) != 2 {
- t.Errorf("Cache not updated should have 2 entries %d",
len(testCache.ugs))
- }
- // check returned info: 4 groups, primary group is duplicate
- if len(ug.Groups) != 4 {
- t.Errorf("User 'testuser3' not resolved correctly: duplicate
primary group not filtered %v", ug)
- }
+
+ assert.Equal(t, 2, testCache.getUGsize(), "Cache not updated should
have 2 entries %d", len(testCache.ugs))
+ assert.Equal(t, 4, testCache.getUGGroupSize("testuser3"), "User
'testuser3' not resolved correctly: duplicate primary group not filtered %v",
ug)
+
+ ug, err = testCache.GetUserGroup("unknown")
+ assert.ErrorContains(t, err, "lookup failed for user: unknown")
+
+ ug, err = testCache.GetUserGroup("testuser4")
+ assert.NilError(t, err)
+
+ ug, err = testCache.GetUserGroup("testuser5")
+ assert.ErrorContains(t, err, "lookup failed for user: testuser5")
+
+ ug, err = testCache.GetUserGroup("invalid-gid-user")
+ assert.ErrorContains(t, err, "lookup failed for user: invalid-gid-user")
+ exceptedGroup := []string{"1_001"}
+ assert.Assert(t, reflect.DeepEqual(ug.Groups, exceptedGroup),
fmt.Errorf("group should be: %v, but got: %v", exceptedGroup, ug.Groups))
}
func TestGetUserGroupFail(t *testing.T) {
testCache := GetUserGroupCache("test")
testCache.resetCache()
// test cache should be empty now
- if len(testCache.ugs) != 0 {
- t.Fatalf("Cache not empty: %v", testCache.ugs)
- }
+ assert.Equal(t, 0, testCache.getUGsize(), "Cache is not empty: %v",
testCache.getUGmap())
// resolve an empty user
ug, err := testCache.GetUserGroup("")
@@ -134,6 +186,7 @@ func TestGetUserGroupFail(t *testing.T) {
if ug.User != "unknown" || len(ug.Groups) != 0 || !ug.failed {
t.Errorf("UserGroup is not empty: %v", ug)
}
+
ug, err = testCache.GetUserGroup("unknown")
if err == nil {
t.Error("Lookup should have failed: unknown user")
@@ -148,9 +201,7 @@ func TestCacheCleanUp(t *testing.T) {
testCache := GetUserGroupCache("test")
testCache.resetCache()
// test cache should be empty now
- if len(testCache.ugs) != 0 {
- t.Fatalf("Cache not empty: %v", testCache.ugs)
- }
+ assert.Equal(t, 0, testCache.getUGsize(), "Cache is not empty: %v",
testCache.getUGmap())
// resolve an existing user
_, err := testCache.GetUserGroup("testuser1")
@@ -162,38 +213,75 @@ func TestCacheCleanUp(t *testing.T) {
t.Error("Lookup should not have failed: testuser2 user")
}
+ testCache.lock.Lock()
ug := testCache.ugs["testuser1"]
if ug.failed {
t.Error("User 'testuser1' not resolved as a success")
}
// expire the successful lookup
ug.resolved -= 2 * poscache
+ testCache.lock.Unlock()
// resolve a non existing user
_, err = testCache.GetUserGroup("unknown")
if err == nil {
t.Error("Lookup should have failed: unknown user")
}
+ testCache.lock.Lock()
ug = testCache.ugs["unknown"]
if !ug.failed {
t.Error("User 'unknown' not resolved as a failure")
}
// expire the failed lookup
ug.resolved -= 2 * negcache
+ testCache.lock.Unlock()
testCache.cleanUpCache()
- if len(testCache.ugs) != 1 {
- t.Errorf("Cache not cleaned up : %v", testCache.ugs)
- }
+ assert.Equal(t, 1, testCache.getUGsize(), "Cache is not empty: %v",
testCache.getUGmap())
+}
+
+func TestIntervalCacheCleanUp(t *testing.T) {
+ testCache := GetUserGroupCache("test")
+ testCache.resetCache()
+ // test cache should be empty now
+ assert.Equal(t, 0, testCache.getUGsize(), "Cache is not empty: %v",
testCache.getUGmap())
+
+ // resolve an existing user
+ user1ug, err := testCache.GetUserGroup("testuser1")
+ assert.NilError(t, err, "Lookup should not have failed: testuser1 user")
+ assert.Assert(t, !user1ug.failed, "User 'testuser1' not resolved as a
success")
+
+ _, err = testCache.GetUserGroup("testuser2")
+ assert.NilError(t, err, "Lookup should not have failed: testuser1 user")
+
+ // expire the successful lookup
+ testCache.lock.Lock()
+ ug := testCache.ugs["testuser1"]
+ ug.resolved -= 2 * poscache
+
+ testCache.lock.Unlock()
+ // resolve a non existing user
+ _, err = testCache.GetUserGroup("unknown")
+ assert.Assert(t, err != nil, "Lookup should have failed: unknown user")
+ testCache.lock.Lock()
+ ug = testCache.ugs["unknown"]
+ assert.Assert(t, ug.failed, "User 'unknown' not resolved as a failure")
+
+ // expire the failed lookup
+ ug.resolved -= 2 * negcache
+ testCache.lock.Unlock()
+
+ // sleep to wait for interval, it will trigger cleanUpCache
+ time.Sleep(testCache.interval + time.Second)
+ assert.Equal(t, 1, testCache.getUGsize(), "Cache not cleaned up : %v",
testCache.getUGmap())
}
func TestConvertUGI(t *testing.T) {
testCache := GetUserGroupCache("test")
testCache.resetCache()
// test cache should be empty now
- if len(testCache.ugs) != 0 {
- t.Fatalf("Cache not empty: %v", testCache.ugs)
- }
+ assert.Equal(t, 0, testCache.getUGsize(), "Cache is not empty: %v",
testCache.getUGmap())
+
ugi := &si.UserGroupInformation{
User: "",
Groups: nil,
@@ -251,4 +339,12 @@ func TestConvertUGI(t *testing.T) {
if err == nil {
t.Errorf("invalid username, convert should have failed: %v",
err)
}
+
+ // try unknown user with empty group when forced
+ ugi.User = "unknown"
+ ugi.Groups = []string{}
+ ug, err = testCache.ConvertUGI(ugi, true)
+ exceptedGroup := []string{common.AnonymousGroup}
+ assert.Assert(t, reflect.DeepEqual(ug.Groups, exceptedGroup), "group
should be: %v, but got: %v", exceptedGroup, ug.Groups)
+ assert.NilError(t, err, "unknown user, no groups, convert should not
have failed")
}
diff --git a/pkg/common/security/usergroup_test_resolver.go
b/pkg/common/security/usergroup_test_resolver.go
index c22ce570..b22b60b2 100644
--- a/pkg/common/security/usergroup_test_resolver.go
+++ b/pkg/common/security/usergroup_test_resolver.go
@@ -63,6 +63,27 @@ func lookup(userName string) (*user.User, error) {
Username: "testuser3",
}, nil
}
+ if userName == "testuser4" {
+ return &user.User{
+ Uid: "901",
+ Gid: "901",
+ Username: "testuser4",
+ }, nil
+ }
+ if userName == "testuser5" {
+ return &user.User{
+ Uid: "1001",
+ Gid: "1001",
+ Username: "testuser5",
+ }, nil
+ }
+ if userName == "invalid-gid-user" {
+ return &user.User{
+ Uid: "1001",
+ Gid: "1_001",
+ Username: "invalid-gid-user",
+ }, nil
+ }
// all other users fail
return nil, fmt.Errorf("lookup failed for user: %s", userName)
}
@@ -95,5 +116,9 @@ func groupIds(osUser *user.User) ([]string, error) {
if osUser.Username == "testuser3" {
return []string{"1002", "1001", "1003", "1004"}, nil
}
+
+ if osUser.Username == "testuser4" {
+ return []string{"901", "902"}, nil
+ }
return nil, fmt.Errorf("lookup failed for user: %s", osUser.Username)
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
