Skip to content

Add Docker mount value object#68

Open
TorstenDittmann wants to merge 2 commits into
mainfrom
feature/mount-value-object
Open

Add Docker mount value object#68
TorstenDittmann wants to merge 2 commits into
mainfrom
feature/mount-value-object

Conversation

@TorstenDittmann
Copy link
Copy Markdown
Contributor

Summary

  • add Mount::bind() and Mount::volume() value object support
  • render Mount objects as Docker API HostConfig.Mounts and Docker CLI --mount flags
  • keep legacy string volumes as binds/--volume and document volume subpath behavior

Tests

  • ./vendor/bin/phpunit --configuration phpunit.xml tests/Orchestration/MountTest.php
  • ./vendor/bin/phpstan analyse --level 6 src tests
  • ./vendor/bin/pint --test

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Jun 1, 2026

Greptile Summary

This PR introduces a Mount value object with bind() and volume() factory methods and renders it correctly to both the Docker API (HostConfig.Mounts) and the Docker CLI (--mount flag), while preserving legacy string volumes as Binds/--volume.

  • DockerCLI::run() is cleanly refactored into run() + protected getRunCommand(), enabling the new unit tests to introspect the assembled command without executing Docker.
  • Tests now cover volume serialisation, bind serialisation, bind-with-subpath path concatenation, and end-to-end integration with both adapters via lightweight stub subclasses.

Confidence Score: 5/5

Safe to merge; the change is additive and all existing behaviour is preserved.

The adapter split between Binds and Mounts is correct, the value object serialises as Docker expects, and the CLI refactoring preserves identical runtime behaviour. The only finding is an edge-case path-with-comma issue in toDockerCLI() that would require deliberately unusual input to trigger.

src/Orchestration/Mount.php — the comma-delimiter guard in toDockerCLI().

Important Files Changed

Filename Overview
src/Orchestration/Mount.php New value object for bind/volume mounts; serialises correctly to Docker API and CLI formats, but toDockerCLI() uses a raw comma delimiter with no escaping, so a comma in source/target silently produces a malformed --mount string.
src/Orchestration/Adapter/DockerAPI.php Correctly splits volumes into Binds (legacy strings) and Mounts (Mount objects); Docker API handles both fields coexisting and accepts empty arrays.
src/Orchestration/Adapter/DockerCLI.php Run logic cleanly extracted into getRunCommand() for testability; Mount objects emit --mount flags, strings keep --volume; refactoring is correct and preserves prior behaviour.
tests/Orchestration/MountTest.php Covers volume, bind, and bind-with-subpath serialization; integration tests verify both adapters route strings vs Mount objects correctly using lightweight subclass stubs.
src/Orchestration/Orchestration.php Docblock updated to reflect the new array<int, string
src/Orchestration/Adapter.php Docblock updated to reflect the new $volumes type; no logic changes.
README.md Documents Mount::bind() and Mount::volume() usage in example, with a note on bind vs volume subpath semantics.

Reviews (2): Last reviewed commit: "Cover bind mount serialization" | Re-trigger Greptile

Comment thread tests/Orchestration/MountTest.php
Comment thread src/Orchestration/Mount.php
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant