fix(enterprise): avoid initial license reconfig if feature isn't enabled (#8586)

coadler · web-flow · commit cc8d0af027e7 · 2023-07-19T10:32:29.000-05:00
diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go
@@ -395,35 +395,40 @@ 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
 		}
 		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
 		}
 		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 {
