#408 if condition around $pathQualifier

[aaronk1]

SUMMARY

If statement around $pathQualifier : in case the path parameter is a UNC path

Fixes #408

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

win_acl_inheritance

ADDITIONAL INFORMATION

Before

New-PSDrive : Cannot bind argument to parameter 'Name' because it is null.
At line:12 char:31
+     $null = New-PSDrive -Name $pathQualifier -PSProvider 'Registry' - ...
+                               ~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidData: (:) [New-PSDrive], ParameterBindingValidationException
    + FullyQualifiedErrorId : ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.NewPSDriveCommand

After

In testing, this completes successfully and the module runs as expected.

[aaronk1]

@briantist please review. Removed the old PR as I messed up a few things accidentally and couldn't squash the ~10 commits. Should all be fixed now. Hoping for successful builds 🤞

[briantist]

The change looks good to me, the only thing I'd like to see is an addition for UNC paths. If those were in the tests the issue would have been caught on the original change. Adding them now will prevent regressions.

The tests can be found in tests/integration/targets/win_acl_inheritance and they are written in Ansible.

Would you be up for trying to add a test there?

[jborean93]

Are you able to add a changelog fragment as documented under https://docs.ansible.com/ansible/latest/community/development_process.html#creating-a-changelog-fragment to document this bugfix please.

[aaronk1]

The change looks good to me, the only thing I'd like to see is an addition for UNC paths. If those were in the tests the issue would have been caught on the original change. Adding them now will prevent regressions.

The tests can be found in tests/integration/targets/win_acl_inheritance and they are written in Ansible.

Would you be up for trying to add a test there?

Sure. May be a little until I can get to it though.

[jborean93]

Just an FYI you can probably just change one of the existing tests so the path is \\localhost\c$\.... The hardest bit will be figuring out how to convert the local path to be \\localhost\c$ but that shouldn't be too hard to achieve.

[github-actions]

This pull request is stale because it has been open for 4 weeks with no activity. Remove stale label or comment or this will be closed in 2 weeks.

[jborean93]

Will look at adding the tests in the future, for now it will be good to have this change in place.