Skip to content

Commit d900479

Browse files
sean-ryanyuan
andcommitted
fix(signer/awsv2): cache credentials to avoid STS hot-path calls
signer/awsv2 stored the caller's aws.Config credentials directly, so every signed request called Credentials.Retrieve. AWS SDK v2 does not cache credentials by default, so for STS-backed providers (assume-role, web identity, IRSA) that is an STS call on the request hot path. Under load it can exhaust the account's STS rate limits and cause account-wide outages of any service that depends on STS, not just this client. Wrap the credentials in aws.NewCredentialsCache in NewSignerWithService, matching signer/aws. Reword the equivalent comment in signer/aws and the signer package doc to describe the failure mode rather than framing the cache as a performance optimization, and drop the stale "migration tracked in a separate issue" note in signer/aws now that the SDK v2 move is done. Co-authored-by: Ryan Yuan <ryan.yuan@crowdstrike.com> Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
1 parent 86f3955 commit d900479

3 files changed

Lines changed: 21 additions & 6 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
223223

224224
### Fixed
225225

226+
- Cache credentials in the `signer/awsv2` constructors. A raw `CredentialsProvider` is wrapped in an `aws.CredentialsCache` (an already-cached provider, such as one from `config.LoadDefaultConfig`, is left as-is), so SigV4 signing no longer calls `Credentials.Retrieve` on every request. For STS-backed providers (assume-role, web identity, IRSA) the previous behavior was a per-request STS call that could exhaust the account's STS rate limits under load. `signer/awsv2` shipped without this in v4.6.0.
226227
- Fix `BulkIndexer` `OnFailure` nil pointer dereference when reading `BulkRespItem.Error` on status-only failures (e.g. HTTP 404 without an `error` object) or transport-level flush errors by ensuring callbacks always receive a non-nil `Error` ([#679](https://github.com/opensearch-project/opensearch-go/issues/679))
227228
- Generate query parameters whose value `0` is meaningful as `*int` instead of `int` so a deliberate `0` reaches the wire. These params previously used the `!= 0` emission guard shared by all integer params, which silently dropped a deliberate `0` -- breaking optimistic-concurrency writes with `if_seq_no=0` (the sequence number of the first document written to a shard) and search `size=0` (aggregations with no hits). `cmd/osgen` now promotes such params to `*int` with a nil guard, mirroring the existing `*bool` treatment. The promotion is scoped per operation (currently `if_seq_no`/`if_primary_term` on `delete`/`index`/`update` and the plugin policy writes `ism.put_policy`/`ism.put_policies`/`rollups.put`/`sm.update_policy`/`transforms.put`, plus `size` on `search`), since the same wire name is a page-size with no meaningful `0` on other operations. The core `_create` operation does not accept `if_seq_no`/`if_primary_term`, so it is intentionally excluded
228229
- Fix `BulkIndexerStats.NumAdded` overcounting items rejected by `Add()` when the caller's context is cancelled before the item could be enqueued: increment `NumAdded` only after the queue accepts the item, and add a new `BulkAddFailCount` counter for items dropped on the `<-ctx.Done()` branch. Migrate `bulkIndexerStats` fields to `sync/atomic.Uint64` typed values so future direct access is a compile-time error rather than a `-race`-only finding ([#783](https://github.com/opensearch-project/opensearch-go/issues/783))

signer/aws/aws.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,6 @@ import (
4444
"strings"
4545
"time"
4646

47-
// NOTE: aws-sdk-go v1 is deprecated. Migration to aws-sdk-go-v2 is tracked
48-
// in a separate issue. These imports will be replaced in a future update.
49-
// See: https://aws.amazon.com/blogs/developer/announcing-end-of-support-for-aws-sdk-for-go-v1-on-july-31-2025/
5047
"github.com/aws/aws-sdk-go-v2/aws"
5148
awsSignerV4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4"
5249
)
@@ -90,9 +87,13 @@ func NewSignerWithService(cfg aws.Config, service string) (*Signer, error) {
9087
return nil, errors.New("service cannot be empty")
9188
}
9289

93-
// Enable credential caching for better performance, especially with STS credentials.
94-
// According to AWS SDK v2 documentation, credential caching is not enabled by default
95-
// and must be explicitly configured using aws.NewCredentialsCache().
90+
// Wrap credentials in a cache. AWS SDK v2 does not cache credentials by
91+
// default, so without this every signed request calls Credentials.Retrieve.
92+
// For STS-backed providers (assume-role, web identity, IRSA) that means an
93+
// STS call on the request hot path: under load it can exhaust the account's
94+
// STS rate limits and cause account-wide outages of any service that depends
95+
// on STS, not just this client. The cache reuses credentials until they near
96+
// expiry and refreshes once.
9697
// See: go doc github.com/aws/aws-sdk-go-v2/aws CredentialsCache
9798
cfg.Credentials = aws.NewCredentialsCache(cfg.Credentials)
9899

signer/awsv2/sdkv2signer.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,19 @@ func NewSignerWithService(cfg aws.Config, service string, opts ...func(options *
5151
return nil, errors.New("service cannot be empty")
5252
}
5353

54+
// Cache credentials by default. AWS SDK v2 does not cache a raw
55+
// CredentialsProvider, so each signed request would call Credentials.Retrieve;
56+
// for STS-backed providers (assume-role, web identity, IRSA) that is a
57+
// per-request STS call, which under load can exhaust the account's STS rate
58+
// limits and degrade every service that depends on STS. Callers needing other
59+
// behavior can supply a pre-wrapped provider or build their own signer. Skip
60+
// the wrap when the provider is already a cache (e.g. from
61+
// config.LoadDefaultConfig) to avoid double-wrapping.
62+
// See: go doc github.com/aws/aws-sdk-go-v2/aws CredentialsCache
63+
if _, ok := cfg.Credentials.(*aws.CredentialsCache); !ok {
64+
cfg.Credentials = aws.NewCredentialsCache(cfg.Credentials)
65+
}
66+
5467
return &awsSdkV2Signer{
5568
service: service,
5669
signer: awsSignerV4.NewSigner(),

0 commit comments

Comments
 (0)