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
win_secedit: Added module with tests/diff mode #26332
Conversation
The test
The test
|
@SamLiu79 @timothyvandenbrande @ar7z1 @blakfeld @brianlloyd @chrishoffman @if-meaton @joshludwig @petemounce @schwartzmx @smadam813 As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking a rename would be good on this one (win_security_policy?). I was never really a fan of naming the modules after the utility they were built on (eg, win_regedit
should've been win_registry
). Couple other little things (pretty much same as win_user_right).
|
||
DOCUMENTATION = r''' | ||
--- | ||
module: win_secedit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be win_security_policy
or something now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to change, the only reason I went this way as I wasn't sure if there were security policies that can't be edited by secedit. If that isn't the case I agree changing it is best.
# check the return code and if the file has been populated, otherwise error out | ||
if (($export_result.rc -ne 0) -or ((Get-Item -Path $secedit_ini_path).Length -eq 0)) { | ||
Remove-Item -Path $secedit_ini_path -Force | ||
Fail-Json $result "Failed to export secedit.ini file to $($secedit_ini_path).`nRC: $($export_result.rc)`nSTDOUT: $($export_result.stdout)`nSTDERR: $($export_result.stderr)`nLOG: $($export_result.log)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment from win_user_right WRT: discrete return keys for these instead of all in msg? Could see either way...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense, I think I went this way as older Ansible versions just ignored attributes for $result
, them being there now makes sense.
''' | ||
|
||
RETURN = r''' | ||
before_value: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redundant w/ diff mode?
|
||
# secedit doesn't error out on improper entries, re-export and verify | ||
# the changes occurred | ||
$export_result = Run-SecEdit -arguments @("/export", "/cfg", $secedit_ini_path, "/quiet") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as win_user_right: couldn't we just factor out the change detection, re-run, and fail on change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could but then we loose the specific error around an invalid keys, I'll try and refactor common code but feel that error would be useful to have.
+label new_module |
|
||
EXAMPLES = r''' | ||
- name: change the guest account name | ||
win_secedit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Examples need update to reflect rename
* win_secedit: Added module with tests/diff mode * fixed up test issues * Added missing return value * change for win_secedit based on review * updated win_security_policy examples for rename
SUMMARY
Added win_secedit module with the following supported
ISSUE TYPE
COMPONENT NAME
win_secedit
ANSIBLE VERSION
ADDITIONAL INFORMATION
This is a continuing of #22775 with tests/check and diff mode added.