Skip to content

Commit 2baa362

Browse files
deansheatherdeansheather
committed
Merge branch 'main' into dean/proxy-derp-map
2 parents 28ae155 + 667d9a7 commit 2baa362

File tree

158 files changed

+3198
-1725
lines changed

Some content is hidden

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

158 files changed

+3198
-1725
lines changed

‎Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,7 @@ cli/testdata/.gen-golden: $(wildcard cli/testdata/*.golden) $(wildcard cli/*.tpl
526526
go test ./cli -run="Test(CommandHelp|ServerYAML)" -update
527527
touch "$@"
528528

529-
helm/tests/testdata/.gen-golden: $(wildcard helm/tests/testdata/*.golden) $(GO_SRC_FILES)
529+
helm/tests/testdata/.gen-golden: $(wildcard helm/tests/testdata/*.yaml) $(wildcard helm/tests/testdata/*.golden) $(GO_SRC_FILES)
530530
go test ./helm/tests -run=TestUpdateGoldenFiles -update
531531
touch "$@"
532532

‎agent/agent.go

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -216,11 +216,12 @@ func (a *agent) collectMetadata(ctx context.Context, md codersdk.WorkspaceAgentM
216216
// if it can guarantee the clocks are synchronized.
217217
CollectedAt: time.Now(),
218218
}
219-
cmd, err := a.sshServer.CreateCommand(ctx, md.Script, nil)
219+
cmdPty, err := a.sshServer.CreateCommand(ctx, md.Script, nil)
220220
if err != nil {
221221
result.Error = fmt.Sprintf("create cmd: %+v", err)
222222
return result
223223
}
224+
cmd := cmdPty.AsExec()
224225

225226
cmd.Stdout = &out
226227
cmd.Stderr = &out
@@ -842,10 +843,11 @@ func (a *agent) runScript(ctx context.Context, lifecycle, script string) error {
842843
}()
843844
}
844845

845-
cmd, err := a.sshServer.CreateCommand(ctx, script, nil)
846+
cmdPty, err := a.sshServer.CreateCommand(ctx, script, nil)
846847
if err != nil {
847848
return xerrors.Errorf("create command: %w", err)
848849
}
850+
cmd := cmdPty.AsExec()
849851
cmd.Stdout = writer
850852
cmd.Stderr = writer
851853
err = cmd.Run()
@@ -1044,16 +1046,6 @@ func (a *agent) handleReconnectingPTY(ctx context.Context, logger slog.Logger, m
10441046
circularBuffer: circularBuffer,
10451047
}
10461048
a.reconnectingPTYs.Store(msg.ID, rpty)
1047-
go func() {
1048-
// CommandContext isn't respected for Windows PTYs right now,
1049-
// so we need to manually track the lifecycle.
1050-
// When the context has been completed either:
1051-
// 1. The timeout completed.
1052-
// 2. The parent context was canceled.
1053-
<-ctx.Done()
1054-
logger.Debug(ctx, "context done", slog.Error(ctx.Err()))
1055-
_ = process.Kill()
1056-
}()
10571049
// We don't need to separately monitor for the process exiting.
10581050
// When it exits, our ptty.OutputReader() will return EOF after
10591051
// reading all process output.

‎agent/agent_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"net/http/httptest"
1313
"net/netip"
1414
"os"
15-
"os/exec"
1615
"os/user"
1716
"path"
1817
"path/filepath"
@@ -1697,7 +1696,7 @@ func setupSSHCommand(t *testing.T, beforeArgs []string, afterArgs []string) (*pt
16971696
"host",
16981697
)
16991698
args = append(args, afterArgs...)
1700-
cmd := exec.Command("ssh", args...)
1699+
cmd := pty.Command("ssh", args...)
17011700
return ptytest.Start(t, cmd)
17021701
}
17031702

‎agent/agentssh/agentssh.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ func (s *Server) sessionStart(session ssh.Session, extraEnv []string) (retErr er
255255
if isPty {
256256
return s.startPTYSession(session, cmd, sshPty, windowSize)
257257
}
258-
return startNonPTYSession(session, cmd)
258+
return startNonPTYSession(session, cmd.AsExec())
259259
}
260260

261261
func startNonPTYSession(session ssh.Session, cmd *exec.Cmd) error {
@@ -287,7 +287,7 @@ type ptySession interface {
287287
RawCommand() string
288288
}
289289

290-
func (s *Server) startPTYSession(session ptySession, cmd *exec.Cmd, sshPty ssh.Pty, windowSize <-chan ssh.Window) (retErr error) {
290+
func (s *Server) startPTYSession(session ptySession, cmd *pty.Cmd, sshPty ssh.Pty, windowSize <-chan ssh.Window) (retErr error) {
291291
ctx := session.Context()
292292
// Disable minimal PTY emulation set by gliderlabs/ssh (NL-to-CRNL).
293293
// See https://github.com/coder/coder/issues/3371.
@@ -413,7 +413,7 @@ func (s *Server) sftpHandler(session ssh.Session) {
413413
// CreateCommand processes raw command input with OpenSSH-like behavior.
414414
// If the script provided is empty, it will default to the users shell.
415415
// This injects environment variables specified by the user at launch too.
416-
func (s *Server) CreateCommand(ctx context.Context, script string, env []string) (*exec.Cmd, error) {
416+
func (s *Server) CreateCommand(ctx context.Context, script string, env []string) (*pty.Cmd, error) {
417417
currentUser, err := user.Current()
418418
if err != nil {
419419
return nil, xerrors.Errorf("get current user: %w", err)
@@ -449,7 +449,7 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string)
449449
}
450450
}
451451

452-
cmd := exec.CommandContext(ctx, shell, args...)
452+
cmd := pty.CommandContext(ctx, shell, args...)
453453
cmd.Dir = manifest.Directory
454454

455455
// If the metadata directory doesn't exist, we run the command

‎agent/agentssh/agentssh_internal_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ import (
77
"context"
88
"io"
99
"net"
10-
"os/exec"
1110
"testing"
1211

1312
gliderssh "github.com/gliderlabs/ssh"
1413
"github.com/spf13/afero"
1514
"github.com/stretchr/testify/assert"
1615
"github.com/stretchr/testify/require"
1716

17+
"github.com/coder/coder/pty"
1818
"github.com/coder/coder/testutil"
1919

2020
"cdr.dev/slog/sloggers/slogtest"
@@ -52,7 +52,7 @@ func Test_sessionStart_orphan(t *testing.T) {
5252
close(windowSize)
5353
// the command gets the session context so that Go will terminate it when
5454
// the session expires.
55-
cmd := exec.CommandContext(sessionCtx, "sh", "-c", longScript)
55+
cmd := pty.CommandContext(sessionCtx, "sh", "-c", longScript)
5656

5757
done := make(chan struct{})
5858
go func() {

‎cli/server.go

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,13 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
256256
// which is caught by goleaks.
257257
defer http.DefaultClient.CloseIdleConnections()
258258

259-
tracerProvider, sqlDriver := ConfigureTraceProvider(ctx, logger, inv, cfg)
259+
tracerProvider, sqlDriver, closeTracing := ConfigureTraceProvider(ctx, logger, inv, cfg)
260+
defer func() {
261+
logger.Debug(ctx, "closing tracing")
262+
traceCloseErr := shutdownWithTimeout(closeTracing, 5*time.Second)
263+
logger.Debug(ctx, "tracing closed", slog.Error(traceCloseErr))
264+
}()
265+
260266
httpServers, err := ConfigureHTTPServers(inv, cfg)
261267
if err != nil {
262268
return xerrors.Errorf("configure http(s): %w", err)
@@ -1179,6 +1185,7 @@ func newProvisionerDaemon(
11791185
return nil, xerrors.Errorf("mkdir %q: %w", cacheDir, err)
11801186
}
11811187

1188+
tracer := coderAPI.TracerProvider.Tracer(tracing.TracerName)
11821189
terraformClient, terraformServer := provisionersdk.MemTransportPipe()
11831190
wg.Add(1)
11841191
go func() {
@@ -1198,6 +1205,7 @@ func newProvisionerDaemon(
11981205
},
11991206
CachePath: cacheDir,
12001207
Logger: logger,
1208+
Tracer: tracer,
12011209
})
12021210
if err != nil && !xerrors.Is(err, context.Canceled) {
12031211
select {
@@ -1863,9 +1871,15 @@ func (s *HTTPServers) Close() {
18631871
}
18641872
}
18651873

1866-
func ConfigureTraceProvider(ctx context.Context, logger slog.Logger, inv *clibase.Invocation, cfg *codersdk.DeploymentValues) (trace.TracerProvider, string) {
1874+
func ConfigureTraceProvider(
1875+
ctx context.Context,
1876+
logger slog.Logger,
1877+
inv *clibase.Invocation,
1878+
cfg *codersdk.DeploymentValues,
1879+
) (trace.TracerProvider, string, func(context.Context) error) {
18671880
var (
1868-
tracerProvider trace.TracerProvider
1881+
tracerProvider = trace.NewNoopTracerProvider()
1882+
closeTracing = func(context.Context) error { return nil }
18691883
sqlDriver = "postgres"
18701884
)
18711885
// Coder tracing should be disabled if telemetry is disabled unless
@@ -1878,19 +1892,14 @@ func ConfigureTraceProvider(ctx context.Context, logger slog.Logger, inv *clibas
18781892
}
18791893

18801894
if cfg.Trace.Enable.Value() || shouldCoderTrace || cfg.Trace.HoneycombAPIKey != "" {
1881-
sdkTracerProvider, closeTracing, err := tracing.TracerProvider(ctx, "coderd", tracing.TracerOpts{
1895+
sdkTracerProvider, _closeTracing, err := tracing.TracerProvider(ctx, "coderd", tracing.TracerOpts{
18821896
Default: cfg.Trace.Enable.Value(),
18831897
Coder: shouldCoderTrace,
18841898
Honeycomb: cfg.Trace.HoneycombAPIKey.String(),
18851899
})
18861900
if err != nil {
18871901
logger.Warn(ctx, "start telemetry exporter", slog.Error(err))
18881902
} else {
1889-
// allow time for traces to flush even if command context is canceled
1890-
defer func() {
1891-
_ = shutdownWithTimeout(closeTracing, 5*time.Second)
1892-
}()
1893-
18941903
d, err := tracing.PostgresDriver(sdkTracerProvider, "coderd.database")
18951904
if err != nil {
18961905
logger.Warn(ctx, "start postgres tracing driver", slog.Error(err))
@@ -1899,9 +1908,10 @@ func ConfigureTraceProvider(ctx context.Context, logger slog.Logger, inv *clibas
18991908
}
19001909

19011910
tracerProvider = sdkTracerProvider
1911+
closeTracing = _closeTracing
19021912
}
19031913
}
1904-
return tracerProvider, sqlDriver
1914+
return tracerProvider, sqlDriver, closeTracing
19051915
}
19061916

19071917
func ConfigureHTTPServers(inv *clibase.Invocation, cfg *codersdk.DeploymentValues) (_ *HTTPServers, err error) {

‎cli/ssh_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ Expire-Date: 0
540540
require.NoError(t, err, "import ownertrust failed: %s", out)
541541

542542
// Start the GPG agent.
543-
agentCmd := exec.CommandContext(ctx, gpgAgentPath, "--no-detach", "--extra-socket", extraSocketPath)
543+
agentCmd := pty.CommandContext(ctx, gpgAgentPath, "--no-detach", "--extra-socket", extraSocketPath)
544544
agentCmd.Env = append(agentCmd.Env, "GNUPGHOME="+gnupgHomeClient)
545545
agentPTY, agentProc, err := pty.Start(agentCmd, pty.WithPTYOption(pty.WithGPGTTY()))
546546
require.NoError(t, err, "launch agent failed")

‎coderd/apidoc/docs.go

Lines changed: 7 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/coderd.go

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,9 @@ func New(options *Options) *API {
235235
if options.SSHConfig.HostnamePrefix == "" {
236236
options.SSHConfig.HostnamePrefix = "coder."
237237
}
238+
if options.TracerProvider == nil {
239+
options.TracerProvider = trace.NewNoopTracerProvider()
240+
}
238241
if options.SetUserGroups == nil {
239242
options.SetUserGroups = func(ctx context.Context, _ database.Store, id uuid.UUID, groups []string) error {
240243
options.Logger.Warn(ctx, "attempted to assign OIDC groups without enterprise license",
@@ -445,7 +448,7 @@ func New(options *Options) *API {
445448
r.Route(fmt.Sprintf("/%s", gitAuthConfig.ID), func(r chi.Router) {
446449
r.Use(
447450
httpmw.ExtractOAuth2(gitAuthConfig, options.HTTPClient, nil),
448-
apiKeyMiddleware,
451+
apiKeyMiddlewareRedirect,
449452
)
450453
r.Get("/callback", api.gitAuthCallback(gitAuthConfig))
451454
})
@@ -792,7 +795,16 @@ func New(options *Options) *API {
792795
r.Get("/swagger/*", globalHTTPSwaggerHandler)
793796
}
794797

795-
r.NotFound(compressHandler(http.HandlerFunc(api.siteHandler.ServeHTTP)).ServeHTTP)
798+
// Add CSP headers to all static assets and pages. CSP headers only affect
799+
// browsers, so these don't make sense on api routes.
800+
cspMW := httpmw.CSPHeaders(func() []string {
801+
if f := api.WorkspaceProxyHostsFn.Load(); f != nil {
802+
return (*f)()
803+
}
804+
// By default we do not add extra websocket connections to the CSP
805+
return []string{}
806+
})
807+
r.NotFound(cspMW(compressHandler(http.HandlerFunc(api.siteHandler.ServeHTTP))).ServeHTTP)
796808
return api
797809
}
798810

@@ -812,8 +824,14 @@ type API struct {
812824
WorkspaceClientCoordinateOverride atomic.Pointer[func(rw http.ResponseWriter) bool]
813825
TailnetCoordinator atomic.Pointer[tailnet.Coordinator]
814826
QuotaCommitter atomic.Pointer[proto.QuotaCommitter]
815-
TemplateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore]
816-
DERPMapper atomic.Pointer[func(derpMap *tailcfg.DERPMap) *tailcfg.DERPMap]
827+
// WorkspaceProxyHostsFn returns the hosts of healthy workspace proxies
828+
// for header reasons.
829+
WorkspaceProxyHostsFn atomic.Pointer[func() []string]
830+
// TemplateScheduleStore is a pointer to an atomic pointer because this is
831+
// passed to another struct, and we want them all to be the same reference.
832+
TemplateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore]
833+
// DERPMapper mutates the DERPMap to include workspace proxies.
834+
DERPMapper atomic.Pointer[func(derpMap *tailcfg.DERPMap) *tailcfg.DERPMap]
817835

818836
HTTPAuth *HTTPAuthorizer
819837

@@ -884,6 +902,7 @@ func compressHandler(h http.Handler) http.Handler {
884902
// CreateInMemoryProvisionerDaemon is an in-memory connection to a provisionerd.
885903
// Useful when starting coderd and provisionerd in the same process.
886904
func (api *API) CreateInMemoryProvisionerDaemon(ctx context.Context, debounce time.Duration) (client proto.DRPCProvisionerDaemonClient, err error) {
905+
tracer := api.TracerProvider.Tracer(tracing.TracerName)
887906
clientSession, serverSession := provisionersdk.MemTransportPipe()
888907
defer func() {
889908
if err != nil {
@@ -923,6 +942,7 @@ func (api *API) CreateInMemoryProvisionerDaemon(ctx context.Context, debounce ti
923942
Provisioners: daemon.Provisioners,
924943
GitAuthConfigs: api.GitAuthConfigs,
925944
Telemetry: api.Telemetry,
945+
Tracer: tracer,
926946
Tags: tags,
927947
QuotaCommitter: &api.QuotaCommitter,
928948
Auditor: &api.Auditor,
@@ -933,14 +953,16 @@ func (api *API) CreateInMemoryProvisionerDaemon(ctx context.Context, debounce ti
933953
if err != nil {
934954
return nil, err
935955
}
936-
server := drpcserver.NewWithOptions(mux, drpcserver.Options{
937-
Log: func(err error) {
938-
if xerrors.Is(err, io.EOF) {
939-
return
940-
}
941-
api.Logger.Debug(ctx, "drpc server error", slog.Error(err))
956+
server := drpcserver.NewWithOptions(&tracing.DRPCHandler{Handler: mux},
957+
drpcserver.Options{
958+
Log: func(err error) {
959+
if xerrors.Is(err, io.EOF) {
960+
return
961+
}
962+
api.Logger.Debug(ctx, "drpc server error", slog.Error(err))
963+
},
942964
},
943-
})
965+
)
944966
go func() {
945967
err := server.Serve(ctx, serverSession)
946968
if err != nil && !xerrors.Is(err, io.EOF) {

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,8 @@ var (
144144
},
145145
}),
146146
Scope: rbac.ScopeAll,
147-
}
147+
}.WithCachedASTValue()
148+
148149
subjectAutostart = rbac.Subject{
149150
ID: uuid.Nil.String(),
150151
Roles: rbac.Roles([]rbac.Role{
@@ -161,7 +162,8 @@ var (
161162
},
162163
}),
163164
Scope: rbac.ScopeAll,
164-
}
165+
}.WithCachedASTValue()
166+
165167
subjectSystemRestricted = rbac.Subject{
166168
ID: uuid.Nil.String(),
167169
Roles: rbac.Roles([]rbac.Role{
@@ -188,7 +190,7 @@ var (
188190
},
189191
}),
190192
Scope: rbac.ScopeAll,
191-
}
193+
}.WithCachedASTValue()
192194
)
193195

194196
// AsProvisionerd returns a context with an actor that has permissions required

‎coderd/database/dump.sql

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)