canonical
feat(alpine): add support for Busybox adduser/addgroup
feat(alpine): add support for Busybox adduser/addgroup
By default Alpine Linux provides Busybox utilities such as adduser
and addgroup for managing users and groups. Optionally the Alpine
"shadow" package provides the traditional useradd/groupadd utilities.
Add fallback support for the Busybox user/group management utilities
for Alpine Linux.
Additional Context
Test Steps
Checklist
- [x] My code follows the process laid out in the documentation
- [x] I have updated or added any unit tests accordingly
- [x] I have updated or added any documentation accordingly
Merge type
- [x] Squash merge using "Proposed Commit Message"
- [ ] Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)
@dermotbradley , thanks for this! It'll be good to have extra busybox support.
First pass, I mentioned this inline, but I'm wondering if we can re-use the
adduserimplementation that already exists rather than re-adding most of the existing code in the Alpine context. If there's a reason we can't callsuper()directly, is there a way we could modify the existing code so that it could be used in both places?
Done.
Am unsure how to resolve the pylint failure:
cloudinit/distros/alpine.py:330: [E1133(not-an-iterable), Distro.add_user] Non-iterable value addn_groups is used in an iterating context
I added a type annotation to addn_groups but that had no effect.
Am unsure how to resolve the pylint failure:
cloudinit/distros/alpine.py:330: [E1133(not-an-iterable), Distro.add_user] Non-iterable value addn_groups is used in an iterating context
I added a type annotation to addn_groups but that had no effect.
I ended up adding a pylint ignore - it seems there are known issues with this warning
One more general is that this Alpine
add_useris becoming quite long. I won't block the PR on it since the parent code it's modeled after is also somewhat long and complicated, but I do think there are some sections like group handling or shadow handling that could easily be moved into helper functions.
Ok, I'll look into this as a future improvement.
For how much was added here, it would be nice if we could get some additional test coverage. There are quite a few lines/functions with no coverage at all. Since us upstream devs don't have the Alpine expertise, I think it'd really benefit you long term so that we don't accidentally break things if we ever need to modify things here.
Agreed, I do intend to extend the testing for this and yes this would become easier once things are split up more.
Thank you for this correction @dermotbradley. I was basing my suggestion off of adduser behavior as seen in incus/lxd alpine cloud images and assumed it'd be comparable for /usr/bin/passwd
~ # cat /etc/os-release NAME="Alpine Linux" ID=alpine VERSION_ID=3.19.1 PRETTY_NAME="Alpine Linux v3.19" HOME_URL="https://alpinelinux.org/" BUG_REPORT_URL="https://gitlab.alpinelinux.org/alpine/aports/-/issues" ~ # ls -l /usr/sbin/adduser lrwxrwxrwx 1 root root 12 Jan 26 17:53 /usr/sbin/adduser -> /bin/busybox
On Thu, May 30, 2024 at 8:10 PM dermotbradley @.***> wrote:
@.**** commented on this pull request.
In cloudinit/distros/alpine.py https://github.com/canonical/cloud-init/pull/5176#discussion_r1621589437 :
(_out, err) = subp.subp(["passwd", "--help"])
if not err.startswith("BusyBox"):Done.
I had to change the logic to check if /usr/bin/passwd not being a softlink or the softlink not pointing to "bbsuid" (Alpine uses "bbsuid" binary rather than "busybox" for commands that require suid)
— Reply to this email directly, view it on GitHub https://github.com/canonical/cloud-init/pull/5176#discussion_r1621589437, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADLZJRRZJ3LNZUEZKQJH7GLZE7L2BAVCNFSM6AAAAABGGNVJQ6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAOBZG43TSNZZG4 . You are receiving this because you were mentioned.Message ID: @.***>
