Skip to content

Commit ae50531

Browse files
BrunoQuaresmaBrunoQuaresma
committed
Merge branch 'main' into bq/fix-immutable-parameters
2 parents b623b24 + 083fc89 commit ae50531

File tree

110 files changed

+1874
-1255
lines changed

Some content is hidden

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

110 files changed

+1874
-1255
lines changed

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ coder server --postgres-url <url> --access-url <url>
8484

8585
> <sup>1</sup> For production deployments, set up an external PostgreSQL instance for reliability.
8686
87-
Use `coder --help` to get a list of flags and environment variables. Use our [quickstart guide](https://coder.com/docs/v2/latest/quickstart) for a full walkthrough.
87+
Use `coder --help` to get a list of flags and environment variables. Use our [install guides](https://coder.com/docs/v2/latest/guides) for a full walkthrough.
8888

8989
## Documentation
9090

‎agent/agent.go

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ type Options struct {
8282
Logger slog.Logger
8383
AgentPorts map[int]string
8484
SSHMaxTimeout time.Duration
85+
TailnetListenPort uint16
8586
}
8687

8788
type Client interface {
@@ -118,6 +119,7 @@ func New(options Options) io.Closer {
118119
}
119120
ctx, cancelFunc := context.WithCancel(context.Background())
120121
a := &agent{
122+
tailnetListenPort: options.TailnetListenPort,
121123
reconnectingPTYTimeout: options.ReconnectingPTYTimeout,
122124
logger: options.Logger,
123125
closeCancel: cancelFunc,
@@ -139,12 +141,13 @@ func New(options Options) io.Closer {
139141
}
140142

141143
type agent struct {
142-
logger slog.Logger
143-
client Client
144-
exchangeToken func(ctx context.Context) (string, error)
145-
filesystem afero.Fs
146-
logDir string
147-
tempDir string
144+
logger slog.Logger
145+
client Client
146+
exchangeToken func(ctx context.Context) (string, error)
147+
tailnetListenPort uint16
148+
filesystem afero.Fs
149+
logDir string
150+
tempDir string
148151
// ignorePorts tells the api handler which ports to ignore when
149152
// listing all listening ports. This is helpful to hide ports that
150153
// are used by the agent, that the user does not care about.
@@ -606,9 +609,10 @@ func (a *agent) trackConnGoroutine(fn func()) error {
606609

607610
func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (_ *tailnet.Conn, err error) {
608611
network, err := tailnet.NewConn(&tailnet.Options{
609-
Addresses: []netip.Prefix{netip.PrefixFrom(codersdk.WorkspaceAgentIP, 128)},
610-
DERPMap: derpMap,
611-
Logger: a.logger.Named("tailnet"),
612+
Addresses: []netip.Prefix{netip.PrefixFrom(codersdk.WorkspaceAgentIP, 128)},
613+
DERPMap: derpMap,
614+
Logger: a.logger.Named("tailnet"),
615+
ListenPort: a.tailnetListenPort,
612616
})
613617
if err != nil {
614618
return nil, xerrors.Errorf("create tailnet: %w", err)

‎cli/agent.go

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,12 @@ import (
3030

3131
func (r *RootCmd) workspaceAgent() *clibase.Cmd {
3232
var (
33-
auth string
34-
logDir string
35-
pprofAddress string
36-
noReap bool
37-
sshMaxTimeout time.Duration
33+
auth string
34+
logDir string
35+
pprofAddress string
36+
noReap bool
37+
sshMaxTimeout time.Duration
38+
tailnetListenPort int64
3839
)
3940
cmd := &clibase.Cmd{
4041
Use: "agent",
@@ -187,9 +188,10 @@ func (r *RootCmd) workspaceAgent() *clibase.Cmd {
187188
}
188189

189190
closer := agent.New(agent.Options{
190-
Client: client,
191-
Logger: logger,
192-
LogDir: logDir,
191+
Client: client,
192+
Logger: logger,
193+
LogDir: logDir,
194+
TailnetListenPort: uint16(tailnetListenPort),
193195
ExchangeToken: func(ctx context.Context) (string, error) {
194196
if exchangeToken == nil {
195197
return client.SDK.SessionToken(), nil
@@ -248,6 +250,13 @@ func (r *RootCmd) workspaceAgent() *clibase.Cmd {
248250
Description: "Specify the max timeout for a SSH connection.",
249251
Value: clibase.DurationOf(&sshMaxTimeout),
250252
},
253+
{
254+
Flag: "tailnet-listen-port",
255+
Default: "0",
256+
Env: "CODER_AGENT_TAILNET_LISTEN_PORT",
257+
Description: "Specify a static port for Tailscale to use for listening.",
258+
Value: clibase.Int64Of(&tailnetListenPort),
259+
},
251260
}
252261

253262
return cmd

‎cli/create.go

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"io"
77
"time"
88

9+
"github.com/google/uuid"
910
"golang.org/x/exp/slices"
1011
"golang.org/x/xerrors"
1112

@@ -213,6 +214,7 @@ type prepWorkspaceBuildArgs struct {
213214
NewWorkspaceName string
214215

215216
UpdateWorkspace bool
217+
WorkspaceID uuid.UUID
216218
}
217219

218220
type buildParameters struct {
@@ -340,8 +342,17 @@ PromptRichParamLoop:
340342
}
341343

342344
if args.UpdateWorkspace && !templateVersionParameter.Mutable {
343-
_, _ = fmt.Fprintln(inv.Stdout, cliui.Styles.Warn.Render(fmt.Sprintf(`Parameter %q is not mutable, so can't be customized after workspace creation.`, templateVersionParameter.Name)))
344-
continue
345+
// Check if the immutable parameter was used in the previous build. If so, then it isn't a fresh one
346+
// and the user should be warned.
347+
exists, err := workspaceBuildParameterExists(ctx, client, args.WorkspaceID, templateVersionParameter)
348+
if err != nil {
349+
return nil, err
350+
}
351+
352+
if exists {
353+
_, _ = fmt.Fprintln(inv.Stdout, cliui.Styles.Warn.Render(fmt.Sprintf(`Parameter %q is not mutable, so can't be customized after workspace creation.`, templateVersionParameter.Name)))
354+
continue
355+
}
345356
}
346357

347358
parameterValue, err := getWorkspaceBuildParameterValueFromMapOrInput(inv, parameterMapFromFile, templateVersionParameter)
@@ -414,3 +425,17 @@ PromptRichParamLoop:
414425
richParameters: richParameters,
415426
}, nil
416427
}
428+
429+
func workspaceBuildParameterExists(ctx context.Context, client *codersdk.Client, workspaceID uuid.UUID, templateVersionParameter codersdk.TemplateVersionParameter) (bool, error) {
430+
lastBuildParameters, err := client.WorkspaceBuildParameters(ctx, workspaceID)
431+
if err != nil {
432+
return false, xerrors.Errorf("can't fetch last workspace build parameters: %w", err)
433+
}
434+
435+
for _, p := range lastBuildParameters {
436+
if p.Name == templateVersionParameter.Name {
437+
return true, nil
438+
}
439+
}
440+
return false, nil
441+
}

‎cli/delete.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,6 @@ func (r *RootCmd) deleteWorkspace() *clibase.Cmd {
3737
}
3838

3939
var state []byte
40-
41-
if orphan {
42-
cliui.Warn(
43-
inv.Stderr,
44-
"Orphaning workspace requires template edit permission",
45-
)
46-
}
47-
4840
build, err := client.CreateWorkspaceBuild(inv.Context(), workspace.ID, codersdk.CreateWorkspaceBuildRequest{
4941
Transition: codersdk.WorkspaceTransitionDelete,
5042
ProvisionerState: state,

‎cli/testdata/coder_agent_--help.golden

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,8 @@ Starts the Coder workspace agent.
1818
--ssh-max-timeout duration, $CODER_AGENT_SSH_MAX_TIMEOUT (default: 0)
1919
Specify the max timeout for a SSH connection.
2020

21+
--tailnet-listen-port int, $CODER_AGENT_TAILNET_LISTEN_PORT (default: 0)
22+
Specify a static port for Tailscale to use for listening.
23+
2124
---
2225
Run `coder --help` for a list of global options.

‎cli/update.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,9 @@ func (r *RootCmd) update() *clibase.Cmd {
5959
ExistingRichParams: existingRichParams,
6060
RichParameterFile: richParameterFile,
6161
NewWorkspaceName: workspace.Name,
62-
UpdateWorkspace: true,
62+
63+
UpdateWorkspace: true,
64+
WorkspaceID: workspace.LatestBuild.ID,
6365
})
6466
if err != nil {
6567
return nil

‎coderd/coderd.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ type Options struct {
123123
SwaggerEndpoint bool
124124
SetUserGroups func(ctx context.Context, tx database.Store, userID uuid.UUID, groupNames []string) error
125125
TemplateScheduleStore schedule.TemplateScheduleStore
126-
// AppSigningKey denotes the symmetric key to use for signing app tickets.
127-
// The key must be 64 bytes long.
126+
// AppSigningKey denotes the symmetric key to use for signing temporary app
127+
// tokens. The key must be 64 bytes long.
128128
AppSigningKey []byte
129129
HealthcheckFunc func(ctx context.Context) (*healthcheck.Report, error)
130130
HealthcheckTimeout time.Duration
@@ -297,7 +297,7 @@ func New(options *Options) *API {
297297
Authorizer: options.Authorizer,
298298
Logger: options.Logger,
299299
},
300-
WorkspaceAppsProvider: workspaceapps.New(
300+
WorkspaceAppsProvider: workspaceapps.NewDBTokenProvider(
301301
options.Logger.Named("workspaceapps"),
302302
options.AccessURL,
303303
options.Authorizer,
@@ -642,7 +642,7 @@ func New(options *Options) *API {
642642
r.Post("/metadata/{key}", api.workspaceAgentPostMetadata)
643643
})
644644
// No middleware on the PTY endpoint since it uses workspace
645-
// application auth and tickets.
645+
// application auth and signed app tokens.
646646
r.Get("/{workspaceagent}/pty", api.workspaceAgentPTY)
647647
r.Route("/{workspaceagent}", func(r chi.Router) {
648648
r.Use(
@@ -788,7 +788,7 @@ type API struct {
788788
metricsCache *metricscache.Cache
789789
workspaceAgentCache *wsconncache.Cache
790790
updateChecker *updatecheck.Checker
791-
WorkspaceAppsProvider *workspaceapps.Provider
791+
WorkspaceAppsProvider workspaceapps.SignedTokenProvider
792792

793793
// Experiments contains the list of experiments currently enabled.
794794
// This is used to gate features that are not yet ready for production.

‎coderd/coderdtest/coderdtest.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ import (
8080
"github.com/coder/coder/testutil"
8181
)
8282

83-
// AppSigningKey is a 64-byte key used to sign JWTs for workspace app tickets in
83+
// AppSigningKey is a 64-byte key used to sign JWTs for workspace app tokens in
8484
// tests.
8585
var AppSigningKey = must(hex.DecodeString("64656164626565666465616462656566646561646265656664656164626565666465616462656566646561646265656664656164626565666465616462656566"))
8686

‎coderd/database/queries.sql.go

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

‎coderd/database/queries/workspaces.sql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -356,16 +356,16 @@ WITH workspaces_with_jobs AS (
356356
build_number DESC
357357
LIMIT
358358
1
359-
) latest_build ON TRUE
359+
) latest_build ON TRUE WHERE deleted = false
360360
), pending_workspaces AS (
361361
SELECT COUNT(*) AS count FROM workspaces_with_jobs WHERE
362362
started_at IS NULL
363363
), building_workspaces AS (
364364
SELECT COUNT(*) AS count FROM workspaces_with_jobs WHERE
365365
started_at IS NOT NULL AND
366366
canceled_at IS NULL AND
367-
updated_at - INTERVAL '30 seconds' < NOW() AND
368-
completed_at IS NULL
367+
completed_at IS NULL AND
368+
updated_at - INTERVAL '30 seconds' < NOW()
369369
), running_workspaces AS (
370370
SELECT COUNT(*) AS count FROM workspaces_with_jobs WHERE
371371
completed_at IS NOT NULL AND

‎coderd/healthcheck/derp.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,14 @@ func (r *DERPNodeReport) Run(ctx context.Context) error {
171171
r.doExchangeMessage(ctx)
172172
r.doSTUNTest(ctx)
173173

174-
if !r.CanExchangeMessages || r.UsesWebsocket || r.STUN.Error != nil {
174+
// We can't exchange messages with the node,
175+
if (!r.CanExchangeMessages && !r.Node.STUNOnly) ||
176+
// A node may use websockets because `Upgrade: DERP` may be blocked on
177+
// the load balancer. This is unhealthy because websockets are slower
178+
// than the regular DERP protocol.
179+
r.UsesWebsocket ||
180+
// The node was marked as STUN compatible but the STUN test failed.
181+
r.STUN.Error != nil {
175182
r.Healthy = false
176183
}
177184
return nil

‎coderd/healthcheck/derp_test.go

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ func TestDERP(t *testing.T) {
6464
for _, node := range region.NodeReports {
6565
assert.True(t, node.Healthy)
6666
assert.True(t, node.CanExchangeMessages)
67-
assert.Positive(t, node.RoundTripPing)
67+
// TODO: test this without serializing time.Time over the wire.
68+
// assert.Positive(t, node.RoundTripPing)
6869
assert.Len(t, node.ClientLogs, 2)
6970
assert.Len(t, node.ClientLogs[0], 1)
7071
assert.Len(t, node.ClientErrs[0], 0)
@@ -105,7 +106,8 @@ func TestDERP(t *testing.T) {
105106
for _, node := range region.NodeReports {
106107
assert.True(t, node.Healthy)
107108
assert.True(t, node.CanExchangeMessages)
108-
assert.Positive(t, node.RoundTripPing)
109+
// TODO: test this without serializing time.Time over the wire.
110+
// assert.Positive(t, node.RoundTripPing)
109111
assert.Len(t, node.ClientLogs, 2)
110112
assert.Len(t, node.ClientLogs[0], 1)
111113
assert.Len(t, node.ClientErrs[0], 0)
@@ -168,7 +170,8 @@ func TestDERP(t *testing.T) {
168170
for _, node := range region.NodeReports {
169171
assert.False(t, node.Healthy)
170172
assert.True(t, node.CanExchangeMessages)
171-
assert.Positive(t, node.RoundTripPing)
173+
// TODO: test this without serializing time.Time over the wire.
174+
// assert.Positive(t, node.RoundTripPing)
172175
assert.Len(t, node.ClientLogs, 2)
173176
assert.Len(t, node.ClientLogs[0], 3)
174177
assert.Len(t, node.ClientLogs[1], 3)
@@ -183,6 +186,49 @@ func TestDERP(t *testing.T) {
183186
}
184187
}
185188
})
189+
190+
t.Run("OK/STUNOnly", func(t *testing.T) {
191+
t.Parallel()
192+
193+
var (
194+
ctx = context.Background()
195+
report = healthcheck.DERPReport{}
196+
opts = &healthcheck.DERPReportOptions{
197+
DERPMap: &tailcfg.DERPMap{Regions: map[int]*tailcfg.DERPRegion{
198+
1: {
199+
EmbeddedRelay: true,
200+
RegionID: 999,
201+
Nodes: []*tailcfg.DERPNode{{
202+
Name: "999stun0",
203+
RegionID: 999,
204+
HostName: "stun.l.google.com",
205+
STUNPort: 19302,
206+
STUNOnly: true,
207+
InsecureForTests: true,
208+
ForceHTTP: true,
209+
}},
210+
},
211+
}},
212+
}
213+
)
214+
215+
err := report.Run(ctx, opts)
216+
require.NoError(t, err)
217+
218+
assert.True(t, report.Healthy)
219+
for _, region := range report.Regions {
220+
assert.True(t, region.Healthy)
221+
for _, node := range region.NodeReports {
222+
assert.True(t, node.Healthy)
223+
assert.False(t, node.CanExchangeMessages)
224+
assert.Len(t, node.ClientLogs, 0)
225+
226+
assert.True(t, node.STUN.Enabled)
227+
assert.True(t, node.STUN.CanSTUN)
228+
assert.NoError(t, node.STUN.Error)
229+
}
230+
}
231+
})
186232
}
187233

188234
func tsDERPMap(ctx context.Context, t testing.TB) *tailcfg.DERPMap {

‎coderd/httpapi/cookie.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func StripCoderCookies(header string) string {
2424
name == codersdk.OAuth2StateCookie ||
2525
name == codersdk.OAuth2RedirectCookie ||
2626
name == codersdk.DevURLSessionTokenCookie ||
27-
name == codersdk.DevURLSessionTicketCookie {
27+
name == codersdk.DevURLSignedAppTokenCookie {
2828
continue
2929
}
3030
cookies = append(cookies, part)

0 commit comments

Comments
 (0)