Skip to content

feat: show banner when workspace is outdated #4926

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 3 commits into from
Nov 7, 2022
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
13 changes: 12 additions & 1 deletion cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,14 +417,25 @@ func isTTY(cmd *cobra.Command) bool {
// This accepts a reader to work with Cobra's "OutOrStdout"
// function for simple testing.
func isTTYOut(cmd *cobra.Command) bool {
return isTTYWriter(cmd, cmd.OutOrStdout)
}

// isTTYErr returns whether the passed reader is a TTY or not.
// This accepts a reader to work with Cobra's "ErrOrStderr"
// function for simple testing.
func isTTYErr(cmd *cobra.Command) bool {
return isTTYWriter(cmd, cmd.ErrOrStderr)
}

func isTTYWriter(cmd *cobra.Command, writer func() io.Writer) bool {
// If the `--force-tty` command is available, and set,
// assume we're in a tty. This is primarily for cases on Windows
// where we may not be able to reliably detect this automatically (ie, tests)
forceTty, err := cmd.Flags().GetBool(varForceTty)
if forceTty && err == nil {
return true
}
file, ok := cmd.OutOrStdout().(*os.File)
file, ok := writer().(*os.File)
if !ok {
return false
}
Expand Down
21 changes: 21 additions & 0 deletions cli/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"io"
"net/url"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -72,6 +73,11 @@ func ssh() *cobra.Command {
return err
}

updateWorkspaceBanner, outdated := verifyWorkspaceOutdated(client, workspace)
if outdated && isTTYErr(cmd) {
_, _ = fmt.Fprintln(cmd.ErrOrStderr(), updateWorkspaceBanner)
}

// OpenSSH passes stderr directly to the calling TTY.
// This is required in "stdio" mode so a connecting indicator can be displayed.
err = cliui.Agent(ctx, cmd.ErrOrStderr(), cliui.AgentOptions{
Expand Down Expand Up @@ -343,3 +349,18 @@ func notifyCondition(ctx context.Context, client *codersdk.Client, workspaceID u
return deadline.Truncate(time.Minute), callback
}
}

// Verify if the user workspace is outdated and prepare an actionable message for user.
func verifyWorkspaceOutdated(client *codersdk.Client, workspace codersdk.Workspace) (string, bool) {
if !workspace.Outdated {
return "", false // workspace is up-to-date
}

workspaceLink := buildWorkspaceLink(client.URL, workspace)
return fmt.Sprintf("👋 Your workspace is outdated! Update it here: %s\n", workspaceLink), true
}

// Build the user workspace link which navigates to the Coder web UI.
func buildWorkspaceLink(serverURL *url.URL, workspace codersdk.Workspace) *url.URL {
return serverURL.ResolveReference(&url.URL{Path: fmt.Sprintf("@%s/%s", workspace.OwnerName, workspace.Name)})
}
58 changes: 58 additions & 0 deletions cli/ssh_internal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package cli

import (
"net/url"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/coder/coder/codersdk"
)

const (
fakeOwnerName = "fake-owner-name"
fakeServerURL = "https://fake-foo-url"
fakeWorkspaceName = "fake-workspace-name"
)

func TestVerifyWorkspaceOutdated(t *testing.T) {
t.Parallel()

serverURL, err := url.Parse(fakeServerURL)
require.NoError(t, err)

client := codersdk.Client{URL: serverURL}

t.Run("Up-to-date", func(t *testing.T) {
Copy link
Member

@mafredri mafredri Nov 7, 2022

Choose a reason for hiding this comment

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

Sorry, I didn't look too closely at these tests on my first pass. I should point out that the preferred way to test in the coder/coder repo is write external tests (package cli_test).

With these types of tests, we'd do a bit more setup for the test so that we can then test the output of running coder ssh ....

(Not a blocker for this PR, but just a heads up for future PRs.)

Copy link
Member Author

@mtojek mtojek Nov 7, 2022

Choose a reason for hiding this comment

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

Ok, I see now. I can use existing coderdtest APIs. I need to create a workspace to simulate a template update (CreateTemplateVersion). Something like setupWorkspaceForAgent, but customized.

I can adjust the implementation, but not sure if in this case the internal_test isn't just simpler (and shorter). Do you have any preference?

t.Parallel()

workspace := codersdk.Workspace{Name: fakeWorkspaceName, OwnerName: fakeOwnerName}

_, outdated := verifyWorkspaceOutdated(&client, workspace)

assert.False(t, outdated, "workspace should be up-to-date")
})
t.Run("Outdated", func(t *testing.T) {
t.Parallel()

workspace := codersdk.Workspace{Name: fakeWorkspaceName, OwnerName: fakeOwnerName, Outdated: true}

updateWorkspaceBanner, outdated := verifyWorkspaceOutdated(&client, workspace)

assert.True(t, outdated, "workspace should be outdated")
assert.NotEmpty(t, updateWorkspaceBanner, "workspace banner should be present")
})
}

func TestBuildWorkspaceLink(t *testing.T) {
t.Parallel()

serverURL, err := url.Parse(fakeServerURL)
require.NoError(t, err)

workspace := codersdk.Workspace{Name: fakeWorkspaceName, OwnerName: fakeOwnerName}
workspaceLink := buildWorkspaceLink(serverURL, workspace)

assert.Equal(t, workspaceLink.String(), fakeServerURL+"/@"+fakeOwnerName+"/"+fakeWorkspaceName)
}