diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index 4ae65c4..56c2e4a 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -16,6 +16,7 @@ jobs: - "8.0" - "8.1" - "8.2" + - "8.3" include: - php-version: "8.0" phpunit: "9.5" diff --git a/Asm/Ansible/Command/AbstractAnsibleCommand.php b/Asm/Ansible/Command/AbstractAnsibleCommand.php index 6dfd8ae..d3e6829 100644 --- a/Asm/Ansible/Command/AbstractAnsibleCommand.php +++ b/Asm/Ansible/Command/AbstractAnsibleCommand.php @@ -176,16 +176,18 @@ protected function runProcess(?callable $callback): int|string // exit code $result = $process->run($callback); - // text-mode - if (null === $callback) { - $result = $process->getOutput(); + // if a callback is set, we return the exit code + if (null !== $callback) { + return $result; + } - if (false === $process->isSuccessful()) { - $process->getErrorOutput(); - } + // if no callback is set, and the process is not successful, we return the output + if (false === $process->isSuccessful()) { + return $process->getErrorOutput(); } - return $result; + // if no callback is set, and the process is successful, we return the output + return $process->getOutput(); } /** diff --git a/Asm/Ansible/Command/AnsiblePlaybook.php b/Asm/Ansible/Command/AnsiblePlaybook.php index 98dcfb4..c830e39 100644 --- a/Asm/Ansible/Command/AnsiblePlaybook.php +++ b/Asm/Ansible/Command/AnsiblePlaybook.php @@ -4,6 +4,7 @@ namespace Asm\Ansible\Command; +use Asm\Ansible\Utils\Str; use InvalidArgumentException; /** @@ -180,12 +181,20 @@ public function diff(): AnsiblePlaybookInterface * * ## String * You can also pass the raw extra vars string directly. - + * * Example: * ```php * $ansible = new Ansible()->playbook()->extraVars('path=/some/path'); * ``` * + * ## JSON String + * You can also use a JSON string to pass the key/value pairs of extra vars. + * + * Example: + * ```php + * $ansible = new Ansible()->playbook()->extraVars('{ "key1": "value1" }'); + * ``` + * * @param string|array $extraVars * @return AnsiblePlaybookInterface */ @@ -210,6 +219,21 @@ public function extraVars(string|array $extraVars = ''): AnsiblePlaybookInterfac throw new InvalidArgumentException(sprintf('Expected string|array, got "%s"', gettype($extraVars))); } + // Trim the string & check if empty before moving on. + $extraVars = trim($extraVars); + + if ($extraVars === '') { + return $this; + } + + // JSON formatted string can be used for extra vars as is. + // The value is automatically escaped & wrapped around single quotes from the Library's process. + if (Str::isJsonFormatted($extraVars)) { + $this->addOption('--extra-vars', $extraVars); + + return $this; + } + if (!str_contains($extraVars, '=')) { throw new InvalidArgumentException('The extra vars raw string should be in the "key=value" form.'); } @@ -701,12 +725,12 @@ public function hostKeyChecking(bool $enable = true): AnsiblePlaybookInterface } /** - * Ansible SSH pipelining option - * https://docs.ansible.com/ansible/latest/reference_appendices/config.html#ansible-pipelining - * - * @param bool $enable - * @return AnsiblePlaybookInterface - **/ + * Ansible SSH pipelining option + * https://docs.ansible.com/ansible/latest/reference_appendices/config.html#ansible-pipelining + * + * @param bool $enable + * @return AnsiblePlaybookInterface + **/ public function sshPipelining(bool $enable = false): AnsiblePlaybookInterface { $enable ? @@ -727,4 +751,4 @@ private function checkInventory(): void $this->inventoryFile($inventory); } } -} +} \ No newline at end of file diff --git a/Asm/Ansible/Utils/Str.php b/Asm/Ansible/Utils/Str.php new file mode 100644 index 0000000..d76f8fe --- /dev/null +++ b/Asm/Ansible/Utils/Str.php @@ -0,0 +1,28 @@ +getSamplesPathFor(AnsiblePlaybook::class) . '/playbook1.yml'; $tests = [ + // Test empty strings (with & without spaces). [ 'input' => '', 'expect' => false, ], + [ + 'input' => ' ', + 'expect' => false, + ], + // Test empty array. [ 'input' => [], 'expect' => false, ], + // Test Arrays [ 'input' => ['key' => 'value'], 'expect' => '--extra-vars=key=value', @@ -817,6 +825,12 @@ public function testExtraVars(): void 'input' => ['key1' => 'value1', 'key2' => 'value2'], 'expect' => '--extra-vars=key1=value1 key2=value2', ], + // Test valid JSON. + [ + 'input' => '{ "key1": "value1", "key2": "value2" }', + 'expect' => '--extra-vars={ "key1": "value1", "key2": "value2" }', + ], + // Test key value string. [ 'input' => 'key=value', 'expect' => '--extra-vars=key=value', @@ -854,7 +868,8 @@ public function testExtraVars(): void $tests = [ 'string without equals', - new DateTime() + '{ key1: "value1" }', // Invalid JSON syntax (missing " from key1) which would trigger string without `=`. + new DateTime() // Invalid type ]; foreach ($tests as $input) { @@ -1023,4 +1038,96 @@ public function testSshPipelining(): void $this->assertEquals($expect, $env['ANSIBLE_SSH_PIPELINING']); } } + + public function testReturnsErrorOutputIfProcessWasNotSuccessful(): void + { + $builder = $this->createMock(ProcessBuilderInterface::class); + $builder + ->expects(self::once()) + ->method('setArguments') + ->willReturnSelf(); + $builder + ->expects(self::once()) + ->method('getProcess') + ->willReturn($process = $this->createMock(Process::class)); + $process + ->expects(self::once()) + ->method('run'); + $process + ->expects(self::once()) + ->method('isSuccessful') + ->willReturn(false); + $process + ->expects(self::once()) + ->method('getErrorOutput') + ->willReturn('error output'); + $process + ->expects(self::never()) + ->method('getOutput'); + + $playbook = new AnsiblePlaybook($builder); + + self::assertEquals('error output', $playbook->execute()); + } + + public function testReturnsNormalOutputIfProcessWasSuccessful(): void + { + $builder = $this->createMock(ProcessBuilderInterface::class); + $builder + ->expects(self::once()) + ->method('setArguments') + ->willReturnSelf(); + $builder + ->expects(self::once()) + ->method('getProcess') + ->willReturn($process = $this->createMock(Process::class)); + $process + ->expects(self::once()) + ->method('run'); + $process + ->expects(self::once()) + ->method('isSuccessful') + ->willReturn(true); + $process + ->expects(self::once()) + ->method('getOutput') + ->willReturn('success'); + $process + ->expects(self::never()) + ->method('getErrorOutput'); + + $playbook = new AnsiblePlaybook($builder); + + self::assertEquals('success', $playbook->execute()); + } + + public function testReturnsExitCodeIfCallbackwasPassed(): void + { + $builder = $this->createMock(ProcessBuilderInterface::class); + $builder + ->expects(self::once()) + ->method('setArguments') + ->willReturnSelf(); + $builder + ->expects(self::once()) + ->method('getProcess') + ->willReturn($process = $this->createMock(Process::class)); + $process + ->expects(self::once()) + ->method('run') + ->willReturn(0); + $process + ->expects(self::never()) + ->method('isSuccessful'); + $process + ->expects(self::never()) + ->method('getOutput'); + $process + ->expects(self::never()) + ->method('getErrorOutput'); + + $playbook = new AnsiblePlaybook($builder); + + self::assertEquals(0, $playbook->execute(fn () => null)); + } } diff --git a/Tests/Asm/Ansible/Utils/StrTest.php b/Tests/Asm/Ansible/Utils/StrTest.php new file mode 100644 index 0000000..2f311e0 --- /dev/null +++ b/Tests/Asm/Ansible/Utils/StrTest.php @@ -0,0 +1,37 @@ +assertTrue(Str::isJsonFormatted($value)); + } + + /** + * Assert string is not valid JSON. + * + * @return void + */ + public function testStringIsNotJson(): void + { + $value = 'something'; + + $this->assertFalse(Str::isJsonFormatted($value)); + } +} diff --git a/composer.json b/composer.json index 6546e3f..8d3272d 100644 --- a/composer.json +++ b/composer.json @@ -15,9 +15,9 @@ } ], "require": { - "php": "^8.0.0|^8.1.0|^8.2.0", - "psr/log": "^1.1", - "symfony/process": "^5.3|^6.0" + "php": "^8.0.0|^8.1.0|^8.2.0|^8.3.0", + "psr/log": "^1.1|^2.0|^3.0", + "symfony/process": "^5.3|^6.0|^7.0" }, "require-dev": { "phpunit/phpunit": "^9.5|^10.0 ",