diff --git a/.github/workflows/coder.yaml b/.github/workflows/coder.yaml index 729c5cdbf5602..538cbef8b4376 100644 --- a/.github/workflows/coder.yaml +++ b/.github/workflows/coder.yaml @@ -6,6 +6,8 @@ on: - main tags: - "*" + paths-ignore: + - "docs/**" pull_request: @@ -217,18 +219,22 @@ jobs: run: go run scripts/datadog-cireport/main.go gotests.xml - uses: codecov/codecov-action@v3 + # This action has a tendency to error out unexpectedly, it has + # the `fail_ci_if_error` option that defaults to `false`, but + # that is no guarantee, see: + # https://github.com/codecov/codecov-action/issues/788 + continue-on-error: true if: github.actor != 'dependabot[bot]' && !github.event.pull_request.head.repo.fork with: token: ${{ secrets.CODECOV_TOKEN }} files: ./gotests.coverage flags: unittest-go-${{ matrix.os }} - # this flakes and sometimes fails the build - fail_ci_if_error: false test-go-postgres: name: "test/go/postgres" runs-on: ubuntu-latest - timeout-minutes: 20 + # This timeout must be greater than go test -timeout. + timeout-minutes: 25 steps: - uses: actions/checkout@v3 @@ -279,13 +285,16 @@ jobs: run: go run scripts/datadog-cireport/main.go gotests.xml - uses: codecov/codecov-action@v3 + # This action has a tendency to error out unexpectedly, it has + # the `fail_ci_if_error` option that defaults to `false`, but + # that is no guarantee, see: + # https://github.com/codecov/codecov-action/issues/788 + continue-on-error: true if: github.actor != 'dependabot[bot]' && !github.event.pull_request.head.repo.fork with: token: ${{ secrets.CODECOV_TOKEN }} files: ./gotests.coverage flags: unittest-go-postgres-${{ matrix.os }} - # this flakes and sometimes fails the build - fail_ci_if_error: false deploy: name: "deploy" @@ -429,13 +438,16 @@ jobs: working-directory: site - uses: codecov/codecov-action@v3 + # This action has a tendency to error out unexpectedly, it has + # the `fail_ci_if_error` option that defaults to `false`, but + # that is no guarantee, see: + # https://github.com/codecov/codecov-action/issues/788 + continue-on-error: true if: github.actor != 'dependabot[bot]' && !github.event.pull_request.head.repo.fork with: token: ${{ secrets.CODECOV_TOKEN }} files: ./site/coverage/lcov.info flags: unittest-js - # this flakes and sometimes fails the build - fail_ci_if_error: false - name: Upload DataDog Trace if: always() && github.actor != 'dependabot[bot]' && !github.event.pull_request.head.repo.fork diff --git a/.github/workflows/stale.yaml b/.github/workflows/stale.yaml index 88388d059db0a..88a493e080d1f 100644 --- a/.github/workflows/stale.yaml +++ b/.github/workflows/stale.yaml @@ -12,5 +12,6 @@ jobs: pull-requests: write steps: - uses: actions/stale@v5.1.0 - stale-issue-label: stale - stale-pr-label: stale + with: + stale-issue-label: stale + stale-pr-label: stale diff --git a/cli/cliflag/cliflag.go b/cli/cliflag/cliflag.go index ddb808b910614..053ea948f04cf 100644 --- a/cli/cliflag/cliflag.go +++ b/cli/cliflag/cliflag.go @@ -17,9 +17,33 @@ import ( "strings" "time" + "github.com/spf13/cobra" "github.com/spf13/pflag" ) +// IsSetBool returns the value of the boolean flag if it is set. +// It returns false if the flag isn't set or if any error occurs attempting +// to parse the value of the flag. +func IsSetBool(cmd *cobra.Command, name string) bool { + val, ok := IsSet(cmd, name) + if !ok { + return false + } + + b, err := strconv.ParseBool(val) + return err == nil && b +} + +// IsSet returns the string value of the flag and whether it was set. +func IsSet(cmd *cobra.Command, name string) (string, bool) { + flag := cmd.Flag(name) + if flag == nil { + return "", false + } + + return flag.Value.String(), flag.Changed +} + // String sets a string flag on the given flag set. func String(flagset *pflag.FlagSet, name, shorthand, env, def, usage string) { v, ok := os.LookupEnv(env) @@ -67,6 +91,22 @@ func Uint8VarP(flagset *pflag.FlagSet, ptr *uint8, name string, shorthand string flagset.Uint8VarP(ptr, name, shorthand, uint8(vi64), fmtUsage(usage, env)) } +func Bool(flagset *pflag.FlagSet, name, shorthand, env string, def bool, usage string) { + val, ok := os.LookupEnv(env) + if !ok || val == "" { + flagset.BoolP(name, shorthand, def, fmtUsage(usage, env)) + return + } + + valb, err := strconv.ParseBool(val) + if err != nil { + flagset.BoolP(name, shorthand, def, fmtUsage(usage, env)) + return + } + + flagset.BoolP(name, shorthand, valb, fmtUsage(usage, env)) +} + // BoolVarP sets a bool flag on the given flag set. func BoolVarP(flagset *pflag.FlagSet, ptr *bool, name string, shorthand string, env string, def bool, usage string) { val, ok := os.LookupEnv(env) diff --git a/cli/create_test.go b/cli/create_test.go index 625edd87411f3..1d8c01af8d912 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -88,7 +88,7 @@ func TestCreate(t *testing.T) { member := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) clitest.SetupConfig(t, member, root) - cmdCtx, done := context.WithTimeout(context.Background(), time.Second*3) + cmdCtx, done := context.WithTimeout(context.Background(), 10*time.Second) go func() { defer done() err := cmd.ExecuteContext(cmdCtx) diff --git a/cli/login.go b/cli/login.go index 9f26a2e30e078..9abc0b48d9c2f 100644 --- a/cli/login.go +++ b/cli/login.go @@ -73,7 +73,9 @@ func login() *cobra.Command { // on a very old client. err = checkVersions(cmd, client) if err != nil { - return xerrors.Errorf("check versions: %w", err) + // Checking versions isn't a fatal error so we print a warning + // and proceed. + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), cliui.Styles.Warn.Render(err.Error())) } hasInitialUser, err := client.HasFirstUser(cmd.Context()) diff --git a/cli/portforward_test.go b/cli/portforward_test.go index 6924a66f16950..95d515b00856a 100644 --- a/cli/portforward_test.go +++ b/cli/portforward_test.go @@ -142,21 +142,22 @@ func TestPortForward(t *testing.T) { }, } + // Setup agent once to be shared between test-cases (avoid expensive + // non-parallel setup). + var ( + client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user = coderdtest.CreateFirstUser(t, client) + _, workspace = runAgent(t, client, user.UserID) + ) + for _, c := range cases { //nolint:paralleltest // the `c := c` confuses the linter c := c - // Avoid parallel test here because setupLocal reserves + // Delay parallel tests here because setupLocal reserves // a free open port which is not guaranteed to be free - // after the listener closes. - //nolint:paralleltest + // between the listener closing and port-forward ready. t.Run(c.name, func(t *testing.T) { - //nolint:paralleltest t.Run("OnePort", func(t *testing.T) { - var ( - client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) - user = coderdtest.CreateFirstUser(t, client) - _, workspace = runAgent(t, client, user.UserID) - p1 = setupTestListener(t, c.setupRemote(t)) - ) + p1 := setupTestListener(t, c.setupRemote(t)) // Create a flag that forwards from local to listener 1. localAddress, localFlag := c.setupLocal(t) @@ -176,6 +177,8 @@ func TestPortForward(t *testing.T) { }() waitForPortForwardReady(t, buf) + t.Parallel() // Port is reserved, enable parallel execution. + // Open two connections simultaneously and test them out of // sync. d := net.Dialer{Timeout: 3 * time.Second} @@ -196,11 +199,8 @@ func TestPortForward(t *testing.T) { //nolint:paralleltest t.Run("TwoPorts", func(t *testing.T) { var ( - client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) - user = coderdtest.CreateFirstUser(t, client) - _, workspace = runAgent(t, client, user.UserID) - p1 = setupTestListener(t, c.setupRemote(t)) - p2 = setupTestListener(t, c.setupRemote(t)) + p1 = setupTestListener(t, c.setupRemote(t)) + p2 = setupTestListener(t, c.setupRemote(t)) ) // Create a flags for listener 1 and listener 2. @@ -223,6 +223,8 @@ func TestPortForward(t *testing.T) { }() waitForPortForwardReady(t, buf) + t.Parallel() // Port is reserved, enable parallel execution. + // Open a connection to both listener 1 and 2 simultaneously and // then test them out of order. d := net.Dialer{Timeout: 3 * time.Second} @@ -246,10 +248,6 @@ func TestPortForward(t *testing.T) { //nolint:paralleltest t.Run("TCP2Unix", func(t *testing.T) { var ( - client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) - user = coderdtest.CreateFirstUser(t, client) - _, workspace = runAgent(t, client, user.UserID) - // Find the TCP and Unix cases so we can use their setupLocal and // setupRemote methods respectively. tcpCase = cases[0] @@ -278,6 +276,8 @@ func TestPortForward(t *testing.T) { }() waitForPortForwardReady(t, buf) + t.Parallel() // Port is reserved, enable parallel execution. + // Open two connections simultaneously and test them out of // sync. d := net.Dialer{Timeout: 3 * time.Second} @@ -299,9 +299,6 @@ func TestPortForward(t *testing.T) { //nolint:paralleltest t.Run("All", func(t *testing.T) { var ( - client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) - user = coderdtest.CreateFirstUser(t, client) - _, workspace = runAgent(t, client, user.UserID) // These aren't fixed size because we exclude Unix on Windows. dials = []addr{} flags = []string{} @@ -339,6 +336,8 @@ func TestPortForward(t *testing.T) { }() waitForPortForwardReady(t, buf) + t.Parallel() // Port is reserved, enable parallel execution. + // Open connections to all items in the "dial" array. var ( d = net.Dialer{Timeout: 3 * time.Second} @@ -425,6 +424,8 @@ func runAgent(t *testing.T, client *codersdk.Client, userID uuid.UUID) ([]coders // setupTestListener starts accepting connections and echoing a single packet. // Returns the listener and the listen port or Unix path. func setupTestListener(t *testing.T, l net.Listener) string { + t.Helper() + // Wait for listener to completely exit before releasing. done := make(chan struct{}) t.Cleanup(func() { @@ -440,6 +441,7 @@ func setupTestListener(t *testing.T, l net.Listener) string { for { c, err := l.Accept() if err != nil { + _ = l.Close() return } @@ -479,6 +481,7 @@ func testAccept(t *testing.T, c net.Conn) { } func assertReadPayload(t *testing.T, r io.Reader, payload []byte) { + t.Helper() b := make([]byte, len(payload)+16) n, err := r.Read(b) assert.NoError(t, err, "read payload") @@ -487,12 +490,14 @@ func assertReadPayload(t *testing.T, r io.Reader, payload []byte) { } func assertWritePayload(t *testing.T, w io.Writer, payload []byte) { + t.Helper() n, err := w.Write(payload) assert.NoError(t, err, "write payload") assert.Equal(t, len(payload), n, "payload length does not match") } func waitForPortForwardReady(t *testing.T, output *threadSafeBuffer) { + t.Helper() for i := 0; i < 100; i++ { time.Sleep(250 * time.Millisecond) diff --git a/cli/root.go b/cli/root.go index a7c1b90ad751f..b38b7c72441f5 100644 --- a/cli/root.go +++ b/cli/root.go @@ -4,7 +4,6 @@ import ( "fmt" "net/url" "os" - "strconv" "strings" "text/template" "time" @@ -40,11 +39,12 @@ const ( varAgentURL = "agent-url" varGlobalConfig = "global-config" varNoOpen = "no-open" + varNoVersionCheck = "no-version-warning" varForceTty = "force-tty" + varVerbose = "verbose" notLoggedInMessage = "You are not logged in. Try logging in using 'coder login '." - noVersionCheckFlag = "no-version-warning" - envNoVersionCheck = "CODER_NO_VERSION_WARNING" + envNoVersionCheck = "CODER_NO_VERSION_WARNING" ) var ( @@ -58,8 +58,6 @@ func init() { } func Root() *cobra.Command { - var varSuppressVersion bool - cmd := &cobra.Command{ Use: "coder", SilenceErrors: true, @@ -68,7 +66,7 @@ func Root() *cobra.Command { `, PersistentPreRun: func(cmd *cobra.Command, args []string) { err := func() error { - if varSuppressVersion { + if cliflag.IsSetBool(cmd, varNoVersionCheck) { return nil } @@ -141,7 +139,7 @@ func Root() *cobra.Command { cmd.SetUsageTemplate(usageTemplate()) cmd.PersistentFlags().String(varURL, "", "Specify the URL to your deployment.") - cliflag.BoolVarP(cmd.PersistentFlags(), &varSuppressVersion, noVersionCheckFlag, "", envNoVersionCheck, false, "Suppress warning when client and server versions do not match.") + cliflag.Bool(cmd.PersistentFlags(), varNoVersionCheck, "", envNoVersionCheck, false, "Suppress warning when client and server versions do not match.") cliflag.String(cmd.PersistentFlags(), varToken, "", envSessionToken, "", fmt.Sprintf("Specify an authentication token. For security reasons setting %s is preferred.", envSessionToken)) cliflag.String(cmd.PersistentFlags(), varAgentToken, "", "CODER_AGENT_TOKEN", "", "Specify an agent authentication token.") _ = cmd.PersistentFlags().MarkHidden(varAgentToken) @@ -152,6 +150,7 @@ func Root() *cobra.Command { _ = cmd.PersistentFlags().MarkHidden(varForceTty) cmd.PersistentFlags().Bool(varNoOpen, false, "Block automatically opening URLs in the browser.") _ = cmd.PersistentFlags().MarkHidden(varNoOpen) + cliflag.Bool(cmd.PersistentFlags(), varVerbose, "v", "CODER_VERBOSE", false, "Enable verbose output") return cmd } @@ -427,18 +426,40 @@ func formatExamples(examples ...example) string { // FormatCobraError colorizes and adds "--help" docs to cobra commands. func FormatCobraError(err error, cmd *cobra.Command) string { helpErrMsg := fmt.Sprintf("Run '%s --help' for usage.", cmd.CommandPath()) - return cliui.Styles.Error.Render(err.Error() + "\n" + helpErrMsg) + + var ( + httpErr *codersdk.Error + output strings.Builder + ) + + if xerrors.As(err, &httpErr) { + _, _ = fmt.Fprintln(&output, httpErr.Friendly()) + } + + // If the httpErr is nil then we just have a regular error in which + // case we want to print out what's happening. + if httpErr == nil || cliflag.IsSetBool(cmd, varVerbose) { + _, _ = fmt.Fprintln(&output, err.Error()) + } + + _, _ = fmt.Fprint(&output, helpErrMsg) + + return cliui.Styles.Error.Render(output.String()) } func checkVersions(cmd *cobra.Command, client *codersdk.Client) error { - flag := cmd.Flag("no-version-warning") - if suppress, _ := strconv.ParseBool(flag.Value.String()); suppress { + if cliflag.IsSetBool(cmd, varNoVersionCheck) { return nil } clientVersion := buildinfo.Version() info, err := client.BuildInfo(cmd.Context()) + // Avoid printing errors that are connection-related. + if codersdk.IsConnectionErr(err) { + return nil + } + if err != nil { return xerrors.Errorf("build info: %w", err) } diff --git a/cli/root_test.go b/cli/root_test.go index f920401c9651c..5dc97bb060cf5 100644 --- a/cli/root_test.go +++ b/cli/root_test.go @@ -4,22 +4,115 @@ import ( "bytes" "testing" + "github.com/spf13/cobra" "github.com/stretchr/testify/require" + "golang.org/x/xerrors" "github.com/coder/coder/buildinfo" "github.com/coder/coder/cli" "github.com/coder/coder/cli/clitest" + "github.com/coder/coder/codersdk" ) func TestRoot(t *testing.T) { t.Run("FormatCobraError", func(t *testing.T) { t.Parallel() - cmd, _ := clitest.New(t, "delete") + t.Run("OK", func(t *testing.T) { + t.Parallel() - cmd, err := cmd.ExecuteC() - errStr := cli.FormatCobraError(err, cmd) - require.Contains(t, errStr, "Run 'coder delete --help' for usage.") + cmd, _ := clitest.New(t, "delete") + + cmd, err := cmd.ExecuteC() + errStr := cli.FormatCobraError(err, cmd) + require.Contains(t, errStr, "Run 'coder delete --help' for usage.") + }) + + t.Run("Verbose", func(t *testing.T) { + t.Parallel() + + // Test that the verbose error is masked without verbose flag. + t.Run("NoVerboseAPIError", func(t *testing.T) { + t.Parallel() + + cmd, _ := clitest.New(t) + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + var err error = &codersdk.Error{ + Response: codersdk.Response{ + Message: "This is a message.", + }, + Helper: "Try this instead.", + } + + err = xerrors.Errorf("wrap me: %w", err) + + return err + } + + cmd, err := cmd.ExecuteC() + errStr := cli.FormatCobraError(err, cmd) + require.Contains(t, errStr, "This is a message. Try this instead.") + require.NotContains(t, errStr, err.Error()) + }) + + // Assert that a regular error is not masked when verbose is not + // specified. + t.Run("NoVerboseRegularError", func(t *testing.T) { + t.Parallel() + + cmd, _ := clitest.New(t) + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + return xerrors.Errorf("this is a non-codersdk error: %w", xerrors.Errorf("a wrapped error")) + } + + cmd, err := cmd.ExecuteC() + errStr := cli.FormatCobraError(err, cmd) + require.Contains(t, errStr, err.Error()) + }) + + // Test that both the friendly error and the verbose error are + // displayed when verbose is passed. + t.Run("APIError", func(t *testing.T) { + t.Parallel() + + cmd, _ := clitest.New(t, "--verbose") + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + var err error = &codersdk.Error{ + Response: codersdk.Response{ + Message: "This is a message.", + }, + Helper: "Try this instead.", + } + + err = xerrors.Errorf("wrap me: %w", err) + + return err + } + + cmd, err := cmd.ExecuteC() + errStr := cli.FormatCobraError(err, cmd) + require.Contains(t, errStr, "This is a message. Try this instead.") + require.Contains(t, errStr, err.Error()) + }) + + // Assert that a regular error is not masked when verbose specified. + t.Run("RegularError", func(t *testing.T) { + t.Parallel() + + cmd, _ := clitest.New(t, "--verbose") + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + return xerrors.Errorf("this is a non-codersdk error: %w", xerrors.Errorf("a wrapped error")) + } + + cmd, err := cmd.ExecuteC() + errStr := cli.FormatCobraError(err, cmd) + require.Contains(t, errStr, err.Error()) + }) + }) }) t.Run("Version", func(t *testing.T) { diff --git a/cli/ssh_test.go b/cli/ssh_test.go index 171907ee06155..c5849e99eada6 100644 --- a/cli/ssh_test.go +++ b/cli/ssh_test.go @@ -91,6 +91,9 @@ func TestSSH(t *testing.T) { // Shells on Mac, Windows, and Linux all exit shells with the "exit" command. pty.WriteLine("exit") + // Read output to prevent hang on macOS, see: + // https://github.com/coder/coder/issues/2122 + pty.ExpectMatch("exit") <-cmdDone }) t.Run("Stdio", func(t *testing.T) { @@ -224,6 +227,9 @@ func TestSSH(t *testing.T) { // And we're done. pty.WriteLine("exit") + // Read output to prevent hang on macOS, see: + // https://github.com/coder/coder/issues/2122 + pty.ExpectMatch("exit") <-cmdDone }) } diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index a6ace8eb20c38..98fd6d1a33b7a 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -230,7 +230,7 @@ func NewProvisionerDaemon(t *testing.T, coderAPI *coderd.API) io.Closer { Logger: slogtest.Make(t, nil).Named("provisionerd").Leveled(slog.LevelDebug), PollInterval: 10 * time.Millisecond, UpdateInterval: 25 * time.Millisecond, - ForceCancelInterval: 25 * time.Millisecond, + ForceCancelInterval: time.Second, Provisioners: provisionerd.Provisioners{ string(database.ProvisionerTypeEcho): proto.NewDRPCProvisionerClient(provisionersdk.Conn(echoClient)), }, @@ -271,7 +271,7 @@ func CreateAnotherUser(t *testing.T, client *codersdk.Client, organizationID uui func createAnotherUserRetry(t *testing.T, client *codersdk.Client, organizationID uuid.UUID, retries int, roles ...string) *codersdk.Client { req := codersdk.CreateUserRequest{ - Email: namesgenerator.GetRandomName(1) + "@coder.com", + Email: namesgenerator.GetRandomName(10) + "@coder.com", Username: randomUsername(), Password: "testpass", OrganizationID: organizationID, @@ -677,7 +677,7 @@ func NewAzureInstanceIdentity(t *testing.T, instanceID string) (x509.VerifyOptio } func randomUsername() string { - return strings.ReplaceAll(namesgenerator.GetRandomName(0), "_", "-") + return strings.ReplaceAll(namesgenerator.GetRandomName(10), "_", "-") } // Used to easily create an HTTP transport! diff --git a/coderd/provisionerjobs.go b/coderd/provisionerjobs.go index 4ff2d6551c4e5..b18845e55d7e5 100644 --- a/coderd/provisionerjobs.go +++ b/coderd/provisionerjobs.go @@ -312,7 +312,11 @@ func convertProvisionerJob(provisionerJob database.ProvisionerJob) codersdk.Prov switch { case provisionerJob.CanceledAt.Valid: if provisionerJob.CompletedAt.Valid { - job.Status = codersdk.ProvisionerJobCanceled + if job.Error == "" { + job.Status = codersdk.ProvisionerJobCanceled + } else { + job.Status = codersdk.ProvisionerJobFailed + } } else { job.Status = codersdk.ProvisionerJobCanceling } diff --git a/coderd/provisionerjobs_internal_test.go b/coderd/provisionerjobs_internal_test.go index 4901f2f1ea9a4..2a9914887227a 100644 --- a/coderd/provisionerjobs_internal_test.go +++ b/coderd/provisionerjobs_internal_test.go @@ -3,6 +3,7 @@ package coderd import ( "context" "crypto/sha256" + "database/sql" "encoding/json" "net/http/httptest" "net/url" @@ -146,6 +147,109 @@ func TestProvisionerJobLogs_Unit(t *testing.T) { }) } +func TestConvertProvisionerJob_Unit(t *testing.T) { + t.Parallel() + validNullTimeMock := sql.NullTime{ + Time: database.Now(), + Valid: true, + } + invalidNullTimeMock := sql.NullTime{} + errorMock := sql.NullString{ + String: "error", + Valid: true, + } + testCases := []struct { + name string + input database.ProvisionerJob + expected codersdk.ProvisionerJob + }{ + { + name: "empty", + input: database.ProvisionerJob{}, + expected: codersdk.ProvisionerJob{ + Status: codersdk.ProvisionerJobPending, + }, + }, + { + name: "cancellation pending", + input: database.ProvisionerJob{ + CanceledAt: validNullTimeMock, + CompletedAt: invalidNullTimeMock, + }, + expected: codersdk.ProvisionerJob{ + Status: codersdk.ProvisionerJobCanceling, + }, + }, + { + name: "cancellation failed", + input: database.ProvisionerJob{ + CanceledAt: validNullTimeMock, + CompletedAt: validNullTimeMock, + Error: errorMock, + }, + expected: codersdk.ProvisionerJob{ + CompletedAt: &validNullTimeMock.Time, + Status: codersdk.ProvisionerJobFailed, + Error: errorMock.String, + }, + }, + { + name: "cancellation succeeded", + input: database.ProvisionerJob{ + CanceledAt: validNullTimeMock, + CompletedAt: validNullTimeMock, + }, + expected: codersdk.ProvisionerJob{ + CompletedAt: &validNullTimeMock.Time, + Status: codersdk.ProvisionerJobCanceled, + }, + }, + { + name: "job pending", + input: database.ProvisionerJob{ + StartedAt: invalidNullTimeMock, + }, + expected: codersdk.ProvisionerJob{ + Status: codersdk.ProvisionerJobPending, + }, + }, + { + name: "job failed", + input: database.ProvisionerJob{ + CompletedAt: validNullTimeMock, + StartedAt: validNullTimeMock, + Error: errorMock, + }, + expected: codersdk.ProvisionerJob{ + CompletedAt: &validNullTimeMock.Time, + StartedAt: &validNullTimeMock.Time, + Error: errorMock.String, + Status: codersdk.ProvisionerJobFailed, + }, + }, + { + name: "job succeeded", + input: database.ProvisionerJob{ + CompletedAt: validNullTimeMock, + StartedAt: validNullTimeMock, + }, + expected: codersdk.ProvisionerJob{ + CompletedAt: &validNullTimeMock.Time, + StartedAt: &validNullTimeMock.Time, + Status: codersdk.ProvisionerJobSucceeded, + }, + }, + } + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + actual := convertProvisionerJob(testCase.input) + assert.Equal(t, testCase.expected, actual) + }) + } +} + type fakePubSub struct { t *testing.T cond *sync.Cond diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 498a1e349a492..6d476599a1506 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -125,8 +125,17 @@ func TestPatchCancelTemplateVersion(t *testing.T) { var apiErr *codersdk.Error require.ErrorAs(t, err, &apiErr) require.Equal(t, http.StatusPreconditionFailed, apiErr.StatusCode()) + require.Eventually(t, func() bool { + var err error + version, err = client.TemplateVersion(context.Background(), version.ID) + return assert.NoError(t, err) && + version.Job.Status == codersdk.ProvisionerJobFailed + return version.Job.Status == codersdk.ProvisionerJobFailed + }, 5*time.Second, 25*time.Millisecond) }) - t.Run("Success", func(t *testing.T) { + // TODO(Cian): until we are able to test cancellation properly, validating + // Running -> Canceling is the best we can do for now. + t.Run("Canceling", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) user := coderdtest.CreateFirstUser(t, client) @@ -150,8 +159,11 @@ func TestPatchCancelTemplateVersion(t *testing.T) { require.Eventually(t, func() bool { var err error version, err = client.TemplateVersion(context.Background(), version.ID) - require.NoError(t, err) - return version.Job.Status == codersdk.ProvisionerJobCanceled + return assert.NoError(t, err) && + // The job will never actually cancel successfully because it will never send a + // provision complete response. + assert.Empty(t, version.Job.Error) && + version.Job.Status == codersdk.ProvisionerJobCanceling }, 5*time.Second, 25*time.Millisecond) }) } diff --git a/coderd/workspacebuilds_test.go b/coderd/workspacebuilds_test.go index 9ee40a6460525..c9f24d0442c14 100644 --- a/coderd/workspacebuilds_test.go +++ b/coderd/workspacebuilds_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/google/uuid" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/coder/coder/coderd/coderdtest" @@ -228,8 +229,11 @@ func TestPatchCancelWorkspaceBuild(t *testing.T) { require.Eventually(t, func() bool { var err error build, err = client.WorkspaceBuild(context.Background(), build.ID) - require.NoError(t, err) - return build.Job.Status == codersdk.ProvisionerJobCanceled + return assert.NoError(t, err) && + // The job will never actually cancel successfully because it will never send a + // provision complete response. + assert.Empty(t, build.Job.Error) && + build.Job.Status == codersdk.ProvisionerJobCanceling }, 5*time.Second, 25*time.Millisecond) } diff --git a/codersdk/client.go b/codersdk/client.go index 06185bdbd7570..1279d477053a4 100644 --- a/codersdk/client.go +++ b/codersdk/client.go @@ -185,6 +185,10 @@ func (e *Error) StatusCode() int { return e.statusCode } +func (e *Error) Friendly() string { + return fmt.Sprintf("%s. %s", strings.TrimSuffix(e.Message, "."), e.Helper) +} + func (e *Error) Error() string { var builder strings.Builder if e.method != "" && e.url != "" { diff --git a/codersdk/error.go b/codersdk/error.go index 1dd16f98cb506..316d15d888c62 100644 --- a/codersdk/error.go +++ b/codersdk/error.go @@ -1,5 +1,11 @@ package codersdk +import ( + "net" + + "golang.org/x/xerrors" +) + // Response represents a generic HTTP response. type Response struct { // Message is an actionable message that depicts actions the request took. @@ -25,3 +31,16 @@ type ValidationError struct { Field string `json:"field" validate:"required"` Detail string `json:"detail" validate:"required"` } + +// IsConnectionErr is a convenience function for checking if the source of an +// error is due to a 'connection refused', 'no such host', etc. +func IsConnectionErr(err error) bool { + var ( + // E.g. no such host + dnsErr *net.DNSError + // Eg. connection refused + opErr *net.OpError + ) + + return xerrors.As(err, &dnsErr) || xerrors.As(err, &opErr) +} diff --git a/codersdk/error_test.go b/codersdk/error_test.go new file mode 100644 index 0000000000000..d03024cbf1782 --- /dev/null +++ b/codersdk/error_test.go @@ -0,0 +1,65 @@ +package codersdk_test + +import ( + "net" + "os" + "testing" + + "github.com/stretchr/testify/require" + "golang.org/x/xerrors" + + "github.com/coder/coder/codersdk" +) + +func TestIsConnectionErr(t *testing.T) { + t.Parallel() + + type tc = struct { + name string + err error + expectedResult bool + } + + cases := []tc{ + { + // E.g. "no such host" + name: "DNSError", + err: &net.DNSError{ + Err: "no such host", + Name: "foofoo", + Server: "1.1.1.1:53", + IsTimeout: false, + IsTemporary: false, + IsNotFound: true, + }, + expectedResult: true, + }, + { + // E.g. "connection refused" + name: "OpErr", + err: &net.OpError{ + Op: "dial", + Net: "tcp", + Source: nil, + Addr: nil, + Err: &os.SyscallError{}, + }, + expectedResult: true, + }, + { + name: "OpaqueError", + err: xerrors.Errorf("I'm opaque!"), + expectedResult: false, + }, + } + + for _, c := range cases { + c := c + + t.Run(c.name, func(t *testing.T) { + t.Parallel() + + require.Equal(t, c.expectedResult, codersdk.IsConnectionErr(c.err)) + }) + } +} diff --git a/go.mod b/go.mod index b2d2ad221f669..ed4d17c316cf2 100644 --- a/go.mod +++ b/go.mod @@ -243,7 +243,7 @@ require ( github.com/rcrowley/go-metrics v0.0.0-20200313005456-10cdbea86bc0 // indirect github.com/rivo/uniseg v0.2.0 // indirect github.com/sirupsen/logrus v1.8.1 // indirect - github.com/spf13/afero v1.9.0 + github.com/spf13/afero v1.9.2 github.com/spf13/cast v1.5.0 // indirect github.com/spf13/jwalterweatherman v1.1.0 // indirect github.com/tadvi/systray v0.0.0-20190226123456-11a2b8fa57af // indirect diff --git a/go.sum b/go.sum index 4fd93ae3b2547..c5b0c23976aca 100644 --- a/go.sum +++ b/go.sum @@ -1699,8 +1699,8 @@ github.com/spf13/afero v1.1.2/go.mod h1:j4pytiNVoe2o6bmDsKpLACNPDBIoEAkihy7loJ1B github.com/spf13/afero v1.2.2/go.mod h1:9ZxEEn6pIJ8Rxe320qSDBk6AsU0r9pR7Q4OcevTdifk= github.com/spf13/afero v1.3.3/go.mod h1:5KUK8ByomD5Ti5Artl0RtHeI5pTF7MIDuXL3yY520V4= github.com/spf13/afero v1.6.0/go.mod h1:Ai8FlHk4v/PARR026UzYexafAt9roJ7LcLMAmO6Z93I= -github.com/spf13/afero v1.9.0 h1:sFSLUHgxdnN32Qy38hK3QkYBFXZj9DKjVjCUCtD7juY= -github.com/spf13/afero v1.9.0/go.mod h1:iUV7ddyEEZPO5gA3zD4fJt6iStLlL+Lg4m2cihcDf8Y= +github.com/spf13/afero v1.9.2 h1:j49Hj62F0n+DaZ1dDCvhABaPNSGNkt32oRFxI33IEMw= +github.com/spf13/afero v1.9.2/go.mod h1:iUV7ddyEEZPO5gA3zD4fJt6iStLlL+Lg4m2cihcDf8Y= github.com/spf13/cast v1.3.0/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= github.com/spf13/cast v1.3.1/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= github.com/spf13/cast v1.4.1/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= diff --git a/peer/channel.go b/peer/channel.go index c0415e50baa1a..d7119d1eafb7d 100644 --- a/peer/channel.go +++ b/peer/channel.go @@ -106,12 +106,15 @@ func (c *Channel) init() { // write operations to block once the threshold is set. c.dc.SetBufferedAmountLowThreshold(bufferedAmountLowThreshold) c.dc.OnBufferedAmountLow(func() { + // Grab the lock to protect the sendMore channel from being + // closed in between the isClosed check and the send. + c.closeMutex.Lock() + defer c.closeMutex.Unlock() if c.isClosed() { return } select { case <-c.closed: - return case c.sendMore <- struct{}{}: default: } @@ -122,15 +125,16 @@ func (c *Channel) init() { }) c.dc.OnOpen(func() { c.closeMutex.Lock() - defer c.closeMutex.Unlock() - c.conn.logger().Debug(context.Background(), "datachannel opening", slog.F("id", c.dc.ID()), slog.F("label", c.dc.Label())) var err error c.rwc, err = c.dc.Detach() if err != nil { + c.closeMutex.Unlock() _ = c.closeWithError(xerrors.Errorf("detach: %w", err)) return } + c.closeMutex.Unlock() + // pion/webrtc will return an io.ErrShortBuffer when a read // is triggerred with a buffer size less than the chunks written. // @@ -189,9 +193,6 @@ func (c *Channel) init() { // // This will block until the underlying DataChannel has been opened. func (c *Channel) Read(bytes []byte) (int, error) { - if c.isClosed() { - return 0, c.closeError - } err := c.waitOpened() if err != nil { return 0, err @@ -228,9 +229,6 @@ func (c *Channel) Write(bytes []byte) (n int, err error) { c.writeMutex.Lock() defer c.writeMutex.Unlock() - if c.isClosed() { - return 0, c.closeWithError(nil) - } err = c.waitOpened() if err != nil { return 0, err @@ -308,6 +306,10 @@ func (c *Channel) isClosed() bool { func (c *Channel) waitOpened() error { select { case <-c.opened: + // Re-check the closed channel to prioritize closure. + if c.isClosed() { + return c.closeError + } return nil case <-c.closed: return c.closeError diff --git a/peer/conn.go b/peer/conn.go index 8eae101ccdbbe..2e67b500ee5fd 100644 --- a/peer/conn.go +++ b/peer/conn.go @@ -3,7 +3,6 @@ package peer import ( "bytes" "context" - "crypto/rand" "io" "sync" @@ -256,7 +255,6 @@ func (c *Conn) init() error { c.logger().Debug(context.Background(), "sending local candidate", slog.F("candidate", iceCandidate.ToJSON().Candidate)) select { case <-c.closed: - break case c.localCandidateChannel <- iceCandidate.ToJSON(): } }() @@ -265,7 +263,6 @@ func (c *Conn) init() error { go func() { select { case <-c.closed: - return case c.dcOpenChannel <- dc: } }() @@ -435,9 +432,6 @@ func (c *Conn) pingEchoChannel() (*Channel, error) { data := make([]byte, pingDataLength) bytesRead, err := c.pingEchoChan.Read(data) if err != nil { - if c.isClosed() { - return - } _ = c.CloseWithError(xerrors.Errorf("read ping echo channel: %w", err)) return } diff --git a/peer/conn_test.go b/peer/conn_test.go index 20f4c84638b0c..d1fbf63d15ab6 100644 --- a/peer/conn_test.go +++ b/peer/conn_test.go @@ -91,6 +91,8 @@ func TestConn(t *testing.T) { // Create a channel that closes on disconnect. channel, err := server.CreateChannel(context.Background(), "wow", nil) assert.NoError(t, err) + defer channel.Close() + err = wan.Stop() require.NoError(t, err) // Once the connection is marked as disconnected, this @@ -107,10 +109,13 @@ func TestConn(t *testing.T) { t.Parallel() client, server, _ := createPair(t) exchange(t, client, server) - cch, err := client.CreateChannel(context.Background(), "hello", &peer.ChannelOptions{}) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + cch, err := client.CreateChannel(ctx, "hello", &peer.ChannelOptions{}) require.NoError(t, err) + defer cch.Close() - sch, err := server.Accept(context.Background()) + sch, err := server.Accept(ctx) require.NoError(t, err) defer sch.Close() @@ -123,9 +128,12 @@ func TestConn(t *testing.T) { t.Parallel() client, server, wan := createPair(t) exchange(t, client, server) - cch, err := client.CreateChannel(context.Background(), "hello", &peer.ChannelOptions{}) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + cch, err := client.CreateChannel(ctx, "hello", &peer.ChannelOptions{}) require.NoError(t, err) - sch, err := server.Accept(context.Background()) + defer cch.Close() + sch, err := server.Accept(ctx) require.NoError(t, err) defer sch.Close() @@ -140,26 +148,44 @@ func TestConn(t *testing.T) { t.Parallel() client, server, _ := createPair(t) exchange(t, client, server) - cch, err := client.CreateChannel(context.Background(), "hello", &peer.ChannelOptions{}) - require.NoError(t, err) - sch, err := server.Accept(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + cch, err := client.CreateChannel(ctx, "hello", &peer.ChannelOptions{}) require.NoError(t, err) - defer sch.Close() + defer cch.Close() + + readErr := make(chan error, 1) go func() { + sch, err := server.Accept(ctx) + if err != nil { + readErr <- err + _ = cch.Close() + return + } + defer sch.Close() + bytes := make([]byte, 4096) - for i := 0; i < 1024; i++ { - _, err := cch.Write(bytes) - require.NoError(t, err) + for { + _, err = sch.Read(bytes) + if err != nil { + readErr <- err + return + } } - _ = cch.Close() }() + bytes := make([]byte, 4096) - for { - _, err = sch.Read(bytes) - if err != nil { - require.ErrorIs(t, err, peer.ErrClosed) - break - } + for i := 0; i < 1024; i++ { + _, err = cch.Write(bytes) + require.NoError(t, err, "write i=%d", i) + } + _ = cch.Close() + + select { + case err = <-readErr: + require.ErrorIs(t, err, peer.ErrClosed, "read error") + case <-ctx.Done(): + require.Fail(t, "timeout waiting for read error") } }) @@ -170,13 +196,29 @@ func TestConn(t *testing.T) { srv, err := net.Listen("tcp", "127.0.0.1:0") require.NoError(t, err) defer srv.Close() + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() go func() { - sch, err := server.Accept(context.Background()) - assert.NoError(t, err) + sch, err := server.Accept(ctx) + if err != nil { + assert.NoError(t, err) + return + } + defer sch.Close() + nc2 := sch.NetConn() + defer nc2.Close() + nc1, err := net.Dial("tcp", srv.Addr().String()) - assert.NoError(t, err) + if err != nil { + assert.NoError(t, err) + return + } + defer nc1.Close() + go func() { + defer nc1.Close() + defer nc2.Close() _, _ = io.Copy(nc1, nc2) }() _, _ = io.Copy(nc2, nc1) @@ -204,7 +246,7 @@ func TestConn(t *testing.T) { c := http.Client{ Transport: defaultTransport, } - req, err := http.NewRequestWithContext(context.Background(), "GET", "http://localhost/", nil) + req, err := http.NewRequestWithContext(ctx, "GET", "http://localhost/", nil) require.NoError(t, err) resp, err := c.Do(req) require.NoError(t, err) @@ -272,14 +314,21 @@ func TestConn(t *testing.T) { t.Parallel() client, server, _ := createPair(t) exchange(t, client, server) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() go func() { - channel, err := client.CreateChannel(context.Background(), "test", nil) - assert.NoError(t, err) + channel, err := client.CreateChannel(ctx, "test", nil) + if err != nil { + assert.NoError(t, err) + return + } + defer channel.Close() _, err = channel.Write([]byte{1, 2}) assert.NoError(t, err) }() - channel, err := server.Accept(context.Background()) + channel, err := server.Accept(ctx) require.NoError(t, err) + defer channel.Close() data := make([]byte, 1) _, err = channel.Read(data) require.NoError(t, err) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 7714ad1965d43..e80e333b0df2b 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -250,7 +250,7 @@ export interface PutExtendWorkspaceRequest { readonly deadline: string } -// From codersdk/error.go:4:6 +// From codersdk/error.go:10:6 export interface Response { readonly message: string readonly detail?: string @@ -386,7 +386,7 @@ export interface UsersRequest extends Pagination { readonly q?: string } -// From codersdk/error.go:24:6 +// From codersdk/error.go:30:6 export interface ValidationError { readonly field: string readonly detail: string diff --git a/site/src/components/AppLink/AppLink.stories.tsx b/site/src/components/AppLink/AppLink.stories.tsx index 1b67d439784e6..b008905ef6c9d 100644 --- a/site/src/components/AppLink/AppLink.stories.tsx +++ b/site/src/components/AppLink/AppLink.stories.tsx @@ -14,7 +14,7 @@ WithIcon.args = { userName: "developer", workspaceName: MockWorkspace.name, appName: "code-server", - appIcon: "/code.svg", + appIcon: "/icon/code.svg", } export const WithoutIcon = Template.bind({}) diff --git a/site/src/components/AppLink/AppLink.tsx b/site/src/components/AppLink/AppLink.tsx index 9fd5d087be38b..4837acc53ec2d 100644 --- a/site/src/components/AppLink/AppLink.tsx +++ b/site/src/components/AppLink/AppLink.tsx @@ -38,6 +38,7 @@ export const AppLink: FC = ({ userName, workspaceName, appName, ap @@ -49,4 +50,8 @@ const useStyles = makeStyles(() => ({ link: { textDecoration: "none !important", }, + + button: { + whiteSpace: "nowrap", + }, })) diff --git a/site/src/components/AvatarData/AvatarData.tsx b/site/src/components/AvatarData/AvatarData.tsx index ad531b039ba61..107b0370039a4 100644 --- a/site/src/components/AvatarData/AvatarData.tsx +++ b/site/src/components/AvatarData/AvatarData.tsx @@ -48,5 +48,6 @@ const useStyles = makeStyles((theme) => ({ }, avatar: { marginRight: theme.spacing(1.5), + background: "hsl(219, 8%, 52%)", }, })) diff --git a/site/src/components/BuildsTable/BuildsTable.tsx b/site/src/components/BuildsTable/BuildsTable.tsx index b5a9c7a8923d5..92229a1a3be16 100644 --- a/site/src/components/BuildsTable/BuildsTable.tsx +++ b/site/src/components/BuildsTable/BuildsTable.tsx @@ -79,7 +79,9 @@ export const BuildsTable: FC = ({ builds, className }) => { - {status.status} + + {status.status} +
@@ -107,7 +109,7 @@ export const BuildsTable: FC = ({ builds, className }) => { const useStyles = makeStyles((theme) => ({ clickableTableRow: { "&:hover td": { - backgroundColor: fade(theme.palette.primary.light, 0.1), + backgroundColor: fade(theme.palette.primary.dark, 0.1), }, "&:focus": { @@ -126,4 +128,7 @@ const useStyles = makeStyles((theme) => ({ arrowCell: { display: "flex", }, + status: { + whiteSpace: "nowrap", + }, })) diff --git a/site/src/components/CodeExample/CodeExample.tsx b/site/src/components/CodeExample/CodeExample.tsx index e317ad47b32a3..95adc25ad4fc3 100644 --- a/site/src/components/CodeExample/CodeExample.tsx +++ b/site/src/components/CodeExample/CodeExample.tsx @@ -29,7 +29,8 @@ const useStyles = makeStyles((theme) => ({ display: "flex", flexDirection: "row", alignItems: "center", - background: theme.palette.background.default, + background: "hsl(223, 27%, 3%)", + border: `1px solid ${theme.palette.divider}`, color: theme.palette.primary.contrastText, fontFamily: MONOSPACE_FONT_FAMILY, fontSize: 14, diff --git a/site/src/components/CopyButton/CopyButton.tsx b/site/src/components/CopyButton/CopyButton.tsx index 6b0c241bd0692..030ca4623a59a 100644 --- a/site/src/components/CopyButton/CopyButton.tsx +++ b/site/src/components/CopyButton/CopyButton.tsx @@ -82,8 +82,6 @@ const useStyles = makeStyles((theme) => ({ }, copyButton: { borderRadius: 7, - background: theme.palette.background.default, - color: theme.palette.primary.contrastText, padding: theme.spacing(0.85), minWidth: 32, diff --git a/site/src/components/NavbarView/NavbarView.tsx b/site/src/components/NavbarView/NavbarView.tsx index 26d9fa2d0fb2a..6d7cb201d3a1a 100644 --- a/site/src/components/NavbarView/NavbarView.tsx +++ b/site/src/components/NavbarView/NavbarView.tsx @@ -104,7 +104,7 @@ const useStyles = makeStyles((theme) => ({ }, link: { alignItems: "center", - color: "#A7A7A7", + color: "hsl(220, 11%, 71%)", display: "flex", fontSize: 16, height: navHeight, @@ -113,7 +113,7 @@ const useStyles = makeStyles((theme) => ({ transition: "background-color 0.3s ease", "&:hover": { - backgroundColor: fade(theme.palette.primary.light, 0.1), + backgroundColor: fade(theme.palette.primary.light, 0.05), }, // NavLink adds this class when the current route matches. diff --git a/site/src/components/Resources/Resources.tsx b/site/src/components/Resources/Resources.tsx index 55d9e4ba07429..119de5240167a 100644 --- a/site/src/components/Resources/Resources.tsx +++ b/site/src/components/Resources/Resources.tsx @@ -100,7 +100,9 @@ export const Resources: FC = ({ {agent.name}
{agent.operating_system} - {agentStatus.status} + + {agentStatus.status} +
@@ -182,4 +184,8 @@ const useStyles = makeStyles((theme) => ({ flexWrap: "wrap", justifyContent: "flex-end", }, + + status: { + whiteSpace: "nowrap", + }, })) diff --git a/site/src/components/UserDropdown/UsersDropdown.tsx b/site/src/components/UserDropdown/UsersDropdown.tsx index 07156c4ca97b4..5b02a29096c5c 100644 --- a/site/src/components/UserDropdown/UsersDropdown.tsx +++ b/site/src/components/UserDropdown/UsersDropdown.tsx @@ -83,7 +83,7 @@ export const useStyles = makeStyles((theme) => ({ padding: `${theme.spacing(1.5)}px ${theme.spacing(2.75)}px`, "&:hover": { - backgroundColor: fade(theme.palette.primary.light, 0.1), + backgroundColor: fade(theme.palette.primary.light, 0.05), transition: "background-color 0.3s ease", }, }, diff --git a/site/src/components/UserDropdownContent/UserDropdownContent.tsx b/site/src/components/UserDropdownContent/UserDropdownContent.tsx index e55be7e4500f4..4d4a840c546b8 100644 --- a/site/src/components/UserDropdownContent/UserDropdownContent.tsx +++ b/site/src/components/UserDropdownContent/UserDropdownContent.tsx @@ -142,7 +142,7 @@ const useStyles = makeStyles((theme) => ({ padding: `${theme.spacing(1.5)}px ${theme.spacing(2.75)}px`, "&:hover": { - backgroundColor: fade(theme.palette.primary.light, 0.1), + backgroundColor: fade(theme.palette.primary.light, 0.05), transition: "background-color 0.3s ease", }, }, diff --git a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx index 6cb8364956cd2..7b9905cced32d 100644 --- a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx +++ b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx @@ -33,7 +33,7 @@ dayjs.extend(timezone) export const Language = { errorNoDayOfWeek: "Must set at least one day of week if start time is set", - errorNoTime: "Start time is required", + errorNoTime: "Start time is required when days of the week are selected", errorTime: "Time must be in HH:mm format (24 hours)", errorTimezone: "Invalid timezone", daysOfWeekLabel: "Days of Week", diff --git a/site/src/components/WorkspacesTable/WorkspacesRow.tsx b/site/src/components/WorkspacesTable/WorkspacesRow.tsx index b048e65bf1339..bfe9b2e99e42e 100644 --- a/site/src/components/WorkspacesTable/WorkspacesRow.tsx +++ b/site/src/components/WorkspacesTable/WorkspacesRow.tsx @@ -91,7 +91,7 @@ export const WorkspacesRow: FC<{ workspaceRef: WorkspaceItemMachineRef }> = ({ w const useStyles = makeStyles((theme) => ({ clickableTableRow: { "&:hover td": { - backgroundColor: fade(theme.palette.primary.light, 0.1), + backgroundColor: fade(theme.palette.primary.dark, 0.1), }, "&:focus": { diff --git a/site/src/pages/TemplatesPage/TemplatesPageView.tsx b/site/src/pages/TemplatesPage/TemplatesPageView.tsx index 074208f2d3747..79d1ec294ccc1 100644 --- a/site/src/pages/TemplatesPage/TemplatesPageView.tsx +++ b/site/src/pages/TemplatesPage/TemplatesPageView.tsx @@ -174,7 +174,7 @@ const useStyles = makeStyles((theme) => ({ }, clickableTableRow: { "&:hover td": { - backgroundColor: fade(theme.palette.primary.light, 0.1), + backgroundColor: fade(theme.palette.primary.dark, 0.1), }, "&:focus": { diff --git a/site/src/theme/overrides.ts b/site/src/theme/overrides.ts index a11deb15cc56c..7310ad77e945a 100644 --- a/site/src/theme/overrides.ts +++ b/site/src/theme/overrides.ts @@ -4,12 +4,23 @@ import { MONOSPACE_FONT_FAMILY } from "./constants" // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types export const getOverrides = (palette: PaletteOptions) => { return { + MuiCssBaseline: { + "@global": { + body: { + backgroundImage: + "linear-gradient(to right bottom, hsl(223, 38%, 14%), hsl(221, 53%, 3%))", + backgroundRepeat: "no-repeat", + backgroundAttachment: "fixed", + letterSpacing: "-0.015em", + }, + }, + }, MuiAvatar: { root: { borderColor: palette.divider, width: 36, height: 36, - fontSize: 20, + fontSize: 18, }, }, MuiButton: { @@ -26,7 +37,7 @@ export const getOverrides = (palette: PaletteOptions) => { contained: { boxShadow: "none", color: palette.text?.primary, - backgroundColor: "#151515", + backgroundColor: "hsl(223, 27%, 3%)", "&:hover": { boxShadow: "none", backgroundColor: "#000000", @@ -62,11 +73,12 @@ export const getOverrides = (palette: PaletteOptions) => { root: { // Gives the appearance of a border! borderRadius: 2, - border: `1px solid ${palette.divider}`, + background: "hsla(222, 31%, 19%, .5)", "& td": { paddingTop: 16, paddingBottom: 16, + background: "transparent", }, }, }, @@ -97,11 +109,15 @@ export const getOverrides = (palette: PaletteOptions) => { }, MuiOutlinedInput: { root: { + "& .MuiOutlinedInput-notchedOutline": { + borderColor: palette.divider, + }, + "& input:-webkit-autofill": { WebkitBoxShadow: `0 0 0 1000px ${palette.background?.paper} inset`, }, "&:hover .MuiOutlinedInput-notchedOutline": { - borderColor: (palette.primary as SimplePaletteColorOptions).light, + borderColor: palette.divider, }, }, }, diff --git a/site/src/theme/palettes.ts b/site/src/theme/palettes.ts index 41433a6240e78..41702d0d73d27 100644 --- a/site/src/theme/palettes.ts +++ b/site/src/theme/palettes.ts @@ -3,29 +3,29 @@ import { PaletteOptions } from "@material-ui/core/styles/createPalette" export const darkPalette: PaletteOptions = { type: "dark", primary: { - main: "#409BF4", - contrastText: "#f8f8f8", - light: "#79B8FF", - dark: "#1976D2", + main: "hsl(215, 81%, 63%)", + contrastText: "hsl(218, 44%, 92%)", + light: "hsl(215, 83%, 70%)", + dark: "hsl(215, 74%, 51%)", }, secondary: { - main: "#008510", - contrastText: "#f8f8f8", - dark: "#7057FF", + main: "hsl(142, 64%, 34%)", + contrastText: "hsl(218, 44%, 92%)", + dark: "hsl(233, 73%, 63%)", }, background: { - default: "#1F1F1F", - paper: "#292929", + default: "hsl(222, 38%, 14%)", + paper: "hsl(222, 32%, 19%)", }, text: { - primary: "#F8F8F8", - secondary: "#C1C1C1", + primary: "hsl(218, 44%, 92%)", + secondary: "hsl(218, 32%, 77%)", }, - divider: "#383838", + divider: "hsl(221, 32%, 26%)", warning: { - main: "#C16800", + main: "hsl(20, 79%, 53%)", }, success: { - main: "#6BBE00", + main: "hsl(142, 58%, 41%)", }, } diff --git a/site/yarn.lock b/site/yarn.lock index 28ca9af921a95..5fa3c97289b78 100644 --- a/site/yarn.lock +++ b/site/yarn.lock @@ -13227,15 +13227,15 @@ terser-webpack-plugin@^5.1.3: terser "^5.7.2" terser@^4.1.2, terser@^4.6.3: - version "4.8.0" - resolved "https://registry.yarnpkg.com/terser/-/terser-4.8.0.tgz#63056343d7c70bb29f3af665865a46fe03a0df17" - integrity sha512-EAPipTNeWsb/3wLPeup1tVPaXfIaU68xMnVdPafIL1TV05OhASArYyIfFvnvJCNrR2NIOvDVNNTFRa+Re2MWyw== + version "4.8.1" + resolved "https://registry.yarnpkg.com/terser/-/terser-4.8.1.tgz#a00e5634562de2239fd404c649051bf6fc21144f" + integrity sha512-4GnLC0x667eJG0ewJTa6z/yXrbLGv80D9Ru6HIpCQmO+Q4PfEtBFi0ObSckqwL6VyQv/7ENJieXHo2ANmdQwgw== dependencies: commander "^2.20.0" source-map "~0.6.1" source-map-support "~0.5.12" -terser@^5.10.0, terser@^5.7.2: +terser@^5.10.0, terser@^5.3.4, terser@^5.7.2: version "5.11.0" resolved "https://registry.yarnpkg.com/terser/-/terser-5.11.0.tgz#2da5506c02e12cd8799947f30ce9c5b760be000f" integrity sha512-uCA9DLanzzWSsN1UirKwylhhRz3aKPInlfmpGfw8VN6jHsAtu8HJtIpeeHHK23rxnE/cDc+yvmq5wqkIC6Kn0A== @@ -13245,15 +13245,6 @@ terser@^5.10.0, terser@^5.7.2: source-map "~0.7.2" source-map-support "~0.5.20" -terser@^5.3.4: - version "5.10.0" - resolved "https://registry.yarnpkg.com/terser/-/terser-5.10.0.tgz#b86390809c0389105eb0a0b62397563096ddafcc" - integrity sha512-AMmF99DMfEDiRJfxfY5jj5wNH/bYO09cniSqhfoyxc8sFoYIgkJy86G04UoZU5VjlpnplVu0K6Tx6E9b5+DlHA== - dependencies: - commander "^2.20.0" - source-map "~0.7.2" - source-map-support "~0.5.20" - test-exclude@^6.0.0: version "6.0.0" resolved "https://registry.yarnpkg.com/test-exclude/-/test-exclude-6.0.0.tgz#04a8698661d805ea6fa293b6cb9e63ac044ef15e"