Skip to content

Add stubbed UI & feature flag for VNet#39963

Merged
ravicious merged 8 commits intomasterfrom
r7s/vnet-stubs-2-ui
Apr 2, 2024
Merged

Add stubbed UI & feature flag for VNet#39963
ravicious merged 8 commits intomasterfrom
r7s/vnet-stubs-2-ui

Conversation

@ravicious
Copy link
Copy Markdown
Member

@ravicious ravicious commented Mar 28, 2024

This is an improved version of what I got so far on the vnet-demo branch, added behind a feature flag so that it doesn't rot on a separate feature branch.

Check out the UX section of the RFD for how it's supposed to work. In short, when VNet is active, you can just send requests to your-tcp-app.cluster.example.com and they'll be automatically authenticated. As this is mostly a "backend" thing, the UI boils down to just an extra option next to the "Connect" button for TCP apps and VNet panel in connections. After some recent discussions we decided to support only TCP apps in the initial version.

At the moment the UI uses a stubbed version of the gRPC service from #39826, so it doesn't have any effect on the system. Once Nic moves the rest of the code from vnet-demo into master, I'm going to set it all up to actually start/stop VNet.

There's a bunch of TODOs remaining, some of them left in the code. I'm going to address them in the future. For example, the VNet item in the connection list is not yet searchable and the VNet panel should expand rather than slide.

Important

To see the new UI, you need to set feature.vnet to true in your app config.

VNet in connections

@ravicious ravicious added the no-changelog Indicates that a PR does not require a changelog entry label Mar 28, 2024
@github-actions github-actions Bot requested review from gzdunek and kimlisa March 28, 2024 15:43
@ravicious ravicious force-pushed the r7s/vnet-stubs-2-ui branch from 09af61a to b4a61c0 Compare March 28, 2024 15:44
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The best way to review b4a61c0 is to start in this file and then go deeper.

flexDirection="column"
css={`
&:empty {
display: none;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is so that when there's no content within this Flex, the extra padding set on line 42 isn't applied and we don't have to manually add p={textSpacing} to each Text within this Flex.

Comment thread web/packages/teleterm/src/ui/Vnet/VnetConnectionItem.tsx
Comment thread web/packages/teleterm/src/ui/Vnet/VnetConnectionItem.tsx
Comment thread web/packages/teleterm/src/ui/Vnet/VnetConnectionItem.tsx Outdated
Comment thread web/packages/teleterm/src/ui/Vnet/VnetConnectionItem.tsx Outdated
Comment thread web/packages/teleterm/src/ui/Vnet/VnetConnectionItem.tsx
Comment thread web/packages/teleterm/src/ui/Vnet/VnetConnectionItem.tsx
Comment thread web/packages/teleterm/src/ui/Vnet/VnetConnectionItem.tsx Outdated
Comment thread web/packages/teleterm/src/ui/Vnet/VnetConnectionItem.tsx
@ravicious ravicious force-pushed the r7s/vnet-stubs-1-grpc branch from 4f000fc to 0b1d15e Compare March 29, 2024 12:21
Base automatically changed from r7s/vnet-stubs-1-grpc to master March 29, 2024 13:51
@ravicious ravicious force-pushed the r7s/vnet-stubs-2-ui branch from b4a61c0 to 9c7b727 Compare March 29, 2024 13:53
@ravicious ravicious requested a review from gzdunek March 29, 2024 15:56
Comment on lines +51 to +60
{startAttempt.status === 'error' && (
<Text>Could not start VNet: {startAttempt.statusText}</Text>
)}
{stopAttempt.status === 'error' && (
<Text>Could not stop VNet: {stopAttempt.statusText}</Text>
)}

{status === 'stopped' && (
<Text>VNet automatically authenticates connections to TCP apps.</Text>
)}
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.

out of curiosity, if none of these are true, then there is a empty styled div in the dom. is that generally okay to do?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, that's generally fine. The only issue is that Text inside VnetSliderStep requires more padding than AppConnectionItem, so that's why this Flex has p={textSpacing}.

When it's empty, it still adds that padding, which I counteract with &:empty { display: none; }. IMHO it's better and more self-contained than adding three different conditions before rendering this Flex or adding p={textSpacing} to each Text.

Comment thread web/packages/teleterm/src/ui/Vnet/VnetSliderStep.tsx
@ravicious ravicious enabled auto-merge April 2, 2024 09:47
@ravicious ravicious added this pull request to the merge queue Apr 2, 2024
Merged via the queue into master with commit 363c9ce Apr 2, 2024
@ravicious ravicious deleted the r7s/vnet-stubs-2-ui branch April 2, 2024 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that a PR does not require a changelog entry size/lg ui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants