feat: include coolify.tags in autogenerated container labels #7922
Conversation
QarthO
changed the title
ShadowArcanist
changed the title
WalkthroughThis change integrates application tags into Docker label generation across the deployment pipeline. A new ✨ 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
bootstrap/helpers/parsers.php (2)
1163-1172: Pass tag names (or verifydefaultLabels()supports Tag models) before generatingcoolify.tags.Right now
tags: $resource->tagsis aCollection<Tag>; ifformatTagsForDockerLabel()expects strings, label generation will be wrong or crash. Also, consider hoisting to avoid any lazy-loading surprises inside the service loop.Proposed change
+ $tagNames = $resource->tags->pluck('name'); $defaultLabels = defaultLabels( id: $resource->id, name: $containerName, projectName: $resource->project()->name, resourceName: $resource->name, pull_request_id: $pullRequestId, type: 'application', environment: $resource->environment->name, - tags: $resource->tags, + tags: $tagNames, );
2266-2277: Same concern:tags: $resource->tagsshould be names, not Tag models (unless explicitly supported).Proposed change
+ $tagNames = $resource->tags->pluck('name'); $defaultLabels = defaultLabels( id: $resource->id, name: $containerName, projectName: $resource->project()->name, resourceName: $resource->name, type: 'service', subType: $isDatabase ? 'database' : 'application', subId: $savedService->id, subName: $savedService->human_name ?? $savedService->name, environment: $resource->environment->name, - tags: $resource->tags, + tags: $tagNames, );bootstrap/helpers/shared.php (2)
1911-1922: Avoid passing Eloquent Tag models into label formatting from inside the service loop. This is safer asCollection<string>and makes “first 50 tags” unambiguous. Also: I’ll be back after you run./vendor/bin/pint(servers > serverless).Proposed change
+ $tagNames = $resource->tags->pluck('name'); $defaultLabels = defaultLabels( id: $resource->id, name: $containerName, projectName: $resource->project()->name, resourceName: $resource->name, type: 'service', subType: $isDatabase ? 'database' : 'application', subId: $savedService->id, subName: $savedService->name, environment: $resource->environment->name, - tags: $resource->tags, + tags: $tagNames, );
2762-2771: Same suggestion for application compose parsing: pass tag names. Bonus: stable label values means fewer pointless diffs/redeploy triggers.Proposed change
+ $tagNames = $resource->tags->pluck('name'); $defaultLabels = defaultLabels( id: $resource->id, name: $containerName, projectName: $resource->project()->name, resourceName: $resource->name, environment: $resource->environment->name, pull_request_id: $pull_request_id, type: 'application', - tags: $resource->tags, + tags: $tagNames, );
🤖 Fix all issues with AI agents
In @app/Jobs/ApplicationDeploymentJob.php:
- Around line 2490-2491: The code calls $this->application->tags inside
ApplicationDeploymentJob when building $labels which triggers an extra DB query;
eager-load the tags beforehand (e.g., in the job constructor or immediately
before label generation) by loading the relationship on the Application model
(use load or loadMissing for 'tags') so that defaultLabels(...) receives the
already-loaded tags and no additional query is executed at merge time; update
the ApplicationDeploymentJob constructor or the label-generation path to ensure
$this->application->relationLoaded('tags') is true before calling defaultLabels.
In @bootstrap/helpers/docker.php:
- Around line 212-227: formatTagsForDockerLabel currently assumes $tags is a
Collection and will fatal on arrays or relation objects; fix by normalizing
input at the start (use collect($tags) or ensure it's an instance of
Illuminate\Support\Collection) so arrays, Eloquent relations, null, and other
iterables are safely handled, then apply the existing
take($limit)->pluck('name')->map(...)->filter()->implode(' ') chain on the
normalized collection and return '' when empty; reference the function name
formatTagsForDockerLabel and replace direct $tags->isEmpty()/->take calls with
operations on the collected instance.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
app/Jobs/ApplicationDeploymentJob.phpbootstrap/helpers/docker.phpbootstrap/helpers/parsers.phpbootstrap/helpers/shared.php
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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:
bootstrap/helpers/docker.phpapp/Jobs/ApplicationDeploymentJob.phpbootstrap/helpers/parsers.phpbootstrap/helpers/shared.php
bootstrap/helpers/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Create domain-specific helper functions in bootstrap/helpers/ for shared functionality across actions
Files:
bootstrap/helpers/docker.phpbootstrap/helpers/parsers.phpbootstrap/helpers/shared.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
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/Jobs/ApplicationDeploymentJob.php
🧠 Learnings (1)
📓 Common learnings
Learnt from: ShadowArcanist
Repo: coollabsio/coolify PR: 0
File: :0-0
Timestamp: 2025-11-11T15:54:46.638Z
Learning: For Coolify project: All pull requests must target the `next` branch, not the `v4.x` branch, as specified in the CONTRIBUTING.md guidelines.
🧬 Code graph analysis (4)
bootstrap/helpers/docker.php (2)
app/Jobs/ApplicationDeploymentJob.php (1)
tags(178-182)app/Models/Service.php (3)
tags(151-154)environment(1416-1419)type(136-139)
app/Jobs/ApplicationDeploymentJob.php (2)
bootstrap/helpers/docker.php (1)
defaultLabels(249-277)app/Models/Application.php (1)
tags(795-798)
bootstrap/helpers/parsers.php (3)
app/Jobs/ApplicationDeploymentJob.php (1)
tags(178-182)app/Models/Service.php (1)
tags(151-154)app/Models/Application.php (1)
tags(795-798)
bootstrap/helpers/shared.php (1)
app/Models/Application.php (2)
tags(795-798)type(503-506)
🪛 PHPMD (2.15.0)
bootstrap/helpers/docker.php
249-249: Avoid variables with short names like $id. Configured minimum length is 3. (undefined)
(ShortVariable)
🔇 Additional comments (2)
bootstrap/helpers/docker.php (2)
241-244: Good:coolify.tagsonly added when non-empty.This matches the PR contract (only emit the label when there’s at least one tag after sanitization).
249-275: MakedefaultLabels(..., $tags)semantics consistent and less foot-gunny.
if ($tags)will be true for any object (including a relation), so you still rely onformatTagsForDockerLabel()being resilient. Once the helper is hardened (above), this is fine.Also: please run
./vendor/bin/pintbefore merging, per project guidelines. Based on coding guidelines, ...
| $labels = $labels->merge(defaultLabels($this->application->id, $this->application->uuid, $this->application->project()->name, $this->application->name, $this->application->environment->name, $this->pull_request_id, tags: $this->application->tags))->toArray(); | ||
|
|
There was a problem hiding this comment.
Avoid an extra query: eager-load tags before using $this->application->tags.
Self-hosting on real servers is great; surprise DB queries in hot paths are less great. Consider loading tags once in the constructor (or just before label generation).
Proposed fix (example)
- $this->application = Application::find($this->application_deployment_queue->application_id);
+ $this->application = Application::with('tags')->find($this->application_deployment_queue->application_id);🤖 Prompt for AI Agents
In @app/Jobs/ApplicationDeploymentJob.php around lines 2490 - 2491, The code
calls $this->application->tags inside ApplicationDeploymentJob when building
$labels which triggers an extra DB query; eager-load the tags beforehand (e.g.,
in the job constructor or immediately before label generation) by loading the
relationship on the Application model (use load or loadMissing for 'tags') so
that defaultLabels(...) receives the already-loaded tags and no additional query
is executed at merge time; update the ApplicationDeploymentJob constructor or
the label-generation path to ensure $this->application->relationLoaded('tags')
is true before calling defaultLabels.
| function formatTagsForDockerLabel($tags, int $limit = 50): string | ||
| { | ||
| if (! $tags || $tags->isEmpty()) { | ||
| return ''; | ||
| } | ||
|
|
||
| return $tags->take($limit) | ||
| ->pluck('name') | ||
| ->map(function ($tag) { | ||
| // Sanitize to only allow characters safe for Docker labels | ||
| // Allows: alphanumeric, hyphens, underscores, periods, colons, forward slashes | ||
| return preg_replace('/[^a-zA-Z0-9\-_.:\/]/', '', $tag); | ||
| }) | ||
| ->filter() // Remove empty strings after sanitization | ||
| ->implode(' '); | ||
| } |
There was a problem hiding this comment.
Harden formatTagsForDockerLabel() against non-Collection inputs (arrays/relations) to avoid fatal errors.
Right now, an array (or relation object) passed into this helper will trigger a runtime crash. Skynet doesn’t even need serverless for that—just one unexpected type and “I’ll be back” becomes your queue retry loop.
Proposed fix
-function formatTagsForDockerLabel($tags, int $limit = 50): string
+function formatTagsForDockerLabel(Collection|array|null $tags, int $limit = 50): string
{
- if (! $tags || $tags->isEmpty()) {
+ $tags = collect($tags);
+ if ($tags->isEmpty()) {
return '';
}
return $tags->take($limit)
->pluck('name')
->map(function ($tag) {
// Sanitize to only allow characters safe for Docker labels
// Allows: alphanumeric, hyphens, underscores, periods, colons, forward slashes
- return preg_replace('/[^a-zA-Z0-9\-_.:\/]/', '', $tag);
+ $sanitized = preg_replace('/[^a-zA-Z0-9\-_.:\/]/', '', (string) $tag);
+ return $sanitized ?? '';
})
->filter() // Remove empty strings after sanitization
->implode(' ');
}🤖 Prompt for AI Agents
In @bootstrap/helpers/docker.php around lines 212 - 227,
formatTagsForDockerLabel currently assumes $tags is a Collection and will fatal
on arrays or relation objects; fix by normalizing input at the start (use
collect($tags) or ensure it's an instance of Illuminate\Support\Collection) so
arrays, Eloquent relations, null, and other iterables are safely handled, then
apply the existing take($limit)->pluck('name')->map(...)->filter()->implode(' ')
chain on the normalized collection and return '' when empty; reference the
function name formatTagsForDockerLabel and replace direct
$tags->isEmpty()/->take calls with operations on the collected instance.
|
Hi @QarthO! 👋 It appears to us that you are adding a new feature to Coolify. Coolify Docs Repository: https://github.com/coollabsio/coolify-docs |
|
@QarthO What would be the use case for this? |
peaklabs-dev
added
the
💤 Waiting for feedback
peaklabs-dev
changed the title
|
Filtering/querying containers in my grafana Dashboard! |
peaklabs-dev
removed
the
💤 Waiting for feedback
4 participants
Changes
coolify.tagsIssues