diff --git a/app/config.php b/app/config.php index 7d5c4ddc..8125e652 100644 --- a/app/config.php +++ b/app/config.php @@ -135,6 +135,7 @@ $c->get(ResultAggregator::class), $c->get(EventDispatcher::class), $c->get(CheckRepository::class), + $c->get(Sandbox::class), ); }, ResultAggregator::class => create(ResultAggregator::class), @@ -214,7 +215,6 @@ 'docker' => new DockerSandbox( $c->get('basePath'), $c->get('appName'), - $c->get('phpschoolGlobalDir') . '/composer-cache', ), default => new HostSandbox(), }; diff --git a/src/ExerciseDispatcher.php b/src/ExerciseDispatcher.php index 7afa33d0..8570fc2d 100644 --- a/src/ExerciseDispatcher.php +++ b/src/ExerciseDispatcher.php @@ -15,6 +15,8 @@ use PhpSchool\PhpWorkshop\Exception\InvalidArgumentException; use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface; use PhpSchool\PhpWorkshop\ExerciseRunner\Context\ExecutionContext; +use PhpSchool\PhpWorkshop\Process\HostSandbox; +use PhpSchool\PhpWorkshop\Process\Sandbox; use PhpSchool\PhpWorkshop\ExerciseRunner\RunnerManager; use PhpSchool\PhpWorkshop\Input\Input; use PhpSchool\PhpWorkshop\Output\OutputInterface; @@ -43,12 +45,14 @@ class ExerciseDispatcher * @param ResultAggregator $results * @param EventDispatcher $eventDispatcher * @param CheckRepository $checkRepository + * @param Sandbox $sandbox Where the code runs, exercises get its host from the context. */ public function __construct( private RunnerManager $runnerManager, private ResultAggregator $results, private EventDispatcher $eventDispatcher, private CheckRepository $checkRepository, + private Sandbox $sandbox = new HostSandbox(), ) {} /** @@ -106,7 +110,7 @@ public function requireCheck(string $requiredCheck): void */ public function verify(ExerciseInterface $exercise, Input $input): ResultAggregator { - $context = ExecutionContext::fromInputAndExercise($input, $exercise); + $context = ExecutionContext::fromInputAndExercise($input, $exercise, $this->sandbox->getHost()); $runner = $this->runnerManager->getRunner($exercise); @@ -161,7 +165,7 @@ public function verify(ExerciseInterface $exercise, Input $input): ResultAggrega */ public function run(ExerciseInterface $exercise, Input $input, OutputInterface $output): bool { - $context = ExecutionContext::fromInputAndExercise($input, $exercise); + $context = ExecutionContext::fromInputAndExercise($input, $exercise, $this->sandbox->getHost()); $runner = $this->runnerManager->getRunner($exercise); diff --git a/src/ExerciseRunner/Context/ExecutionContext.php b/src/ExerciseRunner/Context/ExecutionContext.php index ba1c300c..c1852ccc 100644 --- a/src/ExerciseRunner/Context/ExecutionContext.php +++ b/src/ExerciseRunner/Context/ExecutionContext.php @@ -4,6 +4,7 @@ use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface; use PhpSchool\PhpWorkshop\Input\Input; +use PhpSchool\PhpWorkshop\Process\SandboxHost; use PhpSchool\PhpWorkshop\Utils\Path; use PhpSchool\PhpWorkshop\Utils\System; @@ -14,9 +15,10 @@ public function __construct( private string $referenceExecutionDirectory, private ExerciseInterface $exercise, private Input $input, + private SandboxHost $sandboxHost = new SandboxHost(), ) {} - public static function fromInputAndExercise(Input $input, ExerciseInterface $exercise): ExecutionContext + public static function fromInputAndExercise(Input $input, ExerciseInterface $exercise, SandboxHost $sandboxHost): ExecutionContext { $program = $input->hasArgument('program') ? dirname($input->getRequiredArgument('program')) : (string) getcwd(); @@ -25,6 +27,7 @@ public static function fromInputAndExercise(Input $input, ExerciseInterface $exe System::randomTempDir(), $exercise, $input, + $sandboxHost, ); } @@ -60,6 +63,11 @@ public function getStudentExecutionDirectory(): string return $this->studentExecutionDirectory; } + public function getSandboxHost(): SandboxHost + { + return $this->sandboxHost; + } + public function getReferenceExecutionDirectory(): string { return $this->referenceExecutionDirectory; diff --git a/src/Process/DockerSandbox.php b/src/Process/DockerSandbox.php index ffc253f8..f81353e6 100644 --- a/src/Process/DockerSandbox.php +++ b/src/Process/DockerSandbox.php @@ -15,27 +15,30 @@ final class DockerSandbox implements Sandbox { private const TIMEOUT = 30; + //a named volume, so it lives on the docker host, which may not be this machine + private const COMPOSER_CACHE_VOLUME = 'php-school-composer-cache'; private ExecutableFinder $executableFinder; private string $basePath; private string $projectName; - private string $composerCacheDir; private string $portsNetwork; + private string $portsHost; public function __construct( string $basePath, string $projectName, - string $composerCacheDir, ?ExecutableFinder $executableFinder = null, string $portsNetwork = 'bridge', + string $portsHost = '127.0.0.1', ) { $this->executableFinder = $executableFinder ?? new ExecutableFinder(); $this->basePath = $basePath; $this->projectName = $projectName; - $this->composerCacheDir = $composerCacheDir; //for exercises that expose ports, eg a server the check connects to. Pass a network without internet access //to stop student code reaching out, the published ports still work $this->portsNetwork = $portsNetwork; + //the address ports are published on, the docker host's private address when it is remote + $this->portsHost = $portsHost; } public function getImageName(): string @@ -57,17 +60,22 @@ public function createBuildProcess(): Process ); } + public function getHost(): SandboxHost + { + return new SandboxHost($this->portsHost); + } + public function create(ProcessInput $processInput): DockerExecution { $isComposer = $processInput->getExecutable() === 'composer'; $args = $processInput->getArgs(); $mounts = [$processInput->getWorkingDirectory() . ':/solution']; - $env = ['COMPOSER_HOME' => '/tmp/composer']; + $env = ['COMPOSER_HOME' => '/composer']; if ($isComposer) { //we need to mount a volume for composer cache, and never run package plugins or scripts - $mounts[] = $this->composerCacheDir . ':/tmp/composer'; + $mounts[] = self::COMPOSER_CACHE_VOLUME . ':/composer'; $args = [...$args, '--no-plugins', '--no-scripts']; } @@ -79,7 +87,7 @@ public function create(ProcessInput $processInput): DockerExecution $network = ['--network', $this->portsNetwork]; foreach ($processInput->getExposedPorts() as $port) { $network[] = '-p'; - $network[] = '127.0.0.1:' . $port . ':' . $port; + $network[] = $this->portsHost . ':' . $port . ':' . $port; } } diff --git a/src/Process/HostSandbox.php b/src/Process/HostSandbox.php index 2154ef08..fce86247 100644 --- a/src/Process/HostSandbox.php +++ b/src/Process/HostSandbox.php @@ -22,6 +22,11 @@ public function __construct(?ExecutableFinder $executableFinder = null) } + public function getHost(): SandboxHost + { + return new SandboxHost(); + } + public function create(ProcessInput $processInput): HostExecution { $executablePath = $this->executableFinder->find($processInput->getExecutable()); diff --git a/src/Process/Sandbox.php b/src/Process/Sandbox.php index 784281f4..b2c054bc 100644 --- a/src/Process/Sandbox.php +++ b/src/Process/Sandbox.php @@ -7,4 +7,6 @@ interface Sandbox { public function create(ProcessInput $processInput): SandboxExecution; + + public function getHost(): SandboxHost; } diff --git a/src/Process/SandboxHost.php b/src/Process/SandboxHost.php new file mode 100644 index 00000000..8a086157 --- /dev/null +++ b/src/Process/SandboxHost.php @@ -0,0 +1,41 @@ +address; + } + + /** + * A port that is free to publish. Asks this machine, so it needs another way when the host is remote. + */ + public function getFreePort(): int + { + $server = @stream_socket_server('tcp://127.0.0.1:0'); + + if ($server === false) { + throw new RuntimeException('Cannot create socket'); + } + + //eg. 127.0.0.1:54321 + $name = (string) stream_socket_get_name($server, false); + fclose($server); + + return (int) substr($name, (int) strrpos($name, ':') + 1); + } +} diff --git a/test/ExerciseRunner/CliRunnerTest.php b/test/ExerciseRunner/CliRunnerTest.php index b2f46c27..46ff49d5 100644 --- a/test/ExerciseRunner/CliRunnerTest.php +++ b/test/ExerciseRunner/CliRunnerTest.php @@ -10,6 +10,7 @@ use PhpSchool\PhpWorkshop\Listener\OutputRunInfoListener; use PhpSchool\PhpWorkshop\Process\HostSandbox; use PhpSchool\PhpWorkshop\Process\SandboxExecution; +use PhpSchool\PhpWorkshop\Process\SandboxHost; use PhpSchool\PhpWorkshop\Output\NullOutput; use PhpSchool\PhpWorkshop\Process\ProcessInput; use PhpSchool\PhpWorkshop\Process\Sandbox; @@ -183,6 +184,11 @@ public function create(ProcessInput $processInput): SandboxExecution return (new HostSandbox())->create($processInput); } + + public function getHost(): SandboxHost + { + return new SandboxHost(); + } }; $runner = new CliRunner($this->exercise, $this->eventDispatcher, $sandbox, new EnvironmentManager(new Filesystem(), $this->eventDispatcher)); diff --git a/test/ExerciseRunner/Context/ExecutionContextTest.php b/test/ExerciseRunner/Context/ExecutionContextTest.php index 09a0e291..ad0ea33b 100644 --- a/test/ExerciseRunner/Context/ExecutionContextTest.php +++ b/test/ExerciseRunner/Context/ExecutionContextTest.php @@ -6,6 +6,7 @@ use PhpSchool\PhpWorkshop\ExerciseRunner\Context\ExecutionContext; use PhpSchool\PhpWorkshop\ExerciseRunner\Context\NoEntryPoint; use PhpSchool\PhpWorkshop\Input\Input; +use PhpSchool\PhpWorkshop\Process\SandboxHost; use PhpSchool\PhpWorkshop\Utils\System; use PHPUnit\Framework\TestCase; @@ -90,7 +91,7 @@ public function testFactory(): void $input = new Input('test', ['program' => $temporaryDirectory . '/solution.php']); $exercise = new MockExercise(); - $context = ExecutionContext::fromInputAndExercise($input, $exercise); + $context = ExecutionContext::fromInputAndExercise($input, $exercise, new SandboxHost('10.0.0.1')); //check that student execution directory uses the parent directory of the program from the input static::assertSame($temporaryDirectory, $context->getStudentExecutionDirectory()); @@ -98,5 +99,7 @@ public function testFactory(): void //check that reference execution directory is a random temporary directory static::assertTrue(str_starts_with($context->getReferenceExecutionDirectory(), System::tempDir())); + + static::assertSame('10.0.0.1', $context->getSandboxHost()->getAddress()); } } diff --git a/test/Process/DockerSandboxTest.php b/test/Process/DockerSandboxTest.php index 85e53515..455d693d 100644 --- a/test/Process/DockerSandboxTest.php +++ b/test/Process/DockerSandboxTest.php @@ -10,7 +10,7 @@ class DockerSandboxTest extends TestCase { - private function factory(string $portsNetwork = 'bridge'): DockerSandbox + private function factory(string $portsNetwork = 'bridge', string $portsHost = '127.0.0.1'): DockerSandbox { $finder = $this->createMock(ExecutableFinder::class); $finder->expects($this->once()) @@ -18,7 +18,7 @@ private function factory(string $portsNetwork = 'bridge'): DockerSandbox ->with('docker') ->willReturn('/usr/local/bin/docker'); - return new DockerSandbox('/docker-dir', 'PHP8Appreciate', '/composer/cache/dir', $finder, $portsNetwork); + return new DockerSandbox('/docker-dir', 'PHP8Appreciate', $finder, $portsNetwork, $portsHost); } /** @@ -52,7 +52,7 @@ public function testCreateThrowsExceptionIfDockerNotFound(): void ->with('docker') ->willReturn(null); - $factory = new DockerSandbox('/docker-dir', 'php8appreciate', '/composer/cache/dir', $finder); + $factory = new DockerSandbox('/docker-dir', 'php8appreciate', $finder); $input = new ProcessInput('composer', [], __DIR__, [], []); $factory->create($input); @@ -64,7 +64,7 @@ public function testCreateIsIsolatedWithNoNetwork(): void $expected = $this->expectedCommand( ['--network', 'none'], - ['-e', 'COMPOSER_HOME=/tmp/composer', '-v', __DIR__ . ':/solution'], + ['-e', 'COMPOSER_HOME=/composer', '-v', __DIR__ . ':/solution'], ['php', 'one', 'two'], ); static::assertSame($expected, $process->getCommandLine()); @@ -78,7 +78,7 @@ public function testComposerGetsNetworkCacheAndNoPluginsOrScripts(): void $expected = $this->expectedCommand( ['--network', 'bridge'], - ['-e', 'COMPOSER_HOME=/tmp/composer', '-v', __DIR__ . ':/solution', '-v', '/composer/cache/dir:/tmp/composer'], + ['-e', 'COMPOSER_HOME=/composer', '-v', __DIR__ . ':/solution', '-v', 'php-school-composer-cache:/composer'], ['composer', 'install', '--no-plugins', '--no-scripts'], ); static::assertSame($expected, $process->getCommandLine()); @@ -90,7 +90,7 @@ public function testExposedPortsArePublishedOnLoopbackOnly(): void $expected = $this->expectedCommand( ['--network', 'bridge', '-p', '127.0.0.1:8000:8000'], - ['-e', 'COMPOSER_HOME=/tmp/composer', '-v', __DIR__ . ':/solution'], + ['-e', 'COMPOSER_HOME=/composer', '-v', __DIR__ . ':/solution'], ['php'], ); static::assertSame($expected, $process->getCommandLine()); @@ -102,19 +102,38 @@ public function testExposedPortsUseTheGivenNetwork(): void $expected = $this->expectedCommand( ['--network', 'php-school-sandbox', '-p', '127.0.0.1:8000:8000'], - ['-e', 'COMPOSER_HOME=/tmp/composer', '-v', __DIR__ . ':/solution'], + ['-e', 'COMPOSER_HOME=/composer', '-v', __DIR__ . ':/solution'], ['php'], ); static::assertSame($expected, $process->getCommandLine()); } + public function testExposedPortsArePublishedOnTheGivenHost(): void + { + $process = $this->factory('bridge', '172.19.0.2')->create(new ProcessInput('php', [], __DIR__, [], [8000]))->process; + + $expected = $this->expectedCommand( + ['--network', 'bridge', '-p', '172.19.0.2:8000:8000'], + ['-e', 'COMPOSER_HOME=/composer', '-v', __DIR__ . ':/solution'], + ['php'], + ); + static::assertSame($expected, $process->getCommandLine()); + } + + public function testHostIsWherePortsArePublished(): void + { + $sandbox = new DockerSandbox('/docker-dir', 'php8appreciate', null, 'bridge', '172.19.0.2'); + + static::assertSame('172.19.0.2', $sandbox->getHost()->getAddress()); + } + public function testCreateWithEnv(): void { $process = $this->factory()->create(new ProcessInput('php', [], __DIR__, ['SOME_VAR' => 'value'], []))->process; $expected = $this->expectedCommand( ['--network', 'none'], - ['-e', 'SOME_VAR=value', '-e', 'COMPOSER_HOME=/tmp/composer', '-v', __DIR__ . ':/solution'], + ['-e', 'SOME_VAR=value', '-e', 'COMPOSER_HOME=/composer', '-v', __DIR__ . ':/solution'], ['php'], ); static::assertSame($expected, $process->getCommandLine()); diff --git a/test/Process/SandboxHostTest.php b/test/Process/SandboxHostTest.php new file mode 100644 index 00000000..d9e72138 --- /dev/null +++ b/test/Process/SandboxHostTest.php @@ -0,0 +1,26 @@ +getAddress()); + static::assertSame('10.0.0.1', (new SandboxHost('10.0.0.1'))->getAddress()); + } + + public function testGetFreePortCanBeListenedOn(): void + { + $port = (new SandboxHost())->getFreePort(); + + $server = stream_socket_server('tcp://127.0.0.1:' . $port); + static::assertNotFalse($server); + fclose($server); + } +}