Add LDAP identity source resource #1010
Conversation
|
/test-all |
1 similar comment
|
/test-all |
| Description: "Username or DN for LDAP authentication", | ||
| Optional: true, | ||
| }, | ||
| "certificates": { |
There was a problem hiding this comment.
No. It's the public cert of the servers to tell NSX to trust. Has to be a string of pem. No path allowed.
Signed-off-by: Shawn Wang <[email protected]>
Signed-off-by: Shawn Wang <[email protected]>
Signed-off-by: Shawn Wang <[email protected]>
|
|
||
| func TestAccResourceNsxtPolicyLdapIdentitySource_basic(t *testing.T) { | ||
| testResourceName := "nsxt_policy_ldap_identity_source.test" | ||
| ldapType := nsxModel.LdapIdentitySource_RESOURCE_TYPE_ACTIVEDIRECTORYIDENTITYSOURCE |
There was a problem hiding this comment.
Do you think we make 2 sets of ENVs (one set for open LDAP another for AD)? Or just add another ENV var to select which type of server to test against?
There was a problem hiding this comment.
Would the same set of ENV variable work for both types? In this case, I would suggest to reuse same vars for both types
There was a problem hiding this comment.
bind_identity will be different. It all boils down to how we expect this acc test to be run in our pipelines in the future. If we want to cover both in one go then we need 2 servers (openldap + ad) anyways, so 2 sets of VARs. Otherwise the VAR names makes sense for both server types. Just different values should be set to them.
| "description": getDataSourceDescriptionSchema(), | ||
| "revision": getRevisionSchema(), | ||
| "tag": getTagsSchema(), | ||
| "type": { |
There was a problem hiding this comment.
Should this be defined as ForceNew, or can resource type be updated on existing object?
| ) | ||
|
|
||
| var ldapServerTypes = [](string){ | ||
| nsxModel.LdapIdentitySource_RESOURCE_TYPE_ACTIVEDIRECTORYIDENTITYSOURCE, |
There was a problem hiding this comment.
I would suggest to offer more user friendly values than OpenLdapIdentitySource - perhaps OpenLdap and ActiveDirectory.
| LdapServers: ldapServers, | ||
| ResourceType: serverType, | ||
| } | ||
| dataValue, errs := converter.ConvertToVapi(obj, nsxModel.LdapIdentitySourceBindingType()) |
There was a problem hiding this comment.
Would this work for the other server type? I think even it if works now, SDK might add validations in future that would fail if structs diverge. I would suggest to convert to the correct type even if those are identical for now.
There was a problem hiding this comment.
Converting using proper binding types now.
Signed-off-by: Shawn Wang <[email protected]>
|
Do we need some infrastructure to test this? |
|
/test-all |
Kobi, I agree that it's worth testing. I can follow up with test related changes when we have a better idea regarding how we add openldap or AD over ldap in our testing env. Merging this PR for now. |
This PR adds policy LDAP identity source resource. Both
ActiveDirectoryIdentitySourceandOpenLdapIdentitySourcetype LDAP are supported. The two resources have different resource type on NSX API, but shares the same attributes.