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 incorrect file permissions in file.line #30168

Merged
merged 1 commit into from Jan 6, 2016

Conversation

Projects
None yet
3 participants
@abednarik
Contributor

abednarik commented Jan 6, 2016

Looks like atomic_open() doesn't take care of preserving file permissions. It will create
a new file, edit it and then move this temp file to the original file ignoring file permissions.
Also updated the check to be sure file exist and is a file. Otherwise code will try to update
directory content and will fail badly and throw a python error.

This was tested on Linux, sorry but I don't have any Windows to try it out.
Fixed #30150

Fix incorrect file permissions in file.line
Looks like atomic_open() doesn't take care of preserving file permissions.  It will create
a new file, edit it and then move this temp file to the original file ignoring file permissions.
Also updated the check to be sure file exist and is a file. Otherwise code will try to update
directory content and will fail badly and throw a python error.
@basepi

This comment has been minimized.

Show comment
Hide comment
@basepi

basepi Jan 6, 2016

Collaborator

This looks great! @twangboy if this function is imported into win_file.py, you might want to double check that this still works.

Collaborator

basepi commented Jan 6, 2016

This looks great! @twangboy if this function is imported into win_file.py, you might want to double check that this still works.

basepi added a commit that referenced this pull request Jan 6, 2016

Merge pull request #30168 from abednarik/2015.8
Fix incorrect file permissions in file.line

@basepi basepi merged commit e5d87a0 into saltstack:2015.8 Jan 6, 2016

2 of 5 checks passed

jenkins/salt-pr-linode-ubuntu14.04-n Salt PR - Linode Ubuntu 14.04 #3959 — FAILURE
Details
jenkins/salt-pr-rs-cent7-n Salt PR - RS CentOS 7 #11101 — FAILURE
Details
default Merged build started.
Details
jenkins/salt-pr-clone Salt PR - Clone Repository #12514 — SUCCESS
Details
jenkins/salt-pr-lint-n Salt PR - Code Lint #12213 — SUCCESS
Details
@abednarik

This comment has been minimized.

Show comment
Hide comment
@abednarik

abednarik Jan 6, 2016

Contributor

Ok, let mee see what I can do @basepi

Contributor

abednarik commented Jan 6, 2016

Ok, let mee see what I can do @basepi

@abednarik abednarik deleted the abednarik:2015.8 branch Jan 6, 2016

@basepi

This comment has been minimized.

Show comment
Hide comment
@basepi

basepi Jan 6, 2016

Collaborator

@abednarik No worries, @twangboy is our resident windows guy so if you don't have access to windows for testing it's not a huge deal. :)

Collaborator

basepi commented Jan 6, 2016

@abednarik No worries, @twangboy is our resident windows guy so if you don't have access to windows for testing it's not a huge deal. :)

@abednarik

This comment has been minimized.

Show comment
Hide comment
@abednarik

abednarik Jan 6, 2016

Contributor

Great! Anyway, if there is something else I can do, just let me know.

Cheers.

Contributor

abednarik commented Jan 6, 2016

Great! Anyway, if there is something else I can do, just let me know.

Cheers.

@cachedout

This comment has been minimized.

Show comment
Hide comment
@cachedout

cachedout Jan 7, 2016

Contributor

This fix has broken the job cache on the salt master as outlined in #30204. I am reverting it. I'll re-open the associated issue as well.

Contributor

cachedout commented Jan 7, 2016

This fix has broken the job cache on the salt master as outlined in #30204. I am reverting it. I'll re-open the associated issue as well.

@abednarik

This comment has been minimized.

Show comment
Hide comment
@abednarik

abednarik Jan 7, 2016

Contributor

My bad, sorry about that.

2016-01-07 14:54 GMT-03:00 Mike Place notifications@github.com:

This fix has broken the job cache on the salt master as outlined in #30204
#30204. I am reverting it. I'll
re-open the associated issue as well.


Reply to this email directly or view it on GitHub
#30168 (comment).

Contributor

abednarik commented Jan 7, 2016

My bad, sorry about that.

2016-01-07 14:54 GMT-03:00 Mike Place notifications@github.com:

This fix has broken the job cache on the salt master as outlined in #30204
#30204. I am reverting it. I'll
re-open the associated issue as well.


Reply to this email directly or view it on GitHub
#30168 (comment).

@cachedout

This comment has been minimized.

Show comment
Hide comment
@cachedout

cachedout Jan 7, 2016

Contributor

No worries at all. Thanks, @abednarik

Contributor

cachedout commented Jan 7, 2016

No worries at all. Thanks, @abednarik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment