Skip to content

Add OCI Generative AI as an embedding provider (Cohere Embed Multilingual via OCI GenAI)#2065

Open
galovics wants to merge 14 commits into
langflow-ai:mainfrom
galovics:feat/oci-genai-embedding-provider
Open

Add OCI Generative AI as an embedding provider (Cohere Embed Multilingual via OCI GenAI)#2065
galovics wants to merge 14 commits into
langflow-ai:mainfrom
galovics:feat/oci-genai-embedding-provider

Conversation

@galovics

@galovics galovics commented Jul 9, 2026

Copy link
Copy Markdown

Fixes #2062

Summary

Adds Oracle Cloud Infrastructure (OCI) Generative AI as a first-class, embedding-only provider, targeting
Cohere Embed Multilingual (oci/cohere.embed-multilingual-v3.0) served through OCI's GenAI service.
litellm==1.84.0 (already an OpenRAG dependency) has dedicated OCI GenAI embedding support
(litellm/llms/oci/embed/transformation.py), so — like the companion Bedrock PR (#2061) — this is additive
wiring on top of an already-embedded dependency, no new SDK required.

  • New OCIConfig (user/fingerprint/tenancy/compartment_id/key or key_file/region) following
    the existing per-provider dataclass pattern, wired through config_manager.py, the settings API
    (including full read/write/removal support — remove_oci_config clears all fields, with the same
    "configure another provider first" guard and embedding-fallback logic other providers have), and
    patched_async_client's credential env-var block.
  • update_model_registry() gets an OCI branch (same fail-fast risk as Bedrock — search_service.py's
    get_litellm_model_name(model, strict=True) raises UnknownEmbeddingProvider on every query without it).
    A dedicated test proves registration→resolution end-to-end.
  • input_type is passed as a plain kwarg (not extra_body — same reasoning as Add AWS Bedrock as an embedding provider (Cohere Embed Multilingual via Bedrock) #2061, this provider
    resolves through a real litellm prefix) at both the query (search_service.py) and ingest
    (processors.py) call sites, extracted into a small shared utils/embedding_kwargs.py helper used by
    both. litellm's OCI transformation maps the lowercase OpenAI-style input_type value to OCI's uppercase
    inputType enum internally, so this PR passes the lowercase value and relies on litellm for that mapping
    rather than re-implementing it.
  • Credential-shape validation requires exactly user/fingerprint/tenancy/compartment_id plus one of
    key/key_file — matching litellm's own validate_environment requirement for this provider, confirmed
    against litellm's source during research rather than guessed.
  • OCI has no Langflow-flow component. Adding oci to the general provider-name list initially leaked into
    langflow_sync.py's Langflow-flow-reapplication loop during development (which has no OCI component and
    would raise) — caught and fixed by introducing a narrower _LANGFLOW_EMBEDDING_PROVIDER_NAMES constant
    scoped to just the providers with a Langflow component, separate from the general embedding-provider list.

Testing

89 new tests: the config dataclass and full settings-API read/write/removal path, credential-shape
validation (missing fields, inline PEM vs. key_file, invalid paths — no live calls), the registry
registration→resolution path end-to-end, input_type as a plain kwarg at both call sites intercepted at the
real production call sites (SearchService.search_tool, TaskProcessor.process_document_standard) with
assertions that non-Cohere models get none of the extra kwargs, the Langflow-allowlist separation described
above, and field-name normalization distinguishing OCI's cohere.embed-multilingual-v3.0 (two dots) from
Bedrock's cohere.embed-multilingual-v3 (one dot, in #2061).

All mocked at the HTTP/embeddings-client layer — no live calls to *.oraclecloud.com anywhere (OCI's
request signing requires a real RSA key pair in an actual tenancy, not available here). Full suite:
uv run pytest tests/unit -q → 973 passed, 13 failed (pre-existing at the base commit, unrelated — identical
failing test names before and after). Zero new failures.

