LCORE-1029: dumping configuration with BYOK section

[tisnik]

Description

LCORE-1029: dumping configuration with BYOK section

Type of change

• Refactor
• New feature
• Bug fix
• CVE fix
• Optimization
• Documentation Update
• Configuration Update
• Bump-up service version
• Bump-up dependent library
• Bump-up library or tool used for development (does not change the final image)
• CI configuration change
• Konflux configuration change
• Unit tests improvement
• Integration tests improvement
• End to end tests improvement

Related Tickets & Documents

• Related Issue #LCORE-1029

Summary by CodeRabbit

• New Features
  • Added support for BYOK (Bring Your Own Knowledge) RAG configuration, enabling users to set up custom RAG deployments with their own vector databases and knowledge bases.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds support for a new public ByokRag entity to configuration model exports and introduces a test function that validates serialization and deserialization of configurations containing byok_rag fields with proper sensitive value redaction and default preservation.

Changes

Cohort / File(s)	Summary
Configuration test updates tests/unit/models/config/test_dump_configuration.py	Added ByokRag import; introduced test_dump_configuration_byok() function validating configuration dumping with byok_rag serialization (rag_id, vector_db_id, db_path, embedded fields), sensitive value redaction, and default quota_handlers preservation

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Single test file with straightforward configuration validation logic
• Test follows established patterns from existing configuration tests
• Focus areas: verify the ByokRag export is correct and test assertions match intended schema

Possibly related PRs

• LCORE-642: BYOK RAG configuration #638: Modifies configuration model to introduce and support ByokRag entity with byok_rag field addition and related defaults/constants in the core Configuration class.

Suggested reviewers

• are-ces

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: adding support for dumping configuration with a BYOK (Bring Your Own Key) RAG section, which aligns with the test additions and entity exports in the changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (3)

tests/unit/models/config/test_dump_configuration.py (3)

505-509: Improve docstring to describe BYOK-specific testing.

The docstring is generic and doesn't indicate this test specifically validates BYOK RAG configuration serialization and deserialization.

Consider updating to something like:

 def test_dump_configuration_byok(tmp_path: Path) -> None:
     """
-    Test that the Configuration object can be serialized to a JSON file and
-    that the resulting file contains all expected sections and values.
+    Test that a Configuration with BYOK RAG entries can be serialized to JSON
+    and that the byok_rag section is correctly dumped with all fields including
+    defaults (embedding_model, embedding_dimension, rag_type).
     """

---

510-558: Consider extracting common configuration setup to reduce duplication.

The configuration setup (lines 510-558) is nearly identical to test_dump_configuration and test_dump_configuration_with_quota_limiters, with only the byok_rag parameter being unique. This creates maintenance overhead.

Consider creating a pytest fixture or helper function:

@pytest.fixture
def base_configuration():
    """Fixture providing a base Configuration for testing."""
    return Configuration(
        name="test_name",
        service=ServiceConfiguration(
            tls_config=TLSConfiguration(
                tls_certificate_path=Path("tests/configuration/server.crt"),
                tls_key_path=Path("tests/configuration/server.key"),
                tls_key_password=Path("tests/configuration/password"),
            ),
            cors=CORSConfiguration(
                allow_origins=["foo_origin", "bar_origin", "baz_origin"],
                allow_credentials=False,
                allow_methods=["foo_method", "bar_method", "baz_method"],
                allow_headers=["foo_header", "bar_header", "baz_header"],
            ),
        ),
        # ... rest of common configuration
    )

Then use it with specific overrides:

def test_dump_configuration_byok(tmp_path: Path, base_configuration: Configuration) -> None:
    cfg = base_configuration.model_copy(
        update={"byok_rag": [ByokRag(...)]}
    )
    # ... rest of test

---

663-672: Consider adding test coverage for multiple BYOK RAG entries.

The current test validates a single byok_rag entry. Following the pattern of test_dump_configuration_with_more_mcp_servers, consider adding a test case for multiple BYOK RAG entries to ensure list serialization works correctly.

This would be consistent with the existing test coverage patterns in this file.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 70d6adb and 60e88c1.

📒 Files selected for processing (1)

• tests/unit/models/config/test_dump_configuration.py (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

tests/unit/models/config/test_dump_configuration.py (1)

src/models/config.py (8)

• ByokRag (582-592)
• Configuration (623-649)
• ServiceConfiguration (149-166)
• LlamaStackConfiguration (177-220)
• UserDataCollection (223-256)
• DatabaseConfiguration (108-146)
• InferenceConfiguration (521-538)
• dump (646-649)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: build-pr
• GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
• GitHub Check: e2e_tests (ci)
• GitHub Check: e2e_tests (azure)

🔇 Additional comments (2)

tests/unit/models/config/test_dump_configuration.py (2)

17-17: LGTM!

The import is properly placed and necessary for the new BYOK RAG test coverage.

---

551-557: The test file exists. No issues found.