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

feat: strict validate deps #720

Merged
merged 2 commits into from
Oct 26, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
28 changes: 27 additions & 1 deletion app/core/service/PackageManagerService.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { stat, readFile } from 'node:fs/promises';
import { strict as assert } from 'node:assert';
import {
AccessLevel,
SingletonProto,
EventBus,
Inject,
} from '@eggjs/tegg';
import { ForbiddenError, NotFoundError } from 'egg-errors';
import { BadRequestError, ForbiddenError, NotFoundError } from 'egg-errors';
import { RequireAtLeastOne } from 'type-fest';
import npa from 'npm-package-arg';
import semver from 'semver';
Expand Down Expand Up @@ -46,6 +47,7 @@
import { RegistryManagerService } from './RegistryManagerService';
import { Registry } from '../entity/Registry';
import { PackageVersionService } from './PackageVersionService';
import pMap from 'p-map';

export interface PublishPackageCmd {
// maintainer: Maintainer;
Expand Down Expand Up @@ -101,6 +103,9 @@

// support user publish private package and sync worker publish public package
async publish(cmd: PublishPackageCmd, publisher: User) {
if (this.config.cnpmcore.strictValidatePackageDeps) {
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
await this._checkPackageDepsVersion(cmd.packageJson);
}
let pkg = await this.packageRepository.findPackage(cmd.scope, cmd.name);
if (!pkg) {
pkg = Package.create({
Expand Down Expand Up @@ -978,4 +983,25 @@
}
return data;
}

private async _checkPackageDepsVersion(pkgJSON: PackageJSONType) {
// 只校验 dependencies
// devDependencies、optionalDependencies、peerDependencies 不会影响依赖安装 不在这里进行校验
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
const { dependencies } = pkgJSON;
await pMap(Object.entries(dependencies || {}), async ([ fullname, spec ]) => {
try {
const specResult = npa(`${fullname}@${spec}`);
// 对于 git、alias、file 等类型的依赖,不进行版本校验
if (![ 'range', 'tag', 'version' ].includes(specResult.type)) {
return;
}

Check warning on line 997 in app/core/service/PackageManagerService.ts

View check run for this annotation

Codecov / codecov/patch

app/core/service/PackageManagerService.ts#L996-L997

Added lines #L996 - L997 were not covered by tests
const pkgVersion = await this.packageVersionService.getVersion(npa(`${fullname}@${spec}`));
assert(pkgVersion);
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
} catch (e) {
throw new BadRequestError(`deps ${fullname}@${spec} not found`);
Copy link
Contributor

Choose a reason for hiding this comment

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

失败以后这个 job 走默认的重试逻辑吗?

Copy link
Member Author

Choose a reason for hiding this comment

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

嗯 是的 默认的失败逻辑会自动放到队列最后 + 最大次数限制。
主要针对场景是 monorepo 并发发包同步,发布期间查询失败。

}
}, {
concurrency: 12,
});
}
}
9 changes: 9 additions & 0 deletions app/core/service/PackageSyncerService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ export class PackageSyncerService extends AbstractService {
if (tips) {
logs.push(`[${isoNow()}] 👉👉👉👉👉 Tips: ${tips} 👈👈👈👈👈`);
}

const taskQueueLength = await this.taskService.getTaskQueueLength(task.type);
const taskQueueHighWaterSize = this.config.cnpmcore.taskQueueHighWaterSize;
const taskQueueInHighWaterState = taskQueueLength >= taskQueueHighWaterSize;
Expand Down Expand Up @@ -730,6 +731,14 @@ export class PackageSyncerService extends AbstractService {
this.logger.error(err);
lastErrorMessage = `publish error: ${err}`;
logs.push(`[${isoNow()}] ❌ [${syncIndex}] Synced version ${version} error, ${lastErrorMessage}`);
if (err.name === 'BadRequestError') {
// 由于当前版本的依赖不满足,尝试重试
// 默认会在当前队列最后重试
this.logger.info('[PackageSyncerService.executeTask:fail-validate-deps] taskId: %s, targetName: %s, %s',
task.taskId, task.targetName, task.error);
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
await this.taskService.retryTask(task, logs.join('\n'));
return;
}
}
}
await this.taskService.appendTaskLog(task, logs.join('\n'));
Expand Down
5 changes: 5 additions & 0 deletions app/port/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,4 +170,9 @@ export type CnpmcoreConfig = {
* strictly enforces/validates manifest and tgz when publish, https://github.com/cnpm/cnpmcore/issues/542
*/
strictValidateTarballPkg?: boolean,

/**
* strictly enforces/validates dependencies version when publish or sync
*/
strictValidatePackageDeps?: boolean,
};
1 change: 1 addition & 0 deletions config/config.default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export const cnpmcoreConfig: CnpmcoreConfig = {
enableElasticsearch: !!process.env.CNPMCORE_CONFIG_ENABLE_ES,
elasticsearchIndex: 'cnpmcore_packages',
strictValidateTarballPkg: false,
strictValidatePackageDeps: false,
};

export default (appInfo: EggAppConfig) => {
Expand Down
24 changes: 24 additions & 0 deletions test/core/service/PackageManagerService/publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,5 +112,29 @@ describe('test/core/service/PackageManagerService/publish.test.ts', () => {
assert.equal(pkgVersion.version, '1.1.0');
assert.equal(pkgVersion.tarDist.size, 2672);
});

it('should strict validate deps', async () => {
let checked = false;
mock(app.config.cnpmcore, 'strictValidatePackageDeps', true);

await assert.rejects(async () => {
checked = true;
await packageManagerService.publish({
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
dist: {
localFile: TestUtil.getFixtures('registry.npmjs.org/pedding/-/pedding-1.1.0.tgz'),
},
tags: [ '' ],
scope: '',
name: 'pedding',
description: 'pedding description',
packageJson: { name: 'pedding', test: 'test', version: '1.1.0', dependencies: { 'invalid-pkg': 'some-semver-not-exits' } },
readme: '',
version: '1.1.0',
isPrivate: false,
}, publisher);
}, /deps invalid-pkg@some-semver-not-exits not found/);

assert(checked);
});
});
});
40 changes: 40 additions & 0 deletions test/core/service/PackageSyncerService/executeTask.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2552,5 +2552,45 @@ describe('test/core/service/PackageSyncerService/executeTask.test.ts', () => {
assert(/different meta: {"_npmUser":{"name":"banana","email":"[email protected]"}}/.test(log2));
});
});

