Merge branch 'master' into hkobew/ec2/remoteConnect/telemetry

Hweinstock · Hweinstock · commit 026d39d1c815 · 2024-10-22T11:23:41.000-04:00
diff --git a/.github/workflows/node.js.yml b/.github/workflows/node.js.yml
@@ -80,17 +80,6 @@ jobs:
               uses: coactions/setup-xvfb@v1
               with:
                   run: npm test
-            - name: Code coverage (Core)
-              env:
-                  # Unset NODE_OPTIONS because of https://github.com/codecov/uploader/issues/475
-                  NODE_OPTIONS: ''
-              if: ${{ github.repository == 'aws/aws-toolkit-vscode' && github.ref == 'master' }}
-              uses: codecov/codecov-action@v4
-              with:
-                  flags: macos-core-unittests
-                  verbose: true
-                  file: ./coverage/core/lcov.info
-                  token: ${{ secrets.CODECOV_TOKEN }}
             - name: Code coverage (Toolkit)
               env:
                   # Unset NODE_OPTIONS because of https://github.com/codecov/uploader/issues/475
@@ -113,17 +102,6 @@ jobs:
                   verbose: true
                   file: ./coverage/amazonq/lcov.info
                   token: ${{ secrets.CODECOV_TOKEN }}
-            - name: Code coverage (CodeWhisperer)
-              env:
-                  # Unset NODE_OPTIONS because of https://github.com/codecov/uploader/issues/475
-                  NODE_OPTIONS: ''
-              if: ${{ github.repository == 'aws/aws-toolkit-vscode' && github.ref == 'master' }}
-              uses: codecov/codecov-action@v4
-              with:
-                  flags: codewhisperer
-                  verbose: true
-                  file: ./coverage/core/lcov.info
-                  token: ${{ secrets.CODECOV_TOKEN }}
 
     web:
         needs: lint-commits
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -218,7 +218,7 @@ To run tests against a specific folder in VSCode, do any one of:
 
 ### Coverage report
 
-You can find the coverage report at `./coverage/amazonq/lcov-report/index.html` and `./coverage/core/lcov-report/index.html` after running the tests. Tests ran from the workspace launch config won't generate a coverage report automatically because it can break file watching.
+You can find the coverage report at `./coverage/amazonq/lcov-report/index.html` and `./coverage/toolkit/lcov-report/index.html` after running the tests. Tests ran from the workspace launch config won't generate a coverage report automatically because it can break file watching.
 
 ### CodeCatalyst Blueprints
 
diff --git a/buildspec/linuxE2ETests.yml b/buildspec/linuxE2ETests.yml
@@ -41,7 +41,7 @@ phases:
             - VCS_COMMIT_ID="${CODEBUILD_RESOLVED_SOURCE_VERSION}"
             - CI_BUILD_URL=$(echo $CODEBUILD_BUILD_URL | sed 's/#/%23/g')
             - CI_BUILD_ID="${CODEBUILD_BUILD_ID}"
-            - test -n "${CODECOV_TOKEN}" && [ "$TARGET_BRANCH" = "master" ] && ./codecov --token=${CODECOV_TOKEN} --branch=${CODEBUILD_RESOLVED_SOURCE_VERSION} --repository=${CODEBUILD_SOURCE_REPO_URL} --file=./coverage/core/lcov.info --file=./coverage/amazonq/lcov.info --file=./coverage/toolkit/lcov.info
+            - test -n "${CODECOV_TOKEN}" && [ "$TARGET_BRANCH" = "master" ] && ./codecov --token=${CODECOV_TOKEN} --branch=${CODEBUILD_RESOLVED_SOURCE_VERSION} --repository=${CODEBUILD_SOURCE_REPO_URL} --file=./coverage/amazonq/lcov.info --file=./coverage/toolkit/lcov.info
         finally:
             - rm -rf ~/.aws/sso/cache || true
 reports:
diff --git a/buildspec/linuxIntegrationTests.yml b/buildspec/linuxIntegrationTests.yml
@@ -75,7 +75,7 @@ phases:
             - VCS_COMMIT_ID="${CODEBUILD_RESOLVED_SOURCE_VERSION}"
             - CI_BUILD_URL=$(echo $CODEBUILD_BUILD_URL | sed 's/#/%23/g')
             - CI_BUILD_ID="${CODEBUILD_BUILD_ID}"
-            - test -n "${CODECOV_TOKEN}" && [ "$TARGET_BRANCH" = "master" ] && ./codecov --token=${CODECOV_TOKEN} --branch=${CODEBUILD_RESOLVED_SOURCE_VERSION} --repository=${CODEBUILD_SOURCE_REPO_URL} --file=./coverage/core/lcov.info --file=./coverage/amazonq/lcov.info --file=./coverage/toolkit/lcov.info
+            - test -n "${CODECOV_TOKEN}" && [ "$TARGET_BRANCH" = "master" ] && ./codecov --token=${CODECOV_TOKEN} --branch=${CODEBUILD_RESOLVED_SOURCE_VERSION} --repository=${CODEBUILD_SOURCE_REPO_URL} --file=./coverage/amazonq/lcov.info --file=./coverage/toolkit/lcov.info
     post_build:
         commands:
             # Destroy .netrc to avoid leaking $GITHUB_READONLY_TOKEN.
diff --git a/buildspec/linuxTests.yml b/buildspec/linuxTests.yml
@@ -48,7 +48,7 @@ phases:
             - VCS_COMMIT_ID="${CODEBUILD_RESOLVED_SOURCE_VERSION}"
             - CI_BUILD_URL=$(echo $CODEBUILD_BUILD_URL | sed 's/#/%23/g') # Encode `#` in the URL because otherwise the url is clipped in the Codecov.io site
             - CI_BUILD_ID="${CODEBUILD_BUILD_ID}"
-            - test -n "${CODECOV_TOKEN}" && [ "$TARGET_BRANCH" = "master" ] && ./codecov --token=${CODECOV_TOKEN} --branch=${CODEBUILD_RESOLVED_SOURCE_VERSION} --repository=${CODEBUILD_SOURCE_REPO_URL} --file=./coverage/core/lcov.info --file=./coverage/amazonq/lcov.info --file=./coverage/toolkit/lcov.info
+            - test -n "${CODECOV_TOKEN}" && [ "$TARGET_BRANCH" = "master" ] && ./codecov --token=${CODECOV_TOKEN} --branch=${CODEBUILD_RESOLVED_SOURCE_VERSION} --repository=${CODEBUILD_SOURCE_REPO_URL} --file=./coverage/amazonq/lcov.info --file=./coverage/toolkit/lcov.info
 
 reports:
     unit-test:
