1. ansible
  2. Commit 3a5bd5be1828703bbaedcda80bf745d1c665f259
Browse code

[stable-2.9] Account for empty strings when splitting the host pattern (#62442) (#62679)

Improve tests
- add more unit test cases
- add specific integration test with more cases

Testing shows no major downside to calling .strip() twice in a comprehension vs. using a regular for loop and only calling .strip() once. Going with the comprehension for ease of maintenance and because comprehensions are optimized in CPython.
(cherry picked from commit 987265a6ef)

Co-authored-by: Sam Doran <sdoran@redhat.com>

Sam Doran authored on 2019/11/13 05:01:12
Showing 6 changed files
changelogs/fragments/split-host-pattern-empty-strings.yaml
History View file @ 3a5bd5b
1 1 
new file mode 100644
... ... 
@@ -0,0 +1,2 @@
0 
+bugfixes:
1 
+  - account for empty strings in when splitting the host pattern (https://github.com/ansible/ansible/issues/61964)
lib/ansible/inventory/manager.py
History View file @ 3a5bd5b
... ... 
@@ -130,7 +130,7 @@ def split_host_pattern(pattern):
130 130 
                 '''), pattern, re.X
131 131 
             )
132 132
133 
-    return [p.strip() for p in patterns]
133 
+    return [p.strip() for p in patterns if p.strip()]
134 134
135 135
136 136 
 class InventoryManager(object):
test/integration/targets/inventory_limit/aliases
History View file @ 3a5bd5b
137 137 
new file mode 100644
... ... 
@@ -0,0 +1 @@
0 
+shippable/posix/group4
test/integration/targets/inventory_limit/hosts.yml
History View file @ 3a5bd5b
0 1 
new file mode 100644
... ... 
@@ -0,0 +1,5 @@
0 
+all:
1 
+  hosts:
2 
+    host1:
3 
+    host2:
4 
+    host3:
test/integration/targets/inventory_limit/runme.sh
History View file @ 3a5bd5b
0 5 
new file mode 100755
... ... 
@@ -0,0 +1,31 @@
0 
+#!/usr/bin/env bash
1 
+
2 
+set -eux
3 
+
4 
+trap 'echo "Host pattern limit test failed"' ERR
5 
+
6 
+# https://github.com/ansible/ansible/issues/61964
7 
+
8 
+# These tests should return all hosts
9 
+ansible -i hosts.yml all --limit ,, --list-hosts | tee out ; grep -q 'hosts (3)' out
10 
+ansible -i hosts.yml ,, --list-hosts | tee out ; grep -q 'hosts (3)' out
11 
+ansible -i hosts.yml , --list-hosts | tee out ; grep -q 'hosts (3)' out
12 
+ansible -i hosts.yml all --limit , --list-hosts | tee out ; grep -q 'hosts (3)' out
13 
+ansible -i hosts.yml all --limit '' --list-hosts | tee out ; grep -q 'hosts (3)' out
14 
+
15 
+
16 
+# Only one host
17 
+ansible -i hosts.yml all --limit ,,host1 --list-hosts | tee out ; grep -q 'hosts (1)' out
18 
+ansible -i hosts.yml ,,host1 --list-hosts | tee out ; grep -q 'hosts (1)' out
19 
+
20 
+ansible -i hosts.yml all --limit host1,, --list-hosts | tee out ; grep -q 'hosts (1)' out
21 
+ansible -i hosts.yml host1,, --list-hosts | tee out ; grep -q 'hosts (1)' out
22 
+
23 
+
24 
+# Only two hosts
25 
+ansible -i hosts.yml all --limit host1,,host3 --list-hosts | tee out ; grep -q 'hosts (2)' out
26 
+ansible -i hosts.yml host1,,host3 --list-hosts | tee out ; grep -q 'hosts (2)' out
27 
+
28 
+ansible -i hosts.yml all --limit 'host1, ,    ,host3' --list-hosts | tee out ; grep -q 'hosts (2)' out
29 
+ansible -i hosts.yml 'host1, ,    ,host3' --list-hosts | tee out ; grep -q 'hosts (2)' out
30 
+
test/units/plugins/inventory/test_inventory.py
History View file @ 3a5bd5b
... ... 
@@ -49,6 +49,10 @@ class TestInventory(unittest.TestCase):
49 49 
         'a:b': ['a', 'b'],
50 50 
         ' a : b ': ['a', 'b'],
51 51 
         'foo:bar:baz[1:2]': ['foo', 'bar', 'baz[1:2]'],
52 
+        'a,,b': ['a', 'b'],
53 
+        'a,  ,b,,c, ,': ['a', 'b', 'c'],
54 
+        ',': [],
55 
+        '': [],
52 56 
     }
53 57
54 58 
     pattern_lists = [