From 2042699afcb31ee76831b5785f6287d2101b007b Mon Sep 17 00:00:00 2001 From: Lisa Kim Date: Fri, 18 Oct 2024 00:26:58 -0700 Subject: [PATCH] Address CR: store namespaces with kube cluster request --- .../RequestCheckout/KubeNamespaceSelector.tsx | 10 +- .../RequestCheckout/RequestCheckout.story.tsx | 8 +- .../RequestCheckout/RequestCheckout.tsx | 2 +- .../AccessRequests/NewRequest/kube.ts | 14 -- .../AccessRequestCheckout.tsx | 4 +- .../useAccessRequestCheckout.ts | 133 ++++++++---------- .../accessRequestsService.ts | 32 ++++- 7 files changed, 102 insertions(+), 101 deletions(-) diff --git a/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/KubeNamespaceSelector.tsx b/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/KubeNamespaceSelector.tsx index 0852090517d09..64022755a5137 100644 --- a/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/KubeNamespaceSelector.tsx +++ b/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/KubeNamespaceSelector.tsx @@ -41,7 +41,7 @@ export function KubeNamespaceSelector({ namespaceRequired, }: { kubeClusterItem: PendingListItem; - fetchKubeNamespaces(p: KubeNamespaceRequest): Promise; + fetchKubeNamespaces(p: KubeNamespaceRequest): Promise; savedResourceItems: PendingListItem[]; toggleResource: (resource: PendingListItem) => void; bulkToggleKubeResources: ( @@ -129,12 +129,16 @@ export function KubeNamespaceSelector({ }; async function handleLoadOptions(input: string) { - const options = await fetchKubeNamespaces({ + const namespaces = await fetchKubeNamespaces({ kubeCluster: kubeClusterItem.id, search: input, }); - return options; + return namespaces.map(namespace => ({ + kind: 'namespace', + value: namespace, + label: namespace, + })); } return ( 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 9a9e451d2218f..f54811f3732af 100644 --- a/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.story.tsx +++ b/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.story.tsx @@ -165,10 +165,10 @@ export const Success = () => ( const baseProps: RequestCheckoutWithSliderProps = { fetchKubeNamespaces: async () => [ - { value: 'namespace1', label: 'namespace1' }, - { value: 'namespace2', label: 'namespace2' }, - { value: 'namespace3', label: 'namespace3' }, - { value: 'namespace4', label: 'namespace4' }, + 'namespace1', + 'namespace2', + 'namespace3', + 'namespace4', ], bulkToggleKubeResources: () => null, createAttempt: { status: '' }, diff --git a/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.tsx b/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.tsx index 88033356d6f25..d4e351812a703 100644 --- a/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.tsx +++ b/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.tsx @@ -904,7 +904,7 @@ export type RequestCheckoutProps = Header?: () => JSX.Element; startTime: Date; onStartTimeChange(t?: Date): void; - fetchKubeNamespaces(p: KubeNamespaceRequest): Promise; + fetchKubeNamespaces(p: KubeNamespaceRequest): Promise; bulkToggleKubeResources( kubeResources: PendingKubeResourceItem[], kubeCluster: T diff --git a/web/packages/shared/components/AccessRequests/NewRequest/kube.ts b/web/packages/shared/components/AccessRequests/NewRequest/kube.ts index 7f70615729d15..3917e89219ed6 100644 --- a/web/packages/shared/components/AccessRequests/NewRequest/kube.ts +++ b/web/packages/shared/components/AccessRequests/NewRequest/kube.ts @@ -19,7 +19,6 @@ import { Attempt } from 'shared/hooks/useAttemptNext'; import { PendingListItem } from './RequestCheckout'; -import { RequestableResourceKind } from './resource'; export type KubeNamespaceRequest = { kubeCluster: string; @@ -93,16 +92,3 @@ export function checkForUnsupportedKubeRequestModes( requiresNamespaceSelect, }; } - -export function requiresKubeResourceSelection({ - dryRun, - requestMode, - kind, -}: { - dryRun: boolean; - requestMode: KubeResourceKind[]; - kind: RequestableResourceKind; -}) { - const requiresKubeResourceSelection = requestMode.length > 0; - return dryRun && kind === 'kube_cluster' && requiresKubeResourceSelection; -} diff --git a/web/packages/teleterm/src/ui/AccessRequestCheckout/AccessRequestCheckout.tsx b/web/packages/teleterm/src/ui/AccessRequestCheckout/AccessRequestCheckout.tsx index 10d69009d83cb..6a90697bbb7cc 100644 --- a/web/packages/teleterm/src/ui/AccessRequestCheckout/AccessRequestCheckout.tsx +++ b/web/packages/teleterm/src/ui/AccessRequestCheckout/AccessRequestCheckout.tsx @@ -114,11 +114,11 @@ export function AccessRequestCheckout() { setShowCheckout(false); } - const filteredData = data?.filter(d => + const filteredData = data.filter(d => excludeKubeClusterWithNamespaces(d, data) ); - const numAddedResources = filteredData?.length; + const numAddedResources = filteredData.length; // We should rather detect how much space we have, // but for simplicity we only count items. diff --git a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.ts b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.ts index 5b810ed34f2cb..6c80ac1d57a42 100644 --- a/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.ts +++ b/web/packages/teleterm/src/ui/AccessRequestCheckout/useAccessRequestCheckout.ts @@ -20,7 +20,6 @@ import { useState, useEffect } from 'react'; import { Timestamp } from 'gen-proto-ts/google/protobuf/timestamp_pb'; import useAttempt from 'shared/hooks/useAttemptNext'; -import { Option } from 'shared/components/Select'; import { getDryRunMaxDuration, @@ -32,10 +31,8 @@ import { CreateRequest } from 'shared/components/AccessRequests/Shared/types'; import { excludeKubeClusterWithNamespaces, KubeNamespaceRequest, - requiresKubeResourceSelection, } from 'shared/components/AccessRequests/NewRequest/kube'; import { PendingKubeResourceItem } from 'shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout'; -import { KubeResourceKind } from 'teleport/services/kube'; import { useAppContext } from 'teleterm/ui/appContextProvider'; import { @@ -64,13 +61,6 @@ export default function useAccessRequestCheckout() { ctx.workspacesService?.getActiveWorkspace()?.localClusterUri; const rootClusterUri = ctx.workspacesService?.getRootClusterUri(); - const loggedInUser = - ctx.clustersService.findCluster(rootClusterUri)?.loggedInUser; - const allowedKubeSubresourceKinds = - loggedInUser?.requestMode?.kubernetesResources?.map( - r => r.kind as KubeResourceKind - ) || []; - const { selectedReviewers, setSelectedReviewers, @@ -122,7 +112,10 @@ export default function useAccessRequestCheckout() { return; } - const data = getPendingAccessRequestsPerResource(pendingAccessRequest); + const data = getPendingAccessRequestsPerResource({ + pendingRequest: pendingAccessRequest, + excludeSubResourceParentResource: true, + }); runFetchResourceRoles(() => retryWithRelogin(ctx, clusterUri, async () => { const { response } = await ctx.tshd.getRequestableRoles({ @@ -161,9 +154,21 @@ export default function useAccessRequestCheckout() { } }, [showCheckout, hasExited, createRequestAttempt.status]); - function getPendingAccessRequestsPerResource( - pendingRequest: PendingAccessRequest - ): PendingListItemWithOriginalItem[] { + /** + * + * @param pendingRequest holds a list or map of resources to process + * @param excludeSubResourceParentResource when true, resources that have + * subresources, will be excluded from the returned list. eg: + * if a kube_cluster resource has a list of namespaces (subresources), + * then if this flag is true, kube_cluster will be excluded from the result. + */ + function getPendingAccessRequestsPerResource({ + pendingRequest, + excludeSubResourceParentResource = false, + }: { + pendingRequest: PendingAccessRequest; + excludeSubResourceParentResource?: boolean; + }): PendingListItemWithOriginalItem[] { const data: PendingListItemWithOriginalItem[] = []; if (!workspaceAccessRequest) { return data; @@ -185,6 +190,34 @@ export default function useAccessRequestCheckout() { } case 'resource': { pendingRequest.resources.forEach(resourceRequest => { + // If this request is a kube cluster and has namespaces + // extract each as own request. + if ( + resourceRequest.kind === 'kube' && + resourceRequest.resource.namespaces?.size > 0 + ) { + // Process each namespace. + resourceRequest.resource.namespaces.forEach(namespaceRequest => { + const { kind, id, name } = + extractResourceRequestProperties(namespaceRequest); + + const item = { + kind, + id, + name, + subResourceName: name, + originalItem: namespaceRequest, + clusterName: ctx.clustersService.findClusterByResource( + namespaceRequest.resource.uri + )?.name, + }; + data.push(item); + }); + if (excludeSubResourceParentResource) { + return; + } + } + const { kind, id, name } = extractResourceRequestProperties(resourceRequest); const item: PendingListItemWithOriginalItem = { @@ -196,10 +229,6 @@ export default function useAccessRequestCheckout() { resourceRequest.resource.uri )?.name, }; - - if (kind === 'namespace') { - item.subResourceName = name; - } data.push(item); }); } @@ -225,42 +254,6 @@ export default function useAccessRequestCheckout() { await workspaceAccessRequest.addOrRemoveResource( pendingListItem.originalItem ); - - if (pendingListItem.kind === 'kube_cluster') { - deleteKubeClustersNamespaces({ - kubeClusterUri: pendingListItem.originalItem.resource.uri, - kubeClusterId: pendingListItem.id, - }); - } - } - - async function deleteKubeClustersNamespaces({ - kubeClusterUri, - kubeClusterId, - }: { - kubeClusterUri: string; - kubeClusterId: string; - }) { - const pending = workspaceAccessRequest.getPendingAccessRequest(); - if (pending.kind === 'role') return; - const hasInsertedItem = pending.resources.has(kubeClusterUri); - - if (!hasInsertedItem) { - const namespacesToDelete: ResourceRequest[] = []; - pending.resources.forEach(value => { - if (value.kind === 'namespace') { - const { kubeId } = routing.parseKubeResourceNamespaceUri( - value.resource.uri - ).params; - if (kubeId === kubeClusterId) { - namespacesToDelete.push(value); - } - } - }); - if (namespacesToDelete.length) { - await workspaceAccessRequest.addOrRemoveResources(namespacesToDelete); - } - } } async function bulkToggleKubeResources( @@ -294,7 +287,10 @@ export default function useAccessRequestCheckout() { * Shared logic used both during dry runs and regular access request creation. */ function prepareAndCreateRequest(req: CreateRequest) { - const data = getPendingAccessRequestsPerResource(pendingAccessRequest); + const data = getPendingAccessRequestsPerResource({ + pendingRequest: pendingAccessRequest, + excludeSubResourceParentResource: true, + }); const params: CreateAccessRequestRequest = { rootClusterUri, @@ -304,17 +300,6 @@ export default function useAccessRequestCheckout() { resourceIds: data .filter(d => d.kind !== 'role') .filter(d => excludeKubeClusterWithNamespaces(d, data)) - // Skip dry running with kube_cluster that requires - // subresource selection. Otherwise the user will see - // an error saying they can't make kube_cluster requests. - .filter( - d => - !requiresKubeResourceSelection({ - dryRun: req.dryRun, - kind: d.kind, - requestMode: allowedKubeSubresourceKinds, - }) - ) .map(d => { if (d.kind === 'namespace') { return { @@ -374,6 +359,7 @@ export default function useAccessRequestCheckout() { }); teletermAccessRequest = accessRequest; } catch { + setCreateRequestAttempt({ status: '' }); return; } @@ -435,7 +421,7 @@ export default function useAccessRequestCheckout() { async function fetchKubeNamespaces({ kubeCluster, search, - }: KubeNamespaceRequest): Promise { + }: KubeNamespaceRequest): Promise { const { response } = await ctx.tshd.listKubernetesResources({ searchKeywords: search, limit: 50, @@ -447,13 +433,7 @@ export default function useAccessRequestCheckout() { kubernetesCluster: kubeCluster, kubernetesNamespace: '', }); - return response.resources.map(i => { - return { - kind: 'namespace', - value: i.name, - label: i.name, - }; - }); + return response.resources.map(i => i.name); } const shouldShowClusterNameColumn = @@ -467,7 +447,9 @@ export default function useAccessRequestCheckout() { isCollapsed, assumedRequests: getAssumedRequests(), toggleResource, - data: getPendingAccessRequestsPerResource(pendingAccessRequest), + data: getPendingAccessRequestsPerResource({ + pendingRequest: pendingAccessRequest, + }), shouldShowClusterNameColumn, createRequest, reset, @@ -497,7 +479,6 @@ export default function useAccessRequestCheckout() { onStartTimeChange, fetchKubeNamespaces, bulkToggleKubeResources, - allowedKubeSubresourceKinds, }; } diff --git a/web/packages/teleterm/src/ui/services/workspacesService/accessRequestsService.ts b/web/packages/teleterm/src/ui/services/workspacesService/accessRequestsService.ts index 81dae179d31fb..5b80cdc05780b 100644 --- a/web/packages/teleterm/src/ui/services/workspacesService/accessRequestsService.ts +++ b/web/packages/teleterm/src/ui/services/workspacesService/accessRequestsService.ts @@ -118,6 +118,10 @@ export class AccessRequestsService { const { resources } = draftState.pending; requestedResources.forEach(request => { + if (request.kind === 'namespace') { + this.addOrRemoveKubeNamespace(request, resources); + return; + } if (resources.has(request.resource.uri)) { resources.delete(request.resource.uri); } else { @@ -127,6 +131,32 @@ export class AccessRequestsService { }); } + async addOrRemoveKubeNamespace( + namespaceResourceRequest: ResourceRequest, + resources: Map + ) { + const { uri: resourceUri } = namespaceResourceRequest.resource; + + const requestedResource = resources.get( + routing.getKubeUri( + routing.parseKubeResourceNamespaceUri(resourceUri).params + ) + ); + if (!requestedResource || requestedResource.kind !== 'kube') { + throw new Error('Cannot add a kube namespace to a non-kube resource'); + } + const kubeResource = requestedResource.resource; + + if (!kubeResource.namespaces) { + kubeResource.namespaces = new Map(); + } + if (kubeResource.namespaces.has(resourceUri)) { + kubeResource.namespaces.delete(resourceUri); + } else { + kubeResource.namespaces.set(resourceUri, namespaceResourceRequest); + } + } + /** * Removes all requested resources, if all the requested resources were already added * or adds all requested resources, if not all requested resources were added. @@ -299,7 +329,7 @@ export type ResourceRequest = kind: 'kube'; resource: { uri: KubeUri; - namespaces?: KubeResourceNamespaceUri[]; + namespaces?: Map; }; } | {