Improve documentation around client/credential caching

stanhu commented 2 months ago

Describe the issue

In https://docs.aws.amazon.com/sdk-for-go/v1/developer-guide/sessions.html, the guidance for caching s3.Session was clear:

You should cache sessions when possible. This is because creating a new session loads all configuration values from the environment and configuration files each time the session is created. Sharing the session value across all of your service clients ensures the configuration is loaded the fewest number of times.

In https://aws.github.io/aws-sdk-go-v2/docs/migrating/ and https://aws.github.io/aws-sdk-go-v2/docs/making-requests/, I'm not clear on what could be cached. We have a long-lived application that might make hundreds of S3 calls over time, and we want to avoid hitting STS limits. If I understand correctly:

import "context"
import "github.com/aws/aws-sdk-go-v2/config"
import "github.com/aws/aws-sdk-go-v2/service/s3"

// ...

cfg, err := config.LoadDefaultConfig(context.TODO())
if err != nil {
    panic(err)
}

client := s3.NewFromConfig(cfg)

In this example:

1.config.LoadDefaultConfig: I think this might make a HTTP request to the instance metadata or STS endpoint if no static credentials are configured.

s3.Client: This creates a new S3 client and loads the credentials from the config.

Is it okay to cache s3.Client indefinitely? I presume that when the credentials are expired, it will automatically refresh them.

It should also be okay to cache config.LoadDefaultConfig to avoid making an HTTP call, but it might just be easier to cache s3.Client.

It'd be nice to update the documentation, especially in the migration guide, about this behavior.

Links

AWS Go SDK V2 Module Versions Used

    github.com/aws/aws-sdk-go v1.55.5
    github.com/aws/aws-sdk-go-v2 v1.30.3
    github.com/aws/aws-sdk-go-v2/config v1.27.27
    github.com/aws/aws-sdk-go-v2/credentials v1.17.27
    github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.17.10
    github.com/aws/aws-sdk-go-v2/service/s3 v1.58.3
    github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.3 // indirect
    github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.11 // indirect
    github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.15 // indirect
    github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.15 // indirect
    github.com/aws/aws-sdk-go-v2/internal/ini v1.8.0 // indirect
    github.com/aws/aws-sdk-go-v2/internal/v4a v1.3.15 // indirect
    github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.3 // indirect
    github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.3.17 // indirect
    github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.17 // indirect
    github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.17.15 // indirect
    github.com/aws/aws-sdk-go-v2/service/sso v1.22.4 // indirect
    github.com/aws/aws-sdk-go-v2/service/ssooidc v1.26.4 // indirect
    github.com/aws/aws-sdk-go-v2/service/sts v1.30.3 // indirect
    github.com/aws/smithy-go v1.20.3 // indirect

RanVaknin commented 2 months ago

Hi @stanhu,

Our developer guide talks about this:

All credential providers passed to or returned by LoadDefaultConfig are wrapped in a CredentialsCache automatically. This enables caching and concurrency safe credential access. If you explicitly configure a provider on aws.Config directly you must explicitly wrap the provider with this type using NewCredentialsCache.

Regarding your question:

Is it okay to cache s3.Client indefinitely? I presume that when the credentials are expired, it will automatically refresh them.

If you are using LoadDefaultConfig then yes. The same is true for Go SDK v1's session.NewSession(). Both instruct the SDK to use the default credential provider chain, which automatically comes with a cache that handles credential rotation for any of the providers. In your case, the SDK would make calls to IMDS to refresh credentials 5 min before the credentials' expiration window.

This isn't covered in the migration guide because the functionality in v1 and v2 is the same with regards to how credentials are rotated. The only difference is the re-naming of the config object, which in v1 was confusingly named "session" even though it does not really represent a session but rather a config object.

I suppose we can add another section in the migration guide about the credential rotation behavior.

Thanks, Ran~

stanhu commented 2 months ago

@RanVaknin Thanks, that helps clarify things!

I suppose we can add another section in the migration guide about the credential rotation behavior.

Yes, that would be helpful.

github-actions[bot] commented 2 months ago

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.

aws / aws-sdk-go-v2