-
Notifications
You must be signed in to change notification settings - Fork 215
Add a generic, extensible rest-endpoint provider SPI #5656
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
noCharger
wants to merge
25
commits into
opensearch-project:main
Choose a base branch
from
noCharger:feature/ppl-rest-framework
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
a64354c
[Feature] Add PPL `rest` command (Calcite system row source)
noCharger 7580a51
Align rest '/_cluster/settings' redaction with native endpoint; CI fi…
noCharger ba90548
Address rest command bot-review findings; register rest doctest
noCharger cc58b4c
Harden decodeRestSpec: reject non-rest-source tokens with a clear error
noCharger eed9621
Fix rest explain IT (JSON payload) and harden cluster-settings/state …
noCharger 7af15b0
Rest command: analytics-engine coexistence IT + hardened source-token…
noCharger 7a79709
[Bugfix] Keep rest command on Calcite path under cluster-composite
noCharger f83b14b
[Refactor] Unify system-index and rest scans behind one catalog table
noCharger 9afadf8
[Feature] Add rest command response redaction and endpoint allow-list
noCharger f2aeb5f
[Refactor] Remove unused REST/TIMEOUT rules from shared language grammar
noCharger b0eff8e
[Test] Add rest command security integration tests
noCharger 0ed1f36
[Bugfix] Make rest redaction and allow-list settings node-level
noCharger dab07e9
Enhance redaction logic
noCharger 387ed50
[Change] Disable all rest endpoints by default (empty allow-list)
noCharger 591fd8b
Add a generic, extensible rest-endpoint provider SPI (with a /_cluste…
noCharger 7077b81
Merge remote-tracking branch 'origin/main' into feature/ppl-rest-fram…
noCharger ac3f194
Remove dead code from the PPL rest framework
noCharger 7637f4d
Redaction: replace the per-facet RedactionClass registry with a singl…
noCharger 7b63421
Trim rest javadoc/comments for concision; mark rest 3.9 in the PPL co…
noCharger 65d3f86
Apply spotless formatting to rest javadoc
noCharger 8d371a8
rest: skip a duplicate endpoint name with a WARN instead of failing s…
noCharger 19818d4
rest: trim stale test allow-list settings to the one registered endpoint
noCharger 6a4e66b
rest: add a ppl-rest-spi README for endpoint contributors
noCharger 0ff4abc
rest: enumerate the supported endpoints in the resolve() rejection me…
noCharger 39950e3
Address rest-command review feedback: allowed_endpoints as single sou…
noCharger File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
25 changes: 25 additions & 0 deletions
25
core/src/main/java/org/opensearch/sql/ast/tree/RestRelation.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package org.opensearch.sql.ast.tree; | ||
|
|
||
| import java.util.Collections; | ||
| import lombok.EqualsAndHashCode; | ||
| import lombok.ToString; | ||
| import org.opensearch.sql.ast.expression.UnresolvedExpression; | ||
|
|
||
| /** | ||
| * Extend Relation to mark a {@code rest} leading command. The single table name is a reserved, | ||
| * encoded token (produced by {@link org.opensearch.sql.utils.SystemIndexUtils#restTable}) that | ||
| * carries the validated REST endpoint spec; it resolves through the storage engine to a REST source | ||
| * table on the Calcite path, exactly as {@link DescribeRelation} resolves to a system index. | ||
| */ | ||
| @ToString | ||
| @EqualsAndHashCode(callSuper = false) | ||
| public class RestRelation extends Relation { | ||
| public RestRelation(UnresolvedExpression tableName) { | ||
| super(Collections.singletonList(tableName)); | ||
| } | ||
| } |
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| # rest | ||
|
|
||
| The `rest` command is a leading command that reads an allow-listed, read-only in-cluster management endpoint and emits the response as PPL rows. Its rows come from the endpoint dispatch, not from an index, so `rest` appears at the start of a query. | ||
|
|
||
| > **Note**: The `rest` command is supported only on the Calcite query engine (`plugins.calcite.enabled=true`). Each endpoint has a fixed output schema, and the dispatch runs under the caller's security context, so a user who cannot call an endpoint directly cannot call it through `rest`. The command is read-only; mutating and non-allow-listed endpoints are rejected. Each endpoint requires the same cluster-monitor privilege as calling it natively, so `rest` grants no extra access. | ||
|
|
||
| The `rest` command is a generic, extensible framework: a plugin contributes additional read-only endpoints through the `RestEndpointProvider` extension point without changing the grammar. This first version ships a single built-in endpoint, `/_cluster/health`. Additional endpoints (for example `/_cat/nodes`, `/_cat/shards`, `/_cluster/state`, `/_cluster/settings`) can be added in follow-ups, together with optional response redaction applied centrally at the endpoint choke point. | ||
|
|
||
| ## Enabling the command | ||
|
|
||
| `/_cluster/health` is **enabled by default**: `plugins.ppl.rest.allowed_endpoints` defaults to `["/_cluster/health"]`. Any other endpoint is rejected until a deployment adds it to the allow-list (a node-level setting, applied at node startup and not changeable at runtime): | ||
|
|
||
| ```yaml | ||
| plugins.ppl.rest.allowed_endpoints: ["/_cluster/health"] | ||
| ``` | ||
|
|
||
| Every endpoint must be listed explicitly by name; there is no wildcard, so a newly installed or upgraded provider is never enabled without an explicit allow-list change. Set an empty list to disable the command entirely. | ||
|
|
||
| ## Syntax | ||
|
|
||
| ```syntax | ||
| rest <endpoint-path> [count=<int>] [<get-arg>=<value> ...] | ||
| ``` | ||
|
|
||
| ## Parameters | ||
|
|
||
| | Parameter | Required/Optional | Description | | ||
| | --- | --- | --- | | ||
| | `<endpoint-path>` | Required | An allow-listed, read-only endpoint path (see the allow-list below), for example `/_cluster/health`. | | ||
| | `count=<int>` | Optional | Caps the number of emitted rows. | | ||
| | `<get-arg>=<value>` | Optional | Endpoint query arguments, validated per endpoint by both key and value (for example `local=true` for `/_cluster/health`). | | ||
|
|
||
| ## Allow-list | ||
|
|
||
| `rest` resolves only an explicit, curated set of read-only endpoints. Anything outside the list, including any mutating endpoint, is rejected with a clear error. | ||
|
|
||
| | Endpoint | Output columns | Accepted args | | ||
| | --- | --- | --- | | ||
| | `/_cluster/health` | `cluster_name` (string), `status` (string), `number_of_nodes` (integer), `number_of_data_nodes` (integer), `active_primary_shards` (integer), `active_shards` (integer), `relocating_shards` (integer), `initializing_shards` (integer), `unassigned_shards` (integer), `timed_out` (boolean) | `local` | | ||
|
|
||
| ## Example: Counting the nodes in the cluster | ||
|
|
||
| The following query reads cluster health and projects a column that is deterministic on a single-node cluster: | ||
|
|
||
| ```ppl | ||
| | rest '/_cluster/health' | fields number_of_nodes | ||
| ``` | ||
|
|
||
| The query returns the following results: | ||
|
|
||
| ```text | ||
| fetched rows / total rows = 1/1 | ||
| +-----------------+ | ||
| | number_of_nodes | | ||
| |-----------------| | ||
| | 1 | | ||
| +-----------------+ | ||
| ``` | ||
|
|
||
| `/_cluster/health` also exposes `status`, `active_shards`, and the other columns listed in the allow-list. The `rest` row source composes with downstream `where`, `sort`, `stats`, and `fields` exactly like an index scan, for example `| rest '/_cluster/health' | where status = 'green' | fields status, active_shards`. |
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -387,6 +387,8 @@ testClusters { | |
| plugin(getGeoSpatialPlugin()) | ||
| plugin ":opensearch-sql-plugin" | ||
| setting "plugins.query.datasources.encryption.masterkey", "1234567812345678" | ||
| // Only /_cluster/health is registered; pin the allow-list to it for the rest ITs. | ||
| setting 'plugins.ppl.rest.allowed_endpoints', '/_cluster/health' | ||
| } | ||
| yamlRestTest { | ||
| testDistribution = 'archive' | ||
|
|
@@ -405,6 +407,8 @@ testClusters { | |
| testDistribution = 'archive' | ||
| plugin(getJobSchedulerPlugin()) | ||
| plugin ":opensearch-sql-plugin" | ||
| // Only /_cluster/health is registered; pin the allow-list to it for RestCommandSecurityIT. | ||
| setting 'plugins.ppl.rest.allowed_endpoints', '/_cluster/health' | ||
| } | ||
| remoteIntegTestWithSecurity { | ||
| testDistribution = 'archive' | ||
|
|
@@ -419,6 +423,8 @@ testClusters { | |
| plugin(getArrowFlightRpcPlugin()) | ||
| plugin(getAnalyticsEnginePlugin()) | ||
| plugin ":opensearch-sql-plugin" | ||
| // Composite-default cluster: PPL queries route to the analytics engine unless excluded. | ||
| setting 'cluster.pluggable.dataformat', 'composite' | ||
|
Comment on lines
+426
to
+427
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR should not releated to analytics engine? |
||
| } | ||
| } | ||
|
|
||
|
|
||
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
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
94 changes: 94 additions & 0 deletions
94
integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLRestIT.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package org.opensearch.sql.calcite.remote; | ||
|
|
||
| import static org.opensearch.sql.util.MatcherUtils.rows; | ||
| import static org.opensearch.sql.util.MatcherUtils.schema; | ||
| import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; | ||
| import static org.opensearch.sql.util.MatcherUtils.verifySchema; | ||
|
|
||
| import java.io.IOException; | ||
| import org.json.JSONObject; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.opensearch.client.ResponseException; | ||
| import org.opensearch.sql.ppl.PPLIntegTestCase; | ||
|
|
||
| /** | ||
| * Integration tests for the {@code rest} leading command on the Calcite path. This first version | ||
| * ships a single built-in endpoint, {@code /_cluster/health} (a deterministic single-row endpoint | ||
| * that carries no network identifiers and needs no redaction). These tests exercise it end to end | ||
| * and verify the allow-list and per-arg gates. | ||
| */ | ||
| public class CalcitePPLRestIT extends PPLIntegTestCase { | ||
|
|
||
| @Override | ||
| public void init() throws Exception { | ||
| super.init(); | ||
| enableCalcite(); | ||
| } | ||
|
|
||
| @Test | ||
| public void testRestClusterHealthSchema() throws IOException { | ||
| JSONObject result = executeQuery("| rest '/_cluster/health' | fields status, number_of_nodes"); | ||
| verifySchema(result, schema("status", "string"), schema("number_of_nodes", "int")); | ||
| } | ||
|
|
||
| @Test | ||
| public void testRestClusterHealthDataRows() throws IOException { | ||
| // Single-node test cluster: exactly one node. | ||
| JSONObject result = executeQuery("| rest '/_cluster/health' | fields number_of_nodes"); | ||
| verifyDataRows(result, rows(1)); | ||
| } | ||
|
|
||
| @Test | ||
| public void testRestClusterHealthComposesDownstream() throws IOException { | ||
| // The rest row source composes with downstream stats exactly like an index scan. | ||
| JSONObject result = executeQuery("| rest '/_cluster/health' | stats count() as cnt"); | ||
| verifyDataRows(result, rows(1)); | ||
| } | ||
|
|
||
| @Test | ||
| public void testRestClusterHealthLocalArg() throws IOException { | ||
| // local=true reads health from the local node; on a single-node cluster the row is unchanged. | ||
| JSONObject result = | ||
| executeQuery("| rest '/_cluster/health' local='true' | fields number_of_nodes"); | ||
| verifyDataRows(result, rows(1)); | ||
| } | ||
|
|
||
| @Test | ||
| public void testRestRejectsNonAllowListedEndpoint() { | ||
| // /_cat/nodes is not registered in this version; it is refused before any transport call. | ||
| assertRestBadRequest("| rest '/_cat/nodes'", "allow-list"); | ||
| } | ||
|
|
||
| @Test | ||
| public void testRestRejectsEmptyEndpoint() { | ||
| assertRestBadRequest("| rest ''", "non-empty path"); | ||
| } | ||
|
|
||
| @Test | ||
| public void testRestRejectsDisallowedArg() { | ||
| assertRestBadRequest("| rest '/_cluster/health' h='name'", "does not accept arg"); | ||
| } | ||
|
|
||
| @Test | ||
| public void testRestRejectsNegativeCount() { | ||
| assertRestBadRequest("| rest '/_cluster/health' count=-1", "non-negative"); | ||
| } | ||
|
|
||
| /** | ||
| * Assert a {@code rest} query is refused as a client error: HTTP 400 (not a 500 system error) | ||
| * with the given substring in the response body. Covers allow-list and bad-argument rejection. | ||
| */ | ||
| private void assertRestBadRequest(String query, String expectedSubstring) { | ||
| ResponseException e = | ||
| org.junit.Assert.assertThrows(ResponseException.class, () -> executeQuery(query)); | ||
| org.junit.Assert.assertEquals(400, e.getResponse().getStatusLine().getStatusCode()); | ||
| org.junit.Assert.assertTrue( | ||
| "expected [" + expectedSubstring + "] in response body: " + e.getMessage(), | ||
| e.getMessage().contains(expectedSubstring)); | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.