feat(aio): enable previews for any PR

This commit introduces the ability to show previews for PRs by any author. It works as follows:

- The build artifacts of all PRs are uploaded to the preview server.
- Automatically verified PRs (i.e. from trusted authors or having a specific label) are deployed and
  publicly accessible as usual.
- PRs that could not be automatically verified are stored for later use (after re-verification).
- A PR can be marked as "trusted" and make its preview publicly accessible by adding the GitHub
  label specified in the `AIO_TRUSTED_PR_LABEL` env var of the preview server.

At the moment, there is no automatic mechanism for notifying the preview server about changes to the
PR's verification status. The PR's "visibility" will be checked and updated every time a new build
is uploaded.
This commit is contained in:
Georgios Kalpakas 2017-06-19 01:15:07 +03:00 committed by Matias Niemelä
parent 0fe685102f
commit 8ae0eec230
20 changed files with 915 additions and 300 deletions

View File

@ -29,6 +29,8 @@ ARG AIO_NGINX_PORT_HTTPS=443
ARG TEST_AIO_NGINX_PORT_HTTPS=4433 ARG TEST_AIO_NGINX_PORT_HTTPS=4433
ARG AIO_REPO_SLUG=angular/angular ARG AIO_REPO_SLUG=angular/angular
ARG TEST_AIO_REPO_SLUG=test-repo/test-slug ARG TEST_AIO_REPO_SLUG=test-repo/test-slug
ARG AIO_TRUSTED_PR_LABEL="aio: preview"
ARG TEST_AIO_TRUSTED_PR_LABEL="aio: preview"
ARG AIO_UPLOAD_HOSTNAME=upload.localhost ARG AIO_UPLOAD_HOSTNAME=upload.localhost
ARG TEST_AIO_UPLOAD_HOSTNAME=upload.localhost ARG TEST_AIO_UPLOAD_HOSTNAME=upload.localhost
ARG AIO_UPLOAD_MAX_SIZE=20971520 ARG AIO_UPLOAD_MAX_SIZE=20971520
@ -48,6 +50,7 @@ ENV AIO_BUILDS_DIR=$AIO_BUILDS_DIR TEST_AIO_BUILDS_DIR=$TEST
AIO_REPO_SLUG=$AIO_REPO_SLUG TEST_AIO_REPO_SLUG=$TEST_AIO_REPO_SLUG \ AIO_REPO_SLUG=$AIO_REPO_SLUG TEST_AIO_REPO_SLUG=$TEST_AIO_REPO_SLUG \
AIO_SCRIPTS_JS_DIR=/usr/share/aio-scripts-js \ AIO_SCRIPTS_JS_DIR=/usr/share/aio-scripts-js \
AIO_SCRIPTS_SH_DIR=/usr/share/aio-scripts-sh \ AIO_SCRIPTS_SH_DIR=/usr/share/aio-scripts-sh \
AIO_TRUSTED_PR_LABEL=$AIO_TRUSTED_PR_LABEL TEST_AIO_TRUSTED_PR_LABEL=$TEST_AIO_TRUSTED_PR_LABEL \
AIO_UPLOAD_HOSTNAME=$AIO_UPLOAD_HOSTNAME TEST_AIO_UPLOAD_HOSTNAME=$TEST_AIO_UPLOAD_HOSTNAME \ AIO_UPLOAD_HOSTNAME=$AIO_UPLOAD_HOSTNAME TEST_AIO_UPLOAD_HOSTNAME=$TEST_AIO_UPLOAD_HOSTNAME \
AIO_UPLOAD_MAX_SIZE=$AIO_UPLOAD_MAX_SIZE TEST_AIO_UPLOAD_MAX_SIZE=$TEST_AIO_UPLOAD_MAX_SIZE \ AIO_UPLOAD_MAX_SIZE=$AIO_UPLOAD_MAX_SIZE TEST_AIO_UPLOAD_MAX_SIZE=$TEST_AIO_UPLOAD_MAX_SIZE \
AIO_UPLOAD_PORT=$AIO_UPLOAD_PORT TEST_AIO_UPLOAD_PORT=$TEST_AIO_UPLOAD_PORT \ AIO_UPLOAD_PORT=$AIO_UPLOAD_PORT TEST_AIO_UPLOAD_PORT=$TEST_AIO_UPLOAD_PORT \

View File

