Skip to content

Add VSCODE_PROXY_URI env var to extension host and terminal #3621

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ rules:
import/order:
[error, { alphabetize: { order: "asc" }, groups: [["builtin", "external", "internal"], "parent", "sibling"] }]
no-async-promise-executor: off
# This isn't a real module, just types, which apparently doesn't resolve.
import/no-unresolved: [error, { ignore: ["express-serve-static-core"] }]
# These aren't real modules, just types, which apparently don't resolve.
import/no-unresolved: [error, { ignore: ["express-serve-static-core", "vscode"] }]

settings:
# Does not work with CommonJS unfortunately.
Expand Down
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ VS Code v0.00.0

### New Features

- item
- Add `VSCODE_PROXY_URI` env var for use in the terminal and extensions (#1510)

### Bug Fixes

Expand Down
17 changes: 12 additions & 5 deletions ci/dev/postinstall.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,20 @@ set -euo pipefail

main() {
cd "$(dirname "$0")/../.."
source ./ci/lib.sh

echo "Installing code-server test dependencies..."
pushd test
echo "Installing dependencies for $PWD"
yarn install
popd

cd test
pushd test/e2e/extensions/test-extension
echo "Installing dependencies for $PWD"
yarn install
cd ..
popd

cd vendor
echo "Installing vendor dependencies..."
pushd vendor
echo "Installing dependencies for $PWD"

# * We install in 'modules' instead of 'node_modules' because VS Code's extensions
# use a webpack config which cannot differentiate between its own node_modules
Expand All @@ -32,6 +37,8 @@ main() {

# Finally, run the vendor `postinstall`
yarn run postinstall

popd
}

main "$@"
5 changes: 5 additions & 0 deletions ci/dev/test-e2e.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ main() {

source ./ci/lib.sh

pushd test/e2e/extensions/test-extension
echo "Building test extension"
yarn build
popd

local dir="$PWD"
if [[ ! ${CODE_SERVER_TEST_ENTRY-} ]]; then
echo "Set CODE_SERVER_TEST_ENTRY to test another build of code-server"
Expand Down
8 changes: 6 additions & 2 deletions ci/dev/test-unit.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@ set -euo pipefail

main() {
cd "$(dirname "$0")/../.."
cd test/unit/node/test-plugin
source ./ci/lib.sh

echo "Building test plugin"
pushd test/unit/node/test-plugin
make -s out/index.js
popd

# We must keep jest in a sub-directory. See ../../test/package.json for more
# information. We must also run it from the root otherwise coverage will not
# include our source files.
cd "$OLDPWD"
CS_DISABLE_PLUGINS=true ./test/node_modules/.bin/jest "$@"
}

Expand Down
47 changes: 28 additions & 19 deletions src/common/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,33 +66,42 @@ export const resolveBase = (base?: string): string => {
return normalize(url.pathname)
}

let options: Options

/**
* Get options embedded in the HTML or query params.
*/
export const getOptions = <T extends Options>(): T => {
let options: T
try {
options = JSON.parse(document.getElementById("coder-options")!.getAttribute("data-settings")!)
} catch (error) {
options = {} as T
}
if (!options) {
try {
options = JSON.parse(document.getElementById("coder-options")!.getAttribute("data-settings")!)
} catch (error) {
console.error(error)
options = {} as T
}

// You can also pass options in stringified form to the options query
// variable. Options provided here will override the ones in the options
// element.
const params = new URLSearchParams(location.search)
const queryOpts = params.get("options")
if (queryOpts) {
options = {
...options,
...JSON.parse(queryOpts),
// You can also pass options in stringified form to the options query
// variable. Options provided here will override the ones in the options
// element.
const params = new URLSearchParams(location.search)
const queryOpts = params.get("options")
if (queryOpts) {
try {
options = {
...options,
...JSON.parse(queryOpts),
}
} catch (error) {
// Don't fail if the query parameters are malformed.
console.error(error)
}
}
}

options.base = resolveBase(options.base)
options.csStaticBase = resolveBase(options.csStaticBase)
options.base = resolveBase(options.base)
options.csStaticBase = resolveBase(options.csStaticBase)
}

return options
return options as T
}

/**
Expand Down
12 changes: 12 additions & 0 deletions test/e2e/extensions.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { describe, test } from "./baseFixture"

describe("Extensions", true, () => {
// 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()

await codeServerPage.executeCommandViaMenus("code-server: Get proxy URI")

await codeServerPage.page.waitForSelector(`text=${address}/proxy/{port}`)
})
})
1 change: 1 addition & 0 deletions test/e2e/extensions/test-extension/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/extension.js
13 changes: 13 additions & 0 deletions test/e2e/extensions/test-extension/extension.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import * as vscode from "vscode"

export function activate(context: vscode.ExtensionContext) {
context.subscriptions.push(
vscode.commands.registerCommand("codeServerTest.proxyUri", () => {
if (process.env.VSCODE_PROXY_URI) {
vscode.window.showInformationMessage(process.env.VSCODE_PROXY_URI)
} else {
vscode.window.showErrorMessage("No proxy URI was set")
}
}),
)
}
29 changes: 29 additions & 0 deletions test/e2e/extensions/test-extension/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"name": "code-server-extension",
"description": "code-server test extension",
"version": "0.0.1",
"publisher": "cdr",
"activationEvents": [
"onCommand:codeServerTest.proxyUri"
],
"engines": {
"vscode": "^1.56.0"
},
"main": "./extension.js",
"contributes": {
"commands": [
{
"command": "codeServerTest.proxyUri",
"title": "Get proxy URI",
"category": "code-server"
}
]
},
"devDependencies": {
"@types/vscode": "^1.56.0",
"typescript": "^4.0.5"
},
"scripts": {
"build": "tsc extension.ts"
}
}
10 changes: 10 additions & 0 deletions test/e2e/extensions/test-extension/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"compilerOptions": {
"target": "es2020",
"module": "commonjs",
"outDir": ".",
"strict": true,
"baseUrl": "./"
},
"include": ["./extension.ts"]
}
13 changes: 13 additions & 0 deletions test/e2e/extensions/test-extension/yarn.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


"@types/vscode@^1.56.0":
version "1.57.0"
resolved "https://registry.yarnpkg.com/@types/vscode/-/vscode-1.57.0.tgz#cc648e0573b92f725cd1baf2621f8da9f8bc689f"
integrity sha512-FeznBFtIDCWRluojTsi9c3LLcCHOXP5etQfBK42+ixo1CoEAchkw39tuui9zomjZuKfUVL33KZUDIwHZ/xvOkQ==

typescript@^4.0.5:
version "4.3.2"
resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.3.2.tgz#399ab18aac45802d6f2498de5054fcbbe716a805"
integrity sha512-zZ4hShnmnoVnAHpVHWpTcxdv7dWP60S2FsydQLV8V5PbS3FifjWFFRiHSWpDJahly88PRyV5teTSLoq4eG7mKw==
2 changes: 2 additions & 0 deletions test/e2e/models/CodeServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ export class CodeServer {
path.join(dir, "config.yaml"),
"--user-data-dir",
dir,
"--extensions-dir",
path.join(__dirname, "../extensions"),
// The last argument is the workspace to open.
dir,
],
Expand Down
2 changes: 0 additions & 2 deletions test/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,10 @@ const config: PlaywrightTestConfig = {
name: "Chromium",
use: { browserName: "chromium" },
},

{
name: "Firefox",
use: { browserName: "firefox" },
},

{
name: "WebKit",
use: { browserName: "webkit" },
Expand Down
68 changes: 48 additions & 20 deletions test/unit/common/util.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { JSDOM } from "jsdom"
import * as util from "../../../src/common/util"
import { createLoggerMock } from "../../utils/helpers"
import * as helpers from "../../utils/helpers"

const dom = new JSDOM()
global.document = dom.window.document
Expand Down Expand Up @@ -111,6 +111,8 @@ describe("util", () => {
})

describe("getOptions", () => {
let getOptions: typeof import("../../../src/common/util").getOptions

beforeEach(() => {
const location: LocationLike = {
pathname: "/healthz",
Expand All @@ -124,54 +126,80 @@ describe("util", () => {
// and tell TS that our location should be looked at
// as Location (even though it's missing some properties)
global.location = location as Location

// Reset and re-import since the options are cached.
jest.resetModules()
getOptions = require("../../../src/common/util").getOptions

helpers.spyOnConsole()
})

afterEach(() => {
jest.restoreAllMocks()
})

it("should return options with base and cssStaticBase even if it doesn't exist", () => {
expect(util.getOptions()).toStrictEqual({
expect(getOptions()).toStrictEqual({
base: "",
csStaticBase: "",
})
expect(console.error).toBeCalledTimes(1)
})

it("should return options when they do exist", () => {
const expected = {
base: ".",
csStaticBase: "./static/development/Users/jp/Dev/code-server",
logLevel: 2,
disableTelemetry: false,
disableUpdateCheck: false,
}

// Mock getElementById
const spy = jest.spyOn(document, "getElementById")
// Create a fake element and set the attribute
// Create a fake element and set the attribute. Options are expected to be
// stringified JSON.
const mockElement = document.createElement("div")
mockElement.setAttribute(
"data-settings",
'{"base":".","csStaticBase":"./static/development/Users/jp/Dev/code-server","logLevel":2,"disableUpdateCheck":false}',
)
mockElement.setAttribute("data-settings", JSON.stringify(expected))
// Return mockElement from the spy
// this way, when we call "getElementById"
// it returns the element
spy.mockImplementation(() => mockElement)

expect(util.getOptions()).toStrictEqual({
expect(getOptions()).toStrictEqual({
...expected,
// The two bases should get resolved. The rest should be unchanged.
base: "",
csStaticBase: "/static/development/Users/jp/Dev/code-server",
disableUpdateCheck: false,
})
expect(console.error).toBeCalledTimes(0)
})

it("should merge options in the query", () => {
// Options provided in the query will override any options provided in the
// HTML. Options are expected to be stringified JSON (same as the
// element).
const expected = {
logLevel: 2,
}
location.search = `?options=${JSON.stringify(expected)}`
expect(getOptions()).toStrictEqual({
...expected,
base: "",
csStaticBase: "",
})
// Once for the element.
expect(console.error).toBeCalledTimes(1)
})

it("should include queryOpts", () => {
// Trying to understand how the implementation works
// 1. It grabs the search params from location.search (i.e. ?)
// 2. it then grabs the "options" param if it exists
// 3. then it creates a new options object
// spreads the original options
// then parses the queryOpts
location.search = '?options={"logLevel":2}'
expect(util.getOptions()).toStrictEqual({
it("should skip bad query options", () => {
location.search = "?options=invalidJson"
expect(getOptions()).toStrictEqual({
base: "",
csStaticBase: "",
logLevel: 2,
})
// Once for the element, once for the query.
expect(console.error).toBeCalledTimes(2)
})
})

Expand Down Expand Up @@ -217,7 +245,7 @@ describe("util", () => {
jest.restoreAllMocks()
})

const loggerModule = createLoggerMock()
const loggerModule = helpers.createLoggerMock()

it("should log an error with the message and stack trace", () => {
const message = "You don't have access to that folder."
Expand Down
Loading