Skip to content

Commit ca261fb

Browse files
matifalimatifali
committed
Merge remote-tracking branch 'origin/main' into build-edge-image
2 parents 01ce271 + 1472cce commit ca261fb

File tree

29 files changed

+1089
-550
lines changed

29 files changed

+1089
-550
lines changed

‎.github/actions/setup-go/action.yaml

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,24 +39,25 @@ runs:
3939
path: |
4040
${{ env.GOMODCACHE }}
4141
key: gomodcache-${{ runner.os }}-${{ hashFiles('**/go.sum') }}-${{ github.job }}
42-
restore-keys: |
43-
gomodcache-${{ runner.os }}-${{ hashFiles('**/go.sum') }}-
44-
gomodcache-${{ runner.os }}-
42+
# restore-keys aren't used because it causes the cache to grow
43+
# infinitely. go.sum changes very infrequently, so rebuilding from
44+
# scratch every now and then isn't terrible.
4545

4646
- name: Cache $GOCACHE
4747
uses: buildjet/cache@v3
4848
with:
4949
path: |
5050
${{ env.GOCACHE }}
51-
# Job name must be included in the key for effective
52-
# test cache reuse.
51+
# Job name must be included in the key for effective test cache reuse.
5352
# The key format is intentionally different than GOMODCACHE, because any
54-
# time a Go file changes we invalidate this cache, whereas GOMODCACHE
55-
# is only invalidated when go.sum changes.
56-
key: gocache-${{ runner.os }}-${{ github.job }}-${{ hashFiles('**/*.go', 'go.**') }}
53+
# time a Go file changes we invalidate this cache, whereas GOMODCACHE is
54+
# only invalidated when go.sum changes.
55+
# The number in the key is incremented when the cache gets too large,
56+
# since this technically grows without bound.
57+
key: gocache2-${{ runner.os }}-${{ github.job }}-${{ hashFiles('**/*.go', 'go.**') }}
5758
restore-keys: |
58-
gocache-${{ runner.os }}-${{ github.job }}-
59-
gocache-${{ runner.os }}-
59+
gocache2-${{ runner.os }}-${{ github.job }}-
60+
gocache2-${{ runner.os }}-
6061
6162
- name: Install gotestsum
6263
shell: bash

‎.github/workflows/ci.yaml

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ jobs:
4040
steps:
4141
- name: Checkout
4242
uses: actions/checkout@v3
43+
with:
44+
fetch-depth: 1
4345
# For pull requests it's not necessary to checkout the code
4446
- name: check changed files
4547
uses: dorny/paths-filter@v2
@@ -109,6 +111,8 @@ jobs:
109111
steps:
110112
- name: Checkout
111113
uses: actions/checkout@v3
114+
with:
115+
fetch-depth: 1
112116

113117
- name: Setup Node
114118
uses: ./.github/actions/setup-node
@@ -162,6 +166,8 @@ jobs:
162166
steps:
163167
- name: Checkout
164168
uses: actions/checkout@v3
169+
with:
170+
fetch-depth: 1
165171

166172
- name: Setup Node
167173
uses: ./.github/actions/setup-node
@@ -207,6 +213,8 @@ jobs:
207213
steps:
208214
- name: Checkout
209215
uses: actions/checkout@v3
216+
with:
217+
fetch-depth: 1
210218

211219
- name: Setup Node
212220
uses: ./.github/actions/setup-node
@@ -256,6 +264,8 @@ jobs:
256264
steps:
257265
- name: Checkout
258266
uses: actions/checkout@v3
267+
with:
268+
fetch-depth: 1
259269

260270
- name: Setup Go
261271
uses: ./.github/actions/setup-go
@@ -323,6 +333,8 @@ jobs:
323333
steps:
324334
- name: Checkout
325335
uses: actions/checkout@v3
336+
with:
337+
fetch-depth: 1
326338

327339
- name: Setup Go
328340
uses: ./.github/actions/setup-go
@@ -369,6 +381,8 @@ jobs:
369381
steps:
370382
- name: Checkout
371383
uses: actions/checkout@v3
384+
with:
385+
fetch-depth: 1
372386

373387
- name: Setup Go
374388
uses: ./.github/actions/setup-go
@@ -488,6 +502,8 @@ jobs:
488502
steps:
489503
- name: Checkout
490504
uses: actions/checkout@v3
505+
with:
506+
fetch-depth: 1
491507

492508
- name: Setup Node
493509
uses: ./.github/actions/setup-node
@@ -516,6 +532,8 @@ jobs:
516532
steps:
517533
- name: Checkout
518534
uses: actions/checkout@v3
535+
with:
536+
fetch-depth: 1
519537

