From aa8d5ed0cc2d87f9aa6e05642a2e530fda238d65 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Fri, 20 Oct 2017 13:51:56 -0700 Subject: [PATCH] Add newS3Config helper to auto-probe signature type. (#2292) --- cmd/client-s3.go | 1 + cmd/config-host-add.go | 104 +++++++++++++++++++++++++++++----------- cmd/config-host-list.go | 6 ++- cmd/typed-errors.go | 2 +- functional-tests.sh | 26 ++++++++++ 5 files changed, 109 insertions(+), 30 deletions(-) diff --git a/cmd/client-s3.go b/cmd/client-s3.go index 79de23d3..479cce40 100644 --- a/cmd/client-s3.go +++ b/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 diff --git a/cmd/config-host-add.go b/cmd/config-host-add.go index 9bed03f4..6e226417 100644 --- a/cmd/config-host-add.go +++ b/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 } diff --git a/cmd/config-host-list.go b/cmd/config-host-list.go index 86501097..404cc930 100644 --- a/cmd/config-host-list.go +++ b/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. - host.Alias = fmt.Sprintf("%-*.*s:", maxAlias, maxAlias, host.Alias) + 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 = "" diff --git a/cmd/typed-errors.go b/cmd/typed-errors.go index 1032e0ff..0cd98074 100644 --- a/cmd/typed-errors.go +++ b/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 -h` for relevant documentation.")).Untrace() } errUnrecognizedDiffType = func(diff differType) *probe.Error { diff --git a/functional-tests.sh b/functional-tests.sh index 228257a3..40e00195 100755 --- a/functional-tests.sh +++ b/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 }