19:59:15 <nitzmahone> #startmeeting Ansible Windows Working Group
19:59:15 <zodbot> Meeting started Tue Jul  2 19:59:15 2019 UTC.
19:59:15 <zodbot> This meeting is logged and archived in a public location.
19:59:15 <zodbot> The chair is nitzmahone. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:59:15 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
19:59:15 <zodbot> The meeting name has been set to 'ansible_windows_working_group'
19:59:24 <nitzmahone> #chair jborean93
19:59:24 <zodbot> Current chairs: jborean93 nitzmahone
19:59:31 <jborean93> too quick :)
19:59:38 <jborean93> o/
19:59:39 <nitzmahone> I did it early :)
20:00:25 <Shachaf92> hey
20:00:36 <nitzmahone> howdy
20:01:04 <jborean93> hey
20:01:28 <nitzmahone> #topic https://github.com/ansible/community/issues/420#issuecomment-505895637
20:01:50 <nitzmahone> Multiple things, going down the list:
20:01:57 <nitzmahone> #topic AD in CI
20:02:08 <Shachaf92> right
20:02:11 <jborean93> heh, this is a fun one
20:02:21 <Shachaf92> possible?
20:02:27 <nitzmahone> We've poked at it before- it's technically possible, but would add an arguably unacceptable amount of time to the CI runs
20:02:36 <jborean93> I know nitzmahone and mattclay have been talking about it but a lot of hurdles to overcome
20:02:55 <jborean93> or if we have a persistent DC then it would be costly and unreliable over time
20:02:58 <nitzmahone> yep
20:03:00 <Shachaf92> any way to make a CI group taht would take longer?
20:03:14 <nitzmahone> The problem is that the workers are independent
20:03:24 <Shachaf92> for each server?
20:03:44 <nitzmahone> Sort of. Our CI does a lot of "interesting" things to prevent abuse
20:04:10 <jborean93> each node is independent and does not know about the other nodes. It talks to a host in AWS/Azure for the Windows tests but they also don't know about each other
20:04:11 <nitzmahone> We don't currently have the concept of a "resource server" (like a DC)
20:04:15 <Shachaf92> if there was no 60 min limit we could have a server promote to DC and the others wait for it
20:04:43 <nitzmahone> It's more complicated than that
20:04:55 <jborean93> it's already annoying enough to wait the 5-10 minutes for the host to come online. Adding another step would just make that worse
20:05:01 <nitzmahone> (that too)
20:05:23 <nitzmahone> But right now, the jobs are all independent, and they can be run one at a time.
20:05:37 <nitzmahone> Shippable doesn't have the concept of "this job needs this one to be running first"
20:05:37 <Shachaf92> Oh i see what you mean now
20:05:57 <Shachaf92> we could run the tests on the server locally...
20:06:02 <nitzmahone> We pay for enough concurrent nodes that *most* of the time they run together, but it's not guaranteed
20:06:15 <Shachaf92> not perfect but could at least give some testing to return values and stuff
20:06:19 <nitzmahone> That works for some things, but can actually fail and/or mask other problems
20:06:36 <nitzmahone> (and still the problem of waiting for promotion, which can take 5-10min on its own)
20:06:39 <jborean93> I am looking into unit tests that could help make things a bit better but mocks only get you so far
20:06:41 <Shachaf92> but doesn't it worth it for the tests it could help with?
20:07:07 <Shachaf92> like users, groups?
20:07:09 <jborean93> it would help but the effort/cost to get there is a bit too high right now
20:07:24 <Shachaf92> even with a DC promotion every time?
20:07:25 <nitzmahone> It's been a cost-benefit tradeoff- I agree that as we get more domain-specific modules, it's becoming more important
20:07:49 <Shachaf92> i mean, not having a single DC 24/7 but for the test
20:08:04 <Shachaf92> as part of the test
20:08:24 <nitzmahone> We'd have to dedicate at least one worker to that to have any hope of the "run everything" runs not breaking the 60min
20:08:28 <jborean93> still takes time to promote it but we could look at moving the AD based modules to their won group
20:08:45 <jborean93> but then yes we come across the issue when it inevitably goes longer than 60 minutes
20:08:52 <jborean93> IIRC it's even 45 minutes now
20:09:06 <jborean93> because 5-10 of those minutes is starting the instance
20:09:17 <nitzmahone> Self-promotion/test-against-self would be OK so long as we can keep the run for the stuff we're testing under 60min
20:09:19 <jborean93> then 5 is padding to make sure we don't breach the hour
20:09:41 <Shachaf92> umm, if i take the local tests cases it should stay in the limit if the tests are only the domain tests
20:09:48 <nitzmahone> @Shachaf92- if you want to experiment with it, you can do your own shippable config in a PR
20:10:07 <Shachaf92> yeah ill add it to the list of stuff to play with and ill update
20:11:11 <nitzmahone> Basically go add a new Windows worker and have a really fast domain prereq test for all the domain module targets that will self-promote if necessary and just continue if already a DC
20:12:20 <Shachaf92> Ill try it first on my own shippable i think, then come with conclusions here
20:12:48 <jborean93> you will probably need to use our Shippable. There's some integrations with ansible-test so it can actually create the WIndows host
20:13:14 <Shachaf92> maybe ill take the chance and learn to integrate them on the fly
20:14:16 <jborean93> If you are testing I would remove most of the other groups in `shippable.yml`. Would just reduce the noise as touching that file runs all tests
20:14:27 <Shachaf92> k
20:15:18 <jborean93> anything more for that topic?
20:15:23 <Shachaf92> nope
20:15:41 <jborean93> #topic https://github.com/ansible/ansible/issues/53402 use of regex options
20:15:49 <jborean93> Not much we can decide here for that
20:16:03 <jborean93> it would need to go to the wider Ansible community meetings to see what the end result is
20:16:11 <Shachaf92> it was opened a while back and haven't seen much comments there
20:16:23 <jborean93> typically that's just involves a group decision on the name we want and then we can go from there
20:16:33 <Shachaf92> i see
20:16:58 <nitzmahone> and meh, alias if we want
20:17:20 <jborean93> yea actually doing the work is fine, it's really just deciding what we want to align on
20:17:25 <nitzmahone> (so either regex or regexp will work, or decide which one's "right" and make sure it's supported everywhere
20:17:48 <Shachaf92> so should i add it to the public agenda? or just leave it be?
20:18:49 <jborean93> if you would like to persue it then adding it to the agenda would be great
20:18:52 <nitzmahone> You can bring it up at the big meeting if you care enough :)
20:19:05 <Shachaf92> k
20:20:01 <jborean93> #topic feature parity for win_stat
20:20:21 <jborean93> I don't see an issue with adding a get_attr flag to control whether we get those values
20:20:36 <jborean93> Probably wouldn't want a get_mime as I don't know of any equivalent to `file` in Windows
20:20:47 <jborean93> if there was then that could change my mind
20:20:47 * nitzmahone doesn't care either way- what's the reason for wanting a flag besides "stat" has it?
20:21:07 <nitzmahone> There technically is- the shell type detection does that, but I think it's deprecated
20:21:11 <Shachaf92> no file equiv.
20:21:32 <Shachaf92> most options you find online use winapi which uses extensions not content
20:22:04 <Shachaf92> do we really need teh get_attr flag?
20:22:31 <nitzmahone> Just "because the Linux module does it" isn't necessarily a great reason
20:22:33 <jborean93> honestly I don't think so
20:22:54 <nitzmahone> I don't know why you'd want to skip that, it's cheap
20:22:58 <jborean93> I have never seen any performance overhead for getting the attributes. Mainly because the object we create already have them
20:23:00 <Shachaf92> so im marking that parity as done for now
20:23:05 <jborean93> cool
20:23:25 <jborean93> #topic feature parity for win_user and win_group
20:23:34 <jborean93> is this parity with user/group or something else?
20:23:59 <Shachaf92> yes
20:24:28 <jborean93> yes for which one :)
20:24:39 <Shachaf92> both lol
20:24:43 <Shachaf92> each on its own
20:25:05 <nitzmahone> so nothing to discuss there, right?
20:25:11 <jborean93> for the user/group comparison if the feature makes sense on Windows then I don't see why not
20:25:23 <jborean93> but Windows does have things not present on linux and vice versa
20:25:44 <jborean93> for the something else I can't read minds so you will have to explain it some more :)
20:25:46 <Shachaf92> yep, i looked at the lists and nothing relevant that we don't have
20:26:18 <Shachaf92> oh i just figured what you meant with "which one"
20:26:41 <Shachaf92> the former in the question
20:26:57 <Shachaf92> it's late here, i'm a bit fuzzy today
20:27:03 <jborean93> that's ok, it's early here
20:27:10 <jborean93> cool so sounds like nothing else to add for that
20:27:11 <nitzmahone> it's lunchtime here :D
20:27:20 * nitzmahone looks longingly at kitchen
20:27:28 <nitzmahone> OK, agowa338, you here?
20:27:28 <Shachaf92> a full day between the three of us
20:27:32 <agowa338> yes
20:27:51 <nitzmahone> #topic plaintext credential fun
20:28:01 <nitzmahone> So a little background:
20:28:47 <nitzmahone> Module args are included in the archive shipped to targets
20:29:22 <nitzmahone> In the normal winrm/psrp cases where this stuff was built, it never hits the disk (due to pipelining, where we shove the content directly into powershell over stdin)
20:29:39 <nitzmahone> On connections that don't support pipelining, though (eg vmware_tools)...
20:29:50 <nitzmahone> The archive can be written to disk
20:30:09 <nitzmahone> There's been questions about whether we can support encryption
20:30:32 <nitzmahone> The short answer is "yes, it's possible", but not without a *lot* of change to the way modules execute
20:30:44 <nitzmahone> (and some architectural changes to connections)
20:31:13 <nitzmahone> We're doing some long-term work on some of that for other reasons, and it's something we've discussed before
20:31:21 <nitzmahone> I can try to include it with that work
20:31:33 <nitzmahone> But I don't think there's going to be a short-term solution here
20:31:55 <nitzmahone> There is one thing we could do to potentially mitigate the risk of a sensitive archive being recovered, though...
20:32:32 <nitzmahone> We could make a "secure_delete" option that would scrub out the file's contents before deleting the tempdir
20:32:50 <nitzmahone> It would slow down module exec on non-pipelined connections even more
20:33:34 <agowa338> and is not 100% secure, because of ware leveling and other low level stuff that we have no control over.
20:33:38 <nitzmahone> exactly
20:33:39 <bcoca> nitzmahone: i thought we had that already ...forget name
20:33:56 <nitzmahone> bcoca: not that I'm aware of
20:34:06 <bcoca> might be python only 'shred' iirc
20:34:15 <nitzmahone> Yeah, shred is a Linux thing
20:34:18 <bcoca> i remember updating shred_file
20:34:25 * nitzmahone looks
20:34:33 <jborean93> wasn't that a proposed state to file?
20:34:44 <nitzmahone> ah yeah, I do recall that
20:35:15 <nitzmahone> But IIRC it had nothing to do with, eg, "securely" cleaning up after non-pipelined exec
20:35:30 <jborean93> `remove` in the shell plugin doesn't have a shred like kwarg
20:35:30 <nitzmahone> vault does it
20:35:42 <jborean93> just calls `rm -rf`
20:36:10 <nitzmahone> vault _shred_file_custom
20:36:55 <jborean93> but anyway it sounds like we could add a shred function to the shell plugin but there's no guarantee that will actually "shred" the file
20:37:21 <nitzmahone> Exactly
20:37:23 <jborean93> adding encryption to the payload would require some deeper changes in the module common loader but hard to tell what without really implementing it
20:37:37 <jborean93> then we have the problem is sharing the secret to decrypt it
20:37:46 <nitzmahone> yep, that's the bigger problem
20:37:57 <agowa338> I think there is no need for shred, as it is not guaranteed to work anyway. We should focus on only writing the file encrypted to disk.
20:38:02 <jborean93> the easiest option would be for an individual connection plugin to implement something right now but that doesn't help Ansible overall
20:38:28 <nitzmahone> Can't even do it connection level
20:38:51 <agowa338> the secred could be shared by parameter to the executed program, so it would not end up on disk and vanish after execution.
20:39:04 <nitzmahone> The connection primitives are too low level- how would you tell the diff between a module that needs encryption and a data file being sent for like win_copy
20:39:12 <jborean93> ahh because we copy the file separately
20:39:25 <nitzmahone> Parameter secrets aren't secret- just moves the problem around
20:39:45 <nitzmahone> Like I said, there's *not* a quick and easy solution here that I can see
20:40:01 <agowa338> parameters don't end up on disk, they are only in memory.
20:40:28 <nitzmahone> and on the command line while the module's running, which means they're visible to anyone on the machine, and also logged
20:40:36 <agowa338> I agree, it is neither quick nor easy. But it is the most covering solution I can think of.
20:41:01 <nitzmahone> It's probably not going to happen anytime soon, since the "supported" combo is pipelined
20:41:30 <agowa338> but what is the fallback if pipelining is not supported?
20:41:49 <agowa338> I'm more for a good than a fast solution.
20:42:29 <nitzmahone> The current behavior. We can include the request in the next-gen-connection/module research that's going on, where it should be much easier to accommodate, but there's not a robust or generic way to implement it on the current connection stack
20:43:23 <agowa338> +1
20:44:04 <jborean93> you could also Pester VMWare to include an stdin option to their API
20:44:15 <jborean93> but I can only imagine how that would go
20:44:19 <nitzmahone> heh
20:44:36 <nitzmahone> #topic https://github.com/ansible/ansible/pull/58383
20:44:46 <nitzmahone> (win_domain_user idem password support)
20:45:35 <agowa338> This one should be quick, can someone review this change? I just saw that I have to rebase it again, so maybe skip it for now?
20:45:36 <jborean93> I need to have a closer look but I think we can remove some of the work you've done to hide the password
20:45:52 <jborean93> but I'll have a look at this sometime today
20:46:07 <nitzmahone> Yeah, MS has basically declared SecureString dead
20:46:26 <jborean93> they've actually ressurected it for PSv7
20:46:30 <nitzmahone> (and kinda pointless considering how it got there)
20:46:35 <jborean93> does plaintext though so not secure on Linux
20:46:38 <nitzmahone> ... did they actually implement it?
20:46:52 <nitzmahone> Yeah, I thought it wasn't even secure on Windows in PS Core
20:46:53 <jborean93> not yet, not sure if they are planning anything before v7 is officially out though
20:47:21 <jborean93> PSCore on Windows is fine, it uses the same codebase as PS Desktop
20:47:22 <nitzmahone> regardless, it's passed through a dozen places already
20:47:36 <jborean93> yep I think we can just simplify it back to how it was before
20:48:10 <Shachaf92> anyway to tell the linter to always ignore that rule?
20:48:16 <jborean93> it should already be
20:48:30 <agowa338> ok, I'll reverte the change.
20:48:38 <nitzmahone> Also, dunno if we want to move the validation into one of the other existing module_utils rather than inlining the PInvoke stuff
20:48:38 <jborean93> https://github.com/ansible/ansible/blob/devel/test/sanity/pslint/settings.psd1#L6
20:49:03 <jborean93> potentially, I didn't for win_user because only that used it
20:49:18 <Shachaf92> oh then we can remove those from teh ignore.txt
20:49:46 <jborean93> it might actually be missing one
20:49:55 <jborean93> I think the use PSCredential as parameter one
20:50:01 <jborean93> which yes we should probably ignore
20:50:27 <nitzmahone> Ah didn't realize that was just copypasted from win_user - definitely don't want to duplicate that
20:50:51 <jborean93> it's slightly different in win_user
20:50:59 <jborean93> that just takes in a string for username/password
20:51:05 <agowa338> nitzmahone: not fully c'n'p ed, but mostly yea ;-)
20:51:38 <nitzmahone> and if you're zapping the securestring stuff, it *can* be the same, yeah?
20:52:05 <jborean93> should be fine
20:52:09 <agowa338> mostly, except for the pass domain.
20:53:12 <agowa338> https://github.com/ansible/ansible/pull/58383/files#diff-a814da53b65fce105cb5fb909b79b267R52
20:53:15 <jborean93> cool, feel free to ping me when you are ready for the review
20:53:34 <nitzmahone> #topic https://github.com/ansible/ansible/pull/57577
20:53:59 <jborean93> any question for this one or just asking for a review?
20:54:00 <nitzmahone> I'd suggest keeping the IPv4 and IPv6 values separated
20:54:29 <agowa338> also just asking for review. And the other ones also just review.
20:55:19 <agowa338> windows does not keep them separate, it causes issues if we do, as we would have to fetch the ipv6 values when setting the ipv4 values and set them together.
20:55:41 <nitzmahone> IIRC, Windows does, but the PS cmdlets don't
20:56:39 <Shachaf92> yeah in the OS it's diffrent
20:56:54 <Shachaf92> diffreant
20:57:09 <Shachaf92> differeant
20:57:16 <agowa338> but as we use the powershell api, we're limited there, or not?
20:57:19 <nitzmahone> IIRC it's pretty easy to split them out though- they come back as different types
20:58:22 <nitzmahone> https://www.irccloud.com/pastebin/gAiN980L/
20:58:58 <nitzmahone> They come back tagged by type, and IIRC the actual address values are also of different types
20:59:24 <nitzmahone> So you'd combine on the set
20:59:25 <agowa338> but try to set them, you have to pass in both ipv4 and ipv6 ones, ...
20:59:37 <nitzmahone> which should be fine?
21:00:24 <agowa338> I don't know. I kinda prefer not differentiating between IPv4 and IPv6 though.
21:00:56 <Shachaf92> powershell wise, unless you talk about address validation there is no difference
21:01:05 <nitzmahone> The existing module UI already does, so that die has sorta been cast
21:01:48 <agowa338> the current module also accepts ipv6 addresses in the ipv4 value... But only partly and silently errors out...
21:02:03 <agowa338> as reported by others
21:02:26 <Shachaf92> personally im +1 to merging and making it work with both in the same list
21:03:28 <jborean93> I honestly don't know enough about this module to really make a decision sorry
21:04:26 <jborean93> but considering the addresses are set in different areas in the GUI I would be more inclined to have them as separate arguments
21:05:31 <nitzmahone> Did you test on 2008/R2? I remember there was a non-PS WMI hack I had to do, but can't remember if that layer treats them separately for setting or not
21:05:34 <Shachaf92> maybe the common ground is the module have it as a single array but accept both ipv2 and ipv4 paramters and a dns_servers one
21:06:38 <nitzmahone> I don't have really strong feelings- I also don't remember offhand if I coded in support for preserving IPv6 DNS servers or not- if I *did*, that could be an (admittedly small) problem...
21:06:50 <agowa338> Like this? https://github.com/ansible/ansible/pull/57577/files#diff-7eddb99b86d85d31f8b675dc31bae170R32
21:07:30 <agowa338> IPv6 dns is preserved in the current module.
21:07:49 <agowa338> at least the one learned from router broadcasts...
21:07:53 <nitzmahone> Nah, I just meant if I preserved existing ipv6 addresses that were set- it doesn't look like I did
21:08:15 <nitzmahone> (eg it would blow them away today, so that problem doesn't get worse)
21:08:51 <agowa338> does it? I had problems with it not removing them. But as I said they were broadcasted by the router...
21:09:01 <nitzmahone> Yeah, I'm talking about manually set
21:09:21 <agowa338> Ok, have not tested that.
21:09:39 <nitzmahone> The "reset" case could be interesting too though, if for some reason v6 and v4 needed different behaviors
21:10:05 <agowa338> what do you mean with different?
21:10:23 <jborean93> in the current impl can you say remove IPv4 DNS IPs but keep IPv6 ones?
21:10:27 * jborean93 is that even valid
21:11:15 <agowa338> No
21:11:22 <nitzmahone> Powershell doesn't appear to support that (just a single "-ResetServerAddresses" arg), but the UI does
21:12:06 <agowa338> you need to hack around that by get-ing the values and setting them in the same call as you specify -ResetServerAddresses...
21:14:30 <nitzmahone> Well anyway, I don't have really strong feelings, so if others want to merge them together and change it up too, WFM
21:15:08 <nitzmahone> I can't think of a case where it's really problematic to do so
21:16:23 <nitzmahone> We're about 15min over time, shall we punt the rest to next week?
21:16:32 <jborean93> looks like we are out of time, most of the remaining items looks like reviews so I'm good to go
21:17:00 <nitzmahone> OK- thanks for all the discussion... Til next week!
21:17:00 <agowa338> yes, we could put that onto the list for the next meeting.
21:17:10 <jborean93> thanks all
21:17:13 <nitzmahone> #endmeeting