Skip to content

feat: add support for changing docker labels via application api #1939

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
m41denx wants to merge 9 commits into from
Closed

feat: add support for changing docker labels via application api #1939

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2254c38
Docker labels
m41denx Oct 28, 2025
12053f7
Rename $payload to $attributes
m41denx Nov 27, 2025
13814b8
Ignore falsy docker_labels
m41denx Nov 27, 2025
785124a
Prevent docker labels from being a flat array and therefore having em…
m41denx Nov 27, 2025
3a401a5
Added docker labels validation
m41denx Nov 27, 2025
70ae7e9
Validate Docker labels rule
m41denx Nov 30, 2025
a5350c2
Fix pint and phpstan errors
m41denx Nov 30, 2025
321e9fe
Update regex
m41denx Dec 2, 2025
605b3e7
Return "required" validator
m41denx Dec 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions app/Http/Requests/Api/Application/Servers/StoreServerRequest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ public function rules(): array
'user' => $rules['owner_id'],
'egg' => $rules['egg_id'],
'docker_image' => 'sometimes|string',
'docker_labels' => 'sometimes|array',
// Docker labels are validated via https://regex101.com/r/FiYrwo/1 following Docker key format
// recommendations: https://docs.docker.com/engine/manage-resources/labels/
'docker_labels.*' => 'required|regex:/^(?!(?:com\.docker\.|io\.docker\.|org\.dockerproject\.))(?=.*[a-z]$)[a-z](?:[a-z0-9]|(?<!\.)\.(?!\.)|(?<!-)-(?!-))*$/',
Copy link

@coderabbitai coderabbitai bot Nov 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Regex validates values instead of keys.

Laravel's docker_labels.* syntax validates array VALUES, not KEYS. Given input {"docker_labels": {"my.label": "myvalue"}}, the regex on line 35 validates "myvalue", not "my.label". This means:

  • Invalid keys (e.g., "Com.Docker.test", "123numeric") pass validation
  • The core requirement to validate Docker label keys per Docker recommendations is not met

To validate keys, implement a custom validation rule:

namespace App\Rules;

use Illuminate\Contracts\Validation\Rule;

class DockerLabelKeysRule implements Rule
{
    private const KEY_PATTERN = '/^(?!(?:com\.docker\.|io\.docker\.|org\.dockerproject\.))(?=.*[a-z]$)[a-z](?:[a-z0-9]|(?<!\.)\.(?!\.)|(?<!-)-(?!-))*$/';

    public function passes($attribute, $value)
    {
        if (!is_array($value)) {
            return false;
        }

        foreach ($value as $key => $val) {
            if (!is_string($val) || !preg_match(self::KEY_PATTERN, $key)) {
                return false;
            }
        }

        return true;
    }

    public function message()
    {
        return 'Docker label keys must follow Docker naming conventions and values must be strings.';
    }
}

Then update the validation rules:

-            'docker_labels' => 'sometimes|array',
-            // Docker labels are validated via https://regex101.com/r/FiYrwo/1 following Docker key format
-            // recommendations: https://docs.docker.com/engine/manage-resources/labels/
-            'docker_labels.*' => 'required|regex:/^(?!(?:com\.docker\.|io\.docker\.|org\.dockerproject\.))(?=.*[a-z]$)[a-z](?:[a-z0-9]|(?<!\.)\.(?!\.)|(?<!-)-(?!-))*$/',
+            // Docker labels are validated following Docker key format recommendations:
+            // https://docs.docker.com/engine/manage-resources/labels/
+            'docker_labels' => ['sometimes', 'array', new \App\Rules\DockerLabelKeysRule()],
🤖 Prompt for AI Agents 
In app/Http/Requests/Api/Application/Servers/StoreServerRequest.php around lines
32–35, the current rule 'docker_labels.*' applies the regex to array values
instead of label keys; replace this by creating a custom validation rule (e.g.,
App\Rules\DockerLabelKeysRule) that: checks the input is an array, iterates over
each key to validate it against the provided Docker key regex and ensures each
value is a string, and returns a descriptive error message; then update the
request rules to use 'docker_labels' => ['sometimes','array', new
\App\Rules\DockerLabelKeysRule()], remove the 'docker_labels.*' regex rule, and
add the appropriate use/import for the new rule.

Copy link
Contributor Author

