8000 fix(cli): add check for DisableOwnerWorkspaceExec in scaletest (#14417) · coder/coder@6914862 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6914862

Browse files
johnstcnjohnstcn
authored
fix(cli): add check for DisableOwnerWorkspaceExec in scaletest (#14417)
- Adds `--use-host-login` to `coder exp scaletest workspace-traffic` - Modifies getScaletestWorkspaces to conditionally filter workspaces if `CODER_DISABLE_OWNER_WORKSPACE_ACCESS` is set - Adds a warning if `CODER_DISABLE_OWNER_WORKSPACE_ACCESS` is set and scaletest workspaces are filtered out due to ownership mismatch. - Modifies `coderdtest.New` to detect cross-test bleed of `CODER_DISABLE_OWNER_WORKSPACE_ACCESS` and fast-fail.
1 parent c8eacc6 commit 6914862

File tree

3 files changed

+125
-11
lines changed

3 files changed

+125
-11
lines changed

‎cli/exp_scaletest.go

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ func (s *scaletestTracingFlags) provider(ctx context.Context) (trace.TracerProvi
117117
}
118118

119119
var closeTracingOnce sync.Once
120-
return tracerProvider, func(ctx context.Context) error {
120+
return tracerProvider, func(_ context.Context) error {
121121
var err error
122122
closeTracingOnce.Do(func() {
123123
// Allow time to upload traces even if ctx is canceled
@@ -430,7 +430,7 @@ func (r *RootCmd) scaletestCleanup() *serpent.Command {
430430
}
431431

432432
cliui.Infof(inv.Stdout, "Fetching scaletest workspaces...")
433-
workspaces, err := getScaletestWorkspaces(ctx, client, template)
433+
workspaces, _, err := getScaletestWorkspaces(ctx, client, "", template)
434434
if err != nil {
435435
return err
436436
}
@@ -863,6 +863,7 @@ func (r *RootCmd) scaletestWorkspaceTraffic() *serpent.Command {
863863
tickInterval time.Duration
864864
bytesPerTick int64
865865
ssh bool
866+
useHostLogin bool
866867
app string
867868
template string
868869
targetWorkspaces string
@@ -926,10 +927,18 @@ func (r *RootCmd) scaletestWorkspaceTraffic() *serpent.Command {
926927
return xerrors.Errorf("get app host: %w", err)
927928
}
928929

929-
workspaces, err := getScaletestWorkspaces(inv.Context(), client, template)
930+
var owner string
931+
if useHostLogin {
932+
owner = codersdk.Me
933+
}
934+
935+
workspaces, numSkipped, err := getScaletestWorkspaces(inv.Context(), client, owner, template)
930936
if err != nil {
931937
return err
932938
}
939+
if numSkipped > 0 {
940+
cliui.Warnf(inv.Stdout, "CODER_DISABLE_OWNER_WORKSPACE_ACCESS is set on the deployment.\n\t%d workspace(s) were skipped due to ownership mismatch.\n\tSet --use-host-login to only target workspaces you own.", numSkipped)
941+
}
933942

934943
if targetWorkspaceEnd == 0 {
935944
targetWorkspaceEnd = len(workspaces)
@@ -1092,6 +1101,13 @@ func (r *RootCmd) scaletestWorkspaceTraffic() *serpent.Command {
10921101
Description: "Send WebSocket traffic to a workspace app (proxied via coderd), cannot be used with --ssh.",
10931102
Value: serpent.StringOf(&app),
10941103
},
1104+
{
1105+
Flag: "use-host-login",
1106+
Env: "CODER_SCALETEST_USE_HOST_LOGIN",
1107+
Default: "false",
1108+
Description: "Connect as the currently logged in user.",
1109+
Value: serpent.BoolOf(&useHostLogin),
1110+
},
10951111
}
10961112

10971113
tracingFlags.attach(&cmd.Options)
@@ -1378,22 +1394,35 @@ func isScaleTestWorkspace(workspace codersdk.Workspace) bool {
13781394
strings.HasPrefix(workspace.Name, "scaletest-")
13791395
}
13801396

1381-
func getScaletestWorkspaces(ctx context.Context, client *codersdk.Client, template string) ([]codersdk.Workspace, error) {
1397+
func getScaletestWorkspaces(ctx context.Context, client *codersdk.Client, owner, template string) ([]codersdk.Workspace, int, error) {
13821398
var (
13831399
pageNumber = 0
13841400
limit = 100
13851401
workspaces []codersdk.Workspace
1402+
skipped int
13861403
)
13871404

1405+
me, err := client.User(ctx, codersdk.Me)
1406+
if err != nil {
1407+
return nil, 0, xerrors.Errorf("check logged-in user")
1408+
}
1409+
1410+
dv, err := client.DeploymentConfig(ctx)
1411+
if err != nil {
1412+
return nil, 0, xerrors.Errorf("fetch deployment config: %w", err)
1413+
}
1414+
noOwnerAccess := dv.Values != nil && dv.Values.DisableOwnerWorkspaceExec.Value()
1415+
13881416
for {
13891417
page, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{
13901418
Name: "scaletest-",
13911419
Template: template,
1420+
Owner: owner,
13921421
Offset: pageNumber * limit,
13931422
Limit: limit,
13941423
})
13951424
if err != nil {
1396-
return nil, xerrors.Errorf("fetch scaletest workspaces page %d: %w", pageNumber, err)
1425+
return nil, 0, xerrors.Errorf("fetch scaletest workspaces page %d: %w", pageNumber, err)
13971426
}
13981427

13991428
pageNumber++
@@ -1403,13 +1432,18 @@ func getScaletestWorkspaces(ctx context.Context, client *codersdk.Client, templa
14031432

14041433
pageWorkspaces := make([]codersdk.Workspace, 0, len(page.Workspaces))
14051434
for _, w := range page.Workspaces {
1406-
if isScaleTestWorkspace(w) {
1407-
pageWorkspaces = append(pageWorkspaces, w)
1435+
if !isScaleTestWorkspace(w) {
1436+
continue
1437+
}
1438+
if noOwnerAccess && w.OwnerID != me.ID {
1439+
skipped++
1440+
continue
14081441
}
1442+
pageWorkspaces = append(pageWorkspaces, w)
14091443
}
14101444
workspaces = append(workspaces, pageWorkspaces...)
14111445
}
1412-
return workspaces, nil
1446+
return workspaces, skipped, nil
14131447
}
14141448

14151449
func getScaletestUsers(ctx context.Context, client *codersdk.Client) ([]codersdk.User, error) {

‎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("Side-effect of DisableOwnerWorkspaceExec detected in unrelated test. Please move the test that requires DisableOwnerWorkspaceExec 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)
0