From a1b3d73e42f29bb9f70bfbcf36e890aeb4167622 Mon Sep 17 00:00:00 2001 From: Michael Smithhisler Date: Thu, 19 Dec 2024 11:02:12 -0500 Subject: [PATCH 1/5] deps: upgrade aws-sdk-go from v1 to v2 --- client/fingerprint/env_aws.go | 153 ++++++++++++++++++--------------- e2e/remotetasks/remotetasks.go | 48 ++++++----- go.mod | 16 +++- go.sum | 28 ++++++ 4 files changed, 156 insertions(+), 89 deletions(-) diff --git a/client/fingerprint/env_aws.go b/client/fingerprint/env_aws.go index 4041a6b0950..d0d6d86c08e 100644 --- a/client/fingerprint/env_aws.go +++ b/client/fingerprint/env_aws.go @@ -4,19 +4,20 @@ package fingerprint import ( + "context" + "errors" "fmt" + "io" "net/http" - "net/url" - "os" "regexp" "strings" "time" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" - "github.com/aws/aws-sdk-go/aws/ec2metadata" - "github.com/aws/aws-sdk-go/aws/session" - cleanhttp "github.com/hashicorp/go-cleanhttp" + "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/feature/ec2/imds" + smithyHttp "github.com/aws/smithy-go/transport/http" + + "github.com/hashicorp/go-cleanhttp" log "github.com/hashicorp/go-hclog" "github.com/hashicorp/nomad/nomad/structs" ) @@ -52,17 +53,13 @@ var ec2NetSpeedTable = map[*regexp.Regexp]int{ type EnvAWSFingerprint struct { StaticFingerprinter - // endpoint for EC2 metadata as expected by AWS SDK - endpoint string - logger log.Logger } // NewEnvAWSFingerprint is used to create a fingerprint from AWS metadata func NewEnvAWSFingerprint(logger log.Logger) Fingerprint { f := &EnvAWSFingerprint{ - logger: logger.Named("env_aws"), - endpoint: strings.TrimSuffix(os.Getenv("AWS_ENV_URL"), "/meta-data/"), + logger: logger.Named("env_aws"), } return f } @@ -77,12 +74,16 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F timeout = 1 * time.Millisecond } - ec2meta, err := ec2MetaClient(f.endpoint, timeout) + ctx, cancel := context.WithTimeout(context.TODO(), timeout) + defer cancel() + + imdsClient, err := imdsClient(ctx) if err != nil { - return fmt.Errorf("failed to setup ec2Metadata client: %v", err) + return fmt.Errorf("failed to setup IMDS client: %v", err) } - if !isAWS(ec2meta) { + if !isAWS(ctx, imdsClient) { + f.logger.Debug("error querying AWS IDMS URL, skipping") return nil } @@ -104,24 +105,24 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F } for k, unique := range keys { - resp, err := ec2meta.GetMetadata(k) - v := strings.TrimSpace(resp) - if v == "" { - f.logger.Debug("read an empty value", "attribute", k) - continue - } else if awsErr, ok := err.(awserr.RequestFailure); ok { - f.logger.Debug("could not read attribute value", "attribute", k, "error", awsErr) + resp, err := imdsClient.GetMetadata(ctx, &imds.GetMetadataInput{ + Path: k, + }) + if err := f.handleImdsError(err, k); err != nil { + return err + } + if resp == nil { continue - } else if awsErr, ok := err.(awserr.Error); ok { - // if it's a URL error, assume we're not in an AWS environment - // TODO: better way to detect AWS? Check xen virtualization? - if _, ok := awsErr.OrigErr().(*url.Error); ok { - return nil - } + } - // not sure what other errors it would return + bytes, err := io.ReadAll(resp.Content) + if err != nil { return err } + v := string(bytes) + if v == "" { + f.logger.Debug("read an empty value", "attribute", k) + } // assume we want blank entries key := "platform.aws." + strings.ReplaceAll(k, "/", ".") @@ -144,7 +145,7 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F Device: "eth0", IP: val, CIDR: val + "/32", - MBits: f.throughput(request, ec2meta, val), + MBits: f.throughput(request, imdsClient, val), }, } } @@ -152,24 +153,24 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F // copy over IPv6 network specific information if val, ok := response.Attributes["unique.platform.aws.mac"]; ok && val != "" { k := "network/interfaces/macs/" + val + "/ipv6s" - addrsStr, err := ec2meta.GetMetadata(k) - addrsStr = strings.TrimSpace(addrsStr) - if addrsStr == "" { - f.logger.Debug("read an empty value", "attribute", k) - } else if awsErr, ok := err.(awserr.RequestFailure); ok { - f.logger.Debug("could not read attribute value", "attribute", k, "error", awsErr) - } else if awsErr, ok := err.(awserr.Error); ok { - // if it's a URL error, assume we're not in an AWS environment - // TODO: better way to detect AWS? Check xen virtualization? - if _, ok := awsErr.OrigErr().(*url.Error); ok { - return nil - } - - // not sure what other errors it would return + resp, err := imdsClient.GetMetadata(ctx, &imds.GetMetadataInput{ + Path: k, + }) + if err := f.handleImdsError(err, k); err != nil { return err - } else { - addrs := strings.SplitN(addrsStr, "\n", 2) - response.AddAttribute("unique.platform.aws.public-ipv6", addrs[0]) + } + if resp != nil { + addrBytes, err := io.ReadAll(resp.Content) + if err != nil { + return err + } + addrsStr := string(addrBytes) + if addrsStr == "" { + f.logger.Debug("read an empty value", "attribute", k) + } else { + addrs := strings.SplitN(addrsStr, "\n", 2) + response.AddAttribute("unique.platform.aws.public-ipv6", addrs[0]) + } } } @@ -184,21 +185,41 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F return nil } -func (f *EnvAWSFingerprint) instanceType(ec2meta *ec2metadata.EC2Metadata) (string, error) { - response, err := ec2meta.GetMetadata("instance-type") +// See https://aws.github.io/aws-sdk-go-v2/docs/handling-errors for +// recommended error handling with aws-sdk-go-v2. +// See also: https://github.com/aws/aws-sdk-go-v2/issues/1306 +func (f *EnvAWSFingerprint) handleImdsError(err error, attr string) error { + var apiErr *smithyHttp.ResponseError + if errors.As(err, &apiErr) { + // In the event of a request error while fetching attributes, just log and return nil. + // This will happen if attributes do not exist for this instance (ex. ipv6, public-ipv4s). + f.logger.Debug("could not read attribute value", "attribute", attr, "error", err) + return nil + } + return err +} + +func (f *EnvAWSFingerprint) instanceType(client *imds.Client) (string, error) { + output, err := client.GetMetadata(context.TODO(), &imds.GetMetadataInput{ + Path: "instance-type", + }) + if err != nil { + return "", err + } + content, err := io.ReadAll(output.Content) if err != nil { return "", err } - return strings.TrimSpace(response), nil + return string(content), nil } -func (f *EnvAWSFingerprint) throughput(request *FingerprintRequest, ec2meta *ec2metadata.EC2Metadata, ip string) int { +func (f *EnvAWSFingerprint) throughput(request *FingerprintRequest, client *imds.Client, ip string) int { throughput := request.Config.NetworkSpeed if throughput != 0 { return throughput } - throughput = f.linkSpeed(ec2meta) + throughput = f.linkSpeed(client) if throughput != 0 { return throughput } @@ -215,8 +236,8 @@ func (f *EnvAWSFingerprint) throughput(request *FingerprintRequest, ec2meta *ec2 } // EnvAWSFingerprint uses lookup table to approximate network speeds -func (f *EnvAWSFingerprint) linkSpeed(ec2meta *ec2metadata.EC2Metadata) int { - instanceType, err := f.instanceType(ec2meta) +func (f *EnvAWSFingerprint) linkSpeed(client *imds.Client) int { + instanceType, err := f.instanceType(client) if err != nil { f.logger.Error("error reading instance-type", "error", err) return 0 @@ -233,26 +254,22 @@ func (f *EnvAWSFingerprint) linkSpeed(ec2meta *ec2metadata.EC2Metadata) int { return netSpeed } -func ec2MetaClient(endpoint string, timeout time.Duration) (*ec2metadata.EC2Metadata, error) { +func imdsClient(ctx context.Context) (*imds.Client, error) { client := &http.Client{ - Timeout: timeout, Transport: cleanhttp.DefaultTransport(), } - - c := aws.NewConfig().WithHTTPClient(client).WithMaxRetries(0) - if endpoint != "" { - c = c.WithEndpoint(endpoint) - } - - sess, err := session.NewSession(c) + cfg, err := config.LoadDefaultConfig(ctx, config.WithHTTPClient(client), config.WithRetryMaxAttempts(0)) if err != nil { return nil, err } - return ec2metadata.New(sess, c), nil + return imds.NewFromConfig(cfg), nil } -func isAWS(ec2meta *ec2metadata.EC2Metadata) bool { - v, err := ec2meta.GetMetadata("ami-id") - v = strings.TrimSpace(v) - return err == nil && v != "" +// isAWS validates the client can reach IMDS. Fetching an ami-id must +// complete error free to be recognized as running on AWS EC2 +func isAWS(ctx context.Context, client *imds.Client) bool { + _, err := client.GetMetadata(ctx, &imds.GetMetadataInput{ + Path: "ami-id", + }) + return err == nil } diff --git a/e2e/remotetasks/remotetasks.go b/e2e/remotetasks/remotetasks.go index e46d10f1167..a71eafcb7af 100644 --- a/e2e/remotetasks/remotetasks.go +++ b/e2e/remotetasks/remotetasks.go @@ -4,14 +4,15 @@ package remotetasks import ( + "context" "fmt" "os" "testing" "time" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/session" - "github.com/aws/aws-sdk-go/service/ecs" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/service/ecs" "github.com/hashicorp/nomad/api" "github.com/hashicorp/nomad/e2e/e2eutil" "github.com/hashicorp/nomad/e2e/framework" @@ -78,7 +79,9 @@ func (tc *RemoteTasksTest) AfterEach(f *framework.F) { func (tc *RemoteTasksTest) TestECSJob(f *framework.F) { t := f.T() - ecsClient := ecsOrSkip(t, tc.Nomad()) + ctx := context.Background() + + ecsClient := ecsOrSkip(ctx, t, tc.Nomad()) jobID := "ecsjob-" + uuid.Generate()[0:8] tc.jobIDs = append(tc.jobIDs, jobID) @@ -92,7 +95,7 @@ func (tc *RemoteTasksTest) TestECSJob(f *framework.F) { arn := arnForAlloc(t, tc.Nomad().Allocations(), allocID) // Use ARN to lookup status of ECS task in AWS - ensureECSRunning(t, ecsClient, arn) + ensureECSRunning(ctx, t, ecsClient, arn) t.Logf("Task %s is running!", arn) @@ -102,10 +105,10 @@ func (tc *RemoteTasksTest) TestECSJob(f *framework.F) { // Ensure it is stopped in ECS input := ecs.DescribeTasksInput{ Cluster: aws.String("nomad-rtd-e2e"), - Tasks: []*string{aws.String(arn)}, + Tasks: []string{arn}, } testutil.WaitForResult(func() (bool, error) { - resp, err := ecsClient.DescribeTasks(&input) + resp, err := ecsClient.DescribeTasks(ctx, &input) if err != nil { return false, err } @@ -121,7 +124,9 @@ func (tc *RemoteTasksTest) TestECSJob(f *framework.F) { func (tc *RemoteTasksTest) TestECSDrain(f *framework.F) { t := f.T() - ecsClient := ecsOrSkip(t, tc.Nomad()) + ctx := context.Background() + + ecsClient := ecsOrSkip(ctx, t, tc.Nomad()) jobID := "ecsjob-" + uuid.Generate()[0:8] tc.jobIDs = append(tc.jobIDs, jobID) @@ -132,7 +137,7 @@ func (tc *RemoteTasksTest) TestECSDrain(f *framework.F) { e2eutil.WaitForAllocsRunning(t, tc.Nomad(), []string{origAlloc}) arn := arnForAlloc(t, tc.Nomad().Allocations(), origAlloc) - ensureECSRunning(t, ecsClient, arn) + ensureECSRunning(ctx, t, ecsClient, arn) t.Logf("Task %s is running! Now to drain the node.", arn) @@ -197,7 +202,9 @@ func (tc *RemoteTasksTest) TestECSDrain(f *framework.F) { func (tc *RemoteTasksTest) TestECSDeployment(f *framework.F) { t := f.T() - ecsClient := ecsOrSkip(t, tc.Nomad()) + ctx := context.Background() + + ecsClient := ecsOrSkip(ctx, t, tc.Nomad()) jobID := "ecsjob-" + uuid.Generate()[0:8] tc.jobIDs = append(tc.jobIDs, jobID) @@ -211,7 +218,7 @@ func (tc *RemoteTasksTest) TestECSDeployment(f *framework.F) { origARN := arnForAlloc(t, tc.Nomad().Allocations(), origAllocID) // Use ARN to lookup status of ECS task in AWS - ensureECSRunning(t, ecsClient, origARN) + ensureECSRunning(ctx, t, ecsClient, origARN) t.Logf("Task %s is running! Updating...", origARN) @@ -271,10 +278,10 @@ func (tc *RemoteTasksTest) TestECSDeployment(f *framework.F) { // Ensure original ARN is stopped in ECS input := ecs.DescribeTasksInput{ Cluster: aws.String("nomad-rtd-e2e"), - Tasks: []*string{aws.String(origARN)}, + Tasks: []string{origARN}, } testutil.WaitForResult(func() (bool, error) { - resp, err := ecsClient.DescribeTasks(&input) + resp, err := ecsClient.DescribeTasks(ctx, &input) if err != nil { return false, err } @@ -287,12 +294,13 @@ func (tc *RemoteTasksTest) TestECSDeployment(f *framework.F) { // ecsOrSkip returns an AWS ECS client or skips the test if ECS is unreachable // by the test runner or the ECS remote task driver isn't healthy. -func ecsOrSkip(t *testing.T, nomadClient *api.Client) *ecs.ECS { - awsSession := session.Must(session.NewSession()) +func ecsOrSkip(ctx context.Context, t *testing.T, nomadClient *api.Client) *ecs.Client { + cfg, err := config.LoadDefaultConfig(ctx, config.WithRegion("us-east-1")) + require.NoError(t, err) - ecsClient := ecs.New(awsSession, aws.NewConfig().WithRegion("us-east-1")) + ecsClient := ecs.NewFromConfig(cfg) - _, err := ecsClient.ListClusters(&ecs.ListClustersInput{}) + _, err = ecsClient.ListClusters(ctx, &ecs.ListClustersInput{}) if err != nil { t.Skipf("Skipping ECS Remote Task Driver Task. Error querying AWS ECS API: %v", err) } @@ -378,14 +386,14 @@ func arnForAlloc(t *testing.T, allocAPI *api.Allocations, allocID string) string } // ensureECSRunning asserts that the given ARN is a running ECS task. -func ensureECSRunning(t *testing.T, ecsClient *ecs.ECS, arn string) { +func ensureECSRunning(ctx context.Context, t *testing.T, ecsClient *ecs.Client, arn string) { t.Logf("Ensuring ARN=%s is running", arn) input := ecs.DescribeTasksInput{ Cluster: aws.String("nomad-rtd-e2e"), - Tasks: []*string{aws.String(arn)}, + Tasks: []string{arn}, } testutil.WaitForResult(func() (bool, error) { - resp, err := ecsClient.DescribeTasks(&input) + resp, err := ecsClient.DescribeTasks(ctx, &input) if err != nil { return false, err } diff --git a/go.mod b/go.mod index 39dbd6b7d26..230446250b5 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,11 @@ require ( github.com/Microsoft/go-winio v0.6.1 github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e github.com/armon/go-metrics v0.5.3 - github.com/aws/aws-sdk-go v1.55.5 + github.com/aws/aws-sdk-go-v2 v1.32.6 + github.com/aws/aws-sdk-go-v2/config v1.28.6 + github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.21 + github.com/aws/aws-sdk-go-v2/service/ecs v1.53.0 + github.com/aws/smithy-go v1.22.1 github.com/brianvoe/gofakeit/v6 v6.20.1 github.com/container-storage-interface/spec v1.10.0 github.com/containerd/go-cni v1.1.9 @@ -175,6 +179,16 @@ require ( github.com/apparentlymart/go-textseg/v13 v13.0.0 // indirect github.com/apparentlymart/go-textseg/v15 v15.0.0 // indirect github.com/armon/go-radix v1.0.0 // indirect + github.com/aws/aws-sdk-go v1.55.5 // indirect + github.com/aws/aws-sdk-go-v2/credentials v1.17.47 // indirect + github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.25 // indirect + github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.25 // indirect + github.com/aws/aws-sdk-go-v2/internal/ini v1.8.1 // indirect + github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.1 // indirect + github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.6 // indirect + github.com/aws/aws-sdk-go-v2/service/sso v1.24.7 // indirect + github.com/aws/aws-sdk-go-v2/service/ssooidc v1.28.6 // indirect + github.com/aws/aws-sdk-go-v2/service/sts v1.33.2 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect github.com/bgentry/speakeasy v0.1.0 // indirect diff --git a/go.sum b/go.sum index e4925ea94ea..ec64c9ee2cb 100644 --- a/go.sum +++ b/go.sum @@ -292,6 +292,34 @@ github.com/aws/aws-sdk-go v1.30.27/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZve github.com/aws/aws-sdk-go v1.44.122/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= github.com/aws/aws-sdk-go v1.55.5 h1:KKUZBfBoyqy5d3swXyiC7Q76ic40rYcbqH7qjh59kzU= github.com/aws/aws-sdk-go v1.55.5/go.mod h1:eRwEWoyTWFMVYVQzKMNHWP5/RV4xIUGMQfXQHfHkpNU= +github.com/aws/aws-sdk-go-v2 v1.32.6 h1:7BokKRgRPuGmKkFMhEg/jSul+tB9VvXhcViILtfG8b4= +github.com/aws/aws-sdk-go-v2 v1.32.6/go.mod h1:P5WJBrYqqbWVaOxgH0X/FYYD47/nooaPOZPlQdmiN2U= +github.com/aws/aws-sdk-go-v2/config v1.28.6 h1:D89IKtGrs/I3QXOLNTH93NJYtDhm8SYa9Q5CsPShmyo= +github.com/aws/aws-sdk-go-v2/config v1.28.6/go.mod h1:GDzxJ5wyyFSCoLkS+UhGB0dArhb9mI+Co4dHtoTxbko= +github.com/aws/aws-sdk-go-v2/credentials v1.17.47 h1:48bA+3/fCdi2yAwVt+3COvmatZ6jUDNkDTIsqDiMUdw= +github.com/aws/aws-sdk-go-v2/credentials v1.17.47/go.mod h1:+KdckOejLW3Ks3b0E3b5rHsr2f9yuORBum0WPnE5o5w= +github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.21 h1:AmoU1pziydclFT/xRV+xXE/Vb8fttJCLRPv8oAkprc0= +github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.21/go.mod h1:AjUdLYe4Tgs6kpH4Bv7uMZo7pottoyHMn4eTcIcneaY= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.25 h1:s/fF4+yDQDoElYhfIVvSNyeCydfbuTKzhxSXDXCPasU= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.25/go.mod h1:IgPfDv5jqFIzQSNbUEMoitNooSMXjRSDkhXv8jiROvU= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.25 h1:ZntTCl5EsYnhN/IygQEUugpdwbhdkom9uHcbCftiGgA= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.25/go.mod h1:DBdPrgeocww+CSl1C8cEV8PN1mHMBhuCDLpXezyvWkE= +github.com/aws/aws-sdk-go-v2/internal/ini v1.8.1 h1:VaRN3TlFdd6KxX1x3ILT5ynH6HvKgqdiXoTxAF4HQcQ= +github.com/aws/aws-sdk-go-v2/internal/ini v1.8.1/go.mod h1:FbtygfRFze9usAadmnGJNc8KsP346kEe+y2/oyhGAGc= +github.com/aws/aws-sdk-go-v2/service/ecs v1.53.0 h1:TCQZX4ztlcWXAcZouKh9qJMcVaH/qTidFTfsvJwUI30= +github.com/aws/aws-sdk-go-v2/service/ecs v1.53.0/go.mod h1:Ghi1OWUv4+VMEULWiHsKH2gNA3KAcMoLWsvU0eRXvIA= +github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.1 h1:iXtILhvDxB6kPvEXgsDhGaZCSC6LQET5ZHSdJozeI0Y= +github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.1/go.mod h1:9nu0fVANtYiAePIBh2/pFUSwtJ402hLnp854CNoDOeE= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.6 h1:50+XsN70RS7dwJ2CkVNXzj7U2L1HKP8nqTd3XWEXBN4= +github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.6/go.mod h1:WqgLmwY7so32kG01zD8CPTJWVWM+TzJoOVHwTg4aPug= +github.com/aws/aws-sdk-go-v2/service/sso v1.24.7 h1:rLnYAfXQ3YAccocshIH5mzNNwZBkBo+bP6EhIxak6Hw= +github.com/aws/aws-sdk-go-v2/service/sso v1.24.7/go.mod h1:ZHtuQJ6t9A/+YDuxOLnbryAmITtr8UysSny3qcyvJTc= +github.com/aws/aws-sdk-go-v2/service/ssooidc v1.28.6 h1:JnhTZR3PiYDNKlXy50/pNeix9aGMo6lLpXwJ1mw8MD4= +github.com/aws/aws-sdk-go-v2/service/ssooidc v1.28.6/go.mod h1:URronUEGfXZN1VpdktPSD1EkAL9mfrV+2F4sjH38qOY= +github.com/aws/aws-sdk-go-v2/service/sts v1.33.2 h1:s4074ZO1Hk8qv65GqNXqDjmkf4HSQqJukaLuuW0TpDA= +github.com/aws/aws-sdk-go-v2/service/sts v1.33.2/go.mod h1:mVggCnIWoM09jP71Wh+ea7+5gAp53q+49wDFs1SW5z8= +github.com/aws/smithy-go v1.22.1 h1:/HPHZQ0g7f4eUeK6HKglFz8uwVfZKgoI25rb/J+dnro= +github.com/aws/smithy-go v1.22.1/go.mod h1:irrKGvNn1InZwb2d7fkIRNucdfwR8R+Ts3wxYa/cJHg= github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8= From 97ae9dbdf71c2f3f1aecb5e732a15474d172a60c Mon Sep 17 00:00:00 2001 From: Michael Smithhisler Date: Thu, 19 Dec 2024 12:57:27 -0500 Subject: [PATCH 2/5] add back endpoint for testing --- client/fingerprint/env_aws.go | 45 ++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/client/fingerprint/env_aws.go b/client/fingerprint/env_aws.go index d0d6d86c08e..b07617ec1bc 100644 --- a/client/fingerprint/env_aws.go +++ b/client/fingerprint/env_aws.go @@ -53,6 +53,9 @@ var ec2NetSpeedTable = map[*regexp.Regexp]int{ type EnvAWSFingerprint struct { StaticFingerprinter + // endpoint for EC2 metadata as expected by AWS SDK + endpoint string + logger log.Logger } @@ -77,7 +80,7 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F ctx, cancel := context.WithTimeout(context.TODO(), timeout) defer cancel() - imdsClient, err := imdsClient(ctx) + imdsClient, err := imdsClient(ctx, f.endpoint) if err != nil { return fmt.Errorf("failed to setup IMDS client: %v", err) } @@ -119,7 +122,7 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F if err != nil { return err } - v := string(bytes) + v := strings.TrimSpace(string(bytes)) if v == "" { f.logger.Debug("read an empty value", "attribute", k) } @@ -164,7 +167,7 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F if err != nil { return err } - addrsStr := string(addrBytes) + addrsStr := strings.TrimSpace(string(addrBytes)) if addrsStr == "" { f.logger.Debug("read an empty value", "attribute", k) } else { @@ -210,7 +213,7 @@ func (f *EnvAWSFingerprint) instanceType(client *imds.Client) (string, error) { if err != nil { return "", err } - return string(content), nil + return strings.TrimSpace(string(content)), nil } func (f *EnvAWSFingerprint) throughput(request *FingerprintRequest, client *imds.Client, ip string) int { @@ -254,22 +257,42 @@ func (f *EnvAWSFingerprint) linkSpeed(client *imds.Client) int { return netSpeed } -func imdsClient(ctx context.Context) (*imds.Client, error) { +func imdsClient(ctx context.Context, endpoint string) (*imds.Client, error) { client := &http.Client{ Transport: cleanhttp.DefaultTransport(), } - cfg, err := config.LoadDefaultConfig(ctx, config.WithHTTPClient(client), config.WithRetryMaxAttempts(0)) + cfg, err := config.LoadDefaultConfig(ctx, + config.WithHTTPClient(client), + config.WithRetryMaxAttempts(0), + ) if err != nil { return nil, err } - return imds.NewFromConfig(cfg), nil + + imdsClient := imds.NewFromConfig(cfg, func(o *imds.Options) { + if endpoint != "" { + o.Endpoint = endpoint + } + }) + return imdsClient, nil } -// isAWS validates the client can reach IMDS. Fetching an ami-id must -// complete error free to be recognized as running on AWS EC2 func isAWS(ctx context.Context, client *imds.Client) bool { - _, err := client.GetMetadata(ctx, &imds.GetMetadataInput{ + resp, err := client.GetMetadata(ctx, &imds.GetMetadataInput{ Path: "ami-id", }) - return err == nil + if err != nil { + return false + } + + var v string + if resp != nil { + b, err := io.ReadAll(resp.Content) + if err != nil { + return false + } + v = strings.TrimSpace(string(b)) + } + + return v != "" } From 5a330623742507cac4811e7122e9cd693110de68 Mon Sep 17 00:00:00 2001 From: Michael Smithhisler Date: Thu, 19 Dec 2024 16:44:18 -0500 Subject: [PATCH 3/5] close all io.ReadClosers --- client/fingerprint/env_aws.go | 18 +++++++++--------- go.mod | 6 +++--- go.sum | 12 ++++++------ 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/client/fingerprint/env_aws.go b/client/fingerprint/env_aws.go index b07617ec1bc..10e8b42c967 100644 --- a/client/fingerprint/env_aws.go +++ b/client/fingerprint/env_aws.go @@ -117,6 +117,7 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F if resp == nil { continue } + defer resp.Content.Close() bytes, err := io.ReadAll(resp.Content) if err != nil { @@ -163,10 +164,13 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F return err } if resp != nil { + defer resp.Content.Close() + addrBytes, err := io.ReadAll(resp.Content) if err != nil { return err } + addrsStr := strings.TrimSpace(string(addrBytes)) if addrsStr == "" { f.logger.Debug("read an empty value", "attribute", k) @@ -284,15 +288,11 @@ func isAWS(ctx context.Context, client *imds.Client) bool { if err != nil { return false } + defer resp.Content.Close() - var v string - if resp != nil { - b, err := io.ReadAll(resp.Content) - if err != nil { - return false - } - v = strings.TrimSpace(string(b)) + b, err := io.ReadAll(resp.Content) + if err != nil { + return false } - - return v != "" + return strings.TrimSpace(string(b)) != "" } diff --git a/go.mod b/go.mod index 230446250b5..59378cb619b 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,7 @@ require ( github.com/Microsoft/go-winio v0.6.1 github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e github.com/armon/go-metrics v0.5.3 - github.com/aws/aws-sdk-go-v2 v1.32.6 + github.com/aws/aws-sdk-go-v2 v1.32.7 github.com/aws/aws-sdk-go-v2/config v1.28.6 github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.21 github.com/aws/aws-sdk-go-v2/service/ecs v1.53.0 @@ -181,8 +181,8 @@ require ( github.com/armon/go-radix v1.0.0 // indirect github.com/aws/aws-sdk-go v1.55.5 // indirect github.com/aws/aws-sdk-go-v2/credentials v1.17.47 // indirect - github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.25 // indirect - github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.25 // indirect + github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.26 // indirect + github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.26 // indirect github.com/aws/aws-sdk-go-v2/internal/ini v1.8.1 // indirect github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.1 // indirect github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.6 // indirect diff --git a/go.sum b/go.sum index ec64c9ee2cb..581696745ae 100644 --- a/go.sum +++ b/go.sum @@ -292,18 +292,18 @@ github.com/aws/aws-sdk-go v1.30.27/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZve github.com/aws/aws-sdk-go v1.44.122/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo= github.com/aws/aws-sdk-go v1.55.5 h1:KKUZBfBoyqy5d3swXyiC7Q76ic40rYcbqH7qjh59kzU= github.com/aws/aws-sdk-go v1.55.5/go.mod h1:eRwEWoyTWFMVYVQzKMNHWP5/RV4xIUGMQfXQHfHkpNU= -github.com/aws/aws-sdk-go-v2 v1.32.6 h1:7BokKRgRPuGmKkFMhEg/jSul+tB9VvXhcViILtfG8b4= -github.com/aws/aws-sdk-go-v2 v1.32.6/go.mod h1:P5WJBrYqqbWVaOxgH0X/FYYD47/nooaPOZPlQdmiN2U= +github.com/aws/aws-sdk-go-v2 v1.32.7 h1:ky5o35oENWi0JYWUZkB7WYvVPP+bcRF5/Iq7JWSb5Rw= +github.com/aws/aws-sdk-go-v2 v1.32.7/go.mod h1:P5WJBrYqqbWVaOxgH0X/FYYD47/nooaPOZPlQdmiN2U= github.com/aws/aws-sdk-go-v2/config v1.28.6 h1:D89IKtGrs/I3QXOLNTH93NJYtDhm8SYa9Q5CsPShmyo= github.com/aws/aws-sdk-go-v2/config v1.28.6/go.mod h1:GDzxJ5wyyFSCoLkS+UhGB0dArhb9mI+Co4dHtoTxbko= github.com/aws/aws-sdk-go-v2/credentials v1.17.47 h1:48bA+3/fCdi2yAwVt+3COvmatZ6jUDNkDTIsqDiMUdw= github.com/aws/aws-sdk-go-v2/credentials v1.17.47/go.mod h1:+KdckOejLW3Ks3b0E3b5rHsr2f9yuORBum0WPnE5o5w= github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.21 h1:AmoU1pziydclFT/xRV+xXE/Vb8fttJCLRPv8oAkprc0= github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.21/go.mod h1:AjUdLYe4Tgs6kpH4Bv7uMZo7pottoyHMn4eTcIcneaY= -github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.25 h1:s/fF4+yDQDoElYhfIVvSNyeCydfbuTKzhxSXDXCPasU= -github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.25/go.mod h1:IgPfDv5jqFIzQSNbUEMoitNooSMXjRSDkhXv8jiROvU= -github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.25 h1:ZntTCl5EsYnhN/IygQEUugpdwbhdkom9uHcbCftiGgA= -github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.25/go.mod h1:DBdPrgeocww+CSl1C8cEV8PN1mHMBhuCDLpXezyvWkE= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.26 h1:I/5wmGMffY4happ8NOCuIUEWGUvvFp5NSeQcXl9RHcI= +github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.26/go.mod h1:FR8f4turZtNy6baO0KJ5FJUmXH/cSkI9fOngs0yl6mA= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.26 h1:zXFLuEuMMUOvEARXFUVJdfqZ4bvvSgdGRq/ATcrQxzM= +github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.26/go.mod h1:3o2Wpy0bogG1kyOPrgkXA8pgIfEEv0+m19O9D5+W8y8= github.com/aws/aws-sdk-go-v2/internal/ini v1.8.1 h1:VaRN3TlFdd6KxX1x3ILT5ynH6HvKgqdiXoTxAF4HQcQ= github.com/aws/aws-sdk-go-v2/internal/ini v1.8.1/go.mod h1:FbtygfRFze9usAadmnGJNc8KsP346kEe+y2/oyhGAGc= github.com/aws/aws-sdk-go-v2/service/ecs v1.53.0 h1:TCQZX4ztlcWXAcZouKh9qJMcVaH/qTidFTfsvJwUI30= From 88a457ca2c6290e57d361dd26b4c6eb61e3b44b4 Mon Sep 17 00:00:00 2001 From: Michael Smithhisler Date: Fri, 20 Dec 2024 12:33:58 -0500 Subject: [PATCH 4/5] refactor handling imds response --- client/fingerprint/env_aws.go | 38 +++++++++++++++---------- client/fingerprint/env_aws_test.go | 45 ++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/client/fingerprint/env_aws.go b/client/fingerprint/env_aws.go index 10e8b42c967..74024b26f2d 100644 --- a/client/fingerprint/env_aws.go +++ b/client/fingerprint/env_aws.go @@ -53,7 +53,7 @@ var ec2NetSpeedTable = map[*regexp.Regexp]int{ type EnvAWSFingerprint struct { StaticFingerprinter - // endpoint for EC2 metadata as expected by AWS SDK + // used to override IMDS endpoint for testing endpoint string logger log.Logger @@ -80,7 +80,7 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F ctx, cancel := context.WithTimeout(context.TODO(), timeout) defer cancel() - imdsClient, err := imdsClient(ctx, f.endpoint) + imdsClient, err := f.imdsClient(ctx) if err != nil { return fmt.Errorf("failed to setup IMDS client: %v", err) } @@ -117,15 +117,15 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F if resp == nil { continue } - defer resp.Content.Close() - bytes, err := io.ReadAll(resp.Content) + v, err := readMetadataResponse(resp) if err != nil { return err } - v := strings.TrimSpace(string(bytes)) + if v == "" { f.logger.Debug("read an empty value", "attribute", k) + continue } // assume we want blank entries @@ -164,14 +164,11 @@ func (f *EnvAWSFingerprint) Fingerprint(request *FingerprintRequest, response *F return err } if resp != nil { - defer resp.Content.Close() - - addrBytes, err := io.ReadAll(resp.Content) + addrsStr, err := readMetadataResponse(resp) if err != nil { return err } - addrsStr := strings.TrimSpace(string(addrBytes)) if addrsStr == "" { f.logger.Debug("read an empty value", "attribute", k) } else { @@ -261,7 +258,7 @@ func (f *EnvAWSFingerprint) linkSpeed(client *imds.Client) int { return netSpeed } -func imdsClient(ctx context.Context, endpoint string) (*imds.Client, error) { +func (f *EnvAWSFingerprint) imdsClient(ctx context.Context) (*imds.Client, error) { client := &http.Client{ Transport: cleanhttp.DefaultTransport(), } @@ -274,8 +271,9 @@ func imdsClient(ctx context.Context, endpoint string) (*imds.Client, error) { } imdsClient := imds.NewFromConfig(cfg, func(o *imds.Options) { - if endpoint != "" { - o.Endpoint = endpoint + // endpoint should only be overridden for testing + if f.endpoint != "" { + o.Endpoint = f.endpoint } }) return imdsClient, nil @@ -288,11 +286,23 @@ func isAWS(ctx context.Context, client *imds.Client) bool { if err != nil { return false } + + s, err := readMetadataResponse(resp) + if err != nil { + return false + } + + return s != "" +} + +// readImdsResponse reads and formats the IMDS response +// and most importantly, closes the io.ReadCloser +func readMetadataResponse(resp *imds.GetMetadataOutput) (string, error) { defer resp.Content.Close() b, err := io.ReadAll(resp.Content) if err != nil { - return false + return "", err } - return strings.TrimSpace(string(b)) != "" + return strings.TrimSpace(string(b)), nil } diff --git a/client/fingerprint/env_aws_test.go b/client/fingerprint/env_aws_test.go index 6131b95281b..f2fd416d334 100644 --- a/client/fingerprint/env_aws_test.go +++ b/client/fingerprint/env_aws_test.go @@ -9,10 +9,13 @@ import ( "net/http/httptest" "testing" + "github.com/aws/smithy-go" + smithyHttp "github.com/aws/smithy-go/transport/http" "github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/nomad/structs" + "github.com/shoenig/test/must" "github.com/stretchr/testify/require" ) @@ -77,6 +80,45 @@ func TestEnvAWSFingerprint_aws(t *testing.T) { } } +func TestEnvAWSFingerprint_handleImdsError(t *testing.T) { + ci.Parallel(t) + + f := NewEnvAWSFingerprint(testlog.HCLogger(t)) + + cases := []struct { + name string + err error + exp error + }{ + { + name: "random errors return error", + err: fmt.Errorf("not http error"), + exp: fmt.Errorf("not http error"), + }, + { + name: "other smithy errors return error", + err: &smithy.OperationError{}, + exp: &smithy.OperationError{}, + }, + { + name: "http response errors correctly handled", + err: &smithyHttp.ResponseError{ + Response: &smithyHttp.Response{ + Response: &http.Response{ + StatusCode: 404, + }, + }, + }, + exp: nil, + }, + } + + for _, c := range cases { + err := f.(*EnvAWSFingerprint).handleImdsError(c.err, "some attribute") + must.Eq(t, c.exp, err) + } +} + func TestNetworkFingerprint_AWS(t *testing.T) { ci.Parallel(t) @@ -192,6 +234,9 @@ func TestNetworkFingerprint_AWS_NoNetwork(t *testing.T) { require.Equal(t, "ami-1234", response.Attributes["platform.aws.ami-id"]) + // assert the key is not present in the Attributes map if the return value was empty + require.NotContains(t, response.Attributes, "unique.platform.aws.local-ipv4") + require.Nil(t, response.NodeResources.Networks) } From ed1dad4539823e1142fa73bc9114229f65f77c8a Mon Sep 17 00:00:00 2001 From: Michael Smithhisler Date: Thu, 9 Jan 2025 15:19:06 -0500 Subject: [PATCH 5/5] adds changelog --- .changelog/24720.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/24720.txt diff --git a/.changelog/24720.txt b/.changelog/24720.txt new file mode 100644 index 00000000000..bcbe59a7472 --- /dev/null +++ b/.changelog/24720.txt @@ -0,0 +1,3 @@ +```release-note:improvement +deps: Upgraded aws-sdk-go from v1 to v2 +```