bekkalokk/gitea/import-users: refactor + add members to groups #70
No reviewers
felixalb
Labels
No Label
art
backup
big
blocked
bug
crash report
disputed
documentation
duplicate
enhancement
good first issue
logging
nixos
question
salt
security
servers n' hardware
wontfix
No Milestone
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: Drift/pvv-nixos-config#70
Loading…
Reference in New Issue
No description provided.
Delete Branch "gitea-import-users-add-to-members"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Fixes: Drift/issues#128
This refactors out most of the gitea API calls, so they are both easy to verify, and potentially switch out later if we need to.
It also adds all users to some common organization groups.
This required some new privileges for the token, so the token has been switched out as well.
It currently has:
Not sure if everything is needed, but I think it's a fair subset. admin read/write was already required to create users.
Should the required token permissions be listed in the source code somewhere?
Some comments about separating "add" and "update", as they don't have to relate to each other at all, but it's not a blocker. This is after all Nitea, you can merge if you disagree.
@ -76,2 +97,2 @@# Read the file, add each userwith open("/tmp/passwd-import", 'r') as f:# Reads out a passwd-file line for line, and filters out# real PVV users (as opposed to system users meant for daemons and such)Here, you are both explicitly allowing very long lines, and strictly wrapping the text. Maybe collapse this type of comment into single lines?
Eh, I'm not intentionally creating long lines, just not breaking the non-important ones (e.g. 100 char strings).
@ -91,0 +133,4 @@existing_users[username] = userelse:user["visibility"] = existing_users[username]["visibility"]This "patch existing users" section should probably be it's own function, and be explained to show what it is / should be doing.
It' not immediately clear to me at the moment how to best split this, when both depend on the same base user settings with name and username as input, and not just creating a bunch of overhead. I'll add a comment describing what it does though. Feel free to fix it up and ping me in another PR.
@ -91,0 +168,4 @@for username, name in passwd_file_parser("/tmp/passwd-import"):print(f"Processing {username}")add_or_patch_gitea_user(username, name, existing_users)The splitting into "add" or "patch" should probably also be done here, rather than in one large function, as they are quite different.
@ -91,0 +169,4 @@for username, name in passwd_file_parser("/tmp/passwd-import"):print(f"Processing {username}")add_or_patch_gitea_user(username, name, existing_users)for org, team_name in COMMON_USER_TEAMS:This makes it explicitly impossible to leave either of those teams for good, is that intended? I suggest moving this into the "add new user" function, and not the "update existing user" section.
This is intended. All PVV members should be able to edit PVV projects. There are no downsides to being a member of the organization (at least when we fix the autowatch thing). We could alternatively provide the projects as an input to the script instead of hardcoding a list so it's more obvious from the nix code, but this will work for now. Maybe when there's more orgs to join
"at all" is some strong wording. They didn't exist in the same function already if they weren't related to some degree. Adding user information and ensuring user information is correct, and those actions being mutually exclusive for every user in the loop is related enough that these were already put into a single function beforehand. But feel free to split it in another PR if you feel it would improve the code.
08fc48cf87to4cd2e6c4b04cd2e6c4b0to4662e6b55d