Out of scope / follow-ups

  • Frontend/Settings-UI wiring (a form for OCI's 6 credential fields, including a PEM-upload-or-textarea
    choice) is not included — backend-only. The settings API is fully wired to support it once a form exists.
  • Secret-at-rest: config_manager.py's encrypt-on-save logic only targets the field literally named
    api_key. OCI's sensitive field is key (an inline PEM private key), which is not run through that same
    encryption path. Not a live issue today (env-var-only configuration is the primary path this PR
    optimizes for), but worth addressing — either by nudging the eventual UI toward key_file (a path, not a
    secret payload) over inline key, or extending the encrypt/decrypt logic to cover key too.
  • provider_health_cache's cache key wasn't extended with OCI-specific fields, so a credential change while
    embedding_provider=oci won't bust the short-TTL health cache mid-window. Low impact (affects only the
    health-check banner's freshness, not embedding correctness).
  • Manual verification against a live OCI tenancy (real credentials, real embed call) hasn't been done and
    couldn't be within this PR's constraints — recommended before this ships to users relying on it in
    production.

Related

Companion PRs for the other two Cohere-hosting backends discussed in the tracking issues: #2060 (custom
gateway base URL support) and #2061 (AWS Bedrock). All three are independent and can be reviewed/merged in
any order, though they touch some of the same files (config_manager.py, models_service.py,
settings.py) and may need a rebase if merged out of order.

Update: instance_principal / workload_identity auth methods

Follow-up to the initial api_key-only version of this PR. Adds OCIConfig.auth_method, a
build_oci_signer() factory, and wires it through both embedding call sites, the model-registry gate,
validation/health-checks, and — this final increment — the settings API and docs, so all three OCI auth
methods are configurable via OCI_AUTH_METHOD (env var) or oci_auth_method (settings API / onboarding),
not just api_key.

  • api_key (default, unchanged): OCI's API Signing Key scheme — user/fingerprint/tenancy/key or
    key_file, as before.
  • instance_principal: authenticates as this workload's identity when running on an OCI Compute
    instance. None of user/fingerprint/tenancy/key/key_file apply. Prerequisites (operator-side, not
    satisfiable by this code): the workload must actually run on an OCI Compute instance, that instance must
    belong to a dynamic group, and an IAM policy must grant that dynamic group use generative-ai-family in
    the target compartment.
  • workload_identity: authenticates as this workload's identity as an OKE pod. None of
    user/fingerprint/tenancy/key/key_file apply. Prerequisites: the OKE cluster must have Workload
    Identity enabled, and the pod's service account must be configured for it (standard
    KUBERNETES_SERVICE_HOST-based auto-detection, per OCI's docs).
  • Validation switches from the existing api_key credential-shape check to constructing the real signer
    (oci.auth.signers.InstancePrincipalsSecurityTokenSigner() /
    oci.auth.signers.get_oke_workload_identity_resource_principal_signer()) and surfacing a
    OCISignerConstructionError naming the concrete unmet prerequisite if that fails.
  • While wiring oci_auth_method into the settings API, found and fixed an ordering bug in
    update_settings(): its embedding-provider validation block resolved oci_auth_method from the
    pre-existing config before the new request-body field could override it, so a PATCH /settings that
    changed auth_method would validate against the stale value while persisting the new one. Fixed with a
    regression test that fails without the fix.

Testing limitation, disclosed explicitly: neither instance_principal nor workload_identity can be
integration-tested end-to-end in this environment — there is no real OCI Compute instance and no real OKE
cluster available here. All coverage for these two auth methods is mocked at the SDK boundary
(oci.auth.signers.InstancePrincipalsSecurityTokenSigner /
oci.auth.signers.get_oke_workload_identity_resource_principal_signer are mocked; the real OCI instance
metadata service / OKE workload-identity proxymux is never called). This mirrors the existing api_key
testing limitation noted above (no live calls to *.oraclecloud.com), extended to cover signer construction
too. Manual verification against a real OCI Compute instance and a real OKE cluster with Workload Identity
enabled hasn't been done and couldn't be within this PR's constraints — recommended before either auth
method ships to users relying on it in production.

Summary by CodeRabbit

  • New Features
    • Added OCI as an embedding-only provider, with onboarding and settings support for region, auth method, and credentials (including api_key, instance_principal, workload_identity).
    • Enabled OCI embedding model discovery and a dedicated OCI model-listing API path.
  • Bug Fixes
    • Improved OCI provider health handling and ensured health results refresh when OCI embedding credentials change.
    • Hardened OCI credential validation and safer OCI removal behavior with fallback to other configured providers.
  • Tests
    • Added comprehensive unit coverage for OCI endpoints, validation, settings read/write, and embedding request credential/kwargs behavior.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

