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

Check valid username first (fixes failing symlink test) #53467

Merged
merged 11 commits into from Jun 19, 2019

Conversation

@twangboy
Copy link
Contributor

commented Jun 12, 2019

What does this PR do?

The failing symlink test had to do with two tests in unit.modules.test_cmdmod that were testing runas on Windows. For some reason, this was causing the symlink test to fail later on. It was getting the system token, then checking to see if the username was valid before returning a CommandExecutionError. I moved the username validation to be the first thing the function does before grabbing the system token. This fixed the failing symlink test.

Fixes log/setup.py so that it doesn't bork if __salt_system_encoding__ is undefined
Fixes modules/file.py _add_content function to pass the correct parameters to the check_perms function when on Windows
Removes some extra error messages that were being used for troubleshooting in the modules/win_file.py symlink function
Improves the error messages displayed by functions in platform/win.py
Fixes states/file.py copy_ function to pass the correct parameters to the check_perms function when on Windows
Fixes the __virtual__ function on states/zfs.py and states/zpool.py when the zfs_support grain doesn't exist (as it doesn't on Windows)
Wraps some code in the runas and runas_unprov functions in utils/win_runas.py salt util in a try/finally block to make sure handles are being closed if an error is encountered.
Fixes some issues with integration.states.test_file when trying to remove files that don't exist. Also uses os.mkdir on Windows as the mode is just ignored. Additionally, uses more descriptive assert statements.
Fixes more issues with unit.modules.test_file for handling the fact that win_file.py namespaces many functions from the file.py module. So this patches those tests to run the correct check_file function. Also skips 2 Linux specific tests on Windows.

What issues does this PR fix or reference?

This is the rest of #53437

Tests written?

Fixes tests

Commits signed with GPG?

Yes

@dwoz dwoz added the 2019.2.1 label Jun 13, 2019
@dwoz
dwoz approved these changes Jun 13, 2019
@twangboy twangboy added Neon 2019.2.1 and removed 2019.2.1 labels Jun 14, 2019
@Akm0d
Akm0d approved these changes Jun 14, 2019
twangboy added 8 commits Jun 12, 2019
modules/file.py:     Pass the right parameters to `check_perms` on Windows
platform/win.py:     Return actual error on runas failure
platform/win.py:     Setup environment for CreateProcessWithLogonW
states/file.py:      Pass the right parameters to `file.check_perms` on
                     Windows
states/zfs.py:       Fix __virtual__ so it doesn't stacktrace in Windows
                     logs
states/zpool.py:     Fix __virtual__ so it doesn't stacktrace in Windows
                     logs
utils/win_runas.py:  Cleanup the handles a little better by using some
                     try/finally blocks.
states/test_file.py: Just use os.mkdir on Windows as the mode is ignored
I wasn't running sudo, but now it tells you that
And verifies that the user is added and the state is run successfully
before continuing with the test
Many of the functions from the `filemod` module are "namespaced" into
the `win_file` module. That's the file module that gets used on Windows,
so we need to make sure the `check_perms` function from the `win_file`
module is used on Windows.
@twangboy twangboy force-pushed the twangboy:fix_test_win_file branch from bc64dac to 6de4db6 Jun 17, 2019
tests/unit/modules/test_file.py Outdated Show resolved Hide resolved
twangboy and others added 2 commits Jun 18, 2019
@codecov

This comment has been minimized.

Copy link

commented Jun 19, 2019

Codecov Report

Merging #53467 into 2019.2.1 will increase coverage by 25.64%.
The diff coverage is 8%.

Impacted file tree graph

@@              Coverage Diff              @@
##           2019.2.1   #53467       +/-   ##
=============================================
+ Coverage     11.45%    37.1%   +25.64%     
=============================================
  Files          1520     1577       +57     
  Lines        252467   269609    +17142     
  Branches      53835    57450     +3615     
=============================================
+ Hits          28922   100028    +71106     
+ Misses       221580   158352    -63228     
- Partials       1965    11229     +9264
Flag Coverage Δ
#arch ?
#centos7 ?
#debian8 35.63% <8%> (?)
#debian9 36.45% <8%> (?)
#proxy ?
#py2 36.84% <8%> (+31.95%) ⬆️
#py3 35.76% <8%> (+24.56%) ⬆️
#ubuntu1604 36.8% <8%> (+31.82%) ⬆️
#ubuntu1804 36.73% <8%> (?)
Impacted Files Coverage Δ
salt/modules/win_file.py 17.09% <ø> (+0.46%) ⬆️
salt/utils/win_runas.py 13.86% <0%> (+13.86%) ⬆️
salt/platform/win.py 0% <0%> (ø) ⬆️
salt/states/zpool.py 70.45% <0%> (+70.45%) ⬆️
salt/states/file.py 55.63% <33.33%> (+50.43%) ⬆️
salt/modules/file.py 55.22% <33.33%> (+44.34%) ⬆️
salt/states/zfs.py 45.67% <33.33%> (+45.67%) ⬆️
salt/log/setup.py 62.03% <40%> (+7.96%) ⬆️
salt/proxy/dummy.py 0% <0%> (-80.87%) ⬇️
salt/proxy/docker.py 0% <0%> (-62.5%) ⬇️
... and 1203 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f72ebba...92950c5. Read the comment docs.

@dwoz dwoz merged commit fd6cb35 into saltstack:2019.2.1 Jun 19, 2019
15 of 17 checks passed
15 of 17 checks passed
jenkins/pr/py3-ubuntu-1804 The py3-ubuntu-1804 job has failed
Details
jenkins/pr/py3-windows-2016 The py3-windows-2016 job has failed
Details
WIP Ready for review
Details
continuous-integration/jenkins/pr-head 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-6 The py2-centos-6 job has passed
Details
jenkins/pr/py2-centos-7 The py2-centos-7 job has passed
Details
jenkins/pr/py2-debian-8 The py2-debian-8 job has passed
Details
jenkins/pr/py2-debian-9 The py2-debian-9 job has passed
Details
jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has passed
Details
jenkins/pr/py2-ubuntu-1804 The py2-ubuntu-1804 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-debian-8 The py3-debian-8 job has passed
Details
jenkins/pr/py3-debian-9 The py3-debian-9 job has passed
Details
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has passed
Details
@twangboy twangboy deleted the twangboy:fix_test_win_file branch Jul 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.