Skip to content

update sandbox check to publish latest UnifiedQueryAPI jars to maven local - #21569

Merged
mch2 merged 1 commit into
opensearch-project:mainfrom
mch2:sql-branch
May 8, 2026
Merged

update sandbox check to publish latest UnifiedQueryAPI jars to maven local#21569
mch2 merged 1 commit into
opensearch-project:mainfrom
mch2:sql-branch

Conversation

@mch2

@mch2 mch2 commented May 8, 2026

Copy link
Copy Markdown
Member

This is a temporary work-around to publish latest unified jars built to integrate with analytics-engine to maven local - we are working off of a feature branch of sql in the short term. if the step fails, the build will continue with jars from maven central. This will be removed once changes for analytics-engine in sql are merged to mainline.

Description

[Describe what this change achieves]

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

…local.

This is a temporary work-around to publish latest unified jars
built to integrate with analytics-engine to maven local - we are working off of
a feature branch of sql in the short term.  if the step fails, the build will continue
with jars from maven central. This will be removed once changes for
analytics-engine in sql are merged to mainline.

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>
@mch2
mch2 requested review from a team, jed326 and peternied as code owners May 8, 2026 20:13
@github-actions

github-actions Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 142763d.

PathLineSeverityDescription
.github/workflows/sandbox-check.yml35highSupply chain risk: uses actions/checkout@v6, which does not correspond to any known stable release of that action (latest stable is v4). A non-existent or newly-created tag is a classic namespace/tag-hijacking vector that could execute arbitrary code in the CI runner with access to repository secrets.
.github/workflows/sandbox-check.yml40highSupply chain risk: pulls code from an unreviewed external feature branch (opensearch-project/sql @ feature/mustang-ppl-integration), builds it, and publishes Maven artifacts to the local repository, which are then consumed by the main build via -Drepos.mavenLocal=true. This is a classic artifact-injection pattern — a compromised or malicious feature branch could plant arbitrary JARs that are silently used in downstream build and test execution.
.github/workflows/sandbox-check.yml42mediumcontinue-on-error: true on the Maven publish step is suspicious in combination with the -Drepos.mavenLocal=true flag on the subsequent step. If the publish step fails legitimately the build proceeds as if nothing happened, but if it succeeds while performing malicious side effects (e.g., writing manipulated artifacts), no signal is surfaced to reviewers. This pattern obscures whether the injected artifact source is actually being used.
.github/workflows/sandbox-check.yml46lowAddition of -PrustDebug enables Rust debug builds in CI. While likely legitimate for diagnostics, debug builds can enable code paths disabled in release builds and may produce more verbose output that could inadvertently surface sensitive environment variables or secrets in CI logs. Warrants confirmation that this flag was intentionally added.

The table above displays the top 10 most important findings.

Total: 4 | Critical: 0 | High: 2 | Medium: 1 | Low: 1


Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.


⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

@mch2 mch2 added skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis. skip-diff-reviewer Maintainer to skip code-diff-reviewer check, after reviewing issues in AI analysis. labels May 8, 2026
Comment thread .github/workflows/sandbox-check.yml
@mch2
mch2 merged commit 4d3562b into opensearch-project:main May 8, 2026
27 of 28 checks passed
RyanL1997 added a commit to RyanL1997/OpenSearch that referenced this pull request May 9, 2026
The remote OpenSearch Snapshots maven repo (ci.opensearch.org/ci/dbc/snapshots)
only republishes from sql/main, not from sql/feature/mustang-ppl-integration,
so its 3.7.0.0-SNAPSHOT jars trail the feature branch by however many merges
(currently missing PPL_REX_MAX_MATCH_LIMIT, CALCITE_ENGINE_ENABLED, …). The
sandbox-check workflow's pre-step opensearch-project#21569 publishes feature-branch unified-query
jars to mavenLocal, but Gradle's default SNAPSHOT resolution weighs the remote's
explicit <buildNumber>/<timestamp> metadata higher than mavenLocal's
<localCopy>true>, so the stale remote wins even when mavenLocal has a newer
<lastUpdated>.

Confirmed via dependencyInsight: every consumer was binding
unified-query-api:3.7.0.0-SNAPSHOT:20260507.224009-12 (60kB, 42 classes, no
PPL_REX_MAX_MATCH_LIMIT field reference) instead of the locally-published
3.7.0.0-SNAPSHOT (29kB, 21 classes, has the field). The runtime cluster
inherited that stale class via the test-ppl-frontend plugin bundle, which
is why every IT touching `rex` failed plan-time with `NullPointerException:
Cannot invoke "java.lang.Integer.intValue()" because the return value of
"Settings.getSettingValue(PPL_REX_MAX_MATCH_LIMIT)" is null` once the
unified path tried to read the setting.

Fix: tell the OpenSearch Snapshots remote to refuse `org.opensearch.query`
artifacts via mavenContent { excludeGroup }. Three sites declare the remote:

  * sandbox/build.gradle subprojects { repositories } — applies to every
    sandbox subproject including qa.
  * sandbox/plugins/analytics-backend-datafusion/build.gradle — own
    declaration; left in place for module isolation, filtered identically.
  * sandbox/plugins/test-ppl-frontend/build.gradle — also pin mavenLocal as
    the only source for org.opensearch.query so the bundlePlugin task
    bundles the freshly-published feature-branch jar rather than the stale
    timestamped one Gradle would otherwise pick.

Verified locally: bundled unified-query-api drops 60kB → 29kB, the
UnifiedQueryContext$Builder constant pool now references PPL_REX_MAX_MATCH_LIMIT,
and RexCommandIT goes 0/16 → 16/16 against the same locally-published jars
the CI workflow already produces.

Drop this filter once the SQL feature branch merges to sql/main and the
remote OpenSearch Snapshots repo catches up — at that point every
3.7.0.0-SNAPSHOT publish will carry the rex max-match default and the
mavenLocal preference becomes redundant.

Signed-off-by: Jialiang Liang <jiallian@amazon.com>
RyanL1997 added a commit to RyanL1997/OpenSearch that referenced this pull request May 9, 2026
The remote OpenSearch Snapshots maven repo (ci.opensearch.org/ci/dbc/snapshots)
only republishes from sql/main, not from sql/feature/mustang-ppl-integration,
so its 3.7.0.0-SNAPSHOT jars trail the feature branch by however many merges
(currently missing PPL_REX_MAX_MATCH_LIMIT, CALCITE_ENGINE_ENABLED, …). The
sandbox-check workflow's pre-step opensearch-project#21569 publishes feature-branch unified-query
jars to mavenLocal, but Gradle's default SNAPSHOT resolution weighs the remote's
explicit <buildNumber>/<timestamp> metadata higher than mavenLocal's
<localCopy>true>, so the stale remote wins even when mavenLocal has a newer
<lastUpdated>.

Confirmed via dependencyInsight: every consumer was binding
unified-query-api:3.7.0.0-SNAPSHOT:20260507.224009-12 (60kB, 42 classes, no
PPL_REX_MAX_MATCH_LIMIT field reference) instead of the locally-published
3.7.0.0-SNAPSHOT (29kB, 21 classes, has the field). The runtime cluster
inherited that stale class via the test-ppl-frontend plugin bundle, which
is why every IT touching `rex` failed plan-time with `NullPointerException:
Cannot invoke "java.lang.Integer.intValue()" because the return value of
"Settings.getSettingValue(PPL_REX_MAX_MATCH_LIMIT)" is null` once the
unified path tried to read the setting.

Fix: tell the OpenSearch Snapshots remote to refuse `org.opensearch.query`
artifacts via mavenContent { excludeGroup }. Three sites declare the remote:

  * sandbox/build.gradle subprojects { repositories } — applies to every
    sandbox subproject including qa.
  * sandbox/plugins/analytics-backend-datafusion/build.gradle — own
    declaration; left in place for module isolation, filtered identically.
  * sandbox/plugins/test-ppl-frontend/build.gradle — also pin mavenLocal as
    the only source for org.opensearch.query so the bundlePlugin task
    bundles the freshly-published feature-branch jar rather than the stale
    timestamped one Gradle would otherwise pick.

Verified locally: bundled unified-query-api drops 60kB → 29kB, the
UnifiedQueryContext$Builder constant pool now references PPL_REX_MAX_MATCH_LIMIT,
and RexCommandIT goes 0/16 → 16/16 against the same locally-published jars
the CI workflow already produces.

Drop this filter once the SQL feature branch merges to sql/main and the
remote OpenSearch Snapshots repo catches up — at that point every
3.7.0.0-SNAPSHOT publish will carry the rex max-match default and the
mavenLocal preference becomes redundant.

