Use JSON patch for editing components (#3674)

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",

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<KubeJsonApiData | null> {
-    if (typeof resource === "string") {
-      resource = jsYaml.safeLoad(resource);
-    }
-
-    const [data = null] = await apiBase.post<KubeJsonApiData[]>("/stack", { data: resource });
-
-    return data;
+export async function update(resource: object | string): Promise<KubeJsonApiData> {
+  if (typeof resource === "string") {
+    resource = jsYaml.safeLoad(resource);
   }
-};
+
+  return apiBase.post<KubeJsonApiData>("/stack", { data: resource });
+}
+
+export async function patch(name: string, kind: string, ns: string, patch: Patch): Promise<KubeJsonApiData> {
+  return apiBase.patch<KubeJsonApiData>("/stack", {
+    data: {
+      name,
+      kind,
+      ns,
+      patch,
+    },
+  });
+}

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<K extends KubeObject> {
   namespaces: string[];
@@ -279,19 +280,29 @@ export abstract class KubeObjectStore<T extends KubeObject> extends ItemStore<T>
     return newItem;
   }
 
-  async update(item: T, data: Partial<T>): Promise<T> {
-    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<T> {
+    return this.postUpdate(await item.patch(patch));
+  }
+
+  async update(item: T, data: Partial<T>): Promise<T> {
+    return this.postUpdate(await item.update(data));
+  }
+
   async remove(item: T) {
     await item.delete();
     this.items.remove(item);

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<K extends KubeObject> = (new (data: KubeJsonApiData | any) => K) & {
   kind?: string;
@@ -191,18 +191,28 @@ export class KubeObject<Metadata extends KubeObjectMetadata = KubeObjectMetadata
     return Object.entries(labels).map(([name, value]) => `${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<Metadata extends KubeObjectMetadata = KubeObjectMetadata
     return JSON.parse(JSON.stringify(this));
   }
 
-  // use unified resource-applier api for updating all k8s objects
-  async update(data: Partial<this>): Promise<KubeJsonApiData | null> {
-    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<KubeJsonApiData | null> {
+    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<this>): Promise<KubeJsonApiData | null> {
+    // use unified resource-applier api for updating all k8s objects
     return resourceApplierApi.update({
       ...this.toPlainObject(),
       ...data,

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<string> {
+    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<string> {
     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<string> {
     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<string>((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<string> {

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);
   }
 }

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);
     }

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;

src/renderer/components/+config-maps/config-map-details.tsx

@@ -72,6 +72,8 @@ export class ConfigMapDetails extends React.Component<Props> {
           <>ConfigMap <b>{configMap.getName()}</b> successfully updated.</>
         </p>
       );
+    } catch (error) {
+      Notifications.error(`Failed to save config map: ${error}`);
     } finally {
       this.isSaving = false;
     }

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("<DeploymentScaleDialog />", () => {

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("<ReplicaSetScaleDialog />", () => {

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("<StatefulSetScaleDialog />", () => {

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<Props> {
     });
   };
 
-  create = async () => {
+  create = async (): Promise<undefined> => {
     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<Props> {
 
     // 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 = (
-      <p>
-        {createdResources.length === 1 ? "Resource" : "Resources"}{" "}
-        <b>{createdResources.join(", ")}</b> successfully created
-      </p>
-    );
 
-    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((
+        <p>
+          {createdResources.length === 1 ? "Resource" : "Resources"}{" "}
+          <b>{createdResources.join(", ")}</b> successfully created
+        </p>
+      ));
+    }
+
+    return undefined;
   };
 
   renderControls(){

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<EditingResource> {
@@ -106,6 +107,10 @@ export class EditResourceStore extends DockTabStore<EditingResource> {
     return dockStore.getTabById(tabId);
   }
 
+  clearInitialDraft(tabId: TabId): void {
+    delete this.getData(tabId)?.firstDraft;
+  }
+
   reset() {
     super.reset();
     Array.from(this.watchers).forEach(([tabId, dispose]) => {

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<Props> {
       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<Props> {
     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 (
       <p>
-        {resourceType} <b>{resourceName}</b> updated.
+        {updatedResource.kind} <b>{updatedResource.getName()}</b> updated.
       </p>
     );
   };
@@ -126,9 +130,9 @@ export class EditResource extends React.Component<Props> {
           submittingMessage="Applying.."
           controls={(
             <div className="resource-info flex gaps align-center">
-              <span>Kind:</span> <Badge label={resource.kind}/>
+              <span>Kind:</span><Badge label={resource.kind}/>
               <span>Name:</span><Badge label={resource.getName()}/>
-              <span>Namespace:</span> <Badge label={resource.getNs() || "global"}/>
+              <span>Namespace:</span><Badge label={resource.getNs() || "global"}/>
             </div>
           )}
         />

src/renderer/components/dock/editor-panel.tsx

@@ -91,10 +91,7 @@ export class EditorPanel extends React.Component<Props> {
 
   onChange = (value: string) => {
     this.validate(value);
-
-    if (this.props.onChange) {
-      this.props.onChange(value, this.yamlError);
-    }
+    this.props.onChange?.(value, this.yamlError);
   };
 
   render() {

src/renderer/components/kube-object-menu/kube-object-menu.tsx

@@ -44,15 +44,11 @@ export class KubeObjectMenu<T extends KubeObject> extends React.Component<KubeOb
   }
 
   get isEditable() {
-    const { editable } = this.props;
-
-    return editable !== undefined ? editable : !!(this.store && this.store.update);
+    return this.props.editable ?? Boolean(this.store?.patch);
   }
 
   get isRemovable() {
-    const { removable } = this.props;
-
-    return removable !== undefined ? removable : !!(this.store && this.store.remove);
+    return this.props.removable ?? Boolean(this.store?.remove);
   }
 
   @boundMethod

yarn.lock

@@ -12277,6 +12277,11 @@ rfc4648@^1.3.0:
   resolved "https://registry.yarnpkg.com/rfc4648/-/rfc4648-1.3.0.tgz#2a69c76f05bc0e388feab933672de9b492af95f1"
   integrity sha512-x36K12jOflpm1V8QjPq3I+pt7Z1xzeZIjiC8J2Oxd7bE1efTrOG241DTYVJByP/SxR9jl1t7iZqYxDX864jgBQ==
 
+rfc6902@^4.0.2:
+  version "4.0.2"
+  resolved "https://registry.yarnpkg.com/rfc6902/-/rfc6902-4.0.2.tgz#ce99d3562b9e3287d403462e6bcc81eead8fcea0"
+  integrity sha512-MJOC4iDSv3Qn5/QvhPbrNoRongti6moXSShcRmtbNqOk0WPxlviEdMV4bb9PaULhSxLUXzWd4AjAMKQ3j3y54w==
+
 rimraf@2.6.3:
   version "2.6.3"
   resolved "https://registry.yarnpkg.com/rimraf/-/rimraf-2.6.3.tgz#b2d104fe0d8fb27cf9e0a1cda8262dd3833c6cab"