@m41denx m41denx Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uhh, I'll wait for maintainers response, I have little idea how laravel works besides basic http contracts

Copy link

@coderabbitai coderabbitai bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🌐 Web query:

Laravel validation array.* validates keys or values associative array

💡 Result:

Short answer: array.* validates the array's values (each element), not the keys. Use the array rule (or Rule::array / array:keys) to restrict allowed keys or the newer in_array_keys/required_array_keys rules to require specific keys. [1][2][3]

Examples:

  • 'items.*' => 'string' validates every value in items (items['foo'] and items[0] must be strings). [2]
  • 'items' => 'array:allowed_key1,allowed_key2' restricts which keys may appear. [1]
  • Use in_array_keys / required_array_keys when you need “at least one of these keys” or “these keys must exist.” [3]

Sources:
[1] Laravel Validation — array rule (allowed keys).
[2] Laravel Validation — validating nested arrays / wildcard (items.* example).
[3] Laravel News / docs — in_array_keys / required_array_keys additions.

@m41denx I can confirm that CodeRabbit's finding is correct. In Laravel, the docker_labels.* syntax validates the values of your array, not the keys.

So with this input:

{
  "docker_labels": {
    "COM.DOCKER.test": "some-value",
    "123invalid": "another-value"
  }
}

Your current validation would check if "some-value" and "another-value" match the Docker key regex (which they won't, but for the wrong reason). The actual keys ("COM.DOCKER.test" and "123invalid") aren't validated at all, even though they violate Docker's naming rules.

Two approaches to fix this:

  1. Custom validation rule (as CodeRabbit suggested) - cleaner but requires creating a new class
  2. Simple inline validation in withValidator() - quick fix without new files:
public function withValidator(Validator $validator): void
{
    $validator->after(function ($validator) {
        if ($this->has('docker_labels')) {
            $pattern = '/^(?!(?:com\.docker\.|io\.docker\.|org\.dockerproject\.))(?=.*[a-z]$)[a-z](?:[a-z0-9]|(?<!\.)\.(?!\.)|(?<!-)-(?!-))*$/';
            foreach ($this->input('docker_labels', []) as $key => $value) {
                if (!preg_match($pattern, $key)) {
                    $validator->errors()->add('docker_labels.' . $key, 'Invalid Docker label key format.');
                }
                if (!is_string($value)) {
                    $validator->errors()->add('docker_labels.' . $key, 'Docker label value must be a string.');
                }
            }
        }
    });
    
    // ... existing withValidator code ...
}

Totally understand waiting for maintainer input on the approach! Let me know if you'd like help implementing either solution once you get direction. 👍

🧠 Learnings used 
Learnt from: m41denx
Repo: pelican-dev/panel PR: 1939
File: app/Models/Server.php:187-188
Timestamp: 2025-11-27T18:07:20.069Z
Learning: In Laravel API request validation, when validating JSON object inputs (e.g., docker_labels), there's no need to validate that keys are strings because JSON object keys are always strings per the JSON specification. Laravel automatically parses JSON requests with string keys.

'startup' => 'sometimes|string',
'environment' => 'present|array',
'skip_scripts' => 'sometimes|boolean',
Expand Down Expand Up @@ -122,6 +126,7 @@ public function validated($key = null, $default = null): array
'allocation_limit' => array_get($data, 'feature_limits.allocations'),
'backup_limit' => array_get($data, 'feature_limits.backups'),
'oom_killer' => array_get($data, 'oom_killer'),
'docker_labels' => array_get($data, 'docker_labels'),
];
}

Expand Down
12 changes: 10 additions & 2 deletions app/Http/Requests/Api/Application/Servers/UpdateServerDetailsRequest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,30 @@ public function rules(): array
'name' => $rules['name'],
'user' => $rules['owner_id'],
'description' => array_merge(['nullable'], $rules['description']),
'docker_labels' => 'sometimes|array',
'docker_labels.*' => 'string',
];
}