This PR adds OCI Generative AI as an embedding provider, including configuration, static model listing, validation and health checks, settings persistence, onboarding, Langflow synchronization limits, and OCI/Cohere embedding-call kwargs.

Changes

OCI Embedding Provider Integration

Layer / File(s) Summary
OCI config and environment loading
src/config/config_manager.py, src/config/settings.py, .env.example, pyproject.toml, tests/unit/config/*
Adds OCI configuration, environment overrides, encrypted inline-key persistence, runtime environment injection, dependency metadata, and related tests.
OCI model listing and registry
src/services/models_service.py, src/api/models.py, src/api/v1/models.py, src/app/routes/internal.py, tests/unit/api/test_models_oci_endpoints.py, tests/unit/services/test_models_service_oci.py
Adds static OCI embedding models, registry gating, model resolution, API dispatch, route wiring, and tests.
OCI validation and health checks
src/api/provider_validation.py, src/api/provider_health.py, src/utils/provider_health_cache.py, src/utils/oci_auth.py, tests/unit/api/*provider*oci.py, tests/unit/utils/test_provider_health_cache.py, tests/unit/utils/test_oci_auth.py
Adds OCI credential-shape and signer-construction validation, health-check credential forwarding, cache-key inputs, and tests.
OCI settings and onboarding
src/api/settings/models.py, src/api/settings/endpoints.py, src/api/settings/helpers.py, src/api/settings/langflow_sync.py, tests/unit/api/test_settings_models_oci.py, tests/unit/test_settings_oci_write_path.py, tests/unit/api/test_langflow_sync_bug_1587.py
Adds OCI settings schemas, persistence and removal, onboarding support, embedding-provider selection, and Langflow update restrictions.
OCI embedding kwargs and call sites
src/utils/embedding_kwargs.py, src/models/processors.py, src/services/search_service.py, tests/unit/utils/test_embedding_kwargs.py, tests/unit/test_processors_oci_embed_kwargs.py, tests/unit/services/test_search_service_oci_embed_kwargs.py, tests/unit/test_embedding_fields.py
Adds Cohere input-type and OCI credential kwargs, signer-aware routing for ingest and search, and model normalization coverage.

Estimated code review effort: 4 (Complex) | ~75 minutes

Possibly related PRs

Suggested labels: enhancement

Suggested reviewers: edwinjosechittilappilly, lucaseduoli, mfortman11, phact

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR misses the linked issue’s frontend settings UI wiring for choosing OCI and entering its credential fields. Add the OCI provider selector and credential fields to the frontend settings UI, or clarify that frontend work is intentionally out of scope.
Out of Scope Changes check ⚠️ Warning The PR adds inline OCI key encryption and provider-health cache key changes, both explicitly called out as out of scope. Remove the OCI key encryption and health-cache key changes, or update the scope if those behaviors are intended.
Docstring Coverage ⚠️ Warning Docstring coverage is 20.21% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding OCI GenAI as an embedding provider.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (2)
src/config/config_manager.py (1)

403-418: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Path instruction: os.getenv() used outside config/settings.py.

The path instruction for src/**/*.py states that os.environ should only be read in config/settings.py. The new OCI env override block follows the pre-existing pattern for all providers, but technically violates this instruction. Consider whether this is an intentional pattern or whether env overrides should be consolidated into settings.py.

As per path instructions, "Config values must come from config/settings.py (the only place os.environ is read); never access os.environ elsewhere in the codebase."

