fix(cli): add check for DisableOwnerWorkspaceExec in scaletest #14417
Conversation
| func TestScaleTestWorkspaceTraffic_UseHostLogin(t *testing.T) { | ||
| ctx, cancelFunc := context.WithTimeout(context.Background(), testutil.WaitMedium) | ||
| defer cancelFunc() | ||
|
|
||
| log := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}) | ||
| client := coderdtest.New(t, &coderdtest.Options{ | ||
| Logger: &log, | ||
| IncludeProvisionerDaemon: true, | ||
| DeploymentValues: coderdtest.DeploymentValues(t, func(dv *codersdk.DeploymentValues) { | ||
| dv.DisableOwnerWorkspaceExec = true | ||
| }), | ||
| }) |
There was a problem hiding this comment.
Does this test not run in go tests?
I ask because these roles are global, and it is an unfortunate consequence atm. (maybe custom roles can solve this without being global in the future).
You can see how I reset the global state here when using this option:
coder/coderd/rbac/roles_test.go
Lines 63 to 66 in e6ed3c3
and of course, no t.Parallel()
There was a problem hiding this comment.
As far as I can tell, each package is its own test binary. So the global variable would only affect tests run in the same package.
There was a problem hiding this comment.
I added a require.FailNow() and validated that the test does indeed get run.
There was a problem hiding this comment.
CI is passing, so the global-ness is not impacting the rest 🤔
There was a problem hiding this comment.
Yeah, Cian is right, you can think of each package as being built as its own test binary (basically that's what actually happens too), so global scope changes are constrained to that package. This is one of the test parallelism mechanisms in Go too (run different packages concurrently) and works irrespective of t.Parallel().
coderd/coderdtest/coderdtest.go
Outdated
| if err := options.Authorizer.Authorize(context.Background(), ownerSubj, policy.ActionSSH, rbac.ResourceWorkspace); err != nil { | ||
| if rbac.IsUnauthorizedError(err) { | ||
| 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!") | ||
| } | ||
| require.NoError(t, err) |
--use-host-logintocoder exp scaletest workspace-trafficgetScaletestWorkspacesto conditionally filter workspaces ifCODER_DISABLE_OWNER_WORKSPACE_ACCESSis setCODER_DISABLE_OWNER_WORKSPACE_ACCESSis set and scaletest workspaces are filtered out due to ownership mismatch.coderdtest.Newto detect cross-test bleed ofCODER_DISABLE_OWNER_WORKSPACE_ACCESSand fast-fail.