Fix for long computer names

[aristotelos]

SUMMARY

Fix issue with long computer hostnames.

Fixes #113

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

microsoft.ad.membership

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/d8f96e5466824a8990863c2cd134345a

✔️ ansible-galaxy-importer SUCCESS in 4m 43s
✔️ build-ansible-collection SUCCESS in 8m 58s

[jborean93]

Thanks for the PR, from a basic look I don't see this being a problem but I'll have to look a bit deeper as I seem to recall GetHostName() may have been problematic in some scenarios. I could have been thinking of another API so I'll have a play around and get back to you!

[jborean93]

I've opened aristotelos#1 which contains the changelog fragment and changes to the existing manual tests to cover this new scenario. I've run them locally to ensure that this tests out your original issue as well as a few other edge cases to ensure we don't break it in the future.

I would probably recommend using $cs.DNSHostName and add DNSHostName to the -Property list on line 129 because:

• It can retrieve Unicode characters, GetHostName() returns ? for those characters
• It doesn't rely on the socket API which may have issues depending on the OS setup (I could not find a way to break it though)

This isn't something you need to change so I left it alone in my PR targeting your branch but it might be worth considering when/if I ever get to adding Unicode support to this module.

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/9e89e907f09b42c49f2f564b478e6f99

✔️ ansible-galaxy-importer SUCCESS in 4m 49s
✔️ build-ansible-collection SUCCESS in 8m 55s

[jborean93]

Thanks for working on this!

[jterpstra1]

@jborean93 can a new release be published with this merge?
We're running into the same issue and a new release with this would be appreciated.

Sorry for hijacking this issue.

[jborean93]

I'm hoping to merge in #117 once it's ready before the next release. Hopefully it'll be out at the end of this week or next week.