diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index 5b68e4b26c20d..f9902ede655cf 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -47,6 +47,6 @@ jobs: # Upload the results to GitHub's code scanning dashboard. - name: "Upload to code-scanning" - uses: github/codeql-action/upload-sarif@60168efe1c415ce0f5521ea06d5c2062adbeed1b # v3.28.17 + uses: github/codeql-action/upload-sarif@ff0a06e83cb2de871e5a09832bc6a81e7276941f # v3.28.18 with: sarif_file: results.sarif diff --git a/.github/workflows/security.yaml b/.github/workflows/security.yaml index f9f461cfe9966..721584b89e202 100644 --- a/.github/workflows/security.yaml +++ b/.github/workflows/security.yaml @@ -38,7 +38,7 @@ jobs: uses: ./.github/actions/setup-go - name: Initialize CodeQL - uses: github/codeql-action/init@60168efe1c415ce0f5521ea06d5c2062adbeed1b # v3.28.17 + uses: github/codeql-action/init@ff0a06e83cb2de871e5a09832bc6a81e7276941f # v3.28.18 with: languages: go, javascript @@ -48,7 +48,7 @@ jobs: rm Makefile - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@60168efe1c415ce0f5521ea06d5c2062adbeed1b # v3.28.17 + uses: github/codeql-action/analyze@ff0a06e83cb2de871e5a09832bc6a81e7276941f # v3.28.18 - name: Send Slack notification on failure if: ${{ failure() }} @@ -150,7 +150,7 @@ jobs: severity: "CRITICAL,HIGH" - name: Upload Trivy scan results to GitHub Security tab - uses: github/codeql-action/upload-sarif@60168efe1c415ce0f5521ea06d5c2062adbeed1b # v3.28.17 + uses: github/codeql-action/upload-sarif@ff0a06e83cb2de871e5a09832bc6a81e7276941f # v3.28.18 with: sarif_file: trivy-results.sarif category: "Trivy" diff --git a/agent/agent.go b/agent/agent.go index ffdacfb64ba75..927612302bf71 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -95,8 +95,8 @@ type Options struct { } type Client interface { - ConnectRPC24(ctx context.Context) ( - proto.DRPCAgentClient24, tailnetproto.DRPCTailnetClient24, error, + ConnectRPC25(ctx context.Context) ( + proto.DRPCAgentClient25, tailnetproto.DRPCTailnetClient25, error, ) RewriteDERPMap(derpMap *tailcfg.DERPMap) } @@ -908,7 +908,7 @@ func (a *agent) run() (retErr error) { a.sessionToken.Store(&sessionToken) // ConnectRPC returns the dRPC connection we use for the Agent and Tailnet v2+ APIs - aAPI, tAPI, err := a.client.ConnectRPC24(a.hardCtx) + aAPI, tAPI, err := a.client.ConnectRPC25(a.hardCtx) if err != nil { return err } diff --git a/agent/agentcontainers/acmock/acmock.go b/agent/agentcontainers/acmock/acmock.go index 93c84e8c54fd3..869d2f7d0923b 100644 --- a/agent/agentcontainers/acmock/acmock.go +++ b/agent/agentcontainers/acmock/acmock.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: .. (interfaces: Lister) +// Source: .. (interfaces: Lister,DevcontainerCLI) // // Generated by this command: // -// mockgen -destination ./acmock.go -package acmock .. Lister +// mockgen -destination ./acmock.go -package acmock .. Lister,DevcontainerCLI // // Package acmock is a generated GoMock package. @@ -13,6 +13,7 @@ import ( context "context" reflect "reflect" + agentcontainers "github.com/coder/coder/v2/agent/agentcontainers" codersdk "github.com/coder/coder/v2/codersdk" gomock "go.uber.org/mock/gomock" ) @@ -55,3 +56,47 @@ func (mr *MockListerMockRecorder) List(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "List", reflect.TypeOf((*MockLister)(nil).List), ctx) } + +// MockDevcontainerCLI is a mock of DevcontainerCLI interface. +type MockDevcontainerCLI struct { + ctrl *gomock.Controller + recorder *MockDevcontainerCLIMockRecorder + isgomock struct{} +} + +// MockDevcontainerCLIMockRecorder is the mock recorder for MockDevcontainerCLI. +type MockDevcontainerCLIMockRecorder struct { + mock *MockDevcontainerCLI +} + +// NewMockDevcontainerCLI creates a new mock instance. +func NewMockDevcontainerCLI(ctrl *gomock.Controller) *MockDevcontainerCLI { + mock := &MockDevcontainerCLI{ctrl: ctrl} + mock.recorder = &MockDevcontainerCLIMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockDevcontainerCLI) EXPECT() *MockDevcontainerCLIMockRecorder { + return m.recorder +} + +// Up mocks base method. +func (m *MockDevcontainerCLI) Up(ctx context.Context, workspaceFolder, configPath string, opts ...agentcontainers.DevcontainerCLIUpOptions) (string, error) { + m.ctrl.T.Helper() + varargs := []any{ctx, workspaceFolder, configPath} + for _, a := range opts { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "Up", varargs...) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Up indicates an expected call of Up. +func (mr *MockDevcontainerCLIMockRecorder) Up(ctx, workspaceFolder, configPath any, opts ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{ctx, workspaceFolder, configPath}, opts...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Up", reflect.TypeOf((*MockDevcontainerCLI)(nil).Up), varargs...) +} diff --git a/agent/agentcontainers/acmock/doc.go b/agent/agentcontainers/acmock/doc.go index 47679708b0fc8..b807efa253b75 100644 --- a/agent/agentcontainers/acmock/doc.go +++ b/agent/agentcontainers/acmock/doc.go @@ -1,4 +1,4 @@ // Package acmock contains a mock implementation of agentcontainers.Lister for use in tests. package acmock -//go:generate mockgen -destination ./acmock.go -package acmock .. Lister +//go:generate mockgen -destination ./acmock.go -package acmock .. Lister,DevcontainerCLI diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index c3393c3fdec9e..f2164c9a874ff 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -69,6 +69,15 @@ func WithClock(clock quartz.Clock) Option { } } +// WithCacheDuration sets the cache duration for the API. +// This is used to control how often the API refreshes the list of +// containers. The default is 10 seconds. +func WithCacheDuration(d time.Duration) Option { + return func(api *API) { + api.cacheDuration = d + } +} + // WithExecer sets the agentexec.Execer implementation to use. func WithExecer(execer agentexec.Execer) Option { return func(api *API) { @@ -336,7 +345,8 @@ func (api *API) getContainers(ctx context.Context) (codersdk.WorkspaceAgentListC } // Check if the container is running and update the known devcontainers. - for _, container := range updated.Containers { + for i := range updated.Containers { + container := &updated.Containers[i] workspaceFolder := container.Labels[DevcontainerLocalFolderLabel] configFile := container.Labels[DevcontainerConfigFileLabel] @@ -344,6 +354,20 @@ func (api *API) getContainers(ctx context.Context) (codersdk.WorkspaceAgentListC continue } + container.DevcontainerDirty = dirtyStates[workspaceFolder] + if container.DevcontainerDirty { + lastModified, hasModTime := api.configFileModifiedTimes[configFile] + if hasModTime && container.CreatedAt.After(lastModified) { + api.logger.Info(ctx, "new container created after config modification, not marking as dirty", + slog.F("container", container.ID), + slog.F("created_at", container.CreatedAt), + slog.F("config_modified_at", lastModified), + slog.F("file", configFile), + ) + container.DevcontainerDirty = false + } + } + // Check if this is already in our known list. if knownIndex := slices.IndexFunc(api.knownDevcontainers, func(dc codersdk.WorkspaceAgentDevcontainer) bool { return dc.WorkspaceFolder == workspaceFolder @@ -356,7 +380,7 @@ func (api *API) getContainers(ctx context.Context) (codersdk.WorkspaceAgentListC } } api.knownDevcontainers[knownIndex].Running = container.Running - api.knownDevcontainers[knownIndex].Container = &container + api.knownDevcontainers[knownIndex].Container = container // Check if this container was created after the config // file was modified. @@ -395,28 +419,14 @@ func (api *API) getContainers(ctx context.Context) (codersdk.WorkspaceAgentListC } } - dirty := dirtyStates[workspaceFolder] - if dirty { - lastModified, hasModTime := api.configFileModifiedTimes[configFile] - if hasModTime && container.CreatedAt.After(lastModified) { - api.logger.Info(ctx, "new container created after config modification, not marking as dirty", - slog.F("container", container.ID), - slog.F("created_at", container.CreatedAt), - slog.F("config_modified_at", lastModified), - slog.F("file", configFile), - ) - dirty = false - } - } - api.knownDevcontainers = append(api.knownDevcontainers, codersdk.WorkspaceAgentDevcontainer{ ID: uuid.New(), Name: name, WorkspaceFolder: workspaceFolder, ConfigPath: configFile, Running: container.Running, - Dirty: dirty, - Container: &container, + Dirty: container.DevcontainerDirty, + Container: container, }) } @@ -510,6 +520,13 @@ func (api *API) handleDevcontainerRecreate(w http.ResponseWriter, r *http.Reques slog.F("name", api.knownDevcontainers[i].Name), ) api.knownDevcontainers[i].Dirty = false + // TODO(mafredri): This should be handled by a service that + // updates the devcontainer state periodically and on-demand. + api.knownDevcontainers[i].Container = nil + // Set the modified time to the zero value to indicate that + // the containers list must be refreshed. This will see to + // it that the new container is re-assigned. + api.mtime = time.Time{} } return } @@ -579,6 +596,9 @@ func (api *API) markDevcontainerDirty(configPath string, modifiedAt time.Time) { slog.F("modified_at", modifiedAt), ) api.knownDevcontainers[i].Dirty = true + if api.knownDevcontainers[i].Container != nil { + api.knownDevcontainers[i].Container.DevcontainerDirty = true + } } } }) diff --git a/agent/agentcontainers/api_internal_test.go b/agent/agentcontainers/api_internal_test.go deleted file mode 100644 index 331c41e8df10b..0000000000000 --- a/agent/agentcontainers/api_internal_test.go +++ /dev/null @@ -1,163 +0,0 @@ -package agentcontainers - -import ( - "math/rand" - "strings" - "testing" - "time" - - "github.com/google/uuid" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "go.uber.org/mock/gomock" - - "cdr.dev/slog" - "cdr.dev/slog/sloggers/slogtest" - "github.com/coder/coder/v2/agent/agentcontainers/acmock" - "github.com/coder/coder/v2/codersdk" - "github.com/coder/coder/v2/testutil" - "github.com/coder/quartz" -) - -func TestAPI(t *testing.T) { - t.Parallel() - - // List tests the API.getContainers method using a mock - // implementation. It specifically tests caching behavior. - t.Run("List", func(t *testing.T) { - t.Parallel() - - fakeCt := fakeContainer(t) - fakeCt2 := fakeContainer(t) - makeResponse := func(cts ...codersdk.WorkspaceAgentContainer) codersdk.WorkspaceAgentListContainersResponse { - return codersdk.WorkspaceAgentListContainersResponse{Containers: cts} - } - - // Each test case is called multiple times to ensure idempotency - for _, tc := range []struct { - name string - // data to be stored in the handler - cacheData codersdk.WorkspaceAgentListContainersResponse - // duration of cache - cacheDur time.Duration - // relative age of the cached data - cacheAge time.Duration - // function to set up expectations for the mock - setupMock func(*acmock.MockLister) - // expected result - expected codersdk.WorkspaceAgentListContainersResponse - // expected error - expectedErr string - }{ - { - name: "no cache", - setupMock: func(mcl *acmock.MockLister) { - mcl.EXPECT().List(gomock.Any()).Return(makeResponse(fakeCt), nil).AnyTimes() - }, - expected: makeResponse(fakeCt), - }, - { - name: "no data", - cacheData: makeResponse(), - cacheAge: 2 * time.Second, - cacheDur: time.Second, - setupMock: func(mcl *acmock.MockLister) { - mcl.EXPECT().List(gomock.Any()).Return(makeResponse(fakeCt), nil).AnyTimes() - }, - expected: makeResponse(fakeCt), - }, - { - name: "cached data", - cacheAge: time.Second, - cacheData: makeResponse(fakeCt), - cacheDur: 2 * time.Second, - expected: makeResponse(fakeCt), - }, - { - name: "lister error", - setupMock: func(mcl *acmock.MockLister) { - mcl.EXPECT().List(gomock.Any()).Return(makeResponse(), assert.AnError).AnyTimes() - }, - expectedErr: assert.AnError.Error(), - }, - { - name: "stale cache", - cacheAge: 2 * time.Second, - cacheData: makeResponse(fakeCt), - cacheDur: time.Second, - setupMock: func(mcl *acmock.MockLister) { - mcl.EXPECT().List(gomock.Any()).Return(makeResponse(fakeCt2), nil).AnyTimes() - }, - expected: makeResponse(fakeCt2), - }, - } { - tc := tc - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - var ( - ctx = testutil.Context(t, testutil.WaitShort) - clk = quartz.NewMock(t) - ctrl = gomock.NewController(t) - mockLister = acmock.NewMockLister(ctrl) - now = time.Now().UTC() - logger = slogtest.Make(t, nil).Leveled(slog.LevelDebug) - api = NewAPI(logger, WithLister(mockLister)) - ) - defer api.Close() - - api.cacheDuration = tc.cacheDur - api.clock = clk - api.containers = tc.cacheData - if tc.cacheAge != 0 { - api.mtime = now.Add(-tc.cacheAge) - } - if tc.setupMock != nil { - tc.setupMock(mockLister) - } - - clk.Set(now).MustWait(ctx) - - // Repeat the test to ensure idempotency - for i := 0; i < 2; i++ { - actual, err := api.getContainers(ctx) - if tc.expectedErr != "" { - require.Empty(t, actual, "expected no data (attempt %d)", i) - require.ErrorContains(t, err, tc.expectedErr, "expected error (attempt %d)", i) - } else { - require.NoError(t, err, "expected no error (attempt %d)", i) - require.Equal(t, tc.expected, actual, "expected containers to be equal (attempt %d)", i) - } - } - }) - } - }) -} - -func fakeContainer(t *testing.T, mut ...func(*codersdk.WorkspaceAgentContainer)) codersdk.WorkspaceAgentContainer { - t.Helper() - ct := codersdk.WorkspaceAgentContainer{ - CreatedAt: time.Now().UTC(), - ID: uuid.New().String(), - FriendlyName: testutil.GetRandomName(t), - Image: testutil.GetRandomName(t) + ":" + strings.Split(uuid.New().String(), "-")[0], - Labels: map[string]string{ - testutil.GetRandomName(t): testutil.GetRandomName(t), - }, - Running: true, - Ports: []codersdk.WorkspaceAgentContainerPort{ - { - Network: "tcp", - Port: testutil.RandomPortNoListen(t), - HostPort: testutil.RandomPortNoListen(t), - //nolint:gosec // this is a test - HostIP: []string{"127.0.0.1", "[::1]", "localhost", "0.0.0.0", "[::]", testutil.GetRandomName(t)}[rand.Intn(6)], - }, - }, - Status: testutil.MustRandString(t, 10), - Volumes: map[string]string{testutil.GetRandomName(t): testutil.GetRandomName(t)}, - } - for _, m := range mut { - m(&ct) - } - return ct -} diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index 2c602de5cff3a..2e173b7d5a6b4 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -3,8 +3,10 @@ package agentcontainers_test import ( "context" "encoding/json" + "math/rand" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -13,11 +15,13 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" "golang.org/x/xerrors" "cdr.dev/slog" "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/v2/agent/agentcontainers" + "github.com/coder/coder/v2/agent/agentcontainers/acmock" "github.com/coder/coder/v2/agent/agentcontainers/watcher" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" @@ -146,6 +150,136 @@ func (w *fakeWatcher) sendEventWaitNextCalled(ctx context.Context, event fsnotif func TestAPI(t *testing.T) { t.Parallel() + // List tests the API.getContainers method using a mock + // implementation. It specifically tests caching behavior. + t.Run("List", func(t *testing.T) { + t.Parallel() + + fakeCt := fakeContainer(t) + fakeCt2 := fakeContainer(t) + makeResponse := func(cts ...codersdk.WorkspaceAgentContainer) codersdk.WorkspaceAgentListContainersResponse { + return codersdk.WorkspaceAgentListContainersResponse{Containers: cts} + } + + // Each test case is called multiple times to ensure idempotency + for _, tc := range []struct { + name string + // data to be stored in the handler + cacheData codersdk.WorkspaceAgentListContainersResponse + // duration of cache + cacheDur time.Duration + // relative age of the cached data + cacheAge time.Duration + // function to set up expectations for the mock + setupMock func(mcl *acmock.MockLister, preReq *gomock.Call) + // expected result + expected codersdk.WorkspaceAgentListContainersResponse + // expected error + expectedErr string + }{ + { + name: "no cache", + setupMock: func(mcl *acmock.MockLister, preReq *gomock.Call) { + mcl.EXPECT().List(gomock.Any()).Return(makeResponse(fakeCt), nil).After(preReq).AnyTimes() + }, + expected: makeResponse(fakeCt), + }, + { + name: "no data", + cacheData: makeResponse(), + cacheAge: 2 * time.Second, + cacheDur: time.Second, + setupMock: func(mcl *acmock.MockLister, preReq *gomock.Call) { + mcl.EXPECT().List(gomock.Any()).Return(makeResponse(fakeCt), nil).After(preReq).AnyTimes() + }, + expected: makeResponse(fakeCt), + }, + { + name: "cached data", + cacheAge: time.Second, + cacheData: makeResponse(fakeCt), + cacheDur: 2 * time.Second, + expected: makeResponse(fakeCt), + }, + { + name: "lister error", + setupMock: func(mcl *acmock.MockLister, preReq *gomock.Call) { + mcl.EXPECT().List(gomock.Any()).Return(makeResponse(), assert.AnError).After(preReq).AnyTimes() + }, + expectedErr: assert.AnError.Error(), + }, + { + name: "stale cache", + cacheAge: 2 * time.Second, + cacheData: makeResponse(fakeCt), + cacheDur: time.Second, + setupMock: func(mcl *acmock.MockLister, preReq *gomock.Call) { + mcl.EXPECT().List(gomock.Any()).Return(makeResponse(fakeCt2), nil).After(preReq).AnyTimes() + }, + expected: makeResponse(fakeCt2), + }, + } { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + var ( + ctx = testutil.Context(t, testutil.WaitShort) + clk = quartz.NewMock(t) + ctrl = gomock.NewController(t) + mockLister = acmock.NewMockLister(ctrl) + now = time.Now().UTC() + logger = slogtest.Make(t, nil).Leveled(slog.LevelDebug) + r = chi.NewRouter() + api = agentcontainers.NewAPI(logger, + agentcontainers.WithCacheDuration(tc.cacheDur), + agentcontainers.WithClock(clk), + agentcontainers.WithLister(mockLister), + ) + ) + defer api.Close() + + r.Mount("/", api.Routes()) + + preReq := mockLister.EXPECT().List(gomock.Any()).Return(tc.cacheData, nil).Times(1) + if tc.setupMock != nil { + tc.setupMock(mockLister, preReq) + } + + if tc.cacheAge != 0 { + clk.Set(now.Add(-tc.cacheAge)).MustWait(ctx) + } else { + clk.Set(now).MustWait(ctx) + } + + // Prime the cache with the initial data. + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + r.ServeHTTP(rec, req) + + clk.Set(now).MustWait(ctx) + + // Repeat the test to ensure idempotency + for i := 0; i < 2; i++ { + req = httptest.NewRequest(http.MethodGet, "/", nil) + rec = httptest.NewRecorder() + r.ServeHTTP(rec, req) + + if tc.expectedErr != "" { + got := &codersdk.Error{} + err := json.NewDecoder(rec.Body).Decode(got) + require.NoError(t, err, "unmarshal response failed") + require.ErrorContains(t, got, tc.expectedErr, "expected error (attempt %d)", i) + } else { + var got codersdk.WorkspaceAgentListContainersResponse + err := json.NewDecoder(rec.Body).Decode(&got) + require.NoError(t, err, "unmarshal response failed") + require.Equal(t, tc.expected, got, "expected containers to be equal (attempt %d)", i) + } + } + }) + } + }) + t.Run("Recreate", func(t *testing.T) { t.Parallel() @@ -660,6 +794,9 @@ func TestAPI(t *testing.T) { require.NoError(t, err) require.Len(t, response.Devcontainers, 1) assert.False(t, response.Devcontainers[0].Dirty, + "devcontainer should not be marked as dirty initially") + require.NotNil(t, response.Devcontainers[0].Container, "container should not be nil") + assert.False(t, response.Devcontainers[0].Container.DevcontainerDirty, "container should not be marked as dirty initially") // Verify the watcher is watching the config file. @@ -689,6 +826,9 @@ func TestAPI(t *testing.T) { require.Len(t, response.Devcontainers, 1) assert.True(t, response.Devcontainers[0].Dirty, "container should be marked as dirty after config file was modified") + require.NotNil(t, response.Devcontainers[0].Container, "container should not be nil") + assert.True(t, response.Devcontainers[0].Container.DevcontainerDirty, + "container should be marked as dirty after config file was modified") mClock.Advance(time.Minute).MustWait(ctx) @@ -707,7 +847,10 @@ func TestAPI(t *testing.T) { require.NoError(t, err) require.Len(t, response.Devcontainers, 1) assert.False(t, response.Devcontainers[0].Dirty, - "dirty flag should be cleared after container recreation") + "dirty flag should be cleared on the devcontainer after container recreation") + require.NotNil(t, response.Devcontainers[0].Container, "container should not be nil") + assert.False(t, response.Devcontainers[0].Container.DevcontainerDirty, + "dirty flag should be cleared on the container after container recreation") }) } @@ -725,3 +868,32 @@ func mustFindDevcontainerByPath(t *testing.T, devcontainers []codersdk.Workspace require.Failf(t, "no devcontainer found with workspace folder %q", path) return codersdk.WorkspaceAgentDevcontainer{} // Unreachable, but required for compilation } + +func fakeContainer(t *testing.T, mut ...func(*codersdk.WorkspaceAgentContainer)) codersdk.WorkspaceAgentContainer { + t.Helper() + ct := codersdk.WorkspaceAgentContainer{ + CreatedAt: time.Now().UTC(), + ID: uuid.New().String(), + FriendlyName: testutil.GetRandomName(t), + Image: testutil.GetRandomName(t) + ":" + strings.Split(uuid.New().String(), "-")[0], + Labels: map[string]string{ + testutil.GetRandomName(t): testutil.GetRandomName(t), + }, + Running: true, + Ports: []codersdk.WorkspaceAgentContainerPort{ + { + Network: "tcp", + Port: testutil.RandomPortNoListen(t), + HostPort: testutil.RandomPortNoListen(t), + //nolint:gosec // this is a test + HostIP: []string{"127.0.0.1", "[::1]", "localhost", "0.0.0.0", "[::]", testutil.GetRandomName(t)}[rand.Intn(6)], + }, + }, + Status: testutil.MustRandString(t, 10), + Volumes: map[string]string{testutil.GetRandomName(t): testutil.GetRandomName(t)}, + } + for _, m := range mut { + m(&ct) + } + return ct +} diff --git a/agent/agenttest/client.go b/agent/agenttest/client.go index 24658c44d6e18..05011971c7c50 100644 --- a/agent/agenttest/client.go +++ b/agent/agenttest/client.go @@ -98,8 +98,8 @@ func (c *Client) Close() { c.derpMapOnce.Do(func() { close(c.derpMapUpdates) }) } -func (c *Client) ConnectRPC24(ctx context.Context) ( - agentproto.DRPCAgentClient24, proto.DRPCTailnetClient24, error, +func (c *Client) ConnectRPC25(ctx context.Context) ( + agentproto.DRPCAgentClient25, proto.DRPCTailnetClient25, error, ) { conn, lis := drpcsdk.MemTransportPipe() c.LastWorkspaceAgent = func() { diff --git a/agent/proto/agent.pb.go b/agent/proto/agent.pb.go index ca454026f4790..562e349df9b2c 100644 --- a/agent/proto/agent.pb.go +++ b/agent/proto/agent.pb.go @@ -954,6 +954,7 @@ type Manifest struct { MotdPath string `protobuf:"bytes,6,opt,name=motd_path,json=motdPath,proto3" json:"motd_path,omitempty"` DisableDirectConnections bool `protobuf:"varint,7,opt,name=disable_direct_connections,json=disableDirectConnections,proto3" json:"disable_direct_connections,omitempty"` DerpForceWebsockets bool `protobuf:"varint,8,opt,name=derp_force_websockets,json=derpForceWebsockets,proto3" json:"derp_force_websockets,omitempty"` + ParentId []byte `protobuf:"bytes,18,opt,name=parent_id,json=parentId,proto3,oneof" json:"parent_id,omitempty"` DerpMap *proto.DERPMap `protobuf:"bytes,9,opt,name=derp_map,json=derpMap,proto3" json:"derp_map,omitempty"` Scripts []*WorkspaceAgentScript `protobuf:"bytes,10,rep,name=scripts,proto3" json:"scripts,omitempty"` Apps []*WorkspaceApp `protobuf:"bytes,11,rep,name=apps,proto3" json:"apps,omitempty"` @@ -1077,6 +1078,13 @@ func (x *Manifest) GetDerpForceWebsockets() bool { return false } +func (x *Manifest) GetParentId() []byte { + if x != nil { + return x.ParentId + } + return nil +} + func (x *Manifest) GetDerpMap() *proto.DERPMap { if x != nil { return x.DerpMap @@ -3665,7 +3673,7 @@ var file_agent_proto_agent_proto_rawDesc = []byte{ 0x69, 0x6d, 0x65, 0x6f, 0x75, 0x74, 0x18, 0x05, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x19, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x44, 0x75, 0x72, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x52, 0x07, 0x74, 0x69, 0x6d, 0x65, 0x6f, 0x75, 0x74, - 0x22, 0xbc, 0x07, 0x0a, 0x08, 0x4d, 0x61, 0x6e, 0x69, 0x66, 0x65, 0x73, 0x74, 0x12, 0x19, 0x0a, + 0x22, 0xec, 0x07, 0x0a, 0x08, 0x4d, 0x61, 0x6e, 0x69, 0x66, 0x65, 0x73, 0x74, 0x12, 0x19, 0x0a, 0x08, 0x61, 0x67, 0x65, 0x6e, 0x74, 0x5f, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x07, 0x61, 0x67, 0x65, 0x6e, 0x74, 0x49, 0x64, 0x12, 0x1d, 0x0a, 0x0a, 0x61, 0x67, 0x65, 0x6e, 0x74, 0x5f, 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x0f, 0x20, 0x01, 0x28, 0x09, 0x52, 0x09, 0x61, 0x67, @@ -3699,32 +3707,35 @@ var file_agent_proto_agent_proto_rawDesc = []byte{ 0x6e, 0x65, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x73, 0x12, 0x32, 0x0a, 0x15, 0x64, 0x65, 0x72, 0x70, 0x5f, 0x66, 0x6f, 0x72, 0x63, 0x65, 0x5f, 0x77, 0x65, 0x62, 0x73, 0x6f, 0x63, 0x6b, 0x65, 0x74, 0x73, 0x18, 0x08, 0x20, 0x01, 0x28, 0x08, 0x52, 0x13, 0x64, 0x65, 0x72, 0x70, 0x46, 0x6f, 0x72, - 0x63, 0x65, 0x57, 0x65, 0x62, 0x73, 0x6f, 0x63, 0x6b, 0x65, 0x74, 0x73, 0x12, 0x34, 0x0a, 0x08, - 0x64, 0x65, 0x72, 0x70, 0x5f, 0x6d, 0x61, 0x70, 0x18, 0x09, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x19, - 0x2e, 0x63, 0x6f, 0x64, 0x65, 0x72, 0x2e, 0x74, 0x61, 0x69, 0x6c, 0x6e, 0x65, 0x74, 0x2e, 0x76, - 0x32, 0x2e, 0x44, 0x45, 0x52, 0x50, 0x4d, 0x61, 0x70, 0x52, 0x07, 0x64, 0x65, 0x72, 0x70, 0x4d, - 0x61, 0x70, 0x12, 0x3e, 0x0a, 0x07, 0x73, 0x63, 0x72, 0x69, 0x70, 0x74, 0x73, 0x18, 0x0a, 0x20, - 0x03, 0x28, 0x0b, 0x32, 0x24, 0x2e, 0x63, 0x6f, 0x64, 0x65, 0x72, 0x2e, 0x61, 0x67, 0x65, 0x6e, - 0x74, 0x2e, 0x76, 0x32, 0x2e, 0x57, 0x6f, 0x72, 0x6b, 0x73, 0x70, 0x61, 0x63, 0x65, 0x41, 0x67, - 0x65, 0x6e, 0x74, 0x53, 0x63, 0x72, 0x69, 0x70, 0x74, 0x52, 0x07, 0x73, 0x63, 0x72, 0x69, 0x70, - 0x74, 0x73, 0x12, 0x30, 0x0a, 0x04, 0x61, 0x70, 0x70, 0x73, 0x18, 0x0b, 0x20, 0x03, 0x28, 0x0b, - 0x32, 0x1c, 0x2e, 0x63, 0x6f, 0x64, 0x65, 0x72, 0x2e, 0x61, 0x67, 0x65, 0x6e, 0x74, 0x2e, 0x76, - 0x32, 0x2e, 0x57, 0x6f, 0x72, 0x6b, 0x73, 0x70, 0x61, 0x63, 0x65, 0x41, 0x70, 0x70, 0x52, 0x04, - 0x61, 0x70, 0x70, 0x73, 0x12, 0x4e, 0x0a, 0x08, 0x6d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, - 0x18, 0x0c, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x32, 0x2e, 0x63, 0x6f, 0x64, 0x65, 0x72, 0x2e, 0x61, - 0x67, 0x65, 0x6e, 0x74, 0x2e, 0x76, 0x32, 0x2e, 0x57, 0x6f, 0x72, 0x6b, 0x73, 0x70, 0x61, 0x63, - 0x65, 0x41, 0x67, 0x65, 0x6e, 0x74, 0x4d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x2e, 0x44, - 0x65, 0x73, 0x63, 0x72, 0x69, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x52, 0x08, 0x6d, 0x65, 0x74, 0x61, - 0x64, 0x61, 0x74, 0x61, 0x12, 0x50, 0x0a, 0x0d, 0x64, 0x65, 0x76, 0x63, 0x6f, 0x6e, 0x74, 0x61, - 0x69, 0x6e, 0x65, 0x72, 0x73, 0x18, 0x11, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x2a, 0x2e, 0x63, 0x6f, - 0x64, 0x65, 0x72, 0x2e, 0x61, 0x67, 0x65, 0x6e, 0x74, 0x2e, 0x76, 0x32, 0x2e, 0x57, 0x6f, 0x72, - 0x6b, 0x73, 0x70, 0x61, 0x63, 0x65, 0x41, 0x67, 0x65, 0x6e, 0x74, 0x44, 0x65, 0x76, 0x63, 0x6f, - 0x6e, 0x74, 0x61, 0x69, 0x6e, 0x65, 0x72, 0x52, 0x0d, 0x64, 0x65, 0x76, 0x63, 0x6f, 0x6e, 0x74, - 0x61, 0x69, 0x6e, 0x65, 0x72, 0x73, 0x1a, 0x47, 0x0a, 0x19, 0x45, 0x6e, 0x76, 0x69, 0x72, 0x6f, - 0x6e, 0x6d, 0x65, 0x6e, 0x74, 0x56, 0x61, 0x72, 0x69, 0x61, 0x62, 0x6c, 0x65, 0x73, 0x45, 0x6e, - 0x74, 0x72, 0x79, 0x12, 0x10, 0x0a, 0x03, 0x6b, 0x65, 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, - 0x52, 0x03, 0x6b, 0x65, 0x79, 0x12, 0x14, 0x0a, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x18, 0x02, - 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x3a, 0x02, 0x38, 0x01, 0x22, + 0x63, 0x65, 0x57, 0x65, 0x62, 0x73, 0x6f, 0x63, 0x6b, 0x65, 0x74, 0x73, 0x12, 0x20, 0x0a, 0x09, + 0x70, 0x61, 0x72, 0x65, 0x6e, 0x74, 0x5f, 0x69, 0x64, 0x18, 0x12, 0x20, 0x01, 0x28, 0x0c, 0x48, + 0x00, 0x52, 0x08, 0x70, 0x61, 0x72, 0x65, 0x6e, 0x74, 0x49, 0x64, 0x88, 0x01, 0x01, 0x12, 0x34, + 0x0a, 0x08, 0x64, 0x65, 0x72, 0x70, 0x5f, 0x6d, 0x61, 0x70, 0x18, 0x09, 0x20, 0x01, 0x28, 0x0b, + 0x32, 0x19, 0x2e, 0x63, 0x6f, 0x64, 0x65, 0x72, 0x2e, 0x74, 0x61, 0x69, 0x6c, 0x6e, 0x65, 0x74, + 0x2e, 0x76, 0x32, 0x2e, 0x44, 0x45, 0x52, 0x50, 0x4d, 0x61, 0x70, 0x52, 0x07, 0x64, 0x65, 0x72, + 0x70, 0x4d, 0x61, 0x70, 0x12, 0x3e, 0x0a, 0x07, 0x73, 0x63, 0x72, 0x69, 0x70, 0x74, 0x73, 0x18, + 0x0a, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x24, 0x2e, 0x63, 0x6f, 0x64, 0x65, 0x72, 0x2e, 0x61, 0x67, + 0x65, 0x6e, 0x74, 0x2e, 0x76, 0x32, 0x2e, 0x57, 0x6f, 0x72, 0x6b, 0x73, 0x70, 0x61, 0x63, 0x65, + 0x41, 0x67, 0x65, 0x6e, 0x74, 0x53, 0x63, 0x72, 0x69, 0x70, 0x74, 0x52, 0x07, 0x73, 0x63, 0x72, + 0x69, 0x70, 0x74, 0x73, 0x12, 0x30, 0x0a, 0x04, 0x61, 0x70, 0x70, 0x73, 0x18, 0x0b, 0x20, 0x03, + 0x28, 0x0b, 0x32, 0x1c, 0x2e, 0x63, 0x6f, 0x64, 0x65, 0x72, 0x2e, 0x61, 0x67, 0x65, 0x6e, 0x74, + 0x2e, 0x76, 0x32, 0x2e, 0x57, 0x6f, 0x72, 0x6b, 0x73, 0x70, 0x61, 0x63, 0x65, 0x41, 0x70, 0x70, + 0x52, 0x04, 0x61, 0x70, 0x70, 0x73, 0x12, 0x4e, 0x0a, 0x08, 0x6d, 0x65, 0x74, 0x61, 0x64, 0x61, + 0x74, 0x61, 0x18, 0x0c, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x32, 0x2e, 0x63, 0x6f, 0x64, 0x65, 0x72, + 0x2e, 0x61, 0x67, 0x65, 0x6e, 0x74, 0x2e, 0x76, 0x32, 0x2e, 0x57, 0x6f, 0x72, 0x6b, 0x73, 0x70, + 0x61, 0x63, 0x65, 0x41, 0x67, 0x65, 0x6e, 0x74, 0x4d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, + 0x2e, 0x44, 0x65, 0x73, 0x63, 0x72, 0x69, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x52, 0x08, 0x6d, 0x65, + 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x12, 0x50, 0x0a, 0x0d, 0x64, 0x65, 0x76, 0x63, 0x6f, 0x6e, + 0x74, 0x61, 0x69, 0x6e, 0x65, 0x72, 0x73, 0x18, 0x11, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x2a, 0x2e, + 0x63, 0x6f, 0x64, 0x65, 0x72, 0x2e, 0x61, 0x67, 0x65, 0x6e, 0x74, 0x2e, 0x76, 0x32, 0x2e, 0x57, + 0x6f, 0x72, 0x6b, 0x73, 0x70, 0x61, 0x63, 0x65, 0x41, 0x67, 0x65, 0x6e, 0x74, 0x44, 0x65, 0x76, + 0x63, 0x6f, 0x6e, 0x74, 0x61, 0x69, 0x6e, 0x65, 0x72, 0x52, 0x0d, 0x64, 0x65, 0x76, 0x63, 0x6f, + 0x6e, 0x74, 0x61, 0x69, 0x6e, 0x65, 0x72, 0x73, 0x1a, 0x47, 0x0a, 0x19, 0x45, 0x6e, 0x76, 0x69, + 0x72, 0x6f, 0x6e, 0x6d, 0x65, 0x6e, 0x74, 0x56, 0x61, 0x72, 0x69, 0x61, 0x62, 0x6c, 0x65, 0x73, + 0x45, 0x6e, 0x74, 0x72, 0x79, 0x12, 0x10, 0x0a, 0x03, 0x6b, 0x65, 0x79, 0x18, 0x01, 0x20, 0x01, + 0x28, 0x09, 0x52, 0x03, 0x6b, 0x65, 0x79, 0x12, 0x14, 0x0a, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, + 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x3a, 0x02, 0x38, + 0x01, 0x42, 0x0c, 0x0a, 0x0a, 0x5f, 0x70, 0x61, 0x72, 0x65, 0x6e, 0x74, 0x5f, 0x69, 0x64, 0x22, 0x8c, 0x01, 0x0a, 0x1a, 0x57, 0x6f, 0x72, 0x6b, 0x73, 0x70, 0x61, 0x63, 0x65, 0x41, 0x67, 0x65, 0x6e, 0x74, 0x44, 0x65, 0x76, 0x63, 0x6f, 0x6e, 0x74, 0x61, 0x69, 0x6e, 0x65, 0x72, 0x12, 0x0e, 0x0a, 0x02, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x02, 0x69, 0x64, 0x12, 0x29, @@ -4901,6 +4912,7 @@ func file_agent_proto_agent_proto_init() { } } } + file_agent_proto_agent_proto_msgTypes[3].OneofWrappers = []interface{}{} file_agent_proto_agent_proto_msgTypes[30].OneofWrappers = []interface{}{} file_agent_proto_agent_proto_msgTypes[33].OneofWrappers = []interface{}{} file_agent_proto_agent_proto_msgTypes[46].OneofWrappers = []interface{}{} diff --git a/agent/proto/agent.proto b/agent/proto/agent.proto index 5bfd867720cfa..f6237980b6fd6 100644 --- a/agent/proto/agent.proto +++ b/agent/proto/agent.proto @@ -90,6 +90,7 @@ message Manifest { string motd_path = 6; bool disable_direct_connections = 7; bool derp_force_websockets = 8; + optional bytes parent_id = 18; coder.tailnet.v2.DERPMap derp_map = 9; repeated WorkspaceAgentScript scripts = 10; diff --git a/agent/proto/agent_drpc_old.go b/agent/proto/agent_drpc_old.go index 63b666a259c5c..e1e6625908c8a 100644 --- a/agent/proto/agent_drpc_old.go +++ b/agent/proto/agent_drpc_old.go @@ -50,3 +50,8 @@ type DRPCAgentClient24 interface { PushResourcesMonitoringUsage(ctx context.Context, in *PushResourcesMonitoringUsageRequest) (*PushResourcesMonitoringUsageResponse, error) ReportConnection(ctx context.Context, in *ReportConnectionRequest) (*emptypb.Empty, error) } + +// DRPCAgentClient25 is the Agent API at v2.5. +type DRPCAgentClient25 interface { + DRPCAgentClient24 +} diff --git a/cli/exp_mcp.go b/cli/exp_mcp.go index 6174f0cffbf0e..fb866666daf4a 100644 --- a/cli/exp_mcp.go +++ b/cli/exp_mcp.go @@ -255,7 +255,7 @@ func (*RootCmd) mcpConfigureClaudeCode() *serpent.Command { { Name: "app-status-slug", Description: "The app status slug to use when running the Coder MCP server.", - Env: "CODER_MCP_CLAUDE_APP_STATUS_SLUG", + Env: "CODER_MCP_APP_STATUS_SLUG", Flag: "claude-app-status-slug", Value: serpent.StringOf(&appStatusSlug), }, diff --git a/cli/parameterresolver.go b/cli/parameterresolver.go index 41c61d5315a77..40625331fa6aa 100644 --- a/cli/parameterresolver.go +++ b/cli/parameterresolver.go @@ -226,7 +226,7 @@ func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuild if p != nil { continue } - // Parameter has not been resolved yet, so CLI needs to determine if user should input it. + // PreviewParameter has not been resolved yet, so CLI needs to determine if user should input it. firstTimeUse := pr.isFirstTimeUse(tvp.Name) promptParameterOption := pr.isLastBuildParameterInvalidOption(tvp) diff --git a/cli/server.go b/cli/server.go index c5532e07e7a81..1794044bce48f 100644 --- a/cli/server.go +++ b/cli/server.go @@ -87,6 +87,7 @@ import ( "github.com/coder/coder/v2/coderd/externalauth" "github.com/coder/coder/v2/coderd/gitsshkey" "github.com/coder/coder/v2/coderd/httpmw" + "github.com/coder/coder/v2/coderd/jobreaper" "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/oauthpki" "github.com/coder/coder/v2/coderd/prometheusmetrics" @@ -95,7 +96,6 @@ import ( "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/telemetry" "github.com/coder/coder/v2/coderd/tracing" - "github.com/coder/coder/v2/coderd/unhanger" "github.com/coder/coder/v2/coderd/updatecheck" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/coderd/util/slice" @@ -1124,14 +1124,14 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. autobuildTicker := time.NewTicker(vals.AutobuildPollInterval.Value()) defer autobuildTicker.Stop() autobuildExecutor := autobuild.NewExecutor( - ctx, options.Database, options.Pubsub, options.PrometheusRegistry, coderAPI.TemplateScheduleStore, &coderAPI.Auditor, coderAPI.AccessControlStore, logger, autobuildTicker.C, options.NotificationsEnqueuer) + ctx, options.Database, options.Pubsub, options.PrometheusRegistry, coderAPI.TemplateScheduleStore, &coderAPI.Auditor, coderAPI.AccessControlStore, logger, autobuildTicker.C, options.NotificationsEnqueuer, coderAPI.Experiments) autobuildExecutor.Run() - hangDetectorTicker := time.NewTicker(vals.JobHangDetectorInterval.Value()) - defer hangDetectorTicker.Stop() - hangDetector := unhanger.New(ctx, options.Database, options.Pubsub, logger, hangDetectorTicker.C) - hangDetector.Start() - defer hangDetector.Close() + jobReaperTicker := time.NewTicker(vals.JobReaperDetectorInterval.Value()) + defer jobReaperTicker.Stop() + jobReaper := jobreaper.New(ctx, options.Database, options.Pubsub, logger, jobReaperTicker.C) + jobReaper.Start() + defer jobReaper.Close() waitForProvisionerJobs := false // Currently there is no way to ask the server to shut diff --git a/cli/testdata/coder_list_--output_json.golden b/cli/testdata/coder_list_--output_json.golden index 5f293787de719..9cdaa98c3f813 100644 --- a/cli/testdata/coder_list_--output_json.golden +++ b/cli/testdata/coder_list_--output_json.golden @@ -15,6 +15,7 @@ "template_allow_user_cancel_workspace_jobs": false, "template_active_version_id": "============[version ID]============", "template_require_active_version": false, + "template_use_classic_parameter_flow": false, "latest_build": { "id": "========[workspace build ID]========", "created_at": "====[timestamp]=====", diff --git a/cli/testdata/coder_provisioner_jobs_list_--help.golden b/cli/testdata/coder_provisioner_jobs_list_--help.golden index 7a72605f0c288..f380a0334867c 100644 --- a/cli/testdata/coder_provisioner_jobs_list_--help.golden +++ b/cli/testdata/coder_provisioner_jobs_list_--help.golden @@ -11,7 +11,7 @@ OPTIONS: -O, --org string, $CODER_ORGANIZATION Select which organization (uuid or name) to use. - -c, --column [id|created at|started at|completed at|canceled at|error|error code|status|worker id|file id|tags|queue position|queue size|organization id|template version id|workspace build id|type|available workers|template version name|template id|template name|template display name|template icon|workspace id|workspace name|organization|queue] (default: created at,id,type,template display name,status,queue,tags) + -c, --column [id|created at|started at|completed at|canceled at|error|error code|status|worker id|worker name|file id|tags|queue position|queue size|organization id|template version id|workspace build id|type|available workers|template version name|template id|template name|template display name|template icon|workspace id|workspace name|organization|queue] (default: created at,id,type,template display name,status,queue,tags) Columns to display in table output. -l, --limit int, $CODER_PROVISIONER_JOB_LIST_LIMIT (default: 50) diff --git a/cli/testdata/coder_provisioner_jobs_list_--output_json.golden b/cli/testdata/coder_provisioner_jobs_list_--output_json.golden index d18e07121f653..e36723765b4df 100644 --- a/cli/testdata/coder_provisioner_jobs_list_--output_json.golden +++ b/cli/testdata/coder_provisioner_jobs_list_--output_json.golden @@ -6,6 +6,7 @@ "completed_at": "====[timestamp]=====", "status": "succeeded", "worker_id": "====[workspace build worker ID]=====", + "worker_name": "test-daemon", "file_id": "=====[workspace build file ID]======", "tags": { "owner": "", @@ -34,6 +35,7 @@ "completed_at": "====[timestamp]=====", "status": "succeeded", "worker_id": "====[workspace build worker ID]=====", + "worker_name": "test-daemon", "file_id": "=====[workspace build file ID]======", "tags": { "owner": "", diff --git a/cli/testdata/coder_provisioner_list.golden b/cli/testdata/coder_provisioner_list.golden index 64941eebf5b89..92ac6e485e68f 100644 --- a/cli/testdata/coder_provisioner_list.golden +++ b/cli/testdata/coder_provisioner_list.golden @@ -1,2 +1,2 @@ -CREATED AT LAST SEEN AT KEY NAME NAME VERSION STATUS TAGS -====[timestamp]===== ====[timestamp]===== built-in test v0.0.0-devel idle map[owner: scope:organization] +CREATED AT LAST SEEN AT KEY NAME NAME VERSION STATUS TAGS +====[timestamp]===== ====[timestamp]===== built-in test-daemon v0.0.0-devel idle map[owner: scope:organization] diff --git a/cli/testdata/coder_provisioner_list_--output_json.golden b/cli/testdata/coder_provisioner_list_--output_json.golden index e8b3637bdffa6..73dd35ff84266 100644 --- a/cli/testdata/coder_provisioner_list_--output_json.golden +++ b/cli/testdata/coder_provisioner_list_--output_json.golden @@ -5,7 +5,7 @@ "key_id": "00000000-0000-0000-0000-000000000001", "created_at": "====[timestamp]=====", "last_seen_at": "====[timestamp]=====", - "name": "test", + "name": "test-daemon", "version": "v0.0.0-devel", "api_version": "1.6", "provisioners": [ diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index fc76a6c2ec8a0..9995a7f389130 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -183,7 +183,7 @@ networking: # Interval to poll for scheduled workspace builds. # (default: 1m0s, type: duration) autobuildPollInterval: 1m0s -# Interval to poll for hung jobs and automatically terminate them. +# Interval to poll for hung and pending jobs and automatically terminate them. # (default: 1m0s, type: duration) jobHangDetectorInterval: 1m0s introspection: diff --git a/coderd/agentapi/manifest.go b/coderd/agentapi/manifest.go index db8a0af3946a9..855ff4b8acd37 100644 --- a/coderd/agentapi/manifest.go +++ b/coderd/agentapi/manifest.go @@ -47,7 +47,6 @@ func (a *ManifestAPI) GetManifest(ctx context.Context, _ *agentproto.GetManifest scripts []database.WorkspaceAgentScript metadata []database.WorkspaceAgentMetadatum workspace database.Workspace - owner database.User devcontainers []database.WorkspaceAgentDevcontainer ) @@ -76,10 +75,6 @@ func (a *ManifestAPI) GetManifest(ctx context.Context, _ *agentproto.GetManifest if err != nil { return xerrors.Errorf("getting workspace by id: %w", err) } - owner, err = a.Database.GetUserByID(ctx, workspace.OwnerID) - if err != nil { - return xerrors.Errorf("getting workspace owner by id: %w", err) - } return err }) eg.Go(func() (err error) { @@ -98,7 +93,7 @@ func (a *ManifestAPI) GetManifest(ctx context.Context, _ *agentproto.GetManifest AppSlugOrPort: "{{port}}", AgentName: workspaceAgent.Name, WorkspaceName: workspace.Name, - Username: owner.Username, + Username: workspace.OwnerUsername, } vscodeProxyURI := vscodeProxyURI(appSlug, a.AccessURL, a.AppHostname) @@ -115,15 +110,20 @@ func (a *ManifestAPI) GetManifest(ctx context.Context, _ *agentproto.GetManifest } } - apps, err := dbAppsToProto(dbApps, workspaceAgent, owner.Username, workspace) + apps, err := dbAppsToProto(dbApps, workspaceAgent, workspace.OwnerUsername, workspace) if err != nil { return nil, xerrors.Errorf("converting workspace apps: %w", err) } + var parentID []byte + if workspaceAgent.ParentID.Valid { + parentID = workspaceAgent.ParentID.UUID[:] + } + return &agentproto.Manifest{ AgentId: workspaceAgent.ID[:], AgentName: workspaceAgent.Name, - OwnerUsername: owner.Username, + OwnerUsername: workspace.OwnerUsername, WorkspaceId: workspace.ID[:], WorkspaceName: workspace.Name, GitAuthConfigs: gitAuthConfigs, @@ -133,6 +133,7 @@ func (a *ManifestAPI) GetManifest(ctx context.Context, _ *agentproto.GetManifest MotdPath: workspaceAgent.MOTDFile, DisableDirectConnections: a.DisableDirectConnections, DerpForceWebsockets: a.DerpForceWebSockets, + ParentId: parentID, DerpMap: tailnet.DERPMapToProto(a.DerpMapFn()), Scripts: dbAgentScriptsToProto(scripts), diff --git a/coderd/agentapi/manifest_test.go b/coderd/agentapi/manifest_test.go index 98e7ccc8c8b52..fc46f5fe480f8 100644 --- a/coderd/agentapi/manifest_test.go +++ b/coderd/agentapi/manifest_test.go @@ -46,9 +46,10 @@ func TestGetManifest(t *testing.T) { Username: "cool-user", } workspace = database.Workspace{ - ID: uuid.New(), - OwnerID: owner.ID, - Name: "cool-workspace", + ID: uuid.New(), + OwnerID: owner.ID, + OwnerUsername: owner.Username, + Name: "cool-workspace", } agent = database.WorkspaceAgent{ ID: uuid.New(), @@ -60,6 +61,13 @@ func TestGetManifest(t *testing.T) { Directory: "/cool/dir", MOTDFile: "/cool/motd", } + childAgent = database.WorkspaceAgent{ + ID: uuid.New(), + Name: "cool-child-agent", + ParentID: uuid.NullUUID{Valid: true, UUID: agent.ID}, + Directory: "/workspace/dir", + MOTDFile: "/workspace/motd", + } apps = []database.WorkspaceApp{ { ID: uuid.New(), @@ -329,7 +337,6 @@ func TestGetManifest(t *testing.T) { }).Return(metadata, nil) mDB.EXPECT().GetWorkspaceAgentDevcontainersByAgentID(gomock.Any(), agent.ID).Return(devcontainers, nil) mDB.EXPECT().GetWorkspaceByID(gomock.Any(), workspace.ID).Return(workspace, nil) - mDB.EXPECT().GetUserByID(gomock.Any(), workspace.OwnerID).Return(owner, nil) got, err := api.GetManifest(context.Background(), &agentproto.GetManifestRequest{}) require.NoError(t, err) @@ -337,6 +344,7 @@ func TestGetManifest(t *testing.T) { expected := &agentproto.Manifest{ AgentId: agent.ID[:], AgentName: agent.Name, + ParentId: nil, OwnerUsername: owner.Username, WorkspaceId: workspace.ID[:], WorkspaceName: workspace.Name, @@ -364,6 +372,69 @@ func TestGetManifest(t *testing.T) { require.Equal(t, expected, got) }) + t.Run("OK/Child", func(t *testing.T) { + t.Parallel() + + mDB := dbmock.NewMockStore(gomock.NewController(t)) + + api := &agentapi.ManifestAPI{ + AccessURL: &url.URL{Scheme: "https", Host: "example.com"}, + AppHostname: "*--apps.example.com", + ExternalAuthConfigs: []*externalauth.Config{ + {Type: string(codersdk.EnhancedExternalAuthProviderGitHub)}, + {Type: "some-provider"}, + {Type: string(codersdk.EnhancedExternalAuthProviderGitLab)}, + }, + DisableDirectConnections: true, + DerpForceWebSockets: true, + + AgentFn: func(ctx context.Context) (database.WorkspaceAgent, error) { + return childAgent, nil + }, + WorkspaceID: workspace.ID, + Database: mDB, + DerpMapFn: derpMapFn, + } + + mDB.EXPECT().GetWorkspaceAppsByAgentID(gomock.Any(), childAgent.ID).Return([]database.WorkspaceApp{}, nil) + mDB.EXPECT().GetWorkspaceAgentScriptsByAgentIDs(gomock.Any(), []uuid.UUID{childAgent.ID}).Return([]database.WorkspaceAgentScript{}, nil) + mDB.EXPECT().GetWorkspaceAgentMetadata(gomock.Any(), database.GetWorkspaceAgentMetadataParams{ + WorkspaceAgentID: childAgent.ID, + Keys: nil, // all + }).Return([]database.WorkspaceAgentMetadatum{}, nil) + mDB.EXPECT().GetWorkspaceAgentDevcontainersByAgentID(gomock.Any(), childAgent.ID).Return([]database.WorkspaceAgentDevcontainer{}, nil) + mDB.EXPECT().GetWorkspaceByID(gomock.Any(), workspace.ID).Return(workspace, nil) + + got, err := api.GetManifest(context.Background(), &agentproto.GetManifestRequest{}) + require.NoError(t, err) + + expected := &agentproto.Manifest{ + AgentId: childAgent.ID[:], + AgentName: childAgent.Name, + ParentId: agent.ID[:], + OwnerUsername: owner.Username, + WorkspaceId: workspace.ID[:], + WorkspaceName: workspace.Name, + GitAuthConfigs: 2, // two "enhanced" external auth configs + EnvironmentVariables: nil, + Directory: childAgent.Directory, + VsCodePortProxyUri: fmt.Sprintf("https://{{port}}--%s--%s--%s--apps.example.com", childAgent.Name, workspace.Name, owner.Username), + MotdPath: childAgent.MOTDFile, + DisableDirectConnections: true, + DerpForceWebsockets: true, + // tailnet.DERPMapToProto() is extensively tested elsewhere, so it's + // not necessary to manually recreate a big DERP map here like we + // did for apps and metadata. + DerpMap: tailnet.DERPMapToProto(derpMapFn()), + Scripts: []*agentproto.WorkspaceAgentScript{}, + Apps: []*agentproto.WorkspaceApp{}, + Metadata: []*agentproto.WorkspaceAgentMetadata_Description{}, + Devcontainers: []*agentproto.WorkspaceAgentDevcontainer{}, + } + + require.Equal(t, expected, got) + }) + t.Run("NoAppHostname", func(t *testing.T) { t.Parallel() @@ -396,7 +467,6 @@ func TestGetManifest(t *testing.T) { }).Return(metadata, nil) mDB.EXPECT().GetWorkspaceAgentDevcontainersByAgentID(gomock.Any(), agent.ID).Return(devcontainers, nil) mDB.EXPECT().GetWorkspaceByID(gomock.Any(), workspace.ID).Return(workspace, nil) - mDB.EXPECT().GetUserByID(gomock.Any(), workspace.OwnerID).Return(owner, nil) got, err := api.GetManifest(context.Background(), &agentproto.GetManifestRequest{}) require.NoError(t, err) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index f744b988956e9..e98197d3b5bb2 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -8606,6 +8606,42 @@ const docTemplate = `{ } } }, + "/workspaceagents/{workspaceagent}/containers/devcontainers/container/{container}/recreate": { + "post": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "tags": [ + "Agents" + ], + "summary": "Recreate devcontainer for workspace agent", + "operationId": "recreate-devcontainer-for-workspace-agent", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Workspace agent ID", + "name": "workspaceagent", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Container ID or name", + "name": "container", + "in": "path", + "required": true + } + ], + "responses": { + "204": { + "description": "No Content" + } + } + } + }, "/workspaceagents/{workspaceagent}/coordinate": { "get": { "security": [ @@ -11962,6 +11998,10 @@ const docTemplate = `{ "dry_run": { "type": "boolean" }, + "enable_dynamic_parameters": { + "description": "EnableDynamicParameters skips some of the static parameter checking.\nIt will default to whatever the template has marked as the default experience.\nRequires the \"dynamic-experiment\" to be used.", + "type": "boolean" + }, "log_level": { "description": "Log level changes the default logging verbosity of a provider (\"info\" if empty).", "enum": [ @@ -14582,6 +14622,9 @@ const docTemplate = `{ "worker_id": { "type": "string", "format": "uuid" + }, + "worker_name": { + "type": "string" } } }, @@ -14858,7 +14901,9 @@ const docTemplate = `{ "application_connect", "assign", "create", + "create_agent", "delete", + "delete_agent", "read", "read_personal", "ssh", @@ -14874,7 +14919,9 @@ const docTemplate = `{ "ActionApplicationConnect", "ActionAssign", "ActionCreate", + "ActionCreateAgent", "ActionDelete", + "ActionDeleteAgent", "ActionRead", "ActionReadPersonal", "ActionSSH", @@ -16967,6 +17014,9 @@ const docTemplate = `{ "template_require_active_version": { "type": "boolean" }, + "template_use_classic_parameter_flow": { + "type": "boolean" + }, "ttl_ms": { "type": "integer" }, @@ -17134,6 +17184,10 @@ const docTemplate = `{ "type": "string", "format": "date-time" }, + "devcontainer_dirty": { + "description": "DevcontainerDirty is true if the devcontainer configuration has changed\nsince the container was created. This is used to determine if the\ncontainer needs to be rebuilt.", + "type": "boolean" + }, "id": { "description": "ID is the unique identifier of the container.", "type": "string" diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 1859a4f6f6214..fa103f55fbe9f 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -7605,6 +7605,40 @@ } } }, + "/workspaceagents/{workspaceagent}/containers/devcontainers/container/{container}/recreate": { + "post": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "tags": ["Agents"], + "summary": "Recreate devcontainer for workspace agent", + "operationId": "recreate-devcontainer-for-workspace-agent", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Workspace agent ID", + "name": "workspaceagent", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Container ID or name", + "name": "container", + "in": "path", + "required": true + } + ], + "responses": { + "204": { + "description": "No Content" + } + } + } + }, "/workspaceagents/{workspaceagent}/coordinate": { "get": { "security": [ @@ -10682,6 +10716,10 @@ "dry_run": { "type": "boolean" }, + "enable_dynamic_parameters": { + "description": "EnableDynamicParameters skips some of the static parameter checking.\nIt will default to whatever the template has marked as the default experience.\nRequires the \"dynamic-experiment\" to be used.", + "type": "boolean" + }, "log_level": { "description": "Log level changes the default logging verbosity of a provider (\"info\" if empty).", "enum": ["debug"], @@ -13216,6 +13254,9 @@ "worker_id": { "type": "string", "format": "uuid" + }, + "worker_name": { + "type": "string" } } }, @@ -13468,7 +13509,9 @@ "application_connect", "assign", "create", + "create_agent", "delete", + "delete_agent", "read", "read_personal", "ssh", @@ -13484,7 +13527,9 @@ "ActionApplicationConnect", "ActionAssign", "ActionCreate", + "ActionCreateAgent", "ActionDelete", + "ActionDeleteAgent", "ActionRead", "ActionReadPersonal", "ActionSSH", @@ -15476,6 +15521,9 @@ "template_require_active_version": { "type": "boolean" }, + "template_use_classic_parameter_flow": { + "type": "boolean" + }, "ttl_ms": { "type": "integer" }, @@ -15643,6 +15691,10 @@ "type": "string", "format": "date-time" }, + "devcontainer_dirty": { + "description": "DevcontainerDirty is true if the devcontainer configuration has changed\nsince the container was created. This is used to determine if the\ncontainer needs to be rebuilt.", + "type": "boolean" + }, "id": { "description": "ID is the unique identifier of the container.", "type": "string" diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index cc4e48b43544c..b0cba60111335 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -27,6 +27,7 @@ import ( "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/wsbuilder" + "github.com/coder/coder/v2/codersdk" ) // Executor automatically starts or stops workspaces. @@ -43,6 +44,7 @@ type Executor struct { // NotificationsEnqueuer handles enqueueing notifications for delivery by SMTP, webhook, etc. notificationsEnqueuer notifications.Enqueuer reg prometheus.Registerer + experiments codersdk.Experiments metrics executorMetrics } @@ -59,7 +61,7 @@ type Stats struct { } // New returns a new wsactions executor. -func NewExecutor(ctx context.Context, db database.Store, ps pubsub.Pubsub, reg prometheus.Registerer, tss *atomic.Pointer[schedule.TemplateScheduleStore], auditor *atomic.Pointer[audit.Auditor], acs *atomic.Pointer[dbauthz.AccessControlStore], log slog.Logger, tick <-chan time.Time, enqueuer notifications.Enqueuer) *Executor { +func NewExecutor(ctx context.Context, db database.Store, ps pubsub.Pubsub, reg prometheus.Registerer, tss *atomic.Pointer[schedule.TemplateScheduleStore], auditor *atomic.Pointer[audit.Auditor], acs *atomic.Pointer[dbauthz.AccessControlStore], log slog.Logger, tick <-chan time.Time, enqueuer notifications.Enqueuer, exp codersdk.Experiments) *Executor { factory := promauto.With(reg) le := &Executor{ //nolint:gocritic // Autostart has a limited set of permissions. @@ -73,6 +75,7 @@ func NewExecutor(ctx context.Context, db database.Store, ps pubsub.Pubsub, reg p accessControlStore: acs, notificationsEnqueuer: enqueuer, reg: reg, + experiments: exp, metrics: executorMetrics{ autobuildExecutionDuration: factory.NewHistogram(prometheus.HistogramOpts{ Namespace: "coderd", @@ -258,6 +261,7 @@ func (e *Executor) runOnce(t time.Time) Stats { builder := wsbuilder.New(ws, nextTransition). SetLastWorkspaceBuildInTx(&latestBuild). SetLastWorkspaceBuildJobInTx(&latestJob). + Experiments(e.experiments). Reason(reason) log.Debug(e.ctx, "auto building workspace", slog.F("transition", nextTransition)) if nextTransition == database.WorkspaceTransitionStart && @@ -349,13 +353,18 @@ func (e *Executor) runOnce(t time.Time) Stats { nextBuildReason = string(nextBuild.Reason) } + templateVersionMessage := activeTemplateVersion.Message + if templateVersionMessage == "" { + templateVersionMessage = "None provided" + } + if _, err := e.notificationsEnqueuer.Enqueue(e.ctx, ws.OwnerID, notifications.TemplateWorkspaceAutoUpdated, map[string]string{ "name": ws.Name, "initiator": "autobuild", "reason": nextBuildReason, "template_version_name": activeTemplateVersion.Name, - "template_version_message": activeTemplateVersion.Message, + "template_version_message": templateVersionMessage, }, "autobuild", // Associate this notification with all the related entities. ws.ID, ws.OwnerID, ws.TemplateID, ws.OrganizationID, diff --git a/coderd/coderd.go b/coderd/coderd.go index c3f45b15e4a30..3989f8a87ea1b 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1326,6 +1326,7 @@ func New(options *Options) *API { r.Get("/listening-ports", api.workspaceAgentListeningPorts) r.Get("/connection", api.workspaceAgentConnection) r.Get("/containers", api.workspaceAgentListContainers) + r.Post("/containers/devcontainers/container/{container}/recreate", api.workspaceAgentRecreateDevcontainer) r.Get("/coordinate", api.workspaceAgentClientCoordinate) // PTY is part of workspaceAppServer. diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index a25f0576e76be..a8f444c8f632e 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -68,6 +68,7 @@ import ( "github.com/coder/coder/v2/coderd/externalauth" "github.com/coder/coder/v2/coderd/gitsshkey" "github.com/coder/coder/v2/coderd/httpmw" + "github.com/coder/coder/v2/coderd/jobreaper" "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/notifications/notificationstest" "github.com/coder/coder/v2/coderd/rbac" @@ -75,7 +76,6 @@ import ( "github.com/coder/coder/v2/coderd/runtimeconfig" "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/telemetry" - "github.com/coder/coder/v2/coderd/unhanger" "github.com/coder/coder/v2/coderd/updatecheck" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/coderd/webpush" @@ -96,6 +96,8 @@ import ( "github.com/coder/coder/v2/testutil" ) +const defaultTestDaemonName = "test-daemon" + type Options struct { // AccessURL denotes a custom access URL. By default we use the httptest // server's URL. Setting this may result in unexpected behavior (especially @@ -352,6 +354,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can auditor.Store(&options.Auditor) ctx, cancelFunc := context.WithCancel(context.Background()) + experiments := coderd.ReadExperiments(*options.Logger, options.DeploymentValues.Experiments) lifecycleExecutor := autobuild.NewExecutor( ctx, options.Database, @@ -363,14 +366,15 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can *options.Logger, options.AutobuildTicker, options.NotificationsEnqueuer, + experiments, ).WithStatsChannel(options.AutobuildStats) lifecycleExecutor.Run() - hangDetectorTicker := time.NewTicker(options.DeploymentValues.JobHangDetectorInterval.Value()) - defer hangDetectorTicker.Stop() - hangDetector := unhanger.New(ctx, options.Database, options.Pubsub, options.Logger.Named("unhanger.detector"), hangDetectorTicker.C) - hangDetector.Start() - t.Cleanup(hangDetector.Close) + jobReaperTicker := time.NewTicker(options.DeploymentValues.JobReaperDetectorInterval.Value()) + defer jobReaperTicker.Stop() + jobReaper := jobreaper.New(ctx, options.Database, options.Pubsub, options.Logger.Named("reaper.detector"), jobReaperTicker.C) + jobReaper.Start() + t.Cleanup(jobReaper.Close) if options.TelemetryReporter == nil { options.TelemetryReporter = telemetry.NewNoop() @@ -602,7 +606,7 @@ func NewWithAPI(t testing.TB, options *Options) (*codersdk.Client, io.Closer, *c setHandler(rootHandler) var provisionerCloser io.Closer = nopcloser{} if options.IncludeProvisionerDaemon { - provisionerCloser = NewTaggedProvisionerDaemon(t, coderAPI, "test", options.ProvisionerDaemonTags, coderd.MemoryProvisionerWithVersionOverride(options.ProvisionerDaemonVersion)) + provisionerCloser = NewTaggedProvisionerDaemon(t, coderAPI, defaultTestDaemonName, options.ProvisionerDaemonTags, coderd.MemoryProvisionerWithVersionOverride(options.ProvisionerDaemonVersion)) } client := codersdk.New(serverURL) t.Cleanup(func() { @@ -646,7 +650,7 @@ func (c *ProvisionerdCloser) Close() error { // well with coderd testing. It registers the "echo" provisioner for // quick testing. func NewProvisionerDaemon(t testing.TB, coderAPI *coderd.API) io.Closer { - return NewTaggedProvisionerDaemon(t, coderAPI, "test", nil) + return NewTaggedProvisionerDaemon(t, coderAPI, defaultTestDaemonName, nil) } func NewTaggedProvisionerDaemon(t testing.TB, coderAPI *coderd.API, name string, provisionerTags map[string]string, opts ...coderd.MemoryProvisionerDaemonOption) io.Closer { diff --git a/coderd/coderdtest/oidctest/idp.go b/coderd/coderdtest/oidctest/idp.go index b82f8a00dedb4..c7f7d35937198 100644 --- a/coderd/coderdtest/oidctest/idp.go +++ b/coderd/coderdtest/oidctest/idp.go @@ -307,7 +307,7 @@ func WithCustomClientAuth(hook func(t testing.TB, req *http.Request) (url.Values // WithLogging is optional, but will log some HTTP calls made to the IDP. func WithLogging(t testing.TB, options *slogtest.Options) func(*FakeIDP) { return func(f *FakeIDP) { - f.logger = slogtest.Make(t, options) + f.logger = slogtest.Make(t, options).Named("fakeidp") } } @@ -794,6 +794,7 @@ func (f *FakeIDP) newToken(t testing.TB, email string, expires time.Time) string func (f *FakeIDP) newRefreshTokens(email string) string { refreshToken := uuid.NewString() f.refreshTokens.Store(refreshToken, email) + f.logger.Info(context.Background(), "new refresh token", slog.F("email", email), slog.F("token", refreshToken)) return refreshToken } @@ -1003,6 +1004,7 @@ func (f *FakeIDP) httpHandler(t testing.TB) http.Handler { return } + f.logger.Info(r.Context(), "http idp call refresh_token", slog.F("token", refreshToken)) _, ok := f.refreshTokens.Load(refreshToken) if !assert.True(t, ok, "invalid refresh_token") { http.Error(rw, "invalid refresh_token", http.StatusBadRequest) @@ -1026,6 +1028,7 @@ func (f *FakeIDP) httpHandler(t testing.TB) http.Handler { f.refreshTokensUsed.Store(refreshToken, true) // Always invalidate the refresh token after it is used. f.refreshTokens.Delete(refreshToken) + f.logger.Info(r.Context(), "refresh token invalidated", slog.F("token", refreshToken)) case "urn:ietf:params:oauth:grant-type:device_code": // Device flow var resp externalauth.ExchangeDeviceCodeResponse diff --git a/coderd/database/db2sdk/db2sdk.go b/coderd/database/db2sdk/db2sdk.go index 18d1d8a6ac788..ed258a07820ab 100644 --- a/coderd/database/db2sdk/db2sdk.go +++ b/coderd/database/db2sdk/db2sdk.go @@ -12,6 +12,7 @@ import ( "time" "github.com/google/uuid" + "github.com/hashicorp/hcl/v2" "golang.org/x/xerrors" "tailscale.com/tailcfg" @@ -24,6 +25,7 @@ import ( "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/provisionersdk/proto" "github.com/coder/coder/v2/tailnet" + previewtypes "github.com/coder/preview/types" ) // List is a helper function to reduce boilerplate when converting slices of @@ -764,3 +766,83 @@ func Chat(chat database.Chat) codersdk.Chat { func Chats(chats []database.Chat) []codersdk.Chat { return List(chats, Chat) } + +func PreviewParameter(param previewtypes.Parameter) codersdk.PreviewParameter { + return codersdk.PreviewParameter{ + PreviewParameterData: codersdk.PreviewParameterData{ + Name: param.Name, + DisplayName: param.DisplayName, + Description: param.Description, + Type: codersdk.OptionType(param.Type), + FormType: codersdk.ParameterFormType(param.FormType), + Styling: codersdk.PreviewParameterStyling{ + Placeholder: param.Styling.Placeholder, + Disabled: param.Styling.Disabled, + Label: param.Styling.Label, + }, + Mutable: param.Mutable, + DefaultValue: PreviewHCLString(param.DefaultValue), + Icon: param.Icon, + Options: List(param.Options, PreviewParameterOption), + Validations: List(param.Validations, PreviewParameterValidation), + Required: param.Required, + Order: param.Order, + Ephemeral: param.Ephemeral, + }, + Value: PreviewHCLString(param.Value), + Diagnostics: PreviewDiagnostics(param.Diagnostics), + } +} + +func HCLDiagnostics(d hcl.Diagnostics) []codersdk.FriendlyDiagnostic { + return PreviewDiagnostics(previewtypes.Diagnostics(d)) +} + +func PreviewDiagnostics(d previewtypes.Diagnostics) []codersdk.FriendlyDiagnostic { + f := d.FriendlyDiagnostics() + return List(f, func(f previewtypes.FriendlyDiagnostic) codersdk.FriendlyDiagnostic { + return codersdk.FriendlyDiagnostic{ + Severity: codersdk.DiagnosticSeverityString(f.Severity), + Summary: f.Summary, + Detail: f.Detail, + Extra: codersdk.DiagnosticExtra{ + Code: f.Extra.Code, + }, + } + }) +} + +func PreviewHCLString(h previewtypes.HCLString) codersdk.NullHCLString { + n := h.NullHCLString() + return codersdk.NullHCLString{ + Value: n.Value, + Valid: n.Valid, + } +} + +func PreviewParameterOption(o *previewtypes.ParameterOption) codersdk.PreviewParameterOption { + if o == nil { + // This should never be sent + return codersdk.PreviewParameterOption{} + } + return codersdk.PreviewParameterOption{ + Name: o.Name, + Description: o.Description, + Value: PreviewHCLString(o.Value), + Icon: o.Icon, + } +} + +func PreviewParameterValidation(v *previewtypes.ParameterValidation) codersdk.PreviewParameterValidation { + if v == nil { + // This should never be sent + return codersdk.PreviewParameterValidation{} + } + return codersdk.PreviewParameterValidation{ + Error: v.Error, + Regex: v.Regex, + Min: v.Min, + Max: v.Max, + Monotonic: v.Monotonic, + } +} diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 928dee0e30ea3..ab3781452dd2d 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -170,14 +170,14 @@ var ( Identifier: rbac.RoleIdentifier{Name: "provisionerd"}, DisplayName: "Provisioner Daemon", Site: rbac.Permissions(map[string][]policy.Action{ - // TODO: Add ProvisionerJob resource type. - rbac.ResourceFile.Type: {policy.ActionRead}, - rbac.ResourceSystem.Type: {policy.WildcardSymbol}, - rbac.ResourceTemplate.Type: {policy.ActionRead, policy.ActionUpdate}, + rbac.ResourceProvisionerJobs.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionCreate}, + rbac.ResourceFile.Type: {policy.ActionRead}, + rbac.ResourceSystem.Type: {policy.WildcardSymbol}, + rbac.ResourceTemplate.Type: {policy.ActionRead, policy.ActionUpdate}, // Unsure why provisionerd needs update and read personal rbac.ResourceUser.Type: {policy.ActionRead, policy.ActionReadPersonal, policy.ActionUpdatePersonal}, rbac.ResourceWorkspaceDormant.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStop}, - rbac.ResourceWorkspace.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop}, + rbac.ResourceWorkspace.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop, policy.ActionCreateAgent}, rbac.ResourceApiKey.Type: {policy.WildcardSymbol}, // When org scoped provisioner credentials are implemented, // this can be reduced to read a specific org. @@ -219,19 +219,20 @@ var ( Scope: rbac.ScopeAll, }.WithCachedASTValue() - // See unhanger package. - subjectHangDetector = rbac.Subject{ - Type: rbac.SubjectTypeHangDetector, - FriendlyName: "Hang Detector", + // See reaper package. + subjectJobReaper = rbac.Subject{ + Type: rbac.SubjectTypeJobReaper, + FriendlyName: "Job Reaper", ID: uuid.Nil.String(), Roles: rbac.Roles([]rbac.Role{ { - Identifier: rbac.RoleIdentifier{Name: "hangdetector"}, - DisplayName: "Hang Detector Daemon", + Identifier: rbac.RoleIdentifier{Name: "jobreaper"}, + DisplayName: "Job Reaper Daemon", Site: rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceSystem.Type: {policy.WildcardSymbol}, - rbac.ResourceTemplate.Type: {policy.ActionRead}, - rbac.ResourceWorkspace.Type: {policy.ActionRead, policy.ActionUpdate}, + rbac.ResourceSystem.Type: {policy.WildcardSymbol}, + rbac.ResourceTemplate.Type: {policy.ActionRead}, + rbac.ResourceWorkspace.Type: {policy.ActionRead, policy.ActionUpdate}, + rbac.ResourceProvisionerJobs.Type: {policy.ActionRead, policy.ActionUpdate}, }), Org: map[string][]rbac.Permission{}, User: []rbac.Permission{}, @@ -338,7 +339,7 @@ var ( rbac.ResourceProvisionerDaemon.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate}, rbac.ResourceUser.Type: rbac.ResourceUser.AvailableActions(), rbac.ResourceWorkspaceDormant.Type: {policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceStop}, - rbac.ResourceWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop, policy.ActionSSH}, + rbac.ResourceWorkspace.Type: {policy.ActionUpdate, policy.ActionDelete, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop, policy.ActionSSH, policy.ActionCreateAgent, policy.ActionDeleteAgent}, rbac.ResourceWorkspaceProxy.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete}, rbac.ResourceDeploymentConfig.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete}, rbac.ResourceNotificationMessage.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, @@ -346,6 +347,7 @@ var ( rbac.ResourceNotificationTemplate.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete}, rbac.ResourceCryptoKey.Type: {policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete}, rbac.ResourceFile.Type: {policy.ActionCreate, policy.ActionRead}, + rbac.ResourceProvisionerJobs.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionCreate}, }), Org: map[string][]rbac.Permission{}, User: []rbac.Permission{}, @@ -407,10 +409,10 @@ func AsAutostart(ctx context.Context) context.Context { return As(ctx, subjectAutostart) } -// AsHangDetector returns a context with an actor that has permissions required -// for unhanger.Detector to function. -func AsHangDetector(ctx context.Context) context.Context { - return As(ctx, subjectHangDetector) +// AsJobReaper returns a context with an actor that has permissions required +// for reaper.Detector to function. +func AsJobReaper(ctx context.Context) context.Context { + return As(ctx, subjectJobReaper) } // AsKeyRotator returns a context with an actor that has permissions required for rotating crypto keys. @@ -1085,11 +1087,10 @@ func (q *querier) AcquireNotificationMessages(ctx context.Context, arg database. return q.db.AcquireNotificationMessages(ctx, arg) } -// TODO: We need to create a ProvisionerJob resource type func (q *querier) AcquireProvisionerJob(ctx context.Context, arg database.AcquireProvisionerJobParams) (database.ProvisionerJob, error) { - // if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil { - // return database.ProvisionerJob{}, err - // } + if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceProvisionerJobs); err != nil { + return database.ProvisionerJob{}, err + } return q.db.AcquireProvisionerJob(ctx, arg) } @@ -1912,14 +1913,6 @@ func (q *querier) GetHealthSettings(ctx context.Context) (string, error) { return q.db.GetHealthSettings(ctx) } -// TODO: We need to create a ProvisionerJob resource type -func (q *querier) GetHungProvisionerJobs(ctx context.Context, hungSince time.Time) ([]database.ProvisionerJob, error) { - // if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceSystem); err != nil { - // return nil, err - // } - return q.db.GetHungProvisionerJobs(ctx, hungSince) -} - func (q *querier) GetInboxNotificationByID(ctx context.Context, id uuid.UUID) (database.InboxNotification, error) { return fetchWithAction(q.log, q.auth, policy.ActionRead, q.db.GetInboxNotificationByID)(ctx, id) } @@ -2307,6 +2300,13 @@ func (q *querier) GetProvisionerJobByID(ctx context.Context, id uuid.UUID) (data return job, nil } +func (q *querier) GetProvisionerJobByIDForUpdate(ctx context.Context, id uuid.UUID) (database.ProvisionerJob, error) { + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceProvisionerJobs); err != nil { + return database.ProvisionerJob{}, err + } + return q.db.GetProvisionerJobByIDForUpdate(ctx, id) +} + func (q *querier) GetProvisionerJobTimingsByJobID(ctx context.Context, jobID uuid.UUID) ([]database.ProvisionerJobTiming, error) { _, err := q.GetProvisionerJobByID(ctx, jobID) if err != nil { @@ -2315,31 +2315,49 @@ func (q *querier) GetProvisionerJobTimingsByJobID(ctx context.Context, jobID uui return q.db.GetProvisionerJobTimingsByJobID(ctx, jobID) } -// TODO: We have a ProvisionerJobs resource, but it hasn't been checked for this use-case. func (q *querier) GetProvisionerJobsByIDs(ctx context.Context, ids []uuid.UUID) ([]database.ProvisionerJob, error) { - // if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil { - // return nil, err - // } - return q.db.GetProvisionerJobsByIDs(ctx, ids) + provisionerJobs, err := q.db.GetProvisionerJobsByIDs(ctx, ids) + if err != nil { + return nil, err + } + orgIDs := make(map[uuid.UUID]struct{}) + for _, job := range provisionerJobs { + orgIDs[job.OrganizationID] = struct{}{} + } + for orgID := range orgIDs { + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceProvisionerJobs.InOrg(orgID)); err != nil { + return nil, err + } + } + return provisionerJobs, nil } -// TODO: We have a ProvisionerJobs resource, but it hasn't been checked for this use-case. func (q *querier) GetProvisionerJobsByIDsWithQueuePosition(ctx context.Context, ids []uuid.UUID) ([]database.GetProvisionerJobsByIDsWithQueuePositionRow, error) { + // TODO: Remove this once we have a proper rbac check for provisioner jobs. + // Details in https://github.com/coder/coder/issues/16160 return q.db.GetProvisionerJobsByIDsWithQueuePosition(ctx, ids) } func (q *querier) GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisioner(ctx context.Context, arg database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerParams) ([]database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerRow, error) { + // TODO: Remove this once we have a proper rbac check for provisioner jobs. + // Details in https://github.com/coder/coder/issues/16160 return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisioner)(ctx, arg) } -// TODO: We have a ProvisionerJobs resource, but it hasn't been checked for this use-case. func (q *querier) GetProvisionerJobsCreatedAfter(ctx context.Context, createdAt time.Time) ([]database.ProvisionerJob, error) { - // if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil { - // return nil, err - // } + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceProvisionerJobs); err != nil { + return nil, err + } return q.db.GetProvisionerJobsCreatedAfter(ctx, createdAt) } +func (q *querier) GetProvisionerJobsToBeReaped(ctx context.Context, arg database.GetProvisionerJobsToBeReapedParams) ([]database.ProvisionerJob, error) { + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceProvisionerJobs); err != nil { + return nil, err + } + return q.db.GetProvisionerJobsToBeReaped(ctx, arg) +} + func (q *querier) GetProvisionerKeyByHashedSecret(ctx context.Context, hashedSecret []byte) (database.ProvisionerKey, error) { return fetch(q.log, q.auth, q.db.GetProvisionerKeyByHashedSecret)(ctx, hashedSecret) } @@ -3162,6 +3180,10 @@ func (q *querier) GetWorkspaceByOwnerIDAndName(ctx context.Context, arg database return fetch(q.log, q.auth, q.db.GetWorkspaceByOwnerIDAndName)(ctx, arg) } +func (q *querier) GetWorkspaceByResourceID(ctx context.Context, resourceID uuid.UUID) (database.Workspace, error) { + return fetch(q.log, q.auth, q.db.GetWorkspaceByResourceID)(ctx, resourceID) +} + func (q *querier) GetWorkspaceByWorkspaceAppID(ctx context.Context, workspaceAppID uuid.UUID) (database.Workspace, error) { return fetch(q.log, q.auth, q.db.GetWorkspaceByWorkspaceAppID)(ctx, workspaceAppID) } @@ -3533,27 +3555,22 @@ func (q *querier) InsertPresetParameters(ctx context.Context, arg database.Inser return q.db.InsertPresetParameters(ctx, arg) } -// TODO: We need to create a ProvisionerJob resource type func (q *querier) InsertProvisionerJob(ctx context.Context, arg database.InsertProvisionerJobParams) (database.ProvisionerJob, error) { - // if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceSystem); err != nil { - // return database.ProvisionerJob{}, err - // } + // TODO: Remove this once we have a proper rbac check for provisioner jobs. + // Details in https://github.com/coder/coder/issues/16160 return q.db.InsertProvisionerJob(ctx, arg) } -// TODO: We need to create a ProvisionerJob resource type func (q *querier) InsertProvisionerJobLogs(ctx context.Context, arg database.InsertProvisionerJobLogsParams) ([]database.ProvisionerJobLog, error) { - // if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceSystem); err != nil { - // return nil, err - // } + // TODO: Remove this once we have a proper rbac check for provisioner jobs. + // Details in https://github.com/coder/coder/issues/16160 return q.db.InsertProvisionerJobLogs(ctx, arg) } -// TODO: We need to create a ProvisionerJob resource type func (q *querier) InsertProvisionerJobTimings(ctx context.Context, arg database.InsertProvisionerJobTimingsParams) ([]database.ProvisionerJobTiming, error) { - // if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceSystem); err != nil { - // return nil, err - // } + if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceProvisionerJobs); err != nil { + return nil, err + } return q.db.InsertProvisionerJobTimings(ctx, arg) } @@ -3700,9 +3717,24 @@ func (q *querier) InsertWorkspace(ctx context.Context, arg database.InsertWorksp } func (q *querier) InsertWorkspaceAgent(ctx context.Context, arg database.InsertWorkspaceAgentParams) (database.WorkspaceAgent, error) { - if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceSystem); err != nil { + // NOTE(DanielleMaywood): + // Currently, the only way to link a Resource back to a Workspace is by following this chain: + // + // WorkspaceResource -> WorkspaceBuild -> Workspace + // + // It is possible for this function to be called without there existing + // a `WorkspaceBuild` to link back to. This means that we want to allow + // execution to continue if there isn't a workspace found to allow this + // behavior to continue. + workspace, err := q.db.GetWorkspaceByResourceID(ctx, arg.ResourceID) + if err != nil && !errors.Is(err, sql.ErrNoRows) { return database.WorkspaceAgent{}, err } + + if err := q.authorizeContext(ctx, policy.ActionCreateAgent, workspace); err != nil { + return database.WorkspaceAgent{}, err + } + return q.db.InsertWorkspaceAgent(ctx, arg) } @@ -4176,15 +4208,17 @@ func (q *querier) UpdateProvisionerDaemonLastSeenAt(ctx context.Context, arg dat return q.db.UpdateProvisionerDaemonLastSeenAt(ctx, arg) } -// TODO: We need to create a ProvisionerJob resource type func (q *querier) UpdateProvisionerJobByID(ctx context.Context, arg database.UpdateProvisionerJobByIDParams) error { - // if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil { - // return err - // } + if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceProvisionerJobs); err != nil { + return err + } return q.db.UpdateProvisionerJobByID(ctx, arg) } func (q *querier) UpdateProvisionerJobWithCancelByID(ctx context.Context, arg database.UpdateProvisionerJobWithCancelByIDParams) error { + // TODO: Remove this once we have a proper rbac check for provisioner jobs. + // Details in https://github.com/coder/coder/issues/16160 + job, err := q.db.GetProvisionerJobByID(ctx, arg.ID) if err != nil { return err @@ -4251,14 +4285,20 @@ func (q *querier) UpdateProvisionerJobWithCancelByID(ctx context.Context, arg da return q.db.UpdateProvisionerJobWithCancelByID(ctx, arg) } -// TODO: We need to create a ProvisionerJob resource type func (q *querier) UpdateProvisionerJobWithCompleteByID(ctx context.Context, arg database.UpdateProvisionerJobWithCompleteByIDParams) error { - // if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil { - // return err - // } + if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceProvisionerJobs); err != nil { + return err + } return q.db.UpdateProvisionerJobWithCompleteByID(ctx, arg) } +func (q *querier) UpdateProvisionerJobWithCompleteWithStartedAtByID(ctx context.Context, arg database.UpdateProvisionerJobWithCompleteWithStartedAtByIDParams) error { + if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceProvisionerJobs); err != nil { + return err + } + return q.db.UpdateProvisionerJobWithCompleteWithStartedAtByID(ctx, arg) +} + func (q *querier) UpdateReplica(ctx context.Context, arg database.UpdateReplicaParams) (database.Replica, error) { if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil { return database.Replica{}, err diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index a0289f222392b..e8b90afbc396d 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -694,9 +694,12 @@ func (s *MethodTestSuite) TestProvisionerJob() { Asserts(v.RBACObject(tpl), []policy.Action{policy.ActionRead, policy.ActionUpdate}).Returns() })) s.Run("GetProvisionerJobsByIDs", s.Subtest(func(db database.Store, check *expects) { - a := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{}) - b := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{}) - check.Args([]uuid.UUID{a.ID, b.ID}).Asserts().Returns(slice.New(a, b)) + o := dbgen.Organization(s.T(), db, database.Organization{}) + a := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{OrganizationID: o.ID}) + b := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{OrganizationID: o.ID}) + check.Args([]uuid.UUID{a.ID, b.ID}). + Asserts(rbac.ResourceProvisionerJobs.InOrg(o.ID), policy.ActionRead). + Returns(slice.New(a, b)) })) s.Run("GetProvisionerLogsAfterID", s.Subtest(func(db database.Store, check *expects) { u := dbgen.User(s.T(), db, database.User{}) @@ -1925,6 +1928,22 @@ func (s *MethodTestSuite) TestWorkspace() { }) check.Args(ws.ID).Asserts(ws, policy.ActionRead) })) + s.Run("GetWorkspaceByResourceID", s.Subtest(func(db database.Store, check *expects) { + u := dbgen.User(s.T(), db, database.User{}) + o := dbgen.Organization(s.T(), db, database.Organization{}) + j := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{Type: database.ProvisionerJobTypeWorkspaceBuild}) + tpl := dbgen.Template(s.T(), db, database.Template{CreatedBy: u.ID, OrganizationID: o.ID}) + tv := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{ + TemplateID: uuid.NullUUID{UUID: tpl.ID, Valid: true}, + JobID: j.ID, + OrganizationID: o.ID, + CreatedBy: u.ID, + }) + ws := dbgen.Workspace(s.T(), db, database.WorkspaceTable{OwnerID: u.ID, TemplateID: tpl.ID, OrganizationID: o.ID}) + _ = dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: j.ID, TemplateVersionID: tv.ID}) + res := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: j.ID}) + check.Args(res.ID).Asserts(ws, policy.ActionRead) + })) s.Run("GetWorkspaces", s.Subtest(func(_ database.Store, check *expects) { // No asserts here because SQLFilter. check.Args(database.GetWorkspacesParams{}).Asserts() @@ -3923,9 +3942,8 @@ func (s *MethodTestSuite) TestSystemFunctions() { check.Args().Asserts(rbac.ResourceSystem, policy.ActionDelete) })) s.Run("GetProvisionerJobsCreatedAfter", s.Subtest(func(db database.Store, check *expects) { - // TODO: add provisioner job resource type _ = dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{CreatedAt: time.Now().Add(-time.Hour)}) - check.Args(time.Now()).Asserts( /*rbac.ResourceSystem, policy.ActionRead*/ ) + check.Args(time.Now()).Asserts(rbac.ResourceProvisionerJobs, policy.ActionRead) })) s.Run("GetTemplateVersionsByIDs", s.Subtest(func(db database.Store, check *expects) { dbtestutil.DisableForeignKeysAndTriggers(s.T(), db) @@ -4008,20 +4026,33 @@ func (s *MethodTestSuite) TestSystemFunctions() { Returns([]database.WorkspaceAgent{agt}) })) s.Run("GetProvisionerJobsByIDs", s.Subtest(func(db database.Store, check *expects) { - // TODO: add a ProvisionerJob resource type - a := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{}) - b := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{}) + o := dbgen.Organization(s.T(), db, database.Organization{}) + a := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{OrganizationID: o.ID}) + b := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{OrganizationID: o.ID}) check.Args([]uuid.UUID{a.ID, b.ID}). - Asserts( /*rbac.ResourceSystem, policy.ActionRead*/ ). + Asserts(rbac.ResourceProvisionerJobs.InOrg(o.ID), policy.ActionRead). Returns(slice.New(a, b)) })) s.Run("InsertWorkspaceAgent", s.Subtest(func(db database.Store, check *expects) { - dbtestutil.DisableForeignKeysAndTriggers(s.T(), db) + u := dbgen.User(s.T(), db, database.User{}) + o := dbgen.Organization(s.T(), db, database.Organization{}) + j := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{Type: database.ProvisionerJobTypeWorkspaceBuild}) + tpl := dbgen.Template(s.T(), db, database.Template{CreatedBy: u.ID, OrganizationID: o.ID}) + tv := dbgen.TemplateVersion(s.T(), db, database.TemplateVersion{ + TemplateID: uuid.NullUUID{UUID: tpl.ID, Valid: true}, + JobID: j.ID, + OrganizationID: o.ID, + CreatedBy: u.ID, + }) + ws := dbgen.Workspace(s.T(), db, database.WorkspaceTable{OwnerID: u.ID, TemplateID: tpl.ID, OrganizationID: o.ID}) + _ = dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: j.ID, TemplateVersionID: tv.ID}) + res := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: j.ID}) check.Args(database.InsertWorkspaceAgentParams{ ID: uuid.New(), + ResourceID: res.ID, Name: "dev", APIKeyScope: database.AgentKeyScopeEnumAll, - }).Asserts(rbac.ResourceSystem, policy.ActionCreate) + }).Asserts(ws, policy.ActionCreateAgent) })) s.Run("InsertWorkspaceApp", s.Subtest(func(db database.Store, check *expects) { dbtestutil.DisableForeignKeysAndTriggers(s.T(), db) @@ -4048,7 +4079,6 @@ func (s *MethodTestSuite) TestSystemFunctions() { }).Asserts(rbac.ResourceSystem, policy.ActionUpdate).Returns() })) s.Run("AcquireProvisionerJob", s.Subtest(func(db database.Store, check *expects) { - // TODO: we need to create a ProvisionerJob resource j := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{ StartedAt: sql.NullTime{Valid: false}, UpdatedAt: time.Now(), @@ -4058,47 +4088,48 @@ func (s *MethodTestSuite) TestSystemFunctions() { OrganizationID: j.OrganizationID, Types: []database.ProvisionerType{j.Provisioner}, ProvisionerTags: must(json.Marshal(j.Tags)), - }).Asserts( /*rbac.ResourceSystem, policy.ActionUpdate*/ ) + }).Asserts(rbac.ResourceProvisionerJobs, policy.ActionUpdate) })) s.Run("UpdateProvisionerJobWithCompleteByID", s.Subtest(func(db database.Store, check *expects) { - // TODO: we need to create a ProvisionerJob resource j := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{}) check.Args(database.UpdateProvisionerJobWithCompleteByIDParams{ ID: j.ID, - }).Asserts( /*rbac.ResourceSystem, policy.ActionUpdate*/ ) + }).Asserts(rbac.ResourceProvisionerJobs, policy.ActionUpdate) + })) + s.Run("UpdateProvisionerJobWithCompleteWithStartedAtByID", s.Subtest(func(db database.Store, check *expects) { + j := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{}) + check.Args(database.UpdateProvisionerJobWithCompleteWithStartedAtByIDParams{ + ID: j.ID, + }).Asserts(rbac.ResourceProvisionerJobs, policy.ActionUpdate) })) s.Run("UpdateProvisionerJobByID", s.Subtest(func(db database.Store, check *expects) { - // TODO: we need to create a ProvisionerJob resource j := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{}) check.Args(database.UpdateProvisionerJobByIDParams{ ID: j.ID, UpdatedAt: time.Now(), - }).Asserts( /*rbac.ResourceSystem, policy.ActionUpdate*/ ) + }).Asserts(rbac.ResourceProvisionerJobs, policy.ActionUpdate) })) s.Run("InsertProvisionerJob", s.Subtest(func(db database.Store, check *expects) { dbtestutil.DisableForeignKeysAndTriggers(s.T(), db) - // TODO: we need to create a ProvisionerJob resource check.Args(database.InsertProvisionerJobParams{ ID: uuid.New(), Provisioner: database.ProvisionerTypeEcho, StorageMethod: database.ProvisionerStorageMethodFile, Type: database.ProvisionerJobTypeWorkspaceBuild, Input: json.RawMessage("{}"), - }).Asserts( /*rbac.ResourceSystem, policy.ActionCreate*/ ) + }).Asserts( /* rbac.ResourceProvisionerJobs, policy.ActionCreate */ ) })) s.Run("InsertProvisionerJobLogs", s.Subtest(func(db database.Store, check *expects) { - // TODO: we need to create a ProvisionerJob resource j := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{}) check.Args(database.InsertProvisionerJobLogsParams{ JobID: j.ID, - }).Asserts( /*rbac.ResourceSystem, policy.ActionCreate*/ ) + }).Asserts( /* rbac.ResourceProvisionerJobs, policy.ActionUpdate */ ) })) s.Run("InsertProvisionerJobTimings", s.Subtest(func(db database.Store, check *expects) { - // TODO: we need to create a ProvisionerJob resource j := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{}) check.Args(database.InsertProvisionerJobTimingsParams{ JobID: j.ID, - }).Asserts( /*rbac.ResourceSystem, policy.ActionCreate*/ ) + }).Asserts(rbac.ResourceProvisionerJobs, policy.ActionUpdate) })) s.Run("UpsertProvisionerDaemon", s.Subtest(func(db database.Store, check *expects) { dbtestutil.DisableForeignKeysAndTriggers(s.T(), db) @@ -4234,8 +4265,8 @@ func (s *MethodTestSuite) TestSystemFunctions() { s.Run("GetFileTemplates", s.Subtest(func(db database.Store, check *expects) { check.Args(uuid.New()).Asserts(rbac.ResourceSystem, policy.ActionRead) })) - s.Run("GetHungProvisionerJobs", s.Subtest(func(db database.Store, check *expects) { - check.Args(time.Time{}).Asserts() + s.Run("GetProvisionerJobsToBeReaped", s.Subtest(func(db database.Store, check *expects) { + check.Args(database.GetProvisionerJobsToBeReapedParams{}).Asserts(rbac.ResourceProvisionerJobs, policy.ActionRead) })) s.Run("UpsertOAuthSigningKey", s.Subtest(func(db database.Store, check *expects) { check.Args("foo").Asserts(rbac.ResourceSystem, policy.ActionUpdate) @@ -4479,6 +4510,9 @@ func (s *MethodTestSuite) TestSystemFunctions() { VapidPrivateKey: "test", }).Asserts(rbac.ResourceDeploymentConfig, policy.ActionUpdate) })) + s.Run("GetProvisionerJobByIDForUpdate", s.Subtest(func(db database.Store, check *expects) { + check.Args(uuid.New()).Asserts(rbac.ResourceProvisionerJobs, policy.ActionRead).Errors(sql.ErrNoRows) + })) } func (s *MethodTestSuite) TestNotifications() { diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index fc5a10cafc481..75c56b9c2324d 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "math" + insecurerand "math/rand" //#nosec // this is only used for shuffling an array to pick random jobs to reap "reflect" "regexp" "slices" @@ -3707,23 +3708,6 @@ func (q *FakeQuerier) GetHealthSettings(_ context.Context) (string, error) { return string(q.healthSettings), nil } -func (q *FakeQuerier) GetHungProvisionerJobs(_ context.Context, hungSince time.Time) ([]database.ProvisionerJob, error) { - q.mutex.RLock() - defer q.mutex.RUnlock() - - hungJobs := []database.ProvisionerJob{} - for _, provisionerJob := range q.provisionerJobs { - if provisionerJob.StartedAt.Valid && !provisionerJob.CompletedAt.Valid && provisionerJob.UpdatedAt.Before(hungSince) { - // clone the Tags before appending, since maps are reference types and - // we don't want the caller to be able to mutate the map we have inside - // dbmem! - provisionerJob.Tags = maps.Clone(provisionerJob.Tags) - hungJobs = append(hungJobs, provisionerJob) - } - } - return hungJobs, nil -} - func (q *FakeQuerier) GetInboxNotificationByID(_ context.Context, id uuid.UUID) (database.InboxNotification, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -4642,6 +4626,13 @@ func (q *FakeQuerier) GetProvisionerJobByID(ctx context.Context, id uuid.UUID) ( return q.getProvisionerJobByIDNoLock(ctx, id) } +func (q *FakeQuerier) GetProvisionerJobByIDForUpdate(ctx context.Context, id uuid.UUID) (database.ProvisionerJob, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + + return q.getProvisionerJobByIDNoLock(ctx, id) +} + func (q *FakeQuerier) GetProvisionerJobTimingsByJobID(_ context.Context, jobID uuid.UUID) ([]database.ProvisionerJobTiming, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -4848,6 +4839,13 @@ func (q *FakeQuerier) GetProvisionerJobsByOrganizationAndStatusWithQueuePosition row.AvailableWorkers = append(row.AvailableWorkers, worker.ID) } } + + // Add daemon name to provisioner job + for _, daemon := range q.provisionerDaemons { + if job.WorkerID.Valid && job.WorkerID.UUID == daemon.ID { + row.WorkerName = daemon.Name + } + } rows = append(rows, row) } @@ -4877,6 +4875,33 @@ func (q *FakeQuerier) GetProvisionerJobsCreatedAfter(_ context.Context, after ti return jobs, nil } +func (q *FakeQuerier) GetProvisionerJobsToBeReaped(_ context.Context, arg database.GetProvisionerJobsToBeReapedParams) ([]database.ProvisionerJob, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + maxJobs := arg.MaxJobs + + hungJobs := []database.ProvisionerJob{} + for _, provisionerJob := range q.provisionerJobs { + if !provisionerJob.CompletedAt.Valid { + if (provisionerJob.StartedAt.Valid && provisionerJob.UpdatedAt.Before(arg.HungSince)) || + (!provisionerJob.StartedAt.Valid && provisionerJob.UpdatedAt.Before(arg.PendingSince)) { + // clone the Tags before appending, since maps are reference types and + // we don't want the caller to be able to mutate the map we have inside + // dbmem! + provisionerJob.Tags = maps.Clone(provisionerJob.Tags) + hungJobs = append(hungJobs, provisionerJob) + if len(hungJobs) >= int(maxJobs) { + break + } + } + } + } + insecurerand.Shuffle(len(hungJobs), func(i, j int) { + hungJobs[i], hungJobs[j] = hungJobs[j], hungJobs[i] + }) + return hungJobs, nil +} + func (q *FakeQuerier) GetProvisionerKeyByHashedSecret(_ context.Context, hashedSecret []byte) (database.ProvisionerKey, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -8028,6 +8053,33 @@ func (q *FakeQuerier) GetWorkspaceByOwnerIDAndName(_ context.Context, arg databa return database.Workspace{}, sql.ErrNoRows } +func (q *FakeQuerier) GetWorkspaceByResourceID(ctx context.Context, resourceID uuid.UUID) (database.Workspace, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + + for _, resource := range q.workspaceResources { + if resource.ID != resourceID { + continue + } + + for _, build := range q.workspaceBuilds { + if build.JobID != resource.JobID { + continue + } + + for _, workspace := range q.workspaces { + if workspace.ID != build.WorkspaceID { + continue + } + + return q.extendWorkspace(workspace), nil + } + } + } + + return database.Workspace{}, sql.ErrNoRows +} + func (q *FakeQuerier) GetWorkspaceByWorkspaceAppID(_ context.Context, workspaceAppID uuid.UUID) (database.Workspace, error) { if err := validateDatabaseType(workspaceAppID); err != nil { return database.Workspace{}, err @@ -10951,6 +11003,30 @@ func (q *FakeQuerier) UpdateProvisionerJobWithCompleteByID(_ context.Context, ar return sql.ErrNoRows } +func (q *FakeQuerier) UpdateProvisionerJobWithCompleteWithStartedAtByID(_ context.Context, arg database.UpdateProvisionerJobWithCompleteWithStartedAtByIDParams) error { + if err := validateDatabaseType(arg); err != nil { + return err + } + + q.mutex.Lock() + defer q.mutex.Unlock() + + for index, job := range q.provisionerJobs { + if arg.ID != job.ID { + continue + } + job.UpdatedAt = arg.UpdatedAt + job.CompletedAt = arg.CompletedAt + job.Error = arg.Error + job.ErrorCode = arg.ErrorCode + job.StartedAt = arg.StartedAt + job.JobStatus = provisionerJobStatus(job) + q.provisionerJobs[index] = job + return nil + } + return sql.ErrNoRows +} + func (q *FakeQuerier) UpdateReplica(_ context.Context, arg database.UpdateReplicaParams) (database.Replica, error) { if err := validateDatabaseType(arg); err != nil { return database.Replica{}, err diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index a5a22aad1a0bf..47ec185915660 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -865,13 +865,6 @@ func (m queryMetricsStore) GetHealthSettings(ctx context.Context) (string, error return r0, r1 } -func (m queryMetricsStore) GetHungProvisionerJobs(ctx context.Context, hungSince time.Time) ([]database.ProvisionerJob, error) { - start := time.Now() - jobs, err := m.s.GetHungProvisionerJobs(ctx, hungSince) - m.queryLatencies.WithLabelValues("GetHungProvisionerJobs").Observe(time.Since(start).Seconds()) - return jobs, err -} - func (m queryMetricsStore) GetInboxNotificationByID(ctx context.Context, id uuid.UUID) (database.InboxNotification, error) { start := time.Now() r0, r1 := m.s.GetInboxNotificationByID(ctx, id) @@ -1194,6 +1187,13 @@ func (m queryMetricsStore) GetProvisionerJobByID(ctx context.Context, id uuid.UU return job, err } +func (m queryMetricsStore) GetProvisionerJobByIDForUpdate(ctx context.Context, id uuid.UUID) (database.ProvisionerJob, error) { + start := time.Now() + r0, r1 := m.s.GetProvisionerJobByIDForUpdate(ctx, id) + m.queryLatencies.WithLabelValues("GetProvisionerJobByIDForUpdate").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m queryMetricsStore) GetProvisionerJobTimingsByJobID(ctx context.Context, jobID uuid.UUID) ([]database.ProvisionerJobTiming, error) { start := time.Now() r0, r1 := m.s.GetProvisionerJobTimingsByJobID(ctx, jobID) @@ -1229,6 +1229,13 @@ func (m queryMetricsStore) GetProvisionerJobsCreatedAfter(ctx context.Context, c return jobs, err } +func (m queryMetricsStore) GetProvisionerJobsToBeReaped(ctx context.Context, arg database.GetProvisionerJobsToBeReapedParams) ([]database.ProvisionerJob, error) { + start := time.Now() + r0, r1 := m.s.GetProvisionerJobsToBeReaped(ctx, arg) + m.queryLatencies.WithLabelValues("GetProvisionerJobsToBeReaped").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m queryMetricsStore) GetProvisionerKeyByHashedSecret(ctx context.Context, hashedSecret []byte) (database.ProvisionerKey, error) { start := time.Now() r0, r1 := m.s.GetProvisionerKeyByHashedSecret(ctx, hashedSecret) @@ -1880,6 +1887,13 @@ func (m queryMetricsStore) GetWorkspaceByOwnerIDAndName(ctx context.Context, arg return workspace, err } +func (m queryMetricsStore) GetWorkspaceByResourceID(ctx context.Context, resourceID uuid.UUID) (database.Workspace, error) { + start := time.Now() + r0, r1 := m.s.GetWorkspaceByResourceID(ctx, resourceID) + m.queryLatencies.WithLabelValues("GetWorkspaceByResourceID").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m queryMetricsStore) GetWorkspaceByWorkspaceAppID(ctx context.Context, workspaceAppID uuid.UUID) (database.Workspace, error) { start := time.Now() workspace, err := m.s.GetWorkspaceByWorkspaceAppID(ctx, workspaceAppID) @@ -2706,6 +2720,13 @@ func (m queryMetricsStore) UpdateProvisionerJobWithCompleteByID(ctx context.Cont return err } +func (m queryMetricsStore) UpdateProvisionerJobWithCompleteWithStartedAtByID(ctx context.Context, arg database.UpdateProvisionerJobWithCompleteWithStartedAtByIDParams) error { + start := time.Now() + r0 := m.s.UpdateProvisionerJobWithCompleteWithStartedAtByID(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateProvisionerJobWithCompleteWithStartedAtByID").Observe(time.Since(start).Seconds()) + return r0 +} + func (m queryMetricsStore) UpdateReplica(ctx context.Context, arg database.UpdateReplicaParams) (database.Replica, error) { start := time.Now() replica, err := m.s.UpdateReplica(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 0d66dcec11848..e3a9a14698e42 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -1743,21 +1743,6 @@ func (mr *MockStoreMockRecorder) GetHealthSettings(ctx any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetHealthSettings", reflect.TypeOf((*MockStore)(nil).GetHealthSettings), ctx) } -// GetHungProvisionerJobs mocks base method. -func (m *MockStore) GetHungProvisionerJobs(ctx context.Context, updatedAt time.Time) ([]database.ProvisionerJob, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetHungProvisionerJobs", ctx, updatedAt) - ret0, _ := ret[0].([]database.ProvisionerJob) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetHungProvisionerJobs indicates an expected call of GetHungProvisionerJobs. -func (mr *MockStoreMockRecorder) GetHungProvisionerJobs(ctx, updatedAt any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetHungProvisionerJobs", reflect.TypeOf((*MockStore)(nil).GetHungProvisionerJobs), ctx, updatedAt) -} - // GetInboxNotificationByID mocks base method. func (m *MockStore) GetInboxNotificationByID(ctx context.Context, id uuid.UUID) (database.InboxNotification, error) { m.ctrl.T.Helper() @@ -2448,6 +2433,21 @@ func (mr *MockStoreMockRecorder) GetProvisionerJobByID(ctx, id any) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetProvisionerJobByID", reflect.TypeOf((*MockStore)(nil).GetProvisionerJobByID), ctx, id) } +// GetProvisionerJobByIDForUpdate mocks base method. +func (m *MockStore) GetProvisionerJobByIDForUpdate(ctx context.Context, id uuid.UUID) (database.ProvisionerJob, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetProvisionerJobByIDForUpdate", ctx, id) + ret0, _ := ret[0].(database.ProvisionerJob) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetProvisionerJobByIDForUpdate indicates an expected call of GetProvisionerJobByIDForUpdate. +func (mr *MockStoreMockRecorder) GetProvisionerJobByIDForUpdate(ctx, id any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetProvisionerJobByIDForUpdate", reflect.TypeOf((*MockStore)(nil).GetProvisionerJobByIDForUpdate), ctx, id) +} + // GetProvisionerJobTimingsByJobID mocks base method. func (m *MockStore) GetProvisionerJobTimingsByJobID(ctx context.Context, jobID uuid.UUID) ([]database.ProvisionerJobTiming, error) { m.ctrl.T.Helper() @@ -2523,6 +2523,21 @@ func (mr *MockStoreMockRecorder) GetProvisionerJobsCreatedAfter(ctx, createdAt a return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetProvisionerJobsCreatedAfter", reflect.TypeOf((*MockStore)(nil).GetProvisionerJobsCreatedAfter), ctx, createdAt) } +// GetProvisionerJobsToBeReaped mocks base method. +func (m *MockStore) GetProvisionerJobsToBeReaped(ctx context.Context, arg database.GetProvisionerJobsToBeReapedParams) ([]database.ProvisionerJob, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetProvisionerJobsToBeReaped", ctx, arg) + ret0, _ := ret[0].([]database.ProvisionerJob) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetProvisionerJobsToBeReaped indicates an expected call of GetProvisionerJobsToBeReaped. +func (mr *MockStoreMockRecorder) GetProvisionerJobsToBeReaped(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetProvisionerJobsToBeReaped", reflect.TypeOf((*MockStore)(nil).GetProvisionerJobsToBeReaped), ctx, arg) +} + // GetProvisionerKeyByHashedSecret mocks base method. func (m *MockStore) GetProvisionerKeyByHashedSecret(ctx context.Context, hashedSecret []byte) (database.ProvisionerKey, error) { m.ctrl.T.Helper() @@ -3948,6 +3963,21 @@ func (mr *MockStoreMockRecorder) GetWorkspaceByOwnerIDAndName(ctx, arg any) *gom return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetWorkspaceByOwnerIDAndName", reflect.TypeOf((*MockStore)(nil).GetWorkspaceByOwnerIDAndName), ctx, arg) } +// GetWorkspaceByResourceID mocks base method. +func (m *MockStore) GetWorkspaceByResourceID(ctx context.Context, resourceID uuid.UUID) (database.Workspace, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetWorkspaceByResourceID", ctx, resourceID) + ret0, _ := ret[0].(database.Workspace) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetWorkspaceByResourceID indicates an expected call of GetWorkspaceByResourceID. +func (mr *MockStoreMockRecorder) GetWorkspaceByResourceID(ctx, resourceID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetWorkspaceByResourceID", reflect.TypeOf((*MockStore)(nil).GetWorkspaceByResourceID), ctx, resourceID) +} + // GetWorkspaceByWorkspaceAppID mocks base method. func (m *MockStore) GetWorkspaceByWorkspaceAppID(ctx context.Context, workspaceAppID uuid.UUID) (database.Workspace, error) { m.ctrl.T.Helper() @@ -5732,6 +5762,20 @@ func (mr *MockStoreMockRecorder) UpdateProvisionerJobWithCompleteByID(ctx, arg a return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateProvisionerJobWithCompleteByID", reflect.TypeOf((*MockStore)(nil).UpdateProvisionerJobWithCompleteByID), ctx, arg) } +// UpdateProvisionerJobWithCompleteWithStartedAtByID mocks base method. +func (m *MockStore) UpdateProvisionerJobWithCompleteWithStartedAtByID(ctx context.Context, arg database.UpdateProvisionerJobWithCompleteWithStartedAtByIDParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateProvisionerJobWithCompleteWithStartedAtByID", ctx, arg) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateProvisionerJobWithCompleteWithStartedAtByID indicates an expected call of UpdateProvisionerJobWithCompleteWithStartedAtByID. +func (mr *MockStoreMockRecorder) UpdateProvisionerJobWithCompleteWithStartedAtByID(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateProvisionerJobWithCompleteWithStartedAtByID", reflect.TypeOf((*MockStore)(nil).UpdateProvisionerJobWithCompleteWithStartedAtByID), ctx, arg) +} + // UpdateReplica mocks base method. func (m *MockStore) UpdateReplica(ctx context.Context, arg database.UpdateReplicaParams) (database.Replica, error) { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 81b8d58758ada..d248780397ead 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -196,7 +196,6 @@ type sqlcQuerier interface { GetGroupMembersCountByGroupID(ctx context.Context, arg GetGroupMembersCountByGroupIDParams) (int64, error) GetGroups(ctx context.Context, arg GetGroupsParams) ([]GetGroupsRow, error) GetHealthSettings(ctx context.Context) (string, error) - GetHungProvisionerJobs(ctx context.Context, updatedAt time.Time) ([]ProvisionerJob, error) GetInboxNotificationByID(ctx context.Context, id uuid.UUID) (InboxNotification, error) // Fetches inbox notifications for a user filtered by templates and targets // param user_id: The user ID @@ -265,11 +264,16 @@ type sqlcQuerier interface { // Previous job information. GetProvisionerDaemonsWithStatusByOrganization(ctx context.Context, arg GetProvisionerDaemonsWithStatusByOrganizationParams) ([]GetProvisionerDaemonsWithStatusByOrganizationRow, error) GetProvisionerJobByID(ctx context.Context, id uuid.UUID) (ProvisionerJob, error) + // Gets a single provisioner job by ID for update. + // This is used to securely reap jobs that have been hung/pending for a long time. + GetProvisionerJobByIDForUpdate(ctx context.Context, id uuid.UUID) (ProvisionerJob, error) GetProvisionerJobTimingsByJobID(ctx context.Context, jobID uuid.UUID) ([]ProvisionerJobTiming, error) GetProvisionerJobsByIDs(ctx context.Context, ids []uuid.UUID) ([]ProvisionerJob, error) GetProvisionerJobsByIDsWithQueuePosition(ctx context.Context, ids []uuid.UUID) ([]GetProvisionerJobsByIDsWithQueuePositionRow, error) GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisioner(ctx context.Context, arg GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerParams) ([]GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerRow, error) GetProvisionerJobsCreatedAfter(ctx context.Context, createdAt time.Time) ([]ProvisionerJob, error) + // To avoid repeatedly attempting to reap the same jobs, we randomly order and limit to @max_jobs. + GetProvisionerJobsToBeReaped(ctx context.Context, arg GetProvisionerJobsToBeReapedParams) ([]ProvisionerJob, error) GetProvisionerKeyByHashedSecret(ctx context.Context, hashedSecret []byte) (ProvisionerKey, error) GetProvisionerKeyByID(ctx context.Context, id uuid.UUID) (ProvisionerKey, error) GetProvisionerKeyByName(ctx context.Context, arg GetProvisionerKeyByNameParams) (ProvisionerKey, error) @@ -418,6 +422,7 @@ type sqlcQuerier interface { GetWorkspaceByAgentID(ctx context.Context, agentID uuid.UUID) (Workspace, error) GetWorkspaceByID(ctx context.Context, id uuid.UUID) (Workspace, error) GetWorkspaceByOwnerIDAndName(ctx context.Context, arg GetWorkspaceByOwnerIDAndNameParams) (Workspace, error) + GetWorkspaceByResourceID(ctx context.Context, resourceID uuid.UUID) (Workspace, error) GetWorkspaceByWorkspaceAppID(ctx context.Context, workspaceAppID uuid.UUID) (Workspace, error) GetWorkspaceModulesByJobID(ctx context.Context, jobID uuid.UUID) ([]WorkspaceModule, error) GetWorkspaceModulesCreatedAfter(ctx context.Context, createdAt time.Time) ([]WorkspaceModule, error) @@ -567,6 +572,7 @@ type sqlcQuerier interface { UpdateProvisionerJobByID(ctx context.Context, arg UpdateProvisionerJobByIDParams) error UpdateProvisionerJobWithCancelByID(ctx context.Context, arg UpdateProvisionerJobWithCancelByIDParams) error UpdateProvisionerJobWithCompleteByID(ctx context.Context, arg UpdateProvisionerJobWithCompleteByIDParams) error + UpdateProvisionerJobWithCompleteWithStartedAtByID(ctx context.Context, arg UpdateProvisionerJobWithCompleteWithStartedAtByIDParams) error UpdateReplica(ctx context.Context, arg UpdateReplicaParams) (Replica, error) UpdateTailnetPeerStatusByCoordinator(ctx context.Context, arg UpdateTailnetPeerStatusByCoordinatorParams) error UpdateTemplateACLByID(ctx context.Context, arg UpdateTemplateACLByIDParams) error diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index ac08d72d0e493..99a8bf4603b57 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -7384,71 +7384,57 @@ func (q *sqlQuerier) AcquireProvisionerJob(ctx context.Context, arg AcquireProvi return i, err } -const getHungProvisionerJobs = `-- name: GetHungProvisionerJobs :many +const getProvisionerJobByID = `-- name: GetProvisionerJobByID :one SELECT id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id, tags, error_code, trace_metadata, job_status FROM provisioner_jobs WHERE - updated_at < $1 - AND started_at IS NOT NULL - AND completed_at IS NULL + id = $1 ` -func (q *sqlQuerier) GetHungProvisionerJobs(ctx context.Context, updatedAt time.Time) ([]ProvisionerJob, error) { - rows, err := q.db.QueryContext(ctx, getHungProvisionerJobs, updatedAt) - if err != nil { - return nil, err - } - defer rows.Close() - var items []ProvisionerJob - for rows.Next() { - var i ProvisionerJob - if err := rows.Scan( - &i.ID, - &i.CreatedAt, - &i.UpdatedAt, - &i.StartedAt, - &i.CanceledAt, - &i.CompletedAt, - &i.Error, - &i.OrganizationID, - &i.InitiatorID, - &i.Provisioner, - &i.StorageMethod, - &i.Type, - &i.Input, - &i.WorkerID, - &i.FileID, - &i.Tags, - &i.ErrorCode, - &i.TraceMetadata, - &i.JobStatus, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil +func (q *sqlQuerier) GetProvisionerJobByID(ctx context.Context, id uuid.UUID) (ProvisionerJob, error) { + row := q.db.QueryRowContext(ctx, getProvisionerJobByID, id) + var i ProvisionerJob + err := row.Scan( + &i.ID, + &i.CreatedAt, + &i.UpdatedAt, + &i.StartedAt, + &i.CanceledAt, + &i.CompletedAt, + &i.Error, + &i.OrganizationID, + &i.InitiatorID, + &i.Provisioner, + &i.StorageMethod, + &i.Type, + &i.Input, + &i.WorkerID, + &i.FileID, + &i.Tags, + &i.ErrorCode, + &i.TraceMetadata, + &i.JobStatus, + ) + return i, err } -const getProvisionerJobByID = `-- name: GetProvisionerJobByID :one +const getProvisionerJobByIDForUpdate = `-- name: GetProvisionerJobByIDForUpdate :one SELECT id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id, tags, error_code, trace_metadata, job_status FROM provisioner_jobs WHERE id = $1 +FOR UPDATE +SKIP LOCKED ` -func (q *sqlQuerier) GetProvisionerJobByID(ctx context.Context, id uuid.UUID) (ProvisionerJob, error) { - row := q.db.QueryRowContext(ctx, getProvisionerJobByID, id) +// Gets a single provisioner job by ID for update. +// This is used to securely reap jobs that have been hung/pending for a long time. +func (q *sqlQuerier) GetProvisionerJobByIDForUpdate(ctx context.Context, id uuid.UUID) (ProvisionerJob, error) { + row := q.db.QueryRowContext(ctx, getProvisionerJobByIDForUpdate, id) var i ProvisionerJob err := row.Scan( &i.ID, @@ -7730,7 +7716,9 @@ SELECT COALESCE(t.display_name, '') AS template_display_name, COALESCE(t.icon, '') AS template_icon, w.id AS workspace_id, - COALESCE(w.name, '') AS workspace_name + COALESCE(w.name, '') AS workspace_name, + -- Include the name of the provisioner_daemon associated to the job + COALESCE(pd.name, '') AS worker_name FROM provisioner_jobs pj LEFT JOIN @@ -7755,6 +7743,9 @@ LEFT JOIN t.id = tv.template_id AND t.organization_id = pj.organization_id ) +LEFT JOIN + -- Join to get the daemon name corresponding to the job's worker_id + provisioner_daemons pd ON pd.id = pj.worker_id WHERE pj.organization_id = $1::uuid AND (COALESCE(array_length($2::uuid[], 1), 0) = 0 OR pj.id = ANY($2::uuid[])) @@ -7770,7 +7761,8 @@ GROUP BY t.display_name, t.icon, w.id, - w.name + w.name, + pd.name ORDER BY pj.created_at DESC LIMIT @@ -7797,6 +7789,7 @@ type GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerRow TemplateIcon string `db:"template_icon" json:"template_icon"` WorkspaceID uuid.NullUUID `db:"workspace_id" json:"workspace_id"` WorkspaceName string `db:"workspace_name" json:"workspace_name"` + WorkerName string `db:"worker_name" json:"worker_name"` } func (q *sqlQuerier) GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisioner(ctx context.Context, arg GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerParams) ([]GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerRow, error) { @@ -7844,6 +7837,7 @@ func (q *sqlQuerier) GetProvisionerJobsByOrganizationAndStatusWithQueuePositionA &i.TemplateIcon, &i.WorkspaceID, &i.WorkspaceName, + &i.WorkerName, ); err != nil { return nil, err } @@ -7905,6 +7899,79 @@ func (q *sqlQuerier) GetProvisionerJobsCreatedAfter(ctx context.Context, created return items, nil } +const getProvisionerJobsToBeReaped = `-- name: GetProvisionerJobsToBeReaped :many +SELECT + id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id, tags, error_code, trace_metadata, job_status +FROM + provisioner_jobs +WHERE + ( + -- If the job has not been started before @pending_since, reap it. + updated_at < $1 + AND started_at IS NULL + AND completed_at IS NULL + ) + OR + ( + -- If the job has been started but not completed before @hung_since, reap it. + updated_at < $2 + AND started_at IS NOT NULL + AND completed_at IS NULL + ) +ORDER BY random() +LIMIT $3 +` + +type GetProvisionerJobsToBeReapedParams struct { + PendingSince time.Time `db:"pending_since" json:"pending_since"` + HungSince time.Time `db:"hung_since" json:"hung_since"` + MaxJobs int32 `db:"max_jobs" json:"max_jobs"` +} + +// To avoid repeatedly attempting to reap the same jobs, we randomly order and limit to @max_jobs. +func (q *sqlQuerier) GetProvisionerJobsToBeReaped(ctx context.Context, arg GetProvisionerJobsToBeReapedParams) ([]ProvisionerJob, error) { + rows, err := q.db.QueryContext(ctx, getProvisionerJobsToBeReaped, arg.PendingSince, arg.HungSince, arg.MaxJobs) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ProvisionerJob + for rows.Next() { + var i ProvisionerJob + if err := rows.Scan( + &i.ID, + &i.CreatedAt, + &i.UpdatedAt, + &i.StartedAt, + &i.CanceledAt, + &i.CompletedAt, + &i.Error, + &i.OrganizationID, + &i.InitiatorID, + &i.Provisioner, + &i.StorageMethod, + &i.Type, + &i.Input, + &i.WorkerID, + &i.FileID, + &i.Tags, + &i.ErrorCode, + &i.TraceMetadata, + &i.JobStatus, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const insertProvisionerJob = `-- name: InsertProvisionerJob :one INSERT INTO provisioner_jobs ( @@ -8113,6 +8180,40 @@ func (q *sqlQuerier) UpdateProvisionerJobWithCompleteByID(ctx context.Context, a return err } +const updateProvisionerJobWithCompleteWithStartedAtByID = `-- name: UpdateProvisionerJobWithCompleteWithStartedAtByID :exec +UPDATE + provisioner_jobs +SET + updated_at = $2, + completed_at = $3, + error = $4, + error_code = $5, + started_at = $6 +WHERE + id = $1 +` + +type UpdateProvisionerJobWithCompleteWithStartedAtByIDParams struct { + ID uuid.UUID `db:"id" json:"id"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + CompletedAt sql.NullTime `db:"completed_at" json:"completed_at"` + Error sql.NullString `db:"error" json:"error"` + ErrorCode sql.NullString `db:"error_code" json:"error_code"` + StartedAt sql.NullTime `db:"started_at" json:"started_at"` +} + +func (q *sqlQuerier) UpdateProvisionerJobWithCompleteWithStartedAtByID(ctx context.Context, arg UpdateProvisionerJobWithCompleteWithStartedAtByIDParams) error { + _, err := q.db.ExecContext(ctx, updateProvisionerJobWithCompleteWithStartedAtByID, + arg.ID, + arg.UpdatedAt, + arg.CompletedAt, + arg.Error, + arg.ErrorCode, + arg.StartedAt, + ) + return err +} + const deleteProvisionerKey = `-- name: DeleteProvisionerKey :exec DELETE FROM provisioner_keys @@ -18042,6 +18143,65 @@ func (q *sqlQuerier) GetWorkspaceByOwnerIDAndName(ctx context.Context, arg GetWo return i, err } +const getWorkspaceByResourceID = `-- name: GetWorkspaceByResourceID :one +SELECT + id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, dormant_at, deleting_at, automatic_updates, favorite, next_start_at, owner_avatar_url, owner_username, organization_name, organization_display_name, organization_icon, organization_description, template_name, template_display_name, template_icon, template_description +FROM + workspaces_expanded as workspaces +WHERE + workspaces.id = ( + SELECT + workspace_id + FROM + workspace_builds + WHERE + workspace_builds.job_id = ( + SELECT + job_id + FROM + workspace_resources + WHERE + workspace_resources.id = $1 + ) + ) +LIMIT + 1 +` + +func (q *sqlQuerier) GetWorkspaceByResourceID(ctx context.Context, resourceID uuid.UUID) (Workspace, error) { + row := q.db.QueryRowContext(ctx, getWorkspaceByResourceID, resourceID) + var i Workspace + err := row.Scan( + &i.ID, + &i.CreatedAt, + &i.UpdatedAt, + &i.OwnerID, + &i.OrganizationID, + &i.TemplateID, + &i.Deleted, + &i.Name, + &i.AutostartSchedule, + &i.Ttl, + &i.LastUsedAt, + &i.DormantAt, + &i.DeletingAt, + &i.AutomaticUpdates, + &i.Favorite, + &i.NextStartAt, + &i.OwnerAvatarUrl, + &i.OwnerUsername, + &i.OrganizationName, + &i.OrganizationDisplayName, + &i.OrganizationIcon, + &i.OrganizationDescription, + &i.TemplateName, + &i.TemplateDisplayName, + &i.TemplateIcon, + &i.TemplateDescription, + ) + return i, err +} + const getWorkspaceByWorkspaceAppID = `-- name: GetWorkspaceByWorkspaceAppID :one SELECT id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, dormant_at, deleting_at, automatic_updates, favorite, next_start_at, owner_avatar_url, owner_username, organization_name, organization_display_name, organization_icon, organization_description, template_name, template_display_name, template_icon, template_description diff --git a/coderd/database/queries/provisionerjobs.sql b/coderd/database/queries/provisionerjobs.sql index 2d544aedb9bd8..88bacc705601c 100644 --- a/coderd/database/queries/provisionerjobs.sql +++ b/coderd/database/queries/provisionerjobs.sql @@ -41,6 +41,18 @@ FROM WHERE id = $1; +-- name: GetProvisionerJobByIDForUpdate :one +-- Gets a single provisioner job by ID for update. +-- This is used to securely reap jobs that have been hung/pending for a long time. +SELECT + * +FROM + provisioner_jobs +WHERE + id = $1 +FOR UPDATE +SKIP LOCKED; + -- name: GetProvisionerJobsByIDs :many SELECT * @@ -160,7 +172,9 @@ SELECT COALESCE(t.display_name, '') AS template_display_name, COALESCE(t.icon, '') AS template_icon, w.id AS workspace_id, - COALESCE(w.name, '') AS workspace_name + COALESCE(w.name, '') AS workspace_name, + -- Include the name of the provisioner_daemon associated to the job + COALESCE(pd.name, '') AS worker_name FROM provisioner_jobs pj LEFT JOIN @@ -185,6 +199,9 @@ LEFT JOIN t.id = tv.template_id AND t.organization_id = pj.organization_id ) +LEFT JOIN + -- Join to get the daemon name corresponding to the job's worker_id + provisioner_daemons pd ON pd.id = pj.worker_id WHERE pj.organization_id = @organization_id::uuid AND (COALESCE(array_length(@ids::uuid[], 1), 0) = 0 OR pj.id = ANY(@ids::uuid[])) @@ -200,7 +217,8 @@ GROUP BY t.display_name, t.icon, w.id, - w.name + w.name, + pd.name ORDER BY pj.created_at DESC LIMIT @@ -256,15 +274,40 @@ SET WHERE id = $1; --- name: GetHungProvisionerJobs :many +-- name: UpdateProvisionerJobWithCompleteWithStartedAtByID :exec +UPDATE + provisioner_jobs +SET + updated_at = $2, + completed_at = $3, + error = $4, + error_code = $5, + started_at = $6 +WHERE + id = $1; + +-- name: GetProvisionerJobsToBeReaped :many SELECT * FROM provisioner_jobs WHERE - updated_at < $1 - AND started_at IS NOT NULL - AND completed_at IS NULL; + ( + -- If the job has not been started before @pending_since, reap it. + updated_at < @pending_since + AND started_at IS NULL + AND completed_at IS NULL + ) + OR + ( + -- If the job has been started but not completed before @hung_since, reap it. + updated_at < @hung_since + AND started_at IS NOT NULL + AND completed_at IS NULL + ) +-- To avoid repeatedly attempting to reap the same jobs, we randomly order and limit to @max_jobs. +ORDER BY random() +LIMIT @max_jobs; -- name: InsertProvisionerJobTimings :many INSERT INTO provisioner_job_timings (job_id, started_at, ended_at, stage, source, action, resource) diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 4ec74c066fe41..44b7dcbf0387d 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -8,6 +8,30 @@ WHERE LIMIT 1; +-- name: GetWorkspaceByResourceID :one +SELECT + * +FROM + workspaces_expanded as workspaces +WHERE + workspaces.id = ( + SELECT + workspace_id + FROM + workspace_builds + WHERE + workspace_builds.job_id = ( + SELECT + job_id + FROM + workspace_resources + WHERE + workspace_resources.id = @resource_id + ) + ) +LIMIT + 1; + -- name: GetWorkspaceByWorkspaceAppID :one SELECT * diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go index d614b37a3d897..4b92848b773e2 100644 --- a/coderd/httpmw/apikey.go +++ b/coderd/httpmw/apikey.go @@ -232,16 +232,21 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon return optionalWrite(http.StatusUnauthorized, resp) } - var ( - link database.UserLink - now = dbtime.Now() - // Tracks if the API key has properties updated - changed = false - ) + now := dbtime.Now() + if key.ExpiresAt.Before(now) { + return optionalWrite(http.StatusUnauthorized, codersdk.Response{ + Message: SignedOutErrorMessage, + Detail: fmt.Sprintf("API key expired at %q.", key.ExpiresAt.String()), + }) + } + + // We only check OIDC stuff if we have a valid APIKey. An expired key means we don't trust the requestor + // really is the user whose key they have, and so we shouldn't be doing anything on their behalf including possibly + // refreshing the OIDC token. if key.LoginType == database.LoginTypeGithub || key.LoginType == database.LoginTypeOIDC { var err error //nolint:gocritic // System needs to fetch UserLink to check if it's valid. - link, err = cfg.DB.GetUserLinkByUserIDLoginType(dbauthz.AsSystemRestricted(ctx), database.GetUserLinkByUserIDLoginTypeParams{ + link, err := cfg.DB.GetUserLinkByUserIDLoginType(dbauthz.AsSystemRestricted(ctx), database.GetUserLinkByUserIDLoginTypeParams{ UserID: key.UserID, LoginType: key.LoginType, }) @@ -258,7 +263,7 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon }) } // Check if the OAuth token is expired - if link.OAuthExpiry.Before(now) && !link.OAuthExpiry.IsZero() && link.OAuthRefreshToken != "" { + if !link.OAuthExpiry.IsZero() && link.OAuthExpiry.Before(now) { if cfg.OAuth2Configs.IsZero() { return write(http.StatusInternalServerError, codersdk.Response{ Message: internalErrorMessage, @@ -267,12 +272,15 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon }) } + var friendlyName string var oauthConfig promoauth.OAuth2Config switch key.LoginType { case database.LoginTypeGithub: oauthConfig = cfg.OAuth2Configs.Github + friendlyName = "GitHub" case database.LoginTypeOIDC: oauthConfig = cfg.OAuth2Configs.OIDC + friendlyName = "OpenID Connect" default: return write(http.StatusInternalServerError, codersdk.Response{ Message: internalErrorMessage, @@ -292,7 +300,13 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon }) } - // If it is, let's refresh it from the provided config + if link.OAuthRefreshToken == "" { + return optionalWrite(http.StatusUnauthorized, codersdk.Response{ + Message: SignedOutErrorMessage, + Detail: fmt.Sprintf("%s session expired at %q. Try signing in again.", friendlyName, link.OAuthExpiry.String()), + }) + } + // We have a refresh token, so let's try it token, err := oauthConfig.TokenSource(r.Context(), &oauth2.Token{ AccessToken: link.OAuthAccessToken, RefreshToken: link.OAuthRefreshToken, @@ -300,28 +314,39 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon }).Token() if err != nil { return write(http.StatusUnauthorized, codersdk.Response{ - Message: "Could not refresh expired Oauth token. Try re-authenticating to resolve this issue.", - Detail: err.Error(), + Message: fmt.Sprintf( + "Could not refresh expired %s token. Try re-authenticating to resolve this issue.", + friendlyName), + Detail: err.Error(), }) } link.OAuthAccessToken = token.AccessToken link.OAuthRefreshToken = token.RefreshToken link.OAuthExpiry = token.Expiry - key.ExpiresAt = token.Expiry - changed = true + //nolint:gocritic // system needs to update user link + link, err = cfg.DB.UpdateUserLink(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLinkParams{ + UserID: link.UserID, + LoginType: link.LoginType, + OAuthAccessToken: link.OAuthAccessToken, + OAuthAccessTokenKeyID: sql.NullString{}, // dbcrypt will update as required + OAuthRefreshToken: link.OAuthRefreshToken, + OAuthRefreshTokenKeyID: sql.NullString{}, // dbcrypt will update as required + OAuthExpiry: link.OAuthExpiry, + // Refresh should keep the same debug context because we use + // the original claims for the group/role sync. + Claims: link.Claims, + }) + if err != nil { + return write(http.StatusInternalServerError, codersdk.Response{ + Message: internalErrorMessage, + Detail: fmt.Sprintf("update user_link: %s.", err.Error()), + }) + } } } - // Checking if the key is expired. - // NOTE: The `RequireAuth` React component depends on this `Detail` to detect when - // the users token has expired. If you change the text here, make sure to update it - // in site/src/components/RequireAuth/RequireAuth.tsx as well. - if key.ExpiresAt.Before(now) { - return optionalWrite(http.StatusUnauthorized, codersdk.Response{ - Message: SignedOutErrorMessage, - Detail: fmt.Sprintf("API key expired at %q.", key.ExpiresAt.String()), - }) - } + // Tracks if the API key has properties updated + changed := false // Only update LastUsed once an hour to prevent database spam. if now.Sub(key.LastUsed) > time.Hour { @@ -363,29 +388,6 @@ func ExtractAPIKey(rw http.ResponseWriter, r *http.Request, cfg ExtractAPIKeyCon Detail: fmt.Sprintf("API key couldn't update: %s.", err.Error()), }) } - // If the API Key is associated with a user_link (e.g. Github/OIDC) - // then we want to update the relevant oauth fields. - if link.UserID != uuid.Nil { - //nolint:gocritic // system needs to update user link - link, err = cfg.DB.UpdateUserLink(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLinkParams{ - UserID: link.UserID, - LoginType: link.LoginType, - OAuthAccessToken: link.OAuthAccessToken, - OAuthAccessTokenKeyID: sql.NullString{}, // dbcrypt will update as required - OAuthRefreshToken: link.OAuthRefreshToken, - OAuthRefreshTokenKeyID: sql.NullString{}, // dbcrypt will update as required - OAuthExpiry: link.OAuthExpiry, - // Refresh should keep the same debug context because we use - // the original claims for the group/role sync. - Claims: link.Claims, - }) - if err != nil { - return write(http.StatusInternalServerError, codersdk.Response{ - Message: internalErrorMessage, - Detail: fmt.Sprintf("update user_link: %s.", err.Error()), - }) - } - } // We only want to update this occasionally to reduce DB write // load. We update alongside the UserLink and APIKey since it's diff --git a/coderd/httpmw/apikey_test.go b/coderd/httpmw/apikey_test.go index bd979e88235ad..6e2e75ace9825 100644 --- a/coderd/httpmw/apikey_test.go +++ b/coderd/httpmw/apikey_test.go @@ -508,6 +508,102 @@ func TestAPIKey(t *testing.T) { require.Equal(t, sentAPIKey.ExpiresAt, gotAPIKey.ExpiresAt) }) + t.Run("APIKeyExpiredOAuthExpired", func(t *testing.T) { + t.Parallel() + var ( + db = dbmem.New() + user = dbgen.User(t, db, database.User{}) + sentAPIKey, token = dbgen.APIKey(t, db, database.APIKey{ + UserID: user.ID, + LastUsed: dbtime.Now().AddDate(0, 0, -1), + ExpiresAt: dbtime.Now().AddDate(0, 0, -1), + LoginType: database.LoginTypeOIDC, + }) + _ = dbgen.UserLink(t, db, database.UserLink{ + UserID: user.ID, + LoginType: database.LoginTypeOIDC, + OAuthExpiry: dbtime.Now().AddDate(0, 0, -1), + }) + + r = httptest.NewRequest("GET", "/", nil) + rw = httptest.NewRecorder() + ) + r.Header.Set(codersdk.SessionTokenHeader, token) + + // Include a valid oauth token for refreshing. If this token is invalid, + // it is difficult to tell an auth failure from an expired api key, or + // an expired oauth key. + oauthToken := &oauth2.Token{ + AccessToken: "wow", + RefreshToken: "moo", + Expiry: dbtime.Now().AddDate(0, 0, 1), + } + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ + DB: db, + OAuth2Configs: &httpmw.OAuth2Configs{ + OIDC: &testutil.OAuth2Config{ + Token: oauthToken, + }, + }, + RedirectToLogin: false, + })(successHandler).ServeHTTP(rw, r) + res := rw.Result() + defer res.Body.Close() + require.Equal(t, http.StatusUnauthorized, res.StatusCode) + + gotAPIKey, err := db.GetAPIKeyByID(r.Context(), sentAPIKey.ID) + require.NoError(t, err) + + require.Equal(t, sentAPIKey.LastUsed, gotAPIKey.LastUsed) + require.Equal(t, sentAPIKey.ExpiresAt, gotAPIKey.ExpiresAt) + }) + + t.Run("APIKeyExpiredOAuthNotExpired", func(t *testing.T) { + t.Parallel() + var ( + db = dbmem.New() + user = dbgen.User(t, db, database.User{}) + sentAPIKey, token = dbgen.APIKey(t, db, database.APIKey{ + UserID: user.ID, + LastUsed: dbtime.Now().AddDate(0, 0, -1), + ExpiresAt: dbtime.Now().AddDate(0, 0, -1), + LoginType: database.LoginTypeOIDC, + }) + _ = dbgen.UserLink(t, db, database.UserLink{ + UserID: user.ID, + LoginType: database.LoginTypeOIDC, + }) + + r = httptest.NewRequest("GET", "/", nil) + rw = httptest.NewRecorder() + ) + r.Header.Set(codersdk.SessionTokenHeader, token) + + oauthToken := &oauth2.Token{ + AccessToken: "wow", + RefreshToken: "moo", + Expiry: dbtime.Now().AddDate(0, 0, 1), + } + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ + DB: db, + OAuth2Configs: &httpmw.OAuth2Configs{ + OIDC: &testutil.OAuth2Config{ + Token: oauthToken, + }, + }, + RedirectToLogin: false, + })(successHandler).ServeHTTP(rw, r) + res := rw.Result() + defer res.Body.Close() + require.Equal(t, http.StatusUnauthorized, res.StatusCode) + + gotAPIKey, err := db.GetAPIKeyByID(r.Context(), sentAPIKey.ID) + require.NoError(t, err) + + require.Equal(t, sentAPIKey.LastUsed, gotAPIKey.LastUsed) + require.Equal(t, sentAPIKey.ExpiresAt, gotAPIKey.ExpiresAt) + }) + t.Run("OAuthRefresh", func(t *testing.T) { t.Parallel() var ( @@ -553,7 +649,67 @@ func TestAPIKey(t *testing.T) { require.NoError(t, err) require.Equal(t, sentAPIKey.LastUsed, gotAPIKey.LastUsed) - require.Equal(t, oauthToken.Expiry, gotAPIKey.ExpiresAt) + // Note that OAuth expiry is independent of APIKey expiry, so an OIDC refresh DOES NOT affect the expiry of the + // APIKey + require.Equal(t, sentAPIKey.ExpiresAt, gotAPIKey.ExpiresAt) + + gotLink, err := db.GetUserLinkByUserIDLoginType(r.Context(), database.GetUserLinkByUserIDLoginTypeParams{ + UserID: user.ID, + LoginType: database.LoginTypeGithub, + }) + require.NoError(t, err) + require.Equal(t, gotLink.OAuthRefreshToken, "moo") + }) + + t.Run("OAuthExpiredNoRefresh", func(t *testing.T) { + t.Parallel() + var ( + ctx = testutil.Context(t, testutil.WaitShort) + db = dbmem.New() + user = dbgen.User(t, db, database.User{}) + sentAPIKey, token = dbgen.APIKey(t, db, database.APIKey{ + UserID: user.ID, + LastUsed: dbtime.Now(), + ExpiresAt: dbtime.Now().AddDate(0, 0, 1), + LoginType: database.LoginTypeGithub, + }) + + r = httptest.NewRequest("GET", "/", nil) + rw = httptest.NewRecorder() + ) + _, err := db.InsertUserLink(ctx, database.InsertUserLinkParams{ + UserID: user.ID, + LoginType: database.LoginTypeGithub, + OAuthExpiry: dbtime.Now().AddDate(0, 0, -1), + OAuthAccessToken: "letmein", + }) + require.NoError(t, err) + + r.Header.Set(codersdk.SessionTokenHeader, token) + + oauthToken := &oauth2.Token{ + AccessToken: "wow", + RefreshToken: "moo", + Expiry: dbtime.Now().AddDate(0, 0, 1), + } + httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{ + DB: db, + OAuth2Configs: &httpmw.OAuth2Configs{ + Github: &testutil.OAuth2Config{ + Token: oauthToken, + }, + }, + RedirectToLogin: false, + })(successHandler).ServeHTTP(rw, r) + res := rw.Result() + defer res.Body.Close() + require.Equal(t, http.StatusUnauthorized, res.StatusCode) + + gotAPIKey, err := db.GetAPIKeyByID(r.Context(), sentAPIKey.ID) + require.NoError(t, err) + + require.Equal(t, sentAPIKey.LastUsed, gotAPIKey.LastUsed) + require.Equal(t, sentAPIKey.ExpiresAt, gotAPIKey.ExpiresAt) }) t.Run("RemoteIPUpdates", func(t *testing.T) { diff --git a/coderd/httpmw/loggermw/logger.go b/coderd/httpmw/loggermw/logger.go index 9eeb07a5f10e5..30e5e2d811ad8 100644 --- a/coderd/httpmw/loggermw/logger.go +++ b/coderd/httpmw/loggermw/logger.go @@ -132,7 +132,7 @@ var actorLogOrder = []rbac.SubjectType{ rbac.SubjectTypeAutostart, rbac.SubjectTypeCryptoKeyReader, rbac.SubjectTypeCryptoKeyRotator, - rbac.SubjectTypeHangDetector, + rbac.SubjectTypeJobReaper, rbac.SubjectTypeNotifier, rbac.SubjectTypePrebuildsOrchestrator, rbac.SubjectTypeProvisionerd, diff --git a/coderd/unhanger/detector.go b/coderd/jobreaper/detector.go similarity index 72% rename from coderd/unhanger/detector.go rename to coderd/jobreaper/detector.go index 14383b1839363..ad5774ee6b95d 100644 --- a/coderd/unhanger/detector.go +++ b/coderd/jobreaper/detector.go @@ -1,11 +1,10 @@ -package unhanger +package jobreaper import ( "context" "database/sql" "encoding/json" - "fmt" - "math/rand" //#nosec // this is only used for shuffling an array to pick random jobs to unhang + "fmt" //#nosec // this is only used for shuffling an array to pick random jobs to unhang "time" "golang.org/x/xerrors" @@ -21,10 +20,14 @@ import ( ) const ( - // HungJobDuration is the duration of time since the last update to a job - // before it is considered hung. + // HungJobDuration is the duration of time since the last update + // to a RUNNING job before it is considered hung. HungJobDuration = 5 * time.Minute + // PendingJobDuration is the duration of time since last update + // to a PENDING job before it is considered dead. + PendingJobDuration = 30 * time.Minute + // HungJobExitTimeout is the duration of time that provisioners should allow // for a graceful exit upon cancellation due to failing to send an update to // a job. @@ -38,16 +41,30 @@ const ( MaxJobsPerRun = 10 ) -// HungJobLogMessages are written to provisioner job logs when a job is hung and -// terminated. -var HungJobLogMessages = []string{ - "", - "====================", - "Coder: Build has been detected as hung for 5 minutes and will be terminated.", - "====================", - "", +// jobLogMessages are written to provisioner job logs when a job is reaped +func JobLogMessages(reapType ReapType, threshold time.Duration) []string { + return []string{ + "", + "====================", + fmt.Sprintf("Coder: Build has been detected as %s for %.0f minutes and will be terminated.", reapType, threshold.Minutes()), + "====================", + "", + } +} + +type jobToReap struct { + ID uuid.UUID + Threshold time.Duration + Type ReapType } +type ReapType string + +const ( + Pending ReapType = "pending" + Hung ReapType = "hung" +) + // acquireLockError is returned when the detector fails to acquire a lock and // cancels the current run. type acquireLockError struct{} @@ -93,10 +110,10 @@ type Stats struct { Error error } -// New returns a new hang detector. +// New returns a new job reaper. func New(ctx context.Context, db database.Store, pub pubsub.Pubsub, log slog.Logger, tick <-chan time.Time) *Detector { - //nolint:gocritic // Hang detector has a limited set of permissions. - ctx, cancel := context.WithCancel(dbauthz.AsHangDetector(ctx)) + //nolint:gocritic // Job reaper has a limited set of permissions. + ctx, cancel := context.WithCancel(dbauthz.AsJobReaper(ctx)) d := &Detector{ ctx: ctx, cancel: cancel, @@ -172,34 +189,42 @@ func (d *Detector) run(t time.Time) Stats { Error: nil, } - // Find all provisioner jobs that are currently running but have not - // received an update in the last 5 minutes. - jobs, err := d.db.GetHungProvisionerJobs(ctx, t.Add(-HungJobDuration)) + // Find all provisioner jobs to be reaped + jobs, err := d.db.GetProvisionerJobsToBeReaped(ctx, database.GetProvisionerJobsToBeReapedParams{ + PendingSince: t.Add(-PendingJobDuration), + HungSince: t.Add(-HungJobDuration), + MaxJobs: MaxJobsPerRun, + }) if err != nil { - stats.Error = xerrors.Errorf("get hung provisioner jobs: %w", err) + stats.Error = xerrors.Errorf("get provisioner jobs to be reaped: %w", err) return stats } - // Limit the number of jobs we'll unhang in a single run to avoid - // timing out. - if len(jobs) > MaxJobsPerRun { - // Pick a random subset of the jobs to unhang. - rand.Shuffle(len(jobs), func(i, j int) { - jobs[i], jobs[j] = jobs[j], jobs[i] - }) - jobs = jobs[:MaxJobsPerRun] - } + jobsToReap := make([]*jobToReap, 0, len(jobs)) - // Send a message into the build log for each hung job saying that it - // has been detected and will be terminated, then mark the job as - // failed. for _, job := range jobs { + j := &jobToReap{ + ID: job.ID, + } + if job.JobStatus == database.ProvisionerJobStatusPending { + j.Threshold = PendingJobDuration + j.Type = Pending + } else { + j.Threshold = HungJobDuration + j.Type = Hung + } + jobsToReap = append(jobsToReap, j) + } + + // Send a message into the build log for each hung or pending job saying that it + // has been detected and will be terminated, then mark the job as failed. + for _, job := range jobsToReap { log := d.log.With(slog.F("job_id", job.ID)) - err := unhangJob(ctx, log, d.db, d.pubsub, job.ID) + err := reapJob(ctx, log, d.db, d.pubsub, job) if err != nil { if !(xerrors.As(err, &acquireLockError{}) || xerrors.As(err, &jobIneligibleError{})) { - log.Error(ctx, "error forcefully terminating hung provisioner job", slog.Error(err)) + log.Error(ctx, "error forcefully terminating provisioner job", slog.F("type", job.Type), slog.Error(err)) } continue } @@ -210,47 +235,34 @@ func (d *Detector) run(t time.Time) Stats { return stats } -func unhangJob(ctx context.Context, log slog.Logger, db database.Store, pub pubsub.Pubsub, jobID uuid.UUID) error { +func reapJob(ctx context.Context, log slog.Logger, db database.Store, pub pubsub.Pubsub, jobToReap *jobToReap) error { var lowestLogID int64 err := db.InTx(func(db database.Store) error { - locked, err := db.TryAcquireLock(ctx, database.GenLockID(fmt.Sprintf("hang-detector:%s", jobID))) - if err != nil { - return xerrors.Errorf("acquire lock: %w", err) - } - if !locked { - // This error is ignored. - return acquireLockError{} - } - // Refetch the job while we hold the lock. - job, err := db.GetProvisionerJobByID(ctx, jobID) + job, err := db.GetProvisionerJobByIDForUpdate(ctx, jobToReap.ID) if err != nil { + if xerrors.Is(err, sql.ErrNoRows) { + return acquireLockError{} + } return xerrors.Errorf("get provisioner job: %w", err) } - // Check if we should still unhang it. - if !job.StartedAt.Valid { - // This shouldn't be possible to hit because the query only selects - // started and not completed jobs, and a job can't be "un-started". - return jobIneligibleError{ - Err: xerrors.New("job is not started"), - } - } if job.CompletedAt.Valid { return jobIneligibleError{ Err: xerrors.Errorf("job is completed (status %s)", job.JobStatus), } } - if job.UpdatedAt.After(time.Now().Add(-HungJobDuration)) { + if job.UpdatedAt.After(time.Now().Add(-jobToReap.Threshold)) { return jobIneligibleError{ Err: xerrors.New("job has been updated recently"), } } log.Warn( - ctx, "detected hung provisioner job, forcefully terminating", - "threshold", HungJobDuration, + ctx, "forcefully terminating provisioner job", + "type", jobToReap.Type, + "threshold", jobToReap.Threshold, ) // First, get the latest logs from the build so we can make sure @@ -260,7 +272,7 @@ func unhangJob(ctx context.Context, log slog.Logger, db database.Store, pub pubs CreatedAfter: 0, }) if err != nil { - return xerrors.Errorf("get logs for hung job: %w", err) + return xerrors.Errorf("get logs for %s job: %w", jobToReap.Type, err) } logStage := "" if len(logs) != 0 { @@ -280,7 +292,7 @@ func unhangJob(ctx context.Context, log slog.Logger, db database.Store, pub pubs Output: nil, } now := dbtime.Now() - for i, msg := range HungJobLogMessages { + for i, msg := range JobLogMessages(jobToReap.Type, jobToReap.Threshold) { // Set the created at in a way that ensures each message has // a unique timestamp so they will be sorted correctly. insertParams.CreatedAt = append(insertParams.CreatedAt, now.Add(time.Millisecond*time.Duration(i))) @@ -291,13 +303,22 @@ func unhangJob(ctx context.Context, log slog.Logger, db database.Store, pub pubs } newLogs, err := db.InsertProvisionerJobLogs(ctx, insertParams) if err != nil { - return xerrors.Errorf("insert logs for hung job: %w", err) + return xerrors.Errorf("insert logs for %s job: %w", job.JobStatus, err) } lowestLogID = newLogs[0].ID // Mark the job as failed. now = dbtime.Now() - err = db.UpdateProvisionerJobWithCompleteByID(ctx, database.UpdateProvisionerJobWithCompleteByIDParams{ + + // If the job was never started (pending), set the StartedAt time to the current + // time so that the build duration is correct. + if job.JobStatus == database.ProvisionerJobStatusPending { + job.StartedAt = sql.NullTime{ + Time: now, + Valid: true, + } + } + err = db.UpdateProvisionerJobWithCompleteWithStartedAtByID(ctx, database.UpdateProvisionerJobWithCompleteWithStartedAtByIDParams{ ID: job.ID, UpdatedAt: now, CompletedAt: sql.NullTime{ @@ -305,12 +326,13 @@ func unhangJob(ctx context.Context, log slog.Logger, db database.Store, pub pubs Valid: true, }, Error: sql.NullString{ - String: "Coder: Build has been detected as hung for 5 minutes and has been terminated by hang detector.", + String: fmt.Sprintf("Coder: Build has been detected as %s for %.0f minutes and has been terminated by the reaper.", jobToReap.Type, jobToReap.Threshold.Minutes()), Valid: true, }, ErrorCode: sql.NullString{ Valid: false, }, + StartedAt: job.StartedAt, }) if err != nil { return xerrors.Errorf("mark job as failed: %w", err) @@ -364,7 +386,7 @@ func unhangJob(ctx context.Context, log slog.Logger, db database.Store, pub pubs if err != nil { return xerrors.Errorf("marshal log notification: %w", err) } - err = pub.Publish(provisionersdk.ProvisionerJobLogsNotifyChannel(jobID), data) + err = pub.Publish(provisionersdk.ProvisionerJobLogsNotifyChannel(jobToReap.ID), data) if err != nil { return xerrors.Errorf("publish log notification: %w", err) } diff --git a/coderd/unhanger/detector_test.go b/coderd/jobreaper/detector_test.go similarity index 73% rename from coderd/unhanger/detector_test.go rename to coderd/jobreaper/detector_test.go index 43eb62bfa884b..28457aeeca3a8 100644 --- a/coderd/unhanger/detector_test.go +++ b/coderd/jobreaper/detector_test.go @@ -1,4 +1,4 @@ -package unhanger_test +package jobreaper_test import ( "context" @@ -20,9 +20,9 @@ import ( "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/jobreaper" "github.com/coder/coder/v2/coderd/provisionerdserver" "github.com/coder/coder/v2/coderd/rbac" - "github.com/coder/coder/v2/coderd/unhanger" "github.com/coder/coder/v2/provisionersdk" "github.com/coder/coder/v2/testutil" ) @@ -39,10 +39,10 @@ func TestDetectorNoJobs(t *testing.T) { db, pubsub = dbtestutil.NewDB(t) log = testutil.Logger(t) tickCh = make(chan time.Time) - statsCh = make(chan unhanger.Stats) + statsCh = make(chan jobreaper.Stats) ) - detector := unhanger.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) + detector := jobreaper.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) detector.Start() tickCh <- time.Now() @@ -62,7 +62,7 @@ func TestDetectorNoHungJobs(t *testing.T) { db, pubsub = dbtestutil.NewDB(t) log = testutil.Logger(t) tickCh = make(chan time.Time) - statsCh = make(chan unhanger.Stats) + statsCh = make(chan jobreaper.Stats) ) // Insert some jobs that are running and haven't been updated in a while, @@ -89,7 +89,7 @@ func TestDetectorNoHungJobs(t *testing.T) { }) } - detector := unhanger.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) + detector := jobreaper.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) detector.Start() tickCh <- now @@ -109,7 +109,7 @@ func TestDetectorHungWorkspaceBuild(t *testing.T) { db, pubsub = dbtestutil.NewDB(t) log = testutil.Logger(t) tickCh = make(chan time.Time) - statsCh = make(chan unhanger.Stats) + statsCh = make(chan jobreaper.Stats) ) var ( @@ -195,7 +195,7 @@ func TestDetectorHungWorkspaceBuild(t *testing.T) { t.Log("previous job ID: ", previousWorkspaceBuildJob.ID) t.Log("current job ID: ", currentWorkspaceBuildJob.ID) - detector := unhanger.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) + detector := jobreaper.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) detector.Start() tickCh <- now @@ -231,7 +231,7 @@ func TestDetectorHungWorkspaceBuildNoOverrideState(t *testing.T) { db, pubsub = dbtestutil.NewDB(t) log = testutil.Logger(t) tickCh = make(chan time.Time) - statsCh = make(chan unhanger.Stats) + statsCh = make(chan jobreaper.Stats) ) var ( @@ -318,7 +318,7 @@ func TestDetectorHungWorkspaceBuildNoOverrideState(t *testing.T) { t.Log("previous job ID: ", previousWorkspaceBuildJob.ID) t.Log("current job ID: ", currentWorkspaceBuildJob.ID) - detector := unhanger.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) + detector := jobreaper.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) detector.Start() tickCh <- now @@ -354,7 +354,7 @@ func TestDetectorHungWorkspaceBuildNoOverrideStateIfNoExistingBuild(t *testing.T db, pubsub = dbtestutil.NewDB(t) log = testutil.Logger(t) tickCh = make(chan time.Time) - statsCh = make(chan unhanger.Stats) + statsCh = make(chan jobreaper.Stats) ) var ( @@ -411,7 +411,7 @@ func TestDetectorHungWorkspaceBuildNoOverrideStateIfNoExistingBuild(t *testing.T t.Log("current job ID: ", currentWorkspaceBuildJob.ID) - detector := unhanger.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) + detector := jobreaper.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) detector.Start() tickCh <- now @@ -439,6 +439,100 @@ func TestDetectorHungWorkspaceBuildNoOverrideStateIfNoExistingBuild(t *testing.T detector.Wait() } +func TestDetectorPendingWorkspaceBuildNoOverrideStateIfNoExistingBuild(t *testing.T) { + t.Parallel() + + var ( + ctx = testutil.Context(t, testutil.WaitLong) + db, pubsub = dbtestutil.NewDB(t) + log = testutil.Logger(t) + tickCh = make(chan time.Time) + statsCh = make(chan jobreaper.Stats) + ) + + var ( + now = time.Now() + thirtyFiveMinAgo = now.Add(-time.Minute * 35) + org = dbgen.Organization(t, db, database.Organization{}) + user = dbgen.User(t, db, database.User{}) + file = dbgen.File(t, db, database.File{}) + template = dbgen.Template(t, db, database.Template{ + OrganizationID: org.ID, + CreatedBy: user.ID, + }) + templateVersion = dbgen.TemplateVersion(t, db, database.TemplateVersion{ + OrganizationID: org.ID, + TemplateID: uuid.NullUUID{ + UUID: template.ID, + Valid: true, + }, + CreatedBy: user.ID, + }) + workspace = dbgen.Workspace(t, db, database.WorkspaceTable{ + OwnerID: user.ID, + OrganizationID: org.ID, + TemplateID: template.ID, + }) + + // First build. + expectedWorkspaceBuildState = []byte(`{"dean":"cool","colin":"also cool"}`) + currentWorkspaceBuildJob = dbgen.ProvisionerJob(t, db, pubsub, database.ProvisionerJob{ + CreatedAt: thirtyFiveMinAgo, + UpdatedAt: thirtyFiveMinAgo, + StartedAt: sql.NullTime{ + Time: time.Time{}, + Valid: false, + }, + OrganizationID: org.ID, + InitiatorID: user.ID, + Provisioner: database.ProvisionerTypeEcho, + StorageMethod: database.ProvisionerStorageMethodFile, + FileID: file.ID, + Type: database.ProvisionerJobTypeWorkspaceBuild, + Input: []byte("{}"), + }) + currentWorkspaceBuild = dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ + WorkspaceID: workspace.ID, + TemplateVersionID: templateVersion.ID, + BuildNumber: 1, + JobID: currentWorkspaceBuildJob.ID, + // Should not be overridden. + ProvisionerState: expectedWorkspaceBuildState, + }) + ) + + t.Log("current job ID: ", currentWorkspaceBuildJob.ID) + + detector := jobreaper.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) + detector.Start() + tickCh <- now + + stats := <-statsCh + require.NoError(t, stats.Error) + require.Len(t, stats.TerminatedJobIDs, 1) + require.Equal(t, currentWorkspaceBuildJob.ID, stats.TerminatedJobIDs[0]) + + // Check that the current provisioner job was updated. + job, err := db.GetProvisionerJobByID(ctx, currentWorkspaceBuildJob.ID) + require.NoError(t, err) + require.WithinDuration(t, now, job.UpdatedAt, 30*time.Second) + require.True(t, job.CompletedAt.Valid) + require.WithinDuration(t, now, job.CompletedAt.Time, 30*time.Second) + require.True(t, job.StartedAt.Valid) + require.WithinDuration(t, now, job.StartedAt.Time, 30*time.Second) + require.True(t, job.Error.Valid) + require.Contains(t, job.Error.String, "Build has been detected as pending") + require.False(t, job.ErrorCode.Valid) + + // Check that the provisioner state was NOT updated. + build, err := db.GetWorkspaceBuildByID(ctx, currentWorkspaceBuild.ID) + require.NoError(t, err) + require.Equal(t, expectedWorkspaceBuildState, build.ProvisionerState) + + detector.Close() + detector.Wait() +} + func TestDetectorHungOtherJobTypes(t *testing.T) { t.Parallel() @@ -447,7 +541,7 @@ func TestDetectorHungOtherJobTypes(t *testing.T) { db, pubsub = dbtestutil.NewDB(t) log = testutil.Logger(t) tickCh = make(chan time.Time) - statsCh = make(chan unhanger.Stats) + statsCh = make(chan jobreaper.Stats) ) var ( @@ -509,7 +603,7 @@ func TestDetectorHungOtherJobTypes(t *testing.T) { t.Log("template import job ID: ", templateImportJob.ID) t.Log("template dry-run job ID: ", templateDryRunJob.ID) - detector := unhanger.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) + detector := jobreaper.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) detector.Start() tickCh <- now @@ -543,6 +637,113 @@ func TestDetectorHungOtherJobTypes(t *testing.T) { detector.Wait() } +func TestDetectorPendingOtherJobTypes(t *testing.T) { + t.Parallel() + + var ( + ctx = testutil.Context(t, testutil.WaitLong) + db, pubsub = dbtestutil.NewDB(t) + log = testutil.Logger(t) + tickCh = make(chan time.Time) + statsCh = make(chan jobreaper.Stats) + ) + + var ( + now = time.Now() + thirtyFiveMinAgo = now.Add(-time.Minute * 35) + org = dbgen.Organization(t, db, database.Organization{}) + user = dbgen.User(t, db, database.User{}) + file = dbgen.File(t, db, database.File{}) + + // Template import job. + templateImportJob = dbgen.ProvisionerJob(t, db, pubsub, database.ProvisionerJob{ + CreatedAt: thirtyFiveMinAgo, + UpdatedAt: thirtyFiveMinAgo, + StartedAt: sql.NullTime{ + Time: time.Time{}, + Valid: false, + }, + OrganizationID: org.ID, + InitiatorID: user.ID, + Provisioner: database.ProvisionerTypeEcho, + StorageMethod: database.ProvisionerStorageMethodFile, + FileID: file.ID, + Type: database.ProvisionerJobTypeTemplateVersionImport, + Input: []byte("{}"), + }) + _ = dbgen.TemplateVersion(t, db, database.TemplateVersion{ + OrganizationID: org.ID, + JobID: templateImportJob.ID, + CreatedBy: user.ID, + }) + ) + + // Template dry-run job. + dryRunVersion := dbgen.TemplateVersion(t, db, database.TemplateVersion{ + OrganizationID: org.ID, + CreatedBy: user.ID, + }) + input, err := json.Marshal(provisionerdserver.TemplateVersionDryRunJob{ + TemplateVersionID: dryRunVersion.ID, + }) + require.NoError(t, err) + templateDryRunJob := dbgen.ProvisionerJob(t, db, pubsub, database.ProvisionerJob{ + CreatedAt: thirtyFiveMinAgo, + UpdatedAt: thirtyFiveMinAgo, + StartedAt: sql.NullTime{ + Time: time.Time{}, + Valid: false, + }, + OrganizationID: org.ID, + InitiatorID: user.ID, + Provisioner: database.ProvisionerTypeEcho, + StorageMethod: database.ProvisionerStorageMethodFile, + FileID: file.ID, + Type: database.ProvisionerJobTypeTemplateVersionDryRun, + Input: input, + }) + + t.Log("template import job ID: ", templateImportJob.ID) + t.Log("template dry-run job ID: ", templateDryRunJob.ID) + + detector := jobreaper.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) + detector.Start() + tickCh <- now + + stats := <-statsCh + require.NoError(t, stats.Error) + require.Len(t, stats.TerminatedJobIDs, 2) + require.Contains(t, stats.TerminatedJobIDs, templateImportJob.ID) + require.Contains(t, stats.TerminatedJobIDs, templateDryRunJob.ID) + + // Check that the template import job was updated. + job, err := db.GetProvisionerJobByID(ctx, templateImportJob.ID) + require.NoError(t, err) + require.WithinDuration(t, now, job.UpdatedAt, 30*time.Second) + require.True(t, job.CompletedAt.Valid) + require.WithinDuration(t, now, job.CompletedAt.Time, 30*time.Second) + require.True(t, job.StartedAt.Valid) + require.WithinDuration(t, now, job.StartedAt.Time, 30*time.Second) + require.True(t, job.Error.Valid) + require.Contains(t, job.Error.String, "Build has been detected as pending") + require.False(t, job.ErrorCode.Valid) + + // Check that the template dry-run job was updated. + job, err = db.GetProvisionerJobByID(ctx, templateDryRunJob.ID) + require.NoError(t, err) + require.WithinDuration(t, now, job.UpdatedAt, 30*time.Second) + require.True(t, job.CompletedAt.Valid) + require.WithinDuration(t, now, job.CompletedAt.Time, 30*time.Second) + require.True(t, job.StartedAt.Valid) + require.WithinDuration(t, now, job.StartedAt.Time, 30*time.Second) + require.True(t, job.Error.Valid) + require.Contains(t, job.Error.String, "Build has been detected as pending") + require.False(t, job.ErrorCode.Valid) + + detector.Close() + detector.Wait() +} + func TestDetectorHungCanceledJob(t *testing.T) { t.Parallel() @@ -551,7 +752,7 @@ func TestDetectorHungCanceledJob(t *testing.T) { db, pubsub = dbtestutil.NewDB(t) log = testutil.Logger(t) tickCh = make(chan time.Time) - statsCh = make(chan unhanger.Stats) + statsCh = make(chan jobreaper.Stats) ) var ( @@ -591,7 +792,7 @@ func TestDetectorHungCanceledJob(t *testing.T) { t.Log("template import job ID: ", templateImportJob.ID) - detector := unhanger.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) + detector := jobreaper.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) detector.Start() tickCh <- now @@ -653,7 +854,7 @@ func TestDetectorPushesLogs(t *testing.T) { db, pubsub = dbtestutil.NewDB(t) log = testutil.Logger(t) tickCh = make(chan time.Time) - statsCh = make(chan unhanger.Stats) + statsCh = make(chan jobreaper.Stats) ) var ( @@ -706,7 +907,7 @@ func TestDetectorPushesLogs(t *testing.T) { require.Len(t, logs, 10) } - detector := unhanger.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) + detector := jobreaper.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) detector.Start() // Create pubsub subscription to listen for new log events. @@ -741,12 +942,19 @@ func TestDetectorPushesLogs(t *testing.T) { CreatedAfter: after, }) require.NoError(t, err) - require.Len(t, logs, len(unhanger.HungJobLogMessages)) + threshold := jobreaper.HungJobDuration + jobType := jobreaper.Hung + if templateImportJob.JobStatus == database.ProvisionerJobStatusPending { + threshold = jobreaper.PendingJobDuration + jobType = jobreaper.Pending + } + expectedLogs := jobreaper.JobLogMessages(jobType, threshold) + require.Len(t, logs, len(expectedLogs)) for i, log := range logs { assert.Equal(t, database.LogLevelError, log.Level) assert.Equal(t, c.expectStage, log.Stage) assert.Equal(t, database.LogSourceProvisionerDaemon, log.Source) - assert.Equal(t, unhanger.HungJobLogMessages[i], log.Output) + assert.Equal(t, expectedLogs[i], log.Output) } // Double check the full log count. @@ -755,7 +963,7 @@ func TestDetectorPushesLogs(t *testing.T) { CreatedAfter: 0, }) require.NoError(t, err) - require.Len(t, logs, c.preLogCount+len(unhanger.HungJobLogMessages)) + require.Len(t, logs, c.preLogCount+len(expectedLogs)) detector.Close() detector.Wait() @@ -771,15 +979,15 @@ func TestDetectorMaxJobsPerRun(t *testing.T) { db, pubsub = dbtestutil.NewDB(t) log = testutil.Logger(t) tickCh = make(chan time.Time) - statsCh = make(chan unhanger.Stats) + statsCh = make(chan jobreaper.Stats) org = dbgen.Organization(t, db, database.Organization{}) user = dbgen.User(t, db, database.User{}) file = dbgen.File(t, db, database.File{}) ) - // Create unhanger.MaxJobsPerRun + 1 hung jobs. + // Create MaxJobsPerRun + 1 hung jobs. now := time.Now() - for i := 0; i < unhanger.MaxJobsPerRun+1; i++ { + for i := 0; i < jobreaper.MaxJobsPerRun+1; i++ { pj := dbgen.ProvisionerJob(t, db, pubsub, database.ProvisionerJob{ CreatedAt: now.Add(-time.Hour), UpdatedAt: now.Add(-time.Hour), @@ -802,14 +1010,14 @@ func TestDetectorMaxJobsPerRun(t *testing.T) { }) } - detector := unhanger.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) + detector := jobreaper.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) detector.Start() tickCh <- now - // Make sure that only unhanger.MaxJobsPerRun jobs are terminated. + // Make sure that only MaxJobsPerRun jobs are terminated. stats := <-statsCh require.NoError(t, stats.Error) - require.Len(t, stats.TerminatedJobIDs, unhanger.MaxJobsPerRun) + require.Len(t, stats.TerminatedJobIDs, jobreaper.MaxJobsPerRun) // Run the detector again and make sure that only the remaining job is // terminated. @@ -823,7 +1031,7 @@ func TestDetectorMaxJobsPerRun(t *testing.T) { } // wrapDBAuthz adds our Authorization/RBAC around the given database store, to -// ensure the unhanger has the right permissions to do its work. +// ensure the reaper has the right permissions to do its work. func wrapDBAuthz(db database.Store, logger slog.Logger) database.Store { return dbauthz.New( db, diff --git a/coderd/oauthpki/okidcpki_test.go b/coderd/oauthpki/okidcpki_test.go index 509da563a9145..7f7dda17bcba8 100644 --- a/coderd/oauthpki/okidcpki_test.go +++ b/coderd/oauthpki/okidcpki_test.go @@ -144,6 +144,7 @@ func TestAzureAKPKIWithCoderd(t *testing.T) { return values, nil }), oidctest.WithServing(), + oidctest.WithLogging(t, nil), ) cfg := fake.OIDCConfig(t, scopes, func(cfg *coderd.OIDCConfig) { cfg.AllowSignups = true diff --git a/coderd/parameters.go b/coderd/parameters.go index c3fc4ffdeeede..1a0c1f92ddbf9 100644 --- a/coderd/parameters.go +++ b/coderd/parameters.go @@ -12,13 +12,14 @@ import ( "golang.org/x/sync/errgroup" "golang.org/x/xerrors" - "github.com/coder/coder/v2/apiversion" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/files" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/util/ptr" + "github.com/coder/coder/v2/coderd/wsbuilder" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/wsjson" sdkproto "github.com/coder/coder/v2/provisionersdk/proto" @@ -69,13 +70,10 @@ func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http return } - major, minor, err := apiversion.Parse(tf.ProvisionerdVersion) - // If the api version is not valid or less than 1.5, we need to use the static parameters - useStaticParams := err != nil || major < 1 || (major == 1 && minor < 6) - if useStaticParams { - api.handleStaticParameters(rw, r, templateVersion.ID) - } else { + if wsbuilder.ProvisionerVersionSupportsDynamicParameters(tf.ProvisionerdVersion) { api.handleDynamicParameters(rw, r, tf, templateVersion) + } else { + api.handleStaticParameters(rw, r, templateVersion.ID) } } @@ -289,10 +287,10 @@ func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request result, diagnostics := render(ctx, map[string]string{}) response := codersdk.DynamicParametersResponse{ ID: -1, // Always start with -1. - Diagnostics: previewtypes.Diagnostics(diagnostics), + Diagnostics: db2sdk.HCLDiagnostics(diagnostics), } if result != nil { - response.Parameters = result.Parameters + response.Parameters = db2sdk.List(result.Parameters, db2sdk.PreviewParameter) } err = stream.Send(response) if err != nil { @@ -317,10 +315,10 @@ func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request result, diagnostics := render(ctx, update.Inputs) response := codersdk.DynamicParametersResponse{ ID: update.ID, - Diagnostics: previewtypes.Diagnostics(diagnostics), + Diagnostics: db2sdk.HCLDiagnostics(diagnostics), } if result != nil { - response.Parameters = result.Parameters + response.Parameters = db2sdk.List(result.Parameters, db2sdk.PreviewParameter) } err = stream.Send(response) if err != nil { diff --git a/coderd/parameters_test.go b/coderd/parameters_test.go index e7fc77f141efc..8edadc9b7e797 100644 --- a/coderd/parameters_test.go +++ b/coderd/parameters_test.go @@ -68,8 +68,8 @@ func TestDynamicParametersOwnerSSHPublicKey(t *testing.T) { require.Equal(t, -1, preview.ID) require.Empty(t, preview.Diagnostics) require.Equal(t, "public_key", preview.Parameters[0].Name) - require.True(t, preview.Parameters[0].Value.Valid()) - require.Equal(t, sshKey.PublicKey, preview.Parameters[0].Value.Value.AsString()) + require.True(t, preview.Parameters[0].Value.Valid) + require.Equal(t, sshKey.PublicKey, preview.Parameters[0].Value.Value) } func TestDynamicParametersWithTerraformValues(t *testing.T) { @@ -103,8 +103,8 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) { require.Len(t, preview.Parameters, 1) require.Equal(t, "jetbrains_ide", preview.Parameters[0].Name) - require.True(t, preview.Parameters[0].Value.Valid()) - require.Equal(t, "CL", preview.Parameters[0].Value.AsString()) + require.True(t, preview.Parameters[0].Value.Valid) + require.Equal(t, "CL", preview.Parameters[0].Value.Value) }) // OldProvisioners use the static parameters in the dynamic param flow @@ -154,8 +154,8 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) { require.Contains(t, preview.Diagnostics[0].Summary, "required metadata to support dynamic parameters") require.Len(t, preview.Parameters, 1) require.Equal(t, "jetbrains_ide", preview.Parameters[0].Name) - require.True(t, preview.Parameters[0].Value.Valid()) - require.Equal(t, defaultValue, preview.Parameters[0].Value.AsString()) + require.True(t, preview.Parameters[0].Value.Valid) + require.Equal(t, defaultValue, preview.Parameters[0].Value.Value) // Test some inputs for _, exp := range []string{defaultValue, "GO", "Invalid", defaultValue} { @@ -182,8 +182,8 @@ func TestDynamicParametersWithTerraformValues(t *testing.T) { require.Len(t, preview.Parameters[0].Diagnostics, 0) } require.Equal(t, "jetbrains_ide", preview.Parameters[0].Name) - require.True(t, preview.Parameters[0].Value.Valid()) - require.Equal(t, exp, preview.Parameters[0].Value.AsString()) + require.True(t, preview.Parameters[0].Value.Valid) + require.Equal(t, exp, preview.Parameters[0].Value.Value) } }) diff --git a/coderd/provisionerjobs.go b/coderd/provisionerjobs.go index 6d75227a14ccd..5a8a0a5126cc0 100644 --- a/coderd/provisionerjobs.go +++ b/coderd/provisionerjobs.go @@ -395,6 +395,7 @@ func convertProvisionerJobWithQueuePosition(pj database.GetProvisionerJobsByOrga QueuePosition: pj.QueuePosition, QueueSize: pj.QueueSize, }) + job.WorkerName = pj.WorkerName job.AvailableWorkers = pj.AvailableWorkers job.Metadata = codersdk.ProvisionerJobMetadata{ TemplateVersionName: pj.TemplateVersionName, diff --git a/coderd/provisionerjobs_test.go b/coderd/provisionerjobs_test.go index 6ec8959102fa5..98da3ae5584e6 100644 --- a/coderd/provisionerjobs_test.go +++ b/coderd/provisionerjobs_test.go @@ -27,162 +27,263 @@ import ( func TestProvisionerJobs(t *testing.T) { t.Parallel() - db, ps := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure()) - client := coderdtest.New(t, &coderdtest.Options{ - IncludeProvisionerDaemon: true, - Database: db, - Pubsub: ps, - }) - owner := coderdtest.CreateFirstUser(t, client) - templateAdminClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.ScopedRoleOrgTemplateAdmin(owner.OrganizationID)) - memberClient, member := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) - - version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil) - coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) - template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID) - - time.Sleep(1500 * time.Millisecond) // Ensure the workspace build job has a different timestamp for sorting. - workspace := coderdtest.CreateWorkspace(t, client, template.ID) - coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID) - - // Create a pending job. - w := dbgen.Workspace(t, db, database.WorkspaceTable{ - OrganizationID: owner.OrganizationID, - OwnerID: member.ID, - TemplateID: template.ID, - }) - wbID := uuid.New() - job := dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{ - OrganizationID: w.OrganizationID, - StartedAt: sql.NullTime{Time: dbtime.Now(), Valid: true}, - Type: database.ProvisionerJobTypeWorkspaceBuild, - Input: json.RawMessage(`{"workspace_build_id":"` + wbID.String() + `"}`), - }) - dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ - ID: wbID, - JobID: job.ID, - WorkspaceID: w.ID, - TemplateVersionID: version.ID, - }) + t.Run("ProvisionerJobs", func(t *testing.T) { + db, ps := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure()) + client := coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + Database: db, + Pubsub: ps, + }) + owner := coderdtest.CreateFirstUser(t, client) + templateAdminClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.ScopedRoleOrgTemplateAdmin(owner.OrganizationID)) + memberClient, member := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) + + version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID) + + time.Sleep(1500 * time.Millisecond) // Ensure the workspace build job has a different timestamp for sorting. + workspace := coderdtest.CreateWorkspace(t, client, template.ID) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID) - // Add more jobs than the default limit. - for i := range 60 { - dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{ + // Create a pending job. + w := dbgen.Workspace(t, db, database.WorkspaceTable{ OrganizationID: owner.OrganizationID, - Tags: database.StringMap{"count": strconv.Itoa(i)}, + OwnerID: member.ID, + TemplateID: template.ID, + }) + wbID := uuid.New() + job := dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{ + OrganizationID: w.OrganizationID, + StartedAt: sql.NullTime{Time: dbtime.Now(), Valid: true}, + Type: database.ProvisionerJobTypeWorkspaceBuild, + Input: json.RawMessage(`{"workspace_build_id":"` + wbID.String() + `"}`), + }) + dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ + ID: wbID, + JobID: job.ID, + WorkspaceID: w.ID, + TemplateVersionID: version.ID, }) - } - t.Run("Single", func(t *testing.T) { - t.Parallel() - t.Run("Workspace", func(t *testing.T) { + // Add more jobs than the default limit. + for i := range 60 { + dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{ + OrganizationID: owner.OrganizationID, + Tags: database.StringMap{"count": strconv.Itoa(i)}, + }) + } + + t.Run("Single", func(t *testing.T) { t.Parallel() - t.Run("OK", func(t *testing.T) { + t.Run("Workspace", func(t *testing.T) { t.Parallel() - ctx := testutil.Context(t, testutil.WaitMedium) - // Note this calls the single job endpoint. - job2, err := templateAdminClient.OrganizationProvisionerJob(ctx, owner.OrganizationID, job.ID) - require.NoError(t, err) - require.Equal(t, job.ID, job2.ID) - - // Verify that job metadata is correct. - assert.Equal(t, job2.Metadata, codersdk.ProvisionerJobMetadata{ - TemplateVersionName: version.Name, - TemplateID: template.ID, - TemplateName: template.Name, - TemplateDisplayName: template.DisplayName, - TemplateIcon: template.Icon, - WorkspaceID: &w.ID, - WorkspaceName: w.Name, + t.Run("OK", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + // Note this calls the single job endpoint. + job2, err := templateAdminClient.OrganizationProvisionerJob(ctx, owner.OrganizationID, job.ID) + require.NoError(t, err) + require.Equal(t, job.ID, job2.ID) + + // Verify that job metadata is correct. + assert.Equal(t, job2.Metadata, codersdk.ProvisionerJobMetadata{ + TemplateVersionName: version.Name, + TemplateID: template.ID, + TemplateName: template.Name, + TemplateDisplayName: template.DisplayName, + TemplateIcon: template.Icon, + WorkspaceID: &w.ID, + WorkspaceName: w.Name, + }) }) }) - }) - t.Run("Template Import", func(t *testing.T) { - t.Parallel() - t.Run("OK", func(t *testing.T) { + t.Run("Template Import", func(t *testing.T) { + t.Parallel() + t.Run("OK", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + // Note this calls the single job endpoint. + job2, err := templateAdminClient.OrganizationProvisionerJob(ctx, owner.OrganizationID, version.Job.ID) + require.NoError(t, err) + require.Equal(t, version.Job.ID, job2.ID) + + // Verify that job metadata is correct. + assert.Equal(t, job2.Metadata, codersdk.ProvisionerJobMetadata{ + TemplateVersionName: version.Name, + TemplateID: template.ID, + TemplateName: template.Name, + TemplateDisplayName: template.DisplayName, + TemplateIcon: template.Icon, + }) + }) + }) + t.Run("Missing", func(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitMedium) // Note this calls the single job endpoint. - job2, err := templateAdminClient.OrganizationProvisionerJob(ctx, owner.OrganizationID, version.Job.ID) - require.NoError(t, err) - require.Equal(t, version.Job.ID, job2.ID) - - // Verify that job metadata is correct. - assert.Equal(t, job2.Metadata, codersdk.ProvisionerJobMetadata{ - TemplateVersionName: version.Name, - TemplateID: template.ID, - TemplateName: template.Name, - TemplateDisplayName: template.DisplayName, - TemplateIcon: template.Icon, - }) + _, err := templateAdminClient.OrganizationProvisionerJob(ctx, owner.OrganizationID, uuid.New()) + require.Error(t, err) }) }) - t.Run("Missing", func(t *testing.T) { + + t.Run("Default limit", func(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitMedium) - // Note this calls the single job endpoint. - _, err := templateAdminClient.OrganizationProvisionerJob(ctx, owner.OrganizationID, uuid.New()) - require.Error(t, err) + jobs, err := templateAdminClient.OrganizationProvisionerJobs(ctx, owner.OrganizationID, nil) + require.NoError(t, err) + require.Len(t, jobs, 50) }) - }) - t.Run("Default limit", func(t *testing.T) { - t.Parallel() - ctx := testutil.Context(t, testutil.WaitMedium) - jobs, err := templateAdminClient.OrganizationProvisionerJobs(ctx, owner.OrganizationID, nil) - require.NoError(t, err) - require.Len(t, jobs, 50) - }) + t.Run("IDs", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + jobs, err := templateAdminClient.OrganizationProvisionerJobs(ctx, owner.OrganizationID, &codersdk.OrganizationProvisionerJobsOptions{ + IDs: []uuid.UUID{workspace.LatestBuild.Job.ID, version.Job.ID}, + }) + require.NoError(t, err) + require.Len(t, jobs, 2) + }) - t.Run("IDs", func(t *testing.T) { - t.Parallel() - ctx := testutil.Context(t, testutil.WaitMedium) - jobs, err := templateAdminClient.OrganizationProvisionerJobs(ctx, owner.OrganizationID, &codersdk.OrganizationProvisionerJobsOptions{ - IDs: []uuid.UUID{workspace.LatestBuild.Job.ID, version.Job.ID}, + t.Run("Status", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + jobs, err := templateAdminClient.OrganizationProvisionerJobs(ctx, owner.OrganizationID, &codersdk.OrganizationProvisionerJobsOptions{ + Status: []codersdk.ProvisionerJobStatus{codersdk.ProvisionerJobRunning}, + }) + require.NoError(t, err) + require.Len(t, jobs, 1) }) - require.NoError(t, err) - require.Len(t, jobs, 2) - }) - t.Run("Status", func(t *testing.T) { - t.Parallel() - ctx := testutil.Context(t, testutil.WaitMedium) - jobs, err := templateAdminClient.OrganizationProvisionerJobs(ctx, owner.OrganizationID, &codersdk.OrganizationProvisionerJobsOptions{ - Status: []codersdk.ProvisionerJobStatus{codersdk.ProvisionerJobRunning}, + t.Run("Tags", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + jobs, err := templateAdminClient.OrganizationProvisionerJobs(ctx, owner.OrganizationID, &codersdk.OrganizationProvisionerJobsOptions{ + Tags: map[string]string{"count": "1"}, + }) + require.NoError(t, err) + require.Len(t, jobs, 1) }) - require.NoError(t, err) - require.Len(t, jobs, 1) - }) - t.Run("Tags", func(t *testing.T) { - t.Parallel() - ctx := testutil.Context(t, testutil.WaitMedium) - jobs, err := templateAdminClient.OrganizationProvisionerJobs(ctx, owner.OrganizationID, &codersdk.OrganizationProvisionerJobsOptions{ - Tags: map[string]string{"count": "1"}, + t.Run("Limit", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + jobs, err := templateAdminClient.OrganizationProvisionerJobs(ctx, owner.OrganizationID, &codersdk.OrganizationProvisionerJobsOptions{ + Limit: 1, + }) + require.NoError(t, err) + require.Len(t, jobs, 1) + }) + + // For now, this is not allowed even though the member has created a + // workspace. Once member-level permissions for jobs are supported + // by RBAC, this test should be updated. + t.Run("MemberDenied", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitMedium) + jobs, err := memberClient.OrganizationProvisionerJobs(ctx, owner.OrganizationID, nil) + require.Error(t, err) + require.Len(t, jobs, 0) }) - require.NoError(t, err) - require.Len(t, jobs, 1) }) - t.Run("Limit", func(t *testing.T) { + // Ensures that when a provisioner job is in the succeeded state, + // the API response includes both worker_id and worker_name fields + t.Run("AssignedProvisionerJob", func(t *testing.T) { t.Parallel() - ctx := testutil.Context(t, testutil.WaitMedium) - jobs, err := templateAdminClient.OrganizationProvisionerJobs(ctx, owner.OrganizationID, &codersdk.OrganizationProvisionerJobsOptions{ - Limit: 1, + + db, ps := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure()) + client, _, coderdAPI := coderdtest.NewWithAPI(t, &coderdtest.Options{ + IncludeProvisionerDaemon: false, + Database: db, + Pubsub: ps, }) + provisionerDaemonName := "provisioner_daemon_test" + provisionerDaemon := coderdtest.NewTaggedProvisionerDaemon(t, coderdAPI, provisionerDaemonName, map[string]string{"owner": "", "scope": "organization"}) + owner := coderdtest.CreateFirstUser(t, client) + templateAdminClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.ScopedRoleOrgTemplateAdmin(owner.OrganizationID)) + + version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID) + + workspace := coderdtest.CreateWorkspace(t, client, template.ID) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID) + + // Stop the provisioner so it doesn't grab any more jobs + err := provisionerDaemon.Close() require.NoError(t, err) - require.Len(t, jobs, 1) - }) - // For now, this is not allowed even though the member has created a - // workspace. Once member-level permissions for jobs are supported - // by RBAC, this test should be updated. - t.Run("MemberDenied", func(t *testing.T) { - t.Parallel() - ctx := testutil.Context(t, testutil.WaitMedium) - jobs, err := memberClient.OrganizationProvisionerJobs(ctx, owner.OrganizationID, nil) - require.Error(t, err) - require.Len(t, jobs, 0) + t.Run("List_IncludesWorkerIDAndName", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + + // Get provisioner daemon responsible for executing the provisioner jobs + provisionerDaemons, err := db.GetProvisionerDaemons(ctx) + require.NoError(t, err) + require.Equal(t, 1, len(provisionerDaemons)) + if assert.NotEmpty(t, provisionerDaemons) { + require.Equal(t, provisionerDaemonName, provisionerDaemons[0].Name) + } + + // Get provisioner jobs + jobs, err := templateAdminClient.OrganizationProvisionerJobs(ctx, owner.OrganizationID, nil) + require.NoError(t, err) + require.Equal(t, 2, len(jobs)) + + for _, job := range jobs { + require.Equal(t, owner.OrganizationID, job.OrganizationID) + require.Equal(t, database.ProvisionerJobStatusSucceeded, database.ProvisionerJobStatus(job.Status)) + + // Guarantee that provisioner jobs contain the provisioner daemon ID and name + if assert.NotEmpty(t, provisionerDaemons) { + require.Equal(t, &provisionerDaemons[0].ID, job.WorkerID) + require.Equal(t, provisionerDaemonName, job.WorkerName) + } + } + }) + + t.Run("Get_IncludesWorkerIDAndName", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + + // Get provisioner daemon responsible for executing the provisioner job + provisionerDaemons, err := db.GetProvisionerDaemons(ctx) + require.NoError(t, err) + require.Equal(t, 1, len(provisionerDaemons)) + if assert.NotEmpty(t, provisionerDaemons) { + require.Equal(t, provisionerDaemonName, provisionerDaemons[0].Name) + } + + // Get all provisioner jobs + jobs, err := templateAdminClient.OrganizationProvisionerJobs(ctx, owner.OrganizationID, nil) + require.NoError(t, err) + require.Equal(t, 2, len(jobs)) + + // Find workspace_build provisioner job ID + var workspaceProvisionerJobID uuid.UUID + for _, job := range jobs { + if job.Type == codersdk.ProvisionerJobTypeWorkspaceBuild { + workspaceProvisionerJobID = job.ID + } + } + require.NotNil(t, workspaceProvisionerJobID) + + // Get workspace_build provisioner job by ID + workspaceProvisionerJob, err := templateAdminClient.OrganizationProvisionerJob(ctx, owner.OrganizationID, workspaceProvisionerJobID) + require.NoError(t, err) + + require.Equal(t, owner.OrganizationID, workspaceProvisionerJob.OrganizationID) + require.Equal(t, database.ProvisionerJobStatusSucceeded, database.ProvisionerJobStatus(workspaceProvisionerJob.Status)) + + // Guarantee that provisioner job contains the provisioner daemon ID and name + if assert.NotEmpty(t, provisionerDaemons) { + require.Equal(t, &provisionerDaemons[0].ID, workspaceProvisionerJob.WorkerID) + require.Equal(t, provisionerDaemonName, workspaceProvisionerJob.WorkerName) + } + }) }) } diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go index d2c6d5d0675be..c63042a2a1363 100644 --- a/coderd/rbac/authz.go +++ b/coderd/rbac/authz.go @@ -65,7 +65,7 @@ const ( SubjectTypeUser SubjectType = "user" SubjectTypeProvisionerd SubjectType = "provisionerd" SubjectTypeAutostart SubjectType = "autostart" - SubjectTypeHangDetector SubjectType = "hang_detector" + SubjectTypeJobReaper SubjectType = "job_reaper" SubjectTypeResourceMonitor SubjectType = "resource_monitor" SubjectTypeCryptoKeyRotator SubjectType = "crypto_key_rotator" SubjectTypeCryptoKeyReader SubjectType = "crypto_key_reader" diff --git a/coderd/rbac/object_gen.go b/coderd/rbac/object_gen.go index 40b7dc87a56f8..f19d90894dd55 100644 --- a/coderd/rbac/object_gen.go +++ b/coderd/rbac/object_gen.go @@ -234,7 +234,9 @@ var ( // ResourceProvisionerJobs // Valid Actions + // - "ActionCreate" :: create provisioner jobs // - "ActionRead" :: read provisioner jobs + // - "ActionUpdate" :: update provisioner jobs ResourceProvisionerJobs = Object{ Type: "provisioner_jobs", } @@ -306,7 +308,9 @@ var ( // Valid Actions // - "ActionApplicationConnect" :: connect to workspace apps via browser // - "ActionCreate" :: create a new workspace + // - "ActionCreateAgent" :: create a new workspace agent // - "ActionDelete" :: delete workspace + // - "ActionDeleteAgent" :: delete an existing workspace agent // - "ActionRead" :: read workspace data to view on the UI // - "ActionSSH" :: ssh into a given workspace // - "ActionWorkspaceStart" :: allows starting a workspace @@ -336,7 +340,9 @@ var ( // Valid Actions // - "ActionApplicationConnect" :: connect to workspace apps via browser // - "ActionCreate" :: create a new workspace + // - "ActionCreateAgent" :: create a new workspace agent // - "ActionDelete" :: delete workspace + // - "ActionDeleteAgent" :: delete an existing workspace agent // - "ActionRead" :: read workspace data to view on the UI // - "ActionSSH" :: ssh into a given workspace // - "ActionWorkspaceStart" :: allows starting a workspace @@ -404,7 +410,9 @@ func AllActions() []policy.Action { policy.ActionApplicationConnect, policy.ActionAssign, policy.ActionCreate, + policy.ActionCreateAgent, policy.ActionDelete, + policy.ActionDeleteAgent, policy.ActionRead, policy.ActionReadPersonal, policy.ActionSSH, diff --git a/coderd/rbac/policy/policy.go b/coderd/rbac/policy/policy.go index 35da0892abfdb..160062283f857 100644 --- a/coderd/rbac/policy/policy.go +++ b/coderd/rbac/policy/policy.go @@ -24,6 +24,9 @@ const ( ActionReadPersonal Action = "read_personal" ActionUpdatePersonal Action = "update_personal" + + ActionCreateAgent Action = "create_agent" + ActionDeleteAgent Action = "delete_agent" ) type PermissionDefinition struct { @@ -67,6 +70,9 @@ var workspaceActions = map[Action]ActionDefinition{ // Running a workspace ActionSSH: actDef("ssh into a given workspace"), ActionApplicationConnect: actDef("connect to workspace apps via browser"), + + ActionCreateAgent: actDef("create a new workspace agent"), + ActionDeleteAgent: actDef("delete an existing workspace agent"), } // RBACPermissions is indexed by the type @@ -182,7 +188,9 @@ var RBACPermissions = map[string]PermissionDefinition{ }, "provisioner_jobs": { Actions: map[Action]ActionDefinition{ - ActionRead: actDef("read provisioner jobs"), + ActionRead: actDef("read provisioner jobs"), + ActionUpdate: actDef("update provisioner jobs"), + ActionCreate: actDef("create provisioner jobs"), }, }, "organization": { diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 56124faee44e2..89f86b567a48d 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -272,7 +272,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // This adds back in the Workspace permissions. Permissions(map[string][]policy.Action{ ResourceWorkspace.Type: ownerWorkspaceActions, - ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate, policy.ActionWorkspaceStop}, + ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate, policy.ActionWorkspaceStop, policy.ActionCreateAgent, policy.ActionDeleteAgent}, })...), Org: map[string][]Permission{}, User: []Permission{}, @@ -291,7 +291,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { User: append(allPermsExcept(ResourceWorkspaceDormant, ResourceUser, ResourceOrganizationMember), Permissions(map[string][]policy.Action{ // Reduced permission set on dormant workspaces. No build, ssh, or exec - ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate, policy.ActionWorkspaceStop}, + ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate, policy.ActionWorkspaceStop, policy.ActionCreateAgent, policy.ActionDeleteAgent}, // Users cannot do create/update/delete on themselves, but they // can read their own details. ResourceUser.Type: {policy.ActionRead, policy.ActionReadPersonal, policy.ActionUpdatePersonal}, @@ -412,7 +412,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { Org: map[string][]Permission{ // Org admins should not have workspace exec perms. organizationID.String(): append(allPermsExcept(ResourceWorkspace, ResourceWorkspaceDormant, ResourceAssignRole), Permissions(map[string][]policy.Action{ - ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate, policy.ActionWorkspaceStop}, + ResourceWorkspaceDormant.Type: {policy.ActionRead, policy.ActionDelete, policy.ActionCreate, policy.ActionUpdate, policy.ActionWorkspaceStop, policy.ActionCreateAgent, policy.ActionDeleteAgent}, ResourceWorkspace.Type: slice.Omit(ResourceWorkspace.AvailableActions(), policy.ActionApplicationConnect, policy.ActionSSH), })...), }, @@ -503,7 +503,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // the ability to create templates and provisioners has // a lot of overlap. ResourceProvisionerDaemon.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, - ResourceProvisionerJobs.Type: {policy.ActionRead}, + ResourceProvisionerJobs.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionCreate}, }), }, User: []Permission{}, @@ -529,6 +529,16 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceType: ResourceWorkspace.Type, Action: policy.ActionDelete, }, + { + Negate: true, + ResourceType: ResourceWorkspace.Type, + Action: policy.ActionCreateAgent, + }, + { + Negate: true, + ResourceType: ResourceWorkspace.Type, + Action: policy.ActionDeleteAgent, + }, }, }, User: []Permission{}, diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index e90c89914fdec..4dfbc8fa2ab31 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -226,6 +226,15 @@ func TestRolePermissions(t *testing.T) { false: {setOtherOrg, setOrgNotMe, memberMe, templateAdmin, userAdmin}, }, }, + { + Name: "CreateDeleteWorkspaceAgent", + Actions: []policy.Action{policy.ActionCreateAgent, policy.ActionDeleteAgent}, + Resource: rbac.ResourceWorkspace.WithID(workspaceID).InOrg(orgID).WithOwner(currentUser.String()), + AuthorizeMap: map[bool][]hasAuthSubjects{ + true: {owner, orgMemberMe, orgAdmin}, + false: {setOtherOrg, memberMe, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin, orgAuditor, orgMemberMeBanWorkspace}, + }, + }, { Name: "Templates", Actions: []policy.Action{policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete}, @@ -462,7 +471,7 @@ func TestRolePermissions(t *testing.T) { }, { Name: "WorkspaceDormant", - Actions: append(crud, policy.ActionWorkspaceStop), + Actions: append(crud, policy.ActionWorkspaceStop, policy.ActionCreateAgent, policy.ActionDeleteAgent), Resource: rbac.ResourceWorkspaceDormant.WithID(uuid.New()).InOrg(orgID).WithOwner(memberMe.Actor.ID), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {orgMemberMe, orgAdmin, owner}, @@ -580,7 +589,7 @@ func TestRolePermissions(t *testing.T) { }, { Name: "ProvisionerJobs", - Actions: []policy.Action{policy.ActionRead}, + Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate, policy.ActionCreate}, Resource: rbac.ResourceProvisionerJobs.InOrg(orgID), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgTemplateAdmin, orgAdmin}, diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index 72a03580121af..8b94566e75715 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -15,6 +15,7 @@ import ( "strings" "time" + "github.com/go-chi/chi/v5" "github.com/google/uuid" "github.com/sqlc-dev/pqtype" "golang.org/x/exp/maps" @@ -893,6 +894,91 @@ func (api *API) workspaceAgentListContainers(rw http.ResponseWriter, r *http.Req httpapi.Write(ctx, rw, http.StatusOK, cts) } +// @Summary Recreate devcontainer for workspace agent +// @ID recreate-devcontainer-for-workspace-agent +// @Security CoderSessionToken +// @Tags Agents +// @Param workspaceagent path string true "Workspace agent ID" format(uuid) +// @Param container path string true "Container ID or name" +// @Success 204 +// @Router /workspaceagents/{workspaceagent}/containers/devcontainers/container/{container}/recreate [post] +func (api *API) workspaceAgentRecreateDevcontainer(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + workspaceAgent := httpmw.WorkspaceAgentParam(r) + + container := chi.URLParam(r, "container") + if container == "" { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Container ID or name is required.", + Validations: []codersdk.ValidationError{ + {Field: "container", Detail: "Container ID or name is required."}, + }, + }) + return + } + + apiAgent, err := db2sdk.WorkspaceAgent( + api.DERPMap(), + *api.TailnetCoordinator.Load(), + workspaceAgent, + nil, + nil, + nil, + api.AgentInactiveDisconnectTimeout, + api.DeploymentValues.AgentFallbackTroubleshootingURL.String(), + ) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error reading workspace agent.", + Detail: err.Error(), + }) + return + } + if apiAgent.Status != codersdk.WorkspaceAgentConnected { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: fmt.Sprintf("Agent state is %q, it must be in the %q state.", apiAgent.Status, codersdk.WorkspaceAgentConnected), + }) + return + } + + // If the agent is unreachable, the request will hang. Assume that if we + // don't get a response after 30s that the agent is unreachable. + dialCtx, dialCancel := context.WithTimeout(ctx, 30*time.Second) + defer dialCancel() + agentConn, release, err := api.agentProvider.AgentConn(dialCtx, workspaceAgent.ID) + if err != nil { + httpapi.Write(dialCtx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error dialing workspace agent.", + Detail: err.Error(), + }) + return + } + defer release() + + err = agentConn.RecreateDevcontainer(ctx, container) + if err != nil { + if errors.Is(err, context.Canceled) { + httpapi.Write(ctx, rw, http.StatusRequestTimeout, codersdk.Response{ + Message: "Failed to recreate devcontainer from agent.", + Detail: "Request timed out.", + }) + return + } + // If the agent returns a codersdk.Error, we can return that directly. + if cerr, ok := codersdk.AsError(err); ok { + httpapi.Write(ctx, rw, cerr.StatusCode(), cerr.Response) + return + } + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error recreating devcontainer.", + Detail: err.Error(), + }) + return + } + + httpapi.Write(ctx, rw, http.StatusNoContent, nil) +} + // @Summary Get connection info for workspace agent // @ID get-connection-info-for-workspace-agent // @Security CoderSessionToken diff --git a/coderd/workspaceagents_test.go b/coderd/workspaceagents_test.go index 10403f1ac00ae..f4f3dcdec9f89 100644 --- a/coderd/workspaceagents_test.go +++ b/coderd/workspaceagents_test.go @@ -8,6 +8,7 @@ import ( "net" "net/http" "os" + "path/filepath" "runtime" "strconv" "strings" @@ -36,6 +37,7 @@ import ( "github.com/coder/coder/v2/agent" "github.com/coder/coder/v2/agent/agentcontainers" "github.com/coder/coder/v2/agent/agentcontainers/acmock" + "github.com/coder/coder/v2/agent/agentcontainers/watcher" "github.com/coder/coder/v2/agent/agenttest" agentproto "github.com/coder/coder/v2/agent/proto" "github.com/coder/coder/v2/coderd/coderdtest" @@ -437,25 +439,55 @@ func TestWorkspaceAgentConnectRPC(t *testing.T) { t.Run("Connect", func(t *testing.T) { t.Parallel() - client, db := coderdtest.NewWithDatabase(t, nil) - user := coderdtest.CreateFirstUser(t, client) - r := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ - OrganizationID: user.OrganizationID, - OwnerID: user.UserID, - }).WithAgent().Do() - _ = agenttest.New(t, client.URL, r.AgentToken) - resources := coderdtest.AwaitWorkspaceAgents(t, client, r.Workspace.ID) + for _, tc := range []struct { + name string + apiKeyScope rbac.ScopeName + }{ + { + name: "empty (backwards compat)", + apiKeyScope: "", + }, + { + name: "all", + apiKeyScope: rbac.ScopeAll, + }, + { + name: "no_user_data", + apiKeyScope: rbac.ScopeNoUserData, + }, + { + name: "application_connect", + apiKeyScope: rbac.ScopeApplicationConnect, + }, + } { + t.Run(tc.name, func(t *testing.T) { + client, db := coderdtest.NewWithDatabase(t, nil) + user := coderdtest.CreateFirstUser(t, client) + r := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OrganizationID: user.OrganizationID, + OwnerID: user.UserID, + }).WithAgent(func(agents []*proto.Agent) []*proto.Agent { + for _, agent := range agents { + agent.ApiKeyScope = string(tc.apiKeyScope) + } - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() + return agents + }).Do() + _ = agenttest.New(t, client.URL, r.AgentToken) + resources := coderdtest.NewWorkspaceAgentWaiter(t, client, r.Workspace.ID).AgentNames([]string{}).Wait() - conn, err := workspacesdk.New(client). - DialAgent(ctx, resources[0].Agents[0].ID, nil) - require.NoError(t, err) - defer func() { - _ = conn.Close() - }() - conn.AwaitReachable(ctx) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + conn, err := workspacesdk.New(client). + DialAgent(ctx, resources[0].Agents[0].ID, nil) + require.NoError(t, err) + defer func() { + _ = conn.Close() + }() + conn.AwaitReachable(ctx) + }) + } }) t.Run("FailNonLatestBuild", func(t *testing.T) { @@ -1347,6 +1379,115 @@ func TestWorkspaceAgentContainers(t *testing.T) { }) } +func TestWorkspaceAgentRecreateDevcontainer(t *testing.T) { + t.Parallel() + + t.Run("Mock", func(t *testing.T) { + t.Parallel() + + var ( + workspaceFolder = t.TempDir() + configFile = filepath.Join(workspaceFolder, ".devcontainer", "devcontainer.json") + dcLabels = map[string]string{ + agentcontainers.DevcontainerLocalFolderLabel: workspaceFolder, + agentcontainers.DevcontainerConfigFileLabel: configFile, + } + devContainer = codersdk.WorkspaceAgentContainer{ + ID: uuid.NewString(), + CreatedAt: dbtime.Now(), + FriendlyName: testutil.GetRandomName(t), + Image: "busybox:latest", + Labels: dcLabels, + Running: true, + Status: "running", + DevcontainerDirty: true, + } + plainContainer = codersdk.WorkspaceAgentContainer{ + ID: uuid.NewString(), + CreatedAt: dbtime.Now(), + FriendlyName: testutil.GetRandomName(t), + Image: "busybox:latest", + Labels: map[string]string{}, + Running: true, + Status: "running", + } + ) + + for _, tc := range []struct { + name string + setupMock func(*acmock.MockLister, *acmock.MockDevcontainerCLI) (status int) + }{ + { + name: "Recreate", + setupMock: func(mcl *acmock.MockLister, mdccli *acmock.MockDevcontainerCLI) int { + mcl.EXPECT().List(gomock.Any()).Return(codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{devContainer}, + }, nil).Times(1) + mdccli.EXPECT().Up(gomock.Any(), workspaceFolder, configFile, gomock.Any()).Return("someid", nil).Times(1) + return 0 + }, + }, + { + name: "Container does not exist", + setupMock: func(mcl *acmock.MockLister, mdccli *acmock.MockDevcontainerCLI) int { + mcl.EXPECT().List(gomock.Any()).Return(codersdk.WorkspaceAgentListContainersResponse{}, nil).Times(1) + return http.StatusNotFound + }, + }, + { + name: "Not a devcontainer", + setupMock: func(mcl *acmock.MockLister, mdccli *acmock.MockDevcontainerCLI) int { + mcl.EXPECT().List(gomock.Any()).Return(codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{plainContainer}, + }, nil).Times(1) + return http.StatusNotFound + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + mcl := acmock.NewMockLister(ctrl) + mdccli := acmock.NewMockDevcontainerCLI(ctrl) + wantStatus := tc.setupMock(mcl, mdccli) + client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{}) + user := coderdtest.CreateFirstUser(t, client) + r := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OrganizationID: user.OrganizationID, + OwnerID: user.UserID, + }).WithAgent(func(agents []*proto.Agent) []*proto.Agent { + return agents + }).Do() + _ = agenttest.New(t, client.URL, r.AgentToken, func(o *agent.Options) { + o.ExperimentalDevcontainersEnabled = true + o.ContainerAPIOptions = append( + o.ContainerAPIOptions, + agentcontainers.WithLister(mcl), + agentcontainers.WithDevcontainerCLI(mdccli), + agentcontainers.WithWatcher(watcher.NewNoop()), + ) + }) + resources := coderdtest.NewWorkspaceAgentWaiter(t, client, r.Workspace.ID).Wait() + require.Len(t, resources, 1, "expected one resource") + require.Len(t, resources[0].Agents, 1, "expected one agent") + agentID := resources[0].Agents[0].ID + + ctx := testutil.Context(t, testutil.WaitLong) + + err := client.WorkspaceAgentRecreateDevcontainer(ctx, agentID, devContainer.ID) + if wantStatus > 0 { + cerr, ok := codersdk.AsError(err) + require.True(t, ok, "expected error to be a coder error") + assert.Equal(t, wantStatus, cerr.StatusCode()) + } else { + require.NoError(t, err, "failed to recreate devcontainer") + } + }) + } + }) +} + func TestWorkspaceAgentAppHealth(t *testing.T) { t.Parallel() client, db := coderdtest.NewWithDatabase(t, nil) @@ -2464,7 +2605,7 @@ func requireGetManifest(ctx context.Context, t testing.TB, aAPI agentproto.DRPCA } func postStartup(ctx context.Context, t testing.TB, client agent.Client, startup *agentproto.Startup) error { - aAPI, _, err := client.ConnectRPC24(ctx) + aAPI, _, err := client.ConnectRPC25(ctx) require.NoError(t, err) defer func() { cErr := aAPI.DRPCConn().Close() @@ -2650,8 +2791,8 @@ func TestReinit(t *testing.T) { db, ps := dbtestutil.NewDB(t) pubsubSpy := pubsubReinitSpy{ - Pubsub: ps, - subscribed: make(chan string), + Pubsub: ps, + triedToSubscribe: make(chan string), } client := coderdtest.New(t, &coderdtest.Options{ Database: db, @@ -2664,9 +2805,9 @@ func TestReinit(t *testing.T) { OwnerID: user.UserID, }).WithAgent().Do() - pubsubSpy.Mutex.Lock() + pubsubSpy.Lock() pubsubSpy.expectedEvent = agentsdk.PrebuildClaimedChannel(r.Workspace.ID) - pubsubSpy.Mutex.Unlock() + pubsubSpy.Unlock() agentCtx := testutil.Context(t, testutil.WaitShort) agentClient := agentsdk.New(client.URL) @@ -2681,7 +2822,7 @@ func TestReinit(t *testing.T) { // We need to subscribe before we publish, lest we miss the event ctx := testutil.Context(t, testutil.WaitShort) - testutil.TryReceive(ctx, t, pubsubSpy.subscribed) // Wait for the appropriate subscription + testutil.TryReceive(ctx, t, pubsubSpy.triedToSubscribe) // Now that we're subscribed, publish the event err := prebuilds.NewPubsubWorkspaceClaimPublisher(ps).PublishWorkspaceClaim(agentsdk.ReinitializationEvent{ @@ -2699,15 +2840,16 @@ func TestReinit(t *testing.T) { type pubsubReinitSpy struct { pubsub.Pubsub sync.Mutex - subscribed chan string - expectedEvent string + triedToSubscribe chan string + expectedEvent string } func (p *pubsubReinitSpy) Subscribe(event string, listener pubsub.Listener) (cancel func(), err error) { + cancel, err = p.Pubsub.Subscribe(event, listener) p.Lock() if p.expectedEvent != "" && event == p.expectedEvent { - close(p.subscribed) + close(p.triedToSubscribe) } p.Unlock() - return p.Pubsub.Subscribe(event, listener) + return cancel, err } diff --git a/coderd/workspaceagentsrpc.go b/coderd/workspaceagentsrpc.go index 43da35410f632..2dcf65bd8c7d5 100644 --- a/coderd/workspaceagentsrpc.go +++ b/coderd/workspaceagentsrpc.go @@ -76,17 +76,8 @@ func (api *API) workspaceAgentRPC(rw http.ResponseWriter, r *http.Request) { return } - owner, err := api.Database.GetUserByID(ctx, workspace.OwnerID) - if err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Internal error fetching user.", - Detail: err.Error(), - }) - return - } - logger = logger.With( - slog.F("owner", owner.Username), + slog.F("owner", workspace.OwnerUsername), slog.F("workspace_name", workspace.Name), slog.F("agent_name", workspaceAgent.Name), ) @@ -170,7 +161,7 @@ func (api *API) workspaceAgentRPC(rw http.ResponseWriter, r *http.Request) { }) streamID := tailnet.StreamID{ - Name: fmt.Sprintf("%s-%s-%s", owner.Username, workspace.Name, workspaceAgent.Name), + Name: fmt.Sprintf("%s-%s-%s", workspace.OwnerUsername, workspace.Name, workspaceAgent.Name), ID: workspaceAgent.ID, Auth: tailnet.AgentCoordinateeAuth{ID: workspaceAgent.ID}, } diff --git a/coderd/workspaceagentsrpc_test.go b/coderd/workspaceagentsrpc_test.go index caea9b39c2f54..5175f80b0b723 100644 --- a/coderd/workspaceagentsrpc_test.go +++ b/coderd/workspaceagentsrpc_test.go @@ -13,6 +13,7 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbfake" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/codersdk/agentsdk" "github.com/coder/coder/v2/provisionersdk/proto" "github.com/coder/coder/v2/testutil" @@ -22,89 +23,150 @@ import ( func TestWorkspaceAgentReportStats(t *testing.T) { t.Parallel() - tickCh := make(chan time.Time) - flushCh := make(chan int, 1) - client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{ - WorkspaceUsageTrackerFlush: flushCh, - WorkspaceUsageTrackerTick: tickCh, - }) - user := coderdtest.CreateFirstUser(t, client) - r := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ - OrganizationID: user.OrganizationID, - OwnerID: user.UserID, - LastUsedAt: dbtime.Now().Add(-time.Minute), - }).WithAgent().Do() + for _, tc := range []struct { + name string + apiKeyScope rbac.ScopeName + }{ + { + name: "empty (backwards compat)", + apiKeyScope: "", + }, + { + name: "all", + apiKeyScope: rbac.ScopeAll, + }, + { + name: "no_user_data", + apiKeyScope: rbac.ScopeNoUserData, + }, + { + name: "application_connect", + apiKeyScope: rbac.ScopeApplicationConnect, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() - ac := agentsdk.New(client.URL) - ac.SetSessionToken(r.AgentToken) - conn, err := ac.ConnectRPC(context.Background()) - require.NoError(t, err) - defer func() { - _ = conn.Close() - }() - agentAPI := agentproto.NewDRPCAgentClient(conn) + tickCh := make(chan time.Time) + flushCh := make(chan int, 1) + client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{ + WorkspaceUsageTrackerFlush: flushCh, + WorkspaceUsageTrackerTick: tickCh, + }) + user := coderdtest.CreateFirstUser(t, client) + r := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OrganizationID: user.OrganizationID, + OwnerID: user.UserID, + LastUsedAt: dbtime.Now().Add(-time.Minute), + }).WithAgent( + func(agent []*proto.Agent) []*proto.Agent { + for _, a := range agent { + a.ApiKeyScope = string(tc.apiKeyScope) + } - _, err = agentAPI.UpdateStats(context.Background(), &agentproto.UpdateStatsRequest{ - Stats: &agentproto.Stats{ - ConnectionsByProto: map[string]int64{"TCP": 1}, - ConnectionCount: 1, - RxPackets: 1, - RxBytes: 1, - TxPackets: 1, - TxBytes: 1, - SessionCountVscode: 1, - SessionCountJetbrains: 0, - SessionCountReconnectingPty: 0, - SessionCountSsh: 0, - ConnectionMedianLatencyMs: 10, - }, - }) - require.NoError(t, err) + return agent + }, + ).Do() + + ac := agentsdk.New(client.URL) + ac.SetSessionToken(r.AgentToken) + conn, err := ac.ConnectRPC(context.Background()) + require.NoError(t, err) + defer func() { + _ = conn.Close() + }() + agentAPI := agentproto.NewDRPCAgentClient(conn) + + _, err = agentAPI.UpdateStats(context.Background(), &agentproto.UpdateStatsRequest{ + Stats: &agentproto.Stats{ + ConnectionsByProto: map[string]int64{"TCP": 1}, + ConnectionCount: 1, + RxPackets: 1, + RxBytes: 1, + TxPackets: 1, + TxBytes: 1, + SessionCountVscode: 1, + SessionCountJetbrains: 0, + SessionCountReconnectingPty: 0, + SessionCountSsh: 0, + ConnectionMedianLatencyMs: 10, + }, + }) + require.NoError(t, err) - tickCh <- dbtime.Now() - count := <-flushCh - require.Equal(t, 1, count, "expected one flush with one id") + tickCh <- dbtime.Now() + count := <-flushCh + require.Equal(t, 1, count, "expected one flush with one id") - newWorkspace, err := client.Workspace(context.Background(), r.Workspace.ID) - require.NoError(t, err) + newWorkspace, err := client.Workspace(context.Background(), r.Workspace.ID) + require.NoError(t, err) - assert.True(t, - newWorkspace.LastUsedAt.After(r.Workspace.LastUsedAt), - "%s is not after %s", newWorkspace.LastUsedAt, r.Workspace.LastUsedAt, - ) + assert.True(t, + newWorkspace.LastUsedAt.After(r.Workspace.LastUsedAt), + "%s is not after %s", newWorkspace.LastUsedAt, r.Workspace.LastUsedAt, + ) + }) + } } func TestAgentAPI_LargeManifest(t *testing.T) { t.Parallel() - ctx := testutil.Context(t, testutil.WaitLong) - client, store := coderdtest.NewWithDatabase(t, nil) - adminUser := coderdtest.CreateFirstUser(t, client) - n := 512000 - longScript := make([]byte, n) - for i := range longScript { - longScript[i] = 'q' + + for _, tc := range []struct { + name string + apiKeyScope rbac.ScopeName + }{ + { + name: "empty (backwards compat)", + apiKeyScope: "", + }, + { + name: "all", + apiKeyScope: rbac.ScopeAll, + }, + { + name: "no_user_data", + apiKeyScope: rbac.ScopeNoUserData, + }, + { + name: "application_connect", + apiKeyScope: rbac.ScopeApplicationConnect, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitLong) + client, store := coderdtest.NewWithDatabase(t, nil) + adminUser := coderdtest.CreateFirstUser(t, client) + n := 512000 + longScript := make([]byte, n) + for i := range longScript { + longScript[i] = 'q' + } + r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{ + OrganizationID: adminUser.OrganizationID, + OwnerID: adminUser.UserID, + }).WithAgent(func(agents []*proto.Agent) []*proto.Agent { + agents[0].Scripts = []*proto.Script{ + { + Script: string(longScript), + }, + } + agents[0].ApiKeyScope = string(tc.apiKeyScope) + return agents + }).Do() + ac := agentsdk.New(client.URL) + ac.SetSessionToken(r.AgentToken) + conn, err := ac.ConnectRPC(ctx) + defer func() { + _ = conn.Close() + }() + require.NoError(t, err) + agentAPI := agentproto.NewDRPCAgentClient(conn) + manifest, err := agentAPI.GetManifest(ctx, &agentproto.GetManifestRequest{}) + require.NoError(t, err) + require.Len(t, manifest.Scripts, 1) + require.Len(t, manifest.Scripts[0].Script, n) + }) } - r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{ - OrganizationID: adminUser.OrganizationID, - OwnerID: adminUser.UserID, - }).WithAgent(func(agents []*proto.Agent) []*proto.Agent { - agents[0].Scripts = []*proto.Script{ - { - Script: string(longScript), - }, - } - return agents - }).Do() - ac := agentsdk.New(client.URL) - ac.SetSessionToken(r.AgentToken) - conn, err := ac.ConnectRPC(ctx) - defer func() { - _ = conn.Close() - }() - require.NoError(t, err) - agentAPI := agentproto.NewDRPCAgentClient(conn) - manifest, err := agentAPI.GetManifest(ctx, &agentproto.GetManifestRequest{}) - require.NoError(t, err) - require.Len(t, manifest.Scripts, 1) - require.Len(t, manifest.Scripts[0].Script, n) } diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index 719d4e2a48123..08b90b834ccca 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -338,6 +338,7 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { RichParameterValues(createBuild.RichParameterValues). LogLevel(string(createBuild.LogLevel)). DeploymentValues(api.Options.DeploymentValues). + Experiments(api.Experiments). TemplateVersionPresetID(createBuild.TemplateVersionPresetID) var ( @@ -383,6 +384,22 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) { builder = builder.State(createBuild.ProvisionerState) } + // Only defer to dynamic parameters if the experiment is enabled. + if api.Experiments.Enabled(codersdk.ExperimentDynamicParameters) { + if createBuild.EnableDynamicParameters != nil { + // Explicit opt-in + builder = builder.DynamicParameters(*createBuild.EnableDynamicParameters) + } + } else { + if createBuild.EnableDynamicParameters != nil { + api.Logger.Warn(ctx, "ignoring dynamic parameter field sent by request, the experiment is not enabled", + slog.F("field", *createBuild.EnableDynamicParameters), + slog.F("user", apiKey.UserID.String()), + slog.F("transition", string(createBuild.Transition)), + ) + } + } + workspaceBuild, provisionerJob, provisionerDaemons, err = builder.Build( ctx, tx, diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 203c9f8599298..fe0c2d3f609a2 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -704,6 +704,8 @@ func createWorkspace( Reason(database.BuildReasonInitiator). Initiator(initiatorID). ActiveVersion(). + Experiments(api.Experiments). + DeploymentValues(api.DeploymentValues). RichParameterValues(req.RichParameterValues) if req.TemplateVersionID != uuid.Nil { builder = builder.VersionID(req.TemplateVersionID) @@ -716,7 +718,7 @@ func createWorkspace( } if req.EnableDynamicParameters && api.Experiments.Enabled(codersdk.ExperimentDynamicParameters) { - builder = builder.UsingDynamicParameters() + builder = builder.DynamicParameters(req.EnableDynamicParameters) } workspaceBuild, provisionerJob, provisionerDaemons, err = builder.Build( @@ -2259,6 +2261,7 @@ func convertWorkspace( TemplateAllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, TemplateActiveVersionID: template.ActiveVersionID, TemplateRequireActiveVersion: template.RequireActiveVersion, + TemplateUseClassicParameterFlow: template.UseClassicParameterFlow, Outdated: workspaceBuild.TemplateVersionID.String() != template.ActiveVersionID.String(), Name: workspace.Name, AutostartSchedule: autostartSchedule, diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index 91638c63e436f..46035f28dda77 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -13,7 +13,9 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/coder/coder/v2/apiversion" "github.com/coder/coder/v2/coderd/rbac/policy" + "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/provisioner/terraform/tfparse" "github.com/coder/coder/v2/provisionersdk" sdkproto "github.com/coder/coder/v2/provisionersdk/proto" @@ -51,9 +53,11 @@ type Builder struct { state stateTarget logLevel string deploymentValues *codersdk.DeploymentValues + experiments codersdk.Experiments - richParameterValues []codersdk.WorkspaceBuildParameter - dynamicParametersEnabled bool + richParameterValues []codersdk.WorkspaceBuildParameter + // dynamicParametersEnabled is non-nil if set externally + dynamicParametersEnabled *bool initiator uuid.UUID reason database.BuildReason templateVersionPresetID uuid.UUID @@ -66,6 +70,7 @@ type Builder struct { template *database.Template templateVersion *database.TemplateVersion templateVersionJob *database.ProvisionerJob + terraformValues *database.TemplateVersionTerraformValue templateVersionParameters *[]database.TemplateVersionParameter templateVersionVariables *[]database.TemplateVersionVariable templateVersionWorkspaceTags *[]database.TemplateVersionWorkspaceTag @@ -155,6 +160,14 @@ func (b Builder) DeploymentValues(dv *codersdk.DeploymentValues) Builder { return b } +func (b Builder) Experiments(exp codersdk.Experiments) Builder { + // nolint: revive + cpy := make(codersdk.Experiments, len(exp)) + copy(cpy, exp) + b.experiments = cpy + return b +} + func (b Builder) Initiator(u uuid.UUID) Builder { // nolint: revive b.initiator = u @@ -187,8 +200,9 @@ func (b Builder) MarkPrebuiltWorkspaceClaim() Builder { return b } -func (b Builder) UsingDynamicParameters() Builder { - b.dynamicParametersEnabled = true +func (b Builder) DynamicParameters(using bool) Builder { + // nolint: revive + b.dynamicParametersEnabled = ptr.Ref(using) return b } @@ -516,6 +530,22 @@ func (b *Builder) getTemplateVersionID() (uuid.UUID, error) { return bld.TemplateVersionID, nil } +func (b *Builder) getTemplateTerraformValues() (*database.TemplateVersionTerraformValue, error) { + if b.terraformValues != nil { + return b.terraformValues, nil + } + v, err := b.getTemplateVersion() + if err != nil { + return nil, xerrors.Errorf("get template version so we can get terraform values: %w", err) + } + vals, err := b.store.GetTemplateVersionTerraformValues(b.ctx, v.ID) + if err != nil { + return nil, xerrors.Errorf("get template version terraform values %s: %w", v.JobID, err) + } + b.terraformValues = &vals + return b.terraformValues, err +} + func (b *Builder) getLastBuild() (*database.WorkspaceBuild, error) { if b.lastBuild != nil { return b.lastBuild, nil @@ -593,30 +623,43 @@ func (b *Builder) getParameters() (names, values []string, err error) { return nil, nil, BuildError{http.StatusBadRequest, "Unable to build workspace with unsupported parameters", err} } + // Dynamic parameters skip all parameter validation. + // Deleting a workspace also should skip parameter validation. + // Pass the user's input as is. + if b.usingDynamicParameters() { + // TODO: The previous behavior was only to pass param values + // for parameters that exist. Since dynamic params can have + // conditional parameter existence, the static frame of reference + // is not sufficient. So assume the user is correct, or pull in the + // dynamic param code to find the actual parameters. + for _, value := range b.richParameterValues { + names = append(names, value.Name) + values = append(values, value.Value) + } + b.parameterNames = &names + b.parameterValues = &values + return names, values, nil + } + resolver := codersdk.ParameterResolver{ Rich: db2sdk.WorkspaceBuildParameters(lastBuildParameters), } + for _, templateVersionParameter := range templateVersionParameters { tvp, err := db2sdk.TemplateVersionParameter(templateVersionParameter) if err != nil { return nil, nil, BuildError{http.StatusInternalServerError, "failed to convert template version parameter", err} } - var value string - if !b.dynamicParametersEnabled { - var err error - value, err = resolver.ValidateResolve( - tvp, - b.findNewBuildParameterValue(templateVersionParameter.Name), - ) - if err != nil { - // At this point, we've queried all the data we need from the database, - // so the only errors are problems with the request (missing data, failed - // validation, immutable parameters, etc.) - return nil, nil, BuildError{http.StatusBadRequest, fmt.Sprintf("Unable to validate parameter %q", templateVersionParameter.Name), err} - } - } else { - value = resolver.Resolve(tvp, b.findNewBuildParameterValue(templateVersionParameter.Name)) + value, err := resolver.ValidateResolve( + tvp, + b.findNewBuildParameterValue(templateVersionParameter.Name), + ) + if err != nil { + // At this point, we've queried all the data we need from the database, + // so the only errors are problems with the request (missing data, failed + // validation, immutable parameters, etc.) + return nil, nil, BuildError{http.StatusBadRequest, fmt.Sprintf("Unable to validate parameter %q", templateVersionParameter.Name), err} } names = append(names, templateVersionParameter.Name) @@ -977,3 +1020,36 @@ func (b *Builder) checkRunningBuild() error { } return nil } + +func (b *Builder) usingDynamicParameters() bool { + if !b.experiments.Enabled(codersdk.ExperimentDynamicParameters) { + // Experiment required + return false + } + + vals, err := b.getTemplateTerraformValues() + if err != nil { + return false + } + + if !ProvisionerVersionSupportsDynamicParameters(vals.ProvisionerdVersion) { + return false + } + + if b.dynamicParametersEnabled != nil { + return *b.dynamicParametersEnabled + } + + tpl, err := b.getTemplate() + if err != nil { + return false // Let another part of the code get this error + } + return !tpl.UseClassicParameterFlow +} + +func ProvisionerVersionSupportsDynamicParameters(version string) bool { + major, minor, err := apiversion.Parse(version) + // If the api version is not valid or less than 1.6, we need to use the static parameters + useStaticParams := err != nil || major < 1 || (major == 1 && minor < 6) + return !useStaticParams +} diff --git a/coderd/wsbuilder/wsbuilder_test.go b/coderd/wsbuilder/wsbuilder_test.go index 00b7b5f0ae08b..abe5e3fe9b8b7 100644 --- a/coderd/wsbuilder/wsbuilder_test.go +++ b/coderd/wsbuilder/wsbuilder_test.go @@ -839,6 +839,32 @@ func TestWorkspaceBuildWithPreset(t *testing.T) { req.NoError(err) } +func TestProvisionerVersionSupportsDynamicParameters(t *testing.T) { + t.Parallel() + + for v, dyn := range map[string]bool{ + "": false, + "na": false, + "0.0": false, + "0.10": false, + "1.4": false, + "1.5": false, + "1.6": true, + "1.7": true, + "1.8": true, + "2.0": true, + "2.17": true, + "4.0": true, + } { + t.Run(v, func(t *testing.T) { + t.Parallel() + + does := wsbuilder.ProvisionerVersionSupportsDynamicParameters(v) + require.Equal(t, dyn, does) + }) + } +} + type txExpect func(mTx *dbmock.MockStore) func expectDB(t *testing.T, opts ...txExpect) *dbmock.MockStore { diff --git a/codersdk/agentsdk/agentsdk.go b/codersdk/agentsdk/agentsdk.go index ba3ff5681b742..9e6df933ce6c3 100644 --- a/codersdk/agentsdk/agentsdk.go +++ b/codersdk/agentsdk/agentsdk.go @@ -246,7 +246,7 @@ func (c *Client) ConnectRPC23(ctx context.Context) ( } // ConnectRPC24 returns a dRPC client to the Agent API v2.4. It is useful when you want to be -// maximally compatible with Coderd Release Versions from 2.xx+ // TODO @vincent: define version +// maximally compatible with Coderd Release Versions from 2.20+ func (c *Client) ConnectRPC24(ctx context.Context) ( proto.DRPCAgentClient24, tailnetproto.DRPCTailnetClient24, error, ) { @@ -257,6 +257,18 @@ func (c *Client) ConnectRPC24(ctx context.Context) ( return proto.NewDRPCAgentClient(conn), tailnetproto.NewDRPCTailnetClient(conn), nil } +// ConnectRPC25 returns a dRPC client to the Agent API v2.5. It is useful when you want to be +// maximally compatible with Coderd Release Versions from 2.xx+ // TODO(DanielleMaywood): Update version +func (c *Client) ConnectRPC25(ctx context.Context) ( + proto.DRPCAgentClient25, tailnetproto.DRPCTailnetClient25, error, +) { + conn, err := c.connectRPCVersion(ctx, apiversion.New(2, 5)) + if err != nil { + return nil, nil, err + } + return proto.NewDRPCAgentClient(conn), tailnetproto.NewDRPCTailnetClient(conn), nil +} + // ConnectRPC connects to the workspace agent API and tailnet API func (c *Client) ConnectRPC(ctx context.Context) (drpc.Conn, error) { return c.connectRPCVersion(ctx, proto.CurrentVersion) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 0741bf9e3844a..39b67feb2c73a 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -345,7 +345,7 @@ type DeploymentValues struct { // HTTPAddress is a string because it may be set to zero to disable. HTTPAddress serpent.String `json:"http_address,omitempty" typescript:",notnull"` AutobuildPollInterval serpent.Duration `json:"autobuild_poll_interval,omitempty"` - JobHangDetectorInterval serpent.Duration `json:"job_hang_detector_interval,omitempty"` + JobReaperDetectorInterval serpent.Duration `json:"job_hang_detector_interval,omitempty"` DERP DERP `json:"derp,omitempty" typescript:",notnull"` Prometheus PrometheusConfig `json:"prometheus,omitempty" typescript:",notnull"` Pprof PprofConfig `json:"pprof,omitempty" typescript:",notnull"` @@ -1287,13 +1287,13 @@ func (c *DeploymentValues) Options() serpent.OptionSet { Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"), }, { - Name: "Job Hang Detector Interval", - Description: "Interval to poll for hung jobs and automatically terminate them.", + Name: "Job Reaper Detect Interval", + Description: "Interval to poll for hung and pending jobs and automatically terminate them.", Flag: "job-hang-detector-interval", Env: "CODER_JOB_HANG_DETECTOR_INTERVAL", Hidden: true, Default: time.Minute.String(), - Value: &c.JobHangDetectorInterval, + Value: &c.JobReaperDetectorInterval, YAML: "jobHangDetectorInterval", Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"), }, diff --git a/codersdk/parameters.go b/codersdk/parameters.go index 881aaf99f573c..d81dc7cf55ca0 100644 --- a/codersdk/parameters.go +++ b/codersdk/parameters.go @@ -7,17 +7,121 @@ import ( "github.com/google/uuid" "github.com/coder/coder/v2/codersdk/wsjson" - previewtypes "github.com/coder/preview/types" "github.com/coder/websocket" ) -// FriendlyDiagnostic is included to guarantee it is generated in the output -// types. This is used as the type override for `previewtypes.Diagnostic`. -type FriendlyDiagnostic = previewtypes.FriendlyDiagnostic +type ParameterFormType string -// NullHCLString is included to guarantee it is generated in the output -// types. This is used as the type override for `previewtypes.HCLString`. -type NullHCLString = previewtypes.NullHCLString +const ( + ParameterFormTypeDefault ParameterFormType = "" + ParameterFormTypeRadio ParameterFormType = "radio" + ParameterFormTypeSlider ParameterFormType = "slider" + ParameterFormTypeInput ParameterFormType = "input" + ParameterFormTypeDropdown ParameterFormType = "dropdown" + ParameterFormTypeCheckbox ParameterFormType = "checkbox" + ParameterFormTypeSwitch ParameterFormType = "switch" + ParameterFormTypeMultiSelect ParameterFormType = "multi-select" + ParameterFormTypeTagSelect ParameterFormType = "tag-select" + ParameterFormTypeTextArea ParameterFormType = "textarea" + ParameterFormTypeError ParameterFormType = "error" +) + +type OptionType string + +const ( + OptionTypeString OptionType = "string" + OptionTypeNumber OptionType = "number" + OptionTypeBoolean OptionType = "bool" + OptionTypeListString OptionType = "list(string)" +) + +type DiagnosticSeverityString string + +const ( + DiagnosticSeverityError DiagnosticSeverityString = "error" + DiagnosticSeverityWarning DiagnosticSeverityString = "warning" +) + +// FriendlyDiagnostic == previewtypes.FriendlyDiagnostic +// Copied to avoid import deps +type FriendlyDiagnostic struct { + Severity DiagnosticSeverityString `json:"severity"` + Summary string `json:"summary"` + Detail string `json:"detail"` + + Extra DiagnosticExtra `json:"extra"` +} + +type DiagnosticExtra struct { + Code string `json:"code"` +} + +// NullHCLString == `previewtypes.NullHCLString`. +type NullHCLString struct { + Value string `json:"value"` + Valid bool `json:"valid"` +} + +type PreviewParameter struct { + PreviewParameterData + Value NullHCLString `json:"value"` + Diagnostics []FriendlyDiagnostic `json:"diagnostics"` +} + +type PreviewParameterData struct { + Name string `json:"name"` + DisplayName string `json:"display_name"` + Description string `json:"description"` + Type OptionType `json:"type"` + FormType ParameterFormType `json:"form_type"` + Styling PreviewParameterStyling `json:"styling"` + Mutable bool `json:"mutable"` + DefaultValue NullHCLString `json:"default_value"` + Icon string `json:"icon"` + Options []PreviewParameterOption `json:"options"` + Validations []PreviewParameterValidation `json:"validations"` + Required bool `json:"required"` + // legacy_variable_name was removed (= 14) + Order int64 `json:"order"` + Ephemeral bool `json:"ephemeral"` +} + +type PreviewParameterStyling struct { + Placeholder *string `json:"placeholder,omitempty"` + Disabled *bool `json:"disabled,omitempty"` + Label *string `json:"label,omitempty"` +} + +type PreviewParameterOption struct { + Name string `json:"name"` + Description string `json:"description"` + Value NullHCLString `json:"value"` + Icon string `json:"icon"` +} + +type PreviewParameterValidation struct { + Error string `json:"validation_error"` + + // All validation attributes are optional. + Regex *string `json:"validation_regex"` + Min *int64 `json:"validation_min"` + Max *int64 `json:"validation_max"` + Monotonic *string `json:"validation_monotonic"` +} + +type DynamicParametersRequest struct { + // ID identifies the request. The response contains the same + // ID so that the client can match it to the request. + ID int `json:"id"` + Inputs map[string]string `json:"inputs"` +} + +type DynamicParametersResponse struct { + ID int `json:"id"` + Diagnostics []FriendlyDiagnostic `json:"diagnostics"` + Parameters []PreviewParameter `json:"parameters"` + // TODO: Workspace tags +} func (c *Client) TemplateVersionDynamicParameters(ctx context.Context, userID, version uuid.UUID) (*wsjson.Stream[DynamicParametersResponse, DynamicParametersRequest], error) { conn, err := c.Dial(ctx, fmt.Sprintf("/api/v2/users/%s/templateversions/%s/parameters", userID, version), nil) diff --git a/codersdk/provisionerdaemons.go b/codersdk/provisionerdaemons.go index 11345a115e07f..5fbda371b8f3f 100644 --- a/codersdk/provisionerdaemons.go +++ b/codersdk/provisionerdaemons.go @@ -178,6 +178,7 @@ type ProvisionerJob struct { ErrorCode JobErrorCode `json:"error_code,omitempty" enums:"REQUIRED_TEMPLATE_VARIABLES" table:"error code"` Status ProvisionerJobStatus `json:"status" enums:"pending,running,succeeded,canceling,canceled,failed" table:"status"` WorkerID *uuid.UUID `json:"worker_id,omitempty" format:"uuid" table:"worker id"` + WorkerName string `json:"worker_name,omitempty" table:"worker name"` FileID uuid.UUID `json:"file_id" format:"uuid" table:"file id"` Tags map[string]string `json:"tags" table:"tags"` QueuePosition int `json:"queue_position" table:"queue position"` diff --git a/codersdk/rbacresources_gen.go b/codersdk/rbacresources_gen.go index 54f65767928d6..95792bb8e2a7b 100644 --- a/codersdk/rbacresources_gen.go +++ b/codersdk/rbacresources_gen.go @@ -49,7 +49,9 @@ const ( ActionApplicationConnect RBACAction = "application_connect" ActionAssign RBACAction = "assign" ActionCreate RBACAction = "create" + ActionCreateAgent RBACAction = "create_agent" ActionDelete RBACAction = "delete" + ActionDeleteAgent RBACAction = "delete_agent" ActionRead RBACAction = "read" ActionReadPersonal RBACAction = "read_personal" ActionSSH RBACAction = "ssh" @@ -90,16 +92,16 @@ var RBACResourceActions = map[RBACResource][]RBACAction{ ResourceOrganization: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, ResourceOrganizationMember: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, ResourceProvisionerDaemon: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceProvisionerJobs: {ActionRead}, + ResourceProvisionerJobs: {ActionCreate, ActionRead, ActionUpdate}, ResourceReplicas: {ActionRead}, ResourceSystem: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, ResourceTailnetCoordinator: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, ResourceTemplate: {ActionCreate, ActionDelete, ActionRead, ActionUpdate, ActionUse, ActionViewInsights}, ResourceUser: {ActionCreate, ActionDelete, ActionRead, ActionReadPersonal, ActionUpdate, ActionUpdatePersonal}, ResourceWebpushSubscription: {ActionCreate, ActionDelete, ActionRead}, - ResourceWorkspace: {ActionApplicationConnect, ActionCreate, ActionDelete, ActionRead, ActionSSH, ActionWorkspaceStart, ActionWorkspaceStop, ActionUpdate}, + ResourceWorkspace: {ActionApplicationConnect, ActionCreate, ActionCreateAgent, ActionDelete, ActionDeleteAgent, ActionRead, ActionSSH, ActionWorkspaceStart, ActionWorkspaceStop, ActionUpdate}, ResourceWorkspaceAgentDevcontainers: {ActionCreate}, ResourceWorkspaceAgentResourceMonitor: {ActionCreate, ActionRead, ActionUpdate}, - ResourceWorkspaceDormant: {ActionApplicationConnect, ActionCreate, ActionDelete, ActionRead, ActionSSH, ActionWorkspaceStart, ActionWorkspaceStop, ActionUpdate}, + ResourceWorkspaceDormant: {ActionApplicationConnect, ActionCreate, ActionCreateAgent, ActionDelete, ActionDeleteAgent, ActionRead, ActionSSH, ActionWorkspaceStart, ActionWorkspaceStop, ActionUpdate}, ResourceWorkspaceProxy: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, } diff --git a/codersdk/templateversions.go b/codersdk/templateversions.go index 42b381fadebce..de8bb7b970957 100644 --- a/codersdk/templateversions.go +++ b/codersdk/templateversions.go @@ -9,8 +9,6 @@ import ( "time" "github.com/google/uuid" - - previewtypes "github.com/coder/preview/types" ) type TemplateVersionWarning string @@ -125,20 +123,6 @@ func (c *Client) CancelTemplateVersion(ctx context.Context, version uuid.UUID) e return nil } -type DynamicParametersRequest struct { - // ID identifies the request. The response contains the same - // ID so that the client can match it to the request. - ID int `json:"id"` - Inputs map[string]string `json:"inputs"` -} - -type DynamicParametersResponse struct { - ID int `json:"id"` - Diagnostics previewtypes.Diagnostics `json:"diagnostics"` - Parameters []previewtypes.Parameter `json:"parameters"` - // TODO: Workspace tags -} - // TemplateVersionParameters returns parameters a template version exposes. func (c *Client) TemplateVersionRichParameters(ctx context.Context, version uuid.UUID) ([]TemplateVersionParameter, error) { res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/templateversions/%s/rich-parameters", version), nil) diff --git a/codersdk/workspaceagents.go b/codersdk/workspaceagents.go index f58338a209901..37048c6c4fcfe 100644 --- a/codersdk/workspaceagents.go +++ b/codersdk/workspaceagents.go @@ -439,6 +439,10 @@ type WorkspaceAgentContainer struct { // Volumes is a map of "things" mounted into the container. Again, this // is somewhat implementation-dependent. Volumes map[string]string `json:"volumes"` + // DevcontainerDirty is true if the devcontainer configuration has changed + // since the container was created. This is used to determine if the + // container needs to be rebuilt. + DevcontainerDirty bool `json:"devcontainer_dirty"` } func (c *WorkspaceAgentContainer) Match(idOrName string) bool { @@ -502,6 +506,19 @@ func (c *Client) WorkspaceAgentListContainers(ctx context.Context, agentID uuid. return cr, json.NewDecoder(res.Body).Decode(&cr) } +// WorkspaceAgentRecreateDevcontainer recreates the devcontainer with the given ID. +func (c *Client) WorkspaceAgentRecreateDevcontainer(ctx context.Context, agentID uuid.UUID, containerIDOrName string) error { + res, err := c.Request(ctx, http.MethodPost, fmt.Sprintf("/api/v2/workspaceagents/%s/containers/devcontainers/container/%s/recreate", agentID, containerIDOrName), nil) + if err != nil { + return err + } + defer res.Body.Close() + if res.StatusCode != http.StatusNoContent { + return ReadBodyAsError(res) + } + return nil +} + //nolint:revive // Follow is a control flag on the server as well. func (c *Client) WorkspaceAgentLogsAfter(ctx context.Context, agentID uuid.UUID, after int64, follow bool) (<-chan []WorkspaceAgentLog, io.Closer, error) { var queryParams []string diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index 311c4bcba35d4..e0f1b9b1e2c2a 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -41,6 +41,7 @@ type Workspace struct { TemplateAllowUserCancelWorkspaceJobs bool `json:"template_allow_user_cancel_workspace_jobs"` TemplateActiveVersionID uuid.UUID `json:"template_active_version_id" format:"uuid"` TemplateRequireActiveVersion bool `json:"template_require_active_version"` + TemplateUseClassicParameterFlow bool `json:"template_use_classic_parameter_flow"` LatestBuild WorkspaceBuild `json:"latest_build"` LatestAppStatus *WorkspaceAppStatus `json:"latest_app_status"` Outdated bool `json:"outdated"` @@ -109,6 +110,10 @@ type CreateWorkspaceBuildRequest struct { LogLevel ProvisionerLogLevel `json:"log_level,omitempty" validate:"omitempty,oneof=debug"` // TemplateVersionPresetID is the ID of the template version preset to use for the build. TemplateVersionPresetID uuid.UUID `json:"template_version_preset_id,omitempty" format:"uuid"` + // EnableDynamicParameters skips some of the static parameter checking. + // It will default to whatever the template has marked as the default experience. + // Requires the "dynamic-experiment" to be used. + EnableDynamicParameters *bool `json:"enable_dynamic_parameters,omitempty"` } type WorkspaceOptions struct { diff --git a/docs/admin/provisioners/manage-provisioner-jobs.md b/docs/admin/provisioners/manage-provisioner-jobs.md index 05d5d9dddff9f..b2581e6020fc6 100644 --- a/docs/admin/provisioners/manage-provisioner-jobs.md +++ b/docs/admin/provisioners/manage-provisioner-jobs.md @@ -48,6 +48,10 @@ Each provisioner job has a lifecycle state: | **Failed** | Provisioner encountered an error while executing the job. | | **Canceled** | Job was manually terminated by an admin. | +The following diagram shows how a provisioner job transitions between lifecycle states: + + + ## When to cancel provisioner jobs A job might need to be cancelled when: diff --git a/docs/admin/setup/index.md b/docs/admin/setup/index.md index 96000292266e2..1a34920e733e8 100644 --- a/docs/admin/setup/index.md +++ b/docs/admin/setup/index.md @@ -140,7 +140,7 @@ To configure Coder behind a corporate proxy, set the environment variables `HTTP_PROXY` and `HTTPS_PROXY`. Be sure to restart the server. Lowercase values (e.g. `http_proxy`) are also respected in this case. -## External Authentication +## Continue your setup with external authentication Coder supports external authentication via OAuth2.0. This allows enabling integrations with Git providers, such as GitHub, GitLab, and Bitbucket. diff --git a/docs/admin/templates/extending-templates/devcontainers.md b/docs/admin/templates/extending-templates/devcontainers.md index 4894a012476a1..d4284bf48efde 100644 --- a/docs/admin/templates/extending-templates/devcontainers.md +++ b/docs/admin/templates/extending-templates/devcontainers.md @@ -122,3 +122,5 @@ resource "docker_container" "workspace" { ## Next Steps - [Dev Containers Integration](../../../user-guides/devcontainers/index.md) +- [Working with Dev Containers](../../../user-guides/devcontainers/working-with-dev-containers.md) +- [Troubleshooting Dev Containers](../../../user-guides/devcontainers/troubleshooting-dev-containers.md) diff --git a/docs/admin/templates/extending-templates/parameters.md b/docs/admin/templates/extending-templates/parameters.md index b5e6473ab6b4f..7f216bd3e64f9 100644 --- a/docs/admin/templates/extending-templates/parameters.md +++ b/docs/admin/templates/extending-templates/parameters.md @@ -252,7 +252,7 @@ data "coder_parameter" "force_rebuild" { ## Validating parameters -Coder supports rich parameters with multiple validation modes: min, max, +Coder supports parameters with multiple validation modes: min, max, monotonic numbers, and regular expressions. ### Number @@ -391,3 +391,548 @@ parameters in one of two ways: ``` Or set the [environment variable](../../setup/index.md), `CODER_EXPERIMENTS=auto-fill-parameters` + +## Dynamic Parameters + +Dynamic Parameters enhances Coder's existing parameter system with real-time validation, +conditional parameter behavior, and richer input types. +This feature allows template authors to create more interactive and responsive workspace creation experiences. + +### Enable Dynamic Parameters (Early Access) + +To use Dynamic Parameters, enable the experiment flag or set the environment variable. + +Note that as of v2.22.0, Dynamic parameters are an unsafe experiment and will not be enabled with the experiment wildcard. + +
[id\|created at\|started at\|completed at\|canceled at\|error\|error code\|status\|worker id\|file id\|tags\|queue position\|queue size\|organization id\|template version id\|workspace build id\|type\|available workers\|template version name\|template id\|template name\|template display name\|template icon\|workspace id\|workspace name\|organization\|queue]
|
-| Default | created at,id,type,template display name,status,queue,tags
|
+| | |
+|---------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| Type | [id\|created at\|started at\|completed at\|canceled at\|error\|error code\|status\|worker id\|worker name\|file id\|tags\|queue position\|queue size\|organization id\|template version id\|workspace build id\|type\|available workers\|template version name\|template id\|template name\|template display name\|template icon\|workspace id\|workspace name\|organization\|queue]
|
+| Default | created at,id,type,template display name,status,queue,tags
|
Columns to display in table output.
diff --git a/dogfood/coder/main.tf b/dogfood/coder/main.tf
index ddfd1f8e95e3d..06da4d79c549a 100644
--- a/dogfood/coder/main.tf
+++ b/dogfood/coder/main.tf
@@ -30,6 +30,81 @@ locals {
container_name = "coder-${data.coder_workspace_owner.me.name}-${lower(data.coder_workspace.me.name)}"
}
+data "coder_workspace_preset" "cpt" {
+ name = "Cape Town"
+ parameters = {
+ (data.coder_parameter.region.name) = "za-cpt"
+ (data.coder_parameter.image_type.name) = "codercom/oss-dogfood:latest"
+ (data.coder_parameter.repo_base_dir.name) = "~"
+ (data.coder_parameter.res_mon_memory_threshold.name) = 80
+ (data.coder_parameter.res_mon_volume_threshold.name) = 90
+ (data.coder_parameter.res_mon_volume_path.name) = "/home/coder"
+ }
+ prebuilds {
+ instances = 1
+ }
+}
+
+data "coder_workspace_preset" "pittsburgh" {
+ name = "Pittsburgh"
+ parameters = {
+ (data.coder_parameter.region.name) = "us-pittsburgh"
+ (data.coder_parameter.image_type.name) = "codercom/oss-dogfood:latest"
+ (data.coder_parameter.repo_base_dir.name) = "~"
+ (data.coder_parameter.res_mon_memory_threshold.name) = 80
+ (data.coder_parameter.res_mon_volume_threshold.name) = 90
+ (data.coder_parameter.res_mon_volume_path.name) = "/home/coder"
+ }
+ prebuilds {
+ instances = 2
+ }
+}
+
+data "coder_workspace_preset" "falkenstein" {
+ name = "Falkenstein"
+ parameters = {
+ (data.coder_parameter.region.name) = "eu-helsinki"
+ (data.coder_parameter.image_type.name) = "codercom/oss-dogfood:latest"
+ (data.coder_parameter.repo_base_dir.name) = "~"
+ (data.coder_parameter.res_mon_memory_threshold.name) = 80
+ (data.coder_parameter.res_mon_volume_threshold.name) = 90
+ (data.coder_parameter.res_mon_volume_path.name) = "/home/coder"
+ }
+ prebuilds {
+ instances = 1
+ }
+}
+
+data "coder_workspace_preset" "sydney" {
+ name = "Sydney"
+ parameters = {
+ (data.coder_parameter.region.name) = "ap-sydney"
+ (data.coder_parameter.image_type.name) = "codercom/oss-dogfood:latest"
+ (data.coder_parameter.repo_base_dir.name) = "~"
+ (data.coder_parameter.res_mon_memory_threshold.name) = 80
+ (data.coder_parameter.res_mon_volume_threshold.name) = 90
+ (data.coder_parameter.res_mon_volume_path.name) = "/home/coder"
+ }
+ prebuilds {
+ instances = 1
+ }
+}
+
+data "coder_workspace_preset" "saopaulo" {
+ name = "São Paulo"
+ parameters = {
+ (data.coder_parameter.region.name) = "sa-saopaulo"
+ (data.coder_parameter.image_type.name) = "codercom/oss-dogfood:latest"
+ (data.coder_parameter.repo_base_dir.name) = "~"
+ (data.coder_parameter.res_mon_memory_threshold.name) = 80
+ (data.coder_parameter.res_mon_volume_threshold.name) = 90
+ (data.coder_parameter.res_mon_volume_path.name) = "/home/coder"
+ }
+ prebuilds {
+ instances = 1
+ }
+}
+
data "coder_parameter" "repo_base_dir" {
type = "string"
name = "Coder Repository Base Directory"
@@ -373,6 +448,17 @@ resource "coder_agent" "dev" {
#!/usr/bin/env bash
set -eux -o pipefail
+ # Stop all running containers and prune the system to clean up
+ # /var/lib/docker to prevent errors during workspace destroy.
+ #
+ # WARNING! This will remove:
+ # - all containers
+ # - all networks
+ # - all images
+ # - all build cache
+ docker ps -q | xargs docker stop
+ docker system prune -a -f
+
# Stop the Docker service to prevent errors during workspace destroy.
sudo service docker stop
EOT
@@ -427,6 +513,14 @@ resource "docker_image" "dogfood" {
}
resource "docker_container" "workspace" {
+ lifecycle {
+ // Ignore changes that would invalidate prebuilds
+ ignore_changes = [
+ name,
+ hostname,
+ labels,
+ ]
+ }
count = data.coder_workspace.me.start_count
image = docker_image.dogfood.name
name = local.container_name
diff --git a/enterprise/cli/testdata/coder_provisioner_jobs_list_--help.golden b/enterprise/cli/testdata/coder_provisioner_jobs_list_--help.golden
index 7a72605f0c288..f380a0334867c 100644
--- a/enterprise/cli/testdata/coder_provisioner_jobs_list_--help.golden
+++ b/enterprise/cli/testdata/coder_provisioner_jobs_list_--help.golden
@@ -11,7 +11,7 @@ OPTIONS:
-O, --org string, $CODER_ORGANIZATION
Select which organization (uuid or name) to use.
- -c, --column [id|created at|started at|completed at|canceled at|error|error code|status|worker id|file id|tags|queue position|queue size|organization id|template version id|workspace build id|type|available workers|template version name|template id|template name|template display name|template icon|workspace id|workspace name|organization|queue] (default: created at,id,type,template display name,status,queue,tags)
+ -c, --column [id|created at|started at|completed at|canceled at|error|error code|status|worker id|worker name|file id|tags|queue position|queue size|organization id|template version id|workspace build id|type|available workers|template version name|template id|template name|template display name|template icon|workspace id|workspace name|organization|queue] (default: created at,id,type,template display name,status,queue,tags)
Columns to display in table output.
-l, --limit int, $CODER_PROVISIONER_JOB_LIST_LIMIT (default: 50)
diff --git a/enterprise/coderd/parameters_test.go b/enterprise/coderd/parameters_test.go
index e6bc564e43da2..76bd5a1eafdbb 100644
--- a/enterprise/coderd/parameters_test.go
+++ b/enterprise/coderd/parameters_test.go
@@ -70,8 +70,8 @@ func TestDynamicParametersOwnerGroups(t *testing.T) {
require.Equal(t, -1, preview.ID)
require.Empty(t, preview.Diagnostics)
require.Equal(t, "group", preview.Parameters[0].Name)
- require.True(t, preview.Parameters[0].Value.Valid())
- require.Equal(t, database.EveryoneGroup, preview.Parameters[0].Value.Value.AsString())
+ require.True(t, preview.Parameters[0].Value.Valid)
+ require.Equal(t, database.EveryoneGroup, preview.Parameters[0].Value.Value)
// Send a new value, and see it reflected
err = stream.Send(codersdk.DynamicParametersRequest{
@@ -83,8 +83,8 @@ func TestDynamicParametersOwnerGroups(t *testing.T) {
require.Equal(t, 1, preview.ID)
require.Empty(t, preview.Diagnostics)
require.Equal(t, "group", preview.Parameters[0].Name)
- require.True(t, preview.Parameters[0].Value.Valid())
- require.Equal(t, group.Name, preview.Parameters[0].Value.Value.AsString())
+ require.True(t, preview.Parameters[0].Value.Valid)
+ require.Equal(t, group.Name, preview.Parameters[0].Value.Value)
// Back to default
err = stream.Send(codersdk.DynamicParametersRequest{
@@ -96,6 +96,6 @@ func TestDynamicParametersOwnerGroups(t *testing.T) {
require.Equal(t, 3, preview.ID)
require.Empty(t, preview.Diagnostics)
require.Equal(t, "group", preview.Parameters[0].Name)
- require.True(t, preview.Parameters[0].Value.Valid())
- require.Equal(t, database.EveryoneGroup, preview.Parameters[0].Value.Value.AsString())
+ require.True(t, preview.Parameters[0].Value.Valid)
+ require.Equal(t, database.EveryoneGroup, preview.Parameters[0].Value.Value)
}
diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go
index 7005c93ca36f5..226232f37bf7f 100644
--- a/enterprise/coderd/workspaces_test.go
+++ b/enterprise/coderd/workspaces_test.go
@@ -1659,6 +1659,119 @@ func TestTemplateDoesNotAllowUserAutostop(t *testing.T) {
})
}
+// TestWorkspaceTemplateParamsChange tests a workspace with a parameter that
+// validation changes on apply. The params used in create workspace are invalid
+// according to the static params on import.
+//
+// This is testing that dynamic params defers input validation to terraform.
+// It does not try to do this in coder/coder.
+func TestWorkspaceTemplateParamsChange(t *testing.T) {
+ mainTfTemplate := `
+ terraform {
+ required_providers {
+ coder = {
+ source = "coder/coder"
+ }
+ }
+ }
+ provider "coder" {}
+ data "coder_workspace" "me" {}
+ data "coder_workspace_owner" "me" {}
+
+ data "coder_parameter" "param_min" {
+ name = "param_min"
+ type = "number"
+ default = 10
+ }
+
+ data "coder_parameter" "param" {
+ name = "param"
+ type = "number"
+ default = 12
+ validation {
+ min = data.coder_parameter.param_min.value
+ }
+ }
+ `
+ tfCliConfigPath := downloadProviders(t, mainTfTemplate)
+ t.Setenv("TF_CLI_CONFIG_FILE", tfCliConfigPath)
+
+ logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: false})
+ dv := coderdtest.DeploymentValues(t)
+ dv.Experiments = []string{string(codersdk.ExperimentDynamicParameters)}
+ client, owner := coderdenttest.New(t, &coderdenttest.Options{
+ Options: &coderdtest.Options{
+ Logger: &logger,
+ // We intentionally do not run a built-in provisioner daemon here.
+ IncludeProvisionerDaemon: false,
+ DeploymentValues: dv,
+ },
+ LicenseOptions: &coderdenttest.LicenseOptions{
+ Features: license.Features{
+ codersdk.FeatureExternalProvisionerDaemons: 1,
+ },
+ },
+ })
+ templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin())
+ member, memberUser := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
+
+ _ = coderdenttest.NewExternalProvisionerDaemonTerraform(t, client, owner.OrganizationID, nil)
+
+ // This can take a while, so set a relatively long timeout.
+ ctx := testutil.Context(t, 2*testutil.WaitSuperLong)
+
+ // Creating a template as a template admin must succeed
+ templateFiles := map[string]string{"main.tf": mainTfTemplate}
+ tarBytes := testutil.CreateTar(t, templateFiles)
+ fi, err := templateAdmin.Upload(ctx, "application/x-tar", bytes.NewReader(tarBytes))
+ require.NoError(t, err, "failed to upload file")
+
+ tv, err := templateAdmin.CreateTemplateVersion(ctx, owner.OrganizationID, codersdk.CreateTemplateVersionRequest{
+ Name: testutil.GetRandomName(t),
+ FileID: fi.ID,
+ StorageMethod: codersdk.ProvisionerStorageMethodFile,
+ Provisioner: codersdk.ProvisionerTypeTerraform,
+ UserVariableValues: []codersdk.VariableValue{},
+ })
+ require.NoError(t, err, "failed to create template version")
+ coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdmin, tv.ID)
+ tpl := coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, tv.ID)
+ require.False(t, tpl.UseClassicParameterFlow, "template to use dynamic parameters")
+
+ // When: we create a workspace build using the above template but with
+ // parameter values that are different from those defined in the template.
+ // The new values are not valid according to the original plan, but are valid.
+ ws, err := member.CreateUserWorkspace(ctx, memberUser.Username, codersdk.CreateWorkspaceRequest{
+ TemplateID: tpl.ID,
+ Name: coderdtest.RandomUsername(t),
+ RichParameterValues: []codersdk.WorkspaceBuildParameter{
+ {
+ Name: "param_min",
+ Value: "5",
+ },
+ {
+ Name: "param",
+ Value: "7",
+ },
+ },
+ EnableDynamicParameters: true,
+ })
+
+ // Then: the build should succeed. The updated value of param_min should be
+ // used to validate param instead of the value defined in the temp
+ require.NoError(t, err, "failed to create workspace")
+ createBuild := coderdtest.AwaitWorkspaceBuildJobCompleted(t, member, ws.LatestBuild.ID)
+ require.Equal(t, createBuild.Status, codersdk.WorkspaceStatusRunning)
+
+ // Now delete the workspace
+ build, err := member.CreateWorkspaceBuild(ctx, ws.ID, codersdk.CreateWorkspaceBuildRequest{
+ Transition: codersdk.WorkspaceTransitionDelete,
+ })
+ require.NoError(t, err)
+ build = coderdtest.AwaitWorkspaceBuildJobCompleted(t, member, build.ID)
+ require.Equal(t, codersdk.WorkspaceStatusDeleted, build.Status)
+}
+
// TestWorkspaceTagsTerraform tests that a workspace can be created with tags.
// This is an end-to-end-style test, meaning that we actually run the
// real Terraform provisioner and validate that the workspace is created
diff --git a/flake.nix b/flake.nix
index bff207662f913..c0f36c3be6e0f 100644
--- a/flake.nix
+++ b/flake.nix
@@ -141,6 +141,7 @@
kubectl
kubectx
kubernetes-helm
+ lazydocker
lazygit
less
mockgen
diff --git a/go.mod b/go.mod
index cdbd61574066f..0c6b482b38f4e 100644
--- a/go.mod
+++ b/go.mod
@@ -74,7 +74,7 @@ replace github.com/spf13/afero => github.com/aslilac/afero v0.0.0-20250403163713
require (
cdr.dev/slog v1.6.2-0.20241112041820-0ec81e6e67bb
- cloud.google.com/go/compute/metadata v0.6.0
+ cloud.google.com/go/compute/metadata v0.7.0
github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d
github.com/adrg/xdg v0.5.0
github.com/ammario/tlru v0.4.0
@@ -96,12 +96,12 @@ require (
github.com/chromedp/chromedp v0.13.3
github.com/cli/safeexec v1.0.1
github.com/coder/flog v1.1.0
- github.com/coder/guts v1.3.1-0.20250428170043-ad369017e95b
+ github.com/coder/guts v1.5.0
github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0
github.com/coder/quartz v0.1.3
github.com/coder/retry v1.5.1
github.com/coder/serpent v0.10.0
- github.com/coder/terraform-provider-coder/v2 v2.4.1
+ github.com/coder/terraform-provider-coder/v2 v2.4.2
github.com/coder/websocket v1.8.13
github.com/coder/wgtunnel v0.1.13-0.20240522110300-ade90dfb2da0
github.com/coreos/go-oidc/v3 v3.14.1
@@ -140,7 +140,7 @@ require (
github.com/hashicorp/go-version v1.7.0
github.com/hashicorp/hc-install v0.9.2
github.com/hashicorp/terraform-config-inspect v0.0.0-20211115214459-90acf1ca460f
- github.com/hashicorp/terraform-json v0.24.0
+ github.com/hashicorp/terraform-json v0.25.0
github.com/hashicorp/yamux v0.1.2
github.com/hinshun/vt10x v0.0.0-20220301184237-5011da428d02
github.com/imulab/go-scim/pkg/v2 v2.2.0
@@ -204,7 +204,7 @@ require (
golang.org/x/sys v0.33.0
golang.org/x/term v0.32.0
golang.org/x/text v0.25.0 // indirect
- golang.org/x/tools v0.32.0
+ golang.org/x/tools v0.33.0
golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da
google.golang.org/api v0.231.0
google.golang.org/grpc v1.72.0
@@ -485,10 +485,10 @@ require (
require (
github.com/anthropics/anthropic-sdk-go v0.2.0-beta.3
- github.com/coder/preview v0.0.2-0.20250510235314-66630669ff6f
+ github.com/coder/preview v0.0.2-0.20250520134327-ac391431027d
github.com/fsnotify/fsnotify v1.9.0
github.com/kylecarbs/aisdk-go v0.0.8
- github.com/mark3labs/mcp-go v0.27.0
+ github.com/mark3labs/mcp-go v0.28.0
github.com/openai/openai-go v0.1.0-beta.10
google.golang.org/genai v0.7.0
)
diff --git a/go.sum b/go.sum
index 3d635991b7abe..0f5638614d275 100644
--- a/go.sum
+++ b/go.sum
@@ -184,8 +184,8 @@ cloud.google.com/go/compute/metadata v0.1.0/go.mod h1:Z1VN+bulIf6bt4P/C37K4DyZYZ
cloud.google.com/go/compute/metadata v0.2.0/go.mod h1:zFmK7XCadkQkj6TtorcaGlCW1hT1fIilQDwofLpJ20k=
cloud.google.com/go/compute/metadata v0.2.1/go.mod h1:jgHgmJd2RKBGzXqF5LR2EZMGxBkeanZ9wwa75XHJgOM=
cloud.google.com/go/compute/metadata v0.2.3/go.mod h1:VAV5nSsACxMJvgaAuX6Pk2AawlZn8kiOGuCv6gTkwuA=
-cloud.google.com/go/compute/metadata v0.6.0 h1:A6hENjEsCDtC1k8byVsgwvVcioamEHvZ4j01OwKxG9I=
-cloud.google.com/go/compute/metadata v0.6.0/go.mod h1:FjyFAW1MW0C203CEOMDTu3Dk1FlqW3Rga40jzHL4hfg=
+cloud.google.com/go/compute/metadata v0.7.0 h1:PBWF+iiAerVNe8UCHxdOt6eHLVc3ydFeOCw78U8ytSU=
+cloud.google.com/go/compute/metadata v0.7.0/go.mod h1:j5MvL9PprKL39t166CoB1uVHfQMs4tFQZZcKwksXUjo=
cloud.google.com/go/contactcenterinsights v1.3.0/go.mod h1:Eu2oemoePuEFc/xKFPjbTuPSj0fYJcPls9TFlPNnHHY=
cloud.google.com/go/contactcenterinsights v1.4.0/go.mod h1:L2YzkGbPsv+vMQMCADxJoT9YiTTnSEd6fEvCeHTYVck=
cloud.google.com/go/contactcenterinsights v1.6.0/go.mod h1:IIDlT6CLcDoyv79kDv8iWxMSTZhLxSCofVV5W6YFM/w=
@@ -905,14 +905,14 @@ github.com/coder/go-httpstat v0.0.0-20230801153223-321c88088322 h1:m0lPZjlQ7vdVp
github.com/coder/go-httpstat v0.0.0-20230801153223-321c88088322/go.mod h1:rOLFDDVKVFiDqZFXoteXc97YXx7kFi9kYqR+2ETPkLQ=
github.com/coder/go-scim/pkg/v2 v2.0.0-20230221055123-1d63c1222136 h1:0RgB61LcNs24WOxc3PBvygSNTQurm0PYPujJjLLOzs0=
github.com/coder/go-scim/pkg/v2 v2.0.0-20230221055123-1d63c1222136/go.mod h1:VkD1P761nykiq75dz+4iFqIQIZka189tx1BQLOp0Skc=
-github.com/coder/guts v1.3.1-0.20250428170043-ad369017e95b h1:tfLKcE2s6D7YpFk7MUUCDE0Xbbmac+k2GqO8KMjv/Ug=
-github.com/coder/guts v1.3.1-0.20250428170043-ad369017e95b/go.mod h1:31NO4z6MVTOD4WaCLqE/hUAHGgNok9sRbuMc/LZFopI=
+github.com/coder/guts v1.5.0 h1:a94apf7xMf5jDdg1bIHzncbRiTn3+BvBZgrFSDbUnyI=
+github.com/coder/guts v1.5.0/go.mod h1:0Sbv5Kp83u1Nl7MIQiV2zmacJ3o02I341bkWkjWXSUQ=
github.com/coder/pq v1.10.5-0.20240813183442-0c420cb5a048 h1:3jzYUlGH7ZELIH4XggXhnTnP05FCYiAFeQpoN+gNR5I=
github.com/coder/pq v1.10.5-0.20240813183442-0c420cb5a048/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0 h1:3A0ES21Ke+FxEM8CXx9n47SZOKOpgSE1bbJzlE4qPVs=
github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0/go.mod h1:5UuS2Ts+nTToAMeOjNlnHFkPahrtDkmpydBen/3wgZc=
-github.com/coder/preview v0.0.2-0.20250510235314-66630669ff6f h1:Q1AcLBpRRR8rf/H+GxcJaZ5Ox4UeIRkDgc6wvvmOJAo=
-github.com/coder/preview v0.0.2-0.20250510235314-66630669ff6f/go.mod h1:GfkwIv5gQLpL01qeGU1/YoxoFtt5trzCqnWZLo77clU=
+github.com/coder/preview v0.0.2-0.20250520134327-ac391431027d h1:MxAAuqcno5hMM45Ihl3KAjVOXbyZyt/+tjSiq9XMTC0=
+github.com/coder/preview v0.0.2-0.20250520134327-ac391431027d/go.mod h1:9bwyhQSVDjcxAWuFFaG6/qBqhaiW5oqF5PEQMhevKLs=
github.com/coder/quartz v0.1.3 h1:hA2nI8uUA2fNN9uhXv2I4xZD4aHkA7oH3g2t03v4xf8=
github.com/coder/quartz v0.1.3/go.mod h1:vsiCc+AHViMKH2CQpGIpFgdHIEQsxwm8yCscqKmzbRA=
github.com/coder/retry v1.5.1 h1:iWu8YnD8YqHs3XwqrqsjoBTAVqT9ml6z9ViJ2wlMiqc=
@@ -925,8 +925,8 @@ github.com/coder/tailscale v1.1.1-0.20250422090654-5090e715905e h1:nope/SZfoLB9M
github.com/coder/tailscale v1.1.1-0.20250422090654-5090e715905e/go.mod h1:1ggFFdHTRjPRu9Yc1yA7nVHBYB50w9Ce7VIXNqcW6Ko=
github.com/coder/terraform-config-inspect v0.0.0-20250107175719-6d06d90c630e h1:JNLPDi2P73laR1oAclY6jWzAbucf70ASAvf5mh2cME0=
github.com/coder/terraform-config-inspect v0.0.0-20250107175719-6d06d90c630e/go.mod h1:Gz/z9Hbn+4KSp8A2FBtNszfLSdT2Tn/uAKGuVqqWmDI=
-github.com/coder/terraform-provider-coder/v2 v2.4.1 h1:+HxLJVENJ+kvGhibQ0jbr8Evi6M857d9691ytxNbv90=
-github.com/coder/terraform-provider-coder/v2 v2.4.1/go.mod h1:2kaBpn5k9ZWtgKq5k4JbkVZG9DzEqR4mJSmpdshcO+s=
+github.com/coder/terraform-provider-coder/v2 v2.4.2 h1:41SJkgwgiA555kwQzGIQcNS3bCm12sVMUmBSa5zGr+A=
+github.com/coder/terraform-provider-coder/v2 v2.4.2/go.mod h1:2kaBpn5k9ZWtgKq5k4JbkVZG9DzEqR4mJSmpdshcO+s=
github.com/coder/trivy v0.0.0-20250409153844-e6b004bc465a h1:yryP7e+IQUAArlycH4hQrjXQ64eRNbxsV5/wuVXHgME=
github.com/coder/trivy v0.0.0-20250409153844-e6b004bc465a/go.mod h1:dDvq9axp3kZsT63gY2Znd1iwzfqDq3kXbQnccIrjRYY=
github.com/coder/websocket v1.8.13 h1:f3QZdXy7uGVz+4uCJy2nTZyM0yTBj8yANEHhqlXZ9FE=
@@ -1385,8 +1385,8 @@ github.com/hashicorp/logutils v1.0.0 h1:dLEQVugN8vlakKOUE3ihGLTZJRB4j+M2cdTm/ORI
github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64=
github.com/hashicorp/terraform-exec v0.23.0 h1:MUiBM1s0CNlRFsCLJuM5wXZrzA3MnPYEsiXmzATMW/I=
github.com/hashicorp/terraform-exec v0.23.0/go.mod h1:mA+qnx1R8eePycfwKkCRk3Wy65mwInvlpAeOwmA7vlY=
-github.com/hashicorp/terraform-json v0.24.0 h1:rUiyF+x1kYawXeRth6fKFm/MdfBS6+lW4NbeATsYz8Q=
-github.com/hashicorp/terraform-json v0.24.0/go.mod h1:Nfj5ubo9xbu9uiAoZVBsNOjvNKB66Oyrvtit74kC7ow=
+github.com/hashicorp/terraform-json v0.25.0 h1:rmNqc/CIfcWawGiwXmRuiXJKEiJu1ntGoxseG1hLhoQ=
+github.com/hashicorp/terraform-json v0.25.0/go.mod h1:sMKS8fiRDX4rVlR6EJUMudg1WcanxCMoWwTLkgZP/vc=
github.com/hashicorp/terraform-plugin-go v0.26.0 h1:cuIzCv4qwigug3OS7iKhpGAbZTiypAfFQmw8aE65O2M=
github.com/hashicorp/terraform-plugin-go v0.26.0/go.mod h1:+CXjuLDiFgqR+GcrM5a2E2Kal5t5q2jb0E3D57tTdNY=
github.com/hashicorp/terraform-plugin-log v0.9.0 h1:i7hOA+vdAItN1/7UrfBqBwvYPQ9TFvymaRGZED3FCV0=
@@ -1506,8 +1506,8 @@ github.com/makeworld-the-better-one/dither/v2 v2.4.0 h1:Az/dYXiTcwcRSe59Hzw4RI1r
github.com/makeworld-the-better-one/dither/v2 v2.4.0/go.mod h1:VBtN8DXO7SNtyGmLiGA7IsFeKrBkQPze1/iAeM95arc=
github.com/marekm4/color-extractor v1.2.1 h1:3Zb2tQsn6bITZ8MBVhc33Qn1k5/SEuZ18mrXGUqIwn0=
github.com/marekm4/color-extractor v1.2.1/go.mod h1:90VjmiHI6M8ez9eYUaXLdcKnS+BAOp7w+NpwBdkJmpA=
-github.com/mark3labs/mcp-go v0.27.0 h1:iok9kU4DUIU2/XVLgFS2Q9biIDqstC0jY4EQTK2Erzc=
-github.com/mark3labs/mcp-go v0.27.0/go.mod h1:rXqOudj/djTORU/ThxYx8fqEVj/5pvTuuebQ2RC7uk4=
+github.com/mark3labs/mcp-go v0.28.0 h1:7yl4y5D1KYU2f/9Uxp7xfLIggfunHoESCRbrjcytcLM=
+github.com/mark3labs/mcp-go v0.28.0/go.mod h1:rXqOudj/djTORU/ThxYx8fqEVj/5pvTuuebQ2RC7uk4=
github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE=
github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc=
@@ -2412,8 +2412,8 @@ golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU=
golang.org/x/tools v0.7.0/go.mod h1:4pg6aUX35JBAogB10C9AtvVL+qowtN4pT3CGSQex14s=
golang.org/x/tools v0.13.0/go.mod h1:HvlwmtVNQAhOuCjW7xxvovg8wbNq7LwfXh/k7wXUl58=
golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d/go.mod h1:aiJjzUbINMkxbQROHiO6hDPo2LHcIPhhQsa9DLh0yGk=
-golang.org/x/tools v0.32.0 h1:Q7N1vhpkQv7ybVzLFtTjvQya2ewbwNDZzUgfXGqtMWU=
-golang.org/x/tools v0.32.0/go.mod h1:ZxrU41P/wAbZD8EDa6dDCa6XfpkhJ7HFMjHJXfBDu8s=
+golang.org/x/tools v0.33.0 h1:4qz2S3zmRxbGIhDIAgjxvFutSvH5EfnsYrRBj0UI0bc=
+golang.org/x/tools v0.33.0/go.mod h1:CIJMaWEY88juyUfo7UbgPqbC8rU2OqfAV1h2Qp0oMYI=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
diff --git a/provisioner/terraform/serve.go b/provisioner/terraform/serve.go
index 562946d8ef92e..3e671b0c68e56 100644
--- a/provisioner/terraform/serve.go
+++ b/provisioner/terraform/serve.go
@@ -16,7 +16,7 @@ import (
"cdr.dev/slog"
"github.com/coder/coder/v2/coderd/database"
- "github.com/coder/coder/v2/coderd/unhanger"
+ "github.com/coder/coder/v2/coderd/jobreaper"
"github.com/coder/coder/v2/provisionersdk"
)
@@ -39,9 +39,9 @@ type ServeOptions struct {
//
// This is a no-op on Windows where the process can't be interrupted.
//
- // Default value: 3 minutes (unhanger.HungJobExitTimeout). This value should
+ // Default value: 3 minutes (jobreaper.HungJobExitTimeout). This value should
// be kept less than the value that Coder uses to mark hung jobs as failed,
- // which is 5 minutes (see unhanger package).
+ // which is 5 minutes (see jobreaper package).
ExitTimeout time.Duration
}
@@ -131,7 +131,7 @@ func Serve(ctx context.Context, options *ServeOptions) error {
options.Tracer = trace.NewNoopTracerProvider().Tracer("noop")
}
if options.ExitTimeout == 0 {
- options.ExitTimeout = unhanger.HungJobExitTimeout
+ options.ExitTimeout = jobreaper.HungJobExitTimeout
}
return provisionersdk.Serve(ctx, &server{
execMut: &sync.Mutex{},
diff --git a/scripts/rules.go b/scripts/rules.go
index 4e16adad06a87..4175287567502 100644
--- a/scripts/rules.go
+++ b/scripts/rules.go
@@ -134,6 +134,42 @@ func databaseImport(m dsl.Matcher) {
Where(m.File().PkgPath.Matches("github.com/coder/coder/v2/codersdk"))
}
+// publishInTransaction detects calls to Publish inside database transactions
+// which can lead to connection starvation.
+//
+//nolint:unused,deadcode,varnamelen
+func publishInTransaction(m dsl.Matcher) {
+ m.Import("github.com/coder/coder/v2/coderd/database/pubsub")
+
+ // Match direct calls to the Publish method of a pubsub instance inside InTx
+ m.Match(`
+ $_.InTx(func($x) error {
+ $*_
+ $_ = $ps.Publish($evt, $msg)
+ $*_
+ }, $*_)
+ `,
+ // Alternative with short variable declaration
+ `
+ $_.InTx(func($x) error {
+ $*_
+ $_ := $ps.Publish($evt, $msg)
+ $*_
+ }, $*_)
+ `,
+ // Without catching error return
+ `
+ $_.InTx(func($x) error {
+ $*_
+ $ps.Publish($evt, $msg)
+ $*_
+ }, $*_)
+ `).
+ Where(m["ps"].Type.Is("pubsub.Pubsub")).
+ At(m["ps"]).
+ Report("Avoid calling pubsub.Publish() inside database transactions as this may lead to connection deadlocks. Move the Publish() call outside the transaction.")
+}
+
// doNotCallTFailNowInsideGoroutine enforces not calling t.FailNow or
// functions that may themselves call t.FailNow in goroutines outside
// the main test goroutine. See testing.go:834 for why.
diff --git a/site/src/api/queries/organizations.ts b/site/src/api/queries/organizations.ts
index c7b42f5f0e79f..608b2fa2a1ac4 100644
--- a/site/src/api/queries/organizations.ts
+++ b/site/src/api/queries/organizations.ts
@@ -187,7 +187,7 @@ const getProvisionerDaemonGroupsKey = (organization: string) => [
"provisionerDaemons",
];
-const provisionerDaemonGroups = (organization: string) => {
+export const provisionerDaemonGroups = (organization: string) => {
return {
queryKey: getProvisionerDaemonGroupsKey(organization),
queryFn: () => API.getProvisionerDaemonGroupsByOrganization(organization),
diff --git a/site/src/api/rbacresourcesGenerated.ts b/site/src/api/rbacresourcesGenerated.ts
index 079dcb4a87a61..885f603c1eb82 100644
--- a/site/src/api/rbacresourcesGenerated.ts
+++ b/site/src/api/rbacresourcesGenerated.ts
@@ -130,7 +130,9 @@ export const RBACResourceActions: Partial<
update: "update a provisioner daemon",
},
provisioner_jobs: {
+ create: "create provisioner jobs",
read: "read provisioner jobs",
+ update: "update provisioner jobs",
},
replicas: {
read: "read replicas",
@@ -171,7 +173,9 @@ export const RBACResourceActions: Partial<
workspace: {
application_connect: "connect to workspace apps via browser",
create: "create a new workspace",
+ create_agent: "create a new workspace agent",
delete: "delete workspace",
+ delete_agent: "delete an existing workspace agent",
read: "read workspace data to view on the UI",
ssh: "ssh into a given workspace",
start: "allows starting a workspace",
@@ -189,7 +193,9 @@ export const RBACResourceActions: Partial<
workspace_dormant: {
application_connect: "connect to workspace apps via browser",
create: "create a new workspace",
+ create_agent: "create a new workspace agent",
delete: "delete workspace",
+ delete_agent: "delete an existing workspace agent",
read: "read workspace data to view on the UI",
ssh: "ssh into a given workspace",
start: "allows starting a workspace",
diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts
index 6c09014c4ed6f..35cd006ec6c55 100644
--- a/site/src/api/typesGenerated.ts
+++ b/site/src/api/typesGenerated.ts
@@ -349,7 +349,7 @@ export interface ConvertLoginRequest {
// From codersdk/chat.go
export interface CreateChatMessageRequest {
readonly model: string;
- // embedded anonymous struct, please fix by naming it
+ // external type "github.com/kylecarbs/aisdk-go.Message", to include this type the package must be explicitly included in the parsing
readonly message: unknown;
readonly thinking: boolean;
}
@@ -490,6 +490,7 @@ export interface CreateWorkspaceBuildRequest {
readonly rich_parameter_values?: readonly WorkspaceBuildParameter[];
readonly log_level?: ProvisionerLogLevel;
readonly template_version_preset_id?: string;
+ readonly enable_dynamic_parameters?: boolean;
}
// From codersdk/workspaceproxy.go
@@ -740,6 +741,19 @@ export interface DeploymentValues {
readonly address?: string;
}
+// From codersdk/parameters.go
+export interface DiagnosticExtra {
+ readonly code: string;
+}
+
+// From codersdk/parameters.go
+export type DiagnosticSeverityString = "error" | "warning";
+
+export const DiagnosticSeverityStrings: DiagnosticSeverityString[] = [
+ "error",
+ "warning",
+];
+
// From codersdk/workspaceagents.go
export type DisplayApp =
| "port_forwarding_helper"
@@ -756,16 +770,16 @@ export const DisplayApps: DisplayApp[] = [
"web_terminal",
];
-// From codersdk/templateversions.go
+// From codersdk/parameters.go
export interface DynamicParametersRequest {
readonly id: number;
readonly inputs: Record- Copy the session token below and - {/* - * This looks silly, but it's a case where you want to hide the space - * visually because it messes up the centering, but you want the space - * to still be available to screen readers - */} - {VISUALLY_HIDDEN_SPACE} - paste it in your terminal. +
+ Copy the session token below and{" "} + paste it in your terminal.
-