Fix kubeconfig-sync issues (#2692)

- Removed `getFreePort` as its use is always a race condition. Change
  all uses of it to retrive the port after listening

- Added `getPortFrom` as a helper function to read a port from a stream

- Remove `Cluster.ownerRef` as it is outdated and no longer needed for 5.0

- Remove `Cluster.enabled`, no longer needed because of above

- Removed `Cluster.init`, moved its contents into `Cluster.constructor`
  as nothing in that function is asyncronous. Currently only being run
  on `main` as a stop gap until `renderer` gets its own version of
  `Cluster`

- Refactored `LensProxy` so as to prevent `pty.node` (a NodeJS
  extension) being included in `webpack.extension.ts`'s run

- Removed the passing around of the proxy port as that can now be
  accessed from an instance of `LensProxy`

- purge ContextHandler's cache on disconnect

Co-authored-by: Sebastian Malton <sebastian@malton.name>
This commit is contained in:
Jari KolehmainenandSebastian Malton authored and GitHub committed 2021-05-04 15:21:15 -04:00
1 parent bf96e6ef4c
commit 8fff064e0c
35 files changed
+563 -893

No files matched your search

+12 -60
View File
@@ -31,15 +31,8 @@ jest.mock("request-promise-native");
import { Console } from "console";
import mockFs from "mock-fs";
import { Cluster } from "../cluster";
import { ContextHandler } from "../context-handler";
import { getFreePort } from "../port";
import { V1ResourceAttributes } from "@kubernetes/client-node";
import { apiResources } from "../../common/rbac";
import request from "request-promise-native";
import { Kubectl } from "../kubectl";
const mockedRequest = request as jest.MockedFunction<typeof request>;
console = new Console(process.stdout, process.stderr); // fix mockFS
describe("create clusters", () => {
@@ -99,54 +92,7 @@ describe("create clusters", () => {
expect(() => c.disconnect()).not.toThrowError();
});
it("init should not throw if everything is in order", async () => {
await c.init(await getFreePort());
expect(logger.info).toBeCalledWith(expect.stringContaining("init success"), {
id: "foo",
apiUrl: "https://192.168.64.3:8443",
context: "minikube",
});
});
it("activating cluster should try to connect to cluster and do a refresh", async () => {
const port = await getFreePort();
jest.spyOn(ContextHandler.prototype, "ensureServer");
const mockListNSs = jest.fn();
const mockKC = {
makeApiClient() {
return {
listNamespace: mockListNSs,
};
}
};
jest.spyOn(Cluster.prototype, "isClusterAdmin").mockReturnValue(Promise.resolve(true));
jest.spyOn(Cluster.prototype, "canUseWatchApi").mockReturnValue(Promise.resolve(true));
jest.spyOn(Cluster.prototype, "canI")
.mockImplementation((attr: V1ResourceAttributes): Promise<boolean> => {
expect(attr.namespace).toBe("default");
expect(attr.verb).toBe("list");
return Promise.resolve(true);
});
jest.spyOn(Cluster.prototype, "getProxyKubeconfig").mockReturnValue(mockKC as any);
mockListNSs.mockImplementationOnce(() => ({
body: {
items: [{
metadata: {
name: "default",
}
}]
}
}));
mockedRequest.mockImplementationOnce(((uri: any) => {
expect(uri).toBe(`http://localhost:${port}/api-kube/version`);
return Promise.resolve({ gitVersion: "1.2.3" });
}) as any);
const c = new class extends Cluster {
// only way to mock protected methods, without these we leak promises
@@ -162,14 +108,20 @@ describe("create clusters", () => {
kubeConfigPath: "minikube-config.yml"
});
await c.init(port);
c.contextHandler = {
ensureServer: jest.fn(),
stopServer: jest.fn()
} as any;
jest.spyOn(c, "reconnect");
jest.spyOn(c, "canI");
jest.spyOn(c, "refreshConnectionStatus");
await c.activate();
expect(ContextHandler.prototype.ensureServer).toBeCalled();
expect(mockedRequest).toBeCalled();
expect(c.accessible).toBe(true);
expect(c.allowedNamespaces.length).toBe(1);
expect(c.allowedResources.length).toBe(apiResources.length);
expect(c.reconnect).toBeCalled();
expect(c.refreshConnectionStatus).toBeCalled();
c.disconnect();
jest.resetAllMocks();
});
+38 -9
View File
@@ -26,10 +26,10 @@ jest.mock("winston", () => ({
jest.mock("../../common/ipc");
jest.mock("child_process");
jest.mock("tcp-port-used");
//jest.mock("../utils/get-port");
import { Cluster } from "../cluster";
import { KubeAuthProxy } from "../kube-auth-proxy";
import { getFreePort } from "../port";
import { broadcastMessage } from "../../common/ipc";
import { ChildProcess, spawn } from "child_process";
import { bundledKubectlPath, Kubectl } from "../kubectl";
@@ -39,6 +39,7 @@ import { Readable } from "stream";
import { UserStore } from "../../common/user-store";
import { Console } from "console";
import { stdout, stderr } from "process";
import mockFs from "mock-fs";
console = new Console(stdout, stderr);
@@ -51,11 +52,41 @@ describe("kube auth proxy tests", () => {
jest.clearAllMocks();
UserStore.resetInstance();
UserStore.createInstance();
const mockMinikubeConfig = {
"minikube-config.yml": JSON.stringify({
apiVersion: "v1",
clusters: [{
name: "minikube",
cluster: {
server: "https://192.168.64.3:8443",
},
}],
"current-context": "minikube",
contexts: [{
context: {
cluster: "minikube",
user: "minikube",
},
name: "minikube",
}],
users: [{
name: "minikube",
}],
kind: "Config",
preferences: {},
})
};
mockFs(mockMinikubeConfig);
});
afterEach(() => {
mockFs.restore();
});
it("calling exit multiple times shouldn't throw", async () => {
const port = await getFreePort();
const kap = new KubeAuthProxy(new Cluster({ id: "foobar", kubeConfigPath: "fake-path.yml" }), port, {});
const kap = new KubeAuthProxy(new Cluster({ id: "foobar", kubeConfigPath: "minikube-config.yml", contextName: "minikube" }), {});
kap.exit();
kap.exit();
@@ -63,13 +94,11 @@ describe("kube auth proxy tests", () => {
});
describe("spawn tests", () => {
let port: number;
let mockedCP: MockProxy<ChildProcess>;
let listeners: Record<string, (...args: any[]) => void>;
let proxy: KubeAuthProxy;
beforeEach(async () => {
port = await getFreePort();
mockedCP = mock<ChildProcess>();
listeners = {};
@@ -89,6 +118,7 @@ describe("kube auth proxy tests", () => {
mockedCP.stdout = mock<Readable>();
mockedCP.stdout.on.mockImplementation((event: string, listener: (message: any, sendHandle: any) => void): Readable => {
listeners[`stdout/${event}`] = listener;
listeners[`stdout/${event}`]("Starting to serve on 127.0.0.1:9191");
return mockedCP.stdout;
});
@@ -98,10 +128,10 @@ describe("kube auth proxy tests", () => {
return mockedCP;
});
mockWaitUntilUsed.mockReturnValueOnce(Promise.resolve());
const cluster = new Cluster({ id: "foobar", kubeConfigPath: "fake-path.yml" });
jest.spyOn(cluster, "apiUrl", "get").mockReturnValue("https://fake.k8s.internal");
proxy = new KubeAuthProxy(cluster, port, {});
const cluster = new Cluster({ id: "foobar", kubeConfigPath: "minikube-config.yml", contextName: "minikube" });
proxy = new KubeAuthProxy(cluster, {});
});
it("should call spawn and broadcast errors", async () => {
@@ -127,7 +157,6 @@ describe("kube auth proxy tests", () => {
it("should call spawn and broadcast stdout serving info", async () => {
await proxy.run();
listeners["stdout/data"]("Starting to serve on");
expect(mockBroadcastIpc).toBeCalledWith("kube-auth:foobar", { data: "Authentication proxy started\n" });
});
+14 -18
View File
@@ -27,7 +27,6 @@ import { KubeconfigManager } from "../kubeconfig-manager";
import mockFs from "mock-fs";
import { Cluster } from "../cluster";
import { ContextHandler } from "../context-handler";
import { getFreePort } from "../port";
import fse from "fs-extra";
import { loadYaml } from "@kubernetes/client-node";
import { Console } from "console";
@@ -36,6 +35,9 @@ import * as path from "path";
console = new Console(process.stdout, process.stderr); // fix mockFS
describe("kubeconfig manager tests", () => {
let cluster: Cluster;
let contextHandler: ContextHandler;
beforeEach(() => {
const mockOpts = {
"minikube-config.yml": JSON.stringify({
@@ -62,6 +64,14 @@ describe("kubeconfig manager tests", () => {
};
mockFs(mockOpts);
cluster = new Cluster({
id: "foo",
contextName: "minikube",
kubeConfigPath: "minikube-config.yml",
});
contextHandler = jest.fn() as any;
jest.spyOn(KubeconfigManager.prototype, "resolveProxyUrl", "get").mockReturnValue("http://127.0.0.1:9191/foo");
});
afterEach(() => {
@@ -69,14 +79,7 @@ describe("kubeconfig manager tests", () => {
});
it("should create 'temp' kube config with proxy", async () => {
const cluster = new Cluster({
id: "foo",
contextName: "minikube",
kubeConfigPath: "minikube-config.yml",
});
const contextHandler = new ContextHandler(cluster);
const port = await getFreePort();
const kubeConfManager = new KubeconfigManager(cluster, contextHandler, port);
const kubeConfManager = new KubeconfigManager(cluster, contextHandler);
expect(logger.error).not.toBeCalled();
expect(await kubeConfManager.getPath()).toBe(`tmp${path.sep}kubeconfig-foo`);
@@ -86,19 +89,12 @@ describe("kubeconfig manager tests", () => {
const yml = loadYaml<any>(file.toString());
expect(yml["current-context"]).toBe("minikube");
expect(yml["clusters"][0]["cluster"]["server"]).toBe(`http://127.0.0.1:${port}/foo`);
expect(yml["clusters"][0]["cluster"]["server"].endsWith("/foo")).toBe(true);
expect(yml["users"][0]["name"]).toBe("proxy");
});
it("should remove 'temp' kube config on unlink and remove reference from inside class", async () => {
const cluster = new Cluster({
id: "foo",
contextName: "minikube",
kubeConfigPath: "minikube-config.yml",
});
const contextHandler = new ContextHandler(cluster);
const port = await getFreePort();
const kubeConfManager = new KubeconfigManager(cluster, contextHandler, port);
const kubeConfManager = new KubeconfigManager(cluster, contextHandler);
const configPath = await kubeConfManager.getPath();
expect(await fse.pathExists(configPath)).toBe(true);