lensapp/lens
1
0
Fork 0
mirror of https://github.com/lensapp/lens.git synced 2024-09-20 13:57:23 +03:00
Code Issues Projects Releases Wiki Activity

Fix extension loader race conditions (#1815)

Browse Source 
* fix extension loader race conditions

Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>

* cleanup

Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>

* fix tests

Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>

* fix remove

Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>

* ensure symlinked (dev) extensions are installed on boot

Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
This commit is contained in:
Jari Kolehmainen 2020-12-23 11:56:11 +02:00 committed by GitHub
parent 507c485113
commit 75c70a1141
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 95 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/extensions/__tests__/extension-discovery.test.ts
View File

@ -10,7 +10,7 @@ jest.mock("chokidar", () => ({
jest.mock("../extension-installer", () => ({
extensionInstaller: {
extensionPackagesRoot: "",
installPackages: jest.fn()
installPackage: jest.fn()
}
}));
@ -41,7 +41,7 @@ describe("ExtensionDiscovery", () => {
// Need to force isLoaded to be true so that the file watching is started
extensionDiscovery.isLoaded = true;
await extensionDiscovery.initMain();
await extensionDiscovery.watchExtensions();
extensionDiscovery.events.on("add", (extension: InstalledExtension) => {
expect(extension).toEqual({
@ -81,7 +81,7 @@ describe("ExtensionDiscovery", () => {
// Need to force isLoaded to be true so that the file watching is started
extensionDiscovery.isLoaded = true;
await extensionDiscovery.initMain();
await extensionDiscovery.watchExtensions();
const onAdd = jest.fn();

67
src/extensions/extension-discovery.ts
View File

@ -55,6 +55,7 @@ export class ExtensionDiscovery {
protected bundledFolderPath: string;
private loadStarted = false;
private extensions: Map<string, InstalledExtension> = new Map();
// True if extensions have been loaded from the disk after app startup
@observable isLoaded = false;
@ -69,13 +70,6 @@ export class ExtensionDiscovery {
this.events = new EventEmitter();
}
// Each extension is added as a single dependency to this object, which is written as package.json.
// Each dependency key is the name of the dependency, and
// each dependency value is the non-symlinked path to the dependency (folder).
protected packagesJson: PackageJson = {
dependencies: {}
};
get localFolderPath(): string {
return path.join(os.homedir(), ".k8slens", "extensions");
}
@ -119,7 +113,6 @@ export class ExtensionDiscovery {
}
async initMain() {
this.watchExtensions();
handleRequest(ExtensionDiscovery.extensionDiscoveryChannel, () => this.toJSON());
reaction(() => this.toJSON(), () => {
@ -141,6 +134,7 @@ export class ExtensionDiscovery {
watch(this.localFolderPath, {
// For adding and removing symlinks to work, the depth has to be 1.
depth: 1,
ignoreInitial: true,
// Try to wait until the file has been completely copied.
// The OS might emit an event for added file even it's not completely written to the filesysten.
awaitWriteFinish: {
@ -176,8 +170,9 @@ export class ExtensionDiscovery {
await this.removeSymlinkByManifestPath(manifestPath);
// Install dependencies for the new extension
await this.installPackages();
await this.installPackage(extension.absolutePath);
this.extensions.set(extension.id, extension);
logger.info(`${logModule} Added extension ${extension.manifest.name}`);
this.events.emit("add", extension);
}
@ -197,23 +192,19 @@ export class ExtensionDiscovery {
const extensionFolderName = path.basename(filePath);
if (path.relative(this.localFolderPath, filePath) === extensionFolderName) {
const extensionName: string | undefined = Object
.entries(this.packagesJson.dependencies)
.find(([, extensionFolder]) => filePath === extensionFolder)?.[0];
const extension = Array.from(this.extensions.values()).find((extension) => extension.absolutePath === filePath);
if (extension) {
const extensionName = extension.manifest.name;
if (extensionName !== undefined) {
// If the extension is deleted manually while the application is running, also remove the symlink
await this.removeSymlinkByPackageName(extensionName);
delete this.packagesJson.dependencies[extensionName];
// Reinstall dependencies to remove the extension from package.json
await this.installPackages();
// The path to the manifest file is the lens extension id
// Note that we need to use the symlinked path
const lensExtensionId = path.join(this.nodeModulesPath, extensionName, manifestFilename);
const lensExtensionId = extension.manifestPath;
this.extensions.delete(extension.id);
logger.info(`${logModule} removed extension ${extensionName}`);
this.events.emit("remove", lensExtensionId as LensExtensionId);
} else {
@ -296,7 +287,7 @@ export class ExtensionDiscovery {
await fs.ensureDir(this.nodeModulesPath);
await fs.ensureDir(this.localFolderPath);
const extensions = await this.loadExtensions();
const extensions = await this.ensureExtensions();
this.isLoaded = true;
@ -335,7 +326,6 @@ export class ExtensionDiscovery {
manifestJson = __non_webpack_require__(manifestPath);
const installedManifestPath = this.getInstalledManifestPath(manifestJson.name);
this.packagesJson.dependencies[manifestJson.name] = path.dirname(manifestPath);
const isEnabled = isBundled || extensionsStore.isEnabled(installedManifestPath);
return {
@ -347,29 +337,46 @@ export class ExtensionDiscovery {
isEnabled
};
} catch (error) {
logger.error(`${logModule}: can't install extension at ${manifestPath}: ${error}`, { manifestJson });
logger.error(`${logModule}: can't load extension manifest at ${manifestPath}: ${error}`, { manifestJson });
return null;
}
}
async loadExtensions(): Promise<Map<LensExtensionId, InstalledExtension>> {
async ensureExtensions(): Promise<Map<LensExtensionId, InstalledExtension>> {
const bundledExtensions = await this.loadBundledExtensions();
await this.installPackages(); // install in-tree as a separate step
const localExtensions = await this.loadFromFolder(this.localFolderPath);
await this.installBundledPackages(this.packageJsonPath, bundledExtensions);
await this.installPackages();
const extensions = bundledExtensions.concat(localExtensions);
const userExtensions = await this.loadFromFolder(this.localFolderPath);
return new Map(extensions.map(extension => [extension.id, extension]));
for (const extension of userExtensions) {
if (await fs.pathExists(extension.manifestPath) === false) {
await this.installPackage(extension.absolutePath);
}
}
const extensions = bundledExtensions.concat(userExtensions);
return this.extensions = new Map(extensions.map(extension => [extension.id, extension]));
}
/**
* Write package.json to file system and install dependencies.
*/
installPackages() {
return extensionInstaller.installPackages(this.packageJsonPath, this.packagesJson);
async installBundledPackages(packageJsonPath: string, extensions: InstalledExtension[]) {
const packagesJson: PackageJson = {
dependencies: {}
};
extensions.forEach((extension) => {
packagesJson.dependencies[extension.manifest.name] = extension.absolutePath;
});
return await extensionInstaller.installPackages(packageJsonPath, packagesJson);
}
async installPackage(name: string) {
return extensionInstaller.installPackage(name);
}
async loadBundledExtensions() {

64
src/extensions/extension-installer.ts
View File

@ -30,12 +30,49 @@ export class ExtensionInstaller {
return __non_webpack_require__.resolve("npm/bin/npm-cli");
}
installDependencies(): Promise<void> {
return new Promise((resolve, reject) => {
/**
* Write package.json to the file system and execute npm install for it.
*/
async installPackages(packageJsonPath: string, packagesJson: PackageJson): Promise<void> {
// Mutual exclusion to install packages in sequence
await this.installLock.acquireAsync();
try {
// Write the package.json which will be installed in .installDependencies()
await fs.writeFile(path.join(packageJsonPath), JSON.stringify(packagesJson, null, 2), {
mode: 0o600
});
logger.info(`${logModule} installing dependencies at ${extensionPackagesRoot()}`);
const child = child_process.fork(this.npmPath, ["install", "--no-audit", "--only=prod", "--prefer-offline", "--no-package-lock"], {
await this.npm(["install", "--no-audit", "--only=prod", "--prefer-offline", "--no-package-lock"]);
logger.info(`${logModule} dependencies installed at ${extensionPackagesRoot()}`);
} finally {
this.installLock.release();
}
}
/**
* Install single package using npm
*/
async installPackage(name: string): Promise<void> {
// Mutual exclusion to install packages in sequence
await this.installLock.acquireAsync();
try {
logger.info(`${logModule} installing package from ${name} to ${extensionPackagesRoot()}`);
await this.npm(["install", "--no-audit", "--only=prod", "--prefer-offline", "--no-package-lock", "--no-save", name]);
logger.info(`${logModule} package ${name} installed to ${extensionPackagesRoot()}`);
} finally {
this.installLock.release();
}
}
private npm(args: string[]): Promise<void> {
return new Promise((resolve, reject) => {
const child = child_process.fork(this.npmPath, args, {
cwd: extensionPackagesRoot(),
silent: true
silent: true,
env: {}
});
let stderr = "";
@ -56,25 +93,6 @@ export class ExtensionInstaller {
});
});
}
/**
* Write package.json to the file system and execute npm install for it.
*/
async installPackages(packageJsonPath: string, packagesJson: PackageJson): Promise<void> {
// Mutual exclusion to install packages in sequence
await this.installLock.acquireAsync();
try {
// Write the package.json which will be installed in .installDependencies()
await fs.writeFile(path.join(packageJsonPath), JSON.stringify(packagesJson, null, 2), {
mode: 0o600
});
await this.installDependencies();
} finally {
this.installLock.release();
}
}
}
export const extensionInstaller = new ExtensionInstaller();

12
src/extensions/extension-loader.ts
View File

@ -12,6 +12,7 @@ import type { LensExtension, LensExtensionConstructor, LensExtensionId } from ".
import type { LensMainExtension } from "./lens-main-extension";
import type { LensRendererExtension } from "./lens-renderer-extension";
import * as registries from "./registries";
import fs from "fs";
// lazy load so that we get correct userData
export function extensionPackagesRoot() {
@ -71,7 +72,7 @@ export class ExtensionLoader {
}
await Promise.all([this.whenLoaded, extensionsStore.whenLoaded]);
// save state on change `extension.isEnabled`
reaction(() => this.storeState, extensionsState => {
extensionsStore.mergeState(extensionsState);
@ -115,7 +116,6 @@ export class ExtensionLoader {
protected async initMain() {
this.isLoaded = true;
this.loadOnMain();
this.broadcastExtensions();
reaction(() => this.toJSON(), () => {
this.broadcastExtensions();
@ -136,7 +136,7 @@ export class ExtensionLoader {
this.syncExtensions(extensions);
const receivedExtensionIds = extensions.map(([lensExtensionId]) => lensExtensionId);
// Remove deleted extensions in renderer side only
this.extensions.forEach((_, lensExtensionId) => {
if (!receivedExtensionIds.includes(lensExtensionId)) {
@ -276,6 +276,12 @@ export class ExtensionLoader {
}
if (extEntrypoint !== "") {
if (!fs.existsSync(extEntrypoint)) {
console.log(`${logModule}: entrypoint ${extEntrypoint} not found, skipping ...`);
return;
}
return __non_webpack_require__(extEntrypoint).default;
}
} catch (err) {

6
src/main/index.ts
View File

@ -103,7 +103,6 @@ app.on("ready", async () => {
}
extensionLoader.init();
extensionDiscovery.init();
windowManager = WindowManager.getInstance<WindowManager>(proxyPort);
@ -111,6 +110,9 @@ app.on("ready", async () => {
try {
const extensions = await extensionDiscovery.load();
// Start watching after bundled extensions are loaded
extensionDiscovery.watchExtensions();
// Subscribe to extensions that are copied or deleted to/from the extensions folder
extensionDiscovery.events.on("add", (extension: InstalledExtension) => {
extensionLoader.addExtension(extension);
@ -122,6 +124,8 @@ app.on("ready", async () => {
extensionLoader.initExtensions(extensions);
} catch (error) {
dialog.showErrorBox("Lens Error", `Could not load extensions${error?.message ? `: ${error.message}` : ""}`);
console.error(error);
console.trace();
}
setTimeout(() => {