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

Improve error messages for external config files #330

Merged
merged 11 commits into from Nov 16, 2022
16 changes: 13 additions & 3 deletions __tests__/config.test.ts
Expand Up @@ -113,7 +113,7 @@ test('it reads an external config file', async () => {

test('raises an error when the the config file was not found', async () => {
setInput('config-file', 'fixtures/i-dont-exist')
await expect(readConfig()).rejects.toThrow(/Unable to fetch config file/)
await expect(readConfig()).rejects.toThrow(/Unable to fetch/)
})

test('it parses options from both sources', async () => {
Expand Down Expand Up @@ -232,6 +232,16 @@ test('it is not possible to disable both checks', async () => {
)
})

test('it supports comma-separated lists', async () => {
setInput(
'config-file',
'./__tests__/fixtures/inline-license-config-sample.yml'
)
let config = await readConfig()

expect(config.allow_licenses).toEqual(['MIT', 'GPL-2.0-only'])
})

describe('licenses that are not valid SPDX licenses', () => {
beforeAll(() => {
jest.spyOn(Utils, 'isSPDXValid').mockReturnValue(false)
Expand All @@ -240,14 +250,14 @@ describe('licenses that are not valid SPDX licenses', () => {
test('it raises an error for invalid licenses in allow-licenses', async () => {
setInput('allow-licenses', ' BSD, GPL 2')
await expect(readConfig()).rejects.toThrow(
'Invalid license(s) in allow-licenses: BSD, GPL 2'
'Invalid license(s) in allow-licenses: BSD,GPL 2'
)
})

test('it raises an error for invalid licenses in deny-licenses', async () => {
setInput('deny-licenses', ' BSD, GPL 2')
await expect(readConfig()).rejects.toThrow(
'Invalid license(s) in deny-licenses: BSD, GPL 2'
'Invalid license(s) in deny-licenses: BSD,GPL 2'
)
})
})
1 change: 1 addition & 0 deletions __tests__/fixtures/inline-license-config-sample.yml
@@ -0,0 +1 @@
allow-licenses: MIT, GPL-2.0-only
22 changes: 19 additions & 3 deletions dist/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

32 changes: 27 additions & 5 deletions src/config.ts
Expand Up @@ -80,12 +80,11 @@ function validateLicenses(
if (licenses === undefined) {
return
}

const invalid_licenses = licenses.filter(license => !isSPDXValid(license))

if (invalid_licenses.length > 0) {
throw new Error(
`Invalid license(s) in ${key}: ${invalid_licenses.join(', ')}`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the formatting since it was making it hard to debug failures. This resulted in a couple of changes in the tests where we perform string comparisons.

)
throw new Error(`Invalid license(s) in ${key}: ${invalid_licenses}`)
}
}

Expand Down Expand Up @@ -113,18 +112,41 @@ async function readConfigFile(
}
return parseConfigFile(data)
} catch (error) {
core.debug(error as string)
throw new Error('Unable to fetch config file')
throw new Error(
`Unable to fetch or parse config file: ${(error as Error).message}`
)
}
}

function parseConfigFile(configData: string): ConfigurationOptionsPartial {
try {
const data = YAML.parse(configData)

// These are the options that we support where the user can provide
// either a YAML list or a comma-separated string.
const listKeys = [
'allow-licenses',
'deny-licenses',
'fail-on-scopes',
'allow-ghsas'
]

for (const key of Object.keys(data)) {
// strings can contain list values (e.g. 'MIT, Apache-2.0'). In this
// case we need to parse that into a list (e.g. ['MIT', 'Apache-2.0']).
if (listKeys.includes(key)) {
const val = data[key]

if (typeof val === 'string') {
data[key] = val.split(',').map(x => x.trim())
}
}

// perform SPDX validation
if (key === 'allow-licenses' || key === 'deny-licenses') {
validateLicenses(key, data[key])
}

// get rid of the ugly dashes from the actions conventions
if (key.includes('-')) {
data[key.replace(/-/g, '_')] = data[key]
Expand Down