From 5d5b5aa07417e49e4ba5f74f4e301b5d6e2041f3 Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Tue, 21 Nov 2023 16:22:08 +0400 Subject: [PATCH] chore: use dbfake for ssh tests rather than provisionerd (#10812) Refactors SSH tests to skip provisionerd and instead use dbfake to insert workspaces and builds. This should make tests faster and more reliable. dbfake.WorkspaceBuild is refactored to use a "builder" pattern with "fluent" options, as the number of options and variants was starting to get out of hand. --- cli/ping_test.go | 2 +- cli/speedtest_test.go | 2 +- cli/ssh_test.go | 128 +++++++++++++++---------------- cli/vscodessh_test.go | 2 +- coderd/database/dbfake/dbfake.go | 122 +++++++++++++++++++++-------- 5 files changed, 152 insertions(+), 104 deletions(-) diff --git a/cli/ping_test.go b/cli/ping_test.go index f2bd4b5ff8..55787bb39b 100644 --- a/cli/ping_test.go +++ b/cli/ping_test.go @@ -19,7 +19,7 @@ func TestPing(t *testing.T) { t.Run("OK", func(t *testing.T) { t.Parallel() - client, workspace, agentToken := setupWorkspaceForAgent(t, nil) + client, workspace, agentToken := setupWorkspaceForAgent(t) inv, root := clitest.New(t, "ping", workspace.Name) clitest.SetupConfig(t, client, root) pty := ptytest.New(t) diff --git a/cli/speedtest_test.go b/cli/speedtest_test.go index 66dcab5abb..9878ff04ab 100644 --- a/cli/speedtest_test.go +++ b/cli/speedtest_test.go @@ -23,7 +23,7 @@ func TestSpeedtest(t *testing.T) { if testing.Short() { t.Skip("This test takes a minimum of 5ms per a hardcoded value in Tailscale!") } - client, workspace, agentToken := setupWorkspaceForAgent(t, nil) + client, workspace, agentToken := setupWorkspaceForAgent(t) _ = agenttest.New(t, client.URL, agentToken) coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID) diff --git a/cli/ssh_test.go b/cli/ssh_test.go index 1a503b55f8..a58811b26b 100644 --- a/cli/ssh_test.go +++ b/cli/ssh_test.go @@ -21,13 +21,11 @@ import ( "testing" "time" - "golang.org/x/xerrors" - - "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/crypto/ssh" gosshagent "golang.org/x/crypto/ssh/agent" + "golang.org/x/xerrors" "cdr.dev/slog" "cdr.dev/slog/sloggers/slogtest" @@ -38,63 +36,28 @@ import ( "github.com/coder/coder/v2/cli/cliui" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbfake" + "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/codersdk" - "github.com/coder/coder/v2/provisioner/echo" "github.com/coder/coder/v2/provisionersdk/proto" "github.com/coder/coder/v2/pty" "github.com/coder/coder/v2/pty/ptytest" "github.com/coder/coder/v2/testutil" ) -const ( - startupScriptPattern = "i-am-ready" -) - -func setupWorkspaceForAgent(t *testing.T, mutate func([]*proto.Agent) []*proto.Agent) (*codersdk.Client, codersdk.Workspace, string) { +func setupWorkspaceForAgent(t *testing.T, mutations ...func([]*proto.Agent) []*proto.Agent) (*codersdk.Client, database.Workspace, string) { t.Helper() - if mutate == nil { - mutate = func(a []*proto.Agent) []*proto.Agent { - return a - } - } - client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) - client.SetLogger(slogtest.Make(t, nil).Named("client").Leveled(slog.LevelDebug)) - user := coderdtest.CreateFirstUser(t, client) - agentToken := uuid.NewString() - version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ - Parse: echo.ParseComplete, - ProvisionPlan: echo.PlanComplete, - ProvisionApply: []*proto.Response{{ - Type: &proto.Response_Apply{ - Apply: &proto.ApplyComplete{ - Resources: []*proto.Resource{{ - Name: "dev", - Type: "google_compute_instance", - Agents: mutate([]*proto.Agent{{ - Id: uuid.NewString(), - Auth: &proto.Agent_Token{ - Token: agentToken, - }, - Scripts: []*proto.Script{ - { - Script: fmt.Sprintf("echo '%s'", startupScriptPattern), - RunOnStart: true, - }, - }, - }}), - }}, - }, - }, - }}, - }) - coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) - template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID) - workspace, err := client.Workspace(context.Background(), workspace.ID) - require.NoError(t, err) - return client, workspace, agentToken + client, store := coderdtest.NewWithDatabase(t, nil) + client.SetLogger(slogtest.Make(t, nil).Named("client").Leveled(slog.LevelDebug)) + first := coderdtest.CreateFirstUser(t, client) + userClient, user := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) + workspace, agentToken := dbfake.WorkspaceWithAgent(t, store, database.Workspace{ + OrganizationID: first.OrganizationID, + OwnerID: user.ID, + }, mutations...) + + return userClient, workspace, agentToken } func TestSSH(t *testing.T) { @@ -102,7 +65,7 @@ func TestSSH(t *testing.T) { t.Run("ImmediateExit", func(t *testing.T) { t.Parallel() - client, workspace, agentToken := setupWorkspaceForAgent(t, nil) + client, workspace, agentToken := setupWorkspaceForAgent(t) inv, root := clitest.New(t, "ssh", workspace.Name) clitest.SetupConfig(t, client, root) pty := ptytest.New(t).Attach(inv) @@ -159,9 +122,17 @@ func TestSSH(t *testing.T) { t.Skip("Windows doesn't seem to clean up the process, maybe #7100 will fix it") } - client, workspace, agentToken := setupWorkspaceForAgent(t, nil) + store, ps := dbtestutil.NewDB(t) + client := coderdtest.New(t, &coderdtest.Options{Pubsub: ps, Database: store}) + client.SetLogger(slogtest.Make(t, nil).Named("client").Leveled(slog.LevelDebug)) + first := coderdtest.CreateFirstUser(t, client) + userClient, user := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) + workspace, agentToken := dbfake.WorkspaceWithAgent(t, store, database.Workspace{ + OrganizationID: first.OrganizationID, + OwnerID: user.ID, + }) inv, root := clitest.New(t, "ssh", workspace.Name) - clitest.SetupConfig(t, client, root) + clitest.SetupConfig(t, userClient, root) pty := ptytest.New(t).Attach(inv) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) @@ -180,7 +151,13 @@ func TestSSH(t *testing.T) { pty.WriteLine("echo hell'o'") pty.ExpectMatchContext(ctx, "hello") - workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + _ = dbfake.WorkspaceBuildBuilder(t, store, workspace). + Seed(database.WorkspaceBuild{ + Transition: database.WorkspaceTransitionStop, + BuildNumber: 2, + }). + Pubsub(ps).Do() + t.Log("stopped workspace") select { case <-cmdDone: @@ -191,7 +168,7 @@ func TestSSH(t *testing.T) { t.Run("Stdio", func(t *testing.T) { t.Parallel() - client, workspace, agentToken := setupWorkspaceForAgent(t, nil) + client, workspace, agentToken := setupWorkspaceForAgent(t) _, _ = tGoContext(t, func(ctx context.Context) { // Run this async so the SSH command has to wait for // the build and agent to connect! @@ -251,7 +228,7 @@ func TestSSH(t *testing.T) { t.Run("Stdio_RemoteForward_Signal", func(t *testing.T) { t.Parallel() - client, workspace, agentToken := setupWorkspaceForAgent(t, nil) + client, workspace, agentToken := setupWorkspaceForAgent(t) _, _ = tGoContext(t, func(ctx context.Context) { // Run this async so the SSH command has to wait for // the build and agent to connect! @@ -312,7 +289,7 @@ func TestSSH(t *testing.T) { t.Run("Stdio_BrokenConn", func(t *testing.T) { t.Parallel() - client, workspace, agentToken := setupWorkspaceForAgent(t, nil) + client, workspace, agentToken := setupWorkspaceForAgent(t) _, _ = tGoContext(t, func(ctx context.Context) { // Run this async so the SSH command has to wait for // the build and agent to connect! @@ -373,7 +350,7 @@ func TestSSH(t *testing.T) { } t.Parallel() ctx := testutil.Context(t, testutil.WaitSuperLong) - client, workspace, agentToken := setupWorkspaceForAgent(t, nil) + client, workspace, agentToken := setupWorkspaceForAgent(t) _, _ = tGoContext(t, func(ctx context.Context) { // Run this async so the SSH command has to wait for // the build and agent to connect! @@ -483,7 +460,17 @@ func TestSSH(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("Windows doesn't seem to clean up the process, maybe #7100 will fix it") } - client, workspace, agentToken := setupWorkspaceForAgent(t, nil) + + store, ps := dbtestutil.NewDB(t) + client := coderdtest.New(t, &coderdtest.Options{Pubsub: ps, Database: store}) + client.SetLogger(slogtest.Make(t, nil).Named("client").Leveled(slog.LevelDebug)) + first := coderdtest.CreateFirstUser(t, client) + userClient, user := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) + workspace, agentToken := dbfake.WorkspaceWithAgent(t, store, database.Workspace{ + OrganizationID: first.OrganizationID, + OwnerID: user.ID, + }) + _, _ = tGoContext(t, func(ctx context.Context) { // Run this async so the SSH command has to wait for // the build and agent to connect. @@ -503,7 +490,7 @@ func TestSSH(t *testing.T) { defer cancel() inv, root := clitest.New(t, "ssh", "--stdio", workspace.Name) - clitest.SetupConfig(t, client, root) + clitest.SetupConfig(t, userClient, root) inv.Stdin = clientOutput inv.Stdout = serverInput inv.Stderr = io.Discard @@ -533,7 +520,14 @@ func TestSSH(t *testing.T) { err = session.Shell() require.NoError(t, err) - workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + _ = dbfake.WorkspaceBuildBuilder(t, store, workspace). + Seed(database.WorkspaceBuild{ + Transition: database.WorkspaceTransitionStop, + BuildNumber: 2, + }). + Pubsub(ps). + Do() + t.Log("stopped workspace") select { case <-cmdDone: @@ -549,7 +543,7 @@ func TestSSH(t *testing.T) { t.Parallel() - client, workspace, agentToken := setupWorkspaceForAgent(t, nil) + client, workspace, agentToken := setupWorkspaceForAgent(t) _ = agenttest.New(t, client.URL, agentToken) coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID) @@ -635,7 +629,7 @@ func TestSSH(t *testing.T) { })) defer httpServer.Close() - client, workspace, agentToken := setupWorkspaceForAgent(t, nil) + client, workspace, agentToken := setupWorkspaceForAgent(t) _ = agenttest.New(t, client.URL, agentToken) coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID) @@ -686,7 +680,7 @@ func TestSSH(t *testing.T) { t.Parallel() - client, workspace, agentToken := setupWorkspaceForAgent(t, nil) + client, workspace, agentToken := setupWorkspaceForAgent(t) _ = agenttest.New(t, client.URL, agentToken) coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID) @@ -733,7 +727,7 @@ func TestSSH(t *testing.T) { logDir := t.TempDir() - client, workspace, agentToken := setupWorkspaceForAgent(t, nil) + client, workspace, agentToken := setupWorkspaceForAgent(t) inv, root := clitest.New(t, "ssh", "-l", logDir, workspace.Name) clitest.SetupConfig(t, client, root) pty := ptytest.New(t).Attach(inv) @@ -908,7 +902,7 @@ Expire-Date: 0 workspaceAgentSocketPath := strings.TrimSpace(stdout.String()) require.NotEqual(t, extraSocketPath, workspaceAgentSocketPath, "socket path should be different") - client, workspace, agentToken := setupWorkspaceForAgent(t, nil) + client, workspace, agentToken := setupWorkspaceForAgent(t) _ = agenttest.New(t, client.URL, agentToken, func(o *agent.Options) { o.EnvironmentVariables = map[string]string{ diff --git a/cli/vscodessh_test.go b/cli/vscodessh_test.go index 2fc83cdb58..a4f6ca1913 100644 --- a/cli/vscodessh_test.go +++ b/cli/vscodessh_test.go @@ -22,7 +22,7 @@ import ( func TestVSCodeSSH(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitLong) - client, workspace, agentToken := setupWorkspaceForAgent(t, nil) + client, workspace, agentToken := setupWorkspaceForAgent(t) user, err := client.User(ctx, codersdk.Me) require.NoError(t, err) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 9d7e0946d6..85164da329 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -14,8 +14,10 @@ import ( "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/database/pubsub" "github.com/coder/coder/v2/coderd/provisionerdserver" "github.com/coder/coder/v2/coderd/telemetry" + "github.com/coder/coder/v2/codersdk" sdkproto "github.com/coder/coder/v2/provisionersdk/proto" ) @@ -39,44 +41,83 @@ func Workspace(t testing.TB, db database.Store, seed database.Workspace) databas // WorkspaceWithAgent is a helper that generates a workspace with a single resource // that has an agent attached to it. The agent token is returned. -func WorkspaceWithAgent(t testing.TB, db database.Store, seed database.Workspace) (database.Workspace, string) { +func WorkspaceWithAgent( + t testing.TB, db database.Store, seed database.Workspace, + mutations ...func([]*sdkproto.Agent) []*sdkproto.Agent, +) ( + database.Workspace, string, +) { t.Helper() authToken := uuid.NewString() + agents := []*sdkproto.Agent{{ + Id: uuid.NewString(), + Auth: &sdkproto.Agent_Token{ + Token: authToken, + }, + }} + for _, m := range mutations { + agents = m(agents) + } ws := Workspace(t, db, seed) WorkspaceBuild(t, db, ws, database.WorkspaceBuild{}, &sdkproto.Resource{ - Name: "example", - Type: "aws_instance", - Agents: []*sdkproto.Agent{{ - Id: uuid.NewString(), - Auth: &sdkproto.Agent_Token{ - Token: authToken, - }, - }}, + Name: "example", + Type: "aws_instance", + Agents: agents, }) return ws, authToken } -// WorkspaceBuild inserts a build and a successful job into the database. -func WorkspaceBuild(t testing.TB, db database.Store, ws database.Workspace, seed database.WorkspaceBuild, resources ...*sdkproto.Resource) database.WorkspaceBuild { - t.Helper() +type BuildBuilder struct { + t testing.TB + db database.Store + ps pubsub.Pubsub + ws database.Workspace + seed database.WorkspaceBuild + resources []*sdkproto.Resource +} + +func WorkspaceBuildBuilder(t testing.TB, db database.Store, ws database.Workspace) BuildBuilder { + return BuildBuilder{t: t, db: db, ws: ws} +} + +func (b BuildBuilder) Pubsub(ps pubsub.Pubsub) BuildBuilder { + //nolint: revive // returns modified struct + b.ps = ps + return b +} + +func (b BuildBuilder) Seed(seed database.WorkspaceBuild) BuildBuilder { + //nolint: revive // returns modified struct + b.seed = seed + return b +} + +func (b BuildBuilder) Resource(resource *sdkproto.Resource) BuildBuilder { + //nolint: revive // returns modified struct + b.resources = append(b.resources, resource) + return b +} + +func (b BuildBuilder) Do() database.WorkspaceBuild { + b.t.Helper() jobID := uuid.New() - seed.ID = uuid.New() - seed.JobID = jobID - seed.WorkspaceID = ws.ID + b.seed.ID = uuid.New() + b.seed.JobID = jobID + b.seed.WorkspaceID = b.ws.ID // Create a provisioner job for the build! payload, err := json.Marshal(provisionerdserver.WorkspaceProvisionJob{ - WorkspaceBuildID: seed.ID, + WorkspaceBuildID: b.seed.ID, }) - require.NoError(t, err) + require.NoError(b.t, err) //nolint:gocritic // This is only used by tests. ctx := dbauthz.AsSystemRestricted(context.Background()) - job, err := db.InsertProvisionerJob(ctx, database.InsertProvisionerJobParams{ + job, err := b.db.InsertProvisionerJob(ctx, database.InsertProvisionerJobParams{ ID: jobID, CreatedAt: dbtime.Now(), UpdatedAt: dbtime.Now(), - OrganizationID: ws.OrganizationID, - InitiatorID: ws.OwnerID, + OrganizationID: b.ws.OrganizationID, + InitiatorID: b.ws.OwnerID, Provisioner: database.ProvisionerTypeEcho, StorageMethod: database.ProvisionerStorageMethodFile, FileID: uuid.New(), @@ -85,8 +126,8 @@ func WorkspaceBuild(t testing.TB, db database.Store, ws database.Workspace, seed Tags: nil, TraceMetadata: pqtype.NullRawMessage{}, }) - require.NoError(t, err, "insert job") - err = db.UpdateProvisionerJobWithCompleteByID(ctx, database.UpdateProvisionerJobWithCompleteByIDParams{ + require.NoError(b.t, err, "insert job") + err = b.db.UpdateProvisionerJobWithCompleteByID(ctx, database.UpdateProvisionerJobWithCompleteByIDParams{ ID: job.ID, UpdatedAt: dbtime.Now(), Error: sql.NullString{}, @@ -96,27 +137,27 @@ func WorkspaceBuild(t testing.TB, db database.Store, ws database.Workspace, seed Valid: true, }, }) - require.NoError(t, err, "complete job") + require.NoError(b.t, err, "complete job") // This intentionally fulfills the minimum requirements of the schema. // Tests can provide a custom version ID if necessary. - if seed.TemplateVersionID == uuid.Nil { + if b.seed.TemplateVersionID == uuid.Nil { jobID := uuid.New() - templateVersion := dbgen.TemplateVersion(t, db, database.TemplateVersion{ + templateVersion := dbgen.TemplateVersion(b.t, b.db, database.TemplateVersion{ JobID: jobID, - OrganizationID: ws.OrganizationID, - CreatedBy: ws.OwnerID, + OrganizationID: b.ws.OrganizationID, + CreatedBy: b.ws.OwnerID, TemplateID: uuid.NullUUID{ - UUID: ws.TemplateID, + UUID: b.ws.TemplateID, Valid: true, }, }) payload, _ := json.Marshal(provisionerdserver.TemplateVersionImportJob{ TemplateVersionID: templateVersion.ID, }) - dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{ + dbgen.ProvisionerJob(b.t, b.db, nil, database.ProvisionerJob{ ID: jobID, - OrganizationID: ws.OrganizationID, + OrganizationID: b.ws.OrganizationID, Input: payload, Type: database.ProvisionerJobTypeTemplateVersionImport, CompletedAt: sql.NullTime{ @@ -124,14 +165,27 @@ func WorkspaceBuild(t testing.TB, db database.Store, ws database.Workspace, seed Valid: true, }, }) - ProvisionerJobResources(t, db, jobID, seed.Transition, resources...) - seed.TemplateVersionID = templateVersion.ID + ProvisionerJobResources(b.t, b.db, jobID, b.seed.Transition, b.resources...) + b.seed.TemplateVersionID = templateVersion.ID + } + build := dbgen.WorkspaceBuild(b.t, b.db, b.seed) + ProvisionerJobResources(b.t, b.db, job.ID, b.seed.Transition, b.resources...) + if b.ps != nil { + err = b.ps.Publish(codersdk.WorkspaceNotifyChannel(build.WorkspaceID), []byte{}) + require.NoError(b.t, err) } - build := dbgen.WorkspaceBuild(t, db, seed) - ProvisionerJobResources(t, db, job.ID, seed.Transition, resources...) return build } +// WorkspaceBuild inserts a build and a successful job into the database. +func WorkspaceBuild(t testing.TB, db database.Store, ws database.Workspace, seed database.WorkspaceBuild, resources ...*sdkproto.Resource) database.WorkspaceBuild { + b := WorkspaceBuildBuilder(t, db, ws).Seed(seed) + for _, r := range resources { + b = b.Resource(r) + } + return b.Do() +} + // ProvisionerJobResources inserts a series of resources into a provisioner job. func ProvisionerJobResources(t testing.TB, db database.Store, job uuid.UUID, transition database.WorkspaceTransition, resources ...*sdkproto.Resource) { t.Helper()