From 24eea731e3752830604c65e8f4262169e6a023b7 Mon Sep 17 00:00:00 2001 From: David Date: Mon, 20 Jan 2025 22:06:42 +1100 Subject: [PATCH 1/7] Initial version - S3 support --- package-lock.json | 4 +- package.json | 2 +- .../validate-name-and-resource-length.spec.ts | 148 ++++++++++++++++++ .../validate-name-and-resource-length.ts | 98 ++++++++++++ 4 files changed, 249 insertions(+), 3 deletions(-) create mode 100644 src/publish-beta/validate-name-and-resource-length.spec.ts create mode 100644 src/publish-beta/validate-name-and-resource-length.ts diff --git a/package-lock.json b/package-lock.json index aa29bcb..c207b19 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@checkdigit/github-actions", - "version": "2.1.0", + "version": "2.2.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@checkdigit/github-actions", - "version": "2.1.0", + "version": "2.2.0", "license": "MIT", "dependencies": { "@actions/core": "^1.10.1", diff --git a/package.json b/package.json index 4d6e469..6fde5b6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@checkdigit/github-actions", - "version": "2.1.0", + "version": "2.2.0", "description": " Provides supporting operations for github action builds.", "author": "Check Digit, LLC", "license": "MIT", diff --git a/src/publish-beta/validate-name-and-resource-length.spec.ts b/src/publish-beta/validate-name-and-resource-length.spec.ts new file mode 100644 index 0000000..01f320d --- /dev/null +++ b/src/publish-beta/validate-name-and-resource-length.spec.ts @@ -0,0 +1,148 @@ +// publish-beta/validate-name-and-resource-length.spec.ts + +import { strict as assert } from 'node:assert'; +import { describe, it } from '@jest/globals'; + +import { type PackageJSON, validateNameAndResourceLength } from './validate-name-and-resource-length'; + +describe('Test name and resource length', () => { + it('No services property', async () => { + const packageJSON: PackageJSON = {}; + await assert.doesNotReject(validateNameAndResourceLength(packageJSON)); + }); + + it('No resources property', async () => { + const packageJSON: PackageJSON = { + service: { + name: 'TestName', + }, + }; + await assert.doesNotReject(validateNameAndResourceLength(packageJSON)); + }); + + it('No aws resources property', async () => { + const packageJSON: PackageJSON = { + service: { + name: 'TestName', + resources: {}, + }, + }; + await assert.doesNotReject(validateNameAndResourceLength(packageJSON)); + }); + + it('Empty aws resources property', async () => { + const packageJSON: PackageJSON = { + service: { + name: 'TestName', + resources: { + aws: {}, + }, + }, + }; + await assert.doesNotReject(validateNameAndResourceLength(packageJSON)); + }); + + it('Empty S3 resources property', async () => { + const packageJSON: PackageJSON = { + service: { + name: 'TestName', + resources: { + aws: { + s3: {}, + }, + }, + }, + }; + await assert.doesNotReject(validateNameAndResourceLength(packageJSON)); + }); + + it('Name all within valid length', async () => { + const packageJSON: PackageJSON = { + service: { + name: 'TestName', + resources: { + aws: { + s3: { + 'valid-s3-name': { + Type: 'AWS::S3::Bucket', + Properties: { + BucketName: 'valid-s3-name', + }, + }, + }, + }, + }, + }, + }; + await assert.doesNotReject(validateNameAndResourceLength(packageJSON)); + }); + + it('Service name too long', async () => { + const packageJSON: PackageJSON = { + service: { + name: 'TestNameThatIsTooLong1', + resources: { + aws: { + s3: { + 'valid-s3-name': { + Type: 'AWS::S3::Bucket', + Properties: { + BucketName: 'valid-s3-name', + }, + }, + }, + }, + }, + }, + }; + await assert.rejects(validateNameAndResourceLength(packageJSON)); + }); + + it('Service name too long - except service', async () => { + const packageJSON: PackageJSON = { + service: { + name: 'teampay-vendor-management', + resources: { + aws: { + s3: { + 'valid-s3-name': { + Type: 'AWS::S3::Bucket', + Properties: { + BucketName: 'valid-s3-name', + }, + }, + }, + }, + }, + }, + }; + await assert.doesNotReject(validateNameAndResourceLength(packageJSON)); + }); + + it('S3 bucket name too long', async () => { + const packageJSON: PackageJSON = { + service: { + name: 'TestName', + resources: { + aws: { + s3: { + bucket1: { + Type: 'AWS::S3::Bucket', + Properties: { + BucketName: 'valid name', + }, + }, + bucket2: { + Type: 'AWS::S3::Bucket', + Properties: { + BucketName: 'invalid-s3-bucket-length-01', + }, + }, + }, + }, + }, + }, + }; + await assert.rejects(validateNameAndResourceLength(packageJSON)); + }); +}); diff --git a/src/publish-beta/validate-name-and-resource-length.ts b/src/publish-beta/validate-name-and-resource-length.ts new file mode 100644 index 0000000..c342d95 --- /dev/null +++ b/src/publish-beta/validate-name-and-resource-length.ts @@ -0,0 +1,98 @@ +// publish-beta/validate-name-and-resource-length.ts + +import path from 'node:path'; +import { readFile } from 'node:fs/promises'; + +import debug from 'debug'; + +const log = debug('github-actions:publish-beta:validate-names'); + +interface S3Properties { + Type: 'AWS::S3::Bucket'; + Properties: { + BucketName: string; + }; +} + +interface Resources { + aws?: { + s3?: Record; + }; +} + +export interface PackageJSON { + service?: { + name: string; + resources?: Resources; + }; +} + +const MAXIMUM_SERVICE_NAME_LENGTH = 20; +const SERVICE_NAME_LENGTH_EXCEPTIONS = new Set([ + 'current-certification', + 'mngs-interchange-file', + 'teampay-card-management', + 'teampay-client-management', + 'teampay-merchant-terminal', + 'teampay-vendor-management', +]); // list of services with names that are longer than limit + +const MAXIMUM_S3_BUCKET_NAME_LENGTH = 25; +const S3_BUCKET_NAME_LENGTH_EXCEPTIONS = new Set(['']); // list of resources with names that are longer than limit + +export async function readPackageJSON(rootProjectDirectory: string): Promise { + const packageJSONPath = path.join(rootProjectDirectory, 'package.json'); + const packageJSON = await readFile(packageJSONPath, 'utf8'); + return JSON.parse(packageJSON) as PackageJSON; +} + +async function validateS3BucketNames(input: Resources) { + if (input.aws?.s3 === undefined) { + log('package.json does not have a service.resources.aws.s3: {} property'); + return; + } + + const s3Resources = input.aws.s3; + + const bucketNames = Object.values(s3Resources) + .map((resource) => resource.Properties.BucketName) + .filter((name) => !S3_BUCKET_NAME_LENGTH_EXCEPTIONS.has(name)) + .filter((name) => name.length > MAXIMUM_S3_BUCKET_NAME_LENGTH); + + if (bucketNames.length > 0) { + throw new Error( + `S3 bucket names are longer than ${MAXIMUM_S3_BUCKET_NAME_LENGTH} characters: ${JSON.stringify(bucketNames)}`, + ); + } +} + +export async function validateNameAndResourceLength(packageJSONWithResources: PackageJSON): Promise { + if (!packageJSONWithResources.service) { + log('package.json does not have a service: {} property'); + return; + } + + const serviceName = packageJSONWithResources.service.name; + + if (!SERVICE_NAME_LENGTH_EXCEPTIONS.has(serviceName) && serviceName.length > MAXIMUM_SERVICE_NAME_LENGTH) { + const message = `Service name ${serviceName} is longer than ${MAXIMUM_SERVICE_NAME_LENGTH} characters`; + log(message); + throw new Error(message); + } + + if (!packageJSONWithResources.service.resources?.aws) { + log('package.json does not have a service.resources.aws: {} property'); + return; + } + const resources = packageJSONWithResources.service.resources; + await validateS3BucketNames(resources); +} + +export default async function (): Promise { + log('Action start'); + + const packageJSONWithResources = await readPackageJSON(process.cwd()); + await validateNameAndResourceLength(packageJSONWithResources); + + log('Action end'); +} From 3c1063d5cdc3844304a20e651781d65e7deeb391 Mon Sep 17 00:00:00 2001 From: David Date: Tue, 21 Jan 2025 16:50:02 +1100 Subject: [PATCH 2/7] Assert has changed its output - fix with regex --- src/check-label/check-label-compare-match-semver.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/check-label/check-label-compare-match-semver.spec.ts b/src/check-label/check-label-compare-match-semver.spec.ts index 2c2edec..11ec571 100644 --- a/src/check-label/check-label-compare-match-semver.spec.ts +++ b/src/check-label/check-label-compare-match-semver.spec.ts @@ -6,9 +6,9 @@ import { describe, it } from '@jest/globals'; import { validateVersion } from './check-label'; -const assertError = 'Version is incorrect based on Pull Request label'; +const assertError = /Version is incorrect based on Pull Request label/u; // expected error message when version is incorrect - assert adds additional information to error, so regex is used -describe('compare and match semver', () => { +describe.only('compare and match semver', () => { it('Test basic patch', async () => { assert.equal(validateVersion('1.0.1', '1.0.0', 'patch'), true); From 327d7fbf7edf50d1c3cedba0c9780699c495ffbf Mon Sep 17 00:00:00 2001 From: David Date: Tue, 21 Jan 2025 16:56:28 +1100 Subject: [PATCH 3/7] lint fix --- src/check-label/check-label-compare-match-semver.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/check-label/check-label-compare-match-semver.spec.ts b/src/check-label/check-label-compare-match-semver.spec.ts index 11ec571..d89d4c1 100644 --- a/src/check-label/check-label-compare-match-semver.spec.ts +++ b/src/check-label/check-label-compare-match-semver.spec.ts @@ -8,7 +8,7 @@ import { validateVersion } from './check-label'; const assertError = /Version is incorrect based on Pull Request label/u; // expected error message when version is incorrect - assert adds additional information to error, so regex is used -describe.only('compare and match semver', () => { +describe('compare and match semver', () => { it('Test basic patch', async () => { assert.equal(validateVersion('1.0.1', '1.0.0', 'patch'), true); From a326ba9d9b34d9018834810de026c87b11d46c3b Mon Sep 17 00:00:00 2001 From: David Date: Wed, 22 Jan 2025 15:30:59 +1100 Subject: [PATCH 4/7] Comment out test --- src/validate-npm-package/validate-npm-package.spec.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/validate-npm-package/validate-npm-package.spec.ts b/src/validate-npm-package/validate-npm-package.spec.ts index 4ba94ae..2ff1eec 100644 --- a/src/validate-npm-package/validate-npm-package.spec.ts +++ b/src/validate-npm-package/validate-npm-package.spec.ts @@ -25,7 +25,10 @@ describe('validate-npm-package', () => { await verifyNpmPackage(); }, 300_000); - it('bad npm package results in error', async () => { + // this test should reject but isn't now + // this did work historically + // eslint-disable-next-line jest/no-disabled-tests + it.skip('bad npm package results in error', async () => { actionsCoreSpy.mockImplementationOnce((name) => { if (name === 'betaPackage') { return '@checkdigit/approval@2.0.0-PR.196-b041'; From b2e9cea47d087d3c9fd50c404b11814a9e4b3fae Mon Sep 17 00:00:00 2001 From: David Date: Thu, 23 Jan 2025 12:25:26 +1100 Subject: [PATCH 5/7] Fix validation tests --- src/validate-npm-package/validate-npm-package.spec.ts | 9 +++++---- src/validate-npm-package/validate-npm-package.ts | 3 +++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/validate-npm-package/validate-npm-package.spec.ts b/src/validate-npm-package/validate-npm-package.spec.ts index 2ff1eec..3171784 100644 --- a/src/validate-npm-package/validate-npm-package.spec.ts +++ b/src/validate-npm-package/validate-npm-package.spec.ts @@ -25,10 +25,11 @@ describe('validate-npm-package', () => { await verifyNpmPackage(); }, 300_000); - // this test should reject but isn't now - // this did work historically - // eslint-disable-next-line jest/no-disabled-tests - it.skip('bad npm package results in error', async () => { + // Test uses a bad version of approval package + // and requires skipLibCheck: false in tsconfig.json + // we set it manually in validate npm package as + // checkdigit/typescript-config is various versions of this setting + it('bad npm package results in error', async () => { actionsCoreSpy.mockImplementationOnce((name) => { if (name === 'betaPackage') { return '@checkdigit/approval@2.0.0-PR.196-b041'; diff --git a/src/validate-npm-package/validate-npm-package.ts b/src/validate-npm-package/validate-npm-package.ts index e66502a..09c49e1 100644 --- a/src/validate-npm-package/validate-npm-package.ts +++ b/src/validate-npm-package/validate-npm-package.ts @@ -63,6 +63,9 @@ async function generateProject(workFolder: string, packageJson: PackageJson): Pr // create tsconfig.json const tsconfigJson = { extends: '@checkdigit/typescript-config', + compilerOptions: { + skipLibCheck: false, + }, }; await fs.writeFile(`${workFolder}/tsconfig.json`, JSON.stringify(tsconfigJson, null, 2)); } From 3b74a404e33925d6b574395093f1df4a1a677150 Mon Sep 17 00:00:00 2001 From: David Date: Thu, 23 Jan 2025 13:04:27 +1100 Subject: [PATCH 6/7] Set S3 bucket length to 20 with exceptions --- .../validate-name-and-resource-length.spec.ts | 33 +++++++++++++++++-- .../validate-name-and-resource-length.ts | 16 +++++++-- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/src/publish-beta/validate-name-and-resource-length.spec.ts b/src/publish-beta/validate-name-and-resource-length.spec.ts index 01f320d..76be77e 100644 --- a/src/publish-beta/validate-name-and-resource-length.spec.ts +++ b/src/publish-beta/validate-name-and-resource-length.spec.ts @@ -98,7 +98,7 @@ describe('Test name and resource length', () => { await assert.rejects(validateNameAndResourceLength(packageJSON)); }); - it('Service name too long - except service', async () => { + it('Service name too long - exempt service', async () => { const packageJSON: PackageJSON = { service: { name: 'teampay-vendor-management', @@ -108,7 +108,7 @@ describe('Test name and resource length', () => { 'valid-s3-name': { Type: 'AWS::S3::Bucket', Properties: { - BucketName: 'valid-s3-name', + BucketName: 'valid-s3-bucket-name', }, }, }, @@ -135,7 +135,7 @@ describe('Test name and resource length', () => { bucket2: { Type: 'AWS::S3::Bucket', Properties: { - BucketName: 'invalid-s3-bucket-length-01', + BucketName: 'invalid-bucket-length', }, }, }, @@ -145,4 +145,31 @@ describe('Test name and resource length', () => { }; await assert.rejects(validateNameAndResourceLength(packageJSON)); }); + + it('S3 bucket name too long - exempt bucket', async () => { + const packageJSON: PackageJSON = { + service: { + name: 'TestName', + resources: { + aws: { + s3: { + bucket1: { + Type: 'AWS::S3::Bucket', + Properties: { + BucketName: 'valid name', + }, + }, + 'ach.teampay.armor.inbound': { + Type: 'AWS::S3::Bucket', + Properties: { + BucketName: 'ach.teampay.armor.inbound', + }, + }, + }, + }, + }, + }, + }; + await assert.doesNotReject(validateNameAndResourceLength(packageJSON)); + }); }); diff --git a/src/publish-beta/validate-name-and-resource-length.ts b/src/publish-beta/validate-name-and-resource-length.ts index c342d95..bf0d8c9 100644 --- a/src/publish-beta/validate-name-and-resource-length.ts +++ b/src/publish-beta/validate-name-and-resource-length.ts @@ -37,8 +37,20 @@ const SERVICE_NAME_LENGTH_EXCEPTIONS = new Set([ 'teampay-vendor-management', ]); // list of services with names that are longer than limit -const MAXIMUM_S3_BUCKET_NAME_LENGTH = 25; -const S3_BUCKET_NAME_LENGTH_EXCEPTIONS = new Set(['']); // list of resources with names that are longer than limit +const MAXIMUM_S3_BUCKET_NAME_LENGTH = 20; +const S3_BUCKET_NAME_LENGTH_EXCEPTIONS = new Set([ + 'abcorp.vault.outbound', + 'ach.inbound.tokenized', + 'payment.vault.inbound', + 'arroweye.vault.inbound', + 'payment.vault.outbound', + 'star.inbound.tokenized', + 'arroweye.vault.outbound', + 'choice.mc.scheme.report', + 'mastercard.armor.inbound', + 'ach.summary.armor.inbound', + 'ach.teampay.armor.inbound', +]); // list of resources with names that are longer than limit export async function readPackageJSON(rootProjectDirectory: string): Promise { const packageJSONPath = path.join(rootProjectDirectory, 'package.json'); From 4f82f935638d4f2643fdb9e91a92de906bc93f31 Mon Sep 17 00:00:00 2001 From: David Date: Tue, 4 Feb 2025 13:41:21 +1100 Subject: [PATCH 7/7] Exceptions come in via environment variables --- .../validate-name-and-resource-length.spec.ts | 36 +++++++++++++++++++ .../validate-name-and-resource-length.ts | 30 ++++------------ 2 files changed, 43 insertions(+), 23 deletions(-) diff --git a/src/publish-beta/validate-name-and-resource-length.spec.ts b/src/publish-beta/validate-name-and-resource-length.spec.ts index 76be77e..f11b2a6 100644 --- a/src/publish-beta/validate-name-and-resource-length.spec.ts +++ b/src/publish-beta/validate-name-and-resource-length.spec.ts @@ -99,6 +99,7 @@ describe('Test name and resource length', () => { }); it('Service name too long - exempt service', async () => { + process.env['SERVICE_NAME_LENGTH_EXCEPTION'] = 'teampay-vendor-management'; const packageJSON: PackageJSON = { service: { name: 'teampay-vendor-management', @@ -147,6 +148,7 @@ describe('Test name and resource length', () => { }); it('S3 bucket name too long - exempt bucket', async () => { + process.env['S3_BUCKET_NAME_LENGTH_EXCEPTIONS'] = 'ach.teampay.armor.inbound'; const packageJSON: PackageJSON = { service: { name: 'TestName', @@ -172,4 +174,38 @@ describe('Test name and resource length', () => { }; await assert.doesNotReject(validateNameAndResourceLength(packageJSON)); }); + + it('S3 bucket name too long - multiple exempt bucket', async () => { + process.env['S3_BUCKET_NAME_LENGTH_EXCEPTIONS'] = 'mastercard.armor.inbound,ach.teampay.armor.inbound'; + const packageJSON: PackageJSON = { + service: { + name: 'TestName', + resources: { + aws: { + s3: { + bucket1: { + Type: 'AWS::S3::Bucket', + Properties: { + BucketName: 'valid name', + }, + }, + 'ach.teampay.armor.inbound': { + Type: 'AWS::S3::Bucket', + Properties: { + BucketName: 'ach.teampay.armor.inbound', + }, + }, + 'mastercard.armor.inbound': { + Type: 'AWS::S3::Bucket', + Properties: { + BucketName: 'mastercard.armor.inbound', + }, + }, + }, + }, + }, + }, + }; + await assert.doesNotReject(validateNameAndResourceLength(packageJSON)); + }); }); diff --git a/src/publish-beta/validate-name-and-resource-length.ts b/src/publish-beta/validate-name-and-resource-length.ts index bf0d8c9..1f66eb9 100644 --- a/src/publish-beta/validate-name-and-resource-length.ts +++ b/src/publish-beta/validate-name-and-resource-length.ts @@ -28,29 +28,8 @@ export interface PackageJSON { } const MAXIMUM_SERVICE_NAME_LENGTH = 20; -const SERVICE_NAME_LENGTH_EXCEPTIONS = new Set([ - 'current-certification', - 'mngs-interchange-file', - 'teampay-card-management', - 'teampay-client-management', - 'teampay-merchant-terminal', - 'teampay-vendor-management', -]); // list of services with names that are longer than limit const MAXIMUM_S3_BUCKET_NAME_LENGTH = 20; -const S3_BUCKET_NAME_LENGTH_EXCEPTIONS = new Set([ - 'abcorp.vault.outbound', - 'ach.inbound.tokenized', - 'payment.vault.inbound', - 'arroweye.vault.inbound', - 'payment.vault.outbound', - 'star.inbound.tokenized', - 'arroweye.vault.outbound', - 'choice.mc.scheme.report', - 'mastercard.armor.inbound', - 'ach.summary.armor.inbound', - 'ach.teampay.armor.inbound', -]); // list of resources with names that are longer than limit export async function readPackageJSON(rootProjectDirectory: string): Promise { const packageJSONPath = path.join(rootProjectDirectory, 'package.json'); @@ -63,6 +42,10 @@ async function validateS3BucketNames(input: Resources) { log('package.json does not have a service.resources.aws.s3: {} property'); return; } + // allow override of s3 bucket name length from action environment + const listOfS3BucketsFromEnvironment = process.env['S3_BUCKET_NAME_LENGTH_EXCEPTIONS'] ?? undefined; + const S3_BUCKET_NAME_LENGTH_EXCEPTIONS = + listOfS3BucketsFromEnvironment === undefined ? new Set() : new Set(listOfS3BucketsFromEnvironment.split(',')); const s3Resources = input.aws.s3; @@ -83,10 +66,11 @@ export async function validateNameAndResourceLength(packageJSONWithResources: Pa log('package.json does not have a service: {} property'); return; } - + // allow override of service name length from action environment + const SERVICE_NAME_LENGTH_EXCEPTION = process.env['SERVICE_NAME_LENGTH_EXCEPTION'] ?? undefined; const serviceName = packageJSONWithResources.service.name; - if (!SERVICE_NAME_LENGTH_EXCEPTIONS.has(serviceName) && serviceName.length > MAXIMUM_SERVICE_NAME_LENGTH) { + if (SERVICE_NAME_LENGTH_EXCEPTION !== serviceName && serviceName.length > MAXIMUM_SERVICE_NAME_LENGTH) { const message = `Service name ${serviceName} is longer than ${MAXIMUM_SERVICE_NAME_LENGTH} characters`; log(message); throw new Error(message);