Fix crash in CRD display (#5256)

src/renderer/components/+custom-resources/crd-resource-details.tsx

@@ -6,7 +6,6 @@
 import "./crd-resource-details.scss";
 
 import React from "react";
-import jsonPath from "jsonpath";
 import { observer } from "mobx-react";
 import { cssNames } from "../../utils";
 import { Badge } from "../badge";
@@ -16,10 +15,11 @@ import { KubeObjectMeta } from "../kube-object-meta";
 import { Input } from "../input";
 import type { AdditionalPrinterColumnsV1 } from "../../../common/k8s-api/endpoints/crd.api";
 import { CustomResourceDefinition } from "../../../common/k8s-api/endpoints/crd.api";
-import { parseJsonPath } from "../../utils/jsonPath";
+import { convertKubectlJsonPathToNodeJsonPath } from "../../utils/jsonPath";
 import type { KubeObjectMetadata, KubeObjectStatus } from "../../../common/k8s-api/kube-object";
 import { KubeObject } from "../../../common/k8s-api/kube-object";
 import logger from "../../../common/logger";
+import { JSONPath } from "@astronautlabs/jsonpath";
 
 export interface CustomResourceDetailsProps extends KubeObjectDetailsProps<KubeObject> {
   crd: CustomResourceDefinition;
@@ -48,9 +48,9 @@ function convertSpecValue(value: any): any {
 @observer
 export class CustomResourceDetails extends React.Component<CustomResourceDetailsProps> {
   renderAdditionalColumns(resource: KubeObject, columns: AdditionalPrinterColumnsV1[]) {
-    return columns.map(({ name, jsonPath: jp }) => (
+    return columns.map(({ name, jsonPath }) => (
       <DrawerItem key={name} name={name} renderBoolean>
-        {convertSpecValue(jsonPath.value(resource, parseJsonPath(jp.slice(1))))}
+        {convertSpecValue(JSONPath.query(resource, convertKubectlJsonPathToNodeJsonPath(jsonPath)))}
       </DrawerItem>
     ));
   }

src/renderer/components/+custom-resources/crd-resources.tsx

@@ -6,14 +6,13 @@
 import "./crd-resources.scss";
 
 import React from "react";
-import { value } from "jsonpath";
 import { observer } from "mobx-react";
 import { KubeObjectListLayout } from "../kube-object-list-layout";
 import type { IComputedValue } from "mobx";
 import { computed, makeObservable } from "mobx";
 import { crdStore } from "./crd.store";
 import { apiManager } from "../../../common/k8s-api/api-manager";
-import { parseJsonPath } from "../../utils/jsonPath";
+import { safeJSONPathValue } from "../../utils/jsonPath";
 import { TabLayout } from "../layout/tab-layout-2";
 import { withInjectables } from "@ogre-tools/injectable-react";
 import customResourcesRouteParametersInjectable from "./custom-resources-route-parameters.injectable";
@@ -70,7 +69,7 @@ class NonInjectedCrdResources extends React.Component<Dependencies> {
             [columnId.age]: customResource => -customResource.getCreationTimestamp(),
             ...Object.fromEntries(extraColumns.map(({ name, jsonPath }) => [
               name,
-              customResource => value(customResource, parseJsonPath(jsonPath.slice(1))),
+              customResource => safeJSONPathValue(customResource, jsonPath),
             ])),
           }}
           searchFilters={[
@@ -95,22 +94,11 @@ class NonInjectedCrdResources extends React.Component<Dependencies> {
             })),
             { title: "Age", className: "age", sortBy: columnId.age, id: columnId.age },
           ]}
-          renderTableContents={crdInstance => [
-            crdInstance.getName(),
-            isNamespaced && crdInstance.getNs(),
-            ...extraColumns.map((column) => {
-              let rawValue = value(crdInstance, parseJsonPath(column.jsonPath.slice(1)));
-
-              if (Array.isArray(rawValue) || typeof rawValue === "object") {
-                rawValue = JSON.stringify(rawValue);
-              }
-
-              return {
-                renderBoolean: true,
-                children: rawValue,
-              };
-            }),
-            <KubeObjectAge key="age" object={crdInstance} />,
+          renderTableContents={customResource => [
+            customResource.getName(),
+            isNamespaced && customResource.getNs(),
+            ...extraColumns.map((column) => safeJSONPathValue(customResource, column.jsonPath)),
+            <KubeObjectAge key="age" object={customResource} />,
           ]}
           failedToLoadMessage={(
             <>

src/renderer/utils/__tests__/jsonPath.test.tsx

@@ -3,44 +3,166 @@
  * Licensed under MIT License. See LICENSE in root directory for more information.
  */
 
-import { parseJsonPath } from "../jsonPath";
+import { convertKubectlJsonPathToNodeJsonPath, safeJSONPathValue } from "../jsonPath";
 
 describe("parseJsonPath", () => {
-  test("should convert \\. to use indexed notation", () => {
-    const res = parseJsonPath(".metadata.labels.kubesphere\\.io/alias-name");
+  it("should convert \\. to use indexed notation", () => {
+    const res = convertKubectlJsonPathToNodeJsonPath(".metadata.labels.kubesphere\\.io/alias-name");
 
-    expect(res).toBe(".metadata.labels['kubesphere.io/alias-name']");
+    expect(res).toBe("$.metadata.labels['kubesphere.io/alias-name']");
   });
 
-  test("should convert keys with escaped characters to use indexed notation", () => {
-    const res = parseJsonPath(".metadata.labels.kubesphere\\\"io/alias-name");
+  it("should convert keys with escaped characters to use indexed notation", () => {
+    const res = convertKubectlJsonPathToNodeJsonPath(".metadata.labels.kubesphere\\\"io/alias-name");
 
-    expect(res).toBe(".metadata.labels['kubesphere\"io/alias-name']");
+    expect(res).toBe("$.metadata.labels['kubesphere\"io/alias-name']");
   });
 
-  test("should convert '-' to use indexed notation", () => {
-    const res = parseJsonPath(".metadata.labels.alias-name");
+  it("should convert '-' to use indexed notation", () => {
+    const res = convertKubectlJsonPathToNodeJsonPath(".metadata.labels.alias-name");
 
-    expect(res).toBe(".metadata.labels['alias-name']");
+    expect(res).toBe("$.metadata.labels['alias-name']");
   });
 
-  test("should handle scenario when both \\. and indexed notation are present", () => {
-    const rest = parseJsonPath(".metadata.labels\\.serving['some.other.item']");
+  it("should drop leading dot if first group is converted to index notation", () => {
+    const res = convertKubectlJsonPathToNodeJsonPath(".metadata\\.labels.alias-name");
 
-    expect(rest).toBe(".metadata['labels.serving']['some.other.item']");
+    expect(res).toBe("$['metadata.labels']['alias-name']");
   });
 
+  it("should handle scenario when both \\. and indexed notation are present", () => {
+    const rest = convertKubectlJsonPathToNodeJsonPath(".metadata.labels\\.serving['some.other.item']");
 
-  test("should not touch given jsonPath if no invalid characters present", () => {
-    const res = parseJsonPath(".status.conditions[?(@.type=='Ready')].status");
-
-    expect(res).toBe(".status.conditions[?(@.type=='Ready')].status");
+    expect(rest).toBe("$.metadata['labels.serving']['some.other.item']");
   });
 
-  test("strips '\\' away from the result", () => {
-    const res = parseJsonPath(".metadata.labels['serving\\.knative\\.dev/configuration']");
+  it("should not touch given jsonPath if no invalid characters present", () => {
+    const res = convertKubectlJsonPathToNodeJsonPath(".status.conditions[?(@.type=='Ready')].status");
 
-    expect(res).toBe(".metadata.labels['serving.knative.dev/configuration']");
+    expect(res).toBe("$.status.conditions[?(@.type=='Ready')].status");
   });
 
+  it("strips '\\' away from the result", () => {
+    const res = convertKubectlJsonPathToNodeJsonPath(".metadata.labels['serving\\.knative\\.dev/configuration']");
+
+    expect(res).toBe("$.metadata.labels['serving.knative.dev/configuration']");
+  });
+
+  it("converts all [] to [0]", () => {
+    const res = convertKubectlJsonPathToNodeJsonPath(".metadata.labels[].foo[]");
+
+    expect(res).toBe("$.metadata.labels[0].foo[0]");
+  });
+
+  it("converts ending .. to ..*", () => {
+    const res = convertKubectlJsonPathToNodeJsonPath(".metadata.labels[]..");
+
+    expect(res).toBe("$.metadata.labels[0]");
+  });
+
+  it("converts ending ...name to ..name", () => {
+    const res = convertKubectlJsonPathToNodeJsonPath(".metadata.labels[]...name");
+
+    expect(res).toBe("$.metadata.labels[0]..name");
+  });
+});
+
+describe("safeJSONPathValue", () => {
+  let oldWarn: typeof console["warn"];
+
+  beforeEach(() => {
+    oldWarn = console.warn;
+    console.warn = jest.fn();
+  });
+
+  afterEach(() => {
+    console.warn = oldWarn;
+  });
+
+  it("should convert boolean values to strings", () => {
+    const res = safeJSONPathValue({ bar: false }, ".bar");
+
+    expect(res).toBe("false");
+  });
+
+  it("should convert number values to strings", () => {
+    const res = safeJSONPathValue({ bar: 0 }, ".bar");
+
+    expect(res).toBe("0");
+  });
+
+  it("should join sliced entries with commas only", () => {
+    const res = safeJSONPathValue({
+      bar: [
+        {
+          foo: 1,
+        },
+        {
+          foo: "hello",
+        },
+      ],
+    }, ".bar[].foo");
+
+    expect(res).toBe("1");
+  });
+
+  it("should join an array of values using JSON.stringify", () => {
+    const res = safeJSONPathValue({
+      bar: [
+        "world",
+        "hello",
+      ],
+    }, ".bar");
+
+    expect(res).toBe(`["world","hello"]`);
+  });
+
+  it("should stringify an object value", () => {
+    const res = safeJSONPathValue({
+      foo: { bar: "bat" },
+    }, ".foo");
+
+    expect(res).toBe(`{"bar":"bat"}`);
+  });
+
+  it("should use convertKubectlJsonPathToNodeJsonPath", () => {
+    const res = safeJSONPathValue({
+      foo: { "hello.world": "bat" },
+    }, ".foo.hello\\.world");
+
+    expect(res).toBe("bat");
+  });
+
+  it("should not throw when given '.spec.metrics[*].external.highWatermark..'", () => {
+    const obj = {
+      spec: {
+        metrics: [
+          {
+            external: {
+              metricName: "cpu",
+              highWatermark: "100",
+            },
+          },
+          {
+            external: {
+              metricName: "memory",
+              highWatermark: "100",
+            },
+          },
+        ],
+      },
+    };
+
+    const res = safeJSONPathValue(obj, ".spec.metrics[*].external.highWatermark..");
+
+    expect(res).toBe("100, 100");
+  });
+
+  it("should not throw if path is invalid jsonpath", () => {
+    const res = safeJSONPathValue({
+      foo: { "hello.world": "bat" },
+    }, "asd[");
+
+    expect(res).toBe("<unknown>");
+  });
 });

src/renderer/utils/jsonPath.ts

@@ -3,28 +3,63 @@
  * Licensed under MIT License. See LICENSE in root directory for more information.
  */
 
-// Helper to convert strings used for jsonPath where \. or - is present to use indexed notation,
-// for example: .metadata.labels.kubesphere\.io/alias-name -> .metadata.labels['kubesphere\.io/alias-name']
+import { JSONPath } from "@astronautlabs/jsonpath";
 
-export function parseJsonPath(jsonPath: string) {
-  let pathExpression = jsonPath;
+const slashDashSearch = /[\\-]/g;
+const pathByBareDots = /(?<=\w)\./;
+const textBeforeFirstSquare = /^.*(?=\[)/g;
+const backSlash = /\\/g;
+const kubectlOptionPrefix = /^\$?\.?(?<pathExpression>.*)/;
+const sliceVersion = /\[]/g;
+const tripleDotName = /\.\.\.(?<trailing>.)/g;
+const trailingDotDot = /\.\.$/;
 
-  if (jsonPath.match(/[\\-]/g)) { // search for '\' and '-'
-    const [first, ...rest] = jsonPath.split(/(?<=\w)\./); // split jsonPath by '.' (\. cases are ignored)
+/**
+ * The GO package that kubectl and kubernetes uses for its JSONpath implementation has some
+ * shorthand conveniences that are not part of the official spec. This function tries to convert
+ * those shorthands to the official spec.
+ *
+ * Known shorthands:
+ * - Leading `$` is optional (but implied)
+ * - The string `\.` is used to denote the "value of '.'" and not "next key"
+ * - The string `-` can be used while not in quotes
+ * - `[]` as shorthand for `[0]`
+ * - Remove `..` at the end of a path, we will just format it slightly differently
+ * - Allow `...foo` as well as `..foo`
+ */
+export function convertKubectlJsonPathToNodeJsonPath(jsonPath: string) {
+  const startMatch = jsonPath.match(kubectlOptionPrefix);
+  let start = "$";
+
+  if (!startMatch) {
+    return start;
+  }
+
+  let { pathExpression } = startMatch.groups;
+
+  if (pathExpression.match(slashDashSearch)) {
+    const [first, ...rest] = pathExpression.split(pathByBareDots);
 
     pathExpression = `${convertToIndexNotation(first, true)}${rest.map(value => convertToIndexNotation(value)).join("")}`;
   }
 
+  pathExpression = pathExpression.replace(trailingDotDot, "");
+  pathExpression = pathExpression.replace(tripleDotName, "..$<trailing>");
+
+  if (!pathExpression.startsWith("[")) {
+    start += ".";
+  }
+
   // strip '\' characters from the result
-  return pathExpression.replace(/\\/g, "");
+  return `${start}${pathExpression.replace(backSlash, "").replace(sliceVersion, "[0]")}`;
 }
 
 function convertToIndexNotation(key: string, firstItem = false) {
-  if (key.match(/[\\-]/g)) { // check if found '\' and '-' in key
+  if (key.match(slashDashSearch)) {
     if (key.includes("[")) { // handle cases where key contains [...]
-      const keyToConvert = key.match(/^.*(?=\[)/g); // get the text from the key before '['
+      const keyToConvert = key.match(textBeforeFirstSquare); // get the text from the key before '['
 
-      if (keyToConvert && keyToConvert[0].match(/[\\-]/g)) { // check if that part contains illegal characters
+      if (keyToConvert && keyToConvert[0].match(slashDashSearch)) { // check if that part contains illegal characters
         return key.replace(keyToConvert[0], `['${keyToConvert[0]}']`); // surround key with '[' and ']'
       } else {
         return `.${key}`; // otherwise return as is with leading '.'
@@ -38,3 +73,35 @@ function convertToIndexNotation(key: string, firstItem = false) {
     return `${prefix}${key}`;
   }
 }
+
+function formatJSONValue(value: unknown) {
+  if (typeof value === "object") {
+    return JSON.stringify(value);
+  }
+
+  return String(value);
+}
+
+/**
+ * This function is a safer version of `JSONPath.value(obj, path)` with untrusted jsonpath strings
+ *
+ * This function will also stringify the value retreived from the object
+ */
+export function safeJSONPathValue(obj: object, path: string): string {
+  try {
+    const parsedPath = JSONPath.parse(convertKubectlJsonPathToNodeJsonPath(path));
+    const isSlice = parsedPath.some((exp: any) => exp.expression.type === "slice" || "wildcard");
+    const value = JSONPath.query(obj, JSONPath.stringify(parsedPath), isSlice ? Infinity : 1);
+
+    if (isSlice) {
+      return value.map(formatJSONValue).join(", ");
+    }
+
+    return formatJSONValue(value[0]);
+  } catch (error) {
+    // something failed
+    console.warn("[JSON-PATH]: failed to parse jsonpath", error);
+
+    return "<unknown>";
+  }
+}