From 86d84e21c0b23e985a2c963bf75268a3b3690679 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 6 Oct 2026 06:59:22 +0200 Subject: [PATCH] perf(databases): load Redis-family passwords once for database lists Connection URLs of KeyDB, Dragonfly and Redis queried their password variables once per database, so the database list API ran extra queries for every database. - Read REDIS_PASSWORD and REDIS_USERNAME from the loaded relation - Load the variables once per type in the database list and the environment details API --- .../Controllers/Api/DatabasesController.php | 13 ++ .../Controllers/Api/ProjectController.php | 4 +- app/Models/StandaloneDragonfly.php | 15 +- app/Models/StandaloneKeydb.php | 15 +- app/Models/StandaloneRedis.php | 21 ++- .../RedisFamilyDatabaseListQueryCountTest.php | 144 ++++++++++++++++++ 6 files changed, 206 insertions(+), 6 deletions(-) create mode 100644 tests/Feature/Api/RedisFamilyDatabaseListQueryCountTest.php diff --git a/app/Http/Controllers/Api/DatabasesController.php b/app/Http/Controllers/Api/DatabasesController.php index 06870f160b..10a213e5f3 100644 --- a/app/Http/Controllers/Api/DatabasesController.php +++ b/app/Http/Controllers/Api/DatabasesController.php @@ -20,11 +20,15 @@ use App\Models\S3Storage; use App\Models\ScheduledDatabaseBackup; use App\Models\Server; use App\Models\StandaloneDocker; +use App\Models\StandaloneDragonfly; +use App\Models\StandaloneKeydb; use App\Models\StandalonePostgresql; +use App\Models\StandaloneRedis; use App\Models\StandaloneSqlite; use App\Models\SwarmDocker; use App\Support\ResourceStartActivity; use App\Support\ValidationPatterns; +use Illuminate\Database\Eloquent\Collection as EloquentCollection; use Illuminate\Database\Eloquent\Model; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; @@ -275,6 +279,14 @@ class DatabasesController extends Controller $databases = $databases->merge($project->databases($databaseRelations)); } + if ($request->attributes->get('can_read_sensitive', false) === true) { + // Their connection URLs read REDIS_PASSWORD; load the variables once per type instead of per database. + $databases + ->filter(fn ($database) => $database instanceof StandaloneRedis || $database instanceof StandaloneKeydb || $database instanceof StandaloneDragonfly) + ->groupBy(fn ($database) => $database::class) + ->each(fn ($group) => (new EloquentCollection($group->all()))->load('runtime_environment_variables')); + } + $databaseIds = $databases->pluck('id')->toArray(); $backupConfigs = ScheduledDatabaseBackup::ownedByCurrentTeamAPI($teamId)->with('latest_log') @@ -284,6 +296,7 @@ class DatabasesController extends Controller $databases = $databases->map(function ($database) use ($backupConfigs) { $database->backup_configs = $backupConfigs->get($database->getMorphClass().':'.$database->id, collect())->values(); + $database->makeHidden('runtime_environment_variables'); return $this->removeSensitiveData($database); }); diff --git a/app/Http/Controllers/Api/ProjectController.php b/app/Http/Controllers/Api/ProjectController.php index 16eff1ba18..ad515f3fc7 100644 --- a/app/Http/Controllers/Api/ProjectController.php +++ b/app/Http/Controllers/Api/ProjectController.php @@ -167,10 +167,12 @@ class ProjectController extends Controller if (! $environment) { return response()->json(['message' => 'Environment not found.'], 404); } - $environment = $environment->load(['applications', 'postgresqls', 'redis', 'mongodbs', 'mysqls', 'mariadbs', 'services']); + // Redis connection URLs read REDIS_PASSWORD and REDIS_USERNAME; load the variables once, not per database. + $environment = $environment->load(['applications', 'postgresqls', 'redis.runtime_environment_variables', 'mongodbs', 'mysqls', 'mariadbs', 'services']); collect(['applications', 'postgresqls', 'redis', 'mongodbs', 'mysqls', 'mariadbs', 'services']) ->flatMap(fn (string $relation) => $environment->{$relation}) ->each(fn ($resource) => exposeSensitiveFields($resource)); + $environment->redis->each(fn ($redis) => $redis->makeHidden('runtime_environment_variables')); return response()->json(serializeApiResponse($environment)); } diff --git a/app/Models/StandaloneDragonfly.php b/app/Models/StandaloneDragonfly.php index 15dea750f8..98f9ff5afc 100644 --- a/app/Models/StandaloneDragonfly.php +++ b/app/Models/StandaloneDragonfly.php @@ -369,6 +369,19 @@ class StandaloneDragonfly extends BaseModel return $this->morphMany(EnvironmentVariable::class, 'resourceable'); } + /** + * Uses the loaded runtime_environment_variables relation when present, so lists that eager + * load it do not run a query per database. + */ + private function runtimeEnvironmentVariable(string $key): ?EnvironmentVariable + { + if ($this->relationLoaded('runtime_environment_variables')) { + return $this->runtime_environment_variables->firstWhere('key', $key); + } + + return $this->runtime_environment_variables()->where('key', $key)->first(); + } + /** * The REDIS_PASSWORD variable that sets the server password instead of the stored password. * Databases created before this release (legacy_password_quoting) keep their v4.3.23 server @@ -376,7 +389,7 @@ class StandaloneDragonfly extends BaseModel */ public function serverPasswordEnvironmentVariable(): ?EnvironmentVariable { - $environmentVariable = $this->runtime_environment_variables()->where('key', 'REDIS_PASSWORD')->first(); + $environmentVariable = $this->runtimeEnvironmentVariable('REDIS_PASSWORD'); if (! $environmentVariable) { return null; diff --git a/app/Models/StandaloneKeydb.php b/app/Models/StandaloneKeydb.php index 35818a8742..bc1e045335 100644 --- a/app/Models/StandaloneKeydb.php +++ b/app/Models/StandaloneKeydb.php @@ -370,6 +370,19 @@ class StandaloneKeydb extends BaseModel return $this->morphMany(EnvironmentVariable::class, 'resourceable'); } + /** + * Uses the loaded runtime_environment_variables relation when present, so lists that eager + * load it do not run a query per database. + */ + private function runtimeEnvironmentVariable(string $key): ?EnvironmentVariable + { + if ($this->relationLoaded('runtime_environment_variables')) { + return $this->runtime_environment_variables->firstWhere('key', $key); + } + + return $this->runtime_environment_variables()->where('key', $key)->first(); + } + /** * The REDIS_PASSWORD variable that sets the server password instead of the stored password. * Databases created before this release (legacy_password_quoting) keep their v4.3.23 server @@ -377,7 +390,7 @@ class StandaloneKeydb extends BaseModel */ public function serverPasswordEnvironmentVariable(): ?EnvironmentVariable { - $environmentVariable = $this->runtime_environment_variables()->where('key', 'REDIS_PASSWORD')->first(); + $environmentVariable = $this->runtimeEnvironmentVariable('REDIS_PASSWORD'); if (! $environmentVariable) { return null; diff --git a/app/Models/StandaloneRedis.php b/app/Models/StandaloneRedis.php index 2e6f158cf1..b8bdbef49b 100644 --- a/app/Models/StandaloneRedis.php +++ b/app/Models/StandaloneRedis.php @@ -384,6 +384,19 @@ class StandaloneRedis extends BaseModel return $this->morphMany(EnvironmentVariable::class, 'resourceable'); } + /** + * Uses the loaded runtime_environment_variables relation when present, so lists that eager + * load it do not run a query per database. + */ + private function runtimeEnvironmentVariable(string $key): ?EnvironmentVariable + { + if ($this->relationLoaded('runtime_environment_variables')) { + return $this->runtime_environment_variables->firstWhere('key', $key); + } + + return $this->runtime_environment_variables()->where('key', $key)->first(); + } + /** * The REDIS_PASSWORD variable whose resolved value sets the server password. Databases created * before this release (legacy_password_quoting) keep their v4.3.23 server password, the stored @@ -391,7 +404,7 @@ class StandaloneRedis extends BaseModel */ public function serverPasswordEnvironmentVariable(): ?EnvironmentVariable { - $environmentVariable = $this->runtime_environment_variables()->where('key', 'REDIS_PASSWORD')->first(); + $environmentVariable = $this->runtimeEnvironmentVariable('REDIS_PASSWORD'); if (! $environmentVariable) { return null; @@ -444,7 +457,7 @@ class StandaloneRedis extends BaseModel { return new Attribute( get: function () { - $password = $this->runtime_environment_variables()->where('key', 'REDIS_PASSWORD')->first(); + $password = $this->runtimeEnvironmentVariable('REDIS_PASSWORD'); if (! $password) { return null; } @@ -459,7 +472,9 @@ class StandaloneRedis extends BaseModel { return new Attribute( get: function () { - $username = $this->runtime_environment_variables()->where('key', 'REDIS_USERNAME')->first(); + // A loaded relation does not hold a variable created by an earlier read, so check the table before creating one. + $username = $this->runtimeEnvironmentVariable('REDIS_USERNAME') + ?? $this->runtime_environment_variables()->where('key', 'REDIS_USERNAME')->first(); if (! $username) { $this->runtime_environment_variables()->create([ 'key' => 'REDIS_USERNAME', diff --git a/tests/Feature/Api/RedisFamilyDatabaseListQueryCountTest.php b/tests/Feature/Api/RedisFamilyDatabaseListQueryCountTest.php new file mode 100644 index 0000000000..bea7b1de5b --- /dev/null +++ b/tests/Feature/Api/RedisFamilyDatabaseListQueryCountTest.php @@ -0,0 +1,144 @@ + InstanceSettings::firstOrCreate(['id' => 0], ['is_api_enabled' => true])); + Server::flushIdentityMap(); + + $this->team = Team::factory()->create(); + $user = User::factory()->create(); + $this->team->members()->attach($user->id, ['role' => 'owner']); + session(['currentTeam' => $this->team]); + $this->bearerToken = $user->createToken('test-token', ['root'])->plainTextToken; + + $server = Server::factory()->create(['team_id' => $this->team->id]); + $this->destination = StandaloneDocker::query()->where('server_id', $server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $this->team->id]); + $this->environment = Environment::factory()->create(['project_id' => $project->id]); +}); + +function createRedisFamilyDatabasesForQueryCount(int $environmentId, StandaloneDocker $destination, int $count): void +{ + foreach (range(1, $count) as $index) { + $public = ['is_public' => true, 'public_port' => 30000 + $index]; + + create_standalone_redis($environmentId, $destination, $public + ['image' => 'redis:7.2']); + + foreach (['create_standalone_keydb', 'create_standalone_dragonfly'] as $create) { + $database = $create($environmentId, $destination, $public); + EnvironmentVariable::create([ + 'key' => 'REDIS_PASSWORD', + 'value' => "variable-password-{$index}", + 'resourceable_type' => $database->getMorphClass(), + 'resourceable_id' => $database->id, + ]); + } + } +} + +/** + * Counts the queries of one database list request. The REDIS_USERNAME lookup of the StandaloneRedis + * "retrieved" hook runs per model before any relation can be eager loaded, so it is not counted. + * Pass $onlyTable to count only the queries that read that table. + * + * @return array{0: int, 1: array>} + */ +function countRedisFamilyDatabaseListQueries(string $bearerToken, string $uri = '/api/v1/databases', ?string $onlyTable = null): array +{ + DB::flushQueryLog(); + DB::enableQueryLog(); + + $response = test()->withHeaders(['Authorization' => 'Bearer '.$bearerToken]) + ->getJson($uri) + ->assertOk(); + + DB::disableQueryLog(); + + $queries = collect(DB::getQueryLog()) + ->reject(fn (array $query) => str_contains($query['query'], 'audit_events') || in_array('REDIS_USERNAME', $query['bindings'], true)) + ->filter(fn (array $query) => $onlyTable === null || str_contains($query['query'], "from \"{$onlyTable}\"")) + ->count(); + + return [$queries, $response->json()]; +} + +it('does not query environment variables per Redis, KeyDB and Dragonfly database in the database list', function () { + createRedisFamilyDatabasesForQueryCount($this->environment->id, $this->destination, 1); + // The first request also authenticates the token; measure warm requests only. + countRedisFamilyDatabaseListQueries($this->bearerToken); + [$queriesForOneEach] = countRedisFamilyDatabaseListQueries($this->bearerToken); + + createRedisFamilyDatabasesForQueryCount($this->environment->id, $this->destination, 4); + [$queriesForFiveEach, $databases] = countRedisFamilyDatabaseListQueries($this->bearerToken); + + expect($databases)->toHaveCount(15) + ->and($queriesForFiveEach)->toBe($queriesForOneEach); + + $modelsByUuid = collect([StandaloneRedis::class, StandaloneKeydb::class, StandaloneDragonfly::class]) + ->flatMap(fn (string $model) => $model::query()->get()) + ->keyBy('uuid'); + + foreach ($databases as $database) { + $model = $modelsByUuid[$database['uuid']]; + + expect($database)->not->toHaveKey('runtime_environment_variables') + ->and($database['internal_db_url'])->toBe($model->internal_db_url) + ->and($database['external_db_url'])->toBe($model->external_db_url) + ->and($database['internal_db_url'])->toStartWith('redis://') + ->and($database['external_db_url'])->toStartWith('redis://'); + + if (str_starts_with($database['name'], 'redis-database-')) { + expect($database['internal_db_url'])->toStartWith('redis://default:'.rawurlencode($model->redis_password).'@'); + } else { + expect($database['internal_db_url'])->toMatch('/^redis:\/\/:variable-password-\d@/'); + } + } +}); + +it('does not query environment variables per Redis database in the environment details', function () { + $uri = "/api/v1/projects/{$this->environment->project->uuid}/{$this->environment->uuid}"; + + createRedisFamilyDatabasesForQueryCount($this->environment->id, $this->destination, 1); + countRedisFamilyDatabaseListQueries($this->bearerToken, $uri); + // The environment details lazy load each database's destination for external URLs; only variable reads are checked here. + [$queriesForOne] = countRedisFamilyDatabaseListQueries($this->bearerToken, $uri, 'environment_variables'); + + createRedisFamilyDatabasesForQueryCount($this->environment->id, $this->destination, 4); + [$queriesForFive, $environment] = countRedisFamilyDatabaseListQueries($this->bearerToken, $uri, 'environment_variables'); + + expect($environment['redis'])->toHaveCount(5) + ->and($queriesForFive)->toBe($queriesForOne); + + foreach ($environment['redis'] as $database) { + $model = StandaloneRedis::query()->where('uuid', $database['uuid'])->firstOrFail(); + + expect($database)->not->toHaveKey('runtime_environment_variables') + ->and($database['internal_db_url'])->toBe($model->internal_db_url) + ->and($database['external_db_url'])->toBe($model->external_db_url); + } +}); + +it('creates a missing Redis username variable only once when the variables are loaded', function () { + $redis = create_standalone_redis($this->environment->id, $this->destination); + $redis->runtime_environment_variables()->where('key', 'REDIS_USERNAME')->delete(); + $redis->load('runtime_environment_variables'); + + expect($redis->redis_username)->toBe('default') + ->and($redis->redis_username)->toBe('default') + ->and($redis->runtime_environment_variables()->where('key', 'REDIS_USERNAME')->count())->toBe(1); +});