pass atomic pointer to autobuild

coder · sreya · Oct 18, 2023 · Oct 11, 2023 · Oct 11, 2023 · Oct 11, 2023
commit 8a4e0bf9de0ccb6d2e833a24731e0ac65b75820f
diff --git a/cli/server.go b/cli/server.go
@@ -938,7 +938,7 @@ 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, coderAPI.TemplateScheduleStore, &coderAPI.Auditor, logger, autobuildTicker.C)
+				ctx, options.Database, options.Pubsub, coderAPI.TemplateScheduleStore, &coderAPI.Auditor, coderAPI.AccessControlStore, logger, autobuildTicker.C)
 			autobuildExecutor.Run()
 
 			hangDetectorTicker := time.NewTicker(vals.JobHangDetectorInterval.Value())

diff --git a/cli/templates.go b/cli/templates.go
@@ -3,10 +3,11 @@ package cli
 import (
 	"time"
 
-	"github.com/coder/pretty"
 	"github.com/google/uuid"
 	"golang.org/x/xerrors"
 
+	"github.com/coder/pretty"
+
 	"github.com/coder/coder/v2/cli/clibase"
 	"github.com/coder/coder/v2/cli/cliui"
 	"github.com/coder/coder/v2/codersdk"

diff --git a/cli/templateversionarchive.go b/cli/templateversionarchive.go
@@ -6,9 +6,10 @@ import (
 	"strings"
 	"time"
 
-	"github.com/coder/pretty"
 	"golang.org/x/xerrors"
 
+	"github.com/coder/pretty"
+
 	"github.com/coder/coder/v2/cli/clibase"
 	"github.com/coder/coder/v2/cli/cliui"
 	"github.com/coder/coder/v2/codersdk"

diff --git a/cli/templateversions.go b/cli/templateversions.go
@@ -5,10 +5,11 @@ import (
 	"strings"
 	"time"
 
-	"github.com/coder/pretty"
 	"github.com/google/uuid"
 	"golang.org/x/xerrors"
 
+	"github.com/coder/pretty"
+
 	"github.com/coder/coder/v2/cli/clibase"
 	"github.com/coder/coder/v2/cli/cliui"
 	"github.com/coder/coder/v2/codersdk"

diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go
@@ -32,6 +32,7 @@ type Executor struct {
 	db                    database.Store
 	ps                    pubsub.Pubsub
 	templateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore]
+	accessControlStore    *atomic.Pointer[dbauthz.AccessControlStore]
 	auditor               *atomic.Pointer[audit.Auditor]
 	log                   slog.Logger
 	tick                  <-chan time.Time
@@ -46,7 +47,7 @@ type Stats struct {
 }
 
 // New returns a new wsactions executor.
-func NewExecutor(ctx context.Context, db database.Store, ps pubsub.Pubsub, tss *atomic.Pointer[schedule.TemplateScheduleStore], auditor *atomic.Pointer[audit.Auditor], log slog.Logger, tick <-chan time.Time) *Executor {
+func NewExecutor(ctx context.Context, db database.Store, ps pubsub.Pubsub, tss *atomic.Pointer[schedule.TemplateScheduleStore], auditor *atomic.Pointer[audit.Auditor], acs *atomic.Pointer[dbauthz.AccessControlStore], log slog.Logger, tick <-chan time.Time) *Executor {
 	le := &Executor{
 		//nolint:gocritic // Autostart has a limited set of permissions.
 		ctx:                   dbauthz.AsAutostart(ctx),
@@ -56,6 +57,7 @@ func NewExecutor(ctx context.Context, db database.Store, ps pubsub.Pubsub, tss *
 		tick:                  tick,
 		log:                   log.Named("autobuild"),
 		auditor:               auditor,
+		accessControlStore:    acs,
 	}
 	return le
 }
@@ -159,6 +161,12 @@ func (e *Executor) runOnce(t time.Time) Stats {
 					return nil
 				}
 
+				template, err := tx.GetTemplateByID(e.ctx, ws.TemplateID)
+				if err != nil {
+					log.Warn(e.ctx, "get template by id", slog.Error(err))
+				}
+				accessControl := (*(e.accessControlStore.Load())).GetTemplateAccessControl(template)
+
 				latestJob, err := tx.GetProvisionerJobByID(e.ctx, latestBuild.JobID)
 				if err != nil {
 					log.Warn(e.ctx, "get last provisioner job for workspace %q: %w", slog.Error(err))
@@ -179,7 +187,7 @@ func (e *Executor) runOnce(t time.Time) Stats {
 						Reason(reason)
 					log.Debug(e.ctx, "auto building workspace", slog.F("transition", nextTransition))
 					if nextTransition == database.WorkspaceTransitionStart &&
-						useActiveVersion(templateSchedule, ws) {
+						useActiveVersion(accessControl, ws) {
 						log.Debug(e.ctx, "autostarting with active version")
 						builder = builder.ActiveVersion()
 					}
@@ -470,6 +478,6 @@ func auditBuild(ctx context.Context, log slog.Logger, auditor audit.Auditor, par
 	})
 }
 
-func useActiveVersion(opts schedule.TemplateScheduleOptions, ws database.Workspace) bool {
+func useActiveVersion(opts dbauthz.TemplateAccessControl, ws database.Workspace) bool {
 	return opts.RequireActiveVersion || ws.AutomaticUpdates == database.AutomaticUpdatesAlways
 }
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -131,6 +131,7 @@ type Options struct {
 	SetUserSiteRoles            func(ctx context.Context, logger slog.Logger, tx database.Store, userID uuid.UUID, roles []string) error
 	TemplateScheduleStore       *atomic.Pointer[schedule.TemplateScheduleStore]
 	UserQuietHoursScheduleStore *atomic.Pointer[schedule.UserQuietHoursScheduleStore]
+	AccessControlStore          *atomic.Pointer[dbauthz.AccessControlStore]
 	// AppSecurityKey is the crypto key used to sign and encrypt tokens related to
 	// workspace applications. It consists of both a signing and encryption key.
 	AppSecurityKey     workspaceapps.SecurityKey
@@ -209,15 +210,17 @@ func New(options *Options) *API {
 		options.Authorizer = rbac.NewCachingAuthorizer(options.PrometheusRegistry)
 	}
 
-	acs := &atomic.Pointer[dbauthz.AccessControlStore]{}
-	var tacs dbauthz.AccessControlStore = dbauthz.AGPLTemplateAccessControlStore{}
-	acs.Store(&tacs)
+	if options.AccessControlStore == nil {
+		options.AccessControlStore = &atomic.Pointer[dbauthz.AccessControlStore]{}
+		var tacs dbauthz.AccessControlStore = dbauthz.AGPLTemplateAccessControlStore{}
+		options.AccessControlStore.Store(&tacs)
+	}
 
 	options.Database = dbauthz.New(
 		options.Database,
 		options.Authorizer,
 		options.Logger.Named("authz_querier"),
-		acs,
+		options.AccessControlStore,
 	)
 
 	experiments := ReadExperiments(
@@ -376,7 +379,7 @@ func New(options *Options) *API {
 		Auditor:                     atomic.Pointer[audit.Auditor]{},
 		TemplateScheduleStore:       options.TemplateScheduleStore,
 		UserQuietHoursScheduleStore: options.UserQuietHoursScheduleStore,
-		AccessControlStore:          acs,
+		AccessControlStore:          options.AccessControlStore,
 		Experiments:                 experiments,
 		healthCheckGroup:            &singleflight.Group[string, *healthcheck.Report]{},
 		Acquirer: provisionerdserver.NewAcquirer(

diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go
@@ -259,6 +259,10 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
 		t.Cleanup(closeBatcher)
 	}
 
+	var accessControlStore = &atomic.Pointer[dbauthz.AccessControlStore]{}
+	var acs dbauthz.AccessControlStore = dbauthz.AGPLTemplateAccessControlStore{}
+	accessControlStore.Store(&acs)
+
 	var templateScheduleStore atomic.Pointer[schedule.TemplateScheduleStore]
 	if options.TemplateScheduleStore == nil {
 		options.TemplateScheduleStore = schedule.NewAGPLTemplateScheduleStore()
@@ -278,6 +282,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
 		options.Pubsub,
 		&templateScheduleStore,
 		&auditor,
+		accessControlStore,
 		*options.Logger,
 		options.AutobuildTicker,
 	).WithStatsChannel(options.AutobuildStats)
@@ -415,6 +420,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
 			Authorizer:                         options.Authorizer,
 			Telemetry:                          telemetry.NewNoop(),
 			TemplateScheduleStore:              &templateScheduleStore,
+			AccessControlStore:                 accessControlStore,
 			TLSCertificates:                    options.TLSCertificates,
 			TrialGenerator:                     options.TrialGenerator,
 			TailnetCoordinator:                 options.Coordinator,

diff --git a/coderd/database/dbauthz/accesscontrol.go b/coderd/database/dbauthz/accesscontrol.go
@@ -12,7 +12,7 @@ import (
 // that is used when determining whether an actor is authorized
 // to interact with an RBAC object.
 type AccessControlStore interface {
-	GetTemplateAccessControl(t database.Template) (TemplateAccessControl, error)
+	GetTemplateAccessControl(t database.Template) TemplateAccessControl
 	SetTemplateAccessControl(ctx context.Context, store database.Store, id uuid.UUID, opts TemplateAccessControl) error
 }
 
@@ -26,10 +26,10 @@ type AGPLTemplateAccessControlStore struct{}
 
 var _ AccessControlStore = AGPLTemplateAccessControlStore{}
 
-func (AGPLTemplateAccessControlStore) GetTemplateAccessControl(database.Template) (TemplateAccessControl, error) {
+func (AGPLTemplateAccessControlStore) GetTemplateAccessControl(database.Template) TemplateAccessControl {
 	return TemplateAccessControl{
 		RequireActiveVersion: false,
-	}, nil
+	}
 }
 
 func (AGPLTemplateAccessControlStore) SetTemplateAccessControl(context.Context, database.Store, uuid.UUID, TemplateAccessControl) error {

diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
@@ -2222,10 +2222,7 @@ func (q *querier) InsertWorkspaceBuild(ctx context.Context, arg database.InsertW
 			return xerrors.Errorf("get template by id: %w", err)
 		}
 
-		accessControl, err := (*q.acs.Load()).GetTemplateAccessControl(t)
-		if err != nil {
-			return xerrors.Errorf("get template access control by id: %w", err)
-		}
+		accessControl := (*q.acs.Load()).GetTemplateAccessControl(t)
 
 		// If the template requires the active version we need to check if
 		// the user is a template admin. If they aren't and are attempting

diff --git a/coderd/database/dbauthz/setup_test.go b/coderd/database/dbauthz/setup_test.go
@@ -409,10 +409,10 @@ func accessControlStorePointer() *atomic.Pointer[dbauthz.AccessControlStore] {
 
 type fakeAccessControlStore struct{}
 
-func (fakeAccessControlStore) GetTemplateAccessControl(t database.Template) (dbauthz.TemplateAccessControl, error) {
+func (fakeAccessControlStore) GetTemplateAccessControl(t database.Template) dbauthz.TemplateAccessControl {
 	return dbauthz.TemplateAccessControl{
 		RequireActiveVersion: t.RequireActiveVersion,
-	}, nil
+	}
 }
 
 func (fakeAccessControlStore) SetTemplateAccessControl(context.Context, database.Store, uuid.UUID, dbauthz.TemplateAccessControl) error {

diff --git a/coderd/schedule/template.go b/coderd/schedule/template.go
@@ -151,9 +151,6 @@ type TemplateScheduleOptions struct {
 	// workspaces whose dormant_at field violates the new template time_til_dormant_autodelete
 	// threshold.
 	UpdateWorkspaceDormantAt bool `json:"update_workspace_dormant_at"`
-	// RequireActiveVersion requires that a starting a workspace uses the active
-	// version for a template.
-	RequireActiveVersion bool `json:"require_active_version"`
 }
 
 // TemplateScheduleStore provides an interface for retrieving template
@@ -203,7 +200,6 @@ func (*agplTemplateScheduleStore) Get(ctx context.Context, db database.Store, te
 		FailureTTL:               0,
 		TimeTilDormant:           0,
 		TimeTilDormantAutoDelete: 0,
-		RequireActiveVersion:     false,
 	}, nil
 }
 

diff --git a/coderd/templates.go b/coderd/templates.go
@@ -347,11 +347,6 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque
 			return xerrors.Errorf("insert template: %s", err)
 		}
 
-		dbTemplate, err = tx.GetTemplateByID(ctx, id)
-		if err != nil {
-			return xerrors.Errorf("get template by id: %s", err)
-		}
-
 		if createTemplate.RequireActiveVersion {
 			err = (*api.AccessControlStore.Load()).SetTemplateAccessControl(ctx, tx, id, dbauthz.TemplateAccessControl{
 				RequireActiveVersion: createTemplate.RequireActiveVersion,
@@ -361,6 +356,11 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque
 			}
 		}
 
+		dbTemplate, err = tx.GetTemplateByID(ctx, id)
+		if err != nil {
+			return xerrors.Errorf("get template by id: %s", err)
+		}
+
 		dbTemplate, err = (*api.TemplateScheduleStore.Load()).Set(ctx, tx, dbTemplate, schedule.TemplateScheduleOptions{
 			UserAutostartEnabled: allowUserAutostart,
 			UserAutostopEnabled:  allowUserAutostop,
@@ -379,7 +379,6 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque
 			FailureTTL:               failureTTL,
 			TimeTilDormant:           dormantTTL,
 			TimeTilDormantAutoDelete: dormantAutoDeletionTTL,
-			RequireActiveVersion:     createTemplate.RequireActiveVersion,
 		})
 		if err != nil {
 			return xerrors.Errorf("set template schedule options: %s", err)
@@ -661,6 +660,7 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
 			if err != nil {
 				return xerrors.Errorf("set template access control: %w", err)
 			}
+			updated.RequireActiveVersion = req.RequireActiveVersion
 		}
 
 		defaultTTL := time.Duration(req.DefaultTTLMillis) * time.Millisecond
@@ -700,7 +700,6 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
 				TimeTilDormantAutoDelete:  timeTilDormantAutoDelete,
 				UpdateWorkspaceLastUsedAt: req.UpdateWorkspaceLastUsedAt,
 				UpdateWorkspaceDormantAt:  req.UpdateWorkspaceDormantAt,
-				RequireActiveVersion:      req.RequireActiveVersion,
 			})
 			if err != nil {
 				return xerrors.Errorf("set template schedule options: %w", err)

diff --git a/enterprise/coderd/dbauthz/accesscontrol.go b/enterprise/coderd/dbauthz/accesscontrol.go
@@ -12,10 +12,10 @@ import (
 
 type EnterpriseTemplateAccessControlStore struct{}
 
-func (EnterpriseTemplateAccessControlStore) GetTemplateAccessControl(t database.Template) (agpldbz.TemplateAccessControl, error) {
+func (EnterpriseTemplateAccessControlStore) GetTemplateAccessControl(t database.Template) agpldbz.TemplateAccessControl {
 	return agpldbz.TemplateAccessControl{
 		RequireActiveVersion: t.RequireActiveVersion,
-	}, nil
+	}
 }
 
 func (EnterpriseTemplateAccessControlStore) SetTemplateAccessControl(ctx context.Context, store database.Store, id uuid.UUID, opts agpldbz.TemplateAccessControl) error {

diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go
@@ -92,7 +92,6 @@ func (s *EnterpriseTemplateScheduleStore) Get(ctx context.Context, db database.S
 		FailureTTL:               time.Duration(tpl.FailureTTL),
 		TimeTilDormant:           time.Duration(tpl.TimeTilDormant),
 		TimeTilDormantAutoDelete: time.Duration(tpl.TimeTilDormantAutoDelete),
-		RequireActiveVersion:     tpl.RequireActiveVersion,
 	}, nil
 }
 
@@ -117,8 +116,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S
 		int64(opts.TimeTilDormant) == tpl.TimeTilDormant &&
 		int64(opts.TimeTilDormantAutoDelete) == tpl.TimeTilDormantAutoDelete &&
 		opts.UserAutostartEnabled == tpl.AllowUserAutostart &&
-		opts.UserAutostopEnabled == tpl.AllowUserAutostop &&
-		opts.RequireActiveVersion == tpl.RequireActiveVersion {
+		opts.UserAutostopEnabled == tpl.AllowUserAutostop {
 		// Avoid updating the UpdatedAt timestamp if nothing will be changed.
 		return tpl, nil
 	}