[ARVADOS] created: 1.3.0-1669-g5612cb854

Git user git at public.curoverse.com
Mon Sep 30 23:24:06 UTC 2019


        at  5612cb8542511ea96108604499b8b7e37e3804c2 (commit)


commit 5612cb8542511ea96108604499b8b7e37e3804c2
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Sep 30 19:22:27 2019 -0400

    15599: Improve error message when no role is defined.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keepstore/s3_volume.go b/services/keepstore/s3_volume.go
index 65950606f..e39d7b79b 100644
--- a/services/keepstore/s3_volume.go
+++ b/services/keepstore/s3_volume.go
@@ -233,7 +233,9 @@ func (v *S3Volume) updateIAMCredentials() (time.Duration, error) {
 			return 0, fmt.Errorf("error getting %s: %s", url, err)
 		}
 		defer resp.Body.Close()
-		if resp.StatusCode != http.StatusOK {
+		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)
+		} else if resp.StatusCode != http.StatusOK {
 			return 0, fmt.Errorf("error getting %s: HTTP status %s", url, resp.Status)
 		}
 		body := bufio.NewReader(resp.Body)

commit fdd56a5f193c4d8c561059c74d3aa4e850a483d1
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Sep 30 19:22:25 2019 -0400

    15599: Warn if multiple roles are assigned.
    
    This is currently impossible on AWS. If it becomes possible in the
    future, this may help with troubleshooting.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keepstore/s3_volume.go b/services/keepstore/s3_volume.go
