diff --git a/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/KubeNamespaceSelector.tsx b/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/KubeNamespaceSelector.tsx index dc4221942bc63..11ee294fdcdbd 100644 --- a/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/KubeNamespaceSelector.tsx +++ b/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/KubeNamespaceSelector.tsx @@ -24,8 +24,6 @@ import { ActionMeta } from 'react-select'; import { Option } from 'shared/components/Select'; import { FieldSelectAsync } from 'shared/components/FieldSelect'; -import { requiredField } from 'shared/components/Validation/rules'; - import { CheckableOptionComponent } from '../CheckableOption'; import { PendingListItem, PendingKubeResourceItem } from './RequestCheckout'; @@ -36,19 +34,15 @@ export function KubeNamespaceSelector({ kubeClusterItem, fetchKubeNamespaces, savedResourceItems, - toggleResource, bulkToggleKubeResources, - namespaceRequired, }: { kubeClusterItem: PendingListItem; fetchKubeNamespaces(p: KubeNamespaceRequest): Promise; savedResourceItems: PendingListItem[]; - toggleResource: (resource: PendingListItem) => void; bulkToggleKubeResources: ( resources: PendingKubeResourceItem[], resource: PendingListItem ) => void; - namespaceRequired: boolean; }) { // Flag is used to determine if we need to perform batch action // eg: When menu is open, we want to apply changes only after @@ -87,13 +81,18 @@ export function KubeNamespaceSelector({ ); return; case 'remove-value': - toggleResource({ - kind: 'namespace', - id: kubeClusterItem.id, - subResourceName: actionMeta.removedValue.value, - clusterName: kubeClusterItem.clusterName, - name: actionMeta.removedValue.value, - }); + bulkToggleKubeResources( + [ + { + kind: 'namespace', + id: kubeClusterItem.id, + subResourceName: actionMeta.removedValue.value, + clusterName: kubeClusterItem.clusterName, + name: actionMeta.removedValue.value, + }, + ], + kubeClusterItem + ); return; } } @@ -144,7 +143,7 @@ export function KubeNamespaceSelector({ return ( setInitOptions(opts)} defaultOptions={initOptions} /> diff --git a/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.story.tsx b/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.story.tsx index 693b08ace5409..91850290654e9 100644 --- a/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.story.tsx +++ b/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.story.tsx @@ -152,15 +152,27 @@ export const FailedResourceRequest = () => ( ); -export const FailedUnsupportedKubeResourceKind = () => ( +export const FailedUnsupportedKubeResourceKindWithTooltip = () => ( + +); + +export const FailedUnsupportedKubeResourceKindWithoutTooltip = () => ( + + diff --git a/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.tsx b/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.tsx index 34253a22854d6..99e4d239e0c5f 100644 --- a/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.tsx +++ b/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.tsx @@ -34,7 +34,6 @@ import { P3, Subtitle2, Text, - Mark, } from 'design'; import { ArrowBack, ChevronDown, ChevronRight, Warning } from 'design/Icon'; import Table, { Cell } from 'design/DataTable'; @@ -42,18 +41,19 @@ import { Danger } from 'design/Alert'; import Validation, { useRule, Validator } from 'shared/components/Validation'; import { Attempt } from 'shared/hooks/useAttemptNext'; -import { listToSentence, pluralize } from 'shared/utils/text'; +import { pluralize } from 'shared/utils/text'; import { Option } from 'shared/components/Select'; import { FieldCheckbox } from 'shared/components/FieldCheckbox'; import { mergeRefs } from 'shared/libs/mergeRefs'; import { TextSelectCopyMulti } from 'shared/components/TextSelectCopy'; import { RequestableResourceKind } from 'shared/components/AccessRequests/NewRequest/resource'; +import { HoverTooltip } from 'shared/components/ToolTip'; import { CreateRequest } from '../../Shared/types'; import { AssumeStartTime } from '../../AssumeStartTime/AssumeStartTime'; import { AccessDurationRequest } from '../../AccessDuration'; import { - checkForUnsupportedKubeRequestModes, + checkSupportForKubeResources, isKubeClusterWithNamespaces, type KubeNamespaceRequest, } from '../kube'; @@ -191,15 +191,10 @@ export function RequestCheckout({ }); } - const { - affectedKubeClusterName, - unsupportedKubeRequestModes, - requiresNamespaceSelect, - } = checkForUnsupportedKubeRequestModes(fetchResourceRequestRolesAttempt); - - const hasUnsupportedKubeRequestModes = !!unsupportedKubeRequestModes; - const showRequestRoleErrBanner = - !hasUnsupportedKubeRequestModes && !requiresNamespaceSelect; + const { requestKubeResourceSupported, isRequestKubeResourceError } = + checkSupportForKubeResources(fetchResourceRequestRolesAttempt); + const hasUnsupporteKubeResourceKinds = + !requestKubeResourceSupported && isRequestKubeResourceError; const isInvalidRoleSelection = resourceRequestRoles.length > 0 && @@ -211,8 +206,7 @@ export function RequestCheckout({ createAttempt.status === 'processing' || isInvalidRoleSelection || (fetchResourceRequestRolesAttempt.status === 'failed' && - hasUnsupportedKubeRequestModes) || - requiresNamespaceSelect || + hasUnsupporteKubeResourceKinds) || fetchResourceRequestRolesAttempt.status === 'processing'; const cancelBtnDisabled = @@ -269,13 +263,8 @@ export function RequestCheckout({ @@ -288,21 +277,31 @@ export function RequestCheckout({ {({ validator }) => ( <> - {showRequestRoleErrBanner && + {!isRequestKubeResourceError && + createAttempt.status !== 'failed' && fetchResourceRequestRolesAttempt.status === 'failed' && ( )} - {hasUnsupportedKubeRequestModes && ( + {hasUnsupporteKubeResourceKinds && ( + 248 + ? fetchResourceRequestRolesAttempt.statusText + : null + } + > + + {fetchResourceRequestRolesAttempt.statusText} + + - You can only request Kubernetes resource{' '} - {pluralize(unsupportedKubeRequestModes.length, 'kind')}{' '} - {listToSentence(unsupportedKubeRequestModes)} for - cluster {affectedKubeClusterName}. Requesting those - resource kinds is currently only supported through the{' '} + The listed allowed kinds are currently only supported through + the{' '} ({ > tsh request search {' '} - command that will help you construct the request. + that will help you construct the request. - + Example: @@ -863,6 +862,12 @@ const StyledTable = styled(Table)` overflow: hidden; ` as typeof Table; +const ShortenedText = styled(Text)` + display: -webkit-box; + -webkit-box-orient: vertical; + -webkit-line-clamp: 6; +`; + export type RequestCheckoutWithSliderProps< T extends PendingListItem = PendingListItem, > = { diff --git a/web/packages/shared/components/AccessRequests/NewRequest/kube.test.ts b/web/packages/shared/components/AccessRequests/NewRequest/kube.test.ts index 33e136e0dc038..b10bde737679e 100644 --- a/web/packages/shared/components/AccessRequests/NewRequest/kube.test.ts +++ b/web/packages/shared/components/AccessRequests/NewRequest/kube.test.ts @@ -16,46 +16,148 @@ * along with this program. If not, see . */ -import { checkForUnsupportedKubeRequestModes } from './kube'; +import { Attempt } from 'shared/hooks/useAttemptNext'; -test('checkForUnsupportedKubeRequestModes: non failed status', () => { - const { - affectedKubeClusterName, - unsupportedKubeRequestModes, - requiresNamespaceSelect, - } = checkForUnsupportedKubeRequestModes({ status: '' }); +import { checkSupportForKubeResources } from './kube'; - expect(affectedKubeClusterName).toBeFalsy(); - expect(unsupportedKubeRequestModes).toBeFalsy(); - expect(requiresNamespaceSelect).toBeFalsy(); -}); - -test('checkForUnsupportedKubeRequestModes: failed status with unsupported kinds', () => { - const { - affectedKubeClusterName, - unsupportedKubeRequestModes, - requiresNamespaceSelect, - } = checkForUnsupportedKubeRequestModes({ - status: 'failed', - statusText: `Your Teleport roles request_mode field restricts you from requesting kinds [kube_cluster] for Kubernetes cluster "pumpkin-kube-cluster". Allowed kinds: [pod secret]`, - }); +describe('requestKubeResourceSupported()', () => { + const testCases: { + name: string; + attempt: Attempt; + expected: { + requestKubeResourceSupported: boolean; + isRequestKubeResourceError: boolean; + }; + }[] = [ + { + name: 'non failed status', + attempt: { status: '' }, + expected: { + requestKubeResourceSupported: true, + isRequestKubeResourceError: false, + }, + }, + { + name: 'non request.kubernetes_resources related error', + attempt: { + status: 'failed', + statusText: `some other error`, + }, + expected: { + requestKubeResourceSupported: true, + isRequestKubeResourceError: false, + }, + }, + { + name: 'with supported namespace in error (single role)', + attempt: { + status: 'failed', + statusText: `your Teleport role's "request.kubernetes_resources" field did not allow requesting \ + to some or all of the requested Kubernetes resources. allowed kinds for \ + each requestable roles: test-role-1: [namespace]`, + }, + expected: { + requestKubeResourceSupported: true, + isRequestKubeResourceError: true, + }, + }, + { + name: 'with supported namespace in error (multi role)', + attempt: { + status: 'failed', + statusText: `your Teleport role's "request.kubernetes_resources" field did not allow requesting \ + to some or all of the requested Kubernetes resources. allowed kinds for \ + each requestable roles: test-role-1: [pod secret], test-role-2: [deployment namespace]`, + }, + expected: { + requestKubeResourceSupported: true, + isRequestKubeResourceError: true, + }, + }, + { + name: 'with supported kube_cluster in error (multi role)', + attempt: { + status: 'failed', + statusText: `your Teleport role's "request.kubernetes_resources" field did not allow requesting \ + to some or all of the requested Kubernetes resources. allowed kinds for \ + each requestable roles: test-role-1: [pod secret], test-role-2: [deployment kube_cluster]`, + }, + expected: { + requestKubeResourceSupported: true, + isRequestKubeResourceError: true, + }, + }, + { + name: 'with supported kube_cluster and namespace in error (multi role)', + attempt: { + status: 'failed', + statusText: `your Teleport role's "request.kubernetes_resources" field did not allow requesting \ + to some or all of the requested Kubernetes resources. allowed kinds for \ + each requestable roles: test-role-1: [pod], test-role-2: [namespace kube_cluster]`, + }, + expected: { + requestKubeResourceSupported: true, + isRequestKubeResourceError: true, + }, + }, + { + name: 'without supported kinds in error', + attempt: { + status: 'failed', + statusText: `your Teleport role's "request.kubernetes_resources" field did not allow requesting \ + to some or all of the requested Kubernetes resources. allowed kinds for \ + each requestable roles: test-role-1: [deployment], test-role-2: [pod secret]`, + }, + expected: { + requestKubeResourceSupported: false, + isRequestKubeResourceError: true, + }, + }, + // empty bracket case can happen from admin configuration error + // where allow and deny canceled each other so nothing is allowed. + { + name: 'empty bracket with space', + attempt: { + status: 'failed', + statusText: `your Teleport role's "request.kubernetes_resources" field did not allow requesting \ + to some or all of the requested Kubernetes resources. allowed kinds for \ + each requestable roles: test-role-1: [ ]`, + }, + expected: { + requestKubeResourceSupported: false, + isRequestKubeResourceError: true, + }, + }, + { + name: 'empty bracket without space', + attempt: { + status: 'failed', + statusText: `your Teleport role's "request.kubernetes_resources" field did not allow requesting \ + to some or all of the requested Kubernetes resources. allowed kinds for \ + each requestable roles: test-role-1: []`, + }, + expected: { + requestKubeResourceSupported: false, + isRequestKubeResourceError: true, + }, + }, + // should never happen but just in case + { + name: 'without any role', + attempt: { + status: 'failed', + statusText: `your Teleport role's "request.kubernetes_resources" field did not allow requesting \ + to some or all of the requested Kubernetes resources. allowed kinds for \ + each requestable roles: `, + }, + expected: { + requestKubeResourceSupported: true, + isRequestKubeResourceError: false, + }, + }, + ]; - expect(affectedKubeClusterName).toEqual(`pumpkin-kube-cluster`); - expect(unsupportedKubeRequestModes).toEqual(['pod', 'secret']); - expect(requiresNamespaceSelect).toBeFalsy(); -}); - -test('checkForUnsupportedKubeRequestModes: failed status with supported namespace', () => { - const { - affectedKubeClusterName, - unsupportedKubeRequestModes, - requiresNamespaceSelect, - } = checkForUnsupportedKubeRequestModes({ - status: 'failed', - statusText: `Your Teleport roles request_mode field restricts you from requesting kinds [kube_cluster] for Kubernetes cluster "pumpkin-kube-cluster". Allowed kinds: [pod secret namespace]`, + test.each(testCases)('$name', ({ attempt, expected }) => { + expect(checkSupportForKubeResources(attempt)).toEqual(expected); }); - - expect(affectedKubeClusterName).toEqual(`pumpkin-kube-cluster`); - expect(unsupportedKubeRequestModes).toBeFalsy(); - expect(requiresNamespaceSelect).toBeTruthy(); }); diff --git a/web/packages/shared/components/AccessRequests/NewRequest/kube.ts b/web/packages/shared/components/AccessRequests/NewRequest/kube.ts index e84ed46c972b4..299c19bec91f7 100644 --- a/web/packages/shared/components/AccessRequests/NewRequest/kube.ts +++ b/web/packages/shared/components/AccessRequests/NewRequest/kube.ts @@ -45,50 +45,55 @@ export function isKubeClusterWithNamespaces( } /** - * Checks each data for kube_cluster or namespace + * Parses error message (in an expected format returned from the backend) to see if + * the message is a type of request.kubernetes_resources related error: + * https://github.com/gravitational/teleport/blob/master/lib/services/access_request.go#L2050 + * + * If error matches, it then checks if namespace or kube_cluster is mentioned + * in the "allowed kinds" portion of the error message. */ -export function checkForUnsupportedKubeRequestModes( - requestRoleAttempt: Attempt -) { - let unsupportedKubeRequestModes: string[]; - let affectedKubeClusterName = ''; - let requiresNamespaceSelect = false; +export function checkSupportForKubeResources(requestRoleAttempt: Attempt) { + let requestKubeResourceSupported = true; + let isRequestKubeResourceError = false; + const retVal = { requestKubeResourceSupported, isRequestKubeResourceError }; if (requestRoleAttempt.status === 'failed') { const errMsg = requestRoleAttempt.statusText.toLowerCase(); - if (errMsg.includes('request_mode') && errMsg.includes('allowed kinds: ')) { - let allowedKinds = errMsg.split('allowed kinds: ')[1]; - - // Web UI supports selecting namespace and wildcard - // which basically means requiring namespace. - if (allowedKinds.includes('*') || allowedKinds.includes('namespace')) { - requiresNamespaceSelect = true; - } else { - if (allowedKinds.startsWith('[')) { - allowedKinds = allowedKinds.slice(1, -1); - } - unsupportedKubeRequestModes = allowedKinds.split(' '); + if ( + errMsg.includes('request.kubernetes_resources') && + errMsg.includes('allowed kinds') + ) { + let splitMsgParts = []; + if (errMsg.includes('requested roles: ')) { + splitMsgParts = errMsg.split('requested roles: '); + } else if (errMsg.includes('requestable roles: ')) { + splitMsgParts = errMsg.split('requestable roles: '); } - const initialSplit = errMsg.split('for kubernetes cluster "'); - if (initialSplit.length > 1) { - affectedKubeClusterName = initialSplit[1] - .split('". allowed kinds')[0] - .trim(); + if (!splitMsgParts.length || !splitMsgParts[1]) { + return retVal; } + const kindParts = splitMsgParts[1].split(', '); + + // Check that at least one of the kind parts have a kind that + // the web UI supports (namespace or kube_cluster): + const isSupported = kindParts.some(part => { + const allowed = part.split(': '); + if ( + allowed[1]?.includes('namespace') || + allowed[1]?.includes('kube_cluster') + ) { + return true; + } + }); return { - affectedKubeClusterName, - requiresNamespaceSelect, - unsupportedKubeRequestModes, + requestKubeResourceSupported: isSupported, + isRequestKubeResourceError: true, }; } } - return { - affectedKubeClusterName, - unsupportedKubeRequestModes, - requiresNamespaceSelect, - }; + return retVal; } diff --git a/web/packages/shared/components/AccessRequests/NewRequest/useSpecifiableFields.ts b/web/packages/shared/components/AccessRequests/NewRequest/useSpecifiableFields.ts index 4570c2453776a..3f7f7d6edd826 100644 --- a/web/packages/shared/components/AccessRequests/NewRequest/useSpecifiableFields.ts +++ b/web/packages/shared/components/AccessRequests/NewRequest/useSpecifiableFields.ts @@ -98,6 +98,16 @@ export function useSpecifiableFields() { ); } + function reset() { + setDryRunResponse(null); + setResourceRequestRoles([]); + setSelectedResourceRequestRoles([]); + setSelectedReviewers([]); + setStartTime(null); + setMaxDuration(null); + setPendingRequestTtl(null); + } + function preselectPendingRequestTtlOption( newMaxDuration: number, dryRequestCreated: Date @@ -190,5 +200,6 @@ export function useSpecifiableFields() { startTime, onStartTimeChange, onDryRunChange, + reset, }; } diff --git a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.test.tsx b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.test.tsx index a4dc881042005..fb1c3285ee53f 100644 --- a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.test.tsx +++ b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.test.tsx @@ -184,3 +184,102 @@ test(`fetching requestable roles for a kube cluster's namespaces only creates re }) ); }); + +test('after creating an access request, pending requests and specifiable fields are reset', async () => { + const kube = makeKube(); + const cluster = makeRootCluster(); + const appContext = new MockAppContext(); + appContext.clustersService.setState(draftState => { + draftState.clusters.set(rootClusterUri, cluster); + }); + await appContext.workspacesService.setActiveWorkspace(rootClusterUri); + await appContext.workspacesService + .getWorkspaceAccessRequestsService(rootClusterUri) + .addOrRemoveResource({ + kind: 'kube', + resource: kube, + }); + + let mockedCreateAccessRequestFn = jest.spyOn( + appContext.tshd, + 'createAccessRequest' + ); + + const wrapper = ({ children }) => ( + + {children} + + ); + + let { result } = renderHook(useAccessRequestCheckout, { + wrapper, + }); + + await waitFor(() => { + result.current.setSelectedReviewers([{ value: 'bob', label: 'bob' }]); + expect(result.current.selectedReviewers).toEqual([ + { value: 'bob', label: 'bob' }, + ]); + }); + + await waitFor(() => { + result.current.setSelectedResourceRequestRoles(['apple', 'banana']); + expect(result.current.selectedResourceRequestRoles).toEqual([ + 'apple', + 'banana', + ]); + }); + + await waitFor(async () => { + await result.current.createRequest({ + suggestedReviewers: result.current.selectedReviewers.map(r => r.value), + }); + expect(mockedCreateAccessRequestFn).toHaveBeenCalledWith({ + rootClusterUri: '/clusters/teleport-local', + resourceIds: [ + { + clusterName: 'teleport-local', + kind: 'kube_cluster', + name: kube.name, + subResourceName: '', + }, + ], + roles: ['apple', 'banana'], + suggestedReviewers: ['bob'], + }); + }); + + // Call create again, should've cleared reviewers and previous roles. + mockedCreateAccessRequestFn.mockClear(); + await waitFor(async () => { + // A successful create would've cleared all selected resources, + // so we add it back here to allow creating again. + expect(result.current.pendingAccessRequests).toHaveLength(0); + await appContext.workspacesService + .getWorkspaceAccessRequestsService(rootClusterUri) + .addOrRemoveResource({ + kind: 'kube', + resource: kube, + }); + }); + + await waitFor(async () => { + await result.current.createRequest({ + suggestedReviewers: result.current.selectedReviewers.map(r => r.value), + }); + expect(mockedCreateAccessRequestFn).toHaveBeenCalledWith({ + rootClusterUri: '/clusters/teleport-local', + resourceIds: [ + { + clusterName: 'teleport-local', + kind: 'kube_cluster', + name: kube.name, + subResourceName: '', + }, + ], + // These fields gotten cleared after the first create. + roles: [], + suggestedReviewers: [], + }); + }); +}); diff --git a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.ts b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.ts index 7f8095a019828..4d70ada109b16 100644 --- a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.ts +++ b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.ts @@ -77,6 +77,7 @@ export default function useAccessRequestCheckout() { onDryRunChange, startTime, onStartTimeChange, + reset: resetSpecifiableFields, } = useSpecifiableFields(); const [showCheckout, setShowCheckout] = useState(false); @@ -371,6 +372,7 @@ export default function useAccessRequestCheckout() { } function reset() { + resetSpecifiableFields(); if (workspaceAccessRequest) { return workspaceAccessRequest.clearPendingAccessRequest(); }