Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make write/push and make-dir chown intermediate dirs when setting user #130

Merged
merged 3 commits into from Jul 29, 2022

Conversation

benhoyt
Copy link
Contributor

@benhoyt benhoyt commented Jul 28, 2022

Per user report, write/push and make-dir are not setting the user/group
on intermediate directories when the user and group is set. This can
lead to the file being unreachable when Pebble is running a workload
with a specific user:group.

Fix these API calls to chown the intermediate directories as needed.
Use the existing osutil.MkdirAllChown where appropriate.

Add tests for the above, including integration tests that require root.

Fixes #124

Per user report, write/push and make-dir are not setting the user/group
on intermediate directories when the user and group is set. This can
lead to the file being unreachable when Pebble is running a workload
with a specific user:group.

Fix these API calls to chown the intermediate directories as needed.
Use the existing osutil.MkdirAllChown where appropriate.

Add tests for the above, including integration tests that require root.

Fixes canonical#124
internal/daemon/api_files.go Outdated Show resolved Hide resolved
Per review feedback, this is so we can reuse the MkdirAllChown mutex
and the create/rename approach.
internal/daemon/api_files.go Outdated Show resolved Hide resolved
internal/daemon/api_files.go Outdated Show resolved Hide resolved
internal/osutil/mkdirallchown.go Outdated Show resolved Hide resolved
internal/osutil/mkdirallchown.go Show resolved Hide resolved
@benhoyt benhoyt merged commit a755cf2 into canonical:master Jul 29, 2022
@benhoyt benhoyt deleted the fix-mkdir-user branch July 29, 2022 01:32
benhoyt added a commit to benhoyt/juju that referenced this pull request Jul 29, 2022
This picks up three recent changes:

* Explicitly set HOME and USER env vars if service user+group set:
  canonical/pebble#129:
* Make write/push and make-dir chown intermediate dirs when setting
  user: canonical/pebble#130
* Make exec checks consistently not inherit parent environment
  variables: canonical/pebble#131
jujubot added a commit to juju/juju that referenced this pull request Jul 29, 2022
#14393

This picks up three recent bug fixes (the first two were reported by users):

* canonical/pebble#129:
* canonical/pebble#130
* canonical/pebble#131

## Checklist

- [ ] ~Code style: imports ordered, good names, simple structure, etc~
- [ ] ~Comments saying why design decisions were made~
- [ ] ~Go unit tests, with comments saying what you're testing~
- [ ] ~[Integration tests](https://github.com/juju/juju/tree/develop/tests), with comments saying what you're testing~
- [ ] ~[doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~

## QA steps

```sh
$ juju bootstrap microk8s
$ juju deploy snappass-test
$ juju status
# browse to http://<ip address>:5000 to ensure it's working
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Container.push() won't use passed user:group to create directories.
2 participants