Enhance fatal errors printing of common issues seen by users (minio#5878

)

cmd/certs.go

@@ -22,7 +22,6 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"encoding/pem"
-	"fmt"
 	"io/ioutil"
 	"os"
 )
@@ -44,19 +43,19 @@ func parsePublicCertFile(certFile string) (x509Certs []*x509.Certificate, err er
 	for len(current) > 0 {
 		var pemBlock *pem.Block
 		if pemBlock, current = pem.Decode(current); pemBlock == nil {
-			return nil, fmt.Errorf("Could not read PEM block from file %s", certFile)
+			return nil, uiErrSSLUnexpectedData(nil).Msg("Could not read PEM block from file %s", certFile)
 		}
 
 		var x509Cert *x509.Certificate
 		if x509Cert, err = x509.ParseCertificate(pemBlock.Bytes); err != nil {
-			return nil, err
+			return nil, uiErrSSLUnexpectedData(err)
 		}
 
 		x509Certs = append(x509Certs, x509Cert)
 	}
 
 	if len(x509Certs) == 0 {
-		return nil, fmt.Errorf("Empty public certificate file %s", certFile)
+		return nil, uiErrSSLUnexpectedData(nil).Msg("Empty public certificate file %s", certFile)
 	}
 
 	return x509Certs, nil
@@ -107,28 +106,32 @@ func getRootCAs(certsCAsDir string) (*x509.CertPool, error) {
 func loadX509KeyPair(certFile, keyFile string) (tls.Certificate, error) {
 	certPEMBlock, err := ioutil.ReadFile(certFile)
 	if err != nil {
-		return tls.Certificate{}, fmt.Errorf("TLS: failed to read cert file: %v", err)
+		return tls.Certificate{}, uiErrSSLUnexpectedError(err)
 	}
 	keyPEMBlock, err := ioutil.ReadFile(keyFile)
 	if err != nil {
-		return tls.Certificate{}, fmt.Errorf("TLS: failed to read private key: %v", err)
+		return tls.Certificate{}, uiErrSSLUnexpectedError(err)
 	}
 	key, rest := pem.Decode(keyPEMBlock)
 	if len(rest) > 0 {
-		return tls.Certificate{}, fmt.Errorf("TLS: private key contains additional data")
+		return tls.Certificate{}, uiErrSSLUnexpectedData(nil).Msg("The private key contains additional data")
 	}
 	if x509.IsEncryptedPEMBlock(key) {
 		password, ok := os.LookupEnv(TLSPrivateKeyPassword)
 		if !ok {
-			return tls.Certificate{}, fmt.Errorf("TLS: private key is encrypted but no password is present - set env var: %s", TLSPrivateKeyPassword)
+			return tls.Certificate{}, uiErrSSLNoPassword(nil)
 		}
 		decryptedKey, decErr := x509.DecryptPEMBlock(key, []byte(password))
 		if decErr != nil {
-			return tls.Certificate{}, fmt.Errorf("TLS: failed to decrypt private key: %v", decErr)
+			return tls.Certificate{}, uiErrSSLWrongPassword(decErr)
 		}
 		keyPEMBlock = pem.EncodeToMemory(&pem.Block{Type: key.Type, Bytes: decryptedKey})
 	}
-	return tls.X509KeyPair(certPEMBlock, keyPEMBlock)
+	cert, err := tls.X509KeyPair(certPEMBlock, keyPEMBlock)
+	if err != nil {
+		return tls.Certificate{}, uiErrSSLUnexpectedData(nil).Msg(err.Error())
+	}
+	return cert, nil
 }
 
 func getSSLConfig() (x509Certs []*x509.Certificate, rootCAs *x509.CertPool, tlsCert *tls.Certificate, secureConn bool, err error) {
@@ -149,7 +152,7 @@ func getSSLConfig() (x509Certs []*x509.Certificate, rootCAs *x509.CertPool, tlsC
 	if priv, ok := cert.PrivateKey.(crypto.Signer); ok {
 		if pub, ok := priv.Public().(*ecdsa.PublicKey); ok {
 			if name := pub.Params().Name; name == "P-384" || name == "P-521" { // unfortunately there is no cleaner way to check
-				return nil, nil, nil, false, fmt.Errorf("TLS: the ECDSA curve '%s' is not supported", name)
+				return nil, nil, nil, false, uiErrSSLUnexpectedData(nil).Msg("tls: the ECDSA curve '%s' is not supported", name)
 			}
 		}
 

cmd/common-main.go

@@ -45,7 +45,7 @@ func initConfig() {
 	// Config file does not exist, we create it fresh and return upon success.
 	if isFile(getConfigFile()) {
 		logger.FatalIf(migrateConfig(), "Config migration failed.")
-		logger.FatalIf(loadConfig(), "Unable to load config version: '%s'.", serverConfigVersion)
+		logger.FatalIf(loadConfig(), "Unable to load the configuration file")
 	} else {
 		logger.FatalIf(newConfig(), "Unable to initialize minio config for the first time.")
 		logger.Info("Created minio configuration file successfully at " + getConfigDir())
@@ -95,7 +95,9 @@ func handleCommonEnvVars() {
 	secretKey := os.Getenv("MINIO_SECRET_KEY")
 	if accessKey != "" && secretKey != "" {
 		cred, err := auth.CreateCredentials(accessKey, secretKey)
-		logger.FatalIf(err, "Invalid access/secret Key set in environment.")
+		if err != nil {
+			logger.Fatal(uiErrInvalidCredentials(err), "Unable to validate credentials inherited from the shell environment")
+		}
 
 		// credential Envs are set globally.
 		globalIsEnvCreds = true
@@ -105,7 +107,7 @@ func handleCommonEnvVars() {
 	if browser := os.Getenv("MINIO_BROWSER"); browser != "" {
 		browserFlag, err := ParseBrowserFlag(browser)
 		if err != nil {
-			logger.FatalIf(errors.New("invalid value"), "Unknown value ‘%s’ in MINIO_BROWSER environment variable.", browser)
+			logger.Fatal(uiErrInvalidBrowserValue(nil).Msg("Unknown value `%s`", browser), "Unable to validate MINIO_BROWSER environment variable")
 		}
 
 		// browser Envs are set globally, this does not represent
@@ -128,18 +130,26 @@ func handleCommonEnvVars() {
 
 	if drives := os.Getenv("MINIO_CACHE_DRIVES"); drives != "" {
 		driveList, err := parseCacheDrives(strings.Split(drives, cacheEnvDelimiter))
-		logger.FatalIf(err, "Invalid value set in environment variable MINIO_CACHE_DRIVES %s.", drives)
+		if err != nil {
+			logger.Fatal(err, "Unable to parse MINIO_CACHE_DRIVES value (%s)", drives)
+		}
 		globalCacheDrives = driveList
 		globalIsDiskCacheEnabled = true
 	}
+
 	if excludes := os.Getenv("MINIO_CACHE_EXCLUDE"); excludes != "" {
 		excludeList, err := parseCacheExcludes(strings.Split(excludes, cacheEnvDelimiter))
-		logger.FatalIf(err, "Invalid value set in environment variable MINIO_CACHE_EXCLUDE %s.", excludes)
+		if err != nil {
+			logger.Fatal(err, "Unable to parse MINIO_CACHE_EXCLUDE value (`%s`).", excludes)
+		}
 		globalCacheExcludes = excludeList
 	}
+
 	if expiryStr := os.Getenv("MINIO_CACHE_EXPIRY"); expiryStr != "" {
 		expiry, err := strconv.Atoi(expiryStr)
-		logger.FatalIf(err, "Invalid value set in environment variable MINIO_CACHE_EXPIRY %s.", expiryStr)
+		if err != nil {
+			logger.Fatal(uiErrInvalidCacheExpiryValue(err), "Unable to parse MINIO_CACHE_EXPIRY value (`%s`)", expiryStr)
+		}
 		globalCacheExpiry = expiry
 	}
 

cmd/config-current.go

@@ -287,7 +287,7 @@ func getValidConfig() (*serverConfig, error) {
 func loadConfig() error {
 	srvCfg, err := getValidConfig()
 	if err != nil {
-		return err
+		return uiErrInvalidConfig(nil).Msg(err.Error())
 	}
 
 	// If env is set override the credentials from config file.

cmd/disk-cache-config.go

@@ -18,7 +18,6 @@ package cmd
 
 import (
 	"encoding/json"
-	"fmt"
 	"path/filepath"
 )
 
@@ -54,7 +53,7 @@ func (cfg *CacheConfig) UnmarshalJSON(data []byte) (err error) {
 func parseCacheDrives(drives []string) ([]string, error) {
 	for _, d := range drives {
 		if !filepath.IsAbs(d) {
-			return nil, fmt.Errorf("cache dir should be absolute path: %s", d)
+			return nil, uiErrInvalidCacheDrivesValue(nil).Msg("cache dir should be absolute path: %s", d)
 		}
 	}
 	return drives, nil
@@ -64,10 +63,10 @@ func parseCacheDrives(drives []string) ([]string, error) {
 func parseCacheExcludes(excludes []string) ([]string, error) {
 	for _, e := range excludes {
 		if len(e) == 0 {
-			return nil, fmt.Errorf("cache exclude path (%s) cannot be empty", e)
+			return nil, uiErrInvalidCacheExcludesValue(nil).Msg("cache exclude path (%s) cannot be empty", e)
 		}
 		if hasPrefix(e, slashSeparator) {
-			return nil, fmt.Errorf("cache exclude pattern (%s) cannot start with / as prefix", e)
+			return nil, uiErrInvalidCacheExcludesValue(nil).Msg("cache exclude pattern (%s) cannot start with / as prefix", e)
 		}
 	}
 	return excludes, nil

cmd/endpoint-ellipses.go

@@ -70,11 +70,10 @@ func getSetIndexes(args []string, totalSizes []uint64) (setIndexes [][]uint64, e
 	}
 
 	setIndexes = make([][]uint64, len(totalSizes))
-	for i, totalSize := range totalSizes {
+	for _, totalSize := range totalSizes {
 		// Check if totalSize has minimum range upto setSize
 		if totalSize < setSizes[0] {
-			return nil, fmt.Errorf("Invalid inputs (%s). Ellipses range or number of args %d should be atleast divisible by least possible set size %d",
-				args[i], totalSize, setSizes[0])
+			return nil, uiErrInvalidNumberOfErasureEndpoints(nil)
 		}
 	}
 
@@ -103,8 +102,7 @@ func getSetIndexes(args []string, totalSizes []uint64) (setIndexes [][]uint64, e
 
 	// Check whether setSize is with the supported range.
 	if !isValidSetSize(setSize) {
-		return nil, fmt.Errorf("Invalid inputs (%s). Ellipses range or number of args %d should be atleast divisible by least possible set size %d",
-			args, setSize, setSizes[0])
+		return nil, uiErrInvalidNumberOfErasureEndpoints(nil)
 	}
 
 	for i := range totalSizes {
@@ -167,14 +165,14 @@ func parseEndpointSet(args ...string) (ep endpointSet, err error) {
 	for i, arg := range args {
 		patterns, perr := ellipses.FindEllipsesPatterns(arg)
 		if perr != nil {
-			return endpointSet{}, perr
+			return endpointSet{}, uiErrInvalidErasureEndpoints(nil).Msg(perr.Error())
 		}
 		argPatterns[i] = patterns
 	}
 
 	ep.setIndexes, err = getSetIndexes(args, getTotalSizes(argPatterns))
 	if err != nil {
-		return endpointSet{}, err
+		return endpointSet{}, uiErrInvalidErasureEndpoints(nil).Msg(err.Error())
 	}
 
 	ep.argPatterns = argPatterns
@@ -223,7 +221,7 @@ func getAllSets(args ...string) ([][]string, error) {
 	for _, sargs := range setArgs {
 		for _, arg := range sargs {
 			if uniqueArgs.Contains(arg) {
-				return nil, fmt.Errorf("Input args (%s) has duplicate ellipses", args)
+				return nil, uiErrInvalidErasureEndpoints(nil).Msg(fmt.Sprintf("Input args (%s) has duplicate ellipses", args))
 			}
 			uniqueArgs.Add(arg)
 		}

cmd/endpoint.go

@@ -252,14 +252,14 @@ func CreateEndpoints(serverAddr string, args ...[]string) (string, EndpointList,
 			return serverAddr, endpoints, setupType, err
 		}
 		if endpoint.Type() != PathEndpointType {
-			return serverAddr, endpoints, setupType, fmt.Errorf("use path style endpoint for FS setup")
+			return serverAddr, endpoints, setupType, uiErrInvalidFSEndpoint(nil).Msg("use path style endpoint for FS setup")
 		}
 		endpoints = append(endpoints, endpoint)
 		setupType = FSSetupType
 
 		// Check for cross device mounts if any.
 		if err = checkCrossDeviceMounts(endpoints); err != nil {
-			return serverAddr, endpoints, setupType, err
+			return serverAddr, endpoints, setupType, uiErrInvalidFSEndpoint(nil).Msg(err.Error())
 		}
 		return serverAddr, endpoints, setupType, nil
 	}
@@ -270,12 +270,12 @@ func CreateEndpoints(serverAddr string, args ...[]string) (string, EndpointList,
 		var eps EndpointList
 		eps, err = NewEndpointList(iargs...)
 		if err != nil {
-			return serverAddr, endpoints, setupType, err
+			return serverAddr, endpoints, setupType, uiErrInvalidErasureEndpoints(nil).Msg(err.Error())
 		}
 
 		// Check for cross device mounts if any.
 		if err = checkCrossDeviceMounts(eps); err != nil {
-			return serverAddr, endpoints, setupType, err
+			return serverAddr, endpoints, setupType, uiErrInvalidErasureEndpoints(nil).Msg(err.Error())
 		}
 
 		for _, ep := range eps {
@@ -316,7 +316,7 @@ func CreateEndpoints(serverAddr string, args ...[]string) (string, EndpointList,
 
 	// No local endpoint found.
 	if localEndpointCount == 0 {
-		return serverAddr, endpoints, setupType, fmt.Errorf("no endpoint found for this host")
+		return serverAddr, endpoints, setupType, uiErrInvalidErasureEndpoints(nil).Msg("no endpoint pointing to the local machine is found")
 	}
 
 	// Check whether same path is not used in endpoints of a host on different port.
@@ -331,8 +331,8 @@ func CreateEndpoints(serverAddr string, args ...[]string) (string, EndpointList,
 			hostIPSet, _ := getHostIP4(host)
 			if IPSet, ok := pathIPMap[endpoint.Path]; ok {
 				if !IPSet.Intersection(hostIPSet).IsEmpty() {
-					err = fmt.Errorf("path '%s' can not be served by different port on same address", endpoint.Path)
-					return serverAddr, endpoints, setupType, err
+					return serverAddr, endpoints, setupType,
+						uiErrInvalidErasureEndpoints(nil).Msg(fmt.Sprintf("path '%s' can not be served by different port on same address", endpoint.Path))
 				}
 				pathIPMap[endpoint.Path] = IPSet.Union(hostIPSet)
 			} else {
@@ -349,8 +349,8 @@ func CreateEndpoints(serverAddr string, args ...[]string) (string, EndpointList,
 				continue
 			}
 			if localPathSet.Contains(endpoint.Path) {
-				err = fmt.Errorf("path '%s' cannot be served by different address on same server", endpoint.Path)
-				return serverAddr, endpoints, setupType, err
+				return serverAddr, endpoints, setupType,
+					uiErrInvalidErasureEndpoints(nil).Msg(fmt.Sprintf("path '%s' cannot be served by different address on same server", endpoint.Path))
 			}
 			localPathSet.Add(endpoint.Path)
 		}
@@ -360,12 +360,11 @@ func CreateEndpoints(serverAddr string, args ...[]string) (string, EndpointList,
 	{
 		if !localPortSet.Contains(serverAddrPort) {
 			if len(localPortSet) > 1 {
-				err = fmt.Errorf("port number in server address must match with one of the port in local endpoints")
-			} else {
-				err = fmt.Errorf("server address and local endpoint have different ports")
+				return serverAddr, endpoints, setupType,
+					uiErrInvalidErasureEndpoints(nil).Msg("port number in server address must match with one of the port in local endpoints")
 			}
-
-			return serverAddr, endpoints, setupType, err
+			return serverAddr, endpoints, setupType,
+				uiErrInvalidErasureEndpoints(nil).Msg("server address and local endpoint have different ports")
 		}
 	}
 
@@ -374,7 +373,7 @@ func CreateEndpoints(serverAddr string, args ...[]string) (string, EndpointList,
 		// If all endpoints have same port number, then this is XL setup using URL style endpoints.
 		if len(localPortSet) == 1 {
 			if len(localServerAddrSet) > 1 {
-				// TODO: Eventhough all endpoints are local, the local host is referred by different IP/name.
+				// TODO: Even though all endpoints are local, the local host is referred by different IP/name.
 				// eg '172.0.0.1', 'localhost' and 'mylocalhostname' point to same local host.
 				//
 				// In this case, we bind to 0.0.0.0 ie to all interfaces.
@@ -388,7 +387,7 @@ func CreateEndpoints(serverAddr string, args ...[]string) (string, EndpointList,
 			return serverAddr, endpoints, setupType, nil
 		}
 
-		// Eventhough all endpoints are local, but those endpoints use different ports.
+		// Even though all endpoints are local, but those endpoints use different ports.
 		// This means it is DistXL setup.
 	} else {
 		// This is DistXL setup.

cmd/endpoint_test.go

@@ -225,8 +225,8 @@ func TestCreateEndpoints(t *testing.T) {
 		{"localhost:10000", [][]string{{"./d1"}}, "localhost:10000", EndpointList{Endpoint{URL: &url.URL{Path: "d1"}, IsLocal: true}}, FSSetupType, nil},
 		{"localhost:10000", [][]string{{`\d1`}}, "localhost:10000", EndpointList{Endpoint{URL: &url.URL{Path: `\d1`}, IsLocal: true}}, FSSetupType, nil},
 		{"localhost:10000", [][]string{{`.\d1`}}, "localhost:10000", EndpointList{Endpoint{URL: &url.URL{Path: `.\d1`}, IsLocal: true}}, FSSetupType, nil},
-		{":8080", [][]string{{"https://example.org/d1", "https://example.org/d2", "https://example.org/d3", "https://example.org/d4"}}, "", EndpointList{}, -1, fmt.Errorf("no endpoint found for this host")},
-		{":8080", [][]string{{"https://example.org/d1", "https://example.com/d2", "https://example.net:8000/d3", "https://example.edu/d1"}}, "", EndpointList{}, -1, fmt.Errorf("no endpoint found for this host")},
+		{":8080", [][]string{{"https://example.org/d1", "https://example.org/d2", "https://example.org/d3", "https://example.org/d4"}}, "", EndpointList{}, -1, fmt.Errorf("no endpoint pointing to the local machine is found")},
+		{":8080", [][]string{{"https://example.org/d1", "https://example.com/d2", "https://example.net:8000/d3", "https://example.edu/d1"}}, "", EndpointList{}, -1, fmt.Errorf("no endpoint pointing to the local machine is found")},
 		{"localhost:9000", [][]string{{"https://127.0.0.1:9000/d1", "https://localhost:9001/d1", "https://example.com/d1", "https://example.com/d2"}}, "", EndpointList{}, -1, fmt.Errorf("path '/d1' can not be served by different port on same address")},
 		{"localhost:9000", [][]string{{"https://127.0.0.1:8000/d1", "https://localhost:9001/d2", "https://example.com/d1", "https://example.com/d2"}}, "", EndpointList{}, -1, fmt.Errorf("port number in server address must match with one of the port in local endpoints")},
 		{"localhost:10000", [][]string{{"https://127.0.0.1:8000/d1", "https://localhost:8000/d2", "https://example.com/d1", "https://example.com/d2"}}, "", EndpointList{}, -1, fmt.Errorf("server address and local endpoint have different ports")},