Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion web/packages/teleterm/src/services/tshd/testHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export const makeServer = (props: Partial<tsh.Server> = {}): tsh.Server => ({
export const databaseUri = `${rootClusterUri}/dbs/foo`;
export const kubeUri = `${rootClusterUri}/kubes/foo`;
export const appUri = `${rootClusterUri}/apps/foo`;
export const windowsDesktopUri = `${rootClusterUri}/windowsDesktops/foo`;
export const windowsDesktopUri = `${rootClusterUri}/windows_desktops/foo`;

export const makeDatabase = (
props: Partial<tsh.Database> = {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ function WindowsDesktop() {
return (
<ConnectWindowsDesktopActionButton
windowsDesktop={makeWindowsDesktop({
uri: `${testCluster.uri}/windowsDesktops/bar`,
uri: `${testCluster.uri}/windows_desktops/bar`,
})}
/>
);
Expand Down
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small issue that's unrelated to this PR: whenever I switch to a tab with a desktop session, there's a flicker as the client sends "[TDPClient] requesting screen spec from client 837 x 881", despite the window size staying the same.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happens because of a resize observer. If you hide the tab, its size goes to 0x0 (we ignore that) and after it becomes visible, it reports the size change (837 x 881 in this case). To avoid this, we could store the last reported size and compare it with the new size. While this is a simple fix, I'm not sure if I'll find time for it 😅

Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,28 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import { useMemo, useState } from 'react';
import { useCallback, useMemo, useState } from 'react';

import { Text } from 'design';
import { ACL } from 'gen-proto-ts/teleport/lib/teleterm/v1/cluster_pb';
import { DesktopSession } from 'shared/components/DesktopSession';
import {
Attempt,
makeProcessingAttempt,
makeSuccessAttempt,
} from 'shared/hooks/useAsync';
import { BrowserFileSystem, TdpClient } from 'shared/libs/tdp';
import { BrowserFileSystem, TdpClient, useListener } from 'shared/libs/tdp';
import { TdpTransport } from 'shared/libs/tdp/client';

import Logger from 'teleterm/logger';
import { cloneAbortSignal, TshdClient } from 'teleterm/services/tshd';
import { useAppContext } from 'teleterm/ui/appContextProvider';
import Document from 'teleterm/ui/Document';
import { useWorkspaceContext } from 'teleterm/ui/Documents';
import { useWorkspaceLoggedInUser } from 'teleterm/ui/hooks/useLoggedInUser';
import { useLogger } from 'teleterm/ui/hooks/useLogger';
import * as types from 'teleterm/ui/services/workspacesService';
import { routing, WindowsDesktopUri } from 'teleterm/ui/uri';
import { DesktopUri, isWindowsDesktopUri, routing } from 'teleterm/ui/uri';

// The check for another active session is disabled in Connect:
// 1. This protection was added to the Web UI to prevent a situation where a user could be tricked
Expand All @@ -50,11 +52,12 @@ export function DocumentDesktopSession(props: {
doc: types.DocumentDesktopSession;
}) {
const logger = useLogger('DocumentDesktopSession');
const { desktopUri, login, origin } = props.doc;
const { desktopUri, login, origin, uri } = props.doc;
const appCtx = useAppContext();
const { documentsService } = useWorkspaceContext();
const loggedInUser = useWorkspaceLoggedInUser();
const acl = useMemo<Attempt<ACL>>(() => {
if (!loggedInUser) {
if (!loggedInUser?.acl) {
return makeProcessingAttempt();
}
return makeSuccessAttempt(loggedInUser.acl);
Expand Down Expand Up @@ -83,8 +86,38 @@ export function DocumentDesktopSession(props: {
}, new BrowserFileSystem())
);

return (
<Document visible={props.visible}>
useListener(
client.onTransportOpen,
useCallback(
() => documentsService.update(uri, { status: 'connected' }),
[documentsService, uri]
)
);
useListener(
client.onTransportClose,
useCallback(
error => documentsService.update(uri, { status: error ? 'error' : '' }),
[documentsService, uri]
)
);
useListener(
client.onError,
useCallback(
() => documentsService.update(uri, { status: 'error' }),
[documentsService, uri]
)
);
Comment on lines +96 to +109
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have combined these events while I was refactoring this, but well, I will keep it as is.


let content = (
<Text m="auto" mt={10} textAlign="center">
Cannot open a connection to "{desktopUri}".
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this always trips me up and i think its going to just print that instead of the desktopUri variable

<br />
Only Windows desktops are supported.
</Text>
);

if (isWindowsDesktopUri(desktopUri)) {
content = (
<DesktopSession
hasAnotherSession={noOtherSession}
desktop={
Expand All @@ -94,14 +127,16 @@ export function DocumentDesktopSession(props: {
username={login}
aclAttempt={acl}
/>
</Document>
);
);
}

return <Document visible={props.visible}>{content}</Document>;
}

async function adaptGRPCStreamToTdpTransport(
stream: ReturnType<TshdClient['connectToDesktop']>,
targetDesktop: {
desktopUri: WindowsDesktopUri;
desktopUri: DesktopUri;
login: string;
},
logger: Logger
Expand Down
6 changes: 3 additions & 3 deletions web/packages/teleterm/src/ui/Search/pickers/results.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ const SearchResultItems = () => {
kind: 'windows_desktop',
requiresRequest: false,
resource: makeWindowsDesktopWithoutDefaultPort({
uri: `${clusterUri}/windowsDesktops/long-name`,
uri: `${clusterUri}/windows_desktops/long-name`,
name: 'super-long-windows-desktop-name-with-uuid-7a96e498-88ec-442f-a25b-569fa9150123c',
labels: makeLabelsList({
'aws/Environment': 'demo-13-biz',
Expand All @@ -499,7 +499,7 @@ const SearchResultItems = () => {
makeResourceResult({
kind: 'windows_desktop',
resource: makeWindowsDesktopWithoutDefaultPort({
uri: `${clusterUri}/windowsDesktops/long-label-list`,
uri: `${clusterUri}/windows_desktops/long-label-list`,
name: 'long-label-list',
labels: makeLabelsList({
'aws/Environment': 'demo-13-biz',
Expand All @@ -514,7 +514,7 @@ const SearchResultItems = () => {
kind: 'windows_desktop',
requiresRequest: true,
resource: makeWindowsDesktopWithoutDefaultPort({
uri: `${clusterUri}/windowsDesktops/short-label-list`,
uri: `${clusterUri}/windows_desktops/short-label-list`,
name: 'short-label-list',
labels: makeLabelsList({
'im-just-a-smol': 'win',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,9 @@ function getKindName(connection: ExtendedTrackedConnection): string {
return 'SSH';
case 'connection.kube':
return 'KUBE';
case 'connection.desktop':
return 'DESKTOP';
default:
// The default branch is triggered when the state read from the disk
// contains a connection not supported by the given Connect version.
//
// For example, the user can open an app connection in Connect v15
// and then downgrade to a version that doesn't support apps.
// That connection should be shown as 'UNKNOWN' in the connection list.
return 'UNKNOWN';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,12 @@ import { assertUnreachable } from 'teleterm/ui/utils';
import { ImmutableStore } from '../immutableStore';
import { TrackedConnectionOperationsFactory } from './trackedConnectionOperationsFactory';
import {
createDesktopConnection,
createGatewayConnection,
createGatewayKubeConnection,
createKubeConnection,
createServerConnection,
getDesktopConnectionByDocument,
getGatewayConnectionByDocument,
getGatewayKubeConnectionByDocument,
getKubeConnectionByDocument,
Expand Down Expand Up @@ -76,15 +78,27 @@ export class ConnectionTrackerService extends ImmutableStore<ConnectionTrackerSt
}

getConnections(): ExtendedTrackedConnection[] {
return this.state.connections.map(connection => {
const { rootClusterUri, leafClusterUri } =
this._trackedConnectionOperationsFactory.create(connection);
const clusterUri = leafClusterUri || rootClusterUri;
const clusterName =
this._clusterService.findCluster(clusterUri)?.name ||
routing.parseClusterName(clusterUri);
return { ...connection, clusterName };
});
return this.state.connections
.map(connection => {
const trackedConnection =
this._trackedConnectionOperationsFactory.create(connection);
// A connection is undefined when the state read from the disk
// contains a connection not supported by the given Connect version.
//
// For example, the user can open a desktop connection in Connect v18
// and then downgrade to a version that doesn't support desktops.
// That connection should be shown as 'UNKNOWN' in the connection list.
if (!trackedConnection) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, it turned out that we didn't properly handle unsupported connections in #37584.
It handled only app connections, which were a new gateway type but not a truly new connection kind. Now that desktop connections have been added, downgrading the app results in a crash, as we were trying to destructure undefined in the line 82.
I will backport this fix to all release branches.

return;
}
const { rootClusterUri, leafClusterUri } = trackedConnection;
const clusterUri = leafClusterUri || rootClusterUri;
const clusterName =
this._clusterService.findCluster(clusterUri)?.name ||
routing.parseClusterName(clusterUri);
return { ...connection, clusterName };
})
.filter(Boolean);
}

async activateItem(
Expand Down Expand Up @@ -123,6 +137,10 @@ export class ConnectionTrackerService extends ImmutableStore<ConnectionTrackerSt
return this.state.connections.find(
getGatewayKubeConnectionByDocument(document)
);
case 'doc.desktop_session':
return this.state.connections.find(
getDesktopConnectionByDocument(document)
);
}
}

Expand Down Expand Up @@ -180,6 +198,8 @@ export class ConnectionTrackerService extends ImmutableStore<ConnectionTrackerSt
return s.targetUri === resourceUri;
case 'connection.kube':
return s.kubeUri === resourceUri;
case 'connection.desktop':
return s.desktopUri === resourceUri;
default:
return assertUnreachable(s);
}
Expand Down Expand Up @@ -239,7 +259,8 @@ export class ConnectionTrackerService extends ImmutableStore<ConnectionTrackerSt
d.kind === 'doc.gateway' ||
d.kind === 'doc.gateway_kube' ||
d.kind === 'doc.terminal_tsh_node' ||
d.kind === 'doc.terminal_tsh_kube'
d.kind === 'doc.terminal_tsh_kube' ||
d.kind === 'doc.desktop_session'
);

if (!docs) {
Expand Down Expand Up @@ -328,6 +349,19 @@ export class ConnectionTrackerService extends ImmutableStore<ConnectionTrackerSt
}
break;
}
case 'doc.desktop_session': {
const desktopConn = draft.connections.find(
getDesktopConnectionByDocument(doc)
);

if (desktopConn) {
desktopConn.connected = doc.status === 'connected';
} else {
const newItem = createDesktopConnection(doc);
draft.connections.push(newItem);
}
break;
}
}
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,23 @@

import { ClustersService } from 'teleterm/ui/services/clusters';
import {
createDesktopSessionDocument,
DocumentGateway,
DocumentOrigin,
WorkspacesService,
} from 'teleterm/ui/services/workspacesService';
import { LeafClusterUri, RootClusterUri, routing } from 'teleterm/ui/uri';

import {
getDesktopDocumentByConnection,
getGatewayDocumentByConnection,
getGatewayKubeDocumentByConnection,
getKubeDocumentByConnection,
getServerDocumentByConnection,
} from './trackedConnectionUtils';
import {
TrackedConnection,
TrackedDesktopConnection,
TrackedGatewayConnection,
TrackedKubeConnection,
TrackedServerConnection,
Expand All @@ -51,6 +54,8 @@ export class TrackedConnectionOperationsFactory {
return this.getConnectionGatewayOperations(connection);
case 'connection.kube':
return this.getConnectionGatewayKubeOperations(connection);
case 'connection.desktop':
return this.getConnectionDesktopOperations(connection);
}
}

Expand Down Expand Up @@ -232,6 +237,50 @@ export class TrackedConnectionOperationsFactory {
};
}

private getConnectionDesktopOperations(
connection: TrackedDesktopConnection
): TrackedConnectionOperations {
const { rootClusterId, leafClusterId } = routing.parseClusterUri(
connection.desktopUri
).params;
const { rootClusterUri, leafClusterUri } = this.getClusterUris({
rootClusterId,
leafClusterId,
});

const documentsService =
this._workspacesService.getWorkspaceDocumentService(rootClusterUri);

return {
rootClusterUri,
leafClusterUri,
activate: params => {
let doc = documentsService
.getDocuments()
.find(getDesktopDocumentByConnection(connection));

if (!doc) {
doc = createDesktopSessionDocument({
desktopUri: connection.desktopUri,
login: connection.login,
origin: params.origin,
});
documentsService.add(doc);
}
documentsService.open(doc.uri);
},
disconnect: async () => {
documentsService
.getDocuments()
.filter(getDesktopDocumentByConnection(connection))
.forEach(document => {
documentsService.close(document.uri);
});
},
remove: async () => {},
};
}

private getClusterUris({ rootClusterId, leafClusterId }) {
const rootClusterUri = routing.getClusterUri({
rootClusterId,
Expand Down
Loading
Loading