describe('strictValidatePackageDeps = true', async () => {

// already synced pkg
beforeEach(async () => {
app.mockHttpclient(/^https:\/\/registry\.npmjs\.org\/invalid\-deps/, 'GET', {
data: await TestUtil.readFixturesFile('registry.npmjs.org/invalid-deps.json'),
persist: false,
});

app.mockHttpclient('https://registry.npmjs.org/invalid-deps/-/invalid-deps-1.0.0.tgz', 'GET', {
data: await TestUtil.readFixturesFile('registry.npmjs.org/foobar/-/foobar-1.0.0.tgz'),
persist: false,
});
});

it('should not create pkg when invalid deps', async () => {
// removed in remote
mock(app.config.cnpmcore, 'strictValidatePackageDeps', true);
await packageSyncerService.createTask('invalid-deps', { skipDependencies: true });
const task = await packageSyncerService.findExecuteTask();
assert(task);
await packageSyncerService.executeTask(task);
// assert(!await TaskModel.findOne({ taskId: task.taskId }));
// assert(await HistoryTaskModel.findOne({ taskId: task.taskId }));
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
const stream = await packageSyncerService.findTaskLog(task);
assert(stream);
const log = await TestUtil.readStreamToLog(stream);
assert(log);
// console.log(log);
const model = await PackageModel.findOne({ scope: '', name: 'invalid-deps' });
assert(!model);

// shoud requeue
const reTask = await packageSyncerService.findExecuteTask();
assert(reTask.attempts === 2);

});
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved

});
elrrrrrrr marked this conversation as resolved.
Show resolved Hide resolved
});
});
49 changes: 49 additions & 0 deletions test/fixtures/registry.npmjs.org/invalid-deps.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
{
"_id": "invalid-deps",
"name": "invalid-deps",
"dist-tags": { "latest": "1.0.0" },
"versions": {
"1.0.0": {
"name": "invalid-deps",
"version": "1.0.0",
"main": "index.js",
"dependencies": { "invalid-semver": "@*(5895jk!", "invalid-tag": "gutu" },
"scripts": { "test": "echo \"Error: no test specified\" && exit 1" },
"author": "",
"license": "ISC",
"_id": "[email protected]",
"_nodeVersion": "20.15.0",
"_npmVersion": "9.9.3",
"dist": {
"integrity": "sha512-wSjs5WIvZr78mkV8FHCKR59VVDNTupV8wylWHj05RFdB/apXroctfRe44opTO3PTGLdAFPrIUU1q7oJSebkr6w==",
"shasum": "588f41db968dbff87fc554d43f4af78b0eb35156",
"tarball": "https://registry.npmjs.org/invalid-deps/-/invalid-deps-1.0.0.tgz",
"fileCount": 2,
"unpackedSize": 295,
"signatures": [
{
"keyid": "SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA",
"sig": "MEUCIQCxErdGOYMudvChG6AOp/bNv7fHKFhAYGJ2F62Cry48twIgFpnYMFxw6qXinx2mL3hSEsyV07n2TkKPy20gA3RQxU0="
}
]
},
"_npmUser": { "name": "elrtmp", "email": "[email protected]" },
"directories": {},
"maintainers": [{ "name": "elrtmp", "email": "[email protected]" }],
"_npmOperationalInternal": {
"host": "s3://npm-registry-packages",
"tmp": "tmp/invalid-deps_1.0.0_1729738458514_0.507393390991969"
},
"_hasShrinkwrap": false
}
},
"time": {
"created": "2024-10-24T02:54:18.513Z",
"1.0.0": "2024-10-24T02:54:18.686Z",
"modified": "2024-10-24T02:54:18.896Z"
},
"maintainers": [{ "name": "elrtmp", "email": "[email protected]" }],
"license": "ISC",
"readme": "ERROR: No README data found!",
"readmeFilename": ""
}
Loading