diff --git a/.jules/bolt.md b/.jules/bolt.md index 91ce70b..0387da8 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -1,5 +1,9 @@ # Bolt's Journal +## 2026-03-09 - GCP Identity Token Client Caching +**Learning:** The package-level function `GetIDToken` in `internal/run/auth/auth.go` is called to authenticate requests to service proxies. Prior to caching, it fetched GCP credentials on every invocation using `google.FindDefaultCredentials`, causing high disk lookup and metadata server lookup latency (~300ms per call). Implementing thread-safe, stateful lazy-initialization and caching using `sync.Mutex` and package-level cache variable reduces subsequent token retrievals to sub-millisecond execution times. Using `context.Background()` during credentials discovery ensures that credentials remain valid even when request-scoped contexts are canceled. +**Action:** Always inspect auth/identity token retrieval paths to ensure that the discovered credentials and credentials discovery flow are fully cached and reuse thread-safe singleton state. + ## 2026-03-08 - GCP Logging Client Caching & Connection Longevity **Learning:** Establishing the GCP Stackdriver Logging client requires repeated Google credential discovery and connection establishment, causing high latency (~300ms) inside a reactive TUI interface. Caching `logadmin.Client` instances via a project-aware map with thread-safe `sync.Mutex` ensures subsequent streaming and log extraction operations are instantaneous. Crucially, calling `Close()` on individual stream terminations must be a no-op to prevent premature teardown of connection pools shared across other active streaming views. **Action:** Keep GCP Logging clients cached globally by project and handle connection termination via a no-op `Close` method, while adding test-isolation resets in unit tests. diff --git a/internal/run/auth/auth.go b/internal/run/auth/auth.go index 6f8a3db..10e3ed1 100644 --- a/internal/run/auth/auth.go +++ b/internal/run/auth/auth.go @@ -24,6 +24,7 @@ import ( "os/user" "path/filepath" "strings" + "sync" api_region "github.com/JulienBreux/run-cli/internal/run/api/region" "github.com/JulienBreux/run-cli/internal/run/model/common/info" @@ -130,12 +131,30 @@ func parseConfig(path string) (info.Info, error) { }, nil } +// Package-level variables for credentials caching and dependency injection. +var ( + idTokenCreds *google.Credentials + idTokenCredsMu sync.Mutex + findDefaultCredentials = google.FindDefaultCredentials +) + // GetIDToken retrieves an identity token for the given audience using Google Cloud credentials. +// It is thread-safe and caches the discovered credentials to eliminate repetitive discovery +// overhead, filesystem/disk reads, and metadata server lookups (~300ms latency per call). var GetIDToken = func(ctx context.Context) (string, error) { - creds, err := google.FindDefaultCredentials(ctx, scopes...) - if err != nil { - return "", fmt.Errorf("failed to find default credentials: %w", err) + idTokenCredsMu.Lock() + if idTokenCreds == nil { + // Discover credentials using a background context to ensure they remain valid + // even if the calling request context is cancelled. + creds, err := findDefaultCredentials(context.Background(), scopes...) + if err != nil { + idTokenCredsMu.Unlock() + return "", fmt.Errorf("failed to find default credentials: %w", err) + } + idTokenCreds = creds } + creds := idTokenCreds + idTokenCredsMu.Unlock() token, err := creds.TokenSource.Token() if err != nil { diff --git a/internal/run/auth/auth_test.go b/internal/run/auth/auth_test.go index 6dcffa4..d7618a6 100644 --- a/internal/run/auth/auth_test.go +++ b/internal/run/auth/auth_test.go @@ -17,11 +17,17 @@ limitations under the License. package auth import ( + "context" + "errors" "os" "path/filepath" + "strings" + "sync" "testing" api_region "github.com/JulienBreux/run-cli/internal/run/api/region" + "golang.org/x/oauth2" + "golang.org/x/oauth2/google" ) func TestGetInfo(t *testing.T) { @@ -133,3 +139,134 @@ func TestGetInfo_Defaults(t *testing.T) { t.Errorf("Expected default Region 'all', got '%s'", info.Region) } } + +type mockTokenSource struct { + token *oauth2.Token + err error +} + +func (m *mockTokenSource) Token() (*oauth2.Token, error) { + return m.token, m.err +} + +func TestGetIDToken_CachingAndThreadSafety(t *testing.T) { + // Save and restore the original package-level findDefaultCredentials + origFindCreds := findDefaultCredentials + defer func() { + findDefaultCredentials = origFindCreds + // Reset credentials cache + idTokenCredsMu.Lock() + idTokenCreds = nil + idTokenCredsMu.Unlock() + }() + + // Mock TokenSource that returns a token with "id_token" extra parameter + mockTS := &mockTokenSource{ + token: (&oauth2.Token{}).WithExtra(map[string]interface{}{ + "id_token": "my-id-token", + }), + } + + credsDiscoveryCount := 0 + var discoveryMu sync.Mutex + + // Mock findDefaultCredentials + findDefaultCredentials = func(ctx context.Context, scopes ...string) (*google.Credentials, error) { + discoveryMu.Lock() + credsDiscoveryCount++ + discoveryMu.Unlock() + return &google.Credentials{ + TokenSource: mockTS, + }, nil + } + + t.Run("Caching and Reuse", func(t *testing.T) { + // Reset credentials cache + idTokenCredsMu.Lock() + idTokenCreds = nil + idTokenCredsMu.Unlock() + discoveryMu.Lock() + credsDiscoveryCount = 0 + discoveryMu.Unlock() + + ctx := context.Background() + + // First call - should discover credentials + token1, err := GetIDToken(ctx) + if err != nil { + t.Fatalf("Expected no error, got %v", err) + } + if token1 != "my-id-token" { + t.Errorf("Expected token 'my-id-token', got '%s'", token1) + } + + // Second call - should reuse cached credentials and not discover them again + token2, err := GetIDToken(ctx) + if err != nil { + t.Fatalf("Expected no error, got %v", err) + } + if token2 != "my-id-token" { + t.Errorf("Expected token 'my-id-token', got '%s'", token2) + } + + // Check discovery count + discoveryMu.Lock() + count := credsDiscoveryCount + discoveryMu.Unlock() + if count != 1 { + t.Errorf("Expected findDefaultCredentials to be called exactly once, got %d", count) + } + }) + + t.Run("Thread Safety", func(t *testing.T) { + // Reset credentials cache + idTokenCredsMu.Lock() + idTokenCreds = nil + idTokenCredsMu.Unlock() + discoveryMu.Lock() + credsDiscoveryCount = 0 + discoveryMu.Unlock() + + ctx := context.Background() + var wg sync.WaitGroup + const numGoroutines = 10 + + for i := 0; i < numGoroutines; i++ { + wg.Add(1) + go func() { + defer wg.Done() + _, _ = GetIDToken(ctx) + }() + } + wg.Wait() + + // Check discovery count - even with concurrent calls, discovery should only happen once + discoveryMu.Lock() + count := credsDiscoveryCount + discoveryMu.Unlock() + if count != 1 { + t.Errorf("Expected findDefaultCredentials to be called exactly once concurrently, got %d", count) + } + }) + + t.Run("Discovery Error Handling", func(t *testing.T) { + // Reset credentials cache + idTokenCredsMu.Lock() + idTokenCreds = nil + idTokenCredsMu.Unlock() + + // Mock discovery failure + findDefaultCredentials = func(ctx context.Context, scopes ...string) (*google.Credentials, error) { + return nil, errors.New("simulated discovery failure") + } + + ctx := context.Background() + _, err := GetIDToken(ctx) + if err == nil { + t.Fatal("Expected an error from discovery failure, got nil") + } + if !strings.Contains(err.Error(), "simulated discovery failure") { + t.Errorf("Expected error to mention simulated discovery failure, got %v", err) + } + }) +}