Skip to content

Commit 8fd4de4

Browse files
improve search engine startup verification script
1 parent f5eeedb commit 8fd4de4

6 files changed

Lines changed: 130 additions & 48 deletions

File tree

.dev/docs/search-engine-integration.md

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ The two clients have compatible APIs for the operations Arranger uses (search, m
9494

9595
**Entry point:** `modules/graphql-router/src/graphqlRoutes.ts::arrangerRoutes`
9696

97+
**Note on startup script vs. application permissions:** Before `arrangerRoutes` runs, the container entrypoint (`scripts/ping-elasticsearch.sh`) probes `GET /_cluster/health` to display cluster status. This probe uses the application user's credentials and requires `cluster:monitor/health`. That permission is a startup-script concern only - no application code path calls `/_cluster/health`. See `docs/setup.md § Startup health display` and the roadmap entry "Decouple startup health check from application credential" for the planned fix.
98+
9799
The following happens once per catalogue, in this order, when an `arrangerRoutes` instance starts:
98100

99101
### 1. Alias resolution
@@ -104,13 +106,13 @@ The following happens once per catalogue, in this order, when an `arrangerRoutes
104106
GET /_cat/aliases?format=json
105107
```
106108

107-
> **Required permission:** `indices:admin/aliases/get*` at the **cluster** level.
109+
> **Required permission:** `indices:admin/aliases/get` as an **index-level** permission on `*`.
108110
109-
This is a cluster-wide listing - no index filter is applied. OpenSearch evaluates it against cluster-level permissions. The `cluster_composite_ops_ro` built-in action group includes `indices:admin/aliases/get*` and is the idiomatic way to grant this. See [OpenSearch CAT aliases API](https://docs.opensearch.org/latest/api-reference/cat/cat-aliases/).
111+
Although `cluster_composite_ops_ro` also contains `indices:admin/aliases/get*`, that is a cluster-type action group grant and does not cover direct alias API calls. OpenSearch's static plugin config (`static_action_groups.yml`) classifies `indices:admin/aliases/get` as an index-level permission: the `manage_aliases` built-in action group has `type: "index"` and includes `indices:admin/aliases*`. A direct `GET /_cat/aliases` request resolves to all indices and is evaluated by the index-level privilege evaluator. Granting `indices:admin/aliases/get` only on a specific index pattern (e.g. `analyses-*`) still results in 403 because the request scope is all indices. The permission must be on `*`. See [OpenSearch CAT aliases API](https://docs.opensearch.org/latest/api-reference/cat/cat-aliases/) and [default action groups](https://docs.opensearch.org/latest/security/access-control/default-action-groups/).
110112

111113
The response lists all aliases the user can see. `checkESAlias` scans the list for the configured `esIndex` value. If found, the actual backing index name (e.g. `analyses-1`) is used for all subsequent calls; otherwise `esIndex` is used as-is.
112114

113-
> **Known issue:** `cat.aliases` retrieves all cluster aliases and filters client-side. A targeted `indices.getAlias({ index: esIndex })` call would achieve the same with index-level `indices:admin/aliases/get` only, removing the cluster-level dependency. See tech-debt.
115+
> **Known issue:** `cat.aliases` retrieves all cluster aliases and filters client-side. A targeted `indices.getAlias({ index: esIndex })` call would achieve the same result with `indices:admin/aliases/get` scoped to the data index pattern only, removing the `*` wildcard requirement. See tech-debt.
114116
115117
### 2. Mapping fetch
116118

@@ -240,9 +242,10 @@ All transport actions Arranger can initiate, grouped by phase:
240242
241243
| Phase | API call | Transport action | Minimum grant |
242244
| --------------------------- | ---------------------------- | --------------------------------------------------- | ---------------------------------------------------- |
245+
| Entrypoint script† | `GET /_cluster/health` | `cluster:monitor/health` | cluster-level explicit (startup script only - not application code) |
243246
| Startup: detection | `GET /` | `cluster:monitor/main` | cluster-level explicit, or set `SEARCH_ENGINE` |
244247
| Startup: detection fallback | `GET /_nodes/_local` | `cluster:monitor/nodes/info` | cluster-level explicit (not needed if `GET /` works) |
245-
| Startup: alias resolution | `GET /_cat/aliases` | `indices:admin/aliases/get*` | `cluster_composite_ops_ro` (cluster-level) |
248+
| Startup: alias resolution | `GET /_cat/aliases` | `indices:admin/aliases/get` | index-level on `*` (explicit; `cluster_composite_ops_ro` does not cover direct alias API calls) |
246249
| Startup: mapping fetch | `GET /<index>/_mapping` | `indices:admin/mappings/get` | explicit on data index |
247250
| Startup: sets check | `HEAD /<setsIndex>` | `indices:admin/exists` | `manage` on sets index |
248251
| Startup: sets creation | `PUT /<setsIndex>` | `indices:admin/create`, `indices:admin/mapping/put` | `manage` on sets index |
@@ -252,15 +255,15 @@ All transport actions Arranger can initiate, grouped by phase:
252255
| saveSet: collect IDs | `POST /<dataIndex>/_search` | `indices:data/read/search` | `read` on data index |
253256
| saveSet: write set | `PUT /<setsIndex>/_doc/<id>` | `indices:data/write/index` | `write` on sets index |
254257
258+
† `cluster:monitor/health` is called by `scripts/ping-elasticsearch.sh` before the Node.js process starts. The application itself never calls `/_cluster/health`. This permission can be omitted if the startup display is not needed; startup still succeeds. See roadmap: "Decouple startup health check from application credential".
259+
255260
**Notes:**
256261
257262
- `read` = `indices:data/read*` + `indices:admin/mappings/fields/get*` + `indices:admin/resolve/index`. Source: [OpenSearch default action groups](https://docs.opensearch.org/latest/security/access-control/default-action-groups/).
258-
- `cluster_composite_ops_ro` = `mget` + `msearch` + `mtv` + `aliases/exists*` + `aliases/get*` + `scroll` + `resolve/index`. Source: same link.
263+
- `cluster_composite_ops_ro` = `mget` + `msearch` + `mtv` + `aliases/exists*` + `aliases/get*` + `scroll` + `resolve/index`. Source: same link. Although the group contains `indices:admin/aliases/get*`, granting it does not cover `GET /_cat/aliases`: that API is evaluated by the index-level privilege evaluator. The cluster-type grant applies only during cluster-coordination operations such as internal alias routing in mget/msearch.
259264
- `manage` = `indices:monitor/*` + `indices:admin/*`. Covers all admin operations including `exists` and `create`.
260265
- Permission names are identical for OpenSearch and Elasticsearch; both security plugins share the same transport action naming (OpenSearch forked the security plugin from the Elasticsearch codebase).
261266
262-
The `cluster_composite_ops_ro` action group is also included in the `readall_and_monitor` built-in role, which OpenSearch uses as the reference pattern for "read everything with cluster monitoring". Arranger's permission needs are a subset of that role.
263-
264267
---
265268
266269
## External references

.dev/roadmap.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,27 @@ _Needs design before implementation. Treat as a blocking design question for any
586586

587587
## Deployment
588588

589+
### Decouple startup health check from application credential
590+
591+
`scripts/ping-elasticsearch.sh` calls `GET /_cluster/health` using the application user's credentials (`ES_USER`/`ES_PASS`). This forces `cluster:monitor/health` to be granted to the application role even though no application code path ever calls `/_cluster/health`. The permission exists solely so a shell script can display a status block on startup.
592+
593+
This violates the principle of least privilege: the application user's role carries a permission it never exercises. Any operator following the documented minimum permissions will grant `cluster:monitor/health` to their search engine user indefinitely, with no indication that it is a startup-script concern rather than an application requirement.
594+
595+
**Correct fix:**
596+
597+
Move the readiness gate into a Kubernetes init container that runs with its own, more privileged credential, separate from the application's runtime credential:
598+
599+
- **Init container:** runs `ping-elasticsearch.sh` (or its successor) with an elevated credential granted `cluster:monitor/main` + `cluster:monitor/health`. It retries `GET /_cluster/health?wait_for_status=yellow&...` until the cluster is ready or the container times out, gating the pod's main container start via the normal init-container mechanism. The cluster status block (cluster name, status, node count, shards) stays as this container's log output, visible via `kubectl logs <pod> -c <init-name>`, and never needs to be a concern for the app container.
600+
- **Main container:** the application runs with a leaner credential, `cluster:monitor/main` only, which is all it needs for `GET /` engine auto-detection. It never holds `cluster:monitor/health`.
601+
602+
This preserves the `wait_for_status=yellow` readiness gate (true cluster-ready signal, not just HTTP connectivity) while still removing the unused permission from the long-running application process: the actual least-privilege violation the rest of this item is about.
603+
604+
A simpler alternative (drop the readiness gate, probe `GET /` only, no init container) was considered and rejected: it trades a real readiness check for a permissions fix, and `GET /` only confirms the process is responding, not that shards are allocated.
605+
606+
**Related:** the script filename (`ping-elasticsearch.sh`) and env var names (`ES_HOST`, `ES_USER`, `ES_PASS`) are also Elasticsearch-first and should be renamed in the same effort. See tech-debt: "Elasticsearch-first naming in startup script and env vars".
607+
608+
_Standalone: yes, once the approach is agreed. Needs a new Vault role/policy for the init container's elevated credential, a second VSO-injected secret, and an `initContainers` stanza added to the Helm chart, in addition to script changes. If the Helm chart lives in a separate repo from this one, that work belongs there. No application logic changes._
609+
589610
### Helm chart update
590611

591612
_Priority: medium. Maintenance burden for production deployments._

.dev/sessions.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ Newest first.
88

99
## 2026-06-30
1010

11-
Fact-checked the search engine permissions reference against the authoritative OpenSearch source (`static_action_groups.yml`); corrected two mistakes introduced in the 2026-06-28 session.
11+
Fact-checked the search engine permissions reference against the authoritative OpenSearch source (`static_action_groups.yml`); corrected two mistakes introduced in the 2026-06-28 session; corrected a further mistake about alias resolution permission level, verified against live cluster behaviour and OpenSearch static plugin config.
1212

1313
- `modules/graphql-router/src/searchClient/index.ts`: fixed wrong permission name in the dual-403 error message: `cluster:monitor/nodes_info` is not a real permission; the correct transport action is `cluster:monitor/nodes/info`
1414
- `docs/setup.md`: (1) corrected the core search section: `read` does not cover `indices:admin/mappings/get` or `indices:admin/aliases/get`; both must be granted explicitly alongside `read`; (2) fixed `cluster:monitor/nodes_info``cluster:monitor/nodes/info` in the auto-detection table; (3) updated the summary table data index column to list all three required permissions
@@ -24,6 +24,13 @@ Fact-checked the search engine permissions reference against the authoritative O
2424
- `modules/types/src/configs/constants.ts`, `modules/graphql-router/src/config/utils/index.ts`, `modules/graphql-router/src/graphqlRoutes.ts`, `apps/search-server/src/configs/fromEnv/localEnvs.ts`, `apps/search-server/src/configs/fromEnv/aggregator.ts`, `integration-tests/server/test/index.test.ts`, `integration-tests/mcp-server/test/index.test.ts`, `apps/search-server/.env.schema`, `modules/graphql-router/README.md`, `.dev/roadmap.md`, `.dev/tech-debt.md`: renamed `DISABLE_SETS`/`disableSets` to `ENABLE_SETS`/`enableSets`; Sets now defaults to disabled and requires explicit opt-in
2525
- `modules/graphql-router/src/config/utils/index.ts`, `modules/graphql-router/src/graphqlRoutes.ts`: fixed multicatalog race condition: `resource_already_exists_exception` during `initializeSets` is now treated as success; Sets initialization failure no longer takes down the catalogue's GraphQL endpoint
2626
- `modules/graphql-router/src/config/utils/index.test.ts`: added tests for the race condition path (exception treated as success) and non-race errors (still propagate)
27+
- `scripts/ping-elasticsearch.sh`: when `SEARCH_ENGINE` is not set, the script now detects the engine type after the cluster is ready by probing `GET /` and checking for `"distribution":"opensearch"`; the "Ready" banner and all subsequent output use the correct label; mirrors Arranger's own Stage 1 auto-detection logic
28+
- `.dev/docs/search-engine-integration.md`, `docs/setup.md`: corrected alias resolution permission: `indices:admin/aliases/get` is an index-level permission (not cluster-level); OpenSearch's `manage_aliases` built-in group is `type: "index"` in `static_action_groups.yml`; `GET /_cat/aliases` is evaluated by the index-level privilege evaluator against all indices and requires the permission on `*`; `cluster_composite_ops_ro`'s `indices:admin/aliases/get*` grant does not cover direct alias API calls; removed `cluster_composite_ops_ro` from the summary table cluster column (it was listed there solely as the alias resolution grant, which was incorrect)
29+
- `scripts/ping-elasticsearch.sh`: fixed two bugs: (1) the retry loop used `> /dev/null` so curl exit code 0 was treated as success even on 403, making the "ready" check a pure TCP check with no HTTP validation; now checks for a 2xx response code using `-w '%{http_code}'` piped to grep; (2) the health display silently showed blank fields and ❓ when `/_cluster/health` returned 403, with no indication of why; now shows a clear message with the HTTP code and the permission name to add; also uses `SEARCH_ENGINE` env var for the label so the output says "OpenSearch" instead of "Elasticsearch" when configured accordingly; requires `cluster:monitor/health` on the reader role for the retry to succeed and for full health display
30+
- `.dev/docs/search-engine-integration.md`, `docs/setup.md`: added `cluster:monitor/health` to startup sequence, permission reference table, and user-facing setup guide; clearly marked as startup-script only (not application code); added footnote distinguishing it from application-level permissions
31+
- `.dev/roadmap.md`: added "Decouple startup health check from application credential" to the Deployment section; describes why the current coupling is wrong and what the correct fix is (`GET /` via `cluster:monitor/main` for liveness; remove `cluster:monitor/health` dependency)
32+
- `scripts/ping-elasticsearch.sh`: removed mutational engine-label logic (placeholder assignment in a `case`, conditionally overwritten later based on a string-equality check); engine detection is now a single-assignment `engine_label()` function; `if [ $? -ne 0 ]` replaced with `if ! command; then`
33+
- `.dev/roadmap.md`: rewrote "Decouple startup health check from application credential" with an init-container design: elevated credential (`cluster:monitor/health`) confined to an init container that owns the `wait_for_status=yellow` readiness gate and the cluster-status log output; main container runs with `cluster:monitor/main` only; scope note updated to flag the new Vault role/policy, VSO secret, and Helm `initContainers` work this requires
2734

2835
---
2936

.dev/tech-debt.md

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,15 @@ The preferred pattern is **(B)**. Mixing the two makes it harder to find tests,
9191
**Fix:** Audit the monorepo and move all `__tests__/` test files to be co-located with their source file, following pattern (B). Update any Jest/node:test config glob patterns that rely on `__tests__/` directory discovery.
9292
**Standalone:** yes; mechanical file moves plus config glob updates, no logic changes
9393

94+
### Elasticsearch-first naming in startup script and env vars
95+
96+
**Files:** `scripts/ping-elasticsearch.sh`; env vars `ES_HOST`, `ES_USER`, `ES_PASS` set by the chart
97+
**Severity:** low (misleading branding; confusing for operators using OpenSearch)
98+
**Kind:** terminology / naming
99+
**Issue:** The startup readiness script is named `ping-elasticsearch.sh` and prints "Elasticsearch Ready" regardless of the configured engine. The env vars exposed by the chart (`ES_HOST`, `ES_USER`, `ES_PASS`) carry the "ES" prefix even when connecting to OpenSearch. The display label in the script has been updated to derive from `SEARCH_ENGINE` (outputs "OpenSearch", "Elasticsearch", or "Search Engine"), but the script filename and chart env var names remain Elasticsearch-first.
100+
**Fix:** Rename `ping-elasticsearch.sh` to `ping-search-engine.sh` (or `ping-cluster.sh`) and update the reference in the Dockerfile/entrypoint. Coordinate with the chart to rename `ES_HOST`, `ES_USER`, `ES_PASS` to engine-neutral names (`SEARCH_HOST`, `SEARCH_USER`, `SEARCH_PASS` or similar). Both changes require a coordinated release since the chart and image must agree on env var names.
101+
**Standalone:** no; script rename is trivially standalone, but env var rename requires a matching chart release
102+
94103
### Inconsistent spelling of `catalogue`
95104

96105
**File:** throughout the monorepo
@@ -274,11 +283,11 @@ When Arranger Server (`apps/search-server`) is updated to use `catalogue`, the M
274283
### `fetchMapping` uses `cat.aliases` instead of `indices.getAlias`
275284

276285
**File:** `modules/graphql-router/src/searchClient/fetchMapping.ts` (`getESAliases`)
277-
**Severity:** low (over-privileged; unnecessary cluster-level dependency)
286+
**Severity:** low (over-privileged; requires `*` index permission for alias lookup)
278287
**Kind:** privilege minimization
279-
**Issue:** `getESAliases` calls `esClient.cat.aliases({ format: 'json' })` with no index filter, retrieving ALL cluster aliases and doing client-side filtering. `GET /_cat/aliases` is a cluster-wide API that requires `indices:admin/aliases/get*` at the cluster level (provided by `cluster_composite_ops_ro`). A targeted `indices.getAlias({ index: esIndex })` call would achieve the same result with only index-level `indices:admin/aliases/get` on the specific index, removing the cluster-level dependency entirely.
280-
**Fix:** Replace `esClient.cat.aliases()` + `checkESAlias` with `esClient.indices.getAlias({ index: esIndex })`. If the alias exists, the response contains the backing index name; if not, handle the 404. This reduces the required cluster permissions for core search (no `cluster_composite_ops_ro` needed just for alias resolution; `cluster:monitor/main` or `SEARCH_ENGINE` bypass becomes the only cluster-level concern).
281-
**Standalone:** yes; confined to `fetchMapping.ts`; update the caller in `fetchMapping` and adjust the docs/setup.md permissions reference when done
288+
**Issue:** `getESAliases` calls `esClient.cat.aliases({ format: 'json' })` with no index filter, retrieving ALL cluster aliases and doing client-side filtering. `GET /_cat/aliases` evaluates `indices:admin/aliases/get` as an index-level permission (OpenSearch `manage_aliases` group is `type: "index"` in the static plugin config) against all indices; the permission must be granted on `*` because the request is unscoped. A targeted `indices.getAlias({ index: esIndex })` call makes a scoped request, so the permission need only be granted on the data index pattern.
289+
**Fix:** Replace `esClient.cat.aliases()` + `checkESAlias` with `esClient.indices.getAlias({ index: esIndex })`. If the alias exists, the response contains the backing index name; if not, handle the 404. The `indices:admin/aliases/get` permission on `*` can then be removed from the role and scoped down to the data index pattern.
290+
**Standalone:** yes; confined to `fetchMapping.ts`; update `docs/setup.md` and `.dev/docs/search-engine-integration.md` permission tables when done
282291

283292
### No unit tests for `getESAliases` alias resolution
284293

0 commit comments

Comments
 (0)