IAM: init IAM with Init() rather than InitStore() in tests (#13643)

- rename InitStore() to initStore() and fix tests

- Use IAMSys.Lock() only when IAMSys struct is being mutated

cmd/admin-handlers_test.go

@@ -28,6 +28,7 @@ import (
 	"net/url"
 	"sync"
 	"testing"
+	"time"
 
 	"github.com/gorilla/mux"
 	"github.com/minio/madmin-go"
@@ -73,7 +74,7 @@ func prepareAdminErasureTestBed(ctx context.Context) (*adminErasureTestBed, erro
 
 	initAllSubsystems(ctx, objLayer)
 
-	globalIAMSys.InitStore(objLayer, globalEtcdClient)
+	globalIAMSys.Init(ctx, objLayer, globalEtcdClient, 2*time.Second)
 
 	// Setup admin mgmt REST API handlers.
 	adminRouter := mux.NewRouter()

cmd/auth-handler_test.go

@@ -364,9 +364,12 @@ func TestIsReqAuthenticated(t *testing.T) {
 
 	newAllSubsystems()
 
-	initAllSubsystems(context.Background(), objLayer)
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
 
-	globalIAMSys.InitStore(objLayer, globalEtcdClient)
+	initAllSubsystems(ctx, objLayer)
+
+	globalIAMSys.Init(ctx, objLayer, globalEtcdClient, 2*time.Second)
 
 	creds, err := auth.CreateCredentials("myuser", "mypassword")
 	if err != nil {
@@ -392,7 +395,6 @@ func TestIsReqAuthenticated(t *testing.T) {
 		{mustNewSignedRequest(http.MethodGet, "http://127.0.0.1:9000", 0, nil, t), ErrNone},
 	}
 
-	ctx := context.Background()
 	// Validates all testcases.
 	for i, testCase := range testCases {
 		s3Error := isReqAuthenticated(ctx, testCase.req, globalServerRegion, serviceS3)
@@ -440,8 +442,8 @@ func TestCheckAdminRequestAuthType(t *testing.T) {
 }
 
 func TestValidateAdminSignature(t *testing.T) {
-
+	ctx, cancel := context.WithCancel(context.Background())
-	ctx := context.Background()
+	defer cancel()
 
 	objLayer, fsDir, err := prepareFS()
 	if err != nil {
@@ -455,9 +457,9 @@ func TestValidateAdminSignature(t *testing.T) {
 
 	newAllSubsystems()
 
-	initAllSubsystems(context.Background(), objLayer)
+	initAllSubsystems(ctx, objLayer)
 
-	globalIAMSys.InitStore(objLayer, globalEtcdClient)
+	globalIAMSys.Init(ctx, objLayer, globalEtcdClient, 2*time.Second)
 
 	creds, err := auth.CreateCredentials("admin", "mypassword")
 	if err != nil {

cmd/iam.go

@@ -143,11 +143,8 @@ func (sys *IAMSys) doIAMConfigMigration(ctx context.Context) error {
 	return sys.store.migrateBackendFormat(ctx)
 }
 
-// InitStore initializes IAM stores
+// initStore initializes IAM stores
-func (sys *IAMSys) InitStore(objAPI ObjectLayer, etcdClient *etcd.Client) {
+func (sys *IAMSys) initStore(objAPI ObjectLayer, etcdClient *etcd.Client) {
-	sys.Lock()
-	defer sys.Unlock()
-
 	if globalLDAPConfig.Enabled {
 		sys.EnableLDAPSys()
 	}
@@ -175,7 +172,7 @@ func (sys *IAMSys) Initialized() bool {
 }
 
 // Load - loads all credentials, policies and policy mappings.
-func (sys *IAMSys) Load(ctx context.Context, store IAMStorageAPI) error {
+func (sys *IAMSys) Load(ctx context.Context) error {
 	err := sys.store.LoadIAMCache(ctx)
 	if err != nil {
 		return err
@@ -191,10 +188,13 @@ func (sys *IAMSys) Load(ctx context.Context, store IAMStorageAPI) error {
 
 // Init - initializes config system by reading entries from config/iam
 func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer, etcdClient *etcd.Client, iamRefreshInterval time.Duration) {
+	sys.Lock()
+	defer sys.Unlock()
+
 	sys.iamRefreshInterval = iamRefreshInterval
 
 	// Initialize IAM store
-	sys.InitStore(objAPI, etcdClient)
+	sys.initStore(objAPI, etcdClient)
 
 	retryCtx, cancel := context.WithCancel(ctx)
 
@@ -258,7 +258,7 @@ func (sys *IAMSys) Init(ctx context.Context, objAPI ObjectLayer, etcdClient *etc
 	}
 
 	for {
-		if err := sys.Load(retryCtx, sys.store); err != nil {
+		if err := sys.Load(retryCtx); err != nil {
 			if configRetriableErrors(err) {
 				logger.Info("Waiting for all MinIO IAM sub-system to be initialized.. possible cause (%v)", err)
 				time.Sleep(time.Duration(r.Float64() * float64(5*time.Second)))
@@ -329,7 +329,7 @@ func (sys *IAMSys) watch(ctx context.Context) {
 	for {
 		select {
 		case <-ticker.C:
-			if err := sys.Load(ctx, sys.store); err != nil {
+			if err := sys.Load(ctx); err != nil {
 				logger.LogIf(ctx, err)
 			}
 		case <-ctx.Done():

cmd/signature-v4-utils_test.go

@@ -22,6 +22,7 @@ import (
 	"net/http"
 	"os"
 	"testing"
+	"time"
 
 	"github.com/minio/madmin-go"
 	"github.com/minio/minio/internal/auth"
@@ -29,6 +30,9 @@ import (
 )
 
 func TestCheckValid(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
 	objLayer, fsDir, err := prepareFS()
 	if err != nil {
 		t.Fatal(err)
@@ -40,9 +44,9 @@ func TestCheckValid(t *testing.T) {
 
 	newAllSubsystems()
 
-	initAllSubsystems(context.Background(), objLayer)
+	initAllSubsystems(ctx, objLayer)
 
-	globalIAMSys.InitStore(objLayer, globalEtcdClient)
+	globalIAMSys.Init(ctx, objLayer, globalEtcdClient, 2*time.Second)
 
 	req, err := newTestRequest(http.MethodGet, "http://example.com:9000/bucket/object", 0, nil)
 	if err != nil {

cmd/test-utils_test.go

@@ -1477,10 +1477,6 @@ func newTestObjectLayer(ctx context.Context, endpointServerPools EndpointServerP
 
 	newAllSubsystems()
 
-	initAllSubsystems(ctx, z)
-
-	globalIAMSys.InitStore(z, globalEtcdClient)
-
 	return z, nil
 }
 
@@ -1522,12 +1518,12 @@ func removeDiskN(disks []string, n int) {
 // initializes the specified API endpoints for the tests.
 // initialies the root and returns its path.
 // return credentials.
-func initAPIHandlerTest(obj ObjectLayer, endpoints []string) (string, http.Handler, error) {
+func initAPIHandlerTest(ctx context.Context, obj ObjectLayer, endpoints []string) (string, http.Handler, error) {
 	newAllSubsystems()
 
-	initAllSubsystems(context.Background(), obj)
+	initAllSubsystems(ctx, obj)
 
-	globalIAMSys.InitStore(obj, globalEtcdClient)
+	globalIAMSys.Init(ctx, obj, globalEtcdClient, 2*time.Second)
 
 	// get random bucket name.
 	bucketName := getRandomBucketName()
@@ -1736,7 +1732,7 @@ func ExecObjectLayerAPITest(t *testing.T, objAPITest objAPITestType, endpoints [
 		t.Fatalf("Initialization of object layer failed for single node setup: %s", err)
 	}
 
-	bucketFS, fsAPIRouter, err := initAPIHandlerTest(objLayer, endpoints)
+	bucketFS, fsAPIRouter, err := initAPIHandlerTest(ctx, objLayer, endpoints)
 	if err != nil {
 		t.Fatalf("Initialization of API handler tests failed: <ERROR> %s", err)
 	}
@@ -1758,7 +1754,7 @@ func ExecObjectLayerAPITest(t *testing.T, objAPITest objAPITestType, endpoints [
 	}
 	defer objLayer.Shutdown(ctx)
 
-	bucketErasure, erAPIRouter, err := initAPIHandlerTest(objLayer, endpoints)
+	bucketErasure, erAPIRouter, err := initAPIHandlerTest(ctx, objLayer, endpoints)
 	if err != nil {
 		t.Fatalf("Initialzation of API handler tests failed: <ERROR> %s", err)
 	}
@@ -1793,13 +1789,11 @@ type objTestDiskNotFoundType func(obj ObjectLayer, instanceType string, dirs []s
 // ExecObjectLayerTest - executes object layer tests.
 // Creates single node and Erasure ObjectLayer instance and runs test for both the layers.
 func ExecObjectLayerTest(t TestErrHandler, objTest objTestType) {
+	{
 		ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-
 		if localMetacacheMgr != nil {
 			localMetacacheMgr.deleteAll()
 		}
-	defer setObjectLayer(newObjectLayerFn())
 
 		objLayer, fsDir, err := prepareFS()
 		if err != nil {
@@ -1814,18 +1808,24 @@ func ExecObjectLayerTest(t TestErrHandler, objTest objTestType) {
 		if err = newTestConfig(globalMinioDefaultRegion, objLayer); err != nil {
 			t.Fatal("Unexpected error", err)
 		}
-
 		initAllSubsystems(ctx, objLayer)
-
+		globalIAMSys.Init(ctx, objLayer, globalEtcdClient, 2*time.Second)
-	globalIAMSys.InitStore(objLayer, globalEtcdClient)
 
 		// Executing the object layer tests for single node setup.
 		objTest(objLayer, FSTestStr, t)
 
+		// Call clean up functions
+		cancel()
+		setObjectLayer(newObjectLayerFn())
+		removeRoots([]string{fsDir})
+	}
+
+	{
+		ctx, cancel := context.WithCancel(context.Background())
+
 		if localMetacacheMgr != nil {
 			localMetacacheMgr.deleteAll()
 		}
-	defer setObjectLayer(newObjectLayerFn())
 
 		newAllSubsystems()
 		objLayer, fsDirs, err := prepareErasureSets32(ctx)
@@ -1833,20 +1833,20 @@ func ExecObjectLayerTest(t TestErrHandler, objTest objTestType) {
 			t.Fatalf("Initialization of object layer failed for Erasure setup: %s", err)
 		}
 		setObjectLayer(objLayer)
-
-	defer objLayer.Shutdown(context.Background())
-
 		initAllSubsystems(ctx, objLayer)
+		globalIAMSys.Init(ctx, objLayer, globalEtcdClient, 2*time.Second)
 
-	globalIAMSys.InitStore(objLayer, globalEtcdClient)
-
-	defer removeRoots(append(fsDirs, fsDir))
 		// Executing the object layer tests for Erasure.
 		objTest(objLayer, ErasureTestStr, t)
 
+		objLayer.Shutdown(context.Background())
 		if localMetacacheMgr != nil {
 			localMetacacheMgr.deleteAll()
 		}
+		setObjectLayer(newObjectLayerFn())
+		cancel()
+		removeRoots(fsDirs)
+	}
 }
 
 // ExecObjectLayerTestWithDirs - executes object layer tests.

go.mod

@@ -85,7 +85,7 @@ require (
 	go.uber.org/atomic v1.9.0
 	go.uber.org/zap v1.19.1
 	golang.org/x/crypto v0.0.0-20210921155107-089bfa567519
-	golang.org/x/net v0.0.0-20211020060615-d418f374d309
+	golang.org/x/net v0.0.0-20211020060615-d418f374d309 // indirect
 	golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f
 	golang.org/x/sys v0.0.0-20211020174200-9d6173849985
 	golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac