canonical / pebble

Pebble is a lightweight Linux service manager with layered configuration and an HTTP API.
https://canonical-pebble.readthedocs-hosted.com/
GNU General Public License v3.0
145 stars 54 forks source link

feat: unified osutil.Mkdir API with options #418

Closed IronCore864 closed 4 months ago

IronCore864 commented 4 months ago

Implement a unified osutil.Mkdir API, refactor existing osutil.MkdirChown and osutil.MkdirAllChown.

Closes #372.

Background

There is an issue (372) where umask settings would affect the permissions of the newly created directories.

There was originally a PR to implement the feature (see here), but the code became even more complicated. As per our discussion (see here), we decide to refactor Mkdir and implement the required feature.

The original PR was closed, using this one instead. This PR only refactors the existing code, the support for chmod will come in a separate PR.

Spec

See here.

API

func Mkdir(path string, perm os.FileMode, options *MkdirOptions) error { ... }

Options

type MkdirOptions struct {
    // If false (default), a missing parent raises an error.
    // If true, any missing parents of this path are created as needed.
    MakeParents bool

    // If false (default), an error is raised if the target directory already exists. In case MakeParents is true but ExistOK is false, an error won't be raised if the parent directory already exists but the target directory doesn't.
    // If true, an error won't be raised unless the given path already exists in the file system and isn't a directory (same behaviour as the POSIX mkdir -p command).
    ExistOK bool

    // If false (default), no explicit chmod is performed. In this case, the permission of the created directories will be affected by umask settings.
    // If true, perform an explicit chmod on any directories created.
    Chmod bool

    // If false (default), no explicit chmod is performed. In this case, the permission of the created directories will be affected by umask settings.
    // If true, perform an explicit chmod on any directories created.
    Chown bool // NOTE: additional bool is needed because 0 is a valid value for UserID and GroupID (root)

    UserID sys.UserID

    GroupID sys.GroupID
}

Manual Tests

Tests Before After
mkdir ~/normal 755, same user/group as pebble, exist not ok same behaviour
mkdir -p ~/nested/folder; mkdir -p ~/nested1 755, parent/child both the same user/group as pebble, exist ok same behaviour
mkdir ~/normal-chown --user test --group test 755, user/group changed exist not ok, error: rename /root/normal-chown.mkdir-new /root/normal-chown: file exists same behaviour, better error msg: error: mkdir /root/normal-chown: file exists
mkdir -p ~/nested1/folder-chown --user test --group test 755, parent/child user/group both changed, exist OK same behaviour
IronCore864 commented 4 months ago

Manual test results:

Tests Before After
mkdir ~/normal 755, same user/group as pebble, exist not ok same behaviour
mkdir -p ~/nested/folder; mkdir -p ~/nested1 755, parent/child both the same user/group as pebble, exist ok same behaviour
mkdir ~/normal-chown --user test --group test 755, user/group changed exist not ok, error: rename /root/normal-chown.mkdir-new /root/normal-chown: file exists same behaviour, better error msg: error: mkdir /root/normal-chown: file exists
mkdir -p ~/nested1/folder-chown --user test --group test 755, parent/child user/group both changed, exist OK same behaviour