Bug 1810333: daemon: Always create tempfiles in target dir #1530
Conversation
openshift-ci-robot
added
the
size/XXL
|
Only compile tested. |
openshift-ci-robot
added
the
approved
|
The change to create in the same dir looks good. Didn't test it though |
|
Wait, sorry I just read the |
When we go to write a file, we need to create the temporary file in the exact target directory, not (potentially) `/tmp`. This will ensure that the right SELinux label is used by default. Currently the `renameio` library's logic tries to optimize things by using `/tmp` if possible, otherwise the target directory. And without SELinux that's a sane optimization. But we can't do it. Force using the target directory by passing it explicitly. Should fix a bug seen with the baremetal config which ended up with a `tmp_t` labeled file in `/etc`.
cgwalters
force-pushed
the
tmpfile-targetdir
branch
from
975da37
to
57ebb74
Compare
openshift-ci-robot
added
size/XS
cgwalters
changed the title
|
OK pushed a much simpler patch that doesn't pull another dep (we should probably teach |
|
Wow, that renameio logic is... interesting. Might be worth a comment about the SELinux context here, but either way: /approve |
There was a problem hiding this comment.
/lgtm
As noted I believe this is what's causing the previous issue: https://github.com/google/renameio/blob/master/tempfile.go#L23
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashcrow, celebdor, cgwalters, jlebon, yuqi-zhang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
flake |
|
/cherrypick release-4.3 |
|
@cgwalters: once the present PR merges, I will cherry-pick it on top of release-4.3 in a new PR and assign it to you. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/retitle Bug 1810333: daemon: Always create tempfiles in target dir |
openshift-ci-robot
changed the title
openshift-ci-robot
added
the
bugzilla/valid-bug
|
@cgwalters: This pull request references Bugzilla bug 1810333, which is valid. 3 validation(s) were run on this bug
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@cgwalters: All pull requests linked via external trackers have merged. Bugzilla bug 1810333 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@cgwalters: new pull request created: #1535 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/cherrypick release-4.4 |
|
@celebdor: new pull request created: #1536 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
When we go to write a file, we need to create the temporary
file in the exact target directory, not (potentially)
/tmp. This willensure that the right SELinux label is used by default.
Currently the
renameiolibrary's logic tries to optimize thingsby using
/tmpif possible, otherwise the target directory.And without SELinux that's a sane optimization. But we
can't do it.
Force using the target directory by passing it explicitly.
Should fix a bug seen with the baremetal config which
ended up with a
tmp_tlabeled file in/etc.