From 7415588a80d27a79d27ac88c16f82b23fe22cb9f Mon Sep 17 00:00:00 2001 From: Rohit Rai Date: Thu, 11 Apr 2024 21:26:36 +0530 Subject: [PATCH] fix(pipeline): fix pipeline rerun actions to run pac build --- .../ApplicationDetails/component-actions.tsx | 12 +-- .../PipelineRunDetailsView.tsx | 28 +----- .../__tests__/PipelineRunDetailsView.spec.tsx | 23 ----- .../pipelinerun-actions.tsx | 10 +-- src/utils/__tests__/component-utils.spec.ts | 73 ++++++++++++++++ src/utils/component-utils.ts | 86 ++++++++----------- 6 files changed, 118 insertions(+), 114 deletions(-) diff --git a/src/components/ApplicationDetails/component-actions.tsx b/src/components/ApplicationDetails/component-actions.tsx index befd64386..eb216dda7 100644 --- a/src/components/ApplicationDetails/component-actions.tsx +++ b/src/components/ApplicationDetails/component-actions.tsx @@ -2,7 +2,7 @@ import * as React from 'react'; import { ComponentModel } from '../../models'; import { Action } from '../../shared/components/action-menu/types'; import { ComponentKind } from '../../types'; -import { isPACEnabled, startNewBuild } from '../../utils/component-utils'; +import { startNewBuild } from '../../utils/component-utils'; import { useAccessReviewForModel } from '../../utils/rbac'; import { useWorkspaceInfo } from '../../utils/workspace-context-utils'; import { createCustomizeComponentPipelineModalLauncher } from '../CustomizedPipeline/CustomizePipelinesModal'; @@ -40,9 +40,7 @@ export const useComponentActions = (component: ComponentKind, name: string): Act workspace, }, }, - ]; - if (!isPACEnabled(component)) { - updatedActions.push({ + { cta: () => startNewBuild(component), id: 'start-new-build', label: 'Start new build', @@ -55,9 +53,7 @@ export const useComponentActions = (component: ComponentKind, name: string): Act app_name: applicationName, workspace, }, - }); - } - updatedActions.push( + }, { cta: { href: `/application-pipeline/workspaces/${workspace}/applications/${applicationName}/components/${name}/deployment-settings`, @@ -81,7 +77,7 @@ export const useComponentActions = (component: ComponentKind, name: string): Act disabled: !canDeleteComponent, disabledTooltip: "You don't have access to delete a component", }, - ); + ]; return updatedActions; }, [ applicationName, diff --git a/src/components/PipelineRunDetailsView/PipelineRunDetailsView.tsx b/src/components/PipelineRunDetailsView/PipelineRunDetailsView.tsx index b8744ffd2..1e0448526 100644 --- a/src/components/PipelineRunDetailsView/PipelineRunDetailsView.tsx +++ b/src/components/PipelineRunDetailsView/PipelineRunDetailsView.tsx @@ -1,16 +1,13 @@ import * as React from 'react'; -import { useNavigate } from 'react-router-dom'; import { Bullseye, Spinner } from '@patternfly/react-core'; import { PipelineRunLabel } from '../../consts/pipelinerun'; -import { useComponent } from '../../hooks/useComponents'; import { usePipelineRun } from '../../hooks/usePipelineRuns'; import { useTaskRuns } from '../../hooks/useTaskRuns'; -import { ComponentModel, PipelineRunModel } from '../../models'; +import { PipelineRunModel } from '../../models'; import DetailsPage from '../../shared/components/details-page/DetailsPage'; import ErrorEmptyState from '../../shared/components/empty-state/ErrorEmptyState'; import { HttpError } from '../../shared/utils/error/http-error'; import { useApplicationBreadcrumbs } from '../../utils/breadcrumb-utils'; -import { isPACEnabled, startNewBuild } from '../../utils/component-utils'; import { pipelineRunCancel, pipelineRunStop } from '../../utils/pipeline-actions'; import { pipelineRunStatus } from '../../utils/pipeline-utils'; import { useAccessReviewForModel } from '../../utils/rbac'; @@ -31,7 +28,6 @@ type PipelineRunDetailsViewProps = { export const PipelineRunDetailsView: React.FC< React.PropsWithChildren > = ({ pipelineRunName }) => { - const navigate = useNavigate(); const { namespace, workspace } = useWorkspaceInfo(); const applicationBreadcrumbs = useApplicationBreadcrumbs(); @@ -39,14 +35,8 @@ export const PipelineRunDetailsView: React.FC< const { cta, isDisabled, disabledTooltip, key, label } = usePipelinererunAction(pipelineRun); const [taskRuns, taskRunsLoaded, taskRunError] = useTaskRuns(namespace, pipelineRunName); - const [canPatchComponent] = useAccessReviewForModel(ComponentModel, 'patch'); const [canPatchPipeline] = useAccessReviewForModel(PipelineRunModel, 'patch'); - const [component, componentLoaded, componentError] = useComponent( - namespace, - pipelineRun?.metadata?.labels?.[PipelineRunLabel.COMPONENT], - ); - const plrStatus = React.useMemo( () => loaded && pipelineRun && pipelineRunStatus(pipelineRun), [loaded, pipelineRun], @@ -64,7 +54,7 @@ export const PipelineRunDetailsView: React.FC< ); } - if (!(loaded && taskRunsLoaded && componentLoaded)) { + if (!(loaded && taskRunsLoaded)) { return ( @@ -98,20 +88,6 @@ export const PipelineRunDetailsView: React.FC< } actions={[ - { - key: 'start-new-build', - label: 'Start new build', - hidden: !component || !!componentError || !isPACEnabled(component), - isDisabled: !canPatchComponent, - disabledTooltip: "You don't have access to start a new build", - onClick: () => { - startNewBuild(component).then(() => - navigate( - `/application-pipeline/workspaces/${workspace}/applications/${component.spec.application}/activity/pipelineruns?name=${component.metadata.name}`, - ), - ); - }, - }, { key, label, diff --git a/src/components/PipelineRunDetailsView/__tests__/PipelineRunDetailsView.spec.tsx b/src/components/PipelineRunDetailsView/__tests__/PipelineRunDetailsView.spec.tsx index f7ccf82ff..97f818e25 100644 --- a/src/components/PipelineRunDetailsView/__tests__/PipelineRunDetailsView.spec.tsx +++ b/src/components/PipelineRunDetailsView/__tests__/PipelineRunDetailsView.spec.tsx @@ -326,29 +326,6 @@ describe('PipelineRunDetailsView', () => { expect(screen.queryByText('Rerun')).toHaveAttribute('aria-disabled', 'true'); }); - it('should render start new build action', async () => { - const navigateMock = jest.fn(); - useNavigateMock.mockImplementation(() => { - return navigateMock; - }); - watchResourceMock - .mockReturnValueOnce([testPipelineRuns[DataState.SUCCEEDED], true]) - .mockReturnValue([[], true]); - - routerRenderer(); - fireEvent.click(screen.queryByRole('button', { name: 'Actions' })); - - const startNewBuildButton = screen.queryByText('Start new build'); - expect(startNewBuildButton).toHaveAttribute('aria-disabled', 'false'); - - fireEvent.click(startNewBuildButton); - await waitFor(() => - expect(navigateMock).toHaveBeenCalledWith( - '/application-pipeline/workspaces/test-ws/applications/test-app/activity/pipelineruns?name=mock-component', - ), - ); - }); - it('should disable Stop and Cancel buttons if user does not have access to patch pipeline run', () => { watchResourceMock .mockReturnValueOnce([testPipelineRuns[DataState.RUNNING], true]) diff --git a/src/components/PipelineRunListView/pipelinerun-actions.tsx b/src/components/PipelineRunListView/pipelinerun-actions.tsx index 5eccc5e2d..199ef1ed4 100644 --- a/src/components/PipelineRunListView/pipelinerun-actions.tsx +++ b/src/components/PipelineRunListView/pipelinerun-actions.tsx @@ -6,7 +6,7 @@ import { useSnapshots } from '../../hooks/useSnapshots'; import { ComponentModel, PipelineRunModel, SnapshotModel } from '../../models'; import { Action } from '../../shared/components/action-menu/types'; import { PipelineRunKind } from '../../types'; -import { isPACEnabled, rerunBuildPipeline } from '../../utils/component-utils'; +import { startNewBuild } from '../../utils/component-utils'; import { pipelineRunCancel, pipelineRunStop } from '../../utils/pipeline-actions'; import { pipelineRunStatus, runStatus } from '../../utils/pipeline-utils'; import { useAccessReviewForModel } from '../../utils/rbac'; @@ -48,7 +48,7 @@ export const usePipelinererunAction = (pipelineRun: PipelineRunKind) => { runType === PipelineRunType.BUILD ? componentLoaded && !componentError && - rerunBuildPipeline(component).then(() => { + startNewBuild(component).then(() => { navigate( `/application-pipeline/workspaces/${workspace}/applications/${component.spec.application}/activity/pipelineruns?name=${component.metadata.name}`, ); @@ -62,14 +62,11 @@ export const usePipelinererunAction = (pipelineRun: PipelineRunKind) => { ); }), isDisabled: - (runType === PipelineRunType.BUILD && - (!canPatchComponent || - (componentLoaded && !componentError && component && !isPACEnabled(component)))) || + (runType === PipelineRunType.BUILD && !canPatchComponent) || (runType === PipelineRunType.TEST && (!canPatchSnapshot || !snapshot || !scenario)), disabledTooltip: (runType === PipelineRunType.BUILD && !canPatchComponent) || - (componentLoaded && !componentError && component && !isPACEnabled(component)) || (runType === PipelineRunType.TEST && !canPatchSnapshot) ? "You don't have access to rerun" : runType === PipelineRunType.TEST && (!snapshot || !scenario) @@ -103,7 +100,6 @@ export const usePipelinerunActions = (pipelineRun: PipelineRunKind): Action[] => ? "You don't have access to stop this pipeline" : undefined, }, - { cta: () => pipelineRunCancel(pipelineRun), id: 'pipelinerun-cancel', diff --git a/src/utils/__tests__/component-utils.spec.ts b/src/utils/__tests__/component-utils.spec.ts index 3badc1ddc..685827557 100644 --- a/src/utils/__tests__/component-utils.spec.ts +++ b/src/utils/__tests__/component-utils.spec.ts @@ -1,19 +1,30 @@ +import { k8sPatchResource } from '@openshift/dynamic-plugin-sdk-utils'; import { renderHook } from '@testing-library/react-hooks'; import { useApplicationPipelineGitHubApp } from '../../hooks/useApplicationPipelineGitHubApp'; +import { ComponentModel } from '../../models'; import { ComponentKind } from '../../types'; import { isPACEnabled, useURLForComponentPRs, useComponentBuildStatus, BUILD_STATUS_ANNOTATION, + startNewBuild, + BUILD_REQUEST_ANNOTATION, + BuildRequest, } from '../component-utils'; jest.mock('../../hooks/useApplicationPipelineGitHubApp', () => ({ useApplicationPipelineGitHubApp: jest.fn(), })); +jest.mock('@openshift/dynamic-plugin-sdk-utils', () => ({ + k8sPatchResource: jest.fn(), +})); + const useApplicationPipelineGitHubAppMock = useApplicationPipelineGitHubApp as jest.Mock; +const k8sPatchResourceMock = k8sPatchResource as jest.Mock; + describe('component-utils', () => { it('should detect pac enabled state', () => { const createComponent = (buildState: string | undefined): ComponentKind => { @@ -31,6 +42,68 @@ describe('component-utils', () => { expect(isPACEnabled(createComponent('disabled'))).toBe(false); }); + it('should start a new PAC build when PAC is enabled', () => { + const createComponent = (buildState: string | undefined): ComponentKind => { + const result = { + metadata: { + annotations: { + [BUILD_STATUS_ANNOTATION]: buildState && JSON.stringify({ pac: { state: buildState } }), + }, + }, + }; + return (result ?? {}) as ComponentKind; + }; + + const component = createComponent('enabled'); + startNewBuild(component); + + expect(k8sPatchResourceMock).toHaveBeenCalledWith({ + model: ComponentModel, + queryOptions: { + name: component.metadata.name, + ns: component.metadata.namespace, + }, + patches: [ + { + op: 'add', + path: `/metadata/annotations/${BUILD_REQUEST_ANNOTATION.replace('/', '~1')}`, + value: BuildRequest.triggerPACBuild, + }, + ], + }); + }); + + it('should start a new simple build when PAC is not enabled', () => { + const createComponent = (buildState: string | undefined): ComponentKind => { + const result = { + metadata: { + annotations: { + [BUILD_STATUS_ANNOTATION]: buildState && JSON.stringify({ pac: { state: buildState } }), + }, + }, + }; + return (result ?? {}) as ComponentKind; + }; + + const component = createComponent('disabled'); + startNewBuild(component); + + expect(k8sPatchResourceMock).toHaveBeenCalledWith({ + model: ComponentModel, + queryOptions: { + name: component.metadata.name, + ns: component.metadata.namespace, + }, + patches: [ + { + op: 'add', + path: `/metadata/annotations/${BUILD_REQUEST_ANNOTATION.replace('/', '~1')}`, + value: BuildRequest.triggerSimpleBuild, + }, + ], + }); + }); + it('should create github URL for component PRs', () => { useApplicationPipelineGitHubAppMock.mockReturnValue({ name: 'appstudio-staging-ci', diff --git a/src/utils/component-utils.ts b/src/utils/component-utils.ts index 016a53249..04d9f510b 100644 --- a/src/utils/component-utils.ts +++ b/src/utils/component-utils.ts @@ -37,11 +37,43 @@ export type ComponentBuildStatus = { message?: string; }; +/** + * If whole pac section is missing, PaC state is considered disabled + * Missing pac section shows that PaC was never requested on this component before, + * where as pac.state=disabled means that it was provisioned and then removed. + * + * https://github.com/redhat-appstudio/build-service/pull/164 + */ +export const getComponentBuildStatus = (component: ComponentKind) => { + const buildStatusJSON = component.metadata?.annotations?.[BUILD_STATUS_ANNOTATION]; + if (!buildStatusJSON) { + return null; + } + try { + const buildStatus = JSON.parse(buildStatusJSON) as ComponentBuildStatus; + return buildStatus; + } catch (e) { + // eslint-disable-next-line no-console + console.error('Error while parsing component build status: ', e); + return null; + } +}; + +export const getPACProvision = (component: ComponentKind) => + getComponentBuildStatus(component)?.pac?.state; + +export const isPACEnabled = (component: ComponentKind) => + getPACProvision(component) === ComponentBuildState.enabled; + export enum BuildRequest { /** * submits a new simple build pipeline. The build could be requested at any time regardless PaC Component configuration */ - triggerBuild = 'trigger-simple-build', + triggerSimpleBuild = 'trigger-simple-build', + /** + * submits a new pac build pipeline. The build could be requested at any time regardless PaC Component configuration + */ + triggerPACBuild = 'trigger-pac-build', /** * requests Pipelines-as-Code provision for the Component */ @@ -50,10 +82,6 @@ export enum BuildRequest { * requests Pipelines-as-Code clean up for the Component */ unconfigurePac = 'unconfigure-pac', - /** - * submits a rerun request. - */ - rerunBuild = 'trigger-simple-build', } export const enablePAC = (component: ComponentKind) => @@ -99,55 +127,13 @@ export const startNewBuild = (component: ComponentKind) => { op: 'add', path: `/metadata/annotations/${BUILD_REQUEST_ANNOTATION.replace('/', '~1')}`, - value: BuildRequest.triggerBuild, - }, - ], - }); - -export const rerunBuildPipeline = (component: ComponentKind) => - k8sPatchResource({ - model: ComponentModel, - queryOptions: { - name: component.metadata.name, - ns: component.metadata.namespace, - }, - patches: [ - { - op: 'add', - path: `/metadata/annotations/${BUILD_REQUEST_ANNOTATION.replace('/', '~1')}`, - value: BuildRequest.rerunBuild, + value: isPACEnabled(component) + ? BuildRequest.triggerPACBuild + : BuildRequest.triggerSimpleBuild, }, ], }); -/** - * If whole pac section is missing, PaC state is considered disabled - * Missing pac section shows that PaC was never requested on this component before, - * where as pac.state=disabled means that it was provisioned and then removed. - * - * https://github.com/redhat-appstudio/build-service/pull/164 - */ -export const getComponentBuildStatus = (component: ComponentKind) => { - const buildStatusJSON = component.metadata?.annotations?.[BUILD_STATUS_ANNOTATION]; - if (!buildStatusJSON) { - return null; - } - try { - const buildStatus = JSON.parse(buildStatusJSON) as ComponentBuildStatus; - return buildStatus; - } catch (e) { - // eslint-disable-next-line no-console - console.error('Error while parsing component build status: ', e); - return null; - } -}; - -export const getPACProvision = (component: ComponentKind) => - getComponentBuildStatus(component)?.pac?.state; - -export const isPACEnabled = (component: ComponentKind) => - getPACProvision(component) === ComponentBuildState.enabled; - const GIT_URL_PREFIX = 'https://github.com/'; export const useURLForComponentPRs = (components: ComponentKind[]): string => {