🤖 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/config/config_manager.py` around lines 403 - 418, The new OCI override
block in config_manager is reading environment variables directly, which
conflicts with the rule that only config/settings.py should access os.environ.
Move the OCI env-to-config mapping into config/settings.py and have
config_manager consume the resolved settings values instead; use the existing
provider config structure and keep the OCI fields populated through the settings
layer rather than repeated os.getenv() calls in config_manager.

Source: Path instructions

src/api/provider_health.py (1)

130-142: 🩺 Stability & Availability | 🔵 Trivial

Health cache key omits OCI credential fields.

The provider_health_cache.cache_key(...) call includes api_key, endpoint, and project_id but not the OCI credential fields (oci_user, oci_fingerprint, oci_tenancy, compartment_id, oci_key, oci_key_file). If OCI credentials change while the cache entry is still valid, the health endpoint returns a stale result. The PR objectives note cache freshness as out-of-scope, but this should be tracked.

🤖 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/api/provider_health.py` around lines 130 - 142, The health cache key in
provider_health_cache.cache_key should also include the OCI credential inputs so
cache entries change when those credentials change. Update the cache_key call in
provider_health.py to pass the OCI fields used by the health check (oci_user,
oci_fingerprint, oci_tenancy, compartment_id, oci_key, oci_key_file), keeping
the existing provider, endpoint, and API key inputs intact.
🤖 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/api/provider_validation.py`:
- Around line 898-912: In _test_oci_credential_shape, both OCI key inputs are
accepted silently even though only one should be used; add a warning when
oci_key and oci_key_file are both set so users know one will be ignored. Update
the logic around the existing oci_key / oci_key_file branch to emit a clear
warning before preferring one source, and keep the PEM validation behavior
unchanged.

In `@src/api/settings/langflow_sync.py`:
- Around line 30-37: The explicit embedding-provider path in langflow_sync still
passes OCI into change_langflow_model_value(), which is rejected by the Langflow
allowlist. Update the embedding-provider handling in langflow_sync so
non-Langflow providers like oci are skipped before calling
change_langflow_model_value(), or make that helper safely no-op for providers
outside _LANGFLOW_EMBEDDING_PROVIDER_NAMES. Use the embedding-provider branch
and change_langflow_model_value() as the key places to adjust.

In `@src/config/config_manager.py`:
- Line 246: The OCI provider config is being persisted with an inline PEM
private key in plaintext because `OCIConfig` is created without
`_decrypt_provider` and `save_config_file` only handles `api_key` encryption.
Update the `OCIConfig` handling path in `config_manager.py` so the `key` field
is included in the same encrypt/decrypt flow as other sensitive provider fields,
and ensure both config loading and saving consistently protect inline OCI keys
before writing `config.yaml`.

---

Nitpick comments:
In `@src/api/provider_health.py`:
- Around line 130-142: The health cache key in provider_health_cache.cache_key
should also include the OCI credential inputs so cache entries change when those
credentials change. Update the cache_key call in provider_health.py to pass the
OCI fields used by the health check (oci_user, oci_fingerprint, oci_tenancy,
compartment_id, oci_key, oci_key_file), keeping the existing provider, endpoint,
and API key inputs intact.

In `@src/config/config_manager.py`:
- Around line 403-418: The new OCI override block in config_manager is reading
environment variables directly, which conflicts with the rule that only
config/settings.py should access os.environ. Move the OCI env-to-config mapping
into config/settings.py and have config_manager consume the resolved settings
values instead; use the existing provider config structure and keep the OCI
fields populated through the settings layer rather than repeated os.getenv()
calls in config_manager.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 21f5ff9c-07ea-495a-b1ae-c01406ecd16e

📥 Commits

Reviewing files that changed from the base of the PR and between 8e0f30b and 9c95566.

📒 Files selected for processing (28)
  • .env.example
  • src/api/models.py
  • src/api/provider_health.py
  • src/api/provider_validation.py
  • src/api/settings/endpoints.py
  • src/api/settings/helpers.py
  • src/api/settings/langflow_sync.py
  • src/api/settings/models.py
  • src/api/v1/models.py
  • src/app/routes/internal.py
  • src/config/config_manager.py
  • src/config/settings.py
  • src/models/processors.py
  • src/services/models_service.py
  • src/services/search_service.py
  • src/utils/embedding_kwargs.py
  • tests/unit/api/test_models_oci_endpoints.py
  • tests/unit/api/test_provider_health_oci.py
  • tests/unit/api/test_provider_validation_oci.py
  • tests/unit/api/test_settings_models_oci.py
  • tests/unit/config/test_config_manager_oci.py
  • tests/unit/config/test_settings_oci_credentials.py
  • tests/unit/services/test_models_service_oci.py
  • tests/unit/services/test_search_service_oci_embed_kwargs.py
  • tests/unit/test_embedding_fields.py
  • tests/unit/test_processors_oci_embed_kwargs.py
  • tests/unit/test_settings_oci_write_path.py
  • tests/unit/utils/test_embedding_kwargs.py

Comment thread src/api/provider_validation.py
Comment thread src/api/settings/langflow_sync.py
Comment thread src/config/config_manager.py Outdated
galovics added a commit to galovics/openrag that referenced this pull request Jul 9, 2026
- langflow_sync.py: guard the explicit embedding-provider branch of
  _update_langflow_model_values() with _LANGFLOW_EMBEDDING_PROVIDER_NAMES,
  same as the reapply_all_settings fallback path. This branch was
  unguarded and passed "oci" straight into change_langflow_model_value(),
  which raises ValueError for any provider outside its own hardcoded
  allowlist - breaking the OCI onboarding flow with a 500 and silently
  failing the Langflow embedding sync on every settings save thereafter.
- config_manager.py: encrypt/decrypt OCI's inline PEM key (oci.key) at
  rest, mirroring the existing api_key mechanism. It was previously
  persisted to config.yaml in plaintext (key_file, a path rather than a
  secret, is left untouched).
- provider_validation.py: log a warning when both oci_key and
  oci_key_file are set, since oci_key silently wins (precedence
  unchanged, per .env.example: "provide exactly one of OCI_KEY_FILE or
  OCI_KEY").
- provider_health.py / provider_health_cache.py: include the OCI
  credential fields in the provider-health cache key so a credential
  change while embedding_provider=oci busts the cache like it does for
  every other provider.

Regression tests added for all four fixes.
@galovics

galovics commented Jul 9, 2026

Copy link
Copy Markdown
Author

Re CodeRabbit's os.environ path-instruction finding on config_manager.py: this matches the exact pre-existing pattern used by every other provider (watsonx, openai, ollama) in this same file, so I'm leaving it as-is rather than fixing it only for OCI (inconsistent scope creep otherwise). Happy to take this on as a separate cross-cutting refactor PR if maintainers want it addressed. Same call made on the identical finding in #2064.

galovics added a commit to galovics/openrag that referenced this pull request Jul 9, 2026
BedrockConfig.secret_access_key was written to config.yaml in plaintext
because the encryption-at-rest logic in config_manager.py only handled
fields literally named api_key. Generalize _decrypt_provider, the
needs_encryption_upgrade migration check, and save_config_file's
encrypt-before-write loop to also cover secret_access_key, mirroring
the same fix already applied for OCI's inline key field on PR langflow-ai#2065.

access_key_id is left untouched - per AWS convention it's an
identifier, not a secret.

Updates the existing round-trip test to assert the secret is actually
encrypted on disk (AES-256-GCM) and correctly decrypts back on reload,
instead of asserting the previous plaintext round-trip.
galovics added 4 commits July 9, 2026 20:38
Adds Oracle Cloud Infrastructure Generative AI (Cohere embed models) as a
first-class embedding-only provider, routed entirely through litellm's
existing oci/ integration (litellm==1.84.0, no new SDK dependency).

- config/config_manager.py: OCIConfig dataclass (user/fingerprint/tenancy/
  compartment_id/key/key_file/region/configured), wired into ProvidersConfig,
  get_provider_config(), the file-loader provider list, and env-override
  loading (OCI_USER/OCI_FINGERPRINT/OCI_TENANCY/OCI_COMPARTMENT_ID/
  OCI_KEY_FILE/OCI_KEY/OCI_REGION).
- services/models_service.py: "oci" added to KNOWN_PREFIXES; get_oci_models()
  returns a static Cohere embed model list (no live API round-trip - OCI auth
  requires RSA-SHA256 request signing, too heavy for a model-listing call);
  update_model_registry() gains an OCI branch gated on the full credential
  set being present, so get_litellm_model_name(strict=True) can resolve OCI
  models at query time instead of raising UnknownEmbeddingProvider.
- utils/embedding_kwargs.py (new): shared helpers for the two landmines this
  provider hits once routed through litellm.aembedding() via agentd's patch:
  (1) input_type must be a plain kwarg on .embeddings.create(), keyed off
  "cohere" in the model name so it also covers future non-OCI Cohere routes;
  (2) litellm's OCI integration reads every credential (oci_user, oci_key,
  oci_compartment_id, etc.) from call-time kwargs only - never from
  environment variables, unlike WatsonX. Verified directly against the
  installed litellm source. Wired into both search_service.py (query-time,
  input_type=search_query) and models/processors.py (ingest-time,
  input_type=search_document).
- config/settings.py: OCI credential env-var block mirroring WatsonX (kept
  for consistency/future SDK use, but not sufficient alone per the kwarg
  requirement above).
- api/models.py, api/v1/models.py, app/routes/internal.py: get_oci_models
  endpoint/dispatch wiring (static list, no credentials required).
- api/provider_validation.py, api/provider_health.py: OCI credential-shape
  validation (no live network call - OCI's request signing is too heavy to
  safely exercise here) wired into the existing lightweight-health-check
  dispatch.
- api/settings/models.py, api/settings/helpers.py: oci added to the
  embedding_provider regex validators and _EMBEDDING_PROVIDER_NAMES;
  OCIProviderConfig response model and oci_* request fields mirroring
  WatsonX. api/settings/langflow_sync.py gets its own narrower
  _LANGFLOW_EMBEDDING_PROVIDER_NAMES (excludes oci) since OCI has no
  Langflow flow component and change_langflow_model_value() rejects
  unknown providers.
- .env.example: documents the new OCI_* env vars.

Tests: unit coverage for every file above, including a dedicated proof that
update_model_registry() + get_litellm_model_name(strict=True) resolve an
OCI model end to end (the langflow-ai#1 correctness risk - a registry miss here raises
UnknownEmbeddingProvider on every query), mocked-embeddings-client tests for
both the query and ingest call sites asserting the exact model string,
input_type kwarg, and oci_* credential kwargs, and a normalize_model_name
test locking in that OCI's Cohere model names produce distinct field names.
No live network calls to any cloud provider anywhere in the test suite.
SettingsUpdateBody/OnboardingBody accepted the 7 oci_* fields and
remove_oci_config, but update_settings()/onboarding() had no branch
that applied them to config.providers.oci - only the GET /settings
read path built OCIProviderConfig. A caller POSTing OCI credentials
got a 200 with the values silently discarded.

Mirrors the existing multi-field WatsonX write-path pattern in both
handlers: persist each oci_* field onto current_config.providers.oci,
thread the merged credentials into validate_provider_setup() when OCI
is the embedding provider, mark OCI "configured" once all 6 required
shape fields are present during onboarding, and clear/fall-back on
remove_oci_config the same way remove_watsonx_config does (minus the
LLM-provider fallback, since OCI is embedding-only).
- langflow_sync.py: guard the explicit embedding-provider branch of
  _update_langflow_model_values() with _LANGFLOW_EMBEDDING_PROVIDER_NAMES,
  same as the reapply_all_settings fallback path. This branch was
  unguarded and passed "oci" straight into change_langflow_model_value(),
  which raises ValueError for any provider outside its own hardcoded
  allowlist - breaking the OCI onboarding flow with a 500 and silently
  failing the Langflow embedding sync on every settings save thereafter.
- config_manager.py: encrypt/decrypt OCI's inline PEM key (oci.key) at
  rest, mirroring the existing api_key mechanism. It was previously
  persisted to config.yaml in plaintext (key_file, a path rather than a
  secret, is left untouched).
- provider_validation.py: log a warning when both oci_key and
  oci_key_file are set, since oci_key silently wins (precedence
  unchanged, per .env.example: "provide exactly one of OCI_KEY_FILE or
  OCI_KEY").
- provider_health.py / provider_health_cache.py: include the OCI
  credential fields in the provider-health cache key so a credential
  change while embedding_provider=oci busts the cache like it does for
  every other provider.

Regression tests added for all four fixes.
Upstream commit 4d1be69 (rebased onto) restructured update_settings()
to stage every mutation on a copy.deepcopy(current_config) and only
call save_config_file() once at the end, so a validation failure
partway through the function can no longer leave the in-memory config
half-updated but unsaved. The OCI field handlers (7 oci_* fields plus
remove_oci_config, including its other_providers_configured guard)
were still writing directly to current_config, which would have
bypassed the new atomicity fix for OCI specifically. Switch them to
working_config to match the rest of the function.

Also update tests/unit/test_settings_oci_write_path.py, which asserted
on in-place mutation of the original config object (the pre-fix
pattern). It now asserts on the config captured from save_config_file
and additionally confirms the live config object is left untouched,
mirroring the pattern already used in the new
tests/unit/test_settings_index_name_validation.py.
galovics added a commit to galovics/openrag that referenced this pull request Jul 9, 2026
BedrockConfig.secret_access_key was written to config.yaml in plaintext
because the encryption-at-rest logic in config_manager.py only handled
fields literally named api_key. Generalize _decrypt_provider, the
needs_encryption_upgrade migration check, and save_config_file's
encrypt-before-write loop to also cover secret_access_key, mirroring
the same fix already applied for OCI's inline key field on PR langflow-ai#2065.

access_key_id is left untouched - per AWS convention it's an
identifier, not a secret.

Updates the existing round-trip test to assert the secret is actually
encrypted on disk (AES-256-GCM) and correctly decrypts back on reload,
instead of asserting the previous plaintext round-trip.
@galovics
galovics force-pushed the feat/oci-genai-embedding-provider branch from e30fa11 to 74496d0 Compare July 9, 2026 18:51
galovics added 6 commits July 10, 2026 12:12
oci_credential_kwargs() now accepts an optional signer kwarg: when set
(instance_principal/workload_identity), it's passed through as
oci_signer and the manual user/fingerprint/tenancy/key fields are
omitted; when None (api_key), behavior is unchanged. Both embedding
call sites (search_service.py, processors.py) now build the signer via
build_oci_signer(oci_config.auth_method) before calling it.

update_model_registry()'s OCI gate no longer unconditionally requires
the api_key credential fields -- it also accepts instance_principal/
workload_identity once compartment_id is set, without eagerly
constructing a signer (that would require a live IMDS call on every
registry refresh).

Also updates two pre-existing OCI embed-kwargs tests
(test_search_service_oci_embed_kwargs.py,
test_processors_oci_embed_kwargs.py) whose fake OCIConfig
SimpleNamespace lacked auth_method, which the new call sites now read.
…uction

Adds _test_oci_signer_construction, which validates the two non-api_key
OCI auth methods by attempting real signer construction via
build_oci_signer(), the same way _test_oci_credential_shape validates
api_key by checking credential shape. Threads a new oci_auth_method
parameter through validate_provider_setup / test_lightweight_health /
test_embedding and every call site that already passes oci_key_file.
Adds OCIProviderConfig.auth_method to the GET /settings response and
SettingsUpdateBody.oci_auth_method / OnboardingBody.oci_auth_method to
the write path, so instance_principal/workload_identity can be
selected via the settings API, not just OCI_AUTH_METHOD.

Also closes an ordering bug in update_settings(): its embedding
validation block resolved oci_auth_method from the pre-existing config
before a request-body override existed, so a PATCH changing
auth_method would validate against the stale value while persisting
the new one. Adds the missing pre-validation override alongside the
working_config write, plus a regression test that fails without it.

Updates .env.example's OCI section to document OCI_AUTH_METHOD and its
three modes.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
src/utils/oci_auth.py (1)

37-88: 🚀 Performance & Scalability | 🔵 Trivial

Consider caching the signer at call sites to avoid per-call IMDS/proxymux overhead.

build_oci_signer itself is correct and well-structured. However, both downstream callers (search_service.py and processors.py) construct a new signer on every embedding call. InstancePrincipalsSecurityTokenSigner() typically makes a network call to the OCI Instance Metadata Service in its constructor, so per-query construction adds latency on the hot search path. A cached signer (module-level singleton or config-scoped with token refresh handling) would eliminate repeated IMDS round-trips.

This is an operational observation about the caller pattern, not a defect in build_oci_signer.

🤖 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/utils/oci_auth.py` around lines 37 - 88, Cache the signer at the
downstream call sites rather than reconstructing it for every embedding request.
Update the OCI signer usage in search_service.py and processors.py to reuse a
module-level or configuration-scoped signer, while preserving token refresh
behavior and existing api_key handling; leave build_oci_signer unchanged.
tests/unit/api/test_settings_models_oci.py (1)

