From 6fcc21b1736ab15860c68cb3c0f022ded78ca3ba Mon Sep 17 00:00:00 2001 From: Abhi Date: Mon, 11 Nov 2024 20:14:22 +0530 Subject: [PATCH] feat(tekton-results): update to use ownerReference id --- .../SecurityEnterpriseContractTab.tsx | 7 +- .../SecurityEnterpriseContractTab.spec.tsx | 28 ++---- ...eEnterpriseContractResultFromLogs.spec.tsx | 14 ++- .../useEnterpriseContractResultFromLogs.tsx | 12 +-- .../PipelineRunDetailsView.tsx | 5 +- .../PipelineRunSidePanel.tsx | 12 +-- .../PipelineRunVisualization.tsx | 5 +- .../__tests__/PipelineRunSidePanel.spec.tsx | 21 ++--- .../sidepanels/TaskRunPanel.tsx | 8 +- .../TaskRunDetailsView/TaskRunDetailsView.tsx | 15 +--- .../tabs/TaskRunLogsTab.tsx | 15 +--- .../tabs/__tests__/TaskRunLogsTab.spec.tsx | 1 - .../__data__/mock-TaskRun-data.ts | 31 +++++++ src/components/TaskRuns/TaskRunLogs.tsx | 9 +- .../TaskRuns/__tests__/TaskRunLogs.spec.tsx | 23 +---- src/hooks/__tests__/useTektonResults.spec.ts | 41 +++++---- src/hooks/useTektonResults.ts | 14 ++- .../pipeline-run-logs/PipelineRunLogs.tsx | 2 - .../__tests__/LogWrapperComponent.spec.tsx | 7 -- .../__tests__/TektonTaskRunLog.spec.tsx | 16 +--- .../logs/LogsWrapperComponent.tsx | 8 +- .../logs/TektonTaskRunLog.tsx | 8 +- .../pipeline-run-logs/logs/log-utils.spec.ts | 88 ++++++++++++++++++- .../pipeline-run-logs/logs/logs-utils.ts | 14 ++- src/utils/__tests__/tekton-results.spec.ts | 24 +---- src/utils/tekton-results.ts | 28 +----- 26 files changed, 219 insertions(+), 237 deletions(-) diff --git a/src/components/EnterpriseContractView/SecurityEnterpriseContractTab.tsx b/src/components/EnterpriseContractView/SecurityEnterpriseContractTab.tsx index 6650e9c6c..cf5740fbc 100644 --- a/src/components/EnterpriseContractView/SecurityEnterpriseContractTab.tsx +++ b/src/components/EnterpriseContractView/SecurityEnterpriseContractTab.tsx @@ -44,11 +44,10 @@ const getResultsSummary = (ECs, ecLoaded) => { export const SecurityEnterpriseContractTab: React.FC< React.PropsWithChildren<{ - pipelineRunName: string; - pipelineRunUID: string; + pipelineRun: string; }> -> = ({ pipelineRunName, pipelineRunUID }) => { - const [ecResult, ecResultLoaded] = useEnterpriseContractResults(pipelineRunName, pipelineRunUID); +> = ({ pipelineRun }) => { + const [ecResult, ecResultLoaded] = useEnterpriseContractResults(pipelineRun); const [nameFilter, setNameFilter] = useSearchParam('name', ''); diff --git a/src/components/EnterpriseContractView/__tests__/SecurityEnterpriseContractTab.spec.tsx b/src/components/EnterpriseContractView/__tests__/SecurityEnterpriseContractTab.spec.tsx index 9c1db39fc..bed21d516 100644 --- a/src/components/EnterpriseContractView/__tests__/SecurityEnterpriseContractTab.spec.tsx +++ b/src/components/EnterpriseContractView/__tests__/SecurityEnterpriseContractTab.spec.tsx @@ -22,23 +22,17 @@ describe('SecurityEnterpriseContractTab', () => { it('should render empty state for security tab when pods are missing', () => { mockUseEnterpriseContractResults.mockReturnValue([undefined, true]); - routerRenderer( - , - ); + routerRenderer(); screen.getByTestId('security-tab-empty-state'); }); it('should render component security tab', () => { - routerRenderer( - , - ); + routerRenderer(); screen.getByText('Missing CVE scan results'); }); it('should filter out results based on the name input field', () => { - routerRenderer( - , - ); + routerRenderer(); screen.getByText('Missing CVE scan results'); fireEvent.input(screen.getByPlaceholderText('Filter by rule...'), { target: { value: 'No tasks' }, @@ -49,9 +43,7 @@ describe('SecurityEnterpriseContractTab', () => { }); it('should filter out based on the status dropdown', async () => { - routerRenderer( - , - ); + routerRenderer(); screen.getByText('Missing CVE scan results'); fireEvent.click(screen.getByRole('button', { name: 'Status filter menu' })); fireEvent.click(screen.getByLabelText('Success')); @@ -61,9 +53,7 @@ describe('SecurityEnterpriseContractTab', () => { }); it('should show empty state when no search result found', () => { - routerRenderer( - , - ); + routerRenderer(); screen.getByText('Missing CVE scan results'); fireEvent.click(screen.getByRole('button', { name: 'Status filter menu' })); fireEvent.click(screen.getByLabelText('Failed')); @@ -78,9 +68,7 @@ describe('SecurityEnterpriseContractTab', () => { }); it('should sort by Status', () => { - routerRenderer( - , - ); + routerRenderer(); const status = screen.getAllByTestId('rule-status'); expect(status[0].textContent.trim()).toEqual('Failed'); fireEvent.click(screen.getAllByText('Status')[1]); @@ -89,9 +77,7 @@ describe('SecurityEnterpriseContractTab', () => { }); it('should render result summary', () => { - routerRenderer( - , - ); + routerRenderer(); const resultSummary = screen.getByTestId('result-summary'); const status = resultSummary.getElementsByTagName('span'); expect(status[0].textContent.trim()).toBe('Failed'); diff --git a/src/components/EnterpriseContractView/__tests__/useEnterpriseContractResultFromLogs.spec.tsx b/src/components/EnterpriseContractView/__tests__/useEnterpriseContractResultFromLogs.spec.tsx index c38309453..9283a38e9 100644 --- a/src/components/EnterpriseContractView/__tests__/useEnterpriseContractResultFromLogs.spec.tsx +++ b/src/components/EnterpriseContractView/__tests__/useEnterpriseContractResultFromLogs.spec.tsx @@ -55,7 +55,7 @@ describe('useEnterpriseContractResultFromLogs', () => { it('should parse valid rules to json', async () => { const { result, waitForNextUpdate } = renderHook(() => - useEnterpriseContractResultFromLogs('dummy-abcd', 'pipelinerun-test'), + useEnterpriseContractResultFromLogs('dummy-abcd'), ); await waitForNextUpdate(); expect(mockCommmonFetchJSON).toHaveBeenCalled(); @@ -69,9 +69,7 @@ describe('useEnterpriseContractResultFromLogs', () => { mockGetTaskRunLogs.mockReturnValue(`asdfcdfadsf [report-json] { "components": [] } `); - const { result } = renderHook(() => - useEnterpriseContractResultFromLogs('dummy-abcd', 'pipelinerun-test'), - ); + const { result } = renderHook(() => useEnterpriseContractResultFromLogs('dummy-abcd')); const [, loaded] = result.current; expect(mockCommmonFetchJSON).toHaveBeenCalled(); expect(loaded).toBe(true); @@ -82,7 +80,7 @@ describe('useEnterpriseContractResultFromLogs', () => { it('should filter out all 404 image url components from EC results', async () => { const { result, waitForNextUpdate } = renderHook(() => - useEnterpriseContractResultFromLogs('dummy-abcd', 'pipelinerun-test'), + useEnterpriseContractResultFromLogs('dummy-abcd'), ); await waitForNextUpdate(); const [ecResult, loaded] = result.current; @@ -95,7 +93,7 @@ describe('useEnterpriseContractResultFromLogs', () => { mockCommmonFetchJSON.mockRejectedValue(new Error('Api error')); const { result, waitForNextUpdate } = renderHook(() => - useEnterpriseContractResultFromLogs('dummy-abcd', 'pipelinerun-test'), + useEnterpriseContractResultFromLogs('dummy-abcd'), ); await waitForNextUpdate(); const [ecResult, loaded] = result.current; @@ -126,7 +124,7 @@ describe('useEnterpriseContractResultFromLogs', () => { `); const { result, waitForNextUpdate } = renderHook(() => - useEnterpriseContractResultFromLogs('dummy-abcd', 'pipelinerun-test'), + useEnterpriseContractResultFromLogs('dummy-abcd'), ); const [, loaded] = result.current; expect(mockCommmonFetchJSON).toHaveBeenCalled(); @@ -179,7 +177,7 @@ describe('useEnterpriseContractResults', () => { it('should return enterprise contract results', async () => { const { result, waitForNextUpdate } = renderHook(() => - useEnterpriseContractResults('dummy-abcd', 'pipelinerun-test'), + useEnterpriseContractResults('dummy-abcd'), ); expect(result.current[0]).toEqual(undefined); expect(result.current[1]).toEqual(false); diff --git a/src/components/EnterpriseContractView/useEnterpriseContractResultFromLogs.tsx b/src/components/EnterpriseContractView/useEnterpriseContractResultFromLogs.tsx index 66ad7a4ac..09569885b 100644 --- a/src/components/EnterpriseContractView/useEnterpriseContractResultFromLogs.tsx +++ b/src/components/EnterpriseContractView/useEnterpriseContractResultFromLogs.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import { commonFetchJSON, getK8sResourceURL } from '@openshift/dynamic-plugin-sdk-utils'; import { useTaskRuns } from '../../hooks/useTaskRuns'; +import { PipelineRunModel } from '../../models'; import { PodModel } from '../../models/pod'; import { getTaskRunLog } from '../../utils/tekton-results'; import { useWorkspaceInfo } from '../../utils/workspace-context-utils'; @@ -14,7 +15,6 @@ import { extractEcResultsFromTaskRunLogs } from './utils'; export const useEnterpriseContractResultFromLogs = ( pipelineRunName: string, - pipelineRunUID: string, ): [ComponentEnterpriseContractResult[], boolean] => { const { namespace, workspace } = useWorkspaceInfo(); const [taskRun, loaded, error] = useTaskRuns(namespace, pipelineRunName, 'verify'); @@ -67,13 +67,16 @@ export const useEnterpriseContractResultFromLogs = ( React.useEffect(() => { let unmount = false; if (fetchTknLogs) { + const pipelineRunUID = taskRun[0]?.metadata?.ownerReferences?.find( + (reference) => reference.kind === PipelineRunModel.kind, + )?.uid; const fetch = async () => { try { const logs = await getTaskRunLog( workspace, taskRun[0].metadata.namespace, pipelineRunUID, - taskRun[0], + taskRun[0].metadata?.uid, ); if (unmount) return; const json = extractEcResultsFromTaskRunLogs(logs); @@ -95,7 +98,7 @@ export const useEnterpriseContractResultFromLogs = ( return () => { unmount = true; }; - }, [fetchTknLogs, taskRun, workspace, pipelineRunUID]); + }, [fetchTknLogs, taskRun, workspace]); const ecResult = React.useMemo(() => { // filter out components for which ec didn't execute because invalid image URL @@ -160,9 +163,8 @@ export const mapEnterpriseContractResultData = ( export const useEnterpriseContractResults = ( pipelineRunName: string, - pipelineRunUID: string, ): [UIEnterpriseContractData[], boolean] => { - const [ec, ecLoaded] = useEnterpriseContractResultFromLogs(pipelineRunName, pipelineRunUID); + const [ec, ecLoaded] = useEnterpriseContractResultFromLogs(pipelineRunName); const ecResult = React.useMemo(() => { return ecLoaded && ec ? mapEnterpriseContractResultData(ec) : undefined; }, [ec, ecLoaded]); diff --git a/src/components/PipelineRunDetailsView/PipelineRunDetailsView.tsx b/src/components/PipelineRunDetailsView/PipelineRunDetailsView.tsx index 24115d143..1e0448526 100644 --- a/src/components/PipelineRunDetailsView/PipelineRunDetailsView.tsx +++ b/src/components/PipelineRunDetailsView/PipelineRunDetailsView.tsx @@ -141,10 +141,7 @@ export const PipelineRunDetailsView: React.FC< key: 'security', label: 'Security', component: ( - + ), }, ] diff --git a/src/components/PipelineRunDetailsView/PipelineRunSidePanel.tsx b/src/components/PipelineRunDetailsView/PipelineRunSidePanel.tsx index cb829f1e7..fec354b85 100644 --- a/src/components/PipelineRunDetailsView/PipelineRunSidePanel.tsx +++ b/src/components/PipelineRunDetailsView/PipelineRunSidePanel.tsx @@ -11,13 +11,9 @@ import { isTaskNode } from './visualization/utils/pipelinerun-graph-utils'; type Props = { scrollIntoView?: (node: Node) => void; - pipelineRunUID: string; }; -const PipelineRunSidePanel: React.FC> = ({ - scrollIntoView, - pipelineRunUID, -}) => { +const PipelineRunSidePanel: React.FC> = ({ scrollIntoView }) => { const [[selectedId], setSelectedIds] = useVisualizationState(SELECTION_STATE, []); const controller = useVisualizationController(); @@ -32,11 +28,7 @@ const PipelineRunSidePanel: React.FC> = ({ }, [controller, selectedId]); const panel = taskNode ? ( - setSelectedIds([])} - taskNode={taskNode} - pipelineRunUID={pipelineRunUID} - /> + setSelectedIds([])} taskNode={taskNode} /> ) : null; const isExpanded = !!panel; diff --git a/src/components/PipelineRunDetailsView/PipelineRunVisualization.tsx b/src/components/PipelineRunDetailsView/PipelineRunVisualization.tsx index c08724dff..529776e4d 100644 --- a/src/components/PipelineRunDetailsView/PipelineRunVisualization.tsx +++ b/src/components/PipelineRunDetailsView/PipelineRunVisualization.tsx @@ -40,10 +40,7 @@ const PipelineRunVisualization = ({ pipelineRun, error, taskRuns }) => { layoutFactory={layoutFactory} model={model} > - + ); diff --git a/src/components/PipelineRunDetailsView/__tests__/PipelineRunSidePanel.spec.tsx b/src/components/PipelineRunDetailsView/__tests__/PipelineRunSidePanel.spec.tsx index 315aa50d8..59f72ca42 100644 --- a/src/components/PipelineRunDetailsView/__tests__/PipelineRunSidePanel.spec.tsx +++ b/src/components/PipelineRunDetailsView/__tests__/PipelineRunSidePanel.spec.tsx @@ -18,7 +18,7 @@ describe('PipelineRunSidePanel', () => { it('should render nothing by default', () => { const setPropsFn = jest.fn(); - render(, { + render(, { wrapper: ({ children }) => ( {} }}> {children} @@ -42,7 +42,7 @@ describe('PipelineRunSidePanel', () => { const setPropsFn = jest.fn(); - render(, { + render(, { wrapper: ({ children }) => ( {} }}> {children} @@ -68,16 +68,13 @@ describe('PipelineRunSidePanel', () => { const setPropsFn = jest.fn(); - render( - , - { - wrapper: ({ children }) => ( - {} }}> - {children} - - ), - }, - ); + render(, { + wrapper: ({ children }) => ( + {} }}> + {children} + + ), + }); expect(setPropsFn).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/src/components/PipelineRunDetailsView/sidepanels/TaskRunPanel.tsx b/src/components/PipelineRunDetailsView/sidepanels/TaskRunPanel.tsx index 21d47ad60..4e5a070d1 100644 --- a/src/components/PipelineRunDetailsView/sidepanels/TaskRunPanel.tsx +++ b/src/components/PipelineRunDetailsView/sidepanels/TaskRunPanel.tsx @@ -20,14 +20,9 @@ import './TaskRunPanel.scss'; type Props = { onClose: () => void; taskNode: GraphElement; - pipelineRunUID: string; }; -const TaskRunPanel: React.FC> = ({ - taskNode, - onClose, - pipelineRunUID, -}) => { +const TaskRunPanel: React.FC> = ({ taskNode, onClose }) => { const task = taskNode.getData().task; const taskRun = taskNode.getData().taskRun; const { status } = taskNode.getData(); @@ -69,7 +64,6 @@ const TaskRunPanel: React.FC> = ({ taskRun={taskRun} namespace={taskNode.getData().namespace} status={status} - pipelineRunUID={pipelineRunUID} /> diff --git a/src/components/TaskRunDetailsView/TaskRunDetailsView.tsx b/src/components/TaskRunDetailsView/TaskRunDetailsView.tsx index 7ed32c6b4..d5e08d196 100644 --- a/src/components/TaskRunDetailsView/TaskRunDetailsView.tsx +++ b/src/components/TaskRunDetailsView/TaskRunDetailsView.tsx @@ -117,7 +117,7 @@ export const TaskRunDetailsView: React.FC ) : ( - + ), }, ...(isEnterpriseContract @@ -125,18 +125,7 @@ export const TaskRunDetailsView: React.FC - ) : ( - - ), + component: , }, ] : []), diff --git a/src/components/TaskRunDetailsView/tabs/TaskRunLogsTab.tsx b/src/components/TaskRunDetailsView/tabs/TaskRunLogsTab.tsx index 3a91882ab..af79616fb 100644 --- a/src/components/TaskRunDetailsView/tabs/TaskRunLogsTab.tsx +++ b/src/components/TaskRunDetailsView/tabs/TaskRunLogsTab.tsx @@ -5,24 +5,13 @@ import TaskRunLogs from '../../TaskRuns/TaskRunLogs'; export type TaskRunLogProps = { taskRun: TaskRunKind; - pipelineRunUID: string; }; -const TaskRunLogsTab: React.FC> = ({ - taskRun, - pipelineRunUID, -}) => { +const TaskRunLogsTab: React.FC> = ({ taskRun }) => { const status = taskRunStatus(taskRun); const namespace = taskRun.metadata?.namespace; - return ( - - ); + return ; }; export default TaskRunLogsTab; diff --git a/src/components/TaskRunDetailsView/tabs/__tests__/TaskRunLogsTab.spec.tsx b/src/components/TaskRunDetailsView/tabs/__tests__/TaskRunLogsTab.spec.tsx index 7c7583b17..380aabd4a 100644 --- a/src/components/TaskRunDetailsView/tabs/__tests__/TaskRunLogsTab.spec.tsx +++ b/src/components/TaskRunDetailsView/tabs/__tests__/TaskRunLogsTab.spec.tsx @@ -45,7 +45,6 @@ describe('TaskRunLogs', () => { ], }, }} - pipelineRunUID="test-id" />, { wrapper: BrowserRouter, diff --git a/src/components/TaskRunListView/__data__/mock-TaskRun-data.ts b/src/components/TaskRunListView/__data__/mock-TaskRun-data.ts index aca9790e2..df41db4e9 100644 --- a/src/components/TaskRunListView/__data__/mock-TaskRun-data.ts +++ b/src/components/TaskRunListView/__data__/mock-TaskRun-data.ts @@ -240,3 +240,34 @@ export const testTaskRuns: TaskRunKind[] = [ }, }, ]; + +export const mockTaskRunsWithOwnerRef: TaskRunKind[] = [ + { + metadata: { + name: 'task1', + uid: 'task-1', + namespace: 'ns1', + ownerReferences: [ + { name: 'plr-test', kind: 'PipelineRun', apiVersion: 'v1alpha1', uid: 'plr-uid' }, + ], + }, + apiVersion: 'v1alpha1', + kind: 'TaskRun', + spec: {}, + status: { podName: 'pod-test' }, + }, + { + metadata: { + name: 'task2', + uid: 'task-2', + namespace: 'ns1', + ownerReferences: [ + { name: 'plr-test', kind: 'PipelineRun', apiVersion: 'v1alpha1', uid: 'plr-uid' }, + ], + }, + apiVersion: 'v1alpha1', + kind: 'TaskRun', + spec: {}, + status: { podName: 'pod-test' }, + }, +]; diff --git a/src/components/TaskRuns/TaskRunLogs.tsx b/src/components/TaskRuns/TaskRunLogs.tsx index d3a21c9a2..cc78e8349 100644 --- a/src/components/TaskRuns/TaskRunLogs.tsx +++ b/src/components/TaskRuns/TaskRunLogs.tsx @@ -8,15 +8,9 @@ type Props = { taskRun: TaskRunKind; namespace: string; status: runStatus; - pipelineRunUID: string; }; -const TaskRunLogs: React.FC> = ({ - taskRun, - namespace, - status, - pipelineRunUID, -}) => { +const TaskRunLogs: React.FC> = ({ taskRun, namespace, status }) => { const podName = taskRun?.status?.podName; if (!podName) { @@ -32,7 +26,6 @@ const TaskRunLogs: React.FC> = ({ return ( { it('should render no logs found', () => { const result = render( - , + , ); expect(result.queryByText('No logs found.')).toBeInTheDocument(); }); it('should render waiting to start', () => { - const result = render( - , - ); + const result = render(); expect(result.queryByText('Waiting on task to start.')).toBeInTheDocument(); }); it('should render no logs due to Skipped status', () => { const result = render( - , + , ); expect(result.queryByText('No logs. This task was skipped.')).toBeInTheDocument(); }); diff --git a/src/hooks/__tests__/useTektonResults.spec.ts b/src/hooks/__tests__/useTektonResults.spec.ts index f1c4726e9..bd43e2f7f 100644 --- a/src/hooks/__tests__/useTektonResults.spec.ts +++ b/src/hooks/__tests__/useTektonResults.spec.ts @@ -7,7 +7,6 @@ import { getPipelineRuns, getTaskRuns, RecordsList, - getTaskRunLogOld, } from '../../utils/tekton-results'; import { useTRPipelineRuns, useTRTaskRunLog, useTRTaskRuns } from '../useTektonResults'; @@ -26,6 +25,7 @@ jest.mock('../../utils/tekton-results', () => { getTaskRuns: jest.fn(), }; }); + jest.mock('@openshift/dynamic-plugin-sdk-utils'); const mockResponse = [ @@ -49,7 +49,6 @@ const mockResponseLogs = 'test-log'; const getPipelineRunsMock = getPipelineRuns as jest.Mock; const getTaskRunsMock = getTaskRuns as jest.Mock; -const getTaskRunLogOldMock = getTaskRunLogOld as jest.Mock; const commonFetchTextMock = commonFetchText as unknown as jest.Mock; describe('useTektonResults', () => { @@ -205,21 +204,11 @@ describe('useTektonResults', () => { describe('useTRTaskRunLog', () => { it('should not attempt to get task run log', () => { - renderHook(() => useTRTaskRunLog(null, null, null)); - expect(commonFetchTextMock).not.toHaveBeenCalled(); - - renderHook(() => - useTRTaskRunLog('test-ns', null, { - apiVersion: 'v1Alpha1', - spec: {}, - kind: 'TaskRun', - metadata: { uid: 'test-id' }, - }), - ); + renderHook(() => useTRTaskRunLog(null, null)); expect(commonFetchTextMock).not.toHaveBeenCalled(); renderHook(() => - useTRTaskRunLog(null, 'pipelinerun-uid', { + useTRTaskRunLog(null, { apiVersion: 'v1Alpha1', spec: {}, kind: 'TaskRun', @@ -228,13 +217,23 @@ describe('useTektonResults', () => { ); expect(commonFetchTextMock).not.toHaveBeenCalled(); - renderHook(() => useTRTaskRunLog('test-ns', 'pipelinerun-uid', null)); + renderHook(() => useTRTaskRunLog('test-ns', null)); expect(commonFetchTextMock).not.toHaveBeenCalled(); }); it('should return task run log', async () => { commonFetchTextMock.mockImplementation(() => mockResponseLogs); - renderHook(() => useTRTaskRunLog('test-ns', 'pipelinerun-uid', mockTaskRun)); + renderHook(() => + useTRTaskRunLog('test-ns', { + ...mockTaskRun, + metadata: { + ...mockTaskRun.metadata, + ownerReferences: [ + { apiVersion: 'v1beta1', name: 'plr1', kind: `PipelineRun`, uid: 'pipelinerun-uid' }, + ], + }, + }), + ); expect(commonFetchTextMock).toHaveBeenCalled(); expect(commonFetchTextMock).toHaveBeenCalledWith( '/plugins/tekton-results/workspaces/test-ws/apis/results.tekton.dev/v1alpha2/parents/test-ns/results/pipelinerun-uid/logs/test-id', @@ -243,13 +242,17 @@ describe('useTektonResults', () => { it('should return error when exception thrown by both FetchLogs', async () => { commonFetchTextMock.mockRejectedValue({}); - getTaskRunLogOldMock.mockRejectedValue({}); const { result } = renderHook(() => - useTRTaskRunLog('test-ns', 'pipelinerun-uid', { + useTRTaskRunLog('test-ns', { apiVersion: 'v1Alpha1', spec: {}, kind: 'TaskRun', - metadata: { uid: 'test-id' }, + metadata: { + uid: 'test-id', + ownerReferences: [ + { apiVersion: 'v1beta1', name: 'plr1', kind: 'PipelineRun', uid: 'pipelinerun-uid' }, + ], + }, }), ); expect(commonFetchTextMock).toHaveBeenCalledWith( diff --git a/src/hooks/useTektonResults.ts b/src/hooks/useTektonResults.ts index 340895f30..c239fd43b 100644 --- a/src/hooks/useTektonResults.ts +++ b/src/hooks/useTektonResults.ts @@ -1,5 +1,6 @@ import React from 'react'; import { K8sResourceCommon } from '@openshift/dynamic-plugin-sdk-utils'; +import { PipelineRunModel } from '../models'; import { PipelineRunKind, TaskRunKind } from '../types'; import { getPipelineRuns, @@ -118,17 +119,26 @@ export const useTRTaskRuns = ( export const useTRTaskRunLog = ( namespace: string, - pipelineRunUID: string, taskRun: TaskRunKind, ): [string, boolean, unknown] => { const { workspace } = useWorkspaceInfo(); const [result, setResult] = React.useState<[string, boolean, unknown]>([null, false, undefined]); + + const pipelineRunUID = taskRun?.metadata?.ownerReferences?.find( + (reference) => reference.kind === PipelineRunModel.kind, + )?.uid; + React.useEffect(() => { let disposed = false; if (namespace && taskRun && pipelineRunUID) { (async () => { try { - const log = await getTaskRunLog(workspace, namespace, pipelineRunUID, taskRun); + const log = await getTaskRunLog( + workspace, + namespace, + pipelineRunUID, + taskRun?.metadata?.uid, + ); if (!disposed) { setResult([log, true, undefined]); } diff --git a/src/shared/components/pipeline-run-logs/PipelineRunLogs.tsx b/src/shared/components/pipeline-run-logs/PipelineRunLogs.tsx index bf901b763..ceedaeb17 100644 --- a/src/shared/components/pipeline-run-logs/PipelineRunLogs.tsx +++ b/src/shared/components/pipeline-run-logs/PipelineRunLogs.tsx @@ -123,7 +123,6 @@ class PipelineRunLogs extends React.Component taskRun.metadata.name === activeItem); @@ -179,7 +178,6 @@ class PipelineRunLogs extends React.Component {activeItem && resource ? ( { namespace: 'test-ns', isList: false, }} - pipelineRunUID="test-id" taskRun={taskRun} downloadAllLabel="Download all task logs" onDownloadAll={downloadAllCallback} @@ -197,7 +196,6 @@ describe('LogWrapperComponent', () => { namespace: 'test-ns', isList: false, }} - pipelineRunUID="test-id" taskRun={taskRun} downloadAllLabel="Download all task logs" onDownloadAll={downloadAll} @@ -223,7 +221,6 @@ describe('LogWrapperComponent', () => { namespace: 'test-ns', isList: false, }} - pipelineRunUID="test-id" taskRun={taskRun} downloadAllLabel={'Download all task logs'} onDownloadAll={downloadAllCallback} @@ -247,7 +244,6 @@ describe('LogWrapperComponent', () => { namespace: 'test-ns', isList: false, }} - pipelineRunUID="test-id" taskRun={taskRun} downloadAllLabel={'Download all task logs'} onDownloadAll={downloadAllCallback} @@ -274,7 +270,6 @@ describe('LogWrapperComponent', () => { namespace: 'test-ns', isList: false, }} - pipelineRunUID="test-id" taskRun={taskRun} downloadAllLabel="Download all task logs" onDownloadAll={downloadAll} @@ -306,7 +301,6 @@ describe('LogWrapperComponent', () => { namespace: 'test-ns', isList: false, }} - pipelineRunUID="test-id" taskRun={taskRun} downloadAllLabel="Download all task logs" onDownloadAll={downloadAll} @@ -330,7 +324,6 @@ describe('LogWrapperComponent', () => { namespace: 'test-ns', isList: false, }} - pipelineRunUID="test-id" taskRun={taskRun} downloadAllLabel="Download all task logs" onDownloadAll={downloadAll} diff --git a/src/shared/components/pipeline-run-logs/__tests__/TektonTaskRunLog.spec.tsx b/src/shared/components/pipeline-run-logs/__tests__/TektonTaskRunLog.spec.tsx index 8ae593411..cc689bbdc 100644 --- a/src/shared/components/pipeline-run-logs/__tests__/TektonTaskRunLog.spec.tsx +++ b/src/shared/components/pipeline-run-logs/__tests__/TektonTaskRunLog.spec.tsx @@ -46,13 +46,7 @@ describe('TektonTaskRunLog', () => { useTRTaskRunLogMock.mockReturnValue(['', false, null]); configure({ testIdAttribute: 'data-test' }); - render( - {}} - pipelineRunUID="pipelinerun-test" - />, - ); + render( {}} />); await waitFor(() => { screen.getByTestId('loading-indicator'); @@ -61,13 +55,7 @@ describe('TektonTaskRunLog', () => { it('should display the tekton log results', async () => { useTRTaskRunLogMock.mockReturnValue(['tekton log results', true, null]); - render( - {}} - pipelineRunUID="pipelinerun-test" - />, - ); + render( {}} />); await waitFor(() => { screen.getByText('tekton log results'); diff --git a/src/shared/components/pipeline-run-logs/logs/LogsWrapperComponent.tsx b/src/shared/components/pipeline-run-logs/logs/LogsWrapperComponent.tsx index 0fba9bbfe..f39ae8f25 100644 --- a/src/shared/components/pipeline-run-logs/logs/LogsWrapperComponent.tsx +++ b/src/shared/components/pipeline-run-logs/logs/LogsWrapperComponent.tsx @@ -14,7 +14,6 @@ import { TektonTaskRunLog } from './TektonTaskRunLog'; type LogsWrapperComponentProps = { taskRun: TaskRunKind; - pipelineRunUID: string; downloadAllLabel?: string; onDownloadAll?: () => Promise; resource: WatchK8sResource; @@ -24,7 +23,6 @@ const LogsWrapperComponent: React.FC { @@ -128,11 +126,7 @@ const LogsWrapperComponent: React.FC ) : ( - + )} ) : ( diff --git a/src/shared/components/pipeline-run-logs/logs/TektonTaskRunLog.tsx b/src/shared/components/pipeline-run-logs/logs/TektonTaskRunLog.tsx index 27d025607..ddc77a357 100644 --- a/src/shared/components/pipeline-run-logs/logs/TektonTaskRunLog.tsx +++ b/src/shared/components/pipeline-run-logs/logs/TektonTaskRunLog.tsx @@ -11,21 +11,15 @@ import './MultiStreamLogs.scss'; type TektonTaskRunLogProps = { taskRun?: TaskRunKind; setCurrentLogsGetter: (getter: () => string) => void; - pipelineRunUID: string; }; export const TektonTaskRunLog: React.FC> = ({ taskRun, - pipelineRunUID, setCurrentLogsGetter, }) => { const scrollPane = React.useRef(); const taskName = taskRun?.spec.taskRef?.name ?? taskRun?.metadata.name; - const [trResults, trLoaded, trError] = useTRTaskRunLog( - taskRun.metadata.namespace, - pipelineRunUID, - taskRun, - ); + const [trResults, trLoaded, trError] = useTRTaskRunLog(taskRun.metadata.namespace, taskRun); React.useEffect(() => { setCurrentLogsGetter(() => scrollPane.current?.innerText); diff --git a/src/shared/components/pipeline-run-logs/logs/log-utils.spec.ts b/src/shared/components/pipeline-run-logs/logs/log-utils.spec.ts index 42b62c2b4..636f2eeae 100644 --- a/src/shared/components/pipeline-run-logs/logs/log-utils.spec.ts +++ b/src/shared/components/pipeline-run-logs/logs/log-utils.spec.ts @@ -1,5 +1,36 @@ +import { commonFetchText, getK8sResourceURL } from '@openshift/dynamic-plugin-sdk-utils'; +import { waitFor } from '@testing-library/react'; +import { saveAs } from 'file-saver'; import { samplePod } from '../../../../__data__/pod-data'; -import { getRenderContainers } from './logs-utils'; +import { mockTaskRunsWithOwnerRef } from '../../../../components/TaskRunListView/__data__/mock-TaskRun-data'; +import { getTaskRunLog } from '../../../../utils/tekton-results'; +import { getRenderContainers, getDownloadAllLogsCallback } from './logs-utils'; + +jest.mock('@openshift/dynamic-plugin-sdk-utils', () => { + const actual = jest.requireActual('@openshift/dynamic-plugin-sdk-utils'); + return { + ...actual, + commonFetchText: jest.fn(), + getK8sResourceURL: jest.fn(), + }; +}); + +jest.mock('file-saver', () => ({ + saveAs: jest.fn(), +})); + +jest.mock('../../../../utils/tekton-results', () => { + const actual = jest.requireActual('../../../../utils/tekton-results'); + return { + ...actual, + getTaskRunLog: jest.fn(), + }; +}); + +const commonFetchTextMock = commonFetchText as jest.Mock; +const getK8sResourceURLMock = getK8sResourceURL as jest.Mock; +const getTaskRunLogMock = getTaskRunLog as jest.Mock; +const fileSaverMock = saveAs as jest.Mock; describe('getRenderContainers', () => { it('should render default values if invalid value is passed', () => { @@ -16,3 +47,58 @@ describe('getRenderContainers', () => { }); }); }); + +describe('getDownloadAllLogsCallback', () => { + beforeEach(() => { + commonFetchTextMock.mockReturnValue('test1'); + getK8sResourceURLMock.mockReturnValue('test1'); + getTaskRunLogMock.mockReturnValue(Promise.resolve('test1')); + fileSaverMock.mockReturnValue('file'); + }); + it('should call getTaskRunLogs once', async () => { + const callBack = getDownloadAllLogsCallback( + ['task1'], + [mockTaskRunsWithOwnerRef[0]], + 'ws1', + 'ns1', + 'plr-test', + ); + callBack(); + await waitFor(() => { + expect(getTaskRunLogMock).toHaveBeenCalled(); + expect(getTaskRunLogMock).toHaveBeenCalledWith('ws1', 'ns1', 'plr-uid', 'task-1'); + }); + }); + + it('should call getTaskRunLogs twice for two tasks', async () => { + getTaskRunLogMock.mockClear().mockReturnValue(Promise.resolve('test1')); + const callBack = getDownloadAllLogsCallback( + ['task1', 'task2'], + mockTaskRunsWithOwnerRef, + 'ws1', + 'ns1', + 'plr-test', + ); + callBack(); + await waitFor(() => { + expect(getTaskRunLogMock).toHaveBeenCalledTimes(2); + expect(getTaskRunLogMock).toHaveBeenLastCalledWith('ws1', 'ns1', 'plr-uid', 'task-2'); + }); + }); + + it('should save log in correct filename', async () => { + fileSaverMock.mockClear().mockImplementation((a, b) => b); + const callBack = getDownloadAllLogsCallback( + ['task1', 'task2'], + mockTaskRunsWithOwnerRef, + 'ws1', + 'ns1', + 'plr-test', + ); + callBack(); + await waitFor(() => { + expect(fileSaverMock).toHaveBeenCalled(); + expect(fileSaverMock).toHaveLastReturnedWith('plr-test.log'); + }); + }); +}); diff --git a/src/shared/components/pipeline-run-logs/logs/logs-utils.ts b/src/shared/components/pipeline-run-logs/logs/logs-utils.ts index a41006a3b..7d333e7cf 100644 --- a/src/shared/components/pipeline-run-logs/logs/logs-utils.ts +++ b/src/shared/components/pipeline-run-logs/logs/logs-utils.ts @@ -4,6 +4,7 @@ import { k8sGetResource, } from '@openshift/dynamic-plugin-sdk-utils'; import { saveAs } from 'file-saver'; +import { PipelineRunModel } from '../../../../models'; import { PodModel } from '../../../../models/pod'; import { TaskRunKind } from '../../../../types'; import { getTaskRunLog } from '../../../../utils/tekton-results'; @@ -77,7 +78,6 @@ export const getDownloadAllLogsCallback = ( workspace: string, namespace: string, pipelineRunName: string, - pipelineRunUID: string, ): (() => Promise) => { const getWatchUrls = async (): Promise => { const stepsList: ContainerStatus[][] = await Promise.all( @@ -150,9 +150,15 @@ export const getDownloadAllLogsCallback = ( } } else { const taskRun = taskRuns.find((t) => t.metadata.name === currTask); - allLogs += await getTaskRunLog(workspace, namespace, pipelineRunUID, taskRun).then( - (log) => `${tasks[currTask].name.toUpperCase()}\n\n${log}\n\n`, - ); + const pipelineRunUID = taskRun?.metadata?.ownerReferences?.find( + (reference) => reference.kind === PipelineRunModel.kind, + )?.uid; + allLogs += await getTaskRunLog( + workspace, + namespace, + pipelineRunUID, + taskRun?.metadata?.uid, + ).then((log) => `${tasks[currTask].name.toUpperCase()}\n\n${log}\n\n`); } } const buffer = new LineBuffer(); diff --git a/src/utils/__tests__/tekton-results.spec.ts b/src/utils/__tests__/tekton-results.spec.ts index 1164331a2..2433f544e 100644 --- a/src/utils/__tests__/tekton-results.spec.ts +++ b/src/utils/__tests__/tekton-results.spec.ts @@ -528,16 +528,9 @@ describe('tekton-results', () => { it('should call commonFetchText with pipeline uid & taskrun uid', async () => { commonFetchJSONMock.mockReturnValueOnce(mockLogsRecordsList); commonFetchTextMock.mockReturnValueOnce(Promise.resolve(mockLogResponse)); - expect( - await getTaskRunLog('test-ws', 'test-ns', 'pipelinerun-uid', { - kind: 'TaskRun', - apiVersion: '', - metadata: { - uid: 'test-id', - }, - spec: {}, - }), - ).toEqual('sample log'); + expect(await getTaskRunLog('test-ws', 'test-ns', 'pipelinerun-uid', 'test-id')).toEqual( + 'sample log', + ); expect(commonFetchTextMock.mock.calls).toEqual([ [ @@ -548,16 +541,7 @@ describe('tekton-results', () => { it('should throw error 404 if record not found', async () => { commonFetchTextMock.mockClear().mockRejectedValue(mockEmptyRecordsList); - await expect( - getTaskRunLog('test-ws', 'test-ns', 'sample-task-run', { - kind: 'TaskRun', - apiVersion: '', - metadata: { - uid: 'test', - }, - spec: {}, - }), - ).rejects.toEqual({ + await expect(getTaskRunLog('test-ws', 'test-ns', 'sample-task-run', 'test')).rejects.toEqual({ code: 404, }); }); diff --git a/src/utils/tekton-results.ts b/src/utils/tekton-results.ts index ba93948c6..a711f2140 100644 --- a/src/utils/tekton-results.ts +++ b/src/utils/tekton-results.ts @@ -7,7 +7,7 @@ import { Selector, } from '@openshift/dynamic-plugin-sdk-utils'; import { PipelineRunLabel } from '../consts/pipelinerun'; -import { PipelineRunKindV1Beta1, TaskRunKind, TaskRunKindV1Beta1 } from '../types'; +import { PipelineRunKindV1Beta1, TaskRunKindV1Beta1 } from '../types'; // REST API spec // https://github.com/tektoncd/results/blob/main/docs/api/rest-api-spec.md @@ -337,32 +337,12 @@ export const getTaskRuns = ( cacheKey?: string, ) => getFilteredTaskRuns(workspace, namespace, '', options, nextPageToken, cacheKey); -const getLog = (workspace: string, taskRunPath: string) => - commonFetchText(`${getTRUrlPrefix(workspace)}/${taskRunPath.replace('/records/', '/logs/')}`); - -export const getTaskRunLogOld = ( - workspace: string, - namespace: string, - taskRunName: string, -): Promise => - getFilteredRecord( - workspace, - namespace, - [DataType.Log, DataType.Log_OLD], - AND(EQ(`data.spec.resource.kind`, 'TaskRun'), EQ(`data.spec.resource.name`, taskRunName)), - { limit: 1 }, - ).then((x) => - x?.[1]?.records.length > 0 - ? getLog(workspace, x?.[1]?.records[0].name).catch(() => throw404()) - : throw404(), - ); - export const getTaskRunLog = ( workspace: string, namespace: string, pid: string, - taskRun: TaskRunKind, + taskRunID: string, ) => commonFetchText( - `${getTRUrlPrefix(workspace)}/${namespace}/results/${pid}/logs/${taskRun?.metadata?.uid}`, - ).catch(() => getTaskRunLogOld(workspace, namespace, taskRun.metadata.name)); + `${getTRUrlPrefix(workspace)}/${namespace}/results/${pid}/logs/${taskRunID}`, + ).catch(() => throw404());