Change uploads directory ownership

canonical / wordpress-k8s-operator

wordpress-k8s-operator - charm repository.

Apache License 2.0

13 stars 7 forks source link

Change uploads directory ownership #154

Closed amandahla closed 1 year ago

amandahla commented 1 year ago

Applicable spec:

Overview

The uploads directory is mounted with permission only to the root user. This prevents the correct behavior of uploading files via WordPress. This PR fixes that by changing the ownership using MongoDB charm as reference.

Rationale

Allow uploads directory to be writable to Wordpress user as expected.

Juju Events Changes

N/A

Module Changes

N/A

Library Changes

N/A

Checklist

[X] The charm style guide was applied
[X] The contributing guide was applied
[X] The changes are compliant with ISD054 - Managing Charm Complexity
[X] The documentation is generated using src-docs
[X] The documentation for charmhub is updated.
[X] The PR is tagged with appropriate label (urgent, trivial, complex)

amandahla commented 1 year ago

Wouldn't it be simpler to do this on the rock?

The uploads is a mounted storage, so doing in the ROCK would only affect the directory before mounting the storage.

weiiwang01 commented 1 year ago

I am a little bit curious why this was not a problem before, anyway the pull request lgtm, thanks!

weiiwang01 commented 1 year ago

I am a little bit curious why this was not a problem before, anyway the pull request lgtm, thanks!

Did a little digging, and it seems like the hostpath-provisioner in microk8s is running with a umask of 0000

$ cat /proc/$(pidof /hostpath-provisioner)/status | grep -i umask
Umask:  0000

so the directory created for hostpath storage will have a permission of 777

$ ls -lah /var/snap/microk8s/common/default-storage/
total 140K
drwxrwxrwx  2 root   root   4.0K Oct 26 16:22 test-wordpress-wordpress-k8s-uploads-31e96c3e-wordpress-k8s-0-pvc-3b6a4639-3308-49c8-9284-ee9c05a702be

amandahla commented 1 year ago

I am a little bit curious why this was not a problem before, anyway the pull request lgtm, thanks!

Did a little digging, and it seems like the hostpath-provisioner in microk8s is running with a umask of 0000
$ cat /proc/$(pidof /hostpath-provisioner)/status | grep -i umask
Umask:  0000
so the directory created for hostpath storage will have a permission of 777
$ ls -lah /var/snap/microk8s/common/default-storage/
total 140K
drwxrwxrwx  2 root   root   4.0K Oct 26 16:22 test-wordpress-wordpress-k8s-uploads-31e96c3e-wordpress-k8s-0-pvc-3b6a4639-3308-49c8-9284-ee9c05a702be

Wow, great finding!

github-actions[bot] commented 1 year ago

Test coverage for def7b8df9fc1ac78da83182a97b5ff68b2c90a58

Name                Stmts   Miss Branch BrPart  Cover   Missing
---------------------------------------------------------------
src/charm.py          530     41    172     31    89%   189-192, 356-357, 557, 588, 594, 637, 672-673, 724-731, 736, 838->843, 842, 844, 849-850, 910, 928, 935, 1025, 1034, 1046, 1067, 1076, 1095, 1099, 1128, 1181, 1313, 1335, 1342->1344, 1387->exit, 1399, 1415, 1452, 1461-1462
src/cos.py             15      0      0      0   100%
src/exceptions.py      17      1      2      1    89%   41
src/types_.py          16      0      0      0   100%
---------------------------------------------------------------
TOTAL                 578     42    174     32    90%

Static code analysis report

Run started:2023-10-30 13:24:10.177692

Test results:
    No issues identified.

Code scanned:
    Total lines of code: 4294
    Total lines skipped (#nosec): 1
    Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
    Total issues (by severity):
        Undefined: 0
        Low: 0
        Medium: 0
        High: 0
    Total issues (by confidence):
        Undefined: 0
        Low: 0
        Medium: 0
        High: 0
Files skipped (0):