Skip to content

Conversation

@iromli
Copy link
Contributor

@iromli iromli commented Dec 26, 2025

Prepare


Description

Target issue

closes #12908

Implementation Details


Test and Document the changes

  • Static code analysis has been run locally and issues have been fixed
  • Relevant unit and integration tests have been added/updated
  • Relevant documentation has been updated if any (i.e. user guides, installation and configuration guides, technical design docs etc)

Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with docs: to indicate documentation changes or if the below checklist is not selected.

  • I confirm that there is no impact on the docs due to the code changes in this PR.

Summary by CodeRabbit

  • Bug Fixes
    • Scopes are now initialized via a dedicated upsert process and logged when imported to ensure they are applied reliably.
    • Scopes are excluded from the generic batch import so remaining configuration files continue to be processed as before.

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

… template

Signed-off-by: iromli <isman.firmansyah@gmail.com>
@iromli iromli self-assigned this Dec 26, 2025
@iromli iromli requested a review from moabu as a code owner December 26, 2025 20:34
@mo-auto
Copy link
Member

mo-auto commented Dec 26, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 26, 2025

📝 Walkthrough

Walkthrough

Performs an upsert of scopes.ldif via a dedicated scope_file and self.client.upsert_from_file(...), removes scopes.ldif from the batch LDIF create loop, and retains batch processing for the remaining LDIF files.

Changes

Cohort / File(s) Summary
Scope file handling
docker-jans-config-api/scripts/bootstrap.py
Adds scope_file pointing to /app/templates/jans-config-api/scopes.ldif, calls self.client.upsert_from_file(scope_file) with logging, removes scopes.ldif from the batch LDIF list so scopes are handled separately.
Batch LDIF processing
docker-jans-config-api/scripts/bootstrap.py
Keeps existing loop for remaining LDIF files (config.ldif, clients.ldif, scim-scopes.ldif, testing-clients.ldif) unchanged, ensuring they are created/processed in batch without scopes.ldif.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • moabu
  • duttarnab

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the feature being added to update config-api scopes from templates, which aligns with the main change in the codebase.
Description check ✅ Passed The description follows the template structure with target issue, implementation details, and test documentation checklists. However, it lacks detailed implementation explanation.
Linked Issues check ✅ Passed The PR addresses the objective from issue #12908 by implementing scope upsert functionality to handle updates when templates change, enabling config-api to sync scope modifications.
Out of Scope Changes check ✅ Passed All changes in bootstrap.py are directly related to the objective of updating scopes from templates. The upsert logic and separate scope handling align with requirements.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch cn-config-api-scopes

📜 Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2161c9f and 8a75129.

📒 Files selected for processing (1)
  • docker-jans-config-api/scripts/bootstrap.py
🧰 Additional context used
🧬 Code graph analysis (1)
docker-jans-config-api/scripts/bootstrap.py (1)
jans-pycloudlib/jans/pycloudlib/persistence/sql.py (1)
  • upsert_from_file (986-1013)
🪛 Ruff (0.14.10)
docker-jans-config-api/scripts/bootstrap.py

413-413: Logging statement uses f-string

(G004)

⏰ 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). (8)
  • GitHub Check: docker (fido2)
  • GitHub Check: docker (auth-server)
  • GitHub Check: docker (persistence-loader)
  • GitHub Check: docker (configurator)
  • GitHub Check: docker (cloudtools)
  • GitHub Check: docker (config-api)
  • GitHub Check: docker (scim)
  • GitHub Check: docker (monolith)
🔇 Additional comments (2)
docker-jans-config-api/scripts/bootstrap.py (2)

411-414: LGTM! The upsert approach correctly implements the PR objective.

The switch from create_from_ldif to upsert_from_file for scopes ensures that template changes are reflected in existing scope entries. The added comment clearly explains the rationale for separate handling.


416-416: LGTM! Correctly excludes scopes.ldif from batch processing.

Since scopes are now handled separately via upsert_from_file (line 414), removing scopes.ldif from the batch list is the correct approach.


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 and usage tips.

@mo-auto mo-auto added comp-docker-jans-config-api kind-feature Issue or PR is a new feature request labels Dec 26, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ec72a0f and 2161c9f.

📒 Files selected for processing (1)
  • docker-jans-config-api/scripts/bootstrap.py
🧰 Additional context used
🪛 Ruff (0.14.10)
docker-jans-config-api/scripts/bootstrap.py

411-411: Logging statement uses f-string

(G004)

⏰ 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). (8)
  • GitHub Check: docker (scim)
  • GitHub Check: docker (monolith)
  • GitHub Check: docker (auth-server)
  • GitHub Check: docker (cloudtools)
  • GitHub Check: docker (config-api)
  • GitHub Check: docker (fido2)
  • GitHub Check: docker (configurator)
  • GitHub Check: docker (persistence-loader)
🔇 Additional comments (1)
docker-jans-config-api/scripts/bootstrap.py (1)

414-414: LGTM!

Correctly removed "scopes.ldif" from the batch processing list since it's now handled separately with upsert logic above.

Signed-off-by: iromli <isman.firmansyah@gmail.com>
@moabu moabu merged commit 17d0462 into main Dec 29, 2025
3 checks passed
@moabu moabu deleted the cn-config-api-scopes branch December 29, 2025 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp-docker-jans-config-api kind-feature Issue or PR is a new feature request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(cloud-native): add feature to update config-api scopes sync from template

4 participants