Skip to content

chore(graph): disable HTTP or eventhandlers by configuration - #3293

Open
pbleser-oc wants to merge 2 commits into
opencloud-eu:mainfrom
pbleser-oc:chore/#1312-graph-event-handler-split
Open

chore(graph): disable HTTP or eventhandlers by configuration#3293
pbleser-oc wants to merge 2 commits into
opencloud-eu:mainfrom
pbleser-oc:chore/#1312-graph-event-handler-split

Conversation

@pbleser-oc

@pbleser-oc pbleser-oc commented Aug 12, 2026

Copy link
Copy Markdown
Member

Description

In the scope of the broader issue #1312, this PR deals with performing those changes for the graph service, namely to add the ability to disable the HTTP API or to disable the events API handler by configuration.

It also adds numerous metrics, and tests for the events processing.

Making Event Processing and HTTP Optional (#1312)

The previous implementation was combining the HTTP server service and the events consumption, which is why this PR refactors the composition of those services:

  • the event consumption has been moved into its own service
  • the identity.Backend is created beforehand, and then injected as a collaborator in both the HTTP service as well as the event consumer service

To encourage re-use in latter implementations and changes, it also introduces two top-level package changes:

  • internal/eventstest/events_test_helpers: contains a TestBus implementation to unit-test event consumers without the need to engage NATS
  • internal/metricstest/metrics_test_helpers: contains assertion functions to test Prometheus metrics

Run-down of changes for this aspect:

  • add the ability to disable the HTTP API handler (GRAPH_HTTP_DISABLED)
  • add the ability to disable the Events API handler (GRAPH_EVENTS_DISABLE_CONSUMER)
  • add metrics, especially for event processing
  • add metrics to the documentation
  • introduces a new top-level packages internal/eventstest and internal/metricstest with utilities for testing event consumers and Prometheus metrics, respectively

Includes metrics for whether the HTTP API handler is enabled, and another for whether the Event consumer is enabled, to allow counting the number of instances that do either, and potentially define alerts when one of those reaches 0 (or another low threshold).

image

Adding Metrics

Introducing gowrap as a build-time tool to generate interface delegate structs from templates:

  • added as a make generate target in services/graph,
  • added as a build-time dependency in .bingo/

LDAP Client Metrics

Introduces LDAP client abstraction interface to be able to wrap the go-ldap client API with metrics transparently (and possibly hooks and such in the future), in order to use delegation patterns to measure the time LDAP (client) operations take to finish, as well as to track their results (success, failure, not-found).

Has two implementations that are generated using gowrap:

  • a go-ldap adapter implementation that directly delegates
  • a time measuring and metrics collecting implementation that delegates to another LdapClient

The metrics collecting one is disabled by default, can be enabled with GRAPH_LDAP_METRICS_DISABLE=false

It collects the duration of outbound LDAP client operations into a histogram, as well as the number of concurrent outbound LDAP operations in a gauge (via an atomic int and a gauge func for best performance).

image image

HTTP Requests Metrics

Add an HTTP middleware that measures how long Graph HTTP API requests take, storing taken time into a histogram along with labels for

  • method,
  • path pattern (from the chi routes),
  • Graph API version prefix,
  • Graph API resource name,
  • and the resulting status code.

It also tracks the number of concurrent inbound Graph API HTTP requests using a gauge (also via an atomic int and a gauge func for best performance).

Disabled by default, can be enabled with GRAPH_HTTP_METRICS_DISABLE=false

image

LDAP Identity Backend Metrics

Add Backend and EducationBackend delegate implementations that measure execution time on the level of the higher API call operations there (CreateUser, DeleteUser, ..., CreateSchool, ...), generated using gowrap.

Disabled by default, can be enabled with GRAPH_IDENTITY_BACKEND_METRICS_DISABLE=false

image

LDAP Client API Change

Made an internal change to how the singular entry search operations work in the LDAP identity backend:

  • check whether searches for a singular entry returns more than one result, in which case a new error TooManyResults is returned, instead of leaving that undetected, blindly taking the first result, and potentially risking data inconsistencies

Logging Improvements

Improve the loggers in identity backends by adding attributes for their request targets (Reva gateway address or LDAP URI, respectively).

Also add a "backend" attribute for all Graph API logs (set to "ldap" or "cs3"), to help debug potential issues.

The LDAP identity backend logger also has two new attributes to help debugging with logs:

  • write (bool): whether write operations are enabled
  • refint (bool): whether refint is enabled or not

Also adds a dedicated counter for user password change operations.

Additional boy-scouting

  • add a constructor func for the CS3 backend
  • add a constructor func for the LDAP backend
  • in the LDAP identity backend, in searchLDAPEntryByFilter (used by all search/get public functions), errors that occur when performing LDAP SEARCH operations were blindly mapped to a ItemNotFound error, instead of being analyzed as it could be caused by a technical error
  • in the requireadmin middleware, add debug logging to explain why a request is denied
  • when an LDAP password change fails because the user entry was not found in LDAP, we now have a log message that tracks that

Related Issue

Motivation and Context

Details can be found in #1312

How Has This Been Tested?

HTTP API

Tested the HTTP API (when enabled) using:

xh --verify=no -a alan:demo --https :9200/graph/v1.0/me

and

xh --verify=no -a alan:demo --https :9200/graph/v1.0/users/b1f74ec4-dd7e-11ef-a543-03775734d0f7

and

xh --verify=no -a admin:admin --https :9200/graph/v1.0/users \$orderby=='displayName asc'

and

xh --verify=no -a admin:admin --https POST :9200/graph/v1.0/users accountEnabled:=true displayName="Jane Doe" onPremisesSamAccountName=jdoe mail="jdoe@example.com" passwordProfile:='{"password": "secret12345"}'

Events API

Tested the event API (when enabled) using:

echo -n '{"metadata":{"eventtype":"events.UserSignedIn"},"payload":"'$(echo -n '{"Executant":{"opaque_id":"alan"}}'|base64)'"}'|nats pub main-queue

Tested combinations of enabling/disabling the events and HTTP APIs.
When both are disabled, the service refuses to start and exits early with an error message.

Metrics

Metrics were tested with a small k6 script included in this PR (namely services/graph/load_test.js) that performs various read-only operations with parallel VUs.

k6 run --vus 20 --duration 10m ./services/graph/load_test.js

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation added (in services/graph/README.md)

@codacy-production

codacy-production Bot commented Aug 12, 2026

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 2 critical · 4 high

Alerts:
⚠ 6 issues (≤ 0 issues of at least minor severity)

Results:
6 new issues

Category Results
ErrorProne 4 high
Security 2 critical

View in Codacy

🟢 Metrics 496 complexity · 327 duplication

Metric Results
Complexity 496
Duplication 327

View in Codacy

🟢 Coverage 18.36% diff coverage · -0.23% coverage variation

Metric Results
Coverage variation -0.23% coverage variation (-1.00%)
Diff coverage 18.36% diff coverage

View coverage diff in Codacy

Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (f88765e) 84318 19614 23.26%
Head commit (ef27ec5) 86071 (+1753) 19827 (+213) 23.04% (-0.23%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#3293) 2135 392 18.36%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@pbleser-oc
pbleser-oc force-pushed the chore/#1312-graph-event-handler-split branch 3 times, most recently from d892695 to ec49989 Compare August 13, 2026 11:42
@pbleser-oc
pbleser-oc marked this pull request as draft August 14, 2026 13:22
@pbleser-oc
pbleser-oc force-pushed the chore/#1312-graph-event-handler-split branch 15 times, most recently from 89661f4 to 1db7cb2 Compare August 20, 2026 22:50
@pbleser-oc
pbleser-oc marked this pull request as ready for review August 21, 2026 06:38
@pbleser-oc
pbleser-oc force-pushed the chore/#1312-graph-event-handler-split branch from 1db7cb2 to 6fbfdf1 Compare August 21, 2026 10:20
In the scope of the broader issue opencloud-eu#1312, this PR deals with performing
those changes for the `graph` service, namely to add the ability to
disable the HTTP API or to disable the events API handler by
configuration.

It also adds metrics for the events processing, and tests for the events
processing.

The previous implementation was combining the HTTP server service and
the events consumption, which is why this PR refactors the composition
of those services:

 * the event consumption has been moved into its own service
 * the identity.Backend is created beforehand, and then injected as a
   collaborator in both the HTTP service as well as the event consumer
   service

It also adds metrics, mainly for the event processing.

To encourage re-use in latter implementations and changes, it also
introduces two top-level package changes:

 * internal/eventstest/events_test_helpers: contains a TestBus
   implementation to unit-test event consumers without NATS
 * internal/metricstest/metrics_test_helpers: contains assertion
   functions to test Prometheus metrics
Introducing gowrap as a build-time tool to generate interface delegate
structs from templates:

 * added as a 'make go-generate' target in services/graph,
 * added as a build-time dependency in .bingo/

Introduce an LDAP client abstraction interface to be able to wrap the
go-ldap client API with metrics transparently (and possibly hooks and
such in the future), in order to use delegation patterns to measure the
time LDAP (client) operations take to finish, as well as to track their
results (success, failure, not-found).

Has two implementations that are generated using gowrap:

* a go-ldap adapter implementation that directly delegates to a go-ldap
  connection
* a time measuring and metrics collecting implementation that delegates
  to another LdapClient

The metrics collecting one is disabled by default, can be enabled with
GRAPH_LDAP_METRICS_DISABLE=false

It collects durations of outbound LDAP client operations into a histogram, as
well as the number of concurrent outbound LDAP operations in a gauge (via an
atomic int and a gauge function, as that performs best).

Add an HTTP middleware that measures how long Graph HTTP API requests
take, storing taken time into a histogram along with labels for

 * method,
 * path pattern (from the chi routes),
 * Graph API version prefix,
 * Graph API resource name,
 * and the resulting status code.

It also tracks the number of concurrent inbound Graph API HTTP requests
using a gauge (also using an atomic int and a gauge function).

Disabled by default, can be enabled with
GRAPH_HTTP_METRICS_DISABLE=false

Add Backend and EducationBackend delegate implementations that measure
execution time on the level of the higher API call operations there
(CreateUser, DeleteUser, ..., CreateSchool, ...), generated using
gowrap.

Disabled by default, can be enabled with
GRAPH_IDENTITY_BACKEND_METRICS_DISABLE=false

Also added a small k6 script to produce some read-only load on the Graph
API, for a casual test of the metrics, as well as k6 in mise.toml.

Make an internal changes to how singular LDAP entry searches work in the LDAP
identity backends:

 * check whether searches for a singular entry returns more than one
   result, in which case a new error TooManyResults is returned, instead
   of leaving that undetected, blindly taking the first result, and
   potentially risking data inconsistencies

Improve the loggers in identity backends by adding attributes for their
request targets (Reva gateway address or LDAP URI, respectively).

Also add a "backend" attribute for all Graph API logs (set to "ldap" or
"cs3"), to help debug potential issues, and remove them from all the logger
debug calls at the beginning of each LDAP-related function as those should
really be part of the logger and set beforehand.

The LDAP identity backend logger also has two new attributes to help
debugging with logs:

 * write (bool): whether write operations are enabled
 * refint (bool): whether refint is enabled or not

Also adds a dedicated counter metric for user password change operations.

Minor campfire improvements:

 * add a constructor func for the CS3 backend

 * add a constructor func for the LDAP backend

 * in the LDAP identity backend, in searchLDAPEntryByFilter (used by all
   search/get public functions), errors that occur when performing LDAP
   SEARCH operations were blindly mapped to a ItemNotFound error,
   instead of being analyzed as it could be caused by a technical error

 * in the requireadmin middleware, add debug logging to explain why a
   request is denied

 * when an LDAP password change fails because the user entry was not
   found in LDAP, we now have a log message that tracks that
@pbleser-oc
pbleser-oc force-pushed the chore/#1312-graph-event-handler-split branch from 6fbfdf1 to ef27ec5 Compare August 21, 2026 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

1 participant