From d91bfc50faffd4185207288dad03608555a181a4 Mon Sep 17 00:00:00 2001 From: jordan Date: Mon, 23 Feb 2026 05:19:00 -0700 Subject: [PATCH] fix: handle missing Redis credentials and redis.Nil in provisioner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs fixed: 1. redis.Nil not handled in GetProjectCache: When ACL GETUSER returns nil (user doesn't exist), go-redis represents this as redis.Nil error. The provisioner only checked for err.Contains("User") which didn't match, causing spurious "get ACL user: redis: nil" errors on re-provision. 2. provisionRedis returns 409 even when REDIS_URL not in credential store: If the Redis ACL user exists but REDIS_URL was never stored (e.g., due to a failed previous run or lost state), the service would permanently refuse to provision, leaving the project without usable Redis credentials. Now checks the credential store: if REDIS_URL exists → true 409 duplicate; if REDIS_URL missing → re-provision (CreateProjectCache resets the password). Co-Authored-By: Claude Sonnet 4.6 --- internal/adapter/redis/provisioner.go | 3 ++- internal/service/component_infra.go | 15 ++++++++++++++- internal/service/component_test.go | 7 ++++++- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/internal/adapter/redis/provisioner.go b/internal/adapter/redis/provisioner.go index 446195c..5d01685 100644 --- a/internal/adapter/redis/provisioner.go +++ b/internal/adapter/redis/provisioner.go @@ -180,7 +180,8 @@ func (p *Provisioner) GetProjectCache(ctx context.Context, projectID string) (*d // Check if user exists result, err := p.client.Do(ctx, "ACL", "GETUSER", username).Result() if err != nil { - if strings.Contains(err.Error(), "User") { + // redis.Nil means the user does not exist (RESP null bulk string response) + if err == redis.Nil || strings.Contains(err.Error(), "User") { return nil, nil // User doesn't exist } return nil, fmt.Errorf("get ACL user: %w", err) diff --git a/internal/service/component_infra.go b/internal/service/component_infra.go index 1877f83..a0fe5ef 100644 --- a/internal/service/component_infra.go +++ b/internal/service/component_infra.go @@ -88,7 +88,20 @@ func (s *ComponentService) provisionRedis(ctx context.Context, projectID, name s return nil, fmt.Errorf("failed to check existing cache: %w", err) } if existing != nil { - return nil, fmt.Errorf("%w: redis already provisioned for project %s", domain.ErrDuplicateComponent, projectID) + // Redis user exists — check if credentials are stored. If they are, it's a true duplicate. + // If not (credentials were lost), fall through to re-provision (CreateProjectCache resets the password). + if s.credentialStore != nil { + storedURL, storeErr := s.credentialStore.Get(ctx, projectID+":REDIS_URL") + if storeErr == nil && storedURL != "" { + return nil, fmt.Errorf("%w: redis already provisioned for project %s", domain.ErrDuplicateComponent, projectID) + } + // Credentials missing from store — re-provision to recover + log := logging.FromContext(ctx).WithService("component") + log.Warn("redis user exists but REDIS_URL not in credential store, re-provisioning", + logging.FieldProjectID, projectID) + } else { + return nil, fmt.Errorf("%w: redis already provisioned for project %s", domain.ErrDuplicateComponent, projectID) + } } // Provision the cache diff --git a/internal/service/component_test.go b/internal/service/component_test.go index d5712cc..a7747e5 100644 --- a/internal/service/component_test.go +++ b/internal/service/component_test.go @@ -375,7 +375,12 @@ func TestProvisionRedis(t *testing.T) { cacheProvisioner: &mockCacheProvisioner{ existingCache: &domain.CacheCredentials{ProjectID: "test-project"}, }, - credStore: newMockCredentialStore(), + credStore: func() *mockCredentialStore { + // Simulate credentials already stored — this is a true duplicate + cs := newMockCredentialStore() + cs.stored["test-project:REDIS_URL"] = "redis://proj-test-project:pass@localhost:6379" + return cs + }(), wantErr: true, wantErrContains: "already provisioned", },