mantle: overhaul aliyun auth

The Aliyun SDK has a native file format it can consume for credentials [1]. This commit proposes we use that file format here and drop the custom JSON parsing that we're doing for Aliyun. This commit also drops the toplevel `--config` argument in `cmd-ore-wrapper` because Aliyun was the only consumer of that argument. [1] https://github.com/aliyun/alibaba-cloud-sdk-go/blob/710fea6cf27cf855f301462a2adcf3399169f63c/docs/2-Client-EN.md#2-credentials-file
coreos · Oct 27, 2022 · 1dd027b · 1dd027b
1 parent 0cb63a8
commit 1dd027b
Show file tree

Hide file tree

Showing 6 changed files with 74 additions and 142 deletions.
diff --git a/docs/mantle/credentials.md b/docs/mantle/credentials.md
@@ -19,45 +19,36 @@ kola spawn -p aws --aws-profile other_profile
 
 ## aliyun
 
-`aliyun` reads the `~/.aliyun/config.json` file used by Aliyun's aliyun command-line tool.
-It can be created using the `aliyun` command:
-```
-$ aliyun configure
-```
-To configure a different profile, use the `--profile` flag
-```
-$ aliyun configure --profile other_profile
-```
+The Access Key/Secret for the Aliyun SDK can be specified either
+in a [credentials file](https://github.com/aliyun/alibaba-cloud-sdk-go/blob/master/docs/2-Client-EN.md#2-credentials-file)
+or directly in [environment variables](https://github.com/aliyun/alibaba-cloud-sdk-go/blob/master/docs/2-Client-EN.md#1-environment-credentials)
+as authentication input.
+
+- credentials file
+    - place the file in `~/.alibabacloud/credentials`
+    - populate the `ALIBABA_CLOUD_CREDENTIALS_FILE` environment variable
+      with the path to the credentials file.
+- environment variables
+    - populate the key in the `ALIBABA_CLOUD_ACCESS_KEY_ID` environment variable
+    - populate the secret in the `ALIBABA_CLOUD_ACCESS_KEY_SECRET` environment variable
+
+Ironically, the `aliyun` CLI uses the slightly different
+[environment variables](https://github.com/aliyun/aliyun-cli#support-for-environment-variables)
+of `ALIBABACLOUD_ACCESS_KEY_ID` and `ALIBABACLOUD_ACCESS_KEY_SECRET`.
+
+
+For mantle populate the credentials file and either place it at `~/.alibabacloud/credentials`
+or populate the `ALIBABA_CLOUD_CREDENTIALS_FILE` environment variable.
 
-The `~/.aliyun/config.json` file can also be populated manually:
 ```
-{
-  "current": "",
-  "profiles": [
-    {
-      "name": "",
-      "mode": "AK",
-      "access_key_id": "ACCESS_KEY_ID",
-      "access_key_secret": "ACCESS_KEY_SECRET",
-      "sts_token": "",
-      "ram_role_name": "",
-      "ram_role_arn": "",
-      "ram_session_name": "",
-      "private_key": "",
-      "key_pair_name": "",
-      "expired_seconds": 0,
-      "verified": "",
-      "region_id": "eu-central-1",
-      "output_format": "json",
-      "language": "zh",
-      "site": "",
-      "retry_timeout": 0,
-      "retry_count": 0
-    }
-  ]
-}
+[default]
+type=access_key
+access_key_id=ACCESS_KEY_ID
+access_key_secret=ACCESS_KEY_SECRET
 ```
 
+To configure a different profile than `default`, use the `--profile` flag.
+
 ## aws
 
 `aws` reads the `~/.aws/credentials` file used by Amazon's aws command-line tool.

diff --git a/mantle/auth/aliyun.go b/mantle/auth/aliyun.go
diff --git a/mantle/cmd/ore/aliyun/aliyun.go b/mantle/cmd/ore/aliyun/aliyun.go
@@ -16,7 +16,6 @@ package aliyun
 
 import (
 	"fmt"
-	"os"
 
 	"github.com/coreos/pkg/capnslog"
 	"github.com/spf13/cobra"
@@ -38,9 +37,7 @@ var (
 )
 
 func init() {
-	defaultConfigPath := os.Getenv("ALIYUN_CONFIG_FILE")
-
-	Aliyun.PersistentFlags().StringVar(&options.ConfigPath, "config-file", defaultConfigPath, "config file (default \""+defaultConfigPath+"\")")
+	Aliyun.PersistentFlags().StringVar(&options.CredentialsFile, "credentials-file", "", "Use custom path for Aliyun credentials file")
 	Aliyun.PersistentFlags().StringVar(&options.Profile, "profile", "", "profile (default \"default\")")
 	Aliyun.PersistentFlags().StringVar(&options.Region, "region", "", "region")
 	cli.WrapPreRun(Aliyun, preflightCheck)

diff --git a/mantle/platform/api/aliyun/api.go b/mantle/platform/api/aliyun/api.go
@@ -17,15 +17,17 @@ package aliyun
 import (
 	"fmt"
 	"io"
+	"os"
 	"sort"
 	"time"
 
-	"github.com/coreos/mantle/auth"
 	"github.com/coreos/mantle/platform"
 	"github.com/coreos/mantle/util"
 	"github.com/coreos/pkg/capnslog"
 	"github.com/coreos/pkg/multierror"
 
+	"github.com/aliyun/alibaba-cloud-sdk-go/sdk/auth/credentials"
+	"github.com/aliyun/alibaba-cloud-sdk-go/sdk/auth/credentials/provider"
 	"github.com/aliyun/alibaba-cloud-sdk-go/sdk/requests"
 	"github.com/aliyun/alibaba-cloud-sdk-go/services/ecs"
 	"github.com/aliyun/aliyun-oss-go-sdk/oss"
@@ -41,8 +43,11 @@ type Options struct {
 	// The aliyun region regional api calls should use
 	Region string
 
-	// Config file. Defaults to ~/.aliyun/config.json
-	ConfigPath string
+
+	// Path to an ALIBABA_CLOUD_CREDENTIALS_FILE
+	// https://github.com/aliyun/alibaba-cloud-sdk-go/blob/master/docs/2-Client-EN.md#2-credentials-file
+	CredentialsFile string
+
 	// The profile to use when resolving credentials, if applicable
 	Profile string
 
@@ -62,30 +67,40 @@ type API struct {
 // standard credentials sources, including the environment and the profile
 // configured in ~/.aliyun.
 func New(opts *Options) (*API, error) {
-	profiles, err := auth.ReadAliyunConfig(opts.ConfigPath)
-	if err != nil {
-		return nil, fmt.Errorf("couldn't read aliyun config: %v", err)
-	}
 
-	if opts.Profile == "" {
-		opts.Profile = "default"
-	}
-
-	profile, ok := profiles[opts.Profile]
-	if !ok {
-		return nil, fmt.Errorf("no such profile %q", opts.Profile)
-	}
-
-	if opts.AccessKeyID == "" {
-		opts.AccessKeyID = profile.AccessKeyID
-	}
-
-	if opts.SecretKey == "" {
-		opts.SecretKey = profile.AccessKeySecret
-	}
+	// If the user didn't provide an access key and secret directly then try to pick up
+	// the credentials from the normal locations defined by the SDK
+	// https://github.com/aliyun/alibaba-cloud-sdk-go/blob/master/docs/2-Client-EN.md#default-credential-provider-chain
+	if opts.AccessKeyID == "" || opts.SecretKey == "" {
+
+		// If the user provided a path to a credential file then
+		// let's set it now the only supported way, which is with
+		// the ALIBABA_CLOUD_CREDENTIALS_FILE env var.
+		if opts.CredentialsFile != "" {
+			os.Setenv(provider.ENVCredentialFile, opts.CredentialsFile)
+			defer os.Unsetenv(provider.ENVCredentialFile)
+		}
 
-	if opts.Region == "" {
-		opts.Region = profile.Region
+		var p provider.Provider
+		if opts.Profile != "" {
+			// If the user specified a profile then they're
+			// using a credentials file with profiles in them.
+			p = provider.NewProfileProvider(opts.Profile)
+		} else {
+			// If not then they could be using environment variables
+			// so use the DefaultChain (includes env var provider)
+			p = provider.DefaultChain
+		}
+		credential, err := p.Resolve()
+		if err != nil {
+			return nil, fmt.Errorf("failed to detect aliyun auth credentials: %w", err)
+		}
+		keycred, ok := credential.(*credentials.AccessKeyCredential)
+		if !ok {
+			return nil, fmt.Errorf("failed to convert the credential to an AccessKeyCredential")
+		}
+		opts.AccessKeyID = keycred.AccessKeyId
+		opts.SecretKey = keycred.AccessKeySecret
 	}
 
 	ecs, err := ecs.NewClientWithAccessKey(opts.Region, opts.AccessKeyID, opts.SecretKey)

diff --git a/src/cmd-ore-wrapper b/src/cmd-ore-wrapper
@@ -72,8 +72,6 @@ Each target has its own sub options. To access them us:
     parser.add_argument("--build-artifact", "--build-if-missing",
                         action='store_true', default=default_build_artifact,
                         help="Build the artifact if missing")
-    parser.add_argument("--config", "--config-file",
-                        help="ore configuration")
     parser.add_argument("--force", action='store_true',
                         help="Force the operation if it has already happened")
     parser.add_argument("--compress", action='store_true',

diff --git a/src/cosalib/aliyun.py b/src/cosalib/aliyun.py
@@ -31,7 +31,7 @@ def aliyun_run_ore_replicate(build, args):
 
     if not args.region:
         args.region = subprocess.check_output([
-            'ore', f'--config-file={args.config}' if args.config else '',
+            'ore', f'--credentials-file={args.credentials_file}' if args.credentials_file else '',
             'aliyun', 'list-regions'
         ]).decode().strip().split()
         log.info(("default: replicating to all regions. If this is not "
@@ -65,8 +65,8 @@ def aliyun_run_ore_replicate(build, args):
         '--wait-for-ready'
     ]
 
-    if args.config:
-        ore_args.extend(['--config-file', args.config])
+    if args.credentials_file:
+        ore_args.extend(['--credentials-file', args.credentials_file])
 
     upload_failed_in_region = None
 
@@ -115,8 +115,8 @@ def make_public(build, args):
         'aliyun', 'visibility', '--public'
     ]
 
-    if args.config:
-        make_public_args.extend(['--config-file', args.config])
+    if args.credentials_file:
+        make_public_args.extend(['--credentials-file', args.credentials_file])
 
     # build out a list of region:image pairs to pass to `ore aliyun visibility`
     region_image_pairs = []
@@ -157,7 +157,7 @@ def aliyun_run_ore(build, args):
         raise Exception("Must supply OSS bucket when uploading")
 
     ore_args.extend([
-        f'--config-file={args.config}' if args.config else '',
+        f'--credentails-file={args.credentials_file}' if args.credentials_file else '',
         'aliyun', 'create-image',
         '--region', region,
         '--bucket', args.bucket,
@@ -187,6 +187,7 @@ def aliyun_run_ore(build, args):
 
 def aliyun_cli(parser):
     parser.add_argument("--bucket", help="OSS Bucket")
+    parser.add_argument("--credentials-file", help="Credentials file to use")
     parser.add_argument("--name-suffix", help="Suffix for uploaded image name")
     parser.add_argument("--public", action="store_true", help="Mark images as publicly available")
     return parser