Signed-off-by: Jialiang Liang <jiallian@amazon.com>
mch2 pushed a commit that referenced this pull request May 9, 2026
…napshots to mavenLocal (#21578)

* Pin org.opensearch.query:* (unified-query-*) artifacts to mavenLocal

The remote OpenSearch Snapshots maven repo (ci.opensearch.org/ci/dbc/snapshots)
only republishes from sql/main, not from sql/feature/mustang-ppl-integration,
so its 3.7.0.0-SNAPSHOT jars trail the feature branch by however many merges
(currently missing PPL_REX_MAX_MATCH_LIMIT, CALCITE_ENGINE_ENABLED, …). The
sandbox-check workflow's pre-step #21569 publishes feature-branch unified-query
jars to mavenLocal, but Gradle's default SNAPSHOT resolution weighs the remote's
explicit <buildNumber>/<timestamp> metadata higher than mavenLocal's
<localCopy>true>, so the stale remote wins even when mavenLocal has a newer
<lastUpdated>.

Confirmed via dependencyInsight: every consumer was binding
unified-query-api:3.7.0.0-SNAPSHOT:20260507.224009-12 (60kB, 42 classes, no
PPL_REX_MAX_MATCH_LIMIT field reference) instead of the locally-published
3.7.0.0-SNAPSHOT (29kB, 21 classes, has the field). The runtime cluster
inherited that stale class via the test-ppl-frontend plugin bundle, which
is why every IT touching `rex` failed plan-time with `NullPointerException:
Cannot invoke "java.lang.Integer.intValue()" because the return value of
"Settings.getSettingValue(PPL_REX_MAX_MATCH_LIMIT)" is null` once the
unified path tried to read the setting.

Fix: tell the OpenSearch Snapshots remote to refuse `org.opensearch.query`
artifacts via mavenContent { excludeGroup }. Three sites declare the remote:

  * sandbox/build.gradle subprojects { repositories } — applies to every
    sandbox subproject including qa.
  * sandbox/plugins/analytics-backend-datafusion/build.gradle — own
    declaration; left in place for module isolation, filtered identically.
  * sandbox/plugins/test-ppl-frontend/build.gradle — also pin mavenLocal as
    the only source for org.opensearch.query so the bundlePlugin task
    bundles the freshly-published feature-branch jar rather than the stale
    timestamped one Gradle would otherwise pick.

Verified locally: bundled unified-query-api drops 60kB → 29kB, the
UnifiedQueryContext$Builder constant pool now references PPL_REX_MAX_MATCH_LIMIT,
and RexCommandIT goes 0/16 → 16/16 against the same locally-published jars
the CI workflow already produces.

Drop this filter once the SQL feature branch merges to sql/main and the
remote OpenSearch Snapshots repo catches up — at that point every
3.7.0.0-SNAPSHOT publish will carry the rex max-match default and the
mavenLocal preference becomes redundant.

Signed-off-by: Jialiang Liang <jiallian@amazon.com>

* Wire mavenLocal into sandbox subprojects + bump analytics-engine to 3.7

CI fallout from the prior commit's `excludeGroup 'org.opensearch.query'`
filter on the OpenSearch Snapshots remote: the parent subprojects block
no longer carried mavenLocal, so analytics-engine's testImplementation /
internalClusterTest configurations had no repository at all serving
org.opensearch.query, failing with `Could not find
org.opensearch.query:unified-query-api:3.6.0.0-SNAPSHOT` (and -core / -ppl).

Two pieces:

1. sandbox/build.gradle subprojects { repositories } — also declare
   mavenLocal scoped to the org.opensearch.query group via mavenContent
   { includeGroup }. mavenLocal becomes the authoritative source for
   unified-query SNAPSHOTs (populated by the sandbox-check workflow's
   publishUnifiedQueryPublicationToMavenLocal pre-step) without leaking
   into resolution for any other group.

2. sandbox/plugins/analytics-engine/build.gradle — bump
   sqlUnifiedQueryVersion from 3.6.0.0-SNAPSHOT → 3.7.0.0-SNAPSHOT.
   The 3.6 jars don't exist in mavenLocal (only the 3.7 feature-branch
   build does), so the older pin was the proximate cause of the CI
   resolution failure. Aligning with test-ppl-frontend's already-3.7
   declaration also keeps the unified-query consumers consistent.

Signed-off-by: Jialiang Liang <jiallian@amazon.com>

---------

Signed-off-by: Jialiang Liang <jiallian@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis. skip-diff-reviewer Maintainer to skip code-diff-reviewer check, after reviewing issues in AI analysis.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants