use os to determine command

Author: Hweinstock

packages/core/src/awsService/ec2/model.ts

@@ -28,7 +28,7 @@ import { showMessageWithCancel } from '../../shared/utilities/messages'
 import { SshConfig } from '../../shared/sshConfig'
 import { SshKeyPair } from './sshKeyPair'
 import { Ec2SessionTracker } from './remoteSessionManager'
-import { getEc2SsmEnv, getRemoveLinesCommand } from './utils'
+import { getEc2SsmEnv } from './utils'
 
 export type Ec2ConnectErrorCode = 'EC2SSMStatus' | 'EC2SSMPermission' | 'EC2SSMConnect' | 'EC2SSMAgentStatus'
 
@@ -38,6 +38,12 @@ export interface Ec2RemoteEnv extends VscodeRemoteConnection {
     ssmSession: SSM.StartSessionResponse
 }
 
+export type Ec2OS = 'Amazon Linux' | 'Ubuntu' | 'macOS'
+interface RemoteUser {
+    os: Ec2OS
+    name: string
+}
+
 export class Ec2Connecter implements vscode.Disposable {
     protected ssmClient: SsmClient
     protected ec2Client: Ec2Client
@@ -195,20 +201,29 @@ export class Ec2Connecter implements vscode.Disposable {
         const remoteEnv = await this.prepareEc2RemoteEnvWithProgress(selection, remoteUser)
 
         try {
-            await startVscodeRemote(remoteEnv.SessionProcess, remoteEnv.hostname, '/', remoteEnv.vscPath, remoteUser)
+            await startVscodeRemote(
+                remoteEnv.SessionProcess,
+                remoteEnv.hostname,
+                '/',
+                remoteEnv.vscPath,
+                remoteUser.name
+            )
         } catch (err) {
             this.throwGeneralConnectionError(selection, err as Error)
         }
     }
 
-    public async prepareEc2RemoteEnvWithProgress(selection: Ec2Selection, remoteUser: string): Promise<Ec2RemoteEnv> {
+    public async prepareEc2RemoteEnvWithProgress(
+        selection: Ec2Selection,
+        remoteUser: RemoteUser
+    ): Promise<Ec2RemoteEnv> {
         const timeout = new Timeout(60000)
         await showMessageWithCancel('AWS: Opening remote connection...', timeout)
         const remoteEnv = await this.prepareEc2RemoteEnv(selection, remoteUser).finally(() => timeout.cancel())
         return remoteEnv
     }
 
-    public async prepareEc2RemoteEnv(selection: Ec2Selection, remoteUser: string): Promise<Ec2RemoteEnv> {
+    public async prepareEc2RemoteEnv(selection: Ec2Selection, remoteUser: RemoteUser): Promise<Ec2RemoteEnv> {
         const logger = this.configureRemoteConnectionLogger(selection.instanceId)
         const { ssm, vsc, ssh } = (await ensureDependencies()).unwrap()
         const keyPair = await this.configureSshKeys(selection, remoteUser)
@@ -253,19 +268,23 @@ export class Ec2Connecter implements vscode.Disposable {
         return logger
     }
 
-    public async configureSshKeys(selection: Ec2Selection, remoteUser: string): Promise<SshKeyPair> {
+    public async configureSshKeys(selection: Ec2Selection, remoteUser: RemoteUser): Promise<SshKeyPair> {
         const keyPair = await SshKeyPair.getSshKeyPair(`aws-ec2-key`, 30000)
         await this.sendSshKeyToInstance(selection, keyPair, remoteUser)
         return keyPair
     }
 
-    private async attemptToCleanKeys(instanceId: string, hintComment: string, remoteAuthorizedKeysPath: string) {
+    private async attemptToCleanKeys(
+        instanceId: string,
+        hintComment: string,
+        hostOS: Ec2OS,
+        remoteAuthorizedKeysPath: string
+    ) {
         try {
-            const deleteExistingKeyCommand = getRemoveLinesCommand(hintComment, remoteAuthorizedKeysPath)
-            const result = await this.ssmClient.sendCommandAndWait(instanceId, 'AWS-RunShellScript', {
+            const deleteExistingKeyCommand = getRemoveLinesCommand(hintComment, hostOS, remoteAuthorizedKeysPath)
+            await this.ssmClient.sendCommandAndWait(instanceId, 'AWS-RunShellScript', {
                 commands: [deleteExistingKeyCommand],
             })
-            console.log(result)
         } catch (e) {
             getLogger().warn(`ec2: failed to clean keys: %O`, e)
         }
@@ -274,32 +293,47 @@ export class Ec2Connecter implements vscode.Disposable {
     public async sendSshKeyToInstance(
         selection: Ec2Selection,
         sshKeyPair: SshKeyPair,
-        remoteUser: string
+        remoteUser: RemoteUser
     ): Promise<void> {
         const sshPubKey = await sshKeyPair.getPublicKey()
         const hintComment = '#AWSToolkitForVSCode'
 
-        const remoteAuthorizedKeysPath = `/home/${remoteUser}/.ssh/authorized_keys`
+        const remoteAuthorizedKeysPath = `/home/${remoteUser.name}/.ssh/authorized_keys`
 
         const appendStr = (s: string) => `echo "${s}" >> ${remoteAuthorizedKeysPath}`
         const writeKeyCommand = appendStr([sshPubKey.replace('\n', ''), hintComment].join(' '))
 
-        await this.attemptToCleanKeys(selection.instanceId, hintComment, remoteAuthorizedKeysPath)
+        await this.attemptToCleanKeys(selection.instanceId, hintComment, remoteUser.os, remoteAuthorizedKeysPath)
         await this.ssmClient.sendCommandAndWait(selection.instanceId, 'AWS-RunShellScript', {
             commands: [writeKeyCommand],
         })
     }
 
-    public async getRemoteUser(instanceId: string) {
-        const osName = await this.ssmClient.getTargetPlatformName(instanceId)
-        if (osName === 'Amazon Linux') {
-            return 'ec2-user'
+    public async getRemoteUser(instanceId: string): Promise<RemoteUser> {
+        const os = await this.ssmClient.getTargetPlatformName(instanceId)
+        if (os === 'Amazon Linux') {
+            return { name: 'ec2-user', os }
         }
 
-        if (osName === 'Ubuntu') {
-            return 'ubuntu'
+        if (os === 'Ubuntu') {
+            return { name: 'ubuntu', os }
         }
 
-        throw new ToolkitError(`Unrecognized OS name ${osName} on instance ${instanceId}`, { code: 'UnknownEc2OS' })
+        throw new ToolkitError(`Unrecognized OS name ${os} on instance ${instanceId}`, { code: 'UnknownEc2OS' })
     }
 }
+
+/**
+ * Generate bash command (as string) to remove lines in file suffixed by `hintComment`.
+ * @param hintComment suffix comment for deleted lines.
+ * @param filepath filepath (as string) to target with the command.
+ * @returns bash command to remove lines from file.
+ */
+export function getRemoveLinesCommand(hintComment: string, hostOS: Ec2OS, filepath: string): string {
+    // Linux allows not passing extension to -i, whereas macOS requires zero length extension.
+    return `sed -i ${isLinux(hostOS) ? '' : "''"} /${hintComment}/d ${filepath}`
+}
+
+function isLinux(os: Ec2OS): boolean {
+    return os === 'Amazon Linux' || os === 'Ubuntu'
+}

packages/core/src/awsService/ec2/utils.ts

@@ -46,12 +46,3 @@ export function getEc2SsmEnv(
         process.env
     )
 }
-/**
- * Generate bash command (as string) to remove lines in file suffixed by `hintComment`.
- * @param hintComment suffix comment for deleted lines.
- * @param filepath filepath (as string) to target with the command.
- * @returns bash command to remove lines from file.
- */
-export function getRemoveLinesCommand(hintComment: string, filepath: string): string {
-    return `sed -i '' /${hintComment}/d ${filepath}`
-}

packages/core/src/shared/vscode/env.ts

@@ -134,7 +134,7 @@ export function isRemoteWorkspace(): boolean {
  * Example: `5.10.220-188.869.amzn2int.x86_64`
  */
 export function isAmazonInternalOs() {
-    return os.release().includes('amzn2int') && process.platform === 'linux'
+    return os.release().includes('amzn2int') && isLinux()
 }
 
 /**
@@ -158,6 +158,10 @@ export function isWin(): boolean {
     return process.platform === 'win32'
 }
 
+export function isLinux(): boolean {
+    return process.platform === 'linux'
+}
+
 const UIKind = {
     [vscode.UIKind.Desktop]: 'desktop',
     [vscode.UIKind.Web]: 'web',

packages/core/src/test/awsService/ec2/model.test.ts

@@ -5,7 +5,7 @@
 
 import assert from 'assert'
 import * as sinon from 'sinon'
-import { Ec2Connecter } from '../../../awsService/ec2/model'
+import { Ec2Connecter, getRemoveLinesCommand } from '../../../awsService/ec2/model'
 import { SsmClient } from '../../../shared/clients/ssmClient'
 import { Ec2Client } from '../../../shared/clients/ec2Client'
 import { Ec2Selection } from '../../../awsService/ec2/prompter'
@@ -15,6 +15,9 @@ import { SshKeyPair } from '../../../awsService/ec2/sshKeyPair'
 import { DefaultIamClient } from '../../../shared/clients/iamClient'
 import { assertNoTelemetryMatch, createTestWorkspaceFolder } from '../../testUtil'
 import { fs } from '../../../shared'
+import path from 'path'
+import { ChildProcess } from '../../../shared/utilities/processUtils'
+import { isLinux, isWin } from '../../../shared/vscode/env'
 
 describe('Ec2ConnectClient', function () {
     let client: Ec2Connecter
@@ -140,7 +143,7 @@ describe('Ec2ConnectClient', function () {
             }
 
             const keys = await SshKeyPair.getSshKeyPair('key', 30000)
-            await client.sendSshKeyToInstance(testSelection, keys, 'test-user')
+            await client.sendSshKeyToInstance(testSelection, keys, { name: 'test-user', os: 'Amazon Linux' })
             sinon.assert.calledWith(sendCommandStub, testSelection.instanceId, 'AWS-RunShellScript')
             sinon.restore()
         })
@@ -154,7 +157,7 @@ describe('Ec2ConnectClient', function () {
             }
             const testWorkspaceFolder = await createTestWorkspaceFolder()
             const keys = await SshKeyPair.getSshKeyPair('key', 60000)
-            await client.sendSshKeyToInstance(testSelection, keys, 'test-user')
+            await client.sendSshKeyToInstance(testSelection, keys, { name: 'test-user', os: 'Amazon Linux' })
             const privKey = await fs.readFileText(keys.getPrivateKeyPath())
             assertNoTelemetryMatch(privKey)
             sinon.restore()
@@ -178,13 +181,13 @@ describe('Ec2ConnectClient', function () {
         it('identifies the user for ubuntu as ubuntu', async function () {
             getTargetPlatformNameStub.resolves('Ubuntu')
             const remoteUser = await client.getRemoteUser('testInstance')
-            assert.strictEqual(remoteUser, 'ubuntu')
+            assert.strictEqual(remoteUser.name, 'ubuntu')
         })
 
         it('identifies the user for amazon linux as ec2-user', async function () {
             getTargetPlatformNameStub.resolves('Amazon Linux')
             const remoteUser = await client.getRemoteUser('testInstance')
-            assert.strictEqual(remoteUser, 'ec2-user')
+            assert.strictEqual(remoteUser.name, 'ec2-user')
         })
 
         it('throws error when not given known OS', async function () {
@@ -198,3 +201,40 @@ describe('Ec2ConnectClient', function () {
         })
     })
 })
+
+describe('getRemoveLinesCommand', async function () {
+    let tempPath: { uri: { fsPath: string } }
+
+    before(async function () {
+        tempPath = await createTestWorkspaceFolder()
+    })
+
+    after(async function () {
+        await fs.delete(tempPath.uri.fsPath, { recursive: true, force: true })
+    })
+
+    it('removes lines prefixed by pattern', async function () {
+        if (isWin()) {
+            this.skip()
+        }
+        // For the test, we only need to distinguish mac and linux
+        const hostOS = isLinux() ? 'Amazon Linux' : 'macOS'
+        const lines = ['line1', 'line2 pattern', 'line3', 'line4 pattern', 'line5', 'line6 pattern', 'line7']
+        const expected = ['line1', 'line3', 'line5', 'line7']
+
+        const lineToStr = (ls: string[]) => ls.join('\n') + '\n'
+
+        const textFile = path.join(tempPath.uri.fsPath, 'test.txt')
+        const originalContent = lineToStr(lines)
+        await fs.writeFile(textFile, originalContent)
+        const [command, ...args] = getRemoveLinesCommand('pattern', hostOS, textFile).split(' ')
+        const process = new ChildProcess(command, args, { collect: true })
+        const result = await process.run()
+
+        assert.strictEqual(result.exitCode, 0, `ChildProcess failed with error=${result.error}`)
+
+        const newContent = await fs.readFileText(textFile)
+        assert.notStrictEqual(newContent, originalContent)
+        assert.strictEqual(newContent, lineToStr(expected))
+    })
+})

packages/core/src/test/awsService/ec2/utils.test.ts

@@ -6,12 +6,8 @@
 import assert from 'assert'
 import * as sinon from 'sinon'
 import { SafeEc2Instance } from '../../../shared/clients/ec2Client'
-import { getIconCode, getRemoveLinesCommand } from '../../../awsService/ec2/utils'
+import { getIconCode } from '../../../awsService/ec2/utils'
 import { DefaultAwsContext } from '../../../shared'
-import { fs } from '../../../shared/fs/fs'
-import { createTestWorkspaceFolder } from '../../testUtil'
-import path from 'path'
-import { ChildProcess } from '../../../shared/utilities/processUtils'
 
 describe('utils', async function () {
     before(function () {
@@ -56,37 +52,4 @@ describe('utils', async function () {
             assert.strictEqual(getIconCode(stoppingInstance), 'loading~spin')
         })
     })
-
-    describe('getRemoveLinesCommand', async function () {
-        let tempPath: { uri: { fsPath: string } }
-
-        before(async function () {
-            tempPath = await createTestWorkspaceFolder()
-        })
-
-        after(async function () {
-            await fs.delete(tempPath.uri.fsPath, { recursive: true, force: true })
-        })
-
-        it('removes lines prefixed by pattern', async function () {
-            const lines = ['line1', 'line2 pattern', 'line3', 'line4 pattern', 'line5', 'line6 pattern', 'line7']
-            const expected = ['line1', 'line3', 'line5', 'line7']
-
-            const lineToStr = (ls: string[]) => ls.join('\n') + '\n'
-
-            const textFile = path.join(tempPath.uri.fsPath, 'test.txt')
-            const originalContent = lineToStr(lines)
-            await fs.writeFile(textFile, originalContent)
-
-            const [command, ...args] = getRemoveLinesCommand('pattern', textFile).split(' ')
-            const process = new ChildProcess(command, args, { collect: true })
-            const result = await process.run()
-
-            assert.strictEqual(result.exitCode, 0, `ChildProcess failed with error=${result.error}`)
-
-            const newContent = await fs.readFileText(textFile)
-            assert.notStrictEqual(newContent, originalContent)
-            assert.strictEqual(newContent, lineToStr(expected))
-        })
-    })
 })