[OCISDEV-969] fix: ocis_multi deployment with proper ldap groups#12466
[OCISDEV-969] fix: ocis_multi deployment with proper ldap groups#12466jvillafanez wants to merge 1 commit into
Conversation
|
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
DeepDiver1975
left a comment
There was a problem hiding this comment.
Reviewed as maintainer. This is a clean, correct fix for the ocis_multi deployment example. ✅
What it does, and why it's right:
-
LDAP group object classes for
ocis-ocm— Onmasterthe workingocisservice already usesOCIS_LDAP_GROUP_OBJECTCLASS: "groupOfNames"+OCIS_LDAP_GROUP_ADDITIONAL_OBJECTCLASSES: "ownCloudGroup", but theocis-ocmservice still carried the oldOCIS_LDAP_GROUP_OBJECTCLASS: "owncloudGroup"with no additional object class. This PR bringsocis-ocmin line withocis, including the exactownCloudGroupcasing. This matches the migration that already landed for the main container and explains the "fixed in ocis but not ocis-ocm" symptom in the description. -
OCM domain replacement in the Keycloak realm import — The entrypoint now adds a second
sedexpression substitutingocis.ocm.owncloud.test→${OCIS_OCM_DOMAIN}, plus the matchingOCIS_OCM_DOMAIN: ${OCIS_OCM_DOMAIN:-ocis.ocm.owncloud.test}env on the keycloak service. Verified againstocis-realm.dist.json: it has 5 occurrences each ofocis.owncloud.testandocis.ocm.owncloud.test, so both now get rewritten when a non-default domain is used. The twosedpatterns are disjoint (ocis.owncloud.testis not a substring ofocis.ocm.owncloud.test, since.ocm.sits between), so left-to-right expression ordering is safe in both directions — no double-substitution or clobbering. The stale commented-out single-expression line was correctly superseded.
Minor / non-blocking:
- The leftover commented-out
# sed -e "s/ocis.ocm..."line just below is now dead and could be dropped for tidiness, but it's harmless. - As the description notes,
ocis.ocm.web.config.jsonstill needs manual domain edits — worth a follow-up to make the example fully domain-agnostic, but out of scope here.
Changelog: This is a deployment-example-only change (no product source touched). Per the repo's prevailing convention, example-only fixes generally ship without a changelog/unreleased/ fragment, so no fragment is required (n-a).
Verdict: approve. Behavior-correct, scoped, and consistent with the already-migrated ocis service.
Description
Some LDAP group changes broke the setup. It was fixed in the ocis container, but not in the ocis-ocm.
In addition, the OCM domain wasn't being replaced, so there were problems if the domain wasn't the default one.
Note that the "ocis.ocm.web.config.json" config file still needs manual changes for the domain.
Related Issue
Motivation and Context
The deployment example should work now as intended.
How Has This Been Tested?
Start the deployment example and login with different users in the ocis and ocis-ocm servers.
Screenshots (if appropriate):
Types of changes
Checklist: