This repository was archived by the owner on Nov 26, 2025. It is now read-only.
Draft
Conversation
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
butonic
force-pushed
the
internal-openldap
branch
from
517d8b2 to
ede7f62
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds an optional internal OpenLDAP server to replace the built-in RocksDB-based IDM, enabling horizontal scaling and serving as a prerequisite for HA.
- Introduces new
ldap.internalvalues and defaults invalues.yaml - Adds Helm templates (Service, Secret, PVC, Deployment, ConfigMap, helper) for an OpenLDAP instance
- Integrates internal LDAP into the main deployment, updates environment variables, README, and bumps chart version
Comments suppressed due to low confidence (2)
charts/opencloud/templates/openldap/secrets.yaml:7
- [nitpick] Consider adding the common chart labels using
{{ include "opencloud.labels" . | nindent 4 }}above this line so that the Secret has consistent release and chart metadata like other resources.
app.kubernetes.io/component: openldap
charts/opencloud/README.md:385
- The documentation claims a random password is generated, but the chart uses a fixed default (
adminpass). Update the docs to reflect actual behavior or implement random password generation.
> If not set, a random password is generated during installation and stored in a Helm-managed secret. This secret uses the annotation `helm.sh/resource-policy: keep` to prevent it from being overwritten on upgrade.
|
|
||
| # Admin password | ||
| # ignored if ldap.internal.existingSecret is set | ||
| adminPassword: adminpass |
There was a problem hiding this comment.
Using a hardcoded default password is insecure. Consider removing the static default or generating a strong random password at install time to prevent accidental exposure.
Suggested change
| adminPassword: adminpass | |
| adminPassword: "" # Provide a password or use an existing secret |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces a new internal ldap server based on bitnami/openldap. It replaces the built in LDAP opencloud server (idm) when
ldap.internat.enabled: true.The built in LDAP server is based on rocksdb and cannot scale horizontally. The internal openldap server at least allows scaling it individually.
In a subsequent PR we can introduce
ldap.externalto use an external ldap server, eg. 389DS or an ldap cluster if necessary.This PR is also a prerequisite for HA of multiple openldap pods.