Skip to content

Add dedicated CI check for sandbox running on Github - #21175

Merged
mch2 merged 3 commits into
opensearch-project:mainfrom
cwperks:sandbox-ci-check
Apr 8, 2026
Merged

Add dedicated CI check for sandbox running on Github#21175
mch2 merged 3 commits into
opensearch-project:mainfrom
cwperks:sandbox-ci-check

Conversation

@cwperks

@cwperks cwperks commented Apr 8, 2026

Copy link
Copy Markdown
Member

Description

This PR creates a separate CI check for sandbox plugins and uses Github as a runner instead of the Jenkins infrastructure. The workflow installs pre-requisites and 1) Uses JDK 25 - needed for virtual threads and 2) installs rust and the protobuf compiler.

Note: This will duplicate effort currently in jenkins. However, if #21125 is merged then sandbox will no longer run on jenkins by default unless -Dsandbox.enabled=true is passed.

Related Issues

Related to discussion on #21174

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.

Signed-off-by: Craig Perkins <cwperx@amazon.com>
@github-actions

github-actions Bot commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 9ae4fbb.

PathLineSeverityDescription
.github/workflows/sandbox-check.yml32highThird-party GitHub Action 'dtolnay/rust-toolchain@stable' is pinned to a mutable tag ('stable') rather than a specific commit SHA. A supply chain attacker who compromises this action's repository could push malicious code under the same tag and have it execute in CI with full runner access.
.github/workflows/sandbox-check.yml21highUses 'actions/checkout@v6', which is beyond the latest known stable release (v4). This version may not exist in the official actions/checkout repository, making it a candidate for namespace hijacking or typosquatting. Maintainers must verify this tag resolves to a legitimate, expected commit SHA.
.github/workflows/sandbox-check.yml26highUses 'actions/setup-java@v5', which exceeds the latest known stable release (v4). Similar to the checkout action concern, this version should be verified to exist and resolve to a trusted commit SHA to rule out supply chain compromise.
.github/workflows/sandbox-check.yml19medium'continue-on-error: true' is set on the sandbox-check job, meaning any failure — including a security-relevant test failure — will not block the workflow from reporting success. For a check workflow, silent failure suppression is anomalous and could mask malicious behavior or broken security gates.

The table above displays the top 10 most important findings.

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


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.

@cwperks cwperks added the skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis. label Apr 8, 2026
@github-actions

github-actions Bot commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

(Review updated until commit 73de7c0)

Here are some key observations to aid the review process:

🧪 No relevant tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Broad Trigger

The workflow triggers on every push to every branch, not just main or specific branches. This could lead to excessive CI resource usage. Consider restricting the push trigger to specific branches (e.g., main, feature branches).

push:
pull_request:
continue-on-error

The continue-on-error: true setting means the job will always report as green/passing even if the sandbox check fails. This could mask real failures and defeat the purpose of having a dedicated CI check. Consider whether this is intentional or if failures should block the PR.

continue-on-error: true
Outdated Action

actions/checkout@v6 does not exist as a released version (latest stable is v4). This will likely cause the workflow to fail. It should be actions/checkout@v4.

- uses: actions/checkout@v6

@github-actions

github-actions Bot commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Latest suggestions up to 73de7c0

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Invalid action version referenced

actions/checkout@v6 does not exist as a released version (latest stable is v4).
Using a non-existent version will cause the workflow to fail. This should be updated
to a valid version such as v4.

.github/workflows/sandbox-check.yml [20]

-- uses: actions/checkout@v6
+- uses: actions/checkout@v4
Suggestion importance[1-10]: 9

__

Why: actions/checkout@v6 does not exist (latest stable is v4), which would cause the workflow to fail immediately. This is a critical bug that needs to be fixed for the workflow to function at all.

High
CI check always passes despite failures

Setting continue-on-error: true means the CI check will always appear as passing
even when the sandbox check fails, defeating the purpose of a dedicated CI check. If
the intent is to have a meaningful gate, this should be removed or set to false. If
it's intentional for non-blocking monitoring, a comment explaining the rationale
would be helpful.

.github/workflows/sandbox-check.yml [17]

-continue-on-error: true
+continue-on-error: false
Suggestion importance[1-10]: 6

__

Why: Setting continue-on-error: true means failures won't block PRs, which may be intentional for a sandbox check but could mask real issues. The suggestion is valid but the intent might be deliberate for non-blocking monitoring.

Low

Previous suggestions

Suggestions up to commit a80a3a5
CategorySuggestion                                                                                                                                    Impact
Possible issue
Invalid action version reference used

actions/checkout@v6 does not exist as a released version (latest stable is v4).
Using a non-existent version will cause the workflow to fail at runtime. This should
be updated to a valid release such as v4.

.github/workflows/sandbox-check.yml [20]

-- uses: actions/checkout@v6
+- uses: actions/checkout@v4
Suggestion importance[1-10]: 9

__

Why: actions/checkout@v6 does not exist (latest is v4), so this would cause the workflow to fail at runtime. This is a critical bug that needs to be fixed for the workflow to function at all.

High
CI check always passes despite failures

Setting continue-on-error: true means the CI check will always appear as passing
even when the sandbox check fails, defeating the purpose of a dedicated CI check. If
the intent is to have a meaningful gate, this should be removed or set to false. If
it's intentional for non-blocking monitoring, a comment explaining the rationale
would be helpful.

.github/workflows/sandbox-check.yml [17]

-continue-on-error: true
+continue-on-error: false
Suggestion importance[1-10]: 6

__

Why: Setting continue-on-error: true means failures won't block PRs, which may be intentional for a sandbox/experimental check, but it does reduce the value of the CI gate. The suggestion is valid but the intent may be deliberate.

Low
Suggestions up to commit 9ae4fbb
CategorySuggestion                                                                                                                                    Impact
Possible issue
Invalid action version referenced

actions/checkout@v6 does not exist as a released version (latest stable is v4).
Using a non-existent version will cause the workflow to fail. This should be updated
to a valid release such as v4.

.github/workflows/sandbox-check.yml [20]

-- uses: actions/checkout@v6
+- uses: actions/checkout@v4
Suggestion importance[1-10]: 9

__

Why: actions/checkout@v6 does not exist as a released version (latest is v4), which would cause the workflow to fail immediately. This is a critical bug that needs to be fixed to @v4.

High
CI check always passes despite failures

Setting continue-on-error: true means the CI check will always report success even
when the sandbox check fails, defeating the purpose of a dedicated CI check. If the
intent is to have a meaningful gate, this should be removed or set to false. If it's
intentional for non-blocking monitoring, a comment explaining the rationale would be
helpful.

.github/workflows/sandbox-check.yml [17]

-continue-on-error: true
+continue-on-error: false
Suggestion importance[1-10]: 5

__

Why: Setting continue-on-error: true does make the job non-blocking, which may be intentional for a sandbox/experimental check. However, if this is meant to be a meaningful CI gate, it should be false. The suggestion is valid but the intent may be deliberate.

Low

Signed-off-by: Craig Perkins <cwperx@amazon.com>
@github-actions

github-actions Bot commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit a80a3a5

Signed-off-by: Craig Perkins <cwperx@amazon.com>
@github-actions

github-actions Bot commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 73de7c0

@cwperks
cwperks marked this pull request as ready for review April 8, 2026 19:38
@cwperks
cwperks requested review from a team, jed326 and peternied as code owners April 8, 2026 19:38
@mch2
mch2 merged commit 9ab639d into opensearch-project:main Apr 8, 2026
17 of 19 checks passed
@andrross

andrross commented Apr 9, 2026

Copy link
Copy Markdown
Member

@cwperks @mch2 I think a broken check is worse than no check at all. Can we remove this github action until it is able to pass?

aparajita31pandey pushed a commit to aparajita31pandey/OpenSearch that referenced this pull request Apr 18, 2026
…ect#21175)

* Add dedicated CI check for sandbox running on Github

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* part1 and part2

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Fix syntax

Signed-off-by: Craig Perkins <cwperx@amazon.com>

---------

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Aparajita Pandey <aparajita31pandey@gmail.com>
pradeep-L pushed a commit to pradeep-L/OpenSearch that referenced this pull request Apr 21, 2026
…ect#21175)

* Add dedicated CI check for sandbox running on Github

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* part1 and part2

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Fix syntax

Signed-off-by: Craig Perkins <cwperx@amazon.com>

---------

Signed-off-by: Craig Perkins <cwperx@amazon.com>
imRishN pushed a commit to imRishN/OpenSearch that referenced this pull request May 8, 2026
…ect#21175)

* Add dedicated CI check for sandbox running on Github

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* part1 and part2

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Fix syntax

Signed-off-by: Craig Perkins <cwperx@amazon.com>

---------

Signed-off-by: Craig Perkins <cwperx@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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants