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

Altering validation of windows directory ownership to compare SID instead of username #50887

Merged
merged 5 commits into from Feb 14, 2019

Conversation

@jalandis
Copy link
Contributor

commented Dec 17, 2018

What does this PR do?

Alters logic checking Windows directory ownership to avoid unnecessary updates.

What issues does this PR fix or reference?

Fixes #50880

Previous Behavior

Windows folder ownership was tested by username with username and domain\username not matching properly.

New Behavior

Windows folder ownership is tested by more uniform SID.

Tests written?

Yes

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@jalandis

This comment has been minimized.

Copy link
Contributor Author

commented Jan 6, 2019

In order to facilitate getting this pull request merged in, I am going to work on adding an integration test tests/integration/states/test_file.py.

If there are any other changes that you would like to recommend, please let me know.

…tead of username
@jalandis jalandis force-pushed the jalandis:windows-directory-test-owner-by-sid branch from 1335775 to 4b50cdd Jan 6, 2019
jalandis and others added 3 commits Jan 6, 2019
@zbarr

This comment has been minimized.

Copy link

commented Feb 11, 2019

What is the status on this? I have just upgraded all of my minions to 2018.3.3 and am seeing this issue consistently. I would like to patch it if I could see the code change.

@twangboy

This comment has been minimized.

Copy link
Contributor

commented Feb 11, 2019

I'd like to see this on the older branches as well... 2018.3 and forward...

@zbarr

This comment has been minimized.

Copy link

commented Feb 12, 2019

I made a bad patch in my environment so I am now aware it is not particularly easy to backport to 2018.3.3. I am personally just interested in seeing this in the 2018.3.4 release now and we are just going to deal with the bug until then. I will continue to watch for a merge. Thank you to whoever has contributed.

twangboy added a commit to twangboy/salt that referenced this pull request Feb 14, 2019
@twangboy twangboy referenced this pull request Feb 14, 2019
dwoz added a commit that referenced this pull request Feb 14, 2019
backport #50887
@dwoz dwoz merged commit edb607f into saltstack:develop Feb 14, 2019
10 checks passed
10 checks passed
WIP Ready for review
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
jenkins/pr/docs The docs job has passed
Details
jenkins/pr/lint Python lint test has passed
Details
jenkins/pr/py2-centos-7 The py2-centos-7 job has passed
Details
jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has passed
Details
jenkins/pr/py2-windows-2016 The py2-windows-2016 job has passed
Details
jenkins/pr/py3-centos-7 The py3-centos-7 job has passed
Details
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has passed
Details
jenkins/pr/py3-windows-2016 The py3-windows-2016 job has passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.