mirror of
https://github.com/kevinveenbirkenbach/computer-playbook.git
synced 2025-12-10 11:26:24 +00:00
Refactor CLI filters: rename --ignore to --exclude and update all related logic and tests
- Updated CLI argument parsing to use --exclude instead of --ignore. - Adjusted help texts, comments, and error messages accordingly. - Updated role filtering logic and references (include → exclude). - Added new unit tests for parse_roles_list(), filter_inventory_by_include(), and filter_inventory_by_ignore(). - Improved wording and consistency in docstrings. This change is part of the refactoring required for the Ansible 2.18 → 2.20 upgrade, ensuring naming clarity and avoiding confusion with Python's 'ignore' semantics. Conversation reference: https://chatgpt.com/share/69307ef2-1fb4-800f-a2ec-d56020019269
This commit is contained in:
@@ -11,8 +11,8 @@ This subcommand:
|
|||||||
host containing all invokable applications.
|
host containing all invokable applications.
|
||||||
2. Optionally filters the resulting groups by:
|
2. Optionally filters the resulting groups by:
|
||||||
- --include: only listed application_ids are kept
|
- --include: only listed application_ids are kept
|
||||||
- --ignore: listed application_ids are removed
|
- --exclude: listed application_ids are removed
|
||||||
- --roles: legacy include filter (used only if --include/--ignore are not set)
|
- --roles: legacy include filter (used only if --include/--exclude are not set)
|
||||||
3. Merges the generated inventory into an existing inventory file, without
|
3. Merges the generated inventory into an existing inventory file, without
|
||||||
deleting or overwriting unrelated entries.
|
deleting or overwriting unrelated entries.
|
||||||
4. Ensures `host_vars/<host>.yml` exists and stores base settings such as:
|
4. Ensures `host_vars/<host>.yml` exists and stores base settings such as:
|
||||||
@@ -192,7 +192,7 @@ def parse_roles_list(raw_roles: Optional[List[str]]) -> Optional[Set[str]]:
|
|||||||
Parse a list of IDs supplied on the CLI. Supports:
|
Parse a list of IDs supplied on the CLI. Supports:
|
||||||
--include web-app-nextcloud web-app-mastodon
|
--include web-app-nextcloud web-app-mastodon
|
||||||
--include web-app-nextcloud,web-app-mastodon
|
--include web-app-nextcloud,web-app-mastodon
|
||||||
Same logic is reused for --ignore and --roles.
|
Same logic is reused for --exclude and --roles.
|
||||||
"""
|
"""
|
||||||
if not raw_roles:
|
if not raw_roles:
|
||||||
return None
|
return None
|
||||||
@@ -768,7 +768,7 @@ def main(argv: Optional[List[str]] = None) -> None:
|
|||||||
nargs="+",
|
nargs="+",
|
||||||
help=(
|
help=(
|
||||||
"Optional legacy list of application_ids to include. "
|
"Optional legacy list of application_ids to include. "
|
||||||
"Used only if neither --include nor --ignore is specified. "
|
"Used only if neither --include nor --exclude is specified. "
|
||||||
"Supports comma-separated values as well."
|
"Supports comma-separated values as well."
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
@@ -777,11 +777,11 @@ def main(argv: Optional[List[str]] = None) -> None:
|
|||||||
nargs="+",
|
nargs="+",
|
||||||
help=(
|
help=(
|
||||||
"Only include the listed application_ids in the inventory. "
|
"Only include the listed application_ids in the inventory. "
|
||||||
"Mutually exclusive with --ignore."
|
"Mutually exclusive with --exclude."
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
parser.add_argument(
|
parser.add_argument(
|
||||||
"--ignore",
|
"--exclude",
|
||||||
nargs="+",
|
nargs="+",
|
||||||
help=(
|
help=(
|
||||||
"Exclude the listed application_ids from the inventory. "
|
"Exclude the listed application_ids from the inventory. "
|
||||||
@@ -814,14 +814,14 @@ def main(argv: Optional[List[str]] = None) -> None:
|
|||||||
|
|
||||||
args = parser.parse_args(argv)
|
args = parser.parse_args(argv)
|
||||||
|
|
||||||
# Parse include/ignore/roles lists
|
# Parse include/exclude/roles lists
|
||||||
include_filter = parse_roles_list(args.include)
|
include_filter = parse_roles_list(args.include)
|
||||||
ignore_filter = parse_roles_list(args.ignore)
|
ignore_filter = parse_roles_list(args.exclude)
|
||||||
roles_filter = parse_roles_list(args.roles)
|
roles_filter = parse_roles_list(args.roles)
|
||||||
|
|
||||||
# Enforce mutual exclusivity: only one of --include / --ignore may be used
|
# Enforce mutual exclusivity: only one of --include / --exclude may be used
|
||||||
if include_filter and ignore_filter:
|
if include_filter and ignore_filter:
|
||||||
fatal("Options --include and --ignore are mutually exclusive. Please use only one of them.")
|
fatal("Options --include and --exclude are mutually exclusive. Please use only one of them.")
|
||||||
|
|
||||||
project_root = detect_project_root()
|
project_root = detect_project_root()
|
||||||
roles_dir = Path(args.roles_dir) if args.roles_dir else (project_root / "roles")
|
roles_dir = Path(args.roles_dir) if args.roles_dir else (project_root / "roles")
|
||||||
@@ -849,7 +849,7 @@ def main(argv: Optional[List[str]] = None) -> None:
|
|||||||
try:
|
try:
|
||||||
vault_password_file.chmod(0o600)
|
vault_password_file.chmod(0o600)
|
||||||
except PermissionError:
|
except PermissionError:
|
||||||
# Best-effort; ignore if chmod is not allowed
|
# Best-effort; exclude if chmod is not allowed
|
||||||
pass
|
pass
|
||||||
else:
|
else:
|
||||||
print(f"[INFO] Using existing vault password file: {vault_password_file}")
|
print(f"[INFO] Using existing vault password file: {vault_password_file}")
|
||||||
@@ -866,7 +866,7 @@ def main(argv: Optional[List[str]] = None) -> None:
|
|||||||
project_root=project_root,
|
project_root=project_root,
|
||||||
)
|
)
|
||||||
|
|
||||||
# 2) Apply filters: include → ignore → legacy roles
|
# 2) Apply filters: include → exclude → legacy roles
|
||||||
if include_filter:
|
if include_filter:
|
||||||
print(f"[INFO] Including only application_ids: {', '.join(sorted(include_filter))}")
|
print(f"[INFO] Including only application_ids: {', '.join(sorted(include_filter))}")
|
||||||
dyn_inv = filter_inventory_by_include(dyn_inv, include_filter)
|
dyn_inv = filter_inventory_by_include(dyn_inv, include_filter)
|
||||||
|
|||||||
@@ -14,6 +14,9 @@ from cli.create.inventory import ( # type: ignore
|
|||||||
merge_inventories,
|
merge_inventories,
|
||||||
ensure_host_vars_file,
|
ensure_host_vars_file,
|
||||||
ensure_become_password,
|
ensure_become_password,
|
||||||
|
parse_roles_list,
|
||||||
|
filter_inventory_by_include,
|
||||||
|
filter_inventory_by_ignore,
|
||||||
)
|
)
|
||||||
|
|
||||||
from ruamel.yaml import YAML
|
from ruamel.yaml import YAML
|
||||||
@@ -21,6 +24,73 @@ from ruamel.yaml.comments import CommentedMap
|
|||||||
|
|
||||||
|
|
||||||
class TestCreateInventory(unittest.TestCase):
|
class TestCreateInventory(unittest.TestCase):
|
||||||
|
|
||||||
|
def test_parse_roles_list_supports_commas_and_spaces(self):
|
||||||
|
"""
|
||||||
|
parse_roles_list() should:
|
||||||
|
- return None for None or empty input,
|
||||||
|
- split comma separated values,
|
||||||
|
- strip whitespace,
|
||||||
|
- deduplicate values.
|
||||||
|
"""
|
||||||
|
self.assertIsNone(parse_roles_list(None))
|
||||||
|
self.assertIsNone(parse_roles_list([]))
|
||||||
|
|
||||||
|
roles = parse_roles_list([
|
||||||
|
"web-app-nextcloud, web-app-matomo",
|
||||||
|
"web-app-phpmyadmin",
|
||||||
|
"web-app-nextcloud", # duplicate
|
||||||
|
])
|
||||||
|
|
||||||
|
self.assertIsInstance(roles, set)
|
||||||
|
self.assertEqual(
|
||||||
|
roles,
|
||||||
|
{"web-app-nextcloud", "web-app-matomo", "web-app-phpmyadmin"},
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_filter_inventory_by_include_keeps_only_selected_groups(self):
|
||||||
|
"""
|
||||||
|
filter_inventory_by_include() must:
|
||||||
|
- keep only groups whose names are in the include set,
|
||||||
|
- preserve the original group data for kept groups,
|
||||||
|
- remove groups not listed in the include set.
|
||||||
|
"""
|
||||||
|
original_inventory = {
|
||||||
|
"all": {
|
||||||
|
"children": {
|
||||||
|
"web-app-nextcloud": {
|
||||||
|
"hosts": {"localhost": {"ansible_host": "127.0.0.1"}},
|
||||||
|
},
|
||||||
|
"web-app-matomo": {
|
||||||
|
"hosts": {"localhost": {"ansible_host": "127.0.0.2"}},
|
||||||
|
},
|
||||||
|
"web-app-phpmyadmin": {
|
||||||
|
"hosts": {"localhost": {"ansible_host": "127.0.0.3"}},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
include_set = {"web-app-nextcloud", "web-app-phpmyadmin"}
|
||||||
|
|
||||||
|
filtered = filter_inventory_by_include(original_inventory, include_set)
|
||||||
|
children = filtered["all"]["children"]
|
||||||
|
|
||||||
|
# Only the included groups must remain
|
||||||
|
self.assertIn("web-app-nextcloud", children)
|
||||||
|
self.assertIn("web-app-phpmyadmin", children)
|
||||||
|
self.assertNotIn("web-app-matomo", children)
|
||||||
|
|
||||||
|
# The content of the kept groups must be identical to the original
|
||||||
|
self.assertEqual(
|
||||||
|
children["web-app-nextcloud"],
|
||||||
|
original_inventory["all"]["children"]["web-app-nextcloud"],
|
||||||
|
)
|
||||||
|
self.assertEqual(
|
||||||
|
children["web-app-phpmyadmin"],
|
||||||
|
original_inventory["all"]["children"]["web-app-phpmyadmin"],
|
||||||
|
)
|
||||||
|
|
||||||
def test_merge_inventories_adds_host_and_preserves_existing(self):
|
def test_merge_inventories_adds_host_and_preserves_existing(self):
|
||||||
"""
|
"""
|
||||||
merge_inventories() must:
|
merge_inventories() must:
|
||||||
|
|||||||
Reference in New Issue
Block a user