[arvados] updated: 2.7.0-6573-g88bc8ff22d

git repository hosting git at public.arvados.org
Thu May 23 20:09:55 UTC 2024


Summary of changes:
 doc/admin/upgrading.html.textile.liquid            |  4 +++
 ...configure-s3-object-storage.html.textile.liquid | 13 +++-------
 .../install-dispatch-cloud.html.textile.liquid     |  6 ++---
 lib/cloud/ec2/ec2.go                               | 30 +++++++++++-----------
 lib/config/config.default.yml                      |  1 -
 sdk/go/arvados/config.go                           |  1 -
 services/keepstore/s3_volume.go                    |  2 --
 services/keepstore/s3_volume_test.go               |  6 ++---
 8 files changed, 26 insertions(+), 37 deletions(-)

       via  88bc8ff22d3a23c7f2fbbb51049d0256e3030cd6 (commit)
       via  0913ec5d061ee81bb98f4364fbc191a161189abf (commit)
       via  15e39bdae0eb5b642de2f44ec85aec954fc89e60 (commit)
      from  e1e4cf1e604f0d2bbf4f959123edbf0b9d3474df (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 88bc8ff22d3a23c7f2fbbb51049d0256e3030cd6
Author: Tom Clegg <tom at curii.com>
Date:   Thu May 23 16:09:41 2024 -0400

    21705: Remove IAMRole config and update docs.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/doc/admin/upgrading.html.textile.liquid b/doc/admin/upgrading.html.textile.liquid
index 320f188812..9d8fd13e80 100644
--- a/doc/admin/upgrading.html.textile.liquid
+++ b/doc/admin/upgrading.html.textile.liquid
@@ -32,6 +32,10 @@ h2(#main). development main
 
 "previous: Upgrading to 2.7.1":#v2_7_1
 
+h3. S3 volume IAMRole configuration entry has been removed
+
+The @IAMRole@ configuration entry for S3 volumes has been removed. You should remove it from your @/etc/arvados/config.yml@ file to avoid warnings when services start up. As before, if @AccessKeyID@ and @SecretAccessKey@ are blank, keepstore will retrieve IAM role credentials from instance metadata. Previously, documentation indicated that keepstore would refuse to use the IAM credentials if @IAMRole@ was specified and did not match the instance metadata, but that check has not been working for some time.
+
 h3. Virtual environments inside distribution Python packages have moved
 
 The distribution packages that we publish for Python packages include an entire virtualenv with all required libraries. In Arvados 3.0 these virtualenvs have moved from @/usr/share/python3/dist/PACKAGE_NAME@ to @/usr/lib/PACKAGE_NAME@ to prevent conflicts with distribution packages and better conform to filesystem standards.
diff --git a/doc/install/configure-s3-object-storage.html.textile.liquid b/doc/install/configure-s3-object-storage.html.textile.liquid
index 31ad994f0b..1aec14984e 100644
--- a/doc/install/configure-s3-object-storage.html.textile.liquid
+++ b/doc/install/configure-s3-object-storage.html.textile.liquid
@@ -38,16 +38,9 @@ h2(#example). Configuration example
           # Bucket name.
           Bucket: <span class="userinput">example-bucket-name</span>
 
-          # IAM role name to use when retrieving credentials from
-          # instance metadata. It can be omitted, in which case the
-          # role name itself will be retrieved from instance metadata
-          # -- but setting it explicitly may protect you from using
-          # the wrong credentials in the event of an
-          # installation/configuration error.
-          IAMRole: <span class="userinput">""</span>
-
-          # If you are not using an IAM role for authentication,
-          # specify access credentials here instead.
+          # Optionally, you can specify S3 access credentials here.
+          # If these are left blank, IAM role credentials will be
+          # retrieved from instance metadata (IMDSv2).
           AccessKeyID: <span class="userinput">""</span>
           SecretAccessKey: <span class="userinput">""</span>
 
diff --git a/doc/install/crunch2-cloud/install-dispatch-cloud.html.textile.liquid b/doc/install/crunch2-cloud/install-dispatch-cloud.html.textile.liquid
index 579ec6e1b3..9eec3fdb79 100644
--- a/doc/install/crunch2-cloud/install-dispatch-cloud.html.textile.liquid
+++ b/doc/install/crunch2-cloud/install-dispatch-cloud.html.textile.liquid
@@ -148,11 +148,9 @@ When @Containers.LocalKeepBlobBuffersPerVCPU@ is non-zero, the compute node will
 
 If the AWS credentials for S3 access are configured in @config.yml@ (i.e. @Volumes.DriverParameters.AccessKeyID@ and @Volumes.DriverParameters.SecretAccessKey@), these credentials will be made available to the local Keepstore on the compute node to access S3 directly and no further configuration is necessary.
 
-Alternatively, if an IAM role is configured in @config.yml@ (i.e. @Volumes.DriverParameters.IAMRole@), the name of an instance profile that corresponds to this role ("often identical to the name of the IAM role":https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/iam-roles-for-amazon-ec2.html#ec2-instance-profile) must be configured in the @CloudVMs.DriverParameters.IAMInstanceProfile@ parameter.
+If @config.yml@ does not have @Volumes.DriverParameters.AccessKeyID@ and @Volumes.DriverParameters.SecretAccessKey@ defined, Keepstore uses instance metadata to retrieve IAM role credentials. The @CloudVMs.DriverParameters.IAMInstanceProfile@ parameter must be configured with the name of a profile whose IAM role has permission to access the S3 bucket(s). With this setup, @arvados-dispatch-cloud@ will attach the IAM role to the compute node as it is created. The instance profile name is "often identical to the name of the IAM role":https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/iam-roles-for-amazon-ec2.html#ec2-instance-profile.
 
-*If you are also using EBS Autoscale feature, the role in IAMInstanceProfile must have both ec2 and s3 permissions.*
-
-Finally, if @config.yml@ does not have @Volumes.DriverParameters.AccessKeyID@, @Volumes.DriverParameters.SecretAccessKey@ or @Volumes.DriverParameters.IAMRole@ defined, Keepstore uses the IAM role attached to the node, whatever it may be called. The @CloudVMs.DriverParameters.IAMInstanceProfile@ parameter must then still be configured with the name of a profile whose IAM role has permission to access the S3 bucket(s). That way, @arvados-dispatch-cloud@ can attach the IAM role to the compute node as it is created.
+*If you are also using EBS Autoscale feature, the role in @IAMInstanceProfile@ must have both ec2 and s3 permissions.*
 
 h3. Minimal configuration example for Amazon EC2
 
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index b045553b23..2438eb430f 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -1724,7 +1724,6 @@ Clusters:
         DriverParameters:
           # for s3 driver -- see
           # https://doc.arvados.org/install/configure-s3-object-storage.html
-          IAMRole: aaaaa
           AccessKeyID: aaaaa
           SecretAccessKey: aaaaa
           Endpoint: ""
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 6725611fd0..4751b37c43 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -309,7 +309,6 @@ type Volume struct {
 }
 
 type S3VolumeDriverParameters struct {
-	IAMRole            string
 	AccessKeyID        string
 	SecretAccessKey    string
 	Endpoint           string
diff --git a/services/keepstore/s3_volume.go b/services/keepstore/s3_volume.go
index 49b94a72a1..dd4666039d 100644
--- a/services/keepstore/s3_volume.go
+++ b/services/keepstore/s3_volume.go
@@ -63,8 +63,6 @@ var (
 // s3Volume implements Volume using an S3 bucket.
 type s3Volume struct {
 	arvados.S3VolumeDriverParameters
-	AuthToken      string    // populated automatically when IAMRole is used
-	AuthExpiration time.Time // populated automatically when IAMRole is used
 
 	cluster    *arvados.Cluster
 	volume     arvados.Volume
diff --git a/services/keepstore/s3_volume_test.go b/services/keepstore/s3_volume_test.go
index 50010b3bef..df9fe72683 100644
--- a/services/keepstore/s3_volume_test.go
+++ b/services/keepstore/s3_volume_test.go
@@ -190,7 +190,6 @@ func (s *stubbedS3Suite) TestIAMRoleCredentials(c *check.C) {
 
 	v := &s3Volume{
 		S3VolumeDriverParameters: arvados.S3VolumeDriverParameters{
-			IAMRole:  s.metadata.URL + "/latest/api/token",
 			Endpoint: stub.URL,
 			Region:   "test-region-1",
 			Bucket:   "test-bucket-name",
@@ -602,9 +601,9 @@ func (s *stubbedS3Suite) newTestableVolume(c *check.C, params newVolumeParams, r
 	endpoint := s.s3server.URL
 	bucketName := fmt.Sprintf("testbucket%d", testBucketSerial.Add(1))
 
-	var metadataURL, iamRole, accessKey, secretKey string
+	var metadataURL, accessKey, secretKey string
 	if s.metadata != nil {
-		metadataURL, iamRole = s.metadata.URL, s.metadata.URL+"/fake-metadata/test-role"
+		metadataURL = s.metadata.URL
 	} else {
 		accessKey, secretKey = "xxx", "xxx"
 	}
@@ -612,7 +611,6 @@ func (s *stubbedS3Suite) newTestableVolume(c *check.C, params newVolumeParams, r
 	v := &testableS3Volume{
 		s3Volume: &s3Volume{
 			S3VolumeDriverParameters: arvados.S3VolumeDriverParameters{
-				IAMRole:            iamRole,
 				AccessKeyID:        accessKey,
 				SecretAccessKey:    secretKey,
 				Bucket:             bucketName,

commit 0913ec5d061ee81bb98f4364fbc191a161189abf
Author: Tom Clegg <tom at curii.com>
Date:   Wed May 15 12:08:18 2024 -0400

    21705: Use context.Background() for aws sdk calls.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/cloud/ec2/ec2.go b/lib/cloud/ec2/ec2.go
index 730309dd8a..a37522345d 100644
--- a/lib/cloud/ec2/ec2.go
+++ b/lib/cloud/ec2/ec2.go
@@ -129,7 +129,7 @@ func newEC2InstanceSet(confRaw json.RawMessage, instanceSetID cloud.InstanceSetI
 	if err != nil {
 		return nil, err
 	}
-	awsConfig, err := config.LoadDefaultConfig(context.TODO(),
+	awsConfig, err := config.LoadDefaultConfig(context.Background(),
 		config.WithRegion(instanceSet.ec2config.Region),
 		config.WithCredentialsCacheOptions(func(o *aws.CredentialsCacheOptions) {
 			o.ExpiryWindow = 5 * time.Minute
@@ -318,7 +318,7 @@ func (instanceSet *ec2InstanceSet) Create(
 			rii.NetworkInterfaces[0].SubnetId = aws.String(trySubnet)
 		}
 		var err error
-		rsv, err = instanceSet.client.RunInstances(context.TODO(), &rii)
+		rsv, err = instanceSet.client.RunInstances(context.Background(), &rii)
 		instanceSet.mInstanceStarts.WithLabelValues(trySubnet, boolLabelValue[err == nil]).Add(1)
 		if !isErrorCapacity(errToReturn) || isErrorCapacity(err) {
 			// We want to return the last capacity error,
@@ -364,7 +364,7 @@ func (instanceSet *ec2InstanceSet) getKeyName(publicKey ssh.PublicKey) (string,
 	if keyname, ok := instanceSet.keys[md5keyFingerprint]; ok {
 		return keyname, nil
 	}
-	keyout, err := instanceSet.client.DescribeKeyPairs(context.TODO(), &ec2.DescribeKeyPairsInput{
+	keyout, err := instanceSet.client.DescribeKeyPairs(context.Background(), &ec2.DescribeKeyPairsInput{
 		Filters: []types.Filter{{
 			Name:   aws.String("fingerprint"),
 			Values: []string{md5keyFingerprint, sha1keyFingerprint},
@@ -377,7 +377,7 @@ func (instanceSet *ec2InstanceSet) getKeyName(publicKey ssh.PublicKey) (string,
 		return *(keyout.KeyPairs[0].KeyName), nil
 	}
 	keyname := "arvados-dispatch-keypair-" + md5keyFingerprint
-	_, err = instanceSet.client.ImportKeyPair(context.TODO(), &ec2.ImportKeyPairInput{
+	_, err = instanceSet.client.ImportKeyPair(context.Background(), &ec2.ImportKeyPairInput{
 		KeyName:           &keyname,
 		PublicKeyMaterial: ssh.MarshalAuthorizedKey(publicKey),
 	})
@@ -399,7 +399,7 @@ func (instanceSet *ec2InstanceSet) Instances(tags cloud.InstanceTags) (instances
 	needAZs := false
 	dii := &ec2.DescribeInstancesInput{Filters: filters}
 	for {
-		dio, err := instanceSet.client.DescribeInstances(context.TODO(), dii)
+		dio, err := instanceSet.client.DescribeInstances(context.Background(), dii)
 		err = wrapError(err, &instanceSet.throttleDelayInstances)
 		if err != nil {
 			return nil, err
@@ -430,7 +430,7 @@ func (instanceSet *ec2InstanceSet) Instances(tags cloud.InstanceTags) (instances
 		az := map[string]string{}
 		disi := &ec2.DescribeInstanceStatusInput{IncludeAllInstances: aws.Bool(true)}
 		for {
-			page, err := instanceSet.client.DescribeInstanceStatus(context.TODO(), disi)
+			page, err := instanceSet.client.DescribeInstanceStatus(context.Background(), disi)
 			if err != nil {
 				instanceSet.logger.WithError(err).Warn("error getting instance statuses")
 				break
@@ -531,7 +531,7 @@ func (instanceSet *ec2InstanceSet) updateSpotPrices(instances []cloud.Instance)
 		},
 	}
 	for {
-		page, err := instanceSet.client.DescribeSpotPriceHistory(context.TODO(), dsphi)
+		page, err := instanceSet.client.DescribeSpotPriceHistory(context.Background(), dsphi)
 		if err != nil {
 			instanceSet.logger.WithError(err).Warn("error retrieving spot instance prices")
 			break
@@ -605,7 +605,7 @@ func (inst *ec2Instance) SetTags(newTags cloud.InstanceTags) error {
 		})
 	}
 
-	_, err := inst.provider.client.CreateTags(context.TODO(), &ec2.CreateTagsInput{
+	_, err := inst.provider.client.CreateTags(context.Background(), &ec2.CreateTagsInput{
 		Resources: []string{*inst.instance.InstanceId},
 		Tags:      ec2tags,
 	})
@@ -624,7 +624,7 @@ func (inst *ec2Instance) Tags() cloud.InstanceTags {
 }
 
 func (inst *ec2Instance) Destroy() error {
-	_, err := inst.provider.client.TerminateInstances(context.TODO(), &ec2.TerminateInstancesInput{
+	_, err := inst.provider.client.TerminateInstances(context.Background(), &ec2.TerminateInstancesInput{
 		InstanceIds: []string{*inst.instance.InstanceId},
 	})
 	return err

commit 15e39bdae0eb5b642de2f44ec85aec954fc89e60
Author: Tom Clegg <tom at curii.com>
Date:   Wed May 15 12:06:31 2024 -0400

    21705: Fixup error wrapping and logging.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/cloud/ec2/ec2.go b/lib/cloud/ec2/ec2.go
index bbc898ab58..730309dd8a 100644
--- a/lib/cloud/ec2/ec2.go
+++ b/lib/cloud/ec2/ec2.go
@@ -204,7 +204,7 @@ func awsKeyFingerprint(pk ssh.PublicKey) (md5fp string, sha1fp string, err error
 		N    *big.Int
 	}
 	if err := ssh.Unmarshal(pk.Marshal(), &rsaPub); err != nil {
-		return "", "", fmt.Errorf("agent: Unmarshal failed to parse public key: %v", err)
+		return "", "", fmt.Errorf("Unmarshal failed to parse public key: %w", err)
 	}
 	rsaPk := rsa.PublicKey{
 		E: int(rsaPub.E.Int64()),
@@ -359,7 +359,7 @@ func (instanceSet *ec2InstanceSet) getKeyName(publicKey ssh.PublicKey) (string,
 	defer instanceSet.keysMtx.Unlock()
 	md5keyFingerprint, sha1keyFingerprint, err := awsKeyFingerprint(publicKey)
 	if err != nil {
-		return "", fmt.Errorf("Could not make key fingerprint: %v", err)
+		return "", fmt.Errorf("Could not make key fingerprint: %w", err)
 	}
 	if keyname, ok := instanceSet.keys[md5keyFingerprint]; ok {
 		return keyname, nil
@@ -371,7 +371,7 @@ func (instanceSet *ec2InstanceSet) getKeyName(publicKey ssh.PublicKey) (string,
 		}},
 	})
 	if err != nil {
-		return "", fmt.Errorf("Could not search for keypair: %v", err)
+		return "", fmt.Errorf("Could not search for keypair: %w", err)
 	}
 	if len(keyout.KeyPairs) > 0 {
 		return *(keyout.KeyPairs[0].KeyName), nil
@@ -382,7 +382,7 @@ func (instanceSet *ec2InstanceSet) getKeyName(publicKey ssh.PublicKey) (string,
 		PublicKeyMaterial: ssh.MarshalAuthorizedKey(publicKey),
 	})
 	if err != nil {
-		return "", fmt.Errorf("Could not import keypair: %v", err)
+		return "", fmt.Errorf("Could not import keypair: %w", err)
 	}
 	instanceSet.keys[md5keyFingerprint] = keyname
 	return keyname, nil
@@ -432,7 +432,7 @@ func (instanceSet *ec2InstanceSet) Instances(tags cloud.InstanceTags) (instances
 		for {
 			page, err := instanceSet.client.DescribeInstanceStatus(context.TODO(), disi)
 			if err != nil {
-				instanceSet.logger.Warnf("error getting instance statuses: %s", err)
+				instanceSet.logger.WithError(err).Warn("error getting instance statuses")
 				break
 			}
 			for _, ent := range page.InstanceStatuses {
@@ -533,7 +533,7 @@ func (instanceSet *ec2InstanceSet) updateSpotPrices(instances []cloud.Instance)
 	for {
 		page, err := instanceSet.client.DescribeSpotPriceHistory(context.TODO(), dsphi)
 		if err != nil {
-			instanceSet.logger.Warnf("error retrieving spot instance prices: %s", err)
+			instanceSet.logger.WithError(err).Warn("error retrieving spot instance prices")
 			break
 		}
 		for _, ent := range page.SpotPriceHistory {

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list