Avoid errors when the process does not have enough permissions

[hhorak]

Sometimes there is not enough permissions to change attributes by fix-permissions. It was for example reported in sclorg/mariadb-container#32. This change should make these warnings disappeared.

[pkubatrh]

I wonder, is there a chance we might mask some future issues when suppressing warnings and errors while running fix-permissions? Nothing comes to my mind right now but in general it seems like a bad idea.

There is also #112 that might get influenced by this change.

How about making fix-permissions take the GID via an argument (would default to 0 so we do not have to change all existing calls) instead? That would get rid of the warnings while additionally making sure the process has the correct access permissions for the files.

[hhorak]

I wonder, is there a chance we might mask some future issues when suppressing warnings and errors while running fix-permissions? Nothing comes to my mind right now but in general it seems like a bad idea.

I share the general worry, but thinking more about it it looks to me like the only real issue that we might mask is when some directory we pass to the script does not exist. That's why there is a check if ! [ -e "$1" ] ; then.. But I agree that we never can be sure.

How about making fix-permissions take the GID via an argument (would default to 0 so we do not have to change all existing calls) instead? That would get rid of the warnings while additionally making sure the process has the correct access permissions for the files.

I don't think it would make all errors disappeared, because symlinks mentioned in #112 that point to root-owned files (correctly read-only) would still produce warnings/errors IMO.. the reason of not having enough permissions is different there.

[omron93]

Problem is that the user is not in root group. So it is not possible to chgrp to such group.

@hhorak For example for mongodb-container - adding usermod -a -G root mongodb in Dockerfile solves this issue.

[pkubatrh]

For example for mongodb-container - adding usermod -a -G root mongodb in Dockerfile solves this issue.

Is this something that we want to have/is expected of containers running in Openshift?

[omron93]

How about making fix-permissions take the GID via an argument (would default to 0 so we do not have to change all existing calls) instead? That would get rid of the warnings while additionally making sure the process has the correct access permissions for the files.

Copies of fix-permissins scripts in mongodb-container [1] and postgresql-container [2] also change owner to default image user. This is needed for running using "pure" docker where default group isn't root.
On the other hand changing owner makes sense only in Dockerfile running under root. So +1 taking arguments to fix-permissions.

[1] https://github.com/sclorg/mongodb-container/blob/master/3.2/root/usr/libexec/fix-permissions
[2] https://github.com/sclorg/postgresql-container/blob/master/9.5/root/usr/libexec/fix-permissions

I don't think it would make all errors disappeared, because symlinks mentioned in #112 that point to root-owned files (correctly read-only) would still produce warnings/errors IMO.. the reason of not having enough permissions is different there.

We should not change file attributes of similar files. Warning makes sense in this case IMPOV.

[omron93]

Is this something that we want to have/is expected of containers running in Openshift?

In OpenShift all containers are running under root GID and random UID. That is why all files should be accessible by root group. @bparees Is it right?

[bparees]

In OpenShift all containers are running under root GID and random UID. That is why all files should be accessible by root group. @bparees Is it right?

sort of. The assemble script actually runs as the default user for the image, and if the default user for the image has specifically assigned groups, then those are the groups they will have, not necessarily the root group.

But if the user is not listed in /etc/groups, then yes they get the root group.

[omron93]

But if the user is not listed in /etc/groups, then yes they get the root group.

@bparees So when running container in OpenShift with "random" UID gets always root GID, or not? Is it possible that "random" UIG will be already listed in /etc/passwd?

Is it problem to add default user to root group from OpenShift POV ?

[bparees]

@bparees So when running container in OpenShift with "random" UID gets always root GID, or not? Is it possible that "random" UIG will be already listed in /etc/passwd?

it's theoretically possible the user could be listed (and thus not have root group permissions) but pretty unlikely. the assigned/random uid range starts at like 10000 or something. i've never seen an image that defines user 10000 in their /etc/passwd and /etc/groups files.

Is it problem to add default user to root group from OpenShift POV ?

no i don't think that should be a problem (but again you don't need to add the default user explicitly as long as the default user isn't otherwise already listed. for most of our images we use the default user 1001, which is why this works ok for us).

[omron93]

no i don't think that should be a problem (but again you don't need to add the default user explicitly as long as the default user isn't otherwise already listed. for most of our images we use the default user 1001, which is why this works ok for us).

In database images default user is different. So without random UID it causes problems.

So solution it to also use 1001 user in database images or to add current default users to root group. @hhorak What to choose?

[hhorak]

I prefer to add the default users to the root group, since this issue does not seem worth changing the default user. So, it should remove the need to hide errors in case of chgrp calls, but there is still the issue with files owned by root (sclorg/s2i-python-container#178 (comment)) where we see error messages when symlinks point to them, which we still should mitigate somehow.

[hhorak]

A fixed solution with suggestions from #112 (comment) is available.

[omron93]

[test]

[pkubatrh]

Operation not permitted warnings still present in RHEL7 CI as it seems to be testing s2i-* images built upon the wrong version of s2i-base (the PR version is tagged as centos/s2i-base-rhel7 instead of the expected rhscl/s2i-base-rhel7).
Would like to get this fixed and run through the RHEL CI properly before merging.

[omron93]

Operation not permitted warnings still present in RHEL7 CI as it seems to be testing s2i-* images built upon the wrong version of s2i-base (the PR version is tagged as centos/s2i-base-rhel7 instead of the expected rhscl/s2i-base-rhel7).
Would like to get this fixed and run through the RHEL CI properly before merging.

You are right! Build scripts for this image does not support centos/ vs rhscl/ tagging... It used openshift/ namespace for both versions, but after #114 namespace is always centos/ instead of openshift/

I suggest to fix this by using https://github.com/sclorg/container-common-scripts (for example by accepting #121)

[pkubatrh]

@hhorak Please rebase to be able run tests including #122