feat(docs-infra): add API endpoint for checking if PR can have preview (#25671)

There several reasons why PRs cannot have (public) previews:
- The PR did not affect any relevant files (e.g. non-spec files in
  `aio/` or `packages/`).
- The PR cannot be automatically verified as "trusted" (based on its
  author or labels).

Note:
The endpoint does not check whether there currently is a (public)
preview for the specified PR; only whether there can be one.

PR Close #25671
This commit is contained in:
George Kalpakas 2018-08-26 00:29:14 +03:00 committed by Kara Erickson
parent f378454c92
commit 6d6b0ff1ad
7 changed files with 304 additions and 9 deletions

View File

@ -66,6 +66,21 @@ server {
return 200 ''; return 200 '';
} }
# Check PRs previewability
location "~^/can-have-public-preview/\d+/?$" {
if ($request_method != "GET") {
add_header Allow "GET";
return 405;
}
proxy_pass_request_headers on;
proxy_redirect off;
proxy_method GET;
proxy_pass http://{{$AIO_PREVIEW_SERVER_HOSTNAME}}:{{$AIO_PREVIEW_SERVER_PORT}}$request_uri;
resolver 127.0.0.1;
}
# Notify about CircleCI builds # Notify about CircleCI builds
location "~^/circle-build/?$" { location "~^/circle-build/?$" {
if ($request_method != "POST") { if ($request_method != "POST") {

View File

@ -64,10 +64,36 @@ export class PreviewServerFactory {
buildCreator: BuildCreator, cfg: PreviewServerConfig): express.Express { buildCreator: BuildCreator, cfg: PreviewServerConfig): express.Express {
const middleware = express(); const middleware = express();
const jsonParser = bodyParser.json(); const jsonParser = bodyParser.json();
const significantFilesRe = new RegExp(cfg.significantFilesPattern);
// RESPOND TO IS-ALIVE PING // RESPOND TO IS-ALIVE PING
middleware.get(/^\/health-check\/?$/, (_req, res) => res.sendStatus(200)); middleware.get(/^\/health-check\/?$/, (_req, res) => res.sendStatus(200));
// RESPOND TO CAN-HAVE-PUBLIC-PREVIEW CHECK
const canHavePublicPreviewRe = /^\/can-have-public-preview\/(\d+)\/?$/;
middleware.get(canHavePublicPreviewRe, async (req, res) => {
try {
const pr = +canHavePublicPreviewRe.exec(req.url)![1];
if (!await buildVerifier.getSignificantFilesChanged(pr, significantFilesRe)) {
// Cannot have preview: PR did not touch relevant files: `aio/` or `packages/` (except for spec files).
res.send({canHavePublicPreview: false, reason: 'No significant files touched.'});
logger.log(`PR:${pr} - Cannot have a public preview, because it did not touch any significant files.`);
} else if (!await buildVerifier.getPrIsTrusted(pr)) {
// Cannot have preview: PR not automatically verifiable as "trusted".
res.send({canHavePublicPreview: false, reason: 'Not automatically verifiable as "trusted".'});
logger.log(`PR:${pr} - Cannot have a public preview, because not automatically verifiable as "trusted".`);
} else {
// Can have preview.
res.send({canHavePublicPreview: true, reason: null});
logger.log(`PR:${pr} - Can have a public preview.`);
}
} catch (err) {
logger.error('Previewability check error', err);
respondWithError(res, err);
}
});
// CIRCLE_CI BUILD COMPLETE WEBHOOK // CIRCLE_CI BUILD COMPLETE WEBHOOK
middleware.post(/^\/circle-build\/?$/, jsonParser, async (req, res) => { middleware.post(/^\/circle-build\/?$/, jsonParser, async (req, res) => {
try { try {
@ -108,7 +134,7 @@ export class PreviewServerFactory {
`Invalid webhook: expected "githubRepo" property to equal "${cfg.githubRepo}" but got "${repo}".`); `Invalid webhook: expected "githubRepo" property to equal "${cfg.githubRepo}" but got "${repo}".`);
// Do not deploy unless this PR has touched relevant files: `aio/` or `packages/` (except for spec files) // Do not deploy unless this PR has touched relevant files: `aio/` or `packages/` (except for spec files)
if (!await buildVerifier.getSignificantFilesChanged(pr, new RegExp(cfg.significantFilesPattern))) { if (!await buildVerifier.getSignificantFilesChanged(pr, significantFilesRe)) {
res.sendStatus(204); res.sendStatus(204);
logger.log(`PR:${pr}, Build:${buildNum} - ` + logger.log(`PR:${pr}, Build:${buildNum} - ` +
`Skipping preview processing because this PR did not touch any significant files.`); `Skipping preview processing because this PR did not touch any significant files.`);

View File

@ -105,7 +105,7 @@ class Helper {
Object.keys(this.portPerScheme).forEach(scheme => suiteFactory(scheme, this.portPerScheme[scheme])); Object.keys(this.portPerScheme).forEach(scheme => suiteFactory(scheme, this.portPerScheme[scheme]));
} }
public verifyResponse(status: number | [number, string], regex = /^/): VerifyCmdResultFn { public verifyResponse(status: number | [number, string], regex: string | RegExp = /^/): VerifyCmdResultFn {
let statusCode: number; let statusCode: number;
let statusText: string; let statusText: string;
@ -180,26 +180,42 @@ class Helper {
} }
} }
interface DefaultCurlOptions {
defaultMethod?: CurlOptions['method'];
defaultOptions?: CurlOptions['options'];
defaultHeaders?: CurlOptions['headers'];
defaultData?: CurlOptions['data'];
defaultExtraPath?: CurlOptions['extraPath'];
}
interface CurlOptions { interface CurlOptions {
method?: string; method?: string;
options?: string; options?: string;
headers?: string[];
data?: any; data?: any;
url?: string; url?: string;
extraPath?: string; extraPath?: string;
} }
export function makeCurl(baseUrl: string) { export function makeCurl(baseUrl: string, {
defaultMethod = 'POST',
defaultOptions = '',
defaultHeaders = ['Content-Type: application/json'],
defaultData = {},
defaultExtraPath = '',
}: DefaultCurlOptions = {}) {
return function curl({ return function curl({
method = 'POST', method = defaultMethod,
options = '', options = defaultOptions,
data = {}, headers = defaultHeaders,
data = defaultData,
url = baseUrl, url = baseUrl,
extraPath = '', extraPath = defaultExtraPath,
}: CurlOptions) { }: CurlOptions) {
const dataString = data ? JSON.stringify(data) : ''; const dataString = data ? JSON.stringify(data) : '';
const cmd = `curl -iLX ${method} ` + const cmd = `curl -iLX ${method} ` +
`${options} ` + `${options} ` +
`--header "Content-Type: application/json" ` + headers.map(header => `--header "${header}" `).join('') +
`--data '${dataString}' ` + `--data '${dataString}' ` +
`${url}${extraPath}`; `${url}${extraPath}`;
return helper.runCmd(cmd); return helper.runCmd(cmd);

View File

@ -3,6 +3,7 @@ import * as path from 'path';
import {rm} from 'shelljs'; import {rm} from 'shelljs';
import {AIO_BUILDS_DIR, AIO_NGINX_HOSTNAME, AIO_NGINX_PORT_HTTP, AIO_NGINX_PORT_HTTPS} from '../common/env-variables'; import {AIO_BUILDS_DIR, AIO_NGINX_HOSTNAME, AIO_NGINX_PORT_HTTP, AIO_NGINX_PORT_HTTPS} from '../common/env-variables';
import {computeShortSha} from '../common/utils'; import {computeShortSha} from '../common/utils';
import {PrNums} from './constants';
import {helper as h} from './helper'; import {helper as h} from './helper';
import {customMatchers} from './jasmine-custom-matchers'; import {customMatchers} from './jasmine-custom-matchers';
@ -252,6 +253,42 @@ describe(`nginx`, () => {
}); });
describe(`${host}/can-have-public-preview`, () => {
const baseUrl = `${scheme}://${host}/can-have-public-preview`;
it('should disallow non-GET requests', async () => {
await Promise.all([
h.runCmd(`curl -iLX POST ${baseUrl}/42`).then(h.verifyResponse([405, 'Not Allowed'])),
h.runCmd(`curl -iLX PUT ${baseUrl}/42`).then(h.verifyResponse([405, 'Not Allowed'])),
h.runCmd(`curl -iLX PATCH ${baseUrl}/42`).then(h.verifyResponse([405, 'Not Allowed'])),
h.runCmd(`curl -iLX DELETE ${baseUrl}/42`).then(h.verifyResponse([405, 'Not Allowed'])),
]);
});
it('should pass requests through to the preview server', async () => {
await h.runCmd(`curl -iLX GET ${baseUrl}/${PrNums.CHANGED_FILES_ERROR}`).
then(h.verifyResponse(500, /CHANGED_FILES_ERROR/));
});
it('should respond with 404 for unknown paths', async () => {
const cmdPrefix = `curl -iLX GET ${baseUrl}`;
await Promise.all([
h.runCmd(`${cmdPrefix}/foo/42`).then(h.verifyResponse(404)),
h.runCmd(`${cmdPrefix}-foo/42`).then(h.verifyResponse(404)),
h.runCmd(`${cmdPrefix}nfoo/42`).then(h.verifyResponse(404)),
h.runCmd(`${cmdPrefix}/42/foo`).then(h.verifyResponse(404)),
h.runCmd(`${cmdPrefix}/f00`).then(h.verifyResponse(404)),
h.runCmd(`${cmdPrefix}/`).then(h.verifyResponse(404)),
]);
});
});
describe(`${host}/circle-build`, () => { describe(`${host}/circle-build`, () => {
it('should disallow non-POST requests', done => { it('should disallow non-POST requests', done => {
@ -287,6 +324,7 @@ describe(`nginx`, () => {
h.runCmd(`${cmdPrefix}/circle-build/42`).then(h.verifyResponse(404)), h.runCmd(`${cmdPrefix}/circle-build/42`).then(h.verifyResponse(404)),
]).then(done); ]).then(done);
}); });
}); });

View File

@ -18,6 +18,92 @@ describe('preview-server', () => {
afterEach(() => h.cleanUp()); afterEach(() => h.cleanUp());
describe(`${host}/can-have-public-preview`, () => {
const curl = makeCurl(`${host}/can-have-public-preview`, {
defaultData: null,
defaultExtraPath: `/${PrNums.TRUST_CHECK_ACTIVE_TRUSTED_USER}`,
defaultHeaders: [],
defaultMethod: 'GET',
});
it('should disallow non-GET requests', async () => {
const bodyRegex = /^Unknown resource in request/;
await Promise.all([
curl({method: 'POST'}).then(h.verifyResponse(404, bodyRegex)),
curl({method: 'PUT'}).then(h.verifyResponse(404, bodyRegex)),
curl({method: 'PATCH'}).then(h.verifyResponse(404, bodyRegex)),
curl({method: 'DELETE'}).then(h.verifyResponse(404, bodyRegex)),
]);
});
it('should respond with 404 for unknown paths', async () => {
const bodyRegex = /^Unknown resource in request/;
await Promise.all([
curl({extraPath: `/foo/${PrNums.TRUST_CHECK_ACTIVE_TRUSTED_USER}`}).then(h.verifyResponse(404, bodyRegex)),
curl({extraPath: `-foo/${PrNums.TRUST_CHECK_ACTIVE_TRUSTED_USER}`}).then(h.verifyResponse(404, bodyRegex)),
curl({extraPath: `nfoo/${PrNums.TRUST_CHECK_ACTIVE_TRUSTED_USER}`}).then(h.verifyResponse(404, bodyRegex)),
curl({extraPath: `/${PrNums.TRUST_CHECK_ACTIVE_TRUSTED_USER}/foo`}).then(h.verifyResponse(404, bodyRegex)),
curl({extraPath: '/f00'}).then(h.verifyResponse(404, bodyRegex)),
curl({extraPath: '/'}).then(h.verifyResponse(404, bodyRegex)),
]);
});
it('should respond with 500 if checking for significant file changes fails', async () => {
await Promise.all([
curl({extraPath: `/${PrNums.CHANGED_FILES_404}`}).then(h.verifyResponse(500, /CHANGED_FILES_404/)),
curl({extraPath: `/${PrNums.CHANGED_FILES_ERROR}`}).then(h.verifyResponse(500, /CHANGED_FILES_ERROR/)),
]);
});
it('should respond with 200 (false) if no significant files were touched', async () => {
const expectedResponse = JSON.stringify({
canHavePublicPreview: false,
reason: 'No significant files touched.',
});
await curl({extraPath: `/${PrNums.CHANGED_FILES_NONE}`}).then(h.verifyResponse(200, expectedResponse));
});
it('should respond with 500 if checking "trusted" status fails', async () => {
await curl({extraPath: `/${PrNums.TRUST_CHECK_ERROR}`}).then(h.verifyResponse(500, 'TRUST_CHECK_ERROR'));
});
it('should respond with 200 (false) if the PR is not automatically verifiable as "trusted"', async () => {
const expectedResponse = JSON.stringify({
canHavePublicPreview: false,
reason: 'Not automatically verifiable as \\"trusted\\".',
});
await Promise.all([
curl({extraPath: `/${PrNums.TRUST_CHECK_INACTIVE_TRUSTED_USER}`}).then(h.verifyResponse(200, expectedResponse)),
curl({extraPath: `/${PrNums.TRUST_CHECK_UNTRUSTED}`}).then(h.verifyResponse(200, expectedResponse)),
]);
});
it('should respond with 200 (true) if the PR can have a public preview', async () => {
const expectedResponse = JSON.stringify({
canHavePublicPreview: true,
reason: null,
});
await Promise.all([
curl({extraPath: `/${PrNums.TRUST_CHECK_ACTIVE_TRUSTED_USER}`}).then(h.verifyResponse(200, expectedResponse)),
curl({extraPath: `/${PrNums.TRUST_CHECK_TRUSTED_LABEL}`}).then(h.verifyResponse(200, expectedResponse)),
]);
});
});
describe(`${host}/circle-build`, () => { describe(`${host}/circle-build`, () => {
const curl = makeCurl(`${host}/circle-build`); const curl = makeCurl(`${host}/circle-build`);

View File

@ -39,6 +39,7 @@ describe('PreviewServerFactory', () => {
significantFilesPattern: '^(?:aio|packages)\\/(?!.*[._]spec\\.[jt]s$)', significantFilesPattern: '^(?:aio|packages)\\/(?!.*[._]spec\\.[jt]s$)',
trustedPrLabel: 'trusted: pr-label', trustedPrLabel: 'trusted: pr-label',
}; };
let loggerErrorSpy: jasmine.Spy;
let loggerInfoSpy: jasmine.Spy; let loggerInfoSpy: jasmine.Spy;
let loggerLogSpy: jasmine.Spy; let loggerLogSpy: jasmine.Spy;
@ -47,9 +48,9 @@ describe('PreviewServerFactory', () => {
PreviewServerFactory.create({...defaultConfig, ...partialConfig}); PreviewServerFactory.create({...defaultConfig, ...partialConfig});
beforeEach(() => { beforeEach(() => {
loggerErrorSpy = spyOn(Logger.prototype, 'error');
loggerInfoSpy = spyOn(Logger.prototype, 'info'); loggerInfoSpy = spyOn(Logger.prototype, 'info');
loggerLogSpy = spyOn(Logger.prototype, 'log'); loggerLogSpy = spyOn(Logger.prototype, 'log');
spyOn(Logger.prototype, 'error');
}); });
describe('create()', () => { describe('create()', () => {
@ -296,6 +297,102 @@ describe('PreviewServerFactory', () => {
}); });
describe('GET /can-have-public-preview/<pr>', () => {
const baseUrl = '/can-have-public-preview';
const pr = 777;
const url = `${baseUrl}/${pr}`;
let bvGetPrIsTrustedSpy: jasmine.Spy;
let bvGetSignificantFilesChangedSpy: jasmine.Spy;
beforeEach(() => {
bvGetPrIsTrustedSpy = spyOn(buildVerifier, 'getPrIsTrusted').and.returnValue(Promise.resolve(true));
bvGetSignificantFilesChangedSpy = spyOn(buildVerifier, 'getSignificantFilesChanged').
and.returnValue(Promise.resolve(true));
});
it('should respond with 404 for non-GET requests', async () => {
await Promise.all([
agent.put(url).expect(404),
agent.post(url).expect(404),
agent.patch(url).expect(404),
agent.delete(url).expect(404),
]);
});
it('should respond with 404 if the path does not match exactly', async () => {
await Promise.all([
agent.get('/can-have-public-preview/42/foo').expect(404),
agent.get('/can-have-public-preview-foo/42').expect(404),
agent.get('/can-have-public-previewnfoo/42').expect(404),
agent.get('/foo/can-have-public-preview/42').expect(404),
agent.get('/foo-can-have-public-preview/42').expect(404),
agent.get('/fooncan-have-public-preview/42').expect(404),
]);
});
it('should respond appropriately if the PR did not touch any significant files', async () => {
bvGetSignificantFilesChangedSpy.and.returnValue(Promise.resolve(false));
const expectedResponse = {canHavePublicPreview: false, reason: 'No significant files touched.'};
const expectedLog = `PR:${pr} - Cannot have a public preview, because it did not touch any significant files.`;
await agent.get(url).expect(200, expectedResponse);
expect(bvGetSignificantFilesChangedSpy).toHaveBeenCalledWith(pr, jasmine.any(RegExp));
expect(bvGetPrIsTrustedSpy).not.toHaveBeenCalled();
expect(loggerLogSpy).toHaveBeenCalledWith(expectedLog);
});
it('should respond appropriately if the PR is not automatically verifiable as "trusted"', async () => {
bvGetPrIsTrustedSpy.and.returnValue(Promise.resolve(false));
const expectedResponse = {canHavePublicPreview: false, reason: 'Not automatically verifiable as "trusted".'};
const expectedLog =
`PR:${pr} - Cannot have a public preview, because not automatically verifiable as "trusted".`;
await agent.get(url).expect(200, expectedResponse);
expect(bvGetSignificantFilesChangedSpy).toHaveBeenCalledWith(pr, jasmine.any(RegExp));
expect(bvGetPrIsTrustedSpy).toHaveBeenCalledWith(pr);
expect(loggerLogSpy).toHaveBeenCalledWith(expectedLog);
});
it('should respond appropriately if the PR can have a preview', async () => {
const expectedResponse = {canHavePublicPreview: true, reason: null};
const expectedLog = `PR:${pr} - Can have a public preview.`;
await agent.get(url).expect(200, expectedResponse);
expect(bvGetSignificantFilesChangedSpy).toHaveBeenCalledWith(pr, jasmine.any(RegExp));
expect(bvGetPrIsTrustedSpy).toHaveBeenCalledWith(pr);
expect(loggerLogSpy).toHaveBeenCalledWith(expectedLog);
});
it('should respond with error if `getSignificantFilesChanged()` fails', async () => {
bvGetSignificantFilesChangedSpy.and.callFake(() => Promise.reject('getSignificantFilesChanged error'));
await agent.get(url).expect(500, 'getSignificantFilesChanged error');
expect(loggerErrorSpy).toHaveBeenCalledWith('Previewability check error', 'getSignificantFilesChanged error');
});
it('should respond with error if `getPrIsTrusted()` fails', async () => {
const error = new Error('getPrIsTrusted error');
bvGetPrIsTrustedSpy.and.callFake(() => { throw error; });
await agent.get(url).expect(500, 'getPrIsTrusted error');
expect(loggerErrorSpy).toHaveBeenCalledWith('Previewability check error', error);
});
});
describe('/circle-build', () => { describe('/circle-build', () => {
let getGithubInfoSpy: jasmine.Spy; let getGithubInfoSpy: jasmine.Spy;
let getSignificantFilesChangedSpy: jasmine.Spy; let getSignificantFilesChangedSpy: jasmine.Spy;

View File

@ -25,6 +25,23 @@ with a brief explanation of what they mean:
File not found. File not found.
## `https://ngbuilds.io/can-have-public-preview/<pr>`
- **200 (OK)**:
Whether the PR can have a public preview (based on its author, label, changed files).
_Response type:_ JSON
_Response format:_
```ts
{
canHavePublicPreview: boolean,
reason: string | null,
}
```
- **405 (Method Not Allowed)**:
Request method other than GET.
## `https://ngbuilds.io/circle-build` ## `https://ngbuilds.io/circle-build`
- **201 (Created)**: - **201 (Created)**: