Do not force any selinux context on volumeDir #12942
Conversation
|
[test] lets see what I break... |
eparis
force-pushed
the
no-chcon-volume-dir
branch
from
ca6cde9
to
e370003
Compare
|
Also we are no longer using svirt_sandbox_file_t and svirt_lxc_net_t, we are now using container_t and container_file_t. Just to prove the point. |
openshift-bot
added
the
needs-rebase
eparis
force-pushed
the
no-chcon-volume-dir
branch
2 times, most recently
from
322d840
to
05302d7
Compare
openshift-bot
removed
the
needs-rebase
eparis
force-pushed
the
no-chcon-volume-dir
branch
from
05302d7
to
84baf89
Compare
|
@pmorie what do you think? |
| @@ -66,20 +66,6 @@ If this shows up in your build logs, restart docker and then resubmit a build: | |||
| $ sudo systemctl restart docker | |||
| $ oc start-build --from-build=<your build identifier> | |||
|
|
|||
| Another item seen stems from how OpenShift operates in a SELinux environment. The SELinux policy requires that host directories that are bind mounted have the svirt_sandbox_file_t label. Generally | |||
There was a problem hiding this comment.
I believe we need to keep this note and change the label to container_file_t
| @@ -108,10 +108,6 @@ ownership/permissions. | |||
| For volume permission problems please consult the Persistent Storage section | |||
| of the Administrator's Guide. | |||
|
|
|||
| In the case of SELinux this may be resolved on the node by running: | |||
There was a problem hiding this comment.
I think this should be okay to remove, unless for some reason we are expecting people having trouble with hostpath PVs (which they should never be using anyway) to look here for a solution
|
Actually, I am cool with this. [merge] |
|
removed the tag from sdodson. I'm scared this will break something and will tag it after this sprint is closed. |
|
ok, upcomingrelease'ing my bug that depends on this |
|
[merge] |
|
[merge] you stoopid flaky thing |
|
[merge] you flaky thing |
|
[merge] again. because? peanuts. |
|
It seems to pretty consistently fail on building the node unit test, do the tests just need to be updated? |
|
[merge][severity: bug] |
docker should be able to handle all of this correctly without the two problems in this patch. It hard codes svirt_sandbox_file_t, which is actually policy specific, even if basically everyone uses our reference policy. It means that things under that directory could (if they can break out of their mount namespace) see this directory. docker doesn't require this anymore. So remove this custom hack.
eparis
force-pushed
the
no-chcon-volume-dir
branch
from
84baf89
to
88e4c71
Compare
|
[merge][severity: bug] |
|
@eparis this requires a blocker merge tag. IIUC this is a blocker fix. please confirm. |
|
[merge][severity: blocker] |
1 similar comment
|
[merge][severity: blocker] |
|
[test] |
|
[merge][severity:blocker] |
|
test_pull_request_origin_extended_conformance_gce -- failed, aborting job. |
|
re[merge] |
|
re[test] |
|
Evaluated for origin test up to 88e4c71 |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/2626/) (Base Commit: 496129f) (PR Branch Commit: 88e4c71) |
|
[merge] last time. then by hand. |
|
Evaluated for origin merge up to 88e4c71 |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/1151/) (Base Commit: c7d9fb8) (PR Branch Commit: 88e4c71) (Extended Tests: blocker, bug) (Image: devenv-rhel7_6403) |
| @@ -1182,7 +1182,7 @@ func SetupHostPathVolumes(c kcoreclient.PersistentVolumeInterface, prefix, capac | |||
| return volumes, err | |||
| } | |||
| if _, err = exec.LookPath("chcon"); err == nil { | |||
| err := exec.Command("chcon", "-t", "svirt_sandbox_file_t", dir).Run() | |||
| err := exec.Command("chcon", "-t", "container_file_t", dir).Run() | |||
There was a problem hiding this comment.
/me shakes fist at @eparis
|
Just to cross reference, the fallout of this change is https://bugzilla.redhat.com/show_bug.cgi?id=1481617 and possibly also #15750. |
docker should be able to handle all of this correctly without the two
problems in this patch.
It hard codes svirt_sandbox_file_t, which is actually policy
specific, even if basically everyone uses our reference policy.
It means that things under that directory could (if they can break
out of their mount namespace) see this directory.
docker doesn't require this anymore. So remove this custom hack.
xref https://bugzilla.redhat.com/show_bug.cgi?id=1421738