From 4705a66632a7a4402f904f698869d3ce21bcec57 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Mon, 4 Oct 2021 16:28:19 -0400 Subject: [PATCH] Use JSON patch for editing components (#3674) --- package.json | 1 + .../k8s-api/endpoints/resource-applier.api.ts | 34 ++++--- src/common/k8s-api/kube-object.store.ts | 21 +++-- src/common/k8s-api/kube-object.ts | 59 ++++++++---- src/main/resource-applier.ts | 89 ++++++++++++++----- src/main/router.ts | 1 + src/main/routes/resource-applier-route.ts | 14 ++- src/main/utils/http-responses.ts | 27 +++++- .../+config-maps/config-map-details.tsx | 2 + .../deployment-scale-dialog.test.tsx | 1 + .../replicaset-scale-dialog.test.tsx | 1 + .../statefulset-scale-dialog.test.tsx | 1 + .../components/dock/create-resource.tsx | 48 +++++----- .../components/dock/edit-resource.store.ts | 5 ++ .../components/dock/edit-resource.tsx | 26 +++--- src/renderer/components/dock/editor-panel.tsx | 5 +- .../kube-object-menu/kube-object-menu.tsx | 8 +- yarn.lock | 5 ++ 18 files changed, 242 insertions(+), 106 deletions(-) diff --git a/package.json b/package.json index 0f4a15ede2..286ac7f97a 100644 --- a/package.json +++ b/package.json @@ -237,6 +237,7 @@ "readable-stream": "^3.6.0", "request": "^2.88.2", "request-promise-native": "^1.0.9", + "rfc6902": "^4.0.2", "semver": "^7.3.2", "serializr": "^2.0.5", "shell-env": "^3.0.1", diff --git a/src/common/k8s-api/endpoints/resource-applier.api.ts b/src/common/k8s-api/endpoints/resource-applier.api.ts index b6a35d3a60..14b5d5ad46 100644 --- a/src/common/k8s-api/endpoints/resource-applier.api.ts +++ b/src/common/k8s-api/endpoints/resource-applier.api.ts @@ -22,19 +22,27 @@ import jsYaml from "js-yaml"; import type { KubeJsonApiData } from "../kube-json-api"; import { apiBase } from "../index"; +import type { Patch } from "rfc6902"; -export const resourceApplierApi = { - annotations: [ - "kubectl.kubernetes.io/last-applied-configuration" - ], +export const annotations = [ + "kubectl.kubernetes.io/last-applied-configuration" +]; - async update(resource: object | string): Promise { - if (typeof resource === "string") { - resource = jsYaml.safeLoad(resource); - } - - const [data = null] = await apiBase.post("/stack", { data: resource }); - - return data; +export async function update(resource: object | string): Promise { + if (typeof resource === "string") { + resource = jsYaml.safeLoad(resource); } -}; + + return apiBase.post("/stack", { data: resource }); +} + +export async function patch(name: string, kind: string, ns: string, patch: Patch): Promise { + return apiBase.patch("/stack", { + data: { + name, + kind, + ns, + patch, + }, + }); +} diff --git a/src/common/k8s-api/kube-object.store.ts b/src/common/k8s-api/kube-object.store.ts index f90eaab8e5..7c8118da1f 100644 --- a/src/common/k8s-api/kube-object.store.ts +++ b/src/common/k8s-api/kube-object.store.ts @@ -32,6 +32,7 @@ import { parseKubeApi } from "./kube-api-parse"; import type { KubeJsonApiData } from "./kube-json-api"; import type { RequestInit } from "node-fetch"; import AbortController from "abort-controller"; +import type { Patch } from "rfc6902"; export interface KubeObjectStoreLoadingParams { namespaces: string[]; @@ -279,19 +280,29 @@ export abstract class KubeObjectStore extends ItemStore return newItem; } - async update(item: T, data: Partial): Promise { - const rawItem = await item.update(data); + private postUpdate(rawItem: KubeJsonApiData): T { const newItem = new this.api.objectConstructor(rawItem); + const index = this.items.findIndex(item => item.getId() === newItem.getId()); ensureObjectSelfLink(this.api, newItem); - const index = this.items.findIndex(item => item.getId() === newItem.getId()); - - this.items.splice(index, 1, newItem); + if (index < 0) { + this.items.push(newItem); + } else { + this.items[index] = newItem; + } return newItem; } + async patch(item: T, patch: Patch): Promise { + return this.postUpdate(await item.patch(patch)); + } + + async update(item: T, data: Partial): Promise { + return this.postUpdate(await item.update(data)); + } + async remove(item: T) { await item.delete(); this.items.remove(item); diff --git a/src/common/k8s-api/kube-object.ts b/src/common/k8s-api/kube-object.ts index c1e40eb2e5..bcbe6bf05f 100644 --- a/src/common/k8s-api/kube-object.ts +++ b/src/common/k8s-api/kube-object.ts @@ -27,9 +27,9 @@ import { autoBind, formatDuration } from "../utils"; import type { ItemObject } from "../item.store"; import { apiKube } from "./index"; import type { JsonApiParams } from "./json-api"; -import { resourceApplierApi } from "./endpoints/resource-applier.api"; +import * as resourceApplierApi from "./endpoints/resource-applier.api"; import { hasOptionalProperty, hasTypedProperty, isObject, isString, bindPredicate, isTypedArray, isRecord } from "../../common/utils/type-narrowing"; -import _ from "lodash"; +import type { Patch } from "rfc6902"; export type KubeObjectConstructor = (new (data: KubeJsonApiData | any) => K) & { kind?: string; @@ -191,18 +191,28 @@ export class KubeObject `${name}=${value}`); } - protected static readonly nonEditableFields = [ - "apiVersion", - "kind", - "metadata.name", - "metadata.selfLink", - "metadata.resourceVersion", - "metadata.uid", - "managedFields", - "status", + /** + * These must be RFC6902 compliant paths + */ + private static readonly nonEditiablePathPrefixes = [ + "/metadata/managedFields", + "/status", ]; + private static readonly nonEditablePaths = new Set([ + "/apiVersion", + "/kind", + "/metadata/name", + "/metadata/selfLink", + "/metadata/resourceVersion", + "/metadata/uid", + ...KubeObject.nonEditiablePathPrefixes, + ]); constructor(data: KubeJsonApiData) { + if (typeof data !== "object") { + throw new TypeError(`Cannot create a KubeObject from ${typeof data}`); + } + Object.assign(this, data); autoBind(this); } @@ -286,14 +296,31 @@ export class KubeObject): Promise { - for (const field of KubeObject.nonEditableFields) { - if (!_.isEqual(_.get(this, field), _.get(data, field))) { - throw new Error(`Failed to update Kube Object: ${field} has been modified`); + async patch(patch: Patch): Promise { + for (const op of patch) { + if (KubeObject.nonEditablePaths.has(op.path)) { + throw new Error(`Failed to update ${this.kind}: JSON pointer ${op.path} has been modified`); + } + + for (const pathPrefix of KubeObject.nonEditiablePathPrefixes) { + if (op.path.startsWith(`${pathPrefix}/`)) { + throw new Error(`Failed to update ${this.kind}: Child JSON pointer of ${op.path} has been modified`); + } } } + return resourceApplierApi.patch(this.getName(), this.kind, this.getNs(), patch); + } + + /** + * Perform a full update (or more specifically a replace) + * + * Note: this is brittle if `data` is not actually partial (but instead whole). + * As fields such as `resourceVersion` will probably out of date. This is a + * common race condition. + */ + async update(data: Partial): Promise { + // use unified resource-applier api for updating all k8s objects return resourceApplierApi.update({ ...this.toPlainObject(), ...data, diff --git a/src/main/resource-applier.ts b/src/main/resource-applier.ts index f4772c5275..a305ff1091 100644 --- a/src/main/resource-applier.ts +++ b/src/main/resource-applier.ts @@ -22,55 +22,96 @@ import type { Cluster } from "./cluster"; import type { KubernetesObject } from "@kubernetes/client-node"; import { exec } from "child_process"; -import fs from "fs"; +import fs from "fs-extra"; import * as yaml from "js-yaml"; import path from "path"; import * as tempy from "tempy"; import logger from "./logger"; import { appEventBus } from "../common/event-bus"; import { cloneJsonObject } from "../common/utils"; +import type { Patch } from "rfc6902"; +import { promiseExecFile } from "./promise-exec"; export class ResourceApplier { - constructor(protected cluster: Cluster) { + constructor(protected cluster: Cluster) {} + + /** + * Patch a kube resource's manifest, throwing any error that occurs. + * @param name The name of the kube resource + * @param kind The kind of the kube resource + * @param patch The list of JSON operations + * @param ns The optional namespace of the kube resource + */ + async patch(name: string, kind: string, patch: Patch, ns?: string): Promise { + appEventBus.emit({ name: "resource", action: "patch" }); + + const kubectl = await this.cluster.ensureKubectl(); + const kubectlPath = await kubectl.getPath(); + const proxyKubeconfigPath = await this.cluster.getProxyKubeconfigPath(); + const args = [ + "--kubeconfig", proxyKubeconfigPath, + "patch", + kind, + name, + ]; + + if (ns) { + args.push("--namespace", ns); + } + + args.push( + "--type", "json", + "--patch", JSON.stringify(patch), + "-o", "json" + ); + + try { + const { stdout } = await promiseExecFile(kubectlPath, args); + + return stdout; + } catch (error) { + throw error.stderr ?? error; + } } async apply(resource: KubernetesObject | any): Promise { resource = this.sanitizeObject(resource); appEventBus.emit({ name: "resource", action: "apply" }); - return await this.kubectlApply(yaml.safeDump(resource)); + return this.kubectlApply(yaml.safeDump(resource)); } protected async kubectlApply(content: string): Promise { const kubectl = await this.cluster.ensureKubectl(); const kubectlPath = await kubectl.getPath(); const proxyKubeconfigPath = await this.cluster.getProxyKubeconfigPath(); + const fileName = tempy.file({ name: "resource.yaml" }); + const args = [ + "apply", + "--kubeconfig", proxyKubeconfigPath, + "-o", "json", + "-f", fileName, + ]; - return new Promise((resolve, reject) => { - const fileName = tempy.file({ name: "resource.yaml" }); + logger.debug(`shooting manifests with ${kubectlPath}`, { args }); - fs.writeFileSync(fileName, content); - const cmd = `"${kubectlPath}" apply --kubeconfig "${proxyKubeconfigPath}" -o json -f "${fileName}"`; + const execEnv = { ...process.env }; + const httpsProxy = this.cluster.preferences?.httpsProxy; - logger.debug(`shooting manifests with: ${cmd}`); - const execEnv: NodeJS.ProcessEnv = Object.assign({}, process.env); - const httpsProxy = this.cluster.preferences?.httpsProxy; + if (httpsProxy) { + execEnv.HTTPS_PROXY = httpsProxy; + } - if (httpsProxy) { - execEnv["HTTPS_PROXY"] = httpsProxy; - } - exec(cmd, { env: execEnv }, - (error, stdout, stderr) => { - if (stderr != "") { - fs.unlinkSync(fileName); - reject(stderr); + try { + await fs.writeFile(fileName, content); + const { stdout } = await promiseExecFile(kubectlPath, args); - return; - } - fs.unlinkSync(fileName); - resolve(JSON.parse(stdout)); - }); - }); + return stdout; + } catch (error) { + throw error?.stderr ?? error; + } finally { + await fs.unlink(fileName); + } } public async kubectlApplyAll(resources: string[], extraArgs = ["-o", "json"]): Promise { diff --git a/src/main/router.ts b/src/main/router.ts index 50175aa9de..d758cc2314 100644 --- a/src/main/router.ts +++ b/src/main/router.ts @@ -198,5 +198,6 @@ export class Router { // Resource Applier API this.router.add({ method: "post", path: `${apiPrefix}/stack` }, ResourceApplierApiRoute.applyResource); + this.router.add({ method: "patch", path: `${apiPrefix}/stack` }, ResourceApplierApiRoute.patchResource); } } diff --git a/src/main/routes/resource-applier-route.ts b/src/main/routes/resource-applier-route.ts index cb716cf1f5..0adf916188 100644 --- a/src/main/routes/resource-applier-route.ts +++ b/src/main/routes/resource-applier-route.ts @@ -30,7 +30,19 @@ export class ResourceApplierApiRoute { try { const resource = await new ResourceApplier(cluster).apply(payload); - respondJson(response, [resource], 200); + respondJson(response, resource, 200); + } catch (error) { + respondText(response, error, 422); + } + } + + static async patchResource(request: LensApiRequest) { + const { response, cluster, payload } = request; + + try { + const resource = await new ResourceApplier(cluster).patch(payload.name, payload.kind, payload.patch, payload.ns); + + respondJson(response, resource, 200); } catch (error) { respondText(response, error, 422); } diff --git a/src/main/utils/http-responses.ts b/src/main/utils/http-responses.ts index 36287806f1..5d57b6dd76 100644 --- a/src/main/utils/http-responses.ts +++ b/src/main/utils/http-responses.ts @@ -21,14 +21,37 @@ import type http from "http"; -export function respondJson(res: http.ServerResponse, content: any, status = 200) { - respond(res, JSON.stringify(content), "application/json", status); +/** + * Respond to a HTTP request with a body of JSON data + * @param res The HTTP response to write data to + * @param content The data or its JSON stringified version of it + * @param status [200] The status code to respond with + */ +export function respondJson(res: http.ServerResponse, content: Object | string, status = 200) { + const normalizedContent = typeof content === "object" + ? JSON.stringify(content) + : content; + + respond(res, normalizedContent, "application/json", status); } +/** + * Respond to a HTTP request with a body of plain text data + * @param res The HTTP response to write data to + * @param content The string data to respond with + * @param status [200] The status code to respond with + */ export function respondText(res: http.ServerResponse, content: string, status = 200) { respond(res, content, "text/plain", status); } +/** + * Respond to a HTTP request with a body of plain text data + * @param res The HTTP response to write data to + * @param content The string data to respond with + * @param contentType The HTTP Content-Type header value + * @param status [200] The status code to respond with + */ export function respond(res: http.ServerResponse, content: string, contentType: string, status = 200) { res.setHeader("Content-Type", contentType); res.statusCode = status; diff --git a/src/renderer/components/+config-maps/config-map-details.tsx b/src/renderer/components/+config-maps/config-map-details.tsx index abbd31c97a..6150aa1ad9 100644 --- a/src/renderer/components/+config-maps/config-map-details.tsx +++ b/src/renderer/components/+config-maps/config-map-details.tsx @@ -72,6 +72,8 @@ export class ConfigMapDetails extends React.Component { <>ConfigMap {configMap.getName()} successfully updated.

); + } catch (error) { + Notifications.error(`Failed to save config map: ${error}`); } finally { this.isSaving = false; } diff --git a/src/renderer/components/+workloads-deployments/deployment-scale-dialog.test.tsx b/src/renderer/components/+workloads-deployments/deployment-scale-dialog.test.tsx index 22c90dda2c..3bc22b8263 100644 --- a/src/renderer/components/+workloads-deployments/deployment-scale-dialog.test.tsx +++ b/src/renderer/components/+workloads-deployments/deployment-scale-dialog.test.tsx @@ -112,6 +112,7 @@ const dummyDeployment: Deployment = { toPlainObject: jest.fn(), update: jest.fn(), delete: jest.fn(), + patch: jest.fn(), }; describe("", () => { diff --git a/src/renderer/components/+workloads-replicasets/replicaset-scale-dialog.test.tsx b/src/renderer/components/+workloads-replicasets/replicaset-scale-dialog.test.tsx index 7f8c31e99c..e948d3a339 100755 --- a/src/renderer/components/+workloads-replicasets/replicaset-scale-dialog.test.tsx +++ b/src/renderer/components/+workloads-replicasets/replicaset-scale-dialog.test.tsx @@ -107,6 +107,7 @@ const dummyReplicaSet: ReplicaSet = { toPlainObject: jest.fn(), update: jest.fn(), delete: jest.fn(), + patch: jest.fn(), }; describe("", () => { diff --git a/src/renderer/components/+workloads-statefulsets/statefulset-scale-dialog.test.tsx b/src/renderer/components/+workloads-statefulsets/statefulset-scale-dialog.test.tsx index b27d8b0892..016d431eb2 100755 --- a/src/renderer/components/+workloads-statefulsets/statefulset-scale-dialog.test.tsx +++ b/src/renderer/components/+workloads-statefulsets/statefulset-scale-dialog.test.tsx @@ -117,6 +117,7 @@ const dummyStatefulSet: StatefulSet = { toPlainObject: jest.fn(), update: jest.fn(), delete: jest.fn(), + patch: jest.fn(), }; describe("", () => { diff --git a/src/renderer/components/dock/create-resource.tsx b/src/renderer/components/dock/create-resource.tsx index a357d0dc62..c7de411e3c 100644 --- a/src/renderer/components/dock/create-resource.tsx +++ b/src/renderer/components/dock/create-resource.tsx @@ -33,10 +33,10 @@ import { createResourceStore } from "./create-resource.store"; import type { DockTab } from "./dock.store"; import { EditorPanel } from "./editor-panel"; import { InfoPanel } from "./info-panel"; -import { resourceApplierApi } from "../../../common/k8s-api/endpoints/resource-applier.api"; -import type { JsonApiErrorParsed } from "../../../common/k8s-api/json-api"; +import * as resourceApplierApi from "../../../common/k8s-api/endpoints/resource-applier.api"; import { Notifications } from "../notifications"; import { monacoModelsManager } from "./monaco-model-manager"; +import logger from "../../../common/logger"; interface Props { className?: string; @@ -95,7 +95,7 @@ export class CreateResource extends React.Component { }); }; - create = async () => { + create = async (): Promise => { if (this.error || !this.data.trim()) { // do not save when field is empty or there is an error return null; @@ -103,31 +103,31 @@ export class CreateResource extends React.Component { // skip empty documents if "---" pasted at the beginning or end const resources = jsYaml.safeLoadAll(this.data).filter(Boolean); - const createdResources: string[] = []; - const errors: string[] = []; - await Promise.all( - resources.map(data => { - return resourceApplierApi.update(data) - .then(item => createdResources.push(item.metadata.name)) - .catch((err: JsonApiErrorParsed) => errors.push(err.toString())); - }) - ); - - if (errors.length) { - errors.forEach(error => Notifications.error(error)); - if (!createdResources.length) throw errors[0]; + if (resources.length === 0) { + return void logger.info("Nothing to create"); } - const successMessage = ( -

- {createdResources.length === 1 ? "Resource" : "Resources"}{" "} - {createdResources.join(", ")} successfully created -

- ); - Notifications.ok(successMessage); + const createdResources: string[] = []; - return successMessage; + for (const result of await Promise.allSettled(resources.map(resourceApplierApi.update))) { + if (result.status === "fulfilled") { + createdResources.push(result.value.metadata.name); + } else { + Notifications.error(result.reason.toString()); + } + } + + if (createdResources.length > 0) { + Notifications.ok(( +

+ {createdResources.length === 1 ? "Resource" : "Resources"}{" "} + {createdResources.join(", ")} successfully created +

+ )); + } + + return undefined; }; renderControls(){ diff --git a/src/renderer/components/dock/edit-resource.store.ts b/src/renderer/components/dock/edit-resource.store.ts index b5be580313..d8b6d06d6b 100644 --- a/src/renderer/components/dock/edit-resource.store.ts +++ b/src/renderer/components/dock/edit-resource.store.ts @@ -31,6 +31,7 @@ import {monacoModelsManager} from "./monaco-model-manager"; export interface EditingResource { resource: string; // resource path, e.g. /api/v1/namespaces/default draft?: string; // edited draft in yaml + firstDraft?: string; } export class EditResourceStore extends DockTabStore { @@ -106,6 +107,10 @@ export class EditResourceStore extends DockTabStore { return dockStore.getTabById(tabId); } + clearInitialDraft(tabId: TabId): void { + delete this.getData(tabId)?.firstDraft; + } + reset() { super.reset(); Array.from(this.watchers).forEach(([tabId, dispose]) => { diff --git a/src/renderer/components/dock/edit-resource.tsx b/src/renderer/components/dock/edit-resource.tsx index 19a238ab30..068855eebb 100644 --- a/src/renderer/components/dock/edit-resource.tsx +++ b/src/renderer/components/dock/edit-resource.tsx @@ -24,7 +24,7 @@ import "./edit-resource.scss"; import React from "react"; import { action, computed, makeObservable, observable } from "mobx"; import { observer } from "mobx-react"; -import jsYaml from "js-yaml"; +import yaml from "js-yaml"; import type { DockTab } from "./dock.store"; import { cssNames } from "../../utils"; import { editResourceStore } from "./edit-resource.store"; @@ -33,6 +33,7 @@ import { Badge } from "../badge"; import { EditorPanel } from "./editor-panel"; import { Spinner } from "../spinner"; import type { KubeObject } from "../../../common/k8s-api/kube-object"; +import { createPatch } from "rfc6902"; interface Props { className?: string; @@ -71,16 +72,17 @@ export class EditResource extends React.Component { return draft; } - return jsYaml.safeDump(this.resource.toPlainObject()); // dump resource first time + return yaml.safeDump(this.resource.toPlainObject()); // dump resource first time } @action saveDraft(draft: string | object) { if (typeof draft === "object") { - draft = draft ? jsYaml.safeDump(draft) : undefined; + draft = draft ? yaml.safeDump(draft) : undefined; } editResourceStore.setData(this.tabId, { + firstDraft: draft, // this must be before the next line ...editResourceStore.getData(this.tabId), draft, }); @@ -95,16 +97,18 @@ export class EditResource extends React.Component { if (this.error) { return null; } - const store = editResourceStore.getStore(this.tabId); - const updatedResource: KubeObject = await store.update(this.resource, jsYaml.safeLoad(this.draft)); - this.saveDraft(updatedResource.toPlainObject()); // update with new resourceVersion to avoid further errors on save - const resourceType = updatedResource.kind; - const resourceName = updatedResource.getName(); + const store = editResourceStore.getStore(this.tabId); + const currentVersion = yaml.safeLoad(this.draft); + const firstVersion = yaml.safeLoad(editResourceStore.getData(this.tabId).firstDraft ?? this.draft); + const patches = createPatch(firstVersion, currentVersion); + const updatedResource = await store.patch(this.resource, patches); + + editResourceStore.clearInitialDraft(this.tabId); return (

- {resourceType} {resourceName} updated. + {updatedResource.kind} {updatedResource.getName()} updated.

); }; @@ -126,9 +130,9 @@ export class EditResource extends React.Component { submittingMessage="Applying.." controls={(
- Kind: + Kind: Name: - Namespace: + Namespace:
)} /> diff --git a/src/renderer/components/dock/editor-panel.tsx b/src/renderer/components/dock/editor-panel.tsx index 31ccc5b2e2..bbf0b96e53 100644 --- a/src/renderer/components/dock/editor-panel.tsx +++ b/src/renderer/components/dock/editor-panel.tsx @@ -91,10 +91,7 @@ export class EditorPanel extends React.Component { onChange = (value: string) => { this.validate(value); - - if (this.props.onChange) { - this.props.onChange(value, this.yamlError); - } + this.props.onChange?.(value, this.yamlError); }; render() { diff --git a/src/renderer/components/kube-object-menu/kube-object-menu.tsx b/src/renderer/components/kube-object-menu/kube-object-menu.tsx index 1848fa2906..ac2a99cc88 100644 --- a/src/renderer/components/kube-object-menu/kube-object-menu.tsx +++ b/src/renderer/components/kube-object-menu/kube-object-menu.tsx @@ -44,15 +44,11 @@ export class KubeObjectMenu extends React.Component