fix: upgrade OpenSearch to 3.6.0#1972
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughOpenSearch, OpenSearch Dashboards, Netty patching, and role permission ordering are updated across build and security configuration. ChangesOpenSearch 3.6 upgrade and compatibility updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
github-actions
Bot
added
bug
mpawlow
approved these changes
Jun 25, 2026
mpawlow
left a comment
mpawlow
left a comment
Collaborator
There was a problem hiding this comment.
Code Review 1
- ✅ Approved / LGTM 🚀
- See PR comments (1a) and (1d) for Minor nits to optionally address
Functional Review 1
- ✅ Successfully verified sanity ingestion, searching, chatting with newly build OpenSearch image
- Note: I re-used an existing OpenSearch index (no factory reset)
Collaborator
|
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile (1)
12-26: 🩺 Stability & Availability | 🟡 MinorAdd
-fto thecurlcommand that downloads the neural-search plugin to fail fast on HTTP errors.When
curl ... > plugin.ziplacks-f, a 4xx/5xx response still writes the error body to the file; the subsequentopensearch-plugin installthen fails with a cryptic message. Use-fLto surface the HTTP error immediately during the build.Replace line 17 with:
curl -fL -s https://github.com/IBM/neural-search-jvector/releases/download/3.6.0.0/opensearch-neural-search-3.6.0.0.zip \ > /tmp/opensearch-neural-search/plugin.zip🤖 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 `@Dockerfile` around lines 12 - 26, Add fail-fast handling to the neural-search plugin download in the Dockerfile by updating the curl command used in the opensearch-neural-search preparation step. The current download writes HTTP error bodies into plugin.zip, so change the curl invocation to use error-on-HTTP-failure behavior for the neural-search artifact while keeping the existing pipeline and opensearch-plugin install steps intact. Locate the fix in the curl command that populates /tmp/opensearch-neural-search/plugin.zip and ensure it surfaces 4xx/5xx failures immediately during build.
🧹 Nitpick comments (1)
Dockerfile (1)
16-18: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd
-fto the neural-search download so HTTP errors fail fast.Unlike the jvector download (Line 12), which pipes into
tarand fails on bad input, thiscurl -L -s > plugin.zipsucceeds on a 404 and writes the HTML error page intoplugin.zip. The failure then surfaces only atopensearch-plugin install(Line 22) with a confusing "invalid zip" error.🛡️ Proposed fix
RUN mkdir -p /tmp/opensearch-neural-search && \ - curl -L -s https://github.com/IBM/neural-search-jvector/releases/download/3.6.0.0/opensearch-neural-search-3.6.0.0.zip \ + curl -fL -s https://github.com/IBM/neural-search-jvector/releases/download/3.6.0.0/opensearch-neural-search-3.6.0.0.zip \ > /tmp/opensearch-neural-search/plugin.zip
🤖 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 `@cloud_securityconfig/roles.yml`:
- Around line 18-20: The shared ACL test is picking the first matching
permission entry from roles.yml, so splitting the DLS permissions into
admin-only and read/DLS entries can break the test contract. Keep the DLS/read
permission block first in the roles configuration, or update the selector in the
shared ACL test to explicitly target the DLS/read entry instead of the first
match; use the existing permission entries in roles.yml and the ACL test
selector logic to locate the change.
In `@patch-netty.sh`:
- Around line 99-105: The fail-safe in the patch-netty.sh script is using
replaced_count, which only tracks actual replacements, so it can incorrectly
fail builds when whitelisted Netty jars are already at NETTY_VERSION. Add a
separate matched_count that increments whenever a whitelisted artifact is found,
including the already-up-to-date continue path, and change the final check to
fail only when matched_count is zero. Keep the existing replaced_count output
for reporting, and update the logic around the jar-processing loop and the final
fail-safe block accordingly.
In `@securityconfig/roles.yml`:
- Around line 18-20: The DLS/read permissions are now ordered after the
admin-only blocks, which breaks the existing contract in tests that pick the
first permissions entry for documents and openrag principals. Update the roles
definition so the DLS/read block for the documents and knowledge_filters
permissions appears before the corresponding admin-only block, or adjust the
test selectors in test_group_acl.py to find entries by allowed_actions and dls
instead of index. Keep the ordering consistent for the affected permission
groups so the first match remains the read/DLS rule.
---
Outside diff comments:
In `@Dockerfile`:
- Around line 12-26: Add fail-fast handling to the neural-search plugin download
in the Dockerfile by updating the curl command used in the
opensearch-neural-search preparation step. The current download writes HTTP
error bodies into plugin.zip, so change the curl invocation to use
error-on-HTTP-failure behavior for the neural-search artifact while keeping the
existing pipeline and opensearch-plugin install steps intact. Locate the fix in
the curl command that populates /tmp/opensearch-neural-search/plugin.zip and
ensure it surfaces 4xx/5xx failures immediately during build.
🪄 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: 4662e1f0-7bf4-4813-9dbe-94b73180fb40
📒 Files selected for processing (4)
Dockerfilecloud_securityconfig/roles.ymlpatch-netty.shsecurityconfig/roles.yml
github-actions
Bot
added
tests
bug
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
securityconfig/roles.yml (1)
15-15: 🎯 Functional Correctness | 🟠 MajorAvoid keeping the broad
readaction inside DLS blocks.In OpenSearch 3.6.0, the
readaction group includes metadata operations (indices:admin/mappings/fields/get*,indices:admin/resolve/index) in addition to data reads. When granted on an index pattern with adlsfilter, these metadata requests are subject to DLS filtering, which can break admin functionality (e.g., mapping inspection) or expose unexpected empty results. You must splitreadon DLS patterns to include only data-specific actions (e.g.,indices:data/read/search) and grant the metadata actions exclusively on corresponding non-DLS patterns.🤖 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 `@securityconfig/roles.yml` at line 15, The DLS role definition still uses the broad read action group on a pattern with a dls filter, which pulls in metadata operations that should not be filtered. Update the affected role entry in roles.yml to split read into data-only actions for the DLS-scoped index pattern, and grant the metadata-related actions separately on a matching non-DLS pattern. Use the role stanza containing the read action to locate and adjust the permission set.
🤖 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.
Outside diff comments:
In `@securityconfig/roles.yml`:
- Line 15: The DLS role definition still uses the broad read action group on a
pattern with a dls filter, which pulls in metadata operations that should not be
filtered. Update the affected role entry in roles.yml to split read into
data-only actions for the DLS-scoped index pattern, and grant the
metadata-related actions separately on a matching non-DLS pattern. Use the role
stanza containing the read action to locate and adjust the permission set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bc14b8b6-7955-4bf0-8d76-9c0d4f1a32d2
📒 Files selected for processing (4)
cloud_securityconfig/roles.ymlpatch-netty.shsecurityconfig/roles.ymltests/unit/test_group_acl.py
🚧 Files skipped from review as they are similar to previous changes (1)
- cloud_securityconfig/roles.yml
github-actions
Bot
added
bug
github-actions
Bot
added
bug
ricofurtado
pushed a commit
that referenced
this pull request
Jun 30, 2026
* upgrade OpenSearch to 3.6.0 * fixed PR review comments * fix nitpicks
ricofurtado
added a commit
that referenced
this pull request
Jul 1, 2026
* feat: add connectors permissions for Saas (#1782) * feat: add SaaS workspace connector permissions with RBAC gates Introduce admin Connectors Permission settings (cloud-only) backed by workspace_config, enforce availability on connector APIs and OAuth init, and sync the new connectors:manage:access permission at startup without an Alembic migration. OSS/dev OSS brand bypasses policy; add dev role toggle and brand-aware proxy headers for local RBAC testing. * style: ruff autofix (auto) * address CodeRabbit comments and fix backend lint unused imports * fix backend lint * feat: workspace connector permissions with aligned SaaS policy guards Add admin Connectors Permission (workspace_config-backed) with RBAC connectors:manage:access, enforce policy on connector APIs and OAuth init, and keep the permission list independent of the live connectors tab. Consolidate frontend connector-access hooks into useGetConnectorsQuery and settings tab helpers into brand.ts. Fix Connectors Permission tab redirects (RSC dev-brand default, wait for permissionsResolved). Let explicitly enabled connectors override deployment visibility filters; add OPENRAG_DEV_CONNECTOR_POLICY for local OSS dev backend enforcement Snapshot and restore all cached connector query keys in connect/disconnect mutations so optimistic updates stay correct when policy context changes mid-mutation (brand toggle, permissions resolving). * style: ruff autofix (auto) * addressed coderabbit comments * remove mentions of dev only envs in the env.example --------- Co-authored-by: Olfa Maslah <[email protected]> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Edwin Jose <[email protected]> * fix: google to support WEBHOOK_BASE_URL (#1849) * Update connector.py * update env in settings * fix webhook url of sharepoint * fix: make connector webhooks work end-to-end (Drive routing, Graph subscriptions, change discovery) (#1867) * fix: alias legacy /connectors/google/webhook to google_drive and reload connections store on webhook channel lookup miss * Fix Sharepoint and Google Drive * fix: propagate deleted items from SharePoint/OneDrive webhook delta query Main's delete-event coverage (#1852) expects handle_webhook to return deleted file ids so sync_specific_files can run its deleted-at-source cleanup (get_file_content -> 404 -> delete indexed chunks). The delta implementation skipped deleted items; now deleted files propagate and deleted folders stay excluded. * use logging _config * Update test_webhook_type_alias.py * fix: forward end-user JWT to MCP/API callers via gateway-placed X-OpenRAG-API-JWT header (#1874) * use fallback header * Notes on new header * raises http exception for missing jwt * feat: Add shared flag for COS ingestion to allow documents to be indexed without an owner * style: ruff autofix (auto) * Update src/api/connectors.py Improving error return Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat: Implement shared flag for document replacement in OpenSearch queries * style: ruff autofix (auto) * refactor: Remove shared flag from document ingestion options in SDK * feat: Update resolve_shared_owner_fields to return anonymous user details for shared documents * style: ruff autofix (auto) * feat: Add anonymous delete permission for shared documents and update related functionality * feat: Update delete permission to include anonymous access for shared documents * style: ruff autofix (auto) * style: apply ruff autofix * feat: Restrict shared flag usage to ibm_cos connector in sync_connector_files method * fix: clarify behavior of documents with owner=null in DLS tests * fix button types (#1875) * refactor: no derived state (#1886) * no derivied state * coderabbit comments * OPENSEARCH_NODE_COUNT_CHECK_ENABLED as deafult flag (#1975) What changed: - Renamed the env var read by the backend to OPENSEARCH_NODE_COUNT_CHECK_ENABLED in src/config/settings.py. - Updated Docker Compose to pass OPENSEARCH_NODE_COUNT_CHECK_ENABLED in docker-compose.yml. - Added the canonical flag to the example env file in .env.example. - Documented the flag in the OpenSearch config table in docs/docs/reference/configuration.mdx. - Updated the existing readiness test wording in tests/unit/test_opensearch_wait_node_count.py. - Added a regression test that loads config.settings with OPENSEARCH_NODE_COUNT_CHECK_ENABLED=false and verifies the canonical flag is what the code reads in tests/unit/config/ test_opensearch_node_count_check_enabled.py. * chore: Segment improvements (#1971) * chore: Segment improvements (#1963) * Update static properties * and knowledge/setting actions * avoid duplicate events * coderabbit comments * fix: onboarding timeout cleanup issue (#1977) * fix the cleanup timeout issue for the embedding step * final step fix * feat: add shared parameter to connector_sync function * feat: add shared parameter to TaskProcessor and update connector_sync for permission checks * fix: improve response handling in delete_document_endpoint for better status reporting * fix: clear completeTimeoutRef after completion to prevent memory leaks * fix: update OPENSEARCH_NODE_COUNT_CHECK_ENABLED to use fallback from OPENSEARCH_NODE_COUNT_CHECK * fix: onboarding timeout cleanup issue (#1977) * fix the cleanup timeout issue for the embedding step * final step fix * fix: support big csv files on langflow-less ingestion (#1968) * fix max tokens when splitting for langflow-less ingestion * fix coderabbit picks * style: ruff autofix (auto) * added non lf ingestion test --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * fix: upgrade OpenSearch to 3.6.0 (#1972) * upgrade OpenSearch to 3.6.0 * fixed PR review comments * fix nitpicks * Added ibm watsonx ai and langchain ibm to Langflow image (#1989) * OPENSEARCH_NODE_COUNT_CHECK_ENABLED as deafult flag (#1975) What changed: - Renamed the env var read by the backend to OPENSEARCH_NODE_COUNT_CHECK_ENABLED in src/config/settings.py. - Updated Docker Compose to pass OPENSEARCH_NODE_COUNT_CHECK_ENABLED in docker-compose.yml. - Added the canonical flag to the example env file in .env.example. - Documented the flag in the OpenSearch config table in docs/docs/reference/configuration.mdx. - Updated the existing readiness test wording in tests/unit/test_opensearch_wait_node_count.py. - Added a regression test that loads config.settings with OPENSEARCH_NODE_COUNT_CHECK_ENABLED=false and verifies the canonical flag is what the code reads in tests/unit/config/ test_opensearch_node_count_check_enabled.py. * fix: onboarding timeout cleanup issue (#1977) * fix the cleanup timeout issue for the embedding step * final step fix * fix: universal file names (#1982) * ascii support for ingest files * update the flow json * style: ruff autofix (auto) * Update langflow_headers.py * Update test_langflow_ingest_callback.py * Update test-ci.yml * Update test_onboarding_sample_docs.py * style: ruff autofix (auto) * Update test_ascii_safe_header_value.py * Update langflow_file_service.py * re trigger commit * fix: purge config_manager singleton in non-langflow ingestion test The _purge_modules() list was missing "config.config_manager", so the ConfigManager singleton retained its cached _config from a previous test. Subsequent calls to get_openrag_config() returned the old config where disable_ingest_with_langflow=False, causing the router to use the Langflow path even though DISABLE_INGEST_WITH_LANGFLOW=true was set in the environment. Adding config.config_manager to the purge list forces a fresh load on the next import, picking up the correct env-var values. * fix: use hash_id to derive expected document_id in CSV ingestion test DocumentFileProcessor ignores Docling's binary_hash field and computes document_id = hash_id(file_path) from the actual file content. The test was asserting against the literal mock value "sha-csv-integration-123" which never appears in the indexed documents, giving 0 hits and a false failure. Now the test derives expected_document_id via hash_id(csv_path) to match what the production code actually indexes. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --------- Co-authored-by: Wallgau <[email protected]> Co-authored-by: Olfa Maslah <[email protected]> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Edwin Jose <[email protected]> Co-authored-by: Edwin Jose <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Mike Fortman <[email protected]> Co-authored-by: Sebastián Estévez <[email protected]> Co-authored-by: Lucas Oliveira <[email protected]>
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
2 participants
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.
This pull request updates the OpenSearch stack to version 3.6.0 across the Dockerfile, Docker Compose, and Helm chart configurations. The main changes ensure that all OpenSearch-related components and plugins are aligned to the latest compatible version, improving compatibility, security, and feature support.
OpenSearch and Plugin Version Upgrades:
Dockerfilefrom3.2.0to3.6.0.Dockerfileto use the3.6.0.0versions foropensearch-jvector-plugin,opensearch-neural-search, andopensearch-prometheus-exporter.OPENSEARCH_VERSIONbuild argument in theDockerfileto3.6.0.Dashboard Version Alignment:
3.6.0in bothdocker-compose.ymland the Helm chart values file (values.yaml). [1] [2]Closes #1969
Summary by CodeRabbit