17590: Rename S3 volume config keys to match AWS terms.
authorTom Clegg <tom@curii.com>
Tue, 4 May 2021 21:24:03 +0000 (17:24 -0400)
committerTom Clegg <tom@curii.com>
Wed, 5 May 2021 17:40:17 +0000 (13:40 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

15 files changed:
doc/install/configure-s3-object-storage.html.textile.liquid
lib/config/config.default.yml
lib/config/deprecated.go
lib/config/deprecated_keepstore.go
lib/config/deprecated_keepstore_test.go
lib/config/deprecated_test.go
lib/config/generated_config.go
lib/config/load.go
lib/config/load_test.go
sdk/go/arvados/config.go
services/keepstore/command.go
services/keepstore/s3_volume.go
services/keepstore/s3_volume_test.go
services/keepstore/s3aws_volume.go
services/keepstore/s3aws_volume_test.go

index 76a2f3ab5723121cb2d0ae9d7e4724c5b2c14d06..6a3c396d60dc71a3002c18eca0c19266e8280b29 100644 (file)
@@ -43,8 +43,8 @@ Volumes are configured in the @Volumes@ section of the cluster configuration fil
 
           # If you are not using an IAM role for authentication,
           # specify access credentials here instead.
-          AccessKey: <span class="userinput">""</span>
-          SecretKey: <span class="userinput">""</span>
+          AccessKeyID: <span class="userinput">""</span>
+          SecretAccessKey: <span class="userinput">""</span>
 
           # Storage provider region. For Google Cloud Storage, use ""
           # or omit.
index ca627d07e8147a7becead4322855d0eee508b266..42388abfdd6a803c5c948281e497ba7c058bb9f4 100644 (file)
@@ -1194,8 +1194,8 @@ Clusters:
           # for s3 driver -- see
           # https://doc.arvados.org/install/configure-s3-object-storage.html
           IAMRole: aaaaa
-          AccessKey: aaaaa
-          SecretKey: aaaaa
+          AccessKeyID: aaaaa
+          SecretAccessKey: aaaaa
           Endpoint: ""
           Region: us-east-1a
           Bucket: aaaaa
index 0552b66adb80bed162ee3f2518a1c649c0c89ec2..acaf0c5da71c2cd064407bbb5fdd0bf7e38574b9 100644 (file)
@@ -5,6 +5,7 @@
 package config
 
 import (
+       "encoding/json"
        "fmt"
        "io/ioutil"
        "net/url"
@@ -119,6 +120,49 @@ func (ldr *Loader) applyDeprecatedConfig(cfg *arvados.Config) error {
        return nil
 }
 
+func (ldr *Loader) applyDeprecatedVolumeDriverParameters(cfg *arvados.Config) error {
+       for clusterID, cluster := range cfg.Clusters {
+               for volID, vol := range cluster.Volumes {
+                       if vol.Driver == "s3" {
+                               var params struct {
+                                       AccessKey       string `json:",omitempty"`
+                                       SecretKey       string `json:",omitempty"`
+                                       AccessKeyID     string
+                                       SecretAccessKey string
+                               }
+                               err := json.Unmarshal(vol.DriverParameters, &params)
+                               if err != nil {
+                                       return fmt.Errorf("error loading %s.Volumes.%s.DriverParameters: %w", clusterID, volID, err)
+                               }
+                               if params.AccessKey != "" || params.SecretKey != "" {
+                                       if params.AccessKeyID != "" || params.SecretAccessKey != "" {
+                                               ldr.Logger.Warnf("ignoring old config keys %s.Volumes.%s.DriverParameters.AccessKey/SecretKey because new keys AccessKeyID/SecretAccessKey are also present", clusterID, volID)
+                                               continue
+                                       }
+                                       var allparams map[string]interface{}
+                                       err = json.Unmarshal(vol.DriverParameters, &allparams)
+                                       if err != nil {
+                                               return fmt.Errorf("error loading %s.Volumes.%s.DriverParameters: %w", clusterID, volID, err)
+                                       }
+                                       for k := range allparams {
+                                               if lk := strings.ToLower(k); lk == "accesskey" || lk == "secretkey" {
+                                                       delete(allparams, k)
+                                               }
+                                       }
+                                       allparams["AccessKeyID"] = params.AccessKey
+                                       allparams["SecretAccessKey"] = params.SecretKey
+                                       vol.DriverParameters, err = json.Marshal(allparams)
+                                       if err != nil {
+                                               return err
+                                       }
+                                       cluster.Volumes[volID] = vol
+                               }
+                       }
+               }
+       }
+       return nil
+}
+
 func applyDeprecatedNodeProfile(hostname string, ssi systemServiceInstance, svc *arvados.Service) {
        scheme := "https"
        if !ssi.TLS {
index 186ffc3371ea6e80a80be442278fbf6b5cf596b4..d9f4815fcfecc6fe8b0a0cb8e7e87e8a59d78c83 100644 (file)
@@ -324,8 +324,8 @@ func (ldr *Loader) translateOldKeepstoreVolume(oldvol oldKeepstoreVolume) (arvad
                        StorageClasses: array2boolmap(oldvol.StorageClasses),
                }
                params = arvados.S3VolumeDriverParameters{
-                       AccessKey:          string(bytes.TrimSpace(accesskeydata)),
-                       SecretKey:          string(bytes.TrimSpace(secretkeydata)),
+                       AccessKeyID:        string(bytes.TrimSpace(accesskeydata)),
+                       SecretAccessKey:    string(bytes.TrimSpace(secretkeydata)),
                        Endpoint:           oldvol.Endpoint,
                        Region:             oldvol.Region,
                        Bucket:             oldvol.Bucket,
index dab308c9d154cf22e7d022d900378584de99ea0f..ff1d8be7bd9763abac7246bb092b51c6f5498427 100644 (file)
@@ -225,8 +225,8 @@ Volumes:
                Driver:      "S3",
                Replication: 4,
        }, &arvados.S3VolumeDriverParameters{
-               AccessKey:          "accesskeydata",
-               SecretKey:          "secretkeydata",
+               AccessKeyID:        "accesskeydata",
+               SecretAccessKey:    "secretkeydata",
                Endpoint:           "https://storage.googleapis.com",
                Region:             "us-east-1z",
                Bucket:             "testbucket",
index 0dd03583d7a178d779e759b163ee4d706bc97150..3fba765888e910e5b7bc8dd6e2ad9dcf744a0ff2 100644 (file)
@@ -47,6 +47,34 @@ func testLoadLegacyConfig(content []byte, mungeFlag string, c *check.C) (*arvado
        return cluster, nil
 }
 
+func (s *LoadSuite) TestLegacyVolumeDriverParameters(c *check.C) {
+       logs := checkEquivalent(c, `
+Clusters:
+ z1111:
+  Volumes:
+   z1111-nyw5e-aaaaaaaaaaaaaaa:
+    Driver: s3
+    DriverParameters:
+     AccessKey: exampleaccesskey
+     SecretKey: examplesecretkey
+     Region: foobar
+     ReadTimeout: 1200s
+`, `
+Clusters:
+ z1111:
+  Volumes:
+   z1111-nyw5e-aaaaaaaaaaaaaaa:
+    Driver: s3
+    DriverParameters:
+     AccessKeyID: exampleaccesskey
+     SecretAccessKey: examplesecretkey
+     Region: foobar
+     ReadTimeout: 1200s
+`)
+       c.Check(logs, check.Matches, `(?ms).*deprecated or unknown config entry: .*AccessKey.*`)
+       c.Check(logs, check.Matches, `(?ms).*deprecated or unknown config entry: .*SecretKey.*`)
+}
+
 func (s *LoadSuite) TestDeprecatedNodeProfilesToServices(c *check.C) {
        hostname, err := os.Hostname()
        c.Assert(err, check.IsNil)
index 1032663973991d2a3f6cc786e1818e5ff28192c0..d413a8006b185c113b078a6f61507ec287a8a080 100644 (file)
@@ -1200,8 +1200,8 @@ Clusters:
           # for s3 driver -- see
           # https://doc.arvados.org/install/configure-s3-object-storage.html
           IAMRole: aaaaa
-          AccessKey: aaaaa
-          SecretKey: aaaaa
+          AccessKeyID: aaaaa
+          SecretAccessKey: aaaaa
           Endpoint: ""
           Region: us-east-1a
           Bucket: aaaaa
index 292e3f6d6f39c107073ef9236585ae77764945f9..cc26cdaecc073bf747d308d7acb0a53388f3f4a6 100644 (file)
@@ -241,30 +241,33 @@ func (ldr *Loader) Load() (*arvados.Config, error) {
                return nil, fmt.Errorf("transcoding config data: %s", err)
        }
 
+       var loadFuncs []func(*arvados.Config) error
        if !ldr.SkipDeprecated {
-               err = ldr.applyDeprecatedConfig(&cfg)
-               if err != nil {
-                       return nil, err
-               }
+               loadFuncs = append(loadFuncs,
+                       ldr.applyDeprecatedConfig,
+                       ldr.applyDeprecatedVolumeDriverParameters,
+               )
        }
        if !ldr.SkipLegacy {
                // legacy file is required when either:
                // * a non-default location was specified
                // * no primary config was loaded, and this is the
                // legacy config file for the current component
-               for _, err := range []error{
-                       ldr.loadOldEnvironmentVariables(&cfg),
-                       ldr.loadOldKeepstoreConfig(&cfg),
-                       ldr.loadOldKeepWebConfig(&cfg),
-                       ldr.loadOldCrunchDispatchSlurmConfig(&cfg),
-                       ldr.loadOldWebsocketConfig(&cfg),
-                       ldr.loadOldKeepproxyConfig(&cfg),
-                       ldr.loadOldGitHttpdConfig(&cfg),
-                       ldr.loadOldKeepBalanceConfig(&cfg),
-               } {
-                       if err != nil {
-                               return nil, err
-                       }
+               loadFuncs = append(loadFuncs,
+                       ldr.loadOldEnvironmentVariables,
+                       ldr.loadOldKeepstoreConfig,
+                       ldr.loadOldKeepWebConfig,
+                       ldr.loadOldCrunchDispatchSlurmConfig,
+                       ldr.loadOldWebsocketConfig,
+                       ldr.loadOldKeepproxyConfig,
+                       ldr.loadOldGitHttpdConfig,
+                       ldr.loadOldKeepBalanceConfig,
+               )
+       }
+       for _, f := range loadFuncs {
+               err = f(&cfg)
+               if err != nil {
+                       return nil, err
                }
        }
 
index 91bd6a74392564e14b98e35bae96e872f7de8a79..6c11ee7803116ab2df30f2050f23f7ce6580a9fc 100644 (file)
@@ -437,10 +437,12 @@ Clusters:
 `)
 }
 
-func checkEquivalent(c *check.C, goty, expectedy string) {
-       gotldr := testLoader(c, goty, nil)
+func checkEquivalent(c *check.C, goty, expectedy string) string {
+       var logbuf bytes.Buffer
+       gotldr := testLoader(c, goty, &logbuf)
        expectedldr := testLoader(c, expectedy, nil)
        checkEquivalentLoaders(c, gotldr, expectedldr)
+       return logbuf.String()
 }
 
 func checkEqualYAML(c *check.C, got, expected interface{}) {
index c0170d1d7f53e61460e2e32155a250fe9ed62cc6..2c6db42d133652d535594b6e13d46c035cf5e5ea 100644 (file)
@@ -279,8 +279,8 @@ type Volume struct {
 
 type S3VolumeDriverParameters struct {
        IAMRole            string
-       AccessKey          string
-       SecretKey          string
+       AccessKeyID        string
+       SecretAccessKey    string
        Endpoint           string
        Region             string
        Bucket             string
index 0927b187048315f0c305d5043c448d5ff38a8002..bf3bf1722c0cee546488ac45c0793554be905706 100644 (file)
@@ -72,8 +72,8 @@ func convertKeepstoreFlagsToServiceFlags(args []string, lgr logrus.FieldLogger)
        flags.String("s3-bucket-volume", "", "Volumes.*.DriverParameters.Bucket")
        flags.String("s3-region", "", "Volumes.*.DriverParameters.Region")
        flags.String("s3-endpoint", "", "Volumes.*.DriverParameters.Endpoint")
-       flags.String("s3-access-key-file", "", "Volumes.*.DriverParameters.AccessKey")
-       flags.String("s3-secret-key-file", "", "Volumes.*.DriverParameters.SecretKey")
+       flags.String("s3-access-key-file", "", "Volumes.*.DriverParameters.AccessKeyID")
+       flags.String("s3-secret-key-file", "", "Volumes.*.DriverParameters.SecretAccessKey")
        flags.String("s3-race-window", "", "Volumes.*.DriverParameters.RaceWindow")
        flags.String("s3-replication", "", "Volumes.*.Replication")
        flags.String("s3-unsafe-delete", "", "Volumes.*.DriverParameters.UnsafeDelete")
index 07bb033c9f1fbc551c34fc59a3a89656cfbf7d79..fdc2ed56ad6494934141bc3e1f1c26e827f59504 100644 (file)
@@ -148,9 +148,9 @@ func (v *S3Volume) GetDeviceID() string {
 }
 
 func (v *S3Volume) bootstrapIAMCredentials() error {
-       if v.AccessKey != "" || v.SecretKey != "" {
+       if v.AccessKeyID != "" || v.SecretAccessKey != "" {
                if v.IAMRole != "" {
-                       return errors.New("invalid DriverParameters: AccessKey and SecretKey must be blank if IAMRole is specified")
+                       return errors.New("invalid DriverParameters: AccessKeyID and SecretAccessKey must be blank if IAMRole is specified")
                }
                return nil
        }
@@ -175,7 +175,7 @@ func (v *S3Volume) bootstrapIAMCredentials() error {
 }
 
 func (v *S3Volume) newS3Client() *s3.S3 {
-       auth := aws.NewAuth(v.AccessKey, v.SecretKey, v.AuthToken, v.AuthExpiration)
+       auth := aws.NewAuth(v.AccessKeyID, v.SecretAccessKey, v.AuthToken, v.AuthExpiration)
        client := s3.New(*auth, v.region)
        if !v.V2Signature {
                client.Signature = aws.V4Signature
@@ -225,7 +225,7 @@ func (v *S3Volume) updateIAMCredentials() (time.Duration, error) {
                }
                defer resp.Body.Close()
                if resp.StatusCode == http.StatusNotFound {
-                       return 0, fmt.Errorf("this instance does not have an IAM role assigned -- either assign a role, or configure AccessKey and SecretKey explicitly in DriverParameters (error getting %s: HTTP status %s)", url, resp.Status)
+                       return 0, fmt.Errorf("this instance does not have an IAM role assigned -- either assign a role, or configure AccessKeyID and SecretAccessKey explicitly in DriverParameters (error getting %s: HTTP status %s)", url, resp.Status)
                } else if resp.StatusCode != http.StatusOK {
                        return 0, fmt.Errorf("error getting %s: HTTP status %s", url, resp.Status)
                }
@@ -260,7 +260,7 @@ func (v *S3Volume) updateIAMCredentials() (time.Duration, error) {
        if err != nil {
                return 0, fmt.Errorf("error decoding credentials from %s: %s", url, err)
        }
-       v.AccessKey, v.SecretKey, v.AuthToken, v.AuthExpiration = cred.AccessKeyID, cred.SecretAccessKey, cred.Token, cred.Expiration
+       v.AccessKeyID, v.SecretAccessKey, v.AuthToken, v.AuthExpiration = cred.AccessKeyID, cred.SecretAccessKey, cred.Token, cred.Expiration
        v.bucket.SetBucket(&s3.Bucket{
                S3:   v.newS3Client(),
                Name: v.Bucket,
index 2736f00b743c791502f78886e716b521a0585eb1..cb08e44ac8626134a03063f5426f74fdf29395d4 100644 (file)
@@ -111,11 +111,11 @@ func (s *StubbedS3Suite) TestSignatureVersion(c *check.C) {
        // Default V4 signature
        vol := S3Volume{
                S3VolumeDriverParameters: arvados.S3VolumeDriverParameters{
-                       AccessKey: "xxx",
-                       SecretKey: "xxx",
-                       Endpoint:  stub.URL,
-                       Region:    "test-region-1",
-                       Bucket:    "test-bucket-name",
+                       AccessKeyID:     "xxx",
+                       SecretAccessKey: "xxx",
+                       Endpoint:        stub.URL,
+                       Region:          "test-region-1",
+                       Bucket:          "test-bucket-name",
                },
                cluster: s.cluster,
                logger:  ctxlog.TestLogger(c),
@@ -130,12 +130,12 @@ func (s *StubbedS3Suite) TestSignatureVersion(c *check.C) {
        // Force V2 signature
        vol = S3Volume{
                S3VolumeDriverParameters: arvados.S3VolumeDriverParameters{
-                       AccessKey:   "xxx",
-                       SecretKey:   "xxx",
-                       Endpoint:    stub.URL,
-                       Region:      "test-region-1",
-                       Bucket:      "test-bucket-name",
-                       V2Signature: true,
+                       AccessKeyID:     "xxx",
+                       SecretAccessKey: "xxx",
+                       Endpoint:        stub.URL,
+                       Region:          "test-region-1",
+                       Bucket:          "test-bucket-name",
+                       V2Signature:     true,
                },
                cluster: s.cluster,
                logger:  ctxlog.TestLogger(c),
@@ -160,8 +160,8 @@ func (s *StubbedS3Suite) TestIAMRoleCredentials(c *check.C) {
        defer s.metadata.Close()
 
        v := s.newTestableVolume(c, s.cluster, arvados.Volume{Replication: 2}, newVolumeMetricsVecs(prometheus.NewRegistry()), 5*time.Minute)
-       c.Check(v.AccessKey, check.Equals, "ASIAIOSFODNN7EXAMPLE")
-       c.Check(v.SecretKey, check.Equals, "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY")
+       c.Check(v.AccessKeyID, check.Equals, "ASIAIOSFODNN7EXAMPLE")
+       c.Check(v.SecretAccessKey, check.Equals, "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY")
        c.Check(v.bucket.bucket.S3.Auth.AccessKey, check.Equals, "ASIAIOSFODNN7EXAMPLE")
        c.Check(v.bucket.bucket.S3.Auth.SecretKey, check.Equals, "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY")
 
@@ -519,8 +519,8 @@ func (s *StubbedS3Suite) newTestableVolume(c *check.C, cluster *arvados.Cluster,
                S3Volume: &S3Volume{
                        S3VolumeDriverParameters: arvados.S3VolumeDriverParameters{
                                IAMRole:            iamRole,
-                               AccessKey:          accessKey,
-                               SecretKey:          secretKey,
+                               AccessKeyID:        accessKey,
+                               SecretAccessKey:    secretKey,
                                Bucket:             TestBucketName,
                                Endpoint:           endpoint,
                                Region:             "test-region-1",
index 8d999e7472ff14f03985e80a2b774632eebe7346..baded411634276140516d3fda24260866f9ac19f 100644 (file)
@@ -192,7 +192,7 @@ func (v *S3AWSVolume) check(ec2metadataHostname string) error {
 
        creds := aws.NewChainProvider(
                []aws.CredentialsProvider{
-                       aws.NewStaticCredentialsProvider(v.AccessKey, v.SecretKey, v.AuthToken),
+                       aws.NewStaticCredentialsProvider(v.AccessKeyID, v.SecretAccessKey, v.AuthToken),
                        ec2rolecreds.New(ec2metadata.New(cfg)),
                })
 
index d9886c07f317904ea141343d2f26554ecae10b3a..0387d52f18f9bc8e4d05a6d6179117d06f53c2ff 100644 (file)
@@ -123,11 +123,11 @@ func (s *StubbedS3AWSSuite) TestSignature(c *check.C) {
        // as of June 24, 2020. Cf. https://forums.aws.amazon.com/ann.jspa?annID=5816
        vol := S3AWSVolume{
                S3VolumeDriverParameters: arvados.S3VolumeDriverParameters{
-                       AccessKey: "xxx",
-                       SecretKey: "xxx",
-                       Endpoint:  stub.URL,
-                       Region:    "test-region-1",
-                       Bucket:    "test-bucket-name",
+                       AccessKeyID:     "xxx",
+                       SecretAccessKey: "xxx",
+                       Endpoint:        stub.URL,
+                       Region:          "test-region-1",
+                       Bucket:          "test-bucket-name",
                },
                cluster: s.cluster,
                logger:  ctxlog.TestLogger(c),
@@ -567,8 +567,8 @@ func (s *StubbedS3AWSSuite) newTestableVolume(c *check.C, cluster *arvados.Clust
                S3AWSVolume: &S3AWSVolume{
                        S3VolumeDriverParameters: arvados.S3VolumeDriverParameters{
                                IAMRole:            iamRole,
-                               AccessKey:          accessKey,
-                               SecretKey:          secretKey,
+                               AccessKeyID:        accessKey,
+                               SecretAccessKey:    secretKey,
                                Bucket:             S3AWSTestBucketName,
                                Endpoint:           endpoint,
                                Region:             "test-region-1",