diff --git a/CHANGELOG.md b/CHANGELOG.md index 2fa642c5..cd7dd412 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -215,6 +215,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Fixed +- **AWS/OpenSearch:** the STS account-ID lookup used for post-purchase tagging caches only a successful result, so one throttled or timed-out `GetCallerIdentity` no longer disables tagging for the client's lifetime (#53). - **AWS/RDS:** `PurchaseCommitment` refuses an offering whose upfront (`FixedPrice` x count) exceeds the approved recommendation upfront by more than 5%, or that is priced in a non-USD currency; a zero approved upfront is not checked (#53). - Bump Go to 1.26.10 to pick up the fix for GO-2026-6809 (crypto/x509), which `govulncheck` now reports against 1.26.9. - **AWS/MemoryDB, OpenSearch, Redshift:** `PurchaseCommitment` refuses an offering whose upfront (`FixedPrice` x count) exceeds the approved recommendation upfront by more than 5%, or that is priced in a non-USD currency (MemoryDB offerings carry no currency, so only the recommendation's currency is checked there), or, for MemoryDB, an upfront offering with a zero price or one bought in a China region (cn-north-1, cn-northwest-1), where the assumed USD price is wrong; a zero approved upfront is not checked. All AWS RI price guards (EC2, RDS, MemoryDB, OpenSearch, Redshift) also refuse a NaN or infinite approved upfront (#53). diff --git a/providers/aws/services/opensearch/client.go b/providers/aws/services/opensearch/client.go index cfbce7f6..66ff62f3 100644 --- a/providers/aws/services/opensearch/client.go +++ b/providers/aws/services/opensearch/client.go @@ -40,12 +40,11 @@ type STSAPI interface { // Client handles AWS OpenSearch Reserved Instances. type Client struct { - client API - stsClient STSAPI - accountErr error - accountID string - region string - accountOnce sync.Once + client API + stsClient STSAPI + region string + accountID string + accountMu sync.Mutex } // NewClient creates a new OpenSearch client with purchase-path retry/timeout @@ -358,22 +357,23 @@ func (c *Client) recoverAlreadyExists(ctx context.Context, token, reservationNam }) } -// resolveAccountID fetches the caller's AWS account ID via STS and caches it. +// resolveAccountID fetches the caller's AWS account ID via STS. Only a +// successful lookup is cached, so a transient failure (throttle, timeout) is +// retried on the next call instead of sticking for the client's lifetime. func (c *Client) resolveAccountID(ctx context.Context) (string, error) { - c.accountOnce.Do(func() { - if c.stsClient == nil { - return - } - out, err := c.stsClient.GetCallerIdentity(ctx, &sts.GetCallerIdentityInput{}) - if err != nil { - c.accountErr = err - return - } - if out != nil && out.Account != nil { - c.accountID = *out.Account - } - }) - return c.accountID, c.accountErr + c.accountMu.Lock() + defer c.accountMu.Unlock() + if c.accountID != "" || c.stsClient == nil { + return c.accountID, nil + } + out, err := c.stsClient.GetCallerIdentity(ctx, &sts.GetCallerIdentityInput{}) + if err != nil { + return "", err + } + if out != nil && out.Account != nil { + c.accountID = *out.Account + } + return c.accountID, nil } // tagReservedInstance constructs the reserved-instance ARN and calls diff --git a/providers/aws/services/opensearch/client_test.go b/providers/aws/services/opensearch/client_test.go index 5896d35a..efa94f07 100644 --- a/providers/aws/services/opensearch/client_test.go +++ b/providers/aws/services/opensearch/client_test.go @@ -3,6 +3,7 @@ package opensearch import ( "bytes" "context" + "errors" "fmt" "log" "strings" @@ -1394,3 +1395,27 @@ func TestClient_PurchaseCommitment_Idempotent_RejectedNotFlagged(t *testing.T) { assert.False(t, result.Success) assert.False(t, result.ExistingCommitment) } + +func TestResolveAccountID_RetriesAfterTransientFailure(t *testing.T) { + mockSTS := &MockOpenSearchSTSClient{} + client := &Client{stsClient: mockSTS, region: "us-east-1"} + acct := "123456789012" + + mockSTS.On("GetCallerIdentity", mock.Anything, mock.Anything). + Return(nil, errors.New("throttled")).Once() + mockSTS.On("GetCallerIdentity", mock.Anything, mock.Anything). + Return(&sts.GetCallerIdentityOutput{Account: &acct}, nil).Once() + + _, err := client.resolveAccountID(context.Background()) + require.Error(t, err) + + got, err := client.resolveAccountID(context.Background()) + require.NoError(t, err) + assert.Equal(t, acct, got) + + // A successful lookup is cached: no third STS call. + got, err = client.resolveAccountID(context.Background()) + require.NoError(t, err) + assert.Equal(t, acct, got) + mockSTS.AssertNumberOfCalls(t, "GetCallerIdentity", 2) +}