mirror of
https://github.com/taigrr/systemctl.git
synced 2026-03-09 00:14:38 -07:00
fix(errors): prioritize permission errors over 'does not exist' warnings (#11)
filterErr checked 'does not exist' before 'Interactive authentication required', so when systemd printed both (common for mask/unmask on non-installed units as a non-root user), the wrong error was returned. Reorder checks so permission, bus, and masked errors take priority over existence warnings. Add tests covering mixed-stderr scenarios. Also: - CI: install and start nginx so user + root tests pass - CI: run tests as both user and root for full coverage - Bump Go 1.26 → 1.26.1
This commit is contained in:
16
.github/workflows/ci.yml
vendored
16
.github/workflows/ci.yml
vendored
@@ -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
|
||||
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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},
|
||||
|
||||
|
||||
22
util.go
22
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:
|
||||
|
||||
Reference in New Issue
Block a user