Merge pull request #145 from openshift/manual-cherry-pick-api-version

jgbernalp · web-flow · commit 3f33a135a622 · 2025-06-05T20:04:54.000+02:00
COO-982: Manual cherry pick api version and sync with main changes
diff --git a/web/src/__tests__/alert.spec.ts b/web/src/__tests__/alert.spec.ts
@@ -1,5 +1,5 @@
-import { URIRef, Query } from '../korrel8r/types';
 import { AlertDomain } from '../korrel8r/alert';
+import { Query, URIRef } from '../korrel8r/types';
 
 describe('AlertNode.fromURL', () => {
   it.each([
@@ -33,7 +33,7 @@ describe('AlertDomain.fromQuery', () => {
     },
     { query: 'alert:alert:{}', url: 'monitoring/alerts' },
   ])('converts $query', ({ url, query }) => {
-    expect(new AlertDomain().queryToLink(Query.parse(query))).toEqual(url);
+    expect(new AlertDomain().queryToLink(Query.parse(query))).toEqual(new URIRef(url));
   });
 
   it('Query => URL => Query', () => {
@@ -42,7 +42,7 @@ describe('AlertDomain.fromQuery', () => {
     const got =
       'monitoring/alerts?alerts=alertname%3DKubePodCrashLooping%2Ccontainer%3Dbad-deployment%2Cnamespace%3Ddefault%2Cpod%3Dbad-pod';
     const want = new AlertDomain().queryToLink(Query.parse(query));
-    expect(want).toEqual(got);
-    expect(new AlertDomain().linkToQuery(new URIRef(want))).toEqual(Query.parse(query));
+    expect(want).toEqual(new URIRef(got));
+    expect(new AlertDomain().linkToQuery(want)).toEqual(Query.parse(query));
   });
 });
diff --git a/web/src/__tests__/all-domains.spec.ts b/web/src/__tests__/all-domains.spec.ts
@@ -72,5 +72,5 @@ it.each([
   },
 ])('convert URL<=>link', ({ url, query, constraint }) => {
   expect(allDomains.linkToQuery(new URIRef(url))).toEqual(Query.parse(query));
-  expect(allDomains.queryToLink(Query.parse(query), Constraint.fromAPI(constraint))).toEqual(url);
+  expect(allDomains.queryToLink(Query.parse(query), Constraint.fromAPI(constraint))).toEqual(new URIRef(url));
 });
diff --git a/web/src/__tests__/k8s.spec.ts b/web/src/__tests__/k8s.spec.ts
@@ -107,44 +107,44 @@ describe('K8sNode.fromQuery', () => {
     // Note "fields" are ignored.
     {
       query: 'k8s:Pod.v1:{"namespace":"default","name":"bad-deployment-000000000-00000"}',
-      u: 'k8s/ns/default/pods/bad-deployment-000000000-00000',
+      url: 'k8s/ns/default/pods/bad-deployment-000000000-00000',
     },
     {
       query: `k8s:Pod:{"namespace":"x","name":"y","labels":{"a":"b","c":"d"},"fields": {"x":"y"}}`,
-      u: `k8s/ns/x/pods/y?labels=${encodeURIComponent('a=b,c=d')}`,
+      url: `k8s/ns/x/pods/y?labels=${encodeURIComponent('a=b,c=d')}&fields=${encodeURIComponent('x=y')}`,
     },
     {
       query:
         'k8s:Event.v1:{"fields":{"involvedObject.namespace":"default","involvedObject.name":"bad-deployment-000000000-00000","involvedObject.apiVersion":"v1","involvedObject.kind":"Pod"}}',
-      u: 'k8s/ns/default/pods/bad-deployment-000000000-00000/events',
+      url: 'k8s/ns/default/pods/bad-deployment-000000000-00000/events',
     },
     {
-      query: `k8s:Pod:{"namespace":"x","name":"y","labels":{"a":"b"}}`,
-      u: `k8s/ns/x/pods/y?labels=${encodeURIComponent('a=b')}`,
+      query: `k8s:Pod:{ "namespace":"x", "name":"y", "labels":{ "a":"b" } }`,
+      url: `k8s/ns/x/pods/y?labels=${encodeURIComponent('a=b')}`,
     },
     {
-      query: `k8s:Pod:{"namespace":"x","labels":{"a":"b"}}`,
-      u: `k8s/ns/x/pods?labels=${encodeURIComponent('a=b')}`,
+      query: `k8s:Pod:{ "namespace":"x", "labels":{ "a":"b" } }`,
+      url: `k8s/ns/x/pods?labels=${encodeURIComponent('a=b')}`,
     },
-    { query: `k8s:Pod.v1:{"namespace":"x","name":"y"}`, u: `k8s/ns/x/pods/y` },
-    { query: `k8s:Pod.v1:{"namespace":"x"}`, u: `k8s/ns/x/pods` },
+    { query: `k8s:Pod.v1:{ "namespace":"x", "name":"y" }`, url: `k8s/ns/x/pods/y` },
+    { query: `k8s:Pod.v1:{ "namespace":"x" }`, url: `k8s/ns/x/pods` },
     {
-      query: `k8s:Pod.v1:{"labels":{"a":"b"}}`,
-      u: `search/all-namespaces?labels=${encodeURIComponent('a=b')}&kind=core~v1~Pod`,
+      query: `k8s:Pod.v1:{ "labels":{ "a":"b" } }`,
+      url: `search/all-namespaces?labels=${encodeURIComponent('a=b')}&kind=core~v1~Pod`,
     },
     {
       query: `k8s:Pod.v1:{"namespace":"x","labels":{"a":"b"}}`,
-      u: `k8s/ns/x/pods?labels=${encodeURIComponent('a=b')}`,
+      url: `k8s/ns/x/pods?labels=${encodeURIComponent('a=b')}`,
     },
 
     // Variations on korrel8r class spec.
     {
-      query: `k8s:Role.v1.rbac.authorization.k8s.io:{"namespace":"x", "name":"y"}`,
-      u: `k8s/ns/x/roles/y`,
+      query: `k8s:Role.v1.rbac.authorization.k8s.io:{ "namespace":"x", "name":"y" }`,
+      url: `k8s/ns/x/roles/y`,
     },
-    { query: `k8s:Pod:{}`, u: 'k8s/all-namespaces/pods' },
-  ])('converts $query to $url', ({ u: u, query }) => {
-    expect(k8s.queryToLink(Query.parse(query))).toEqual(u);
+    { query: `k8s:Pod:{}`, url: 'k8s/all-namespaces/pods' },
+  ])('converts $query to $url', ({ url, query }) => {
+    expect(k8s.queryToLink(Query.parse(query))).toEqual(new URIRef(url));
   });
 
   it.each([{ query: `foo:bar:baz` }])('raises error on $query', ({ query }) =>
diff --git a/web/src/__tests__/log.spec.ts b/web/src/__tests__/log.spec.ts
@@ -13,7 +13,7 @@ describe('LogDomain.fromURL', () => {
     {
       url: `monitoring/logs?q=${encodeURIComponent(
         '{kubernetes_namespace_name="default",' +
-          'kubernetes_pod_name="foo",log_type="infrastructure"}',
+        'kubernetes_pod_name="foo",log_type="infrastructure"}',
       )}`,
       query:
         `log:infrastructure:{kubernetes_namespace_name="default",` +
@@ -75,7 +75,7 @@ describe('LogDomain.fromQuery', () => {
       }),
     },
   ])('$query', ({ url, query, constraint }) =>
-    expect(new LogDomain().queryToLink(Query.parse(query), constraint)).toEqual(url),
+    expect(new LogDomain().queryToLink(Query.parse(query), constraint)).toEqual(new URIRef(url)),
   );
 });
 
diff --git a/web/src/__tests__/metric.spec.ts b/web/src/__tests__/metric.spec.ts
@@ -1,63 +1,61 @@
-import { Query, URIRef } from '../korrel8r/types';
 import { MetricDomain } from '../korrel8r/metric';
+import { Query, URIRef } from '../korrel8r/types';
 
 /**
  * Bad deployment is the suggested deployment from korrel8r to show its functionality within the
  * logging plugin. https://korrel8r.github.io/korrel8r/#troubleshooting-no-related-logs
  *
  */
-describe('Test MetricNode Parsing', () => {
-  it('Query => URL => Query', () => {
-    const query =
-      'metric:metric:max_over_time(kube_pod_container_status_waiting_reason{job="kube-state-metrics",namespace=~"(openshift-.*|kube-.*|default)",reason="CrashLoopBackOff"}[5m]) <= 1';
-    const expectedKorrel8rURL =
-      'monitoring/query-browser?query0=max_over_time(kube_pod_container_status_waiting_reason{job="kube-state-metrics",namespace=~"(openshift-.*|kube-.*|default)",reason="CrashLoopBackOff"}[5m]) <= 1';
-    const d = new MetricDomain();
-    const actualKorrel8rURL = d.queryToLink(Query.parse(query));
-    expect(actualKorrel8rURL).toEqual(expectedKorrel8rURL);
-    expect(d.linkToQuery(new URIRef(actualKorrel8rURL))).toEqual(Query.parse(query));
-  });
+describe('metric', () => {
+  const metric = new MetricDomain()
+
+  describe('query-link round trip', () => {
+    it.each([
+      {
+        query: 'metric:metric:max_over_time(kube_pod_container_status_waiting_reason{job="kube-state-metrics",namespace=~"(openshift-.*|kube-.*|default)",reason="CrashLoopBackOff"}[5m]) <= 1',
+        link: 'monitoring/query-browser?query0=max_over_time%28kube_pod_container_status_waiting_reason%7Bjob%3D%22kube-state-metrics%22%2Cnamespace%3D%7E%22%28openshift-.*%7Ckube-.*%7Cdefault%29%22%2Creason%3D%22CrashLoopBackOff%22%7D%5B5m%5D%29+%3C%3D+1'
+      },
+      {
+        // Verify issue: https://github.com/openshift/troubleshooting-panel-console-plugin/issues/127
+        query: 'metric:metric:(1 - sum(node_memory_MemFree_bytes + node_memory_Buffers_bytes + node_memory_Cached_bytes and on (instance) label_replace(kube_node_role{role="master"}, "instance", "$1", "node", "(.+)")) / sum(node_memory_MemTotal_bytes and on (instance) label_replace(kube_node_role{role="master"}, "instance", "$1", "node", "(.+)"))) * 100 > 60',
+        link: 'monitoring/query-browser?query0=%281+-+sum%28node_memory_MemFree_bytes+%2B+node_memory_Buffers_bytes+%2B+node_memory_Cached_bytes+and+on+%28instance%29+label_replace%28kube_node_role%7Brole%3D%22master%22%7D%2C+%22instance%22%2C+%22%241%22%2C+%22node%22%2C+%22%28.%2B%29%22%29%29+%2F+sum%28node_memory_MemTotal_bytes+and+on+%28instance%29+label_replace%28kube_node_role%7Brole%3D%22master%22%7D%2C+%22instance%22%2C+%22%241%22%2C+%22node%22%2C+%22%28.%2B%29%22%29%29%29+*+100+%3E+60'
+      },
+    ])('', ({ query, link }) => {
+      const q = Query.parse(query)
+      expect(metric.queryToLink(q).toString()).toEqual(link)
+      expect(metric.linkToQuery(new URIRef(link))).toEqual(q);
+    })
 
-  it('URL => Query => URL', () => {
-    const url =
-      'monitoring/query-browser?query0=max_over_time(kube_pod_container_status_waiting_reason{job="kube-state-metrics",namespace=~"(openshift-.*|kube-.*|default)",reason="CrashLoopBackOff"}[5m]) <= 1';
-    const expectedQuery =
-      'metric:metric:max_over_time(kube_pod_container_status_waiting_reason{job="kube-state-metrics",namespace=~"(openshift-.*|kube-.*|default)",reason="CrashLoopBackOff"}[5m]) <= 1';
-    const d = new MetricDomain();
-    const actualQuery = d.linkToQuery(new URIRef(url));
-    expect(actualQuery).toEqual(Query.parse(expectedQuery));
-    expect(d.queryToLink(Query.parse(expectedQuery))).toEqual(url);
   });
 
-  describe('Test url to query parsing with expected errors', () => {
-    [
+  describe('linkToQuery errors', () => {
+    it.each([
       {
         url: 'foobar',
-        expected: new TypeError('domain metric: invalid link: foobar'),
+        error: new TypeError('domain metric: invalid link: foobar'),
       },
       {
         url: 'monitoring/query-browser',
-        expected: 'domain metric: invalid link: monitoring/query-browser',
+        error: 'domain metric: invalid link: monitoring/query-browser',
       },
-    ].forEach(({ url, expected }) => {
-      it(`converts from ${url}`, () => {
-        expect(() => new MetricDomain().linkToQuery(new URIRef(url))).toThrow(expected);
-      });
+    ])('$url', ({ url, error }) => {
+      expect(() => metric.linkToQuery(new URIRef(url))).toThrow(error);
+
     });
   });
 
-  it('Test query to url parsing with expected errors', () => {
-    [
+  describe('queryToLink errors', () => {
+    it.each([
       {
         query: 'wrongdomain:metric:foo',
-        expected: 'domain metric: invalid query, wrong domain: wrongdomain:metric:foo',
+        error: 'domain metric: invalid query, wrong domain: wrongdomain:metric:foo',
       },
       {
         query: 'metric:badclass:foo',
-        expected: 'domain metric: invalid class: badclass',
+        error: 'domain metric: invalid class: badclass',
       },
-    ].forEach(({ query, expected }) => {
-      expect(() => new MetricDomain().queryToLink(Query.parse(query))).toThrow(expected);
+    ])('$query', ({ query, error }) => {
+      expect(() => metric.queryToLink(Query.parse(query))).toThrow(error);
     });
   });
 });
diff --git a/web/src/__tests__/netflow.spec.ts b/web/src/__tests__/netflow.spec.ts
@@ -46,7 +46,7 @@ describe('NetflowNode.fromQuery', () => {
   ])(`from $query`, ({ query, url, constraint }) =>
     expect(
       new NetflowDomain().queryToLink(Query.parse(query), Constraint.fromAPI(constraint)),
-    ).toEqual(url),
+    ).toEqual(new URIRef(url)),
   );
 });
 
diff --git a/web/src/__tests__/trace.spec.ts b/web/src/__tests__/trace.spec.ts
@@ -1,5 +1,5 @@
-import { URIRef, Query } from '../korrel8r/types';
 import { TraceDomain } from '../korrel8r/trace';
+import { Query, URIRef } from '../korrel8r/types';
 
 const tempo = 'namespace=openshift-tracing&name=platform&tenant=platform';
 
@@ -38,6 +38,6 @@ describe('TraceDomain.fromQuery', () => {
       url: `observe/traces?${tempo}&q=%7Bresource.service.name%3D%22shop-backend%22%7D`,
     },
   ])('$query', ({ query, url }) => {
-    expect(new TraceDomain().queryToLink(Query.parse(query))).toEqual(url);
+    expect(new TraceDomain().queryToLink(Query.parse(query)).toString()).toEqual(url);
   });
 });
diff --git a/web/src/__tests__/types.spec.ts b/web/src/__tests__/types.spec.ts
@@ -36,10 +36,9 @@ class FakeDomain extends Domain {
     return new Class(this.name, m[1]).query(link.searchParams.toString());
   }
 
-  queryToLink(query: Query, constraint?: Constraint): string {
+  queryToLink(query: Query, constraint?: Constraint): URIRef {
     if (!query || !query.class || query.class.domain != this.name) throw this.badQuery(query);
-    const c = constraint ? '&constraint=' + JSON.stringify(constraint) : '';
-    return `${query.class.domain}/${query.class.name}?${query.selector}${c}`;
+    return new URIRef(`${query.class.domain}/${query.class.name}?${query.selector}`, { constraint: constraint });
   }
 }
 
@@ -65,7 +64,7 @@ describe('Domain', () => {
   const d = new FakeDomain('a');
   const abc = d.class('b').query('c=d');
   it('queryToLink', () => {
-    expect(d.queryToLink(abc)).toEqual('a/b?c=d');
+    expect(d.queryToLink(abc).toString()).toEqual('a/b?c=d');
     const query = Query.parse('x:b:c');
     expect(() => d.queryToLink(query)).toThrow('domain a: invalid query: x:b:c');
   });
@@ -81,8 +80,8 @@ describe('Domains', () => {
   const abc = Query.parse('a:b:c=d');
   const xyz = Query.parse('x:y:z=z');
   it('queryToLink', () => {
-    expect(ds.queryToLink(abc)).toEqual('a/b?c=d');
-    expect(ds.queryToLink(xyz)).toEqual('x/y?z=z');
+    expect(ds.queryToLink(abc).toString()).toEqual('a/b?c=d');
+    expect(ds.queryToLink(xyz).toString()).toEqual('x/y?z=z');
     expect(() => ds.queryToLink(Query.parse('z:b:c'))).toThrow(/unknown domain .*: z:b:c/);
   });
   it('linkToQuery', () => {
diff --git a/web/src/components/topology/Korrel8rTopology.tsx b/web/src/components/topology/Korrel8rTopology.tsx
@@ -147,8 +147,8 @@ export const Korrel8rTopology: React.FC<{
   const navigateToQuery = React.useCallback(
     (query: korrel8r.Query) => {
       try {
-        const link = allDomains.queryToLink(query);
-        navigate(link.startsWith('/') ? link : `/${link}`);
+        const link = allDomains.queryToLink(query)?.toString();
+        if (link) navigate(link.startsWith('/') ? link : `/${link}`);
       } catch (e) {
         // eslint-disable-next-line no-console
         console.error(`korrel8r navigateToQuery: ${e}`, "\nquery", query);
diff --git a/web/src/hooks/useLocationQuery.ts b/web/src/hooks/useLocationQuery.ts
@@ -8,8 +8,6 @@ export const useLocationQuery = (): Query | undefined => {
   try {
     const link = new URIRef(location.pathname + location.search);
     const q = allDomains.linkToQuery(link);
-    // eslint-disable-next-line no-console
-    console.log('korrel8r linkToQuery', "\nlink", link, "\nquery", q);
     return q;
   } catch (e) {
     // eslint-disable-next-line no-console
diff --git a/web/src/korrel8r/alert.ts b/web/src/korrel8r/alert.ts
@@ -1,4 +1,4 @@
-import { Domain, Class, Query, URIRef, keyValueList, parseKeyValueList } from './types';
+import { Class, Domain, Query, URIRef, keyValueList, parseKeyValueList } from './types';
 
 export class AlertDomain extends Domain {
   constructor() {
@@ -28,8 +28,8 @@ export class AlertDomain extends Domain {
     return new Query(this.class('alert'), JSON.stringify(selectors));
   }
 
-  queryToLink(query: Query): string {
+  queryToLink(query: Query): URIRef {
     const selectors = keyValueList(JSON.parse(query.selector));
-    return `monitoring/alerts${selectors ? `?alerts=${encodeURIComponent(selectors)}` : ''}`;
+    return new URIRef(`monitoring/alerts`, { alerts: selectors || undefined })
   }
 }
diff --git a/web/src/korrel8r/k8s.ts b/web/src/korrel8r/k8s.ts
@@ -21,7 +21,8 @@ type Selector = {
 const PATH_RE = new RegExp(
   '(?:k8s|search)/(?:(?:ns/([^/]+))|cluster|all-namespaces)(?:/([^/]+)(?:/([^/]+))?)?(/events)?/?$',
 );
-const CLASS_RE = new RegExp('^([^./]+)(?:.(v[0-9]+(?:(?:alpha|beta)[0-9]*)?))?(?:.([^/]*))?$');
+const VERSION_RE = '(v[0-9]+(?:(?:alpha|beta)[0-9]*)?)';
+const CLASS_RE = new RegExp(`^([^./]+)(?:.${VERSION_RE})?(?:.([^/]*))?$`);
 
 export class K8sDomain extends Domain {
   constructor() {
@@ -71,45 +72,50 @@ export class K8sDomain extends Domain {
     }
   }
 
-  queryToLink(query: Query): string {
+  queryToLink(query: Query): URIRef {
     let data: Selector;
     try {
       data = JSON.parse(query.selector) as Selector;
     } catch (e) {
       throw this.badQuery(query, e.message);
     }
     const m = query.class.name.match(/^([^.]+)(?:\.([^.]*)(?:\.(.*))?)?$/) ?? [];
-    if (!m) throw this.badQuery(query, "incorrect format");
+    if (!m) throw this.badQuery(query, 'incorrect format');
     let model = findGVK(m[3], m[2], m[1]);
-    if (!model) throw this.badQuery(query, "no matching resource");
+    if (!model) throw this.badQuery(query, 'no matching resource');
     let namespace = data.namespace;
     let name = data.name;
     let events = '';
     if (isEvent(model)) {
       // Special treatment for event objects: focus on the involved object, not the event.
       events = '/events';
       const about = eventAboutField(model);
-      const [group, version] = data.fields[`${about}.apiVersion`]?.split("/") ?? []
-      const kind = data.fields[`${about}.kind`]
-      model = findGVK(group, version, kind)
-      if (!model) throw this.badQuery(query, `no resource for group=${group} version=${version} kind=${kind}`);
+      const [group, version] = parseAPIVersion(data.fields[`${about}.apiVersion`]);
+      const kind = data.fields[`${about}.kind`];
+      model = findGVK(group, version, kind);
+      if (!model)
+        throw this.badQuery(
+          query,
+          `no resource for group=${group} version=${version} kind=${kind}`,
+        );
       namespace = data.fields[`${about}.namespace`] || '';
       name = data.fields[`${about}.name`] || '';
     }
     // Prepare parts of the URL
     const nsPath = namespace ? `ns/${namespace}` : 'all-namespaces';
-    const labelsParam = data.labels
-      ? `?labels=${encodeURIComponent(keyValueList(data.labels))}`
-      : '';
-    if (!name && !namespace && labelsParam) {
-      // Search URL
-      return (
-        `search/${nsPath}${labelsParam}&kind=` +
-        `${model.apiGroup || 'core'}~${model.apiVersion}~${model.kind}`
-      );
+    const params = {
+      labels: keyValueList(data.labels) || undefined,
+      fields: (!events && keyValueList(data.fields)) || undefined,
+    };
+    if (!name && !namespace && (params.labels || params.fields)) {
+      // This is a search URL
+      return new URIRef(`search/${nsPath}`, {
+        ...params,
+        kind: `${model.apiGroup || 'core'}~${model.apiVersion}~${model.kind}`,
+      });
     } else {
-      // Resource URL
-      return `k8s/${nsPath}/${model.path}${name ? `/${name}` : ''}${events}${labelsParam}`;
+      // Specific resource URL
+      return new URIRef(`k8s/${nsPath}/${model.path}${name ? `/${name}` : ''}${events}`, params);
     }
   }
 
@@ -182,3 +188,16 @@ function findResource(resource: string, kind: string): Model {
     parts[2],
   );
 }
+
+const VERSION_ONLY_RE = new RegExp(`^${VERSION_RE}$`);
+function parseAPIVersion(apiVersion: string): [group: string, version: string] | undefined {
+  const gv = apiVersion.split('/') || [];
+  switch (gv.length) {
+    case 1:
+      return gv[0].match(VERSION_ONLY_RE) ? ['', gv[0]] : [gv[0], ''];
+    case 2:
+      return [gv[0], gv[1]];
+    default:
+      return ['', ''];
+  }
+}
diff --git a/web/src/korrel8r/log.ts b/web/src/korrel8r/log.ts
diff --git a/web/src/korrel8r/metric.ts b/web/src/korrel8r/metric.ts
diff --git a/web/src/korrel8r/netflow.ts b/web/src/korrel8r/netflow.ts
diff --git a/web/src/korrel8r/trace.ts b/web/src/korrel8r/trace.ts
diff --git a/web/src/korrel8r/types.ts b/web/src/korrel8r/types.ts
