From d263595c6397f9c2dd150590ac4a44ee6d389b1d Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Thu, 13 Apr 2017 00:38:21 +0300 Subject: [PATCH] ci(aio): do not fail when re-deploying preview for the same PR/SHA Previously, when trying to upload the build artifacts for a PR/SHA that was already successfully deployed (e.g. when re-running a Travis job), the preview server would return a 403 and the build would fail. Since we have other mechanisms to verify that the PR author is trusted and the artifacts do indeed come from the specified PR and since the new artifacts should be the same with the already deployed ones (same SHA), there is no reason to fail the build. The preview server will reject the request with a special HTTP status code (409 - Conflict), which the `deploy-preview` script will recognize and exit with 0. --- .../scripts-js/lib/upload-server/build-creator.ts | 2 +- .../scripts-js/lib/verify-setup/server-integration.e2e.ts | 2 +- .../scripts-js/lib/verify-setup/upload-server.e2e.ts | 2 +- .../scripts-js/test/upload-server/build-creator.spec.ts | 2 +- aio/scripts/deploy-preview.sh | 5 +++-- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/upload-server/build-creator.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/upload-server/build-creator.ts index 4197da7ab36d2..0ae0b20da8faf 100644 --- a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/upload-server/build-creator.ts +++ b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/upload-server/build-creator.ts @@ -26,7 +26,7 @@ export class BuildCreator extends EventEmitter { all([this.exists(prDir), this.exists(shaDir)]). then(([prDirExisted, shaDirExisted]) => { if (shaDirExisted) { - throw new UploadError(403, `Request to overwrite existing directory: ${shaDir}`); + throw new UploadError(409, `Request to overwrite existing directory: ${shaDir}`); } dirToRemoveOnError = prDirExisted ? shaDir : prDir; diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/server-integration.e2e.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/server-integration.e2e.ts index 93ad1fcb88567..3f0ffa1418740 100644 --- a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/server-integration.e2e.ts +++ b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/server-integration.e2e.ts @@ -73,7 +73,7 @@ h.runForAllSupportedSchemes((scheme, port) => describe(`integration (on ${scheme h.createDummyArchive(pr9, sha9, archivePath); uploadBuild(pr9, sha9, archivePath). - then(h.verifyResponse(403)). + then(h.verifyResponse(409)). then(() => Promise.all([ getFile(pr9, sha9, 'index.html').then(h.verifyResponse(200, idxContentRegex9)), getFile(pr9, sha9, 'foo/bar.js').then(h.verifyResponse(200, barContentRegex9)), diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/upload-server.e2e.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/upload-server.e2e.ts index 84ea36d8f775f..198192a522212 100644 --- a/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/upload-server.e2e.ts +++ b/aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/upload-server.e2e.ts @@ -101,7 +101,7 @@ describe('upload-server (on HTTP)', () => { expect(h.readBuildFile(pr, sha9, 'index.html')).toBe('My content'); h.runCmd(`${curl} http://${host}/create-build/${pr}/${sha9}`). - then(h.verifyResponse(403, /^Request to overwrite existing directory/)). + then(h.verifyResponse(409, /^Request to overwrite existing directory/)). then(() => expect(h.readBuildFile(pr, sha9, 'index.html')).toBe('My content')). then(done); }); diff --git a/aio/aio-builds-setup/dockerbuild/scripts-js/test/upload-server/build-creator.spec.ts b/aio/aio-builds-setup/dockerbuild/scripts-js/test/upload-server/build-creator.spec.ts index 329b41b2e4765..962b38b83ce1d 100644 --- a/aio/aio-builds-setup/dockerbuild/scripts-js/test/upload-server/build-creator.spec.ts +++ b/aio/aio-builds-setup/dockerbuild/scripts-js/test/upload-server/build-creator.spec.ts @@ -66,7 +66,7 @@ describe('BuildCreator', () => { it('should throw if the build does already exist', done => { bcExistsSpy.and.returnValue(true); bc.create(pr, sha, archive).catch(err => { - expectToBeUploadError(err, 403, `Request to overwrite existing directory: ${shaDir}`); + expectToBeUploadError(err, 409, `Request to overwrite existing directory: ${shaDir}`); done(); }); }); diff --git a/aio/scripts/deploy-preview.sh b/aio/scripts/deploy-preview.sh index b032388fd222a..9c94d7da4fcf9 100755 --- a/aio/scripts/deploy-preview.sh +++ b/aio/scripts/deploy-preview.sh @@ -24,8 +24,9 @@ httpCode=$( | sed 's/HTTP_CODE: //' ) -# Exit with an error if the request failed -if [ $httpCode -lt 200 ] || [ $httpCode -ge 400 ]; then +# Exit with an error if the request failed. +# (Ignore 409 failures, which mean trying to re-deploy for the same PR/SHA.) +if [ $httpCode -lt 200 ] || ([ $httpCode -ge 400 ] && [ $httpCode -ne 409 ]); then exit 1 fi