From 6a779fcbf3b70913fa13f0b8d8cc5878c7c29c11 Mon Sep 17 00:00:00 2001 From: Rowan Manning <138944+rowanmanning@users.noreply.github.com> Date: Thu, 29 Feb 2024 15:52:31 +0000 Subject: [PATCH 1/4] Stop using Moment.js to subtract a date Moment really isn't needed for this. We can subtract using regular date objects and timestamps. The following code is a proof: ```js function runMoment(samplePeriod) { const timeWindow = samplePeriod * 1.5; const now = moment(); const startTime = now.subtract(timeWindow, 'seconds'); return startTime.toISOString(); } function runDate(samplePeriod) { const timeWindowInMs = (samplePeriod * 1.5) * 1000; const now = new Date(); const startTime = new Date(now.getTime() - timeWindowInMs); return startTime.toISOString(); } console.log("moment", runMoment(60 * 5)); console.log("js", runDate(60 * 5)); ``` The output values are almost exactly the same, the 1-2ms difference can be accounted for by the Moment code taking a while to run. --- src/checks/cloudWatchThreshold.check.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/checks/cloudWatchThreshold.check.js b/src/checks/cloudWatchThreshold.check.js index cecbd16..cba3d6a 100644 --- a/src/checks/cloudWatchThreshold.check.js +++ b/src/checks/cloudWatchThreshold.check.js @@ -1,7 +1,6 @@ 'use strict'; const AWS = require('aws-sdk'); -const moment = require('moment'); const logger = require('@dotcom-reliability-kit/logger'); const status = require('./status'); const Check = require('./check'); @@ -32,11 +31,12 @@ class CloudWatchThresholdCheck extends Check { generateParams() { // use a larger window when gathering stats, because CloudWatch // can take its sweet time with populating new datapoints. - let timeWindow = this.samplePeriod * 1.5; - const now = moment(); + let timeWindowInMs = this.samplePeriod * 1.5 * 1000; + const now = new Date(); + const startTime = new Date(now.getTime() - timeWindowInMs); return { EndTime: now.toISOString(), - StartTime: now.subtract(timeWindow, 'seconds').toISOString(), + StartTime: startTime.toISOString(), MetricName: this.cloudWatchMetricName, Namespace: this.cloudWatchNamespace, Period: this.samplePeriod, From dced74ec0336b52e81e476274958804c29f804d0 Mon Sep 17 00:00:00 2001 From: Rowan Manning <138944+rowanmanning@users.noreply.github.com> Date: Thu, 29 Feb 2024 16:10:46 +0000 Subject: [PATCH 2/4] Stop using Moment.js for date parsing/manipulation This really isn't needed, Moment.js is a massive library and you can do the equivalent with plain JavaScript nowadays. Firstly, date comparison. Dates can be compared with regular operators: ```js console.log(new Date('2023-01-01') > new Date('2024-01-01')); ``` Date parsing is generally fine too especially in ISO format, we can do a little bit of extra validation but tbh I don't think it's needed - we can just trust the date format that the Fastly API returns. Example of date parsing just working: ```js console.log(moment("2024-01-26T10:00:00Z", 'YYYY-MM-DDTHH:mm:ssZ').toISOString()); console.log(new Date("2024-01-26T10:00:00Z").toISOString()); ``` Lastly there's no need to use Moment for date addition. The following are equivalent: ```js function runMoment() { const now = moment(); const limitDate = moment().add(2, 'weeks'); return limitDate.toISOString(); } function runDate() { const now = new Date(); const limitDate = new Date(); limitDate.setDate(now.getDate() + (2 * 7)); return limitDate.toISOString(); } console.log("moment", runMoment()); console.log("js", runDate()); ``` --- src/checks/fastlyKeyExpiration.check.js | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/checks/fastlyKeyExpiration.check.js b/src/checks/fastlyKeyExpiration.check.js index 0bbd089..8003910 100644 --- a/src/checks/fastlyKeyExpiration.check.js +++ b/src/checks/fastlyKeyExpiration.check.js @@ -1,9 +1,10 @@ const fetch = require('node-fetch'); -const moment = require('moment'); const logger = require('@dotcom-reliability-kit/logger'); const Check = require('./check'); const status = require('./status'); +const twoWeeksInDays = 2 * 7; + const fastlyApiEndpoint = 'https://api.fastly.com/tokens/self'; const defaultPanicGuide = 'Generate a new key in your own Fastly account with the same permissions and update it in Vault or your app'; @@ -79,8 +80,15 @@ class FastlyKeyExpirationCheck extends Check { } parseStringDate(stringDate) { - const date = moment(stringDate, 'YYYY-MM-DDTHH:mm:ssZ'); - if (!date.isValid()) { + let dateIsValid = true; + let date; + try { + date = new Date(stringDate); + dateIsValid = Number.isNaN(date.getDate()); + } catch (error) { + dateIsValid = false; + } + if (!dateIsValid) { logger.warn(`Invalid Fastly Key expiration date ${stringDate}`); this.setState(this.states.FAILED_DATE); throw new Error('Invalid date'); @@ -95,12 +103,13 @@ class FastlyKeyExpirationCheck extends Check { } checkExpirationDate(expirationDate) { - const now = moment(); - const limitDate = moment().add(2, 'weeks'); + const now = new Date(); + const limitDate = new Date(); + limitDate.setDate(now.getDate() + twoWeeksInDays); switch (true) { - case expirationDate.isAfter(limitDate): + case expirationDate > limitDate: return this.states.PASSED; - case expirationDate.isBefore(now): + case expirationDate < now: return this.states.FAILED_URGENT_VALIDATION; default: return this.states.FAILED_VALIDATION; From b7023efabed4e1582cbab6929f29cde1bc1e6abf Mon Sep 17 00:00:00 2001 From: Rowan Manning <138944+rowanmanning@users.noreply.github.com> Date: Thu, 29 Feb 2024 16:14:52 +0000 Subject: [PATCH 3/4] Uninstall moment - it's no longer used --- package-lock.json | 14 -------------- package.json | 1 - 2 files changed, 15 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4235450..92da71c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,6 @@ "@dotcom-reliability-kit/logger": "^3.0.3", "aws-sdk": "^2.6.10", "fetchres": "^1.5.1", - "moment": "^2.29.4", "ms": "^2.0.0", "node-fetch": "^2.6.7" }, @@ -7685,14 +7684,6 @@ "integrity": "sha1-z4tP9PKWQGdNbN0CsOO8UjwrvcA=", "dev": true }, - "node_modules/moment": { - "version": "2.29.4", - "resolved": "https://registry.npmjs.org/moment/-/moment-2.29.4.tgz", - "integrity": "sha512-5LC9SOxjSc2HF6vO2CyuTDNivEdoz2IvyJJGj6X8DJ0eFyfszE0QiEd+iXmBvUP3WHxSjFH/vIsA0EN00cgr8w==", - "engines": { - "node": "*" - } - }, "node_modules/ms": { "version": "2.1.3", "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz", @@ -18570,11 +18561,6 @@ "integrity": "sha1-z4tP9PKWQGdNbN0CsOO8UjwrvcA=", "dev": true }, - "moment": { - "version": "2.29.4", - "resolved": "https://registry.npmjs.org/moment/-/moment-2.29.4.tgz", - "integrity": "sha512-5LC9SOxjSc2HF6vO2CyuTDNivEdoz2IvyJJGj6X8DJ0eFyfszE0QiEd+iXmBvUP3WHxSjFH/vIsA0EN00cgr8w==" - }, "ms": { "version": "2.1.3", "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz", diff --git a/package.json b/package.json index 01b0ed0..f8b7d6f 100644 --- a/package.json +++ b/package.json @@ -42,7 +42,6 @@ "@dotcom-reliability-kit/logger": "^3.0.3", "aws-sdk": "^2.6.10", "fetchres": "^1.5.1", - "moment": "^2.29.4", "ms": "^2.0.0", "node-fetch": "^2.6.7" }, From 4ad01ad32976413a2f84d2ee6a635d728bd2278f Mon Sep 17 00:00:00 2001 From: Rowan Manning <138944+rowanmanning@users.noreply.github.com> Date: Thu, 29 Feb 2024 16:30:35 +0000 Subject: [PATCH 4/4] Fix the tests Nothing wrong with the date stuff, just some silly logic errors --- src/checks/fastlyKeyExpiration.check.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/checks/fastlyKeyExpiration.check.js b/src/checks/fastlyKeyExpiration.check.js index 8003910..24d1335 100644 --- a/src/checks/fastlyKeyExpiration.check.js +++ b/src/checks/fastlyKeyExpiration.check.js @@ -84,7 +84,7 @@ class FastlyKeyExpirationCheck extends Check { let date; try { date = new Date(stringDate); - dateIsValid = Number.isNaN(date.getDate()); + dateIsValid = !Number.isNaN(date.getDate()); } catch (error) { dateIsValid = false; } @@ -109,7 +109,7 @@ class FastlyKeyExpirationCheck extends Check { switch (true) { case expirationDate > limitDate: return this.states.PASSED; - case expirationDate < now: + case expirationDate <= now: return this.states.FAILED_URGENT_VALIDATION; default: return this.states.FAILED_VALIDATION;