From feae9e1ed6060693e951e06b711cf4a4fd75b1f1 Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Wed, 19 Jul 2023 04:31:00 +0000 Subject: [PATCH] fix(enterprise): avoid initial license reconfig if feature isn't enabled --- enterprise/coderd/coderd.go | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 701eb405b9242..ab517375a457f 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -395,19 +395,24 @@ func (api *API) updateEntitlements(ctx context.Context) error { return nil } - featureChanged := func(featureName codersdk.FeatureName) (changed bool, enabled bool) { + featureChanged := func(featureName codersdk.FeatureName) (initial, changed, enabled bool) { if api.entitlements.Features == nil { - return true, entitlements.Features[featureName].Enabled + return true, false, entitlements.Features[featureName].Enabled } oldFeature := api.entitlements.Features[featureName] newFeature := entitlements.Features[featureName] if oldFeature.Enabled != newFeature.Enabled { - return true, newFeature.Enabled + return false, true, newFeature.Enabled } - return false, newFeature.Enabled + return false, false, newFeature.Enabled } - if changed, enabled := featureChanged(codersdk.FeatureAuditLog); changed { + shouldUpdate := func(initial, changed, enabled bool) bool { + // Avoid an initial tick on startup unless the feature is enabled. + return changed || (initial && enabled) + } + + if initial, changed, enabled := featureChanged(codersdk.FeatureAuditLog); shouldUpdate(initial, changed, enabled) { auditor := agplaudit.NewNop() if enabled { auditor = api.AGPL.Options.Auditor @@ -415,7 +420,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { api.AGPL.Auditor.Store(&auditor) } - if changed, enabled := featureChanged(codersdk.FeatureBrowserOnly); changed { + if initial, changed, enabled := featureChanged(codersdk.FeatureBrowserOnly); shouldUpdate(initial, changed, enabled) { var handler func(rw http.ResponseWriter) bool if enabled { handler = api.shouldBlockNonBrowserConnections @@ -423,7 +428,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { api.AGPL.WorkspaceClientCoordinateOverride.Store(&handler) } - if changed, enabled := featureChanged(codersdk.FeatureTemplateRBAC); changed { + if initial, changed, enabled := featureChanged(codersdk.FeatureTemplateRBAC); shouldUpdate(initial, changed, enabled) { if enabled { committer := committer{Database: api.Database} ptr := proto.QuotaCommitter(&committer) @@ -433,7 +438,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { } } - if changed, enabled := featureChanged(codersdk.FeatureAdvancedTemplateScheduling); changed { + if initial, changed, enabled := featureChanged(codersdk.FeatureAdvancedTemplateScheduling); shouldUpdate(initial, changed, enabled) { if enabled { store := &EnterpriseTemplateScheduleStore{} ptr := schedule.TemplateScheduleStore(store) @@ -444,8 +449,8 @@ func (api *API) updateEntitlements(ctx context.Context) error { } } - if changed, enabled := featureChanged(codersdk.FeatureHighAvailability); changed { - coordinator := agpltailnet.NewCoordinator(api.Logger) + if initial, changed, enabled := featureChanged(codersdk.FeatureHighAvailability); shouldUpdate(initial, changed, enabled) { + var coordinator agpltailnet.Coordinator if enabled { var haCoordinator agpltailnet.Coordinator if api.AGPL.Experiments.Enabled(codersdk.ExperimentTailnetHACoordinator) { @@ -457,9 +462,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { api.Logger.Error(ctx, "unable to set up high availability coordinator", slog.Error(err)) // If we try to setup the HA coordinator and it fails, nothing // is actually changing. - changed = false } else { - _ = coordinator.Close() coordinator = haCoordinator } @@ -472,6 +475,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { _ = api.updateEntitlements(ctx) }) } else { + coordinator = agpltailnet.NewCoordinator(api.Logger) api.derpMesh.SetAddresses([]string{}, false) api.replicaManager.SetCallback(func() { // If the amount of replicas change, so should our entitlements. @@ -481,7 +485,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { } // Recheck changed in case the HA coordinator failed to set up. - if changed { + if coordinator != nil { oldCoordinator := *api.AGPL.TailnetCoordinator.Swap(&coordinator) err := oldCoordinator.Close() if err != nil {