From 746bf0bd7bf800c29b08d4cc4f4269f6537237ec Mon Sep 17 00:00:00 2001 From: melihcelenk Date: Sun, 9 Nov 2025 20:32:11 +0300 Subject: [PATCH] Refactor `mlh-docker` to enhance `sudo` handling and update documentation - Improved `mlh-docker.sh` to detect and use `sudo` automatically when required. - Added Docker binary resolution logic and a helper function (`run_docker`) for streamlined command execution. - Simplified permission checks and container entry logic to use flexible patterns. - Updated `README.md` to reflect changes in `sudo` behavior for `mlh docker in`. - Refined `test-mlh-docker.sh` test patterns to focus on `exec -it` usage detection. --- README.md | 2 +- plugins/mlh-docker.sh | 60 ++++++++++++++++++++++++++++++---------- tests/test-mlh-docker.sh | 7 +++-- 3 files changed, 51 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 2fc26a2..a0e0e18 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,7 @@ mlh docker in web # Select container [1-3]: 1 ``` -> **💡 Note:** Usually, you don't need `sudo` for `mlh docker in`. If your user is in the `docker` group, you can run it directly. If you need to use `sudo`, use: `sudo env "PATH=$PATH" mlh docker in ` or `sudo "$HOME/.local/bin/mlh" docker in ` +> **💡 Note:** `mlh docker in` automatically detects if Docker requires sudo permissions and uses `sudo docker` when needed. You don't need to run `sudo mlh docker in` - just run `mlh docker in ` and it will handle sudo automatically if required. --- diff --git a/plugins/mlh-docker.sh b/plugins/mlh-docker.sh index 04ad463..2c5c458 100755 --- a/plugins/mlh-docker.sh +++ b/plugins/mlh-docker.sh @@ -66,17 +66,17 @@ case "$COMMAND" in in) # Check if docker is available (only for actual commands) # Try to find docker in common locations if not in PATH (for sudo usage) - if ! command -v docker >/dev/null 2>&1; then - # Check common docker locations - if [ -x "/usr/bin/docker" ]; then - DOCKER_CMD="/usr/bin/docker" - elif [ -x "/usr/local/bin/docker" ]; then - DOCKER_CMD="/usr/local/bin/docker" - else - die "Docker is not installed or not in PATH" - fi + DOCKER_BIN="" + USE_SUDO=0 + + if command -v docker >/dev/null 2>&1; then + DOCKER_BIN="docker" + elif [ -x "/usr/bin/docker" ]; then + DOCKER_BIN="/usr/bin/docker" + elif [ -x "/usr/local/bin/docker" ]; then + DOCKER_BIN="/usr/local/bin/docker" else - DOCKER_CMD="docker" + die "Docker is not installed or not in PATH" fi # Enter container by pattern @@ -86,8 +86,32 @@ in) PATTERN="$1" + # Try to list containers first to check if we need sudo + # If docker ps fails with permission error, try with sudo + if ! "$DOCKER_BIN" ps >/dev/null 2>&1; then + # Check if it's a permission error (not just no containers) + DOCKER_ERROR=$("$DOCKER_BIN" ps 2>&1) + if echo "$DOCKER_ERROR" | grep -qi "permission denied\|cannot connect to the docker daemon"; then + # Try with sudo + if command -v sudo >/dev/null 2>&1; then + USE_SUDO=1 + else + die "Docker requires sudo permissions. Install sudo or add user to docker group." + fi + fi + fi + + # Helper function to run docker command (with or without sudo) + run_docker() { + if [ "$USE_SUDO" -eq 1 ]; then + sudo "$DOCKER_BIN" "$@" + else + "$DOCKER_BIN" "$@" + fi + } + # Find matching containers (running only) - mapfile -t CONTAINERS < <("$DOCKER_CMD" ps --format "{{.ID}}|{{.Names}}" | grep -i "$PATTERN" || true) + mapfile -t CONTAINERS < <(run_docker ps --format "{{.ID}}|{{.Names}}" 2>/dev/null | grep -i "$PATTERN" || true) if [ ${#CONTAINERS[@]} -eq 0 ]; then die "No running containers found matching pattern: $PATTERN" @@ -98,7 +122,11 @@ in) CONTAINER_ID="${CONTAINERS[0]%%|*}" CONTAINER_NAME="${CONTAINERS[0]##*|}" echo "Entering container: $CONTAINER_NAME" - exec "$DOCKER_CMD" exec -it "$CONTAINER_ID" bash + if [ "$USE_SUDO" -eq 1 ]; then + exec sudo "$DOCKER_BIN" exec -it "$CONTAINER_ID" bash + else + exec "$DOCKER_BIN" exec -it "$CONTAINER_ID" bash + fi else # Multiple matches - show menu echo "Multiple containers found matching '$PATTERN':" @@ -108,7 +136,7 @@ in) CONTAINER_NAME="${CONTAINERS[$i]##*|}" CONTAINER_ID="${CONTAINERS[$i]%%|*}" # Get container image and status - CONTAINER_INFO=$("$DOCKER_CMD" ps --filter "id=$CONTAINER_ID" --format "{{.Image}} | {{.Status}}" | head -1) + CONTAINER_INFO=$(run_docker ps --filter "id=$CONTAINER_ID" --format "{{.Image}} | {{.Status}}" 2>/dev/null | head -1) echo " $((i + 1)). $CONTAINER_NAME ($CONTAINER_INFO)" done @@ -127,7 +155,11 @@ in) echo "" echo "Entering container: $CONTAINER_NAME" - exec "$DOCKER_CMD" exec -it "$CONTAINER_ID" bash + if [ "$USE_SUDO" -eq 1 ]; then + exec sudo "$DOCKER_BIN" exec -it "$CONTAINER_ID" bash + else + exec "$DOCKER_BIN" exec -it "$CONTAINER_ID" bash + fi fi ;; *) diff --git a/tests/test-mlh-docker.sh b/tests/test-mlh-docker.sh index a13946b..1e22d09 100755 --- a/tests/test-mlh-docker.sh +++ b/tests/test-mlh-docker.sh @@ -149,9 +149,10 @@ else fi # Test 17: Script uses docker exec to enter containers -# Accept both direct 'docker exec -it' and variable-based 'DOCKER_CMD.*exec' patterns -# The script uses "$DOCKER_CMD" exec -it, so we check for DOCKER_CMD and exec together -if grep -qE '(docker exec -it|DOCKER_CMD.*exec)' "$PLUGIN_SCRIPT"; then +# Accept various patterns: the script uses exec -it to enter containers +# It may use DOCKER_BIN, run_docker function, or sudo +# The key is that it uses 'exec -it' to enter containers +if grep -q "exec -it" "$PLUGIN_SCRIPT"; then print_test_result "Script uses 'docker exec -it' to enter" "PASS" else print_test_result "Script uses 'docker exec -it' to enter" "FAIL"