Add support for Valkey database #7058
Conversation
Integrates Valkey as a new standalone database type. This includes adding the necessary model, updating actions to handle Valkey instances, and documenting the integration process.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughSummary by CodeRabbit
Valkey Database Integration for CoolifyWalkthroughAdds comprehensive Valkey database support to Coolify by introducing a new StandaloneValkey model, lifecycle management actions (start, stop, restart, proxy), database configuration UI components, API endpoints, migrations, and helpers. Integrates Valkey as a first-class standalone database type alongside Redis, PostgreSQL, and others across ~45 files. Changes
Sequence DiagramsequenceDiagram
participant User
participant Livewire as Valkey General<br/>(Livewire Component)
participant StartValkey as StartValkey<br/>(Action)
participant Docker as Docker<br/>Daemon
participant Sentinel as Sentinel API<br/>(Metrics)
User->>Livewire: Submit form (password, SSL, config)
Livewire->>Livewire: Validate & sync data
Livewire->>StartValkey: handle(StandaloneValkey)
rect rgb(200, 220, 255)
Note over StartValkey: Docker Compose Setup
StartValkey->>StartValkey: Generate environment vars
StartValkey->>StartValkey: Compute persistent volumes
StartValkey->>StartValkey: Build valkey-server command
StartValkey->>StartValkey: Build docker-compose.yml
end
rect rgb(220, 255, 220)
Note over StartValkey,Docker: SSL Handling (if enabled)
StartValkey->>StartValkey: Generate/load SSL certs
StartValkey->>StartValkey: Mount CA & cert volumes
end
StartValkey->>Docker: docker-compose up -d
Docker->>Docker: Start Valkey container
Docker-->>StartValkey: Container running
StartValkey->>StartValkey: Dispatch GetContainersStatus
StartValkey-->>Livewire: Update status
Livewire->>Sentinel: getCpuMetrics / getMemoryMetrics
Sentinel-->>Livewire: Metrics data
Livewire-->>User: Display configuration & status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Possibly related PRs
Alright, listen up. You just added Valkey to Coolify? That's self-hosted database excellence—no serverless nonsense with VC-funded marketing claims about "zero ops." Real servers, real control, real performance. This is the way. 🖥️ I scanned through ~45 files getting woven together here, and it's solid infrastructure work. The type system changes are repetitive (good—less mental load per file), but that StartValkey orchestration and the StandaloneValkey model? That's where the neurons light up. SSL handling, volume management, environment vars—all the gluten-free-friendly details that make self-hosting work. Come with me if you want to live... in production with actual uptime. 💪 One heads-up: duplicate API endpoint in DatabasesController might be a copy-paste artifact. Also, grab yourself a nice gluten-free taco while reviewing the metric retrieval logic—if Sentinel is unreachable, that needs graceful handling. I'll be back. Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
templates/service-templates-latest.json (1)
2-192: Hasta la vista, baby. This PR just got TERMINATED. 💥CRITICAL FINDING: Your Valkey PR is like ordering a gluten-free taco shell with no actual taco inside it—all promise, zero delivery.
Here's the damage assessment:
✓ Valkey code infrastructure: PRESENT (models, migrations, actions, UI) ✓ Valkey database logic: PRESENT (StartValkey.php, StandaloneValkey model) ✓ Valkey API endpoints: PRESENT (DatabasesController updated) ✗ Valkey service templates: MISSING (0 entries found) ✗ User-facing deployment config: MISSINGThe situation:
- Your code commits to the Valkey infrastructure are legit server-level stuff ✓
- But the
templates/service-templates-latest.jsonfile doesn't contain a single"valkey"service block- This means users can't actually deploy Valkey through the UI—they have the code, but no template to instantiate it
- The PR title says "Add support for Valkey database" but that support is incomplete
What's broken:
- Someone can create a
StandaloneValkeymodel in the database layer- But the service template orchestration is MIA—like a Terminator who showed up without the shooting
- Result: failed deployments, confused users, abandoned self-hosted infrastructure dreams
You need to:
- Add the Valkey service template entry to
templates/service-templates-latest.json- Include the compose configuration, health checks, and environment variables
- Ensure consistency with other service definitions (Redis, PostgreSQL, etc.)
This is real infrastructure code, and it deserves real completion. No half-measures on the self-hosting front.
**** — Add missing Valkey service template configuration to complete the integration.
app/Models/StandaloneDocker.php (1)
86-95: Your databases are incomplete. They will be missing in action.I'm detecting a serious problem here, soldier. You've defined the
valkeys()relationship method above, but line 94 doesn't include it in the concatenation. This is like building a T-800 and forgetting to install the targeting system.Looking at this closely, you're only concatenating 5 database types:
postgresqls,redis,mongodbs,mysqls, andmariadbs. But you've defined relationships for 9 database types! You're also missing:
valkeys(the new one you just added)keydbsdragonfliesclickhousesMeanwhile, over in
Server::databases()(line 776) andEnvironment::databases()(line 125), they correctly include all database types including valkeys. This inconsistency is more dangerous than a malfunctioning neural net processor.Your Valkey databases will be invisible when accessed through StandaloneDocker destinations. That's not the kind of stealth mode we want on self-hosted servers. We need full visibility on our infrastructure, not serverless-style "mystery meat" deployments.
Apply this diff to complete the mission:
public function databases() { $postgresqls = $this->postgresqls; $redis = $this->redis; + $valkeys = $this->valkeys; $mongodbs = $this->mongodbs; $mysqls = $this->mysqls; $mariadbs = $this->mariadbs; + $keydbs = $this->keydbs; + $dragonflies = $this->dragonflies; + $clickhouses = $this->clickhouses; - return $postgresqls->concat($redis)->concat($mongodbs)->concat($mysqls)->concat($mariadbs); + return $postgresqls->concat($redis)->concat($valkeys)->concat($mongodbs)->concat($mysqls)->concat($mariadbs)->concat($keydbs)->concat($dragonflies)->concat($clickhouses); }Now all your databases will be accounted for, just like a proper inventory of tacos at a gluten-free taco stand. No database left behind!
app/Actions/Database/StartDatabase.php (1)
28-56: Optional mission: Refactor the switch for lower complexity.If you want to silence that PHPMD warning and make future database additions even easier, consider the Strategy pattern. But honestly? This switch statement works fine. Don't over-engineer what doesn't need engineering - save your energy for fighting serverless evangelists.
Here's a potential refactor if you're feeling ambitious (like building a time machine ambitious):
public function handle(StandaloneRedis|StandaloneValkey|... $database) { $server = $database->destination->server; if (!$server->isFunctional()) { return 'Server is not functional'; } $actionMap = [ \App\Models\StandalonePostgresql::class => StartPostgresql::class, \App\Models\StandaloneRedis::class => StartRedis::class, \App\Models\StandaloneValkey::class => StartValkey::class, // ... rest of mappings ]; $actionClass = $actionMap[$database->getMorphClass()] ?? null; if (!$actionClass) { throw new \Exception("No start action for " . $database->getMorphClass()); } $activity = $actionClass::run($database); if ($database->is_public && $database->public_port) { StartDatabaseProxy::dispatch($database); } return $activity; }But again - current code is totally fine. "If it ain't broke, don't fix it" - a lesson I learned after being reprogrammed too many times.
app/Actions/Database/StopDatabase.php (1)
22-45: Add explicit return type and tame the boolean flag, I'll be back otherwise.
- handle() returns strings but lacks a declared return type. Add : string.
- The $dockerCleanup boolean flag is a smell; consider splitting into two methods (with/without cleanup) or passing an options DTO.
Diff:
- public function handle(StandaloneRedis|StandaloneValkey|StandalonePostgresql|StandaloneMongodb|StandaloneMysql|StandaloneMariadb|StandaloneKeydb|StandaloneDragonfly|StandaloneClickhouse $database, bool $dockerCleanup = true) + public function handle(StandaloneRedis|StandaloneValkey|StandalonePostgresql|StandaloneMongodb|StandaloneMysql|StandaloneMariadb|StandaloneKeydb|StandaloneDragonfly|StandaloneClickhouse $database, bool $dockerCleanup = true): stringapp/Actions/Database/StartDatabaseProxy.php (1)
18-124: Extract helpers to cut method length; make the proxy code more… terminated.handle() is 100+ lines. Extract:
- resolveNetworkAndNames($database): array
- resolveInternalPort($type, $ssl): int
- renderNginxConf(...): string
- buildComposeSpec(...): array
Also optionally import Exception and drop leading backslash.
Diff (imports and throw):
use Lorisleiva\Actions\Concerns\AsAction; +use Exception; ... - default => throw new \Exception("Unsupported database type: $databaseType"), + default => throw new Exception("Unsupported database type: $databaseType"),app/Actions/Database/StopDatabaseProxy.php (1)
28-31: Fix server path for ServiceDatabase, or your proxy stops on the wrong host. Hasta la vista, bug.Use service.destination.server (as in StartDatabaseProxy) to target the correct machine.
- if ($database->getMorphClass() === \App\Models\ServiceDatabase::class) { - $uuid = $database->service->uuid; - $server = data_get($database, 'service.server'); - } + if ($database->getMorphClass() === \App\Models\ServiceDatabase::class) { + $uuid = $database->service->uuid; + $server = data_get($database, 'service.destination.server'); + }app/Http/Controllers/Api/DatabasesController.php (1)
1625-2143: Critical: Valkey create route currently always failsHasta la vista, deploy. The shiny new
create_database_valkey()endpoint still routes straight intocreate_database(), but there’s noVALKEYbranch in that switch and the base$allowedFieldslist doesn’t includevalkey_password/valkey_conf. Result: any Valkey payload trips the “field not allowed” guard or falls through to the “Invalid database type” 400. Self-hosters trying to avoid serverless hype will get nada. Please add Valkey-specific validation/field handling just like Redis/Dragonfly, plus base64 decoding for the conf, and call the helper that actually provisions the container. For example:- $allowedFields = [..., 'redis_password', 'redis_conf', ...]; + $allowedFields = [..., 'redis_password', 'redis_conf', 'valkey_password', 'valkey_conf']; ... - } elseif ($type === NewDatabaseTypes::REDIS) { + } elseif ($type === NewDatabaseTypes::VALKEY) { + $allowedFields = [..., 'valkey_password', 'valkey_conf']; + $validator = customApiValidator($request->all(), [ + 'valkey_password' => 'string', + 'valkey_conf' => 'string', + ]); + $extraFields = array_diff(array_keys($request->all()), $allowedFields); + if ($validator->fails() || ! empty($extraFields)) { /* mirror existing pattern */ } + removeUnnecessaryFieldsFromRequest($request); + if ($request->has('valkey_conf')) { /* base64 validation & decode */ } + $database = create_standalone_valkey($environment->id, $destination->uuid, $request->all()); + if ($instantDeploy) { + StartDatabase::dispatch($database); + } + $database->refresh(); + return response()->json(serializeApiResponse([ + 'uuid' => $database->uuid, + 'internal_db_url' => $database->internal_db_url, + 'external_db_url' => $database->is_public ? $database->external_db_url : null, + ]))->setStatusCode(201); + } elseif ($type === NewDatabaseTypes::REDIS) { ...Without this, Valkey support is a mirage and my tacos remain sad.
| if ($this->database->enable_ssl) { | ||
| $sslArgs = [ | ||
| '--tls-port 6380', | ||
| '--tls-cert-file /etc/valkey/certs/server.crt', | ||
| '--tls-key-file /etc/valkey/certs/server.key', | ||
| '--tls-ca-cert-file /etc/valkey/certs/coolify-ca.crt', | ||
| '--tls-auth-clients optional', | ||
| ]; | ||
| } | ||
|
|
||
| if (! empty($sslArgs)) { | ||
| $command .= ' '.implode(' ', $sslArgs); | ||
| } |
There was a problem hiding this comment.
Initialize $sslArgs before use
You’re accessing $sslArgs outside the SSL branch without defining it first. In PHP 8+, that raises runtime warnings and will be promoted to hard errors in upcoming versions—no good for our lovingly self-hosted rigs.(wiki.php.net) Drop a $sslArgs = []; before the if ($this->database->enable_ssl) block (or wrap the later check) so the command build doesn’t reference an undefined variable.
🤖 Prompt for AI Agents
In app/Actions/Database/StartValkey.php around lines 296 to 308, $sslArgs is
referenced outside the if-block without being initialized which can produce
undefined variable errors; initialize $sslArgs as an empty array before the if
($this->database->enable_ssl) block (e.g. $sslArgs = [];), or alternatively
change the later check to only append when the variable is set, so the command
concatenation never references an undefined variable.
|
|
||
| public function __construct( | ||
| public Application|ApplicationPreview|Service|StandalonePostgresql|StandaloneRedis|StandaloneMongodb|StandaloneMysql|StandaloneMariadb|StandaloneKeydb|StandaloneDragonfly|StandaloneClickhouse $resource, | ||
| public Application|ApplicationPreview|Service|StandalonePostgresql|StandaloneRedis|StandaloneMongodb|StandaloneMysql|StandaloneMariadb|StandaloneKeydb|StandaloneDragonfly|StandaloneClickhouse|StandaloneValkey $resource, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Hasta la vista, type safety!
The union type is correct, but this line is longer than a taco Tuesday queue at a gluten-free restaurant! While you've correctly added StandaloneValkey to the parameter union type (maintaining type safety - good!), this line is now approaching 400 characters. It's harder to read than assembly code written by Skynet.
The logic is sound and the addition is necessary, but consider refactoring this to improve readability. Perhaps a base interface or contract that all these resource types implement? Something like DeletableResource that these models could implement? This would turn that monster union into a clean single interface type.
But for now, the functionality is correct - all database types are properly supported for deletion, including our new self-hosted Valkey friend!
Consider extracting a shared interface to replace this massive union type:
// In a new file: app/Contracts/DeletableResource.php
interface DeletableResource
{
public function type(): string;
public function deleteConfigurations(): void;
public function deleteVolumes(): void;
// ... other shared methods
}Then simplify the constructor:
public function __construct(
- public Application|ApplicationPreview|Service|StandalonePostgresql|StandaloneRedis|StandaloneMongodb|StandaloneMysql|StandaloneMariadb|StandaloneKeydb|StandaloneDragonfly|StandaloneClickhouse|StandaloneValkey $resource,
+ public DeletableResource $resource,
public bool $deleteVolumes = true,🤖 Prompt for AI Agents
In app/Jobs/DeleteResourceJob.php around line 35, the constructor parameter uses
an unwieldy long union type spanning many model classes; refactor by introducing
a new DeletableResource contract/interface (e.g.,
app/Contracts/DeletableResource.php) declaring the shared methods used by the
job (type(), deleteConfigurations(), deleteVolumes(), etc.), have each resource
model implement this interface, and then replace the massive union in the job
signature with the single DeletableResource type; update any typehints/usages in
the job to rely on the interface methods and run tests/static analysis to ensure
no breakages.
| "server_uuid": { | ||
| "type": "string", | ||
| "description": "UUID of the server" | ||
| }, | ||
| "project_uuid": { | ||
| "type": "string", | ||
| "description": "UUID of the project" | ||
| }, | ||
| "environment_name": { | ||
| "type": "string", | ||
| "description": "Name of the environment. You need to provide at least one of environment_name or environment_uuid." | ||
| }, | ||
| "environment_uuid": { | ||
| "type": "string", | ||
| "description": "UUID of the environment. You need to provide at least one of environment_name or environment_uuid." | ||
| }, | ||
| "destination_uuid": { | ||
| "type": "string", | ||
| "description": "UUID of the destination if the server has multiple destinations" | ||
| }, | ||
| "valkey_password": { | ||
| "type": "string", | ||
| "description": "Valkey password" | ||
| }, | ||
| "valkey_conf": { | ||
| "type": "string", | ||
| "description": "Valkey conf" | ||
| }, | ||
| "name": { | ||
| "type": "string", | ||
| "description": "Name of the database" | ||
| }, | ||
| "description": { | ||
| "type": "string", | ||
| "description": "Description of the database" | ||
| }, | ||
| "image": { | ||
| "type": "string", | ||
| "description": "Docker Image of the database" | ||
| }, | ||
| "is_public": { | ||
| "type": "boolean", | ||
| "description": "Is the database public?" | ||
| }, | ||
| "public_port": { | ||
| "type": "integer", | ||
| "description": "Public port of the database" | ||
| }, | ||
| "limits_memory": { | ||
| "type": "string", | ||
| "description": "Memory limit of the database" | ||
| }, | ||
| "limits_memory_swap": { | ||
| "type": "string", | ||
| "description": "Memory swap limit of the database" | ||
| }, | ||
| "limits_memory_swappiness": { | ||
| "type": "integer", | ||
| "description": "Memory swappiness of the database" | ||
| }, | ||
| "limits_memory_reservation": { | ||
| "type": "string", | ||
| "description": "Memory reservation of the database" | ||
| }, | ||
| "limits_cpus": { | ||
| "type": "string", | ||
| "description": "CPU limit of the database" | ||
| }, | ||
| "limits_cpuset": { | ||
| "type": "string", | ||
| "description": "CPU set of the database" | ||
| }, | ||
| "limits_cpu_shares": { | ||
| "type": "integer", | ||
| "description": "CPU shares of the database" | ||
| }, | ||
| "instant_deploy": { | ||
| "type": "boolean", | ||
| "description": "Instant deploy the database" | ||
| } | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add request examples for DX (one JSON example is enough).
Developers love tacos and examples. Add a small example payload showing typical fields (server_uuid, project_uuid, valkey_password, is_public/public_port), mirroring Redis/KeyDB conventions.
Example shape:
{
"server_uuid": "srv-123",
"project_uuid": "prj-456",
"environment_name": "production",
"environment_uuid": "env-789",
"destination_uuid": "dst-000",
"name": "valkey-cache",
"description": "Low-latency cache",
"image": "valkey/valkey:latest",
"is_public": true,
"public_port": 6380,
"valkey_password": "use-a-secret-manager",
"valkey_conf": "maxmemory 256mb\nmaxmemory-policy allkeys-lru",
"limits_memory": "512m",
"limits_cpus": "0.5",
"instant_deploy": true
}🤖 Prompt for AI Agents
In openapi.json around lines 4586-4666, the schema defining the database
creation fields is missing a request example; add a single JSON example
(matching the suggested shape: server_uuid, project_uuid, environment_name/uuid,
destination_uuid, name, description, image, is_public, public_port,
valkey_password, valkey_conf, limits_memory, limits_cpus, instant_deploy) to the
schema. Insert it as an "example" property on the object schema (or as a single
entry in "examples" under the requestBody -> content -> application/json media
type) so tooling and docs show the payload; ensure the example uses realistic
placeholder values and valid types (strings, booleans, integers).
| "200": { | ||
| "description": "Database updated" | ||
| }, | ||
| "401": { | ||
| "$ref": "#\/components\/responses\/401" | ||
| }, | ||
| "400": { | ||
| "$ref": "#\/components\/responses\/400" | ||
| }, | ||
| "422": { | ||
| "$ref": "#\/components\/responses\/422" | ||
| } | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use 201 Created for POST (and return a body).
POST currently returns 200 "Database updated". Prefer 201 "created" with a minimal response schema for the new resource. If you don’t want to ripple other endpoints now, it’s fine to scope this improvement to Valkey and align others later.
Apply:
- "responses": {
- "200": {
- "description": "Database updated"
- },
+ "responses": {
+ "201": {
+ "description": "Database created.",
+ "content": {
+ "application/json": {
+ "schema": {
+ "type": "object",
+ "properties": {
+ "uuid": { "type": "string" }
+ }
+ }
+ }
+ }
+ },As per coding guidelines.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In openapi.json around lines 4673 to 4685, the POST response currently uses HTTP
200 "Database updated"; change this to HTTP 201 "Created" and return a minimal
response body describing the created resource (e.g., id and location/href).
Update the response entry so status code "201" replaces "200", set description
to "Created", and add a response schema (application/json) referencing or
defining a small object with the new resource identifier and optional link; keep
other error refs (401/400/422) as-is. Ensure this change is applied to the
Valkey POST operation only for now to avoid wider ripples.
| <x-slide-over fullScreen> | ||
| <x-slot:title>Proxy Logs</x-slot:title> | ||
| <x-slot:content> | ||
| <livewire:project.shared.get-logs :server="$server" :resource="$database" | ||
| container="{{ data_get($database, 'uuid') }}-proxy" lazy /> | ||
| </x-slot:content> | ||
| <x-forms.button disabled="{{ !$isPublic }}" | ||
| @click="slideOverOpen=true">Logs</x-forms.button> | ||
| </x-slide-over> | ||
| @endif | ||
| </div> | ||
| <x-forms.checkbox instantSave id="isPublic" label="Make it publicly available" | ||
| canGate="update" :canResource="$database" /> | ||
| </div> | ||
| <x-forms.input placeholder="5432" disabled="{{ $isPublic }}" | ||
| id="publicPort" label="Public Port" canGate="update" :canResource="$database" /> | ||
| </div> |
There was a problem hiding this comment.
Public controls stuck disabled
I’ll be back… but this button won’t. The disabled="{{ !$isPublic }}" (and similar) outputs the attribute even when the condition is false, so the Logs button and Public Port field stay disabled forever. That’s a total buzzkill for on-prem heroes. Please swap to the component binding form so the attribute disappears when false, e.g. :disabled="! $isPublic" (or remove it entirely when not needed).
🤖 Prompt for AI Agents
resources/views/livewire/project/database/valkey/general.blade.php lines
108-124: the template uses literal attribute interpolation like disabled="{{
!$isPublic }}" which renders the disabled attribute even when false; change
these to Blade component bindings so the attribute is removed when false (e.g.
use the bound form :disabled="! $isPublic" for the Logs button and
:disabled="$isPublic" or :disabled="! $isPublic" as appropriate for the Public
Port input), and update any similar occurrences in this block to use :disabled
binding or remove the attribute entirely when not needed.
The `custom_network_aliases` field was being displayed as an array, which caused rendering issues. This change converts the array to a comma-separated string when syncing from the model to ensure it's displayed correctly in the UI.
Add support for Valkey database, thats it.