[v17] Teleterm: fix incorrect cluster URI when fetching kube namespac…

…es on a leaf cluster (access request) (#48989) * Use tc.SiteName over cluster.Name * Teleterm: fix fetching namespaces with incorrect cluster URI If you add a root and leaf kube cluster,and on leaf Teleport cluster, listing namespaces for root kube cluster was referring to incorrect Teleport cluster URI * Teleterm: fix re-ordering of selected namespaces when done selecting Web UI still has this weird re-ordering because web UI still stores resource ids in a map which doesn't preserve order * Add requested length test * Address CR * Address CRs
gravitational · Nov 14, 2024 · 4a43cea · 4a43cea
1 parent ffe43c3
commit 4a43cea
Show file tree

Hide file tree

Showing 16 changed files with 1,019 additions and 890 deletions.
diff --git a/gen/proto/go/teleport/lib/teleterm/v1/service.pb.go b/gen/proto/go/teleport/lib/teleterm/v1/service.pb.go
diff --git a/gen/proto/ts/teleport/lib/teleterm/v1/service_pb.ts b/gen/proto/ts/teleport/lib/teleterm/v1/service_pb.ts
diff --git a/lib/teleterm/apiserver/handler/handler_kubes.go b/lib/teleterm/apiserver/handler/handler_kubes.go
@@ -38,16 +38,18 @@ func (s *Handler) ListKubernetesResources(ctx context.Context, req *api.ListKube
 		return nil, trace.Wrap(err)
 	}
 
-	resp, err := s.DaemonService.ListKubernetesResources(ctx, clusterURI, req)
+	resources, err := s.DaemonService.ListKubernetesResources(ctx, clusterURI, req)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
 
-	response := &api.ListKubernetesResourcesResponse{
-		NextKey: resp.NextKey,
-	}
+	response := &api.ListKubernetesResourcesResponse{}
 
-	for _, kubeResource := range resp.Resources {
+	for _, resource := range resources {
+		kubeResource, ok := resource.(*types.KubernetesResourceV1)
+		if !ok {
+			return nil, trace.BadParameter("expected resource type %T, got %T", types.KubernetesResourceV1{}, resource)
+		}
 		response.Resources = append(response.Resources, newApiKubeResource(kubeResource, req.GetKubernetesCluster(), clusterURI))
 	}
 

diff --git a/lib/teleterm/daemon/daemon.go b/lib/teleterm/daemon/daemon.go
@@ -30,6 +30,7 @@ import (
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 
+	apiclient "github.com/gravitational/teleport/api/client"
 	"github.com/gravitational/teleport/api/client/proto"
 	accesslistv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/accesslist/v1"
 	devicepb "github.com/gravitational/teleport/api/gen/proto/go/teleport/devicetrust/v1"
@@ -799,21 +800,21 @@ func (s *Service) AssumeRole(ctx context.Context, req *api.AssumeRoleRequest) er
 
 // ListKubernetesResourcesRequest defines a request to retrieve kube resources paginated.
 // Only one type of kube resource can be retrieved per request (eg: namespace, pods, secrets, etc.)
-func (s *Service) ListKubernetesResources(ctx context.Context, clusterURI uri.ResourceURI, req *api.ListKubernetesResourcesRequest) (*kubeproto.ListKubernetesResourcesResponse, error) {
-	cluster, tc, err := s.ResolveClusterURI(clusterURI)
+func (s *Service) ListKubernetesResources(ctx context.Context, clusterURI uri.ResourceURI, req *api.ListKubernetesResourcesRequest) ([]types.ResourceWithLabels, error) {
+	_, tc, err := s.ResolveClusterURI(clusterURI)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
 
-	var resources *kubeproto.ListKubernetesResourcesResponse
+	var resources []types.ResourceWithLabels
 
 	err = clusters.AddMetadataToRetryableError(ctx, func() error {
-		kubenetesServiceClient, err := tc.NewKubernetesServiceClient(ctx, cluster.Name)
+		kubeServiceClient, err := tc.NewKubernetesServiceClient(ctx, tc.SiteName)
 		if err != nil {
 			return trace.Wrap(err)
 		}
 
-		resources, err = kubenetesServiceClient.ListKubernetesResources(ctx, &kubeproto.ListKubernetesResourcesRequest{
+		req := &kubeproto.ListKubernetesResourcesRequest{
 			ResourceType:        req.GetResourceType(),
 			Limit:               req.GetLimit(),
 			StartKey:            req.GetNextKey(),
@@ -822,8 +823,10 @@ func (s *Service) ListKubernetesResources(ctx context.Context, clusterURI uri.Re
 			UseSearchAsRoles:    req.GetUseSearchAsRoles(),
 			KubernetesCluster:   req.GetKubernetesCluster(),
 			KubernetesNamespace: req.GetKubernetesNamespace(),
-			TeleportCluster:     cluster.Name,
-		})
+			TeleportCluster:     tc.SiteName,
+		}
+
+		resources, err = apiclient.GetKubernetesResourcesWithFilters(ctx, kubeServiceClient, req)
 		return trace.Wrap(err)
 	})
 

diff --git a/proto/teleport/lib/teleterm/v1/service.proto b/proto/teleport/lib/teleterm/v1/service.proto
@@ -344,7 +344,8 @@ message ListKubernetesResourcesRequest {
 
 message ListKubernetesResourcesResponse {
   repeated KubeResource resources = 1;
-  string next_key = 2;
+  reserved 2;
+  reserved "next_key";
 }
 
 // CredentialInfo holds fields related to a user's WebAuthn credential.

diff --git a/...ges/shared/components/AccessRequests/NewRequest/RequestCheckout/KubeNamespaceSelector.tsx b/...ges/shared/components/AccessRequests/NewRequest/RequestCheckout/KubeNamespaceSelector.tsx
@@ -28,18 +28,19 @@ import { CheckableOptionComponent } from '../CheckableOption';
 
 import { PendingListItem, PendingKubeResourceItem } from './RequestCheckout';
 
-import type { KubeNamespaceRequest } from '../kube';
-
 export function KubeNamespaceSelector({
   kubeClusterItem,
   fetchKubeNamespaces,
   savedResourceItems,
-  bulkToggleKubeResources,
+  updateNamespacesForKubeCluster,
 }: {
   kubeClusterItem: PendingListItem;
-  fetchKubeNamespaces(p: KubeNamespaceRequest): Promise<string[]>;
+  fetchKubeNamespaces(
+    search: string,
+    kubeCluster: PendingListItem
+  ): Promise<string[]>;
   savedResourceItems: PendingListItem[];
-  bulkToggleKubeResources: (
+  updateNamespacesForKubeCluster: (
     resources: PendingKubeResourceItem[],
     resource: PendingListItem
   ) => void;
@@ -57,7 +58,9 @@ export function KubeNamespaceSelector({
 
   const currKubeClustersNamespaceItems = savedResourceItems.filter(
     resource =>
-      resource.kind === 'namespace' && resource.id === kubeClusterItem.id
+      resource.kind === 'namespace' &&
+      resource.id === kubeClusterItem.id &&
+      resource.clusterName === kubeClusterItem.clusterName
   ) as PendingKubeResourceItem[];
 
   const [selectedOpts, setSelectedOpts] = useState<Option[]>(() =>
@@ -75,63 +78,42 @@ export function KubeNamespaceSelector({
 
     switch (actionMeta.action) {
       case 'clear':
-        bulkToggleKubeResources(
-          currKubeClustersNamespaceItems,
-          kubeClusterItem
-        );
+        updateNamespacesForKubeCluster([], kubeClusterItem);
         return;
       case 'remove-value':
-        bulkToggleKubeResources(
-          [
-            {
-              kind: 'namespace',
-              id: kubeClusterItem.id,
-              subResourceName: actionMeta.removedValue.value,
-              clusterName: kubeClusterItem.clusterName,
-              name: actionMeta.removedValue.value,
-            },
-          ],
+        const selectedOptions = options || [];
+        updateNamespacesForKubeCluster(
+          selectedOptions.map(o => optionToKubeNamespace(o)),
           kubeClusterItem
         );
         return;
     }
   }
 
   const handleMenuClose = () => {
+    const selectedOptions = selectedOpts || [];
     setIsMenuOpen(false);
-
-    const currNamespaces = currKubeClustersNamespaceItems.map(
-      n => n.subResourceName
-    );
-    const selectedNamespaceIds = selectedOpts.map(o => o.value);
-    const toKeep = selectedNamespaceIds.filter(id =>
-      currNamespaces.includes(id)
-    );
-
-    const toInsert = selectedNamespaceIds.filter(o => !toKeep.includes(o));
-    const toRemove = currNamespaces.filter(n => !toKeep.includes(n));
-
-    if (!toInsert.length && !toRemove.length) {
-      return;
-    }
-
-    bulkToggleKubeResources(
-      [...toRemove, ...toInsert].map(namespace => ({
-        kind: 'namespace',
-        id: kubeClusterItem.id,
-        subResourceName: namespace,
-        clusterName: kubeClusterItem.clusterName,
-        name: namespace,
-      })),
+    updateNamespacesForKubeCluster(
+      selectedOptions.map(o => optionToKubeNamespace(o)),
       kubeClusterItem
     );
   };
 
+  function optionToKubeNamespace(
+    selectedOption: Option
+  ): PendingKubeResourceItem {
+    const namespace = selectedOption.value;
+    return {
+      kind: 'namespace',
+      id: kubeClusterItem.id,
+      subResourceName: namespace,
+      clusterName: kubeClusterItem.clusterName,
+      name: namespace,
+    };
+  }
+
   async function handleLoadOptions(input: string) {
-    const namespaces = await fetchKubeNamespaces({
-      kubeCluster: kubeClusterItem.id,
-      search: input,
-    });
+    const namespaces = await fetchKubeNamespaces(input, kubeClusterItem);
 
     return namespaces.map(namespace => ({
       kind: 'namespace',
@@ -141,10 +123,10 @@ export function KubeNamespaceSelector({
   }
 
   return (
-    <Box width="100%" mb={-3}>
+    <Box width="100%" mb={-3} mt={2}>
       <StyledSelect
         label={`Namespaces`}
-        inputId={kubeClusterItem.id}
+        inputId={`${kubeClusterItem.id}-${kubeClusterItem.clusterName}`}
         width="100%"
         placeholder="Start typing a namespace and press enter"
         isMulti

diff --git a/...ges/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.story.tsx b/...ges/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.story.tsx
@@ -196,7 +196,7 @@ const baseProps: RequestCheckoutWithSliderProps = {
     'namespace3',
     'namespace4',
   ],
-  bulkToggleKubeResources: () => null,
+  updateNamespacesForKubeCluster: () => null,
   createAttempt: { status: '' },
   fetchResourceRequestRolesAttempt: { status: '' },
   isResourceRequest: false,

diff --git a/...ages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.test.tsx b/...ages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.test.tsx
@@ -175,5 +175,5 @@ const props: RequestCheckoutWithSliderProps = {
   startTime: null,
   onStartTimeChange: () => null,
   fetchKubeNamespaces: () => null,
-  bulkToggleKubeResources: () => null,
+  updateNamespacesForKubeCluster: () => null,
 };
diff --git a/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.tsx b/web/packages/shared/components/AccessRequests/NewRequest/RequestCheckout/RequestCheckout.tsx
@@ -55,7 +55,6 @@ import { AccessDurationRequest } from '../../AccessDuration';
 import {
   checkSupportForKubeResources,
   isKubeClusterWithNamespaces,
-  type KubeNamespaceRequest,
 } from '../kube';
 
 import { ReviewerOption } from './types';
@@ -170,7 +169,7 @@ export function RequestCheckout<T extends PendingListItem>({
   startTime,
   onStartTimeChange,
   fetchKubeNamespaces,
-  bulkToggleKubeResources,
+  updateNamespacesForKubeCluster,
 }: RequestCheckoutProps<T>) {
   const [reason, setReason] = useState('');
 
@@ -241,7 +240,7 @@ export function RequestCheckout<T extends PendingListItem>({
   function customRow(item: T) {
     if (item.kind === 'kube_cluster') {
       return (
-        <td colSpan={3}>
+        <td colSpan={showClusterNameColumn ? 4 : 3}>
           <Flex>
             <Flex flexWrap="wrap">
               <Flex
@@ -251,6 +250,7 @@ export function RequestCheckout<T extends PendingListItem>({
                 alignItems="center"
               >
                 <Flex gap={5}>
+                  {showClusterNameColumn && <Box>{item.clusterName}</Box>}
                   <Box>{getPrettyResourceKind(item.kind)}</Box>
                   <Box>{item.name}</Box>
                 </Flex>
@@ -265,7 +265,7 @@ export function RequestCheckout<T extends PendingListItem>({
                 kubeClusterItem={item}
                 savedResourceItems={pendingAccessRequests}
                 fetchKubeNamespaces={fetchKubeNamespaces}
-                bulkToggleKubeResources={bulkToggleKubeResources}
+                updateNamespacesForKubeCluster={updateNamespacesForKubeCluster}
               />
             </Flex>
           </Flex>
@@ -934,8 +934,8 @@ export type RequestCheckoutProps<T extends PendingListItem = PendingListItem> =
     Header?: () => JSX.Element;
     startTime: Date;
     onStartTimeChange(t?: Date): void;
-    fetchKubeNamespaces(p: KubeNamespaceRequest): Promise<string[]>;
-    bulkToggleKubeResources(
+    fetchKubeNamespaces(search: string, kubeCluster: T): Promise<string[]>;
+    updateNamespacesForKubeCluster(
       kubeResources: PendingKubeResourceItem[],
       kubeCluster: T
     ): void;

diff --git a/web/packages/shared/components/AccessRequests/NewRequest/index.ts b/web/packages/shared/components/AccessRequests/NewRequest/index.ts
@@ -20,5 +20,4 @@ export * from './RequestCheckout';
 export * from './ResourceList';
 export type { ResourceMap, RequestableResourceKind } from './resource';
 export { getEmptyResourceState } from './resource';
-export type { KubeNamespaceRequest } from './kube';
 export { isKubeClusterWithNamespaces } from './kube';
diff --git a/web/packages/shared/components/AccessRequests/NewRequest/kube.ts b/web/packages/shared/components/AccessRequests/NewRequest/kube.ts
@@ -20,11 +20,6 @@ import { Attempt } from 'shared/hooks/useAttemptNext';
 
 import { PendingListItem } from './RequestCheckout';
 
-export type KubeNamespaceRequest = {
-  kubeCluster: string;
-  search: string;
-};
-
 /**
  * Returns true if the item is a kube cluster or is a namespace
  * of the item.

diff --git a/web/packages/teleterm/src/ui/AccessRequestCheckout/AccessRequestCheckout.tsx b/web/packages/teleterm/src/ui/AccessRequestCheckout/AccessRequestCheckout.tsx
@@ -104,7 +104,7 @@ export function AccessRequestCheckout() {
     startTime,
     onStartTimeChange,
     fetchKubeNamespaces,
-    bulkToggleKubeResources,
+    updateNamespacesForKubeCluster,
   } = useAccessRequestCheckout();
 
   const isRoleRequest = pendingAccessRequests[0]?.kind === 'role';
@@ -284,7 +284,7 @@ export function AccessRequestCheckout() {
             startTime={startTime}
             onStartTimeChange={onStartTimeChange}
             fetchKubeNamespaces={fetchKubeNamespaces}
-            bulkToggleKubeResources={bulkToggleKubeResources}
+            updateNamespacesForKubeCluster={updateNamespacesForKubeCluster}
           />
         )}
       </Transition>