Skip to content

Commit

Permalink
[v16] Teleterm: fix incorrect cluster URI when fetching kube namespac…
Browse files Browse the repository at this point in the history
…es on a leaf cluster (#48990)

* Teleterm: fix incorrect cluster URI when fetching kube namespaces on a leaf cluster (access request) (#48786)

* 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

* Update snaps
  • Loading branch information
kimlisa authored Nov 14, 2024
1 parent 13d6fe1 commit 25b442f
Show file tree
Hide file tree
Showing 17 changed files with 1,080 additions and 939 deletions.
1,487 changes: 739 additions & 748 deletions gen/proto/go/teleport/lib/teleterm/v1/service.pb.go

Large diffs are not rendered by default.

14 changes: 1 addition & 13 deletions gen/proto/ts/teleport/lib/teleterm/v1/service_pb.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 7 additions & 5 deletions lib/teleterm/apiserver/handler/handler_kubes.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,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))
}

Expand Down
17 changes: 10 additions & 7 deletions lib/teleterm/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -816,21 +817,21 @@ func (s *Service) GetKubes(ctx context.Context, req *api.GetKubesRequest) (*clus

// 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(),
Expand All @@ -839,8 +840,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)
})

Expand Down
3 changes: 2 additions & 1 deletion proto/teleport/lib/teleterm/v1/service.proto
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,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;
Expand All @@ -66,7 +67,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[]>(() =>
Expand All @@ -90,63 +93,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 = '') {
const namespaces = await fetchKubeNamespaces({
kubeCluster: kubeClusterItem.id,
search: input,
});
const namespaces = await fetchKubeNamespaces(input, kubeClusterItem);

return namespaces.map(namespace => ({
kind: 'namespace',
Expand All @@ -156,10 +138,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ const baseProps: RequestCheckoutWithSliderProps = {
'namespace3',
'namespace4',
],
bulkToggleKubeResources: () => null,
updateNamespacesForKubeCluster: () => null,
createAttempt: { status: '' },
fetchResourceRequestRolesAttempt: { status: '' },
isResourceRequest: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,5 +175,5 @@ const props: RequestCheckoutWithSliderProps = {
startTime: null,
onStartTimeChange: () => null,
fetchKubeNamespaces: () => null,
bulkToggleKubeResources: () => null,
updateNamespacesForKubeCluster: () => null,
};
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ import { AccessDurationRequest } from '../../AccessDuration';
import {
checkSupportForKubeResources,
isKubeClusterWithNamespaces,
type KubeNamespaceRequest,
} from '../kube';

import { ReviewerOption } from './types';
Expand Down Expand Up @@ -160,7 +159,7 @@ export function RequestCheckout<T extends PendingListItem>({
startTime,
onStartTimeChange,
fetchKubeNamespaces,
bulkToggleKubeResources,
updateNamespacesForKubeCluster,
}: RequestCheckoutProps<T>) {
const [reason, setReason] = useState('');

Expand Down Expand Up @@ -231,7 +230,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
Expand All @@ -241,6 +240,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>
Expand All @@ -255,7 +255,7 @@ export function RequestCheckout<T extends PendingListItem>({
kubeClusterItem={item}
savedResourceItems={pendingAccessRequests}
fetchKubeNamespaces={fetchKubeNamespaces}
bulkToggleKubeResources={bulkToggleKubeResources}
updateNamespacesForKubeCluster={updateNamespacesForKubeCluster}
/>
</Flex>
</Flex>
Expand Down Expand Up @@ -898,8 +898,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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ exports[`failed state 1`] = `
.c18 {
box-sizing: border-box;
margin-bottom: -16px;
margin-top: 8px;
width: 100%;
}
Expand Down Expand Up @@ -1113,7 +1114,7 @@ exports[`failed state 1`] = `
>
<label
class="c21"
for="app-name"
for="app-name-undefined"
>
Namespaces
</label>
Expand Down Expand Up @@ -1146,7 +1147,7 @@ exports[`failed state 1`] = `
autocapitalize="none"
autocomplete="off"
autocorrect="off"
id="app-name"
id="app-name-undefined"
spellcheck="false"
style="box-sizing: content-box; width: 2px; border: 0px; opacity: 1; outline: 0; padding: 0px;"
tabindex="0"
Expand Down Expand Up @@ -1695,6 +1696,7 @@ exports[`loaded state 1`] = `
.c17 {
box-sizing: border-box;
margin-bottom: -16px;
margin-top: 8px;
width: 100%;
}
Expand Down Expand Up @@ -2764,7 +2766,7 @@ exports[`loaded state 1`] = `
>
<label
class="c20"
for="app-name"
for="app-name-undefined"
>
Namespaces
</label>
Expand Down Expand Up @@ -2797,7 +2799,7 @@ exports[`loaded state 1`] = `
autocapitalize="none"
autocomplete="off"
autocorrect="off"
id="app-name"
id="app-name-undefined"
spellcheck="false"
style="box-sizing: content-box; width: 2px; border: 0px; opacity: 1; outline: 0; padding: 0px;"
tabindex="0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Loading

0 comments on commit 25b442f

Please sign in to comment.