feat(ldap): [OCISDEV-1031] add opt-in LDAP connection pool - #12688
Merged
Conversation
✅ 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. |
LukasHirt
marked this pull request as draft
July 31, 2026 09:39
Contributor
Author
|
Marking as draft: this branch currently pins `go.mod` to the tip of the unmerged reva PR owncloud/reva#680, not a commit reachable from `stable-8.2`. If that PR is merged via squash/rebase (or force-pushed), this pin will point at an orphaned/unreachable commit. Blocked on owncloud/reva#680 merging. Once it does, I'll re-bump `go.mod`/`go.sum`/`vendor` here to the actual merged commit on `stable-8.2` and mark this ready for review. |
mmattel
reviewed
Jul 31, 2026
| UserSchema LDAPUserSchema `yaml:"user_schema"` | ||
| GroupSchema LDAPGroupSchema `yaml:"group_schema"` | ||
|
|
||
| PoolEnabled bool `yaml:"pool_enabled" env:"OCIS_LDAP_POOL_ENABLED;AUTH_BASIC_LDAP_POOL_ENABLED" desc:"Enable a bounded pool of LDAP connections instead of a single long-lived reconnecting connection. Concurrent requests then no longer serialize on one connection." introductionVersion:"8.0.7"` |
Contributor
There was a problem hiding this comment.
Info: technically this must be 8.2.0 (major/minor).
We never use patch release for new envvars.
If you are going to backport this to 8.1, it is ok to have it as 8.2.0 there.
Member
There was a problem hiding this comment.
Thanks,. Bumped introductionVersion to 8.2.0 on all pool env vars across auth-basic, graph, groups, and users.
mmattel
reviewed
Jul 31, 2026
|
|
||
| PoolEnabled bool `yaml:"pool_enabled" env:"OCIS_LDAP_POOL_ENABLED;GRAPH_LDAP_POOL_ENABLED" desc:"Enable a bounded pool of LDAP connections instead of a single long-lived reconnecting connection. Concurrent requests then no longer serialize on one connection." introductionVersion:"8.0.7"` | ||
| PoolSize int `yaml:"pool_size" env:"OCIS_LDAP_POOL_SIZE;GRAPH_LDAP_POOL_SIZE" desc:"Maximum number of concurrently open LDAP connections when 'OCIS_LDAP_POOL_ENABLED' is 'true'. Defaults to 5 when unset or <= 0." introductionVersion:"8.0.7"` | ||
| PoolCheckoutTimeout time.Duration `yaml:"pool_checkout_timeout" env:"OCIS_LDAP_POOL_CHECKOUT_TIMEOUT;GRAPH_LDAP_POOL_CHECKOUT_TIMEOUT" desc:"Maximum time to wait for a pooled LDAP connection to become available once the pool is exhausted. Defaults to 30s when unset or <= 0. See the Environment Variable Types description for more details." introductionVersion:"8.0.7"` |
Contributor
There was a problem hiding this comment.
Regarding versioning, see comment on the other new envvar location.
mmattel
reviewed
Jul 31, 2026
| UserSchema LDAPUserSchema `yaml:"user_schema"` | ||
| GroupSchema LDAPGroupSchema `yaml:"group_schema"` | ||
|
|
||
| PoolEnabled bool `yaml:"pool_enabled" env:"OCIS_LDAP_POOL_ENABLED;GROUPS_LDAP_POOL_ENABLED" desc:"Enable a bounded pool of LDAP connections instead of a single long-lived reconnecting connection. Concurrent requests then no longer serialize on one connection." introductionVersion:"8.0.7"` |
Contributor
There was a problem hiding this comment.
Regarding versioning, see comment on the other new envvar location.
mmattel
reviewed
Jul 31, 2026
Bump the vendored reva dependency to pick up the opt-in bounded LDAP connection pool (owncloud/reva#658, #665), and wire it into oCIS: add pool_enabled/pool_size/pool_checkout_timeout config to auth-basic, users, groups and graph, forward it into the reva config map for the first three, and unify graph's identity backend onto reva's LDAP client (utils.GetLDAPClientFromConfig) instead of its own hand-rolled reconnecting client, now that reva implements PasswordModify and ModifyWithResult on both LDAP clients. Pooling is off by default and fully backwards compatible. Signed-off-by: Lukas Hirt <info@hirt.cz> test(ldap): [OCISDEV-1031] run graph LDAP suites with pooling enabled in CI Add a dedicated ldap-pool-tests job to acceptance-tests.yml that reruns apiGraphUser/apiGraph/apiGraphGroup with OCIS_LDAP_POOL_ENABLED=true, so concurrent behat requests exercise checkout/release, eviction and the pooled PasswordModify/ModifyWithResult path end-to-end, in addition to the existing pooling-disabled runs of the same suites. Signed-off-by: Lukas Hirt <info@hirt.cz> fix(ldap): [OCISDEV-1031] set real pool_size/pool_checkout_timeout defaults Only PoolEnabled: false was set in the four defaultconfig.go files; PoolSize/PoolCheckoutTimeout were left as Go zero values (0/0s), relying on reva's NewLDAPPool substituting 5/30s when <= 0. That reads as "zero connections" to an operator in the generated yaml config rather than "default 5". Set the real defaults (5, 30s) so documented defaults match what's emitted. Signed-off-by: Lukas Hirt <info@hirt.cz> chore(deps): [OCISDEV-1031] bump reva to pick up LDAP TLS fixes Bumps the vendored reva dependency to owncloud/reva@8ee2020, picking up owncloud/reva#672 (restore MinVersion: tls.VersionTLS12 on LDAP TLS connections) and owncloud/reva#674 (restore pinned-CA-only trust and PEM validation for the CA-cert path), both flagged during review of this PR. Signed-off-by: Lukas Hirt <info@hirt.cz> docs: point changelog fragment at forward-port PR #12688
Bump the vendored reva dependency to pick up the opt-in bounded LDAP connection pool (owncloud/reva#658, #665), and wire it into oCIS: add pool_enabled/pool_size/pool_checkout_timeout config to auth-basic, users, groups and graph, forward it into the reva config map for the first three, and unify graph's identity backend onto reva's LDAP client (utils.GetLDAPClientFromConfig) instead of its own hand-rolled reconnecting client, now that reva implements PasswordModify and ModifyWithResult on both LDAP clients. Pooling is off by default and fully backwards compatible. Signed-off-by: Lukas Hirt <info@hirt.cz> test(ldap): [OCISDEV-1031] run graph LDAP suites with pooling enabled in CI Add a dedicated ldap-pool-tests job to acceptance-tests.yml that reruns apiGraphUser/apiGraph/apiGraphGroup with OCIS_LDAP_POOL_ENABLED=true, so concurrent behat requests exercise checkout/release, eviction and the pooled PasswordModify/ModifyWithResult path end-to-end, in addition to the existing pooling-disabled runs of the same suites. Signed-off-by: Lukas Hirt <info@hirt.cz> fix(ldap): [OCISDEV-1031] set real pool_size/pool_checkout_timeout defaults Only PoolEnabled: false was set in the four defaultconfig.go files; PoolSize/PoolCheckoutTimeout were left as Go zero values (0/0s), relying on reva's NewLDAPPool substituting 5/30s when <= 0. That reads as "zero connections" to an operator in the generated yaml config rather than "default 5". Set the real defaults (5, 30s) so documented defaults match what's emitted. Signed-off-by: Lukas Hirt <info@hirt.cz> chore(deps): [OCISDEV-1031] bump reva to pick up LDAP TLS fixes Bumps the vendored reva dependency to owncloud/reva@8ee2020, picking up owncloud/reva#672 (restore MinVersion: tls.VersionTLS12 on LDAP TLS connections) and owncloud/reva#674 (restore pinned-CA-only trust and PEM validation for the CA-cert path), both flagged during review of this PR. Signed-off-by: Lukas Hirt <info@hirt.cz> docs: point changelog fragment at forward-port PR #12688
mklos-kw
force-pushed
the
feat/ocisdev-1031-ldap-pool-stable-8.2
branch
from
August 4, 2026 13:07
268dfdc to
3307995
Compare
mklos-kw
marked this pull request as ready for review
August 4, 2026 13:40
2403905
approved these changes
Aug 4, 2026
deyankiteworks
approved these changes
Aug 4, 2026
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Summary
stable-8.0) tostable-8.2.pool_enabled/pool_size/pool_checkout_timeoutconfig into auth-basic, users, groups and graph.utils.GetLDAPClientFromConfiginstead of its own hand-rolled reconnecting client.Test plan
go build ./...go test ./services/graph/... ./services/auth-basic/... ./services/users/... ./services/groups/...