diff --git a/packages/core/.c8rc.json b/packages/core/.c8rc.json
diff --git a/packages/core/src/awsService/ec2/sshKeyPair.ts b/packages/core/src/awsService/ec2/sshKeyPair.ts
@@ -45,11 +45,11 @@ export class SshKeyPair {
 
     public static async generateSshKeyPair(keyPath: string): Promise<void> {
         const keyGenerated = await SshKeyPair.tryKeyTypes(keyPath, ['ed25519', 'rsa'])
-        await SshKeyPair.assertGenerated(keyPath, keyGenerated)
         // Should already be the case, but just in case we assert permissions.
         // skip on Windows since it only allows write permission to be changed.
         if (!globals.isWeb && os.platform() !== 'win32') {
             await fs.chmod(keyPath, 0o600)
+            await SshKeyPair.assertGenerated(keyPath, keyGenerated)
         }
     }
     /**
diff --git a/packages/core/src/codewhisperer/commands/basicCommands.ts b/packages/core/src/codewhisperer/commands/basicCommands.ts
@@ -41,8 +41,9 @@ import { ToolkitError, getTelemetryReason, getTelemetryReasonDesc } from '../../
 import { isRemoteWorkspace } from '../../shared/vscode/env'
 import { isBuilderIdConnection } from '../../auth/connection'
 import globals from '../../shared/extensionGlobals'
-import { getVscodeCliPath, tryRun } from '../../shared/utilities/pathFind'
+import { getVscodeCliPath } from '../../shared/utilities/pathFind'
 import { setContext } from '../../shared/vscode/setContext'
+import { tryRun } from '../../shared/utilities/pathFind'
 
 const MessageTimeOut = 5_000
 
diff --git a/packages/core/src/shared/remoteSession.ts b/packages/core/src/shared/remoteSession.ts
@@ -30,7 +30,7 @@ export interface MissingTool {
     readonly reason?: string
 }
 
-const minimumSsmActions = [
+export const minimumSsmActions = [
     'ssmmessages:CreateControlChannel',
     'ssmmessages:CreateDataChannel',
     'ssmmessages:OpenControlChannel',
diff --git a/packages/core/src/shared/telemetry/telemetryService.ts b/packages/core/src/shared/telemetry/telemetryService.ts
@@ -20,6 +20,7 @@ import { ClassToInterfaceType } from '../utilities/tsUtils'
 import { getClientId, validateMetricEvent } from './util'
 import { telemetry, MetricBase } from './telemetry'
 import fs from '../fs/fs'
+import fsNode from 'fs/promises'
 import * as collectionUtil from '../utilities/collectionUtils'
 
 export type TelemetryService = ClassToInterfaceType<DefaultTelemetryService>
@@ -116,7 +117,10 @@ export class DefaultTelemetryService {
             })
 
             try {
-                await fs.writeFile(this.persistFilePath, JSON.stringify(this._eventQueue))
+                /**
+                 * This function runs in deactivate() so we must use node fs. See the vscode behavior doc for more info.
+                 */
+                await fsNode.writeFile(this.persistFilePath, JSON.stringify(this._eventQueue))
             } catch {}
         }
     }
