Skip to content

Enable PPL rest command endpoints for sql 3.8.0 integ-test cluster - #6299

Merged
gaiksaya merged 1 commit into
opensearch-project:mainfrom
ahkcs:sql-rest-allowed-endpoints-3.8.0
Jul 17, 2026
Merged

gaiksaya merged 1 commit into
opensearch-project:mainfrom
ahkcs:sql-rest-allowed-endpoints-3.8.0

Conversation

@ahkcs

@ahkcs ahkcs commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Description

The PPL rest command (opensearch-project/sql#5599) is disabled by default: it is gated behind plugins.ppl.rest.allowed_endpoints, a node-scoped setting whose default is an empty allow-list. Because the setting is NodeScope (non-dynamic), it must be provided at cluster startup.

The sql repo's own integ-test/build.gradle opts its test clusters into the registered endpoints, which is why the tests pass in the sql repo's CI. But the 3.8.0 distribution test manifest was never updated to match, so the release integ-test cluster boots with the command disabled.

As a result, every | rest ... integration test fails on the distribution cluster with the rest command is disabled on this cluster — across both with-security and without-security, on every platform/arch/dist. Affected classes:

  • org.opensearch.sql.calcite.remote.CalcitePPLRestIT
  • org.opensearch.sql.calcite.remote.CalciteExplainIT (explainRestCommand)
  • org.opensearch.sql.calcite.remote.CalciteNewAddedCommandsIT (testRest)
  • org.opensearch.sql.calcite.CalciteNoPushdownIT (suite runner aggregating the above)

This adds the same endpoint allow-list the sql repo enables so the release integ-test cluster exercises the enabled path. The value is rendered into opensearch.yml as a native YAML list (matching how plugins.destination.host.deny_list and path.repo are already configured for other components in this manifest), which is the correct representation for an OpenSearch listSetting.

Only the 3.8.0 manifest is affected — the rest command first ships in 3.8.0, so earlier release trains (3.7.x, 3.6.x) do not have the feature and need no change.

Issues Resolved

Fixes the current failure mode in opensearch-project/sql#5620 (build 12088).

Testing

  • Manifest validated through the repo's TestManifest loader (Cerberus schema): loads and validates OK.
  • Confirmed yaml.dump of the config renders plugins.ppl.rest.allowed_endpoints as a native YAML list in the generated opensearch.yml.

Check List

  • Commits are signed per the DCO using --signoff.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

The PPL `rest` command (opensearch-project/sql#5599) is disabled by
default via an empty, node-scoped allow-list (plugins.ppl.rest.allowed_endpoints).
The sql repo's own integ-test clusters opt into the registered endpoints,
but the 3.8.0 distribution test manifest was not updated, so the release
integ-test cluster boots with the command disabled. Every '| rest ...'
test in CalcitePPLRestIT, CalciteExplainIT, NewAddedCommandsIT and the
CalciteNoPushdownIT suite then fails with 'the rest command is disabled
on this cluster' (opensearch-project/sql#5620).

Add the same endpoint allow-list the sql repo uses so the release
integ-test cluster exercises the enabled path.

Signed-off-by: Kai Huang <ahkcs@amazon.com>
@github-actions

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 No relevant tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@gaiksaya

Copy link
Copy Markdown
Member

@ahkcs I believe it would need some code changes on python level as well. AFAIK additional-cluster-configs only accepts key value pair.
@rishabh6788 can you please take a look into this.
Thanks!

@rishabh6788

Copy link
Copy Markdown
Collaborator

@ahkcs I believe it would need some code changes on python level as well. AFAIK additional-cluster-configs only accepts key value pair. @rishabh6788 can you please take a look into this. Thanks!

It looks like a valid key-pair with list values.

@gaiksaya

Copy link
Copy Markdown
Member

It looks like a valid key-pair with list values.

Yes I misread the error. My bad. Looks like re-run is fixing it

@gaiksaya
gaiksaya merged commit a82fad2 into opensearch-project:main Jul 17, 2026
24 of 25 checks passed
@github-project-automation github-project-automation Bot moved this from 👀 In Review to ✅ Done in Engineering Effectiveness Board Jul 17, 2026
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.68%. Comparing base (618aba2) to head (8657e34).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6299   +/-   ##
=======================================
  Coverage   96.68%   96.68%           
=======================================
  Files         407      407           
  Lines       19443    19443           
=======================================
  Hits        18799    18799           
  Misses        644      644           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ahkcs
ahkcs deleted the sql-rest-allowed-endpoints-3.8.0 branch July 20, 2026 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

3 participants