asf-tooling commented on issue #651:
URL:
https://github.com/apache/tooling-trusted-releases/issues/651#issuecomment-4407563756
<!-- gofannon-issue-triage-bot v2 -->
**Automated triage** — analyzed at `main@751c2146`
**Type:** `bug_fix` • **Classification:** `actionable` • **Confidence:**
`high`
**Application domain(s):** `authentication_authorization`,
`announcements_reporting`
### Summary
The issue requested SVN credentials for ATR to commit to dist/release.
Infrastructure has set up the `atr-svn-rw` service account (confirmed by @sbp
on 2026-04-10, who tested by adding a README). The remaining work is to fix the
existing `commit()` function in `atr/svn/__init__.py` to use the proxy commit
pattern: authenticate as the service account (`atr-svn-rw`) and pass the actual
committer's UID via `--with-revprop asf:uid=<UID>`. Currently, the code
incorrectly uses the committer's username directly for `--username`
authentication, and does not set `asf:uid` revprop.
### Where this lives in the code today
#### `atr/svn/__init__.py` — `commit` (lines 94-115)
_needs modification_
Uses committer UID as --username for authentication, but proxy commits
require the service account for auth and the committer UID in a revprop.
```python
async def commit(path: pathlib.Path, url: str, username: str, revision: str,
message: str) -> str:
log.debug(f"running svn commit for user '{username}' to '{url}'")
# The username here is the ASF UID of the committer
svn_token = config.get().SVN_TOKEN
if svn_token is None:
raise ValueError("SVN_TOKEN must be set")
return await _run_svnmucc_command(
"put",
str(path),
url,
"--username",
username,
"--password",
svn_token,
"--non-interactive",
"--with-revprop",
f"asf:tool={_ASF_TOOL}",
"-r",
revision,
"-m",
message,
)
```
#### `atr/svn/__init__.py` — `get_diff` (lines 118-126)
_needs modification_
Uses literal 'atr' as username for SVN auth; should use the service account
username from config.
```python
async def get_diff(path: pathlib.Path, revision: int) -> str:
log.debug(f"running svn diff for '{path}': r{revision}")
svn_token = config.get().SVN_TOKEN
if svn_token is None:
raise ValueError("SVN_TOKEN must be set")
# TODO: Or omit username entirely?
return await _run_svn_command(
"diff", str(path), "-c", str(revision), "--username", _ASF_TOOL,
"--password", svn_token
)
```
#### `atr/svn/__init__.py` — `get_log` (lines 129-137)
_needs modification_
Same issue as get_diff - uses 'atr' as username instead of the service
account.
```python
async def get_log(path: pathlib.Path) -> SvnLog:
log.debug(f"running svn log for '{path}'")
svn_token = config.get().SVN_TOKEN
if svn_token is None:
raise ValueError("SVN_TOKEN must be set")
# TODO: Or omit username entirely?
log_output = await _run_svn_command("log", str(path), "--xml",
"--username", _ASF_TOOL, "--password", svn_token)
root = ElementTree.fromstring(log_output)
return SvnLog.from_xml_tree(root)
```
#### `atr/svn/commits.py` — `handle` (lines 34-39)
_currently does this_
Handles PubSub notifications for SVN commits - related infrastructure for
the dist repo integration.
```python
async def handle(payload: dict, working_copy_root: pathlib.Path) -> None:
pubsub_path = str(payload.get("pubsub_path", ""))
# Ignore commits outside dist/dev or dist/release
if pubsub_path.startswith(_WATCHED_PREFIXES):
log.debug(f"PubSub payload: {payload}")
await _process_payload(payload, working_copy_root)
```
### Where new code would go
- `atr/config.py` — near SVN_TOKEN definition
A new SVN_USERNAME config option is needed to store the service account
name (atr-svn-rw), complementing the existing SVN_TOKEN.
### Proposed approach
The fix involves two changes: (1) Add an `SVN_USERNAME` config option for
the service account name (`atr-svn-rw`), and (2) Modify the `commit()` function
to authenticate as the service account and pass the actual committer's UID via
`--with-revprop asf:uid=<UID>` for proxy commit attribution. The `get_diff` and
`get_log` functions should also be updated to use the service account username
from config rather than the literal `_ASF_TOOL` string.
This follows the proxy commit pattern discussed by @sbp (who mentioned
needing `asf:uid=<UID>` and `asf:tool=atr` revprops, as BAT does), and
@dave2wave (who confirmed the approach). The `roleaccount_hooks.lua` inclusion
for `atr-svn-rw` was also requested in INFRA-27688, which has been completed.
### Suggested patches
#### `atr/svn/__init__.py`
Fix commit() to use service account auth with proxy commit revprops, and fix
get_diff/get_log to use service account username from config.
````diff
--- a/atr/svn/__init__.py
+++ b/atr/svn/__init__.py
@@ -71,17 +71,22 @@
async def commit(path: pathlib.Path, url: str, username: str, revision:
str, message: str) -> str:
- log.debug(f"running svn commit for user '{username}' to '{url}'")
- # The username here is the ASF UID of the committer
+ log.debug(f"running svn commit on behalf of '{username}' to '{url}'")
+ # The username here is the ASF UID of the committer, used for proxy
commit attribution
svn_token = config.get().SVN_TOKEN
if svn_token is None:
raise ValueError("SVN_TOKEN must be set")
+ svn_username = config.get().SVN_USERNAME # TODO: confirm this config
attribute exists
+ if svn_username is None:
+ raise ValueError("SVN_USERNAME must be set")
return await _run_svnmucc_command(
"put",
str(path),
url,
"--username",
- username,
+ svn_username,
"--password",
svn_token,
"--non-interactive",
+ "--with-revprop",
+ f"asf:uid={username}",
"--with-revprop",
f"asf:tool={_ASF_TOOL}",
"-r",
@@ -93,16 +98,20 @@
async def get_diff(path: pathlib.Path, revision: int) -> str:
log.debug(f"running svn diff for '{path}': r{revision}")
svn_token = config.get().SVN_TOKEN
if svn_token is None:
raise ValueError("SVN_TOKEN must be set")
- # TODO: Or omit username entirely?
- return await _run_svn_command(
- "diff", str(path), "-c", str(revision), "--username", _ASF_TOOL,
"--password", svn_token
- )
+ svn_username = config.get().SVN_USERNAME # TODO: confirm this config
attribute exists
+ if svn_username is None:
+ raise ValueError("SVN_USERNAME must be set")
+ return await _run_svn_command(
+ "diff", str(path), "-c", str(revision), "--username", svn_username,
"--password", svn_token
+ )
async def get_log(path: pathlib.Path) -> SvnLog:
log.debug(f"running svn log for '{path}'")
svn_token = config.get().SVN_TOKEN
if svn_token is None:
raise ValueError("SVN_TOKEN must be set")
- # TODO: Or omit username entirely?
- log_output = await _run_svn_command("log", str(path), "--xml",
"--username", _ASF_TOOL, "--password", svn_token)
+ svn_username = config.get().SVN_USERNAME # TODO: confirm this config
attribute exists
+ if svn_username is None:
+ raise ValueError("SVN_USERNAME must be set")
+ log_output = await _run_svn_command("log", str(path), "--xml",
"--username", svn_username, "--password", svn_token)
root = ElementTree.fromstring(log_output)
return SvnLog.from_xml_tree(root)
````
### Open questions
- Does `atr/config.py` already have an `SVN_USERNAME` attribute, or does it
need to be added? The existing code only references `SVN_TOKEN`.
- Should the `asf:uid` revprop be omitted when the committer IS a PMC member
with direct commit access (per @dave2wave's note that proxy is only needed for
non-PMC members in certain groups)?
- Has the eyaml at
`infrastructure-p6/data/nodes/tooling-vm-ec2-de.apache.org.eyaml` been updated
with the credentials, or is that still pending?
- The roleaccount_hooks.lua inclusion for `atr-svn-rw` (INFRA-27688) - has
this been confirmed as completed alongside the account setup?
### Files examined
- `atr/svn/__init__.py`
- `atr/principal.py`
- `atr/tasks/svn.py`
- `atr/ssh.py`
- `atr/svn/commits.py`
- `atr/storage/writers/ssh.py`
- `atr/ldap.py`
- `atr/sessions.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]
