Keep service auth PostgreSQL-owned in ConfigMap mode

[jiaenren]

Description

Issue - None

ConfigMap mode currently drops service_auth from the in-memory snapshot when the field is omitted from the ConfigMap. Every subsequent ServiceConfig construction then invokes the default factory and generates a new RSA signing identity. Tokens minted by one request can therefore fail validation against JWKS returned by the next request, which prevents backend listeners and workers from authenticating.

This change keeps the JWT signing identity PostgreSQL-owned while leaving ConfigMap data authoritative for all other managed configuration:

• ConfigMap-supplied service.service_auth is discarded before secret resolution and cannot override the persisted identity;
• every new watcher hydrates the identity from PostgreSQL, even when an older global snapshot exists;
• reloads reuse only the watcher-local identity originally derived from PostgreSQL;
• loads fail closed if PostgreSQL does not provide a stable identity.

The change does not generate or persist new signing keys and does not hydrate any other ConfigMap-managed field from PostgreSQL.

Validation

• bazel test //src/service/core/config/tests:test_configmap_loader_unit --test_output=errors (93 tests passed; target also ran mypy)
• Regression coverage verifies cold-start PostgreSQL hydration, ConfigMap override rejection before secret-file access, and DB-derived identity reuse across reloads.
• Live dev instance ConfigMap-mode verification using the initial fix confirmed stable JWKS keys, connected Isaac HIL listener/worker streams, successful queue synchronization, and healthy Argo applications.

Local testcontainer-backed integration targets could not start because this machine has no Docker socket; they run in the supported GitHub CI environment instead.

Checklist

• I am familiar with the Contributing Guidelines.
• New or existing tests cover these changes.
• No user-facing documentation change is required.

Summary by CodeRabbit

Bug Fixes

• Service authentication is now restored from persisted configuration when it is missing from a ConfigMap.
• Persisted authentication is retained across configuration reloads and takes precedence over ConfigMap-provided values.
• Configuration validation now requires service authentication and reports a permanent failure when no valid source is available.
• Configuration startup and reconciliation now consistently support persisted authentication.

Documentation

• Startup behavior and authentication fallback handling have been clarified.

[coderabbitai]

📝 Walkthrough

Walkthrough

ConfigMapWatcher now restores service_auth from Postgres on the first load and preserves it across reloads. Runtime validation requires the hydrated authentication. Tests cover hydration, precedence, failure, startup, reconciliation, and integration paths.

Changes

Service authentication hydration

Layer / File(s)	Summary
Hydration and runtime validation src/service/core/config/configmap_loader.py	ConfigMapWatcher retains the Postgres connector, hydrates service_auth from cached or persisted configuration, documents the fallback behavior, and validates authentication after hydration.
Hydration behavior tests src/service/core/config/tests/test_configmap_loader_unit.py	Tests cover Postgres hydration, reload preservation, Postgres precedence over ConfigMap authentication, new-watcher hydration, and failure when no authentication source exists.
Reconciliation and startup integration coverage src/service/core/config/tests/test_configmap_loader_unit.py, src/service/core/config/tests/test_configmap_loader_integration.py	Startup, reload, reconciliation, pool-resolution, retry, cleanup, non-API, and integration tests use Postgres-backed service authentication.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ConfigMapWatcher
  participant Postgres
  participant RuntimeValidation
  ConfigMapWatcher->>ConfigMapWatcher: Load ConfigMap and resolve other secrets
  ConfigMapWatcher->>Postgres: Read persisted service_auth on first load
  ConfigMapWatcher->>ConfigMapWatcher: Reuse cached service_auth on reload
  ConfigMapWatcher->>RuntimeValidation: Validate hydrated configuration

Loading

Possibly related PRs

• NVIDIA/OSMO#1264: Modifies the same ConfigMap loader and tests for persisted service-auth hydration.
• NVIDIA/OSMO#1266: Modifies related ConfigMap service-auth hydration and preservation behavior.

Suggested labels: external

Suggested reviewers: vvnpn-nv, aruns-nv

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: keeping service authentication owned by PostgreSQL in ConfigMap mode.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jiaenr/fix-configmap-service-auth-stability

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

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 `@src/service/core/config/configmap_loader.py`:
- Around line 298-301: Update _load_and_apply() around the Postgres
get_service_configs() hydration path to catch connector read exceptions, record
the failure, and return LoadResult.TRANSIENT_FAILURE so cold starts can retry.
Preserve validation-failure handling for invalid persisted authentication data,
and add a unit test configuring get_service_configs.side_effect to verify the
transient result and recorded failure.

🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a8239961-7bb0-47ed-849e-9d7da714be89

📥 Commits

Reviewing files that changed from the base of the PR and between ad89291 and 9647930.

📒 Files selected for processing (2)

• src/service/core/config/configmap_loader.py
• src/service/core/config/tests/test_configmap_loader_unit.py

[codecov]

Codecov Report

❌ Patch coverage is 85.71429% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.04%. Comparing base (02b98ae) to head (b128e4a).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
src/service/core/config/configmap_loader.py	85.71%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1267      +/-   ##
==========================================
+ Coverage   67.00%   67.04%   +0.04%     
==========================================
  Files         203      203              
  Lines       26109    26156      +47     
  Branches     3952     3963      +11     
==========================================
+ Hits        17494    17536      +42     
- Misses       7854     7856       +2     
- Partials      761      764       +3     

Flag	Coverage Δ
backend	69.65% <85.71%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/service/core/config/configmap_loader.py	81.84% <85.71%> (+0.10%)	⬆️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[cypres]

Why is "JWT signing identity" PostgreSQL-owned ?
Sounds like something we should move to a Kubernetes secret if it's secret, and config map if not secret

[cypres]

This fixes a problem related to changes in #1155