From e8c63147706a9063223ec99ab235f19a3e39c420 Mon Sep 17 00:00:00 2001 From: Aditya Manthramurthy Date: Thu, 11 Nov 2021 21:03:02 -0800 Subject: [PATCH] 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 | 3 +- cmd/auth-handler_test.go | 16 ++--- cmd/iam.go | 18 +++--- cmd/signature-v4-utils_test.go | 8 ++- cmd/test-utils_test.go | 104 ++++++++++++++++----------------- go.mod | 2 +- 6 files changed, 79 insertions(+), 72 deletions(-) diff --git a/cmd/admin-handlers_test.go b/cmd/admin-handlers_test.go index 1410fb3d2..1951d2225 100644 --- a/cmd/admin-handlers_test.go +++ b/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() diff --git a/cmd/auth-handler_test.go b/cmd/auth-handler_test.go index e1cb9227a..62ae41576 100644 --- a/cmd/auth-handler_test.go +++ b/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 := context.Background() + ctx, cancel := context.WithCancel(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 { diff --git a/cmd/iam.go b/cmd/iam.go index 1c176738c..4d6fa54dd 100644 --- a/cmd/iam.go +++ b/cmd/iam.go @@ -143,11 +143,8 @@ func (sys *IAMSys) doIAMConfigMigration(ctx context.Context) error { return sys.store.migrateBackendFormat(ctx) } -// InitStore initializes IAM stores -func (sys *IAMSys) InitStore(objAPI ObjectLayer, etcdClient *etcd.Client) { - sys.Lock() - defer sys.Unlock() - +// initStore initializes IAM stores +func (sys *IAMSys) initStore(objAPI ObjectLayer, etcdClient *etcd.Client) { 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(): diff --git a/cmd/signature-v4-utils_test.go b/cmd/signature-v4-utils_test.go index 998505a43..0d770a4cf 100644 --- a/cmd/signature-v4-utils_test.go +++ b/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 { diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index 70a01c4b8..2d5ef6985 100644 --- a/cmd/test-utils_test.go +++ b/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: %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: %s", err) } @@ -1793,59 +1789,63 @@ 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() + { + ctx, cancel := context.WithCancel(context.Background()) + if localMetacacheMgr != nil { + localMetacacheMgr.deleteAll() + } - if localMetacacheMgr != nil { - localMetacacheMgr.deleteAll() - } - defer setObjectLayer(newObjectLayerFn()) + objLayer, fsDir, err := prepareFS() + if err != nil { + t.Fatalf("Initialization of object layer failed for single node setup: %s", err) + } + setObjectLayer(objLayer) - objLayer, fsDir, err := prepareFS() - if err != nil { - t.Fatalf("Initialization of object layer failed for single node setup: %s", err) - } - setObjectLayer(objLayer) + newAllSubsystems() - newAllSubsystems() + // initialize the server and obtain the credentials and root. + // credentials are necessary to sign the HTTP request. + if err = newTestConfig(globalMinioDefaultRegion, objLayer); err != nil { + t.Fatal("Unexpected error", err) + } + initAllSubsystems(ctx, objLayer) + globalIAMSys.Init(ctx, objLayer, globalEtcdClient, 2*time.Second) - // initialize the server and obtain the credentials and root. - // credentials are necessary to sign the HTTP request. - if err = newTestConfig(globalMinioDefaultRegion, objLayer); err != nil { - t.Fatal("Unexpected error", err) + // Executing the object layer tests for single node setup. + objTest(objLayer, FSTestStr, t) + + // Call clean up functions + cancel() + setObjectLayer(newObjectLayerFn()) + removeRoots([]string{fsDir}) } - initAllSubsystems(ctx, objLayer) + { + ctx, cancel := context.WithCancel(context.Background()) - globalIAMSys.InitStore(objLayer, globalEtcdClient) + if localMetacacheMgr != nil { + localMetacacheMgr.deleteAll() + } - // Executing the object layer tests for single node setup. - objTest(objLayer, FSTestStr, t) + newAllSubsystems() + objLayer, fsDirs, err := prepareErasureSets32(ctx) + if err != nil { + t.Fatalf("Initialization of object layer failed for Erasure setup: %s", err) + } + setObjectLayer(objLayer) + initAllSubsystems(ctx, objLayer) + globalIAMSys.Init(ctx, objLayer, globalEtcdClient, 2*time.Second) - if localMetacacheMgr != nil { - localMetacacheMgr.deleteAll() - } - defer setObjectLayer(newObjectLayerFn()) + // Executing the object layer tests for Erasure. + objTest(objLayer, ErasureTestStr, t) - newAllSubsystems() - objLayer, fsDirs, err := prepareErasureSets32(ctx) - if err != nil { - t.Fatalf("Initialization of object layer failed for Erasure setup: %s", err) - } - setObjectLayer(objLayer) - - defer objLayer.Shutdown(context.Background()) - - initAllSubsystems(ctx, objLayer) - - globalIAMSys.InitStore(objLayer, globalEtcdClient) - - defer removeRoots(append(fsDirs, fsDir)) - // Executing the object layer tests for Erasure. - objTest(objLayer, ErasureTestStr, t) - - if localMetacacheMgr != nil { - localMetacacheMgr.deleteAll() + objLayer.Shutdown(context.Background()) + if localMetacacheMgr != nil { + localMetacacheMgr.deleteAll() + } + setObjectLayer(newObjectLayerFn()) + cancel() + removeRoots(fsDirs) } } diff --git a/go.mod b/go.mod index 624af4e5f..e850ec782 100644 --- a/go.mod +++ b/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