Merge pull request #93 from itk-devops/develop

Allow PHP 8.3 and Symfony 7.0

.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"

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();
     }
 
     /**

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);
         }
     }
-}
+}

Asm/Ansible/Utils/Str.php

@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Asm\Ansible\Utils;
+
+use JsonException;
+
+class Str
+{
+    /**
+     * Validate the provided string is JSON formatted.
+     *
+     * Not JSON if result is not an object (stdClass).
+     * Silent return false on exceptions e.g. Invalid/Incorrect encoding, Array depth more than 512 etc.
+     *
+     * @param string $value
+     * @return bool
+     */
+    public static function isJsonFormatted(string $value): bool
+    {
+        try {
+            return is_object(json_decode($value, false, 512, JSON_THROW_ON_ERROR));
+        } catch (JsonException) {
+            return false;
+        }
+    }
+}

Tests/Asm/Ansible/Command/AnsiblePlaybookTest.php

@@ -5,6 +5,7 @@
 namespace Asm\Ansible\Command;
 
 use Asm\Ansible\Process\ProcessBuilder;
+use Asm\Ansible\Process\ProcessBuilderInterface;
 use Asm\Ansible\Testing\AnsibleTestCase;
 use Asm\Ansible\Utils\Env;
 use DateTime;
@@ -801,14 +802,21 @@ public function testExtraVars(): void
         //$playbookFile = $this->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));
+    }
 }

Tests/Asm/Ansible/Utils/StrTest.php

@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Asm\Ansible\Utils;
+
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @group utils
+ */
+class StrTest extends TestCase
+{
+    /**
+     * Assert valid JSON string is correctly checked.
+     *
+     * @return void
+     */
+    public function testJsonWithValidFormat(): void
+    {
+        $value = '{ "key1": "value1" }';
+
+        $this->assertTrue(Str::isJsonFormatted($value));
+    }
+
+    /**
+     * Assert string is not valid JSON.
+     *
+     * @return void
+     */
+    public function testStringIsNotJson(): void
+    {
+        $value = 'something';
+
+        $this->assertFalse(Str::isJsonFormatted($value));
+    }
+}

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 ",