[ARVADOS] created: 2.1.0-763-gb44a05493
Git user
git at public.arvados.org
Wed May 5 17:40:38 UTC 2021
at b44a05493cba8cc40c81fc487cbea5ba33662d3c (commit)
commit b44a05493cba8cc40c81fc487cbea5ba33662d3c
Author: Tom Clegg <tom at curii.com>
Date: Tue May 4 17:24:03 2021 -0400
17590: Rename S3 volume config keys to match AWS terms.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/doc/install/configure-s3-object-storage.html.textile.liquid b/doc/install/configure-s3-object-storage.html.textile.liquid
index 76a2f3ab5..6a3c396d6 100644
--- a/doc/install/configure-s3-object-storage.html.textile.liquid
+++ b/doc/install/configure-s3-object-storage.html.textile.liquid
@@ -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.
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index ca627d07e..42388abfd 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -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
diff --git a/lib/config/deprecated.go b/lib/config/deprecated.go
index 0552b66ad..acaf0c5da 100644
--- a/lib/config/deprecated.go
+++ b/lib/config/deprecated.go
@@ -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, ¶ms)
+ 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 {
diff --git a/lib/config/deprecated_keepstore.go b/lib/config/deprecated_keepstore.go
index 186ffc337..d9f4815fc 100644
--- a/lib/config/deprecated_keepstore.go
+++ b/lib/config/deprecated_keepstore.go
@@ -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,
diff --git a/lib/config/deprecated_keepstore_test.go b/lib/config/deprecated_keepstore_test.go
index dab308c9d..ff1d8be7b 100644
--- a/lib/config/deprecated_keepstore_test.go
+++ b/lib/config/deprecated_keepstore_test.go
@@ -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",
diff --git a/lib/config/deprecated_test.go b/lib/config/deprecated_test.go
index 0dd03583d..3fba76588 100644
--- a/lib/config/deprecated_test.go
+++ b/lib/config/deprecated_test.go
@@ -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)
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 103266397..d413a8006 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -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
diff --git a/lib/config/load.go b/lib/config/load.go
index 292e3f6d6..cc26cdaec 100644
--- a/lib/config/load.go
+++ b/lib/config/load.go
@@ -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
}
}
diff --git a/lib/config/load_test.go b/lib/config/load_test.go
index 91bd6a743..6c11ee780 100644
--- a/lib/config/load_test.go
+++ b/lib/config/load_test.go
@@ -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{}) {
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index c0170d1d7..2c6db42d1 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -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
diff --git a/services/keepstore/command.go b/services/keepstore/command.go
index 0927b1870..bf3bf1722 100644
--- a/services/keepstore/command.go
+++ b/services/keepstore/command.go
@@ -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")
diff --git a/services/keepstore/s3_volume.go b/services/keepstore/s3_volume.go
index 07bb033c9..fdc2ed56a 100644
--- a/services/keepstore/s3_volume.go
+++ b/services/keepstore/s3_volume.go
@@ -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,
diff --git a/services/keepstore/s3_volume_test.go b/services/keepstore/s3_volume_test.go
index 2736f00b7..cb08e44ac 100644
--- a/services/keepstore/s3_volume_test.go
+++ b/services/keepstore/s3_volume_test.go
@@ -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",
diff --git a/services/keepstore/s3aws_volume.go b/services/keepstore/s3aws_volume.go
index 8d999e747..baded4116 100644
--- a/services/keepstore/s3aws_volume.go
+++ b/services/keepstore/s3aws_volume.go
@@ -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)),
})
diff --git a/services/keepstore/s3aws_volume_test.go b/services/keepstore/s3aws_volume_test.go
index d9886c07f..0387d52f1 100644
--- a/services/keepstore/s3aws_volume_test.go
+++ b/services/keepstore/s3aws_volume_test.go
@@ -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",
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list