Skip to content

chore(search): allow explicit index true#2144

Merged
gbirman merged 1 commit intomainfrom
gab/chore/allow-explicit-index-true
Mar 24, 2026
Merged

chore(search): allow explicit index true#2144
gbirman merged 1 commit intomainfrom
gab/chore/allow-explicit-index-true

Conversation

@gbirman
Copy link
Copy Markdown
Contributor

@gbirman gbirman commented Mar 24, 2026

the configure slow logs scripts uses _all so we need this setting back to its default. we don't really require this extra safety layer atm

@gbirman gbirman requested a review from a team as a code owner March 24, 2026 16:08
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 24, 2026

Walkthrough

The OpenSearch domain configuration in the infrastructure stack was modified to enable explicit index handling in multi-action requests. Specifically, the rest.action.multi.allow_explicit_index advanced setting within the OpenSearch domain configuration was changed from disabled to enabled. This represents a single-line modification to the domain settings without alterations to resource dependencies, exports, or overall logic flow.

Poem

🐰 Bounce, bounce, hop with glee!
An index flag now set to be,
From false to true, a rabbit's cheer,
OpenSearch explicit, crystal clear!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: enabling the explicit index setting in OpenSearch configuration.
Description check ✅ Passed The description explains the rationale for the change: slow logs scripts require this setting and the safety layer is not needed currently.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch gab/chore/allow-explicit-index-true

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

@github-actions github-actions bot added the infra label Mar 24, 2026
Copy link
Copy Markdown
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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@infra/stacks/opensearch/index.ts`:
- Around line 148-151: The global advancedOptions entry
'rest.action.multi.allow_explicit_index': 'true' should be gated behind a
stack/config toggle instead of enabled for every client; add a boolean config
flag (e.g., allowExplicitIndex or enable_multi_explicit_index) defaulting to
false and use that flag when building the advancedOptions object in this file so
the key is only added when true, and update the slow-log helper
(configure_slow_logs.ts) or the stack deployment invocation to read/enable this
flag so the slow-log script runs with the explicit-index permission only when
the stack opt-in is set.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c1785626-9f55-432a-ae56-f02340762b13

📥 Commits

Reviewing files that changed from the base of the PR and between 980df33 and d118afb.

📒 Files selected for processing (1)
  • infra/stacks/opensearch/index.ts

Comment on lines 148 to 151
advancedOptions: {
// when set to false, bulk operations requires more specific index targeting for safety, preventing accidental operations across multiple indices.
'rest.action.multi.allow_explicit_index': 'false',
'rest.action.multi.allow_explicit_index': 'true',
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== OpenSearch wildcard/multi-action usage =="
rg -n -C2 -g '!**/node_modules/**' -g '!**/dist/**' \
  '\b_all\b|/_all\b|_bulk\b|_msearch\b|_delete_by_query\b|_update_by_query\b|allow_explicit_index'

echo
echo "== Pulumi stack config related to this setting =="
fd '^Pulumi\..*\.ya?ml$' -t f -x sh -c '
  echo "-- {} --"
  rg -n "opensearch-allow-explicit-index|allow_explicit_index" "{}" || true
'

Repository: macro-inc/macro

Length of output: 50371


🏁 Script executed:

cat -n infra/stacks/opensearch/index.ts | head -200

Repository: macro-inc/macro

Length of output: 7768


🏁 Script executed:

# Search for actual OpenSearch operations, not DB bulk operations
rg -n "opensearch|OpenSearch" infra/stacks/opensearch/index.ts -A 5 -B 5

Repository: macro-inc/macro

Length of output: 5035


🏁 Script executed:

# Look for scripts or operations that might use _all
rg -n "slowlog|slow.log|script|maintenance" infra/stacks/opensearch/index.ts

Repository: macro-inc/macro

Length of output: 486


🏁 Script executed:

# Check if config infrastructure exists for stack-specific settings
fd -t f "\.ts$" infra/ | xargs rg -l "getBoolean|config.*stack" | head -20

Repository: macro-inc/macro

Length of output: 860


🏁 Script executed:

# Look for OpenSearch client library usage
rg -n "opensearch|OpenSearch" rust/ --type rust -l | head -20

Repository: macro-inc/macro

Length of output: 1314


🏁 Script executed:

# Check how config.getBoolean is used elsewhere
rg -n "config\.getBoolean|config\.get\(" infra/stacks/ -A 1 | head -40

Repository: macro-inc/macro

Length of output: 2100


🏁 Script executed:

# Search for actual client operations with _all or bulk operations
rg -n "_all|_bulk|_msearch" --type ts --type js infra/ rust/

Repository: macro-inc/macro

Length of output: 547


🏁 Script executed:

cat -n infra/stacks/opensearch/helpers/scripts/configure_slow_logs.ts

Repository: macro-inc/macro

Length of output: 2653


🏁 Script executed:

# Check how this script is invoked
rg -n "configure_slow_logs" infra/ -B 3 -A 3

Repository: macro-inc/macro

Length of output: 41


Gate this setting via config instead of enabling globally for all clients.

The 'rest.action.multi.allow_explicit_index': 'true' setting is required only by the slow-log configuration script at infra/stacks/opensearch/helpers/scripts/configure_slow_logs.ts. Rather than enabling _all operations for every client, make this an explicit stack-aware toggle so production defaults to restrictive behavior.

Suggested hardening diff
+const allowExplicitIndex =
+  config.getBoolean('opensearch-allow-explicit-index') ??
+  (stack !== 'prod');
+
 ...
     advancedOptions: {
       // when set to false, bulk operations requires more specific index targeting for safety, preventing accidental operations across multiple indices.
-      'rest.action.multi.allow_explicit_index': 'true',
+      'rest.action.multi.allow_explicit_index': allowExplicitIndex ? 'true' : 'false',
     },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@infra/stacks/opensearch/index.ts` around lines 148 - 151, The global
advancedOptions entry 'rest.action.multi.allow_explicit_index': 'true' should be
gated behind a stack/config toggle instead of enabled for every client; add a
boolean config flag (e.g., allowExplicitIndex or enable_multi_explicit_index)
defaulting to false and use that flag when building the advancedOptions object
in this file so the key is only added when true, and update the slow-log helper
(configure_slow_logs.ts) or the stack deployment invocation to read/enable this
flag so the slow-log script runs with the explicit-index permission only when
the stack opt-in is set.

@gbirman gbirman merged commit cc8467c into main Mar 24, 2026
25 checks passed
@gbirman gbirman deleted the gab/chore/allow-explicit-index-true branch March 24, 2026 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant