Skip to content

Commit

Permalink
[v15] Fetch resources using ListUnifiedResources in the search bar (#…
Browse files Browse the repository at this point in the history
…41400)

* Get rid of `clusters.App` type

We shouldn't have added it, `addrWithProtocol` is calculated from other `tsh.App` properties, so we can just do it when needed.
This type prevents assigning `listUnifiedResources` response to the type returned from `searchResources`.
Also, this change will make setting up mocks and tests simpler.

* Remove resource kind from `ResourceSearchError`

We will fetch all resources for a cluster in a single request, so it is no longer needed.

* Replace separate calls for resources with `listUnifiedResources`

* In preview mode, fetch 20 items instead of 5

Previously we fetched 5 items x 4 resource kinds.

* Move `getAppAddrWithProtocol` to `services/tshd/app.ts`

* Extract `MAX_RANKED_RESULTS`
  • Loading branch information
gzdunek authored May 10, 2024
1 parent 461a9ca commit 30df269
Show file tree
Hide file tree
Showing 14 changed files with 141 additions and 296 deletions.
25 changes: 25 additions & 0 deletions web/packages/teleterm/src/services/tshd/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,28 @@ export function isWebApp(app: App): boolean {
app.endpointUri.startsWith('https://')
);
}

/**
* Returns address with protocol which is an app protocol + a public address.
* If the public address is empty, it falls back to the endpoint URI.
*
* Always empty for SAML applications.
*/
export function getAppAddrWithProtocol(source: App): string {
const { publicAddr, endpointUri } = source;

const isTcp = endpointUri && endpointUri.startsWith('tcp://');
const isCloud = endpointUri && endpointUri.startsWith('cloud://');
let addrWithProtocol = endpointUri;
if (publicAddr) {
if (isCloud) {
addrWithProtocol = `cloud://${publicAddr}`;
} else if (isTcp) {
addrWithProtocol = `tcp://${publicAddr}`;
} else {
addrWithProtocol = `https://${publicAddr}`;
}
}

return addrWithProtocol;
}
5 changes: 1 addition & 4 deletions web/packages/teleterm/src/services/tshd/testHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
import * as tsh from './types';
import { TshdRpcError } from './cloneableClient';

import type { App } from 'teleterm/ui/services/clusters';

export const rootClusterUri = '/clusters/teleport-local';
export const leafClusterUri = `${rootClusterUri}/leaves/leaf`;

Expand Down Expand Up @@ -60,7 +58,7 @@ export const makeKube = (props: Partial<tsh.Kube> = {}): tsh.Kube => ({
...props,
});

export const makeApp = (props: Partial<App> = {}): App => ({
export const makeApp = (props: Partial<tsh.App> = {}): tsh.App => ({
name: 'foo',
labels: [],
endpointUri: 'tcp://localhost:3000',
Expand All @@ -71,7 +69,6 @@ export const makeApp = (props: Partial<App> = {}): App => ({
fqdn: 'local-app.example.com:3000',
samlApp: false,
uri: appUri,
addrWithProtocol: 'tcp://local-app.example.com:3000',
awsRoles: [],
...props,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
makeDatabase,
makeServer,
makeKube,
makeApp,
} from 'teleterm/ui/services/clusters';
import { retryWithRelogin } from 'teleterm/ui/utils';

Expand All @@ -39,6 +38,7 @@ import {
} from 'teleterm/services/tshd/types';
import { routing } from 'teleterm/ui/uri';
import { useWorkspaceLoggedInUser } from 'teleterm/ui/hooks/useLoggedInUser';
import { getAppAddrWithProtocol } from 'teleterm/services/tshd/app';

import type {
ResourceLabel,
Expand Down Expand Up @@ -96,7 +96,9 @@ export default function useNewRequest() {
teleportApps.App,
'name' | 'labels' | 'description' | 'userGroups' | 'addrWithProtocol'
> = {
...makeApp(source),
name: tshdApp.name,
labels: tshdApp.labels,
addrWithProtocol: getAppAddrWithProtocol(source),
description: tshdApp.desc,
//TODO(gzdunek): Enable requesting apps via user groups in Connect.
// To make this work, we need
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ import {
DocumentCluster,
DocumentClusterResourceKind,
} from 'teleterm/ui/services/workspacesService';
import { makeApp } from 'teleterm/ui/services/clusters';
import { getAppAddrWithProtocol } from 'teleterm/services/tshd/app';

import {
ConnectServerActionButton,
Expand Down Expand Up @@ -346,15 +346,15 @@ const mapToSharedResource = (
};
}
case 'app': {
const app = makeApp(resource.resource);
const { resource: app } = resource;

return {
resource: {
kind: 'app' as const,
labels: app.labels,
name: app.name,
id: app.name,
addrWithProtocol: app.addrWithProtocol,
addrWithProtocol: getAppAddrWithProtocol(app),
awsConsole: app.awsConsole,
description: app.desc,
friendlyName: app.friendlyName,
Expand Down
24 changes: 0 additions & 24 deletions web/packages/teleterm/src/ui/Search/ResourceSearchErrors.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,46 +34,22 @@ export const Story = () => (
errors={[
new ResourceSearchError(
'/clusters/foo',
'server',
new Error(
'14 UNAVAILABLE: connection error: desc = "transport: authentication handshake failed: EOF"'
)
),
new ResourceSearchError(
'/clusters/bar',
'database',
new Error(
'2 UNKNOWN: Unable to connect to ssh proxy at teleport.local:443. Confirm connectivity and availability.\n dial tcp: lookup teleport.local: no such host'
)
),
new ResourceSearchError(
'/clusters/baz',
'kube',
new Error(
'14 UNAVAILABLE: connection error: desc = "transport: authentication handshake failed: EOF"'
)
),
new ResourceSearchError(
'/clusters/foo',
'server',
new Error(
'2 UNKNOWN: Unable to connect to ssh proxy at teleport.local:443. Confirm connectivity and availability.\n dial tcp: lookup teleport.local: no such host'
)
),
new ResourceSearchError(
'/clusters/baz',
'kube',
new Error(
'14 UNAVAILABLE: connection error: desc = "transport: authentication handshake failed: EOF"'
)
),
new ResourceSearchError(
'/clusters/foo',
'server',
new Error(
'2 UNKNOWN: Unable to connect to ssh proxy at teleport.local:443. Confirm connectivity and availability.\n dial tcp: lookup teleport.local: no such host'
)
),
]}
/>
);
5 changes: 1 addition & 4 deletions web/packages/teleterm/src/ui/Search/SearchBar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ it('notifies about resource search errors and allows to display details', () =>

const resourceSearchError = new ResourceSearchError(
'/clusters/foo',
'server',
new Error('whoops')
);

Expand Down Expand Up @@ -206,7 +205,7 @@ it('notifies about resource search errors and allows to display details', () =>
expect(results).toHaveTextContent(
'Some of the search results are incomplete.'
);
expect(results).toHaveTextContent('Could not fetch servers from foo');
expect(results).toHaveTextContent('Could not fetch resources from foo');
expect(results).not.toHaveTextContent(resourceSearchError.cause['message']);

act(() => screen.getByText('Show details').click());
Expand All @@ -226,7 +225,6 @@ it('maintains focus on the search input after closing a resource search error mo

const resourceSearchError = new ResourceSearchError(
'/clusters/foo',
'server',
new Error('whoops')
);

Expand Down Expand Up @@ -282,7 +280,6 @@ it('shows a login modal when a request to a cluster from the current workspace f
const cluster = makeRootCluster();
const resourceSearchError = new ResourceSearchError(
cluster.uri,
'server',
makeRetryableError()
);
const resourceSearchResult = {
Expand Down
23 changes: 3 additions & 20 deletions web/packages/teleterm/src/ui/Search/pickers/ActionPicker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,11 @@ describe('getActionPickerStatus', () => {
it('partitions resource search errors into clusters with expired certs and non-retryable errors', () => {
const retryableError = new ResourceSearchError(
'/clusters/foo',
'server',
makeRetryableError()
);

const nonRetryableError = new ResourceSearchError(
'/clusters/bar',
'database',
new Error('whoops')
);

Expand Down Expand Up @@ -68,7 +66,6 @@ describe('getActionPickerStatus', () => {
const offlineCluster = makeRootCluster({ connected: false });
const retryableError = new ResourceSearchError(
'/clusters/foo',
'server',
makeRetryableError()
);

Expand All @@ -95,17 +92,8 @@ describe('getActionPickerStatus', () => {

it('includes a cluster with expired cert only once even if multiple requests fail with retryable errors', () => {
const retryableErrors = [
new ResourceSearchError(
'/clusters/foo',
'server',
makeRetryableError()
),
new ResourceSearchError(
'/clusters/foo',
'database',
makeRetryableError()
),
new ResourceSearchError('/clusters/foo', 'kube', makeRetryableError()),
new ResourceSearchError('/clusters/foo', makeRetryableError()),
new ResourceSearchError('/clusters/foo', makeRetryableError()),
];
const status = getActionPickerStatus({
inputValue: 'foo',
Expand Down Expand Up @@ -186,15 +174,10 @@ describe('getActionPickerStatus', () => {
it('returns non-retryable errors when fetching a preview after selecting a filter fails', () => {
const nonRetryableError = new ResourceSearchError(
'/clusters/bar',
'server',
new Error('non-retryable error')
);
const resourceSearchErrors = [
new ResourceSearchError(
'/clusters/foo',
'server',
makeRetryableError()
),
new ResourceSearchError('/clusters/foo', makeRetryableError()),
nonRetryableError,
];
const status = getActionPickerStatus({
Expand Down
3 changes: 1 addition & 2 deletions web/packages/teleterm/src/ui/Search/pickers/ActionPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ import { ResourceSearchError } from 'teleterm/ui/services/resources';
import { isRetryable } from 'teleterm/ui/utils/retryWithRelogin';
import { assertUnreachable } from 'teleterm/ui/utils';
import { isWebApp } from 'teleterm/services/tshd/app';
import { App } from 'teleterm/ui/services/clusters';

import { SearchAction } from '../actions';
import { useSearchContext } from '../SearchContext';
Expand Down Expand Up @@ -790,7 +789,7 @@ export function AppItem(props: SearchResultItem<SearchResultApp>) {
);
}

function getAppItemCopy($appName: React.JSX.Element, app: App) {
function getAppItemCopy($appName: React.JSX.Element, app: tsh.App) {
if (app.samlApp) {
return <>Log in to {$appName} in the browser</>;
}
Expand Down
27 changes: 15 additions & 12 deletions web/packages/teleterm/src/ui/Search/pickers/results.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import { makeSuccessAttempt } from 'shared/hooks/useAsync';

import { Flex } from 'design';

import { App } from 'gen-proto-ts/teleport/lib/teleterm/v1/app_pb';

import { routing } from 'teleterm/ui/uri';
import {
makeDatabase,
Expand All @@ -31,6 +33,7 @@ import {
makeApp,
} from 'teleterm/services/tshd/testHelpers';
import { ResourceSearchError } from 'teleterm/ui/services/resources';
import { getAppAddrWithProtocol } from 'teleterm/services/tshd/app';

import { SearchResult } from '../searchResult';
import { makeResourceResult } from '../testHelpers';
Expand Down Expand Up @@ -102,6 +105,11 @@ export const ResultsNarrow = () => {
return <Results maxWidth="300px" />;
};

function makeAppWithAddr(props: Partial<App>) {
const app = makeApp(props);
return { ...app, addrWithProtocol: getAppAddrWithProtocol(app) };
}

const SearchResultItems = () => {
const searchResults: SearchResult[] = [
makeResourceResult({
Expand Down Expand Up @@ -168,11 +176,10 @@ const SearchResultItems = () => {
}),
makeResourceResult({
kind: 'app',
resource: makeApp({
resource: makeAppWithAddr({
uri: `${clusterUri}/apps/web-app`,
name: 'web-app',
endpointUri: 'http://localhost:3000',
addrWithProtocol: 'http://local-app.example.com:3000',
desc: '',
labels: makeLabelsList({
access: 'cloudwatch-metrics,ec2,s3,cloudtrail',
Expand All @@ -185,11 +192,10 @@ const SearchResultItems = () => {
}),
makeResourceResult({
kind: 'app',
resource: makeApp({
resource: makeAppWithAddr({
uri: `${clusterUri}/apps/saml-app`,
name: 'saml-app',
endpointUri: '',
addrWithProtocol: '',
samlApp: true,
desc: 'SAML Application',
labels: makeLabelsList({
Expand All @@ -203,7 +209,7 @@ const SearchResultItems = () => {
}),
makeResourceResult({
kind: 'app',
resource: makeApp({
resource: makeAppWithAddr({
uri: `${clusterUri}/apps/no-desc`,
name: 'no-desc',
desc: '',
Expand All @@ -218,7 +224,7 @@ const SearchResultItems = () => {
}),
makeResourceResult({
kind: 'app',
resource: makeApp({
resource: makeAppWithAddr({
uri: `${clusterUri}/apps/short-desc`,
name: 'short-desc',
desc: 'Lorem ipsum',
Expand All @@ -233,7 +239,7 @@ const SearchResultItems = () => {
}),
makeResourceResult({
kind: 'app',
resource: makeApp({
resource: makeAppWithAddr({
uri: `${clusterUri}/apps/long-desc`,
name: 'long-desc',
desc: 'Eget dignissim lectus nisi vitae nunc',
Expand All @@ -248,7 +254,7 @@ const SearchResultItems = () => {
}),
makeResourceResult({
kind: 'app',
resource: makeApp({
resource: makeAppWithAddr({
uri: `${clusterUri}/apps/super-long-desc`,
name: 'super-long-desc',
desc: 'Duis id tortor at purus tincidunt finibus. Mauris eu semper orci, non commodo lacus. Praesent sollicitudin magna id laoreet porta. Nunc lobortis varius sem vel fringilla.',
Expand All @@ -263,7 +269,7 @@ const SearchResultItems = () => {
}),
makeResourceResult({
kind: 'app',
resource: makeApp({
resource: makeAppWithAddr({
name: 'super-long-app-with-uuid-1f96e498-88ec-442f-a25b-569fa915041c',
desc: 'short-desc',
uri: `${longClusterUri}/apps/super-long-desc`,
Expand Down Expand Up @@ -528,7 +534,6 @@ const AuxiliaryItems = () => {
errors={[
new ResourceSearchError(
'/clusters/foo',
'server',
new Error(
'14 UNAVAILABLE: connection error: desc = "transport: authentication handshake failed: EOF"'
)
Expand All @@ -542,14 +547,12 @@ const AuxiliaryItems = () => {
errors={[
new ResourceSearchError(
'/clusters/bar',
'database',
new Error(
'2 UNKNOWN: Unable to connect to ssh proxy at teleport.local:443. Confirm connectivity and availability.\n dial tcp: lookup teleport.local: no such host'
)
),
new ResourceSearchError(
'/clusters/foo',
'server',
new Error(
'14 UNAVAILABLE: connection error: desc = "transport: authentication handshake failed: EOF"'
)
Expand Down
Loading

0 comments on commit 30df269

Please sign in to comment.