@ -6,6 +6,7 @@ import {GithubApi} from './github-api';
export interface PullRequest { export interface PullRequest {
number: number; number: number;
user: {login: string}; user: {login: string};
labels: {name: string}[];
} }
export type PullRequestState = 'all' | 'closed' | 'open'; export type PullRequestState = 'all' | 'closed' | 'open';
@ -30,7 +31,8 @@ export class GithubPullRequests extends GithubApi {
} }
public fetch(pr: number): Promise<PullRequest> { public fetch(pr: number): Promise<PullRequest> {
return this.get<PullRequest>(`/repos/${this.repoSlug}/pulls/${pr}`); // Using the `/issues/` URL, because the `/pulls/` one does not provide labels.
return this.get<PullRequest>(`/repos/${this.repoSlug}/issues/${pr}`);
} }
public fetchAll(state: PullRequestState = 'all'): Promise<PullRequest[]> { public fetchAll(state: PullRequestState = 'all'): Promise<PullRequest[]> {

View File

@ -5,11 +5,14 @@ import * as fs from 'fs';
import * as path from 'path'; import * as path from 'path';
import * as shell from 'shelljs'; import * as shell from 'shelljs';
import {assertNotMissingOrEmpty} from '../common/utils'; import {assertNotMissingOrEmpty} from '../common/utils';
import {CreatedBuildEvent} from './build-events'; import {ChangedPrVisibilityEvent, CreatedBuildEvent} from './build-events';
import {UploadError} from './upload-error'; import {UploadError} from './upload-error';
// Classes // Classes
export class BuildCreator extends EventEmitter { export class BuildCreator extends EventEmitter {
// Properties - Public, Static
public static HIDDEN_DIR_PREFIX = 'hidden--';
// Constructor // Constructor
constructor(protected buildsDir: string) { constructor(protected buildsDir: string) {
super(); super();
@ -17,13 +20,43 @@ export class BuildCreator extends EventEmitter {
} }
// Methods - Public // Methods - Public
public create(pr: string, sha: string, archivePath: string): Promise<any> { public changePrVisibility(pr: string, makePublic: boolean): Promise<void> {
const prDir = path.join(this.buildsDir, pr); const {oldPrDir, newPrDir} = this.getCandidatePrDirs(pr, makePublic);
return Promise.
all([this.exists(oldPrDir), this.exists(newPrDir)]).
then(([oldPrDirExisted, newPrDirExisted]) => {
if (!oldPrDirExisted) {
throw new UploadError(404, `Request to move non-existing directory '${oldPrDir}' to '${newPrDir}'.`);
} else if (newPrDirExisted) {
throw new UploadError(409, `Request to move '${oldPrDir}' to existing directory '${newPrDir}'.`);
}
return Promise.resolve().
then(() => shell.mv(oldPrDir, newPrDir)).
then(() => this.listShasByDate(newPrDir)).
then(shas => this.emit(ChangedPrVisibilityEvent.type, new ChangedPrVisibilityEvent(+pr, shas, makePublic))).
then(() => undefined);
}).
catch(err => {
if (!(err instanceof UploadError)) {
err = new UploadError(500, `Error while making PR ${pr} ${makePublic ? 'public' : 'hidden'}.\n${err}`);
}
throw err;
});
}
public create(pr: string, sha: string, archivePath: string, isPublic: boolean): Promise<void> {
const {oldPrDir: otherVisPrDir, newPrDir: prDir} = this.getCandidatePrDirs(pr, isPublic);
const shaDir = path.join(prDir, sha); const shaDir = path.join(prDir, sha);
let dirToRemoveOnError: string; let dirToRemoveOnError: string;
return Promise. return Promise.resolve().
all([this.exists(prDir), this.exists(shaDir)]). then(() => this.exists(otherVisPrDir)).
// If the same PR exists with different visibility, update the visibility first.
then(otherVisPrDirExisted => (otherVisPrDirExisted && this.changePrVisibility(pr, isPublic)) as any).
then(() => Promise.all([this.exists(prDir), this.exists(shaDir)])).
then(([prDirExisted, shaDirExisted]) => { then(([prDirExisted, shaDirExisted]) => {
if (shaDirExisted) { if (shaDirExisted) {
throw new UploadError(409, `Request to overwrite existing directory: ${shaDir}`); throw new UploadError(409, `Request to overwrite existing directory: ${shaDir}`);
@ -34,7 +67,8 @@ export class BuildCreator extends EventEmitter {
return Promise.resolve(). return Promise.resolve().
then(() => shell.mkdir('-p', shaDir)). then(() => shell.mkdir('-p', shaDir)).
then(() => this.extractArchive(archivePath, shaDir)). then(() => this.extractArchive(archivePath, shaDir)).
then(() => this.emit(CreatedBuildEvent.type, new CreatedBuildEvent(+pr, sha))); then(() => this.emit(CreatedBuildEvent.type, new CreatedBuildEvent(+pr, sha, isPublic))).
then(() => undefined);
}). }).
catch(err => { catch(err => {
if (dirToRemoveOnError) { if (dirToRemoveOnError) {
@ -78,4 +112,26 @@ export class BuildCreator extends EventEmitter {
}); });
}); });
} }
protected getCandidatePrDirs(pr: string, isPublic: boolean) {
const hiddenPrDir = path.join(this.buildsDir, BuildCreator.HIDDEN_DIR_PREFIX + pr);
const publicPrDir = path.join(this.buildsDir, pr);
const oldPrDir = isPublic ? hiddenPrDir : publicPrDir;
const newPrDir = isPublic ? publicPrDir : hiddenPrDir;
return {oldPrDir, newPrDir};
}
protected listShasByDate(inputDir: string): Promise<string[]> {
return Promise.resolve().
then(() => shell.ls('-l', inputDir) as any as Promise<(fs.Stats & {name: string})[]>).
// Keep directories only.
// (Also, convert to standard Array - ShellJS provides custom `sort()` method for sorting file contents.)
then(items => items.filter(item => item.isDirectory())).
// Sort by modification date.
then(items => items.sort((a, b) => a.mtime.getTime() - b.mtime.getTime())).
// Return directory names.
then(items => items.map(item => item.name));
}
} }

View File

@ -1,8 +1,16 @@
// Classes // Classes
export class ChangedPrVisibilityEvent {
// Properties - Public, Static
public static type = 'pr.changedVisibility';
// Constructor
constructor(public pr: number, public shas: string[], public isPublic: boolean) {}
}
export class CreatedBuildEvent { export class CreatedBuildEvent {
// Properties - Public, Static // Properties - Public, Static
public static type = 'build.created'; public static type = 'build.created';
// Constructor // Constructor
constructor(public pr: number, public sha: string) {} constructor(public pr: number, public sha: string, public isPublic: boolean) {}
} }

View File

@ -1,6 +1,6 @@
// Imports // Imports
import * as jwt from 'jsonwebtoken'; import * as jwt from 'jsonwebtoken';
import {GithubPullRequests} from '../common/github-pull-requests'; import {GithubPullRequests, PullRequest} from '../common/github-pull-requests';
import {GithubTeams} from '../common/github-teams'; import {GithubTeams} from '../common/github-teams';
import {assertNotMissingOrEmpty} from '../common/utils'; import {assertNotMissingOrEmpty} from '../common/utils';
import {UploadError} from './upload-error'; import {UploadError} from './upload-error';
@ -11,6 +11,12 @@ interface JwtPayload {
'pull-request': number; 'pull-request': number;
} }
// Enums
export enum BUILD_VERIFICATION_STATUS {
verifiedAndTrusted,
verifiedNotTrusted,
}
// Classes // Classes
export class BuildVerifier { export class BuildVerifier {
// Properties - Protected // Properties - Protected
@ -19,27 +25,27 @@ export class BuildVerifier {
// Constructor // Constructor
constructor(protected secret: string, githubToken: string, protected repoSlug: string, organization: string, constructor(protected secret: string, githubToken: string, protected repoSlug: string, organization: string,
protected allowedTeamSlugs: string[]) { protected allowedTeamSlugs: string[], protected trustedPrLabel: string) {
assertNotMissingOrEmpty('secret', secret); assertNotMissingOrEmpty('secret', secret);
assertNotMissingOrEmpty('githubToken', githubToken); assertNotMissingOrEmpty('githubToken', githubToken);
assertNotMissingOrEmpty('repoSlug', repoSlug); assertNotMissingOrEmpty('repoSlug', repoSlug);
assertNotMissingOrEmpty('organization', organization); assertNotMissingOrEmpty('organization', organization);
assertNotMissingOrEmpty('allowedTeamSlugs', allowedTeamSlugs && allowedTeamSlugs.join('')); assertNotMissingOrEmpty('allowedTeamSlugs', allowedTeamSlugs && allowedTeamSlugs.join(''));
assertNotMissingOrEmpty('trustedPrLabel', trustedPrLabel);
this.githubPullRequests = new GithubPullRequests(githubToken, repoSlug); this.githubPullRequests = new GithubPullRequests(githubToken, repoSlug);
this.githubTeams = new GithubTeams(githubToken, organization); this.githubTeams = new GithubTeams(githubToken, organization);
} }
// Methods - Public // Methods - Public
public getPrAuthorTeamMembership(pr: number): Promise<{author: string, isMember: boolean}> { public getPrIsTrusted(pr: number): Promise<boolean> {
return Promise.resolve(). return Promise.resolve().
then(() => this.githubPullRequests.fetch(pr)). then(() => this.githubPullRequests.fetch(pr)).
then(prInfo => prInfo.user.login). then(prInfo => this.hasLabel(prInfo, this.trustedPrLabel) ||
then(author => this.githubTeams.isMemberBySlug(author, this.allowedTeamSlugs). this.githubTeams.isMemberBySlug(prInfo.user.login, this.allowedTeamSlugs));
then(isMember => ({author, isMember})));
} }
public verify(expectedPr: number, authHeader: string): Promise<void> { public verify(expectedPr: number, authHeader: string): Promise<BUILD_VERIFICATION_STATUS> {
return Promise.resolve(). return Promise.resolve().
then(() => this.extractJwtString(authHeader)). then(() => this.extractJwtString(authHeader)).
then(jwtString => this.verifyJwt(expectedPr, jwtString)). then(jwtString => this.verifyJwt(expectedPr, jwtString)).
@ -52,6 +58,10 @@ export class BuildVerifier {
return input.replace(/^token +/i, ''); return input.replace(/^token +/i, '');
} }
protected hasLabel(prInfo: PullRequest, label: string) {
return prInfo.labels.some(labelObj => labelObj.name === label);
}
protected verifyJwt(expectedPr: number, token: string): Promise<JwtPayload> { protected verifyJwt(expectedPr: number, token: string): Promise<JwtPayload> {
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
jwt.verify(token, this.secret, {issuer: 'Travis CI, GmbH'}, (err, payload: JwtPayload) => { jwt.verify(token, this.secret, {issuer: 'Travis CI, GmbH'}, (err, payload: JwtPayload) => {
@ -68,11 +78,10 @@ export class BuildVerifier {
}); });
} }
protected verifyPr(pr: number): Promise<void> { protected verifyPr(pr: number): Promise<BUILD_VERIFICATION_STATUS> {
return this.getPrAuthorTeamMembership(pr). return this.getPrIsTrusted(pr).
then(({author, isMember}) => isMember ? Promise.resolve() : Promise.reject( then(isTrusted => Promise.resolve(isTrusted ?
`User '${author}' is not an active member of any of the following teams: ` + BUILD_VERIFICATION_STATUS.verifiedAndTrusted :
`${this.allowedTeamSlugs.join(', ')}`, BUILD_VERIFICATION_STATUS.verifiedNotTrusted));
));
} }
} }

View File

@ -12,28 +12,28 @@ function _main() {
const repoSlug = getEnvVar('AIO_REPO_SLUG'); const repoSlug = getEnvVar('AIO_REPO_SLUG');
const organization = getEnvVar('AIO_GITHUB_ORGANIZATION'); const organization = getEnvVar('AIO_GITHUB_ORGANIZATION');
const allowedTeamSlugs = getEnvVar('AIO_GITHUB_TEAM_SLUGS').split(','); const allowedTeamSlugs = getEnvVar('AIO_GITHUB_TEAM_SLUGS').split(',');
const trustedPrLabel = getEnvVar('AIO_TRUSTED_PR_LABEL');
const pr = +getEnvVar('AIO_PREVERIFY_PR'); const pr = +getEnvVar('AIO_PREVERIFY_PR');
const buildVerifier = new BuildVerifier(secret, githubToken, repoSlug, organization, allowedTeamSlugs); const buildVerifier = new BuildVerifier(secret, githubToken, repoSlug, organization, allowedTeamSlugs,
trustedPrLabel);
// Exit codes: // Exit codes:
// - 0: The PR author is a member. // - 0: The PR can be automatically trusted (i.e. author belongs to trusted team or PR has the "trusted PR" label).
// - 1: An error occurred. // - 1: An error occurred.
// - 2: The PR author is not a member. // - 2: The PR cannot be automatically trusted.
buildVerifier.getPrAuthorTeamMembership(pr). buildVerifier.getPrIsTrusted(pr).
then(({author, isMember}) => { then(isTrusted => {
if (isMember) { if (!isTrusted) {
process.exit(0); console.warn(
} else { `The PR cannot be automatically verified, because it doesn't have the "${trustedPrLabel}" label and the ` +
const errorMessage = `User '${author}' is not an active member of any of the following teams: ` + `the author is not an active member of any of the following teams: ${allowedTeamSlugs.join(', ')}`);
`${allowedTeamSlugs.join(', ')}`;
onError(errorMessage, 2);
}
}).
catch(err => onError(err, 1));
} }
function onError(err: string, exitCode: number) { process.exit(isTrusted ? 0 : 2);
}).
catch(err => {
console.error(err); console.error(err);
process.exit(exitCode || 1); process.exit(1);
});
} }

View File

@ -1,10 +1,10 @@
// Imports // Imports
import {GithubPullRequests} from '../common/github-pull-requests'; import {GithubPullRequests} from '../common/github-pull-requests';
import {BuildVerifier} from './build-verifier'; import {BUILD_VERIFICATION_STATUS, BuildVerifier} from './build-verifier';
// Run // Run
// TODO(gkalpak): Add e2e tests to cover these interactions as well. // TODO(gkalpak): Add e2e tests to cover these interactions as well.
GithubPullRequests.prototype.addComment = () => Promise.resolve(); GithubPullRequests.prototype.addComment = () => Promise.resolve();
BuildVerifier.prototype.verify = () => Promise.resolve(); BuildVerifier.prototype.verify = () => Promise.resolve(BUILD_VERIFICATION_STATUS.verifiedAndTrusted);
// tslint:disable-next-line: no-var-requires // tslint:disable-next-line: no-var-requires
require('./index'); require('./index');

View File

@ -10,6 +10,7 @@ const AIO_GITHUB_TEAM_SLUGS = getEnvVar('AIO_GITHUB_TEAM_SLUGS');
const AIO_GITHUB_TOKEN = getEnvVar('AIO_GITHUB_TOKEN'); const AIO_GITHUB_TOKEN = getEnvVar('AIO_GITHUB_TOKEN');
const AIO_PREVIEW_DEPLOYMENT_TOKEN = getEnvVar('AIO_PREVIEW_DEPLOYMENT_TOKEN'); const AIO_PREVIEW_DEPLOYMENT_TOKEN = getEnvVar('AIO_PREVIEW_DEPLOYMENT_TOKEN');
const AIO_REPO_SLUG = getEnvVar('AIO_REPO_SLUG'); const AIO_REPO_SLUG = getEnvVar('AIO_REPO_SLUG');
const AIO_TRUSTED_PR_LABEL = getEnvVar('AIO_TRUSTED_PR_LABEL');
const AIO_UPLOAD_HOSTNAME = getEnvVar('AIO_UPLOAD_HOSTNAME'); const AIO_UPLOAD_HOSTNAME = getEnvVar('AIO_UPLOAD_HOSTNAME');
const AIO_UPLOAD_PORT = +getEnvVar('AIO_UPLOAD_PORT'); const AIO_UPLOAD_PORT = +getEnvVar('AIO_UPLOAD_PORT');
const AIO_WWW_USER = getEnvVar('AIO_WWW_USER'); const AIO_WWW_USER = getEnvVar('AIO_WWW_USER');
@ -29,6 +30,7 @@ function _main() {
githubToken: AIO_GITHUB_TOKEN, githubToken: AIO_GITHUB_TOKEN,
repoSlug: AIO_REPO_SLUG, repoSlug: AIO_REPO_SLUG,
secret: AIO_PREVIEW_DEPLOYMENT_TOKEN, secret: AIO_PREVIEW_DEPLOYMENT_TOKEN,
trustedPrLabel: AIO_TRUSTED_PR_LABEL,
}). }).
listen(AIO_UPLOAD_PORT, AIO_UPLOAD_HOSTNAME); listen(AIO_UPLOAD_PORT, AIO_UPLOAD_HOSTNAME);
} }

View File

@ -4,8 +4,8 @@ import * as http from 'http';
import {GithubPullRequests} from '../common/github-pull-requests'; import {GithubPullRequests} from '../common/github-pull-requests';
import {assertNotMissingOrEmpty} from '../common/utils'; import {assertNotMissingOrEmpty} from '../common/utils';
import {BuildCreator} from './build-creator'; import {BuildCreator} from './build-creator';
import {CreatedBuildEvent} from './build-events'; import {ChangedPrVisibilityEvent, CreatedBuildEvent} from './build-events';
import {BuildVerifier} from './build-verifier'; import {BUILD_VERIFICATION_STATUS, BuildVerifier} from './build-verifier';
import {UploadError} from './upload-error'; import {UploadError} from './upload-error';
// Constants // Constants
@ -21,6 +21,7 @@ interface UploadServerConfig {
githubToken: string; githubToken: string;
repoSlug: string; repoSlug: string;
secret: string; secret: string;
trustedPrLabel: string;
} }
// Classes // Classes
@ -34,10 +35,12 @@ class UploadServerFactory {
githubToken, githubToken,
repoSlug, repoSlug,
secret, secret,
trustedPrLabel,
}: UploadServerConfig): http.Server { }: UploadServerConfig): http.Server {
assertNotMissingOrEmpty('domainName', domainName); assertNotMissingOrEmpty('domainName', domainName);
const buildVerifier = new BuildVerifier(secret, githubToken, repoSlug, githubOrganization, githubTeamSlugs); const buildVerifier = new BuildVerifier(secret, githubToken, repoSlug, githubOrganization, githubTeamSlugs,
trustedPrLabel);
const buildCreator = this.createBuildCreator(buildsDir, githubToken, repoSlug, domainName); const buildCreator = this.createBuildCreator(buildsDir, githubToken, repoSlug, domainName);
const middleware = this.createMiddleware(buildVerifier, buildCreator); const middleware = this.createMiddleware(buildVerifier, buildCreator);
@ -56,12 +59,24 @@ class UploadServerFactory {
domainName: string): BuildCreator { domainName: string): BuildCreator {
const buildCreator = new BuildCreator(buildsDir); const buildCreator = new BuildCreator(buildsDir);
const githubPullRequests = new GithubPullRequests(githubToken, repoSlug); const githubPullRequests = new GithubPullRequests(githubToken, repoSlug);
const postPreviewsComment = (pr: number, shas: string[]) => {
const body = shas.
map(sha => `You can preview ${sha} at https://pr${pr}-${sha}.${domainName}/.`).
join('\n');
buildCreator.on(CreatedBuildEvent.type, ({pr, sha}: CreatedBuildEvent) => { return githubPullRequests.addComment(pr, body);
const body = `The angular.io preview for ${sha} is available [here][1].\n\n` + };
`[1]: https://pr${pr}-${sha}.${domainName}/`;
githubPullRequests.addComment(pr, body); buildCreator.on(CreatedBuildEvent.type, ({pr, sha, isPublic}: CreatedBuildEvent) => {
if (isPublic) {
postPreviewsComment(pr, [sha]);
}
});
buildCreator.on(ChangedPrVisibilityEvent.type, ({pr, shas, isPublic}: ChangedPrVisibilityEvent) => {
if (isPublic && shas.length) {
postPreviewsComment(pr, shas);
}
}); });
return buildCreator; return buildCreator;
@ -83,8 +98,9 @@ class UploadServerFactory {
} else { } else {
buildVerifier. buildVerifier.
verify(+pr, authHeader). verify(+pr, authHeader).
then(() => buildCreator.create(pr, sha, archive)). then(verStatus => verStatus === BUILD_VERIFICATION_STATUS.verifiedAndTrusted).
then(() => res.sendStatus(201)). then(isPublic => buildCreator.create(pr, sha, archive, isPublic).
then(() => res.sendStatus(isPublic ? 201 : 202))).
catch(err => this.respondWithError(res, err)); catch(err => this.respondWithError(res, err));
} }
}); });

View File

@ -5,7 +5,7 @@ import * as fs from 'fs';
import * as path from 'path'; import * as path from 'path';
import * as shell from 'shelljs'; import * as shell from 'shelljs';
import {BuildCreator} from '../../lib/upload-server/build-creator'; import {BuildCreator} from '../../lib/upload-server/build-creator';
import {CreatedBuildEvent} from '../../lib/upload-server/build-events'; import {ChangedPrVisibilityEvent, CreatedBuildEvent} from '../../lib/upload-server/build-events';
import {UploadError} from '../../lib/upload-server/upload-error'; import {UploadError} from '../../lib/upload-server/upload-error';
import {expectToBeUploadError} from './helpers'; import {expectToBeUploadError} from './helpers';
@ -15,8 +15,10 @@ describe('BuildCreator', () => {
const sha = '9'.repeat(40); const sha = '9'.repeat(40);
const archive = 'snapshot.tar.gz'; const archive = 'snapshot.tar.gz';
const buildsDir = 'builds/dir'; const buildsDir = 'builds/dir';
const prDir = path.join(buildsDir, pr); const hiddenPrDir = path.join(buildsDir, `hidden--${pr}`);
const shaDir = path.join(prDir, sha); const publicPrDir = path.join(buildsDir, pr);
const hiddenShaDir = path.join(hiddenPrDir, sha);
const publicShaDir = path.join(publicPrDir, sha);
let bc: BuildCreator; let bc: BuildCreator;
beforeEach(() => bc = new BuildCreator(buildsDir)); beforeEach(() => bc = new BuildCreator(buildsDir));
@ -39,7 +41,160 @@ describe('BuildCreator', () => {
}); });
describe('changePrVisibility()', () => {
let bcEmitSpy: jasmine.Spy;
let bcExistsSpy: jasmine.Spy;
let bcListShasByDate: jasmine.Spy;
let shellMvSpy: jasmine.Spy;
beforeEach(() => {
bcEmitSpy = spyOn(bc, 'emit');
bcExistsSpy = spyOn(bc as any, 'exists');
bcListShasByDate = spyOn(bc as any, 'listShasByDate');
shellMvSpy = spyOn(shell, 'mv');
bcExistsSpy.and.returnValues(Promise.resolve(true), Promise.resolve(false));
bcListShasByDate.and.returnValue([]);
});
it('should return a promise', done => {
const promise = bc.changePrVisibility(pr, true);
promise.then(done); // Do not complete the test (and release the spies) synchronously
// to avoid running the actual `extractArchive()`.
expect(promise).toEqual(jasmine.any(Promise));
});
[true, false].forEach(makePublic => {
const oldPrDir = makePublic ? hiddenPrDir : publicPrDir;
const newPrDir = makePublic ? publicPrDir : hiddenPrDir;
it('should rename the directory', done => {
bc.changePrVisibility(pr, makePublic).
then(() => expect(shellMvSpy).toHaveBeenCalledWith(oldPrDir, newPrDir)).
then(done);
});
it('should emit a ChangedPrVisibilityEvent on success', done => {
let emitted = false;
bcEmitSpy.and.callFake((type: string, evt: ChangedPrVisibilityEvent) => {
expect(type).toBe(ChangedPrVisibilityEvent.type);
expect(evt).toEqual(jasmine.any(ChangedPrVisibilityEvent));
expect(evt.pr).toBe(+pr);
expect(evt.shas).toEqual(jasmine.any(Array));
expect(evt.isPublic).toBe(makePublic);
emitted = true;
});
bc.changePrVisibility(pr, makePublic).
then(() => expect(emitted).toBe(true)).
then(done);
});
it('should include all shas in the emitted event', done => {
const shas = ['foo', 'bar', 'baz'];
let emitted = false;
bcListShasByDate.and.returnValue(Promise.resolve(shas));
bcEmitSpy.and.callFake((type: string, evt: ChangedPrVisibilityEvent) => {
expect(bcListShasByDate).toHaveBeenCalledWith(newPrDir);
expect(type).toBe(ChangedPrVisibilityEvent.type);
expect(evt).toEqual(jasmine.any(ChangedPrVisibilityEvent));
expect(evt.pr).toBe(+pr);
expect(evt.shas).toBe(shas);
expect(evt.isPublic).toBe(makePublic);
emitted = true;
});
bc.changePrVisibility(pr, makePublic).
then(() => expect(emitted).toBe(true)).
then(done);
});
describe('on error', () => {
it('should abort and skip further operations if the old directory does not exist', done => {
bcExistsSpy.and.callFake((dir: string) => dir !== oldPrDir);
bc.changePrVisibility(pr, makePublic).catch(err => {
expectToBeUploadError(err, 404, `Request to move non-existing directory '${oldPrDir}' to '${newPrDir}'.`);
expect(shellMvSpy).not.toHaveBeenCalled();
expect(bcListShasByDate).not.toHaveBeenCalled();
expect(bcEmitSpy).not.toHaveBeenCalled();
done();
});
});
it('should abort and skip further operations if the new directory does already exist', done => {
bcExistsSpy.and.returnValue(true);
bc.changePrVisibility(pr, makePublic).catch(err => {
expectToBeUploadError(err, 409, `Request to move '${oldPrDir}' to existing directory '${newPrDir}'.`);
expect(shellMvSpy).not.toHaveBeenCalled();
expect(bcListShasByDate).not.toHaveBeenCalled();
expect(bcEmitSpy).not.toHaveBeenCalled();
done();
});
});
it('should abort and skip further operations if it fails to rename the directory', done => {
shellMvSpy.and.throwError('');
bc.changePrVisibility(pr, makePublic).catch(() => {
expect(shellMvSpy).toHaveBeenCalled();
expect(bcListShasByDate).not.toHaveBeenCalled();
expect(bcEmitSpy).not.toHaveBeenCalled();
done();
});
});
it('should abort and skip further operations if it fails to list the SHAs', done => {
bcListShasByDate.and.throwError('');
bc.changePrVisibility(pr, makePublic).catch(() => {
expect(shellMvSpy).toHaveBeenCalled();
expect(bcListShasByDate).toHaveBeenCalled();
expect(bcEmitSpy).not.toHaveBeenCalled();
done();
});
});
it('should reject with an UploadError', done => {
shellMvSpy.and.callFake(() => { throw 'Test'; });
bc.changePrVisibility(pr, makePublic).catch(err => {
expectToBeUploadError(err, 500, `Error while making PR ${pr} ${makePublic ? 'public' : 'hidden'}.\nTest`);
done();
});
});
it('should pass UploadError instances unmodified', done => {
shellMvSpy.and.callFake(() => { throw new UploadError(543, 'Test'); });
bc.changePrVisibility(pr, makePublic).catch(err => {
expectToBeUploadError(err, 543, 'Test');
done();
});
});
});
});
});
describe('create()', () => { describe('create()', () => {
let bcChangePrVisibilitySpy: jasmine.Spy;
let bcEmitSpy: jasmine.Spy; let bcEmitSpy: jasmine.Spy;
let bcExistsSpy: jasmine.Spy; let bcExistsSpy: jasmine.Spy;
let bcExtractArchiveSpy: jasmine.Spy; let bcExtractArchiveSpy: jasmine.Spy;
@ -47,6 +202,7 @@ describe('BuildCreator', () => {
let shellRmSpy: jasmine.Spy; let shellRmSpy: jasmine.Spy;
beforeEach(() => { beforeEach(() => {
bcChangePrVisibilitySpy = spyOn(bc, 'changePrVisibility');
bcEmitSpy = spyOn(bc, 'emit'); bcEmitSpy = spyOn(bc, 'emit');
bcExistsSpy = spyOn(bc as any, 'exists'); bcExistsSpy = spyOn(bc as any, 'exists');
bcExtractArchiveSpy = spyOn(bc as any, 'extractArchive'); bcExtractArchiveSpy = spyOn(bc as any, 'extractArchive');
@ -55,8 +211,14 @@ describe('BuildCreator', () => {
}); });
[true, false].forEach(isPublic => {
const otherVisPrDir = isPublic ? hiddenPrDir : publicPrDir;
const prDir = isPublic ? publicPrDir : hiddenPrDir;
const shaDir = isPublic ? publicShaDir : hiddenShaDir;
it('should return a promise', done => { it('should return a promise', done => {
const promise = bc.create(pr, sha, archive); const promise = bc.create(pr, sha, archive, isPublic);
promise.then(done); // Do not complete the test (and release the spies) synchronously promise.then(done); // Do not complete the test (and release the spies) synchronously
// to avoid running the actual `extractArchive()`. // to avoid running the actual `extractArchive()`.
@ -64,24 +226,35 @@ describe('BuildCreator', () => {
}); });
it('should throw if the build does already exist', done => { it('should not update the PR\'s visibility first if not necessary', done => {
bcExistsSpy.and.returnValue(true); bc.create(pr, sha, archive, isPublic).
bc.create(pr, sha, archive).catch(err => { then(() => expect(bcChangePrVisibilitySpy).not.toHaveBeenCalled()).
expectToBeUploadError(err, 409, `Request to overwrite existing directory: ${shaDir}`); then(done);
done();
}); });
it('should update the PR\'s visibility first if necessary', done => {
bcChangePrVisibilitySpy.and.callFake(() => expect(shellMkdirSpy).not.toHaveBeenCalled());
bcExistsSpy.and.callFake((dir: string) => dir === otherVisPrDir);
bc.create(pr, sha, archive, isPublic).
then(() => {
expect(bcChangePrVisibilitySpy).toHaveBeenCalledWith(pr, isPublic);
expect(shellMkdirSpy).toHaveBeenCalled();
}).
then(done);
}); });
it('should create the build directory (and any parent directories)', done => { it('should create the build directory (and any parent directories)', done => {
bc.create(pr, sha, archive). bc.create(pr, sha, archive, isPublic).
then(() => expect(shellMkdirSpy).toHaveBeenCalledWith('-p', shaDir)). then(() => expect(shellMkdirSpy).toHaveBeenCalledWith('-p', shaDir)).
then(done); then(done);
}); });
it('should extract the archive contents into the build directory', done => { it('should extract the archive contents into the build directory', done => {
bc.create(pr, sha, archive). bc.create(pr, sha, archive, isPublic).
then(() => expect(bcExtractArchiveSpy).toHaveBeenCalledWith(archive, shaDir)). then(() => expect(bcExtractArchiveSpy).toHaveBeenCalledWith(archive, shaDir)).
then(done); then(done);
}); });
@ -95,21 +268,79 @@ describe('BuildCreator', () => {
expect(evt).toEqual(jasmine.any(CreatedBuildEvent)); expect(evt).toEqual(jasmine.any(CreatedBuildEvent));
expect(evt.pr).toBe(+pr); expect(evt.pr).toBe(+pr);
expect(evt.sha).toBe(sha); expect(evt.sha).toBe(sha);
expect(evt.isPublic).toBe(isPublic);
emitted = true; emitted = true;
}); });
bc.create(pr, sha, archive). bc.create(pr, sha, archive, isPublic).
then(() => expect(emitted).toBe(true)). then(() => expect(emitted).toBe(true)).
then(done); then(done);
}); });
describe('on error', () => { describe('on error', () => {
let existsValues: {[dir: string]: boolean};
beforeEach(() => {
existsValues = {
[otherVisPrDir]: false,
[prDir]: false,
[shaDir]: false,
};
bcExistsSpy.and.callFake((dir: string) => existsValues[dir]);
});
it('should abort and skip further operations if changing the PR\'s visibility fails', done => {
const mockError = new UploadError(543, 'Test');
existsValues[otherVisPrDir] = true;
bcChangePrVisibilitySpy.and.returnValue(Promise.reject(mockError));
bc.create(pr, sha, archive, isPublic).catch(err => {
expect(err).toBe(mockError);
expect(bcExistsSpy).toHaveBeenCalledTimes(1);
expect(shellMkdirSpy).not.toHaveBeenCalled();
expect(bcExtractArchiveSpy).not.toHaveBeenCalled();
expect(bcEmitSpy).not.toHaveBeenCalled();
done();
});
});
it('should abort and skip further operations if the build does already exist', done => {
existsValues[shaDir] = true;
bc.create(pr, sha, archive, isPublic).catch(err => {
expectToBeUploadError(err, 409, `Request to overwrite existing directory: ${shaDir}`);
expect(shellMkdirSpy).not.toHaveBeenCalled();
expect(bcExtractArchiveSpy).not.toHaveBeenCalled();
expect(bcEmitSpy).not.toHaveBeenCalled();
done();
});
});
it('should detect existing build directory after visibility change', done => {
existsValues[otherVisPrDir] = true;
bcChangePrVisibilitySpy.and.callFake(() => existsValues[prDir] = existsValues[shaDir] = true);
bc.create(pr, sha, archive, isPublic).catch(err => {
expectToBeUploadError(err, 409, `Request to overwrite existing directory: ${shaDir}`);
expect(shellMkdirSpy).not.toHaveBeenCalled();
expect(bcExtractArchiveSpy).not.toHaveBeenCalled();
expect(bcEmitSpy).not.toHaveBeenCalled();
done();
});
});
it('should abort and skip further operations if it fails to create the directories', done => { it('should abort and skip further operations if it fails to create the directories', done => {
shellMkdirSpy.and.throwError(''); shellMkdirSpy.and.throwError('');
bc.create(pr, sha, archive).catch(() => { bc.create(pr, sha, archive, isPublic).catch(() => {
expect(shellMkdirSpy).toHaveBeenCalled(); expect(shellMkdirSpy).toHaveBeenCalled();
expect(bcExtractArchiveSpy).not.toHaveBeenCalled(); expect(bcExtractArchiveSpy).not.toHaveBeenCalled();
expect(bcEmitSpy).not.toHaveBeenCalled(); expect(bcEmitSpy).not.toHaveBeenCalled();
@ -120,7 +351,7 @@ describe('BuildCreator', () => {
it('should abort and skip further operations if it fails to extract the archive', done => { it('should abort and skip further operations if it fails to extract the archive', done => {
bcExtractArchiveSpy.and.throwError(''); bcExtractArchiveSpy.and.throwError('');
bc.create(pr, sha, archive).catch(() => { bc.create(pr, sha, archive, isPublic).catch(() => {
expect(shellMkdirSpy).toHaveBeenCalled(); expect(shellMkdirSpy).toHaveBeenCalled();
expect(bcExtractArchiveSpy).toHaveBeenCalled(); expect(bcExtractArchiveSpy).toHaveBeenCalled();
expect(bcEmitSpy).not.toHaveBeenCalled(); expect(bcEmitSpy).not.toHaveBeenCalled();
@ -131,7 +362,7 @@ describe('BuildCreator', () => {
it('should delete the PR directory (for new PR)', done => { it('should delete the PR directory (for new PR)', done => {
bcExtractArchiveSpy.and.throwError(''); bcExtractArchiveSpy.and.throwError('');
bc.create(pr, sha, archive).catch(() => { bc.create(pr, sha, archive, isPublic).catch(() => {
expect(shellRmSpy).toHaveBeenCalledWith('-rf', prDir); expect(shellRmSpy).toHaveBeenCalledWith('-rf', prDir);
done(); done();
}); });
@ -139,10 +370,10 @@ describe('BuildCreator', () => {
it('should delete the SHA directory (for existing PR)', done => { it('should delete the SHA directory (for existing PR)', done => {
bcExistsSpy.and.callFake((path: string) => path !== shaDir); existsValues[prDir] = true;
bcExtractArchiveSpy.and.throwError(''); bcExtractArchiveSpy.and.throwError('');
bc.create(pr, sha, archive).catch(() => { bc.create(pr, sha, archive, isPublic).catch(() => {
expect(shellRmSpy).toHaveBeenCalledWith('-rf', shaDir); expect(shellRmSpy).toHaveBeenCalledWith('-rf', shaDir);
done(); done();
}); });
@ -151,7 +382,7 @@ describe('BuildCreator', () => {
it('should reject with an UploadError', done => { it('should reject with an UploadError', done => {
shellMkdirSpy.and.callFake(() => { throw 'Test'; }); shellMkdirSpy.and.callFake(() => { throw 'Test'; });
bc.create(pr, sha, archive).catch(err => { bc.create(pr, sha, archive, isPublic).catch(err => {
expectToBeUploadError(err, 500, `Error while uploading to directory: ${shaDir}\nTest`); expectToBeUploadError(err, 500, `Error while uploading to directory: ${shaDir}\nTest`);
done(); done();
}); });
@ -160,7 +391,7 @@ describe('BuildCreator', () => {
it('should pass UploadError instances unmodified', done => { it('should pass UploadError instances unmodified', done => {
shellMkdirSpy.and.callFake(() => { throw new UploadError(543, 'Test'); }); shellMkdirSpy.and.callFake(() => { throw new UploadError(543, 'Test'); });
bc.create(pr, sha, archive).catch(err => { bc.create(pr, sha, archive, isPublic).catch(err => {
expectToBeUploadError(err, 543, 'Test'); expectToBeUploadError(err, 543, 'Test');
done(); done();
}); });
@ -170,6 +401,8 @@ describe('BuildCreator', () => {
}); });
});
// Protected methods // Protected methods
@ -318,4 +551,101 @@ describe('BuildCreator', () => {
}); });
describe('listShasByDate()', () => {
let shellLsSpy: jasmine.Spy;
const lsResult = (name: string, mtimeMs: number, isDirectory = true) => ({
isDirectory: () => isDirectory,
mtime: new Date(mtimeMs),
name,
});
beforeEach(() => {
shellLsSpy = spyOn(shell, 'ls').and.returnValue([]);
});
it('should return a promise', done => {
const promise = (bc as any).listShasByDate('input/dir');
promise.then(done); // Do not complete the test (and release the spies) synchronously
// to avoid running the actual `ls()`.
expect(promise).toEqual(jasmine.any(Promise));
});
it('should `ls()` files with their metadata', done => {
(bc as any).listShasByDate('input/dir').
then(() => expect(shellLsSpy).toHaveBeenCalledWith('-l', 'input/dir')).
then(done);
});
it('should reject if listing files fails', done => {
shellLsSpy.and.returnValue(Promise.reject('Test'));
(bc as any).listShasByDate('input/dir').catch((err: string) => {
expect(err).toBe('Test');
done();
});
});
it('should return the filenames', done => {
shellLsSpy.and.returnValue(Promise.resolve([
lsResult('foo', 100),
lsResult('bar', 200),
lsResult('baz', 300),
]));
(bc as any).listShasByDate('input/dir').
then((shas: string[]) => expect(shas).toEqual(['foo', 'bar', 'baz'])).
then(done);
});
it('should sort by date', done => {
shellLsSpy.and.returnValue(Promise.resolve([
lsResult('foo', 300),
lsResult('bar', 100),
lsResult('baz', 200),
]));
(bc as any).listShasByDate('input/dir').
then((shas: string[]) => expect(shas).toEqual(['bar', 'baz', 'foo'])).
then(done);
});
it('should not break with ShellJS\' custom `sort()` method', done => {
const mockArray = [
lsResult('foo', 300),
lsResult('bar', 100),
lsResult('baz', 200),
];
mockArray.sort = jasmine.createSpy('sort');
shellLsSpy.and.returnValue(Promise.resolve(mockArray));
(bc as any).listShasByDate('input/dir').
then((shas: string[]) => {
expect(shas).toEqual(['bar', 'baz', 'foo']);
expect(mockArray.sort).not.toHaveBeenCalled();
}).
then(done);
});
it('should only include directories', done => {
shellLsSpy.and.returnValue(Promise.resolve([
lsResult('foo', 100),
lsResult('bar', 200, false),
lsResult('baz', 300),
]));
(bc as any).listShasByDate('input/dir').
then((shas: string[]) => expect(shas).toEqual(['foo', 'baz'])).
then(done);
});
});
}); });

View File

@ -1,11 +1,39 @@
// Imports // Imports
import {CreatedBuildEvent} from '../../lib/upload-server/build-events'; import {ChangedPrVisibilityEvent, CreatedBuildEvent} from '../../lib/upload-server/build-events';
// Tests // Tests
describe('ChangedPrVisibilityEvent', () => {
let evt: ChangedPrVisibilityEvent;
beforeEach(() => evt = new ChangedPrVisibilityEvent(42, ['foo', 'bar'], true));
it('should have a static \'type\' property', () => {
expect(ChangedPrVisibilityEvent.type).toBe('pr.changedVisibility');
});
it('should have a \'pr\' property', () => {
expect(evt.pr).toBe(42);
});
it('should have a \'shas\' property', () => {
expect(evt.shas).toEqual(['foo', 'bar']);
});
it('should have an \'isPublic\' property', () => {
expect(evt.isPublic).toBe(true);
});
});
describe('CreatedBuildEvent', () => { describe('CreatedBuildEvent', () => {
let evt: CreatedBuildEvent; let evt: CreatedBuildEvent;
beforeEach(() => evt = new CreatedBuildEvent(42, 'bar')); beforeEach(() => evt = new CreatedBuildEvent(42, 'bar', true));
it('should have a static \'type\' property', () => { it('should have a static \'type\' property', () => {
@ -22,4 +50,9 @@ describe('CreatedBuildEvent', () => {
expect(evt.sha).toBe('bar'); expect(evt.sha).toBe('bar');
}); });
it('should have an \'isPublic\' property', () => {
expect(evt.isPublic).toBe(true);
});
}); });

View File

@ -1,8 +1,8 @@
// Imports // Imports
import * as jwt from 'jsonwebtoken'; import * as jwt from 'jsonwebtoken';
import {GithubPullRequests} from '../../lib/common/github-pull-requests'; import {GithubPullRequests, PullRequest} from '../../lib/common/github-pull-requests';
import {GithubTeams} from '../../lib/common/github-teams'; import {GithubTeams} from '../../lib/common/github-teams';
import {BuildVerifier} from '../../lib/upload-server/build-verifier'; import {BUILD_VERIFICATION_STATUS, BuildVerifier} from '../../lib/upload-server/build-verifier';
import {expectToBeUploadError} from './helpers'; import {expectToBeUploadError} from './helpers';
// Tests // Tests
@ -13,6 +13,7 @@ describe('BuildVerifier', () => {
organization: 'organization', organization: 'organization',
repoSlug: 'repo/slug', repoSlug: 'repo/slug',
secret: 'secret', secret: 'secret',
trustedPrLabel: 'trusted: pr-label',
}; };
let bv: BuildVerifier; let bv: BuildVerifier;
@ -20,7 +21,7 @@ describe('BuildVerifier', () => {
const createBuildVerifier = (partialConfig: Partial<typeof defaultConfig> = {}) => { const createBuildVerifier = (partialConfig: Partial<typeof defaultConfig> = {}) => {
const cfg = {...defaultConfig, ...partialConfig} as typeof defaultConfig; const cfg = {...defaultConfig, ...partialConfig} as typeof defaultConfig;
return new BuildVerifier(cfg.secret, cfg.githubToken, cfg.repoSlug, cfg.organization, return new BuildVerifier(cfg.secret, cfg.githubToken, cfg.repoSlug, cfg.organization,
cfg.allowedTeamSlugs); cfg.allowedTeamSlugs, cfg.trustedPrLabel);
}; };
beforeEach(() => bv = createBuildVerifier()); beforeEach(() => bv = createBuildVerifier());
@ -28,7 +29,8 @@ describe('BuildVerifier', () => {
describe('constructor()', () => { describe('constructor()', () => {
['secret', 'githubToken', 'repoSlug', 'organization', 'allowedTeamSlugs'].forEach(param => { ['secret', 'githubToken', 'repoSlug', 'organization', 'allowedTeamSlugs', 'trustedPrLabel'].
forEach(param => {
it(`should throw if '${param}' is missing or empty`, () => { it(`should throw if '${param}' is missing or empty`, () => {
expect(() => createBuildVerifier({[param]: ''})). expect(() => createBuildVerifier({[param]: ''})).
toThrowError(`Missing or empty required parameter '${param}'!`); toThrowError(`Missing or empty required parameter '${param}'!`);
@ -44,6 +46,122 @@ describe('BuildVerifier', () => {
}); });
describe('getPrIsTrusted()', () => {
const pr = 9;
let mockPrInfo: PullRequest;
let prsFetchSpy: jasmine.Spy;
let teamsIsMemberBySlugSpy: jasmine.Spy;
beforeEach(() => {
mockPrInfo = {
labels: [
{name: 'foo'},
{name: 'bar'},
],
number: 9,
user: {login: 'username'},
};
prsFetchSpy = spyOn(GithubPullRequests.prototype, 'fetch').
and.returnValue(Promise.resolve(mockPrInfo));
teamsIsMemberBySlugSpy = spyOn(GithubTeams.prototype, 'isMemberBySlug').
and.returnValue(Promise.resolve(true));
});
it('should return a promise', done => {
const promise = bv.getPrIsTrusted(pr);
promise.then(done); // Do not complete the test (and release the spies) synchronously
// to avoid running the actual `GithubTeams#isMemberBySlug()`.
expect(promise).toEqual(jasmine.any(Promise));
});
it('should fetch the corresponding PR', done => {
bv.getPrIsTrusted(pr).then(() => {
expect(prsFetchSpy).toHaveBeenCalledWith(pr);
done();
});
});
it('should fail if fetching the PR errors', done => {
prsFetchSpy.and.callFake(() => Promise.reject('Test'));
bv.getPrIsTrusted(pr).catch(err => {
expect(err).toBe('Test');
done();
});
});
describe('when the PR has the "trusted PR" label', () => {
beforeEach(() => mockPrInfo.labels.push({name: 'trusted: pr-label'}));
it('should resolve to true', done => {
bv.getPrIsTrusted(pr).then(isTrusted => {
expect(isTrusted).toBe(true);
done();
});
});
it('should not try to verify the author\'s membership status', done => {
bv.getPrIsTrusted(pr).then(() => {
expect(teamsIsMemberBySlugSpy).not.toHaveBeenCalled();
done();
});
});
});
describe('when the PR does not have the "trusted PR" label', () => {
it('should verify the PR author\'s membership in the specified teams', done => {
bv.getPrIsTrusted(pr).then(() => {
expect(teamsIsMemberBySlugSpy).toHaveBeenCalledWith('username', ['team1', 'team2']);
done();
});
});
it('should fail if verifying membership errors', done => {
teamsIsMemberBySlugSpy.and.callFake(() => Promise.reject('Test'));
bv.getPrIsTrusted(pr).catch(err => {
expect(err).toBe('Test');
done();
});
});
it('should resolve to true if the PR\'s author is a member', done => {
teamsIsMemberBySlugSpy.and.returnValue(Promise.resolve(true));
bv.getPrIsTrusted(pr).then(isTrusted => {
expect(isTrusted).toBe(true);
done();
});
});
it('should resolve to false if the PR\'s author is not a member', done => {
teamsIsMemberBySlugSpy.and.returnValue(Promise.resolve(false));
bv.getPrIsTrusted(pr).then(isTrusted => {
expect(isTrusted).toBe(false);
done();
});
});
});
});
describe('verify()', () => { describe('verify()', () => {
const pr = 9; const pr = 9;
const defaultJwt = { const defaultJwt = {
@ -53,22 +171,21 @@ describe('BuildVerifier', () => {
'pull-request': pr, 'pull-request': pr,
'slug': defaultConfig.repoSlug, 'slug': defaultConfig.repoSlug,
}; };
let bvGetPrAuthorTeamMembership: jasmine.Spy; let bvGetPrIsTrusted: jasmine.Spy;
// Heleprs // Heleprs
const createAuthHeader = (partialJwt: Partial<typeof defaultJwt> = {}, secret: string = defaultConfig.secret) => const createAuthHeader = (partialJwt: Partial<typeof defaultJwt> = {}, secret: string = defaultConfig.secret) =>
`Token ${jwt.sign({...defaultJwt, ...partialJwt}, secret)}`; `Token ${jwt.sign({...defaultJwt, ...partialJwt}, secret)}`;
beforeEach(() => { beforeEach(() => {
bvGetPrAuthorTeamMembership = spyOn(bv, 'getPrAuthorTeamMembership'). bvGetPrIsTrusted = spyOn(bv, 'getPrIsTrusted').and.returnValue(Promise.resolve(true));
and.returnValue(Promise.resolve({author: 'some-author', isMember: true}));
}); });
it('should return a promise', done => { it('should return a promise', done => {
const promise = bv.verify(pr, createAuthHeader()); const promise = bv.verify(pr, createAuthHeader());
promise.then(done); // Do not complete the test (and release the spies) synchronously promise.then(done); // Do not complete the test (and release the spies) synchronously
// to avoid running the actual `bvGetPrAuthorTeamMembership()`. // to avoid running the actual `bvGetPrIsTrusted()`.
expect(promise).toEqual(jasmine.any(Promise)); expect(promise).toEqual(jasmine.any(Promise));
}); });
@ -148,16 +265,16 @@ describe('BuildVerifier', () => {
}); });
it('should call \'getPrAuthorTeamMembership()\' if the token is valid', done => { it('should call \'getPrIsTrusted()\' if the token is valid', done => {
bv.verify(pr, createAuthHeader()).then(() => { bv.verify(pr, createAuthHeader()).then(() => {
expect(bvGetPrAuthorTeamMembership).toHaveBeenCalledWith(pr); expect(bvGetPrIsTrusted).toHaveBeenCalledWith(pr);
done(); done();
}); });
}); });
it('should fail if \'getPrAuthorTeamMembership()\' rejects', done => { it('should fail if \'getPrIsTrusted()\' rejects', done => {
bvGetPrAuthorTeamMembership.and.callFake(() => Promise.reject('Test')); bvGetPrIsTrusted.and.callFake(() => Promise.reject('Test'));
bv.verify(pr, createAuthHeader()).catch(err => { bv.verify(pr, createAuthHeader()).catch(err => {
expectToBeUploadError(err, 403, `Error while verifying upload for PR ${pr}: Test`); expectToBeUploadError(err, 403, `Error while verifying upload for PR ${pr}: Test`);
done(); done();
@ -165,97 +282,22 @@ describe('BuildVerifier', () => {
}); });
it('should fail if \'getPrAuthorTeamMembership()\' reports no membership', done => { it('should resolve to `verifiedNotTrusted` if \'getPrIsTrusted()\' returns false', done => {
const errorMessage = `Error while verifying upload for PR ${pr}: User 'test' is not an active member of any of ` + bvGetPrIsTrusted.and.returnValue(Promise.resolve(false));
'the following teams: team1, team2'; bv.verify(pr, createAuthHeader()).then(value => {
expect(value).toBe(BUILD_VERIFICATION_STATUS.verifiedNotTrusted);
bvGetPrAuthorTeamMembership.and.returnValue(Promise.resolve({author: 'test', isMember: false}));
bv.verify(pr, createAuthHeader()).catch(err => {
expectToBeUploadError(err, 403, errorMessage);
done(); done();
}); });
}); });
it('should succeed if everything checks outs', done => { it('should resolve to `verifiedAndTrusted` if \'getPrIsTrusted()\' returns true', done => {
bv.verify(pr, createAuthHeader()).then(done); bv.verify(pr, createAuthHeader()).then(value => {
}); expect(value).toBe(BUILD_VERIFICATION_STATUS.verifiedAndTrusted);
});
describe('getPrAuthorTeamMembership()', () => {
const pr = 9;
let prsFetchSpy: jasmine.Spy;
let teamsIsMemberBySlugSpy: jasmine.Spy;
beforeEach(() => {
prsFetchSpy = spyOn(GithubPullRequests.prototype, 'fetch').
and.returnValue(Promise.resolve({user: {login: 'username'}}));
teamsIsMemberBySlugSpy = spyOn(GithubTeams.prototype, 'isMemberBySlug').
and.returnValue(Promise.resolve(true));
});
it('should return a promise', done => {
const promise = bv.getPrAuthorTeamMembership(pr);
promise.then(done); // Do not complete the test (and release the spies) synchronously
// to avoid running the actual `GithubTeams#isMemberBySlug()`.
expect(promise).toEqual(jasmine.any(Promise));
});
it('should fetch the corresponding PR', done => {
bv.getPrAuthorTeamMembership(pr).then(() => {
expect(prsFetchSpy).toHaveBeenCalledWith(pr);
done(); done();
}); });
}); });
it('should fail if fetching the PR errors', done => {
prsFetchSpy.and.callFake(() => Promise.reject('Test'));
bv.getPrAuthorTeamMembership(pr).catch(err => {
expect(err).toBe('Test');
done();
});
});
it('should verify the PR author\'s membership in the specified teams', done => {
bv.getPrAuthorTeamMembership(pr).then(() => {
expect(teamsIsMemberBySlugSpy).toHaveBeenCalledWith('username', ['team1', 'team2']);
done();
});
});
it('should fail if verifying membership errors', done => {
teamsIsMemberBySlugSpy.and.callFake(() => Promise.reject('Test'));
bv.getPrAuthorTeamMembership(pr).catch(err => {
expect(err).toBe('Test');
done();
});
});
it('should return the PR\'s author and whether they are members', done => {
teamsIsMemberBySlugSpy.and.returnValues(Promise.resolve(true), Promise.resolve(false));
Promise.all([
bv.getPrAuthorTeamMembership(pr).then(({author, isMember}) => {
expect(author).toBe('username');
expect(isMember).toBe(true);
}),
bv.getPrAuthorTeamMembership(pr).then(({author, isMember}) => {
expect(author).toBe('username');
expect(isMember).toBe(false);
}),
]).then(done);
});
}); });
}); });

View File

@ -4,8 +4,8 @@ import * as http from 'http';
import * as supertest from 'supertest'; import * as supertest from 'supertest';
import {GithubPullRequests} from '../../lib/common/github-pull-requests'; import {GithubPullRequests} from '../../lib/common/github-pull-requests';
import {BuildCreator} from '../../lib/upload-server/build-creator'; import {BuildCreator} from '../../lib/upload-server/build-creator';
import {CreatedBuildEvent} from '../../lib/upload-server/build-events'; import {ChangedPrVisibilityEvent, CreatedBuildEvent} from '../../lib/upload-server/build-events';
import {BuildVerifier} from '../../lib/upload-server/build-verifier'; import {BUILD_VERIFICATION_STATUS, BuildVerifier} from '../../lib/upload-server/build-verifier';
import {uploadServerFactory as usf} from '../../lib/upload-server/upload-server-factory'; import {uploadServerFactory as usf} from '../../lib/upload-server/upload-server-factory';
// Tests // Tests
@ -18,6 +18,7 @@ describe('uploadServerFactory', () => {
githubToken: '12345', githubToken: '12345',
repoSlug: 'repo/slug', repoSlug: 'repo/slug',
secret: 'secret', secret: 'secret',
trustedPrLabel: 'trusted: pr-label',
}; };
// Helpers // Helpers
@ -75,6 +76,12 @@ describe('uploadServerFactory', () => {
}); });
it('should throw if \'trustedPrLabel\' is missing or empty', () => {
expect(() => createUploadServer({trustedPrLabel: ''})).
toThrowError('Missing or empty required parameter \'trustedPrLabel\'!');
});
it('should return an http.Server', () => { it('should return an http.Server', () => {
const httpCreateServerSpy = spyOn(http, 'createServer').and.callThrough(); const httpCreateServerSpy = spyOn(http, 'createServer').and.callThrough();
const server = createUploadServer(); const server = createUploadServer();
@ -141,26 +148,71 @@ describe('uploadServerFactory', () => {
}); });
it('should post a comment on GitHub on \'build.created\'', () => { describe('on \'build.created\'', () => {
const prsAddCommentSpy = spyOn(GithubPullRequests.prototype, 'addComment'); let prsAddCommentSpy: jasmine.Spy;
const commentBody = 'The angular.io preview for 1234567890 is available [here][1].\n\n' +
'[1]: https://pr42-1234567890.domain.name/';
buildCreator.emit(CreatedBuildEvent.type, {pr: 42, sha: '1234567890'}); beforeEach(() => prsAddCommentSpy = spyOn(GithubPullRequests.prototype, 'addComment'));
it('should post a comment on GitHub for public previews', () => {
const commentBody = 'You can preview 1234567890 at https://pr42-1234567890.domain.name/.';
buildCreator.emit(CreatedBuildEvent.type, {pr: 42, sha: '1234567890', isPublic: true});
expect(prsAddCommentSpy).toHaveBeenCalledWith(42, commentBody); expect(prsAddCommentSpy).toHaveBeenCalledWith(42, commentBody);
}); });
it('should not post a comment on GitHub for non-public previews', () => {
buildCreator.emit(CreatedBuildEvent.type, {pr: 42, sha: '1234567890', isPublic: false});
expect(prsAddCommentSpy).not.toHaveBeenCalled();
});
});
describe('on \'pr.changedVisibility\'', () => {
let prsAddCommentSpy: jasmine.Spy;
beforeEach(() => prsAddCommentSpy = spyOn(GithubPullRequests.prototype, 'addComment'));
it('should post a comment on GitHub (for all SHAs) for PRs made public', () => {
const commentBody = 'You can preview 12345 at https://pr42-12345.domain.name/.\n' +
'You can preview 67890 at https://pr42-67890.domain.name/.';
buildCreator.emit(ChangedPrVisibilityEvent.type, {pr: 42, shas: ['12345', '67890'], isPublic: true});
expect(prsAddCommentSpy).toHaveBeenCalledWith(42, commentBody);
});
it('should not post a comment on GitHub if no SHAs were affected', () => {
buildCreator.emit(ChangedPrVisibilityEvent.type, {pr: 42, shas: [], isPublic: true});
expect(prsAddCommentSpy).not.toHaveBeenCalled();
});
it('should not post a comment on GitHub for PRs made non-public', () => {
buildCreator.emit(ChangedPrVisibilityEvent.type, {pr: 42, shas: ['12345', '67890'], isPublic: false});
expect(prsAddCommentSpy).not.toHaveBeenCalled();
});
});
it('should pass the correct \'githubToken\' and \'repoSlug\' to GithubPullRequests', () => { it('should pass the correct \'githubToken\' and \'repoSlug\' to GithubPullRequests', () => {
const prsAddCommentSpy = spyOn(GithubPullRequests.prototype, 'addComment'); const prsAddCommentSpy = spyOn(GithubPullRequests.prototype, 'addComment');
buildCreator.emit(CreatedBuildEvent.type, {pr: 42, sha: '1234567890'}); buildCreator.emit(CreatedBuildEvent.type, {pr: 42, sha: '1234567890', isPublic: true});
const prs = prsAddCommentSpy.calls.mostRecent().object; buildCreator.emit(ChangedPrVisibilityEvent.type, {pr: 42, shas: ['12345', '67890'], isPublic: true});
const allCalls = prsAddCommentSpy.calls.all();
const prs = allCalls[0].object;
expect(prsAddCommentSpy).toHaveBeenCalledTimes(2);
expect(prs).toBe(allCalls[1].object);
expect(prs).toEqual(jasmine.any(GithubPullRequests)); expect(prs).toEqual(jasmine.any(GithubPullRequests));
expect((prs as any).repoSlug).toBe('repo/slug'); expect(prs.repoSlug).toBe('repo/slug');
expect((prs as any).requestHeaders.Authorization).toContain('12345'); expect(prs.requestHeaders.Authorization).toContain('12345');
}); });
}); });
@ -184,6 +236,7 @@ describe('uploadServerFactory', () => {
defaultConfig.repoSlug, defaultConfig.repoSlug,
defaultConfig.githubOrganization, defaultConfig.githubOrganization,
defaultConfig.githubTeamSlugs, defaultConfig.githubTeamSlugs,
defaultConfig.trustedPrLabel,
); );
buildCreator = new BuildCreator(defaultConfig.buildsDir); buildCreator = new BuildCreator(defaultConfig.buildsDir);
agent = supertest.agent((usf as any).createMiddleware(buildVerifier, buildCreator)); agent = supertest.agent((usf as any).createMiddleware(buildVerifier, buildCreator));
@ -199,7 +252,8 @@ describe('uploadServerFactory', () => {
let buildCreatorCreateSpy: jasmine.Spy; let buildCreatorCreateSpy: jasmine.Spy;
beforeEach(() => { beforeEach(() => {
buildVerifierVerifySpy = spyOn(buildVerifier, 'verify').and.returnValue(Promise.resolve()); const verStatus = BUILD_VERIFICATION_STATUS.verifiedAndTrusted;
buildVerifierVerifySpy = spyOn(buildVerifier, 'verify').and.returnValue(Promise.resolve(verStatus));
buildCreatorCreateSpy = spyOn(buildCreator, 'create').and.returnValue(Promise.resolve()); buildCreatorCreateSpy = spyOn(buildCreator, 'create').and.returnValue(Promise.resolve());
}); });
@ -284,14 +338,17 @@ describe('uploadServerFactory', () => {
it('should call \'BuildCreator#create()\' with the correct arguments', done => { it('should call \'BuildCreator#create()\' with the correct arguments', done => {
const req = agent. buildVerifierVerifySpy.and.returnValues(
get(`/create-build/${pr}/${sha}`). Promise.resolve(BUILD_VERIFICATION_STATUS.verifiedAndTrusted),
set('AUTHORIZATION', 'foo'). Promise.resolve(BUILD_VERIFICATION_STATUS.verifiedNotTrusted));
set('X-FILE', 'bar');
promisifyRequest(req). const req1 = agent.get(`/create-build/${pr}/${sha}`).set('AUTHORIZATION', 'foo').set('X-FILE', 'bar');
then(() => expect(buildCreatorCreateSpy).toHaveBeenCalledWith(pr, sha, 'bar')). const req2 = agent.get(`/create-build/${pr}/${sha}`).set('AUTHORIZATION', 'foo').set('X-FILE', 'bar');
then(done, done.fail);
Promise.all([
promisifyRequest(req1).then(() => expect(buildCreatorCreateSpy).toHaveBeenCalledWith(pr, sha, 'bar', true)),
promisifyRequest(req2).then(() => expect(buildCreatorCreateSpy).toHaveBeenCalledWith(pr, sha, 'bar', false)),
]).then(done, done.fail);
}); });
@ -307,7 +364,7 @@ describe('uploadServerFactory', () => {
}); });
it('should respond with 201 on successful upload', done => { it('should respond with 201 on successful upload (for public builds)', done => {
const req = agent. const req = agent.
get(`/create-build/${pr}/${sha}`). get(`/create-build/${pr}/${sha}`).
set('AUTHORIZATION', 'foo'). set('AUTHORIZATION', 'foo').
@ -318,6 +375,18 @@ describe('uploadServerFactory', () => {
}); });
it('should respond with 202 on successful upload (for hidden builds)', done => {
buildVerifierVerifySpy.and.returnValue(Promise.resolve(BUILD_VERIFICATION_STATUS.verifiedNotTrusted));
const req = agent.
get(`/create-build/${pr}/${sha}`).
set('AUTHORIZATION', 'foo').
set('X-FILE', 'bar').
expect(202, http.STATUS_CODES[202]);
verifyRequests([req], done);
});
it('should reject PRs with leading zeros', done => { it('should reject PRs with leading zeros', done => {
verifyRequests([agent.get(`/create-build/0${pr}/${sha}`).expect(404)], done); verifyRequests([agent.get(`/create-build/0${pr}/${sha}`).expect(404)], done);
}); });

View File

@ -8,6 +8,7 @@ export AIO_GITHUB_ORGANIZATION=$TEST_AIO_GITHUB_ORGANIZATION
export AIO_GITHUB_TEAM_SLUGS=$TEST_AIO_GITHUB_TEAM_SLUGS export AIO_GITHUB_TEAM_SLUGS=$TEST_AIO_GITHUB_TEAM_SLUGS
export AIO_PREVIEW_DEPLOYMENT_TOKEN=$TEST_AIO_PREVIEW_DEPLOYMENT_TOKEN export AIO_PREVIEW_DEPLOYMENT_TOKEN=$TEST_AIO_PREVIEW_DEPLOYMENT_TOKEN
export AIO_REPO_SLUG=$TEST_AIO_REPO_SLUG export AIO_REPO_SLUG=$TEST_AIO_REPO_SLUG
export AIO_TRUSTED_PR_LABEL=$TEST_AIO_TRUSTED_PR_LABEL
export AIO_UPLOAD_HOSTNAME=$TEST_AIO_UPLOAD_HOSTNAME export AIO_UPLOAD_HOSTNAME=$TEST_AIO_UPLOAD_HOSTNAME
export AIO_UPLOAD_PORT=$TEST_AIO_UPLOAD_PORT export AIO_UPLOAD_PORT=$TEST_AIO_UPLOAD_PORT

View File

@ -17,7 +17,7 @@ you don't need to specify values for those.
The domain name of the server. The domain name of the server.
- `AIO_GITHUB_ORGANIZATION`: - `AIO_GITHUB_ORGANIZATION`:
The GitHub organization whose teams arew whitelisted for accepting uploads. The GitHub organization whose teams are whitelisted for accepting uploads.
See also `AIO_GITHUB_TEAM_SLUGS`. See also `AIO_GITHUB_TEAM_SLUGS`.
- `AIO_GITHUB_TEAM_SLUGS`: - `AIO_GITHUB_TEAM_SLUGS`:
@ -39,6 +39,11 @@ you don't need to specify values for those.
- `AIO_REPO_SLUG`: - `AIO_REPO_SLUG`:
The repository slug (in the form `<user>/<repo>`) for which PRs will be uploaded. The repository slug (in the form `<user>/<repo>`) for which PRs will be uploaded.
- `AIO_TRUSTED_PR_LABEL`:
The PR whose presence indicates the PR has been manually verified and is allowed to have its
build artifacts publicly served. This is useful for enabling previews for any PR (not only those
from trusted authors).
- `AIO_UPLOAD_HOSTNAME`: - `AIO_UPLOAD_HOSTNAME`:
The internal hostname for accessing the Node.js upload-server. This is used by nginx for The internal hostname for accessing the Node.js upload-server. This is used by nginx for
delegating upload requests and also for performing a periodic health-check. delegating upload requests and also for performing a periodic health-check.

View File

@ -33,36 +33,46 @@ container:
### On CI (Travis) ### On CI (Travis)
- Build job completes successfully (i.e. build succeeds and tests pass). - Build job completes successfully.
- The CI script checks whether the build job was initiated by a PR against the angular/angular - The CI script checks whether the build job was initiated by a PR against the angular/angular
master branch. master branch.
- The CI script checks whether the PR has touched any files inside the angular.io project directory - The CI script checks whether the PR has touched any files that might affect the angular.io app
(currently `aio/`). (currently the `aio/` or `packages/` directories, ignoring spec files).
- The CI script checks whether the author of the PR is a member of one of the whitelisted GitHub - Optionally, the CI script can check whether the PR can be automatically verified (i.e. if the
teams (and therefore allowed to upload). author of the PR is a member of one of the whitelisted GitHub teams or the PR has the specified
"trusted PR" label).
**Note:** **Note:**
For security reasons, the same checks will be performed on the server as well. This is an optional For security reasons, the same checks will be performed on the server as well. This is an optional
step with the purpose of: step that can be used in case one wants to apply special logic depending on the outcome of the
1. Avoiding the wasted overhead associated with uploads that are going to be rejected (e.g. pre-verification. For example:
building the artifacts, sending them to the server, running checks on the server, etc). 1. One might want to deploy automatically verified PRs only. In that case, the pre-verification
2. Avoiding failing the build (due to an error response from the server) or requiring additional helps avoid the wasted overhead associated with uploads that are going to be rejected (e.g.
logic for detecting the reasons of the failure. building the artifacts, sending them to the server, running checks on the server, detecting the
- The CI script gzip and upload the build artifacts to the server. reasons of deployment failure and whether to fail the build, etc).
2. One might want to apply additional logic (e.g. different tests) depending on whether the PR is
automatically verified or not).
- The CI script gzips and uploads the build artifacts to the server.
More info on how to set things up on CI can be found [here](misc--integrate-with-ci.md). More info on how to set things up on CI can be found [here](misc--integrate-with-ci.md).
### Uploading build artifacts ### Uploading build artifacts
- nginx receives upload request. - nginx receives the upload request.
- nginx checks that the uploaded gzip archive does not exceed the specified max file size, stores it - nginx checks that the uploaded gzip archive does not exceed the specified max file size, stores it
in a temporary location and passes the filepath to the Node.js upload-server. in a temporary location and passes the filepath to the Node.js upload-server.
- The upload-server verifies that the uploaded file is not trying to overwrite an existing build, - The upload-server runs several checks to determine whether the request should be accepted and
and runs several checks to determine whether the request should be accepted (more details can be whether it should be publicly accessible or stored for later verification (more details can be
found [here](overview--security-model.md)). found [here](overview--security-model.md)).
- The upload-server changes the "visibility" of the associated PR, if necessary. For example, if
builds for the same PR had been previously deployed as non-public and the current build has been
automatically verified, all previous builds are made public.
If the PR transitions from "non-public" to "public", the upload-server posts a comment on the
corresponding PR on GitHub mentioning the SHAs and the links where the previews can be found.
- The upload-server verifies that the uploaded file is not trying to overwrite an existing build.
- The upload-server deploys the artifacts to a sub-directory named after the PR number and SHA: - The upload-server deploys the artifacts to a sub-directory named after the PR number and SHA:
`<PR>/<SHA>/` `<PR>/<SHA>/` (Non-publicly accessible PRs will be stored in a different location.)
- The upload-server posts a comment on the corresponding PR on GitHub mentioning the SHA and the - If the PR is publicly accessible, the upload-server posts a comment on the corresponding PR on
the link where the preview can be found. GitHub mentioning the SHA and the link where the preview can be found.
### Serving build artifacts ### Serving build artifacts

View File

@ -17,10 +17,11 @@ available:
useful for CI integration. See [here](misc--integrate-with-ci.md) for more info. useful for CI integration. See [here](misc--integrate-with-ci.md) for more info.
- `travis-preverify-pr.sh` - `travis-preverify-pr.sh`
Can be used for "preverifying" a PR before uploading the artifacts to the server. It checks that Can be used for "pre-verifying" a PR before uploading the artifacts to the server. It checks
the author of the PR is a member of one of the specified GitHub teams and therefore allowed to whether the author of the PR is a member of one of the specified GitHub teams (therefore allowed
upload build artifacts. This is useful for CI integration. See [here](misc--integrate-with-ci.md) to upload build artifacts) or the PR has the specified "trusted PR" label (meaning it has been
for more info. manually verified by a trusted member). This is useful for CI integration.
See [here](misc--integrate-with-ci.md) for more info.
- `update-preview-server.sh` - `update-preview-server.sh`
Can be used for updating the docker container (and image) based on the latest changes checked out Can be used for updating the docker container (and image) based on the latest changes checked out

View File

@ -41,12 +41,13 @@ part of the CI setup and serving them publicly.
The implemented approach can be broken up to the following sub-tasks: The implemented approach can be broken up to the following sub-tasks:
1. Verify which PR the uploaded artifacts correspond to. 1. Verify which PR the uploaded artifacts correspond to.
2. Determine the author of the PR. 2. Fetch the PR's metadata, including author and labels.
3. Check whether the PR author is a member of some whitelisted GitHub team. 3. Check whether the PR can be automatically verified as "trusted" (based on its author or labels).
4. Deploy the artifacts to the corresponding PR's directory. 4. If necessary, update the corresponding PR's verification status.
5. Prevent overwriting previously deployed artifacts (which ensures that the guarantees established 5. Deploy the artifacts to the corresponding PR's directory.
6. Prevent overwriting previously deployed artifacts (which ensures that the guarantees established
during deployment will remain valid until the artifacts are removed). during deployment will remain valid until the artifacts are removed).
6. Prevent uploaded files from accessing anything outside their directory. 7. Prevent uploaded files from accessing anything outside their directory.
### Implementation details ### Implementation details
@ -65,35 +66,51 @@ This section describes how each of the aforementioned sub-tasks is accomplished:
_There are currently certain limitation in the implementation of the JWT addon._ _There are currently certain limitation in the implementation of the JWT addon._
_See the next section for more details._ _See the next section for more details._
2. **Determine the author of the PR.** 2. **Fetch the PR's metadata, including author and labels**.
Once we have securely associated the uploaded artifaacts to a PR, we retrieve the PR's metadata - Once we have securely associated the uploaded artifacts to a PR, we retrieve the PR's metadata -
including the author's username - using the [GitHub API](https://developer.github.com/v3/). including the author's username and the labels - using the
[GitHub API](https://developer.github.com/v3/).
To avoid rate-limit restrictions, we use a Personal Access Token (issued by To avoid rate-limit restrictions, we use a Personal Access Token (issued by
[@mary-poppins](https://github.com/mary-poppins)). [@mary-poppins](https://github.com/mary-poppins)).
3. **Check whether the PR author is a member of some whitelisted GitHub team.** 3. **Check whether the PR can be automatically verified as "trusted"**.
Again using the GitHub API, we can verify the author's membership in one of the "Trusted" means that we are confident that the build artifacts are suitable for being deployed
whitelisted/trusted GitHub teams. For this operation, we need a PErsonal Access Token with the and publicly accessible on the preview server. There are two ways to check that:
1. We can verify that the PR has a pre-determined label, which marks it as "safe for preview".
Such a label can only have been added by a maintainer (with the necessary rights) and
designates that they have manually verified the PR contents.
2. We can verify (again using the GitHub API) the author's membership in one of the
whitelisted/trusted GitHub teams. For this operation, we need a Personal Access Token with the
`read:org` scope issued by a user that can "see" the specified GitHub organization. `read:org` scope issued by a user that can "see" the specified GitHub organization.
Here too, we use token by @mary-poppins. Here too, we use the token by @mary-poppins.
4. **Deploy the artifacts to the corresponding PR's directory.** 4. **If necessary update the corresponding PR's verification status**.
Once we have determined whether the PR is considered "trusted", we update its "visibility" (i.e.
whether it is publicly accessible or not), based on the new verification status. For example, if
a PR was initially considered "not trusted" but the check triggered by a new build determined
otherwise, the PR (and all the previously uploaded previews) are made public. It works the same
way if a PR has gone from "trusted" to "not trusted".
5. **Deploy the artifacts to the corresponding PR's directory.**
With the preceeding steps, we have verified that the uploaded artifacts have been uploaded by With the preceeding steps, we have verified that the uploaded artifacts have been uploaded by
Travis and correspond to a PR whose author is a member of a trusted team. Essentially, as long as Travis. Additionally, we have determined whether the PR can be trusted to have its previews
sub-tasks 1, 2 and 3 can be securely accomplished, it is possible to "project" the trust we have publicly accessible or whether further verification is necessary. The artifacts will be stored to
in a team's members through the PR and Travis to the build artifacts. the PR's directory, but will not be publicly accessible unless the PR has been verified.
Essentially, as long as sub-tasks 1, 2 and 3 can be securely accomplished, it is possible to
"project" the trust we have in a team's members through the PR and Travis to the build artifacts.
5. **Prevent overwriting previously deployed artifacts**. 6. **Prevent overwriting previously deployed artifacts**.
In order to enforce this restriction (and ensure that the deployed artifacts validity is In order to enforce this restriction (and ensure that the deployed artifacts' validity is
preserved throughout their "lifetime"), the server that handles the upload (currently a Node.js preserved throughout their "lifetime"), the server that handles the upload (currently a Node.js
Express server) rejects uploads that target an existing directory. Express server) rejects uploads that target an existing directory.
_Note: A PR can contain multiple uploads; one for each SHA that was built on Travis._ _Note: A PR can contain multiple uploads; one for each SHA that was built on Travis._
6. **Prevent uploaded files from accessing anything outside their directory.** 7. **Prevent uploaded files from accessing anything outside their directory.**
Nginx (which is used to serve the uploaded artifacts) has been configured to not follow symlinks Nginx (which is used to serve the uploaded artifacts) has been configured to not follow symlinks
outside of the directory where the build artifacts are stored. outside of the directory where the build artifacts are stored.
@ -104,6 +121,11 @@ This section describes how each of the aforementioned sub-tasks is accomplished:
- Each trusted PR author has full control over the content that is uploaded for their PRs. Part of - Each trusted PR author has full control over the content that is uploaded for their PRs. Part of
the security model relies on the trustworthiness of these authors. the security model relies on the trustworthiness of these authors.
- Adding the specified label on a PR and marking it as trusted, gives the author full control over
the content that is uploaded for the specific PR (e.g. by pushing more commits to it). The user
adding the label is responsible for ensuring that this control is not abused and that the PR is
either closed (one way of another) or the access is revoked.
- If anyone gets access to the `PREVIEW_DEPLOYMENT_TOKEN` (a.k.a. `NGBUILDS_IO_KEY` on - If anyone gets access to the `PREVIEW_DEPLOYMENT_TOKEN` (a.k.a. `NGBUILDS_IO_KEY` on
angular/angular) variable generated for each Travis job, they will be able to impersonate the angular/angular) variable generated for each Travis job, they will be able to impersonate the
corresponding PR's author on the preview server for as long as the token is valid (currently 90 corresponding PR's author on the preview server for as long as the token is valid (currently 90

View File

@ -25,7 +25,7 @@ The following commands would create a docker image from GitHub repo `foo/bar` to
--build-arg AIO_REPO_SLUG=foo/bar \ --build-arg AIO_REPO_SLUG=foo/bar \
--build-arg AIO_DOMAIN_NAME=foobar-builds.io \ --build-arg AIO_DOMAIN_NAME=foobar-builds.io \
--build-arg AIO_GITHUB_ORGANIZATION=foo \ --build-arg AIO_GITHUB_ORGANIZATION=foo \
--build-arg AIO_GITHUB_TEMA_SLUGS=bar-core,bar-docs-authors --build-arg AIO_GITHUB_TEAM_SLUGS=bar-core,bar-docs-authors
``` ```
A full list of the available environment variables can be found A full list of the available environment variables can be found

View File

@ -16,5 +16,11 @@ AIO_GITHUB_ORGANIZATION="angular" \
AIO_GITHUB_TEAM_SLUGS="angular-core,aio-contributors" \ AIO_GITHUB_TEAM_SLUGS="angular-core,aio-contributors" \
AIO_GITHUB_TOKEN=$(echo ${GITHUB_TEAM_MEMBERSHIP_CHECK_KEY} | rev) \ AIO_GITHUB_TOKEN=$(echo ${GITHUB_TEAM_MEMBERSHIP_CHECK_KEY} | rev) \
AIO_REPO_SLUG=$TRAVIS_REPO_SLUG \ AIO_REPO_SLUG=$TRAVIS_REPO_SLUG \
AIO_TRUSTED_PR_LABEL="aio: preview" \
AIO_PREVERIFY_PR=$TRAVIS_PULL_REQUEST \ AIO_PREVERIFY_PR=$TRAVIS_PULL_REQUEST \
node "$SCRIPTS_JS_DIR/dist/lib/upload-server/index-preverify-pr" node "$SCRIPTS_JS_DIR/dist/lib/upload-server/index-preverify-pr"
# Exit codes:
# - 0: The PR can be automatically trusted (i.e. author belongs to trusted team or PR has the "trusted PR" label).
# - 1: An error occurred.
# - 2: The PR cannot be automatically trusted.