Regression in service/s3/v1.48.1 with use of CredentialsProviderFunc (panic)

[brandur]

Describe the bug

We upgraded from service/s3/v1.48.0 to service/s3/v1.48.1 and started getting this exception on S3 requests:

panic: runtime error: comparing uncomparable type aws.CredentialsProviderFunc

goroutine 454 [running]:
github.com/aws/aws-sdk-go-v2/service/s3.finalizeOperationExpressCredentials(...)
	/tmp/codon/tmp/cache/go-path/pkg/mod/github.com/aws/aws-sdk-go-v2/service/s3@v1.48.1/express_resolve.go:34
github.com/aws/aws-sdk-go-v2/service/s3.(*Client).invokeOperation(0xc00069e1e0, {0x2214d00?, 0xc000b0a960?}, {0x1ec03c1, 0x9}, {0x1ac5cc0, 0xc000716300}, {0x0, 0x0, 0x0}, ...)
	/tmp/codon/tmp/cache/go-path/pkg/mod/github.com/aws/aws-sdk-go-v2/service/s3@v1.48.1/api_client.go:112 +0x52d
github.com/aws/aws-sdk-go-v2/service/s3.(*Client).GetObject(0x7b606f?, {0x2214d00?, 0xc000b0a960?}, 0x1dd9880?, {0x0?, 0xc000716300?, 0x1ac5cc0?})
	/tmp/codon/tmp/cache/go-path/pkg/mod/github.com/aws/aws-sdk-go-v2/service/s3@v1.48.1/api_op_GetObject.go:118 +0x112
github.com/crunchydata/priv-all-platform/pclient/awsclient.(*Client).S3_GetObject.func1({0x2214d00, 0xc000b0a960})
	/tmp/build_550a8554/pclient/awsclient/aws_client.go:149 +0x3e
github.com/crunchydata/priv-all-platform/pclient.CallSDK[...](0xc000696000?, {0x2214d00, 0xc000b0a960}, {0x1ec49ce, 0xc}, 0x0, {0x1ac5cc0?, 0xc000716300}, 0xc000745ca0)
	/tmp/build_550a8554/pclient/client.go:112 +0x310
github.com/crunchydata/priv-all-platform/pclient/awsclient.(*Client).S3_GetObject(0xc00061c3f0, {0x2214d00, 0xc000b0a960}, 0xc000716300, {0x0, 0x0, 0x0})
	/tmp/build_550a8554/pclient/awsclient/aws_client.go:148 +0x1d9
github.com/crunchydata/priv-all-platform/server/apiresourcekind.(*Query).LoadBundle.func9()
	/tmp/build_550a8554/server/apiresourcekind/query.go:210 +0x123
golang.org/x/sync/errgroup.(*Group).Go.func1()
	/tmp/codon/tmp/cache/go-path/pkg/mod/golang.org/x/sync@v0.6.0/errgroup/errgroup.go:78 +0x56
created by golang.org/x/sync/errgroup.(*Group).Go in goroutine 423
	/tmp/codon/tmp/cache/go-path/pkg/mod/golang.org/x/sync@v0.6.0/errgroup/errgroup.go:75 +0x96

Digging in, the problem is emanating from this line, where two credentials are compared, and comparing functions isn't allowed.

aws-sdk-go-v2/service/s3/express_resolve.go

Lines 34 to 36 in 1ff8d79

	if c.options.Credentials != o.Credentials {
	o.ExpressCredentials = p.CloneWithBaseCredentials(o.Credentials)
	}

I didn't full confirm this (so take it with a grain of salt), but I believe the problem may be related to #2465 from @lucix-aws. In api_client.go, resolveCredentialProvider was removed, which looks like it may have wrapped a CredentialsProviderFunc into an adapter that might've made it comparable.

o.Credentials = &v4a.SymmetricCredentialAdaptor{SymmetricProvider: o.Credentials}

With the removal of that, comparing the raw CredentialsProviderFunc credentials is a panic.

https://github.com/aws/aws-sdk-go-v2/pull/2465/files#diff-52ebcb8665d547278ab74478a642e6eab70ec56dc566673ef1d7a1e292e7f285

Expected Behavior

S3 requests work without a panic.

Current Behavior

Panic and program crash occurs.

Reproduction Steps

Make sure to use CredentialsProviderFunc for credentials. e.g.

		credentialsProvider := aws.CredentialsProviderFunc(func(context.Context) (aws.Credentials, error) {
			return aws.Credentials{
				AccessKeyID:     config.AccessKeyID,
				SecretAccessKey: config.SecretAccessKey,
			}, nil
		})

		s3Client := s3.New(s3.Options{
			Credentials: credentialsProvider,
			Logger:      loggerShim,
			Region:      region,
		}),

		out, err := s3Client.GetObject(ctx, params, optFns...)

Possible Solution

No response

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

	github.com/aws/aws-sdk-go-v2 v1.24.1
	github.com/aws/aws-sdk-go-v2/service/marketplacemetering v1.19.6
	github.com/aws/aws-sdk-go-v2/service/s3 v1.48.1
	github.com/aws/aws-sdk-go-v2/service/sqs v1.29.7

Compiler and Version used

go version go1.21.4 darwin/arm64

Operating System and version

macOS

[RanVaknin]

Hi @brandur ,

Thanks for reaching out. I'm able to reproduce the behavior. Will have someone look at it with priority.

All the best,
Ran~

[lucix-aws]

The removal of that asymmetric wrapper was correct but it's exposed the potential to fail on that comparison as you've observed. This should only be affecting S3, this check is a customization for S3Express and shouldn't occur anywhere else to my knowledge.

The correct way forward here will be to correct that comparison in express_resolve itself.

A workaround until this is fixed would be to switch to a using a comparable value that satisfies the credentials provider interface.

[RanVaknin]

@brandur,

A workaround might look like:

	staticCreds := credentials.NewStaticCredentialsProvider(awsAccessKeyID, awsSecretAccessKey, "")

	cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithRegion("us-east-1"))
	if err != nil {
		log.Fatalf("unable to load SDK config, %v", err)
	}

	client := s3.NewFromConfig(cfg, func(options *s3.Options) {
		options.Credentials = staticCreds
	})

Or you can pass it to your config directly:

	cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithRegion("us-east-1"), config.WithCredentialsProvider(credentials.NewStaticCredentialsProvider(awsAccessKeyID, awsSecretAccessKey, "")))
	if err != nil {
		log.Fatalf("unable to load SDK config, %v", err)
	}

Thanks,
Ran~

[brandur]

@RanVaknin Thanks!

We needed a fast fix so I just downgraded the S3 package on Monday to get things working again. The use of StaticCredentialsProvider looks like it'd work okay for us — I might switch over to that depending on how long bug resolution takes.

[github-actions]

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.