520538
- name: Setup Node
521539
uses: ./.github/actions/setup-node
@@ -619,6 +637,7 @@ jobs:
619637
- name: Checkout
620638
uses: actions/checkout@v3
621639
with:
640+
# 0 is required here for version.sh to work.
622641
fetch-depth: 0
623642

624643
- name: Setup Node

‎.github/workflows/release.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,8 @@ jobs:
407407
- name: Comment on PR
408408
if: ${{ !inputs.dry_run }}
409409
run: |
410+
# wait 30 seconds
411+
Start-Sleep -Seconds 30.0
410412
# Find the PR that wingetcreate just made.
411413
$version = "${{ needs.release.outputs.version }}".Trim('v')
412414
$pr_list = gh pr list --repo microsoft/winget-pkgs --search "author:cdrci Coder.Coder version ${version}" --limit 1 --json number | `

‎agent/agent.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ type Client interface {
8585

8686
type Agent interface {
8787
HTTPDebug() http.Handler
88+
// TailnetConn may be nil.
89+
TailnetConn() *tailnet.Conn
8890
io.Closer
8991
}
9092

@@ -200,6 +202,10 @@ type agent struct {
200202
metrics *agentMetrics
201203
}
202204

205+
func (a *agent) TailnetConn() *tailnet.Conn {
206+
return a.network
207+
}
208+
203209
func (a *agent) init(ctx context.Context) {
204210
sshSrv, err := agentssh.NewServer(ctx, a.logger.Named("ssh-server"), a.prometheusRegistry, a.filesystem, a.sshMaxTimeout, "")
205211
if err != nil {

‎agent/agent_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1818,6 +1818,15 @@ func TestAgent_UpdatedDERP(t *testing.T) {
18181818
})
18191819
require.NoError(t, err)
18201820

1821+
require.Eventually(t, func() bool {
1822+
conn := closer.TailnetConn()
1823+
if conn == nil {
1824+
return false
1825+
}
1826+
regionIDs := conn.DERPMap().RegionIDs()
1827+
return len(regionIDs) == 1 && regionIDs[0] == 2 && conn.Node().PreferredDERP == 2
1828+
}, testutil.WaitLong, testutil.IntervalFast)
1829+
18211830
// Connect from a second client and make sure it uses the new DERP map.
18221831
conn2 := newClientConn(newDerpMap)
18231832
require.Equal(t, []int{2}, conn2.DERPMap().RegionIDs())

‎coderd/coderdtest/coderdtest.go

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -303,9 +303,21 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
303303
accessURL = serverURL
304304
}
305305

306-
stunAddr, stunCleanup := stuntest.ServeWithPacketListener(t, nettype.Std{})
307-
stunAddr.IP = net.ParseIP("127.0.0.1")
308-
t.Cleanup(stunCleanup)
306+
// If the STUNAddresses setting is empty or the default, start a STUN
307+
// server. Otherwise, use the value as is.
308+
var (
309+
stunAddresses []string
310+
dvStunAddresses = options.DeploymentValues.DERP.Server.STUNAddresses.Value()
311+
)
312+
if len(dvStunAddresses) == 0 || (len(dvStunAddresses) == 1 && dvStunAddresses[0] == "stun.l.google.com:19302") {
313+
stunAddr, stunCleanup := stuntest.ServeWithPacketListener(t, nettype.Std{})
314+
stunAddr.IP = net.ParseIP("127.0.0.1")
315+
t.Cleanup(stunCleanup)
316+
stunAddresses = []string{stunAddr.String()}
317+
options.DeploymentValues.DERP.Server.STUNAddresses = stunAddresses
318+
} else if dvStunAddresses[0] != "disable" {
319+
stunAddresses = options.DeploymentValues.DERP.Server.STUNAddresses.Value()
320+
}
309321

310322
derpServer := derp.NewServer(key.NewNode(), tailnet.Logger(slogtest.Make(t, nil).Named("derp").Leveled(slog.LevelDebug)))
311323
derpServer.SetMeshKey("test-key")
@@ -346,7 +358,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
346358
if !options.DeploymentValues.DERP.Server.Enable.Value() {
347359
region = nil
348360
}
349-
derpMap, err := tailnet.NewDERPMap(ctx, region, []string{stunAddr.String()}, "", "", options.DeploymentValues.DERP.Config.BlockDirect.Value())
361+
derpMap, err := tailnet.NewDERPMap(ctx, region, stunAddresses, "", "", options.DeploymentValues.DERP.Config.BlockDirect.Value())
350362
require.NoError(t, err)
351363

352364
return func(h http.Handler) {

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -953,14 +953,9 @@ func (q *querier) GetLatestWorkspaceBuilds(ctx context.Context) ([]database.Work
953953
}
954954

955955
func (q *querier) GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]database.WorkspaceBuild, error) {
956-
// This is not ideal as not all builds will be returned if the workspace cannot be read.
957-
// This should probably be handled differently? Maybe join workspace builds with workspace
958-
// ownership properties and filter on that.
959-
for _, id := range ids {
960-
_, err := q.GetWorkspaceByID(ctx, id)
961-
if err != nil {
962-
return nil, err
963-
}
956+
// This function is a system function until we implement a join for workspace builds.
957+
if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil {
958+
return nil, err
964959
}
965960

966961
return q.db.GetLatestWorkspaceBuildsByWorkspaceIDs(ctx, ids)

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,11 +1024,6 @@ func (s *MethodTestSuite) TestWorkspace() {
10241024
b := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID})
10251025
check.Args(ws.ID).Asserts(ws, rbac.ActionRead).Returns(b)
10261026
}))
1027-
s.Run("GetLatestWorkspaceBuildsByWorkspaceIDs", s.Subtest(func(db database.Store, check *expects) {
1028-
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
1029-
b := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID})
1030-
check.Args([]uuid.UUID{ws.ID}).Asserts(ws, rbac.ActionRead).Returns(slice.New(b))
1031-
}))
10321027
s.Run("GetWorkspaceAgentByID", s.Subtest(func(db database.Store, check *expects) {
10331028
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
10341029
build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()})
@@ -1298,6 +1293,11 @@ func (s *MethodTestSuite) TestSystemFunctions() {
12981293
LoginType: database.LoginTypeGithub,
12991294
}).Asserts(rbac.ResourceSystem, rbac.ActionUpdate).Returns(l)
13001295
}))
1296+
s.Run("GetLatestWorkspaceBuildsByWorkspaceIDs", s.Subtest(func(db database.Store, check *expects) {
1297+
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
1298+
b := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID})
1299+
check.Args([]uuid.UUID{ws.ID}).Asserts(rbac.ResourceSystem, rbac.ActionRead).Returns(slice.New(b))
1300+
}))
13011301
s.Run("UpsertDefaultProxy", s.Subtest(func(db database.Store, check *expects) {
13021302
check.Args(database.UpsertDefaultProxyParams{}).Asserts(rbac.ResourceSystem, rbac.ActionUpdate).Returns()
13031303
}))

‎coderd/healthcheck/derp.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ func (r *DERPNodeReport) derpURL() *url.URL {
172172
if r.Node.HostName == "" {
173173
derpURL.Host = r.Node.IPv4
174174
}
175-
if r.Node.DERPPort != 0 {
175+
if r.Node.DERPPort != 0 && !(r.Node.DERPPort == 443 && derpURL.Scheme == "https") && !(r.Node.DERPPort == 80 && derpURL.Scheme == "http") {
176176
derpURL.Host = fmt.Sprintf("%s:%d", derpURL.Host, r.Node.DERPPort)
177177
}
178178

‎coderd/workspaceagents.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -866,15 +866,14 @@ func (api *API) derpMapUpdates(rw http.ResponseWriter, r *http.Request) {
866866
lastDERPMap = derpMap
867867
}
868868

869+
ticker.Reset(api.Options.DERPMapUpdateFrequency)
869870
select {
870871
case <-ctx.Done():
871872
return
872873
case <-api.ctx.Done():
873874
return
874875
case <-ticker.C:
875876
}
876-
877-
ticker.Reset(api.Options.DERPMapUpdateFrequency)
878877
}
879878
}
880879

‎coderd/workspaceagents_test.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,11 +1270,9 @@ func TestWorkspaceAgent_UpdatedDERP(t *testing.T) {
12701270
defer closer.Close()
12711271
user := coderdtest.CreateFirstUser(t, client)
12721272

1273-
originalDerpMap := api.DERPMap()
1274-
require.NotNil(t, originalDerpMap)
1275-
12761273
// Change the DERP mapper to our custom one.
12771274
var currentDerpMap atomic.Pointer[tailcfg.DERPMap]
1275+
originalDerpMap, _ := tailnettest.RunDERPAndSTUN(t)
12781276
currentDerpMap.Store(originalDerpMap)
12791277
derpMapFn := func(_ *tailcfg.DERPMap) *tailcfg.DERPMap {
12801278
return currentDerpMap.Load().Clone()
@@ -1304,10 +1302,8 @@ func TestWorkspaceAgent_UpdatedDERP(t *testing.T) {
13041302
resources := coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)
13051303
agentID := resources[0].Agents[0].ID
13061304

1307-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
1308-
defer cancel()
1309-
13101305
// Connect from a client.
1306+
ctx := testutil.Context(t, testutil.WaitLong)
13111307
conn1, err := client.DialWorkspaceAgent(ctx, agentID, &codersdk.DialWorkspaceAgentOptions{
13121308
Logger: logger.Named("client1"),
13131309
})
@@ -1328,7 +1324,14 @@ func TestWorkspaceAgent_UpdatedDERP(t *testing.T) {
13281324
currentDerpMap.Store(newDerpMap)
13291325

13301326
// Wait for the agent's DERP map to be updated.
1331-
// TODO: this
1327+
require.Eventually(t, func() bool {
1328+
conn := agentCloser.TailnetConn()
1329+
if conn == nil {
1330+
return false
1331+
}
1332+
regionIDs := conn.DERPMap().RegionIDs()
1333+
return len(regionIDs) == 1 && regionIDs[0] == 2 && conn.Node().PreferredDERP == 2
1334+
}, testutil.WaitLong, testutil.IntervalFast)
13321335

13331336
// Wait for the DERP map to be updated on the existing client.
13341337
require.Eventually(t, func() bool {

‎coderd/workspacebuilds.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -635,9 +635,6 @@ type workspaceBuildsData struct {
635635

636636
func (api *API) workspaceBuildsData(ctx context.Context, workspaces []database.Workspace, workspaceBuilds []database.WorkspaceBuild) (workspaceBuildsData, error) {
637637
userIDs := make([]uuid.UUID, 0, len(workspaceBuilds))
638-
for _, build := range workspaceBuilds {
639-
userIDs = append(userIDs, build.InitiatorID)
640-
}
641638
for _, workspace := range workspaces {
642639
userIDs = append(userIDs, workspace.OwnerID)
643640
}

‎coderd/workspaces.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"cdr.dev/slog"
1818
"github.com/coder/coder/coderd/audit"
1919
"github.com/coder/coder/coderd/database"
20+
"github.com/coder/coder/coderd/database/dbauthz"
2021
"github.com/coder/coder/coderd/httpapi"
2122
"github.com/coder/coder/coderd/httpmw"
2223
"github.com/coder/coder/coderd/rbac"
@@ -1031,7 +1032,9 @@ func (api *API) workspaceData(ctx context.Context, workspaces []database.Workspa
10311032
return workspaceData{}, xerrors.Errorf("get templates: %w", err)
10321033
}
10331034

1034-
builds, err := api.Database.GetLatestWorkspaceBuildsByWorkspaceIDs(ctx, workspaceIDs)
1035+
// This query must be run as system restricted to be efficient.
1036+
// nolint:gocritic
1037+
builds, err := api.Database.GetLatestWorkspaceBuildsByWorkspaceIDs(dbauthz.AsSystemRestricted(ctx), workspaceIDs)
10351038
if err != nil && !errors.Is(err, sql.ErrNoRows) {
10361039
return workspaceData{}, xerrors.Errorf("get workspace builds: %w", err)
10371040
}

‎codersdk/agentsdk/agentsdk.go

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -211,18 +211,26 @@ func (c *Client) DERPMapUpdates(ctx context.Context) (<-chan DERPMapUpdate, io.C
211211
if err != nil {
212212
update.Err = err
213213
update.DERPMap = nil
214-
return
215214
}
216-
err = c.rewriteDerpMap(update.DERPMap)
217-
if err != nil {
218-
update.Err = err
219-
update.DERPMap = nil
220-
return
215+
if update.DERPMap != nil {
216+
err = c.rewriteDerpMap(update.DERPMap)
217+
if err != nil {
218+
update.Err = err
219+
update.DERPMap = nil
220+
}
221221
}
222222

223223
select {
224224
case updates <- update:
225225
case <-ctx.Done():
226+
// Unblock the caller if they're waiting for an update.
227+
select {
228+
case updates <- DERPMapUpdate{Err: ctx.Err()}:
229+
default:
230+
}
231+
return
232+
}
233+
if update.Err != nil {
226234
return
227235
}
228236
}
@@ -231,8 +239,8 @@ func (c *Client) DERPMapUpdates(ctx context.Context) (<-chan DERPMapUpdate, io.C
231239
return updates, &closer{
232240
closeFunc: func() error {
233241
cancelFunc()
234-
_ = wsNetConn.Close()
235242
<-pingClosed
243+
_ = conn.Close(websocket.StatusGoingAway, "Listen closed")
236244
<-updatesClosed
237245
return nil
238246
},

0 commit comments

Comments
 (0)