Skip to content

Commit e6ed3c3

Browse files
johnstcnjohnstcn
committed
fix DisableOwnerWorkspaceExec breaking other tests
1 parent 61ed855 commit e6ed3c3

File tree

3 files changed

+83
-58
lines changed

3 files changed

+83
-58
lines changed

‎cli/exp_scaletest_test.go

Lines changed: 0 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package cli_test
22

33
import (
4-
"bytes"
54
"context"
65
"path/filepath"
76
"testing"
@@ -12,7 +11,6 @@ import (
1211

1312
"github.com/coder/coder/v2/cli/clitest"
1413
"github.com/coder/coder/v2/coderd/coderdtest"
15-
"github.com/coder/coder/v2/codersdk"
1614
"github.com/coder/coder/v2/pty/ptytest"
1715
"github.com/coder/coder/v2/testutil"
1816
)
@@ -118,59 +116,6 @@ func TestScaleTestWorkspaceTraffic_Template(t *testing.T) {
118116
require.ErrorContains(t, err, "could not find template \"doesnotexist\" in any organization")
119117
}
120118

121-
// This test validates that the scaletest CLI filters out workspaces not owned
122-
// when disable owner workspace access is set.
123-
// nolint:paralleltest // DisableOwnerWorkspaceExec is not safe for parallel tests.
124-
func TestScaleTestWorkspaceTraffic_UseHostLogin(t *testing.T) {
125-
ctx, cancelFunc := context.WithTimeout(context.Background(), testutil.WaitMedium)
126-
defer cancelFunc()
127-
128-
log := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
129-
client := coderdtest.New(t, &coderdtest.Options{
130-
Logger: &log,
131-
IncludeProvisionerDaemon: true,
132-
DeploymentValues: coderdtest.DeploymentValues(t, func(dv *codersdk.DeploymentValues) {
133-
dv.DisableOwnerWorkspaceExec = true
134-
}),
135-
})
136-
owner := coderdtest.CreateFirstUser(t, client)
137-
tv := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil)
138-
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, tv.ID)
139-
tpl := coderdtest.CreateTemplate(t, client, owner.OrganizationID, tv.ID)
140-
// Create a workspace owned by a different user
141-
memberClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
142-
_ = coderdtest.CreateWorkspace(t, memberClient, tpl.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
143-
cwr.Name = "scaletest-workspace"
144-
})
145-
146-
// Test without --use-host-login first.
147-
inv, root := clitest.New(t, "exp", "scaletest", "workspace-traffic",
148-
"--template", tpl.Name,
149-
)
150-
// nolint:gocritic // We are intentionally testing this as the owner.
151-
clitest.SetupConfig(t, client, root)
152-
var stdoutBuf bytes.Buffer
153-
inv.Stdout = &stdoutBuf
154-
155-
err := inv.WithContext(ctx).Run()
156-
require.ErrorContains(t, err, "no scaletest workspaces exist")
157-
require.Contains(t, stdoutBuf.String(), `1 workspace(s) were skipped`)
158-
159-
// Test once again with --use-host-login.
160-
inv, root = clitest.New(t, "exp", "scaletest", "workspace-traffic",
161-
"--template", tpl.Name,
162-
"--use-host-login",
163-
)
164-
// nolint:gocritic // We are intentionally testing this as the owner.
165-
clitest.SetupConfig(t, client, root)
166-
stdoutBuf.Reset()
167-
inv.Stdout = &stdoutBuf
168-
169-
err = inv.WithContext(ctx).Run()
170-
require.ErrorContains(t, err, "no scaletest workspaces exist")
171-
require.NotContains(t, stdoutBuf.String(), `1 workspace(s) were skipped`)
172-
}
173-
174119
// This test just validates that the CLI command accepts its known arguments.
175120
func TestScaleTestWorkspaceTraffic_TargetWorkspaces(t *testing.T) {
176121
t.Parallel()

‎cli/exptest/exptest_scaletest_test.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
package exptest_test
2+
3+
import (
4+
"bytes"
5+
"context"
6+
"testing"
7+
8+
"github.com/stretchr/testify/require"
9+
10+
"cdr.dev/slog/sloggers/slogtest"
11+
"github.com/coder/coder/v2/cli/clitest"
12+
"github.com/coder/coder/v2/coderd/coderdtest"
13+
"github.com/coder/coder/v2/codersdk"
14+
"github.com/coder/coder/v2/testutil"
15+
)
16+
17+
// This test validates that the scaletest CLI filters out workspaces not owned
18+
// when disable owner workspace access is set.
19+
// This test is in its own package because it mutates a global variable that
20+
// can influence other tests in the same package.
21+
// nolint:paralleltest
22+
func TestScaleTestWorkspaceTraffic_UseHostLogin(t *testing.T) {
23+
ctx, cancelFunc := context.WithTimeout(context.Background(), testutil.WaitMedium)
24+
defer cancelFunc()
25+
26+
log := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
27+
client := coderdtest.New(t, &coderdtest.Options{
28+
Logger: &log,
29+
IncludeProvisionerDaemon: true,
30+
DeploymentValues: coderdtest.DeploymentValues(t, func(dv *codersdk.DeploymentValues) {
31+
dv.DisableOwnerWorkspaceExec = true
32+
}),
33+
})
34+
owner := coderdtest.CreateFirstUser(t, client)
35+
tv := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil)
36+
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, tv.ID)
37+
tpl := coderdtest.CreateTemplate(t, client, owner.OrganizationID, tv.ID)
38+
// Create a workspace owned by a different user
39+
memberClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
40+
_ = coderdtest.CreateWorkspace(t, memberClient, tpl.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
41+
cwr.Name = "scaletest-workspace"
42+
})
43+
44+
// Test without --use-host-login first.g
45+
inv, root := clitest.New(t, "exp", "scaletest", "workspace-traffic",
46+
"--template", tpl.Name,
47+
)
48+
// nolint:gocritic // We are intentionally testing this as the owner.
49+
clitest.SetupConfig(t, client, root)
50+
var stdoutBuf bytes.Buffer
51+
inv.Stdout = &stdoutBuf
52+
53+
err := inv.WithContext(ctx).Run()
54+
require.ErrorContains(t, err, "no scaletest workspaces exist")
55+
require.Contains(t, stdoutBuf.String(), `1 workspace(s) were skipped`)
56+
57+
// Test once again with --use-host-login.
58+
inv, root = clitest.New(t, "exp", "scaletest", "workspace-traffic",
59+
"--template", tpl.Name,
60+
"--use-host-login",
61+
)
62+
// nolint:gocritic // We are intentionally testing this as the owner.
63+
clitest.SetupConfig(t, client, root)
64+
stdoutBuf.Reset()
65+
inv.Stdout = &stdoutBuf
66+
67+
err = inv.WithContext(ctx).Run()
68+
require.ErrorContains(t, err, "no scaletest workspaces exist")
69+
require.NotContains(t, stdoutBuf.String(), `1 workspace(s) were skipped`)
70+
}

