feat(identity): make persons-seed CLI-only with a scheduled CronJob (#1690) - #2046
Merged
Conversation
…1690) The identity org projection (persons / account_person_map / org_chart) froze after the bootstrap seed: nothing re-ran the persons-seed while connectors kept identity_inputs fresh daily, so Team-view rosters went stale for everyone re-parented since the last run. The seed becomes a first-class scheduled operation of the service; the HTTP trigger is removed (team decision — the .NET service and its POST die together): - new `seed` subcommand runs the same domain pipeline synchronously and exits (0 ok / 1 failed / 2 lock busy / 3 input guard); subcommand paths now install a tracing subscriber (migrate logs were silent no-ops before) - runs serialize on a per-tenant MariaDB GET_LOCK held on a dedicated single-connection session — covers cron-vs-manual overlap and multiple instances sharing one database, crash-safe by construction - input guards (--force overrides, refusals journaled as failed operations): empty identity_inputs read (broken/misconfigured pipeline) and wrong-tenant run (would mint a parallel person universe under a wrong tenant — the #1550 failure mode) - every run writes the operations journal (author = nil SYSTEM_AUTHOR, the legacy Python seed convention); zombie sweep moves to CLI start, so a killed Job cannot strand a `running` row - POST /v1/persons-seed + its in-process queue/worker/503 path are removed; the GET journal routes stay as the observability window - Helm: seed CronJob in the identity-resolution subchart (same config/secret wiring as the deployment; distinct pod labels so the Service never routes to seed pods), daily 06:30 UTC after overnight syncs; optional seed.tenantDefaultId env-overrides the Secret for standalone installs; the umbrella validates a tenant is present when it composes the config Secret itself (credentials.autoGenerate) - e2e: POST cases gated to dotnet (die with .NET), CLI cases gated to rust (guards, lock, exit codes, journal contract); coverage gate skips the removed POST on the rust lane and drops its REQUIRED_EXTRA Verified: 67 unit tests, clippy/fmt clean; identity contract suite green on both lanes (rust 111 passed, dotnet 108 passed) with coverage gates rc=0; helm lint + render matrix for both charts. Closes #1690 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eed (#1690) The suite verified the persons half of the seed but never built an org chart from inputs — the roster carried no parent_email at all, so the projection #1690 is about went unexercised end to end. - roster grows a manager chain: a boss, a parent_email on the shared person, and a deliberately unresolvable parent (ghost) on the solo one - test_seed_org_chart_matches_inputs: after a CLI seed the open edges match the inputs — resolvable manager → edge to the right parent, unresolvable → NULL-parent membership row (ADR-0010: no stub persons), top-of-tree → Path-B NULL-parent row; asserted over SQL on purpose (the read projection filters by org_chart_source_type and is covered by the read tests over the handcrafted fixture) - test_seed_manager_change_reaches_org_chart: THE #1690 regression — a newer parent_email lands in identity_inputs, a re-run moves the open edge to the new manager and closes the old one (SCD2 history intact) The manager-change cast is per-run unique (uuid-suffixed accounts): the MariaDB persons log outlives sessions on a kept local stack (the session seed wipes only reason='e2e-seed' rows), so re-parenting a shared roster account poisons the next session's latest-observation race whenever runs land seconds apart. A fresh child has no cross-session history by construction. CI is unaffected either way (fresh containers per lane). Verified: three back-to-back rust-lane runs 113 passed each, dotnet lane 108 passed, both coverage gates rc=0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a guarded ChangesPersons-seed lifecycle
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
… zombie sweep (#1690) Review found three coverage gaps; all three are now pinned. Scheduling mechanism (the essence of #1690 — the bug WAS the absence of a schedule, and the functional-k3s lane doesn't deploy identityResolution): - helm/tests/test_seed_cronjob_contract.py — pure `helm template` + PyYAML assertions, no cluster: CronJob exists by default with the documented schedule and Forbid; exact seed command/args (and never --force — forcing is a deliberate manual act); Secret/ConfigMap equal the deployment's own wiring (compared against its manifest, not literals); seed.tenantDefaultId env-overrides the Secret and is absent by default; seed.enabled=false removes only the CronJob; seed pod labels never satisfy the Service selector; umbrella refuses an enabled seed with no tenant and renders with either tenant source or with the seed disabled - .github/workflows/identity-resolution-helm.yml runs helm lint (both charts) + these tests on any subchart/umbrella change (gateway.yml precedent) Exit code 1 through the real binary: - run_seed_cli grows extra_env; the test points clickhouse_url at a closed port → rc 1 AND a failed journal row carrying exactly the generic "persons-seed failed; see job logs" (pins the no-leak rule: raw driver text must never reach the GET-returned error_message) Zombie sweep: - synthetic running operations at 2h and 5min; a seed run flips the stale one to failed ("aborted by pod restart") and leaves the fresh one alone — both sides of the 1h cutoff asserted; synthetic rows cleaned up Verified: helm contract 10/10; rust lane 115 passed twice back-to-back, dotnet lane 108 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
mozhaev-dev
marked this pull request as ready for review
July 30, 2026 06:02
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/backend/services/identity-resolution/src/infra/db/seed_repo.rs (1)
81-107: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winFull
personstable scan on every seed run — consider existence checks instead ofSUM.
input_guardsonly needs zero-vs-non-zero forown_rows/other_rows, but this query aggregates over the entirepersonstable (all tenants) with noWHERE. As the table grows across tenants, this becomes an increasingly costly per-run full scan/aggregate ahead of every seed pipeline execution.Two indexed
EXISTS(orSELECT 1 ... LIMIT 1) queries oninsight_tenant_idwould let the optimizer short-circuit instead of scanning/aggregating the whole table.⚡ Proposed existence-based rewrite
- const SQL: &str = r" - SELECT - CAST(COALESCE(SUM(insight_tenant_id = ?), 0) AS SIGNED) AS own_rows, - CAST(COALESCE(SUM(insight_tenant_id <> ?), 0) AS SIGNED) AS other_rows - FROM persons - "; + const SQL: &str = r" + SELECT + EXISTS(SELECT 1 FROM persons WHERE insight_tenant_id = ?) AS own_rows, + EXISTS(SELECT 1 FROM persons WHERE insight_tenant_id <> ?) AS other_rows + ";Note: this changes
own_rows/other_rowssemantics from exact counts to 0/1 booleans — update the operator-facing guard message inseed_runner::input_guardsaccordingly if adopted.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/services/identity-resolution/src/infra/db/seed_repo.rs` around lines 81 - 107, Update tenant_presence to use indexed existence checks for matching and non-matching insight_tenant_id values instead of aggregating SUM across persons, while preserving the TenantPresence zero/non-zero contract. Adjust seed_runner::input_guards operator-facing messaging to describe presence/existence rather than exact row counts if it exposes those values.src/backend/services/identity-resolution/helm/values.yaml (1)
88-94: 🚀 Performance & Scalability | 🔵 TrivialVerify resources sized for the batch seed workload, not just copied from the API service.
These request/limit numbers are identical to the HTTP service's defaults (
charts/insight/values.yamlidentityResolution.resources), but the seed job does a full per-tenant projection recompute against ClickHouse rather than serving light requests. For larger tenants this may be under-provisioned, and sincerestartPolicy: Never, an OOMKill just burns abackoffLimitretry silently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/services/identity-resolution/helm/values.yaml` around lines 88 - 94, Update the resources under the seed workload’s values configuration to reflect full per-tenant projection recomputation against ClickHouse rather than copying the HTTP service defaults. Size CPU and memory requests and limits for larger tenants, and ensure the batch job’s resource settings reduce the risk of OOMKilled retries under its existing restart policy.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/backend/services/identity-resolution/src/seed_runner.rs`:
- Around line 117-177: Bound all database operations performed by run_locked,
including sweep_zombies, enqueue, try_start, complete, and fail, with the same
SEED_TIMEOUT policy used by guarded_seed. Ensure a timeout returns the
appropriate SeedRunError while preserving operation journaling and failure
handling, so the tenant lock cannot be held indefinitely by any critical-section
call.
---
Nitpick comments:
In `@src/backend/services/identity-resolution/helm/values.yaml`:
- Around line 88-94: Update the resources under the seed workload’s values
configuration to reflect full per-tenant projection recomputation against
ClickHouse rather than copying the HTTP service defaults. Size CPU and memory
requests and limits for larger tenants, and ensure the batch job’s resource
settings reduce the risk of OOMKilled retries under its existing restart policy.
In `@src/backend/services/identity-resolution/src/infra/db/seed_repo.rs`:
- Around line 81-107: Update tenant_presence to use indexed existence checks for
matching and non-matching insight_tenant_id values instead of aggregating SUM
across persons, while preserving the TenantPresence zero/non-zero contract.
Adjust seed_runner::input_guards operator-facing messaging to describe
presence/existence rather than exact row counts if it exposes those values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 24ec0d2e-9ace-4f8a-a2aa-3af5d9e06996
⛔ Files ignored due to path filters (1)
src/backend/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
.github/workflows/identity-resolution-helm.ymlcharts/insight/templates/secrets.yamlcharts/insight/values.yamlsrc/backend/services/identity-resolution/Cargo.tomlsrc/backend/services/identity-resolution/helm/templates/seed-cronjob.yamlsrc/backend/services/identity-resolution/helm/tests/test_seed_cronjob_contract.pysrc/backend/services/identity-resolution/helm/values.yamlsrc/backend/services/identity-resolution/src/api/mod.rssrc/backend/services/identity-resolution/src/api/seed.rssrc/backend/services/identity-resolution/src/domain/seed_service.rssrc/backend/services/identity-resolution/src/gear.rssrc/backend/services/identity-resolution/src/infra/db/mod.rssrc/backend/services/identity-resolution/src/infra/db/ops_repo.rssrc/backend/services/identity-resolution/src/infra/db/seed_repo.rssrc/backend/services/identity-resolution/src/main.rssrc/backend/services/identity-resolution/src/seed_runner.rssrc/ingestion/tests/e2e/identity/test_error_contracts.pysrc/ingestion/tests/e2e/identity/test_meta_gate.pysrc/ingestion/tests/e2e/identity/test_persons_seed.pysrc/ingestion/tests/e2e/lib/api_coverage.pysrc/ingestion/tests/e2e/lib/identity.py
…ction, EXISTS guard probe, pinned actions (#1690) CodeRabbit (major): SEED_TIMEOUT bounded only the read+pipeline, while the zombie sweep and the journal writes are MariaDB calls under the same advisory lock — a hang there would hold the lock past every next tick. RUN_TIMEOUT (12m) now backstops the whole critical section; a run cut off by it may leave its operations row `running`, which the next run's sweep reclaims, and the chart's activeDeadlineSeconds (900s) stays the final out-of-process backstop. CodeRabbit (nitpick): tenant_presence aggregated over the whole persons table ahead of every seed; the guard only needs zero-vs-non-zero, so it is now two EXISTS probes that short-circuit on idx_tenant_person (TenantPresence carries has_own/has_other booleans, guard message adjusted). Semgrep: the new identity-resolution-helm workflow used mutable action tags; pinned to full SHAs (same convention as semgrep.yml/trivy.yml). Skipped (with reason): the seed Job resources nitpick — the live dev run processed 4,780 accounts / 26,652 input rows in 12s well inside the current requests/limits; revisit if an order-of-magnitude larger tenant appears. Verified: 67 unit tests, clippy -D warnings, fmt; rust-lane e2e 115 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ktursunov
approved these changes
Jul 30, 2026
cyberantonz
reviewed
Jul 30, 2026
|
|
||
| - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 | ||
| with: | ||
| python-version: "3.12" |
Contributor
Author
There was a problem hiding this comment.
Fair catch — the action SHAs were copied from semgrep.yml and lagged behind; bumped to the current releases in ddf9490 (checkout v7.0.1, setup-python v7.0.0). python-version: 3.12 itself is intentional: it's the repo-wide standard (every workflow, the e2e runner image, and requires-python in the e2e pyproject all pin 3.12) — happy to bump repo-wide in a separate PR if we want to move.
The SHAs were copied from semgrep.yml and lagged behind (checkout v5.0.1, setup-python v5.6.0). Pinned to the latest releases instead (checkout v7.0.1, setup-python v7.0.0). python-version stays 3.12 — the repo standard (every workflow, the e2e runner image, and requires-python in the e2e pyproject all pin 3.12). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cyberantonz
reviewed
Jul 30, 2026
| return Err(SeedRunError::LockBusy); | ||
| } | ||
|
|
||
| let result = tokio::time::timeout(RUN_TIMEOUT, run_locked(&db, config, tenant, mode, force)) |
Contributor
There was a problem hiding this comment.
If you crash here, you will have lock held forever. I think it should be done via RAII pattern, i.e. lock held until the variable is in the scope. When you leave scope - the lock automatically removed.
Contributor
Author
There was a problem hiding this comment.
Done in ff8d3af — SeedLockGuard now owns the lock's session (RAII), every exit path releases it. Worth noting: GET_LOCK is session-scoped, so a crash already released it server-side — the guard formalizes that invariant.
cyberantonz
reviewed
Jul 30, 2026
| "unsupported mode '{mode}'; only '{LINK_BY_EMAIL_MODE}' is available" | ||
| ))); | ||
| } | ||
| if config.tenant_default_id.is_empty() { |
Contributor
There was a problem hiding this comment.
It is true for one installation. But at least dev has no default tenant id at all and in perfect world you need to grab all tenants from DB and run for each. Currently you will break multitenancy.
Contributor
Author
There was a problem hiding this comment.
Agreed on direction, but a per-tenant loop is blocked one level lower: the identity_inputs reader reads the whole table with no tenant filter (HOTFIX(#1550) — the dbt producer hashes tenant ids), so each tenant's run would ingest every other tenant's rows. Documented this at the validation site in ff8d3af (grep HOTFIX(#1550) for the full blast radius); the runner below is already per-tenant (lock, journal, writes), so the loop drops in once the producer fix lands.
Contributor
Author
There was a problem hiding this comment.
Follow-up in 6c68bfb: since existing config Secrets (dev included) can't be touched right now, an EMPTY tenant_default_id no longer hard-fails — the runner infers the SOLE distinct tenant from the persons log (WARN-logged); zero or several tenants still refuse with a clear message, so nothing is ever guessed ambiguously. Dev works out of the box with this.
…radius (#2046 review) Review asked for RAII on the advisory lock and for the multi-tenant limitation to be visible at the tenant-validation site. - SeedLockGuard replaces the try_acquire/release pair: the guard OWNS the lock's dedicated single-connection session, so the lock's lifetime is tied to the guard's scope by construction — early return, future cancellation, and process crash all release it via session teardown (GET_LOCK is session-scoped; a stale lock was already impossible, the guard formalizes the invariant against future refactors). Happy path still issues an explicit RELEASE_LOCK for the fastest handover. - The single-tenant contract is now documented AT the tenant validation in seed_runner::run: a true "enumerate tenants, seed each" mode is blocked by the HOTFIX(#1550) reader (whole-table read, no tenant filter — the dbt producer hashes tenant ids), and what unblocks it; the runner below is already per-tenant (lock name, journal, writes). - HOTFIX(#1550) is now a uniform greppable tag across every dependent site (reader anchor declares it; runner, tenant_presence, e2e harness comments carry it) — unwinding the hotfix starts from one grep. Verified: 67 unit tests, clippy, fmt; rust-lane e2e 115 passed on a runner image rebuilt from this tree. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/identity-resolution-helm.yml:
- Line 34: Update the actions/checkout step in the identity resolution workflow
to set persist-credentials to false, preventing the GITHUB_TOKEN from being
stored in local git configuration while preserving the existing checkout action
and pinned revision.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bf95f69e-006b-4a0b-8294-5f40d8c78e46
📒 Files selected for processing (8)
.github/workflows/identity-resolution-helm.ymlcharts/insight/values.yamlsrc/backend/services/identity-resolution/src/infra/db/mod.rssrc/backend/services/identity-resolution/src/infra/db/seed_repo.rssrc/backend/services/identity-resolution/src/infra/identity_inputs.rssrc/backend/services/identity-resolution/src/seed_runner.rssrc/ingestion/tests/e2e/identity/test_persons_seed.pysrc/ingestion/tests/e2e/lib/identity_seed.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/ingestion/tests/e2e/identity/test_persons_seed.py
| steps: | ||
| # Pinned to full SHAs (mutable tags are repointable — supply-chain | ||
| # hardening, same as semgrep.yml / trivy.yml). | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Set persist-credentials: false on checkout.
This job only runs helm lint/pytest and never pushes back to the repo, so persisting the GITHUB_TOKEN in the local git config is unnecessary exposure (flagged by zizmor's artipacked check).
🔒 Proposed fix
- - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+ - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | |
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 34-34: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/identity-resolution-helm.yml at line 34, Update the
actions/checkout step in the identity resolution workflow to set
persist-credentials to false, preventing the GITHUB_TOKEN from being stored in
local git configuration while preserving the existing checkout action and pinned
revision.
Source: Linters/SAST tools
…figured (#2046 review) Existing installs' pre-created config Secrets predate the seed and carry no tenant_default_id (dev is one), and those Secrets cannot be touched right now — as shipped, the CronJob would fail there on every tick. The runner now resolves the tenant instead of hard-requiring it: - a configured tenant_default_id always wins; - an EMPTY config falls back to the SOLE distinct tenant in the persons log (WARN-logged) — writing under the one tenant the data already lives under is exactly what an operator would configure; - zero tenants (fresh install) or several → refuse with an operator-facing message; guessing there would recreate the HOTFIX(#1550) wrong-tenant hazard. resolve_tenant is a pure function with unit tests for every branch; the e2e harness gained tenant=None (leaves the config empty) and an ambiguous-refusal case over the multi-tenant fixture dataset. Verified: 72 unit tests, clippy, fmt; rust-lane e2e 116 passed on a runner image rebuilt from this tree. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mitasovr
approved these changes
Jul 30, 2026
mozhaev-dev
enabled auto-merge
July 30, 2026 07:56
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ingestion/tests/e2e/lib/identity.py (1)
284-308: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClear inherited tenant configuration when
tenant=None.
_rust_env()copies the parent environment, so an inherited tenant override prevents the intended empty-config inference path from being tested.Proposed fix
env = self._rust_env() - if tenant is not None: - env["APP__gears__identity-resolution__config__tenant_default_id"] = tenant + tenant_key = "APP__gears__identity-resolution__config__tenant_default_id" + if tenant is None: + env.pop(tenant_key, None) + else: + env[tenant_key] = tenant🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ingestion/tests/e2e/lib/identity.py` around lines 284 - 308, Update the seed CLI environment setup in the method containing _rust_env() and the tenant_default_id assignment so tenant=None explicitly removes APP__gears__identity-resolution__config__tenant_default_id from env, while preserving the existing override when tenant is provided.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/ingestion/tests/e2e/lib/identity.py`:
- Around line 284-308: Update the seed CLI environment setup in the method
containing _rust_env() and the tenant_default_id assignment so tenant=None
explicitly removes APP__gears__identity-resolution__config__tenant_default_id
from env, while preserving the existing override when tenant is provided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 77e3239e-0c26-4f2d-b4a2-d49946d60163
📒 Files selected for processing (5)
charts/insight/values.yamlsrc/backend/services/identity-resolution/src/infra/db/seed_repo.rssrc/backend/services/identity-resolution/src/seed_runner.rssrc/ingestion/tests/e2e/identity/test_persons_seed.pysrc/ingestion/tests/e2e/lib/identity.py
This was referenced Jul 30, 2026
cyberantonz
added a commit
that referenced
this pull request
Jul 31, 2026
) Manual equivalent of the publish-chart job, which does not yet run on release branches (CI gap, being fixed separately in #2097): pin the branch-built image tags into the subchart appVersions + the toolbox ref, and patch-bump the umbrella version so the chart carrying the identity-resolution seed CronJob template (#2046 backport) can be published and consumed from gitops. - backend subcharts + toolbox: 2026.07.31.06.52-bf09d6a.release-2026.07.1 - frontend subchart: 2026.07.31.06.52-0bb785c.release-2026.07.1 - umbrella: 0.4.68 -> 0.4.70 (appVersion = max subchart appVersion) Signed-off-by: Anton Zelenov <antonz@constructor.tech> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 5, 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
5 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.
Closes #1690.
Problem
The identity org projection (
persons/account_person_map/org_chartin MariaDB) is materialized and was only rebuilt by the persons-seed at bootstrap. Nothing re-ran it, while connectors keep its input (identity.identity_inputsin ClickHouse) fresh daily — so every manager/org change ingested after the last seed run never reached the Team view (527 re-parented people invisible on the reporting Virtuozzo instance).How it works now
The seed becomes a first-class scheduled operation of the identity-resolution service; the HTTP trigger is removed (team decision — the .NET service and its
POST /v1/persons-seedare being deleted together).Exit codes (diagnosable straight from the Job status):
0ok ·1failed ·2another run holds the lock ·3refused by an input guard.Concurrency. Runs serialize on the advisory lock — it lives on the MariaDB server, so a cron Job, a manual Job, and even a second Insight instance sharing the same database all serialize through it, crash-safe by construction (the lock dies with the connection).
concurrencyPolicy: Forbidon the CronJob is belt-and-braces for cron-vs-cron only. A concurrent run fails fast (exit 2) rather than queueing a stale re-run.Input guards (both
--force-overridable, both journaled as afailedoperation soGET /v1/persons-seedexplains why nothing was written):identity_inputsrows means a broken/misconfigured pipeline (wrong ClickHouse URL/database, wiped stand), not "no people";personsholds rows under other tenants and none under the configured one: seeding would mint a parallel person universe under a wrong tenant (the identity: persons-seed silently reads 0 rows — identity_inputs tenant is sipHash-derived but read by the raw tenant #1550 failure mode), which the append-only log cannot undo. A genuinely fresh install (emptypersons) passes.What stays / what goes.
POST /v1/persons-seed, the in-process queue/worker and the 503 queue-full path are gone. The read-only journal routesGET /v1/persons-seedandGET /v1/persons-seed/{id}stay as the observability window over CLI runs. The subcommand paths now install a tracing subscriber — previouslymigratelogged into the void.Helm. The CronJob lives in the identity-resolution subchart (same image/config/secret wiring as the deployment; distinct pod labels so the Service never routes traffic to seed pods). Values:
identityResolution.seed.{enabled, schedule}(+ subchart-onlyseed.tenantDefaultIdthat env-overrides the Secret for standalone installs). The umbrella fails the render when the seed is enabled but no tenant is configured — only on the path where the umbrella composes the config Secret itself (credentials.autoGenerate); pre-created-secret installs are the operator's source of truth.Running it manually
Inside the cluster, from the CronJob (same image/env/config — nothing to remember):
Watching a run / history over HTTP (admin JWT):
Overriding the guards knowingly (e.g. a legitimate first seed while another tenant's data exists):
(or simplest:
kubectl create job --from=cronjob/... seed-force, then edit the Job's args to append--forcebefore creating — the CronJob itself never runs forced.)Tests
--force, fresh-install and steady-state pass), mode/tenant validation, pipeline fakes.src/ingestion/tests/e2e/identity/, both CI lanes):trigger: "cli", empty-input guard (exit 3, journaled), wrong-tenant guard (exit 3, journaled), lock busy (exit 2 against a heldGET_LOCK);identity_inputs(resolvable manager → edge, unresolvable → NULL-parent membership, top-of-tree → Path-B row);parent_emaillands in the inputs → a re-run moves the open edge to the new manager and closes the old one (SCD2 history intact);Verification
cargo fmt/clippy -D warnings/ 67 unit tests green; release binary smoke-tested (seed --help, exit codes).helm lintboth charts; render matrix: umbrella fails fast on enabled-seed-without-tenant, renders withglobal.tenantDefaultId/seed.tenantDefaultId/seed.enabled=false; functional-ci values render unchanged.🤖 Generated with Claude Code
Summary by CodeRabbit