[arvados] created: 2.7.0-6623-g3acd4fb0ab

git repository hosting git at public.arvados.org
Fri May 24 13:11:18 UTC 2024


        at  3acd4fb0ab283a6788b81167e3f196d51230e036 (commit)


commit 3acd4fb0ab283a6788b81167e3f196d51230e036
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 23b701dcd1..9c297ad6ab 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. Legacy container logging system has been removed
 
 The following configuration keys are no longer supported. Remove them from your @/etc/arvados/config.yml@ file to avoid warnings when services start up.
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 e32c168e2d..f84869d7fb 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -1685,7 +1685,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 8572613e94..7b63ca0ed7 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 0024841988c467214b05416785714c5e9b981676
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 9aa54df377a9bc86d98f9dfe5f767551ecd4977e
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