Skip to content

Allow user-data-dir and extension-dir in config.yaml #1679

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

Merged
merged 1 commit into from
May 19, 2020
Merged
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
16 changes: 13 additions & 3 deletions src/node/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,12 @@ export const optionDescriptions = (): string[] => {
)
}

export const parse = async (
export const parse = (
argv: string[],
opts?: {
configFile: string
},
): Promise<Args> => {
): Args => {
const error = (msg: string): Error => {
if (opts?.configFile) {
msg = `error reading ${opts.configFile}: ${msg}`
Expand Down Expand Up @@ -288,18 +288,28 @@ export const parse = async (
break
case LogLevel.Debug:
logger.level = Level.Debug
args.verbose = false
break
case LogLevel.Info:
logger.level = Level.Info
args.verbose = false
break
case LogLevel.Warn:
logger.level = Level.Warning
args.verbose = false
break
case LogLevel.Error:
logger.level = Level.Error
args.verbose = false
break
}

return args
}

export async function setDefaults(args: Args): Promise<Args> {
args = { ...args }

if (!args["user-data-dir"]) {
await copyOldMacOSDataDir()
args["user-data-dir"] = paths.data
Expand Down Expand Up @@ -353,7 +363,7 @@ export async function readConfigFile(configPath?: string): Promise<Args> {
}
return `--${optName}=${opt}`
})
const args = await parse(configFileArgv, {
const args = parse(configFileArgv, {
configFile: configPath,
})
return {
Expand Down
8 changes: 5 additions & 3 deletions src/node/entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { ProxyHttpProvider } from "./app/proxy"
import { StaticHttpProvider } from "./app/static"
import { UpdateHttpProvider } from "./app/update"
import { VscodeHttpProvider } from "./app/vscode"
import { Args, bindAddrFromAllSources, optionDescriptions, parse, readConfigFile } from "./cli"
import { Args, bindAddrFromAllSources, optionDescriptions, parse, readConfigFile, setDefaults } from "./cli"
import { AuthType, HttpServer, HttpServerOptions } from "./http"
import { generateCertificate, hash, open, humanPath } from "./util"
import { ipcMain, wrap } from "./wrapper"
Expand Down Expand Up @@ -43,6 +43,8 @@ const main = async (cliArgs: Args): Promise<void> => {
}
}

args = await setDefaults(args)

logger.info(`Using user-data-dir ${humanPath(args["user-data-dir"])}`)

logger.trace(`Using extensions-dir ${humanPath(args["extensions-dir"])}`)
Expand Down Expand Up @@ -127,9 +129,9 @@ const main = async (cliArgs: Args): Promise<void> => {
}

async function entry(): Promise<void> {
const tryParse = async (): Promise<Args> => {
const tryParse = (): Args => {
try {
return await parse(process.argv.slice(2))
return parse(process.argv.slice(2))
} catch (error) {
console.error(error.message)
process.exit(1)
Expand Down
86 changes: 40 additions & 46 deletions test/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { logger, Level } from "@coder/logger"
import * as assert from "assert"
import * as path from "path"
import { parse } from "../src/node/cli"
import { paths } from "../src/node/util"

describe("cli", () => {
beforeEach(() => {
Expand All @@ -12,17 +11,15 @@ describe("cli", () => {
// The parser will always fill these out.
const defaults = {
_: [],
"extensions-dir": path.join(paths.data, "extensions"),
"user-data-dir": paths.data,
}

it("should set defaults", async () => {
assert.deepEqual(await await parse([]), defaults)
it("should set defaults", () => {
assert.deepEqual(parse([]), defaults)
})

it("should parse all available options", async () => {
it("should parse all available options", () => {
assert.deepEqual(
await await parse([
parse([
"--bind-addr=192.169.0.1:8080",
"--auth",
"none",
Expand Down Expand Up @@ -84,8 +81,8 @@ describe("cli", () => {
)
})

it("should work with short options", async () => {
assert.deepEqual(await parse(["-vvv", "-v"]), {
it("should work with short options", () => {
assert.deepEqual(parse(["-vvv", "-v"]), {
...defaults,
log: "trace",
verbose: true,
Expand All @@ -95,17 +92,18 @@ describe("cli", () => {
assert.equal(logger.level, Level.Trace)
})

it("should use log level env var", async () => {
it("should use log level env var", () => {
process.env.LOG_LEVEL = "debug"
assert.deepEqual(await parse([]), {
assert.deepEqual(parse([]), {
...defaults,
log: "debug",
verbose: false,
})
assert.equal(process.env.LOG_LEVEL, "debug")
assert.equal(logger.level, Level.Debug)

process.env.LOG_LEVEL = "trace"
assert.deepEqual(await parse([]), {
assert.deepEqual(parse([]), {
...defaults,
log: "trace",
verbose: true,
Expand All @@ -116,23 +114,25 @@ describe("cli", () => {

it("should prefer --log to env var and --verbose to --log", async () => {
process.env.LOG_LEVEL = "debug"
assert.deepEqual(await parse(["--log", "info"]), {
assert.deepEqual(parse(["--log", "info"]), {
...defaults,
log: "info",
verbose: false,
})
assert.equal(process.env.LOG_LEVEL, "info")
assert.equal(logger.level, Level.Info)

process.env.LOG_LEVEL = "trace"
assert.deepEqual(await parse(["--log", "info"]), {
assert.deepEqual(parse(["--log", "info"]), {
...defaults,
log: "info",
verbose: false,
})
assert.equal(process.env.LOG_LEVEL, "info")
assert.equal(logger.level, Level.Info)

process.env.LOG_LEVEL = "warn"
assert.deepEqual(await parse(["--log", "info", "--verbose"]), {
assert.deepEqual(parse(["--log", "info", "--verbose"]), {
...defaults,
log: "trace",
verbose: true,
Expand All @@ -141,68 +141,62 @@ describe("cli", () => {
assert.equal(logger.level, Level.Trace)
})

it("should ignore invalid log level env var", async () => {
it("should ignore invalid log level env var", () => {
process.env.LOG_LEVEL = "bogus"
assert.deepEqual(await parse([]), defaults)
assert.deepEqual(parse([]), defaults)
})

it("should error if value isn't provided", async () => {
await assert.rejects(async () => await parse(["--auth"]), /--auth requires a value/)
await assert.rejects(async () => await parse(["--auth=", "--log=debug"]), /--auth requires a value/)
await assert.rejects(async () => await parse(["--auth", "--log"]), /--auth requires a value/)
await assert.rejects(async () => await parse(["--auth", "--invalid"]), /--auth requires a value/)
await assert.rejects(async () => await parse(["--bind-addr"]), /--bind-addr requires a value/)
it("should error if value isn't provided", () => {
assert.throws(() => parse(["--auth"]), /--auth requires a value/)
assert.throws(() => parse(["--auth=", "--log=debug"]), /--auth requires a value/)
assert.throws(() => parse(["--auth", "--log"]), /--auth requires a value/)
assert.throws(() => parse(["--auth", "--invalid"]), /--auth requires a value/)
assert.throws(() => parse(["--bind-addr"]), /--bind-addr requires a value/)
})

it("should error if value is invalid", async () => {
await assert.rejects(async () => await parse(["--port", "foo"]), /--port must be a number/)
await assert.rejects(async () => await parse(["--auth", "invalid"]), /--auth valid values: \[password, none\]/)
await assert.rejects(
async () => await parse(["--log", "invalid"]),
/--log valid values: \[trace, debug, info, warn, error\]/,
)
it("should error if value is invalid", () => {
assert.throws(() => parse(["--port", "foo"]), /--port must be a number/)
assert.throws(() => parse(["--auth", "invalid"]), /--auth valid values: \[password, none\]/)
assert.throws(() => parse(["--log", "invalid"]), /--log valid values: \[trace, debug, info, warn, error\]/)
})

it("should error if the option doesn't exist", async () => {
await assert.rejects(async () => await parse(["--foo"]), /Unknown option --foo/)
it("should error if the option doesn't exist", () => {
assert.throws(() => parse(["--foo"]), /Unknown option --foo/)
})

it("should not error if the value is optional", async () => {
assert.deepEqual(await parse(["--cert"]), {
it("should not error if the value is optional", () => {
assert.deepEqual(parse(["--cert"]), {
...defaults,
cert: {
value: undefined,
},
})
})

it("should not allow option-like values", async () => {
await assert.rejects(async () => await parse(["--socket", "--socket-path-value"]), /--socket requires a value/)
it("should not allow option-like values", () => {
assert.throws(() => parse(["--socket", "--socket-path-value"]), /--socket requires a value/)
// If you actually had a path like this you would do this instead:
assert.deepEqual(await parse(["--socket", "./--socket-path-value"]), {
assert.deepEqual(parse(["--socket", "./--socket-path-value"]), {
...defaults,
socket: path.resolve("--socket-path-value"),
})
await assert.rejects(
async () => await parse(["--cert", "--socket-path-value"]),
/Unknown option --socket-path-value/,
)
assert.throws(() => parse(["--cert", "--socket-path-value"]), /Unknown option --socket-path-value/)
})

it("should allow positional arguments before options", async () => {
assert.deepEqual(await parse(["foo", "test", "--auth", "none"]), {
it("should allow positional arguments before options", () => {
assert.deepEqual(parse(["foo", "test", "--auth", "none"]), {
...defaults,
_: ["foo", "test"],
auth: "none",
})
})

it("should support repeatable flags", async () => {
assert.deepEqual(await parse(["--proxy-domain", "*.coder.com"]), {
it("should support repeatable flags", () => {
assert.deepEqual(parse(["--proxy-domain", "*.coder.com"]), {
...defaults,
"proxy-domain": ["*.coder.com"],
})
assert.deepEqual(await parse(["--proxy-domain", "*.coder.com", "--proxy-domain", "test.com"]), {
assert.deepEqual(parse(["--proxy-domain", "*.coder.com", "--proxy-domain", "test.com"]), {
...defaults,
"proxy-domain": ["*.coder.com", "test.com"],
})
Expand Down