Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix copy module to reset filesystem acls #51868

Merged
merged 5 commits into from
Apr 3, 2019
Merged

Conversation

mkrizek
Copy link
Contributor

@mkrizek mkrizek commented Feb 7, 2019

SUMMARY

Alternative to #50419 and #51296

The controller's fixup_perms2 uses filesystem acls to make the temporary
file for copy readable by an unprivileged become user. On Python3, the
acls are then copied to the destination filename so we have to remove
them from there.

We can't remove them prior to the copy because we may not have
permission to read the file if the acls are not present. We can't
remove them in atomic_move() because the move function shouldn't know
anything about controller features. We may want to generalize this into
a helper function, though.

Fixes #44412

Co-authored-by: Toshio Kuratomi a.badger@gmail.com

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

copy
assemble
template

ADDITIONAL INFORMATION

The below action plugins use _transfer_file and fixup_perms2() too. However, the files that are transferred to the remote system are only kept in ansible temp dir and removed after they are used. Therefore it is not necessary to remove ACLs in those cases.

lib/ansible/plugins/action/patch.py
lib/ansible/plugins/action/script.py
lib/ansible/plugins/action/unarchive.py
lib/ansible/plugins/action/uri.py

@ansibot
Copy link
Contributor

ansibot commented Feb 7, 2019

cc @ptux
click here for bot help

@ansibot ansibot added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. affects_2.8 This issue/PR affects Ansible v2.8 bug This issue/PR relates to a bug. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. python3 support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Feb 7, 2019
@mkrizek mkrizek force-pushed the copy-acls2 branch 3 times, most recently from d7b6538 to fbe63ef Compare February 11, 2019 11:02
@mkrizek mkrizek changed the title [WIP] Fix copy module to reset filesystem acls 2 Fix copy module to reset filesystem acls 2 Feb 11, 2019
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. labels Feb 11, 2019
@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label Feb 12, 2019
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Feb 20, 2019
@mkrizek mkrizek requested a review from abadger March 4, 2019 19:29
@ansibot ansibot added the files Files category label Mar 4, 2019
@mkrizek mkrizek changed the title Fix copy module to reset filesystem acls 2 Fix copy module to reset filesystem acls Mar 22, 2019
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. core_review In order to be merged, this PR must follow the core review workflow. and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. core_review In order to be merged, this PR must follow the core review workflow. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Mar 22, 2019
@mkrizek mkrizek requested a review from bcoca March 28, 2019 09:53
@ansibot ansibot removed the core_review In order to be merged, this PR must follow the core review workflow. label Mar 28, 2019
@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Mar 28, 2019
mkrizek and others added 2 commits April 1, 2019 11:57
The controller's fixup_perms2 uses filesystem acls to make the temporary
file for copy readable by an unprivileged become user. On Python3, the
acls are then copied to the destination filename so we have to remove
them from there.

We can't remove them prior to the copy because we may not have
permission to read the file if the acls are not present. We can't
remove them in atomic_move() because the move function shouldn't know
anything about controller features. We may want to generalize this into
a helper function, though.

Fixes ansible#44412

Co-authored-by: Toshio Kuratomi <a.badger@gmail.com>
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Apr 3, 2019
@mkrizek mkrizek merged commit d15812f into ansible:devel Apr 3, 2019
@mkrizek mkrizek deleted the copy-acls2 branch April 3, 2019 16:38
mkrizek added a commit to mkrizek/ansible that referenced this pull request Apr 3, 2019
The controller's fixup_perms2 uses filesystem acls to make the temporary
file for copy readable by an unprivileged become user. On Python3, the
acls are then copied to the destination filename so we have to remove
them from there.

We can't remove them prior to the copy because we may not have
permission to read the file if the acls are not present. We can't
remove them in atomic_move() because the move function shouldn't know
anything about controller features. We may want to generalize this into
a helper function, though.

Fixes ansible#44412

Co-authored-by: Toshio Kuratomi <a.badger@gmail.com>
(cherry picked from commit d15812f)
mkrizek added a commit to mkrizek/ansible that referenced this pull request Apr 3, 2019
The controller's fixup_perms2 uses filesystem acls to make the temporary
file for copy readable by an unprivileged become user. On Python3, the
acls are then copied to the destination filename so we have to remove
them from there.

We can't remove them prior to the copy because we may not have
permission to read the file if the acls are not present. We can't
remove them in atomic_move() because the move function shouldn't know
anything about controller features. We may want to generalize this into
a helper function, though.

Fixes ansible#44412

Co-authored-by: Toshio Kuratomi <a.badger@gmail.com>
(cherry picked from commit d15812f)
mkrizek added a commit to mkrizek/ansible that referenced this pull request Apr 3, 2019
The controller's fixup_perms2 uses filesystem acls to make the temporary
file for copy readable by an unprivileged become user. On Python3, the
acls are then copied to the destination filename so we have to remove
them from there.

We can't remove them prior to the copy because we may not have
permission to read the file if the acls are not present. We can't
remove them in atomic_move() because the move function shouldn't know
anything about controller features. We may want to generalize this into
a helper function, though.

Fixes ansible#44412

Co-authored-by: Toshio Kuratomi <a.badger@gmail.com>
(cherry picked from commit d15812f)
abadger pushed a commit that referenced this pull request Apr 4, 2019
The controller's fixup_perms2 uses filesystem acls to make the temporary
file for copy readable by an unprivileged become user. On Python3, the
acls are then copied to the destination filename so we have to remove
them from there.

We can't remove them prior to the copy because we may not have
permission to read the file if the acls are not present. We can't
remove them in atomic_move() because the move function shouldn't know
anything about controller features. We may want to generalize this into
a helper function, though.

Fixes #44412

Co-authored-by: Toshio Kuratomi <a.badger@gmail.com>
(cherry picked from commit d15812f)
abadger pushed a commit that referenced this pull request Apr 4, 2019
* Fix copy module to reset filesystem acls (#51868)

The controller's fixup_perms2 uses filesystem acls to make the temporary
file for copy readable by an unprivileged become user. On Python3, the
acls are then copied to the destination filename so we have to remove
them from there.

We can't remove them prior to the copy because we may not have
permission to read the file if the acls are not present. We can't
remove them in atomic_move() because the move function shouldn't know
anything about controller features. We may want to generalize this into
a helper function, though.

Fixes #44412

Co-authored-by: Toshio Kuratomi <a.badger@gmail.com>
(cherry picked from commit d15812f)

* Fix get_bin_path usage
@ansible ansible locked and limited conversation to collaborators Jul 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.8 This issue/PR affects Ansible v2.8 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. files Files category module This issue/PR relates to a module. python3 support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ansible applies unwanted ACL rules
4 participants