WIP: Remove passwordless sudo check in lib/common.sh - #1716
WIP: Remove passwordless sudo check in lib/common.sh#1716UncivilizedGhost wants to merge 1 commit into
Conversation
metal3-io-bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
metal3-io-bot
added
the
size/M
af5055b to
e7bb7a3
Compare
metal3-io-bot
added
size/L
|
/test metal3-dev-env-integration-test-ubuntu-main |
|
/copilot-review |
|
Copilot code review has been requested by @UncivilizedGhost. Please allow a few moments for the review to be added. |
There was a problem hiding this comment.
Pull request overview
Transitions Metal3 dev-env scripts to root execution and removes host-side sudo usage.
Changes:
- Replaces passwordless-sudo validation with a root check.
- Removes
sudofrom setup, configuration, launch, and verification commands. - Updates privileged container, networking, and libvirt operations.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
lib/common.sh |
Adds root enforcement and removes shared sudo calls. |
01_prepare_host.sh |
Runs dependency installation directly as root. |
02_configure_host.sh |
Removes sudo from host and container configuration. |
03_launch_mgmt_cluster.sh |
Removes sudo from cluster launch operations. |
04_verify.sh |
Runs verification commands directly as root. |
Suppressed comments (2)
lib/common.sh:9
- When this is invoked as instructed via
sudo,whoamiabove setsUSER=root. As a result, the documentedconfig_${user}.shis replaced by a newly generatedconfig_root.sh, working directories are owned by root, and kind/minikube are launched as root instead of as the invoking user. Preserve the caller identity (for example, fromSUDO_USER) while usingEUIDonly for this privilege check.
if [[ "${EUID}" -ne 0 ]]; then
lib/common.sh:10
- The README quickstart still tells users to run each script directly or run plain
make(README.md:85–95), both of which now exit here. Update the documented invocation and configuration/environment guidance to match the new root-only entry point.
echo "ERROR: metal3-dev-env must be run as root. Call the script with sudo!"
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if [[ -n "${MINIKUBE_BMNET_V6_IP:-}" ]]; then | ||
| sudo su -l -c "minikube ssh -- sudo sysctl -w net.ipv6.conf.all.disable_ipv6=0" "${USER}" | ||
| sudo su -l -c "minikube ssh -- sudo ip addr add ${MINIKUBE_BMNET_V6_IP}/64 dev eth3" "${USER}" | ||
| su -l -c "minikube ssh -- sysctl -w net.ipv6.conf.all.disable_ipv6=0" "${USER}" |
| # Verify the script is running as root | ||
| if [[ "${EUID}" -ne 0 ]]; then |
e7bb7a3 to
1c88dfa
Compare
|
/copilot-review |
|
Copilot code review has been requested by @UncivilizedGhost. Please allow a few moments for the review to be added. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
lib/common.sh:10
- This changes every script sourcing
common.shto reject the existing unprivileged invocation, but the user documentation still tells users to runmakeor./01_prepare_host.shdirectly (README.md:85-95). Update the deployment and cleanup/test instructions to show the required privileged entry point so the documented workflow does not immediately exit here.
echo "ERROR: metal3-dev-env must be run as root. Call the script with sudo!"
4a64e86 to
8effbc0
Compare
|
/test metal3-dev-env-integration-test-ubuntu-main |
|
@UncivilizedGhost: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
0aab6fb to
b79cff3
Compare
Replace sudo check with root check then remove all sudo instances Signed-off-by: Saad Zia <saad.zia@est.tech>
b79cff3 to
8379b62
Compare
Replace sudo check with root check then remove all sudo instances
Fix #1288