Add newS3Config helper to auto-probe signature type. (#2292)

cmd/client-s3.go

@@ -182,6 +182,7 @@ func newFactory() func(config *Config) (Client, *probe.Error) {
 			// Cache the new minio client with hash of config as key.
 			clientCache[confSum] = api
 		}
+
 		// Store the new api object.
 		s3Clnt.api = api
 

cmd/config-host-add.go

@@ -17,11 +17,10 @@
 package cmd
 
 import (
-	urlpkg "net/url"
-
 	"github.com/fatih/color"
 	"github.com/minio/cli"
 	"github.com/minio/mc/pkg/console"
+	"github.com/minio/mc/pkg/probe"
 )
 
 var configHostAddCmd = cli.Command{
@@ -53,6 +52,14 @@ EXAMPLES:
      $ {{.HelpName}} mys3-accel https://s3-accelerate.amazonaws.com \
                  BKIKJAA5BMMU2RHO6IBB V8f1CwQqAcwo80UEIJEjc5gVQUSSx5ohQ9GSrr12
      $ set -o history
+
+  3. Add Amazon S3 IAM temporary credentials with limited access, please make sure to override the signature probe by explicitly
+     providing the signature type.
+     $ set +o history
+     $ {{.HelpName}} mys3-iam https://s3.amazonaws.com \
+                 BKIKJAA5BMMU2RHO6IBB V8f1CwQqAcwo80UEIJEjc5gVQUSSx5ohQ9GSrr12 s3v4
+     $ set -o history
+
 `,
 }
 
@@ -116,36 +123,79 @@ func addHost(alias string, hostCfgV8 hostConfigV8) {
 	})
 }
 
+func newS3Config(args cli.Args) (*Config, *probe.Error) {
+	var (
+		url       = args.Get(0)
+		accessKey = args.Get(1)
+		secretKey = args.Get(2)
+		api       = args.Get(3)
+	)
+
+	// If api is provided we do not auto probe signature, this is
+	// required in situations when signature type is provided by the user.
+	if api != "" {
+		return &Config{
+			AccessKey: accessKey,
+			SecretKey: secretKey,
+			Signature: api,
+			HostURL:   url,
+		}, nil
+	}
+
+	s3Config := &Config{
+		AccessKey: accessKey,
+		SecretKey: secretKey,
+		Signature: "s3v4",
+		HostURL:   urlJoinPath(url, "probe-bucket-sign"),
+	}
+
+	s3Client, err := s3New(s3Config)
+	if err != nil {
+		return nil, err.Trace(args...)
+	}
+
+	if _, err = s3Client.Stat(false); err != nil {
+		switch err.ToGoError().(type) {
+		case BucketDoesNotExist:
+			// Bucket doesn't exist, means signature probing worked V4.
+		default:
+			// Attempt with signature v2, since v4 seem to have failed.
+			s3Config.Signature = "s3v2"
+			s3Client, err = s3New(s3Config)
+			if err != nil {
+				return nil, err.Trace(args...)
+			}
+			if _, err = s3Client.Stat(false); err != nil {
+				switch err.ToGoError().(type) {
+				case BucketDoesNotExist:
+					// Bucket doesn't exist, means signature probing worked with V2.
+				default:
+					return nil, err.Trace(args...)
+				}
+			}
+		}
+	}
+
+	// Set the host url with original URL.
+	s3Config.HostURL = url
+
+	// Success.
+	return s3Config, nil
+}
+
 func mainConfigHostAdd(ctx *cli.Context) error {
 	checkConfigHostAddSyntax(ctx)
 
 	console.SetColor("HostMessage", color.New(color.FgGreen))
 
-	args := ctx.Args()
-	alias := args.Get(0)
-	url := args.Get(1)
-	accessKey := args.Get(2)
-	secretKey := args.Get(3)
-	api := args.Get(4)
+	s3Config, err := newS3Config(ctx.Args().Tail())
+	fatalIf(err.Trace(ctx.Args()...), "Unable to initialize new config from the provided credentials")
 
-	parsedURL, _ := urlpkg.Parse(url)
-
-	if isGoogle(parsedURL.Host) {
-		if api == "S3v4" {
-			fatalIf(errInvalidArgument().Trace(api), "Unsupported API signature for url. Please use `mc config host add "+alias+" "+url+" "+accessKey+" "+secretKey+" S3v2` instead.")
-		}
-		api = "S3v2"
-	}
-
-	if api == "" {
-		api = "S3v4"
-	}
-	hostCfg := hostConfigV8{
-		URL:       url,
-		AccessKey: accessKey,
-		SecretKey: secretKey,
-		API:       api,
-	}
-	addHost(alias, hostCfg) // Add a host with specified credentials.
+	addHost(ctx.Args().Get(0), hostConfigV8{
+		URL:       s3Config.HostURL,
+		AccessKey: s3Config.AccessKey,
+		SecretKey: s3Config.SecretKey,
+		API:       s3Config.Signature,
+	}) // Add a host with specified credentials.
 	return nil
 }

cmd/config-host-list.go

@@ -93,8 +93,10 @@ func printHosts(hosts ...hostMessage) {
 		}
 	}
 	for _, host := range hosts {
-		// Format properly for alignment based on alias length.
+		if !globalJSON {
+			// Format properly for alignment based on alias length only in non json mode.
 			host.Alias = fmt.Sprintf("%-*.*s:", maxAlias, maxAlias, host.Alias)
+		}
 		if host.AccessKey == "" || host.SecretKey == "" {
 			host.AccessKey = ""
 			host.SecretKey = ""

cmd/typed-errors.go

@@ -30,7 +30,7 @@ var (
 	}
 
 	errInvalidArgument = func() *probe.Error {
-		return probe.NewError(errors.New("Invalid arguments provided, cannot proceed")).Untrace()
+		return probe.NewError(errors.New("Invalid arguments provided, please refer " + "`mc <command> -h` for relevant documentation.")).Untrace()
 	}
 
 	errUnrecognizedDiffType = func(diff differType) *probe.Error {

functional-tests.sh

@@ -457,6 +457,29 @@ function test_watch_object()
     log_success "$start_time" "${FUNCNAME[0]}"
 }
 
+
+function test_config_host_add()
+{
+    show "${FUNCNAME[0]}"
+    start_time=$(get_time)
+
+    assert_success "$start_time" "${FUNCNAME[0]}" mc_cmd config host add "${SERVER_ALIAS}1" "$ENDPOINT" "$ACCESS_KEY" "$SECRET_KEY"
+    assert_success "$start_time" "${FUNCNAME[0]}" mc_cmd config host list "${SERVER_ALIAS}1"
+}
+
+function test_config_host_add_error()
+{
+    show "${FUNCNAME[0]}"
+    start_time=$(get_time)
+
+    out=$("${MC_CMD[@]}" --json config host add "${SERVER_ALIAS}1" "$ENDPOINT" "$ACCESS_KEY" "invalid-secret")
+    assert_failure "$start_time" "${FUNCNAME[0]}" fail $? "adding host should fail"
+    got_code=$(echo "$out" | jq -r .error.cause.error.Code)
+    if [ "${got_code}" != "SignatureDoesNotMatch" ]; then
+        assert_failure "$start_time" "${FUNCNAME[0]}" fail 1 "incorrect error code ${got_code} returned by server"
+    fi
+}
+
 function run_test()
 {
     test_make_bucket
@@ -479,6 +502,9 @@ function run_test()
         test_watch_object
     fi
 
+    test_config_host_add
+    test_config_host_add_error
+
     teardown
 }