‎coderd/coderdtest/coderdtest.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ import (
6666
"github.com/coder/coder/v2/coderd/httpmw"
6767
"github.com/coder/coder/v2/coderd/notifications"
6868
"github.com/coder/coder/v2/coderd/rbac"
69+
"github.com/coder/coder/v2/coderd/rbac/policy"
6970
"github.com/coder/coder/v2/coderd/schedule"
7071
"github.com/coder/coder/v2/coderd/telemetry"
7172
"github.com/coder/coder/v2/coderd/unhanger"
@@ -268,9 +269,18 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
268269
if options.DeploymentValues == nil {
269270
options.DeploymentValues = DeploymentValues(t)
270271
}
271-
// This value is not safe to run in parallel.
272-
if options.DeploymentValues.DisableOwnerWorkspaceExec {
273-
t.Logf("WARNING: DisableOwnerWorkspaceExec is set, this is not safe in parallel tests!")
272+
// DisableOwnerWorkspaceExec modifies the 'global' RBAC roles. Fast-fail tests if we detect this.
273+
if !options.DeploymentValues.DisableOwnerWorkspaceExec.Value() {
274+
ownerSubj := rbac.Subject{
275+
Roles: rbac.RoleIdentifiers{rbac.RoleOwner()},
276+
Scope: rbac.ScopeAll,
277+
}
278+
if err := options.Authorizer.Authorize(context.Background(), ownerSubj, policy.ActionSSH, rbac.ResourceWorkspace); err != nil {
279+
if rbac.IsUnauthorizedError(err) {
280+
t.Fatal("DisableOwnerWorkspaceExec was set in some other test. Please move this test to its own package so that it does not impact other tests!")
281+
}
282+
require.NoError(t, err)
283+
}
274284
}
275285

276286
// If no ratelimits are set, disable all rate limiting for tests.

0 commit comments

Comments
 (0)