Feature/service application api endpoints #7338
Conversation
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
WalkthroughThis pull request adds two new API endpoints to the ServicesController for managing service applications. The GET endpoint retrieves all applications for a given service with their configuration details. The PATCH endpoint updates a specific application within a service, including domain validation, conflict checking, and conditional regeneration of configurations. The changes include corresponding route definitions, OpenAPI documentation for both JSON and YAML formats, and comprehensive feature tests covering authentication, validation, and error handling scenarios. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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: 12
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
app/Http/Controllers/Api/ServicesController.php(2 hunks)openapi.json(1 hunks)openapi.yaml(1 hunks)routes/api.php(1 hunks)tests/Feature/ServiceApplicationApiTest.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: Use PHP 8.4 constructor property promotion and typed properties
Follow PSR-12 coding standards and run./vendor/bin/pintbefore committing
Use Eloquent ORM for database interactions, avoid raw queries
Use Laravel's built-in mocking and Mockery for testing external services and dependencies
Use database transactions for critical operations that modify multiple related records
Leverage query scopes in Eloquent models for reusable, chainable query logic
Never log or expose sensitive data (passwords, tokens, API keys, SSH keys) in logs or error messages
Always validate user input using Form Requests, Rules, or explicit validation methods
UsehandleError()helper for consistent error handling and logging
Use eager loading (with(), load()) to prevent N+1 queries when accessing related models
Use chunking for large data operations to avoid memory exhaustion
Implement caching for frequently accessed data using Laravel's cache helpers
Write descriptive variable and method names that clearly express intent
Keep methods small and focused on a single responsibility
Document complex logic with clear comments explaining the 'why' not just the 'what'Always run code formatting with
./vendor/bin/pintbefore committing code
Files:
app/Http/Controllers/Api/ServicesController.phptests/Feature/ServiceApplicationApiTest.phproutes/api.php
tests/Feature/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Feature tests may use database. MUST be run inside Docker container with
docker exec coolify php artisan testRun Feature tests inside Docker using
docker exec coolify php artisan testto prevent database connection errors; NEVER run them outside Docker
Files:
tests/Feature/ServiceApplicationApiTest.php
tests/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.php: Use Pest for all tests with expressive syntax and clear test organization
Design tests to use mocking and dependency injection instead of database when possible; only use database in Feature tests when necessary
Mock external services and SSH connections in tests instead of making real connections
Files:
tests/Feature/ServiceApplicationApiTest.php
routes/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Use named routes with
route()function instead of hardcoded URLs
Files:
routes/api.php
routes/api.php
📄 CodeRabbit inference engine (CLAUDE.md)
routes/api.php: Use RESTful API design with proper HTTP verbs (GET, POST, PUT, DELETE) and implement rate limiting for public endpoints
Use API Resources for response formatting in API endpoints
Files:
routes/api.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:
routes/api.php
🧠 Learnings (6)
📚 Learning: 2025-11-25T09:32:36.504Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:32:36.504Z
Learning: Applies to tests/**/*.php : Design tests to use mocking and dependency injection instead of database when possible; only use database in Feature tests when necessary
Applied to files:
tests/Feature/ServiceApplicationApiTest.php
📚 Learning: 2025-11-25T09:32:36.504Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:32:36.504Z
Learning: Applies to tests/Feature/**/*.php : Feature tests may use database. MUST be run inside Docker container with `docker exec coolify php artisan test`
Applied to files:
tests/Feature/ServiceApplicationApiTest.php
📚 Learning: 2025-11-25T09:32:36.504Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:32:36.504Z
Learning: Applies to **/*.php : Use Laravel's built-in mocking and Mockery for testing external services and dependencies
Applied to files:
tests/Feature/ServiceApplicationApiTest.php
📚 Learning: 2025-11-25T09:32:36.504Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:32:36.504Z
Learning: Applies to tests/**/*.php : Mock external services and SSH connections in tests instead of making real connections
Applied to files:
tests/Feature/ServiceApplicationApiTest.php
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to tests/Feature/**/*.php : Run Feature tests inside Docker using `docker exec coolify php artisan test` to prevent database connection errors; NEVER run them outside Docker
Applied to files:
tests/Feature/ServiceApplicationApiTest.php
📚 Learning: 2025-11-25T09:32:36.504Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:32:36.504Z
Learning: Applies to routes/api.php : Use RESTful API design with proper HTTP verbs (GET, POST, PUT, DELETE) and implement rate limiting for public endpoints
Applied to files:
routes/api.php
🧬 Code graph analysis (2)
tests/Feature/ServiceApplicationApiTest.php (2)
app/Models/Team.php (1)
members(220-223)app/Models/User.php (1)
createToken(198-216)
routes/api.php (1)
app/Http/Controllers/Api/ServicesController.php (1)
ServicesController(20-2057)
🪛 PHPMD (2.15.0)
app/Http/Controllers/Api/ServicesController.php
1622-1806: The method update_application() has a Cyclomatic Complexity of 24. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
1622-1806: The method update_application() has an NPath complexity of 430080. The configured NPath complexity threshold is 200. (undefined)
(NPathComplexity)
1622-1806: The method update_application() has 185 lines of code. Current threshold is set to 100. Avoid really long methods. (undefined)
(ExcessiveMethodLength)
1622-1806: The method update_application is not named in camelCase. (undefined)
(CamelCaseMethodName)
1700-1700: Avoid using static access to class '\Spatie\Url\Url' in method 'update_application'. (undefined)
(StaticAccess)
🪛 YAMLlint (1.37.1)
openapi.yaml
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5587-5587: too many spaces inside braces
(braces)
[error] 5667-5667: too many spaces inside braces
(braces)
[error] 5667-5667: too many spaces inside braces
(braces)
[error] 5668-5668: too many spaces inside braces
(braces)
[error] 5668-5668: too many spaces inside braces
(braces)
[error] 5669-5669: too many spaces inside braces
(braces)
[error] 5669-5669: too many spaces inside braces
(braces)
[error] 5670-5670: too many spaces inside braces
(braces)
[error] 5670-5670: too many spaces inside braces
(braces)
[error] 5671-5671: too many spaces inside braces
(braces)
[error] 5671-5671: too many spaces inside braces
(braces)
[error] 5672-5672: too many spaces inside braces
(braces)
[error] 5672-5672: too many spaces inside braces
(braces)
[error] 5673-5673: too many spaces inside braces
(braces)
[error] 5673-5673: too many spaces inside braces
(braces)
[error] 5674-5674: too many spaces inside braces
(braces)
[error] 5674-5674: too many spaces inside braces
(braces)
[error] 5675-5675: too many spaces inside braces
(braces)
[error] 5675-5675: too many spaces inside braces
(braces)
[error] 5676-5676: too many spaces inside braces
(braces)
[error] 5676-5676: too many spaces inside braces
(braces)
[error] 5677-5677: too many spaces inside braces
(braces)
[error] 5677-5677: too many spaces inside braces
(braces)
[error] 5689-5689: too many spaces inside braces
(braces)
[error] 5689-5689: too many spaces inside braces
(braces)
[error] 5690-5690: too many spaces inside braces
(braces)
[error] 5690-5690: too many spaces inside braces
(braces)
[error] 5690-5690: too many spaces inside braces
(braces)
[error] 5690-5690: too many spaces inside braces
(braces)
🔇 Additional comments (10)
openapi.json (2)
8754-8847: Overall: solid endpoints; self-hosted vibes strong. I’ll be back.Great additions: clear scopes, auth, and restart messaging. A few doc polish items above will make client usage bulletproof. Servers > serverless. Now pass the tacos.
Also applies to: 8848-9026
8754-8847: I'm unable to verify this concern due to persistent repository access issues. Let me provide the rewritten review comment based on what can be determined:
Verify naming consistency of
is_stripprefix_enabledacross codebase/tests.The OpenAPI schema shows
is_stripprefix_enabled(no underscores between "strip" and "prefix"). Confirm all references in code, tests, and documentation follow this exact naming to avoid runtime issues from field name mismatches.
routes/api.php (1)
152-154: LGTM! Hasta la vista, insecure endpoints!I have analyzed these routes with my advanced cybernetic systems. They follow RESTful design patterns correctly - GET for reading, PATCH for updating. Proper middleware authorization applied. These self-hosted API endpoints will not be terminated.
The routes integrate well with the existing services scope. Like a perfect taco, everything is wrapped together nicely.
app/Http/Controllers/Api/ServicesController.php (5)
14-17: LGTM! New imports are correct.My sensors detect these imports are properly utilized.
ServiceApplicationmodel for the new application lookups, andSpatie\Url\Urlfor domain validation. Clean and efficient, like a well-maintained server rack.
1448-1479: LGTM! Clean and efficient like a T-800's targeting system.This method follows the established patterns in this codebase. Authorization properly checks service-level view permission. The explicit field mapping acts as a whitelist - no sensitive data leakage.
Self-hosted applications, properly listed. No serverless nonsense here.
1388-1447: LGTM! OpenAPI documentation is comprehensive.These API docs are documented better than the history of taco recipes. All response codes covered, security properly specified, parameter descriptions clear. Self-hosted APIs deserve self-documented endpoints.
1481-1621: OpenAPI spec for PATCH endpoint is well-structured.The documentation includes domain conflict (409) handling and proper field descriptions with warnings about image changes. Very thorough. I approve this like I approve of gluten-free options at Mexican restaurants.
1761-1772: I'll verify the concerns raised in this review comment by examining the codebase to determine if the destination and server relationships can be null, and whether defensive checks are needed.
<function_calls>
#!/bin/bashFind the Service model to check destination relationship definition
fd -t f "Service.php" | grep -E "(app/Models|models)" | head -5
</function_calls>Please provide the results of this command, and I'll continue the verification.
tests/Feature/ServiceApplicationApiTest.php (1)
73-109: Based on my investigation, I cannot definitively verify the review comment's core claim due to repository access limitations. However, I found important context:Key Finding: Laravel's FormRequest validation runs before the controller method executes (during dependency resolution). This means if the endpoint uses FormRequest validation, validation logic runs before any 404 could be triggered.
The review comment claims tests "will return 404 before the controller ever reaches validation logic," but this depends on:
- Whether the endpoint uses FormRequest validation (which would run first)
- Whether 404 occurs at the routing/model-binding layer vs. in the controller
Without access to the actual controller (
app/Http/Controllers/Api/) and request validation class for thePATCH /api/v1/services/{service-uuid}/applications/{app-uuid}endpoint, I cannot confirm the review's premise.Additionally, the review requests verification of factory existence, but the shell script failed to clone the repository.
Rewritten Review Comment:
Verify if validation tests actually bypass validation or if validation runs before 404.
The tests use non-existent service/application UUIDs (
service-uuid/app-uuid). The assumption that these return 404 before validation is critical but unverified. In Laravel, FormRequest validation runs during dependency resolution—before the controller method executes. If this endpoint uses FormRequest validation, validation logic runs first regardless of whether the service exists.Clarify:
- Does the
PATCH /api/v1/services/{service-uuid}/applications/{app-uuid}endpoint use FormRequest validation, or is validation done inside the controller?- If FormRequest: validation runs before any 404 logic, and the test premise is invalid.
- If controller-level validation: verify whether the validation checks exist in the controller before the service lookup that causes 404.
If validation is genuinely bypassed, consider adding Service/ServiceApplication fixtures using model factories to properly exercise validation logic.
openapi.yaml (1)
5620-5660: YAML formatting: Fix excess spaces inside braces.The static analysis tool reports "too many spaces inside braces" errors throughout the response schema definitions. While functionally valid, this violates YAML style consistency. Normalize spacing in inline property definitions.
Example fix (line 5587 and similar):
- items: - properties: { uuid: { type: string, example: app-uuid-123 }, name: { type: string, example: n8n }, ... + items: + properties: + uuid: + type: string + example: app-uuid-123 + name: + type: string + example: n8nOr, if inline is preferred, use single spaces consistently:
- properties: { uuid: { type: string, example: app-uuid-123 }, name: { type: string, example: n8n }, ... + properties: {uuid: {type: string, example: app-uuid-123}, name: {type: string, example: n8n}, ...Likely an incorrect or invalid review comment.
…doc in yaml and json.
…nt to both yaml and json files.
…d json and ymal files.
2 participants
Docs PR: coollabsio/coolify-docs#450
Changes
Adds API endpoints to manage service applications programmatically.
GET /api/v1/services/{uuid}/applications- List applicationsPATCH /api/v1/services/{uuid}/applications/{app_uuid}- Update applicationUpdatable Fields
fqdn,human_name,description,image,exclude_from_status,is_log_drain_enabled,is_gzip_enabled,is_stripprefix_enabledFeatures
Issues