20:00:14 <jborean93> #startmeeting Ansible Windows Working Group
20:00:14 <zodbot> Meeting started Tue Jul 10 20:00:14 2018 UTC.
20:00:14 <zodbot> This meeting is logged and archived in a public location.
20:00:14 <zodbot> The chair is jborean93. Information about MeetBot at http://wiki.debian.org/MeetBot.
20:00:14 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
20:00:14 <zodbot> The meeting name has been set to 'ansible_windows_working_group'
20:00:51 <jborean93> hey all
20:01:00 <jhawkesworth_> hey
20:01:24 <jhawkesworth_> agenda is here for anyone who is lurking: https://github.com/ansible/community/issues/294
20:01:34 <jborean93> #chair jhawkesworth_
20:01:34 <zodbot> Current chairs: jborean93 jhawkesworth_
20:02:55 <it-praktyk[m]> win_pester: ansible/ansible#37343
20:02:56 <it-praktyk[m]> I can try review
20:03:24 <it-praktyk[m]> I'm a member of the Pester team
20:04:09 <it-praktyk[m]> so we are happy that Pester tests can be run using Ansible too
20:04:27 <jborean93> it-praktk[m] just an fyi, that's been merged and included in the 2.6 release. Any issues or features should be raised as issues
20:04:28 <jhawkesworth_> nice.  That one is merged already
20:05:58 <it-praktyk[m]> so please update https://github.com/ansible/community/issues/294#issuecomment-356972070
20:06:06 <jhawkesworth_> anyone else joining today?
20:06:08 <it-praktyk[m]> ;)
20:06:41 <jborean93> hoping nitzmahone would join us but we can get started
20:06:56 <jborean93> #topic https://github.com/ansible/community/issues/294#issuecomment-403730265 securestr param type
20:07:22 <jhawkesworth_> comment updated!
20:07:26 <jborean93> I'm have mixed feelings about this as I don't know what direction Ansible will head in the future
20:07:41 <jhawkesworth_> this is my concern too.
20:08:01 <jborean93> I can see the "benefit" for this but really to get a secure string it's a 1 liner to bypass
20:08:04 <jhawkesworth_> on the face of it, its a nice refactoring, but need to consider python world as well as powershell
20:08:45 <jborean93> it also isn't really secure, just a simple wrapper around converting an existing string so that we can use it for modules that expect that type
20:09:11 <jhawkesworth_> I wouldn't want to make declarative arg spec harder, just for a relatively small number of modules
20:10:13 <jborean93> with psrp you can actually serialise a secure string (in an encrypted form) but that doesn't really fit into the Ansible model and is really psrp/winrm specific
20:10:14 <jhawkesworth_> yeah good point its not like it can pass through all the layers of ansible (yaml, jinja2) and stay obscured
20:12:18 <jhawkesworth_> I like the idea of being able to say 'this string should be obscured' but would want something generic
20:12:51 <jborean93> that's the whole module invocation return as well which doesn't happen in powershell right now
20:12:58 <jhawkesworth_> I know you can vault stuff but its all still displayable at runtime
20:13:03 <jborean93> in Python you can set `no_log=True` in the arg spec and it won't display those args
20:13:19 <jborean93> for the return value back to the controller
20:13:45 <jhawkesworth_> ConvertTo-Json is not that sophisticated unfortunately
20:14:31 <jhawkesworth_> I feel a bit bad as clearly some time and effort has gone into it
20:14:37 <jborean93> yea, one of the reasons why nitzmahone and I are thinking about a new wrapper that replicates the basic part of basic.py
20:14:54 <jborean93> but the question around whether the controller or the module side should be doing that keeps on stopping us
20:16:25 <jhawkesworth_> that sounds like one of those things that you can only really settle by prototyping both ways - lot of work probably
20:16:32 <jborean93> yep
20:16:36 * nitzmahone is here
20:16:43 <jborean93> woot, just in time
20:16:45 <jhawkesworth_> hey nitzmahone
20:16:46 <jborean93> #chair nitzmahone
20:16:46 <zodbot> Current chairs: jborean93 jhawkesworth_ nitzmahone
20:16:48 <nitzmahone> sorry, got stuck in an impromptu meeting
20:16:58 <jborean93> #chair it-praktyk[m]>
20:16:58 <zodbot> Current chairs: it-praktyk[m]> jborean93 jhawkesworth_ nitzmahone
20:17:01 <jborean93> #chair it-praktyk[m]
20:17:01 <zodbot> Current chairs: it-praktyk[m] it-praktyk[m]> jborean93 jhawkesworth_ nitzmahone
20:17:26 <jhawkesworth_> no worries, we were just discussing the PR that wants to add securestr as a module parameter type
20:17:40 <jborean93> I was on the side of no thanks
20:18:33 <nitzmahone> My only worry there is an implication that we're going to treat it specially on the controller side (eg assumptions around security that aren't there).
20:18:47 <jhawkesworth_> that is a good point
20:19:08 <jhawkesworth_> it would be hard to achieve through all the layers (yaml parsing, jinja2)
20:19:09 <nitzmahone> Or that we're treating it properly in memory within the module for that matter (which is also not true)
20:19:22 <jborean93> yep those are my 2 negative points
20:20:04 <jhawkesworth_> false sense of security
20:20:08 <nitzmahone> Exactly
20:20:45 <nitzmahone> I think supporting it natively implies security that is most certainly not in place.
20:21:17 <jhawkesworth_> I get the impression it was really meant for interactive use and the ability to specify programmatically perhaps came later
20:21:40 <jhawkesworth_> (PS secure strings, that is).
20:22:15 <jborean93> cool so sounds like that's a no thanks
20:22:35 <jborean93> feel bad because the user did put in some effort there but that's the way the cookie crumbles
20:22:38 <nitzmahone> (unless it's in conjunction with actually supporting that properly end-to-end, which is a MUCH bigger project)
20:22:46 <jhawkesworth_> I guess so, hate to turn down first time contribution too
20:22:47 <jborean93> yep
20:23:38 <jborean93> #action jborean93 to update agenda info and post the reasoning on the PR
20:23:55 <jhawkesworth_> thanks for being the bearer of bad news.
20:23:59 <jborean93> #topic https://github.com/ansible/community/issues/294#issuecomment-403732051 win_package name
20:24:08 <jborean93> I only skimmed this yesterday but thought it would be good to talk about
20:24:27 <jborean93> pretty much allows you to specify the name of a product like you would with `product_id`
20:24:41 <ksubileau> Hi, PR author here, I've follow you're discussion, no worries i understand your points ;)
20:25:48 <jhawkesworth_> hi ksubileau, thanks for joining the discussion
20:25:50 <jborean93> no worries, welcome
20:25:58 <jhawkesworth_> I like the idea as these are the values you see  in Add / Remove Programs (or whatever the new name for that is).
20:27:05 <nitzmahone> I like the idea, but feels like the UI might need some tweakage
20:27:15 <jborean93> Will need to review it and potentially ask for some tests
20:27:23 <jhawkesworth_> I have a concern, which I have not tested, that maybe windows doesn't allways display exactly what is in the registry keys.
20:27:39 <nitzmahone> That's def a legitimate concern
20:27:41 <jhawkesworth_> ie if one key is empty, does it use another
20:27:50 <jborean93> I wouldn't be surprised, there's probably cases where no display name key is created at all
20:27:51 <nitzmahone> I think there are lots of fallbacks, and mostly undocumented IIRC
20:28:00 <jhawkesworth_> its just a fear, not based on any testing
20:28:32 <jhawkesworth_> but having looked in the registry uninstall keys for a few packages, there seems to be almost no standardization of which elements are used
20:28:53 <jborean93> most MSIs should be somewhat similar, exe installers are their own wild beast
20:29:30 <jborean93> cool so sounds like something we would like to have and just needs a standard review on the PR
20:29:42 <jhawkesworth_> I fear bug reports for some favoured package which doesn't obey the module's expectations
20:29:59 <jhawkesworth_> yes would like it if its workable.
20:30:01 <nitzmahone> The other potentially scary bit is i18n of the display strings
20:30:17 <jhawkesworth_> good point
20:30:58 <nitzmahone> Lots of ways that one can fail- not necessarily a reason to reject the PR, but seems like lots of caution tape around its use would be necessary (eg, "favor product_id, but if you just can't, realize that XYZ can break idempotency here")
20:31:52 <jhawkesworth_> I'm ok with adding caution tape.
20:32:02 <jborean93> yea same can be said for the `create_*` options as well, product_id is always the best option but we have the secondary options to make our lives easier
20:32:19 * jborean93 thinks we should just add a big warning banner saying use win_chocolatey instead
20:32:23 <nitzmahone> lol
20:32:28 <jhawkesworth_> :-)
20:32:35 <jborean93> sounds good
20:32:52 <nitzmahone> Still plenty of caution tape necessary there, too :D
20:33:01 <jhawkesworth_> I actually don't have that many places where I use win_package so my opinion may be skewed
20:33:14 <jborean93> after trying to get chocolatey server up and running the docs do need some TLC
20:33:15 <jhawkesworth_> and I use chocolatey even less!
20:33:23 <jborean93> #topic https://github.com/ansible/ansible/pull/39464
20:33:35 <nitzmahone> I have yet to publish a choco package for something in anger to avoid using win_package, but the thought has crossed my mind.
20:33:48 <jborean93> this one is it-praktyk[m]
20:34:01 <jborean93> we discussed it here https://meetbot.fedoraproject.org/ansible-windows/2018-01-16/ansible_windows_working_group.2018-01-16-20.00.log.html
20:34:13 <nitzmahone> The biggest problem with that is we're paying to ship the docs on every module exec.
20:34:16 <jborean93> but it seems vague as to whether we wanted to go ahead with it
20:34:30 <nitzmahone> Tis why I've been reticent to include docs in the past
20:34:50 <jborean93> in saying that, we do have some comments above it, it is mostly moving it to the "correct" PS standard
20:34:52 <nitzmahone> We don't want to strip them at runtime either, because then stacktraces are wrong
20:35:44 <nitzmahone> Yeah, I'm not opposed to doing that, although if we're going to do it in a broad way, we should probably have automated enforcement to keep it that way.
20:35:51 <jhawkesworth_> I wonder if it actually results in more soap traffic
20:36:01 <jborean93> probably want to revert some of the stylistic changes that change the case from `If` to `if` so we keep the attribution in those spots
20:37:23 <jborean93> jhawkesworth_: I believe the changes adds a few more bytes to the payload, nothing major
20:37:38 <nitzmahone> Yeah- as a general rule, Ansible hasn't accepted one-off style PRs except in the context of adding automated enforcement of a given style
20:37:41 <jborean93> if it was on the border, there's probably a bigger change the module options would tip the balance back and force
20:38:05 <jborean93> nitzmahone: I believe we are ignoring some rules right now in that file which can be removed
20:38:07 <nitzmahone> If we can't do that, we shouldn't accept the inline style changes (mainly for attribution)
20:38:24 <nitzmahone> Yeah, if it's stuff that allows us to turn on a rule and keep it that way, I'm OK with it
20:38:24 <jborean93> I know we went through pylinting PRs previously to bring that ignore list down
20:38:43 <jborean93> I don't think the `If/if` stuff is, but some of the alias changes would
20:38:47 <nitzmahone> But if it's just a broad stylistic change that we can't enforce programmatically, -1
20:39:00 <jborean93> So I think doing the alias expansion and function docs are good to go
20:39:05 <jborean93> the stylistic stuff should stay the same
20:39:09 <nitzmahone> (as the original attribution is lost for effectively no good reason)
20:39:20 <jhawkesworth_> right.
20:39:22 <jborean93> yep
20:40:09 <jhawkesworth_> the $null -eq .. changes are justified on grounds of avoiding lint errors then?
20:40:19 <jborean93> there's a reasoning behind that as well
20:40:32 <it-praktyk[m]> OK, I'll update the PR for If/Else/ElseIf
20:40:32 <it-praktyk[m]> anything else?
20:40:34 <jborean93> can't remember what but PS-isms means that is a safer comparison
20:40:56 <nitzmahone> It's for the common case where someone uses = instead of -eq
20:41:02 <jborean93> it-praktk[m] probably remove the ignored rules from the ignore list now that they are gone
20:41:03 <nitzmahone> $null is not assignable, so it goes bang
20:41:24 <jhawkesworth_> there are a couple of places where aliases have been reqplaced with actual cmdlet names.
20:41:40 <jborean93> here is the ignore list https://github.com/ansible/ansible/blob/devel/test/sanity/pslint/ignore.txt
20:42:05 <it-praktyk[m]> https://rencore.com/blog/powershell-null-comparison/
20:42:13 <jhawkesworth_> `echo` -> `Write-Output`
20:42:36 <jborean93> yep, that's a linting rule we currently ignore for the file
20:42:46 <jborean93> so changing that (and other locations) means we can remove that ignore
20:42:52 <it-praktyk[m]> I couldn't find the ignore list, yes I'll update it
20:43:33 <jhawkesworth_> cool, more idiomatic powershell in module_utils makes sense to me
20:43:38 <jborean93> also the tests will probably fail due to an existing bug with win_reboot, hopefully we get that fixed soon so we can have passing tests
20:43:47 <nitzmahone> I don't recall if that sanity test uses the ignore list for negative failures, but most do (eg, if nothing trips the ignore, that's actually a failure too because it means we need to remove it from the ignore list)
20:44:16 <jborean93> yea I'm pretty sure mattclay added that, believe I've had a test failure when I forgot to remove it from the ignore list
20:45:26 <nitzmahone> The other docs stuff may become less important if/when we start doing module/module_utils caching on the remotes, but probably not until at least 2.8
20:46:01 <nitzmahone> (ie we may be more willing to accept actual PS doc strings in the future when we're not carting them around on every task)
20:47:15 <jborean93> yep but in this case we already have those doc comments, it is just moving them in the correct way so IDEs can understand it better
20:49:28 <jhawkesworth_> given its the main 'api' for ansible powershell modules it makes sense to me that its friendly to newcomers
20:51:33 <jhawkesworth_> only objection would be f its going to cause problems parsing docmentation, in which sticking with # comments makes most sense
20:52:04 <jborean93> I would be surprised if we ever went that route. If we were to do something it should be agnostic of the language of the module
20:52:13 <jhawkesworth_> seems unlikely
20:52:15 <jborean93> e.g. a module.yml file or something like that
20:52:49 <jhawkesworth_> by the time you want to know about the cmdlets available, you probably want to look at the source code, not some generated web page doc
20:53:51 <jhawkesworth_> on balance I'm happy with having the powershell style docs then
20:54:50 <jhawkesworth_> sounds like a plan?
20:54:51 <it-praktyk[m]> or try to do it using 'the PowerShell way' Import-Module <name>, Get-Command -Module <name>, Get-Help <Command_name>
20:54:51 <jborean93> cool so I think we are in agreement, use ps style docs on the functions, convert the aliases and remove the ignored entries
20:55:28 <nitzmahone> (but don't add additional PS docs, eg parameter-level descriptions etc)
20:55:39 <jborean93> yep, just move the existing ones that are there
20:55:46 <it-praktyk[m]> it's an exact of the reason to update that documentation
20:56:12 <nitzmahone> ?
20:56:33 <it-praktyk[m]> OK
20:57:06 <it-praktyk[m]> good luck, good night (Poland, 22:56 CEST)
20:57:15 <jborean93> good night, thanks for your time
20:57:15 <nitzmahone> 'night- thanks!
20:57:27 <jborean93> ok 3 minutes left
20:57:30 <jhawkesworth_> thanks it-praktyk[m]
20:57:32 <jborean93> anything else to discuss
20:57:39 <jborean93> (briefly)
20:57:39 <jhawkesworth_> nothing else from me
20:57:55 <nitzmahone> nawp
20:58:11 <jhawkesworth_> ok well till next time, cheers
20:58:14 * jhawkesworth_ afk
20:58:15 <jborean93> cool
20:58:18 <jborean93> thanks everyone
20:58:20 <jborean93> #endmeeting