fix: handle missing Redis credentials and redis.Nil in provisioner
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
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 <noreply@anthropic.com>
This commit is contained in:
parent
a593605caa
commit
d91bfc50fa
@ -180,7 +180,8 @@ func (p *Provisioner) GetProjectCache(ctx context.Context, projectID string) (*d
|
|||||||
// Check if user exists
|
// Check if user exists
|
||||||
result, err := p.client.Do(ctx, "ACL", "GETUSER", username).Result()
|
result, err := p.client.Do(ctx, "ACL", "GETUSER", username).Result()
|
||||||
if err != nil {
|
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, nil // User doesn't exist
|
||||||
}
|
}
|
||||||
return nil, fmt.Errorf("get ACL user: %w", err)
|
return nil, fmt.Errorf("get ACL user: %w", err)
|
||||||
|
|||||||
@ -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)
|
return nil, fmt.Errorf("failed to check existing cache: %w", err)
|
||||||
}
|
}
|
||||||
if existing != nil {
|
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
|
// Provision the cache
|
||||||
|
|||||||
@ -375,7 +375,12 @@ func TestProvisionRedis(t *testing.T) {
|
|||||||
cacheProvisioner: &mockCacheProvisioner{
|
cacheProvisioner: &mockCacheProvisioner{
|
||||||
existingCache: &domain.CacheCredentials{ProjectID: "test-project"},
|
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,
|
wantErr: true,
|
||||||
wantErrContains: "already provisioned",
|
wantErrContains: "already provisioned",
|
||||||
},
|
},
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user