mirror of
https://github.com/kevinveenbirkenbach/computer-playbook.git
synced 2026-11-10 12:47:04 +00:00
perf(spot): re-render only the users SPOT after a token store write
A token store write re-rendered the whole applications SPOT: merged_applications_cache_key carried the token store digest, so the applications_carrier park in sys-token-store/tasks/write_many.yml missed its key and rendered all 269 applications again (stalwart compose roundtrip 18.3 s and 10.2 s per pass, swarm pass 1 12.2 s and 12.6 s).
Rendered from the real roles tree under an empty, a realistic and an exhaustive token store (298 users x 269 applications), the applications tree differs only under <app>.users.<key>.tokens (28783 leaves in 46 applications, 0 elsewhere); the 69 password leaves that also differ change between two renders of the same store. No code reads a token through the applications tree: every reader goes through lookup('users'), lookup('config', <app>, 'users.<key>...') (served from get_merged_users), lookup('email') or the sys_token_store_users fact, and the users carrier keys on the store.
The token digest therefore moves from merged_applications_cache_key into merged_users_cache_key, whose value stays the same. A store write keeps the applications carrier and the write_many park re-renders the users alone; the task is renamed to match. applications.md now says that user records inside the payload can predate every store write and that application config must not derive a value from a token.
Real ansible-playbook harness on 2.20.6 with the real roles, group_vars and write_many.yml: the re-park after a first mint and after a rotation drops from 4.0 s to 0.55 s on one host (applications renders 1 -> 0), and every token probe (users, config users path, email password, stalwart_users) equals the old code in compose and swarm mode. Dropping the digest from the users key as well leaves those probes stale, which the harness and two unit tests catch.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
91f29d332e
commit
96ebefb9db
10 files changed
+88
-39
No files matched your search
+13
-9
@@ -390,7 +390,7 @@ class TestGetMergedApplicationsCarrierFact(unittest.TestCase):
|
||||
merged["web-app-foo"]["services"]["foo"]["image"], "override"
|
||||
)
|
||||
|
||||
def test_a_changed_tokens_file_rejects_the_carrier(self):
|
||||
def test_a_changed_tokens_file_keeps_the_carrier(self):
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
roles = _seed_minimal_roles(Path(tmp))
|
||||
tokens = Path(tmp) / "tokens.yml"
|
||||
@@ -398,9 +398,10 @@ class TestGetMergedApplicationsCarrierFact(unittest.TestCase):
|
||||
variables = self._variables()
|
||||
variables["FILE_TOKENS"] = str(tokens)
|
||||
key = merged_applications_cache_key(variables, roles_dir=roles)
|
||||
carried = {"web-app-foo": {"carried": True}}
|
||||
variables[APPLICATIONS_RENDERED_FACT] = {
|
||||
"key": [key[0], list(key[1])],
|
||||
"applications": {"web-app-foo": {"carried": True}},
|
||||
"applications": carried,
|
||||
}
|
||||
tokens.write_text(
|
||||
"users: {no-reply: {tokens: {web-app-stalwart: minted}}}\n",
|
||||
@@ -409,7 +410,7 @@ class TestGetMergedApplicationsCarrierFact(unittest.TestCase):
|
||||
merged = cache_apps.get_merged_applications(
|
||||
variables=variables, roles_dir=roles, templar=None
|
||||
)
|
||||
self.assertNotIn("carried", merged["web-app-foo"])
|
||||
self.assertIs(merged, carried)
|
||||
|
||||
def _park_from_the_tokens_file(self, tmp: Path, token: str) -> tuple:
|
||||
roles = _seed_minimal_roles(tmp)
|
||||
@@ -441,28 +442,31 @@ class TestGetMergedApplicationsCarrierFact(unittest.TestCase):
|
||||
)
|
||||
self.assertIs(merged, carried)
|
||||
|
||||
def test_the_store_fact_with_other_tokens_than_the_file_rejects_the_carrier(
|
||||
def test_the_store_fact_with_other_tokens_than_the_file_keeps_the_carrier(
|
||||
self,
|
||||
):
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
roles, variables, _ = self._park_from_the_tokens_file(Path(tmp), "minted")
|
||||
roles, variables, carried = self._park_from_the_tokens_file(
|
||||
Path(tmp), "minted"
|
||||
)
|
||||
variables["sys_token_store_users"] = {
|
||||
"no-reply": {"tokens": {"web-app-stalwart": "rotated"}}
|
||||
}
|
||||
merged = cache_apps.get_merged_applications(
|
||||
variables=variables, roles_dir=roles, templar=None
|
||||
)
|
||||
self.assertNotIn("carried", merged["web-app-foo"])
|
||||
self.assertIs(merged, carried)
|
||||
|
||||
def test_a_changed_store_fact_rejects_the_carrier(self):
|
||||
def test_a_changed_store_fact_keeps_the_carrier(self):
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
roles = _seed_minimal_roles(Path(tmp))
|
||||
variables = self._variables()
|
||||
variables["sys_token_store_users"] = {}
|
||||
key = merged_applications_cache_key(variables, roles_dir=roles)
|
||||
carried = {"web-app-foo": {"carried": True}}
|
||||
variables[APPLICATIONS_RENDERED_FACT] = {
|
||||
"key": [key[0], list(key[1])],
|
||||
"applications": {"web-app-foo": {"carried": True}},
|
||||
"applications": carried,
|
||||
}
|
||||
variables["sys_token_store_users"] = {
|
||||
"no-reply": {"tokens": {"web-app-stalwart": "minted"}}
|
||||
@@ -470,7 +474,7 @@ class TestGetMergedApplicationsCarrierFact(unittest.TestCase):
|
||||
merged = cache_apps.get_merged_applications(
|
||||
variables=variables, roles_dir=roles, templar=None
|
||||
)
|
||||
self.assertNotIn("carried", merged["web-app-foo"])
|
||||
self.assertIs(merged, carried)
|
||||
|
||||
def test_malformed_carrier_is_ignored(self):
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
|
||||
@@ -519,6 +519,46 @@ class TestGetMergedUsersCarrierFact(unittest.TestCase):
|
||||
merged["rendered"]["foo"]["tokens"]["web-app-foo"], "rotated"
|
||||
)
|
||||
|
||||
def _park_from_the_tokens_file(self, tmp: Path, token: str) -> tuple:
|
||||
roles = _seed_minimal_user_role(tmp)
|
||||
tokens = tmp / "tokens.yml"
|
||||
tokens.write_text(
|
||||
f"users: {{foo: {{tokens: {{web-app-foo: {token}}}}}}}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
variables = self._variables()
|
||||
del variables[cache_users_store.STORE_FACT]
|
||||
variables["FILE_TOKENS"] = str(tokens)
|
||||
carried = self._park(roles, variables, {"foo": {"carried": True}})
|
||||
return roles, variables, tokens, carried
|
||||
|
||||
def test_a_changed_tokens_file_rejects_the_carrier(self):
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
roles, variables, tokens, _ = self._park_from_the_tokens_file(
|
||||
Path(tmp), "minted"
|
||||
)
|
||||
tokens.write_text(
|
||||
"users: {foo: {tokens: {web-app-foo: rotated-token}}}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
merged, renders = self._merged(roles, variables)
|
||||
self.assertEqual(renders, 1)
|
||||
self.assertEqual(
|
||||
merged["rendered"]["foo"]["tokens"]["web-app-foo"], "rotated-token"
|
||||
)
|
||||
|
||||
def test_the_store_fact_with_the_file_tokens_keeps_the_carrier(self):
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
roles, variables, _, carried = self._park_from_the_tokens_file(
|
||||
Path(tmp), "minted"
|
||||
)
|
||||
variables[cache_users_store.STORE_FACT] = {
|
||||
"foo": {"tokens": {"web-app-foo": "minted"}}
|
||||
}
|
||||
merged, renders = self._merged(roles, variables)
|
||||
self.assertIs(merged, carried)
|
||||
self.assertEqual(renders, 0)
|
||||
|
||||
def test_a_rotated_inventory_password_rejects_the_carrier(self):
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
roles = _seed_minimal_user_role(Path(tmp))
|
||||
|
||||
Reference in new issue
Block a user