Skip to content

Commit 8e7f15b

Browse files
ci: run dash-licenses as part of GH workflow (eclipse-theia#9953)
Add a "3PP FOSS license check" GitHub workflow that uses the Eclipse Foundation dash-licences tool to validate the licenses of our npm dependencies. One can also run the license check locally with command: "yarn license:check" I have considered 3 alternatives: 1) Add the license check as a step of the build job of the build workflow. The license check step needs to report success or it will fail the build job and the publish job (when it applies). If the license check reports success, it's buried in the multiple steps of its job, not discoverable from the PR page. 2) Add the license check as a new job in the build workflow. This works better but there is redundancy in the job - we need to re-install node, checkout the repo, etc. One advantage of this approach is that the license check has visibility in the PR's CI checks section and can be made to fail without impacting the other jobs. 3) _(chosen)_ Add a new workflow for the license check. Like option 2 this gives the license check visibility in the PR's CI checks section and the job can fail without impacting other workflows/jobs. This option has the additional benefit that it's quite generic (for npm/yarn projects) and could be re-used more easily, without changing the main workflow of the adopting repo. --- It is worth noting that there are some dependencies that are not yet completely cleared in this repository (which most likely come from the time before Theia was an Eclipse project). The tool exits with an error set because of those. The solution implemented here is to have a "baseline" of dependencies known to make `dash-licenses` trip, and to ignore the scan error only if everything that is being reported is part of the baseline. This means that the GitHub Action check on PRs should only fail if new "restricted" dependencies are introduced, which indicates that actions must be taken, such as creating CQs. Even when green it is worth checking the logs of the check since it will report any dependency that is no longer required in the baseline: This will happen once the various databases are updated following the CQs we fill! Signed-off-by: Marc Dumais <marc.dumais@ericsson.com> Co-authored-by: Paul Marechal <paul.marechal@ericsson.com>
1 parent 23806ee commit 8e7f15b

File tree

7 files changed

+315
-22
lines changed

7 files changed

+315
-22
lines changed

.github/PULL_REQUEST_TEMPLATE.md

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@ Thank you for your Pull Request. Please provide a description and review
33
the requirements below.
44
55
Contributors guide: https://github.com/theia-ide/theia/blob/master/CONTRIBUTING.md
6-
-->
76
8-
<!--
97
Note: Security vulnerabilities should not be disclosed on GitHub, through a PR or any
108
other means. See SECURITY.md at the root of this repository, to learn how to report
119
vulnerabilities.
@@ -19,9 +17,8 @@ vulnerabilities.
1917

2018
#### Review checklist
2119

