diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e5ed274..9b7f84e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,14 +25,24 @@ jobs: go-version: ${{ matrix.go-version }} cache: true - - name: Run tests with race detection and coverage - run: go test -race -coverprofile=coverage.out -covermode=atomic ./... + - name: Install and start nginx + run: | + sudo apt-get update -qq + sudo apt-get install -y -qq nginx + sudo systemctl start nginx + sudo systemctl enable nginx + + - name: Run tests (user) + run: go test -race -coverprofile=coverage-user.out -covermode=atomic ./... + + - name: Run tests (root) + run: sudo go test -race -coverprofile=coverage-root.out -covermode=atomic ./... - name: Upload coverage to Codecov if: matrix.go-version == '1.26' uses: codecov/codecov-action@v5 with: - files: coverage.out + files: coverage-user.out,coverage-root.out token: ${{ secrets.CODECOV_TOKEN }} fail_ci_if_error: false diff --git a/filtererr_test.go b/filtererr_test.go index e7689d2..c1cd4ba 100644 --- a/filtererr_test.go +++ b/filtererr_test.go @@ -61,6 +61,21 @@ func TestFilterErr(t *testing.T) { stderr: "Failed to do something unknown", want: ErrUnspecified, }, + { + name: "does not exist with auth required prioritizes permission error", + stderr: "Unit nginx.service does not exist, proceeding anyway.\nFailed to mask unit: Interactive authentication required.", + want: ErrInsufficientPermissions, + }, + { + name: "does not exist with access denied prioritizes permission error", + stderr: "Unit foo.service does not exist, proceeding anyway.\nAccess denied", + want: ErrInsufficientPermissions, + }, + { + name: "does not exist with bus failure prioritizes bus error", + stderr: "Unit foo.service does not exist, proceeding anyway.\n$DBUS_SESSION_BUS_ADDRESS not set", + want: ErrBusFailure, + }, { name: "unrecognized warning", stderr: "Warning: something benign happened", diff --git a/go.mod b/go.mod index 6a43b6f..d172535 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ module github.com/taigrr/systemctl -go 1.26 +go 1.26.1 diff --git a/systemctl_test.go b/systemctl_test.go index 0e246f6..af8817a 100644 --- a/systemctl_test.go +++ b/systemctl_test.go @@ -290,7 +290,7 @@ func TestMask(t *testing.T) { // try existing unit in user mode as user {"syncthing", nil, Options{UserMode: true}, true}, // try nonexisting unit in system mode as user - {"nonexistant", ErrDoesNotExist, Options{UserMode: false}, true}, + {"nonexistant", ErrInsufficientPermissions, Options{UserMode: false}, true}, // try existing unit in system mode as user {"nginx", ErrInsufficientPermissions, Options{UserMode: false}, true}, @@ -521,7 +521,7 @@ func TestUnmask(t *testing.T) { // try existing unit in user mode as user {"syncthing", nil, Options{UserMode: true}, true}, // try nonexisting unit in system mode as user - {"nonexistant", ErrDoesNotExist, Options{UserMode: false}, true}, + {"nonexistant", ErrInsufficientPermissions, Options{UserMode: false}, true}, // try existing unit in system mode as user {"nginx", ErrInsufficientPermissions, Options{UserMode: false}, true}, diff --git a/util.go b/util.go index 84257c1..a726f77 100644 --- a/util.go +++ b/util.go @@ -70,15 +70,13 @@ func prepareArgs(base string, opts Options, extra ...string) []string { } func filterErr(stderr string) error { + // Order matters: check higher-priority errors first. + // For example, `systemctl mask nginx` as a non-root user on a system + // without nginx prints both "does not exist, proceeding anyway" (a + // warning) and "Interactive authentication required" (the real error). + // Permission and bus errors must be checked before "does not exist" so + // the actual failure reason is returned. switch { - case strings.Contains(stderr, `does not exist`): - return errors.Join(ErrDoesNotExist, fmt.Errorf("stderr: %s", stderr)) - case strings.Contains(stderr, `not found.`): - return errors.Join(ErrDoesNotExist, fmt.Errorf("stderr: %s", stderr)) - case strings.Contains(stderr, `not loaded.`): - return errors.Join(ErrUnitNotLoaded, fmt.Errorf("stderr: %s", stderr)) - case strings.Contains(stderr, `No such file or directory`): - return errors.Join(ErrDoesNotExist, fmt.Errorf("stderr: %s", stderr)) case strings.Contains(stderr, `Interactive authentication required`): return errors.Join(ErrInsufficientPermissions, fmt.Errorf("stderr: %s", stderr)) case strings.Contains(stderr, `Access denied`): @@ -87,6 +85,14 @@ func filterErr(stderr string) error { return errors.Join(ErrBusFailure, fmt.Errorf("stderr: %s", stderr)) case strings.Contains(stderr, `is masked`): return errors.Join(ErrMasked, fmt.Errorf("stderr: %s", stderr)) + case strings.Contains(stderr, `does not exist`): + return errors.Join(ErrDoesNotExist, fmt.Errorf("stderr: %s", stderr)) + case strings.Contains(stderr, `not found.`): + return errors.Join(ErrDoesNotExist, fmt.Errorf("stderr: %s", stderr)) + case strings.Contains(stderr, `not loaded.`): + return errors.Join(ErrUnitNotLoaded, fmt.Errorf("stderr: %s", stderr)) + case strings.Contains(stderr, `No such file or directory`): + return errors.Join(ErrDoesNotExist, fmt.Errorf("stderr: %s", stderr)) case strings.Contains(stderr, `Failed`): return errors.Join(ErrUnspecified, fmt.Errorf("stderr: %s", stderr)) default: