Set correct permissions on /var/lib/pulp/ if it pre-exists #178

dralley · 2019-10-15T13:15:00Z

closes #5553
https://pulp.plan.io/issues/5553

bmbouter · 2019-10-15T14:57:41Z

roles/pulp/tasks/install.yml

@@ -94,12 +94,10 @@
    - name: Reset ssh conn to allow user changes to affect when ssh user and pulp user are the same
      meta: reset_connection

-    - name: Create cache dir for Pulp
+    - name: Set permissions on {{ pulp_user_home }}


Do we want to make the user's perms other user readable?

https://pulp.plan.io/issues/5553

They are already, at least, on a fresh Pulp 3 install. 775 "vagrant" "pulp" across the board.

The present issue is that if /var/lib/pulp/ is already present (because of an existing Pulp 2 install) then the whole directory is owned by "apache" with group "pulp" but due to the lack of group write permissions (755) Pulp 3 can't write to /var/lib/pulp/

With Pulp 2, what are the permissions?

It is a security practice to not relax permissions during software upgrades.

Unlike "others", the group might be an example where you can make an exception and relax permissions, if the group generally doesn't have arbitrary users on it.

See Tanya's comment on the issue -- that's what the perms are

roles/pulp/tasks/install.yml

mikedep333 · 2019-10-15T15:26:39Z

roles/pulp/tasks/install.yml

+        group: '{{ pulp_group }}'
+        mode: 0775
+
+    - name: Set permissions on {{ pulp_user_home }}/static/


This creates the directory when it does not exist, and sets those permissions only. It probably creates them as root:root, and that's probably why this is failing.

If you want to modify its permissions only when it exists, you'll need to use a stat module to check if it exists (and is a directory), register the result, and then run this task with a "when" condition. Follow that link and search the page for "p.stat.isdir" for a good example).

mikedep333 · 2019-10-15T15:27:53Z

roles/pulp/tasks/install.yml

        state: directory
-        owner: '{{ pulp_user }}'
-        group: '{{ pulp_group }}'
        mode: 0775


You're not setting the owner/group on purpose, correct?

mikedep333 · 2019-10-15T16:12:57Z

@dralley Also, we have this for molecule: https://github.com/pulp/ansible-pulp/blob/master/molecule/source/prepare.yml#L36

dralley · 2019-10-15T19:50:25Z

Re: the molecule solution: I can do that if you think it would be better, I was just trying to keep the permissions minimal.

edit: of course, molecule doesn't like my solution due to imdempotence...

mikedep333

Looks good

mikedep333 · 2019-10-15T20:27:16Z

I approved it, but I would like to see more comments above these tasks about why we have them. Either a reasonably detailed explanation, or a very short one and then a reference to the ticket URL.

dralley · 2019-10-21T11:50:50Z

@mikedep333 Still having idempotence issues :(

ERROR: Idempotence test failed because of the following tasks:

* [fedora-30] => pulp : Check if user exists

* [debian-10] => pulp : Check if user exists

* [centos-7] => pulp : Check if user exists

* [fedora-30] => pulp : Set permissions on '/var/lib/pulp'

* [centos-7] => pulp : Set permissions on '/var/lib/pulp'

* [debian-10] => pulp : Set permissions on '/var/lib/pulp'

mikedep333 · 2019-11-14T21:25:26Z

@dralley Do you need help with this?

dralley · 2019-12-02T17:39:12Z

@mikedep333 I've not looked at it in a month but to be honest, probably I do yes.

mikedep333 · 2019-12-02T20:41:15Z

@dralley I have some open-ended questions about what permissions are needed under the 2 scenarios. Let's do a video call when you have a chance, and then let's summarize it here.

closes #5553 https://pulp.plan.io/issues/5553

dralley · 2019-12-09T02:18:45Z

On my machine, this seems to work. Pulp 2 and Pulp 3 sync correctly (well, without error at least) and of course vagrant up works. Not sure if there are still other problems as we discussed the other day though, I didn't test super thoroughly.

This seems fixed: https://pulp.plan.io/issues/5742

roles/pulp/tasks/install.yml

mikedep333 · 2019-12-04T19:32:40Z

roles/pulp/tasks/install.yml

+            state: directory
+            owner: '{{ pulp_user }}'
+            group: '{{ pulp_group }}'
+            mode: 0775


This may resolve the idempotence failure:
https://serverfault.com/a/104850
If it doesn't, it means that files are being created under it with different permissions. In this case, your needed logic would be more complex. Either setting umask for pulp processes, or having different logic for different files.

roles/pulp/tasks/install.yml

bmbouter

This looks good to me. I see the code setting the group to the pulp3 expected group, adding apache to it, and setting setgid so new files from Pulp2 get the correct group also. If this is all correct please merge.

Thanks @mikedep333 and @dralley !

dralley force-pushed the master branch 4 times, most recently from 1b5d3eb to 45584da Compare October 15, 2019 14:47

bmbouter reviewed Oct 15, 2019

View reviewed changes

mikedep333 reviewed Oct 15, 2019

View reviewed changes

roles/pulp/tasks/install.yml Outdated Show resolved Hide resolved

mikedep333 reviewed Oct 15, 2019

View reviewed changes

dralley force-pushed the master branch 4 times, most recently from 4fb45b2 to d2742ab Compare October 15, 2019 17:02

mikedep333 approved these changes Oct 15, 2019

View reviewed changes

dralley force-pushed the master branch 14 times, most recently from 6512744 to b004209 Compare October 21, 2019 11:26

dralley force-pushed the master branch 2 times, most recently from b004209 to 83fabd9 Compare October 28, 2019 16:09

evgeni mentioned this pull request Nov 13, 2019

allow installing pulp from packages #188

Closed

mikedep333 closed this Dec 6, 2019

mikedep333 force-pushed the master branch from 83fabd9 to c8d0f5c Compare December 6, 2019 20:30

mikedep333 reopened this Dec 6, 2019

Set correct permissions on /var/lib/pulp/ if it pre-exists

522f2e7

closes #5553 https://pulp.plan.io/issues/5553

mikedep333 force-pushed the master branch from 51149c6 to 522f2e7 Compare December 6, 2019 21:23

bmbouter reviewed Dec 9, 2019

View reviewed changes

roles/pulp/tasks/install.yml Show resolved Hide resolved

mikedep333 reviewed Dec 17, 2019

View reviewed changes

bmbouter approved these changes Dec 18, 2019

View reviewed changes

mikedep333 merged commit 5eae817 into pulp:master Dec 18, 2019

Set correct permissions on /var/lib/pulp/ if it pre-exists #178

Set correct permissions on /var/lib/pulp/ if it pre-exists #178

dralley commented Oct 15, 2019

bmbouter Oct 15, 2019

dralley Oct 15, 2019 •

edited

mikedep333 Oct 16, 2019

dralley Oct 16, 2019

mikedep333 Oct 15, 2019

mikedep333 Oct 15, 2019

mikedep333 commented Oct 15, 2019

dralley commented Oct 15, 2019 •

edited

mikedep333 left a comment

mikedep333 commented Oct 15, 2019

dralley commented Oct 21, 2019

mikedep333 commented Nov 14, 2019

dralley commented Dec 2, 2019

mikedep333 commented Dec 2, 2019

dralley commented Dec 9, 2019 •

edited

mikedep333 Dec 4, 2019

bmbouter left a comment

Set correct permissions on /var/lib/pulp/ if it pre-exists #178

Set correct permissions on /var/lib/pulp/ if it pre-exists #178

Conversation

dralley commented Oct 15, 2019

bmbouter Oct 15, 2019

Choose a reason for hiding this comment

dralley Oct 15, 2019 • edited

Choose a reason for hiding this comment

mikedep333 Oct 16, 2019

Choose a reason for hiding this comment

dralley Oct 16, 2019

Choose a reason for hiding this comment

mikedep333 Oct 15, 2019

Choose a reason for hiding this comment

mikedep333 Oct 15, 2019

Choose a reason for hiding this comment

mikedep333 commented Oct 15, 2019

dralley commented Oct 15, 2019 • edited

mikedep333 left a comment

Choose a reason for hiding this comment

mikedep333 commented Oct 15, 2019

dralley commented Oct 21, 2019

mikedep333 commented Nov 14, 2019

dralley commented Dec 2, 2019

mikedep333 commented Dec 2, 2019

dralley commented Dec 9, 2019 • edited

mikedep333 Dec 4, 2019

Choose a reason for hiding this comment

bmbouter left a comment

Choose a reason for hiding this comment

dralley Oct 15, 2019 •

edited

dralley commented Oct 15, 2019 •

edited

dralley commented Dec 9, 2019 •

edited