57-64: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Add validation for oci_auth_method at the API boundary.

SettingsUpdateBody.oci_auth_method and OnboardingBody.oci_auth_method have no pattern constraint, unlike embedding_provider which validates against ^(openai|watsonx|ollama|oci)$. Invalid values (e.g., "invalid") pass through to build_oci_signer, which raises ValueError — a confusing error for an API consumer expecting a 422. Add a pattern constraint and a test for invalid values.

♻️ Proposed changes (model + test)

In src/api/settings/models.py, for both SettingsUpdateBody and OnboardingBody:

-    oci_auth_method: str | None = None
+    oci_auth_method: str | None = Field(None, pattern="^(api_key|instance_principal|workload_identity)$")

In this test file:

+    def test_settings_update_body_rejects_invalid_oci_auth_method(self):
+        with pytest.raises(ValidationError):
+            SettingsUpdateBody(oci_auth_method="invalid")
+
+    def test_onboarding_body_rejects_invalid_oci_auth_method(self):
+        with pytest.raises(ValidationError):
+            OnboardingBody(oci_auth_method="invalid")

Also applies to: 79-101

🤖 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 `@tests/unit/api/test_settings_models_oci.py` around lines 57 - 64, Add an
API-level pattern constraint to the oci_auth_method fields in SettingsUpdateBody
and OnboardingBody, allowing only the supported OCI authentication methods and
rejecting invalid values with validation errors. Extend the existing tests to
assert that an invalid value such as "invalid" is rejected rather than reaching
build_oci_signer.
🤖 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.

