Commit Graph
41 Commits
Author SHA1 Message Date
Hugo Dutka a7e828593f chore: retry failing tests in CI (#17681)
This PR introduces failing test retries in CI for e2e tests, Go tests
with the in-memory database, Go tests with Postgres, and the CLI tests.
Retries are not enabled for race tests.

The goal is to reduce how often flakes disrupt developers' workflows.
2025-05-06 16:53:26 +02:00
Hugo Dutka 6b4d3f83bc chore: reduce "Upload tests to datadog" times in CI (#17668)
This PR speeds up the "Upload tests to datadog" step by downloading the
`datadog-ci` binary directly from GitHub releases. Most of the time used
to be spent in `npm install`, which consistently timed out on Windows
after a minute. [Now it takes 3
seconds](https://github.com/coder/coder/actions/runs/14834976784/job/41644230049?pr=17668#step:10:1).

I updated it to version v2.48.0 because v2.21.0 didn't have the
artifacts for arm64 macOS.
2025-05-05 18:49:58 +02:00
Hugo Dutka 87f4535357 chore: optimize CI setup time on Windows (#17666)
This PR focuses on optimizing go-test CI times on Windows. It:

- backs the `$RUNNER_TEMP` directory with a RAM disk. This directory is
used by actions like cache, setup-go, and setup-terraform as a staging
area
- backs `GOCACHE`, `GOMODCACHE`, and `GOPATH` with a RAM disk
- backs `$GITHUB_WORKSPACE` with a RAM disk - that's where the
repository is checked out
- uses preinstalled Go on Windows runners
- starts using the depot Windows runner

From what I've seen, these changes bring test times down to be on par
with Linux and macOS. The biggest improvement comes from backing
frequently accessed paths with RAM disks. The C drive is surprisingly
slow - I ran some performance tests with
[fio](https://fio.readthedocs.io/en/latest/fio_doc.html#) where I tested
IOPS on many small files, and the RAM disk was 100x faster.

Additionally, the depot runners seem to have more consistent performance
than the ones provided by GitHub.
2025-05-05 14:26:30 +02:00
Hugo Dutka b47d54d777 chore: cache terraform providers between CI test runs (#17373)
Addresses https://github.com/coder/internal/issues/322.

This PR starts caching Terraform providers used by `TestProvision` in
`provisioner/terraform/provision_test.go`. The goal is to improve the
reliability of this test by cutting down on the number of network calls
to external services. It leverages GitHub Actions cache, which [on depot
runners is persisted for 14 days by
default](https://depot.dev/docs/github-actions/overview#cache-retention-policy).

Other than the aforementioned `TestProvision`, I couldn't find any other
tests which depend on external terraform providers.
2025-04-28 10:57:24 +02:00
Hugo Dutka 2f99d70640 fix: configure start workspace action after version upgrade (#17398)
Dependabot recently upgraded `coder/start-workspace-action` to the
latest version. Compared to the version we were using previously, the
new version expects a different configuration.
2025-04-15 12:50:57 +05:00
Hugo Dutka 7d08bf0afe chore: improve error logging in TestServer/EphemeralDeployment (#17184)
There's a flake reported in https://github.com/coder/internal/issues/549
that was caused by the built-in Postgres failing to start. However, the
test was written in a way that didn't log the actual error which caused
Postgres to fail. This PR improves error logging in the affected test so
that the next time the error happens, we know what it is.
2025-04-01 13:23:06 +02:00
Hugo Dutka d5557fcbf5 fix: implement device auth rate limit handling (#17079)
The [OAuth2
specification](https://datatracker.ietf.org/doc/html/rfc8628) describes
how clients in the device flow should handle retrying requests when they
are rate limited.

We didn't respect it, which sometimes prevented users from logging in or
setting up external auth. They'd see a `slow_down` error in the UI and
would be unable to complete the authentication flow. This PR implements
rate limit handling according to the spec.
2025-03-25 11:32:03 +01:00
Hugo Dutka de6080c46d chore: update comment on the users.github_com_user_id field (#17037)
Follow up to https://github.com/coder/coder/pull/17029.
2025-03-21 13:31:17 +01:00
Hugo Dutka a71aa202dc feat: filter users by github user id in the users list CLI command (#17029)
Add the `--github-user-id` option to `coder users list`, which makes the
command only return users with a matching GitHub user id. This will
enable https://github.com/coder/start-workspace-action to find a Coder
user that corresponds to a GitHub user requesting to start a workspace.
2025-03-21 13:30:47 +01:00
Hugo Dutka 8d5e6f3cc0 fix: fix IsGithubDotComURL check (#17022)
When DeviceFlow with GitHub OAuth2 is configured, the
`api.GithubOAuth2Config.AuthCode` is
[overridden](https://github.com/coder/coder/blob/b08c8c9e1ee8edf18e9ba575098d99533062a240/coderd/userauth.go#L779)
and returns a value that doesn't pass the `IsGithubDotComURL` check.
This PR ensures the original `AuthCodeURL` method is used instead.
2025-03-20 13:24:38 +00:00
Hugo Dutka 6a7c43d76c chore: add the start workspace workflow (#17002)
The workflow triggers when an issue is created or a comment is posted.
If the string "@coder" appears in the body of the issue or comment, a
Coder workspace owned by the issue creator or commentator will be
created with the parameters specified in the workflow.
2025-03-19 12:39:37 -04:00
Hugo Dutka 24f3445e00 chore: track workspace resource monitors in telemetry (#16776)
Addresses https://github.com/coder/nexus/issues/195. Specifically, just
the "tracking templates" requirement:

> ## Tracking in templates
> To enable resource alerts, a user must add the resource_monitoring
block to a template's coder_agent resource. We'd like to track if
customers have any resource monitoring enabled on a per-deployment
basis. Even better, we could identify which templates are using resource
monitoring.
2025-03-03 18:41:01 +01:00
Hugo Dutka 95347b2b93 fix: allow orgs with default github provider (#16755)
This PR fixes 2 bugs:

## Problem 1

The server would fail to start when the default github provider was
configured and the flag `--oauth2-github-allowed-orgs` was set. The
error was

```
error: configure github oauth2: allow everyone and allowed orgs cannot be used together
```

This PR fixes it by enabling "allow everone" with the default provider
only if "allowed orgs" isn't set.

## Problem 2

The default github provider uses the device flow to authorize users, and
that's handled differently by our web UI than the standard oauth flow.
In particular, the web UI only handles JSON responses rather than HTTP
redirects. There were 2 code paths that returned redirects, and the PR
changes them to return JSON messages instead if the device flow is
configured.
2025-03-03 16:05:45 +01:00
Hugo Dutka 0f4f6bd147 docs: describe default sign up behavior with GitHub (#16765)
Document the sign up behavior with the default GitHub OAuth2 app.
2025-03-03 13:23:12 +01:00
Hugo Dutka 9c5d4966ee docs: suggest disabling the default GitHub OAuth2 provider on k8s (#16758)
For production deployments we recommend disabling the default GitHub
OAuth2 app managed by Coder. This PR mentions it in k8s installation
docs and the helm README so users can stumble upon it more easily.
2025-03-03 12:32:27 +01:00
Hugo Dutka a5842e5ad1 docs: document default GitHub OAuth2 configuration and device flow (#16663)
Document the changes made in https://github.com/coder/coder/pull/16629
and https://github.com/coder/coder/pull/16585.
2025-03-03 12:31:56 +01:00
Hugo DutkaandEdwardAngert 6acc3a9469 docs: update the quickstart page (#16666)
## Changes
1. Update the `0.0.0.0:3001` web UI address to `localhost:3000`. Coder
starts on port 3000 by default. It'd use 3001 only if 3000 was already
taken.
2. Update the screenshot of the `/setup` page to reflect how it will
look like after merging https://github.com/coder/coder/pull/16662. Note:
this PR should be merged only after the other one is.
3. Minor phrasing changes.

---------

Co-authored-by: EdwardAngert <17991901+EdwardAngert@users.noreply.github.com>
2025-02-25 16:32:20 +01:00
Hugo Dutka d3a56ae3ef feat: enable GitHub OAuth2 login by default on new deployments (#16662)
Third and final PR to address
https://github.com/coder/coder/issues/16230.

This PR enables GitHub OAuth2 login by default on new deployments.
Combined with https://github.com/coder/coder/pull/16629, this will allow
the first admin user to sign up with GitHub rather than email and
password.

We take care not to enable the default on deployments that would upgrade
to a Coder version with this change.

To disable the default provider an admin can set the
`CODER_OAUTH2_GITHUB_DEFAULT_PROVIDER` env variable to false.
2025-02-25 16:31:33 +01:00
Hugo Dutka 67d89bb102 feat: implement sign up with GitHub for the first user (#16629)
Second PR to address https://github.com/coder/coder/issues/16230. See
the issue for more context and discussion.

It adds a "Continue with GitHub" button to the `/setup` page, so the
deployment's admin can sign up with it. It also removes the "Username"
and "Full Name" fields to make signing up with email faster. In the
email flow, the username is now auto-generated based on the email, and
full name is left empty.

<img width="1512" alt="Screenshot 2025-02-21 at 17 51 22"
src="https://github.com/user-attachments/assets/e7c6986b-c05e-458b-bb01-c3aea3b74c0e"
/>

There's a separate, follow up issue to visually align the `/setup` page
with the new design system: https://github.com/coder/coder/issues/16653
2025-02-25 15:54:38 +01:00
Hugo Dutka c8abf58e29 chore: reduce prominence of Scratch starter and emphasize Docker in UI (#16665) 2025-02-24 20:59:21 +01:00
Hugo Dutka 8f33c6d8d1 chore: track users' login methods in telemetry (#16664)
Addresses https://github.com/coder/nexus/issues/191.
2025-02-24 18:00:26 +00:00
Hugo Dutka ac88c9ba17 fix: ensure the web UI doesn't break when license telemetry required check fails (#16667)
Addresses https://github.com/coder/coder/issues/16455.

## Changes

- Initialize default entitlements in a Set to include all features
- Initialize entitlements' `Warnings` and `Errors` fields to arrays
rather than `nil`s.
- Minor changes in formatting on the frontend

## Reasoning

I had to change how entitlements are initialized to match the `codersdk`
[generated
types](https://github.com/coder/coder/blob/33d62619225702257fa2542f40ecc26bfd0d1fa6/site/src/api/typesGenerated.ts#L727),
which the frontend assumes are correct, and doesn't run additional
checks on.

- `features: Record<FeatureName, Feature>`: this type signifies that
every `FeatureName` is present in the record, but on `main`, that's not
true if there's a telemetry required error
- `warnings: readonly string[];` and `errors: readonly string[];`: these
types mean that the fields are not `null`, but that's not always true

With a valid license, the [`LicensesEntitlements`
function](https://github.com/coder/coder/blob/33d62619225702257fa2542f40ecc26bfd0d1fa6/enterprise/coderd/license/license.go#L92)
ensures that all features are present in the entitlements. It's called
by the [`Entitlements`
function](https://github.com/coder/coder/blob/33d62619225702257fa2542f40ecc26bfd0d1fa6/enterprise/coderd/license/license.go#L42),
which is called by
[`api.updateEnittlements`](https://github.com/coder/coder/blob/33d62619225702257fa2542f40ecc26bfd0d1fa6/enterprise/coderd/coderd.go#L687).
However, when a license requires telemetry and telemetry is disabled,
the entitlements with all features [are
discarded](https://github.com/coder/coder/blob/33d62619225702257fa2542f40ecc26bfd0d1fa6/enterprise/coderd/coderd.go#L704)
in an early exit from the same function. By initializing entitlements
with all the features from the get go, we avoid this problem.

## License issue banner after the changes

<img width="1512" alt="Screenshot 2025-02-23 at 20 25 42"
src="https://github.com/user-attachments/assets/ee0134b3-f745-45d9-8333-bfa1661e33d2"
/>
2025-02-24 16:02:33 +01:00
Hugo Dutka 8c5e7007cd feat: support the OAuth2 device flow with GitHub for signing in (#16585)
First PR in a series to address
https://github.com/coder/coder/issues/16230.

Introduces support for logging in via the [GitHub OAuth2 Device
Flow](https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/authorizing-oauth-apps#device-flow).

It's previously been possible to configure external auth with the device
flow, but it's not been possible to use it for logging in. This PR
builds on the existing support we had to extend it to sign ins.

When a user clicks "sign in with GitHub" when device auth is configured,
they are redirected to the new `/login/device` page, which makes the
flow possible from the client's side. The recording below shows the full
flow.


https://github.com/user-attachments/assets/90c06f1f-e42f-43e9-a128-462270c80fdd

I've also manually tested that it works for converting from
password-based auth to oauth.

Device auth can be enabled by a deployment's admin by setting the
`CODER_OAUTH2_GITHUB_DEVICE_FLOW` env variable or a corresponding config
setting.
2025-02-21 18:42:16 +01:00
Hugo Dutka 44499315ed chore: reduce log volume on server startup (#16608)
Addresses https://github.com/coder/coder/issues/16231.

This PR reduces the volume of logs we print after server startup in
order to surface the web UI URL better.

Here are the logs after the changes a couple of seconds after starting
the server:

<img width="868" alt="Screenshot 2025-02-18 at 16 31 32"
src="https://github.com/user-attachments/assets/786dc4b8-7383-48c8-a5c3-a997c01ca915"
/>

The warning is due to running a development site-less build. It wouldn't
show in a release build.
2025-02-20 16:33:14 +01:00
Hugo Dutka 5e96fb5985 fix: explicitly set encoding to UTF8 on embedded postgres (#16604)
Fixes https://github.com/coder/coder/issues/16228.

I've verified that the setting does not affect existing databases.
2025-02-18 15:29:10 +01:00
Hugo Dutka a68d11506c chore: track disabled telemetry (#16347)
Addresses https://github.com/coder/nexus/issues/116.

## Core Concept

Send one final telemetry report after the user disables telemetry with
the message that the telemetry was disabled. No other information about
the deployment is sent in this report.

This final report is submitted only if the deployment ever had telemetry
on.

## Changes

1. Refactored how our telemetry is initialized.
2. Introduced the `TelemetryEnabled` telemetry item, which allows to
decide whether a final report should be sent.
3. Added the `RecordTelemetryStatus` telemetry method, which decides
whether a final report should be sent and updates the telemetry item.
4. Added tests to ensure the implementation is correct.
2025-02-03 14:50:55 +01:00
Hugo Dutka 2ace044e0b chore: track the first time html is served in telemetry (#16334)
Addresses https://github.com/coder/nexus/issues/175.

## Changes

- Adds the `telemetry_items` database table. It's a key value store for
telemetry events that don't fit any other database tables.
- Adds a telemetry report when HTML is served for the first time in
`site.go`.
2025-01-31 13:55:46 +01:00
Hugo Dutka 92d22e296b chore: track usage of organizations in telemetry (#16323)
Addresses https://github.com/coder/internal/issues/317.

## Changes

Requirements are quoted below:

> how many orgs does deployment have

Adds the Organization entity to telemetry.

> ensuring resources are associated with orgs

All resources that reference an org already report the org id to
telemetry. Adds a test to check that.

> whether org sync is configured

Adds the `IDPOrgSync` boolean field to the Deployment entity.

## Implementation of the org sync check

While there's an `OrganizationSyncEnabled` method on the IDPSync
interface, I decided not to use it directly and implemented a
counterpart just for telemetry purposes. It's a compromise I'm not happy
about, but I found that it's a simpler approach than the alternative.
There are multiple reasons:

1. The telemetry package cannot statically access the IDPSync interface
due to a circular import.
2. We can't dynamically pass a reference to the
`OrganizationSyncEnabled` function at the time of instantiating the
telemetry object, because our server initialization logic depends on the
telemetry object being created before the IDPSync object.
3. If we circumvent that problem by passing the reference as an
initially empty pointer, initializing telemetry, then IDPSync, then
updating the pointer to point to `OrganizationSyncEnabled`, we have to
refactor the initialization logic of the telemetry object itself to
avoid a race condition where the first telemetry report is performed
without a valid reference.

I actually implemented that approach in
https://github.com/coder/coder/pull/16307, but realized I'm unable to
fully test it. It changed the initialization order in the server
command, and I wanted to test our CLI with Org Sync configured with a
premium license. As far as I'm aware, we don't have the tooling to do
that. I couldn't figure out a way to start the CLI with a mock license,
and I didn't want to go down further into the refactoring rabbit hole.

So I decided that reimplementing the org sync checking logic is simpler.
2025-01-29 15:54:31 +01:00
Hugo Dutka 23cf61aff6 chore: add the --ephemeral server flag (#16126)
Another PR to address https://github.com/coder/coder/issues/15109.

Changes:
- Introduces the `--ephemeral` flag, which changes the Coder config
directory to a temporary location. The config directory is where the
built-in PostgreSQL stores its data, so using a new one results in a
deployment with a fresh state.

The `--ephemeral` flag is set to replace the `--in-memory` flag once the
in-memory database is removed.
2025-01-20 14:31:16 +01:00
Hugo Dutka dd29997b9c chore: reduce parallelism for test-go-pg on macOS (#16116)
We're seeing test-go-pg flakes on macOS in CI. We've had the same
problem on Windows, and reducing test parallelism in
https://github.com/coder/coder/pull/16090 seemed to help. This PR makes
the same change on macOS.
2025-01-13 16:57:48 +01:00
Hugo Dutka 899836d47a chore: reduce Windows PG tests flakiness (#16090)
This PR:

- Reduces test parallelism on Windows in CI
- Unifies wait intervals on Windows with Linux and macOS. Previously we
had custom intervals for Windows to reduce test flakiness on smaller CI
workers, but we don't run tests on small CI workers anymore. Due to how
our CI file is defined, forks run tests on small CI machines, but I'm
not sure if the different intervals actually help or whether that's a
heuristic that happened to fix issues on a particular day and was it
ever reevaluated. I propose we make the change and if someone complains,
revert it.

In particular, reduced test parallelism seems to actually help: I was
able to run Windows tests 5 times in a row without flakes. Not sure if
that's going to fix the problem long term, but it seems worth trying.
2025-01-10 15:21:03 +01:00
Hugo Dutka 106b1cd3bc chore: convert dbauthz tests to also run with Postgres (#15862)
Another PR to address https://github.com/coder/coder/issues/15109.

- adds the DisableForeignKeysAndTriggers utility, which simplifies
converting tests from in-mem to postgres
- converts the dbauthz test suite to pass on both the in-mem db and
Postgres
2025-01-08 16:22:51 +01:00
Hugo Dutka 75f417630b fix: persist the devtunnel file in a docker volume (#15731)
Addresses https://github.com/coder/coder/issues/15663.

This PR saves the entire coder home directory in a Docker volume to make
the dev tunnel URL persistent across container restarts.

I initially wanted to persist only the config directory, but Docker
Compose cannot set permissions on a named volume unless the directory
it’s mounted on already exists within the container. The
`/home/coder/.config` directory, however, is not created by default in
the Dockerfile. When I attempt to mount it, [Docker creates it with root
permissions](https://github.com/moby/moby/issues/2259#issue-21132999),
and Coder cannot write to it. I encounter the following error:

```
coder-1     | Started HTTP listener at http://0.0.0.0:7080
coder-1     | Opening tunnel so workspaces can connect to your deployment. For production scenarios, specify an external access URL
coder-1     | Encountered an error running "coder server", see "coder server --help" for more information
coder-1     | error: create tunnel: read or generate config: get config path: mkdirall config dir "/home/coder/.config/coderv2": mkdir /home/coder/.config/coderv2: permission denied
```

Creating the directory in the Dockerfile would resolve the issue for new
images but would break `docker-compose.yml` for all existing Coder
images. Mounting the entire home directory avoids this problem, but it
makes it less clear to admins which files need to be persisted. It’s a
trade-off - I believe keeping Docker Compose backwards-compatible is
more important, and I hope the added comment clarifies the purpose of
the volume for new users.
2024-12-04 12:21:54 +01:00
Hugo Dutka c7c35ef4d7 chore: run macOS, windows, and race tests with Postgres in CI (#15520)
This PR is the second in a series aimed at closing
https://github.com/coder/coder/issues/15109.

## Changes

- adds `scripts/embedded-pg/main.go`, which can start a native Postgres
database. This is used to set up PG on Windows and macOS, as these
platforms don't support Docker in Github Actions.
- runs the `test-go-pg` job on macOS and Windows too
- adds the `test-go-race-go` job, which runs race tests with Postgres on
Linux
2024-12-03 13:33:17 +01:00
Hugo Dutka b830c05e3e chore: track usage of built-in example templates (#15671)
Addresses https://github.com/coder/nexus/issues/99.

Changes:
- Save the id of the built-in example template used to create a template
version in the database
- Include the example id in telemetry
2024-11-27 20:01:08 +01:00
Hugo Dutka 83c493e832 chore: fix more flaky tests on Windows with Postgres (#15629)
Addresses the following flakes:

- https://github.com/coder/internal/issues/222
- https://github.com/coder/internal/issues/223
- https://github.com/coder/internal/issues/224
- https://github.com/coder/internal/issues/225
- https://github.com/coder/internal/issues/226
- https://github.com/coder/internal/issues/227
- https://github.com/coder/internal/issues/228
- https://github.com/coder/internal/issues/229
- https://github.com/coder/internal/issues/230
2024-11-26 11:56:07 +01:00
Hugo Dutka 6ed76921dd chore: fix windows postgres tests (#15593)
Patches tests that caused Windows Postgres CI in
https://github.com/coder/coder/pull/15520 to consistently fail.

I tested this by temporarily adding Postgres Windows CI to this PR.
However, I reverted those changes to merge them with
https://github.com/coder/coder/pull/15520. For reference, here's [a
passing CI
run](https://github.com/coder/coder/actions/runs/11918816662/job/33219786238)
from an earlier commit.

**Note:** Although Windows tests now pass, they remain quite flaky. I
recommend running Postgres Windows CI to gather data on these flakes,
but I don’t think it should be a required job just yet.
2024-11-20 13:30:31 +01:00
Hugo Dutka 97ce44a77d chore: track terraform module source type in telemetry (#15590) 2024-11-20 11:03:48 +01:00
Hugo Dutka aa0dc2daa1 chore: track terraform modules in telemetry (#15450)
Addresses https://github.com/coder/nexus/issues/35.

This PR:

- Adds a `workspace_modules` table to track modules used by the
Terraform provisioner in provisioner jobs.
- Adds a `module_path` column to the `workspace_resources` table,
allowing to identify which module a resource originates from.
- Starts pushing this new information into telemetry.

For the person reviewing this PR, do not fret about the 1,500 new lines
- ~1,000 of them are auto-generated.
2024-11-16 21:56:19 +01:00
Hugo Dutka f85ef84cc5 fix: use container with Postgres 13 for dump.sql (#15411) 2024-11-07 16:49:26 +01:00
Hugo Dutka 1bfa7d42e8 chore: add postgres template caching for tests (#15336)
This PR is the first in a series aimed at closing
[#15109](https://github.com/coder/coder/issues/15109).

### Changes

- **Template Database Creation:**  
`dbtestutil.Open` now has the ability to create a template database if
none is provided via `DB_FROM`. The template database’s name is derived
from a hash of the migration files, ensuring that it can be reused
across tests and is automatically updated whenever migrations change.

- **Optimized Database Handling:**  
Previously, `dbtestutil.Open` would spin up a new container for each
test when `DB_FROM` was unset. Now, it first checks for an active
PostgreSQL instance on `localhost:5432`. If none is found, it creates a
single container that remains available for subsequent tests,
eliminating repeated container startups.

These changes address the long individual test times (10+ seconds)
reported by some users, likely due to the time Docker took to start and
complete migrations.
2024-11-04 17:23:31 +01:00