Merge pull request #168 from alanconway/podlog-url

NO-JIRA: fix: Generate correct URL for pod logs.

Author: openshift-merge-bot[bot]

Makefile

@@ -49,14 +49,14 @@ start-backend:
 install: install-frontend install-backend
 
 .PHONY: build-image
-build-image: test-frontend
+build-image: build-frontend test-frontend
 	./scripts/build-image.sh
 
 .PHONY: start-forward
 start-forward:
 	./scripts/start-forward.sh
 
-export REGISTRY_ORG?=openshift-observability-ui
+export REGISTRY_ORG?=	openshift-observability-ui
 export TAG?=latest
 IMAGE=quay.io/${REGISTRY_ORG}/troubleshooting-panel-console-plugin:${TAG}
 

web/src/__tests__/log.spec.ts

@@ -1,6 +1,23 @@
 import { LogDomain } from '../korrel8r/log';
 import { Constraint, Query, URIRef } from '../korrel8r/types';
 
+beforeAll(() => {
+  // Mock API pod resource for pod log queries.
+  const resources = {
+    consoleVersion: 'x.y.z',
+    models: [
+      {
+        kind: 'Pod',
+        apiVersion: 'v1',
+        path: 'pods',
+        verbs: ['watch'],
+      },
+    ],
+  };
+  localStorage.setItem('bridge/api-discovery-resources', JSON.stringify(resources));
+  window['SERVER_FLAGS'] = { consoleVersion: 'x.y.z' };
+});
+
 describe('LogDomain.linkToQuery', () => {
   it.each([
     {
@@ -55,27 +72,31 @@ describe('LogDomain.linkToQuery', () => {
 describe('LogDomain.queryToLink', () => {
   it.each([
     {
-      url: `monitoring/logs?q=${encodeURIComponent(
-        '{kubernetes_namespace_name="default",kubernetes_pod_name="foo"}',
-      )}&tenant=infrastructure&start=1742896800000&end=1742940000000`,
+      // LogQL query
       query: `log:infrastructure:{kubernetes_namespace_name="default",kubernetes_pod_name="foo"}`,
       constraint: Constraint.fromAPI({
         start: '2025-03-25T10:00:00.000Z',
         end: '2025-03-25T22:00:00.000Z',
       }),
+      url: new URIRef(`monitoring/logs`, {
+        q: '{kubernetes_namespace_name="default",kubernetes_pod_name="foo"}',
+        tenant: 'infrastructure',
+        start: 1742896800000,
+        end: 1742940000000,
+      }),
     },
     {
-      url: `monitoring/logs?q=${encodeURIComponent(
-        '{kubernetes_namespace_name="default",log_type="infrastructure"}',
-      )}&tenant=infrastructure&start=1742896800000&end=1742940000000`,
-      query: 'log:infrastructure:{kubernetes_namespace_name="default",log_type="infrastructure"}',
+      // k8s Pod query
+      query: 'log:infrastructure:{"namespace":"default","name":"foo"}',
       constraint: Constraint.fromAPI({
         start: '2025-03-25T10:00:00.000Z',
         end: '2025-03-25T22:00:00.000Z',
       }),
+
+      url: new URIRef('k8s/ns/default/core~v1~Pod/foo/aggregated-logs'),
     },
   ])('$query', ({ url, query, constraint }) =>
-    expect(new LogDomain().queryToLink(Query.parse(query), constraint)).toEqual(new URIRef(url)),
+    expect(new LogDomain().queryToLink(Query.parse(query), constraint)).toEqual(url),
   );
 });
 

web/src/__tests__/types.spec.ts

@@ -10,6 +10,7 @@ import {
   Node,
   Query,
   URIRef,
+  joinPath,
 } from '../korrel8r/types';
 
 describe('Query', () => {
@@ -184,3 +185,46 @@ describe('Graph', () => {
   expect(g.nodes).toEqual(a.nodes.map((n) => new Node(n)));
   expect(g.edges).toEqual(a.edges.map((e) => new Edge(g.node(e.start), g.node(e.goal))));
 });
+
+describe('joinPath', () => {
+  it.each([
+    // Basic path joining
+    ['path1', 'path2', 'path1/path2'],
+    ['path1', 'path2', 'path3', 'path1/path2/path3'],
+
+    // Handling trailing slash on first path
+    ['path1/', 'path2', 'path1/path2'],
+    ['path1//', 'path2', 'path1/path2'],
+
+    // Handling leading slashes on subsequent paths
+    ['path1', '/path2', 'path1/path2'],
+    ['path1', '//path2', 'path1/path2'],
+
+    // Handling trailing slashes on subsequent paths
+    ['path1', 'path2/', 'path1/path2'],
+    ['path1', 'path2//', 'path1/path2'],
+
+    // Complex combinations
+    ['path1/', '/path2/', '/path3/', 'path1/path2/path3'],
+    ['/path1/', '//path2//', '///path3///', '/path1/path2/path3'],
+
+    // Empty paths
+    ['', 'path2', '/path2'],
+    ['path1', '', 'path1/'],
+    ['', '', '/'],
+
+    // Single path
+    ['single', 'single'],
+    ['single/', 'single'],
+    ['/single/', '/single'],
+
+    // Absolute paths
+    ['/absolute', 'relative', '/absolute/relative'],
+    ['/absolute/', '/relative/', '/absolute/relative'],
+  ] as Array<string[]>)('joins paths correctly: %s', (...args: string[]) => {
+    const expected = args.pop() as string;
+    const paths = args as string[];
+    const [first, ...rest] = paths;
+    expect(joinPath(first, ...rest)).toEqual(expected);
+  });
+});

web/src/components/Korrel8rPanel.tsx

@@ -12,6 +12,7 @@ import {
   FlexItem,
   Form,
   FormGroup,
+  Spinner,
   TextArea,
   Title,
   Tooltip,
@@ -42,7 +43,6 @@ import './korrel8rpanel.css';
 import { SearchFormGroup } from './SearchFormGroup';
 import TimeRangeFormGroup from './TimeRangeFormGroup';
 import { Korrel8rTopology } from './topology/Korrel8rTopology';
-import { LoadingTopology } from './topology/LoadingTopology';
 
 export default function Korrel8rPanel() {
   const { t } = useTranslation('plugin__troubleshooting-panel-console-plugin');
@@ -156,14 +156,14 @@ export default function Korrel8rPanel() {
     <Tooltip content={locationQuery ? focusTip : cannotFocus}>
       <Button
         isAriaDisabled={!locationQuery}
-        onClick={() =>
+        onClick={() => {
           runSearch({
             ...defaultSearch,
             queryStr: locationQuery?.toString(),
             constraint: searchResult?.search?.constraint,
             period: searchResult?.search?.period,
-          })
-        }
+          });
+        }}
       >
         {t('Focus')}
       </Button>
@@ -185,7 +185,12 @@ export default function Korrel8rPanel() {
 
   const refreshButton = (
     <Tooltip content={t('Refresh the graph using the current settings')}>
-      <Button isAriaDisabled={!search?.queryStr} onClick={() => runSearch(search)}>
+      <Button
+        isAriaDisabled={!search?.queryStr}
+        onClick={() => {
+          runSearch(search);
+        }}
+      >
         <SyncIcon />
       </Button>
     </Tooltip>
@@ -301,18 +306,20 @@ const Topology: React.FC<TopologyProps> = ({ domains, result, t, constraint }) =
   );
 };
 
-const Loading: React.FC = () => (
-  <>
+const Loading: React.FC = () => {
+  const { t } = useTranslation('plugin__troubleshooting-panel-console-plugin');
+  return (
     <div className="tp-plugin__panel-topology-info">
-      <div className={'co-m-loader co-an-fade-in-out tp-plugin__panel-topology-info'}>
-        <div className="co-m-loader-dot__one" />
-        <div className="co-m-loader-dot__two" />
-        <div className="co-m-loader-dot__three" />
-      </div>
+      <EmptyState variant={EmptyStateVariant.sm}>
+        <EmptyStateHeader
+          titleText={t('Loading')}
+          headingLevel="h4"
+          icon={<EmptyStateIcon icon={Spinner} />}
+        />
+      </EmptyState>
     </div>
-    <LoadingTopology />
-  </>
-);
+  );
+};
 
 interface TopologyInfoStateProps {
   titleText: string;

web/src/components/topology/LoadingTopology.tsx

@@ -1,22 +1,30 @@
-import { Class, Constraint, Domain, Query, unixMilliseconds, URIRef } from './types';
+import { K8sDomain } from './k8s';
+import { Class, Constraint, Domain, joinPath, Query, unixMilliseconds, URIRef } from './types';
 
 enum LogClass {
   application = 'application',
   infrastructure = 'infrastructure',
   audit = 'audit',
 }
 
+// TODO: Aggregated log links and k8s:pod style log queries ignore the containers parameter.
+
 export class LogDomain extends Domain {
+  private k8s: K8sDomain;
+  private pod: Class;
+
   constructor() {
     super('log');
+    this.k8s = new K8sDomain();
+    this.pod = new Class('k8s', 'Pod');
   }
 
   class(name: string): Class {
     if (!LogClass[name]) throw this.badClass(name);
     return new Class(this.name, name);
   }
 
-  // There are 2 types of URL: pod logs, and log search.
+  // There are 2 types of URL: pod logs, and logQL searches.
   linkToQuery(link: URIRef): Query {
     // First check for aggregated pod logs URL
     const [, namespace, name] =
@@ -42,11 +50,19 @@ export class LogDomain extends Domain {
   queryToLink(query: Query, constraint?: Constraint): URIRef {
     const logClass = LogClass[query.class.name as keyof typeof LogClass];
     if (!logClass) throw this.badQuery(query, 'unknown class');
-    return new URIRef('monitoring/logs', {
-      q: query.selector,
-      tenant: logClass,
-      start: unixMilliseconds(constraint?.start),
-      end: unixMilliseconds(constraint?.end),
-    });
+    try {
+      // First try to parse the selector as k8s pod selector
+      const link = this.k8s.queryToLink(this.pod.query(query.selector));
+      link.pathname = joinPath(link.pathname, 'aggregated-logs');
+      return link;
+    } catch {
+      // Otherwise assume it is a LogQL query.
+      return new URIRef('monitoring/logs', {
+        q: query.selector,
+        tenant: logClass,
+        start: unixMilliseconds(constraint?.start),
+        end: unixMilliseconds(constraint?.end),
+      });
+    }
   }
 }

web/src/korrel8r/log.ts

@@ -170,6 +170,12 @@ export class URIRef {
   }
 }
 
+// Join and normalize paths, no leading, trailing or repeated '/' charactersa
+export const joinPath = (path0: string, ...paths: string[]): string => {
+  const clean = paths.map((path: string) => path.replaceAll(/(^\/*)|(\/*$)/g, ''));
+  return [path0.replace(/\/*$/, ''), ...clean].join('/');
+};
+
 // Encode an object as a comma-separated, key=value list: 'key=value,key=value...'. No URI encoding.
 export const keyValueList = (obj: { [key: string]: string }): string => {
   return Object.keys(obj || {})