index 20bd99255..65950606f 100644
--- a/services/keepstore/s3_volume.go
+++ b/services/keepstore/s3_volume.go
@@ -5,6 +5,7 @@
 package main
 
 import (
+	"bufio"
 	"bytes"
 	"context"
 	"crypto/sha256"
@@ -235,11 +236,15 @@ func (v *S3Volume) updateIAMCredentials() (time.Duration, error) {
 		if resp.StatusCode != http.StatusOK {
 			return 0, fmt.Errorf("error getting %s: HTTP status %s", url, resp.Status)
 		}
+		body := bufio.NewReader(resp.Body)
 		var role string
-		_, err = fmt.Fscanf(resp.Body, "%s\n", &role)
+		_, err = fmt.Fscanf(body, "%s\n", &role)
 		if err != nil {
 			return 0, fmt.Errorf("error reading response from %s: %s", url, err)
 		}
+		if n, _ := body.Read(make([]byte, 64)); n > 0 {
+			v.logger.Warnf("ignoring additional data returned by metadata endpoint %s after the single role name that we expected", url)
+		}
 		v.logger.WithField("Role", role).Debug("looked up IAM role name")
 		url = url + role
 	}

commit e8e461a99945a74968de8dd438e54649ce006216
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Fri Sep 27 17:30:47 2019 -0400

    15599: Look up IAM role name from metadata if not configured.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/doc/install/configure-s3-object-storage.html.textile.liquid b/doc/install/configure-s3-object-storage.html.textile.liquid
index 4007e6d22..d6366aa6a 100644
--- a/doc/install/configure-s3-object-storage.html.textile.liquid
+++ b/doc/install/configure-s3-object-storage.html.textile.liquid
@@ -32,13 +32,16 @@ Volumes are configured in the @Volumes@ section of the cluster configuration fil
 
         Driver: S3
         DriverParameters:
-          # If an IAM role name is given here, credentials to access
-          # the bucket will be retrieved at runtime from instance
-          # metadata.
+          # IAM role name to use when retrieving credentials from
+          # instance metadata. This is optional (if omitted, the role
+          # name itself is retrieved from instance metadata) but it
+          # may protect you from using the wrong credentials in the
+          # event of an installation/configuration error.
           IAMRole: s3access
 
           # The credentials to use to access the bucket. Omit or leave
-          # blank if IAMRole is configured.
+          # blank to use the credentials provided by the instance's
+          # IAM role.
           AccessKey: aaaaa
           SecretKey: aaaaa
 
diff --git a/services/keepstore/s3_volume.go b/services/keepstore/s3_volume.go
index ad04a2e22..20bd99255 100644
--- a/services/keepstore/s3_volume.go
+++ b/services/keepstore/s3_volume.go
@@ -48,9 +48,6 @@ func (v *S3Volume) check() error {
 	if v.Bucket == "" {
 		return errors.New("DriverParameters: Bucket must be provided")
 	}
-	if v.IAMRole == "" && (v.AccessKey == "" || v.SecretKey == "") {
-		return errors.New("DriverParameters: either IAMRole or literal credentials (AccessKey and SecretKey) must be provided")
-	}
 	if v.IndexPageSize == 0 {
 		v.IndexPageSize = 1000
 	}
@@ -161,7 +158,7 @@ func (v *S3Volume) GetDeviceID() string {
 }
 
 func (v *S3Volume) bootstrapIAMCredentials() error {
-	if v.IAMRole == "" {
+	if v.AccessKey != "" || v.SecretKey != "" {
 		return nil
 	}
 	ttl, err := v.updateIAMCredentials()
@@ -213,15 +210,40 @@ func (v *S3Volume) updateIAMCredentials() (time.Duration, error) {
 	ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(time.Minute))
 	defer cancel()
 
+	metadataBaseURL := "http://169.254.169.254/latest/meta-data/iam/security-credentials/"
+
 	var url string
 	if strings.Contains(v.IAMRole, "://") {
 		// Configuration provides complete URL (used by tests)
 		url = v.IAMRole
-	} else {
+	} else if v.IAMRole != "" {
 		// Configuration provides IAM role name and we use the
 		// AWS metadata endpoint
-		url = "http://169.254.169.254/latest/meta-data/iam/security-credentials/" + v.IAMRole
+		url = metadataBaseURL + v.IAMRole
+	} else {
+		url = metadataBaseURL
+		v.logger.WithField("URL", url).Debug("looking up IAM role name")
+		req, err := http.NewRequest("GET", url, nil)
+		if err != nil {
+			return 0, fmt.Errorf("error setting up request %s: %s", url, err)
+		}
+		resp, err := http.DefaultClient.Do(req.WithContext(ctx))
+		if err != nil {
+			return 0, fmt.Errorf("error getting %s: %s", url, err)
+		}
+		defer resp.Body.Close()
+		if resp.StatusCode != http.StatusOK {
+			return 0, fmt.Errorf("error getting %s: HTTP status %s", url, resp.Status)
+		}
+		var role string
+		_, err = fmt.Fscanf(resp.Body, "%s\n", &role)
+		if err != nil {
+			return 0, fmt.Errorf("error reading response from %s: %s", url, err)
+		}
+		v.logger.WithField("Role", role).Debug("looked up IAM role name")
+		url = url + role
 	}
+
 	v.logger.WithField("URL", url).Debug("getting credentials")
 	req, err := http.NewRequest("GET", url, nil)
 	if err != nil {

commit 25a7acb26323d159a60e43b037a19e255b6abdbd
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Fri Sep 27 16:40:19 2019 -0400

    15599: Fix misleading error message when address already in use.
    
    Also warn about unexpected errors encountered while looking for the
    current host's entry in InternalURLs.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/service/cmd.go b/lib/service/cmd.go
index ddff5f47a..0391c5a04 100644
--- a/lib/service/cmd.go
+++ b/lib/service/cmd.go
@@ -103,7 +103,7 @@ func (c *command) RunCommand(prog string, args []string, stdin io.Reader, stdout
 	})
 	ctx := ctxlog.Context(c.ctx, logger)
 
-	listenURL, err := getListenAddr(cluster.Services, c.svcName)
+	listenURL, err := getListenAddr(cluster.Services, c.svcName, log)
 	if err != nil {
 		return 1
 	}
@@ -159,7 +159,7 @@ func (c *command) RunCommand(prog string, args []string, stdin io.Reader, stdout
 
 const rfc3339NanoFixed = "2006-01-02T15:04:05.000000000Z07:00"
 
-func getListenAddr(svcs arvados.Services, prog arvados.ServiceName) (arvados.URL, error) {
+func getListenAddr(svcs arvados.Services, prog arvados.ServiceName, log logrus.FieldLogger) (arvados.URL, error) {
 	svc, ok := svcs.Map()[prog]
 	if !ok {
 		return arvados.URL{}, fmt.Errorf("unknown service name %q", prog)
@@ -172,6 +172,12 @@ func getListenAddr(svcs arvados.Services, prog arvados.ServiceName) (arvados.URL
 		if err == nil {
 			listener.Close()
 			return url, nil
+		} else if strings.Contains(err.Error(), "cannot assign requested address") {
+			continue
+		} else if strings.Contains(err.Error(), "address already in use") {
+			return url, err
+		} else {
+			log.Warn(err)
 		}
 	}
 	return arvados.URL{}, fmt.Errorf("configuration does not enable the %s service on this host", prog)

commit 1d54582d7d80362602a017ebf9dbf79d2712b6f0
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Fri Sep 27 16:39:51 2019 -0400

    15599: Support IAM role credentials in keepstore.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/doc/install/configure-s3-object-storage.html.textile.liquid b/doc/install/configure-s3-object-storage.html.textile.liquid
index 6e725b344..4007e6d22 100644
--- a/doc/install/configure-s3-object-storage.html.textile.liquid
+++ b/doc/install/configure-s3-object-storage.html.textile.liquid
@@ -32,7 +32,13 @@ Volumes are configured in the @Volumes@ section of the cluster configuration fil
 
         Driver: S3
         DriverParameters:
-          # The credentials to use to access the bucket.
+          # If an IAM role name is given here, credentials to access
+          # the bucket will be retrieved at runtime from instance
+          # metadata.
+          IAMRole: s3access
+
+          # The credentials to use to access the bucket. Omit or leave
+          # blank if IAMRole is configured.
           AccessKey: aaaaa
           SecretKey: aaaaa
 
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 572a2558e..7c2223317 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -815,6 +815,7 @@ Clusters:
 
           # for s3 driver -- see
           # https://doc.arvados.org/install/configure-s3-object-storage.html
+          IAMRole: aaaaa
           AccessKey: aaaaa
           SecretKey: aaaaa
           Endpoint: ""
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 32c101a5a..d18251b27 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -821,6 +821,7 @@ Clusters:
 
           # for s3 driver -- see
           # https://doc.arvados.org/install/configure-s3-object-storage.html
+          IAMRole: aaaaa
           AccessKey: aaaaa
           SecretKey: aaaaa
           Endpoint: ""
diff --git a/services/keepstore/s3_volume.go b/services/keepstore/s3_volume.go
index 9fc87045e..ad04a2e22 100644
--- a/services/keepstore/s3_volume.go
+++ b/services/keepstore/s3_volume.go
@@ -45,8 +45,11 @@ func newS3Volume(cluster *arvados.Cluster, volume arvados.Volume, logger logrus.
 }
 
 func (v *S3Volume) check() error {
-	if v.Bucket == "" || v.AccessKey == "" || v.SecretKey == "" {
-		return errors.New("DriverParameters: Bucket, AccessKey, and SecretKey must be provided")
+	if v.Bucket == "" {
+		return errors.New("DriverParameters: Bucket must be provided")
+	}
+	if v.IAMRole == "" && (v.AccessKey == "" || v.SecretKey == "") {
+		return errors.New("DriverParameters: either IAMRole or literal credentials (AccessKey and SecretKey) must be provided")
 	}
 	if v.IndexPageSize == 0 {
 		v.IndexPageSize = 1000
@@ -55,7 +58,8 @@ func (v *S3Volume) check() error {
 		return errors.New("DriverParameters: RaceWindow must not be negative")
 	}
 
-	region, ok := aws.Regions[v.Region]
+	var ok bool
+	v.region, ok = aws.Regions[v.Region]
 	if v.Endpoint == "" {
 		if !ok {
 			return fmt.Errorf("unrecognized region %+q; try specifying endpoint instead", v.Region)
@@ -64,18 +68,13 @@ func (v *S3Volume) check() error {
 		return fmt.Errorf("refusing to use AWS region name %+q with endpoint %+q; "+
 			"specify empty endpoint or use a different region name", v.Region, v.Endpoint)
 	} else {
-		region = aws.Region{
+		v.region = aws.Region{
 			Name:                 v.Region,
 			S3Endpoint:           v.Endpoint,
 			S3LocationConstraint: v.LocationConstraint,
 		}
 	}
 
-	auth := aws.Auth{
-		AccessKey: v.AccessKey,
-		SecretKey: v.SecretKey,
-	}
-
 	// Zero timeouts mean "wait forever", which is a bad
 	// default. Default to long timeouts instead.
 	if v.ConnectTimeout == 0 {
@@ -85,16 +84,9 @@ func (v *S3Volume) check() error {
 		v.ReadTimeout = s3DefaultReadTimeout
 	}
 
-	client := s3.New(auth, region)
-	if region.EC2Endpoint.Signer == aws.V4Signature {
-		// Currently affects only eu-central-1
-		client.Signature = aws.V4Signature
-	}
-	client.ConnectTimeout = time.Duration(v.ConnectTimeout)
-	client.ReadTimeout = time.Duration(v.ReadTimeout)
 	v.bucket = &s3bucket{
-		Bucket: &s3.Bucket{
-			S3:   client,
+		bucket: &s3.Bucket{
+			S3:   v.newS3Client(),
 			Name: v.Bucket,
 		},
 	}
@@ -102,6 +94,11 @@ func (v *S3Volume) check() error {
 	lbls := prometheus.Labels{"device_id": v.GetDeviceID()}
 	v.bucket.stats.opsCounters, v.bucket.stats.errCounters, v.bucket.stats.ioBytes = v.metrics.getCounterVecsFor(lbls)
 
+	err := v.bootstrapIAMCredentials()
+	if err != nil {
+		return fmt.Errorf("error getting IAM credentials: %s", err)
+	}
+
 	return nil
 }
 
@@ -136,6 +133,9 @@ func s3regions() (okList []string) {
 type S3Volume struct {
 	AccessKey          string
 	SecretKey          string
+	AuthToken          string    // populated automatically when IAMRole is used
+	AuthExpiration     time.Time // populated automatically when IAMRole is used
+	IAMRole            string
 	Endpoint           string
 	Region             string
 	Bucket             string
@@ -151,6 +151,7 @@ type S3Volume struct {
 	logger    logrus.FieldLogger
 	metrics   *volumeMetricsVecs
 	bucket    *s3bucket
+	region    aws.Region
 	startOnce sync.Once
 }
 
@@ -159,6 +160,107 @@ func (v *S3Volume) GetDeviceID() string {
 	return "s3://" + v.Endpoint + "/" + v.Bucket
 }
 
+func (v *S3Volume) bootstrapIAMCredentials() error {
+	if v.IAMRole == "" {
+		return nil
+	}
+	ttl, err := v.updateIAMCredentials()
+	if err != nil {
+		return err
+	}
+	go func() {
+		for {
+			time.Sleep(ttl)
+			ttl, err = v.updateIAMCredentials()
+			if err != nil {
+				v.logger.WithError(err).Warnf("failed to update credentials for IAM role %q", v.IAMRole)
+				ttl = time.Second
+			} else if ttl < time.Second {
+				v.logger.WithField("TTL", ttl).Warnf("received stale credentials for IAM role %q", v.IAMRole)
+				ttl = time.Second
+			}
+		}
+	}()
+	return nil
+}
+
+func (v *S3Volume) newS3Client() *s3.S3 {
+	auth := aws.NewAuth(v.AccessKey, v.SecretKey, v.AuthToken, v.AuthExpiration)
+	client := s3.New(*auth, v.region)
+	if v.region.EC2Endpoint.Signer == aws.V4Signature {
+		// Currently affects only eu-central-1
+		client.Signature = aws.V4Signature
+	}
+	client.ConnectTimeout = time.Duration(v.ConnectTimeout)
+	client.ReadTimeout = time.Duration(v.ReadTimeout)
+	return client
+}
+
+// returned by AWS metadata endpoint .../security-credentials/${rolename}
+type iamCredentials struct {
+	Code            string
+	LastUpdated     time.Time
+	Type            string
+	AccessKeyID     string
+	SecretAccessKey string
+	Token           string
+	Expiration      time.Time
+}
+
+// Returns TTL of updated credentials, i.e., time to sleep until next
+// update.
+func (v *S3Volume) updateIAMCredentials() (time.Duration, error) {
+	ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(time.Minute))
+	defer cancel()
+
+	var url string
+	if strings.Contains(v.IAMRole, "://") {
+		// Configuration provides complete URL (used by tests)
+		url = v.IAMRole
+	} else {
+		// Configuration provides IAM role name and we use the
+		// AWS metadata endpoint
+		url = "http://169.254.169.254/latest/meta-data/iam/security-credentials/" + v.IAMRole
+	}
+	v.logger.WithField("URL", url).Debug("getting credentials")
+	req, err := http.NewRequest("GET", url, nil)
+	if err != nil {
+		return 0, fmt.Errorf("error setting up request %s: %s", url, err)
+	}
+	resp, err := http.DefaultClient.Do(req.WithContext(ctx))
+	if err != nil {
+		return 0, fmt.Errorf("error getting %s: %s", url, err)
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		return 0, fmt.Errorf("error getting %s: HTTP status %s", url, resp.Status)
+	}
+	var cred iamCredentials
+	err = json.NewDecoder(resp.Body).Decode(&cred)
+	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.bucket.SetBucket(&s3.Bucket{
+		S3:   v.newS3Client(),
+		Name: v.Bucket,
+	})
+	// TTL is time from now to expiration, minus 5m.  "We make new
+	// credentials available at least five minutes before the
+	// expiration of the old credentials."  --
+	// https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/iam-roles-for-amazon-ec2.html#instance-metadata-security-credentials
+	// (If that's not true, the returned ttl might be zero or
+	// negative, which the caller can handle.)
+	ttl := cred.Expiration.Sub(time.Now()) - 5*time.Minute
+	v.logger.WithFields(logrus.Fields{
+		"AccessKeyID": cred.AccessKeyID,
+		"LastUpdated": cred.LastUpdated,
+		"Expiration":  cred.Expiration,
+		"TTL":         arvados.Duration(ttl),
+	}).Debug("updated credentials")
+	return ttl, nil
+}
+
 func (v *S3Volume) getReaderWithContext(ctx context.Context, loc string) (rdr io.ReadCloser, err error) {
 	ready := make(chan bool)
 	go func() {
@@ -410,13 +512,13 @@ func (v *S3Volume) Mtime(loc string) (time.Time, error) {
 func (v *S3Volume) IndexTo(prefix string, writer io.Writer) error {
 	// Use a merge sort to find matching sets of X and recent/X.
 	dataL := s3Lister{
-		Bucket:   v.bucket.Bucket,
+		Bucket:   v.bucket.Bucket(),
 		Prefix:   prefix,
 		PageSize: v.IndexPageSize,
 		Stats:    &v.bucket.stats,
 	}
 	recentL := s3Lister{
-		Bucket:   v.bucket.Bucket,
+		Bucket:   v.bucket.Bucket(),
 		Prefix:   "recent/" + prefix,
 		PageSize: v.IndexPageSize,
 		Stats:    &v.bucket.stats,
@@ -531,15 +633,15 @@ func (v *S3Volume) checkRaceWindow(loc string) error {
 // (PutCopy returns 200 OK if the request was received, even if the
 // copy failed).
 func (v *S3Volume) safeCopy(dst, src string) error {
-	resp, err := v.bucket.PutCopy(dst, s3ACL, s3.CopyOptions{
+	resp, err := v.bucket.Bucket().PutCopy(dst, s3ACL, s3.CopyOptions{
 		ContentType:       "application/octet-stream",
 		MetadataDirective: "REPLACE",
-	}, v.bucket.Name+"/"+src)
+	}, v.bucket.Bucket().Name+"/"+src)
 	err = v.translateError(err)
 	if os.IsNotExist(err) {
 		return err
 	} else if err != nil {
-		return fmt.Errorf("PutCopy(%q ← %q): %s", dst, v.bucket.Name+"/"+src, err)
+		return fmt.Errorf("PutCopy(%q ← %q): %s", dst, v.bucket.Bucket().Name+"/"+src, err)
 	}
 	if t, err := time.Parse(time.RFC3339Nano, resp.LastModified); err != nil {
 		return fmt.Errorf("PutCopy succeeded but did not return a timestamp: %q: %s", resp.LastModified, err)
@@ -769,7 +871,7 @@ func (v *S3Volume) EmptyTrash() {
 	}
 
 	trashL := s3Lister{
-		Bucket:   v.bucket.Bucket,
+		Bucket:   v.bucket.Bucket(),
 		Prefix:   "trash/",
 		PageSize: v.IndexPageSize,
 		Stats:    &v.bucket.stats,
@@ -848,14 +950,29 @@ func (lister *s3Lister) pop() (k *s3.Key) {
 	return
 }
 
-// s3bucket wraps s3.bucket and counts I/O and API usage stats.
+// s3bucket wraps s3.bucket and counts I/O and API usage stats. The
+// wrapped bucket can be replaced atomically with SetBucket in order
+// to update credentials.
 type s3bucket struct {
-	*s3.Bucket
-	stats s3bucketStats
+	bucket *s3.Bucket
+	stats  s3bucketStats
+	mu     sync.Mutex
+}
+
+func (b *s3bucket) Bucket() *s3.Bucket {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	return b.bucket
+}
+
+func (b *s3bucket) SetBucket(bucket *s3.Bucket) {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	b.bucket = bucket
 }
 
 func (b *s3bucket) GetReader(path string) (io.ReadCloser, error) {
-	rdr, err := b.Bucket.GetReader(path)
+	rdr, err := b.Bucket().GetReader(path)
 	b.stats.TickOps("get")
 	b.stats.Tick(&b.stats.Ops, &b.stats.GetOps)
 	b.stats.TickErr(err)
@@ -863,7 +980,7 @@ func (b *s3bucket) GetReader(path string) (io.ReadCloser, error) {
 }
 
 func (b *s3bucket) Head(path string, headers map[string][]string) (*http.Response, error) {
-	resp, err := b.Bucket.Head(path, headers)
+	resp, err := b.Bucket().Head(path, headers)
 	b.stats.TickOps("head")
 	b.stats.Tick(&b.stats.Ops, &b.stats.HeadOps)
 	b.stats.TickErr(err)
@@ -882,7 +999,7 @@ func (b *s3bucket) PutReader(path string, r io.Reader, length int64, contType st
 	} else {
 		r = NewCountingReader(r, b.stats.TickOutBytes)
 	}
-	err := b.Bucket.PutReader(path, r, length, contType, perm, options)
+	err := b.Bucket().PutReader(path, r, length, contType, perm, options)
 	b.stats.TickOps("put")
 	b.stats.Tick(&b.stats.Ops, &b.stats.PutOps)
 	b.stats.TickErr(err)
@@ -890,7 +1007,7 @@ func (b *s3bucket) PutReader(path string, r io.Reader, length int64, contType st
 }
 
 func (b *s3bucket) Del(path string) error {
-	err := b.Bucket.Del(path)
+	err := b.Bucket().Del(path)
 	b.stats.TickOps("delete")
 	b.stats.Tick(&b.stats.Ops, &b.stats.DelOps)
 	b.stats.TickErr(err)
diff --git a/services/keepstore/s3_volume_test.go b/services/keepstore/s3_volume_test.go
index 5c639d629..49ea24aa0 100644
--- a/services/keepstore/s3_volume_test.go
+++ b/services/keepstore/s3_volume_test.go
@@ -10,6 +10,7 @@ import (
 	"crypto/md5"
 	"encoding/json"
 	"fmt"
+	"io"
 	"log"
 	"net/http"
 	"net/http/httptest"
@@ -45,6 +46,7 @@ var _ = check.Suite(&StubbedS3Suite{})
 
 type StubbedS3Suite struct {
 	s3server *httptest.Server
+	metadata *httptest.Server
 	cluster  *arvados.Cluster
 	handler  *handler
 	volumes  []*TestableS3Volume
@@ -52,6 +54,7 @@ type StubbedS3Suite struct {
 
 func (s *StubbedS3Suite) SetUpTest(c *check.C) {
 	s.s3server = nil
+	s.metadata = nil
 	s.cluster = testCluster(c)
 	s.cluster.Volumes = map[string]arvados.Volume{
 		"zzzzz-nyw5e-000000000000000": {Driver: "S3"},
@@ -99,6 +102,40 @@ func (s *StubbedS3Suite) TestIndex(c *check.C) {
 	}
 }
 
+func (s *StubbedS3Suite) TestIAMRoleCredentials(c *check.C) {
+	s.metadata = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		upd := time.Now().UTC().Add(-time.Hour).Format(time.RFC3339)
+		exp := time.Now().UTC().Add(time.Hour).Format(time.RFC3339)
+		// Literal example from
+		// https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/iam-roles-for-amazon-ec2.html#instance-metadata-security-credentials
+		// but with updated timestamps
+		io.WriteString(w, `{"Code":"Success","LastUpdated":"`+upd+`","Type":"AWS-HMAC","AccessKeyId":"ASIAIOSFODNN7EXAMPLE","SecretAccessKey":"wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY","Token":"token","Expiration":"`+exp+`"}`)
+	}))
+	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.bucket.bucket.S3.Auth.AccessKey, check.Equals, "ASIAIOSFODNN7EXAMPLE")
+	c.Check(v.bucket.bucket.S3.Auth.SecretKey, check.Equals, "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY")
+
+	s.metadata = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+	}))
+	deadv := &S3Volume{
+		IAMRole:  s.metadata.URL + "/fake-metadata/test-role",
+		Endpoint: "http://localhost:12345",
+		Region:   "test-region-1",
+		Bucket:   "test-bucket-name",
+		cluster:  s.cluster,
+		logger:   ctxlog.TestLogger(c),
+		metrics:  newVolumeMetricsVecs(prometheus.NewRegistry()),
+	}
+	err := deadv.check()
+	c.Check(err, check.ErrorMatches, `.*/fake-metadata/test-role.*`)
+	c.Check(err, check.ErrorMatches, `.*404.*`)
+}
+
 func (s *StubbedS3Suite) TestStats(c *check.C) {
 	v := s.newTestableVolume(c, s.cluster, arvados.Volume{Replication: 2}, newVolumeMetricsVecs(prometheus.NewRegistry()), 5*time.Minute)
 	stats := func() string {
@@ -229,7 +266,7 @@ func (s *StubbedS3Suite) TestBackendStates(c *check.C) {
 			return
 		}
 		v.serverClock.now = &t
-		v.bucket.Put(key, data, "application/octet-stream", s3ACL, s3.Options{})
+		v.bucket.Bucket().Put(key, data, "application/octet-stream", s3ACL, s3.Options{})
 	}
 
 	t0 := time.Now()
@@ -425,10 +462,16 @@ func (s *StubbedS3Suite) newTestableVolume(c *check.C, cluster *arvados.Cluster,
 		endpoint = s.s3server.URL
 	}
 
+	iamRole, accessKey, secretKey := "", "xxx", "xxx"
+	if s.metadata != nil {
+		iamRole, accessKey, secretKey = s.metadata.URL+"/fake-metadata/test-role", "", ""
+	}
+
 	v := &TestableS3Volume{
 		S3Volume: &S3Volume{
-			AccessKey:          "xxx",
-			SecretKey:          "xxx",
+			AccessKey:          accessKey,
+			SecretKey:          secretKey,
+			IAMRole:            iamRole,
 			Bucket:             TestBucketName,
 			Endpoint:           endpoint,
 			Region:             "test-region-1",
@@ -445,7 +488,7 @@ func (s *StubbedS3Suite) newTestableVolume(c *check.C, cluster *arvados.Cluster,
 		serverClock: clock,
 	}
 	c.Assert(v.S3Volume.check(), check.IsNil)
-	c.Assert(v.bucket.PutBucket(s3.ACL("private")), check.IsNil)
+	c.Assert(v.bucket.Bucket().PutBucket(s3.ACL("private")), check.IsNil)
 	// We couldn't set RaceWindow until now because check()
 	// rejects negative values.
 	v.S3Volume.RaceWindow = arvados.Duration(raceWindow)
@@ -454,11 +497,11 @@ func (s *StubbedS3Suite) newTestableVolume(c *check.C, cluster *arvados.Cluster,
 
 // PutRaw skips the ContentMD5 test
 func (v *TestableS3Volume) PutRaw(loc string, block []byte) {
-	err := v.bucket.Put(loc, block, "application/octet-stream", s3ACL, s3.Options{})
+	err := v.bucket.Bucket().Put(loc, block, "application/octet-stream", s3ACL, s3.Options{})
 	if err != nil {
 		log.Printf("PutRaw: %s: %+v", loc, err)
 	}
-	err = v.bucket.Put("recent/"+loc, nil, "application/octet-stream", s3ACL, s3.Options{})
+	err = v.bucket.Bucket().Put("recent/"+loc, nil, "application/octet-stream", s3ACL, s3.Options{})
 	if err != nil {
 		log.Printf("PutRaw: recent/%s: %+v", loc, err)
 	}
@@ -469,7 +512,7 @@ func (v *TestableS3Volume) PutRaw(loc string, block []byte) {
 // while we do this.
 func (v *TestableS3Volume) TouchWithDate(locator string, lastPut time.Time) {
 	v.serverClock.now = &lastPut
-	err := v.bucket.Put("recent/"+locator, nil, "application/octet-stream", s3ACL, s3.Options{})
+	err := v.bucket.Bucket().Put("recent/"+locator, nil, "application/octet-stream", s3ACL, s3.Options{})
 	if err != nil {
 		panic(err)
 	}

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list