diff --git a/packages/core/src/shared/utilities/pathFind.ts b/packages/core/src/shared/utilities/pathFind.ts
@@ -115,8 +115,8 @@ export async function findTypescriptCompiler(): Promise<string | undefined> {
  * Gets the configured `ssh` path, or falls back to "ssh" (not absolute),
  * or tries common locations, or returns undefined.
  */
-export async function findSshPath(): Promise<string | undefined> {
-    if (sshPath !== undefined) {
+export async function findSshPath(useCache: boolean = true): Promise<string | undefined> {
+    if (useCache && sshPath !== undefined) {
         return sshPath
     }
 
@@ -133,7 +133,7 @@ export async function findSshPath(): Promise<string | undefined> {
             continue
         }
         if (await tryRun(p, ['-G', 'x'], 'noresult' /* "ssh -G" prints quasi-sensitive info. */)) {
-            sshPath = p
+            sshPath = useCache ? p : sshPath
             return p
         }
     }
diff --git a/packages/core/src/test/shared/remoteSession.test.ts b/packages/core/src/test/shared/remoteSession.test.ts
@@ -0,0 +1,32 @@
+/*!
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import assert from 'assert'
+import { minimumSsmActions, promptToAddInlinePolicy } from '../../shared/remoteSession'
+import { IamClient } from '../../shared/clients/iamClient'
+import { getTestWindow } from './vscode/window'
+import { cancel } from '../../shared'
+
+describe('minimumSsmActions', function () {
+    it('should contain minimal actions needed for ssm connection', function () {
+        assert.deepStrictEqual(minimumSsmActions, [
+            'ssmmessages:CreateControlChannel',
+            'ssmmessages:CreateDataChannel',
+            'ssmmessages:OpenControlChannel',
+            'ssmmessages:OpenDataChannel',
+            'ssm:DescribeAssociation',
+            'ssm:ListAssociations',
+            'ssm:UpdateInstanceInformation',
+        ])
+    })
+
+    it('prompts the user for confirmation before adding policies and allow cancels', async function () {
+        getTestWindow().onDidShowMessage((message) => {
+            assert.ok(message.message.includes('add'), 'should prompt to add policies')
+            getTestWindow().getFirstMessage().selectItem(cancel)
+        })
+        const added = await promptToAddInlinePolicy({} as IamClient, 'roleArnTest')
+        assert.ok(!added, 'should not add policies by default')
+    })
+})
diff --git a/packages/core/src/test/shared/utilities/pathFind.test.ts b/packages/core/src/test/shared/utilities/pathFind.test.ts
@@ -7,16 +7,15 @@ import assert from 'assert'
 import * as vscode from 'vscode'
 import * as os from 'os'
 import * as path from 'path'
-
 import * as testutil from '../../testUtil'
-import { findTypescriptCompiler, getVscodeCliPath } from '../../../shared/utilities/pathFind'
 import { fs } from '../../../shared'
+import { findSshPath, findTypescriptCompiler, getVscodeCliPath, tryRun } from '../../../shared/utilities/pathFind'
+import { isCI, isWin } from '../../../shared/vscode/env'
 
 describe('pathFind', function () {
     it('findTypescriptCompiler()', async function () {
-        const iswin = process.platform === 'win32'
         const workspace = vscode.workspace.workspaceFolders![0]
-        const tscNodemodules = path.join(workspace.uri.fsPath, `foo/bar/node_modules/.bin/tsc${iswin ? '.cmd' : ''}`)
+        const tscNodemodules = path.join(workspace.uri.fsPath, `foo/bar/node_modules/.bin/tsc${isWin() ? '.cmd' : ''}`)
         await fs.delete(tscNodemodules, { force: true })
 
         // The test workspace normally doesn't have node_modules so this will
@@ -42,4 +41,90 @@ describe('pathFind', function () {
         const regex = /bin[\\\/](code|code-insiders)$/
         assert.ok(regex.test(vscPath), `expected regex ${regex} to match: "${vscPath}"`)
     })
+
+    describe('findSshPath', function () {
+        let previousPath: string | undefined
+
+        beforeEach(function () {
+            previousPath = process.env.PATH
+        })
+
+        afterEach(function () {
+            process.env.PATH = previousPath
+        })
+
+        it('first tries ssh in $PATH (Non-Windows)', async function () {
+            // skip on windows because ssh in path will never work with .exe extension.
+            if (isWin()) {
+                return
+            }
+            const workspace = await testutil.createTestWorkspaceFolder()
+            const fakeSshPath = path.join(workspace.uri.fsPath, `ssh`)
+
+            process.env.PATH = workspace.uri.fsPath
+            const firstResult = await findSshPath(false)
+
+            await testutil.createExecutableFile(fakeSshPath, '')
+
+            const secondResult = await findSshPath(false)
+
+            assert.notStrictEqual(firstResult, secondResult)
+            assert.strictEqual(secondResult, 'ssh')
+        })
+
+        it('only returns valid executable ssh path (Non-Windows)', async function () {
+            if (isWin()) {
+                return
+            }
+            // On non-windows, we can overwrite path and create our own executable to find.
+            const workspace = await testutil.createTestWorkspaceFolder()
+            const fakeSshPath = path.join(workspace.uri.fsPath, `ssh`)
+
+            process.env.PATH = workspace.uri.fsPath
+
+            await testutil.createExecutableFile(fakeSshPath, '')
+
+            const ssh = await findSshPath(false)
+            assert.ok(ssh)
+            const result = await tryRun(ssh, [], 'yes')
+            assert.ok(result)
+        })
+
+        it('caches result from previous runs (Non-Windows)', async function () {
+            if (isWin()) {
+                return
+            }
+            // On non-windows, we can overwrite path and create our own executable to find.
+            const workspace = await testutil.createTestWorkspaceFolder()
+            // We move the ssh to a temp directory temporarily to test if cache works.
+            const fakeSshPath = path.join(workspace.uri.fsPath, `ssh`)
+
+            process.env.PATH = workspace.uri.fsPath
+
+            await testutil.createExecutableFile(fakeSshPath, '')
+
+            const ssh1 = (await findSshPath(true))!
+
+            await fs.delete(fakeSshPath)
+
+            const ssh2 = await findSshPath(true)
+
+            assert.strictEqual(ssh1, ssh2)
+        })
+
+        it('finds valid executable path (Windows CI)', async function () {
+            // Don't want to be messing with System32 on peoples local machines.
+            if (!isWin() || !isCI()) {
+                return
+            }
+            const expectedPathInCI = 'C:/Windows/System32/OpenSSH/ssh.exe'
+
+            if (!(await fs.exists(expectedPathInCI))) {
+                await testutil.createExecutableFile(expectedPathInCI, '')
+            }
+            const ssh = (await findSshPath(true))!
+            const result = await tryRun(ssh, ['-G', 'x'], 'noresult')
+            assert.ok(result)
+        })
+    })
 })
diff --git a/packages/core/src/test/shared/utilities/testUtils.test.ts b/packages/core/src/test/shared/utilities/testUtils.test.ts
@@ -0,0 +1,39 @@
+/*!
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import assert from 'assert'
+import { createExecutableFile, createTestWorkspaceFolder, copyEnv } from '../../testUtil'
+import path from 'path'
+import { fs } from '../../../shared'
+import { isWin } from '../../../shared/vscode/env'
+import { tryRun } from '../../../shared/utilities/pathFind'
+
+describe('copyEnv', function () {
+    it('modifies the node environment variables (Non-Windows)', function () {
+        // PATH returns undefined on Windows.
+        if (isWin()) {
+            this.skip()
+        }
+
+        const originalPath = copyEnv().PATH
+        const fakePath = 'fakePath'
+        process.env.PATH = fakePath
+        assert.strictEqual(copyEnv().PATH, fakePath)
+
+        process.env.PATH = originalPath
+        assert.strictEqual(copyEnv().PATH, originalPath)
+    })
+})
+
+describe('createExecutableFile', function () {
+    it('creates a file that can be executed', async function () {
+        const tempDir = await createTestWorkspaceFolder()
+        const filePath = path.join(tempDir.uri.fsPath, `exec${isWin() ? '.cmd' : ''}`)
+        await createExecutableFile(filePath, '')
+
+        const result = await tryRun(filePath, [], 'yes')
+        assert.ok(result)
+        await fs.delete(tempDir.uri, { force: true, recursive: true })
+    })
+})
diff --git a/packages/core/src/test/testUtil.ts b/packages/core/src/test/testUtil.ts
@@ -630,3 +630,7 @@ export function tryRegister(command: DeclaredCommand<() => Promise<any>>) {
 export function getFetchStubWithResponse(response: Partial<Response>) {
     return stub(request, 'fetch').returns({ response: new Promise((res, _) => res(response)) } as any)
 }
+
+export function copyEnv(): NodeJS.ProcessEnv {
+    return { ...process.env }
+}
diff --git a/packages/toolkit/.c8rc.json b/packages/toolkit/.c8rc.json
@@ -0,0 +1,6 @@
+{
+    "report-dir": "../../coverage/toolkit",
+    "reporter": ["lcov"],
+    "all": true,
+    "exclude": ["**/test*/**", "**/node_modules/**", "**/ssmServer.js", "**/ssmDocument/external *"]
+}

Original file line number	Diff line number	Diff line change
`@@ -45,11 +45,11 @@ export class SshKeyPair {`
`45`	`45`
`46`	`46`	`public static async generateSshKeyPair(keyPath: string): Promise<void> {`
`47`	`47`	`const keyGenerated = await SshKeyPair.tryKeyTypes(keyPath, ['ed25519', 'rsa'])`
`48`		`- await SshKeyPair.assertGenerated(keyPath, keyGenerated)`
`49`	`48`	`// Should already be the case, but just in case we assert permissions.`
`50`	`49`	`// skip on Windows since it only allows write permission to be changed.`
`51`	`50`	`if (!globals.isWeb && os.platform() !== 'win32') {`
`52`	`51`	`await fs.chmod(keyPath, 0o600)`
	`52`	`+ await SshKeyPair.assertGenerated(keyPath, keyGenerated)`
`53`	`53`	`}`
`54`	`54`	`}`
`55`	`55`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,7 @@ export interface MissingTool {`
`30`	`30`	`readonly reason?: string`
`31`	`31`	`}`
`32`	`32`
`33`		`-const minimumSsmActions = [`
	`33`	`+export const minimumSsmActions = [`
`34`	`34`	`'ssmmessages:CreateControlChannel',`
`35`	`35`	`'ssmmessages:CreateDataChannel',`
`36`	`36`	`'ssmmessages:OpenControlChannel',`