Skip to content

Commit 395a261

Browse files
dannykoppingdannykopping
committed
Merge branch 'yevhenii/512-claim-prebuild' of github.com:/coder/coder into dk/enable-prebuilds
2 parents b6f378c + 7f25f24 commit 395a261

File tree

29 files changed

+577
-142
lines changed

29 files changed

+577
-142
lines changed

‎Makefile

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -813,8 +813,8 @@ coderd/apidoc/swagger.json: site/node_modules/.installed coderd/apidoc/.gen
813813
touch "$@"
814814

815815
update-golden-files:
816-
echo 'WARNING: This target is deprecated. Use "make gen/golden-files" instead.' 2>&1
817-
echo 'Running "make gen/golden-files"' 2>&1
816+
echo 'WARNING: This target is deprecated. Use "make gen/golden-files" instead.' >&2
817+
echo 'Running "make gen/golden-files"' >&2
818818
make gen/golden-files
819819
.PHONY: update-golden-files
820820

@@ -834,39 +834,39 @@ clean/golden-files:
834834
.PHONY: clean/golden-files
835835

836836
cli/testdata/.gen-golden: $(wildcard cli/testdata/*.golden) $(wildcard cli/*.tpl) $(GO_SRC_FILES) $(wildcard cli/*_test.go)
837-
go test ./cli -run="Test(CommandHelp|ServerYAML|ErrorExamples|.*Golden)" -update
837+
TZ=UTC go test ./cli -run="Test(CommandHelp|ServerYAML|ErrorExamples|.*Golden)" -update
838838
touch "$@"
839839

840840
enterprise/cli/testdata/.gen-golden: $(wildcard enterprise/cli/testdata/*.golden) $(wildcard cli/*.tpl) $(GO_SRC_FILES) $(wildcard enterprise/cli/*_test.go)
841-
go test ./enterprise/cli -run="TestEnterpriseCommandHelp" -update
841+
TZ=UTC go test ./enterprise/cli -run="TestEnterpriseCommandHelp" -update
842842
touch "$@"
843843

844844
tailnet/testdata/.gen-golden: $(wildcard tailnet/testdata/*.golden.html) $(GO_SRC_FILES) $(wildcard tailnet/*_test.go)
845-
go test ./tailnet -run="TestDebugTemplate" -update
845+
TZ=UTC go test ./tailnet -run="TestDebugTemplate" -update
846846
touch "$@"
847847

848848
enterprise/tailnet/testdata/.gen-golden: $(wildcard enterprise/tailnet/testdata/*.golden.html) $(GO_SRC_FILES) $(wildcard enterprise/tailnet/*_test.go)
849-
go test ./enterprise/tailnet -run="TestDebugTemplate" -update
849+
TZ=UTC go test ./enterprise/tailnet -run="TestDebugTemplate" -update
850850
touch "$@"
851851

852852
helm/coder/tests/testdata/.gen-golden: $(wildcard helm/coder/tests/testdata/*.yaml) $(wildcard helm/coder/tests/testdata/*.golden) $(GO_SRC_FILES) $(wildcard helm/coder/tests/*_test.go)
853-
go test ./helm/coder/tests -run=TestUpdateGoldenFiles -update
853+
TZ=UTC go test ./helm/coder/tests -run=TestUpdateGoldenFiles -update
854854
touch "$@"
855855

856856
helm/provisioner/tests/testdata/.gen-golden: $(wildcard helm/provisioner/tests/testdata/*.yaml) $(wildcard helm/provisioner/tests/testdata/*.golden) $(GO_SRC_FILES) $(wildcard helm/provisioner/tests/*_test.go)
857-
go test ./helm/provisioner/tests -run=TestUpdateGoldenFiles -update
857+
TZ=UTC go test ./helm/provisioner/tests -run=TestUpdateGoldenFiles -update
858858
touch "$@"
859859

860860
coderd/.gen-golden: $(wildcard coderd/testdata/*/*.golden) $(GO_SRC_FILES) $(wildcard coderd/*_test.go)
861-
go test ./coderd -run="Test.*Golden$$" -update
861+
TZ=UTC go test ./coderd -run="Test.*Golden$$" -update
862862
touch "$@"
863863

864864
coderd/notifications/.gen-golden: $(wildcard coderd/notifications/testdata/*/*.golden) $(GO_SRC_FILES) $(wildcard coderd/notifications/*_test.go)
865-
go test ./coderd/notifications -run="Test.*Golden$$" -update
865+
TZ=UTC go test ./coderd/notifications -run="Test.*Golden$$" -update
866866
touch "$@"
867867

868868
provisioner/terraform/testdata/.gen-golden: $(wildcard provisioner/terraform/testdata/*/*.golden) $(GO_SRC_FILES) $(wildcard provisioner/terraform/*_test.go)
869-
go test ./provisioner/terraform -run="Test.*Golden$$" -update
869+
TZ=UTC go test ./provisioner/terraform -run="Test.*Golden$$" -update
870870
touch "$@"
871871

872872
provisioner/terraform/testdata/version:

‎coderd/apidoc/docs.go

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

‎coderd/apidoc/swagger.json

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

‎coderd/prebuilds/api.go

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import (
55

66
"github.com/google/uuid"
77
"golang.org/x/xerrors"
8-
9-
"github.com/coder/coder/v2/coderd/database"
108
)
119

1210
var ErrNoClaimablePrebuiltWorkspaces = xerrors.New("no claimable prebuilt workspaces found")
@@ -26,38 +24,14 @@ type ReconciliationOrchestrator interface {
2624
Stop(ctx context.Context, cause error)
2725
}
2826

29-
// Reconciler defines the core operations for managing prebuilds.
30-
// It provides both high-level orchestration (ReconcileAll) and lower-level operations
31-
// for more fine-grained control (SnapshotState, ReconcilePreset, CalculateActions).
32-
// All database operations must be performed within repeatable-read transactions
33-
// to ensure consistency.
3427
type Reconciler interface {
3528
// ReconcileAll orchestrates the reconciliation of all prebuilds across all templates.
3629
// It takes a global snapshot of the system state and then reconciles each preset
3730
// in parallel, creating or deleting prebuilds as needed to reach their desired states.
38-
// For more fine-grained control, you can use the lower-level methods SnapshotState
39-
// and ReconcilePreset directly.
4031
ReconcileAll(ctx context.Context) error
41-
42-
// SnapshotState captures the current state of all prebuilds across templates.
43-
// It creates a global database snapshot that can be viewed as a collection of PresetSnapshots,
44-
// each representing the state of prebuilds for a specific preset.
45-
// MUST be called inside a repeatable-read transaction.
46-
SnapshotState(ctx context.Context, store database.Store) (*GlobalSnapshot, error)
47-
48-
// ReconcilePreset handles a single PresetSnapshot, determining and executing
49-
// the required actions (creating or deleting prebuilds) based on the current state.
50-
// MUST be called inside a repeatable-read transaction.
51-
ReconcilePreset(ctx context.Context, snapshot PresetSnapshot) error
52-
53-
// CalculateActions determines what actions are needed to reconcile a preset's prebuilds
54-
// to their desired state. This includes creating new prebuilds, deleting excess ones,
55-
// or waiting due to backoff periods.
56-
// MUST be called inside a repeatable-read transaction.
57-
CalculateActions(ctx context.Context, state PresetSnapshot) (*ReconciliationActions, error)
5832
}
5933

6034
type Claimer interface {
61-
Claim(ctx context.Context, store database.Store, userID uuid.UUID, name string, presetID uuid.UUID) (*uuid.UUID, error)
35+
Claim(ctx context.Context, userID uuid.UUID, name string, presetID uuid.UUID) (*uuid.UUID, error)
6236
Initiator() uuid.UUID
6337
}

‎coderd/prebuilds/noop.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ var DefaultReconciler ReconciliationOrchestrator = NoopReconciler{}
2525

2626
type NoopClaimer struct{}
2727

28-
func (NoopClaimer) Claim(context.Context, database.Store, uuid.UUID, string, uuid.UUID) (*uuid.UUID, error) {
28+
func (NoopClaimer) Claim(context.Context, uuid.UUID, string, uuid.UUID) (*uuid.UUID, error) {
2929
// Not entitled to claim prebuilds in AGPL version.
3030
return nil, ErrNoClaimablePrebuiltWorkspaces
3131
}

‎coderd/provisionerdserver/provisionerdserver.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,9 @@ func (s *server) acquireProtoJob(ctx context.Context, job database.ProvisionerJo
515515
}
516516

517517
var workspaceOwnerOIDCAccessToken string
518-
if s.OIDCConfig != nil {
518+
// The check `s.OIDCConfig != nil` is not as strict, since it can be an interface
519+
// pointing to a typed nil.
520+
if !reflect.ValueOf(s.OIDCConfig).IsNil() {
519521
workspaceOwnerOIDCAccessToken, err = obtainOIDCAccessToken(ctx, s.Database, s.OIDCConfig, owner.ID)
520522
if err != nil {
521523
return nil, failJob(fmt.Sprintf("obtain OIDC access token: %s", err))

‎coderd/workspaces.go

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -639,12 +639,11 @@ func createWorkspace(
639639
provisionerDaemons []database.GetEligibleProvisionerDaemonsByProvisionerJobIDsRow
640640
)
641641

642-
prebuildsClaimer := *api.PrebuildsClaimer.Load()
643-
644642
err = api.Database.InTx(func(db database.Store) error {
645643
var (
646644
workspaceID uuid.UUID
647645
claimedWorkspace *database.Workspace
646+
prebuildsClaimer = *api.PrebuildsClaimer.Load()
648647
)
649648

650649
// If a template preset was chosen, try claim a prebuilt workspace.
@@ -706,11 +705,14 @@ func createWorkspace(
706705
if req.TemplateVersionPresetID != uuid.Nil {
707706
builder = builder.TemplateVersionPresetID(req.TemplateVersionPresetID)
708707
}
709-
710708
if claimedWorkspace != nil {
711709
builder = builder.MarkPrebuildClaimedBy(owner.ID)
712710
}
713711

712+
if req.EnableDynamicParameters && api.Experiments.Enabled(codersdk.ExperimentDynamicParameters) {
713+
builder = builder.UsingDynamicParameters()
714+
}
715+
714716
workspaceBuild, provisionerJob, provisionerDaemons, err = builder.Build(
715717
ctx,
716718
db,
@@ -876,20 +878,16 @@ func requestTemplate(ctx context.Context, rw http.ResponseWriter, req codersdk.C
876878
func claimPrebuild(ctx context.Context, claimer prebuilds.Claimer, db database.Store, logger slog.Logger, req codersdk.CreateWorkspaceRequest, owner workspaceOwner) (*database.Workspace, error) {
877879
prebuildsCtx := dbauthz.AsPrebuildsOrchestrator(ctx)
878880

879-
// TODO: do we need a timeout here?
880-
claimCtx, cancel := context.WithTimeout(prebuildsCtx, time.Second*10)
881-
defer cancel()
882-
883-
claimedID, err := claimer.Claim(claimCtx, db, owner.ID, req.Name, req.TemplateVersionPresetID)
881+
claimedID, err := claimer.Claim(prebuildsCtx, owner.ID, req.Name, req.TemplateVersionPresetID)
884882
if err != nil {
885883
// TODO: enhance this by clarifying whether this *specific* prebuild failed or whether there are none to claim.
886884
return nil, xerrors.Errorf("claim prebuild: %w", err)
887885
}
888886

889887
lookup, err := db.GetWorkspaceByID(prebuildsCtx, *claimedID)
890888
if err != nil {
891-
logger.Error(ctx, "unable to find claimed workspace by ID", slog.Error(err), slog.F("claimed_prebuild_id", (*claimedID).String()))
892-
return nil, xerrors.Errorf("find claimed workspace by ID %q: %w", (*claimedID).String(), err)
889+
logger.Error(ctx, "unable to find claimed workspace by ID", slog.Error(err), slog.F("claimed_prebuild_id", claimedID.String()))
890+
return nil, xerrors.Errorf("find claimed workspace by ID %q: %w", claimedID.String(), err)
893891
}
894892
return &lookup, nil
895893
}

‎coderd/workspaces_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4349,3 +4349,51 @@ func TestWorkspaceTimings(t *testing.T) {
43494349
require.Contains(t, err.Error(), "not found")
43504350
})
43514351
}
4352+
4353+
// TestOIDCRemoved emulates a user logging in with OIDC, then that OIDC
4354+
// auth method being removed.
4355+
func TestOIDCRemoved(t *testing.T) {
4356+
t.Parallel()
4357+
4358+
owner, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{
4359+
IncludeProvisionerDaemon: true,
4360+
})
4361+
first := coderdtest.CreateFirstUser(t, owner)
4362+
4363+
user, userData := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.ScopedRoleOrgAdmin(first.OrganizationID))
4364+
4365+
ctx := testutil.Context(t, testutil.WaitMedium)
4366+
//nolint:gocritic // unit test
4367+
_, err := db.UpdateUserLoginType(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLoginTypeParams{
4368+
NewLoginType: database.LoginTypeOIDC,
4369+
UserID: userData.ID,
4370+
})
4371+
require.NoError(t, err)
4372+
4373+
//nolint:gocritic // unit test
4374+
_, err = db.InsertUserLink(dbauthz.AsSystemRestricted(ctx), database.InsertUserLinkParams{
4375+
UserID: userData.ID,
4376+
LoginType: database.LoginTypeOIDC,
4377+
LinkedID: "random",
4378+
OAuthAccessToken: "foobar",
4379+
OAuthAccessTokenKeyID: sql.NullString{},
4380+
OAuthRefreshToken: "refresh",
4381+
OAuthRefreshTokenKeyID: sql.NullString{},
4382+
OAuthExpiry: time.Now().Add(time.Hour * -1),
4383+
Claims: database.UserLinkClaims{},
4384+
})
4385+
require.NoError(t, err)
4386+
4387+
version := coderdtest.CreateTemplateVersion(t, owner, first.OrganizationID, nil)
4388+
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, owner, version.ID)
4389+
template := coderdtest.CreateTemplate(t, owner, first.OrganizationID, version.ID)
4390+
4391+
wrk := coderdtest.CreateWorkspace(t, user, template.ID)
4392+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, owner, wrk.LatestBuild.ID)
4393+
4394+
deleteBuild, err := owner.CreateWorkspaceBuild(ctx, wrk.ID, codersdk.CreateWorkspaceBuildRequest{
4395+
Transition: codersdk.WorkspaceTransitionDelete,
4396+
})
4397+
require.NoError(t, err, "delete the workspace")
4398+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, owner, deleteBuild.ID)
4399+
}

‎coderd/wsbuilder/wsbuilder.go

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,11 @@ type Builder struct {
5151
logLevel string
5252
deploymentValues *codersdk.DeploymentValues
5353

54-
richParameterValues []codersdk.WorkspaceBuildParameter
55-
initiator uuid.UUID
56-
reason database.BuildReason
57-
templateVersionPresetID uuid.UUID
54+
richParameterValues []codersdk.WorkspaceBuildParameter
55+
dynamicParametersEnabled bool
56+
initiator uuid.UUID
57+
reason database.BuildReason
58+
templateVersionPresetID uuid.UUID
5859

5960
// used during build, makes function arguments less verbose
6061
ctx context.Context
@@ -185,6 +186,11 @@ func (b Builder) MarkPrebuildClaimedBy(userID uuid.UUID) Builder {
185186
return b
186187
}
187188

189+
func (b Builder) UsingDynamicParameters() Builder {
190+
b.dynamicParametersEnabled = true
191+
return b
192+
}
193+
188194
// SetLastWorkspaceBuildInTx prepopulates the Builder's cache with the last workspace build. This allows us
189195
// to avoid a repeated database query when the Builder's caller also needs the workspace build, e.g. auto-start &
190196
// auto-stop.
@@ -586,6 +592,7 @@ func (b *Builder) getParameters() (names, values []string, err error) {
586592
if err != nil {
587593
return nil, nil, BuildError{http.StatusBadRequest, "Unable to build workspace with unsupported parameters", err}
588594
}
595+
589596
resolver := codersdk.ParameterResolver{
590597
Rich: db2sdk.WorkspaceBuildParameters(lastBuildParameters),
591598
}
@@ -594,16 +601,24 @@ func (b *Builder) getParameters() (names, values []string, err error) {
594601
if err != nil {
595602
return nil, nil, BuildError{http.StatusInternalServerError, "failed to convert template version parameter", err}
596603
}
597-
value, err := resolver.ValidateResolve(
598-
tvp,
599-
b.findNewBuildParameterValue(templateVersionParameter.Name),
600-
)
601-
if err != nil {
602-
// At this point, we've queried all the data we need from the database,
603-
// so the only errors are problems with the request (missing data, failed
604-
// validation, immutable parameters, etc.)
605-
return nil, nil, BuildError{http.StatusBadRequest, fmt.Sprintf("Unable to validate parameter %q", templateVersionParameter.Name), err}
604+
605+
var value string
606+
if !b.dynamicParametersEnabled {
607+
var err error
608+
value, err = resolver.ValidateResolve(
609+
tvp,
610+
b.findNewBuildParameterValue(templateVersionParameter.Name),
611+
)
612+
if err != nil {
613+
// At this point, we've queried all the data we need from the database,
614+
// so the only errors are problems with the request (missing data, failed
615+
// validation, immutable parameters, etc.)
616+
return nil, nil, BuildError{http.StatusBadRequest, fmt.Sprintf("Unable to validate parameter %q", templateVersionParameter.Name), err}
617+
}
618+
} else {
619+
value = resolver.Resolve(tvp, b.findNewBuildParameterValue(templateVersionParameter.Name))
606620
}
621+
607622
names = append(names, templateVersionParameter.Name)
608623
values = append(values, value)
609624
}

‎codersdk/organizations.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ type CreateWorkspaceRequest struct {
227227
RichParameterValues []WorkspaceBuildParameter `json:"rich_parameter_values,omitempty"`
228228
AutomaticUpdates AutomaticUpdates `json:"automatic_updates,omitempty"`
229229
TemplateVersionPresetID uuid.UUID `json:"template_version_preset_id,omitempty" format:"uuid"`
230+
EnableDynamicParameters bool `json:"enable_dynamic_parameters,omitempty"`
230231
}
231232

232233
func (c *Client) OrganizationByName(ctx context.Context, name string) (Organization, error) {

0 commit comments

Comments
 (0)