Fix flaky SSH security integration tests on Windows CI#2764
Open
jbonofre wants to merge 1 commit into
Open
Conversation
…etion The SSH command security integration tests (SshCommandTestBase) wrote a command to the SSH channel and immediately sent "logout" to tear down the session. On slower runners - in particular the Windows CI - the session could be closed while the command output was still in flight, producing truncated output (e.g. a partial command echo "she" instead of "shell:nano") and spurious assertion failures such as "Should contain 'Command not found'". Introduce a writeCommandAndWait() helper that appends a sentinel "echo <marker>" command and blocks until the unique marker appears in the captured output before closing the channel. Because the remote shell reads and executes its input line by line, the marker cannot appear before the command under test has been fully executed and flushed back to the client, providing a deterministic completion signal that also works for the OK case (which has no positive marker of its own). "echo" is a gogo built-in that is not restricted by any command ACL, so it is safe for every test user. The helper is also used by addUsers()/addViewer() so the JAAS users are fully created before the test logs in as them.
There was a problem hiding this comment.
Pull request overview
This PR aims to eliminate a known Windows CI flake in Karaf SSH security integration tests by ensuring commands fully complete (and their output is fully received) before the SSH session is torn down.
Changes:
- Added
writeCommandAndWait()which appends a uniqueecho <marker>sentinel and waits for the marker to appear in captured output. - Updated
assertCommand(),addUsers(), andaddViewer()to use the new synchronization helper. - Introduced Awaitility timeout handling and supporting imports.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
54
to
+57
| void addUsers(String manageruser, String vieweruser) throws Exception { | ||
| ByteArrayOutputStream out = new ByteArrayOutputStream(); | ||
| OutputStream pipe = openSshChannel("karaf", "karaf", out); | ||
| pipe.write(("jaas:realm-manage --realm=karaf" | ||
| writeCommandAndWait(pipe, out, "jaas:realm-manage --realm=karaf" |
Comment on lines
70
to
+73
| void addViewer(String vieweruser) throws Exception { | ||
| ByteArrayOutputStream out = new ByteArrayOutputStream(); | ||
| OutputStream pipe = openSshChannel("karaf", "karaf", out); | ||
| pipe.write(("jaas:realm-manage --realm=karaf" | ||
| writeCommandAndWait(pipe, out, "jaas:realm-manage --realm=karaf" |
Comment on lines
+114
to
+117
| String assertCommand(String user, String command, Result result) throws Exception { | ||
| ByteArrayOutputStream out = new ByteArrayOutputStream(); | ||
| OutputStream pipe = openSshChannel(user, user, out, out); | ||
| pipe.write(command.getBytes()); | ||
| pipe.flush(); | ||
| writeCommandAndWait(pipe, out, command); |
Comment on lines
+108
to
+111
| } catch (ConditionTimeoutException e) { | ||
| // Fall through: proceed with whatever output was captured so far. For assertions | ||
| // this yields a more useful failure message than the timeout itself. | ||
| } |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
The SSH command security integration tests (
ShellCommandSecurityTest,JaasSshCommandSecurityTest, and the otherSshCommandTestBasesubclasses) intermittently fail on the Windows CI job with errors like:The captured output is truncated to a partial command echo (
sheinstead ofshell:nano), so the assertion fails. This is a well-known flake on thetest (windows-latest)job (it also hitsmainindependently of any given PR).Root cause
SshCommandTestBase.assertCommand()wrote the command to the SSH channel and then immediately sentlogoutincloseSshChannel()to tear down the session. On slower runners — in particular Windows — the session could be closed while the command output was still in flight, so only a partial, truncated output was captured before the channel was read.Fix
Introduce a
writeCommandAndWait()helper that appends a sentinelecho <marker>command after the command under test and blocks until the unique marker appears in the captured output before closing the channel.Because the remote shell reads and executes its input line by line, the marker cannot appear before the command under test has been fully executed and flushed back to the client. This provides a deterministic completion signal that also works for the
OKcase, which has no positive marker of its own.echois a gogo built-in that is not restricted by any command ACL, so it is safe for every test user.The helper is also used by
addUsers()/addViewer()so the JAAS users are fully created before the test logs in as them.Testing
ShellCommandSecurityTestpasses locally on Temurin JDK 17 (matching the CI runtime):