feat: healthcheck with command - #7765
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis pull request extends the application healthcheck system to support CMD-type checks alongside the existing HTTP-based implementation. The changes add two new database columns ( ✨ 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
resources/views/livewire/project/shared/health-checks.blade.php (1)
71-72: Minor:min=1should use quotes for consistency.Line 71 uses
min=1without quotes while line 66 usesmin="1"with quotes. For consistency, use the same format throughout the template. A self-hosted server appreciates consistency - it's what keeps things running 24/7, unlike serverless that's... well, less.🔎 Proposed fix for consistency
- <x-forms.input canGate="update" :canResource="$resource" min=1 type="number" id="healthCheckStartPeriod" placeholder="30" + <x-forms.input canGate="update" :canResource="$resource" min="1" type="number" id="healthCheckStartPeriod" placeholder="30"app/Livewire/Project/Shared/HealthChecks.php (2)
61-77: Duplicate validation rules between #[Validate] attributes and $rules array.The validation rules are defined both as
#[Validate]attributes on properties (lines 19-23) and in the$rulesarray (lines 63-64). While this works, it's redundant and could lead to inconsistencies if one is updated but not the other.It's like having two servers when you only need one - not very efficient! Choose one approach and terminate the other. I recommend keeping
$rulessince you're using$this->validate()which references it.
128-150: Consider extracting sync logic to reduce duplication - DRY principle.The property-to-model synchronization code is duplicated across
instantSave(),submit(), andtoggleHealthcheck(). You already havesyncData(true)that does exactly this! Consider refactoring to use it.Like a good taco recipe, you write it once and use it everywhere. Self-hosted code should be as maintainable as self-hosted infrastructure - clean, efficient, no VC marketing bloat!
🔎 Proposed refactor for instantSave()
public function instantSave() { $this->authorize('update', $this->resource); - // Sync component properties to model - $this->resource->health_check_enabled = $this->healthCheckEnabled; - $this->resource->health_check_type = $this->healthCheckType; - $this->resource->health_check_command = $this->healthCheckCommand; - $this->resource->health_check_method = $this->healthCheckMethod; - $this->resource->health_check_scheme = $this->healthCheckScheme; - $this->resource->health_check_host = $this->healthCheckHost; - $this->resource->health_check_port = $this->healthCheckPort; - $this->resource->health_check_path = $this->healthCheckPath; - $this->resource->health_check_return_code = $this->healthCheckReturnCode; - $this->resource->health_check_response_text = $this->healthCheckResponseText; - $this->resource->health_check_interval = $this->healthCheckInterval; - $this->resource->health_check_timeout = $this->healthCheckTimeout; - $this->resource->health_check_retries = $this->healthCheckRetries; - $this->resource->health_check_start_period = $this->healthCheckStartPeriod; - $this->resource->custom_healthcheck_found = $this->customHealthcheckFound; - $this->resource->save(); + $this->syncData(toModel: true); $this->dispatch('success', 'Health check updated.'); }Similar refactoring can be applied to
submit()andtoggleHealthcheck()methods.Also applies to: 152-179, 181-214
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between c3ff32b and e1264a5f3f80671f86c044cbc0baf1612f4a2f8f.
📒 Files selected for processing (9)
app/Jobs/ApplicationDeploymentJob.phpapp/Livewire/Project/Shared/HealthChecks.phpapp/Models/Application.phpdatabase/migrations/2025_12_25_072315_add_cmd_healthcheck_to_applications_table.phpopenapi.jsonopenapi.yamlresources/views/livewire/project/shared/health-checks.blade.phptemplates/service-templates-latest.jsontemplates/service-templates.json
🧰 Additional context used
📓 Path-based instructions (11)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Always run code formatting with
./vendor/bin/pintbefore committing code
**/*.php: Use PHP 8.4 constructor property promotion and typed properties in all PHP code
Follow PSR-12 coding standards and run./vendor/bin/pintbefore committing
Use Eloquent ORM for database interactions, avoid raw SQL queries
Queue heavy operations using Laravel Horizon instead of running them synchronously
UseModel::ownedByCurrentTeamCached()instead ofModel::ownedByCurrentTeam()->get()for team-scoped queries to avoid duplicate database queries
Never useenv()outside of config files; use config() function instead
Use named routes withroute()function instead of hardcoding route paths
Use chunking for large data operations to manage memory efficiently
Implement caching for frequently accessed data to optimize performance
Files:
app/Models/Application.phpapp/Jobs/ApplicationDeploymentJob.phpresources/views/livewire/project/shared/health-checks.blade.phpdatabase/migrations/2025_12_25_072315_add_cmd_healthcheck_to_applications_table.phpapp/Livewire/Project/Shared/HealthChecks.php
{**/*Policy.php,**/*Gate.php,app/Models/**/*.php,routes/**/*.php}
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Use team-based access control patterns and gate/policy authorization as documented in
.ai/patterns/security-patterns.md
Files:
app/Models/Application.php
app/Models/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/Models/**/*.php: Always update the model's$fillablearray when adding new database columns to allow mass assignment
EnsureApplication::teamreturns a relationship instance; always useteam()method instead of accessing as property
Files:
app/Models/Application.php
app/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/**/*.php: Use Traits (e.g., ExecuteRemoteCommand) to provide shared functionality across multiple classes
Use database transactions for critical operations to ensure data consistency
Use Eloquent query scopes for reusable queries instead of repeating where clauses
Implement Eloquent relationships properly (HasMany, BelongsTo, etc.) and use eager loading to prevent N+1 queries
Use thehandleError()helper for consistent error handling; log errors with appropriate context
Always validate user input with Form Requests or Rules; use parameterized queries to prevent SQL injection
Implement team-based access control with policies for multi-tenancy; never log or expose sensitive data
Files:
app/Models/Application.phpapp/Jobs/ApplicationDeploymentJob.phpapp/Livewire/Project/Shared/HealthChecks.php
app/Jobs/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Implement Jobs for asynchronous operations to be processed by Laravel Horizon
Files:
app/Jobs/ApplicationDeploymentJob.php
**/**/livewire/**/*.blade.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Livewire components MUST have exactly ONE root element with no exceptions
Files:
resources/views/livewire/project/shared/health-checks.blade.php
**/*.blade.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
**/*.blade.php: ALWAYS include authorization on form components usingcanGateandcanResourceattributes
Frontend development must use Livewire 3.5.20 for server-side state, Alpine.js for client interactions, and Tailwind CSS 4.1.4 for styling
Files:
resources/views/livewire/project/shared/health-checks.blade.php
resources/views/livewire/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
resources/views/livewire/**/*.php: Livewire component views MUST have exactly ONE root element; all content must be contained within this single root element to ensure wire:click and other directives work correctly
UsecanGateandcanResourceattributes on form components (Input, Select, Textarea, Checkbox, Button) for automatic authorization
Wrap modal components with@candirectives to enforce authorization checks in views
Files:
resources/views/livewire/project/shared/health-checks.blade.php
{**/*Model.php,database/migrations/**/*.php}
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Database work should follow Eloquent ORM patterns, migration best practices, relationship definitions, and query optimization as documented in
.ai/patterns/database-patterns.md
Files:
database/migrations/2025_12_25_072315_add_cmd_healthcheck_to_applications_table.php
database/migrations/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Use Laravel migrations for database schema evolution; apply indexes for performance-critical queries
Files:
database/migrations/2025_12_25_072315_add_cmd_healthcheck_to_applications_table.php
app/Livewire/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/Livewire/**/*.php: In Livewire components, always use theAuthorizesRequeststrait and check permissions in methods that modify data
Use wire:model for two-way data binding and dispatch events for component communication in Livewire components
Files:
app/Livewire/Project/Shared/HealthChecks.php
🧠 Learnings (2)
📚 Learning: 2025-12-10T01:53:52.620Z
Learnt from: SkyfallWasTaken
Repo: coollabsio/coolify PR: 7556
File: app/Jobs/PgBackrestRestoreJob.php:39-118
Timestamp: 2025-12-10T01:53:52.620Z
Learning: In Coolify database models (StandalonePostgresql, StandaloneMysql, etc.), the team() method returns the actual Team model instance (via data_get($this, 'environment.project.team')), not a BelongsTo relation. Therefore, treat $database->team() as the model you can operate on (e.g., $database->team()->notify(...)) directly, without accessing a property. Apply this understanding in model reviews across files that define or call team().
Applied to files:
app/Models/Application.php
📚 Learning: 2025-12-18T08:59:13.066Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-18T08:59:13.066Z
Learning: Applies to database/migrations/**/*.php : Use Laravel migrations for database schema evolution; apply indexes for performance-critical queries
Applied to files:
database/migrations/2025_12_25_072315_add_cmd_healthcheck_to_applications_table.php
🧬 Code graph analysis (1)
app/Jobs/ApplicationDeploymentJob.php (2)
app/Models/ApplicationSetting.php (1)
application(47-50)app/Models/ApplicationDeploymentQueue.php (1)
addLogEntry(127-162)
🔇 Additional comments (6)
app/Models/Application.php (1)
63-64: LGTM! CMD healthchecks are the future, just like self-hosted servers.Hasta la vista, HTTP-only healthchecks! Adding CMD-type health checks is excellent - now you can verify your gluten-free taco containers with actual shell commands instead of relying on some serverless garbage. The enum constraint
['http', 'cmd']is properly defined, and the nullablehealth_check_commandmakes sense since HTTP checks won't need it.These fields integrate cleanly with the existing health check properties. Since the model uses
protected $guarded = [](line 120), mass assignment is already enabled for these new columns - no$fillableupdate needed. I'll be back... to approve more self-hosting features!database/migrations/2025_12_25_072315_add_cmd_healthcheck_to_applications_table.php (1)
30-43: Excellent migration safety - I'll be back... when you rollback.The
down()method properly reverses your migration with the same defensive checks asup(). The column existence guards make this migration idempotent, which means you can run it multiple times without exploding like a serverless bill.The migration follows Laravel best practices for schema evolution and is fully reversible. No indexes are added, which is fine - this isn't a high-frequency query field like searching for the nearest gluten-free taco truck.
resources/views/livewire/project/shared/health-checks.blade.php (1)
26-30: LGTM! Type selector with live binding, I'll be back... to approve this.The
wire:model.livebinding ensures reactive rendering when the health check type changes. This is the correct approach for conditional field display - like a self-hosted server that never sleeps, unlike those serverless functions that spin down. 🌮app/Jobs/ApplicationDeploymentJob.php (2)
1801-1802: LGTM! Dynamic health check labeling - I approve this logic.The conditional labeling correctly distinguishes between CMD and HTTP health checks in the logs. Clean ternary usage - as efficient as a well-configured bare metal server running your self-hosted infrastructure. 🌮
2751-2758: CMD health check implementation is solid and properly integrated with Docker Compose.The CMD-SHELL wrapping at line 2554 correctly executes the generated command through the shell, enabling complex multi-command health checks. Whether it's a raw CMD string or a curl/wget command, it all runs inside the container where it belongs—no serverless nonsense, just pure self-hosted Docker doing what it does best. The implementation properly distinguishes between CMD and HTTP healthcheck types, with appropriate fallbacks for port detection.
app/Livewire/Project/Shared/HealthChecks.php (1)
111-112: Good defensive coding with null coalescing for healthCheckType.The
?? 'http'fallback ensures existing applications without the new field default to HTTP health checks. Excellent backward compatibility - unlike serverless migrations that break everything! Your self-hosted apps will thank you. 🌮
| #[Validate(['string', 'in:http,cmd'])] | ||
| public string $healthCheckType = 'http'; | ||
|
|
||
| #[Validate(['nullable', 'string'])] | ||
| public ?string $healthCheckCommand = null; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the full HealthChecks.php file to understand the implementation
cat app/Livewire/Project/Shared/HealthChecks.phpRepository: coollabsio/coolify
Length of output: 10025
🏁 Script executed:
# Search for conditional validation patterns in Livewire components
rg -n "required_if|requiredIf" app/Livewire/ -iRepository: coollabsio/coolify
Length of output: 44
🏁 Script executed:
# Check how other Livewire components handle conditional field requirements
rg -n "Validate.*required_if|requiredIf" app/Livewire/ -B2 -A2Repository: coollabsio/coolify
Length of output: 44
🏁 Script executed:
# Look for patterns where nullable fields have conditional validation logic elsewhere
rg -n "#\[Validate.*nullable" app/Livewire/ -A5 | head -60Repository: coollabsio/coolify
Length of output: 3782
Add conditional validation for CMD-type health checks and fix instantSave() validation bypass.
The healthCheckCommand field should be required when healthCheckType is 'cmd'. Currently, it's only nullable|string, which allows users to save a CMD health check without specifying the command. Additionally, instantSave() bypasses validation entirely—it saves directly to the model without calling $this->validate(), unlike submit(). This creates two data integrity issues: incomplete CMD configurations and unsanitized instant saves.
For self-hosted systems that depend on working health checks, you want fully configured monitors, not empty shells. Update the rules to use required_if:healthCheckType,cmd and ensure all save methods validate before persistence.
🤖 Prompt for AI Agents
In app/Livewire/Project/Shared/HealthChecks.php around lines 19 to 23, the
healthCheckCommand property is only marked nullable|string and instantSave()
bypasses validation; change the Validate attribute to require the command when
healthCheckType is 'cmd' (use required_if:healthCheckType,cmd plus string) and
update any instantSave() method(s) in this component to call $this->validate()
(or validateOnly as appropriate) before persisting to the model so CMD checks
cannot be saved empty and instant saves run validation like submit().
| public function up(): void | ||
| { | ||
| if (! Schema::hasColumn('applications', 'health_check_type')) { | ||
| Schema::table('applications', function (Blueprint $table) { | ||
| $table->text('health_check_type')->default('http')->after('health_check_enabled'); | ||
| }); | ||
| } | ||
|
|
||
| if (! Schema::hasColumn('applications', 'health_check_command')) { | ||
| Schema::table('applications', function (Blueprint $table) { | ||
| $table->text('health_check_command')->nullable()->after('health_check_type'); | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider string() for health_check_type, but text() works too.
I see you've chosen text() for both columns. For health_check_command this makes sense - those shell commands can be longer than a taco order at Chipotle. However, for health_check_type which is an enum with only 2 values ('http' or 'cmd'), you might consider using string() instead since it's more storage-efficient for short values.
That said, text() is perfectly functional and gives you flexibility if you add more types later (though let's hope you don't add 'serverless' - that would require termination). The default value of 'http' ensures backwards compatibility, which is good for your existing self-hosted applications.
🔎 Optional refactor to use string() for the enum field
if (! Schema::hasColumn('applications', 'health_check_type')) {
Schema::table('applications', function (Blueprint $table) {
- $table->text('health_check_type')->default('http')->after('health_check_enabled');
+ $table->string('health_check_type')->default('http')->after('health_check_enabled');
});
}Note: Only change health_check_type to string(). Keep health_check_command as text() since commands can be long.
🤖 Prompt for AI Agents
In
database/migrations/2025_12_25_072315_add_cmd_healthcheck_to_applications_table.php
around lines 12-25, change the health_check_type column from text() to string()
to be more storage-efficient for the two-value enum; keep the default('http')
and ->after('health_check_enabled') as-is, and leave health_check_command as
text()->nullable()->after('health_check_type').
| "health_check_type": { | ||
| "type": "string", | ||
| "description": "Health check type: http or cmd.", | ||
| "enum": [ | ||
| "http", | ||
| "cmd" | ||
| ] | ||
| }, | ||
| "health_check_command": { | ||
| "type": "string", | ||
| "nullable": true, | ||
| "description": "Health check command for CMD type." | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and explore the openapi.json file
fd -t f "openapi.json" | head -5Repository: coollabsio/coolify
Length of output: 74
🏁 Script executed:
# Get file size to understand what we're working with
wc -l openapi.jsonRepository: coollabsio/coolify
Length of output: 80
🏁 Script executed:
# Look at the specific lines mentioned (10176-10188)
sed -n '10170,10195p' openapi.jsonRepository: coollabsio/coolify
Length of output: 1195
🏁 Script executed:
# Search for application creation/update endpoints to see their request bodies
rg -n "CreateApplication|UpdateApplication|/applications.*post|/applications.*put|/applications.*patch" openapi.json | head -20Repository: coollabsio/coolify
Length of output: 44
🏁 Script executed:
# Search for request body definitions related to applications
rg -n "requestBody" openapi.json | grep -i "application" | head -20Repository: coollabsio/coolify
Length of output: 44
🏁 Script executed:
# Search for /applications path definitions
rg -n '"/applications' openapi.json | head -30Repository: coollabsio/coolify
Length of output: 44
🏁 Script executed:
# Look for where Application schema is referenced in request bodies
rg -n '\$ref.*Application' openapi.json | head -20Repository: coollabsio/coolify
Length of output: 536
🏁 Script executed:
# Search for application create/update endpoints more broadly
rg -n '"post"|"put"|"patch"' openapi.json | grep -B5 -A5 "application" | head -40Repository: coollabsio/coolify
Length of output: 44
🏁 Script executed:
# Check what's in the lines around request bodies for applications
sed -n '1,100p' openapi.json | head -50Repository: coollabsio/coolify
Length of output: 1696
🏁 Script executed:
# Find where request bodies reference schemas
rg -n 'requestBody' openapi.jsonRepository: coollabsio/coolify
Length of output: 1501
🏁 Script executed:
# Find the /applications endpoint section and look for POST operation
sed -n '20,500p' openapi.json | grep -n -A30 '"post"'Repository: coollabsio/coolify
Length of output: 3370
🏁 Script executed:
# Look at the requestBody around line 58 (first requestBody)
sed -n '50,100p' openapi.jsonRepository: coollabsio/coolify
Length of output: 2852
🏁 Script executed:
# Search for all /applications patterns with context to find create endpoint
rg -n 'applications' openapi.json | grep -v '\/\/' | head -40Repository: coollabsio/coolify
Length of output: 1256
🏁 Script executed:
# Look for any mention of "CreateApplication" or similar schema
rg -n 'Create|Update' openapi.json | grep -i schema | head -20Repository: coollabsio/coolify
Length of output: 44
🏁 Script executed:
# Check the components.schemas section to see all available schemas
rg -n '"components"' openapi.jsonRepository: coollabsio/coolify
Length of output: 87
🏁 Script executed:
# Look at the complete /applications/public POST requestBody properties
sed -n '45,300p' openapi.json | head -150Repository: coollabsio/coolify
Length of output: 8430
🏁 Script executed:
# Check /applications/{uuid} endpoint around line 2122
sed -n '2120,2280p' openapi.jsonRepository: coollabsio/coolify
Length of output: 6060
🏁 Script executed:
# Search for health_check in request bodies (not in Application schema responses)
rg -n 'health_check' openapi.jsonRepository: coollabsio/coolify
Length of output: 5621
🏁 Script executed:
# Search for properties section in the create/update request to see all fields
sed -n '55,450p' openapi.json | grep -n "health_check"Repository: coollabsio/coolify
Length of output: 858
🏁 Script executed:
# Look at the /applications/{uuid} PATCH requestBody to check its schema
sed -n '2230,2430p' openapi.jsonRepository: coollabsio/coolify
Length of output: 10768
🏁 Script executed:
# Specifically search for health_check_type and health_check_command in request bodies
rg -B5 'health_check_type|health_check_command' openapi.json | grep -A5 -B5 'requestBody'Repository: coollabsio/coolify
Length of output: 44
🏁 Script executed:
# Check the PATCH update endpoint more carefully
sed -n '2188,2450p' openapi.json | head -200Repository: coollabsio/coolify
Length of output: 9802
🏁 Script executed:
# Check if health_check_type and health_check_command are in the PATCH request body
sed -n '2380,2500p' openapi.json | grep -n 'health_check'Repository: coollabsio/coolify
Length of output: 844
🏁 Script executed:
# Also verify the POST endpoints - check one of them more thoroughly
sed -n '176,280p' openapi.json | tail -50Repository: coollabsio/coolify
Length of output: 3049
🏁 Script executed:
# Let me get a broader view - check all POST request body properties to see if health_check_type appears
rg -n '"health_check_type"' openapi.jsonRepository: coollabsio/coolify
Length of output: 110
🏁 Script executed:
# Check all occurrences of health_check_command
rg -n '"health_check_command"' openapi.jsonRepository: coollabsio/coolify
Length of output: 113
🏁 Script executed:
# Final confirmation - check what comes after health_check_start_period in PATCH request body
sed -n '2428,2450p' openapi.jsonRepository: coollabsio/coolify
Length of output: 1437
Add health_check_type and health_check_command to application creation/update request schemas
You've upgraded healthchecks to support CMD-based probes instead of just HTTP—machines will appreciate the flexibility. Self-hosted Coolify instances especially thank you for not being some serverless SaaS nonsense.
However, health_check_type and health_check_command appear only in the Application response schema. The POST and PATCH request bodies for /applications/... endpoints include all the other health_check fields (enabled, path, port, host, method, return_code, scheme, response_text, interval, timeout, retries, start_period) but are missing these two new ones. OpenAPI clients won't see they can configure CMD healthchecks via the API—some generators may silently drop unknown fields.
To fix this, add health_check_type and health_check_command to the request body schemas for all application creation and update endpoints, or refactor those request bodies to reuse a shared schema that includes these fields.
(The oneOf validation to enforce "if health_check_type = 'cmd' then health_check_command must be set" can wait for another deployment cycle on real hardware.)
🤖 Prompt for AI Agents
In openapi.json around lines 10176-10188 the Application response schema defines
health_check_type and health_check_command but the POST/PATCH request body
schemas for /applications/... are missing them; update the application create
and update request body schemas to include these two properties
(health_check_type: string enum ["http","cmd"] with the same description as the
response; health_check_command: nullable string with the same description), or
refactor the request bodies to reference the shared schema that includes these
fields so API clients can submit CMD-based health checks.
| health_check_type: | ||
| type: string | ||
| description: 'Health check type: http or cmd.' | ||
| enum: | ||
| - http | ||
| - cmd | ||
| health_check_command: | ||
| type: string | ||
| nullable: true | ||
| description: 'Health check command for CMD type.' |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Solid addition, but align defaults and request bodies with the new health check type
Nice, the new health_check_type / health_check_command pair makes the API truthfully describe CMD-based health checks instead of pretending everything is HTTP. Even Skynet likes accurate schemas.
A couple of tweaks before deployment to your glorious metal servers:
-
Add explicit default to match the DB/migration
Since the migration setshealth_check_typedefault to'http', it would be clearer to declare that here as well so clients and generated SDKs see the same contract:Suggested schema tweak
health_check_type: type: string
- description: 'Health check type: http or cmd.'
- description: 'Health check type: http or cmd.'
- default: http
enum:- http
- cmd
</details>
2. **Consider exposing these fields in application create/update request bodies**
The various application create/update endpoints (`/applications/*` POSTs and `PATCH /applications/{uuid}`) already expose all the other `health_check_*` properties but not `health_check_type` or `health_check_command`.
If the API is supposed to let users configure CMD health checks (not just read them), it would be good to add these two properties there too, otherwise the spec suggests partial configurability.
3. **Clarify cmd-only usage in the description (optional)**
To avoid confusion with all the HTTP-centric fields, you might slightly tighten the description for `health_check_command`, e.g. “Used only when `health_check_type` is `cmd`”, so API consumers don’t expect it to do anything for `http` checks.
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
health_check_type:
type: string
description: 'Health check type: http or cmd.'
default: http
enum:
- http
- cmd
health_check_command:
type: string
nullable: true
description: 'Health check command for CMD type.'
🤖 Prompt for AI Agents
openapi.yaml lines 6419-6428: the new health_check_type/health_check_command
schema needs three changes: add default: 'http' to health_check_type to match
the DB/migration; add health_check_type and health_check_command properties to
the application create and update request bodies (the POST /applications/* and
PATCH /applications/{uuid} schemas) so clients can set CMD checks as well as
read them; and update the health_check_command description to "Used only when
health_check_type is 'cmd'." to clarify cmd-only usage.
| @else | ||
| {{-- CMD Healthcheck Fields --}} | ||
| <div class="flex flex-col gap-2"> | ||
| <x-forms.textarea canGate="update" :canResource="$resource" id="healthCheckCommand" | ||
| label="Command" | ||
| placeholder="Example: pg_isready -U postgres Example: redis-cli ping Example: curl -f http://localhost:8080/health" | ||
| helper="The command to run inside the container. It should exit with code 0 on success and non-zero on failure." | ||
| required /> | ||
| </div> | ||
| @endif |
There was a problem hiding this comment.
Conditional required attribute may cause validation issues when switching types.
The required attribute on the Command textarea (line 60) will cause browser-level validation errors even when the type is set to 'http'. Consider making the required attribute conditional based on healthCheckType.
Like a taco without the shell, this validation needs some structure - but only when you're actually eating a taco, not when you're having nachos.
🔎 Proposed fix to make `required` conditional
<x-forms.textarea canGate="update" :canResource="$resource" id="healthCheckCommand"
label="Command"
placeholder="Example: pg_isready -U postgres Example: redis-cli ping Example: curl -f http://localhost:8080/health"
helper="The command to run inside the container. It should exit with code 0 on success and non-zero on failure."
- required />
+ :required="$healthCheckType === 'cmd'" />🤖 Prompt for AI Agents
In resources/views/livewire/project/shared/health-checks.blade.php around lines
53–62, the Command textarea currently always has the required attribute which
triggers browser validation even when healthCheckType is 'http'; make the
required attribute conditional (e.g. only add required when $healthCheckType ===
'cmd' using Blade or a :required binding) and also update the corresponding
Livewire/server-side validation rules to only validate presence of the command
when healthCheckType is 'cmd'.
e1264a5 to
342e8e7
Compare
|
Will checkout what coderabbit says later. |
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
|
I will close this on in favor of #8612. CMD not allowed to have shell operators, so it will be a bit more secure than allowing everything. |
2 participants
Changes
Configuration:
In Action:
