From 2d918c784590488e708598a6995d3db74cd195cc Mon Sep 17 00:00:00 2001 From: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> Date: Fri, 1 Nov 2024 00:03:12 -0400 Subject: [PATCH 1/7] feat: added method to validate the ssh connection Signed-off-by: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> --- packages/zosuss/src/Shell.ts | 62 +++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 23 deletions(-) diff --git a/packages/zosuss/src/Shell.ts b/packages/zosuss/src/Shell.ts index dc7ff271f5..52f506f360 100644 --- a/packages/zosuss/src/Shell.ts +++ b/packages/zosuss/src/Shell.ts @@ -9,7 +9,7 @@ * */ -import { Logger, ImperativeError } from "@zowe/imperative"; +import { Logger, ImperativeError, IProfileLoaded } from "@zowe/imperative"; import { ClientChannel, Client } from "ssh2"; import { SshSession } from "./SshSession"; import { ZosUssMessages } from "./constants/ZosUss.messages"; @@ -24,19 +24,9 @@ export class Shell { public static executeSsh(session: SshSession, command: string, stdoutHandler: (data: string) => void): Promise { - const authsAllowed = ["none"]; let hasAuthFailed = false; const promise = new Promise((resolve, reject) => { const conn = new Client(); - - // These are needed for authenticationHandler - // The order is critical as this is the order of authentication that will be used. - if (session.ISshSession.privateKey != null && session.ISshSession.privateKey !== "undefined") { - authsAllowed.push("publickey"); - } - if (session.ISshSession.password != null && session.ISshSession.password !== "undefined") { - authsAllowed.push("password"); - } conn.on("ready", () => { conn.shell((err: any, stream: ClientChannel) => { if (err) { throw err; } @@ -77,6 +67,7 @@ export class Shell { return; } dataBuffer += data; + if (dataBuffer.includes("\r")) { // when data is not received with complete lines, // slice the last incomplete line and put it back to dataBuffer until it gets the complete line, @@ -101,6 +92,7 @@ export class Shell { else if (isUserCommand && dataToPrint.length != 0) { if (!dataToPrint.startsWith('\r\n$ '+cmd) && !dataToPrint.startsWith('\r<')){ //only prints command output + if (dataToPrint.startsWith("\r\n$ ")) dataToPrint = dataToPrint.replace(/\r\n\$\s/, "\r\n"); stdoutHandler(dataToPrint); dataToPrint = ""; } @@ -140,22 +132,37 @@ export class Shell { })); } }); - conn.connect({ - host: session.ISshSession.hostname, - port: session.ISshSession.port, - username: session.ISshSession.user, - password: session.ISshSession.password, - privateKey: session.ISshSession.privateKey != null && session.ISshSession.privateKey !== "undefined" ? - require("fs").readFileSync(session.ISshSession.privateKey) : "", - passphrase: session.ISshSession.keyPassphrase, - authHandler: this.authenticationHandler(authsAllowed), - readyTimeout: session.ISshSession.handshakeTimeout != null && session.ISshSession.handshakeTimeout !== undefined ? - session.ISshSession.handshakeTimeout : 0 - } as any); + Shell.connect(conn, session); }); return promise; } + private static connect(connection: Client, session: SshSession) { + const authsAllowed = ["none"]; + + // These are needed for authenticationHandler + // The order is critical as this is the order of authentication that will be used. + if (session.ISshSession.privateKey != null && session.ISshSession.privateKey !== "undefined") { + authsAllowed.push("publickey"); + } + if (session.ISshSession.password != null && session.ISshSession.password !== "undefined") { + authsAllowed.push("password"); + } + + connection.connect({ + host: session.ISshSession.hostname, + port: session.ISshSession.port, + username: session.ISshSession.user, + password: session.ISshSession.password, + privateKey: session.ISshSession.privateKey != null && session.ISshSession.privateKey !== "undefined" ? + require("fs").readFileSync(session.ISshSession.privateKey) : "", + passphrase: session.ISshSession.keyPassphrase, + authHandler: this.authenticationHandler(authsAllowed), + readyTimeout: session.ISshSession.handshakeTimeout != null && session.ISshSession.handshakeTimeout !== undefined ? + session.ISshSession.handshakeTimeout : 0 + } as any); + } + public static async executeSshCwd(session: SshSession, command: string, cwd: string, @@ -164,6 +171,15 @@ export class Shell { return this.executeSsh(session, cwdCommand, stdoutHandler); } + public static async isConnectionValid(session: SshSession): Promise{ + return new Promise((resolve, _) => { + const conn = new Client(); + conn.on("ready", () => conn.end() && resolve(true)) + .on("error", () => resolve(false)); + Shell.connect(conn, session); + }); + } + private static authenticationHandler(authsAllowed: string[]) { let authPos = 0; return (methodsLeft: string[], partialSuccess: boolean, callback: any) => { From 54fd52348b05a226dc064b211cd919ff5970db3d Mon Sep 17 00:00:00 2001 From: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> Date: Fri, 1 Nov 2024 08:51:52 -0400 Subject: [PATCH 2/7] chore: update changelog Signed-off-by: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> --- packages/cli/CHANGELOG.md | 4 ++++ packages/zosuss/CHANGELOG.md | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index 9e57df0fef..752e3464dd 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes to the Zowe CLI package will be documented in this file. +## Recent Changes + +- BugFix: Removed unnecessary `$ ` characters in front of most output. [zowe-explorer#3079(comment)](https://github.com/zowe/zowe-explorer-vscode/pull/3079#pullrequestreview-2408842655) + ## `8.6.0` - Enhancement: Added support for running applications on TSO/E address spaces. Start applications and receive/transmit messages using the new `tso start`, `tso receive` and `tso send` commands. [#2280](https://github.com/zowe/zowe-cli/pull/2280) diff --git a/packages/zosuss/CHANGELOG.md b/packages/zosuss/CHANGELOG.md index 5ebdb64df4..f57ec1a7d5 100644 --- a/packages/zosuss/CHANGELOG.md +++ b/packages/zosuss/CHANGELOG.md @@ -2,6 +2,11 @@ All notable changes to the Zowe z/OS USS SDK package will be documented in this file. +## Recent Changes + +- BugFix: Removed unnecessary `$ ` characters in front of most output. [zowe-explorer#3079(comment)](https://github.com/zowe/zowe-explorer-vscode/pull/3079#pullrequestreview-2408842655) +- Enhancement: Added the ability to validate of an SSH profile is able to establish a connection. [zowe-explorer#3079(comment)](https://github.com/zowe/zowe-explorer-vscode/pull/3079#discussion_r1825783867) + ## `8.1.1` - BugFix: Updated peer dependencies to `^8.0.0`, dropping support for versions tagged `next`. [#2287](https://github.com/zowe/zowe-cli/pull/2287) From ed25b84539f8efc574d33d96e26b50a565247413 Mon Sep 17 00:00:00 2001 From: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> Date: Fri, 1 Nov 2024 11:14:11 -0400 Subject: [PATCH 3/7] rewview: fix lint issue Signed-off-by: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> --- packages/zosuss/__tests__/__system__/Shell.system.test.ts | 3 ++- packages/zosuss/src/Shell.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/zosuss/__tests__/__system__/Shell.system.test.ts b/packages/zosuss/__tests__/__system__/Shell.system.test.ts index 1f8c07360d..589bcad11f 100644 --- a/packages/zosuss/__tests__/__system__/Shell.system.test.ts +++ b/packages/zosuss/__tests__/__system__/Shell.system.test.ts @@ -149,7 +149,8 @@ describe("zowe uss issue ssh api call test", () => { expect(error.toString()).toContain(ZosUssMessages.connectionRefused.message); } else { expect(error.toString().includes(ZosUssMessages.allAuthMethodsFailed.message) || - error.toString().includes(ZosUssMessages.connectionRefused.message)).toBe(true); + error.toString().includes(ZosUssMessages.connectionRefused.message) || + error.toString().includes(ZosUssMessages.unexpected.message)).toBe(true); } }, TIME_OUT); diff --git a/packages/zosuss/src/Shell.ts b/packages/zosuss/src/Shell.ts index 52f506f360..f6e7caec16 100644 --- a/packages/zosuss/src/Shell.ts +++ b/packages/zosuss/src/Shell.ts @@ -9,7 +9,7 @@ * */ -import { Logger, ImperativeError, IProfileLoaded } from "@zowe/imperative"; +import { Logger, ImperativeError } from "@zowe/imperative"; import { ClientChannel, Client } from "ssh2"; import { SshSession } from "./SshSession"; import { ZosUssMessages } from "./constants/ZosUss.messages"; From 59055b3a80fa512313a5bdae91c3c6c37605569a Mon Sep 17 00:00:00 2001 From: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> Date: Fri, 1 Nov 2024 15:30:30 -0400 Subject: [PATCH 4/7] review: add unit test coverage for new method Signed-off-by: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> --- .../__tests__/__unit__/Shell.unit.test.ts | 17 ++++++++++++++++- packages/zosuss/src/Shell.ts | 3 +-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/zosuss/__tests__/__unit__/Shell.unit.test.ts b/packages/zosuss/__tests__/__unit__/Shell.unit.test.ts index f8c3b397a7..b02fd7d9be 100644 --- a/packages/zosuss/__tests__/__unit__/Shell.unit.test.ts +++ b/packages/zosuss/__tests__/__unit__/Shell.unit.test.ts @@ -54,7 +54,7 @@ const mockShell = jest.fn().mockImplementation((callback) => { (Client as any).mockImplementation(() => { mockClient.connect = mockConnect; mockClient.shell = mockShell; - mockClient.end = jest.fn(); + mockClient.end = jest.fn().mockReturnValue(mockClient); return mockClient; }); @@ -103,6 +103,21 @@ describe("Shell", () => { checkMockFunctionsWithCommand(command); }); + describe("Connection validation", () => { + it("should determine that the connection is valid", async () => { + const response = await Shell.isConnectionValid(fakeSshSession); + expect(response).toBe(true); + }); + it("should determine that the connection is invalid", async () => { + mockConnect.mockImplementationOnce(() => { + mockClient.emit("error", new Error(Shell.connRefusedFlag)); + mockStream.emit("exit", 0); + }); + const response = await Shell.isConnectionValid(fakeSshSession); + expect(response).toBe(false); + }); + }); + describe("Error handling", () => { it("should fail when password is expired", async () => { mockShell.mockImplementationOnce((callback) => { diff --git a/packages/zosuss/src/Shell.ts b/packages/zosuss/src/Shell.ts index f6e7caec16..768d796ed0 100644 --- a/packages/zosuss/src/Shell.ts +++ b/packages/zosuss/src/Shell.ts @@ -174,8 +174,7 @@ export class Shell { public static async isConnectionValid(session: SshSession): Promise{ return new Promise((resolve, _) => { const conn = new Client(); - conn.on("ready", () => conn.end() && resolve(true)) - .on("error", () => resolve(false)); + conn.on("ready", () => conn.end() && resolve(true)).on("error", () => resolve(false)); Shell.connect(conn, session); }); } From e95f9fd41951ca5f1968fca5b6ad167c4816f755 Mon Sep 17 00:00:00 2001 From: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> Date: Sat, 2 Nov 2024 16:16:30 -0400 Subject: [PATCH 5/7] review: add system test for connection validation function :yum: Signed-off-by: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> --- .../__tests__/__system__/Shell.system.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/zosuss/__tests__/__system__/Shell.system.test.ts b/packages/zosuss/__tests__/__system__/Shell.system.test.ts index 9d69797df3..936531fe0f 100644 --- a/packages/zosuss/__tests__/__system__/Shell.system.test.ts +++ b/packages/zosuss/__tests__/__system__/Shell.system.test.ts @@ -35,6 +35,19 @@ describe("zowe uss issue ssh api call test", () => { await TestEnvironment.cleanUp(TEST_ENVIRONMENT); }); + describe("Function isConnectionValid", () => { + it("should verify that the connection is valid", async () => { + const response = await Shell.isConnectionValid(SSH_SESSION); + expect(response).toBe(true); + }); + it("should verify that the connection is invalid", async () => { + const fakeSession: SshSession = TestEnvironment.createSshSession(TEST_ENVIRONMENT); + fakeSession.ISshSession.hostname = "fake-host"; + const response = await Shell.isConnectionValid(fakeSession); + expect(response).toBe(false); + }); + }); + it ("should execute uname command on the remote system by ssh and return operating system name", async () => { const command = "uname"; let stdoutData = ""; From d26a2d033ab2af9dff93cd35af8a49e65969acf0 Mon Sep 17 00:00:00 2001 From: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> Date: Mon, 4 Nov 2024 15:09:52 -0500 Subject: [PATCH 6/7] rewview: update changelog entry for now :yum: Signed-off-by: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> --- packages/zosuss/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/zosuss/CHANGELOG.md b/packages/zosuss/CHANGELOG.md index f57ec1a7d5..7c8438e3d2 100644 --- a/packages/zosuss/CHANGELOG.md +++ b/packages/zosuss/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to the Zowe z/OS USS SDK package will be documented in this ## Recent Changes - BugFix: Removed unnecessary `$ ` characters in front of most output. [zowe-explorer#3079(comment)](https://github.com/zowe/zowe-explorer-vscode/pull/3079#pullrequestreview-2408842655) -- Enhancement: Added the ability to validate of an SSH profile is able to establish a connection. [zowe-explorer#3079(comment)](https://github.com/zowe/zowe-explorer-vscode/pull/3079#discussion_r1825783867) +- Enhancement: Added the ability to validate if an SSH profile can successfully establish a connection, ensuring quicker troubleshooting of connection issues. [zowe-explorer#3079(comment)](https://github.com/zowe/zowe-explorer-vscode/pull/3079#discussion_r1825783867) ## `8.1.1` From deea16fd420867155994fe66fc5afda48ec6fe6c Mon Sep 17 00:00:00 2001 From: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> Date: Mon, 4 Nov 2024 15:55:00 -0500 Subject: [PATCH 7/7] review: add opt-in flag for executeSsh commands Signed-off-by: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> --- .../zosuss/__tests__/__unit__/Shell.unit.test.ts | 8 ++++++++ packages/zosuss/src/Shell.ts | 12 ++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/zosuss/__tests__/__unit__/Shell.unit.test.ts b/packages/zosuss/__tests__/__unit__/Shell.unit.test.ts index b02fd7d9be..7ddb5058aa 100644 --- a/packages/zosuss/__tests__/__unit__/Shell.unit.test.ts +++ b/packages/zosuss/__tests__/__unit__/Shell.unit.test.ts @@ -103,6 +103,14 @@ describe("Shell", () => { checkMockFunctionsWithCommand(command); }); + it("Should execute ssh command with cwd option and no extra characters in the output", async () => { + const cwd = "/"; + const command = "commandtest"; + await Shell.executeSshCwd(fakeSshSession, command, cwd, stdoutHandler, true); + + checkMockFunctionsWithCommand(command); + }); + describe("Connection validation", () => { it("should determine that the connection is valid", async () => { const response = await Shell.isConnectionValid(fakeSshSession); diff --git a/packages/zosuss/src/Shell.ts b/packages/zosuss/src/Shell.ts index 768d796ed0..ed319fbbd0 100644 --- a/packages/zosuss/src/Shell.ts +++ b/packages/zosuss/src/Shell.ts @@ -23,7 +23,8 @@ export class Shell { public static executeSsh(session: SshSession, command: string, - stdoutHandler: (data: string) => void): Promise { + stdoutHandler: (data: string) => void, + removeExtraCharactersFromOutput = false): Promise { let hasAuthFailed = false; const promise = new Promise((resolve, reject) => { const conn = new Client(); @@ -92,7 +93,8 @@ export class Shell { else if (isUserCommand && dataToPrint.length != 0) { if (!dataToPrint.startsWith('\r\n$ '+cmd) && !dataToPrint.startsWith('\r<')){ //only prints command output - if (dataToPrint.startsWith("\r\n$ ")) dataToPrint = dataToPrint.replace(/\r\n\$\s/, "\r\n"); + if (removeExtraCharactersFromOutput && dataToPrint.startsWith("\r\n$ ")) + dataToPrint = dataToPrint.replace(/\r\n\$\s/, "\r\n"); stdoutHandler(dataToPrint); dataToPrint = ""; } @@ -166,9 +168,11 @@ export class Shell { public static async executeSshCwd(session: SshSession, command: string, cwd: string, - stdoutHandler: (data: string) => void): Promise { + stdoutHandler: (data: string) => void, + removeExtraCharactersFromOutput = false + ): Promise { const cwdCommand = `cd ${cwd} && ${command}`; - return this.executeSsh(session, cwdCommand, stdoutHandler); + return this.executeSsh(session, cwdCommand, stdoutHandler, removeExtraCharactersFromOutput); } public static async isConnectionValid(session: SshSession): Promise{