smolnar82 commented on code in PR #1504:
URL: https://github.com/apache/knox/pull/1504#discussion_r4536840200
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManager.java:
##########
@@ -527,10 +527,10 @@ private void createBaseEntriesForDn(SchemaManager
schemaManager, String dn) thro
if (!directoryService.getAdminSession().exists(baseDnName)) {
Entry baseDnEntry = new DefaultEntry(schemaManager);
baseDnEntry.setDn(baseDnName);
- baseDnEntry.add("objectClass", "top", "domain");
- // Extract dc value from baseDn (e.g., "dc=proxy,dc=com" ->
"proxy")
- String dcValue = dn.split(",")[0].split("=")[1];
- baseDnEntry.add("dc", dcValue);
+ // An entry must have at least one STRUCTURAL class, which
'top'/'extensibleObject' alone are not.
+ // 'locality' is a STRUCTURAL objectClass with no MUST attributes;
'extensibleObject'
+ // (AUXILIARY) then permits whatever attribute the base DN's RDN
uses (dc, cn, ou, o, ...).
+ baseDnEntry.add("objectClass", "top", "locality",
"extensibleObject");
Review Comment:
This method also creates the proxy root (`gateway.ldap.base.dn`), so with
this change `dc=proxy,dc=com` becomes `locality`/`extensibleObject` instead of
`domain` on fresh installs. Existing installs keep `domain`, because the saved
entry isn't recreated. Could we keep `domain` when the RDN is `dc` and only use
the `locality` + `extensibleObject` fallback for other RDN types (`cn`, `ou`,
…)? Optionally `ou` → `organizationalUnit` and `o` → `organization`.
##########
gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java:
##########
@@ -398,6 +398,58 @@ public void testStartWithLdapProxyBackend() throws
Exception {
new Dn(serverManager.directoryService.getSchemaManager(),
interceptor.getBackend().getBaseDn()));
}
+ @Test
+ public void testStartWithLdapProxyBackendWithCnBaseDn() throws Exception {
Review Comment:
This test and `testStartWithLdapProxyBackendWithOuBaseDn` (line 428) are
copies of `testStartWithLdapProxyBackend`. Could they be one parameterized
test? It would also help to assert the created entry's objectClass, not only
that the partition exists.
##########
gateway-server/src/test/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManagerTest.java:
##########
@@ -398,6 +398,58 @@ public void testStartWithLdapProxyBackend() throws
Exception {
new Dn(serverManager.directoryService.getSchemaManager(),
interceptor.getBackend().getBaseDn()));
}
+ @Test
+ public void testStartWithLdapProxyBackendWithCnBaseDn() throws Exception {
+ GatewayConfig mockConfig =
EasyMock.createNiceMock(GatewayConfig.class);
+
expect(mockConfig.getGatewayDataDir()).andReturn(tempWorkDir.getParent()).anyTimes();
Review Comment:
`getGatewayDataDir()` returns `tempWorkDir.getParent()`, so the
`ldap-server` work dir is shared and survives between runs. When I ran these
new tests without the fix they passed, because the entry was still saved from
an earlier run. On a clean dir they fail correctly with `ERR_277`. This is
older than your PR, but returning `tempWorkDir.getAbsolutePath()` would make
these regression tests reliable on dev machines.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
