Skip to content
This repository was archived by the owner on Nov 26, 2025. It is now read-only.

Conversation

@butonic
Copy link
Collaborator

@butonic butonic commented Jul 1, 2025

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.external to 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.

@butonic butonic self-assigned this Jul 1, 2025
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@butonic butonic force-pushed the internal-openldap branch from 517d8b2 to ede7f62 Compare July 1, 2025 14:28
@butonic butonic requested review from michaelstingl and wrenix July 4, 2025 20:26
@michaelstingl michaelstingl requested a review from Copilot July 4, 2025 20:47
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.internal values and defaults in values.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
Copy link

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copilot uses AI. Check for mistakes.
@butonic butonic marked this pull request as draft July 15, 2025 12:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants