diff --git a/CHANGELOG.md b/CHANGELOG.md index cc2618f4..e47fcc6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.11.1] - 2026-10-23 + +### Fixed +- Reuse authenticated OpenStack clients across reconciliations with identical provider credentials, reducing Keystone authentication requests. Failed authentication is retried on the next reconciliation, cached clients expire before their tokens and are invalidated on HTTP 401, and shared reauthentication is limited to 30 seconds. Swift authentication remains uncached. + ## [0.11.0] - 2026-10-07 ### Added diff --git a/go.mod b/go.mod index eb2d4c6c..7eaa4daa 100644 --- a/go.mod +++ b/go.mod @@ -10,11 +10,13 @@ require ( github.com/crossplane/crossplane-runtime/v2 v2.1.0 github.com/crossplane/crossplane-tools v0.0.0-20251017183449-dd4517244339 github.com/crossplane/upjet/v2 v2.2.0 + github.com/gophercloud/gophercloud/v2 v2.8.0 github.com/hashicorp/go-cty v1.5.0 github.com/hashicorp/terraform-json v0.27.2 github.com/hashicorp/terraform-plugin-sdk/v2 v2.38.2 github.com/pkg/errors v0.9.1 github.com/terraform-provider-openstack/terraform-provider-openstack/v3 v3.4.0 + golang.org/x/sync v0.19.0 google.golang.org/grpc v1.79.2 k8s.io/api v0.35.1 k8s.io/apiextensions-apiserver v0.35.1 @@ -56,7 +58,6 @@ require ( github.com/google/gnostic-models v0.7.0 // indirect github.com/google/go-cmp v0.7.0 // indirect github.com/google/uuid v1.6.0 // indirect - github.com/gophercloud/gophercloud/v2 v2.8.0 // indirect github.com/gophercloud/utils/v2 v2.0.0-20250710092215-8f6f0255f600 // indirect github.com/hashicorp/go-hclog v1.6.3 // indirect github.com/hashicorp/go-plugin v1.7.0 // indirect @@ -118,7 +119,6 @@ require ( golang.org/x/mod v0.31.0 // indirect golang.org/x/net v0.48.0 // indirect golang.org/x/oauth2 v0.34.0 // indirect - golang.org/x/sync v0.19.0 // indirect golang.org/x/sys v0.40.0 // indirect golang.org/x/term v0.38.0 // indirect golang.org/x/text v0.33.0 // indirect diff --git a/internal/clients/cache.go b/internal/clients/cache.go new file mode 100644 index 00000000..45e80e4b --- /dev/null +++ b/internal/clients/cache.go @@ -0,0 +1,254 @@ +package clients + +import ( + "container/list" + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "net/http" + "sync" + "time" + + tokens2 "github.com/gophercloud/gophercloud/v2/openstack/identity/v2/tokens" + tokens3 "github.com/gophercloud/gophercloud/v2/openstack/identity/v3/tokens" + osprovider "github.com/terraform-provider-openstack/terraform-provider-openstack/v3/openstack" + "golang.org/x/sync/singleflight" +) + +const ( + defaultNoForkClientCacheSize = 128 + noForkTokenExpiryClockSkew = 5 * time.Minute + noForkReauthTimeout = 30 * time.Second +) + +var errNoForkInitializationCanceled = errors.New("OpenStack client initialization canceled") + +// noForkClientCache shares authenticated provider Meta values across reconciles +// with identical configurations. Only successful Keystone authentication with a +// known token expiry is cached; failures and Swift authentication stay uncached. +// A separate configured provider copy is still needed for each configuration so +// that credentials from different ProviderConfigs cannot mix. +type noForkClientCache struct { + maxSize int + now func() time.Time + mu sync.Mutex + entries map[string]*list.Element + // order contains exactly one element per entry, oldest first. Expiration + // and eviction remove entries from both structures under the same lock. + order list.List + inflight singleflight.Group +} + +type noForkClientCacheEntry struct { + key string + meta any + expires time.Time +} + +func newNoForkClientCache(maxSize int) *noForkClientCache { + if maxSize <= 0 { + maxSize = defaultNoForkClientCacheSize + } + return &noForkClientCache{ + maxSize: maxSize, + now: time.Now, + entries: make(map[string]*list.Element), + } +} + +// getOrCreate coalesces simultaneous misses for one configuration without +// blocking initialization of other configurations. A waiting reconcile can +// cancel independently. The creator must not mutate the caller's Setup because +// creation may finish after that caller has canceled. +func (c *noForkClientCache) getOrCreate(ctx context.Context, config map[string]any, create func() (any, error)) (any, error) { + key, err := hashNoForkClientConfig(config) + if err != nil { + return nil, err + } + for { + if err := ctx.Err(); err != nil { + return nil, err + } + if meta, ok := c.get(key); ok { + return meta, nil + } + + result := c.inflight.DoChan(key, func() (any, error) { + return c.initialize(ctx, key, create) + }) + select { + case <-ctx.Done(): + return nil, ctx.Err() + case r := <-result: + // A live caller must not inherit the initializing caller's canceled + // context. The failed flight is already removed, so retrying here + // coalesces a fresh initialization under a live caller's context. + if errors.Is(r.Err, errNoForkInitializationCanceled) { + continue + } + return r.Val, r.Err + } + } +} + +func (c *noForkClientCache) initialize(ctx context.Context, key string, create func() (any, error)) (any, error) { + // Another creator may have populated the cache since the first check. + if meta, ok := c.get(key); ok { + return meta, nil + } + meta, err := create() + if err != nil { + if ctx.Err() != nil { + return nil, errors.Join(errNoForkInitializationCanceled, ctx.Err(), err) + } + return nil, err + } + if _, ok := tokenExpiry(meta, c.now()); ok { + cfg := meta.(*osprovider.Config) + base := cfg.OsClient.HTTPClient.Transport + if base == nil { + base = http.DefaultTransport + } + // Install hooks before publication. A revoked token must not remain + // sticky until its scheduled expiry, even with allow_reauth=false. + cfg.OsClient.HTTPClient.Transport = &noForkClientTransport{ + base: base, + invalidate: func() { c.invalidate(key, cfg) }, + } + cfg.OsClient.ReauthFunc = withReauthTimeout(cfg.OsClient.ReauthFunc, noForkReauthTimeout) + } + c.set(key, meta) + return meta, nil +} + +func (c *noForkClientCache) get(key string) (any, bool) { + c.mu.Lock() + defer c.mu.Unlock() + + element, ok := c.entries[key] + if !ok { + return nil, false + } + entry := element.Value.(*noForkClientCacheEntry) + if !c.now().Before(entry.expires) { + c.remove(element) + return nil, false + } + return entry.meta, true +} + +func (c *noForkClientCache) set(key string, meta any) { + expires, ok := tokenExpiry(meta, c.now()) + if !ok { + return + } + + c.mu.Lock() + defer c.mu.Unlock() + + entry := &noForkClientCacheEntry{key: key, meta: meta, expires: expires} + if element, ok := c.entries[key]; ok { + element.Value = entry + return + } + if len(c.entries) >= c.maxSize { + c.remove(c.order.Front()) + } + c.entries[key] = c.order.PushBack(entry) +} + +// remove requires c.mu to be held. +func (c *noForkClientCache) remove(element *list.Element) { + delete(c.entries, element.Value.(*noForkClientCacheEntry).key) + c.order.Remove(element) +} + +func (c *noForkClientCache) invalidate(key string, meta *osprovider.Config) { + c.mu.Lock() + defer c.mu.Unlock() + // In-flight operations may still use an evicted client. A late 401 from + // that generation must not invalidate its replacement. + if element, ok := c.entries[key]; ok && element.Value.(*noForkClientCacheEntry).meta == meta { + c.remove(element) + } +} + +type noForkClientTransport struct { + base http.RoundTripper + invalidate func() +} + +func (t *noForkClientTransport) RoundTrip(request *http.Request) (*http.Response, error) { + response, err := t.base.RoundTrip(request) + if response != nil && response.StatusCode == http.StatusUnauthorized { + t.invalidate() + } + return response, err +} + +// Gophercloud's shared reauthentication wait does not observe each waiting +// request's context. Bound the authentication itself so a stalled Keystone +// request cannot hold other reconciles until an async operation's hour deadline. +func withReauthTimeout(reauth func(context.Context) error, timeout time.Duration) func(context.Context) error { + if reauth == nil { + return nil + } + return func(ctx context.Context) error { + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + return reauth(ctx) + } +} + +// JSON orders map keys deterministically. Hash the whole provider configuration +// so credential rotation or a change in scope/region never reuses an old client. +// Only the digest is retained as the key; credentials are never logged. +func hashNoForkClientConfig(config map[string]any) (string, error) { + data, err := json.Marshal(config) + if err != nil { + return "", err + } + sum := sha256.Sum256(data) + return hex.EncodeToString(sum[:]), nil +} + +// tokenExpiry uses the token obtained before cache insertion. Without a known +// expiry we cannot safely reuse a client, particularly with allow_reauth=false. +func tokenExpiry(meta any, now time.Time) (time.Time, bool) { + cfg, ok := meta.(*osprovider.Config) + if !ok || cfg == nil || cfg.Swauth || cfg.OsClient == nil || cfg.OsClient.Token() == "" { + return time.Time{}, false + } + + expiresAt := keystoneTokenExpiry(cfg.OsClient.GetAuthResult()) + remaining := expiresAt.Sub(now) + if remaining <= 0 { + return time.Time{}, false + } + // A fixed five-minute margin would disable caching for short-lived tokens. + // Reserve ten percent of their remaining lifetime, capped at five minutes. + margin := min(noForkTokenExpiryClockSkew, remaining/10) + return expiresAt.Add(-margin), true +} + +func keystoneTokenExpiry(result any) time.Time { + switch result := result.(type) { + case interface { + ExtractToken() (*tokens3.Token, error) + }: + token, err := result.ExtractToken() + if err == nil && token != nil { + return token.ExpiresAt + } + case interface { + ExtractToken() (*tokens2.Token, error) + }: + token, err := result.ExtractToken() + if err == nil && token != nil { + return token.ExpiresAt + } + } + return time.Time{} +} diff --git a/internal/clients/cache_recovery_test.go b/internal/clients/cache_recovery_test.go new file mode 100644 index 00000000..bd4009a4 --- /dev/null +++ b/internal/clients/cache_recovery_test.go @@ -0,0 +1,130 @@ +package clients + +import ( + "context" + "errors" + "net/http" + "testing" + "time" + + "github.com/crossplane/upjet/v2/pkg/terraform" + "github.com/gophercloud/gophercloud/v2" + osprovider "github.com/terraform-provider-openstack/terraform-provider-openstack/v3/openstack" +) + +func cacheRecoveryRequest(t *testing.T, ctx context.Context, client *gophercloud.ProviderClient, endpoint string) error { + t.Helper() + response, err := client.Request(ctx, http.MethodGet, endpoint, &gophercloud.RequestOpts{}) + if response != nil { + if closeErr := response.Body.Close(); closeErr != nil { + t.Errorf("close service response: %v", closeErr) + } + } + return err +} + +func TestNoForkClientCacheInvalidatesRevokedToken(t *testing.T) { + clearOpenstackTestEnvironment(t) + keystone := newKeystoneTestServer(t, time.Hour, nil) + keystone.onRequest = func(w http.ResponseWriter, r *http.Request) bool { + if r.URL.Path != "/revoked" { + return false + } + w.WriteHeader(http.StatusUnauthorized) + return true + } + cache := newNoForkClientCache(1) + provider := osprovider.Provider() + configuration := keystoneTestConfiguration(keystone) + configuration["allow_reauth"] = "false" + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + first := terraform.Setup{Configuration: configuration} + if err := cache.configure(ctx, &first, provider); err != nil { + t.Fatal(err) + } + old := authenticatedOpenstackTestConfig(t, first) + if err := cacheRecoveryRequest(t, ctx, old.OsClient, keystone.server.URL+"/revoked"); err == nil { + t.Fatal("expected an authentication rejection") + } + if got := keystone.tokenRequests.Load(); got != 1 { + t.Fatalf("allow_reauth=false made an automatic token request: %d", got) + } + second := terraform.Setup{Configuration: configuration} + if err := cache.configure(ctx, &second, provider); err != nil { + t.Fatal(err) + } + if second.Meta == first.Meta || keystone.tokenRequests.Load() != 2 { + t.Fatal("a revoked token remained cached until its scheduled expiry") + } + // An old asynchronous operation can reject the old token after the + // replacement is already cached. It must not evict that replacement. + if err := cacheRecoveryRequest(t, ctx, old.OsClient, keystone.server.URL+"/revoked"); err == nil { + t.Fatal("expected the old token to remain rejected") + } + third := terraform.Setup{Configuration: configuration} + if err := cache.configure(ctx, &third, provider); err != nil { + t.Fatal(err) + } + if third.Meta != second.Meta || keystone.tokenRequests.Load() != 2 { + t.Fatal("a late rejection invalidated the replacement generation") + } +} + +func TestNoForkClientCacheAutomaticReauthentication(t *testing.T) { + clearOpenstackTestEnvironment(t) + keystone := newKeystoneTestServer(t, time.Hour, nil) + keystone.onRequest = func(w http.ResponseWriter, r *http.Request) bool { + if r.URL.Path != "/service" { + return false + } + if r.Header.Get("X-Auth-Token") == "dummy-token-1" { + w.WriteHeader(http.StatusUnauthorized) + } else { + w.WriteHeader(http.StatusOK) + } + return true + } + cache := newNoForkClientCache(1) + provider := osprovider.Provider() + setup := terraform.Setup{Configuration: keystoneTestConfiguration(keystone)} + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + if err := cache.configure(ctx, &setup, provider); err != nil { + t.Fatal(err) + } + cfg := authenticatedOpenstackTestConfig(t, setup) + if err := cacheRecoveryRequest(t, ctx, cfg.OsClient, keystone.server.URL+"/service"); err != nil { + t.Fatalf("automatic reauthentication failed: %v", err) + } + if cfg.OsClient.Token() != "dummy-token-2" || keystone.tokenRequests.Load() != 2 { + t.Fatal("the existing client did not automatically refresh its rejected token") + } +} + +func TestNoForkClientCacheReauthenticationTimeout(t *testing.T) { + if withReauthTimeout(nil, time.Second) != nil { + t.Fatal("a client without automatic reauthentication must remain unchanged") + } + t.Run("bounded", func(t *testing.T) { + wrapped := withReauthTimeout(func(ctx context.Context) error { + deadline, ok := ctx.Deadline() + if !ok || time.Until(deadline) > time.Millisecond { + t.Error("reauthentication has no bounded deadline") + } + <-ctx.Done() + return ctx.Err() + }, time.Millisecond) + if err := wrapped(t.Context()); !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected a bounded authentication timeout, got %v", err) + } + }) + t.Run("canceled parent", func(t *testing.T) { + ctx, cancel := context.WithCancel(t.Context()) + cancel() + wrapped := withReauthTimeout(func(ctx context.Context) error { return ctx.Err() }, time.Minute) + if err := wrapped(ctx); !errors.Is(err, context.Canceled) { + t.Fatalf("authentication ignored parent cancellation: %v", err) + } + }) +} diff --git a/internal/clients/cache_swift_test.go b/internal/clients/cache_swift_test.go new file mode 100644 index 00000000..e9c165aa --- /dev/null +++ b/internal/clients/cache_swift_test.go @@ -0,0 +1,62 @@ +package clients + +import ( + "sync" + "testing" + + "github.com/crossplane/upjet/v2/pkg/terraform" + osprovider "github.com/terraform-provider-openstack/terraform-provider-openstack/v3/openstack" +) + +func TestNoForkClientCacheSwiftNotShared(t *testing.T) { + for _, fromEnvironment := range []bool{false, true} { + name := "explicit" + if fromEnvironment { + name = "environment default" + } + t.Run(name, func(t *testing.T) { + configuration := configFor("dummy-password") + if fromEnvironment { + t.Setenv("OS_SWAUTH", "true") + } else { + configuration["swauth"] = "true" + } + // The unsafe non-delayed Swift path must also bypass sharing. + configuration["delayed_auth"] = "false" + cache := newNoForkClientCache(1) + provider := osprovider.Provider() + const callers = 10 + metas := make(chan *osprovider.Config, callers) + var wg sync.WaitGroup + for range callers { + wg.Add(1) + go func() { + defer wg.Done() + setup := terraform.Setup{Configuration: configuration} + if err := cache.configure(t.Context(), &setup, provider); err != nil { + t.Errorf("Swift configuration failed: %v", err) + return + } + meta, ok := setup.Meta.(*osprovider.Config) + if !ok || !meta.Swauth { + t.Error("expected a Swift configuration") + return + } + metas <- meta + }() + } + wg.Wait() + close(metas) + seen := make(map[*osprovider.Config]bool) + for meta := range metas { + if seen[meta] { + t.Error("concurrent callers shared a Swift configuration") + } + seen[meta] = true + } + if len(seen) != callers || len(cache.entries) != 0 || cache.order.Len() != 0 { + t.Fatalf("Swift clients=%d, cache entries=%d, eviction records=%d", len(seen), len(cache.entries), cache.order.Len()) + } + }) + } +} diff --git a/internal/clients/cache_test.go b/internal/clients/cache_test.go new file mode 100644 index 00000000..038b71c2 --- /dev/null +++ b/internal/clients/cache_test.go @@ -0,0 +1,298 @@ +package clients + +import ( + "context" + "errors" + "net/http" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/gophercloud/gophercloud/v2" + "github.com/gophercloud/gophercloud/v2/openstack" + tokens2 "github.com/gophercloud/gophercloud/v2/openstack/identity/v2/tokens" + tokens3 "github.com/gophercloud/gophercloud/v2/openstack/identity/v3/tokens" + osprovider "github.com/terraform-provider-openstack/terraform-provider-openstack/v3/openstack" +) + +func configFor(secret string) map[string]any { + return map[string]any{ + "auth_url": "https://identity.example.com/v3", + "user_name": "crossplane", + "password": secret, + "insecure": "false", + } +} + +func cacheTestKey(t *testing.T, config map[string]any) string { + t.Helper() + key, err := hashNoForkClientConfig(config) + if err != nil { + t.Fatal(err) + } + return key +} + +func cacheTestClient(t *testing.T, expires time.Time) *osprovider.Config { + t.Helper() + result := tokens3.CreateResult{} + result.Header = http.Header{"X-Subject-Token": {"dummy-token"}} + result.Body = map[string]any{"token": map[string]any{"expires_at": expires.Format(time.RFC3339Nano)}} + return cacheTestClientWithResult(t, result) +} + +func cacheTestClientWithResult(t *testing.T, result gophercloud.AuthResult) *osprovider.Config { + t.Helper() + client, err := openstack.NewClient("https://identity.example.com/v3") + if err != nil { + t.Fatal(err) + } + if err := client.SetTokenAndAuthResult(result); err != nil { + t.Fatal(err) + } + cfg := &osprovider.Config{} + cfg.OsClient = client + return cfg +} + +func TestNoForkClientCacheHashConfig(t *testing.T) { + a := cacheTestKey(t, map[string]any{"auth_url": "https://identity.example.com/v3", "user_name": "crossplane"}) + b := cacheTestKey(t, map[string]any{"user_name": "crossplane", "auth_url": "https://identity.example.com/v3"}) + if a != b { + t.Fatal("identical configurations must hash identically") + } + for _, field := range []string{"password", "tenant_id", "region", "auth_url", "insecure"} { + t.Run(field, func(t *testing.T) { + config := configFor("dummy-password") + before := cacheTestKey(t, config) + config[field] = "changed" + if before == cacheTestKey(t, config) { + t.Fatal("changed configuration reused the same key") + } + }) + } +} + +func TestNoForkClientCacheGetSet(t *testing.T) { + cache := newNoForkClientCache(2) + key := cacheTestKey(t, configFor("dummy-password")) + if _, ok := cache.get(key); ok { + t.Fatal("expected an initial cache miss") + } + meta := cacheTestClient(t, time.Now().Add(time.Hour)) + cache.set(key, meta) + if got, ok := cache.get(key); !ok || got != meta { + t.Fatal("expected the original authenticated client") + } + if _, ok := cache.get(cacheTestKey(t, configFor("rotated-password"))); ok { + t.Fatal("rotated credentials must not reuse the previous client") + } +} + +func TestNoForkClientCacheExpirationAndEviction(t *testing.T) { + now := time.Now() + cache := newNoForkClientCache(1) + cache.now = func() time.Time { return now } + + // Reproduce stale FIFO records: expire A, insert B, then insert C. + cache.set("A", cacheTestClient(t, now.Add(time.Minute))) + now = now.Add(time.Minute) + if _, ok := cache.get("A"); ok { + t.Fatal("expired entry was returned") + } + if len(cache.entries) != 0 || cache.order.Len() != 0 { + t.Fatal("expiration left stale eviction records") + } + cache.set("B", cacheTestClient(t, now.Add(time.Hour))) + cache.set("C", cacheTestClient(t, now.Add(time.Hour))) + if _, ok := cache.get("B"); ok { + t.Fatal("oldest live entry was not evicted") + } + if _, ok := cache.get("C"); !ok { + t.Fatal("newest entry was evicted") + } + + // Repeated expiration/reinsertion must not grow either structure. + for range 300 { + now = now.Add(2 * time.Hour) + cache.get("C") + cache.set("C", cacheTestClient(t, now.Add(time.Hour))) + if len(cache.entries) != 1 || cache.order.Len() != 1 { + t.Fatalf("cache exceeded capacity: map=%d, order=%d", len(cache.entries), cache.order.Len()) + } + } +} + +func TestNoForkClientCacheReplaceEntry(t *testing.T) { + cache := newNoForkClientCache(2) + expires := time.Now().Add(time.Hour) + cache.set("A", cacheTestClient(t, expires)) + cache.set("B", cacheTestClient(t, expires)) + replacement := cacheTestClient(t, expires) + cache.set("A", replacement) + if got, ok := cache.get("A"); !ok || got != replacement { + t.Fatal("replacement entry was not returned") + } + if len(cache.entries) != 2 || cache.order.Len() != 2 { + t.Fatal("replacement changed the number of entries") + } + cache.set("C", cacheTestClient(t, expires)) + if _, ok := cache.get("A"); ok { + t.Fatal("replacement must not duplicate or reorder FIFO records") + } +} + +func TestNoForkClientCacheUncacheable(t *testing.T) { + now := time.Now() + swift := cacheTestClient(t, now.Add(time.Hour)) + swift.Swauth = true + clientWithoutToken, err := openstack.NewClient("https://identity.example.com/v3") + if err != nil { + t.Fatal(err) + } + unauthenticated := &osprovider.Config{} + unauthenticated.OsClient = clientWithoutToken + for _, tt := range []struct { + name string + meta any + }{ + {name: "nil"}, + {name: "typed nil", meta: (*osprovider.Config)(nil)}, + {name: "unknown meta", meta: "not-a-config"}, + {name: "missing client", meta: &osprovider.Config{}}, + {name: "unauthenticated", meta: unauthenticated}, + {name: "missing expiry", meta: cacheTestClient(t, time.Time{})}, + {name: "expired token", meta: cacheTestClient(t, now.Add(-time.Second))}, + {name: "Swift", meta: swift}, + } { + t.Run(tt.name, func(t *testing.T) { + cache := newNoForkClientCache(1) + cache.now = func() time.Time { return now } + cache.set("key", tt.meta) + if len(cache.entries) != 0 || cache.order.Len() != 0 { + t.Fatal("unsafe client was cached") + } + }) + } +} + +func TestNoForkClientCacheTokenExpiry(t *testing.T) { + now := time.Now().UTC() + for _, lifetime := range []time.Duration{time.Hour, 10 * time.Minute, time.Minute} { + t.Run(lifetime.String(), func(t *testing.T) { + meta := cacheTestClient(t, now.Add(lifetime)) + expires, ok := tokenExpiry(meta, now) + want := now.Add(lifetime - min(noForkTokenExpiryClockSkew, lifetime/10)) + if !ok || !expires.Equal(want) { + t.Fatalf("expiry = %v, %t; want %v", expires, ok, want) + } + cache := newNoForkClientCache(1) + cache.now = func() time.Time { return now } + cache.set("key", meta) + now = expires + if _, ok := cache.get("key"); ok { + t.Fatal("client was returned at its cache expiry boundary") + } + }) + } +} + +func TestNoForkClientCacheV2TokenExpiry(t *testing.T) { + now := time.Now().UTC().Truncate(time.Millisecond) + result := tokens2.CreateResult{Result: gophercloud.Result{Body: map[string]any{ + "access": map[string]any{"token": map[string]any{ + "id": "dummy-token", "expires": now.Add(time.Hour).Format(gophercloud.RFC3339Milli), + }}, + }}} + meta := cacheTestClientWithResult(t, result) + if expires, ok := tokenExpiry(meta, now); !ok || !expires.Equal(now.Add(55*time.Minute)) { + t.Fatalf("Keystone v2 expiry = %v, %t", expires, ok) + } +} + +func TestNoForkClientCacheConcurrentCreation(t *testing.T) { + cache := newNoForkClientCache(1) + meta := cacheTestClient(t, time.Now().Add(time.Hour)) + var calls atomic.Int32 + var wg sync.WaitGroup + start := make(chan struct{}) + for range 20 { + wg.Add(1) + go func() { + defer wg.Done() + <-start + got, err := cache.getOrCreate(t.Context(), configFor("dummy-password"), func() (any, error) { + calls.Add(1) + return meta, nil + }) + if err != nil || got != meta { + t.Errorf("getOrCreate = %v, %v", got, err) + } + }() + } + close(start) + wg.Wait() + if got := calls.Load(); got != 1 { + t.Fatalf("creator called %d times, want 1", got) + } +} + +func TestNoForkClientCacheFailedCreationRetries(t *testing.T) { + cache := newNoForkClientCache(1) + wantErr := errors.New("temporary authentication failure") + if _, err := cache.getOrCreate(t.Context(), configFor("dummy-password"), func() (any, error) { + return nil, wantErr + }); !errors.Is(err, wantErr) { + t.Fatalf("expected authentication error, got %v", err) + } + meta := cacheTestClient(t, time.Now().Add(time.Hour)) + got, err := cache.getOrCreate(t.Context(), configFor("dummy-password"), func() (any, error) { + return meta, nil + }) + if err != nil || got != meta { + t.Fatalf("retry did not recover immediately: %v, %v", got, err) + } +} + +func TestNoForkClientCacheCanceledWaiter(t *testing.T) { + cache := newNoForkClientCache(2) + meta := cacheTestClient(t, time.Now().Add(time.Hour)) + started, release, done := make(chan struct{}), make(chan struct{}), make(chan struct{}) + var releaseOnce sync.Once + unblock := func() { releaseOnce.Do(func() { close(release) }) } + go func() { + defer close(done) + _, err := cache.getOrCreate(t.Context(), configFor("dummy-password"), func() (any, error) { + close(started) + <-release + return meta, nil + }) + if err != nil { + t.Errorf("leader failed: %v", err) + } + }() + defer func() { + unblock() + <-done + }() + <-started + ctx, cancel := context.WithCancel(t.Context()) + cancel() + if _, err := cache.getOrCreate(ctx, configFor("dummy-password"), func() (any, error) { + t.Error("canceled waiter invoked the creator") + return nil, nil + }); !errors.Is(err, context.Canceled) { + t.Fatalf("expected cancellation, got %v", err) + } + // An unrelated key must initialize even while the first key is blocked. + if _, err := cache.getOrCreate(t.Context(), configFor("other-password"), func() (any, error) { + return meta, nil + }); err != nil { + t.Fatal(err) + } + // Release and wait before the test ends so no background writes escape it. + // The deferred unblock also releases the leader if an assertion fails above. + unblock() + <-done +} diff --git a/internal/clients/openstack.go b/internal/clients/openstack.go index b9ee3d10..f1b1382b 100644 --- a/internal/clients/openstack.go +++ b/internal/clients/openstack.go @@ -5,13 +5,13 @@ import ( "encoding/json" "strconv" + "github.com/crossplane/crossplane-runtime/v2/pkg/resource" + "github.com/crossplane/upjet/v2/pkg/terraform" "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" tfsdk "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" - - "github.com/crossplane/crossplane-runtime/v2/pkg/resource" - "github.com/crossplane/upjet/v2/pkg/terraform" "github.com/pkg/errors" + osprovider "github.com/terraform-provider-openstack/terraform-provider-openstack/v3/openstack" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -28,13 +28,21 @@ const ( errUnmarshalCredentials = "cannot unmarshal openstack credentials as JSON" ) -// TerraformSetupBuilder builds Terraform a terraform.SetupFn function which -// returns Terraform provider setup configuration - // TerraformSetupBuilder returns Terraform setup with provider specific // configuration like provider credentials used to connect to cloud APIs in the // expected form of a Terraform provider. +// +// The returned SetupFn caches configured provider clients per provider +// configuration (see noForkClientCache), so that managed resources sharing a +// ProviderConfig reuse one authenticated OpenStack client across +// reconciliations instead of re-authenticating against the identity service +// on every reconciliation. func TerraformSetupBuilder(tfProvider *schema.Provider) terraform.SetupFn { //nolint:gocyclo + // The cache is scoped to this SetupFn, which is instantiated once per + // provider flavor (cluster-scoped and namespaced), so entries of + // different flavors never mix. + cache := newNoForkClientCache(defaultNoForkClientCacheSize) + return func(ctx context.Context, client client.Client, mg resource.Managed) (terraform.Setup, error) { ps := terraform.Setup{} @@ -73,11 +81,68 @@ func TerraformSetupBuilder(tfProvider *schema.Provider) terraform.SetupFn { //no ps.Configuration[credField] = v } } - return ps, errors.Wrap(configureNoForkOpenstackClient(ctx, &ps, *tfProvider), "failed to configure the no-fork OpenStack client") + + err = cache.configure(ctx, &ps, tfProvider) + return ps, errors.Wrap(err, "failed to configure the no-fork OpenStack client") } } -func configureNoForkOpenstackClient(ctx context.Context, ps *terraform.Setup, p schema.Provider) error { +func (c *noForkClientCache) configure(ctx context.Context, ps *terraform.Setup, tfProvider *schema.Provider) error { + configuration := ps.Configuration + // Swift's non-delayed path writes directly to the ProviderClient token. + // Do not share it even through singleflight while configuration is in + // progress. Read its schema default too, to respect OS_SWAUTH. + swauth, err := swiftAuthenticationEnabled(configuration, tfProvider) + if err != nil { + return errors.Wrap(err, "failed to parse 'swauth' configuration") + } + if swauth { + return configureNoForkOpenstackClient(ctx, ps, tfProvider) + } + + meta, err := c.getOrCreate(ctx, configuration, func() (any, error) { + // Keep this setup local to the creator. A caller canceled while waiting + // must not race with a later write from the initialization goroutine. + fresh := terraform.Setup{Configuration: configuration} + if err := configureNoForkOpenstackClient(ctx, &fresh, tfProvider); err != nil { + return nil, err + } + return fresh.Meta, nil + }) + if err != nil { + return err + } + ps.Meta = meta + return nil +} + +func swiftAuthenticationEnabled(configuration map[string]any, tfProvider *schema.Provider) (bool, error) { + value, ok := configuration["swauth"] + if !ok && tfProvider.Schema["swauth"] != nil { + var err error + value, err = tfProvider.Schema["swauth"].DefaultValue() + if err != nil { + return false, err + } + } + switch v := value.(type) { + case string: + return strconv.ParseBool(v) + case bool: + return v, nil + default: + return false, nil + } +} + +func configureNoForkOpenstackClient(ctx context.Context, ps *terraform.Setup, tfProvider *schema.Provider) error { + // Configure a copy of the Terraform provider so that its Meta value + // (holding the authenticated OpenStack client) can be cached and reused + // across reconciliations, without mutating the shared provider singleton + // whose Meta must stay unset for future configurations with different + // credentials. + p := *tfProvider + // Populate the config, as well as the raw config for compatibility. config := tfsdk.NewResourceConfigRaw(ps.Configuration) @@ -93,11 +158,22 @@ func configureNoForkOpenstackClient(ctx context.Context, ps *terraform.Setup, p "insecure": cty.BoolVal(insecure), }) - diag := p.Configure(context.WithoutCancel(ctx), config) + diag := p.Configure(ctx, config) if diag != nil && diag.HasError() { return errors.Errorf("failed to configure the provider: %v", diag) } - ps.Meta = p.Meta() + meta := p.Meta() + if cfg, ok := meta.(*osprovider.Config); ok && cfg != nil && !cfg.Swauth { + // Delayed authentication normally happens during the first resource + // operation. Finish it before publishing a shared client: upstream + // retains initial authentication errors on Config permanently, and + // token expiry is unavailable until authentication succeeds. + if err := cfg.Authenticate(ctx); err != nil { + return errors.Wrap(err, "failed to authenticate the OpenStack client") + } + } + // openstack.NewClient already initializes Gophercloud's token/reauth locks. + ps.Meta = meta return nil } diff --git a/internal/clients/openstack_test.go b/internal/clients/openstack_test.go new file mode 100644 index 00000000..cf3e3df2 --- /dev/null +++ b/internal/clients/openstack_test.go @@ -0,0 +1,725 @@ +package clients + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "os" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + xpv1 "github.com/crossplane/crossplane-runtime/v2/apis/common/v1" + "github.com/crossplane/crossplane-runtime/v2/pkg/resource" + "github.com/crossplane/upjet/v2/pkg/terraform" + "github.com/gophercloud/gophercloud/v2" + tokens3 "github.com/gophercloud/gophercloud/v2/openstack/identity/v3/tokens" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + osprovider "github.com/terraform-provider-openstack/terraform-provider-openstack/v3/openstack" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + clustercomputev1alpha1 "github.com/crossplane-contrib/provider-openstack/apis/cluster/compute/v1alpha1" + clusterv1beta1 "github.com/crossplane-contrib/provider-openstack/apis/cluster/v1beta1" + namespacedcomputev1alpha1 "github.com/crossplane-contrib/provider-openstack/apis/namespaced/compute/v1alpha1" + namespacedv1beta1 "github.com/crossplane-contrib/provider-openstack/apis/namespaced/v1beta1" +) + +// These tests intentionally use the upstream provider's real configuration and +// gophercloud authentication paths. In particular, Config.Authenticate remembers +// its first failure, so caching an unauthenticated Config poisons later setups. +type keystoneTestServer struct { + server *httptest.Server + tokenRequests atomic.Int32 + lifetime time.Duration + now func() time.Time + onToken func(http.ResponseWriter, *http.Request, int32) bool + onRequest func(http.ResponseWriter, *http.Request) bool + mu sync.Mutex + credentials []keystoneTestCredentials +} + +type keystoneTestCredentials struct { + password string + tenantID string +} + +func newKeystoneTestServer(t *testing.T, lifetime time.Duration, onToken func(http.ResponseWriter, *http.Request, int32) bool) *keystoneTestServer { + t.Helper() + k := &keystoneTestServer{lifetime: lifetime, now: time.Now, onToken: onToken} + k.server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/": + // Exercise version discovery, rather than replacing gophercloud's + // authenticator or supplying a preconfigured ProviderClient. + if err := json.NewEncoder(w).Encode(map[string]any{ + "versions": map[string]any{"values": []any{map[string]any{ + "id": "v3.14", "status": "stable", + "links": []any{map[string]any{"rel": "self", "href": k.server.URL + "/v3/"}}, + }}}, + }); err != nil { + t.Errorf("encode Keystone discovery: %v", err) + } + case r.Method == http.MethodPost && r.URL.Path == "/v3/auth/tokens": + attempt := k.tokenRequests.Add(1) + var request struct { + Auth struct { + Identity struct { + Password struct { + User struct { + Name string `json:"name"` + Password string `json:"password"` + } `json:"user"` + } `json:"password"` + } `json:"identity"` + Scope struct { + Project struct { + ID string `json:"id"` + } `json:"project"` + } `json:"scope"` + } `json:"auth"` + } + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + t.Errorf("decode Keystone authentication: %v", err) + http.Error(w, "invalid authentication JSON", http.StatusBadRequest) + return + } + if request.Auth.Identity.Password.User.Name != "dummy-user" || request.Auth.Identity.Password.User.Password == "" || request.Auth.Scope.Project.ID == "" { + t.Error("Keystone request did not contain the configured user, password, and project scope") + http.Error(w, "missing credentials", http.StatusUnauthorized) + return + } + k.mu.Lock() + k.credentials = append(k.credentials, keystoneTestCredentials{ + password: request.Auth.Identity.Password.User.Password, + tenantID: request.Auth.Scope.Project.ID, + }) + k.mu.Unlock() + if k.onToken != nil && k.onToken(w, r, attempt) { + return + } + now := k.now().UTC() + w.Header().Set("X-Subject-Token", fmt.Sprintf("dummy-token-%d", attempt)) + w.WriteHeader(http.StatusCreated) + if err := json.NewEncoder(w).Encode(map[string]any{ + "token": map[string]any{ + "methods": []string{"password"}, + "issued_at": now.Format(time.RFC3339), + "expires_at": now.Add(k.lifetime).Format(time.RFC3339), + "user": map[string]any{"id": "dummy-user-id", "name": "dummy-user"}, + "project": map[string]any{"id": request.Auth.Scope.Project.ID, "name": "dummy-project"}, + "catalog": []any{map[string]any{ + "id": "dummy-identity", "name": "keystone", "type": "identity", + "endpoints": []any{map[string]any{ + "id": "dummy-endpoint", "interface": "public", "region": "dummy-region", "url": k.server.URL + "/v3/", + }}, + }}, + }, + }); err != nil { + t.Errorf("encode Keystone token: %v", err) + } + default: + if k.onRequest != nil && k.onRequest(w, r) { + return + } + t.Errorf("unexpected Keystone request: %s %s", r.Method, r.URL.Path) + http.NotFound(w, r) + } + })) + t.Cleanup(k.server.Close) + return k +} + +func clearOpenstackTestEnvironment(t *testing.T) { + t.Helper() + // Do not let a developer's openrc or CI credentials change provider defaults + // or direct a test at anything other than its local Keystone fixture. + for _, entry := range os.Environ() { + name, _, _ := strings.Cut(entry, "=") + if strings.HasPrefix(name, "OS_") { + t.Setenv(name, "") + } + } + t.Setenv("TF_LOG", "") +} + +func keystoneTestConfiguration(k *keystoneTestServer) map[string]any { + return map[string]any{ + "auth_url": k.server.URL, + "user_name": "dummy-user", + "password": "dummy-password", + "tenant_id": "dummy-project", + "user_domain_id": "default", + "region": "dummy-region", + "insecure": "false", + "max_retries": "0", + } +} + +func authenticatedOpenstackTestConfig(t *testing.T, ps terraform.Setup) *osprovider.Config { + t.Helper() + cfg, ok := ps.Meta.(*osprovider.Config) + if !ok || cfg == nil || cfg.OsClient == nil { + t.Fatalf("expected an OpenStack Config with a provider client, got %T", ps.Meta) + } + if cfg.OsClient.Token() == "" { + t.Fatal("setup returned successfully without authenticating its provider client") + } + result, ok := cfg.OsClient.GetAuthResult().(tokens3.CreateResult) + if !ok { + t.Fatalf("expected a real Keystone v3 authentication result, got %T", cfg.OsClient.GetAuthResult()) + } + if _, err := result.ExtractToken(); err != nil { + t.Fatalf("extract authenticated Keystone token: %v", err) + } + return cfg +} + +func TestConfigureNoForkOpenstackClientAuthenticates(t *testing.T) { + clearOpenstackTestEnvironment(t) + for _, tc := range []struct { + name string + delayedAuth string + wantDelayed bool + }{ + {name: "DefaultDelayedAuth", wantDelayed: true}, + {name: "EagerAuth", delayedAuth: "false"}, + } { + t.Run(tc.name, func(t *testing.T) { + keystone := newKeystoneTestServer(t, time.Hour, nil) + provider := osprovider.Provider() + ps := terraform.Setup{Configuration: keystoneTestConfiguration(keystone)} + if tc.delayedAuth != "" { + ps.Configuration["delayed_auth"] = tc.delayedAuth + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if err := configureNoForkOpenstackClient(ctx, &ps, provider); err != nil { + t.Fatalf("configure: %v", err) + } + cfg := authenticatedOpenstackTestConfig(t, ps) + if cfg.DelayedAuth != tc.wantDelayed { + t.Errorf("DelayedAuth = %t, want %t", cfg.DelayedAuth, tc.wantDelayed) + } + if got := keystone.tokenRequests.Load(); got != 1 { + t.Errorf("token requests after configure = %d, want 1", got) + } + if err := cfg.Authenticate(ctx); err != nil { + t.Fatalf("authenticate already configured client: %v", err) + } + if got := keystone.tokenRequests.Load(); got != 1 { + t.Errorf("authenticating an already configured client requested %d tokens, want 1", got) + } + endpoint, err := cfg.OsClient.EndpointLocator(gophercloud.EndpointOpts{ + Type: "identity", Region: "dummy-region", Availability: gophercloud.AvailabilityPublic, + }) + if err != nil || endpoint != keystone.server.URL+"/v3/" { + t.Errorf("authenticated catalog lookup = (%q, %v), want local identity endpoint", endpoint, err) + } + if provider.Meta() != nil { + t.Error("configuration mutated the shared provider singleton's Meta") + } + }) + } +} + +func TestNoForkClientCacheConfigureCoalescesAuthentication(t *testing.T) { + clearOpenstackTestEnvironment(t) + for _, delayedAuth := range []string{"default", "false"} { + t.Run(delayedAuth, func(t *testing.T) { + tokenStarted := make(chan struct{}) + releaseToken := make(chan struct{}) + keystone := newKeystoneTestServer(t, time.Hour, func(_ http.ResponseWriter, r *http.Request, attempt int32) bool { + if attempt == 1 { + close(tokenStarted) + select { + case <-releaseToken: + case <-r.Context().Done(): + return true + } + } + return false + }) + var releaseOnce sync.Once + release := func() { releaseOnce.Do(func() { close(releaseToken) }) } + t.Cleanup(release) + provider := osprovider.Provider() + cache := newNoForkClientCache(8) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + const callers = 16 + start := make(chan struct{}) + entered := make(chan struct{}, callers) + type result struct { + setup terraform.Setup + err error + } + results := make(chan result, callers) + for range callers { + go func() { + <-start + ps := terraform.Setup{Configuration: keystoneTestConfiguration(keystone)} + if delayedAuth != "default" { + ps.Configuration["delayed_auth"] = delayedAuth + } + entered <- struct{}{} + err := cache.configure(ctx, &ps, provider) + results <- result{setup: ps, err: err} + }() + } + close(start) + for range callers { + select { + case <-entered: + case <-ctx.Done(): + t.Fatal("timed out starting concurrent setups") + } + } + select { + case <-tokenStarted: + case <-ctx.Done(): + t.Fatal("concurrent setups did not reach Keystone authentication") + } + release() + var shared *osprovider.Config + for range callers { + select { + case result := <-results: + if result.err != nil { + t.Fatalf("concurrent configure: %v", result.err) + } + cfg := authenticatedOpenstackTestConfig(t, result.setup) + if shared == nil { + shared = cfg + } else if cfg != shared { + t.Error("concurrent setups did not share the same authenticated Config") + } + case <-ctx.Done(): + t.Fatal("timed out waiting for concurrent setups") + } + } + ps := terraform.Setup{Configuration: keystoneTestConfiguration(keystone)} + if delayedAuth != "default" { + ps.Configuration["delayed_auth"] = delayedAuth + } + if err := cache.configure(ctx, &ps, provider); err != nil { + t.Fatalf("subsequent configure: %v", err) + } + if ps.Meta != shared || keystone.tokenRequests.Load() != 1 { + t.Errorf("expected one shared Config and token across concurrent and subsequent setups; token requests = %d", keystone.tokenRequests.Load()) + } + if provider.Meta() != nil { + t.Error("concurrent configuration mutated the shared provider singleton's Meta") + } + }) + } +} + +func TestNoForkClientCacheConfigureRetriesInitialRateLimit(t *testing.T) { + clearOpenstackTestEnvironment(t) + keystone := newKeystoneTestServer(t, time.Hour, func(w http.ResponseWriter, _ *http.Request, attempt int32) bool { + if attempt != 1 { + return false + } + w.WriteHeader(http.StatusTooManyRequests) + _, _ = w.Write([]byte(`{"error":{"code":429,"title":"Too Many Requests","message":"dummy rate limit"}}`)) + return true + }) + provider := osprovider.Provider() + cache := newNoForkClientCache(8) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + first := terraform.Setup{Configuration: keystoneTestConfiguration(keystone)} + if err := cache.configure(ctx, &first, provider); err == nil { + t.Fatal("expected the initial Keystone 429 to fail setup") + } + if got := keystone.tokenRequests.Load(); got != 1 { + t.Fatalf("initial failed setup requested %d tokens, want 1", got) + } + // No clock advance, backoff, or eviction: a failed delayed authentication + // must not leave its sticky authFailed Config in the cache. + retry := terraform.Setup{Configuration: keystoneTestConfiguration(keystone)} + if err := cache.configure(ctx, &retry, provider); err != nil { + t.Fatalf("immediate retry after initial 429: %v", err) + } + shared := authenticatedOpenstackTestConfig(t, retry) + if err := shared.Authenticate(ctx); err != nil { + t.Fatalf("retry retained a failed authentication: %v", err) + } + third := terraform.Setup{Configuration: keystoneTestConfiguration(keystone)} + if err := cache.configure(ctx, &third, provider); err != nil { + t.Fatalf("configure after successful retry: %v", err) + } + if third.Meta != shared || keystone.tokenRequests.Load() != 2 { + t.Errorf("successful retry was not reused; token requests = %d, want 2", keystone.tokenRequests.Load()) + } + if provider.Meta() != nil { + t.Error("failed or retried configuration mutated the provider singleton's Meta") + } +} + +func TestNoForkClientCacheConfigureDoesNotCacheCanceledAuthentication(t *testing.T) { + clearOpenstackTestEnvironment(t) + tokenStarted := make(chan struct{}) + releaseToken := make(chan struct{}) + keystone := newKeystoneTestServer(t, time.Hour, func(_ http.ResponseWriter, r *http.Request, attempt int32) bool { + if attempt != 1 { + return false + } + close(tokenStarted) + select { + case <-r.Context().Done(): + case <-releaseToken: + } + return true + }) + t.Cleanup(func() { close(releaseToken) }) + provider := osprovider.Provider() + cache := newNoForkClientCache(8) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + first := terraform.Setup{Configuration: keystoneTestConfiguration(keystone)} + done := make(chan error, 1) + go func() { done <- cache.configure(ctx, &first, provider) }() + select { + case <-tokenStarted: + case <-time.After(5 * time.Second): + t.Fatal("setup did not reach initial authentication") + } + cancel() + select { + case err := <-done: + // The upstream HTTP retry transport formats its underlying error with + // %s, so cancellation is not necessarily retained in the error chain. + if err == nil || (!errors.Is(err, context.Canceled) && !strings.Contains(err.Error(), context.Canceled.Error())) { + t.Fatalf("canceled authentication error = %v, want context.Canceled", err) + } + case <-time.After(5 * time.Second): + t.Fatal("initial authentication did not honor context cancellation") + } + retryCtx, retryCancel := context.WithTimeout(context.Background(), 5*time.Second) + defer retryCancel() + retry := terraform.Setup{Configuration: keystoneTestConfiguration(keystone)} + if err := cache.configure(retryCtx, &retry, provider); err != nil { + t.Fatalf("retry after canceled initial authentication: %v", err) + } + authenticatedOpenstackTestConfig(t, retry) + if got := keystone.tokenRequests.Load(); got != 2 { + t.Errorf("token requests = %d, want a fresh authentication after cancellation (2)", got) + } + if provider.Meta() != nil { + t.Error("canceled configuration mutated the shared provider singleton's Meta") + } +} + +func TestNoForkClientCacheConfigureUsesAuthenticatedTokenExpiry(t *testing.T) { + clearOpenstackTestEnvironment(t) + initial := time.Now().UTC().Truncate(time.Second) + var clock atomic.Int64 + clock.Store(initial.UnixNano()) + now := func() time.Time { return time.Unix(0, clock.Load()).UTC() } + keystone := newKeystoneTestServer(t, 10*time.Minute, nil) + keystone.now = now + cache := newNoForkClientCache(8) + cache.now = now + provider := osprovider.Provider() + configuration := keystoneTestConfiguration(keystone) + configuration["allow_reauth"] = "false" + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + first := terraform.Setup{Configuration: configuration} + if err := cache.configure(ctx, &first, provider); err != nil { + t.Fatalf("initial configure: %v", err) + } + firstConfig := authenticatedOpenstackTestConfig(t, first) + if firstConfig.AllowReauth || firstConfig.OsClient.ReauthFunc != nil { + t.Fatal("test requires allow_reauth=false, without automatic token refresh") + } + beforeExpiry := terraform.Setup{Configuration: configuration} + if err := cache.configure(ctx, &beforeExpiry, provider); err != nil { + t.Fatalf("configure before token expiry: %v", err) + } + if beforeExpiry.Meta != first.Meta || keystone.tokenRequests.Load() != 1 { + t.Fatal("unexpired authenticated token was not cached") + } + // This is past the actual ten-minute token lifetime, but well before the + // original thirty-minute fallback for an unauthenticated Config. + clock.Store(initial.Add(11 * time.Minute).UnixNano()) + afterExpiry := terraform.Setup{Configuration: configuration} + if err := cache.configure(ctx, &afterExpiry, provider); err != nil { + t.Fatalf("configure after actual token expiry: %v", err) + } + fresh := authenticatedOpenstackTestConfig(t, afterExpiry) + if fresh == firstConfig || fresh.OsClient == firstConfig.OsClient || fresh.OsClient.Token() == firstConfig.OsClient.Token() { + t.Error("expired non-reauthenticating client was reused instead of freshly authenticated") + } + if got := keystone.tokenRequests.Load(); got != 2 { + t.Errorf("token requests after actual expiry = %d, want 2", got) + } + if provider.Meta() != nil { + t.Error("expiry refresh mutated the shared provider singleton's Meta") + } +} + +func TestNoForkClientCacheConfigureIsolatesCredentials(t *testing.T) { + clearOpenstackTestEnvironment(t) + keystone := newKeystoneTestServer(t, time.Hour, nil) + cache := newNoForkClientCache(8) + provider := osprovider.Provider() + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + credentials := []keystoneTestCredentials{ + {password: "dummy-password", tenantID: "dummy-project"}, + {password: "dummy-rotated-password", tenantID: "dummy-project"}, + {password: "dummy-password", tenantID: "dummy-other-project"}, + } + configs := make([]*osprovider.Config, len(credentials)) + for round := range 2 { + for i, credential := range credentials { + configuration := keystoneTestConfiguration(keystone) + configuration["password"] = credential.password + configuration["tenant_id"] = credential.tenantID + ps := terraform.Setup{Configuration: configuration} + if err := cache.configure(ctx, &ps, provider); err != nil { + t.Fatalf("configure credential set %d, round %d: %v", i, round, err) + } + cfg := authenticatedOpenstackTestConfig(t, ps) + if round == 0 { + for _, previous := range configs[:i] { + if cfg == previous || cfg.OsClient == previous.OsClient || cfg.OsClient.Token() == previous.OsClient.Token() { + t.Errorf("credential set %d shared a client or token with different credentials", i) + } + } + configs[i] = cfg + } else if cfg != configs[i] { + t.Errorf("credential set %d did not reuse its own cached Config", i) + } + } + } + if got := keystone.tokenRequests.Load(); got != int32(len(credentials)) { + t.Errorf("token requests = %d, want one per distinct credential set (%d)", got, len(credentials)) + } + keystone.mu.Lock() + defer keystone.mu.Unlock() + if len(keystone.credentials) != len(credentials) { + t.Fatalf("Keystone received %d credential sets, want %d", len(keystone.credentials), len(credentials)) + } + for i, credential := range credentials { + if keystone.credentials[i] != credential { + t.Errorf("Keystone request %d did not use its configured password and project scope", i) + } + } + if provider.Meta() != nil { + t.Error("different credentials mutated the shared provider singleton's Meta") + } +} + +func TestNoForkClientCacheConfigureBypassesSwauth(t *testing.T) { + clearOpenstackTestEnvironment(t) + keystone := newKeystoneTestServer(t, time.Hour, nil) + cache := newNoForkClientCache(8) + provider := osprovider.Provider() + var previous any + for range 2 { + configuration := keystoneTestConfiguration(keystone) + configuration["swauth"] = "true" + ps := terraform.Setup{Configuration: configuration} + if err := cache.configure(context.Background(), &ps, provider); err != nil { + t.Fatalf("configure swauth: %v", err) + } + cfg, ok := ps.Meta.(*osprovider.Config) + if !ok || cfg == nil || !cfg.Swauth { + t.Fatalf("expected an actual swauth provider Config, got %T", ps.Meta) + } + if ps.Meta == previous { + t.Error("swauth Config was cached") + } + previous = ps.Meta + } + if got := keystone.tokenRequests.Load(); got != 0 { + t.Errorf("swauth configuration requested %d Keystone tokens, want 0", got) + } + if provider.Meta() != nil { + t.Error("swauth configuration mutated the shared provider singleton's Meta") + } +} + +func TestNoForkClientCacheConfigureBypassesUnknownMeta(t *testing.T) { + clearOpenstackTestEnvironment(t) + var calls int + provider := &schema.Provider{ + Schema: map[string]*schema.Schema{ + "insecure": {Type: schema.TypeBool, Optional: true}, + }, + ConfigureContextFunc: func(context.Context, *schema.ResourceData) (any, diag.Diagnostics) { + calls++ + return &struct{ call int }{call: calls}, nil + }, + } + cache := newNoForkClientCache(8) + var previous any + for range 2 { + ps := terraform.Setup{Configuration: map[string]any{"insecure": "false"}} + if err := cache.configure(context.Background(), &ps, provider); err != nil { + t.Fatalf("configure unknown Meta: %v", err) + } + if ps.Meta == nil || ps.Meta == previous { + t.Error("unknown provider Meta was cached or not configured") + } + previous = ps.Meta + } + if calls != 2 { + t.Errorf("provider configure calls = %d, want 2 for uncacheable Meta", calls) + } + if provider.Meta() != nil { + t.Error("unknown Meta configuration mutated the shared provider singleton's Meta") + } +} + +func TestTerraformSetupBuilderReusesAuthenticatedClient(t *testing.T) { + clearOpenstackTestEnvironment(t) + for _, flavor := range []string{"Cluster", "Namespaced", "NamespacedClusterProviderConfig"} { + t.Run(flavor, func(t *testing.T) { + keystone := newKeystoneTestServer(t, time.Hour, nil) + crClient, managed := openstackTestKubernetesObjects(t, flavor, keystoneTestConfiguration(keystone)) + provider := osprovider.Provider() + setup := TerraformSetupBuilder(provider) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + var shared *osprovider.Config + // Two different managed resources share their ProviderConfig, and a + // subsequent reconciliation of the first resource must reuse it too. + for _, mg := range []resource.Managed{managed[0], managed[1], managed[0]} { + ps, err := setup(ctx, crClient, mg) + if err != nil { + t.Fatalf("Terraform setup for %s: %v", mg.GetName(), err) + } + cfg := authenticatedOpenstackTestConfig(t, ps) + if shared == nil { + shared = cfg + } else if cfg != shared { + t.Error("resources sharing a ProviderConfig did not share their authenticated Config") + } + } + if got := keystone.tokenRequests.Load(); got != 1 { + t.Errorf("Terraform setups requested %d tokens, want 1", got) + } + if provider.Meta() != nil { + t.Error("Terraform setup mutated the shared provider singleton's Meta") + } + if flavor == "Cluster" { + usages := &clusterv1beta1.ProviderConfigUsageList{} + if err := crClient.List(ctx, usages); err != nil { + t.Fatalf("list cluster ProviderConfig usages: %v", err) + } + if len(usages.Items) != 2 { + t.Errorf("cluster ProviderConfig usages = %d, want 2", len(usages.Items)) + } + } else { + usages := &namespacedv1beta1.ProviderConfigUsageList{} + if err := crClient.List(ctx, usages, client.InNamespace("dummy-resources")); err != nil { + t.Fatalf("list namespaced ProviderConfig usages: %v", err) + } + if len(usages.Items) != 2 { + t.Errorf("namespaced ProviderConfig usages = %d, want 2", len(usages.Items)) + } + } + }) + } +} + +func openstackTestKubernetesObjects(t *testing.T, flavor string, configuration map[string]any) (client.Client, []resource.Managed) { + t.Helper() + scheme := runtime.NewScheme() + for _, add := range []func(*runtime.Scheme) error{ + corev1.AddToScheme, + clusterv1beta1.SchemeBuilder.AddToScheme, + namespacedv1beta1.SchemeBuilder.AddToScheme, + clustercomputev1alpha1.AddToScheme, + namespacedcomputev1alpha1.AddToScheme, + } { + if err := add(scheme); err != nil { + t.Fatalf("register test Kubernetes types: %v", err) + } + } + data, err := json.Marshal(configuration) + if err != nil { + t.Fatalf("marshal dummy provider credentials: %v", err) + } + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "dummy-credentials", Namespace: "dummy-system"}, + Data: map[string][]byte{"credentials": data}, + } + selectors := xpv1.CommonCredentialSelectors{SecretRef: &xpv1.SecretKeySelector{ + SecretReference: xpv1.SecretReference{Name: secret.Name, Namespace: secret.Namespace}, Key: "credentials", + }} + objects := []client.Object{secret} + managed := make([]resource.Managed, 0, 2) + if flavor == "Cluster" { + objects = append(objects, &clusterv1beta1.ProviderConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "dummy-provider"}, + Spec: clusterv1beta1.ProviderConfigSpec{Credentials: clusterv1beta1.ProviderCredentials{ + Source: xpv1.CredentialsSourceSecret, CommonCredentialSelectors: selectors, + }}, + }) + for _, name := range []string{"dummy-resource-a", "dummy-resource-b"} { + mg := &clustercomputev1alpha1.FlavorV2{ + TypeMeta: metav1.TypeMeta{APIVersion: clustercomputev1alpha1.CRDGroupVersion.String(), Kind: "FlavorV2"}, + ObjectMeta: metav1.ObjectMeta{Name: name, UID: types.UID(name)}, + } + mg.SetProviderConfigReference(&xpv1.Reference{Name: "dummy-provider"}) + managed = append(managed, mg) + objects = append(objects, mg) + } + } else { + kind := "ClusterProviderConfig" + pcSpec := namespacedv1beta1.ProviderConfigSpec{Credentials: namespacedv1beta1.ProviderCredentials{ + Source: xpv1.CredentialsSourceSecret, CommonCredentialSelectors: selectors, + }} + if flavor == "Namespaced" { + kind = "ProviderConfig" + secret.Namespace = "dummy-resources" + // Namespaced ProviderConfig credentials must resolve in the managed + // resource's namespace, not this deliberately incorrect selector. + pcSpec.Credentials.SecretRef.Namespace = "dummy-ignored-namespace" + objects = append(objects, &namespacedv1beta1.ProviderConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "dummy-provider", Namespace: "dummy-resources"}, Spec: pcSpec, + }) + } else { + objects = append(objects, &namespacedv1beta1.ClusterProviderConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "dummy-provider"}, Spec: pcSpec, + }) + } + for _, name := range []string{"dummy-resource-a", "dummy-resource-b"} { + mg := &namespacedcomputev1alpha1.FlavorV2{ + TypeMeta: metav1.TypeMeta{APIVersion: namespacedcomputev1alpha1.CRDGroupVersion.String(), Kind: "FlavorV2"}, + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "dummy-resources", UID: types.UID(name)}, + } + mg.SetProviderConfigReference(&xpv1.ProviderConfigReference{Name: "dummy-provider", Kind: kind}) + managed = append(managed, mg) + objects = append(objects, mg) + } + } + return fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).WithInterceptorFuncs(interceptor.Funcs{ + Get: func(ctx context.Context, c client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + // The real API client ignores a namespace in a cluster-scoped GET; + // the fake's object tracker does not know resource scopes. + if _, ok := obj.(*namespacedv1beta1.ClusterProviderConfig); ok { + key.Namespace = "" + } + return c.Get(ctx, key, obj, opts...) + }, + }).Build(), managed +}