diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 49a1e209b..8330ddb05 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -461,7 +461,6 @@ jobs: uses: actions/checkout@v3 with: fetch-depth: 0 - submodules: true - name: Install Node.js v16 uses: actions/setup-node@v3 @@ -491,7 +490,7 @@ jobs: - name: Install dependencies if: steps.cache-yarn.outputs.cache-hit != 'true' - run: yarn --frozen-lockfile + run: SKIP_SUBMODULE_DEPS=1 yarn --frozen-lockfile - name: Install Playwright OS dependencies run: | @@ -511,6 +510,93 @@ jobs: - name: Remove release packages and test artifacts run: rm -rf ./release-packages ./test/test-results + test-e2e-proxy: + name: End-to-end tests behind proxy + needs: package-linux-amd64 + runs-on: ubuntu-latest + timeout-minutes: 25 + env: + # Since we build code-server we might as well run tests from the release + # since VS Code will load faster due to the bundling. + CODE_SERVER_TEST_ENTRY: "./release-packages/code-server-linux-amd64" + steps: + - name: Checkout repo + uses: actions/checkout@v3 + with: + fetch-depth: 0 + + - name: Install Node.js v16 + uses: actions/setup-node@v3 + with: + node-version: "16" + + - name: Fetch dependencies from cache + id: cache-yarn + uses: actions/cache@v3 + with: + path: "**/node_modules" + key: yarn-build-${{ hashFiles('**/yarn.lock') }} + restore-keys: | + yarn-build- + + - name: Download release packages + uses: actions/download-artifact@v3 + with: + name: release-packages + path: ./release-packages + + - name: Untar code-server release + run: | + cd release-packages + tar -xzf code-server*-linux-amd64.tar.gz + mv code-server*-linux-amd64 code-server-linux-amd64 + + - name: Install dependencies + if: steps.cache-yarn.outputs.cache-hit != 'true' + run: SKIP_SUBMODULE_DEPS=1 yarn --frozen-lockfile + + - name: Install Playwright OS dependencies + run: | + ./test/node_modules/.bin/playwright install-deps + ./test/node_modules/.bin/playwright install + + - name: Cache Caddy + uses: actions/cache@v2 + id: caddy-cache + with: + path: | + ~/.cache/caddy + key: cache-caddy-2.5.2 + + - name: Install Caddy + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + if: steps.caddy-cache.outputs.cache-hit != 'true' + run: | + gh release download v2.5.2 --repo caddyserver/caddy --pattern "caddy_2.5.2_linux_amd64.tar.gz" + mkdir -p ~/.cache/caddy + tar -xzf caddy_2.5.2_linux_amd64.tar.gz --directory ~/.cache/caddy + + - name: Start Caddy + run: sudo ~/.cache/caddy/caddy start --config ./ci/Caddyfile + + - name: Run end-to-end tests + run: yarn test:e2e:proxy + + - name: Stop Caddy + if: always() + run: sudo ~/.cache/caddy/caddy stop --config ./ci/Caddyfile + + - name: Upload test artifacts + if: always() + uses: actions/upload-artifact@v3 + with: + name: failed-test-videos-proxy + path: ./test/test-results + + - name: Remove release packages and test artifacts + run: rm -rf ./release-packages ./test/test-results + trivy-scan-repo: permissions: contents: read # for actions/checkout to fetch code diff --git a/ci/Caddyfile b/ci/Caddyfile new file mode 100644 index 000000000..52e4da6da --- /dev/null +++ b/ci/Caddyfile @@ -0,0 +1,15 @@ +{ + admin localhost:4444 +} +:8000 { + @portLocalhost path_regexp port ^/([0-9]+)\/ide + handle @portLocalhost { + uri strip_prefix {re.port.1}/ide + reverse_proxy localhost:{re.port.1} + } + + handle { + respond "Bad hostname" 400 + } + +} diff --git a/docs/MAINTAINING.md b/docs/MAINTAINING.md index 18e3a8f0e..a8944ba10 100644 --- a/docs/MAINTAINING.md +++ b/docs/MAINTAINING.md @@ -175,7 +175,7 @@ If you're the current release manager, follow these steps: 1. Bump chart version in `Chart.yaml`. 1. Summarize the major changes in the release notes and link to the relevant issues. -1. Change the @ to target the version branch. Example: `v3.9.0 @ Target: v3.9.0` +1. Change the @ to target the version branch. Example: `v3.9.0 @ Target: release/v3.9.0` 1. Wait for the `npm-package`, `release-packages` and `release-images` artifacts to build. 1. Run `yarn release:github-assets` to download the `release-packages` artifact. diff --git a/package.json b/package.json index 1bb923c29..406e23b80 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,7 @@ "release:github-assets": "./ci/build/release-github-assets.sh", "release:prep": "./ci/build/release-prep.sh", "test:e2e": "VSCODE_IPC_HOOK_CLI= ./ci/dev/test-e2e.sh", + "test:e2e:proxy": "USE_PROXY=1 ./ci/dev/test-e2e.sh", "test:unit": "./ci/dev/test-unit.sh --forceExit --detectOpenHandles", "test:integration": "./ci/dev/test-integration.sh", "test:scripts": "./ci/dev/test-scripts.sh", diff --git a/test/e2e/baseFixture.ts b/test/e2e/baseFixture.ts index 1efad7f7a..a1293f061 100644 --- a/test/e2e/baseFixture.ts +++ b/test/e2e/baseFixture.ts @@ -1,4 +1,3 @@ -import { field, logger } from "@coder/logger" import { test as base } from "@playwright/test" import { CodeServer, CodeServerPage } from "./models/CodeServer" @@ -11,14 +10,13 @@ import { CodeServer, CodeServerPage } from "./models/CodeServer" */ export const describe = ( name: string, - includeCredentials: boolean, codeServerArgs: string[], codeServerEnv: NodeJS.ProcessEnv, fn: (codeServer: CodeServer) => void, ) => { test.describe(name, () => { // This will spawn on demand so nothing is necessary on before. - const codeServer = new CodeServer(name, codeServerArgs, codeServerEnv) + const codeServer = new CodeServer(name, codeServerArgs, codeServerEnv, undefined) // Kill code-server after the suite has ended. This may happen even without // doing it explicitly but it seems prudent to be sure. @@ -26,22 +24,10 @@ export const describe = ( await codeServer.close() }) - const storageState = JSON.parse(process.env.STORAGE || "{}") - - // Sanity check to ensure the cookie is set. - const cookies = storageState?.cookies - if (includeCredentials && (!cookies || cookies.length !== 1 || !!cookies[0].key)) { - logger.error("no cookies", field("storage", JSON.stringify(cookies))) - throw new Error("no credentials to include") - } - test.use({ // Makes `codeServer` and `authenticated` available to the extend call // below. codeServer, - authenticated: includeCredentials, - // This provides a cookie that authenticates with code-server. - storageState: includeCredentials ? storageState : {}, // NOTE@jsjoeio some tests use --cert which uses a self-signed certificate // without this option, those tests will fail. ignoreHTTPSErrors: true, @@ -52,7 +38,6 @@ export const describe = ( } interface TestFixtures { - authenticated: boolean codeServer: CodeServer codeServerPage: CodeServerPage } @@ -62,15 +47,14 @@ interface TestFixtures { * ready. */ export const test = base.extend({ - authenticated: false, codeServer: undefined, // No default; should be provided through `test.use`. - codeServerPage: async ({ authenticated, codeServer, page }, use) => { + codeServerPage: async ({ codeServer, page }, use) => { // It's possible code-server might prevent navigation because of unsaved // changes (seems to happen based on timing even if no changes have been // made too). In these cases just accept. page.on("dialog", (d) => d.accept()) - const codeServerPage = new CodeServerPage(codeServer, page, authenticated) + const codeServerPage = new CodeServerPage(codeServer, page) await codeServerPage.navigate() await use(codeServerPage) }, diff --git a/test/e2e/codeServer.test.ts b/test/e2e/codeServer.test.ts index 0dcbbce13..679a7bd7d 100644 --- a/test/e2e/codeServer.test.ts +++ b/test/e2e/codeServer.test.ts @@ -3,10 +3,11 @@ import { promises as fs } from "fs" import * as os from "os" import * as path from "path" import * as util from "util" +import { getMaybeProxiedCodeServer } from "../utils/helpers" import { describe, test, expect } from "./baseFixture" import { CodeServer } from "./models/CodeServer" -describe("code-server", true, [], {}, () => { +describe("code-server", [], {}, () => { // TODO@asher: Generalize this? Could be nice if we were to ever need // multiple migration tests in other suites. const instances = new Map() @@ -48,7 +49,8 @@ describe("code-server", true, [], {}, () => { const url = codeServerPage.page.url() // We use match because there may be a / at the end // so we don't want it to fail if we expect http://localhost:8080 to match http://localhost:8080/ - expect(url).toMatch(await codeServerPage.address()) + const address = await getMaybeProxiedCodeServer(codeServerPage) + expect(url).toMatch(address) }) test("should always see the code-server editor", async ({ codeServerPage }) => { @@ -70,7 +72,9 @@ describe("code-server", true, [], {}, () => { test("should migrate state to avoid collisions", async ({ codeServerPage }) => { // This can take a very long time in development because of how long pages // take to load and we are doing a lot of that here. - test.slow() + if (process.env.VSCODE_DEV === "1") { + test.slow() + } const dir = await codeServerPage.workspaceDir const files = [path.join(dir, "foo"), path.join(dir, "bar")] @@ -90,6 +94,7 @@ describe("code-server", true, [], {}, () => { // domain and can write to the same database. const cs = await spawn("4.0.2", dir) const address = new URL(await cs.address()) + await codeServerPage.navigate("/proxy/" + address.port + "/") await codeServerPage.openFile(files[1]) expect(await codeServerPage.tabIsVisible(files[0])).toBe(false) diff --git a/test/e2e/downloads.test.ts b/test/e2e/downloads.test.ts index 2079c7eba..b1c9a01fd 100644 --- a/test/e2e/downloads.test.ts +++ b/test/e2e/downloads.test.ts @@ -3,7 +3,7 @@ import * as path from "path" import { clean } from "../utils/helpers" import { describe, test, expect } from "./baseFixture" -describe("Downloads (enabled)", true, [], {}, async () => { +describe("Downloads (enabled)", [], {}, async () => { const testName = "downloads-enabled" test.beforeAll(async () => { await clean(testName) @@ -25,7 +25,7 @@ describe("Downloads (enabled)", true, [], {}, async () => { }) }) -describe("Downloads (disabled)", true, ["--disable-file-downloads"], {}, async () => { +describe("Downloads (disabled)", ["--disable-file-downloads"], {}, async () => { const testName = "downloads-disabled" test.beforeAll(async () => { await clean(testName) diff --git a/test/e2e/extensions.test.ts b/test/e2e/extensions.test.ts index 72734c9f5..70785bca6 100644 --- a/test/e2e/extensions.test.ts +++ b/test/e2e/extensions.test.ts @@ -1,23 +1,36 @@ import * as path from "path" -import { describe, test } from "./baseFixture" +import { test as base } from "@playwright/test" +import { describe, test, expect } from "./baseFixture" +import { getMaybeProxiedCodeServer } from "../utils/helpers" function runTestExtensionTests() { // This will only work if the test extension is loaded into code-server. test("should have access to VSCODE_PROXY_URI", async ({ codeServerPage }) => { - const address = await codeServerPage.address() + const address = await getMaybeProxiedCodeServer(codeServerPage) await codeServerPage.executeCommandViaMenus("code-server: Get proxy URI") - await codeServerPage.page.waitForSelector(`text=${address}/proxy/{{port}}`) + const text = await codeServerPage.page.locator(".notification-list-item-message").textContent() + // Remove end slash in address + const normalizedAddress = address.replace(/\/+$/, "") + expect(text).toBe(`${normalizedAddress}/proxy/{{port}}`) }) } const flags = ["--extensions-dir", path.join(__dirname, "./extensions")] -describe("Extensions", true, flags, {}, () => { +describe("Extensions", flags, {}, () => { runTestExtensionTests() }) -describe("Extensions with --cert", true, [...flags, "--cert"], {}, () => { - runTestExtensionTests() -}) +if (process.env.USE_PROXY !== "1") { + describe("Extensions with --cert", [...flags, "--cert"], {}, () => { + runTestExtensionTests() + }) +} else { + base.describe("Extensions with --cert", () => { + base.skip("skipped because USE_PROXY is set", () => { + // Playwright will not show this without a function. + }) + }) +} diff --git a/test/e2e/github.test.ts b/test/e2e/github.test.ts index f45c9e714..1d056aec9 100644 --- a/test/e2e/github.test.ts +++ b/test/e2e/github.test.ts @@ -2,7 +2,7 @@ import { test as base } from "@playwright/test" import { describe, expect, test } from "./baseFixture" if (process.env.GITHUB_TOKEN) { - describe("GitHub token", true, [], {}, () => { + describe("GitHub token", [], {}, () => { test("should be logged in to pull requests extension", async ({ codeServerPage }) => { await codeServerPage.exec("git init") await codeServerPage.exec("git remote add origin https://github.com/coder/code-server") @@ -16,7 +16,7 @@ if (process.env.GITHUB_TOKEN) { }) }) - describe("No GitHub token", true, [], { GITHUB_TOKEN: "" }, () => { + describe("No GitHub token", [], { GITHUB_TOKEN: "" }, () => { test("should not be logged in to pull requests extension", async ({ codeServerPage }) => { await codeServerPage.exec("git init") await codeServerPage.exec("git remote add origin https://github.com/coder/code-server") diff --git a/test/e2e/globalSetup.test.ts b/test/e2e/globalSetup.test.ts deleted file mode 100644 index ba1cf03e3..000000000 --- a/test/e2e/globalSetup.test.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { describe, test, expect } from "./baseFixture" - -// This test is to make sure the globalSetup works as expected -// meaning globalSetup ran and stored the storageState -describe("globalSetup", true, [], {}, () => { - test("should keep us logged in using the storageState", async ({ codeServerPage }) => { - // Make sure the editor actually loaded - expect(await codeServerPage.isEditorVisible()).toBe(true) - }) -}) diff --git a/test/e2e/login.test.ts b/test/e2e/login.test.ts index 9506e7063..e6956316e 100644 --- a/test/e2e/login.test.ts +++ b/test/e2e/login.test.ts @@ -1,7 +1,7 @@ import { PASSWORD } from "../utils/constants" import { describe, test, expect } from "./baseFixture" -describe("login", false, [], {}, () => { +describe("login", ["--auth", "password"], {}, () => { test("should see the login page", async ({ codeServerPage }) => { // It should send us to the login page expect(await codeServerPage.page.title()).toBe("code-server login") diff --git a/test/e2e/models/CodeServer.ts b/test/e2e/models/CodeServer.ts index 4100a1660..f24500098 100644 --- a/test/e2e/models/CodeServer.ts +++ b/test/e2e/models/CodeServer.ts @@ -4,10 +4,10 @@ import { promises as fs } from "fs" import * as path from "path" import { Page } from "playwright" import * as util from "util" -import { logError, plural } from "../../../src/common/util" +import { logError, normalize, plural } from "../../../src/common/util" import { onLine } from "../../../src/node/util" import { PASSWORD, workspaceDir } from "../../utils/constants" -import { idleTimer, tmpdir } from "../../utils/helpers" +import { getMaybeProxiedCodeServer, idleTimer, tmpdir } from "../../utils/helpers" interface CodeServerProcess { process: cp.ChildProcess @@ -58,6 +58,7 @@ export class CodeServer { this.process = this.spawn() } const { address } = await this.process + return address } @@ -104,6 +105,8 @@ export class CodeServer { this.entry, "--extensions-dir", path.join(dir, "extensions"), + "--auth", + "none", ...this.args, // Using port zero will spawn on a random port. "--bind-addr", @@ -124,6 +127,10 @@ export class CodeServer { env: { ...process.env, ...this.env, + // Set to empty string to prevent code-server from + // using the existing instance when running the e2e tests + // from an integrated terminal. + VSCODE_IPC_HOOK_CLI: "", PASSWORD, }, }) @@ -183,6 +190,13 @@ export class CodeServer { proc.kill() } } + + /** + * Whether or not authentication is enabled. + */ + authEnabled(): boolean { + return this.args.includes("password") + } } /** @@ -195,11 +209,7 @@ export class CodeServer { export class CodeServerPage { private readonly editorSelector = "div.monaco-workbench" - constructor( - private readonly codeServer: CodeServer, - public readonly page: Page, - private readonly authenticated: boolean, - ) { + constructor(private readonly codeServer: CodeServer, public readonly page: Page) { this.page.on("console", (message) => { this.codeServer.logger.debug(message.text()) }) @@ -224,12 +234,16 @@ export class CodeServerPage { * editor to become available. */ async navigate(endpoint = "/") { - const to = new URL(endpoint, await this.codeServer.address()) + const address = await getMaybeProxiedCodeServer(this.codeServer) + const noramlizedUrl = normalize(address + endpoint, true) + const to = new URL(noramlizedUrl) + + this.codeServer.logger.info(`navigating to ${to}`) await this.page.goto(to.toString(), { waitUntil: "networkidle" }) - // Only reload editor if authenticated. Otherwise we'll get stuck + // Only reload editor if auth is not enabled. Otherwise we'll get stuck // reloading the login page. - if (this.authenticated) { + if (!this.codeServer.authEnabled()) { await this.reloadUntilEditorIsReady() } } diff --git a/test/e2e/openHelpAbout.test.ts b/test/e2e/openHelpAbout.test.ts index 74b54c812..fc7b25f96 100644 --- a/test/e2e/openHelpAbout.test.ts +++ b/test/e2e/openHelpAbout.test.ts @@ -1,7 +1,7 @@ import { version } from "../../src/node/constants" import { describe, test, expect } from "./baseFixture" -describe("Open Help > About", true, [], {}, () => { +describe("Open Help > About", [], {}, () => { test("should see code-server version in about dialog", async ({ codeServerPage }) => { // Open using the menu. await codeServerPage.navigateMenus(["Help", "About"]) diff --git a/test/e2e/terminal.test.ts b/test/e2e/terminal.test.ts index 175d51c29..df7ceeebf 100644 --- a/test/e2e/terminal.test.ts +++ b/test/e2e/terminal.test.ts @@ -2,10 +2,10 @@ import * as cp from "child_process" import { promises as fs } from "fs" import * as path from "path" import util from "util" -import { clean, tmpdir } from "../utils/helpers" +import { clean, getMaybeProxiedCodeServer, tmpdir } from "../utils/helpers" import { describe, expect, test } from "./baseFixture" -describe("Integrated Terminal", true, [], {}, () => { +describe("Integrated Terminal", [], {}, () => { const testName = "integrated-terminal" test.beforeAll(async () => { await clean(testName) @@ -26,7 +26,8 @@ describe("Integrated Terminal", true, [], {}, () => { await codeServerPage.page.keyboard.press("Enter") const { stdout } = await output - expect(stdout).toMatch(await codeServerPage.address()) + const address = await getMaybeProxiedCodeServer(codeServerPage) + expect(stdout).toMatch(address) }) test("should be able to invoke `code-server` to open a file", async ({ codeServerPage }) => { diff --git a/test/playwright.config.ts b/test/playwright.config.ts index 599914777..bcaa86540 100644 --- a/test/playwright.config.ts +++ b/test/playwright.config.ts @@ -12,6 +12,8 @@ const config: PlaywrightTestConfig = { testDir: path.join(__dirname, "e2e"), // Search for tests in this directory. timeout: 60000, // Each test is given 60 seconds. retries: process.env.CI ? 2 : 1, // Retry in CI due to flakiness. + // Limit the number of failures on CI to save resources + maxFailures: process.env.CI ? 3 : undefined, globalSetup: require.resolve("./utils/globalE2eSetup.ts"), reporter: "list", // Put any shared options on the top level. diff --git a/test/utils/constants.ts b/test/utils/constants.ts index 9b5c2b6e8..642a19162 100644 --- a/test/utils/constants.ts +++ b/test/utils/constants.ts @@ -1,2 +1,4 @@ export const PASSWORD = "e45432jklfdsab" export const workspaceDir = "workspaces" +export const REVERSE_PROXY_BASE_PATH = process.env.CS_TEST_REVERSE_PROXY_BASE_PATH || "ide" +export const REVERSE_PROXY_PORT = process.env.CS_TEST_REVERSE_PROXY_PORT || "8000" diff --git a/test/utils/globalE2eSetup.ts b/test/utils/globalE2eSetup.ts index 712938300..3596ed33f 100644 --- a/test/utils/globalE2eSetup.ts +++ b/test/utils/globalE2eSetup.ts @@ -1,7 +1,4 @@ -import { Cookie } from "playwright" -import { CookieKeys } from "../../src/common/http" -import { hash } from "../../src/node/util" -import { PASSWORD, workspaceDir } from "./constants" +import { workspaceDir } from "./constants" import { clean } from "./helpers" import * as wtfnode from "./wtfnode" @@ -20,25 +17,5 @@ export default async function () { wtfnode.setup() } - // TODO: Replace this with a call to code-server to get the cookie. To avoid - // too much overhead we can do an http POST request and avoid spawning a - // browser for it. - const cookies: Cookie[] = [ - { - domain: "localhost", - expires: -1, - httpOnly: false, - name: CookieKeys.Session, - path: "/", - sameSite: "Lax", - secure: false, - value: await hash(PASSWORD), - }, - ] - - // Save storage state and store as an env variable - // More info: https://playwright.dev/docs/auth/#reuse-authentication-state - process.env.STORAGE = JSON.stringify({ cookies }) - console.log("✅ Global Setup for Playwright End-to-End Tests is now complete.") } diff --git a/test/utils/helpers.ts b/test/utils/helpers.ts index 3f9399d4b..889ea48ce 100644 --- a/test/utils/helpers.ts +++ b/test/utils/helpers.ts @@ -3,6 +3,8 @@ import { promises as fs } from "fs" import * as net from "net" import * as os from "os" import * as path from "path" +import { CodeServer, CodeServerPage } from "../e2e/models/CodeServer" +import { REVERSE_PROXY_PORT, REVERSE_PROXY_BASE_PATH } from "./constants" /** * Spy on the logger and console and replace with mock implementations to @@ -119,3 +121,18 @@ export function isAddressInfo(address: unknown): address is net.AddressInfo { (address as net.AddressInfo).address !== undefined ) } + +/** + * If using a proxy, return the address of the proxy. + * + * Otherwise, return the direct address of code-server. + */ +export async function getMaybeProxiedCodeServer(codeServer: CodeServerPage | CodeServer): Promise { + const address = await codeServer.address() + if (process.env.USE_PROXY === "1") { + const uri = new URL(address) + return `http://${uri.hostname}:${REVERSE_PROXY_PORT}/${uri.port}/${REVERSE_PROXY_BASE_PATH}/` + } + + return address +}