[RORDEV_2109] reproduction#104
Open
coutoPL wants to merge 2 commits into
Open
Conversation
This comment was marked as spam.
This comment was marked as spam.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@ror-demo-cluster/conf/es/readonlyrest.yml`:
- Around line 14-32: The demo ACL credentials are inconsistent: the rules in
readonlyrest.yml only match user1:test and user2:test, while test-remote-ccs.sh
authenticates as user:test, so the intended Test remote/local allow and forbid
paths never trigger. Update the relevant ACL entries in the readonlyrest
configuration to use the same username as the script, and verify the matching
rules still separate the remote and local cases through the existing Test
remote, Test remote (forbid), Test local, and Test local (forbid) blocks.
- Around line 14-16: The “Test remote” ACL block currently groups all index
patterns under a single rule, but the probes in test-remote-ccs.sh are
differentiated by the test header value. Update the readonlyrest.yml rule by
adding a headers matcher keyed on the test header and split the index patterns
into separate ACL entries so each test:$x case maps to its own rule. Keep the
existing auth_key on the Test remote rule and use the rule name plus headers
matcher to locate the right block.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65aebc77-aa69-432b-8f0e-f9faf67dc4cd
📒 Files selected for processing (4)
ror-demo-cluster/.env-showcaseror-demo-cluster/conf/es/readonlyrest.ymlror-demo-cluster/docker-compose.ymlshared/init-scripts/init.sh
💤 Files with no reviewable changes (1)
- ror-demo-cluster/docker-compose.yml
Comment on lines
+14
to
+16
| - name: "Test remote" | ||
| auth_key: user1:test | ||
| indices: ["*:asdasd", "*:*asdasd", "*:asdasd*", "*:*asdasd*"] |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files ==\n'
git ls-files 'ror-demo-cluster/conf/es/readonlyrest.yml' 'ror-demo-cluster/*test-remote-ccs.sh' 'ror-demo-cluster/**/test-remote-ccs.sh' 'ror-demo-cluster/**readonlyrest*' | sed 's#^`#-` #'
printf '\n== readonlyrest.yml (selected) ==\n'
cat -n ror-demo-cluster/conf/es/readonlyrest.yml | sed -n '1,120p'
printf '\n== search for test header / test-remote-ccs ==\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' 'test-remote-ccs|test:\$|headers|indices:' ror-demo-clusterRepository: beshu-tech/ror-sandbox
Length of output: 2068
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== test-remote-ccs.sh (selected) ==\n'
cat -n ror-demo-cluster/test-remote-ccs.sh | sed -n '1,120p'
printf '\n== headers matcher / test header references in repo ==\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' 'headers:|test:\$x|test:\$|header.*test|X-.*test' ror-demo-clusterRepository: beshu-tech/ror-sandbox
Length of output: 2932
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== es-remote readonlyrest config ==\n'
cat -n ror-demo-cluster/conf/es-remote/readonlyrest.yml | sed -n '1,220p'
printf '\n== all readonlyrest configs mentioning Test remote or headers ==\n'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' 'Test remote|headers:|header:' ror-demo-cluster/confRepository: beshu-tech/ror-sandbox
Length of output: 1019
Split the Test remote rule by test header
ror-demo-cluster/conf/es/readonlyrest.yml:14-16 groups all four index patterns under one ACL block, but ror-demo-cluster/test-remote-ccs.sh uses test:$x to distinguish the probes. Without a headers matcher, every request hits the same rule set, so the per-pattern cases can’t be exercised independently.
🤖 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 `@ror-demo-cluster/conf/es/readonlyrest.yml` around lines 14 - 16, The “Test
remote” ACL block currently groups all index patterns under a single rule, but
the probes in test-remote-ccs.sh are differentiated by the test header value.
Update the readonlyrest.yml rule by adding a headers matcher keyed on the test
header and split the index patterns into separate ACL entries so each test:$x
case maps to its own rule. Keep the existing auth_key on the Test remote rule
and use the rule name plus headers matcher to locate the right block.
Comment on lines
+14
to
+32
| - name: "Test remote" | ||
| auth_key: user1:test | ||
| indices: ["*:asdasd", "*:*asdasd", "*:asdasd*", "*:*asdasd*"] | ||
|
|
||
| - username: user1 | ||
| - name: "Test remote (forbid)" | ||
| type: | ||
| policy: forbid | ||
| response_message: "test" | ||
| auth_key: user1:test | ||
| groups: | ||
| - id: "EndUsers" | ||
| name: "End Users" | ||
| - id: "BusinessUsers" | ||
| name: "Business Users" | ||
|
|
||
| - username: user2 | ||
| - name: "Test local" | ||
| auth_key: user2:test | ||
| groups: | ||
| - id: "EndUsers" | ||
| name: "End Users" | ||
| indices: ["asdasd", "*asdasd", "asdasd*", "*asdasd*"] | ||
|
|
||
| - username: "*" | ||
| ror_kbn_auth: | ||
| name: "kbn1" | ||
| groups: ["*"] | ||
| groups: | ||
| - local_group: | ||
| id: "EndUsers" | ||
| name: "End Users" | ||
| external_group_ids: [ "extEndUsers" ] | ||
| - local_group: | ||
| id: "BusinessUsers" | ||
| name: "Business Users" | ||
| external_group_ids: [ "extBusinessUsers" ] | ||
| - name: "Test local (forbid)" | ||
| type: | ||
| policy: forbid | ||
| response_message: "test" | ||
| auth_key: user2:test |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the credentials used across the CCS demo files.
rg -n --no-heading -e 'auth_key' ror-demo-cluster/conf/es/readonlyrest.yml
rg -n --no-heading -e '-u [a-zA-Z0-9]+:' ror-demo-cluster/test-remote-ccs.sh ror-demo-cluster/conf/es-remote/init-remote-indices.shRepository: beshu-tech/ror-sandbox
Length of output: 748
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant ACL block with line numbers.
sed -n '1,120p' ror-demo-cluster/conf/es/readonlyrest.yml | cat -n
printf '\n---\n'
# Inspect the test script around the CCS calls.
sed -n '1,120p' ror-demo-cluster/test-remote-ccs.sh | cat -n
printf '\n---\n'
# Search for any other ACL/auth_key entries that could match user:test.
rg -n --no-heading 'auth_key:|policy: forbid|name: "Admins"|name: "Test ' ror-demo-cluster/conf/es/readonlyrest.ymlRepository: beshu-tech/ror-sandbox
Length of output: 3958
Align the demo credentials
test-remote-ccs.sh authenticates as user:test, but these ACLs only allow user1:test and user2:test. As written, the script won’t match the intended Test remote/local rules, so the allow/forbid paths never get exercised.
Use the same username on both sides.
🤖 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 `@ror-demo-cluster/conf/es/readonlyrest.yml` around lines 14 - 32, The demo ACL
credentials are inconsistent: the rules in readonlyrest.yml only match
user1:test and user2:test, while test-remote-ccs.sh authenticates as user:test,
so the intended Test remote/local allow and forbid paths never trigger. Update
the relevant ACL entries in the readonlyrest configuration to use the same
username as the script, and verify the matching rules still separate the remote
and local cases through the existing Test remote, Test remote (forbid), Test
local, and Test local (forbid) blocks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Running
cd ror-demo-cluster && ./clean.sh && ./run.shYou can change ROR and ES/KBN versions in the
.env-showcasefile.Summary by CodeRabbit
New Features
Bug Fixes
Chores