Skip to content

Commit 9af7844

Browse files
committed
feat: propagate caller context through credential providers
Add an optional Context field to credentials.CredContext and thread it through every credential-internal HTTP request: the SSO portal call, the IAM/IMDS flows, and the STS providers. Provider-level timeout bounds still apply; a nil Context keeps today's behavior. Client call sites attach the caller context. newRequest serves cached credentials inline and runs retrievals through the per-bucket group with per-caller cancellation, detached from any single caller so one caller's cancellation cannot fail concurrent waiters. Fixes #2266
1 parent 802bd60 commit 9af7844

17 files changed

Lines changed: 524 additions & 47 deletions

api-cred-context_test.go

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
/*
2+
* MinIO Go Library for Amazon S3 Compatible Cloud Storage
3+
* Copyright 2026 MinIO, Inc.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package minio
19+
20+
import (
21+
"context"
22+
"errors"
23+
"net/http"
24+
"net/http/httptest"
25+
"sync"
26+
"testing"
27+
"time"
28+
29+
"github.com/minio/minio-go/v7/pkg/credentials"
30+
)
31+
32+
// blockingProvider is a credentials.Provider whose retrieval blocks until
33+
// released, honoring the CredContext caller context like real providers do.
34+
type blockingProvider struct {
35+
startedOnce sync.Once
36+
started chan struct{}
37+
release chan struct{}
38+
}
39+
40+
func (p *blockingProvider) RetrieveWithCredContext(cc *credentials.CredContext) (credentials.Value, error) {
41+
p.startedOnce.Do(func() { close(p.started) })
42+
var done <-chan struct{}
43+
if cc != nil && cc.Context != nil {
44+
done = cc.Context.Done()
45+
}
46+
select {
47+
case <-p.release:
48+
case <-done:
49+
return credentials.Value{}, cc.Context.Err()
50+
}
51+
return credentials.Value{
52+
AccessKeyID: "accessKey",
53+
SecretAccessKey: "secret",
54+
SignerType: credentials.SignatureV4,
55+
}, nil
56+
}
57+
58+
func (p *blockingProvider) Retrieve() (credentials.Value, error) {
59+
return p.RetrieveWithCredContext(nil)
60+
}
61+
62+
func (p *blockingProvider) IsExpired() bool { return true }
63+
64+
// TestCredsCancelDoesNotPoisonWaiters verifies that canceling one caller
65+
// during a de-duplicated credential retrieval fails only that caller: the
66+
// shared retrieval is detached from any single caller's context, so a
67+
// concurrent waiter on the same bucket still succeeds.
68+
func TestCredsCancelDoesNotPoisonWaiters(t *testing.T) {
69+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
70+
w.WriteHeader(http.StatusOK)
71+
}))
72+
defer srv.Close()
73+
74+
provider := &blockingProvider{started: make(chan struct{}), release: make(chan struct{})}
75+
clnt, err := New(srv.Listener.Addr().String(), &Options{
76+
Creds: credentials.New(provider),
77+
Region: "us-east-1",
78+
})
79+
if err != nil {
80+
t.Fatal(err)
81+
}
82+
83+
ctx, cancel := context.WithCancel(context.Background())
84+
defer cancel()
85+
86+
canceledErr := make(chan error, 1)
87+
go func() {
88+
_, err := clnt.BucketExists(ctx, "test-bucket")
89+
canceledErr <- err
90+
}()
91+
92+
<-provider.started
93+
94+
waiterErr := make(chan error, 1)
95+
go func() {
96+
_, err := clnt.BucketExists(context.Background(), "test-bucket")
97+
waiterErr <- err
98+
}()
99+
100+
// Cancel the first caller mid-retrieval; the waiter must not inherit
101+
// that cancellation, whether it blocks on the credentials mutex or
102+
// joins the de-dup group.
103+
cancel()
104+
105+
if err := <-canceledErr; !errors.Is(err, context.Canceled) {
106+
t.Fatalf("Expected context.Canceled for the canceled caller, got %v", err)
107+
}
108+
109+
close(provider.release)
110+
111+
if err := <-waiterErr; err != nil {
112+
t.Fatalf("Expected the concurrent waiter to succeed, got %v", err)
113+
}
114+
}
115+
116+
// panicProvider is a credentials.Provider whose retrieval always panics.
117+
type panicProvider struct{}
118+
119+
func (panicProvider) RetrieveWithCredContext(*credentials.CredContext) (credentials.Value, error) {
120+
panic("boom")
121+
}
122+
123+
func (p panicProvider) Retrieve() (credentials.Value, error) {
124+
return p.RetrieveWithCredContext(nil)
125+
}
126+
127+
func (panicProvider) IsExpired() bool { return true }
128+
129+
// TestCredsRetrievalPanicPropagates verifies that a provider panic inside
130+
// the de-duplicated credential retrieval resumes on the caller's goroutine
131+
// instead of crashing the process from the retrieval goroutine.
132+
func TestCredsRetrievalPanicPropagates(t *testing.T) {
133+
clnt, err := New("s3.amazonaws.com", &Options{
134+
Creds: credentials.New(panicProvider{}),
135+
Region: "us-east-1",
136+
})
137+
if err != nil {
138+
t.Fatal(err)
139+
}
140+
141+
defer func() {
142+
if r := recover(); r != "boom" {
143+
t.Fatalf("Expected panic value %q on the caller goroutine, got %v", "boom", r)
144+
}
145+
}()
146+
exists, err := clnt.BucketExists(context.Background(), "test-bucket")
147+
t.Fatalf("Expected a panic before return, got exists=%v err=%v", exists, err)
148+
}
149+
150+
// TestPresignCredsCallerContext verifies that a direct (non-de-duplicated)
151+
// credential retrieval receives the caller context: a canceled caller
152+
// context aborts presigning inside the provider.
153+
func TestPresignCredsCallerContext(t *testing.T) {
154+
provider := &blockingProvider{started: make(chan struct{}), release: make(chan struct{})}
155+
clnt, err := New("s3.amazonaws.com", &Options{
156+
Creds: credentials.New(provider),
157+
Region: "us-east-1",
158+
})
159+
if err != nil {
160+
t.Fatal(err)
161+
}
162+
163+
ctx, cancel := context.WithCancel(context.Background())
164+
cancel()
165+
166+
policy := NewPostPolicy()
167+
if err := policy.SetBucket("test-bucket"); err != nil {
168+
t.Fatal(err)
169+
}
170+
if err := policy.SetKey("obj"); err != nil {
171+
t.Fatal(err)
172+
}
173+
if err := policy.SetExpires(time.Now().UTC().Add(time.Hour)); err != nil {
174+
t.Fatal(err)
175+
}
176+
177+
_, _, err = clnt.PresignedPostPolicy(ctx, policy)
178+
if !errors.Is(err, context.Canceled) {
179+
t.Fatalf("Expected context.Canceled from the credential provider, got %v", err)
180+
}
181+
}

api-presigned.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ func (c *Client) PresignedPostPolicy(ctx context.Context, p *PostPolicy) (u *url
140140
}
141141

142142
// Get credentials from the configured credentials provider.
143-
credValues, err := c.credsProvider.GetWithContext(c.CredContext())
143+
credValues, err := c.credsProvider.GetWithContext(c.credContext(ctx))
144144
if err != nil {
145145
return nil, nil, err
146146
}

api.go

Lines changed: 67 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -848,8 +848,20 @@ func (c *Client) executeMethod(ctx context.Context, method string, metadata requ
848848
return res, err
849849
}
850850

851+
// credsRetrievalPanic carries a panic value out of the de-duplicated
852+
// credential retrieval goroutine so newRequest can resume it on each
853+
// caller's goroutine.
854+
type credsRetrievalPanic struct{ value any }
855+
856+
func (p credsRetrievalPanic) Error() string {
857+
return fmt.Sprintf("credentials retrieval panicked: %v", p.value)
858+
}
859+
851860
// newRequest - instantiate a new HTTP request for a given method.
852861
func (c *Client) newRequest(ctx context.Context, method string, metadata requestMetadata) (req *http.Request, err error) {
862+
if ctx == nil {
863+
return nil, errInvalidArgument("context cannot be nil")
864+
}
853865
// If no method is supplied default to 'POST'.
854866
if method == "" {
855867
method = http.MethodPost
@@ -882,23 +894,58 @@ func (c *Client) newRequest(ctx context.Context, method string, metadata request
882894
return nil, err
883895
}
884896

885-
if c.httpTrace != nil {
886-
ctx = httptrace.WithClientTrace(ctx, c.httpTrace)
887-
}
888-
889-
// make sure to de-dup calls to credential services, this reduces
890-
// the overall load to the endpoint generating credential service.
891-
value, err, _ := c.credsGroup.Do(metadata.bucketName, func() (credentials.Value, error) {
892-
if s3utils.IsS3ExpressBucket(metadata.bucketName) && s3utils.IsAmazonEndpoint(*c.endpointURL) {
893-
return c.CreateSession(ctx, metadata.bucketName, SessionReadWrite)
897+
// Cached, unexpired credentials (or an absent provider — anonymous
898+
// access) are served inline; a retrieval that races expiry runs
899+
// inline too, serialized by the Credentials mutex. S3 Express
900+
// sessions always go through the de-dup group.
901+
express := s3utils.IsS3ExpressBucket(metadata.bucketName) && s3utils.IsAmazonEndpoint(*c.endpointURL)
902+
var value credentials.Value
903+
if !express && (c.credsProvider == nil || !c.credsProvider.IsExpired()) {
904+
value, err = c.credsProvider.GetWithContext(c.credContext(ctx))
905+
} else {
906+
// The provider retrieval is detached (context.WithoutCancel) so
907+
// one caller's cancellation cannot fail concurrent waiters; each
908+
// waiter stops waiting when its own context ends, though a
909+
// caller arriving mid-retrieval blocks in IsExpired on the
910+
// Credentials mutex until the retrieval completes. The S3
911+
// Express session request keeps the caller context: a full S3
912+
// operation's retries must stay cancellable, so its waiters
913+
// share the winner's fate. A retrieval panic resumes on each
914+
// waiting caller's goroutine (credsRetrievalPanic); a
915+
// runtime.Goexit is not propagated — waiters wait out their own
916+
// contexts.
917+
resCh := c.credsGroup.DoChan(metadata.bucketName, func() (v credentials.Value, rerr error) {
918+
defer func() {
919+
if r := recover(); r != nil {
920+
rerr = credsRetrievalPanic{value: r}
921+
}
922+
}()
923+
if express {
924+
return c.CreateSession(ctx, metadata.bucketName, SessionReadWrite)
925+
}
926+
// Get credentials from the configured credentials provider.
927+
return c.credsProvider.GetWithContext(c.credContext(context.WithoutCancel(ctx)))
928+
})
929+
select {
930+
case res := <-resCh:
931+
if cp, ok := res.Err.(credsRetrievalPanic); ok {
932+
panic(cp.value)
933+
}
934+
value, err = res.Val, res.Err
935+
case <-ctx.Done():
936+
return nil, ctx.Err()
894937
}
895-
// Get credentials from the configured credentials provider.
896-
return c.credsProvider.GetWithContext(c.CredContext())
897-
})
938+
}
898939
if err != nil {
899940
return nil, err
900941
}
901942

943+
// Attach the trace after credential retrieval so credential
944+
// requests stay untraced.
945+
if c.httpTrace != nil {
946+
ctx = httptrace.WithClientTrace(ctx, c.httpTrace)
947+
}
948+
902949
// Initialize a new HTTP request for the method.
903950
req, err = http.NewRequestWithContext(ctx, method, targetURL.String(), nil)
904951
if err != nil {
@@ -1172,6 +1219,14 @@ func (c *Client) CredContext() *credentials.CredContext {
11721219
}
11731220
}
11741221

1222+
// credContext returns the client's CredContext with ctx attached as the
1223+
// caller context for credential retrieval.
1224+
func (c *Client) credContext(ctx context.Context) *credentials.CredContext {
1225+
cc := c.CredContext()
1226+
cc.Context = ctx
1227+
return cc
1228+
}
1229+
11751230
// GetCreds returns the access creds for the client
11761231
func (c *Client) GetCreds() (credentials.Value, error) {
11771232
if c.credsProvider == nil {

bucket-cache.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ func (c *Client) getBucketLocationRequest(ctx context.Context, bucketName string
172172
c.setUserAgent(req)
173173

174174
// Get credentials from the configured credentials provider.
175-
value, err := c.credsProvider.GetWithContext(c.CredContext())
175+
value, err := c.credsProvider.GetWithContext(c.credContext(ctx))
176176
if err != nil {
177177
return nil, err
178178
}

create-session.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ func (c *Client) createSessionRequest(ctx context.Context, bucketName string, se
143143
c.setUserAgent(req)
144144

145145
// Get credentials from the configured credentials provider.
146-
value, err := c.credsProvider.GetWithContext(c.CredContext())
146+
value, err := c.credsProvider.GetWithContext(c.credContext(ctx))
147147
if err != nil {
148148
return nil, err
149149
}

pkg/credentials/assume_role.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package credentials
1919

2020
import (
2121
"bytes"
22+
"context"
2223
"crypto/sha256"
2324
"encoding/hex"
2425
"encoding/xml"
@@ -142,7 +143,7 @@ func closeResponse(resp *http.Response) {
142143
}
143144
}
144145

145-
func getAssumeRoleCredentials(clnt *http.Client, endpoint string, opts STSAssumeRoleOptions) (AssumeRoleResponse, error) {
146+
func getAssumeRoleCredentials(ctx context.Context, clnt *http.Client, endpoint string, opts STSAssumeRoleOptions) (AssumeRoleResponse, error) {
146147
v := url.Values{}
147148
v.Set("Action", "AssumeRole")
148149
v.Set("Version", STSVersion)
@@ -180,7 +181,7 @@ func getAssumeRoleCredentials(clnt *http.Client, endpoint string, opts STSAssume
180181
}
181182
postBody.Seek(0, 0)
182183

183-
req, err := http.NewRequest(http.MethodPost, u.String(), postBody)
184+
req, err := http.NewRequestWithContext(ctx, http.MethodPost, u.String(), postBody)
184185
if err != nil {
185186
return AssumeRoleResponse{}, err
186187
}
@@ -245,7 +246,7 @@ func (m *STSAssumeRole) RetrieveWithCredContext(cc *CredContext) (Value, error)
245246
return Value{}, errors.New("STS endpoint unknown")
246247
}
247248

248-
a, err := getAssumeRoleCredentials(client, stsEndpoint, m.Options)
249+
a, err := getAssumeRoleCredentials(cc.requestContext(), client, stsEndpoint, m.Options)
249250
if err != nil {
250251
return Value{}, err
251252
}

0 commit comments

Comments
 (0)