Adopt dsync interface changes and major cleanup on RPC server/client.

* Rename GenericArgs to AuthRPCArgs
* Rename GenericReply to AuthRPCReply
* Remove authConfig.loginMethod and add authConfig.ServiceName
* Rename loginServer to AuthRPCServer
* Rename RPCLoginArgs to LoginRPCArgs
* Rename RPCLoginReply to LoginRPCReply
* Version and RequestTime are added to LoginRPCArgs and verified by
  server side, not client side.
* Fix data race in lockMaintainence loop.

cmd/admin-rpc-client.go

@@ -55,15 +55,15 @@ func (lc localAdminClient) Restart() error {
 
 // Stop - Sends stop command to remote server via RPC.
 func (rc remoteAdminClient) Stop() error {
-	args := GenericArgs{}
-	reply := GenericReply{}
+	args := AuthRPCArgs{}
+	reply := AuthRPCReply{}
 	return rc.Call("Service.Shutdown", &args, &reply)
 }
 
 // Restart - Sends restart command to remote server via RPC.
 func (rc remoteAdminClient) Restart() error {
-	args := GenericArgs{}
-	reply := GenericReply{}
+	args := AuthRPCArgs{}
+	reply := AuthRPCReply{}
 	return rc.Call("Service.Restart", &args, &reply)
 }
 
@@ -90,6 +90,7 @@ func makeAdminPeers(eps []*url.URL) adminPeers {
 	})
 	seenAddr[globalMinioAddr] = true
 
