From 56b86bb79090319c84b60985c96050b75937fa60 Mon Sep 17 00:00:00 2001 From: bambamboole Date: Sun, 30 Apr 2023 14:44:42 +0200 Subject: [PATCH 1/4] remove quotes around hosts --- Asm/Ansible/Command/AnsiblePlaybook.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Asm/Ansible/Command/AnsiblePlaybook.php b/Asm/Ansible/Command/AnsiblePlaybook.php index 98dcfb4..5e229b9 100644 --- a/Asm/Ansible/Command/AnsiblePlaybook.php +++ b/Asm/Ansible/Command/AnsiblePlaybook.php @@ -276,7 +276,7 @@ public function inventory(array $hosts = []): AnsiblePlaybookInterface $hostList .= ','; } - $this->addOption('--inventory', sprintf('"%s"', $hostList)); + $this->addOption('--inventory', $hostList); $this->hasInventory = true; return $this; From 1d713ae24991bb3955f097af2aa9460ecd0874dc Mon Sep 17 00:00:00 2001 From: bambamboole Date: Sun, 30 Apr 2023 18:13:28 +0200 Subject: [PATCH 2/4] Wrapping each host into quotes instead of having doubles quotes around all hosts --- Asm/Ansible/Command/AnsiblePlaybook.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Asm/Ansible/Command/AnsiblePlaybook.php b/Asm/Ansible/Command/AnsiblePlaybook.php index 5e229b9..c1a8543 100644 --- a/Asm/Ansible/Command/AnsiblePlaybook.php +++ b/Asm/Ansible/Command/AnsiblePlaybook.php @@ -265,6 +265,9 @@ public function inventory(array $hosts = []): AnsiblePlaybookInterface return $this; } + // Wrapping each host in double quotes to avoid issues with spaces in host names. + $hosts = array_map(fn ($host) => sprintf('"%s"', $host), $hosts); + // In order to let ansible-playbook understand that the given option is a list of hosts, the list must end by // comma "," if it contains just an entry. For example, supposing just a single host, "localhosts": // From 757b22619e42c3d2507e4a947148907e4a507b9d Mon Sep 17 00:00:00 2001 From: bambamboole Date: Sun, 30 Apr 2023 19:55:42 +0200 Subject: [PATCH 3/4] Add test for quotes --- Tests/Asm/Ansible/Command/AnsiblePlaybookTest.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Tests/Asm/Ansible/Command/AnsiblePlaybookTest.php b/Tests/Asm/Ansible/Command/AnsiblePlaybookTest.php index b1ce0f2..08042e5 100644 --- a/Tests/Asm/Ansible/Command/AnsiblePlaybookTest.php +++ b/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; @@ -1023,4 +1024,12 @@ public function testSshPipelining(): void $this->assertEquals($expect, $env['ANSIBLE_SSH_PIPELINING']); } } + + public function testItWrapsTheHostsInQuotes(): void + { + $ansible = new AnsiblePlaybook($this->createMock(ProcessBuilderInterface::class)); + $ansible->inventory(['host1', 'host 2']); + + self::assertContains('--inventory="host1", "host 2"', $ansible->getCommandlineArguments()); + } } From 4d64a703322cbebe9bb121cd6a7df8694908de76 Mon Sep 17 00:00:00 2001 From: bambamboole Date: Sun, 30 Apr 2023 22:20:41 +0200 Subject: [PATCH 4/4] Adapt tests and remove space after comma --- Asm/Ansible/Command/AnsiblePlaybook.php | 2 +- Tests/Asm/Ansible/Command/AnsiblePlaybookTest.php | 12 ++---------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/Asm/Ansible/Command/AnsiblePlaybook.php b/Asm/Ansible/Command/AnsiblePlaybook.php index 9a618bb..fb16809 100644 --- a/Asm/Ansible/Command/AnsiblePlaybook.php +++ b/Asm/Ansible/Command/AnsiblePlaybook.php @@ -289,7 +289,7 @@ public function inventory(array $hosts = []): AnsiblePlaybookInterface // // Wrong: --inventory="locahost" // Correct: --inventory="locahost," - $hostList = implode(', ', $hosts); + $hostList = implode(',', $hosts); if (count($hosts) === 1) { $hostList .= ','; diff --git a/Tests/Asm/Ansible/Command/AnsiblePlaybookTest.php b/Tests/Asm/Ansible/Command/AnsiblePlaybookTest.php index 4872be7..32f43ab 100644 --- a/Tests/Asm/Ansible/Command/AnsiblePlaybookTest.php +++ b/Tests/Asm/Ansible/Command/AnsiblePlaybookTest.php @@ -901,11 +901,11 @@ public function testInventory(): void ], [ 'input' => ['localhost'], - 'expect' => '--inventory="localhost,"', + 'expect' => '--inventory="localhost",', ], [ 'input' => ['localhost', 'host1'], - 'expect' => '--inventory="localhost, host1"', + 'expect' => '--inventory="localhost","host1"', ], ]; @@ -1117,12 +1117,4 @@ public function testReturnsExitCodeIfCallbackwasPassed(): void self::assertEquals(0, $playbook->execute(fn () => null)); } - - public function testItWrapsTheHostsInQuotes(): void - { - $ansible = new AnsiblePlaybook($this->createMock(ProcessBuilderInterface::class)); - $ansible->inventory(['host1', 'host 2']); - - self::assertContains('--inventory="host1", "host 2"', $ansible->getCommandlineArguments()); - } }