feat(e2e): add support running behind proxy (#5348)

* docs: update maintaining * chore(e2e): add maxFailures to playwright * fix(ci): skip submodule in e2e job We don't need the submodules for the e2e job. This will speed up the checkout step. * feat(ci): add test-e2e-proxy job This adds a new job to CI to run our tests behind Caddy and simulate code-server running against a reverse-proxy. * refactor: make e2e work with reverse proxy This refactors the e2e test in a couple ways: - remove setting cookie in localStorage (instead we pass --auth none) - refactor address() method to account for reverse proxy logic * Update test/e2e/models/CodeServer.ts * Update test/playwright.config.ts * Update test/utils/constants.ts Co-authored-by: Asher <ash@coder.com> * Update test/utils/helpers.ts Co-authored-by: Asher <ash@coder.com> Co-authored-by: Asher <ash@coder.com>
2022-08-09 13:24:37 -05:00 · 2022-08-09 13:24:37 -05:00 · f178f0400b
commit f178f0400b
parent efb5baec83
18 changed files with 192 additions and 85 deletions
--- 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
--- a/ci/Caddyfile
+++ 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
+        }
+
+}
--- 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.
--- 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",
--- 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<TestFixtures>({
-  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)
  },
--- 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<string, CodeServer>()
@ -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.
+    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)
--- 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)
--- 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"], {}, () => {
+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.
+    })
+  })
+}
--- 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")
--- a/test/e2e/globalSetup.test.ts
+++ b/test/e2e/globalSetup.test.ts
@ -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)
-  })
-})
--- 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")
--- 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()
    }
  }
--- 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"])
--- 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 }) => {
--- 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.
--- 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"
--- 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.")
 }
--- 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<string> {
+  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
+}