From 6e40f18cc1563b126ffe3b60c61945712a533206 Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Mon, 22 Jul 2019 17:55:32 +0200 Subject: [PATCH 1/2] fix(vcs): fixed support for GitHub SSH URLs and added tests --- garden-service/package-lock.json | 55 ++++++++------ garden-service/package.json | 2 + garden-service/src/config/common.ts | 70 +++++++++++++----- garden-service/src/vcs/git.ts | 7 +- garden-service/test/unit/src/config/common.ts | 74 ++++++++++++++++++- .../test/unit/src/config/project.ts | 6 +- 6 files changed, 169 insertions(+), 45 deletions(-) diff --git a/garden-service/package-lock.json b/garden-service/package-lock.json index a2761462b8..30af662be3 100644 --- a/garden-service/package-lock.json +++ b/garden-service/package-lock.json @@ -1,6 +1,6 @@ { "name": "garden-service", - "version": "0.10.0", + "version": "0.10.1", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -163,7 +163,7 @@ "dependencies": { "chalk": { "version": "2.3.1", - "resolved": "http://registry.npmjs.org/chalk/-/chalk-2.3.1.tgz", + "resolved": "https://registry.npmjs.org/chalk/-/chalk-2.3.1.tgz", "integrity": "sha512-QUU4ofkDoMIVO7hcx1iPTISs88wsO8jA92RQIm4JAwZvFGGAV2hSAA1NX7oVj2Ej2Q6NDTcRDjPTFrMCRZoJ6g==", "dev": true, "requires": { @@ -690,6 +690,12 @@ "rxjs": ">=6.4.0" } }, + "@types/is-git-url": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/@types/is-git-url/-/is-git-url-1.0.0.tgz", + "integrity": "sha512-Hozx1nI6wJi3ZcRyIcJq7eg/tA9WHu0jEXBSYijogCAhGY3qer/yC3x5buTSpiWEA51wfd1NjYsGRhmMSMuTag==", + "dev": true + }, "@types/js-yaml": { "version": "3.12.1", "resolved": "https://registry.npmjs.org/@types/js-yaml/-/js-yaml-3.12.1.tgz", @@ -2655,7 +2661,7 @@ }, "minimist": { "version": "1.2.0", - "resolved": "http://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=", "dev": true } @@ -3802,7 +3808,7 @@ }, "es6-promisify": { "version": "5.0.0", - "resolved": "http://registry.npmjs.org/es6-promisify/-/es6-promisify-5.0.0.tgz", + "resolved": "https://registry.npmjs.org/es6-promisify/-/es6-promisify-5.0.0.tgz", "integrity": "sha1-UQnWLz5W6pZ8S2NQWu8IKRyKUgM=", "dev": true, "requires": { @@ -4504,7 +4510,7 @@ }, "readable-stream": { "version": "1.1.14", - "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-1.1.14.tgz", + "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-1.1.14.tgz", "integrity": "sha1-fPTFTvZI44EwhMY23SB54WbAgdk=", "dev": true, "requires": { @@ -4647,7 +4653,7 @@ }, "minimist": { "version": "1.2.0", - "resolved": "http://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=", "dev": true } @@ -5536,7 +5542,7 @@ }, "got": { "version": "6.7.1", - "resolved": "http://registry.npmjs.org/got/-/got-6.7.1.tgz", + "resolved": "https://registry.npmjs.org/got/-/got-6.7.1.tgz", "integrity": "sha1-JAzQV4WpoY5WHcG0S0HHY+8ejbA=", "dev": true, "requires": { @@ -6228,7 +6234,7 @@ }, "http-errors": { "version": "1.6.3", - "resolved": "http://registry.npmjs.org/http-errors/-/http-errors-1.6.3.tgz", + "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-1.6.3.tgz", "integrity": "sha1-i1VoC7S+KDoLW/TqLjhYC+HZMg0=", "requires": { "depd": "~1.1.2", @@ -6590,6 +6596,11 @@ "resolved": "https://registry.npmjs.org/is-generator-function/-/is-generator-function-1.0.7.tgz", "integrity": "sha512-YZc5EwyO4f2kWCax7oegfuSr9mFz1ZvieNYBEjmukLxgXfBUbxAWGVF7GZf0zidYtoBl3WvC07YK0wT76a+Rtw==" }, + "is-git-url": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/is-git-url/-/is-git-url-1.0.0.tgz", + "integrity": "sha1-U/aEzRQyhbUsMkS05vKCU1J69ms=" + }, "is-glob": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/is-glob/-/is-glob-4.0.1.tgz", @@ -6627,7 +6638,7 @@ }, "is-obj": { "version": "1.0.1", - "resolved": "http://registry.npmjs.org/is-obj/-/is-obj-1.0.1.tgz", + "resolved": "https://registry.npmjs.org/is-obj/-/is-obj-1.0.1.tgz", "integrity": "sha1-PkcprB9f3gJc19g6iW2rn09n2w8=", "dev": true }, @@ -6825,7 +6836,7 @@ }, "async": { "version": "1.5.2", - "resolved": "http://registry.npmjs.org/async/-/async-1.5.2.tgz", + "resolved": "https://registry.npmjs.org/async/-/async-1.5.2.tgz", "integrity": "sha1-7GphrlZIDAw8skHJVhjiCJL5Zyo=" }, "escodegen": { @@ -8545,7 +8556,7 @@ }, "async": { "version": "1.5.2", - "resolved": "http://registry.npmjs.org/async/-/async-1.5.2.tgz", + "resolved": "https://registry.npmjs.org/async/-/async-1.5.2.tgz", "integrity": "sha1-7GphrlZIDAw8skHJVhjiCJL5Zyo=", "dev": true }, @@ -8592,7 +8603,7 @@ }, "os-locale": { "version": "1.4.0", - "resolved": "http://registry.npmjs.org/os-locale/-/os-locale-1.4.0.tgz", + "resolved": "https://registry.npmjs.org/os-locale/-/os-locale-1.4.0.tgz", "integrity": "sha1-IPnxeuKe00XoveWDsT0gCYA8FNk=", "dev": true, "requires": { @@ -8612,7 +8623,7 @@ }, "strip-ansi": { "version": "3.0.1", - "resolved": "http://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", + "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", "dev": true, "requires": { @@ -8621,7 +8632,7 @@ }, "wrap-ansi": { "version": "2.1.0", - "resolved": "http://registry.npmjs.org/wrap-ansi/-/wrap-ansi-2.1.0.tgz", + "resolved": "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-2.1.0.tgz", "integrity": "sha1-2Pw9KE3QV5T+hJc8rs3Rz4JP3YU=", "dev": true, "requires": { @@ -8637,7 +8648,7 @@ }, "yargs": { "version": "3.32.0", - "resolved": "http://registry.npmjs.org/yargs/-/yargs-3.32.0.tgz", + "resolved": "https://registry.npmjs.org/yargs/-/yargs-3.32.0.tgz", "integrity": "sha1-AwiOnr+edWtpdRYR0qXvWRSCyZU=", "dev": true, "requires": { @@ -9877,7 +9888,7 @@ "dependencies": { "minimist": { "version": "1.2.0", - "resolved": "http://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=" } } @@ -10082,7 +10093,7 @@ }, "kind-of": { "version": "1.1.0", - "resolved": "http://registry.npmjs.org/kind-of/-/kind-of-1.1.0.tgz", + "resolved": "https://registry.npmjs.org/kind-of/-/kind-of-1.1.0.tgz", "integrity": "sha1-FAo9LUGjbS78+pN3tiwk+ElaXEQ=" }, "plugin-error": { @@ -10104,7 +10115,7 @@ }, "readable-stream": { "version": "2.0.6", - "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.0.6.tgz", + "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.0.6.tgz", "integrity": "sha1-j5A0HmilPMySh4jaz80Rs265t44=", "requires": { "core-util-is": "~1.0.0", @@ -10122,12 +10133,12 @@ }, "string_decoder": { "version": "0.10.31", - "resolved": "http://registry.npmjs.org/string_decoder/-/string_decoder-0.10.31.tgz", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-0.10.31.tgz", "integrity": "sha1-YuIDvEF2bGwoyfyEMB2rHFMQ+pQ=" }, "through2": { "version": "2.0.1", - "resolved": "http://registry.npmjs.org/through2/-/through2-2.0.1.tgz", + "resolved": "https://registry.npmjs.org/through2/-/through2-2.0.1.tgz", "integrity": "sha1-OE51MU1J8y3hLuu4E2uOtrXVnak=", "requires": { "readable-stream": "~2.0.0", @@ -10515,7 +10526,7 @@ "dependencies": { "kind-of": { "version": "2.0.1", - "resolved": "http://registry.npmjs.org/kind-of/-/kind-of-2.0.1.tgz", + "resolved": "https://registry.npmjs.org/kind-of/-/kind-of-2.0.1.tgz", "integrity": "sha1-AY7HpM5+OobLkUG+UZ0kyPqpgbU=", "dev": true, "requires": { @@ -12633,7 +12644,7 @@ }, "xmlbuilder": { "version": "9.0.7", - "resolved": "http://registry.npmjs.org/xmlbuilder/-/xmlbuilder-9.0.7.tgz", + "resolved": "https://registry.npmjs.org/xmlbuilder/-/xmlbuilder-9.0.7.tgz", "integrity": "sha1-Ey7mPS7FVlxVfiD0wi35rKaGsQ0=", "dev": true }, diff --git a/garden-service/package.json b/garden-service/package.json index dc957ab2e3..ecdd0ea05e 100644 --- a/garden-service/package.json +++ b/garden-service/package.json @@ -62,6 +62,7 @@ "ignore": "^5.1.1", "indent-string": "^4.0.0", "inquirer": "^6.3.1", + "is-git-url": "^1.0.0", "js-yaml": "^3.13.1", "json-diff": "^0.5.4", "json-merge-patch": "^0.2.3", @@ -127,6 +128,7 @@ "@types/hapi__joi": "^15.0.2", "@types/has-ansi": "^3.0.0", "@types/inquirer": "6.0.1", + "@types/is-git-url": "^1.0.0", "@types/js-yaml": "^3.12.1", "@types/json-merge-patch": "0.0.4", "@types/json-stringify-safe": "^5.0.0", diff --git a/garden-service/src/config/common.ts b/garden-service/src/config/common.ts index 903b425e45..ce9bd6e8a3 100644 --- a/garden-service/src/config/common.ts +++ b/garden-service/src/config/common.ts @@ -12,6 +12,8 @@ import * as uuid from "uuid" import { ConfigurationError, LocalConfigError } from "../exceptions" import chalk from "chalk" import { relative } from "path" +import { splitLast } from "../util/util" +import isGitUrl = require("is-git-url") export type Primitive = string | number | boolean | null @@ -26,6 +28,10 @@ export const enumToArray = Enum => ( Object.values(Enum).filter(k => typeof k === "string") as string[] ) +interface JoiGitUrlParams { + requireHash?: boolean +} + interface JoiPathParams { absoluteOnly?: boolean relativeOnly?: boolean @@ -34,6 +40,7 @@ interface JoiPathParams { // Extend the Joi module with our custom rules export interface CustomStringSchema extends Joi.StringSchema { + gitUrl: (params: JoiGitUrlParams) => CustomStringSchema posixPath: (params?: JoiPathParams) => CustomStringSchema } @@ -45,12 +52,48 @@ export const joi: Joi.Root = Joi.extend({ base: Joi.string(), name: "string", language: { + gitUrl: "must be a valid Git repository URL", + requireHash: "must specify a branch/tag hash", posixPath: "must be a POSIX-style path", // Used below as 'string.posixPath' absoluteOnly: "must be a an absolute path", relativeOnly: "must be a relative path (may not be an absolute path)", subPathOnly: "must be a relative sub-path (may not contain '..' or be an absolute path)", }, rules: [ + { + name: "gitUrl", + params: { + options: Joi.object() + .keys({ + requireHash: Joi.boolean() + .description("Only allow Git URLs with a branch/tag hash."), + }), + }, + validate(params: { options?: JoiGitUrlParams }, value: string, state, prefs) { + // Make sure it's a string + const baseSchema = Joi.string() + const result = baseSchema.validate(value) + + if (result.error) { + return result.error + } + + if (!isGitUrl(value)) { + // tslint:disable-next-line:no-invalid-this + return this.createError("string.gitUrl", { v: value }, state, prefs) + } + + if (params.options && params.options.requireHash === true) { + const url = splitLast(value, "#")[0] + if (!url) { + // tslint:disable-next-line:no-invalid-this + return this.createError("string.requireHash", { v: value }, state, prefs) + } + } + + return value + }, + }, { name: "posixPath", params: { @@ -127,7 +170,7 @@ export const joiProviderName = (name: string) => joiIdentifier().required() .default(name) .example(name) -export const joiStringMap = (valueSchema: JoiObject) => Joi +export const joiStringMap = (valueSchema: JoiObject) => joi .object().pattern(/.+/, valueSchema) export const joiUserIdentifier = () => joi.string() @@ -138,18 +181,18 @@ export const joiUserIdentifier = () => joi.string() "or be longer than 63 characters.", ) -export const joiIdentifierMap = (valueSchema: JoiObject) => Joi +export const joiIdentifierMap = (valueSchema: JoiObject) => joi .object().pattern(identifierRegex, valueSchema) .default(() => ({}), "{}") .description("Key/value map. Keys must be valid identifiers.") -export const joiVariables = () => Joi +export const joiVariables = () => joi .object().pattern(/[\w\d]+/i, joiPrimitive()) .default(() => ({}), "{}") .unknown(false) .description("Key/value map. Keys may contain letters and numbers, and values must be primitives.") -export const joiEnvVars = () => Joi +export const joiEnvVars = () => joi .object().pattern(envVarRegex, joiPrimitive()) .default(() => ({}), "{}") .unknown(false) @@ -158,22 +201,15 @@ export const joiEnvVars = () => Joi "(must not start with `GARDEN`) and values must be primitives.", ) -export const joiArray = (schema) => Joi +export const joiArray = (schema) => joi .array().items(schema) .default(() => [], "[]") -export const joiRepositoryUrl = () => Joi - .string() - .uri({ - scheme: [ - "git", - /git\+https?/, - /git\+ssh?/, - "https", - "file", - "ssh", - ], - }) +export const joiRepositoryUrl = () => joi.alternatives( + joi.string().gitUrl({ requireHash: true }), + // Allow file URLs as well + joi.string().uri({ scheme: ["file"] }), +) .description( "A remote repository URL. Currently only supports git servers. Must contain a hash suffix" + " pointing to a specific branch or tag, with the format: #", diff --git a/garden-service/src/vcs/git.ts b/garden-service/src/vcs/git.ts index 01e32dae29..2288c3e081 100644 --- a/garden-service/src/vcs/git.ts +++ b/garden-service/src/vcs/git.ts @@ -17,6 +17,7 @@ import { ConfigurationError, RuntimeError } from "../exceptions" import * as Bluebird from "bluebird" import { matchGlobs } from "../util/fs" import { deline } from "../util/string" +import { splitLast } from "../util/util" export function getCommitIdFromRefList(refList: string[]): string { try { @@ -27,9 +28,8 @@ export function getCommitIdFromRefList(refList: string[]): string { } export function parseGitUrl(url: string) { - const parts = url.split("#") - const parsed = { repositoryUrl: parts[0], hash: parts[1] } - if (!parsed.hash) { + const parts = splitLast(url, "#") + if (!parts[0]) { throw new ConfigurationError( deline` Repository URLs must contain a hash part pointing to a specific branch or tag @@ -37,6 +37,7 @@ export function parseGitUrl(url: string) { { repositoryUrl: url }, ) } + const parsed = { repositoryUrl: parts[0], hash: parts[1] } return parsed } diff --git a/garden-service/test/unit/src/config/common.ts b/garden-service/test/unit/src/config/common.ts index 0194e60975..2c8b725d15 100644 --- a/garden-service/test/unit/src/config/common.ts +++ b/garden-service/test/unit/src/config/common.ts @@ -1,6 +1,13 @@ import { expect } from "chai" const stripAnsi = require("strip-ansi") -import { identifierRegex, validate, envVarRegex, userIdentifierRegex, joi } from "../../../../src/config/common" +import { + identifierRegex, + validate, + envVarRegex, + userIdentifierRegex, + joi, + joiRepositoryUrl, +} from "../../../../src/config/common" import { expectError } from "../../../helpers" describe("envVarRegex", () => { @@ -238,3 +245,68 @@ describe("joi.posixPath", () => { expect(result.error).to.be.null }) }) + +describe("joiRepositoryUrl", () => { + it("should accept a git:// URL", () => { + const url = "git://github.com/garden-io/garden-example-remote-sources-web-services.git#my-tag" + const schema = joiRepositoryUrl() + const result = schema.validate(url) + expect(result.error).to.be.null + }) + + it("should accept an HTTPS Git URL", () => { + const url = "https://github.com/garden-io/garden-example-remote-sources-web-services.git#my-tag" + const schema = joiRepositoryUrl() + const result = schema.validate(url) + expect(result.error).to.be.null + }) + + it("should accept an scp-like SSH GitHub URL", () => { + const url = "git@github.com:garden-io/garden-example-remote-sources-web-services.git#my-tag" + const schema = joiRepositoryUrl() + const result = schema.validate(url) + expect(result.error).to.be.null + }) + + it("should accept an ssh:// GitHub URL", () => { + const url = "ssh://git@github.com/garden-io/garden-example-remote-sources-web-services.git#my-tag" + const schema = joiRepositoryUrl() + const result = schema.validate(url) + expect(result.error).to.be.null + }) + + it("should accept a git+https// URL", () => { + const url = "git+https://git@github.com:garden-io/garden-example-remote-sources-web-services.git#my-tag" + const schema = joiRepositoryUrl() + const result = schema.validate(url) + expect(result.error).to.be.null + }) + + it("should accept a git+ssh// URL", () => { + const url = "git+ssh://git@github.com:garden-io/garden-example-remote-sources-web-services.git#my-tag" + const schema = joiRepositoryUrl() + const result = schema.validate(url) + expect(result.error).to.be.null + }) + + it("should accept a local file:// URL", () => { + const url = "file:///some/dir" + const schema = joiRepositoryUrl() + const result = schema.validate(url) + expect(result.error).to.be.null + }) + + it("should reject non-string values", () => { + const url = 123 + const schema = joiRepositoryUrl() + const result = schema.validate(url) + expect(result.error).to.exist + }) + + it("should require a branch/tag name", () => { + const url = "https://github.com/garden-io/garden-example-remote-sources-web-services.git" + const schema = joiRepositoryUrl() + const result = schema.validate(url) + expect(result.error).to.exist + }) +}) diff --git a/garden-service/test/unit/src/config/project.ts b/garden-service/test/unit/src/config/project.ts index 901e6c8f23..66786721a0 100644 --- a/garden-service/test/unit/src/config/project.ts +++ b/garden-service/test/unit/src/config/project.ts @@ -65,6 +65,8 @@ describe("resolveProjectConfig", () => { }) it("should resolve template strings on fields other than provider configs", async () => { + const repositoryUrl = "git://github.com/foo/bar.git#boo" + const config: ProjectConfig = { apiVersion: DEFAULT_API_VERSION, kind: "Project", @@ -90,7 +92,7 @@ describe("resolveProjectConfig", () => { sources: [ { name: "\${local.env.TEST_ENV_VAR}", - repositoryUrl: "git://\${local.env.TEST_ENV_VAR}", + repositoryUrl, }, ], variables: { @@ -118,7 +120,7 @@ describe("resolveProjectConfig", () => { sources: [ { name: "foo", - repositoryUrl: "git://foo", + repositoryUrl, }, ], variables: { From 7b061ecbb82e6c7f94346ae373997497eebb754f Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Mon, 22 Jul 2019 18:34:30 +0200 Subject: [PATCH 2/2] docs: improve rendering of "alternatives" schema type --- docs/reference/config.md | 6 +++--- garden-service/src/docs/config.ts | 20 +++++++++++++++++++- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/docs/reference/config.md b/docs/reference/config.md index 41032bd013..04b45dbf6a 100644 --- a/docs/reference/config.md +++ b/docs/reference/config.md @@ -136,9 +136,9 @@ A key/value map of variables that modules can reference when using this environm A list of environments to configure for the project. -| Type | Required | -| -------------- | -------- | -| `alternatives` | No | +| Type | Required | +| ------------------------------- | -------- | +| `array[object] | array[string]` | No | Example: diff --git a/garden-service/src/docs/config.ts b/garden-service/src/docs/config.ts index 3a92aee61a..ebbefdf193 100644 --- a/garden-service/src/docs/config.ts +++ b/garden-service/src/docs/config.ts @@ -143,8 +143,11 @@ function normalizeKeyDescription(description: Description): NormalizedDescriptio let hasChildren: boolean = false let arrayType: string | undefined const { type } = description + const formattedType = formatType(description) + const children = type === "object" && Object.entries(description.children || {}) const items = type === "array" && description.items + if (children && children.length > 0) { hasChildren = true } else if (items && items.length > 0) { @@ -164,7 +167,6 @@ function normalizeKeyDescription(description: Description): NormalizedDescriptio } const formattedName = type === "array" ? `${description.name}[]` : description.name - const formattedType = (type === "array" && arrayType ? `array[${arrayType}]` : type) || "" return { ...description, @@ -178,6 +180,22 @@ function normalizeKeyDescription(description: Description): NormalizedDescriptio } } +function formatType(description: Description) { + const { type } = description + const items = type === "array" && description.items + + if (items && items.length > 0) { + // We don't consider an array of primitives as children + const arrayType = items[0].type + return `array[${arrayType}]` + } else if (type === "alternatives") { + // returns e.g. "string|number" + return uniq(description.alternatives.map(formatType)).join(" | ") + } else { + return type || "" + } +} + export function getDefaultValue(description: Joi.Description) { const defaultSpec = get(description, "flags.default")