opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(cli): add check for DisableOwnerWorkspaceExec in scaletest (#14417)

Browse Source 
- 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.
This commit is contained in:
Cian Johnston
2024-08-26 12:02:54 +01:00
committed by GitHub
parent c8eacc6df7
commit 6914862903
3 changed files with 125 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cli/exp_scaletest.go
+42 -8
View File
@@ -117,7 +117,7 @@ func (s *scaletestTracingFlags) provider(ctx context.Context) (trace.TracerProvi
}
var closeTracingOnce sync.Once
return tracerProvider, func(ctx context.Context) error {
return tracerProvider, func(_ context.Context) error {
var err error
closeTracingOnce.Do(func() {
// Allow time to upload traces even if ctx is canceled
@@ -430,7 +430,7 @@ func (r *RootCmd) scaletestCleanup() *serpent.Command {
}
cliui.Infof(inv.Stdout, "Fetching scaletest workspaces...")
workspaces, err := getScaletestWorkspaces(ctx, client, template)
workspaces, _, err := getScaletestWorkspaces(ctx, client, "", template)
if err != nil {
return err
}
@@ -863,6 +863,7 @@ func (r *RootCmd) scaletestWorkspaceTraffic() *serpent.Command {
tickInterval time.Duration
bytesPerTick int64
ssh bool
useHostLogin bool
app string
template string
targetWorkspaces string
@@ -926,10 +927,18 @@ func (r *RootCmd) scaletestWorkspaceTraffic() *serpent.Command {
return xerrors.Errorf("get app host: %w", err)
}
workspaces, err := getScaletestWorkspaces(inv.Context(), client, template)
var owner string
if useHostLogin {
owner = codersdk.Me
}
workspaces, numSkipped, err := getScaletestWorkspaces(inv.Context(), client, owner, template)
if err != nil {
return err
}
if numSkipped > 0 {
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)
}
if targetWorkspaceEnd == 0 {
targetWorkspaceEnd = len(workspaces)
@@ -1092,6 +1101,13 @@ func (r *RootCmd) scaletestWorkspaceTraffic() *serpent.Command {
Description: "Send WebSocket traffic to a workspace app (proxied via coderd), cannot be used with --ssh.",
Value: serpent.StringOf(&app),
},
{
Flag: "use-host-login",
Env: "CODER_SCALETEST_USE_HOST_LOGIN",
Default: "false",
Description: "Connect as the currently logged in user.",
Value: serpent.BoolOf(&useHostLogin),
},
}
tracingFlags.attach(&cmd.Options)
@@ -1378,22 +1394,35 @@ func isScaleTestWorkspace(workspace codersdk.Workspace) bool {
strings.HasPrefix(workspace.Name, "scaletest-")
}
func getScaletestWorkspaces(ctx context.Context, client *codersdk.Client, template string) ([]codersdk.Workspace, error) {
func getScaletestWorkspaces(ctx context.Context, client *codersdk.Client, owner, template string) ([]codersdk.Workspace, int, error) {
var (
pageNumber = 0
limit = 100
workspaces []codersdk.Workspace
skipped int
)
me, err := client.User(ctx, codersdk.Me)
if err != nil {
return nil, 0, xerrors.Errorf("check logged-in user")
}
dv, err := client.DeploymentConfig(ctx)
if err != nil {
return nil, 0, xerrors.Errorf("fetch deployment config: %w", err)
}
noOwnerAccess := dv.Values != nil && dv.Values.DisableOwnerWorkspaceExec.Value()
for {
page, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{
Name: "scaletest-",
Template: template,
Owner: owner,
Offset: pageNumber * limit,
Limit: limit,
})
if err != nil {
return nil, xerrors.Errorf("fetch scaletest workspaces page %d: %w", pageNumber, err)
return nil, 0, xerrors.Errorf("fetch scaletest workspaces page %d: %w", pageNumber, err)
}
pageNumber++
@@ -1403,13 +1432,18 @@ func getScaletestWorkspaces(ctx context.Context, client *codersdk.Client, templa
pageWorkspaces := make([]codersdk.Workspace, 0, len(page.Workspaces))
for _, w := range page.Workspaces {
if isScaleTestWorkspace(w) {
pageWorkspaces = append(pageWorkspaces, w)
if !isScaleTestWorkspace(w) {
continue
}
if noOwnerAccess && w.OwnerID != me.ID {
skipped++
continue
}
pageWorkspaces = append(pageWorkspaces, w)
}
workspaces = append(workspaces, pageWorkspaces...)
}
return workspaces, nil
return workspaces, skipped, nil
}
func getScaletestUsers(ctx context.Context, client *codersdk.Client) ([]codersdk.User, error) {
cli/exptest/exptest_scaletest_test.go
+70
View File
@@ -0,0 +1,70 @@
package exptest_test
import (
"bytes"
"context"
"testing"
"github.com/stretchr/testify/require"
"cdr.dev/slog/sloggers/slogtest"
"github.com/coder/coder/v2/cli/clitest"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/testutil"
)
// This test validates that the scaletest CLI filters out workspaces not owned
// when disable owner workspace access is set.
// This test is in its own package because it mutates a global variable that
// can influence other tests in the same package.
// nolint:paralleltest
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
}),
})
owner := coderdtest.CreateFirstUser(t, client)
tv := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil)
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, tv.ID)
tpl := coderdtest.CreateTemplate(t, client, owner.OrganizationID, tv.ID)
// Create a workspace owned by a different user
memberClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
_ = coderdtest.CreateWorkspace(t, memberClient, tpl.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
cwr.Name = "scaletest-workspace"
})
// Test without --use-host-login first.g
inv, root := clitest.New(t, "exp", "scaletest", "workspace-traffic",
"--template", tpl.Name,
)
// nolint:gocritic // We are intentionally testing this as the owner.
clitest.SetupConfig(t, client, root)
var stdoutBuf bytes.Buffer
inv.Stdout = &stdoutBuf
err := inv.WithContext(ctx).Run()
require.ErrorContains(t, err, "no scaletest workspaces exist")
require.Contains(t, stdoutBuf.String(), `1 workspace(s) were skipped`)
// Test once again with --use-host-login.
inv, root = clitest.New(t, "exp", "scaletest", "workspace-traffic",
"--template", tpl.Name,
"--use-host-login",
)
// nolint:gocritic // We are intentionally testing this as the owner.
clitest.SetupConfig(t, client, root)
stdoutBuf.Reset()
inv.Stdout = &stdoutBuf
err = inv.WithContext(ctx).Run()
require.ErrorContains(t, err, "no scaletest workspaces exist")
require.NotContains(t, stdoutBuf.String(), `1 workspace(s) were skipped`)
}
coderd/coderdtest/coderdtest.go
+13 -3
View File
@@ -66,6 +66,7 @@ import (
"github.com/coder/coder/v2/coderd/httpmw"
"github.com/coder/coder/v2/coderd/notifications"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/coderd/rbac/policy"
"github.com/coder/coder/v2/coderd/schedule"
"github.com/coder/coder/v2/coderd/telemetry"
"github.com/coder/coder/v2/coderd/unhanger"
@@ -268,9 +269,18 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
if options.DeploymentValues == nil {
options.DeploymentValues = DeploymentValues(t)
}
// This value is not safe to run in parallel.
if options.DeploymentValues.DisableOwnerWorkspaceExec {
t.Logf("WARNING: DisableOwnerWorkspaceExec is set, this is not safe in parallel tests!")
// DisableOwnerWorkspaceExec modifies the 'global' RBAC roles. Fast-fail tests if we detect this.
if !options.DeploymentValues.DisableOwnerWorkspaceExec.Value() {
ownerSubj := rbac.Subject{
Roles: rbac.RoleIdentifiers{rbac.RoleOwner()},
Scope: rbac.ScopeAll,
}
if err := options.Authorizer.Authorize(context.Background(), ownerSubj, policy.ActionSSH, rbac.ResourceWorkspace); err != nil {
if rbac.IsUnauthorizedError(err) {
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!")
}
require.NoError(t, err)
}
}
// If no ratelimits are set, disable all rate limiting for tests.