/**
* Convert the posted data into the correct format that is expected by the application.
*
* @return array<array-key, string>
* @return array<string, mixed>
*/
public function validated($key = null, $default = null): array
{
return [
$attributes = [
'external_id' => $this->input('external_id'),
'name' => $this->input('name'),
'owner_id' => $this->input('user'),
'description' => $this->input('description'),
];

if ($this->has('docker_labels')) {
$attributes['docker_labels'] = $this->input('docker_labels');
}

return $attributes;
}

/**
Expand Down
5 changes: 5 additions & 0 deletions app/Models/Server.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,11 @@ class Server extends Model implements HasAvatar, Validatable
'skip_scripts' => ['sometimes', 'boolean'],
'image' => ['required', 'string', 'max:255'],
'icon' => ['sometimes', 'nullable', 'string'],
'docker_labels' => ['array'],
// Docker labels are validated via https://regex101.com/r/FiYrwo/1 following Docker key format
// recommendations: https://docs.docker.com/engine/manage-resources/labels/

'docker_labels.*' => ['required', 'regex:/^(?!(?:com\.docker\.|io\.docker\.|org\.dockerproject\.))(?=.*[a-z]$)[a-z](?:[a-z0-9]|(?<!\.)\.(?!\.)|(?<!-)-(?!-))*$/'],
'database_limit' => ['present', 'nullable', 'integer', 'min:0'],
'allocation_limit' => ['sometimes', 'nullable', 'integer', 'min:0'],
'backup_limit' => ['present', 'nullable', 'integer', 'min:0'],
Expand Down
13 changes: 10 additions & 3 deletions app/Services/Servers/DetailsModificationService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ public function __construct(private ConnectionInterface $connection, private Dae
* external_id: int,
* owner_id: int,
* name: string,
* description?: ?string
* description?: ?string,
* docker_labels?: array<string, string>
* } $data
*
* @throws Throwable
Expand All @@ -36,12 +37,18 @@ public function handle(Server $server, array $data): Server
return $this->connection->transaction(function () use ($data, $server) {
$owner = $server->owner_id;

$server->forceFill([
$attributes = [
'external_id' => Arr::get($data, 'external_id'),
'owner_id' => Arr::get($data, 'owner_id'),
'name' => Arr::get($data, 'name'),
'description' => Arr::get($data, 'description') ?? '',
])->saveOrFail();
];

if ($labels = Arr::get($data, 'docker_labels')) {
$attributes['docker_labels'] = $labels;
}

$server->forceFill($attributes)->saveOrFail();

// If the owner_id value is changed we need to revoke any tokens that exist for the server
// on the daemon instance so that the old owner no longer has any permission to access the
Expand Down
1 change: 1 addition & 0 deletions app/Transformers/Api/Application/ServerTransformer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ public function transform($server): array
'container' => [
'startup_command' => $server->startup,
'image' => $server->image,
'docker_labels' => $server->docker_labels ?? [],
// This field is deprecated, please use "status".
'installed' => $server->isInstalled() ? 1 : 0,
'environment' => $this->environmentService->handle($server),
Expand Down
51 changes: 51 additions & 0 deletions tests/Integration/Services/Servers/ServerCreationServiceTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,57 @@ public function test_server_without_allocation_is_created_with_deployment_object
$this->assertNull($response->allocation_id);
}

/**
* Test that docker labels are persisted when provided during server creation.
*/
public function test_server_creation_accepts_docker_labels(): void
{
/** @var User $user */
$user = User::factory()->create();

/** @var Node $node */
$node = Node::factory()->create();

/** @var Allocation $allocation */
$allocation = Allocation::factory()->create([
'node_id' => $node->id,
]);

$egg = $this->cloneEggAndVariables($this->bungeecord);

$labels = [
'com.example/app' => 'panel',
'tier' => 'production',
];

$data = [
'name' => $this->faker->name(),
'description' => $this->faker->sentence(),
'owner_id' => $user->id,
'allocation_id' => $allocation->id,
'node_id' => $allocation->node_id,
'memory' => 256,
'swap' => 128,
'disk' => 100,
'io' => 500,
'cpu' => 0,
'startup' => 'java -jar server.jar',
'image' => 'java:8',
'egg_id' => $egg->id,
'environment' => [
'BUNGEE_VERSION' => '123',
'SERVER_JARFILE' => 'server.jar',
],
'docker_labels' => $labels,
];

$this->daemonServerRepository->expects('setServer->create')->with(false)->andReturnUndefined();

$server = $this->getService()->handle($data);

$this->assertSame($labels, $server->docker_labels);
}

/**
* Test that a server is deleted from the Panel if daemon returns an error during the creation
* process.
Expand Down