[CopyFilesOverSSH] Migrated from scp2 to ssh2-sftp-client (#13084)

* [CopyFilesOverSSHV0] Migrated from scp2 library (#13048) * [CopyFilesOverSSHV0] Added info about ssh2-sftp-client to ThirdPartyNotice.txt * [CopyFilesOverSSHV0] Addressed code review items. * [CopyFilesOverSSH] Added missing dependencies * [CopyFilesOverSSH] Fixed issue with e2e launch
microsoft · Jun 22, 2020 · 89dfff6 · 89dfff6
1 parent e5a5aae
commit 89dfff6
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 80 deletions.
diff --git a/Tasks/CopyFilesOverSSHV0/ThirdPartyNotice.txt b/Tasks/CopyFilesOverSSHV0/ThirdPartyNotice.txt
@@ -24,7 +24,7 @@ This Azure DevOps extension (CopyFilesOverSSHV0) is based on or incorporates mat
 18.	path-is-absolute (git+https://github.com/sindresorhus/path-is-absolute.git)
 19.	q (git://github.com/kriskowal/q.git)
 20.	readable-stream (git://github.com/isaacs/readable-stream.git)
-21.	scp2 (git+https://github.com/lepture/node-scp2.git)
+21.	ssh2-sftp-client (git+https://https://github.com/theophilusx/ssh2-sftp-client.git)
 22.	semver (git+https://github.com/npm/node-semver.git)
 23.	shelljs (git://github.com/arturadib/shelljs.git)
 24.	ssh2 (git+ssh://[email protected]/mscdex/ssh2.git)

diff --git a/Tasks/CopyFilesOverSSHV0/copyfilesoverssh.ts b/Tasks/CopyFilesOverSSHV0/copyfilesoverssh.ts
@@ -247,14 +247,14 @@ async function run() {
         // close the client connection to halt build execution
         if (sshHelper) {
             tl.debug('Closing the client connection');
-            sshHelper.closeConnection();
+            await sshHelper.closeConnection();
         }
     }
 }
 
 run().then(() => {
-        tl.debug('Task successfully accomplished');
-    })
+    tl.debug('Task successfully accomplished');
+})
     .catch(err => {
         tl.debug('Run was unexpectedly failed due to: ' + err);
     });

diff --git a/Tasks/CopyFilesOverSSHV0/package.json b/Tasks/CopyFilesOverSSHV0/package.json
@@ -17,8 +17,8 @@
   },
   "homepage": "https://github.com/Microsoft.com/vsts-tasks#readme",
   "dependencies": {
-    "scp2": "^0.5.0",
     "ssh2": "^0.8.2",
+    "ssh2-sftp-client": "^5.1.2",
     "minimatch": "^3.0.4",
     "azure-pipelines-task-lib": "^2.9.3"
   }

diff --git a/Tasks/CopyFilesOverSSHV0/sshhelper.ts b/Tasks/CopyFilesOverSSHV0/sshhelper.ts
@@ -1,18 +1,17 @@
 import Q = require('q');
 import tl = require('azure-pipelines-task-lib/task');
 var Ssh2Client = require('ssh2').Client;
-var Scp2Client = require('scp2').Client;
+var SftpClient = require('ssh2-sftp-client');
 
 export class RemoteCommandOptions {
-    public failOnStdErr : boolean;
+    public failOnStdErr: boolean;
 }
 
 export class SshHelper {
     private sshConfig: any;
     private sshClient: any;
-    private scpClient: any;
     private sftpClient: any;
-    
+
     /**
      * Constructor that takes a configuration object of format
      * {
@@ -28,7 +27,7 @@ export class SshHelper {
         this.sshConfig = sshConfig;
     }
 
-    private async setupSshClientConnection() : Promise<void> {
+    private async setupSshClientConnection(): Promise<void> {
         const defer = Q.defer<void>();
         this.sshClient = new Ssh2Client();
         this.sshClient.once('ready', () => {
@@ -39,18 +38,16 @@ export class SshHelper {
         await defer.promise;
     }
 
-    private async setupScpConnection() : Promise<void> {
+    private async setupSftpConnection(): Promise<void> {
         const defer = Q.defer<void>();
-        this.scpClient = new Scp2Client();
-        this.scpClient.defaults(this.sshConfig);
-        this.scpClient.sftp((err, sftp) => {
-            if(err) {
-                defer.reject(tl.loc('ConnectionFailed', err));
-            } else {
-                this.sftpClient = sftp;
-                defer.resolve();
-            }
-        })
+        try {
+            this.sftpClient = new SftpClient();
+            await this.sftpClient.connect(this.sshConfig)
+            defer.resolve();
+        } catch (err) {
+            this.sftpClient = null;
+            defer.reject(tl.loc('ConnectionFailed', err));
+        }
         await defer.promise;
     }
 
@@ -61,25 +58,22 @@ export class SshHelper {
         console.log(tl.loc('SettingUpSSHConnection', this.sshConfig.host));
         try {
             await this.setupSshClientConnection();
-            await this.setupScpConnection();
-        } catch(err) {
+            await this.setupSftpConnection();
+        } catch (err) {
             throw new Error(tl.loc('ConnectionFailed', err));
         }
     }
 
     /**
      * Close any open client connections for SSH, SCP and SFTP
      */
-    closeConnection() {
+    async closeConnection() {
         try {
             if (this.sftpClient) {
-                this.sftpClient.on('error', (err) => {
-                    tl.debug('sftpClient: Ignoring error diconnecting: ' + err);
-                }); // ignore logout errors; see: https://github.com/mscdex/node-imap/issues/695
-                this.sftpClient.close();
+                await this.sftpClient.end();
                 this.sftpClient = null;
             }
-        } catch(err) {
+        } catch (err) {
             tl.debug('Failed to close SFTP client: ' + err);
         }
         try {
@@ -90,21 +84,10 @@ export class SshHelper {
                 this.sshClient.end();
                 this.sshClient = null;
             }
-        } catch(err) {
+        } catch (err) {
             tl.debug('Failed to close SSH client: ' + err);
         }
 
-        try {
-            if (this.scpClient) {
-                this.scpClient.on('error', (err) => {
-                    tl.debug('scpClient: Ignoring error diconnecting: ' + err);
-                }); // ignore logout errors; see: https://github.com/mscdex/node-imap/issues/695
-                this.scpClient.close();
-                this.scpClient = null;
-            }
-        } catch(err) {
-            tl.debug('Failed to close SCP client: ' + err);
-        }
     }
 
     /**
@@ -113,19 +96,18 @@ export class SshHelper {
      * @param dest, folders will be created if they do not exist on remote server
      * @returns {Promise<string>}
      */
-    uploadFile(sourceFile: string, dest: string) : Q.Promise<string> {
+    async uploadFile(sourceFile: string, dest: string): Promise<string> {
         tl.debug('Upload ' + sourceFile + ' to ' + dest + ' on remote machine.');
         var defer = Q.defer<string>();
-        if(!this.scpClient) {
+        if (!this.sftpClient) {
             defer.reject(tl.loc('ConnectionNotSetup'));
         }
-        this.scpClient.upload(sourceFile, dest, (err) => {
-            if(err) {
-                defer.reject(tl.loc('UploadFileFailed', sourceFile, dest, err));
-            } else {
-                defer.resolve(dest);
-            }
-        })
+        try {
+            await this.sftpClient.put(sourceFile, dest);
+            defer.resolve(dest);
+        } catch (err) {
+            defer.reject(tl.loc('UploadFileFailed', sourceFile, dest, err));
+        }
         return defer.promise;
     }
 
@@ -134,21 +116,20 @@ export class SshHelper {
      * @param path
      * @returns {Promise<boolean>}
      */
-    checkRemotePathExists(path: string) : Q.Promise<boolean> {
+    async checkRemotePathExists(path: string): Promise<boolean> {
         var defer = Q.defer<boolean>();
 
-        if(!this.sftpClient) {
+        if (!this.sftpClient) {
             defer.reject(tl.loc('ConnectionNotSetup'));
         }
-        this.sftpClient.stat(path, function(err, attr) {
-            if(err) {
-                //path does not exist
-                defer.resolve(false);
-            } else {
-                //path exists
-                defer.resolve(true);
-            }
-        })
+
+        if (await this.sftpClient.stat(path)) {
+            //path exists
+            defer.resolve(true);
+        } else {
+            //path does not exist
+            defer.resolve(false);
+        }
 
         return defer.promise;
     }
@@ -159,41 +140,41 @@ export class SshHelper {
      * @param options
      * @returns {Promise<string>}
      */
-    runCommandOnRemoteMachine(command: string, options: RemoteCommandOptions) : Q.Promise<string> {
-        var defer = Q.defer<string>();
-        var stdErrWritten:boolean = false;
+    runCommandOnRemoteMachine(command: string, options: RemoteCommandOptions): Q.Promise<string> {
+        const defer = Q.defer<string>();
+        let stdErrWritten: boolean = false;
 
-        if(!this.sshClient) {
+        if (!this.sshClient) {
             defer.reject(tl.loc('ConnectionNotSetup'));
         }
 
-        if(!options) {
+        if (!options) {
             tl.debug('Options not passed to runCommandOnRemoteMachine, setting defaults.');
             var options = new RemoteCommandOptions();
             options.failOnStdErr = true;
         }
 
         var cmdToRun = command;
-        if(cmdToRun.indexOf(';') > 0) {
+        if (cmdToRun.indexOf(';') > 0) {
             //multiple commands were passed separated by ;
             cmdToRun = cmdToRun.replace(/;/g, '\n');
         }
         tl.debug('cmdToRun = ' + cmdToRun);
 
         this.sshClient.exec(cmdToRun, (err, stream) => {
-            if(err) {
+            if (err) {
                 defer.reject(tl.loc('RemoteCmdExecutionErr', cmdToRun, err))
             }
             stream.on('close', (code, signal) => {
                 tl.debug('code = ' + code + ', signal = ' + signal);
-                if(code && code != 0) {
+                if (code && code !== 0) {
                     //non zero exit code - fail
                     defer.reject(tl.loc('RemoteCmdNonZeroExitCode', cmdToRun, code));
                 } else {
                     //no exit code or exit code of 0
 
                     //based on the options decide whether to fail the build or not if data was written to STDERR
-                    if(stdErrWritten === true && options.failOnStdErr === true) {
+                    if (stdErrWritten && options.failOnStdErr) {
                         //stderr written - fail the build
                         defer.reject(tl.loc('RemoteCmdExecutionErr', cmdToRun, tl.loc('CheckLogForStdErr')));
                     } else {
@@ -204,12 +185,12 @@ export class SshHelper {
             }).on('data', (data) => {
                 console.log(data);
             }).stderr.on('data', (data) => {
-                    stdErrWritten = true;
-                    tl.debug('stderr = ' + data);
-                    if(data && data.toString().trim() !== '') {
-                        tl.error(data);
-                    }
-                });
+                stdErrWritten = true;
+                tl.debug('stderr = ' + data);
+                if (data && data.toString().trim() !== '') {
+                    tl.error(data);
+                }
+            });
         });
         return defer.promise;
     }

diff --git a/Tasks/CopyFilesOverSSHV0/task.json b/Tasks/CopyFilesOverSSHV0/task.json
@@ -17,7 +17,7 @@
     "author": "Microsoft Corporation",
     "version": {
         "Major": 0,
-        "Minor": 171,
+        "Minor": 172,
         "Patch": 0
     },
     "demands": [],
@@ -123,7 +123,7 @@
         }
     ],
     "execution": {
-        "Node": {
+        "Node10": {
             "target": "copyfilesoverssh.js",
             "argumentFormat": ""
         }

diff --git a/Tasks/CopyFilesOverSSHV0/task.loc.json b/Tasks/CopyFilesOverSSHV0/task.loc.json
@@ -17,7 +17,7 @@
   "author": "Microsoft Corporation",
   "version": {
     "Major": 0,
-    "Minor": 171,
+    "Minor": 172,
     "Patch": 0
   },
   "demands": [],
@@ -123,7 +123,7 @@
     }
   ],
   "execution": {
-    "Node": {
+    "Node10": {
       "target": "copyfilesoverssh.js",
       "argumentFormat": ""
     }