22-
- [ ] as an author, I have thoroughly tested my changes and carefully followed [the review guidelines](https://github.com/theia-ide/theia/blob/master/doc/pull-requests.md#requesting-a-review)
20+
- [ ] As an author, I have thoroughly tested my changes and carefully followed [the review guidelines](https://github.com/theia-ide/theia/blob/master/doc/pull-requests.md#requesting-a-review)
2321

2422
#### Reminder for reviewers
2523

26-
- as a reviewer, I agree to behave in accordance with [the review guidelines](https://github.com/theia-ide/theia/blob/master/doc/pull-requests.md#reviewing)
27-
24+
- As a reviewer, I agree to behave in accordance with [the review guidelines](https://github.com/theia-ide/theia/blob/master/doc/pull-requests.md#reviewing)
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
name: 3PP License Check
2+
3+
on:
4+
push:
5+
branches:
6+
- master
7+
workflow_dispatch:
8+
pull_request:
9+
branches:
10+
- master
11+
schedule:
12+
- cron: '0 4 * * *' # Runs every day at 4am: https://docs.github.com/en/actions/reference/events-that-trigger-workflows#scheduled-events-schedule
13+
14+
jobs:
15+
16+
License-check:
17+
name: ${{ matrix.os }}, Node.js v${{ matrix.node }}
18+
19+
strategy:
20+
fail-fast: false
21+
matrix:
22+
os: [ubuntu-18.04]
23+
node: ['12.x']
24+
java: ['11']
25+
26+
runs-on: ${{ matrix.os }}
27+
timeout-minutes: 60
28+
29+
steps:
30+
- name: Checkout
31+
uses: actions/checkout@v2
32+
with:
33+
fetch-depth: 2
34+
35+
- name: Use Node.js ${{ matrix.node }}
36+
uses: actions/setup-node@v1
37+
with:
38+
node-version: ${{ matrix.node }}
39+
registry-url: 'https://registry.npmjs.org'
40+
41+
- name: Use Java ${{ matrix.java }}
42+
uses: actions/setup-java@v2
43+
with:
44+
distribution: 'adopt'
45+
java-version: ${{ matrix.java }}
46+
47+
- name: Run dash-licenses
48+
if: matrix.tests != 'skip'
49+
shell: bash
50+
run: |
51+
yarn license:check

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,5 @@ gh-pages
2828
dev-packages/electron/compile_commands.json
2929
*.tsbuildinfo
3030
.eslintcache
31+
scripts/download
32+
license-check-summary.txt*

doc/pull-requests.md

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,37 +16,37 @@ If a rule causes distress during discussions itself, it has to be reviewed on [t
1616

1717
<a name="pr-template"></a>
1818
- [1.](#pr-template) Each PR description has to follow the following template:
19-
```
20-
<!--
21-
Thank you for your Pull Request. Please provide a description and review
22-
the requirements below.
2319

24-
Contributors guide: https://github.com/eclipse-theia/theia/blob/master/CONTRIBUTING.md
25-
-->
20+
```md
21+
<!--
22+
Thank you for your Pull Request. Please provide a description and review
23+
the requirements below.
2624
27-
#### What it does
28-
<!-- Include relevant issues and describe how they are addressed. -->
25+
Contributors guide: https://github.com/eclipse-theia/theia/blob/master/CONTRIBUTING.md
26+
-->
2927

30-
#### How to test
31-
<!-- Explain how a reviewer can reproduce a bug, test new functionality or verify performance improvements. -->
28+
#### What it does
29+
<!-- Include relevant issues and describe how they are addressed. -->
3230

33-
#### Review checklist
31+
#### How to test
32+
<!-- Explain how a reviewer can reproduce a bug, test new functionality or verify performance improvements. -->
3433

35-
- [ ] as an author, I have thoroughly tested my changes and carefully followed [the review guidelines](https://github.com/eclipse-theia/theia/blob/master/doc/pull-requests.md#requesting-a-review)
34+
#### Review checklist
3635

37-
#### Reminder for reviewers
36+
- [ ] As an author, I have thoroughly tested my changes and carefully followed [the review guidelines](https://github.com/eclipse-theia/theia/blob/master/doc/pull-requests.md#requesting-a-review)
3837

39-
- as a reviewer, I agree to review in accordance with [the review guidelines](https://github.com/eclipse-theia/theia/blob/master/doc/pull-requests.md#reviewing)
38+
#### Reminder for reviewers
4039

41-
```
40+
- As a reviewer, I agree to review in accordance with [the review guidelines](https://github.com/eclipse-theia/theia/blob/master/doc/pull-requests.md#reviewing)
41+
```
4242

4343
<a name="design-review"></a>
4444
- [2.](#design-review) A PR can be opened early for the design review before going into the detailed implementation.
4545
- A request on the design review should be an explicit comment.
4646
- Such PR should be marked as a draft or with the WIP prefix.
4747

4848
<a name="fixups"></a>
49-
- [3.](#fixups) Changes done _after_ the PR has been opened should be kept in separate commits while the review process is not finished. This allows reviewers to re-review only the updated parts of the PR and to determine what needs to be tested again. The "fixup" commits must be squashed before merging in order to keep a clean history.
49+
- [3.](#fixups) Changes done _after_ the PR has been opened should be kept in separate commits while the review process is not finished. This allows reviewers to re-review only the updated parts of the PR and to determine what needs to be tested again. The "fixup" commits must be squashed before merging in order to keep a clean history.
5050

5151
## Requesting a Review
5252

@@ -75,6 +75,8 @@ Contributors guide: https://github.com/eclipse-theia/theia/blob/master/CONTRIBUT
7575
- [5.](#checklist-dependencies) New dependencies are justified and [verified](https://github.com/eclipse-theia/theia/wiki/Registering-CQs#wip---new-ecd-theia-intellectual-property-clearance-approach-experimental).
7676
<a name="checklist-copied-code"></a>
7777
- [6.](#checklist-copied-code) Copied code is justified and [approved via a CQ](https://github.com/eclipse-theia/theia/wiki/Registering-CQs#case-3rd-party-project-code-copiedforked-from-another-project-into-eclipse-theia-maintained-by-us).
78+
- Look closely at the GitHub actions running for your PR: the 3pp/dash license check should be green.
79+
- If red: it most likely mean you need to create a CQ.
7880
<a name="checklist-copyright"></a>
7981
- [7.](#checklist-copyright) Each new file has proper copyright with the current year and the name of contributing entity (individual or company).
8082
<a name="checklist-sign-off"></a>

license-check-baseline.json

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
{
2+
"npm/npmjs/-/autoprefixer/6.7.7": null,
3+
"npm/npmjs/-/big.js/3.2.0": null,
4+
"npm/npmjs/-/eslint-plugin-deprecation/1.2.1": "Approved as 'works-with': https://dev.eclipse.org/ipzilla/show_bug.cgi?id=22573",
5+
"npm/npmjs/-/esprima/4.0.1": null,
6+
"npm/npmjs/-/esquery/1.4.0": null,
7+
"npm/npmjs/-/extsprintf/1.4.0": null,
8+
"npm/npmjs/-/from/0.1.7": null,
9+
"npm/npmjs/-/fs-extra/4.0.3": null,
10+
"npm/npmjs/-/gauge/2.7.4": null,
11+
"npm/npmjs/-/glob/7.1.3": null,
12+
"npm/npmjs/-/jschardet/2.3.0": "Approved for Eclipse Theia: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=22481",
13+
"npm/npmjs/-/jsdom/11.12.0": "CQ under review: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=23640https://dev.eclipse.org/ipzilla/show_bug.cgi?id=23640",
14+
"npm/npmjs/-/jsmin/1.0.1": "CQ under review: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=23640https://dev.eclipse.org/ipzilla/show_bug.cgi?id=23640",
15+
"npm/npmjs/-/json-schema/0.2.3": null,
16+
"npm/npmjs/-/json5/0.5.1": null,
17+
"npm/npmjs/-/less-loader/2.2.3": null,
18+
"npm/npmjs/-/npmlog/4.1.2": null,
19+
"npm/npmjs/-/parse-json/2.2.0": null,
20+
"npm/npmjs/-/postcss-reduce-initial/1.0.1": null,
21+
"npm/npmjs/-/q/1.5.1": null,
22+
"npm/npmjs/-/rc/1.2.8": null,
23+
"npm/npmjs/-/shelljs/0.8.4": null,
24+
"npm/npmjs/-/spdx-correct/3.1.1": null,
25+
"npm/npmjs/-/spdx-license-ids/3.0.9": null,
26+
"npm/npmjs/-/style-loader/0.13.2": null,
27+
"npm/npmjs/-/through/2.3.8": null,
28+
"npm/npmjs/-/ts-md5/1.2.9": null,
29+
"npm/npmjs/-/tweetnacl/0.14.5": null,
30+
"npm/npmjs/-/uc.micro/1.0.6": null,
31+
"npm/npmjs/-/uri-js/4.4.1": null
32+
}

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@
7676
"start:electron": "yarn rebuild:electron && yarn --cwd examples/electron start",
7777
"debug:browser": "yarn rebuild:browser && yarn --cwd examples/browser start:debug",
7878
"debug:electron": "yarn rebuild:electron && yarn --cwd examples/electron start:debug",
79-
"download:plugins": "theia download:plugins"
79+
"download:plugins": "theia download:plugins",
80+
"license:check": "node scripts/check_3pp_licenses.js"
8081
},
8182
"workspaces": [
8283
"dev-packages/*",

scripts/check_3pp_licenses.js

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
/********************************************************************************
2+
* Copyright (c) 2021 Ericsson and others
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License v. 2.0 which is available at
6+
* http://www.eclipse.org/legal/epl-2.0.
7+
*
8+
* This Source Code may also be made available under the following Secondary
9+
* Licenses when the conditions for such availability set forth in the Eclipse
10+
* Public License v. 2.0 are satisfied: GNU General Public License, version 2
11+
* with the GNU Classpath Exception which is available at
12+
* https://www.gnu.org/software/classpath/license.html.
13+
*
14+
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
15+
********************************************************************************/
16+
// @ts-check
17+
18+
const cp = require('child_process');
19+
const fs = require('fs');
20+
const path = require('path');
21+
const readline = require('readline');
22+
23+
const NO_COLOR = Boolean(process.env['NO_COLOR']);
24+
const dashLicensesJar = path.resolve(__dirname, 'download/dash-licenses.jar');
25+
const dashLicensesSummary = path.resolve(__dirname, '../license-check-summary.txt');
26+
const dashLicensesBaseline = path.resolve(__dirname, '../license-check-baseline.json');
27+
const dashLicensesUrl = 'https://repo.eclipse.org/service/local/artifact/maven/redirect?r=dash-licenses&g=org.eclipse.dash&a=org.eclipse.dash.licenses&v=LATEST';
28+
29+
main().catch(error => {
30+
console.error(error);
31+
process.exit(1);
32+
});
33+
34+
async function main() {
35+
if (!fs.existsSync(dashLicensesJar)) {
36+
info('Fetching dash-licenses...');
37+
fs.mkdirSync(path.dirname(dashLicensesJar), { recursive: true });
38+
const curlError = getErrorFromStatus(spawn(
39+
'curl', ['-L', dashLicensesUrl, '-o', dashLicensesJar],
40+
));
41+
if (curlError) {
42+
error(curlError);
43+
process.exit(1);
44+
}
45+
}
46+
if (fs.existsSync(dashLicensesSummary)) {
47+
info('Backing up previous summary...');
48+
fs.renameSync(dashLicensesSummary, `${dashLicensesSummary}.old`);
49+
}
50+
info('Running dash-licenses...');
51+
const dashError = getErrorFromStatus(spawn(
52+
'java', ['-jar', dashLicensesJar, 'yarn.lock', '-batch', '50', '-timeout', '240', '-summary', dashLicensesSummary],
53+
{ stdio: ['ignore', 'ignore', 'inherit'] },
54+
));
55+
if (dashError) {
56+
warn(dashError);
57+
}
58+
const restricted = await getRestrictedDependenciesFromSummary(dashLicensesSummary);
59+
if (restricted.length > 0) {
60+
if (fs.existsSync(dashLicensesBaseline)) {
61+
info('Checking results against the baseline...');
62+
const baseline = readBaseline(dashLicensesBaseline);
63+
const unmatched = new Set(baseline.keys());
64+
const unhandled = restricted.filter(entry => {
65+
unmatched.delete(entry.dependency);
66+
return !baseline.has(entry.dependency);
67+
});
68+
if (unmatched.size > 0) {
69+
warn('Some entries in the baseline did not match anything from dash-licences output:');
70+
for (const dependency of unmatched) {
71+
console.log(magenta(`> ${dependency}`));
72+
const data = baseline.get(dependency);
73+
if (data) {
74+
console.warn(`${dependency}:`, data);
75+
}
76+
}
77+
}
78+
if (unhandled.length > 0) {
79+
error(`Found results that aren't part of the baseline!`);
80+
logRestrictedDashSummaryEntries(unhandled);
81+
process.exit(1);
82+
}
83+
} else {
84+
error(`Found unhandled restricted dependencies!`);
85+
logRestrictedDashSummaryEntries(restricted);
86+
process.exit(1);
87+
}
88+
}
89+
info('Done.');
90+
process.exit(0);
91+
}
92+
93+
/**
94+
* @param {Iterable<DashSummaryEntry>} entries
95+
* @return {void}
96+
*/
97+
function logRestrictedDashSummaryEntries(entries) {
98+
for (const { dependency: entry, license } of entries) {
99+
console.log(red(`X ${entry}, ${license}`));
100+
}
101+
}
102+
103+
/**
104+
* @param {string} summary path to the summary file.
105+
* @returns {Promise<DashSummaryEntry[]>} list of restriced dependencies.
106+
*/
107+
async function getRestrictedDependenciesFromSummary(summary) {
108+
const restricted = [];
109+
for await (const entry of readSummaryLines(summary)) {
110+
if (entry.status.toLocaleLowerCase() === 'restricted') {
111+
restricted.push(entry);
112+
}
113+
}
114+
return restricted.sort(
115+
(a, b) => a.dependency.localeCompare(b.dependency)
116+
);
117+
}
118+
119+
/**
120+
* Read each entry from dash's summary file and collect each entry.
121+
* This is essentially a cheap CSV parser.
122+
* @param {string} summary path to the summary file.
123+
* @returns {AsyncIterableIterator<DashSummaryEntry>} reading completed.
124+
*/
125+
async function* readSummaryLines(summary) {
126+
for await (const line of readline.createInterface(fs.createReadStream(summary))) {
127+
const [dependency, license, status, source] = line.split(', ');
128+
yield { dependency, license, status, source };
129+
}
130+
}
131+
132+
/**
133+
* Handle both list and object format for the baseline json file.
134+
* @param {string} baseline path to the baseline json file.
135+
* @returns {Map<string, any>} map of dependencies to ignore if restricted, value is an optional data field.
136+
*/
137+
function readBaseline(baseline) {
138+
const json = JSON.parse(fs.readFileSync(baseline, 'utf8'));
139+
if (Array.isArray(json)) {
140+
return new Map(json.map(element => [element, null]));
141+
} else if (typeof json === 'object' && json !== null) {
142+
return new Map(Object.entries(json));
143+
}
144+
console.error(`ERROR: Invalid format for "${baseline}"`);
145+
process.exit(1);
146+
}
147+
148+
/**
149+
* Spawn a process. Exits with code 1 on spawn error (e.g. file not found).
150+
* @param {string} bin
151+
* @param {string[]} args
152+
* @param {import('child_process').SpawnSyncOptions} [opts]
153+
* @returns {import('child_process').SpawnSyncReturns}
154+
*/
155+
function spawn(bin, args, opts = {}) {
156+
opts = { stdio: 'inherit', ...opts };
157+
/** @type {any} */
158+
const status = cp.spawnSync(bin, args, opts);
159+
// Add useful fields to the returned status object:
160+
status.bin = bin;
161+
status.args = args;
162+
status.opts = opts;
163+
// Abort on spawn error:
164+
if (status.error) {
165+
console.error(status.error);
166+
process.exit(1);
167+
}
168+
return status;
169+
}
170+
171+
/**
172+
* @param {import('child_process').SpawnSyncReturns} status
173+
* @returns {string | undefined} Error message if the process errored, `undefined` otherwise.
174+
*/
175+
function getErrorFromStatus(status) {
176+
if (typeof status.signal === 'string') {
177+
return `Command ${prettyCommand(status)} exited with signal: ${status.signal}`;
178+
} else if (status.status !== 0) {
179+
return `Command ${prettyCommand(status)} exited with code: ${status.status}`;
180+
}
181+
}
182+
183+
/**
184+
* @param {any} status
185+
* @param {number} [indent]
186+
* @returns {string} Pretty command with both bin and args as stringified JSON.
187+
*/
188+
function prettyCommand(status, indent = 2) {
189+
return JSON.stringify([status.bin, ...status.args], undefined, indent);
190+
}
191+
192+
function info(text) { console.warn(cyan(`INFO: ${text}`)); }
193+
function warn(text) { console.warn(yellow(`WARN: ${text}`)); }
194+
function error(text) { console.error(red(`ERROR: ${text}`)); }
195+
196+
function style(code, text) { return NO_COLOR ? text : `\x1b[${code}m${text}\x1b[0m`; }
197+
function cyan(text) { return style(96, text); }
198+
function magenta(text) { return style(95, text); }
199+
function yellow(text) { return style(93, text); }
200+
function red(text) { return style(91, text); }
201+
202+
/**
203+
* @typedef {object} DashSummaryEntry
204+
* @property {string} dependency
205+
* @property {string} license
206+
* @property {string} status
207+
* @property {string} source
208+
*/

0 commit comments

Comments
 (0)