Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basic support for target port in gateways in Connect #50912

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
4 changes: 4 additions & 0 deletions web/packages/design/src/keyframes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,7 @@ export const blink = keyframes`
opacity: 100%;
}
`;

export const disappear = keyframes`
to { opacity: 0; }
`;
2 changes: 1 addition & 1 deletion web/packages/shared/components/FieldInput/FieldInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ export type FieldInputProps = BoxProps & {
id?: string;
name?: string;
value?: string;
label?: string;
label?: React.ReactNode;
helperText?: React.ReactNode;
icon?: React.ComponentType<IconProps>;
size?: InputSize;
Expand Down
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 @@ -290,7 +290,7 @@ export const makeAppGateway = (
targetUri: appUri,
localAddress: 'localhost',
localPort: '1337',
targetSubresourceName: 'bar',
targetSubresourceName: undefined,
gatewayCliCommand: {
path: '',
preview: 'curl http://localhost:1337',
Expand Down
181 changes: 153 additions & 28 deletions web/packages/teleterm/src/ui/DocumentGatewayApp/AppGateway.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,28 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import { useMemo, useRef } from 'react';
import {
ChangeEvent,
ChangeEventHandler,
PropsWithChildren,
useEffect,
useMemo,
useState,
} from 'react';
import styled from 'styled-components';

import {
Alert,
Box,
ButtonSecondary,
disappear,
Flex,
H1,
Indicator,
Link,
rotate360,
Text,
} from 'design';
import { Check, Spinner } from 'design/Icon';
import { Gateway } from 'gen-proto-ts/teleport/lib/teleterm/v1/gateway_pb';
import { TextSelectCopy } from 'shared/components/TextSelectCopy';
import Validation from 'shared/components/Validation';
Expand All @@ -41,25 +51,41 @@ export function AppGateway(props: {
disconnectAttempt: Attempt<void>;
changeLocalPort(port: string): void;
changeLocalPortAttempt: Attempt<void>;
changeTargetPort(port: string): void;
changeTargetPortAttempt: Attempt<void>;
disconnect(): void;
}) {
const { gateway } = props;
const formRef = useRef<HTMLFormElement>();

const { changeLocalPort } = props;
const handleChangeLocalPort = useMemo(() => {
return debounce((value: string) => {
if (formRef.current.reportValidity()) {
changeLocalPort(value);
}
}, 1000);
}, [changeLocalPort]);
const {
changeLocalPort,
changeLocalPortAttempt,
changeTargetPort,
changeTargetPortAttempt,
disconnectAttempt,
} = props;
// It must be possible to update local port while target port is invalid, hence why
// useDebouncedPortChangeHandler checks the validity of only one input at a time. Otherwise the UI
// would lose updates to the local port while the target port was invalid.
const handleLocalPortChange = useDebouncedPortChangeHandler(changeLocalPort);
const handleTargetPortChange =
useDebouncedPortChangeHandler(changeTargetPort);

let address = `${gateway.localAddress}:${gateway.localPort}`;
if (gateway.protocol === 'HTTP') {
address = `http://${address}`;
}

// AppGateway doesn't have access to the app resource itself, so it has to decide whether the
// app is multi-port or not in some other way.
// For multi-port apps, DocumentGateway comes with targetSubresourceName prefilled to the first
// port number found in TCP ports. Single-port apps have this field empty.
// So, if targetSubresourceName is present, then the app must be multi-port. In this case, the
// user is free to change it and can never provide an empty targetSubresourceName.
// When the app is not multi-port, targetSubresourceName is empty and the user cannot change it.
const isMultiPort =
gateway.protocol === 'TCP' && gateway.targetSubresourceName;

return (
<Box maxWidth="680px" width="100%" mx="auto" mt="4" px="5">
<Flex justifyContent="space-between" mb="3" flexWrap="wrap" gap={2}>
Expand All @@ -69,38 +95,54 @@ export function AppGateway(props: {
</ButtonSecondary>
</Flex>

{props.disconnectAttempt.status === 'error' && (
<Alert details={props.disconnectAttempt.statusText}>
{disconnectAttempt.status === 'error' && (
<Alert details={disconnectAttempt.statusText}>
Could not close the connection
</Alert>
)}

<Flex as="form" ref={formRef} gap={2}>
<Flex as="form" gap={2}>
<Validation>
<PortFieldInput
label="Port"
label={
<LabelWithAttemptStatus
text="Local Port"
attempt={changeLocalPortAttempt}
/>
}
defaultValue={gateway.localPort}
onChange={e => handleChangeLocalPort(e.target.value)}
onChange={handleLocalPortChange}
mb={2}
/>
{isMultiPort && (
<PortFieldInput
label={
<LabelWithAttemptStatus
text="Target Port"
attempt={changeTargetPortAttempt}
/>
}
required
defaultValue={gateway.targetSubresourceName}
onChange={handleTargetPortChange}
mb={2}
/>
)}
</Validation>
{props.changeLocalPortAttempt.status === 'processing' && (
<Indicator
size="large"
pt={3} // aligns the spinner to be at the center of the port input
css={`
display: flex;
`}
/>
)}
</Flex>

<Text>Access the app at:</Text>
<TextSelectCopy my={1} text={address} bash={false} />

{props.changeLocalPortAttempt.status === 'error' && (
<Alert details={props.changeLocalPortAttempt.statusText}>
Could not change the port number
{changeLocalPortAttempt.status === 'error' && (
<Alert details={changeLocalPortAttempt.statusText}>
Could not change the local port
</Alert>
)}

{changeTargetPortAttempt.status === 'error' && (
<Alert details={changeTargetPortAttempt.statusText}>
Could not change the target port
</Alert>
)}

Expand All @@ -118,3 +160,86 @@ export function AppGateway(props: {
</Box>
);
}

const LabelWithAttemptStatus = (props: {
text: string;
attempt: Attempt<unknown>;
}) => (
<Flex
alignItems="center"
justifyContent="space-between"
css={`
position: relative;
`}
>
{props.text}
{props.attempt.status === 'processing' && (
<AnimatedSpinner color="interactive.tonal.neutral.2" size="medium" />
)}
{props.attempt.status === 'success' && (
// CSS animations are repeated whenever the parent goes from `display: none` to something
// else. As a result, we need to unmount the animated check so that the animation is not
// repeated when the user switches to this tab.
// https://www.w3.org/TR/css-animations-1/#example-4e34d7ba
<UnmountAfter timeoutMs={disappearanceDelayMs + disappearanceDurationMs}>
<DisappearingCheck color="success.main" size="medium" />
</UnmountAfter>
)}
</Flex>
);

/**
* useDebouncedPortChangeHandler returns a debounced change handler that calls the change function
* only if the input from which the event originated is valid.
*/
const useDebouncedPortChangeHandler = (
changeFunc: (port: string) => void
): ChangeEventHandler<HTMLInputElement> =>
useMemo(
() =>
debounce((event: ChangeEvent<HTMLInputElement>) => {
if (event.target.reportValidity()) {
changeFunc(event.target.value);
}
}, 1000),
[changeFunc]
);

const AnimatedSpinner = styled(Spinner)`
animation: ${rotate360} 1.5s infinite linear;
// The spinner needs to be positioned absolutely so that the fact that it's spinning
// doesn't affect the size of the parent.
position: absolute;
right: 0;
top: 0;
`;

const disappearanceDelayMs = 1000;
const disappearanceDurationMs = 200;

const DisappearingCheck = styled(Check)`
opacity: 1;
animation: ${disappear};
animation-delay: ${disappearanceDelayMs}ms;
animation-duration: ${disappearanceDurationMs}ms;
animation-fill-mode: forwards;
`;

const UnmountAfter = ({
timeoutMs,
children,
}: PropsWithChildren<{ timeoutMs: number }>) => {
const [isMounted, setIsMounted] = useState(true);

useEffect(() => {
const timeout = setTimeout(() => {
setIsMounted(false);
}, timeoutMs);

return () => {
clearTimeout(timeout);
};
}, [timeoutMs]);

return isMounted ? children : null;
};
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ import { MockWorkspaceContextProvider } from 'teleterm/ui/fixtures/MockWorkspace
import * as types from 'teleterm/ui/services/workspacesService';

type StoryProps = {
appType: 'web' | 'tcp';
appType: 'web' | 'tcp' | 'tcp-multi-port';
online: boolean;
changeLocalPort: 'succeed' | 'throw-error';
changeTargetPort: 'succeed' | 'throw-error';
disconnect: 'succeed' | 'throw-error';
};

Expand All @@ -42,13 +43,18 @@ const meta: Meta<StoryProps> = {
argTypes: {
appType: {
control: { type: 'radio' },
options: ['web', 'tcp'],
options: ['web', 'tcp', 'tcp-multi-port'],
},
changeLocalPort: {
if: { arg: 'online' },
control: { type: 'radio' },
options: ['succeed', 'throw-error'],
},
changeTargetPort: {
if: { arg: 'online' },
control: { type: 'radio' },
options: ['succeed', 'throw-error'],
},
disconnect: {
if: { arg: 'online' },
control: { type: 'radio' },
Expand All @@ -59,6 +65,7 @@ const meta: Meta<StoryProps> = {
appType: 'web',
online: true,
changeLocalPort: 'succeed',
changeTargetPort: 'succeed',
disconnect: 'succeed',
},
};
Expand All @@ -70,6 +77,10 @@ export function Story(props: StoryProps) {
if (props.appType === 'tcp') {
gateway.protocol = 'TCP';
}
if (props.appType === 'tcp-multi-port') {
gateway.protocol = 'TCP';
gateway.targetSubresourceName = '4242';
}
const documentGateway: types.DocumentGateway = {
kind: 'doc.gateway',
targetUri: '/clusters/bar/apps/quux',
Expand All @@ -85,6 +96,9 @@ export function Story(props: StoryProps) {
if (!props.online) {
documentGateway.gatewayUri = undefined;
}
if (props.appType === 'tcp-multi-port') {
documentGateway.targetSubresourceName = '4242';
}

const appContext = new MockAppContext();
appContext.workspacesService.setState(draftState => {
Expand All @@ -106,12 +120,30 @@ export function Story(props: StoryProps) {
wait(1000).then(
() =>
new MockedUnaryCall(
{ ...gateway, localPort },
{
...appContext.clustersService.findGateway(gateway.uri),
localPort,
},
props.changeLocalPort === 'throw-error'
? new Error('something went wrong')
: undefined
)
);
appContext.tshd.setGatewayTargetSubresourceName = ({
targetSubresourceName,
}) =>
wait(1000).then(
() =>
new MockedUnaryCall(
{
...appContext.clustersService.findGateway(gateway.uri),
targetSubresourceName,
},
props.changeTargetPort === 'throw-error'
? new Error('something went wrong')
: undefined
)
);
appContext.tshd.removeGateway = () =>
wait(50).then(
() =>
Expand Down
Copy link
Member Author

Choose a reason for hiding this comment

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

It's possible for the OfflineGateway state to "soft lock". If you create a doc for target port 1337, then close the app, then remove that port from TCP ports of the app, then reopen the app, creating the gateway will fail. You won't be able to change the target port until you remove the gateway from the connections list.

This is also something I'm going to address in another PR. But this whole thing definitely requires too much work for how niche it's going to be. 😭

Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ export function DocumentGatewayApp(props: {
disconnect,
disconnectAttempt,
reconnect,
changeTargetSubresourceName: changeTargetPort,
changeTargetSubresourceNameAttempt: changeTargetPortAttempt,
} = useGateway(doc);

return (
Expand All @@ -47,6 +49,7 @@ export function DocumentGatewayApp(props: {
targetName={doc.targetName}
gatewayPort={{ isSupported: true, defaultPort: doc.port }}
reconnect={reconnect}
portFieldLabel="Local Port (optional)"
/>
) : (
<AppGateway
Expand All @@ -55,6 +58,8 @@ export function DocumentGatewayApp(props: {
disconnectAttempt={disconnectAttempt}
changeLocalPort={changeLocalPort}
changeLocalPortAttempt={changeLocalPortAttempt}
changeTargetPort={changeTargetPort}
changeTargetPortAttempt={changeTargetPortAttempt}
/>
)}
</Document>
Expand Down
5 changes: 4 additions & 1 deletion web/packages/teleterm/src/ui/components/FieldInputs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,8 @@ export const PortFieldInput = styled(ConfigFieldInput).attrs({
type: 'number',
min: 1,
max: 65535,
width: '110px',
// Without a min width, the stepper controls end up being to close to a long port number such
// as 65535. minWidth instead of width allows the field to grow with the label, so that e.g.
// a custom label of "Local Port (optional)" is displayed on a single line.
minWidth: '110px',
})``;
Loading