feat(aio): use shorter URLs for previews

Use the 7 first characters of the 40-chars long SHAs for shorter/cleaner URLs.
The collision probability is extremely low (since all SHAs are further
"namespaced" under the corresponding PR). In case of a collision, the second PR
will not be deployed, in order to avoid overwriting the original build.

(This is a design decision to keep the implementation simple. It can be changed
later if necessary.)
This commit is contained in:
Georgios Kalpakas 2017-06-25 22:13:03 +03:00 committed by Matias Niemelä
parent 3c4eef99be
commit 4268c82898
9 changed files with 161 additions and 50 deletions

View File

@ -15,7 +15,7 @@ server {
# Serve PR-preview requests # Serve PR-preview requests
server { server {
server_name "~^pr(?<pr>[1-9][0-9]*)-(?<sha>[0-9a-f]{40})\."; server_name "~^pr(?<pr>[1-9][0-9]*)-(?<sha>[0-9a-f]{7,40})\.";
listen {{$AIO_NGINX_PORT_HTTPS}} ssl http2; listen {{$AIO_NGINX_PORT_HTTPS}} ssl http2;
listen [::]:{{$AIO_NGINX_PORT_HTTPS}} ssl http2; listen [::]:{{$AIO_NGINX_PORT_HTTPS}} ssl http2;

View File

@ -1,2 +1,3 @@
// Constants // Constants
export const HIDDEN_DIR_PREFIX = 'hidden--'; export const HIDDEN_DIR_PREFIX = 'hidden--';
export const SHORT_SHA_LEN = 7;

View File

@ -4,7 +4,7 @@ import {EventEmitter} from 'events';
import * as fs from 'fs'; 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 {HIDDEN_DIR_PREFIX} from '../common/constants'; import {HIDDEN_DIR_PREFIX, SHORT_SHA_LEN} from '../common/constants';
import {assertNotMissingOrEmpty} from '../common/utils'; import {assertNotMissingOrEmpty} from '../common/utils';
import {ChangedPrVisibilityEvent, CreatedBuildEvent} from './build-events'; import {ChangedPrVisibilityEvent, CreatedBuildEvent} from './build-events';
import {UploadError} from './upload-error'; import {UploadError} from './upload-error';
@ -46,6 +46,9 @@ export class BuildCreator extends EventEmitter {
} }
public create(pr: string, sha: string, archivePath: string, isPublic: boolean): Promise<void> { public create(pr: string, sha: string, archivePath: string, isPublic: boolean): Promise<void> {
// Use only part of the SHA for more readable URLs.
sha = sha.substr(0, SHORT_SHA_LEN);
const {oldPrDir: otherVisPrDir, newPrDir: prDir} = this.getCandidatePrDirs(pr, isPublic); 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;

View File

@ -4,7 +4,7 @@ import * as fs from 'fs';
import * as http from 'http'; import * as http from 'http';
import * as path from 'path'; import * as path from 'path';
import * as shell from 'shelljs'; import * as shell from 'shelljs';
import {HIDDEN_DIR_PREFIX} from '../common/constants'; import {HIDDEN_DIR_PREFIX, SHORT_SHA_LEN} from '../common/constants';
import {getEnvVar} from '../common/utils'; import {getEnvVar} from '../common/utils';
// Constans // Constans
@ -51,8 +51,9 @@ class Helper {
} }
// Methods - Public // Methods - Public
public buildExists(pr: string, sha = '', isPublic = true): boolean { public buildExists(pr: string, sha = '', isPublic = true, legacy = false): boolean {
const dir = path.join(this.getPrDir(pr, isPublic), sha); const prDir = this.getPrDir(pr, isPublic);
const dir = !sha ? prDir : this.getShaDir(prDir, sha, legacy);
return fs.existsSync(dir); return fs.existsSync(dir);
} }
@ -68,7 +69,7 @@ class Helper {
} }
public createDummyArchive(pr: string, sha: string, archivePath: string): CleanUpFn { public createDummyArchive(pr: string, sha: string, archivePath: string): CleanUpFn {
const inputDir = path.join(this.buildsDir, 'uploaded', pr, sha); const inputDir = this.getShaDir(this.getPrDir(`uploaded/${pr}`, true), sha);
const cmd1 = `tar --create --gzip --directory "${inputDir}" --file "${archivePath}" .`; const cmd1 = `tar --create --gzip --directory "${inputDir}" --file "${archivePath}" .`;
const cmd2 = `chown ${this.wwwUser} ${archivePath}`; const cmd2 = `chown ${this.wwwUser} ${archivePath}`;
@ -80,9 +81,9 @@ class Helper {
return this.createCleanUpFn(() => shell.rm('-rf', archivePath)); return this.createCleanUpFn(() => shell.rm('-rf', archivePath));
} }
public createDummyBuild(pr: string, sha: string, isPublic = true, force = false): CleanUpFn { public createDummyBuild(pr: string, sha: string, isPublic = true, force = false, legacy = false): CleanUpFn {
const prDir = this.getPrDir(pr, isPublic); const prDir = this.getPrDir(pr, isPublic);
const shaDir = path.join(prDir, sha); const shaDir = this.getShaDir(prDir, sha, legacy);
const idxPath = path.join(shaDir, 'index.html'); const idxPath = path.join(shaDir, 'index.html');
const barPath = path.join(shaDir, 'foo', 'bar.js'); const barPath = path.join(shaDir, 'foo', 'bar.js');
@ -108,9 +109,17 @@ class Helper {
return path.join(this.buildsDir, prDirName); return path.join(this.buildsDir, prDirName);
} }
public readBuildFile(pr: string, sha: string, relFilePath: string, isPublic = true): string { public getShaDir(prDir: string, sha: string, legacy = false): string {
const prDir = this.getPrDir(pr, isPublic); return path.join(prDir, legacy ? sha : this.getShordSha(sha));
const absFilePath = path.join(prDir, sha, relFilePath); }
public getShordSha(sha: string): string {
return sha.substr(0, SHORT_SHA_LEN);
}
public readBuildFile(pr: string, sha: string, relFilePath: string, isPublic = true, legacy = false): string {
const shaDir = this.getShaDir(this.getPrDir(pr, isPublic), sha, legacy);
const absFilePath = path.join(shaDir, relFilePath);
return fs.readFileSync(absFilePath, 'utf8'); return fs.readFileSync(absFilePath, 'utf8');
} }
@ -156,8 +165,10 @@ class Helper {
}; };
} }
public writeBuildFile(pr: string, sha: string, relFilePath: string, content: string, isPublic = true): CleanUpFn { public writeBuildFile(pr: string, sha: string, relFilePath: string, content: string, isPublic = true,
const absFilePath = path.join(this.getPrDir(pr, isPublic), sha, relFilePath); legacy = false): CleanUpFn {
const shaDir = this.getShaDir(this.getPrDir(pr, isPublic), sha, legacy);
const absFilePath = path.join(shaDir, relFilePath);
return this.writeFile(absFilePath, {content}, true); return this.writeFile(absFilePath, {content}, true);
} }

View File

@ -37,6 +37,8 @@ describe(`nginx`, () => {
const pr = '9'; const pr = '9';
const sha9 = '9'.repeat(40); const sha9 = '9'.repeat(40);
const sha0 = '0'.repeat(40); const sha0 = '0'.repeat(40);
const shortSha9 = h.getShordSha(sha9);
const shortSha0 = h.getShordSha(sha0);
describe(`pr<pr>-<sha>.${host}/*`, () => { describe(`pr<pr>-<sha>.${host}/*`, () => {
@ -50,9 +52,23 @@ describe(`nginx`, () => {
it('should return /index.html', done => { it('should return /index.html', done => {
const origin = `${scheme}://pr${pr}-${shortSha9}.${host}`;
const bodyRegex = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /index\\.html$`);
Promise.all([
h.runCmd(`curl -iL ${origin}/index.html`).then(h.verifyResponse(200, bodyRegex)),
h.runCmd(`curl -iL ${origin}/`).then(h.verifyResponse(200, bodyRegex)),
h.runCmd(`curl -iL ${origin}`).then(h.verifyResponse(200, bodyRegex)),
]).then(done);
});
it('should return /index.html (for legacy builds)', done => {
const origin = `${scheme}://pr${pr}-${sha9}.${host}`; const origin = `${scheme}://pr${pr}-${sha9}.${host}`;
const bodyRegex = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /index\\.html$`); const bodyRegex = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /index\\.html$`);
h.createDummyBuild(pr, sha9, true, false, true);
Promise.all([ Promise.all([
h.runCmd(`curl -iL ${origin}/index.html`).then(h.verifyResponse(200, bodyRegex)), h.runCmd(`curl -iL ${origin}/index.html`).then(h.verifyResponse(200, bodyRegex)),
h.runCmd(`curl -iL ${origin}/`).then(h.verifyResponse(200, bodyRegex)), h.runCmd(`curl -iL ${origin}/`).then(h.verifyResponse(200, bodyRegex)),
@ -64,7 +80,19 @@ describe(`nginx`, () => {
it('should return /foo/bar.js', done => { it('should return /foo/bar.js', done => {
const bodyRegex = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /foo/bar\\.js$`); const bodyRegex = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /foo/bar\\.js$`);
h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha9}.${host}/foo/bar.js`). h.runCmd(`curl -iL ${scheme}://pr${pr}-${shortSha9}.${host}/foo/bar.js`).
then(h.verifyResponse(200, bodyRegex)).
then(done);
});
it('should return /foo/bar.js (for legacy builds)', done => {
const origin = `${scheme}://pr${pr}-${sha9}.${host}`;
const bodyRegex = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /foo/bar\\.js$`);
h.createDummyBuild(pr, sha9, true, false, true);
h.runCmd(`curl -iL ${origin}/foo/bar.js`).
then(h.verifyResponse(200, bodyRegex)). then(h.verifyResponse(200, bodyRegex)).
then(done); then(done);
}); });
@ -72,58 +100,59 @@ describe(`nginx`, () => {
it('should respond with 403 for directories', done => { it('should respond with 403 for directories', done => {
Promise.all([ Promise.all([
h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha9}.${host}/foo/`).then(h.verifyResponse(403)), h.runCmd(`curl -iL ${scheme}://pr${pr}-${shortSha9}.${host}/foo/`).then(h.verifyResponse(403)),
h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha9}.${host}/foo`).then(h.verifyResponse(403)), h.runCmd(`curl -iL ${scheme}://pr${pr}-${shortSha9}.${host}/foo`).then(h.verifyResponse(403)),
]).then(done); ]).then(done);
}); });
it('should respond with 404 for unknown paths to files', done => { it('should respond with 404 for unknown paths to files', done => {
h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha9}.${host}/foo/baz.css`). h.runCmd(`curl -iL ${scheme}://pr${pr}-${shortSha9}.${host}/foo/baz.css`).
then(h.verifyResponse(404)). then(h.verifyResponse(404)).
then(done); then(done);
}); });
it('should rewrite to \'index.html\' for unknown paths that don\'t look like files', done => { it('should rewrite to \'index.html\' for unknown paths that don\'t look like files', done => {
const origin = `${scheme}://pr${pr}-${shortSha9}.${host}`;
const bodyRegex = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /index\\.html$`); const bodyRegex = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /index\\.html$`);
Promise.all([ Promise.all([
h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha9}.${host}/foo/baz`).then(h.verifyResponse(200, bodyRegex)), h.runCmd(`curl -iL ${origin}/foo/baz`).then(h.verifyResponse(200, bodyRegex)),
h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha9}.${host}/foo/baz/`).then(h.verifyResponse(200, bodyRegex)), h.runCmd(`curl -iL ${origin}/foo/baz/`).then(h.verifyResponse(200, bodyRegex)),
]).then(done); ]).then(done);
}); });
it('should respond with 404 for unknown PRs/SHAs', done => { it('should respond with 404 for unknown PRs/SHAs', done => {
const otherPr = 54321; const otherPr = 54321;
const otherSha = '8'.repeat(40); const otherShortSha = h.getShordSha('8'.repeat(40));
Promise.all([ Promise.all([
h.runCmd(`curl -iL ${scheme}://pr${pr}9-${sha9}.${host}`).then(h.verifyResponse(404)), h.runCmd(`curl -iL ${scheme}://pr${pr}9-${shortSha9}.${host}`).then(h.verifyResponse(404)),
h.runCmd(`curl -iL ${scheme}://pr${otherPr}-${sha9}.${host}`).then(h.verifyResponse(404)), h.runCmd(`curl -iL ${scheme}://pr${otherPr}-${shortSha9}.${host}`).then(h.verifyResponse(404)),
h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha9}9.${host}`).then(h.verifyResponse(404)), h.runCmd(`curl -iL ${scheme}://pr${pr}-${shortSha9}9.${host}`).then(h.verifyResponse(404)),
h.runCmd(`curl -iL ${scheme}://pr${pr}-${otherSha}.${host}`).then(h.verifyResponse(404)), h.runCmd(`curl -iL ${scheme}://pr${pr}-${otherShortSha}.${host}`).then(h.verifyResponse(404)),
]).then(done); ]).then(done);
}); });
it('should respond with 404 if the subdomain format is wrong', done => { it('should respond with 404 if the subdomain format is wrong', done => {
Promise.all([ Promise.all([
h.runCmd(`curl -iL ${scheme}://xpr${pr}-${sha9}.${host}`).then(h.verifyResponse(404)), h.runCmd(`curl -iL ${scheme}://xpr${pr}-${shortSha9}.${host}`).then(h.verifyResponse(404)),
h.runCmd(`curl -iL ${scheme}://prx${pr}-${sha9}.${host}`).then(h.verifyResponse(404)), h.runCmd(`curl -iL ${scheme}://prx${pr}-${shortSha9}.${host}`).then(h.verifyResponse(404)),
h.runCmd(`curl -iL ${scheme}://xx${pr}-${sha9}.${host}`).then(h.verifyResponse(404)), h.runCmd(`curl -iL ${scheme}://xx${pr}-${shortSha9}.${host}`).then(h.verifyResponse(404)),
h.runCmd(`curl -iL ${scheme}://p${pr}-${sha9}.${host}`).then(h.verifyResponse(404)), h.runCmd(`curl -iL ${scheme}://p${pr}-${shortSha9}.${host}`).then(h.verifyResponse(404)),
h.runCmd(`curl -iL ${scheme}://r${pr}-${sha9}.${host}`).then(h.verifyResponse(404)), h.runCmd(`curl -iL ${scheme}://r${pr}-${shortSha9}.${host}`).then(h.verifyResponse(404)),
h.runCmd(`curl -iL ${scheme}://${pr}-${sha9}.${host}`).then(h.verifyResponse(404)), h.runCmd(`curl -iL ${scheme}://${pr}-${shortSha9}.${host}`).then(h.verifyResponse(404)),
h.runCmd(`curl -iL ${scheme}://pr${pr}${sha9}.${host}`).then(h.verifyResponse(404)), h.runCmd(`curl -iL ${scheme}://pr${pr}${shortSha9}.${host}`).then(h.verifyResponse(404)),
h.runCmd(`curl -iL ${scheme}://pr${pr}_${sha9}.${host}`).then(h.verifyResponse(404)), h.runCmd(`curl -iL ${scheme}://pr${pr}_${shortSha9}.${host}`).then(h.verifyResponse(404)),
]).then(done); ]).then(done);
}); });
it('should reject PRs with leading zeros', done => { it('should reject PRs with leading zeros', done => {
h.runCmd(`curl -iL ${scheme}://pr0${pr}-${sha9}.${host}`). h.runCmd(`curl -iL ${scheme}://pr0${pr}-${shortSha9}.${host}`).
then(h.verifyResponse(404)). then(h.verifyResponse(404)).
then(done); then(done);
}); });
@ -134,9 +163,9 @@ describe(`nginx`, () => {
const bodyRegex0 = new RegExp(`^PR: ${pr} | SHA: ${sha0} | File: /index\\.html$`); const bodyRegex0 = new RegExp(`^PR: ${pr} | SHA: ${sha0} | File: /index\\.html$`);
Promise.all([ Promise.all([
h.runCmd(`curl -iL ${scheme}://pr${pr}-0${sha9}.${host}`).then(h.verifyResponse(404)), h.runCmd(`curl -iL ${scheme}://pr${pr}-0${shortSha9}.${host}`).then(h.verifyResponse(404)),
h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha9}.${host}`).then(h.verifyResponse(200, bodyRegex9)), h.runCmd(`curl -iL ${scheme}://pr${pr}-${shortSha9}.${host}`).then(h.verifyResponse(200, bodyRegex9)),
h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha0}.${host}`).then(h.verifyResponse(200, bodyRegex0)), h.runCmd(`curl -iL ${scheme}://pr${pr}-${shortSha0}.${host}`).then(h.verifyResponse(200, bodyRegex0)),
]).then(done); ]).then(done);
}); });
@ -145,13 +174,31 @@ describe(`nginx`, () => {
describe('(for hidden builds)', () => { describe('(for hidden builds)', () => {
beforeEach(() => h.createDummyBuild(pr, sha9, false));
it('should respond with 404 for any file or directory', done => { it('should respond with 404 for any file or directory', done => {
const origin = `${scheme}://pr${pr}-${shortSha9}.${host}`;
const assert404 = h.verifyResponse(404);
h.createDummyBuild(pr, sha9, false);
expect(h.buildExists(pr, sha9, false)).toBe(true);
Promise.all([
h.runCmd(`curl -iL ${origin}/index.html`).then(assert404),
h.runCmd(`curl -iL ${origin}/`).then(assert404),
h.runCmd(`curl -iL ${origin}`).then(assert404),
h.runCmd(`curl -iL ${origin}/foo/bar.js`).then(assert404),
h.runCmd(`curl -iL ${origin}/foo/`).then(assert404),
h.runCmd(`curl -iL ${origin}/foo`).then(assert404),
]).then(done);
});
it('should respond with 404 for any file or directory (for legacy builds)', done => {
const origin = `${scheme}://pr${pr}-${sha9}.${host}`; const origin = `${scheme}://pr${pr}-${sha9}.${host}`;
const assert404 = h.verifyResponse(404); const assert404 = h.verifyResponse(404);
h.createDummyBuild(pr, sha9, false, false, true);
expect(h.buildExists(pr, sha9, false, true)).toBe(true);
Promise.all([ Promise.all([
h.runCmd(`curl -iL ${origin}/index.html`).then(assert404), h.runCmd(`curl -iL ${origin}/index.html`).then(assert404),
h.runCmd(`curl -iL ${origin}/`).then(assert404), h.runCmd(`curl -iL ${origin}/`).then(assert404),
@ -272,7 +319,7 @@ describe(`nginx`, () => {
describe(`${host}/*`, () => { describe(`${host}/*`, () => {
it('should respond with 404 for unkown URLs (even if the resource exists)', done => { it('should respond with 404 for unknown URLs (even if the resource exists)', done => {
['index.html', 'foo.js', 'foo/index.html'].forEach(relFilePath => { ['index.html', 'foo.js', 'foo/index.html'].forEach(relFilePath => {
const absFilePath = path.join(h.buildsDir, relFilePath); const absFilePath = path.join(h.buildsDir, relFilePath);
h.writeFile(absFilePath, {content: `File: /${relFilePath}`}); h.writeFile(absFilePath, {content: `File: /${relFilePath}`});

View File

@ -12,7 +12,7 @@ h.runForAllSupportedSchemes((scheme, port) => describe(`integration (on ${scheme
const archivePath = path.join(h.buildsDir, 'snapshot.tar.gz'); const archivePath = path.join(h.buildsDir, 'snapshot.tar.gz');
const getFile = (pr: string, sha: string, file: string) => const getFile = (pr: string, sha: string, file: string) =>
h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha}.${host}/${file}`); h.runCmd(`curl -iL ${scheme}://pr${pr}-${h.getShordSha(sha)}.${host}/${file}`);
const uploadBuild = (pr: string, sha: string, archive: string, authHeader = 'Token FOO') => { const uploadBuild = (pr: string, sha: string, archive: string, authHeader = 'Token FOO') => {
// Using `FAKE_VERIFICATION_ERROR` or `FAKE_VERIFIED_NOT_TRUSTED` as `authHeader`, // Using `FAKE_VERIFICATION_ERROR` or `FAKE_VERIFIED_NOT_TRUSTED` as `authHeader`,
// we can fake the response of the overwritten `BuildVerifier.verify()` method. // we can fake the response of the overwritten `BuildVerifier.verify()` method.

View File

@ -125,6 +125,27 @@ describe('upload-server (on HTTP)', () => {
}); });
it('should not overwrite existing builds (even if the SHA is different)', done => {
// Since only the first few characters of the SHA are used, it is possible for two different
// SHAs to correspond to the same directory. In that case, we don't want the second SHA to
// overwrite the first.
const sha9Almost = sha9.replace(/.$/, '8');
expect(sha9Almost).not.toBe(sha9);
h.createDummyBuild(pr, sha9, isPublic);
expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toContain('index.html');
h.writeBuildFile(pr, sha9, 'index.html', 'My content', isPublic);
expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toBe('My content');
h.runCmd(`${cmdPrefix} http://${host}/create-build/${pr}/${sha9Almost}`).
then(h.verifyResponse(409, /^Request to overwrite existing directory/)).
then(() => expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toBe('My content')).
then(done);
});
it('should delete the PR directory on error (for new PR)', done => { it('should delete the PR directory on error (for new PR)', done => {
h.runCmd(`${cmdPrefix} http://${host}/create-build/${pr}/${sha9}`). h.runCmd(`${cmdPrefix} http://${host}/create-build/${pr}/${sha9}`).
then(h.verifyResponse(500)). then(h.verifyResponse(500)).
@ -175,7 +196,7 @@ describe('upload-server (on HTTP)', () => {
it(`should create files/directories owned by '${h.wwwUser}'`, done => { it(`should create files/directories owned by '${h.wwwUser}'`, done => {
const prDir = h.getPrDir(pr, isPublic); const prDir = h.getPrDir(pr, isPublic);
const shaDir = path.join(prDir, sha9); const shaDir = h.getShaDir(prDir, sha9);
const idxPath = path.join(shaDir, 'index.html'); const idxPath = path.join(shaDir, 'index.html');
const barPath = path.join(shaDir, 'foo', 'bar.js'); const barPath = path.join(shaDir, 'foo', 'bar.js');
@ -204,7 +225,7 @@ describe('upload-server (on HTTP)', () => {
it('should make the build directory non-writable', done => { it('should make the build directory non-writable', done => {
const prDir = h.getPrDir(pr, isPublic); const prDir = h.getPrDir(pr, isPublic);
const shaDir = path.join(prDir, sha9); const shaDir = h.getShaDir(prDir, sha9);
const idxPath = path.join(shaDir, 'index.html'); const idxPath = path.join(shaDir, 'index.html');
const barPath = path.join(shaDir, 'foo', 'bar.js'); const barPath = path.join(shaDir, 'foo', 'bar.js');
@ -224,6 +245,30 @@ describe('upload-server (on HTTP)', () => {
then(done); then(done);
}); });
it('should ignore a legacy 40-chars long build directory (even if it starts with the same chars)', done => {
// It is possible that 40-chars long build directories exist, if they had been deployed
// before implementing the shorter build directory names. In that case, we don't want the
// second (shorter) name to be considered the same as the old one (even if they originate
// from the same SHA).
h.createDummyBuild(pr, sha9, isPublic, false, true);
expect(h.readBuildFile(pr, sha9, 'index.html', isPublic, true)).toContain('index.html');
h.writeBuildFile(pr, sha9, 'index.html', 'My content', isPublic, true);
expect(h.readBuildFile(pr, sha9, 'index.html', isPublic, true)).toBe('My content');
h.runCmd(`${cmdPrefix} http://${host}/create-build/${pr}/${sha9}`).
then(h.verifyResponse(statusCode)).
then(() => {
expect(h.buildExists(pr, sha9, isPublic)).toBe(true);
expect(h.buildExists(pr, sha9, isPublic, true)).toBe(true);
expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toContain('index.html');
expect(h.readBuildFile(pr, sha9, 'index.html', isPublic, true)).toBe('My content');
}).
then(done);
});
}); });
@ -277,7 +322,7 @@ describe('upload-server (on HTTP)', () => {
it('should reject the request if it fails to update the PR\'s visibility', done => { it('should reject the request if it fails to update the PR\'s visibility', done => {
// One way to cause an error is to have both a public and a hidden directory for the sme PR. // One way to cause an error is to have both a public and a hidden directory for the same PR.
h.createDummyBuild(pr, sha0, isPublic); h.createDummyBuild(pr, sha0, isPublic);
expect(h.buildExists(pr, sha0, isPublic)).toBe(true); expect(h.buildExists(pr, sha0, isPublic)).toBe(true);

View File

@ -4,6 +4,7 @@ import {EventEmitter} from 'events';
import * as fs from 'fs'; 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 {SHORT_SHA_LEN} from '../../lib/common/constants';
import {BuildCreator} from '../../lib/upload-server/build-creator'; import {BuildCreator} from '../../lib/upload-server/build-creator';
import {ChangedPrVisibilityEvent, 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';
@ -13,12 +14,13 @@ import {expectToBeUploadError} from './helpers';
describe('BuildCreator', () => { describe('BuildCreator', () => {
const pr = '9'; const pr = '9';
const sha = '9'.repeat(40); const sha = '9'.repeat(40);
const shortSha = sha.substr(0, SHORT_SHA_LEN);
const archive = 'snapshot.tar.gz'; const archive = 'snapshot.tar.gz';
const buildsDir = 'builds/dir'; const buildsDir = 'builds/dir';
const hiddenPrDir = path.join(buildsDir, `hidden--${pr}`); const hiddenPrDir = path.join(buildsDir, `hidden--${pr}`);
const publicPrDir = path.join(buildsDir, pr); const publicPrDir = path.join(buildsDir, pr);
const hiddenShaDir = path.join(hiddenPrDir, sha); const hiddenShaDir = path.join(hiddenPrDir, shortSha);
const publicShaDir = path.join(publicPrDir, sha); const publicShaDir = path.join(publicPrDir, shortSha);
let bc: BuildCreator; let bc: BuildCreator;
beforeEach(() => bc = new BuildCreator(buildsDir)); beforeEach(() => bc = new BuildCreator(buildsDir));
@ -267,7 +269,7 @@ describe('BuildCreator', () => {
expect(type).toBe(CreatedBuildEvent.type); expect(type).toBe(CreatedBuildEvent.type);
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(shortSha);
expect(evt.isPublic).toBe(isPublic); expect(evt.isPublic).toBe(isPublic);
emitted = true; emitted = true;

View File

@ -65,12 +65,14 @@ More info on how to set things up on CI can be found [here](misc--integrate-with
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 - 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 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. automatically verified, all previous builds are made public as well.
If the PR transitions from "non-public" to "public", the upload-server posts a comment on the 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. 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 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 the first
`<PR>/<SHA>/` (Non-publicly accessible PRs will be stored in a different location.) few characters of the SHA: `<PR>/<SHA>/`
(Non-publicly accessible PRs will be stored in a different location, but again derived from the PR
number and SHA.)
- If the PR is publicly accessible, the upload-server posts a comment on the corresponding PR on - If the PR is publicly accessible, the upload-server posts a comment on the corresponding PR on
GitHub mentioning the SHA and the link where the preview can be found. GitHub mentioning the SHA and the link where the preview can be found.