Skip to content

chore: touch ups to wsproxy UX #8350

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

Merged
merged 15 commits into from
Jul 7, 2023
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
16 changes: 1 addition & 15 deletions enterprise/cli/proxyserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (*RootCmd) proxyServer() *clibase.Cmd {
),
Handler: func(inv *clibase.Invocation) error {
if !(primaryAccessURL.Scheme == "http" || primaryAccessURL.Scheme == "https") {
return xerrors.Errorf("primary access URL must be http or https: url=%s", primaryAccessURL.String())
return xerrors.Errorf("'--primary-access-url' value must be http or https: url=%s", primaryAccessURL.String())
}

var closers closers
Expand Down Expand Up @@ -175,20 +175,6 @@ func (*RootCmd) proxyServer() *clibase.Cmd {
defer httpClient.CloseIdleConnections()
closers.Add(httpClient.CloseIdleConnections)

// Warn the user if the access URL appears to be a loopback address.
isLocal, err := cli.IsLocalURL(ctx, cfg.AccessURL.Value())
if isLocal || err != nil {
reason := "could not be resolved"
if isLocal {
reason = "isn't externally reachable"
}
cliui.Warnf(
inv.Stderr,
"The access URL %s %s, this may cause unexpected problems when creating workspaces. Generate a unique *.try.coder.app URL by not specifying an access URL.\n",
cliui.DefaultStyles.Field.Render(cfg.AccessURL.String()), reason,
)
}

// A newline is added before for visibility in terminal output.
cliui.Infof(inv.Stdout, "\nView the Web UI: %s", cfg.AccessURL.String())

Expand Down
79 changes: 54 additions & 25 deletions enterprise/cli/workspaceproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ func (r *RootCmd) workspaceProxy() *clibase.Cmd {
Use: "workspace-proxy",
Short: "Workspace proxies provide low-latency experiences for geo-distributed teams.",
Long: "Workspace proxies provide low-latency experiences for geo-distributed teams. " +
"It will act as a connection gateway to your workspace providing a lower latency solution " +
"to connecting to your workspace if Coder and your workspace are deployed in different regions.",
"It will act as a connection gateway to your workspace. " +
"Best used if Coder and your workspace are deployed in different regions.",
Aliases: []string{"wsproxy"},
Hidden: true,
Handler: func(inv *clibase.Invocation) error {
Expand Down Expand Up @@ -51,6 +51,7 @@ func (r *RootCmd) regenerateProxyToken() *clibase.Cmd {
),
Handler: func(inv *clibase.Invocation) error {
ctx := inv.Context()
formatter.primaryAccessURL = client.URL.String()
// This is cheeky, but you can also use a uuid string in
// 'DeleteWorkspaceProxyByName' and it will work.
proxy, err := client.WorkspaceProxyByName(ctx, inv.Args[0])
Expand Down Expand Up @@ -120,6 +121,7 @@ func (r *RootCmd) patchProxy() *clibase.Cmd {
Handler: func(inv *clibase.Invocation) error {
ctx := inv.Context()
if proxyIcon == "" && displayName == "" && proxyName == "" {
_ = inv.Command.HelpHandler(inv)
return xerrors.Errorf("specify at least one field to update")
}

Expand Down Expand Up @@ -187,13 +189,32 @@ func (r *RootCmd) deleteProxy() *clibase.Cmd {
cmd := &clibase.Cmd{
Use: "delete <name|id>",
Short: "Delete a workspace proxy",
Options: clibase.OptionSet{
cliui.SkipPromptOption(),
},
Middleware: clibase.Chain(
clibase.RequireNArgs(1),
r.InitClient(client),
),
Handler: func(inv *clibase.Invocation) error {
ctx := inv.Context()
err := client.DeleteWorkspaceProxyByName(ctx, inv.Args[0])

wsproxy, err := client.WorkspaceProxyByName(ctx, inv.Args[0])
if err != nil {
return xerrors.Errorf("fetch workspace proxy %q: %w", inv.Args[0], err)
}

// Confirm deletion of the template.
_, err = cliui.Prompt(inv, cliui.PromptOptions{
Text: fmt.Sprintf("Delete this workspace proxy: %s?", cliui.DefaultStyles.Code.Render(wsproxy.DisplayName)),
IsConfirm: true,
Default: cliui.ConfirmNo,
})
if err != nil {
return err
}

err = client.DeleteWorkspaceProxyByName(ctx, inv.Args[0])
if err != nil {
return xerrors.Errorf("delete workspace proxy %q: %w", inv.Args[0], err)
}
Expand Down Expand Up @@ -225,6 +246,7 @@ func (r *RootCmd) createProxy() *clibase.Cmd {
),
Handler: func(inv *clibase.Invocation) error {
ctx := inv.Context()
formatter.primaryAccessURL = client.URL.String()
var err error
if proxyName == "" && !noPrompts {
proxyName, err = cliui.Prompt(inv, cliui.PromptOptions{
Expand Down Expand Up @@ -367,8 +389,9 @@ func (r *RootCmd) listProxies() *clibase.Cmd {

// updateProxyResponseFormatter is used for both create and regenerate proxy commands.
type updateProxyResponseFormatter struct {
onlyToken bool
formatter *cliui.OutputFormatter
onlyToken bool
formatter *cliui.OutputFormatter
primaryAccessURL string
}

func (f *updateProxyResponseFormatter) Format(ctx context.Context, data codersdk.UpdateWorkspaceProxyResponse) (string, error) {
Expand All @@ -392,31 +415,37 @@ func (f *updateProxyResponseFormatter) AttachOptions(opts *clibase.OptionSet) {
func newUpdateProxyResponseFormatter() *updateProxyResponseFormatter {
up := &updateProxyResponseFormatter{
onlyToken: false,
formatter: cliui.NewOutputFormatter(
// Text formatter should be human readable.
cliui.ChangeFormatterData(cliui.TextFormat(), func(data any) (any, error) {
}
up.formatter = cliui.NewOutputFormatter(
// Text formatter should be human readable.
cliui.ChangeFormatterData(cliui.TextFormat(), func(data any) (any, error) {
response, ok := data.(codersdk.UpdateWorkspaceProxyResponse)
if !ok {
return nil, xerrors.Errorf("unexpected type %T", data)
}

return fmt.Sprintf("Workspace Proxy %[1]q updated successfully.\n"+
cliui.DefaultStyles.Placeholder.Render("—————————————————————————————————————————————————")+"\n"+
"Save this authentication token, it will not be shown again.\n"+
"Token: %[2]s\n"+
"\n"+
"Start the proxy by running:\n"+
cliui.DefaultStyles.Code.Render("CODER_PROXY_SESSION_TOKEN=%[2]s coder wsproxy server --primary-access-url %[3]s --http-address=0.0.0.0:3001")+
// This is required to turn off the code style. Otherwise it appears in the code block until the end of the line.
cliui.DefaultStyles.Placeholder.Render(""),
response.Proxy.Name, response.ProxyToken, up.primaryAccessURL), nil
}),
cliui.JSONFormat(),
// Table formatter expects a slice, make a slice of one.
cliui.ChangeFormatterData(cliui.TableFormat([]codersdk.UpdateWorkspaceProxyResponse{}, []string{"proxy name", "proxy url", "proxy token"}),
func(data any) (any, error) {
response, ok := data.(codersdk.UpdateWorkspaceProxyResponse)
if !ok {
return nil, xerrors.Errorf("unexpected type %T", data)
}

return fmt.Sprintf("Workspace Proxy %q updated successfully.\n"+
cliui.DefaultStyles.Placeholder.Render("—————————————————————————————————————————————————")+"\n"+
"Save this authentication token, it will not be shown again.\n"+
"Token: %s\n", response.Proxy.Name, response.ProxyToken), nil
return []codersdk.UpdateWorkspaceProxyResponse{response}, nil
}),
cliui.JSONFormat(),
// Table formatter expects a slice, make a slice of one.
cliui.ChangeFormatterData(cliui.TableFormat([]codersdk.UpdateWorkspaceProxyResponse{}, []string{"proxy name", "proxy url", "proxy token"}),
func(data any) (any, error) {
response, ok := data.(codersdk.UpdateWorkspaceProxyResponse)
if !ok {
return nil, xerrors.Errorf("unexpected type %T", data)
}
return []codersdk.UpdateWorkspaceProxyResponse{response}, nil
}),
),
}
)

return up
}
2 changes: 1 addition & 1 deletion enterprise/cli/workspaceproxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func Test_ProxyCRUD(t *testing.T) {

inv, conf := newCLI(
t,
"wsproxy", "delete", expectedName,
"wsproxy", "delete", "-y", expectedName,
)

pty := ptytest.New(t)
Expand Down
2 changes: 1 addition & 1 deletion site/src/components/DeploySettingsLayout/Sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export const Sidebar: React.FC = () => {
href="workspace-proxies"
icon={<SidebarNavItemIcon icon={HubOutlinedIcon} />}
>
Workspace Proxy
Workspace Proxies
</SidebarNavItem>
)}
<SidebarNavItem
Expand Down
37 changes: 37 additions & 0 deletions site/src/components/Navbar/NavbarView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ import Skeleton from "@mui/material/Skeleton"
import { BUTTON_SM_HEIGHT } from "theme/theme"
import { ProxyStatusLatency } from "components/ProxyStatusLatency/ProxyStatusLatency"
import { usePermissions } from "hooks/usePermissions"
import {
HelpTooltip,
HelpTooltipText,
HelpTooltipTitle,
} from "components/Tooltips/HelpTooltip"

export const USERS_LINK = `/users?filter=${encodeURIComponent("status:active")}`

Expand Down Expand Up @@ -186,6 +191,7 @@ export const NavbarView: FC<NavbarViewProps> = ({
const ProxyMenu: FC<{ proxyContextValue: ProxyContextValue }> = ({
proxyContextValue,
}) => {
const styles = useStyles()
const buttonRef = useRef<HTMLButtonElement>(null)
const [isOpen, setIsOpen] = useState(false)
const [refetchDate, setRefetchDate] = useState<Date>()
Expand Down Expand Up @@ -269,6 +275,34 @@ const ProxyMenu: FC<{ proxyContextValue: ProxyContextValue }> = ({
onClose={closeMenu}
sx={{ "& .MuiMenu-paper": { py: 1 } }}
>
<MenuItem
sx={[
{ fontSize: 14 },
{ "&:hover": { backgroundColor: "transparent" } },
{ wordWrap: "break-word" },
{ inlineSize: "200px" },
{ whiteSpace: "normal" },
{ textAlign: "center" },
]}
onClick={(e) => {
// Stop the menu from closing
e.stopPropagation()
}}
>
<div>
Reduce workspace latency by selecting the region nearest you.
{/* This was always on a newline below the text. This puts it on the same line.
It still doesn't look great, but it is marginally better. */}
<HelpTooltip buttonClassName={styles.displayInitial}>
<HelpTooltipTitle>Workspace Proxy Selection</HelpTooltipTitle>
<HelpTooltipText>
Only applies to web connections. Local ssh connections will
automatically select the nearest region based on latency.
</HelpTooltipText>
</HelpTooltip>
</div>
</MenuItem>
<Divider sx={{ borderColor: (theme) => theme.palette.divider }} />
{proxyContextValue.proxies?.map((proxy) => (
<MenuItem
onClick={() => {
Expand Down Expand Up @@ -335,6 +369,9 @@ const ProxyMenu: FC<{ proxyContextValue: ProxyContextValue }> = ({
}

const useStyles = makeStyles((theme) => ({
displayInitial: {
display: "initial",
},
root: {
height: navHeight,
background: theme.palette.background.paper,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { AvatarData } from "components/AvatarData/AvatarData"
import { Avatar } from "components/Avatar/Avatar"
import TableCell from "@mui/material/TableCell"
import TableRow from "@mui/material/TableRow"
import { FC } from "react"
import { FC, useState } from "react"
import {
HealthyBadge,
NotHealthyBadge,
Expand All @@ -12,22 +12,51 @@ import {
} from "components/DeploySettingsLayout/Badges"
import { ProxyLatencyReport } from "contexts/useProxyLatency"
import { getLatencyColor } from "utils/latency"
import Collapse from "@mui/material/Collapse"
import { makeStyles } from "@mui/styles"
import { combineClasses } from "utils/combineClasses"
import ListItem from "@mui/material/ListItem"
import List from "@mui/material/List"
import ListSubheader from "@mui/material/ListSubheader"
import { Maybe } from "components/Conditionals/Maybe"
import { CodeExample } from "components/CodeExample/CodeExample"

export const ProxyRow: FC<{
latency?: ProxyLatencyReport
proxy: Region
}> = ({ proxy, latency }) => {
const styles = useStyles()
// If we have a more specific proxy status, use that.
// All users can see healthy/unhealthy, some can see more.
let statusBadge = <ProxyStatus proxy={proxy} />
let shouldShowMessages = false
if ("status" in proxy) {
statusBadge = <DetailedProxyStatus proxy={proxy as WorkspaceProxy} />
const wsproxy = proxy as WorkspaceProxy
statusBadge = <DetailedProxyStatus proxy={wsproxy} />
shouldShowMessages = Boolean(
(wsproxy.status?.report?.warnings &&
wsproxy.status?.report?.warnings.length > 0) ||
(wsproxy.status?.report?.errors &&
wsproxy.status?.report?.errors.length > 0),
)
}

const [isMsgsOpen, setIsMsgsOpen] = useState(false)
const toggle = () => {
if (shouldShowMessages) {
setIsMsgsOpen((v) => !v)
}
}
return (
<>
<TableRow key={proxy.name} data-testid={`${proxy.name}`}>
<TableCell>
<TableRow
className={combineClasses({
[styles.clickable]: shouldShowMessages,
})}
key={proxy.name}
data-testid={`${proxy.name}`}
>
<TableCell onClick={toggle}>
<AvatarData
title={
proxy.display_name && proxy.display_name.length > 0
Expand All @@ -44,6 +73,7 @@ export const ProxyRow: FC<{
/>
)
}
subtitle={shouldShowMessages ? "Click to view details" : undefined}
/>
</TableCell>

Expand All @@ -62,10 +92,60 @@ export const ProxyRow: FC<{
{latency ? `${latency.latencyMS.toFixed(0)} ms` : "Not available"}
</TableCell>
</TableRow>
<Maybe condition={shouldShowMessages}>
<TableRow>
<TableCell colSpan={4} sx={{ padding: "0px !important" }}>
<Collapse in={isMsgsOpen}>
<ProxyMessagesRow proxy={proxy as WorkspaceProxy} />
</Collapse>
</TableCell>
</TableRow>
</Maybe>
</>
)
}

const ProxyMessagesRow: FC<{
proxy: WorkspaceProxy
}> = ({ proxy }) => {
return (
<>
<ProxyMessagesList
title="Errors"
messages={proxy.status?.report?.errors}
/>
<ProxyMessagesList
title="Warnings"
messages={proxy.status?.report?.warnings}
/>
</>
)
}

const ProxyMessagesList: FC<{
title: string
messages?: string[]
}> = ({ title, messages }) => {
if (!messages) {
return <></>
}
return (
<List
subheader={
<ListSubheader component="div" id="nested-list-subheader">
{title}
</ListSubheader>
}
>
{messages.map((error, index) => (
<ListItem key={"message" + index}>
<CodeExample code={error} />
</ListItem>
))}
</List>
)
}

// DetailedProxyStatus allows a more precise status to be displayed.
const DetailedProxyStatus: FC<{
proxy: WorkspaceProxy
Expand Down Expand Up @@ -100,3 +180,13 @@ const ProxyStatus: FC<{

return icon
}

const useStyles = makeStyles((theme) => ({
clickable: {
cursor: "pointer",

"&:hover": {
backgroundColor: theme.palette.action.hover,
},
},
}))