20:01:02 <nitzmahone> #startmeeting Windows Working Group
20:01:02 <zodbot> Meeting started Tue Nov 14 20:01:02 2017 UTC.  The chair is nitzmahone. Information about MeetBot at http://wiki.debian.org/MeetBot.
20:01:02 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
20:01:02 <zodbot> The meeting name has been set to 'windows_working_group'
20:01:59 <jborean93> Howdy
20:02:22 <nitzmahone> #chair jborean93
20:02:22 <zodbot> Current chairs: jborean93 nitzmahone
20:02:40 <ar7z1> Hi!
20:03:08 <nitzmahone> #chair ar7z1
20:03:08 <zodbot> Current chairs: ar7z1 jborean93 nitzmahone
20:03:20 <nitzmahone> Howdy!
20:04:36 <nitzmahone> #chair dag
20:04:36 <zodbot> Current chairs: ar7z1 dag jborean93 nitzmahone
20:04:57 <nitzmahone> #info agenda: https://github.com/ansible/community/issues/195
20:05:45 <nitzmahone> #topic overlapping firewall rules: https://github.com/ansible/community/issues/195#issuecomment-342850950
20:06:26 <jborean93> This is a tricky one
20:06:58 * jborean93 reads through the comments again
20:10:14 <jborean93> Haven't read through it all but I would say the best action would be to match rules based on the name and profile. If multiple profiles match then updated all of them
20:10:27 <jborean93> s/multiple profiles/multiple rules/
20:10:44 <ar7z1> I thought about @dag comments once again and I think we can merge similar rules. But I think we shouldn't do it automatically without explicit option. We can add for example "merge_duplicates : yes" option.
20:11:22 <ar7z1> So we will not break back compatibility :-)
20:11:39 <jborean93> I agree, we shouldn't do something unexpected unless a user tells us to
20:12:47 <ar7z1> And I agree with @jborean93, we should slightly improve current search algorithm.
20:13:46 <ar7z1> It should not be too difficult :-)
20:15:15 <jborean93> Should there also be a filter on allow/deny rules, idealy the display name should be different but that might not be the case all of the time
20:21:58 <ar7z1> May be, but I'm not sure. I think it'll be ok if we modify both of them. And set single value ('allow' or 'deny') for all rules with same name.
20:23:44 <jborean93> Well can a rule have both allow and deny?
20:23:51 <jborean93> I would have thought it would be one or the other
20:23:51 <ar7z1> No
20:23:54 <jborean93> otherwise it's just deny
20:24:06 <ar7z1> Yep, it just deny
20:24:55 <jborean93> Then I would say that should be a part of the filtering if multiple rules are found based on the name and profile
20:25:02 <ar7z1> If there are two rules (one - with 'allow', another - with 'deny') for 80 port and user want to allow connections, he can't do it because of filter by allow/deny
20:25:52 <jborean93> the port could be different?
20:25:59 <jborean93> hmm I see where you are getting at
20:26:33 <ar7z1> in my example - the same
20:27:53 <jborean93> Well I think in the end, if multiple rules are found by name and/or profile, they should all be modified in the one task
20:28:40 <ar7z1> I think it'll be ok
20:30:03 <jhawkesworth_> hey sorry I'm late, catching up
20:31:29 <jborean93> hey jhawkesworth_
20:31:38 <nitzmahone> #chair jhawkesworth_
20:31:38 <zodbot> Current chairs: ar7z1 dag jborean93 jhawkesworth_ nitzmahone
20:34:02 <ar7z1> We can add filter by name and profile as first win_firewall_rule improvement. And merging of identical rules as the second.
20:34:22 <ar7z1> What do you think about it?
20:34:26 <jborean93> Yea, I'm still a bit iffy about merging but that sounds like a good plan
20:35:44 <jborean93> I'm concerned that someone would have done something weird and have 2 rules that are very similar but contain some differences in their rules. Merging that based on name and profile may be fine but in some cases it could be bad
20:35:54 <nitzmahone> Yeah, merging will quickly turn into a big mess, I suspect
20:36:16 <nitzmahone> Better thing would be to just add support for optionally filtering by profile
20:36:41 <nitzmahone> Sounds like that's the plan, but I'm not sure I'd go down that second route at all
20:37:20 <jborean93> Yea, +1 to filtering by profiles, -1 to merging
20:37:45 <jborean93> If there is multiple matches with profile I would say update them both
20:38:00 <nitzmahone> Hrm, I dunno
20:38:49 <jborean93> So keep throwing an error?
20:38:50 <ar7z1> "netsh advfirewall firewall" does so
20:39:00 <nitzmahone> which- update both, or error?
20:39:06 <ar7z1> update both
20:39:39 <ar7z1> https://technet.microsoft.com/ru-ru/library/dd734783%28v=ws.10%29.aspx?f=255&MSPPError=-2147217396#BKMK_3_show
20:39:47 <nitzmahone> I guess there's at least precedent then, but that still seems more dangerous than requiring that you target exactly one rule (even if it means needing to add more filter caps)
20:40:35 <jborean93> so filter on all values and only add if nothing is found?
20:41:06 <nitzmahone> ... and only edit if exactly one
20:41:35 <jborean93> what about state: absent with multiple
20:41:53 <nitzmahone> Ugh, that's messy too though- feels like the same thing as the ec2 "count_tags" stuff- basically separating selection criteria from "make it look like this"
20:42:51 <nitzmahone> Maybe just a mode flag like `allow_multiple: yes` for those cases, but that feels dangerous too
20:42:59 <nitzmahone> I guess they had to opt in
20:44:16 <ar7z1> So we can try to implement the safest thing:
20:44:16 <ar7z1> 1. Filter by name. If multiple rules found then:
20:44:16 <ar7z1> 2. Filter selected  in #1 rules by profile. If multiple rules found then throw old error “Multiple rules found”
20:45:01 <ar7z1> I think this algorithm will resolve @charlesgreen initial problem
20:45:52 <jborean93> I would have thought
20:45:52 <jborean93> 1. find all rules based on all args
20:45:52 <jborean93> 2. error if multiple found unless (allow_multiple: yes) is set
20:45:52 <jborean93> 3. perform action (add/modify/remove) on the 1 rule or muliple if that is allowed
20:46:16 <nitzmahone> I don't think you can use all args though
20:46:22 <jborean93> all set args?
20:46:27 <nitzmahone> Chicken/egg- would prevent modification?
20:46:38 <jborean93> ahh true
20:47:26 <nitzmahone> I really hate separating the selection criteria from the creation criteria- it's just bad UX, but I think we might have to to enable all the different ways this could work
20:47:38 <nitzmahone> s/creation criteria/creation data/
20:48:58 <jborean93> ok, sounds like ar7z1 is a good bypass for the current issue but there is more to be decided for another PR at some point
20:51:21 <nitzmahone> Moving on, then?
20:52:05 <jborean93> I'm good
20:52:24 <nitzmahone> #topic wiki deletions https://github.com/ansible/community/issues/195#issuecomment-344080845
20:52:34 <nitzmahone> @jhawkesworth_ Works for me
20:52:49 <jhawkesworth_> didn't think it would be a long topic this one.
20:52:53 <jborean93> Yep, I'm cool with deleting it
20:53:06 <jborean93> If I missed anything in the coversion feel free to create a PR
20:53:07 <jhawkesworth_> just wanted to be sure nothing there to keep.
20:53:34 <jhawkesworth_> sure, I'll check again and then prune the wiki pages
20:53:46 <nitzmahone> Cool- thanks!
20:53:52 <nitzmahone> #topic open floor
20:55:09 * jborean93 chirp
20:55:23 <jhawkesworth_> just to update that I've not forgotten about the | Out-Null to >$null stuff but am struggling to prove anything at the moment.  As previously discussed too much jitter to prove anything by running integration tests
20:55:47 <jborean93> Might be easier to just run locally where things are at least more consistent
20:55:55 <nitzmahone> Yeah, I definitely think that's a microperf thing in most cases. The module overhead would dwarf most of the time
20:56:06 <nitzmahone> Agreed on local run
20:56:57 <nitzmahone> ok, if nothing else, done in 5...
20:57:03 <nitzmahone> 4..
20:57:03 <jhawkesworth_> it was certainly hopeless trying to run on laptop vms (esp as phone is router when on the train)!
20:57:10 <nitzmahone> (heh)
20:57:15 <nitzmahone> 3..
20:57:23 <nitzmahone> 2..
20:57:28 <nitzmahone> 1..
20:57:36 <nitzmahone> Thanks all!
20:57:38 <nitzmahone> #endmeeting