Skip to content

Conversation

@tisnik
Copy link
Contributor

@tisnik tisnik commented Sep 3, 2025

Description

LCORE-632: config diagram update

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-632

Summary by CodeRabbit

  • New Features

    • Added validation for regular expression patterns in role-rule configuration, providing immediate feedback on invalid patterns and improving reliability of access controls.
  • Documentation

    • Updated configuration docs/diagrams to reflect the new regex validation behavior.
    • Added guidance and examples for writing acceptable regex patterns in role-rule settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 3, 2025

Walkthrough

Adds two public members to JwtRoleRule: a compiled_regex attribute and a check_regex_pattern() -> Self method, reflected in docs/config.puml. No other changes.

Changes

Cohort / File(s) Summary
UML docs update
docs/config.puml
Updated class diagram for src.models.config.JwtRoleRule to include new public attribute compiled_regex and method check_regex_pattern() -> Self; no removals or other member changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant JR as JwtRoleRule
  participant RE as Regex Engine

  Dev->>JR: check_regex_pattern()
  activate JR
  JR->>RE: compile(pattern)
  alt compile succeeds
    RE-->>JR: compiled_regex
    JR-->>Dev: Self (with compiled_regex)
  else compile fails
    RE-->>JR: error
    JR-->>Dev: raise/return error state
  end
  deactivate JR

  note right of JR: New method validates regex and stores compiled_regex
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • Regenerated config diagram #473 — Introduces JwtRoleRule with jsonpath/roles methods; this PR extends the same class with compiled_regex and a regex validation method.

Poem

I twitched my whiskers, typed with glee,
A regex now compiles for me!
JwtRoleRule hums, patterns tight,
Check, confirm, then hop to flight.
In docs it’s drawn, precise and spry—
Regex carrots, quantified. 🥕✨

✨ Finishing Touches
🧪 Generate unit tests
  • 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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

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

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

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: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f5a90a0 and ffa078f.

⛔ Files ignored due to path filters (2)
  • docs/config.png is excluded by !**/*.png
  • docs/config.svg is excluded by !**/*.svg
📒 Files selected for processing (1)
  • docs/config.puml (1 hunks)
⏰ 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). (2)
  • GitHub Check: build-pr
  • GitHub Check: e2e_tests

username_claim : str
}
class "JwtRoleRule" as src.models.config.JwtRoleRule {
compiled_regex
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Mark compiled_regex as derived/transient and add an explicit type.

If this is a runtime cache of a compiled regex (not user-supplied config), document it as derived and avoid implying it’s part of the input schema. Also add the type for clarity.

Apply in-diagram tweak:

-  compiled_regex
+  /compiled_regex : Pattern[str]

Optionally add a note to the class (outside this line range):

note right of src.models.config.JwtRoleRule
  /compiled_regex is a derived cache (compiled from `value` when operator == 'regex');
  not user-configurable and not persisted.
end note

Run to verify the underlying model treats this as non-serialized (e.g., PrivateAttr or Field(exclude=True)):


🏁 Script executed:

#!/bin/bash
# Inspect model and field treatment
rg -nP -C3 'class\s+JwtRoleRule\b' --type=py
rg -nP -C2 '\bcompiled_regex\b' --type=py
rg -nP -C2 'PrivateAttr|Field\([^)]*exclude\s*=\s*(True|1)\)' --type=py

Length of output: 2451


🏁 Script executed:

#!/bin/bash
# Locate ConfigurationBase definition to confirm model base class behavior
rg -nP 'class\s+ConfigurationBase' -C5 src/models/config.py

Length of output: 303


Mark compiled_regex as a derived property in the UML and specify its return type.

The compiled_regex property is implemented with @cached_property and is not part of the Pydantic model fields (it isn’t serialized); reflect this in the diagram:

-  compiled_regex
+  /compiled_regex : Pattern[str]

Optional:

note right of src.models.config.JwtRoleRule
  /compiled_regex is a derived cache (compiled from `value` when operator == 'regex');
  not user-configurable and not persisted.
end note
🤖 Prompt for AI Agents
In docs/config.puml around line 76, the UML currently shows compiled_regex as a
normal attribute but it is a @cached_property (not serialized) — mark it as a
derived property and indicate its return type (e.g., java.util.regex.Pattern or
simply Pattern). Update the JwtRoleRule class attribute to use a derived marker
(e.g., /compiled_regex or <<derived>> compiled_regex: Pattern) so it’s clear
it’s computed/not persisted; optionally add the provided PlantUML note to the
right of src.models.config.JwtRoleRule explaining it’s a cached/derived value
compiled from `value` when operator == 'regex' and not user-configurable or
persisted.

roles : list[str]
value : Any
check_jsonpath() -> Self
check_regex_pattern() -> Self
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Clarify behavior of check_regex_pattern() and ensure it’s represented in code.

Document that it validates the pattern and populates /compiled_regex (or raises on invalid). If it only applies when operator == 'regex', state that in a note.

Run to confirm existence and expected usage:


🏁 Script executed:

#!/bin/bash
# Method presence and basic behavior hints
rg -nP -C3 'def\s+check_regex_pattern\s*\(' --type=py
# Look for compilation and caching
rg -nP -C2 're\.compile\(|compiled_regex' --type=py
# Optional: where it's invoked in config validation
rg -nP -C3 'check_regex_pattern\s*\(' --type=py

Length of output: 4140


Update docs/config.puml to clarify check_regex_pattern validation and compiled_regex behavior

  • Under check_regex_pattern() (at docs/config.puml:83), note that it uses re.compile() to validate the pattern and raises ValueError on invalid input, and only runs when operator == JsonPathOperator.MATCH (source code)
  • Add documentation for the compiled_regex cached property: returns a compiled Pattern[str] for MATCH operators (None otherwise)
🤖 Prompt for AI Agents
In docs/config.puml around line 83, update the check_regex_pattern()
documentation to state that it validates the regex by calling re.compile() and
will raise a ValueError for invalid patterns, and that this validation is only
performed when operator == JsonPathOperator.MATCH; also add a note for the
compiled_regex cached property explaining it returns a compiled Pattern[str]
when the operator is MATCH and None for all other operators.

@tisnik tisnik merged commit ddbe64d into lightspeed-core:main Sep 3, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant