From 1ae11c0d1a2c86588b467e2c1dc8c3b2bd99ebd7 Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar <16217941+nija-at@users.noreply.github.com> Date: Tue, 11 Jun 2019 13:07:28 +0100 Subject: [PATCH] fix(aws-cdk): Move version check TTL file to home directory (#2774) Originally, the version check TTL file was located alongside the CDK node installation directory. This caused issues for people who had CDK installed on a readonly filesystem. This change moves this file to the user's home directory, specifically under $HOMEDIR/.cdk/cache. This directory is already being used by account-cache.ts. The old logic is maintained where a repo check is run once a day against NPM, when a CDK command is invoked, and when a new version is present will generate a console banner notifying the user of this. The edge case with the updated implementation is when there are 2+ active installations of CDK and they are used interchangably. When this happens, the user will only get one notification per non-latest installation of CDK per day. --- packages/aws-cdk/.gitignore | 1 - packages/aws-cdk/lib/version.ts | 58 ++++++++++++++++--------- packages/aws-cdk/test/test.version.ts | 62 ++++++++++++++++++++++++--- 3 files changed, 93 insertions(+), 28 deletions(-) diff --git a/packages/aws-cdk/.gitignore b/packages/aws-cdk/.gitignore index 00f5e67ea2b57..93f15b0ddeed7 100644 --- a/packages/aws-cdk/.gitignore +++ b/packages/aws-cdk/.gitignore @@ -7,7 +7,6 @@ dist # Generated by generate.sh build-info.json -.LAST_VERSION_CHECK .LAST_BUILD .nyc_output diff --git a/packages/aws-cdk/lib/version.ts b/packages/aws-cdk/lib/version.ts index 7d9933b6b8f03..8e4f9128b56e2 100644 --- a/packages/aws-cdk/lib/version.ts +++ b/packages/aws-cdk/lib/version.ts @@ -1,17 +1,16 @@ import { exec as _exec } from 'child_process'; import colors = require('colors/safe'); -import { close as _close, open as _open, stat as _stat } from 'fs'; +import fs = require('fs-extra'); +import os = require('os'); +import path = require('path'); import semver = require('semver'); import { promisify } from 'util'; -import { debug, print, warning } from '../lib/logging'; +import { debug, print } from '../lib/logging'; import { formatAsBanner } from '../lib/util/console-formatters'; const ONE_DAY_IN_SECONDS = 1 * 24 * 60 * 60; -const close = promisify(_close); const exec = promisify(_exec); -const open = promisify(_open); -const stat = promisify(_stat); export const DISPLAY_VERSION = `${versionNumber()} (build ${commit()})`; @@ -23,23 +22,39 @@ function commit(): string { return require('../build-info.json').commit; } -export class TimestampFile { +export class VersionCheckTTL { + public static timestampFilePath(): string { + // Get the home directory from the OS, first. Fallback to $HOME. + const homedir = os.userInfo().homedir || os.homedir(); + if (!homedir || !homedir.trim()) { + throw new Error('Cannot determine home directory'); + } + // Using the same path from account-cache.ts + return path.join(homedir, '.cdk', 'cache', 'repo-version-ttl'); + } + private readonly file: string; - // File modify times are accurate only till the second, hence using seconds as precision + // File modify times are accurate only to the second private readonly ttlSecs: number; - constructor(file: string, ttlSecs: number) { - this.file = file; - this.ttlSecs = ttlSecs; + constructor(file?: string, ttlSecs?: number) { + this.file = file || VersionCheckTTL.timestampFilePath(); + try { + fs.mkdirsSync(path.dirname(this.file)); + fs.accessSync(path.dirname(this.file), fs.constants.W_OK); + } catch { + throw new Error(`Directory (${path.dirname(this.file)}) is not writable.`); + } + this.ttlSecs = ttlSecs || ONE_DAY_IN_SECONDS; } public async hasExpired(): Promise { try { - const lastCheckTime = (await stat(this.file)).mtimeMs; + const lastCheckTime = (await fs.stat(this.file)).mtimeMs; const today = new Date().getTime(); - if ((today - lastCheckTime) / 1000 > this.ttlSecs) { // convert ms to secs + if ((today - lastCheckTime) / 1000 > this.ttlSecs) { // convert ms to sec return true; } return false; @@ -52,15 +67,17 @@ export class TimestampFile { } } - public async update(): Promise { - const fd = await open(this.file, 'w'); - await close(fd); + public async update(latestVersion?: string): Promise { + if (!latestVersion) { + latestVersion = ''; + } + await fs.writeFile(this.file, latestVersion); } } // Export for unit testing only. // Don't use directly, use displayVersionMessage() instead. -export async function latestVersionIfHigher(currentVersion: string, cacheFile: TimestampFile): Promise { +export async function latestVersionIfHigher(currentVersion: string, cacheFile: VersionCheckTTL): Promise { if (!(await cacheFile.hasExpired())) { return null; } @@ -74,7 +91,7 @@ export async function latestVersionIfHigher(currentVersion: string, cacheFile: T throw new Error(`npm returned an invalid semver ${latestVersion}`); } const isNewer = semver.gt(latestVersion, currentVersion); - await cacheFile.update(); + await cacheFile.update(latestVersion); if (isNewer) { return latestVersion; @@ -83,14 +100,13 @@ export async function latestVersionIfHigher(currentVersion: string, cacheFile: T } } -const versionCheckCache = new TimestampFile(`${__dirname}/../.LAST_VERSION_CHECK`, ONE_DAY_IN_SECONDS); - export async function displayVersionMessage(): Promise { if (!process.stdout.isTTY) { return; } try { + const versionCheckCache = new VersionCheckTTL(); const laterVersion = await latestVersionIfHigher(versionNumber(), versionCheckCache); if (laterVersion) { const bannerMsg = formatAsBanner([ @@ -100,6 +116,6 @@ export async function displayVersionMessage(): Promise { bannerMsg.forEach((e) => print(e)); } } catch (err) { - warning(`Could not run version check due to error ${err.message}`); + debug(`Could not run version check - ${err.message}`); } -} \ No newline at end of file +} diff --git a/packages/aws-cdk/test/test.version.ts b/packages/aws-cdk/test/test.version.ts index e0df94382b900..d4c31b67be7b6 100644 --- a/packages/aws-cdk/test/test.version.ts +++ b/packages/aws-cdk/test/test.version.ts @@ -1,7 +1,11 @@ +import fs = require('fs-extra'); import { Test } from 'nodeunit'; +import os = require('os'); +import path = require('path'); +import sinon = require('sinon'); import { setTimeout as _setTimeout } from 'timers'; import { promisify } from 'util'; -import { latestVersionIfHigher, TimestampFile } from '../lib/version'; +import { latestVersionIfHigher, VersionCheckTTL } from '../lib/version'; const setTimeout = promisify(_setTimeout); @@ -10,14 +14,29 @@ function tmpfile(): string { } export = { + 'tearDown'(callback: () => void) { + sinon.restore(); + callback(); + }, + + 'initialization fails on unwritable directory'(test: Test) { + test.expect(1); + const cacheFile = tmpfile(); + sinon.stub(fs, 'mkdirsSync').withArgs(path.dirname(cacheFile)).throws('Cannot make directory'); + test.throws(() => new VersionCheckTTL(cacheFile), /not writable/); + test.done(); + }, + async 'cache file responds correctly when file is not present'(test: Test) { - const cache = new TimestampFile(tmpfile(), 1); + test.expect(1); + const cache = new VersionCheckTTL(tmpfile(), 1); test.strictEqual(await cache.hasExpired(), true); test.done(); }, async 'cache file honours the specified TTL'(test: Test) { - const cache = new TimestampFile(tmpfile(), 1); + test.expect(2); + const cache = new VersionCheckTTL(tmpfile(), 1); await cache.update(); test.strictEqual(await cache.hasExpired(), false); await setTimeout(1001); // Just above 1 sec in ms @@ -26,14 +45,16 @@ export = { }, async 'Skip version check if cache has not expired'(test: Test) { - const cache = new TimestampFile(tmpfile(), 100); + test.expect(1); + const cache = new VersionCheckTTL(tmpfile(), 100); await cache.update(); test.equal(await latestVersionIfHigher('0.0.0', cache), null); test.done(); }, async 'Return later version when exists & skip recent re-check'(test: Test) { - const cache = new TimestampFile(tmpfile(), 100); + test.expect(3); + const cache = new VersionCheckTTL(tmpfile(), 100); const result = await latestVersionIfHigher('0.0.0', cache); test.notEqual(result, null); test.ok((result as string).length > 0); @@ -44,9 +65,38 @@ export = { }, async 'Return null if version is higher than npm'(test: Test) { - const cache = new TimestampFile(tmpfile(), 100); + test.expect(1); + const cache = new VersionCheckTTL(tmpfile(), 100); const result = await latestVersionIfHigher('100.100.100', cache); test.equal(result, null); test.done(); }, + + 'No homedir for the given user'(test: Test) { + test.expect(1); + sinon.stub(os, 'homedir').returns(''); + sinon.stub(os, 'userInfo').returns({ username: '', uid: 10, gid: 11, shell: null, homedir: ''}); + test.throws(() => new VersionCheckTTL(), /Cannot determine home directory/); + test.done(); + }, + + async 'Version specified is stored in the TTL file'(test: Test) { + test.expect(1); + const cacheFile = tmpfile(); + const cache = new VersionCheckTTL(cacheFile, 1); + await cache.update('1.1.1'); + const storedVersion = fs.readFileSync(cacheFile, 'utf8'); + test.equal(storedVersion, '1.1.1'); + test.done(); + }, + + async 'No Version specified for storage in the TTL file'(test: Test) { + test.expect(1); + const cacheFile = tmpfile(); + const cache = new VersionCheckTTL(cacheFile, 1); + await cache.update(); + const storedVersion = fs.readFileSync(cacheFile, 'utf8'); + test.equal(storedVersion, ''); + test.done(); + }, };