Add support for WinRS shell command execution#31
Open
dino2gnt wants to merge 7 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds WinRS (MS-WSMV) “shell command execution” support to the wsman client library, including a WinRS orchestration client, CXF-backed wire operations, SOAP body/header builders, public API types, and CLI support.
Changes:
- Introduces a WinRS shell orchestration flow (Create → Command → Receive loop → Delete) with parsing/building utilities under
cxf/.../shell. - Extends the public
WSManClientAPI withrunCommand(...)and adds newshellAPI types (ShellOptions,CommandResult). - Adds a new CLI operation (
-o SHELL) and adjusts CLI shaded dependencies to usejavax.*APIs.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
cxf/src/test/java/org/opennms/core/wsman/cxf/shell/WinRSClientTest.java |
Adds unit tests for WinRS lifecycle orchestration and edge cases. |
cxf/src/test/java/org/opennms/core/wsman/cxf/shell/ShellSoapHeadersTest.java |
Adds unit tests for SOAP header element builders (partial coverage). |
cxf/src/test/java/org/opennms/core/wsman/cxf/shell/ShellResponseParserTest.java |
Adds unit tests for response parsing (ShellId/CommandId/Receive). |
cxf/src/test/java/org/opennms/core/wsman/cxf/shell/ShellBodyBuilderTest.java |
Adds unit tests for SOAP body builders for Create/Command/Receive/Signal/Delete. |
cxf/src/main/java/org/opennms/core/wsman/cxf/shell/WinRSClient.java |
Implements WinRS orchestration client (Create/Command/Receive/Terminate/Delete). |
cxf/src/main/java/org/opennms/core/wsman/cxf/shell/ShellSoapHeaders.java |
Adds builders for WS-Man SOAP header elements needed for WinRS. |
cxf/src/main/java/org/opennms/core/wsman/cxf/shell/ShellResponseParser.java |
Adds DOM-based parsing for WinRS responses and Receive stream decoding. |
cxf/src/main/java/org/opennms/core/wsman/cxf/shell/ShellOperations.java |
Defines transport-agnostic interface for the 5 WinRS operations. |
cxf/src/main/java/org/opennms/core/wsman/cxf/shell/ShellConstants.java |
Adds protocol constants (URIs, actions, stream names, command states). |
cxf/src/main/java/org/opennms/core/wsman/cxf/shell/ShellBodyBuilder.java |
Adds DOM builders for WinRS SOAP bodies (Create/Command/Receive/Signal). |
cxf/src/main/java/org/opennms/core/wsman/cxf/shell/CxfShellOperations.java |
Implements WinRS operations using CXF Dispatch with per-request headers. |
cxf/src/main/java/org/opennms/core/wsman/cxf/CXFWSManClient.java |
Wires runCommand(...) into the existing CXF WS-Man client implementation. |
cli/src/main/java/org/opennms/core/wsman/WSManCli.java |
Adds SHELL operation and argument parsing via -- sentinel + timeout flag. |
cli/pom.xml |
Switches CLI shaded JAXB/JAX-WS APIs to javax.* (2.3.1) for CXF 3.5.x compatibility. |
api/src/main/java/org/opennms/core/wsman/WSManClient.java |
Extends public API with runCommand(...) (+ default overload). |
api/src/main/java/org/opennms/core/wsman/shell/ShellOptions.java |
Adds shell options type (no-profile, codepage, idle timeout, env, working dir). |
api/src/main/java/org/opennms/core/wsman/shell/CommandResult.java |
Adds command result type (exit code + stdout/stderr strings). |
Comments suppressed due to low confidence (3)
cxf/src/main/java/org/opennms/core/wsman/cxf/shell/ShellSoapHeaders.java:137
- String.format(...) uses the default JVM Locale; in locales that use ',' as the decimal separator this will emit something like "PT60,000S", which is not a valid ISO-8601 duration for OperationTimeout. Use String.format(Locale.ROOT, ...) (or equivalent) to guarantee '.' as the separator.
public static Element operationTimeout(Duration timeout) {
Document doc = newDocument();
Element el = doc.createElementNS(WSManConstants.XML_NS_DMTF_WSMAN_V1, "wsman:OperationTimeout");
long millis = timeout.toMillis();
long whole = millis / 1000;
long frac = millis % 1000;
el.setTextContent(String.format("PT%d.%03dS", whole, frac));
return el;
cxf/src/main/java/org/opennms/core/wsman/cxf/shell/ShellSoapHeaders.java:95
- The optionSet() Javadoc says it "Returns null if both are at defaults" but the implementation always creates and returns an OptionSet (and the comment below says so too). Please align the Javadoc with actual behavior to avoid misleading callers.
/**
* Builds {@code <wsman:OptionSet>} for the standard WinRS shell options:
* {@code WINRS_NOPROFILE} and {@code WINRS_CODEPAGE}. Returns {@code null} if both
* are at defaults that the server would already assume — but in practice we always
* emit at least the code page to avoid surprises, so we always return a non-null
* element when called.
*/
cxf/src/main/java/org/opennms/core/wsman/cxf/shell/ShellSoapHeaders.java:151
- maxEnvelopeSize(), operationTimeout(), and locale() are new public helpers but currently have no unit tests, while resourceUri/selectorSet/optionSet do. Adding focused tests here would help prevent subtle wire-format regressions (attributes/namespaces/formatting).
/**
* Builds {@code <wsman:MaxEnvelopeSize mustUnderstand="true">N</wsman:MaxEnvelopeSize>}.
* WinRM rejects shell Create requests that omit this header (the MS-WSMV reference
* envelope marks it as {@code mustUnderstand}). The pywinrm default is 153600 bytes.
*/
public static Element maxEnvelopeSize(long bytes) {
Document doc = newDocument();
Element el = doc.createElementNS(WSManConstants.XML_NS_DMTF_WSMAN_V1, "wsman:MaxEnvelopeSize");
el.setAttributeNS("http://www.w3.org/2003/05/soap-envelope", "soap:mustUnderstand", "true");
el.setTextContent(Long.toString(bytes));
return el;
}
/**
* Builds {@code <wsman:OperationTimeout>PT…S</wsman:OperationTimeout>}. Formats the
* {@link Duration} as an ISO-8601 duration string with millisecond precision, e.g.
* {@code PT60.000S}, which is what pywinrm and the MS-WSMV reference envelopes emit.
*/
public static Element operationTimeout(Duration timeout) {
Document doc = newDocument();
Element el = doc.createElementNS(WSManConstants.XML_NS_DMTF_WSMAN_V1, "wsman:OperationTimeout");
long millis = timeout.toMillis();
long whole = millis / 1000;
long frac = millis % 1000;
el.setTextContent(String.format("PT%d.%03dS", whole, frac));
return el;
}
/**
* Builds {@code <wsman:Locale xml:lang="…" mustUnderstand="false"/>}. WinRM uses this
* to pick the language of error messages; the element is required to be present even
* when the value doesn't matter for the operation.
*/
public static Element locale(String langTag) {
Document doc = newDocument();
Element el = doc.createElementNS(WSManConstants.XML_NS_DMTF_WSMAN_V1, "wsman:Locale");
el.setAttributeNS("http://www.w3.org/XML/1998/namespace", "xml:lang", langTag);
el.setAttributeNS("http://www.w3.org/2003/05/soap-envelope", "soap:mustUnderstand", "false");
return el;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment on lines
+133
to
+137
| long millis = timeout.toMillis(); | ||
| long whole = millis / 1000; | ||
| long frac = millis % 1000; | ||
| el.setTextContent(String.format("PT%d.%03dS", whole, frac)); | ||
| return el; |
Comment on lines
+23
to
+32
| /** | ||
| * Options passed to {@code rsp:Shell} on shell creation. Defaults are tuned for monitoring | ||
| * workloads: skip user profile load, UTF-8 code page, short idle timeout. | ||
| */ | ||
| public final class ShellOptions { | ||
| private final boolean noProfile; | ||
| private final int codepage; | ||
| private final Duration idleTimeout; | ||
| private final String workingDirectory; | ||
| private final Map<String, String> environment; |
Comment on lines
+54
to
+71
| public static Element buildCreateBody(ShellOptions options) { | ||
| Document doc = newDocument(); | ||
| Element shell = doc.createElementNS(ShellConstants.NS_SHELL, "rsp:Shell"); | ||
| doc.appendChild(shell); | ||
|
|
||
| Element inputStreams = doc.createElementNS(ShellConstants.NS_SHELL, "rsp:InputStreams"); | ||
| inputStreams.setTextContent(ShellConstants.STREAM_STDIN); | ||
| shell.appendChild(inputStreams); | ||
|
|
||
| Element outputStreams = doc.createElementNS(ShellConstants.NS_SHELL, "rsp:OutputStreams"); | ||
| outputStreams.setTextContent(ShellConstants.STREAM_STDOUT + " " + ShellConstants.STREAM_STDERR); | ||
| shell.appendChild(outputStreams); | ||
|
|
||
| if (options.getWorkingDirectory() != null && !options.getWorkingDirectory().isEmpty()) { | ||
| Element wd = doc.createElementNS(ShellConstants.NS_SHELL, "rsp:WorkingDirectory"); | ||
| wd.setTextContent(options.getWorkingDirectory()); | ||
| shell.appendChild(wd); | ||
| } |
Comment on lines
+231
to
+240
| try { | ||
| DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); | ||
| dbf.setNamespaceAware(true); | ||
| DOMResult result = new DOMResult(dbf.newDocumentBuilder().newDocument()); | ||
| TransformerFactory.newInstance().newTransformer().transform(response, result); | ||
| Document doc = (Document) result.getNode(); | ||
| return doc == null ? null : doc.getDocumentElement(); | ||
| } catch (Exception e) { | ||
| throw new IllegalStateException("Failed to materialise shell response Source to DOM", e); | ||
| } |
| TrustManager[] simpleTrustManager = new TrustManager[] { new X509TrustManager() { | ||
| public void checkClientTrusted(java.security.cert.X509Certificate[] certs, String authType) {} | ||
| public void checkServerTrusted(java.security.cert.X509Certificate[] certs, String authType) {} | ||
| public java.security.cert.X509Certificate[] getAcceptedIssuers() { return null; } |
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.
Add support for WinRS shell command execution to the
wsmanclient library. This is built largely around mimicking howpywinrmdoes it, and what its wire format and soap envelopes looks like.It is built with the expectation of short-lived shell sessions executing simple commands or scripts; it isn't designed with long-lived shell sessions or persistent remote shell interfaces as a requirement. The expectation is that this will be leveraged by OpenNMS within the context of a single poll or collection cycle and have a lifespan measured in ~seconds. The code changes were intended to be as self-contained and non-invasive to the existing library as possible.
I have function-tested this end to end in my lab against a Windows 2019 server, configured for Kerberos authenticated WinRM, using the CLI client jar.
I was heavily and shamefully assisted by Claude. I apologize to my future reviewers. (Except Copilot)