mirror of
https://github.com/kevinveenbirkenbach/computer-playbook.git
synced 2026-11-09 12:17:13 +00:00
refactor(roles): let a role declare its entity name and drop the get_ prefix
The entity name keys a role's instance directory, its compose project and its container name, and was derived only by stripping the longest matching category prefix. Two roles under different categories whose names share a suffix therefore collapse onto one entity and overwrite each other's rendered stack, which no provides: can repair. A literal entity_name in vars/main.yml now wins over the derivation. web-app-seaweedfs-console takes its shorter name back and declares the entity seaweedfs-console, which frees the seaweedfs key in its own services map for the engine consumer and removes the provides:, covers: and alias entries that stood in for it. A new lint pairs the declaration with the primary key it has to match, because a drift between them would drop the role from the provider set in silence. Compared across all 288 roles, exactly one entity changes, and it restores what main already keeps on disk, so no instance directory moves. The function and the filter lose their get_ prefix. Two lints read that prefix as the discriminator between the variable and the filter that fills it; both discriminate on the pipe now. Ansible's templar resolves a filter and a same-named variable independently, verified against ansible-core 2.20.2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
8cf3ad12db
commit
a05071fe78
424 files changed
+3175
-657
No files matched your search
@@ -7,7 +7,7 @@ class TestGetDockerPaths(unittest.TestCase):
|
||||
import plugins.filter.get.docker_paths as m
|
||||
|
||||
with patch(
|
||||
"utils.docker.paths_utils.get_entity_name", lambda app_id: "myentity"
|
||||
"utils.docker.paths_utils.entity_name", lambda app_id: "myentity"
|
||||
):
|
||||
out = m.get_docker_paths("web-app-anything", "/opt/compose/")
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@ from unittest.mock import patch
|
||||
|
||||
from utils.cache.yaml import dump_yaml
|
||||
from utils.roles.categories import categories_file
|
||||
from utils.roles.mapping import ROLE_FILE_VARS_MAIN
|
||||
|
||||
|
||||
class TestGetEntityNameFilter(unittest.TestCase):
|
||||
@@ -38,41 +39,64 @@ class TestGetEntityNameFilter(unittest.TestCase):
|
||||
root.start()
|
||||
self.addCleanup(root.stop)
|
||||
|
||||
entity_root = patch(
|
||||
"utils.roles.entity.name.PROJECT_ROOT", Path(self.temp_dir)
|
||||
)
|
||||
entity_root.start()
|
||||
self.addCleanup(entity_root.stop)
|
||||
|
||||
plugin_path = str(Path.cwd() / "plugins" / "filter")
|
||||
if plugin_path not in sys.path and Path(plugin_path).is_dir():
|
||||
sys.path.insert(0, plugin_path)
|
||||
from plugins.filter.get.entity_name import get_entity_name
|
||||
from plugins.filter.get.entity_name import entity_name
|
||||
|
||||
self.get_entity_name = get_entity_name
|
||||
self.entity_name = entity_name
|
||||
|
||||
def tearDown(self):
|
||||
shutil.rmtree(self.temp_dir)
|
||||
|
||||
def test_entity_name_web_app(self):
|
||||
self.assertEqual(self.get_entity_name("web-app-snipe-it"), "snipe-it")
|
||||
self.assertEqual(self.get_entity_name("web-app-nextcloud"), "nextcloud")
|
||||
self.assertEqual(self.get_entity_name("web-svc-file"), "file")
|
||||
self.assertEqual(self.entity_name("web-app-snipe-it"), "snipe-it")
|
||||
self.assertEqual(self.entity_name("web-app-nextcloud"), "nextcloud")
|
||||
self.assertEqual(self.entity_name("web-svc-file"), "file")
|
||||
|
||||
def test_entity_name_sys_bkp(self):
|
||||
self.assertEqual(
|
||||
self.get_entity_name("sys-ctl-bkp-directory-validator"),
|
||||
self.entity_name("sys-ctl-bkp-directory-validator"),
|
||||
"directory-validator",
|
||||
)
|
||||
|
||||
def test_entity_name_sys_hlth(self):
|
||||
self.assertEqual(self.get_entity_name("sys-ctl-hlth-btrfs"), "btrfs")
|
||||
self.assertEqual(self.entity_name("sys-ctl-hlth-btrfs"), "btrfs")
|
||||
|
||||
def test_no_category_match(self):
|
||||
self.assertEqual(self.get_entity_name("foobar-role"), "foobar-role")
|
||||
self.assertEqual(self.entity_name("foobar-role"), "foobar-role")
|
||||
|
||||
def test_exact_category_match_with_parent_prefix_strips(self):
|
||||
self.assertEqual(self.get_entity_name("web-app"), "app")
|
||||
self.assertEqual(self.entity_name("web-app"), "app")
|
||||
|
||||
def test_exact_category_match_without_parent_prefix(self):
|
||||
self.assertEqual(self.get_entity_name("web"), "")
|
||||
self.assertEqual(self.entity_name("web"), "")
|
||||
|
||||
def test_role_equal_to_category_path_strips_parent(self):
|
||||
self.assertEqual(self.get_entity_name("svc-db"), "db")
|
||||
self.assertEqual(self.entity_name("svc-db"), "db")
|
||||
|
||||
def _declare(self, role_name, entity_name):
|
||||
dump_yaml(
|
||||
str(Path(self.roles_dir) / role_name / ROLE_FILE_VARS_MAIN),
|
||||
{"application_id": role_name, "entity_name": entity_name},
|
||||
)
|
||||
|
||||
def test_declared_entity_name_overrides_the_derivation(self):
|
||||
self._declare("web-app-seaweedfs", "seaweedfs-console")
|
||||
self.assertEqual(self.entity_name("web-app-seaweedfs"), "seaweedfs-console")
|
||||
|
||||
def test_declared_jinja_expression_falls_back_to_the_derivation(self):
|
||||
self._declare("web-app-nextcloud", "{{ application_id | entity_name }}")
|
||||
self.assertEqual(self.entity_name("web-app-nextcloud"), "nextcloud")
|
||||
|
||||
def test_role_without_vars_file_falls_back_to_the_derivation(self):
|
||||
self.assertEqual(self.entity_name("web-app-mastodon"), "mastodon")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
@@ -15,12 +15,12 @@ class TestMemoryFilters(unittest.TestCase):
|
||||
# -----------------------------
|
||||
def _with_conf(self, mem_limit: str, mem_res: str):
|
||||
"""
|
||||
Patch get/get_entity_name so that mem_limit and mem_reservation
|
||||
Patch get/entity_name so that mem_limit and mem_reservation
|
||||
can be controlled in tests.
|
||||
"""
|
||||
patches = [
|
||||
patch(
|
||||
"plugins.filter.memory_filters.get_entity_name",
|
||||
"plugins.filter.memory_filters.entity_name",
|
||||
return_value="confluence",
|
||||
),
|
||||
patch(
|
||||
@@ -78,7 +78,7 @@ class TestMemoryFilters(unittest.TestCase):
|
||||
def test_invalid_unit_raises(self):
|
||||
with (
|
||||
patch(
|
||||
"plugins.filter.memory_filters.get_entity_name",
|
||||
"plugins.filter.memory_filters.entity_name",
|
||||
return_value="confluence",
|
||||
),
|
||||
patch(
|
||||
@@ -94,7 +94,7 @@ class TestMemoryFilters(unittest.TestCase):
|
||||
def test_zero_limit_raises(self):
|
||||
with (
|
||||
patch(
|
||||
"plugins.filter.memory_filters.get_entity_name",
|
||||
"plugins.filter.memory_filters.entity_name",
|
||||
return_value="confluence",
|
||||
),
|
||||
patch(
|
||||
@@ -110,7 +110,7 @@ class TestMemoryFilters(unittest.TestCase):
|
||||
def test_zero_reservation_raises(self):
|
||||
with (
|
||||
patch(
|
||||
"plugins.filter.memory_filters.get_entity_name",
|
||||
"plugins.filter.memory_filters.entity_name",
|
||||
return_value="confluence",
|
||||
),
|
||||
patch(
|
||||
@@ -125,12 +125,12 @@ class TestMemoryFilters(unittest.TestCase):
|
||||
|
||||
def test_entity_name_is_derived_not_passed(self):
|
||||
"""
|
||||
Ensure get_entity_name() is called internally and the app_id is not
|
||||
Ensure entity_name() is called internally and the app_id is not
|
||||
passed around manually from the template.
|
||||
"""
|
||||
with (
|
||||
patch(
|
||||
"plugins.filter.memory_filters.get_entity_name",
|
||||
"plugins.filter.memory_filters.entity_name",
|
||||
return_value="confluence",
|
||||
) as mock_entity,
|
||||
patch(
|
||||
@@ -181,13 +181,13 @@ class TestMemoryFilters(unittest.TestCase):
|
||||
with self.assertRaises(memory_filters.AnsibleFilterError):
|
||||
memory_filters.redis_maxmemory_mb(self.apps, self.app_id)
|
||||
|
||||
def test_redis_maxmemory_does_not_call_get_entity_name(self):
|
||||
def test_redis_maxmemory_does_not_call_entity_name(self):
|
||||
"""
|
||||
Ensure redis_maxmemory_mb does NOT rely on entity name resolution
|
||||
(it should always use the hard-coded 'redis' service name).
|
||||
"""
|
||||
patches = [
|
||||
patch("plugins.filter.memory_filters.get_entity_name"),
|
||||
patch("plugins.filter.memory_filters.entity_name"),
|
||||
patch(
|
||||
"plugins.filter.memory_filters.get",
|
||||
side_effect=lambda apps, app_id, key, required=True, **kwargs: (
|
||||
@@ -221,7 +221,7 @@ class TestMemoryFilters(unittest.TestCase):
|
||||
side_effect=fake_get,
|
||||
),
|
||||
patch(
|
||||
"plugins.filter.memory_filters.get_entity_name",
|
||||
"plugins.filter.memory_filters.entity_name",
|
||||
return_value="confluence",
|
||||
),
|
||||
):
|
||||
|
||||
@@ -14,10 +14,10 @@ class TestResourceFilter(unittest.TestCase):
|
||||
self.key = "cpus"
|
||||
|
||||
self.patcher_conf = patch.object(plugin_module, "get")
|
||||
self.patcher_entity = patch.object(plugin_module, "get_entity_name")
|
||||
self.patcher_entity = patch.object(plugin_module, "entity_name")
|
||||
self.mock_get = self.patcher_conf.start()
|
||||
self.mock_get_entity_name = self.patcher_entity.start()
|
||||
self.mock_get_entity_name.return_value = "foo"
|
||||
self.mock_entity_name = self.patcher_entity.start()
|
||||
self.mock_entity_name.return_value = "foo"
|
||||
|
||||
def tearDown(self):
|
||||
self.patcher_conf.stop()
|
||||
@@ -44,8 +44,8 @@ class TestResourceFilter(unittest.TestCase):
|
||||
plugin_module._UNSET,
|
||||
)
|
||||
|
||||
def test_service_name_empty_uses_get_entity_name(self):
|
||||
"""When service_name is empty, it resolves via get_entity_name(application_id)."""
|
||||
def test_service_name_empty_uses_entity_name(self):
|
||||
"""When service_name is empty, it resolves via entity_name(application_id)."""
|
||||
self.mock_get.return_value = "1.0"
|
||||
|
||||
result = plugin_module.resource_filter(
|
||||
@@ -57,7 +57,7 @@ class TestResourceFilter(unittest.TestCase):
|
||||
)
|
||||
|
||||
self.assertEqual(result, "1.0")
|
||||
self.mock_get_entity_name.assert_called_once_with(self.application_id)
|
||||
self.mock_entity_name.assert_called_once_with(self.application_id)
|
||||
self.mock_get.assert_called_once_with(
|
||||
self.applications,
|
||||
self.application_id,
|
||||
|
||||
@@ -28,7 +28,7 @@ class TestBoundedNameLookup(unittest.TestCase):
|
||||
side_effect=lambda domains, app: domains[app],
|
||||
),
|
||||
patch(
|
||||
"plugins.lookup.bounded_name.get_entity_name",
|
||||
"plugins.lookup.bounded_name.entity_name",
|
||||
side_effect=lambda app: app.rsplit("-", 1)[-1],
|
||||
),
|
||||
]
|
||||
|
||||
@@ -32,7 +32,7 @@ class TestCpusLookup(unittest.TestCase):
|
||||
_, call = _run(["web-app-hermes", "hermes"], "50%")
|
||||
self.assertEqual(call.args[0], ["web-app-hermes", "services.hermes.cpus"])
|
||||
|
||||
@patch("plugins.lookup.cpus.get_entity_name", return_value="docs")
|
||||
@patch("plugins.lookup.cpus.entity_name", return_value="docs")
|
||||
def test_the_entity_name_is_the_default_service(self, _entity):
|
||||
_, call = _run(["web-app-docs"], "50%")
|
||||
self.assertEqual(call.args[0], ["web-app-docs", "services.docs.cpus"])
|
||||
|
||||
@@ -44,7 +44,7 @@ class DatabaseLookupTests(unittest.TestCase):
|
||||
return lm
|
||||
|
||||
@staticmethod
|
||||
def _fake_get_entity_name(role_name: str) -> str:
|
||||
def _fake_entity_name(role_name: str) -> str:
|
||||
"""
|
||||
Make entity resolution deterministic for unit tests (no filesystem access).
|
||||
Mirrors the typical behavior for your role naming.
|
||||
@@ -90,8 +90,8 @@ class DatabaseLookupTests(unittest.TestCase):
|
||||
|
||||
with patch.object(
|
||||
self.db_lookup_mod,
|
||||
"get_entity_name",
|
||||
side_effect=self._fake_get_entity_name,
|
||||
"entity_name",
|
||||
side_effect=self._fake_entity_name,
|
||||
):
|
||||
out = lookup.run(["web-app-foo"], variables=vars_)[0]
|
||||
|
||||
@@ -117,8 +117,8 @@ class DatabaseLookupTests(unittest.TestCase):
|
||||
|
||||
with patch.object(
|
||||
self.db_lookup_mod,
|
||||
"get_entity_name",
|
||||
side_effect=self._fake_get_entity_name,
|
||||
"entity_name",
|
||||
side_effect=self._fake_entity_name,
|
||||
):
|
||||
self.assertEqual(
|
||||
lookup.run(["web-app-foo", "url_full"], variables=vars_)[0], ""
|
||||
@@ -152,8 +152,8 @@ class DatabaseLookupTests(unittest.TestCase):
|
||||
|
||||
with patch.object(
|
||||
self.db_lookup_mod,
|
||||
"get_entity_name",
|
||||
side_effect=self._fake_get_entity_name,
|
||||
"entity_name",
|
||||
side_effect=self._fake_entity_name,
|
||||
):
|
||||
out = lookup.run(["web-app-foo"], variables=vars_)[0]
|
||||
|
||||
@@ -184,8 +184,8 @@ class DatabaseLookupTests(unittest.TestCase):
|
||||
|
||||
with patch.object(
|
||||
self.db_lookup_mod,
|
||||
"get_entity_name",
|
||||
side_effect=self._fake_get_entity_name,
|
||||
"entity_name",
|
||||
side_effect=self._fake_entity_name,
|
||||
):
|
||||
self.assertEqual(
|
||||
lookup.run(["web-app-foo", "url_full"], variables=vars_)[0],
|
||||
@@ -223,8 +223,8 @@ class DatabaseLookupTests(unittest.TestCase):
|
||||
|
||||
with patch.object(
|
||||
self.db_lookup_mod,
|
||||
"get_entity_name",
|
||||
side_effect=self._fake_get_entity_name,
|
||||
"entity_name",
|
||||
side_effect=self._fake_entity_name,
|
||||
):
|
||||
out = lookup.run(["web-app-foo"], variables=vars_)[0]
|
||||
|
||||
@@ -272,8 +272,8 @@ class DatabaseLookupTests(unittest.TestCase):
|
||||
|
||||
with patch.object(
|
||||
self.db_lookup_mod,
|
||||
"get_entity_name",
|
||||
side_effect=self._fake_get_entity_name,
|
||||
"entity_name",
|
||||
side_effect=self._fake_entity_name,
|
||||
):
|
||||
out = lookup.run(["web-app-foo"], variables=vars_)[0]
|
||||
|
||||
@@ -317,8 +317,8 @@ class DatabaseLookupTests(unittest.TestCase):
|
||||
|
||||
with patch.object(
|
||||
self.db_lookup_mod,
|
||||
"get_entity_name",
|
||||
side_effect=self._fake_get_entity_name,
|
||||
"entity_name",
|
||||
side_effect=self._fake_entity_name,
|
||||
):
|
||||
out = lookup.run(["web-app-foo"], variables=vars_)[0]
|
||||
|
||||
@@ -347,8 +347,8 @@ class DatabaseLookupTests(unittest.TestCase):
|
||||
|
||||
with patch.object(
|
||||
self.db_lookup_mod,
|
||||
"get_entity_name",
|
||||
side_effect=self._fake_get_entity_name,
|
||||
"entity_name",
|
||||
side_effect=self._fake_entity_name,
|
||||
):
|
||||
out = lookup.run(["web-app-foo"], variables=vars_)[0]
|
||||
|
||||
@@ -359,8 +359,8 @@ class DatabaseLookupTests(unittest.TestCase):
|
||||
|
||||
with patch.object(
|
||||
self.db_lookup_mod,
|
||||
"get_entity_name",
|
||||
side_effect=self._fake_get_entity_name,
|
||||
"entity_name",
|
||||
side_effect=self._fake_entity_name,
|
||||
):
|
||||
self.assertFalse(lookup.run(["web-app-foo", "local"], variables=vars_)[0])
|
||||
|
||||
@@ -439,8 +439,8 @@ class DatabaseLookupTests(unittest.TestCase):
|
||||
lookup = self._make_lookup(vars_)
|
||||
with patch.object(
|
||||
self.db_lookup_mod,
|
||||
"get_entity_name",
|
||||
side_effect=self._fake_get_entity_name,
|
||||
"entity_name",
|
||||
side_effect=self._fake_entity_name,
|
||||
):
|
||||
return lookup.run(["web-app-foo"], variables=vars_)[0]
|
||||
|
||||
|
||||
@@ -246,7 +246,7 @@ class TestImageLookup(unittest.TestCase):
|
||||
|
||||
def test_custom_true_uses_entity_name_custom(self):
|
||||
with patch(
|
||||
"plugins.lookup.image.get_entity_name",
|
||||
"plugins.lookup.image.entity_name",
|
||||
return_value="mattermost",
|
||||
):
|
||||
out = _run(
|
||||
@@ -259,7 +259,7 @@ class TestImageLookup(unittest.TestCase):
|
||||
|
||||
def test_custom_true_with_version_override(self):
|
||||
with patch(
|
||||
"plugins.lookup.image.get_entity_name",
|
||||
"plugins.lookup.image.entity_name",
|
||||
return_value="mattermost",
|
||||
):
|
||||
out = _run(
|
||||
@@ -273,7 +273,7 @@ class TestImageLookup(unittest.TestCase):
|
||||
|
||||
def test_custom_true_with_image_override_image_wins(self):
|
||||
with patch(
|
||||
"plugins.lookup.image.get_entity_name",
|
||||
"plugins.lookup.image.entity_name",
|
||||
return_value="mattermost",
|
||||
) as get_name:
|
||||
out = _run(
|
||||
@@ -288,7 +288,7 @@ class TestImageLookup(unittest.TestCase):
|
||||
|
||||
def test_custom_true_swarm_prefix_applied(self):
|
||||
with patch(
|
||||
"plugins.lookup.image.get_entity_name",
|
||||
"plugins.lookup.image.entity_name",
|
||||
return_value="mattermost",
|
||||
):
|
||||
out = _run(
|
||||
@@ -315,7 +315,7 @@ class TestImageLookup(unittest.TestCase):
|
||||
def test_custom_true_ignores_services_image(self):
|
||||
apps = _apps(image="mattermost/mattermost-team-edition", version="11.8.0")
|
||||
with patch(
|
||||
"plugins.lookup.image.get_entity_name",
|
||||
"plugins.lookup.image.entity_name",
|
||||
return_value="mattermost",
|
||||
):
|
||||
out = _run(
|
||||
@@ -329,7 +329,7 @@ class TestImageLookup(unittest.TestCase):
|
||||
|
||||
def test_custom_true_tag_only_returns_bare(self):
|
||||
with patch(
|
||||
"plugins.lookup.image.get_entity_name",
|
||||
"plugins.lookup.image.entity_name",
|
||||
return_value="mattermost",
|
||||
):
|
||||
out = _run(
|
||||
@@ -355,7 +355,7 @@ class TestImageLookup(unittest.TestCase):
|
||||
apps = _apps()
|
||||
apps["web-app-mattermost"]["services"]["mattermost"]["custom"] = True
|
||||
with patch(
|
||||
"plugins.lookup.image.get_entity_name",
|
||||
"plugins.lookup.image.entity_name",
|
||||
return_value="mattermost",
|
||||
):
|
||||
out = _run(
|
||||
@@ -395,7 +395,7 @@ class TestImageLookup(unittest.TestCase):
|
||||
"services"
|
||||
].pop("mattermost")
|
||||
with patch(
|
||||
"plugins.lookup.image.get_entity_name",
|
||||
"plugins.lookup.image.entity_name",
|
||||
return_value="mattermost",
|
||||
) as get_name:
|
||||
out = _run(
|
||||
@@ -430,7 +430,7 @@ class TestImageLookup(unittest.TestCase):
|
||||
def test_custom_true_empty_entity_name_raises(self):
|
||||
with (
|
||||
patch(
|
||||
"plugins.lookup.image.get_entity_name",
|
||||
"plugins.lookup.image.entity_name",
|
||||
return_value="",
|
||||
),
|
||||
self.assertRaises(AnsibleError),
|
||||
|
||||
@@ -61,11 +61,11 @@ def _run(terms, *, mode, apps):
|
||||
) as native_loader_mock,
|
||||
patch("plugins.lookup.scrape_target.lookup_loader") as scrape_loader_mock,
|
||||
patch(
|
||||
"plugins.lookup.native_metrics_target.get_entity_name",
|
||||
"plugins.lookup.native_metrics_target.entity_name",
|
||||
side_effect=lambda app_id: ENTITIES.get(app_id, ""),
|
||||
),
|
||||
patch(
|
||||
"plugins.lookup.scrape_target.get_entity_name",
|
||||
"plugins.lookup.scrape_target.entity_name",
|
||||
side_effect=lambda app_id: ENTITIES.get(app_id, ""),
|
||||
),
|
||||
):
|
||||
|
||||
@@ -41,7 +41,7 @@ class ObjstoreLookupTests(unittest.TestCase):
|
||||
return lm
|
||||
|
||||
@staticmethod
|
||||
def _fake_get_entity_name(role_name: str) -> str:
|
||||
def _fake_entity_name(role_name: str) -> str:
|
||||
"""
|
||||
Make entity resolution deterministic for unit tests (no filesystem access).
|
||||
Mirrors the typical behavior for your role naming.
|
||||
@@ -59,7 +59,7 @@ class ObjstoreLookupTests(unittest.TestCase):
|
||||
with (
|
||||
mock.patch.object(self.mod, "lookup_loader") as loader_mock,
|
||||
patch.object(
|
||||
self.mod, "get_entity_name", side_effect=self._fake_get_entity_name
|
||||
self.mod, "entity_name", side_effect=self._fake_entity_name
|
||||
),
|
||||
):
|
||||
loader_mock.get.return_value = mock.MagicMock(
|
||||
|
||||
Reference in new issue
Block a user