Nitpick comments:
In `@src/utils/oci_auth.py`:
- Around line 37-88: Cache the signer at the downstream call sites rather than
reconstructing it for every embedding request. Update the OCI signer usage in
search_service.py and processors.py to reuse a module-level or
configuration-scoped signer, while preserving token refresh behavior and
existing api_key handling; leave build_oci_signer unchanged.

In `@tests/unit/api/test_settings_models_oci.py`:
- Around line 57-64: Add an API-level pattern constraint to the oci_auth_method
fields in SettingsUpdateBody and OnboardingBody, allowing only the supported OCI
authentication methods and rejecting invalid values with validation errors.
Extend the existing tests to assert that an invalid value such as "invalid" is
rejected rather than reaching build_oci_signer.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 320f0dbc-a2b4-4706-a2d9-c2dc7f87e103

📥 Commits

Reviewing files that changed from the base of the PR and between 74496d0 and 81a4e8e.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (22)
  • .env.example
  • pyproject.toml
  • src/api/provider_health.py
  • src/api/provider_validation.py
  • src/api/settings/endpoints.py
  • src/api/settings/models.py
  • src/config/config_manager.py
  • src/models/processors.py
  • src/services/models_service.py
  • src/services/search_service.py
  • src/utils/embedding_kwargs.py
  • src/utils/oci_auth.py
  • tests/unit/api/test_provider_health_oci.py
  • tests/unit/api/test_provider_validation_oci.py
  • tests/unit/api/test_settings_models_oci.py
  • tests/unit/config/test_config_manager_oci.py
  • tests/unit/services/test_models_service_oci.py
  • tests/unit/services/test_search_service_oci_embed_kwargs.py
  • tests/unit/test_processors_oci_embed_kwargs.py
  • tests/unit/test_settings_oci_write_path.py
  • tests/unit/utils/test_embedding_kwargs.py
  • tests/unit/utils/test_oci_auth.py
🚧 Files skipped from review as they are similar to previous changes (11)
  • tests/unit/test_processors_oci_embed_kwargs.py
  • src/models/processors.py
  • tests/unit/services/test_models_service_oci.py
  • src/services/models_service.py
  • src/config/config_manager.py
  • src/api/settings/models.py
  • tests/unit/services/test_search_service_oci_embed_kwargs.py
  • src/api/provider_health.py
  • tests/unit/config/test_config_manager_oci.py
  • src/services/search_service.py
  • src/api/settings/endpoints.py

galovics added 4 commits July 10, 2026 13:52
CodeRabbit flagged that build_oci_signer() was being reconstructed on
every search/ingest embed call, discarding the IMDS/OKE-proxymux round
trip and the self-refreshing token behavior these signer objects are
designed to have when reused for the process lifetime. Added
get_cached_oci_signer() and switched the two hot-path call sites
(search_service.py, processors.py) to it, keeping build_oci_signer()
itself and the health-check path (provider_validation.py) unchanged so
health checks keep verifying connectivity on every poll.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend 🔷 Issues related to backend services (OpenSearch, Langflow, APIs) community tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add OCI Generative AI as an embedding provider (Cohere Embed Multilingual via OCI GenAI)

1 participant