asf-tooling commented on issue #520:
URL:
https://github.com/apache/tooling-trusted-releases/issues/520#issuecomment-4410183428
<!-- gofannon-issue-triage-bot v2 -->
**Automated triage** — analyzed at `main@2da7807a`
**Type:** `new_feature` • **Classification:** `actionable` •
**Confidence:** `medium`
**Application domain(s):** `authentication_authorization`,
`project_committee_management`
### Summary
The issue requests that PMC members be able to designate committers as
'release managers' who get the same write (but not vote) permissions as PMC
members. The final comment from @sbp clarifies 'We are not going to subset
which committee members can release,' meaning this is purely about elevating
committer permissions, not restricting PMC members. The SQL model already has a
`release_managers` field on Committee (evidenced by its use in
`_update_tooling`), but the authorization system (`atr/user.py`,
`atr/principal.py`, storage writers) doesn't yet use it to grant elevated
access to designated committers.
### Where this lives in the code today
#### `atr/user.py` — `projects` (lines 109-131)
_needs modification_
This function determines which projects a user can access. It checks
`committee_members` and `committers` but doesn't check `release_managers`. A
release manager who is only a committer would need elevated access comparable
to `committee_only` level.
```python
async def projects(uid: str, committee_only: bool = False, super_project:
bool = False) -> list[sql.Project]:
user_projects: list[sql.Project] = []
async with db.session() as data:
# Must have releases, because this is used in candidate_drafts
projects = await data.project(
status=sql.ProjectStatus.ACTIVE, _committee=True,
_releases=True, _super_project=super_project
).all()
for p in projects:
if p.committee is None:
continue
# Allow access to test project in Test mode
# This means that the Test project will show in the user
interface for everyone
if config.is_test_mode() and (p.committee.key == "test"):
user_projects.append(p)
continue
if committee_only:
if uid in p.committee.committee_members:
user_projects.append(p)
else:
if (uid in p.committee.committee_members) or (uid in
p.committee.committers):
user_projects.append(p)
```
#### `atr/user.py` — `is_committee_member` (lines 97-100)
_needs modification_
This function checks if a user is a committee (PMC) member. Release managers
need write permissions equivalent to PMC members but shouldn't appear as PMC
members for voting purposes. A parallel check (e.g., `is_release_manager`) is
needed.
```python
def is_committee_member(committee: sql.Committee | None, uid: str) -> bool:
if committee is None:
return False
return any((member_uid == uid) for member_uid in
committee.committee_members)
```
#### `atr/storage/writers/project.py` — `CommitteeMember` (lines 77-93)
_extension point_
The storage writer hierarchy (GeneralPublic → FoundationCommitter →
CommitteeParticipant → CommitteeMember) is where write authorization is
enforced. Release managers need access at CommitteeMember level for write
operations but not for vote operations.
```python
class CommitteeMember(CommitteeParticipant):
def __init__(
self,
write: storage.Write,
write_as: storage.WriteAsCommitteeMember,
data: db.Session,
committee_key: str,
):
super().__init__(write, write_as, data, committee_key)
self.__write = write
self.__write_as = write_as
self.__data = data
asf_uid = write.authorisation.asf_uid
if asf_uid is None:
raise storage.AccessError("Not authorized", status=403)
self.__asf_uid = asf_uid
self.__committee_key = committee_key
```
#### `atr/user.py` — `is_binding_for_release` (lines 75-94)
_currently does this_
Binding vote logic correctly uses `is_committee_member` - release managers
should NOT get binding votes, so this function should remain unchanged.
```python
async def is_binding_for_release(
committee: sql.Committee,
asf_uid: str,
vote_round: int | None,
caller_data: db.Session | None = None,
) -> tuple[bool, str]:
if not committee.is_podling:
if vote_round is not None:
raise ValueError("Non-podling votes require vote_round to be
None")
return is_committee_member(committee, asf_uid),
committee.display_name
if vote_round is None:
raise ValueError("Podling votes require vote_round 1 or 2")
if vote_round not in (1, 2):
raise ValueError(f"Unexpected podling vote_round: {vote_round!r}")
if vote_round == 1:
return is_committee_member(committee, asf_uid),
committee.display_name
async with db.ensure_session(caller_data) as data:
incubator = await data.committee(key="incubator").get()
return is_committee_member(incubator, asf_uid), "Incubator"
```
### Where new code would go
- `atr/user.py` — after symbol is_committer
Add an `is_release_manager` function parallel to `is_committee_member` and
`is_committer` that checks the `release_managers` field on Committee.
- `atr/storage/writers/project.py` — after symbol CommitteeParticipant
Optionally add a `ReleaseManager` writer class between
CommitteeParticipant and CommitteeMember, or modify CommitteeMember
authorization to also admit release managers.
- `atr/storage/__init__.py` — new file or extension
The storage Write class needs to be updated to recognize release managers
as having elevated write access (equivalent to CommitteeMember) without
granting vote permissions.
### Proposed approach
The implementation has two main parts: (1) Using the existing
`release_managers` field on `sql.Committee` to store designated release
managers per committee/project, and (2) updating the authorization system to
grant release managers write access equivalent to PMC members.
For part 1, a PMC member needs to be able to designate committers as release
managers (via a project settings form or the Start Release form, as @dave2wave
outlined). The `release_managers` field already exists on the Committee model.
For part 2, the key changes are: add an `is_release_manager()` check in
`atr/user.py`; update `projects()` to recognize release managers at the
committee-member level when determining write access; ensure the storage writer
authorization (in `atr/storage/__init__.py`) admits release managers for write
operations but NOT for vote binding. The vote logic in `is_binding_for_release`
must remain unchanged—release managers should not get binding votes. Since I
don't have the full `atr/storage/__init__.py` or `atr/models/sql.py` source,
the diffs below are limited to files I can verify.
### Suggested patches
#### `atr/user.py`
Add `is_release_manager` helper function and update `projects()` to
recognize release managers at the committee-member access level for write
operations.
````diff
--- a/atr/user.py
+++ b/atr/user.py
@@ -90,6 +90,12 @@ def is_committer(committee: sql.Committee | None, uid:
str) -> bool:
return any((committer_uid == uid) for committer_uid in
committee.committers)
+def is_release_manager(committee: sql.Committee | None, uid: str) -> bool:
+ if committee is None:
+ return False
+ return any((rm_uid == uid) for rm_uid in committee.release_managers)
+
+
async def projects(uid: str, committee_only: bool = False, super_project:
bool = False) -> list[sql.Project]:
user_projects: list[sql.Project] = []
async with db.session() as data:
@@ -109,7 +115,11 @@ async def projects(uid: str, committee_only: bool =
False, super_project: bool =
if committee_only:
if uid in p.committee.committee_members:
user_projects.append(p)
+ elif uid in p.committee.release_managers:
+ user_projects.append(p)
else:
if (uid in p.committee.committee_members) or (uid in
p.committee.committers):
user_projects.append(p)
+ elif uid in p.committee.release_managers:
+ user_projects.append(p)
return user_projects
````
### Open questions
- What does the `release_managers` field on `sql.Committee` look like (type,
constraints)? I need `atr/models/sql.py` to confirm its structure.
- How is `atr/storage/__init__.py` structured? The `Write` class and
`WriteAsCommitteeMember` need to be updated to admit release managers, but I
don't have that source.
- Should release managers be designated per-committee (as current
`release_managers` field suggests) or per-project or per-release? @dave2wave's
comment #5 suggests both per-project defaults and per-release overrides.
- What UI form changes are needed? The Start Release form and Project form
need modification to allow PMC members to designate release managers.
- How should the `release_managers` list be persisted and managed? Through a
project settings endpoint? Through `.asf.yaml` in the future?
### Files examined
- `atr/user.py`
- `atr/principal.py`
- `atr/ldap.py`
- `atr/cache.py`
- `atr/storage/writers/project.py`
- `atr/storage/writers/tokens.py`
- `atr/datasources/apache.py`
---
*Draft from a triage agent. A human reviewer should validate before merging
any change. The agent did not run tests or verify diffs apply.*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
