20:00:30 <nitzmahone> #startmeeting Ansible Windows Working Group
20:00:30 <zodbot> Meeting started Tue Jan 17 20:00:30 2023 UTC.
20:00:30 <zodbot> This meeting is logged and archived in a public location.
20:00:30 <zodbot> The chair is nitzmahone. Information about MeetBot at https://fedoraproject.org/wiki/Zodbot#Meeting_Functions.
20:00:30 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
20:00:30 <zodbot> The meeting name has been set to 'ansible_windows_working_group'
20:00:41 <nitzmahone> asleep at the switch
20:00:43 <briantist> yo
20:00:44 <nitzmahone> #chair jborean93
20:00:44 <zodbot> Current chairs: jborean93 nitzmahone
20:00:48 <nitzmahone> hey hey
20:00:52 <jborean93> yo
20:01:10 <nitzmahone> #info agenda https://github.com/ansible/community/issues/682
20:01:16 <nitzmahone> nothing new there, so
20:01:20 <nitzmahone> #topic open floor
20:02:06 <nitzmahone> I think Jordan's got news about the AD collection?
20:02:54 <briantist> yeah I got nothing specific, was just going to say the AD collection is looking great
20:03:01 <jborean93> Only exciting thing is the new collection `microsoft.ad`. It's very close to being ready for its first release. The only thing left for me to do is figure out if I want to change the option format for the `spn` and `groups` option in the `user` module https://ansible-collections.github.io/microsoft.ad/branch/main/collections/microsoft/ad/user_module.html#ansible-collections-microsoft-ad-user-module
20:03:52 <briantist> change in which way?
20:03:54 <jborean93> I've been moving towards these types of options being a dict with `add`, `remove`, `set` but those ones mentioned are still the list of values with a potential other option to control add/remove/set
20:04:05 <jborean93> let me give a concrete example
20:04:32 <briantist> ah I see
20:04:43 <jborean93> https://www.irccloud.com/pastebin/ml8JVdYi/
20:05:11 <jborean93> The benefits here is that you can add and remove in the same module invocation, vs now where it's either `set` all the time or you can only add or remove, not both at the same time
20:05:27 <jborean93> The new `attributes` stuff works like the `add`, `remove`, `set` sub keys
20:05:45 <briantist> another pro is consistency with the attributes and such in the collection
20:05:57 <jborean93> yep, I think consistency is the main reason why I want to do it
20:05:58 <nitzmahone> I personally prefer the nested dict for not needing to have a separate "action" option
20:06:06 <briantist> what the cons from your pov?
20:06:25 <jborean93> nested dicts aren't always that nice to use
20:07:27 <nitzmahone> The declarative argspec stuff tends to be a little less robust with nested dicts- it's gotten better, but there are still some gotchas
20:07:30 <jborean93> they don't really work with adhoc, you have to use json as the `-a` value which is pretty poor. I also think it's at the borderline of usable yaml but I think I'm floating towards making it consistent
20:08:26 <jborean93> the bright side is that if they all use the same add/remove/set dict format then people should understand it better when they see it again
20:08:41 <briantist> ah ok these are good points.. though I still think I'm leaving toward consistency as well
20:09:08 <briantist> maybe we can handle some of the usability issues with the guides, examples for ad-hoc and that kind of thing
20:09:11 <jborean93> Yea by far the majority of its uses will be in a task so I think consistency outweighs it all
20:10:13 <briantist> I did have one other thing I wanted to talk about in the new collection, and it's kind of an ongoing ansible debate of sorts that I think we've talked about before: https://ansible-collections.github.io/microsoft.ad/branch/main/collections/microsoft/ad/docsite/guide_migration.html#module-win-domain-user
20:10:23 <briantist> namely, the info returned from modules
20:10:49 <briantist> I was going to look into this more deeply before asking but might as well while in the meeting
20:11:07 <jborean93> Yea I stripped it right back to really just the dynamically created values that the user wouldn't know
20:11:34 <briantist> I tend to feel that if we have info, we should return it, since it's basically "free", and from a user perspective I don't like having the make another call to get info that could've been returned before
20:12:26 <jborean93> My point is, if you set the description in a task you know what the description is so why return it
20:12:27 <briantist> I wanted to look at the implementation first to see what info we returned before that we don't now, and whether that information is already present or not, so I may be missing some context
20:12:44 <jborean93> It complicates the module, especially when you start dealing with check mode and making return values consistent so it won't break the run when in check mode
20:13:12 <jborean93> But for things you can't really know ahead of time (object guid, sid, DN) then it will return those
20:13:22 <nitzmahone> (not even jumping into the whole resource module argument :D )
20:13:29 <briantist> ok I can see that
20:13:51 <jborean93> the win_domain_user was also a lot more complicated because they had a `state: query` option
20:14:27 <briantist> ftr I don't tend toward needlessly returning everything, but I also don't ruthlessly trim down either, if the information is already in play in the module, if that makes sense (not saying that's what happening here)
20:14:34 <jborean93> You could also argue that more data to be returned takes longer to go across the wire and be processed but that's really negligible and me being facetious
20:14:34 <briantist> oh I definitely agree with removing `state: query`
20:14:42 <briantist> haha
20:15:12 <briantist> especially on Windows, setting up the "wire" takes orders of magnitude longer, so yeah 😁
20:15:28 <nitzmahone> Fat module output does definitely bloat things up for ~Tower~ Controller users as well
20:16:13 <nitzmahone> I guess I still have to old school format that as Tower^W Controller 😆
20:16:35 <nitzmahone> (since that all gets stored in the DB)
20:16:44 <jborean93> yea it'll forever be Tower for me
20:16:47 <briantist> well, I understood `~` format, the ctrl W one... lost on me
20:17:17 <nitzmahone> (old-school keyboard shortcut for "delete last word")
20:17:54 <jborean93> but ultimately my reluctant of returning unneeded info is the complexity it adds to the module. Especially when you need to deal with check mode and deciding when to return that extra info (state: absent if deleted, only if that option was set, etc)
20:18:05 <jborean93> reluctance*
20:18:05 <briantist> yeah, I used to know that but it's been so long, I wouldn't have made the connection (though ^M I still see as return/enter lol)
20:18:48 <briantist> anyway, yeah for this case I do agree with it, I just hadn't looked deeply yet, and I think I'm less on the trimming end than others so wanted to touch on it a bit
20:19:38 <jborean93> all good, at least for now I prefer starting with a fresh slate and be more consistent on what is returned
20:19:42 <briantist> maybe the only other thing I might suggest to return is `sAMAccountName` since that's likely to get used a lot
20:19:48 <nitzmahone> it's always a tough call, but short of defining a format to return the entire object ala a facts/resource module, there's always going to be something you wish you had, and I don't think the resource module concept makes as much sense here
20:19:49 <jborean93> it's easier to add than remove return values
20:20:10 <briantist> heh definitely agree, easier to add
20:20:12 <nitzmahone> (heh, and we already know sAMAccountName can be an unexpected value )
20:20:37 <nitzmahone> s/be an unexpected/have a weird/
20:21:02 <jborean93> I would be open to returning`sAMAccountName`. While it typically matches the `name` there is a chance it might be something else if they don't explicitly define one
20:21:36 <jborean93> I do also want to migrate the `laps_password` lookup and look into an ldap inventory plugin. Looking at the landscape of Python LDAP libraries there are 2 - `python-ldap` and `ldap3`. The `python-ldap` essentially calls OpenLDAP which has a pretty poor UX when it comes to authentication compatibility with Microsoft AD. Plus setting it up consistently on various OS types is annoying.
20:21:41 <jborean93> The `ldap3` library is pure Python but doesn't support SASL encryption over LDAP so you are essentially limited to LDAPS on modern environments which sucks. I was going to try and send a PR their way to add support but there has been one sitting for a few months with no comment on the maintainer. There is also no commits since 2021 so I'm concerned it's a deal library.
20:21:41 <briantist> I definitely have groups where it annoyingly doesn't match, often happens during renames which can be tricky for AD objects
20:21:45 <jborean93> That essentially leaves writing our own library which I'm investigating but it will take time.
20:22:25 <jborean93> dead library*
20:22:27 <briantist> do you think the DNS-based inventory plugin I mentioned should be separate from the LDAP plugin? It seems like it should maybe
20:23:00 <jborean93> potentially, I don't know enough about inventory plugins right now to give an educated answer
20:23:19 <briantist> nitzmahone: for context, that would be an inventory plugin returning domain controllers only, by way of DNS discovery (so no auth needed)
20:23:20 <jborean93> I'm assuming the DNS stuff is building the "site info" based on the DNS records it can extract?
20:23:51 <briantist> it wouldn't be able to do a full dc-locator type of thing in terms of telling you which DC is closest to _you_ but it does have info on which DCs are in which site
20:23:59 <jborean93> that's fair
20:24:24 <jborean93> that particuiar functionality should also be in the ldap lookup to find the DC if a user doesn't specify one making it somewhat work out of the box with little configuration
20:24:58 <briantist> it sounds like they could have some overlap, but might still be nice as separate plugins
20:25:30 <briantist> or maybe combined at a later time, but I like having an authentication-less option for some light DC discovery (sometimes the site stuff is not even that important)
20:26:17 <jborean93> yea I think having both is good
20:26:44 <jborean93> I think having our own ldap library might not be too bad, we are only really concerned about get operations which isn't too hard to do
20:27:01 * jborean93 just another Python library to maintain though
20:27:19 <briantist> that's certainly beyond what I would consider doing, so I applaud the effort, but... yeah what you just said
20:28:00 <jborean93> I just know that getting OpenLDAP to work is even harder than getting Kerberos/GSSAPI setup and people still have difficulties with that
20:28:12 <nitzmahone> inventory composes pretty nicely anyway, so as long as the `inventory_hostnames` match up, you could make them completely separate and use both
20:28:52 <jborean93> Channel Binding Token support (that MS has ways to enforce) is also pretty absent in OpenLDAP/Cyrus-SASL unless you combine the latter from source
20:29:35 <jborean93> actually that's a lie they've finally made a release, so won't work unless you are running the bleeding edge distros
20:29:40 <briantist> nitzmahone yeah for sure, I figure groups would be created out of some of the additional info in DNS (site names, GCs, etc.), so maybe coordination there too if really needed but definitely agree inventory plugins can work together easily that way
20:33:16 <jborean93> anyway that's it from me, I'll look at unifying those options we talked about and continue poking the ldap bear
20:33:48 <jborean93> thanks briantist for your help on the docs side, made things so much better
20:34:24 <nitzmahone> OK cool, if nothing else then we'll wrap it up for this week
20:34:41 <nitzmahone> Thanks all, til next time!
20:34:44 <nitzmahone> #endmeeting