Skip to content

Commit 277b0eb

Browse files
EmyrkEmyrk
committed
Merge remote-tracking branch 'origin/main' into stevenmasley/rbac_org_endpoints
2 parents a2fdb42 + e2ed581 commit 277b0eb

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+876
-440
lines changed

‎Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ coderd/database/dump.sql: $(wildcard coderd/database/migrations/*.sql)
2020
coderd/database/querier.go: coderd/database/dump.sql $(wildcard coderd/database/queries/*.sql)
2121
coderd/database/generate.sh
2222

23-
dev: build
23+
dev:
2424
./scripts/develop.sh
2525
.PHONY: dev
2626

‎agent/agent_test.go

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

33
import (
4+
"bufio"
45
"context"
56
"encoding/json"
67
"fmt"
@@ -204,6 +205,11 @@ func TestAgent(t *testing.T) {
204205
id := uuid.NewString()
205206
netConn, err := conn.ReconnectingPTY(id, 100, 100)
206207
require.NoError(t, err)
208+
bufRead := bufio.NewReader(netConn)
209+
210+
// Brief pause to reduce the likelihood that we send keystrokes while
211+
// the shell is simultaneously sending a prompt.
212+
time.Sleep(100 * time.Millisecond)
207213

208214
data, err := json.Marshal(agent.ReconnectingPTYRequest{
209215
Data: "echo test\r\n",
@@ -212,28 +218,35 @@ func TestAgent(t *testing.T) {
212218
_, err = netConn.Write(data)
213219
require.NoError(t, err)
214220

215-
findEcho := func() {
221+
expectLine := func(matcher func(string) bool) {
216222
for {
217-
read, err := netConn.Read(data)
223+
line, err := bufRead.ReadString('\n')
218224
require.NoError(t, err)
219-
if strings.Contains(string(data[:read]), "test") {
225+
if matcher(line) {
220226
break
221227
}
222228
}
223229
}
230+
matchEchoCommand := func(line string) bool {
231+
return strings.Contains(line, "echo test")
232+
}
233+
matchEchoOutput := func(line string) bool {
234+
return strings.Contains(line, "test") && !strings.Contains(line, "echo")
235+
}
224236

225237
// Once for typing the command...
226-
findEcho()
238+
expectLine(matchEchoCommand)
227239
// And another time for the actual output.
228-
findEcho()
240+
expectLine(matchEchoOutput)
229241

230242
_ = netConn.Close()
231243
netConn, err = conn.ReconnectingPTY(id, 100, 100)
232244
require.NoError(t, err)
245+
bufRead = bufio.NewReader(netConn)
233246

234247
// Same output again!
235-
findEcho()
236-
findEcho()
248+
expectLine(matchEchoCommand)
249+
expectLine(matchEchoOutput)
237250
})
238251

239252
t.Run("Dial", func(t *testing.T) {

‎cli/cliui/prompt.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,6 @@ func Prompt(cmd *cobra.Command, opts PromptOptions) (string, error) {
3434
_, _ = fmt.Fprint(cmd.OutOrStdout(), Styles.Placeholder.Render("("+opts.Default+") "))
3535
}
3636
interrupt := make(chan os.Signal, 1)
37-
signal.Notify(interrupt, os.Interrupt)
38-
defer signal.Stop(interrupt)
3937

4038
errCh := make(chan error, 1)
4139
lineCh := make(chan string)
@@ -45,8 +43,12 @@ func Prompt(cmd *cobra.Command, opts PromptOptions) (string, error) {
4543

4644
inFile, isInputFile := cmd.InOrStdin().(*os.File)
4745
if opts.Secret && isInputFile && isatty.IsTerminal(inFile.Fd()) {
46+
// we don't install a signal handler here because speakeasy has its own
4847
line, err = speakeasy.Ask("")
4948
} else {
49+
signal.Notify(interrupt, os.Interrupt)
50+
defer signal.Stop(interrupt)
51+
5052
reader := bufio.NewReader(cmd.InOrStdin())
5153
line, err = reader.ReadString('\n')
5254

‎cli/cliui/prompt_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,16 @@ package cliui_test
22

33
import (
44
"context"
5+
"os"
6+
"os/exec"
57
"testing"
8+
"time"
69

710
"github.com/spf13/cobra"
811
"github.com/stretchr/testify/require"
912

1013
"github.com/coder/coder/cli/cliui"
14+
"github.com/coder/coder/pty"
1115
"github.com/coder/coder/pty/ptytest"
1216
)
1317

@@ -110,3 +114,56 @@ func newPrompt(ptty *ptytest.PTY, opts cliui.PromptOptions) (string, error) {
110114
cmd.SetIn(ptty.Input())
111115
return value, cmd.ExecuteContext(context.Background())
112116
}
117+
118+
func TestPasswordTerminalState(t *testing.T) {
119+
if os.Getenv("TEST_SUBPROCESS") == "1" {
120+
passwordHelper()
121+
return
122+
}
123+
t.Parallel()
124+
125+
ptty := ptytest.New(t)
126+
ptyWithFlags, ok := ptty.PTY.(pty.WithFlags)
127+
if !ok {
128+
t.Skip("unable to check PTY local echo on this platform")
129+
}
130+
131+
cmd := exec.Command(os.Args[0], "-test.run=TestPasswordTerminalState") //nolint:gosec
132+
cmd.Env = append(os.Environ(), "TEST_SUBPROCESS=1")
133+
// connect the child process's stdio to the PTY directly, not via a pipe
134+
cmd.Stdin = ptty.Input().Reader
135+
cmd.Stdout = ptty.Output().Writer
136+
cmd.Stderr = os.Stderr
137+
err := cmd.Start()
138+
require.NoError(t, err)
139+
process := cmd.Process
140+
defer process.Kill()
141+
142+
ptty.ExpectMatch("Password: ")
143+
time.Sleep(100 * time.Millisecond) // wait for child process to turn off echo and start reading input
144+
145+
echo, err := ptyWithFlags.EchoEnabled()
146+
require.NoError(t, err)
147+
require.False(t, echo, "echo is on while reading password")
148+
149+
err = process.Signal(os.Interrupt)
150+
require.NoError(t, err)
151+
_, err = process.Wait()
152+
require.NoError(t, err)
153+
154+
echo, err = ptyWithFlags.EchoEnabled()
155+
require.NoError(t, err)
156+
require.True(t, echo, "echo is off after reading password")
157+
}
158+
159+
func passwordHelper() {
160+
cmd := &cobra.Command{
161+
Run: func(cmd *cobra.Command, args []string) {
162+
cliui.Prompt(cmd, cliui.PromptOptions{
163+
Text: "Password:",
164+
Secret: true,
165+
})
166+
},
167+
}
168+
cmd.ExecuteContext(context.Background())
169+
}

‎cli/list.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func list() *cobra.Command {
2929
if err != nil {
3030
return err
3131
}
32-
workspaces, err := client.WorkspacesByUser(cmd.Context(), codersdk.Me)
32+
workspaces, err := client.Workspaces(cmd.Context(), codersdk.WorkspaceFilter{})
3333
if err != nil {
3434
return err
3535
}

‎cli/resetpassword_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@ import (
1515
"github.com/coder/coder/pty/ptytest"
1616
)
1717

18+
// nolint:paralleltest
1819
func TestResetPassword(t *testing.T) {
19-
t.Parallel()
20+
// postgres.Open() seems to be creating race conditions when run in parallel.
21+
// t.Parallel()
2022

2123
if runtime.GOOS != "linux" || testing.Short() {
2224
// Skip on non-Linux because it spawns a PostgreSQL instance.

‎cli/server_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,11 @@ import (
3232
)
3333

3434
// This cannot be ran in parallel because it uses a signal.
35-
// nolint:tparallel
35+
// nolint:paralleltest
3636
func TestServer(t *testing.T) {
3737
t.Run("Production", func(t *testing.T) {
38-
t.Parallel()
38+
// postgres.Open() seems to be creating race conditions when run in parallel.
39+
// t.Parallel()
3940
if runtime.GOOS != "linux" || testing.Short() {
4041
// Skip on non-Linux because it spawns a PostgreSQL instance.
4142
t.SkipNow()

‎coderd/autobuild/executor/lifecycle_executor.go

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func (e *Executor) runOnce(t time.Time) error {
5757

5858
for _, ws := range eligibleWorkspaces {
5959
// Determine the workspace state based on its latest build.
60-
priorHistory, err := db.GetWorkspaceBuildByWorkspaceIDWithoutAfter(e.ctx, ws.ID)
60+
priorHistory, err := db.GetLatestWorkspaceBuildByWorkspaceID(e.ctx, ws.ID)
6161
if err != nil {
6262
e.log.Warn(e.ctx, "get latest workspace build",
6363
slog.F("workspace_id", ws.ID),
@@ -152,12 +152,8 @@ func build(ctx context.Context, store database.Store, workspace database.Workspa
152152
return xerrors.Errorf("get workspace template: %w", err)
153153
}
154154

155-
priorHistoryID := uuid.NullUUID{
156-
UUID: priorHistory.ID,
157-
Valid: true,
158-
}
155+
priorBuildNumber := priorHistory.BuildNumber
159156

160-
var newWorkspaceBuild database.WorkspaceBuild
161157
// This must happen in a transaction to ensure history can be inserted, and
162158
// the prior history can update it's "after" column to point at the new.
163159
workspaceBuildID := uuid.New()
@@ -186,13 +182,13 @@ func build(ctx context.Context, store database.Store, workspace database.Workspa
186182
if err != nil {
187183
return xerrors.Errorf("insert provisioner job: %w", err)
188184
}
189-
newWorkspaceBuild, err = store.InsertWorkspaceBuild(ctx, database.InsertWorkspaceBuildParams{
185+
_, err = store.InsertWorkspaceBuild(ctx, database.InsertWorkspaceBuildParams{
190186
ID: workspaceBuildID,
191187
CreatedAt: now,
192188
UpdatedAt: now,
193189
WorkspaceID: workspace.ID,
194190
TemplateVersionID: priorHistory.TemplateVersionID,
195-
BeforeID: priorHistoryID,
191+
BuildNumber: priorBuildNumber + 1,
196192
Name: namesgenerator.GetRandomName(1),
197193
ProvisionerState: priorHistory.ProvisionerState,
198194
InitiatorID: workspace.OwnerID,
@@ -202,21 +198,5 @@ func build(ctx context.Context, store database.Store, workspace database.Workspa
202198
if err != nil {
203199
return xerrors.Errorf("insert workspace build: %w", err)
204200
}
205-
206-
if priorHistoryID.Valid {
207-
// Update the prior history entries "after" column.
208-
err = store.UpdateWorkspaceBuildByID(ctx, database.UpdateWorkspaceBuildByIDParams{
209-
ID: priorHistory.ID,
210-
ProvisionerState: priorHistory.ProvisionerState,
211-
UpdatedAt: now,
212-
AfterID: uuid.NullUUID{
213-
UUID: newWorkspaceBuild.ID,
214-
Valid: true,
215-
},
216-
})
217-
if err != nil {
218-
return xerrors.Errorf("update prior workspace build: %w", err)
219-
}
220-
}
221201
return nil
222202
}

‎coderd/autobuild/executor/lifecycle_executor_test.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,10 +419,17 @@ func TestExecutorAutostartMultipleOK(t *testing.T) {
419419
require.NotEqual(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected a workspace build to occur")
420420
require.Equal(t, codersdk.ProvisionerJobSucceeded, ws.LatestBuild.Job.Status, "expected provisioner job to have succeeded")
421421
require.Equal(t, database.WorkspaceTransitionStart, ws.LatestBuild.Transition, "expected latest transition to be start")
422-
builds, err := client.WorkspaceBuilds(ctx, ws.ID)
422+
builds, err := client.WorkspaceBuilds(ctx, codersdk.WorkspaceBuildsRequest{WorkspaceID: ws.ID})
423423
require.NoError(t, err, "fetch list of workspace builds from primary")
424424
// One build to start, one stop transition, and one autostart. No more.
425+
require.Equal(t, database.WorkspaceTransitionStart, builds[0].Transition)
426+
require.Equal(t, database.WorkspaceTransitionStop, builds[1].Transition)
427+
require.Equal(t, database.WorkspaceTransitionStart, builds[2].Transition)
425428
require.Len(t, builds, 3, "unexpected number of builds for workspace from primary")
429+
430+
// Builds are returned most recent first.
431+
require.True(t, builds[0].CreatedAt.After(builds[1].CreatedAt))
432+
require.True(t, builds[1].CreatedAt.After(builds[2].CreatedAt))
426433
}
427434

428435
func mustProvisionWorkspace(t *testing.T, client *codersdk.Client) codersdk.Workspace {

‎coderd/coderd.go

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,6 @@ func New(options *Options) (http.Handler, func()) {
251251
})
252252
r.Get("/gitsshkey", api.gitSSHKey)
253253
r.Put("/gitsshkey", api.regenerateGitSSHKey)
254-
r.Get("/workspaces", api.workspacesByUser)
255254
})
256255
})
257256
})
@@ -287,23 +286,28 @@ func New(options *Options) (http.Handler, func()) {
287286
)
288287
r.Get("/", api.workspaceResource)
289288
})
290-
r.Route("/workspaces/{workspace}", func(r chi.Router) {
289+
r.Route("/workspaces", func(r chi.Router) {
291290
r.Use(
292291
apiKeyMiddleware,
293292
authRolesMiddleware,
294-
httpmw.ExtractWorkspaceParam(options.Database),
295293
)
296-
r.Get("/", api.workspace)
297-
r.Route("/builds", func(r chi.Router) {
298-
r.Get("/", api.workspaceBuilds)
299-
r.Post("/", api.postWorkspaceBuilds)
300-
r.Get("/{workspacebuildname}", api.workspaceBuildByName)
301-
})
302-
r.Route("/autostart", func(r chi.Router) {
303-
r.Put("/", api.putWorkspaceAutostart)
304-
})
305-
r.Route("/autostop", func(r chi.Router) {
306-
r.Put("/", api.putWorkspaceAutostop)
294+
r.Get("/", api.workspaces)
295+
r.Route("/{workspace}", func(r chi.Router) {
296+
r.Use(
297+
httpmw.ExtractWorkspaceParam(options.Database),
298+
)
299+
r.Get("/", api.workspace)
300+
r.Route("/builds", func(r chi.Router) {
301+
r.Get("/", api.workspaceBuilds)
302+
r.Post("/", api.postWorkspaceBuilds)
303+
r.Get("/{workspacebuildname}", api.workspaceBuildByName)
304+
})
305+
r.Route("/autostart", func(r chi.Router) {
306+
r.Put("/", api.putWorkspaceAutostart)
307+
})
308+
r.Route("/autostop", func(r chi.Router) {
309+
r.Put("/", api.putWorkspaceAutostop)
310+
})
307311
})
308312
})
309313
r.Route("/workspacebuilds/{workspacebuild}", func(r chi.Router) {

‎coderd/coderd_test.go

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,18 +84,22 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
8484
"GET:/api/v2/workspaceagents/me/metadata": {NoAuthorize: true},
8585
"GET:/api/v2/workspaceagents/me/turn": {NoAuthorize: true},
8686
"GET:/api/v2/workspaceagents/{workspaceagent}": {NoAuthorize: true},
87-
"GET:/api/v2/workspaceagents/{workspaceagent}/": {NoAuthorize: true},
8887
"GET:/api/v2/workspaceagents/{workspaceagent}/dial": {NoAuthorize: true},
8988
"GET:/api/v2/workspaceagents/{workspaceagent}/iceservers": {NoAuthorize: true},
9089
"GET:/api/v2/workspaceagents/{workspaceagent}/pty": {NoAuthorize: true},
9190
"GET:/api/v2/workspaceagents/{workspaceagent}/turn": {NoAuthorize: true},
9291

9392
// TODO: @emyrk these need to be fixed by adding authorize calls
94-
"GET:/api/v2/workspaceresources/{workspaceresource}": {NoAuthorize: true},
93+
"GET:/api/v2/workspaceresources/{workspaceresource}": {NoAuthorize: true},
94+
"GET:/api/v2/workspacebuilds/{workspacebuild}": {NoAuthorize: true},
95+
"GET:/api/v2/workspacebuilds/{workspacebuild}/logs": {NoAuthorize: true},
96+
"GET:/api/v2/workspacebuilds/{workspacebuild}/resources": {NoAuthorize: true},
97+
"GET:/api/v2/workspacebuilds/{workspacebuild}/state": {NoAuthorize: true},
98+
"PATCH:/api/v2/workspacebuilds/{workspacebuild}/cancel": {NoAuthorize: true},
99+
"GET:/api/v2/workspaces/{workspace}/builds/{workspacebuildname}": {NoAuthorize: true},
95100

96101
"GET:/api/v2/users/oauth2/github/callback": {NoAuthorize: true},
97102

98-
"POST:/api/v2/users/{user}/organizations/": {NoAuthorize: true},
99103
"PUT:/api/v2/organizations/{organization}/members/{user}/roles": {NoAuthorize: true},
100104
"GET:/api/v2/organizations/{organization}/provisionerdaemons": {NoAuthorize: true},
101105
"POST:/api/v2/organizations/{organization}/templates": {NoAuthorize: true},
@@ -133,6 +137,8 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
133137
"GET:/api/v2/users/{user}/organizations": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceOrganization},
134138
"GET:/api/v2/users/{user}/workspaces": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceWorkspace},
135139
"GET:/api/v2/organizations/{organization}/workspaces/{user}": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceWorkspace},
140+
"GET:/api/v2/organizations/{organization}/workspaces/{user}/{workspace}": {
141+
AssertObject: rbac.ResourceWorkspace.InOrg(organization.ID).WithID(workspace.ID.String()).WithOwner(workspace.OwnerID.String()),
136142
"GET:/api/v2/workspaces/{workspace}/builds/{workspacebuildname}": {
137143
AssertAction: rbac.ActionRead,
138144
AssertObject: workspaceRBACObj,
@@ -184,6 +190,15 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
184190
"POST:/api/v2/workspaces/{workspace}/builds": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
185191
}
186192

193+
for k, v := range assertRoute {
194+
noTrailSlash := strings.TrimRight(k, "/")
195+
if _, ok := assertRoute[noTrailSlash]; ok && noTrailSlash != k {
196+
t.Errorf("route %q & %q is declared twice", noTrailSlash, k)
197+
t.FailNow()
198+
}
199+
assertRoute[noTrailSlash] = v
200+
}
201+
187202
c, _ := srv.Config.Handler.(*chi.Mux)
188203
err = chi.Walk(c, func(method string, route string, handler http.Handler, middlewares ...func(http.Handler) http.Handler) error {
189204
name := method + ":" + route

0 commit comments

Comments
 (0)