vmware / PowerCLI-Example-Scripts

http://blogs.vmware.com/powercli
Other
743 stars 602 forks source link

Cannot add OpenLDAP Identity source #551

Closed DisasteR closed 2 years ago

DisasteR commented 2 years ago

Describe the bug

Function Add-LDAPIdentitySource should allow OpenLdap as ServerType but it's not present in parameter ValidateSet. It's fully working if we add OpenLdap in ValidateSet.

https://github.com/vmware/PowerCLI-Example-Scripts/blob/829307318f9de88094579c9f95822d65a7f46c1d/Modules/VMware.vSphere.SsoAdmin/IdentitySource.ps1#L366

Reproduction steps

1. Try to create an LDAP Identity source
2. It fail because OpenLdap is not allowed in ServerType ValidateSet

Expected behavior

LDAP Identity Source should be created.

Additional context

No response

kamennikolov commented 2 years ago

Hi DisasteR, Feel free to contribute the fix that you've applied locally.

DisasteR commented 2 years ago

Hi @kamennikolov,

Just added a PR with the change i have made locally. Also i think the parameter DomainServerType in function Add-ExternalDomainIdentitySource can be removed as the function is dedicated to ActiveDirectory source type.

https://github.com/vmware/PowerCLI-Example-Scripts/blob/91ab53cdf7a49f0fad9c474770a4b2a7d60db4c5/Modules/VMware.vSphere.SsoAdmin/IdentitySource.ps1#L42-L43

https://github.com/vmware/PowerCLI-Example-Scripts/blob/91ab53cdf7a49f0fad9c474770a4b2a7d60db4c5/Modules/VMware.vSphere.SsoAdmin/IdentitySource.ps1#L66

https://github.com/vmware/PowerCLI-Example-Scripts/blob/91ab53cdf7a49f0fad9c474770a4b2a7d60db4c5/Modules/VMware.vSphere.SsoAdmin/IdentitySource.ps1#L136-L143

https://github.com/vmware/PowerCLI-Example-Scripts/blob/91ab53cdf7a49f0fad9c474770a4b2a7d60db4c5/Modules/VMware.vSphere.SsoAdmin/IdentitySource.ps1#L174-L183

Why not hardcoding the type here ?

kamennikolov commented 2 years ago

Hi @DisasteR, Thanks for the fix. I've merged it. I can see that the DomainServerType parameter is also available in the API that is used. My guess is that it may be there as a possible extension point in the future, but I'm not sure about that.