This is an automated email from the ASF dual-hosted git repository.
sbp pushed a commit to branch sbp
in repository https://gitbox.apache.org/repos/asf/tooling-trusted-releases.git
The following commit(s) were added to refs/heads/sbp by this push:
new f24a7399 Improve key deletion methods in the storage interface
f24a7399 is described below
commit f24a739904fb6ea6be33cfefd5844a75ec77b157
Author: Sean B. Palmer <[email protected]>
AuthorDate: Thu Apr 2 20:12:53 2026 +0100
Improve key deletion methods in the storage interface
---
atr/storage/writers/keys.py | 87 +++++++++++++--------
tests/unit/test_keys_writer.py | 170 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 224 insertions(+), 33 deletions(-)
diff --git a/atr/storage/writers/keys.py b/atr/storage/writers/keys.py
index cebad86c..c8d8c627 100644
--- a/atr/storage/writers/keys.py
+++ b/atr/storage/writers/keys.py
@@ -84,13 +84,11 @@ class FoundationCommitter(GeneralPublic):
apache_uid=self.__asf_uid,
_committees=True,
).demand(storage.AccessError(f"Key not found: {fingerprint}"))
+ affected_committee_keys = {committee.key for committee in
key.committees}
await self.__data.delete(key)
await self.__data.commit()
- for committee in key.committees:
- wacm =
self.__write.as_committee_member_outcome(committee.key).result_or_none()
- if wacm is None:
- continue
- await wacm.keys.autogenerate_keys_file()
+ for committee_key in sorted(affected_committee_keys):
+ await self._sync_committee_keys_file(committee_key)
return outcome.Result(key)
except Exception as e:
return outcome.Error(e)
@@ -144,8 +142,11 @@ class FoundationCommitter(GeneralPublic):
committee = await self.__data.committee(key=committee_key,
_public_signing_keys=True).demand(
storage.AccessError(f"Committee not found: {committee_key}")
)
+ return await self._keys_file_text(committee)
+
+ async def _keys_file_text(self, committee: sql.Committee) -> str:
if not committee.public_signing_keys:
- raise storage.AccessError(f"No keys found for committee
{committee_key} to generate KEYS file.")
+ raise storage.AccessError(f"No keys found for committee
{committee.key} to generate KEYS file.")
sorted_keys = sorted(committee.public_signing_keys, key=lambda k:
k.fingerprint)
@@ -178,11 +179,46 @@ class FoundationCommitter(GeneralPublic):
key_count_for_header = len(committee.public_signing_keys)
return await self.__keys_file_format(
- committee_key=committee_key,
+ committee_key=committee.key,
key_count_for_header=key_count_for_header,
key_blocks_str=key_blocks_str,
)
+ def _committee_keys_path(self, committee: sql.Committee) -> pathlib.Path:
+ base_downloads_dir = paths.get_downloads_dir()
+ if committee.is_podling:
+ return base_downloads_dir / "incubator" / committee.key / "KEYS"
+ return base_downloads_dir / committee.key / "KEYS"
+
+ async def _sync_committee_keys_file(self, committee_key: str) -> str |
None:
+ committee = await self.__data.committee(key=committee_key,
_public_signing_keys=True).demand(
+ storage.AccessError(f"Committee not found: {committee_key}")
+ )
+ committee_keys_path = self._committee_keys_path(committee)
+ committee_keys_dir = committee_keys_path.parent
+
+ if not committee.public_signing_keys:
+ try:
+ if await aiofiles.os.path.exists(committee_keys_path):
+ await aiofiles.os.remove(committee_keys_path)
+ except OSError as e:
+ raise storage.AccessError(f"Failed to remove KEYS file for
committee {committee_key}: {e}") from e
+ return None
+
+ full_keys_file_content = await self._keys_file_text(committee)
+ try:
+ await aiofiles.os.makedirs(committee_keys_dir, exist_ok=True)
+ await asyncio.to_thread(util.chmod_directories,
committee_keys_dir, permissions=0o755)
+ await asyncio.to_thread(committee_keys_path.write_text,
full_keys_file_content, encoding="utf-8")
+ except OSError as e:
+ raise storage.AccessError(f"Failed to write KEYS file for
committee {committee_key}: {e}") from e
+ except Exception as e:
+ log.exception(f"An unexpected error occurred writing KEYS for
committee {committee_key}: {e}")
+ raise storage.AccessError(
+ f"An unexpected error occurred writing KEYS for committee
{committee_key}: {e}"
+ ) from e
+ return str(committee_keys_path)
+
async def test_user_delete_all(self, test_uid: str) ->
outcome.Outcome[int]:
"""Delete all OpenPGP keys and their links for a test user."""
if not config.is_test_mode():
@@ -259,9 +295,7 @@ class FoundationCommitter(GeneralPublic):
await self.__data.commit()
for committee_key in sorted(affected):
- wacp =
self.__write.as_committee_participant_outcome(committee_key).result_or_none()
- if wacp is not None:
- await wacp.keys.autogenerate_keys_file()
+ await self._sync_committee_keys_file(committee_key)
return affected
@@ -472,32 +506,19 @@ class CommitteeParticipant(FoundationCommitter):
self,
) -> outcome.Outcome[str]:
try:
- base_downloads_dir = paths.get_downloads_dir()
-
committee = await self.committee()
- is_podling = committee.is_podling
-
- full_keys_file_content = await
self.keys_file_text(self.__committee_key)
- if is_podling:
- committee_keys_dir = base_downloads_dir / "incubator" /
self.__committee_key
- else:
- committee_keys_dir = base_downloads_dir / self.__committee_key
- committee_keys_path = committee_keys_dir / "KEYS"
+ if not committee.public_signing_keys:
+ return outcome.Error(
+ storage.AccessError(f"No keys found for committee
{self.__committee_key} to generate KEYS file.")
+ )
+ synced_path = await
self._sync_committee_keys_file(self.__committee_key)
except Exception as e:
return outcome.Error(e)
-
- try:
- await aiofiles.os.makedirs(committee_keys_dir, exist_ok=True)
- await asyncio.to_thread(util.chmod_directories,
committee_keys_dir, permissions=0o755)
- await asyncio.to_thread(committee_keys_path.write_text,
full_keys_file_content, encoding="utf-8")
- except OSError as e:
- error_msg = f"Failed to write KEYS file for committee
{self.__committee_key}: {e}"
- return outcome.Error(storage.AccessError(error_msg))
- except Exception as e:
- error_msg = f"An unexpected error occurred writing KEYS for
committee {self.__committee_key}: {e}"
- log.exception(f"An unexpected error occurred writing KEYS for
committee {self.__committee_key}: {e}")
- return outcome.Error(storage.AccessError(error_msg))
- return outcome.Result(str(committee_keys_path))
+ if synced_path is None:
+ return outcome.Error(
+ storage.AccessError(f"No keys found for committee
{self.__committee_key} to generate KEYS file.")
+ )
+ return outcome.Result(synced_path)
async def committee(self) -> sql.Committee:
return await self.__data.committee(key=self.__committee_key,
_public_signing_keys=True).demand(
diff --git a/tests/unit/test_keys_writer.py b/tests/unit/test_keys_writer.py
new file mode 100644
index 00000000..1c1da45d
--- /dev/null
+++ b/tests/unit/test_keys_writer.py
@@ -0,0 +1,170 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import unittest.mock as mock
+from types import SimpleNamespace
+
+import pytest
+
+import atr.storage.outcome as outcome
+import atr.storage.writers.keys as keys_writer
+
+
+class Query:
+ def __init__(self, value):
+ self._value = value
+
+ async def get(self):
+ return self._value
+
+ async def demand(self, error: Exception):
+ if self._value is None:
+ raise error
+ return self._value
+
+
+class MockData:
+ def __init__(self, key, committees_after_commit: dict[str, object]):
+ self._key = key
+ self._committees_after_commit = committees_after_commit
+ self.begin_immediate = mock.AsyncMock()
+ self.commit = mock.AsyncMock()
+ self.delete = mock.AsyncMock()
+ self.execute = mock.AsyncMock()
+
+ def public_signing_key(self, **_kwargs):
+ return Query(self._key)
+
+ def committee(self, *, key: str, _public_signing_keys: bool = False):
+ assert _public_signing_keys is True
+ return Query(self._committees_after_commit[key])
+
+
[email protected]
+async def test_delete_key_removal_deletes_empty_keys_file(tmp_path):
+ owned_key = SimpleNamespace(
+ fingerprint="fp1",
+ committees=[SimpleNamespace(key="alpha")],
+ )
+ data = MockData(
+ owned_key,
+ committees_after_commit={"alpha": _committee("alpha", [])},
+ )
+ writer, _write = _make_foundation_committer(data)
+
+ keys_path = tmp_path / "alpha" / "KEYS"
+ keys_path.parent.mkdir(parents=True)
+ keys_path.write_text("stale content", encoding="utf-8")
+
+ with (
+ mock.patch.object(keys_writer.paths, "get_downloads_dir",
return_value=tmp_path),
+ mock.patch.object(keys_writer.util, "chmod_directories"),
+ ):
+ result = await writer.delete_key("fp1")
+
+ assert isinstance(result, outcome.Result)
+ assert not keys_path.exists()
+ data.delete.assert_awaited_once_with(owned_key)
+ data.commit.assert_awaited_once()
+
+
[email protected]
+async def
test_update_committee_associations_removal_deletes_empty_keys_file(tmp_path):
+ owned_key = SimpleNamespace(fingerprint="fp1",
committees=[SimpleNamespace(key="alpha")])
+ data = MockData(
+ owned_key,
+ committees_after_commit={"alpha": _committee("alpha", [])},
+ )
+ writer, write = _make_foundation_committer(data)
+
+ keys_path = tmp_path / "alpha" / "KEYS"
+ keys_path.parent.mkdir(parents=True)
+ keys_path.write_text("stale content", encoding="utf-8")
+
+ with (
+ mock.patch.object(keys_writer.paths, "get_downloads_dir",
return_value=tmp_path),
+ mock.patch.object(keys_writer.util, "chmod_directories"),
+ ):
+ affected = await writer.update_committee_associations("fp1", [])
+
+ assert affected == {"alpha"}
+ assert not keys_path.exists()
+ assert write.as_committee_participant.call_count == 0
+ data.begin_immediate.assert_awaited_once()
+ data.commit.assert_awaited_once()
+
+
[email protected]
+async def
test_update_committee_associations_removal_rewrites_keys_file_with_remaining_keys(tmp_path):
+ owned_key = SimpleNamespace(fingerprint="fp1",
committees=[SimpleNamespace(key="alpha")])
+ remaining_key = _public_key("bbbbccccdddd1111")
+ data = MockData(
+ owned_key,
+ committees_after_commit={"alpha": _committee("alpha",
[remaining_key])},
+ )
+ writer, write = _make_foundation_committer(data)
+
+ keys_path = tmp_path / "alpha" / "KEYS"
+ keys_path.parent.mkdir(parents=True)
+ keys_path.write_text("stale content", encoding="utf-8")
+
+ with (
+ mock.patch.object(keys_writer.paths, "get_downloads_dir",
return_value=tmp_path),
+ mock.patch.object(keys_writer.util, "chmod_directories"),
+ ):
+ affected = await writer.update_committee_associations("fp1", [])
+
+ assert affected == {"alpha"}
+ assert keys_path.exists()
+ content = keys_path.read_text(encoding="utf-8")
+ assert "stale content" not in content
+ assert remaining_key.fingerprint.upper() in content
+ assert "Signing keys for the alpha committee" in content
+ assert write.as_committee_participant.call_count == 0
+
+
+def _committee(key: str, public_signing_keys: list[object], *, is_podling:
bool = False):
+ return SimpleNamespace(
+ key=key,
+ is_podling=is_podling,
+ public_signing_keys=public_signing_keys,
+ )
+
+
+def _make_foundation_committer(data: MockData):
+ write = mock.MagicMock()
+ write.authorisation.asf_uid = "alice"
+ write.as_committee_participant = mock.MagicMock()
+ write_as = mock.MagicMock()
+ return keys_writer.FoundationCommitter(write, write_as, data), write
+
+
+def _public_key(
+ fingerprint: str,
+ *,
+ apache_uid: str = "bob",
+ primary_declared_uid: str = "Bob <[email protected]>",
+ ascii_armored_key: str | None = None,
+):
+ if ascii_armored_key is None:
+ ascii_armored_key = "-----BEGIN PGP PUBLIC KEY
BLOCK-----\nbody\n-----END PGP PUBLIC KEY BLOCK-----\n"
+ return SimpleNamespace(
+ fingerprint=fingerprint,
+ apache_uid=apache_uid,
+ primary_declared_uid=primary_declared_uid,
+ ascii_armored_key=ascii_armored_key,
+ )
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