+	serverCred := serverConfig.GetCredential()
 	// iterate over endpoints to find new remote peers and add
 	// them to ret.
 	for _, ep := range eps {
@@ -100,17 +101,17 @@ func makeAdminPeers(eps []*url.URL) adminPeers {
 		// Check if the remote host has been added already
 		if !seenAddr[ep.Host] {
 			cfg := authConfig{
-				accessKey:   serverConfig.GetCredential().AccessKey,
-				secretKey:   serverConfig.GetCredential().SecretKey,
-				address:     ep.Host,
-				secureConn:  isSSL(),
-				path:        path.Join(reservedBucket, servicePath),
-				loginMethod: "Service.LoginHandler",
+				accessKey:       serverCred.AccessKey,
+				secretKey:       serverCred.SecretKey,
+				serverAddr:      ep.Host,
+				secureConn:      isSSL(),
+				serviceEndpoint: path.Join(reservedBucket, servicePath),
+				serviceName:     "Service",
 			}
 
 			servicePeers = append(servicePeers, adminPeer{
 				addr:    ep.Host,
-				svcClnt: &remoteAdminClient{newAuthClient(&cfg)},
+				svcClnt: &remoteAdminClient{newAuthRPCClient(cfg)},
 			})
 			seenAddr[ep.Host] = true
 		}

cmd/admin-rpc-server.go

@@ -27,23 +27,25 @@ const servicePath = "/admin/service"
 // serviceCmd - exports RPC methods for service status, stop and
 // restart commands.
 type serviceCmd struct {
-	loginServer
+	AuthRPCServer
 }
 
 // Shutdown - Shutdown this instance of minio server.
-func (s *serviceCmd) Shutdown(args *GenericArgs, reply *GenericReply) error {
-	if !isAuthTokenValid(args.Token) {
-		return errInvalidToken
+func (s *serviceCmd) Shutdown(args *AuthRPCArgs, reply *AuthRPCReply) error {
+	if err := args.IsAuthenticated(); err != nil {
+		return err
 	}
+
 	globalServiceSignalCh <- serviceStop
 	return nil
 }
 
 // Restart - Restart this instance of minio server.
-func (s *serviceCmd) Restart(args *GenericArgs, reply *GenericReply) error {
-	if !isAuthTokenValid(args.Token) {
-		return errInvalidToken
+func (s *serviceCmd) Restart(args *AuthRPCArgs, reply *AuthRPCReply) error {
+	if err := args.IsAuthenticated(); err != nil {
+		return err
 	}
+
 	globalServiceSignalCh <- serviceRestart
 	return nil
 }

cmd/admin-rpc-server_test.go

@@ -30,9 +30,14 @@ func testAdminCmd(cmd cmdType, t *testing.T) {
 
 	adminServer := serviceCmd{}
 	creds := serverConfig.GetCredential()
-	reply := RPCLoginReply{}
-	args := RPCLoginArgs{Username: creds.AccessKey, Password: creds.SecretKey}
-	err = adminServer.LoginHandler(&args, &reply)
+	args := LoginRPCArgs{
+		Username:    creds.AccessKey,
+		Password:    creds.SecretKey,
+		Version:     Version,
+		RequestTime: time.Now().UTC(),
+	}
+	reply := LoginRPCReply{}
+	err = adminServer.Login(&args, &reply)
 	if err != nil {
 		t.Fatalf("Failed to login to admin server - %v", err)
 	}
@@ -42,37 +47,16 @@ func testAdminCmd(cmd cmdType, t *testing.T) {
 		<-globalServiceSignalCh
 	}()
 
-	validToken := reply.Token
-	timeNow := time.Now().UTC()
-	testCases := []struct {
-		ga          GenericArgs
-		expectedErr error
-	}{
-		// Valid case
-		{
-			ga:          GenericArgs{Token: validToken, Timestamp: timeNow},
-			expectedErr: nil,
-		},
-		// Invalid token
-		{
-			ga:          GenericArgs{Token: "invalidToken", Timestamp: timeNow},
-			expectedErr: errInvalidToken,
-		},
-	}
-
-	genReply := GenericReply{}
-	for i, test := range testCases {
-		switch cmd {
-		case stopCmd:
-			err = adminServer.Shutdown(&test.ga, &genReply)
-			if err != test.expectedErr {
-				t.Errorf("Test %d: Expected error %v but received %v", i+1, test.expectedErr, err)
-			}
-		case restartCmd:
-			err = adminServer.Restart(&test.ga, &genReply)
-			if err != test.expectedErr {
-				t.Errorf("Test %d: Expected error %v but received %v", i+1, test.expectedErr, err)
-			}
+	ga := AuthRPCArgs{AuthToken: reply.AuthToken, RequestTime: time.Now().UTC()}
+	genReply := AuthRPCReply{}
+	switch cmd {
+	case stopCmd:
+		if err = adminServer.Shutdown(&ga, &genReply); err != nil {
+			t.Errorf("stopCmd: Expected: <nil>, got: %v", err)
+		}
+	case restartCmd:
+		if err = adminServer.Restart(&ga, &genReply); err != nil {
+			t.Errorf("restartCmd: Expected: <nil>, got: %v", err)
 		}
 	}
 }

cmd/auth-rpc-client.go

@@ -26,170 +26,120 @@ import (
 // giving up on the remote RPC entirely.
 const globalAuthRPCRetryThreshold = 1
 
-// GenericReply represents any generic RPC reply.
-type GenericReply struct{}
-
-// GenericArgs represents any generic RPC arguments.
-type GenericArgs struct {
-	Token string // Used to authenticate every RPC call.
-	// Used to verify if the RPC call was issued between
-	// the same Login() and disconnect event pair.
-	Timestamp time.Time
-
-	// Indicates if args should be sent to remote peers as well.
-	Remote bool
-}
-
-// SetToken - sets the token to the supplied value.
-func (ga *GenericArgs) SetToken(token string) {
-	ga.Token = token
-}
-
-// SetTimestamp - sets the timestamp to the supplied value.
-func (ga *GenericArgs) SetTimestamp(tstamp time.Time) {
-	ga.Timestamp = tstamp
-}
-
-// RPCLoginArgs - login username and password for RPC.
-type RPCLoginArgs struct {
-	Username string
-	Password string
-}
-
-// RPCLoginReply - login reply provides generated token to be used
-// with subsequent requests.
-type RPCLoginReply struct {
-	Token         string
-	Timestamp     time.Time
-	ServerVersion string
-}
-
-// Auth config represents authentication credentials and Login method name to be used
-// for fetching JWT tokens from the RPC server.
+// authConfig requires to make new AuthRPCClient.
 type authConfig struct {
-	accessKey   string // Username for the server.
-	secretKey   string // Password for the server.
-	secureConn  bool   // Ask for a secured connection
-	address     string // Network address path of RPC server.
-	path        string // Network path for HTTP dial.
-	loginMethod string // RPC service name for authenticating using JWT
+	accessKey        string // Access key (like username) for authentication.
+	secretKey        string // Secret key (like Password) for authentication.
+	serverAddr       string // RPC server address.
+	serviceEndpoint  string // Endpoint on the server to make any RPC call.
+	secureConn       bool   // Make TLS connection to RPC server or not.
+	serviceName      string // Service name of auth server.
+	disableReconnect bool   // Disable reconnect on failure or not.
 }
 
-// AuthRPCClient is a wrapper type for RPCClient which provides JWT based authentication across reconnects.
+// AuthRPCClient is a authenticated RPC client which does authentication before doing Call().
 type AuthRPCClient struct {
-	mu            sync.Mutex
-	config        *authConfig
-	rpc           *RPCClient // reconnect'able rpc client built on top of net/rpc Client
-	serverToken   string     // Disk rpc JWT based token.
-	serverVersion string     // Server version exchanged by the RPC.
+	sync.Mutex            // Mutex to lock this object.
+	rpcClient  *RPCClient // Reconnectable RPC client to make any RPC call.
+	config     authConfig // Authentication configuration information.
+	authToken  string     // Authentication token.
 }
 
-// newAuthClient - returns a jwt based authenticated (go) rpc client, which does automatic reconnect.
-func newAuthClient(cfg *authConfig) *AuthRPCClient {
+// newAuthRPCClient - returns a JWT based authenticated (go) rpc client, which does automatic reconnect.
+func newAuthRPCClient(config authConfig) *AuthRPCClient {
 	return &AuthRPCClient{
-		// Save the config.
-		config: cfg,
-		// Initialize a new reconnectable rpc client.
-		rpc: newRPCClient(cfg.address, cfg.path, cfg.secureConn),
+		rpcClient: newRPCClient(config.serverAddr, config.serviceEndpoint, config.secureConn),
+		config:    config,
 	}
 }
 
-// Close - closes underlying rpc connection.
-func (authClient *AuthRPCClient) Close() error {
-	authClient.mu.Lock()
-	// reset token on closing a connection
-	authClient.serverToken = ""
-	authClient.mu.Unlock()
-	return authClient.rpc.Close()
-}
-
 // Login - a jwt based authentication is performed with rpc server.
 func (authClient *AuthRPCClient) Login() (err error) {
-	authClient.mu.Lock()
-	// As soon as the function returns unlock,
-	defer authClient.mu.Unlock()
+	authClient.Lock()
+	defer authClient.Unlock()
 
 	// Return if already logged in.
-	if authClient.serverToken != "" {
+	if authClient.authToken != "" {
 		return nil
 	}
 
-	reply := RPCLoginReply{}
-	if err = authClient.rpc.Call(authClient.config.loginMethod, RPCLoginArgs{
-		Username: authClient.config.accessKey,
-		Password: authClient.config.secretKey,
-	}, &reply); err != nil {
-		return err
+	// Call login.
+	args := LoginRPCArgs{
+		Username:    authClient.config.accessKey,
+		Password:    authClient.config.secretKey,
+		Version:     Version,
+		RequestTime: time.Now().UTC(),
 	}
 
-	// Validate if version do indeed match.
-	if reply.ServerVersion != Version {
-		return errServerVersionMismatch
+	reply := LoginRPCReply{}
+	serviceMethod := authClient.config.serviceName + loginMethodName
+	if err = authClient.rpcClient.Call(serviceMethod, &args, &reply); err != nil {
+		return err
 	}
 
-	// Validate if server timestamp is skewed.
-	curTime := time.Now().UTC()
-	if curTime.Sub(reply.Timestamp) > globalMaxSkewTime {
-		return errServerTimeMismatch
-	}
+	// Logged in successfully.
+	authClient.authToken = reply.AuthToken
 
-	// Set token, time stamp as received from a successful login call.
-	authClient.serverToken = reply.Token
-	authClient.serverVersion = reply.ServerVersion
 	return nil
 }
 
-// Call - If rpc connection isn't established yet since previous disconnect,
-// connection is established, a jwt authenticated login is performed and then
-// the call is performed.
-func (authClient *AuthRPCClient) Call(serviceMethod string, args interface {
-	SetToken(token string)
-	SetTimestamp(tstamp time.Time)
+// call makes a RPC call after logs into the server.
+func (authClient *AuthRPCClient) call(serviceMethod string, args interface {
+	SetAuthToken(authToken string)
+	SetRequestTime(requestTime time.Time)
 }, reply interface{}) (err error) {
-	loginAndCallFn := func() error {
-		// On successful login, proceed to attempt the requested service method.
-		if err = authClient.Login(); err == nil {
-			// Set token and timestamp before the rpc call.
-			args.SetToken(authClient.serverToken)
-			args.SetTimestamp(time.Now().UTC())
-
-			// Finally make the network call using net/rpc client.
-			err = authClient.rpc.Call(serviceMethod, args, reply)
-		}
-		return err
+	// On successful login, execute RPC call.
+	if err = authClient.Login(); err == nil {
+		// Set token and timestamp before the rpc call.
+		args.SetAuthToken(authClient.authToken)
+		args.SetRequestTime(time.Now().UTC())
+
+		// Do RPC call.
+		err = authClient.rpcClient.Call(serviceMethod, args, reply)
 	}
+	return err
+}
+
+// Call executes RPC call till success or globalAuthRPCRetryThreshold on ErrShutdown.
+func (authClient *AuthRPCClient) Call(serviceMethod string, args interface {
+	SetAuthToken(authToken string)
+	SetRequestTime(requestTime time.Time)
+}, reply interface{}) (err error) {
 	doneCh := make(chan struct{})
 	defer close(doneCh)
-	for i := range newRetryTimer(time.Second, time.Second*30, MaxJitter, doneCh) {
-		// Invalidate token, and mark it for re-login and
-		// reconnect upon rpc shutdown.
-		if err = loginAndCallFn(); err == rpc.ErrShutdown {
-			// Close the underlying connection, and proceed to reconnect
-			// if we haven't reached the retry threshold.
+	for i := range newRetryTimer(time.Second, 30*time.Second, MaxJitter, doneCh) {
+		if err = authClient.call(serviceMethod, args, reply); err == rpc.ErrShutdown {
+			// As connection at server side is closed, close the rpc client.
 			authClient.Close()
 
-			// No need to return error until the retry count threshold has reached.
-			if i < globalAuthRPCRetryThreshold {
-				continue
+			// Retry if reconnect is not disabled.
+			if !authClient.config.disableReconnect {
+				// Retry until threshold reaches.
+				if i < globalAuthRPCRetryThreshold {
+					continue
+				}
 			}
 		}
 		break
 	}
 	return err
 }
 
-// Node returns the node (network address) of the connection
-func (authClient *AuthRPCClient) Node() (node string) {
-	if authClient.rpc != nil {
-		node = authClient.rpc.node
-	}
-	return node
+// Close closes underlying RPC Client.
+func (authClient *AuthRPCClient) Close() error {
+	authClient.Lock()
+	defer authClient.Unlock()
+
+	authClient.authToken = ""
+	return authClient.rpcClient.Close()
 }
 
-// RPCPath returns the RPC path of the connection
-func (authClient *AuthRPCClient) RPCPath() (rpcPath string) {
-	if authClient.rpc != nil {
-		rpcPath = authClient.rpc.rpcPath
-	}
-	return rpcPath
+// ServerAddr returns the serverAddr (network address) of the connection.
+func (authClient *AuthRPCClient) ServerAddr() string {
+	return authClient.config.serverAddr
+}
+
+// ServiceEndpoint returns the RPC service endpoint of the connection.
+func (authClient *AuthRPCClient) ServiceEndpoint() string {
+	return authClient.config.serviceEndpoint
 }