Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding plugin metadata to validator #2297

Merged
merged 5 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions govuk-prototype-kit.config.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
{
"meta": {
"description": "Quickly make interactive, accessible and realistic prototypes of GOV.UK services.",
"urls": {
"documentation": "https://prototype-kit.service.gov.uk/?linkFromKit={{kitVersion}}%3A{{version}}",
"versionHistory": "https://github.com/alphagov/govuk-prototype-kit/releases",
"releaseNotes": "https://github.com/alphagov/govuk-prototype-kit/releases/tag/v{{version}}"
}
},
"assets": [
"/lib/assets/images",
"/lib/assets/javascripts/optional",
Expand Down
115 changes: 94 additions & 21 deletions lib/plugins/plugin-validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,19 @@ const ansiColors = require('ansi-colors')

const errors = []

function getKnownKeys () {
const knownKeys = [
'assets',
'importNunjucksMacrosInto',
'nunjucksPaths',
'nunjucksMacros',
'nunjucksFilters',
'sass',
'scripts',
'stylesheets',
'templates',
'pluginDependencies'
]
return knownKeys
}
const knownKeys = [
'assets',
'importNunjucksMacrosInto',
'nunjucksPaths',
'nunjucksMacros',
'nunjucksFilters',
'sass',
'scripts',
'stylesheets',
'templates',
'pluginDependencies',
'meta'
]

function checkPathExists (executionPath, pathToValidate, key) {
const absolutePathToValidate = path.join(executionPath, pathToValidate)
Expand Down Expand Up @@ -46,9 +44,12 @@ function checkNunjucksMacroExists (executionPath, nunjucksFileName, nunjucksPath
if (!nunjucksPathExists) errors.push(`The nunjucks file '${nunjucksFileName}' does not exist`)
}

function reportInvalidKeys (invalidKeys) {
errors.push(`The following invalid keys exist in your config: ${invalidKeys}`)
}

function validateConfigKeys (pluginConfig) {
console.log('Config file exists, validating contents.')
const knownKeys = getKnownKeys()
const invalidKeys = []

const validKeysPluginConfig = Object.fromEntries(Object.entries(pluginConfig).filter(([key]) => {
Expand All @@ -61,12 +62,81 @@ function validateConfigKeys (pluginConfig) {

// Add any invalid config keys to the list of errors
if (invalidKeys.length > 0) {
errors.push(`The following invalid keys exist in your config: ${invalidKeys}`)
reportInvalidKeys(invalidKeys)
}

return validKeysPluginConfig
}

function reportUnknownKeys (objectToEvaluate, allowedKeys, keyPath) {
const invalidMetaUrlKeys = Object.keys(objectToEvaluate).filter(key => !allowedKeys.includes(key)).map(key => `${keyPath || ''}${key}`)
if (invalidMetaUrlKeys.length > 0) {
reportInvalidKeys(invalidMetaUrlKeys)
}
}

function isValidUrl (url) {
if (!url.startsWith('https://') && !url.startsWith('http://')) {
return false
}
if (!url.split('://')[1].split('/')[0].includes('.')) {
return false
}
return true
}

function validateMetaUrls (metaUrls) {
if (typeof metaUrls === 'undefined') {
return
}

if (typeof metaUrls !== 'object') {
errors.push('The meta.urls must be an object if entered')
return
}

const keyPath = 'meta.urls.'
reportUnknownKeys(metaUrls, [
'documentation',
'releaseNotes',
'versionHistory'
], keyPath)

const allowedVariables = ['version', 'kitVersion'].map(variable => `{{${variable}}}`)

Object.keys(metaUrls).forEach(key => {
const url = metaUrls[key]
if (!isValidUrl(url)) {
errors.push(`meta.urls.${key} doesn't appear to be a public URL`)
}

const unknownVariables = (url.match(/\{\{(\w+)\}\}/g) || []).map(x => `${x}`).filter(variable => !allowedVariables.includes(variable))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Webstorm is reporting Redundant character escape '\}' in RegExp. The url.match can be replaced with url.match(/\{\{(\w+)}}/g)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, would you mind re-reviewing?


unknownVariables.forEach(variable => errors.push(`The URL ${keyPath}${key} contains an unknown variable ${variable}`))
})
}

function validateMeta (meta) {
const metaKeys = ['urls', 'description']

if (typeof meta !== 'object') {
errors.push('The meta must be an object if entered')
return
}

if (typeof meta.description !== 'string' && typeof meta.description !== 'undefined') {
errors.push('The meta.description must be a string if entered')
return
}

const invalidMetaUrlKeys = Object.keys(meta).filter(key => !metaKeys.includes(key)).map(key => `meta.${key}`)
if (invalidMetaUrlKeys.length > 0) {
reportInvalidKeys(invalidMetaUrlKeys)
}

validateMetaUrls(meta.urls)
}

function validatePluginDependency (key, configEntry) {
if (typeof configEntry === 'string') {
return
Expand All @@ -82,15 +152,15 @@ function validatePluginDependency (key, configEntry) {
}
// The minVersion is optional but must be a string if entered
if (Object.keys(configEntry).includes('minVersion') && typeof minVersion !== 'string') {
errors.push(`In section ${key}, the minVersion '${minVersion}' should be a valid version`)
errors.push(`In section ${key}, the minVersion '${minVersion}' should be a valid version if entered`)
}
// The maxVersion is optional but must be a string if entered
if (Object.keys(configEntry).includes('maxVersion') && typeof maxVersion !== 'string' && typeof maxVersion !== 'undefined') {
errors.push(`In section ${key}, the maxVersion '${maxVersion}' should be a valid version if entered`)
}
}

function validateConfigPaths (pluginConfig, executionPath) {
function validateConfigurationValues (pluginConfig, executionPath) {
console.log('Validating whether config paths meet criteria.')
const keysToValidate = Object.keys(pluginConfig)

Expand All @@ -103,7 +173,9 @@ function validateConfigPaths (pluginConfig, executionPath) {

criteriaConfig.forEach((configEntry) => {
try {
if (key === 'pluginDependencies') {
if (key === 'meta') {
validateMeta(configEntry)
} else if (key === 'pluginDependencies') {
validatePluginDependency(key, configEntry)
} else if ((key === 'templates' && configEntry.path[0] === '/') ||
(key === 'scripts' && configEntry.path !== undefined && configEntry.path[0] === '/')) {
Expand Down Expand Up @@ -152,7 +224,7 @@ async function validatePlugin (executionPath) {
} else {
// Perform the rest of the checks if the config file has contents
const validKeysPluginConfig = validateConfigKeys(pluginConfig)
validateConfigPaths(validKeysPluginConfig, executionPath)
validateConfigurationValues(validKeysPluginConfig, executionPath)
}
}
} else {
Expand Down Expand Up @@ -185,5 +257,6 @@ module.exports = {
clearErrors,
getErrors,
validatePlugin,
validateMeta,
validatePluginDependency
}
69 changes: 67 additions & 2 deletions lib/plugins/plugin-validator.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { validatePluginDependency, clearErrors, getErrors } = require('./plugin-validator')
const { validatePluginDependency, clearErrors, getErrors, validateMeta } = require('./plugin-validator')

describe('plugin-validator', () => {
beforeEach(() => {
Expand Down Expand Up @@ -56,9 +56,74 @@ describe('plugin-validator', () => {
maxVersion: 100
})
expect(getErrors()).toEqual([
'In section pluginDependencies, the minVersion \'null\' should be a valid version',
'In section pluginDependencies, the minVersion \'null\' should be a valid version if entered',
'In section pluginDependencies, the maxVersion \'100\' should be a valid version if entered'
])
})
})
describe('meta', () => {
it('should allow all the valid options', () => {
validateMeta({
description: 'Hello world',
urls: {
documentation: 'https://example.com/',
releaseNotes: 'http://example.com/',
versionHistory: 'https://example.com/'
}
})
expect(getErrors()).toEqual([])
})
it('should check that urls is an object', () => {
validateMeta({
urls: 'abc'
})
expect(getErrors()).toEqual(['The meta.urls must be an object if entered'])
})
it('should check that urls contain a valid protocol', () => {
validateMeta({
urls: {
documentation: 'ftp://example.com'
}
})
expect(getErrors()).toEqual(['meta.urls.documentation doesn\'t appear to be a public URL'])
})
it('should fail if URL doesn\'t contain a tld', () => {
validateMeta({
urls: {
releaseNotes: 'https://example'
}
})
expect(getErrors()).toEqual(['meta.urls.releaseNotes doesn\'t appear to be a public URL'])
})
it('should fail if URL doesn\'t contain a tld but does contain a path with a dot', () => {
validateMeta({
urls: {
versionHistory: 'https://example/index.html'
}
})
expect(getErrors()).toEqual(['meta.urls.versionHistory doesn\'t appear to be a public URL'])
})
it('should fail if URL doesn\'t contain a tld but does contain a path with a dot', () => {
validateMeta({
urls: {
versionHistory: 'https://example/index.html'
}
})
expect(getErrors()).toEqual(['meta.urls.versionHistory doesn\'t appear to be a public URL'])
})
it('should fail if unknown URL provided', () => {
validateMeta({
urls: {
versionHistory2: 'https://example.com/index.html'
}
})
expect(getErrors()).toEqual(['The following invalid keys exist in your config: meta.urls.versionHistory2'])
})
it('should fail if description is not a string', () => {
validateMeta({
description: {}
})
expect(getErrors()).toEqual(['The meta.description must be a string if entered'])
})
})
})
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"cypress:e2e:plugins": "cypress run --spec \"cypress/e2e/plugins/*/*\"",
"cypress:e2e:errors": "cypress run --spec \"cypress/e2e/errors/*/*\"",
"test:heroku": "cross-env KIT_TEST_DIR=tmp/test-prototype start-server-and-test start:test:heroku 3000 cypress:e2e:smoke",
"test:acceptance": "npm run test:acceptance:dev && npm run test:acceptance:prod && npm run test:acceptance:smoke && npm run test:acceptance:styles && npm run test:acceptance:plugins && npm run test:acceptance:errors",
"test:acceptance:dev": "cross-env KIT_TEST_DIR=tmp/test-prototype start-server-and-test start:test 3000 cypress:e2e:dev",
"test:acceptance:prod": "cross-env PASSWORD=password KIT_TEST_DIR=tmp/test-prototype start-server-and-test start:test:prod 3000 cypress:e2e:prod",
"test:acceptance:smoke": "cross-env KIT_TEST_DIR=tmp/test-prototype start-server-and-test start:test 3000 cypress:e2e:smoke",
Expand All @@ -55,6 +56,7 @@
"test:acceptance:open": "cross-env KIT_TEST_DIR=tmp/test-prototype start-server-and-test start:test 3000 cypress:open",
"test:unit": "jest --detectOpenHandles lib bin migrator",
"test:integration": "cross-env CREATE_KIT_TIMEOUT=240000 jest --detectOpenHandles --testTimeout=60000 __tests__",
"test:full": "npm test && npm run test:acceptance",
"test": "npm run test:unit && npm run test:integration && npm run lint"
},
"dependencies": {
Expand Down