Conditionally set ownership and permissions in entrypoint

[kocolosk]

Overview

Three significant changes here:

1. Check ownership/permissions and only execute chown/chmod when necessary
2. Switch target permissions to those set by CouchDB itself
3. Drop the -f flag and the silent || true guard on these commands

The last one is perhaps worth discussion. I couldn't find any explanation for why it was added in the first place, and I didn't see it being used on comparable docker-entrypoint files in other projects. I figured that if we feel these commands are important enough to place in the entry point we probably don't want them to be silently ignored.

GitHub issue number

Fixes apache/couchdb-documentation#109

Checklist

• Code is written and works correctly;
• Changes are covered by tests;
• Documentation reflects the changes;

[wohali]

@kocolosk see #73 (comment) for apache/couchdb-documentation#3. I think we can't drop this.

[wohali]

Probably can't drop the -f

[kocolosk]

Oh geez we already had this conversation back in apache/couchdb-documentation#80 as well didn't we? I will add the -f back in for the config stuff.

[wohali]

Needs a -f

[kocolosk]

So ... I want to test that one a bit. I've already scoped down the chown so it only executes on files that have incorrect ownership. I'm struggling to think of a case where we have files in /opt/couchdb that are not owned by couchdb and yet we're OK to proceed without that ownership change. I guess the best example would be a config file with 0644 permissions owned by root coming in from a volume mount?

[wohali]

Right, the big issues are config and data files owned by the wrong person. We could conceivably skip changing their ownership as long as we have read/write access....

Another option is to have Couch itself complain when it sees config files it can't read/write, skip all the checks here, and let Couch crash out. We don't do that yet.

It'd be harder to do that for data files since generally we don't start opening db/view files until asked to do so by the user.

[wohali]

Should do -f for L37-38

[wohali]

Missed a couple - once those are fixed this is good to go.

Hopefully all the stat(2) calls that find(1) imposes won't continue to aggravate the problem the Hyperledger people saw over in apache/couchdb#4211.

[kocolosk]

OK after sleeping on it I went ahead and applied the -f flag everywhere and rebased things into a clean set.

I also tried a smoke test on whether this is actually helping by commenting out all of this code in the entry point and then timing the two chown options manually inside a container running on my laptop:

root@002a9518bc6f:/opt/couchdb# time find /opt/couchdb \! \( -user couchdb -group couchdb \) -exec chown -f couchdb:couchdb '{}' +

real    0m0.015s
user    0m0.000s
sys     0m0.000s
root@002a9518bc6f:/opt/couchdb# time chown -R couchdb:couchdb /opt/couchdb

real    0m3.992s
user    0m0.000s
sys     0m0.370s

Seems like a good idea 😄 There are about 1600 files in a vanilla CouchDB install so anyone with lots of (correctly-permissioned) databases would likely see an even bigger improvement.

[wohali]

+1 and thanks for the test.