Merge remote-tracking branch 'origin/main' into merge-ephemeral-gc-requeue

infrastructure_files/migrate-to-enterprise.sh

@@ -15,6 +15,12 @@ set -o pipefail
 #   2. Postgres migration — add Postgres, migrate SQLite data via migrate-store.
 #   3. Traffic flow       — add NATS + flow-enricher + flow-receiver.
 #
+# Step 2 is skipped when the deployment already runs on Postgres
+# (server.store.engine: postgres in config.yaml). Nothing is provisioned or
+# migrated in that case and the store config is left exactly as the operator
+# wrote it — the enterprise image reads the same Postgres the community image
+# did. Such a deployment gets the image swap, and can still opt into step 3.
+#
 # If any step fails once the stack has been touched, the script rolls itself
 # back automatically: generated files are removed, the Postgres volume this run
 # created is dropped, and the original deployment is started again.
@@ -38,6 +44,18 @@ ENV_BACKUP=""
 PG_VOLUME_NAME=""
 BACKUP_DIR=""
 
+# Store state. STORE_ENGINE is what the deployment runs on today; when it is
+# already postgres, MIGRATE_POSTGRES stays "no" and nothing is provisioned.
+# POSTGRES_SERVICE is empty when Postgres lives outside this compose project.
+STORE_ENGINE=""
+EXISTING_POSTGRES="no"
+POSTGRES_DSN=""
+POSTGRES_SERVICE=""
+POSTGRES_DEPENDS_CONDITION="service_healthy"
+# Whether this run needs to generate config.yaml.enterprise at all. A pure
+# image swap does not.
+ENTERPRISE_CONFIG="no"
+
 NETBIRD_EULA_URL="https://netbird.io/self-hosted-EULA"
 
 check_docker_compose() {
@@ -192,6 +210,85 @@ detect_exposed_address() {
   yq eval '.server.exposedAddress // ""' "$CONFIG_YAML_HOST"
 }
 
+# The engine is a config.yaml-only setting — there is no env override for it
+# (combined/cmd/root.go reads it from YAML and derives the env vars), so
+# config.yaml is authoritative. Absent means the sqlite default.
+detect_store_engine() {
+  local engine
+  engine=$(yq eval '.server.store.engine // ""' "$CONFIG_YAML_HOST")
+  if [[ -z "$engine" ]] || [[ "$engine" == "null" ]]; then
+    engine="sqlite"
+  fi
+  echo "$engine" | tr '[:upper:]' '[:lower:]'
+}
+
+detect_store_dsn() {
+  yq eval '.server.store.dsn // ""' "$CONFIG_YAML_HOST"
+}
+
+# config.yaml is where a combined deployment carries its DSN; this only covers
+# hand-rolled installs that keep it in the environment instead.
+detect_store_dsn_from_compose() {
+  # `compose config` re-escapes a literal $ as $$ on the way out, so undo that
+  # to get the value the container actually receives.
+  $DOCKER_COMPOSE_COMMAND config 2>/dev/null | yq eval "
+    .services[\"$COMBINED_SERVICE\"].environment.NB_STORE_ENGINE_POSTGRES_DSN //
+    .services[\"$COMBINED_SERVICE\"].environment.NETBIRD_STORE_ENGINE_POSTGRES_DSN // \"\"
+  " - 2>/dev/null | sed 's/\$\$/$/g'
+}
+
+# Reads either DSN form: "host=db ..." or "postgres://user:pass@db:5432/name".
+dsn_host() {
+  local dsn="$1"
+  case "$dsn" in
+    *://*) printf '%s' "$dsn" | sed -n 's,^[a-zA-Z+]*://\([^/?]*\).*,\1,p' | sed -e 's,.*@,,' -e 's,:.*,,' ;;
+    *) printf '%s' "$dsn" | sed -n 's/.*[[:space:]]*host=\([^[:space:]]*\).*/\1/p' ;;
+  esac
+}
+
+# flow-enricher is its own container, so a loopback host or a socket path would
+# reach the enricher rather than Postgres. Only flag hosts we can positively
+# identify — an unparseable DSN must not leave the operator with no way forward.
+dsn_host_reachable() {
+  local dsn="$1"
+  case "$(dsn_host "$dsn")" in
+    localhost | 127.* | ::1 | 0.0.0.0 | /*) return 1 ;;
+    *) return 0 ;;
+  esac
+}
+
+# Names the compose service running this deployment's Postgres, for depends_on.
+# Empty means external — the DSN host matched no service. A DSN with no readable
+# host falls back to matching on image.
+detect_postgres_service() {
+  local host
+  host=$(dsn_host "$POSTGRES_DSN")
+  if [[ -n "$host" ]]; then
+    if [[ "$(host="$host" yq eval '.services | has(env(host))' "$COMPOSE_FILE" 2>/dev/null)" == "true" ]]; then
+      echo "$host"
+    fi
+    return
+  fi
+  yq eval '.services | to_entries | map(select(.value.image // "" | test("(^|/)(postgres|postgis|pgvector|timescaledb)(:|@|$)"))) | .[0].key // ""' "$COMPOSE_FILE"
+}
+
+# depends_on: service_healthy is only legal if the service defines a healthcheck.
+detect_postgres_depends_condition() {
+  local tag
+  tag=$(yq eval ".services[\"$POSTGRES_SERVICE\"].healthcheck | tag" "$COMPOSE_FILE" 2>/dev/null)
+  if [[ "$tag" == "!!map" ]]; then
+    echo "service_healthy"
+  else
+    echo "service_started"
+  fi
+}
+
+env_value() {
+  local value="$1"
+  value=$(printf '%s' "$value" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' -e 's/\$/$$/g')
+  printf '"%s"' "$value"
+}
+
 detect_compose_network() {
   local tag
   tag=$(yq eval ".services[\"$COMBINED_SERVICE\"].networks | tag" "$COMPOSE_FILE" 2>/dev/null)
@@ -228,16 +325,30 @@ services:
       NETBIRD_LICENSE_SERVER_BASE_URL: \${NETBIRD_LICENSE_SERVER_BASE_URL}
 EOF
 
+  # An existing Postgres is already wired up by the operator's own compose file,
+  # so only a Postgres this run creates needs a depends_on.
   if [[ "$MIGRATE_POSTGRES" == "yes" ]]; then
     cat <<EOF
     depends_on:
-      postgres:
-        condition: service_healthy
+      ${POSTGRES_SERVICE}:
+        condition: ${POSTGRES_DEPENDS_CONDITION}
+EOF
+  fi
+
+  # The server is only pointed at a different config file when this run
+  # generates one. A pure image swap leaves it on its original config.yaml.
+  if [[ "$ENTERPRISE_CONFIG" == "yes" ]]; then
+    cat <<EOF
     volumes:
       - ./${ENTERPRISE_CONFIG_FILE}:/etc/netbird/config.yaml.enterprise:ro
     command: ["--config", "/etc/netbird/config.yaml.enterprise"]
+EOF
+  fi
 
-  postgres:
+  if [[ "$MIGRATE_POSTGRES" == "yes" ]]; then
+    cat <<EOF
+
+  ${POSTGRES_SERVICE}:
     image: postgres:17
     container_name: netbird-postgres
     restart: unless-stopped
@@ -257,6 +368,14 @@ EOF
   fi
 
   if [[ "$ENABLE_FLOW" == "yes" ]]; then
+    # Nothing to wait on when Postgres is managed outside this compose project.
+    local enricher_depends=""
+    if [[ -n "$POSTGRES_SERVICE" ]]; then
+      enricher_depends="
+      ${POSTGRES_SERVICE}:
+        condition: ${POSTGRES_DEPENDS_CONDITION}"
+    fi
+
     cat <<EOF
 
   nats:
@@ -273,9 +392,7 @@ EOF
     container_name: netbird-flow-enricher
     restart: unless-stopped
     networks: [${COMPOSE_NETWORK}]
-    depends_on:
-      postgres:
-        condition: service_healthy
+    depends_on:${enricher_depends}
       nats:
         condition: service_started
     environment:
@@ -283,10 +400,10 @@ EOF
       NETBIRD_LICENSE_SERVER_BASE_URL: \${NETBIRD_LICENSE_SERVER_BASE_URL}
       NB_DATADIR: /var/lib/netbird
       NB_MANAGEMENT_STORE_ENGINE: postgres
-      NB_MANAGEMENT_POSTGRES_DSN: "host=postgres user=netbird password=\${POSTGRES_PASSWORD} dbname=netbird port=5432 sslmode=disable"
-      NB_STORE_ENGINE_POSTGRES_DSN: "host=postgres user=netbird password=\${POSTGRES_PASSWORD} dbname=netbird port=5432 sslmode=disable"
+      NB_MANAGEMENT_POSTGRES_DSN: \${NB_ENTERPRISE_POSTGRES_DSN}
+      NB_STORE_ENGINE_POSTGRES_DSN: \${NB_ENTERPRISE_POSTGRES_DSN}
       NB_TRAFFIC_EVENT_STORE_ENGINE: postgres
-      NB_TRAFFIC_EVENT_POSTGRES_DSN: "host=postgres user=netbird password=\${POSTGRES_PASSWORD} dbname=netbird port=5432 sslmode=disable"
+      NB_TRAFFIC_EVENT_POSTGRES_DSN: \${NB_ENTERPRISE_POSTGRES_DSN}
       NB_MANAGEMENT_STORE_KEY: \${NETBIRD_ENCRYPTION_KEY}
       NB_FLOW_ADAPTER_TYPE: nats
       NB_FLOW_NATS_ENDPOINTS: nats://nats:4222
@@ -343,27 +460,41 @@ EOF
   fi
 }
 
-# Build config.yaml.enterprise by yq-editing the operator's existing
-# config.yaml. We don't touch the original file.
+# Build config.yaml.enterprise from the operator's existing config.yaml. We
+# don't touch the original file. Values go through strenv() so a DSN carrying
+# quotes, backslashes or $ cannot break out of the expression.
 render_enterprise_config() {
-  local pg_dsn="host=postgres user=netbird password=${POSTGRES_PASSWORD} dbname=netbird port=5432 sslmode=disable"
+  {
+    echo "# Generated by migrate-to-enterprise.sh from ${CONFIG_YAML_HOST}."
+    echo "# The enterprise server is started with --config pointing at this file,"
+    echo "# so later edits to ${CONFIG_YAML_HOST} have no effect until copied here."
+    cat "$CONFIG_YAML_HOST"
+  } > "$ENTERPRISE_CONFIG_FILE"
 
-  yq eval "
-    .server.store.engine = \"postgres\" |
-    .server.store.dsn = \"$pg_dsn\" |
-    .server.activityStore.engine = \"postgres\" |
-    .server.activityStore.dsn = \"$pg_dsn\" |
-    .server.authStore.engine = \"postgres\" |
-    .server.authStore.dsn = \"$pg_dsn\"
-  " "$CONFIG_YAML_HOST" > "$ENTERPRISE_CONFIG_FILE"
+  if [[ "$MIGRATE_POSTGRES" == "yes" ]]; then
+    # Fresh Postgres: point every store section at it. migrate-store carries the
+    # SQLite contents across.
+    POSTGRES_DSN="$POSTGRES_DSN" yq eval -i '
+      .server.store.engine = "postgres" |
+      .server.store.dsn = strenv(POSTGRES_DSN) |
+      .server.activityStore.engine = "postgres" |
+      .server.activityStore.dsn = strenv(POSTGRES_DSN) |
+      .server.authStore.engine = "postgres" |
+      .server.authStore.dsn = strenv(POSTGRES_DSN)
+    ' "$ENTERPRISE_CONFIG_FILE"
+  fi
+  # Otherwise the store config is the operator's and stays untouched.
+  # activityStore and authStore do not inherit from server.store — each falls
+  # back to its own SQLite file under dataDir — so repointing them at Postgres
+  # here would silently strand the existing audit log and the embedded IdP's
+  # users, with no migrate-store run to carry them over.
 
   if [[ "$ENABLE_FLOW" == "yes" ]]; then
-    local flow_addr="${NETBIRD_DOMAIN}"
-    yq eval -i "
+    NETBIRD_DOMAIN="$NETBIRD_DOMAIN" yq eval -i '
       .server.trafficFlow.enabled = true |
-      .server.trafficFlow.address = \"$flow_addr\" |
-      .server.trafficFlow.interval = \"60s\"
-    " "$ENTERPRISE_CONFIG_FILE"
+      .server.trafficFlow.address = strenv(NETBIRD_DOMAIN) |
+      .server.trafficFlow.interval = "60s"
+    ' "$ENTERPRISE_CONFIG_FILE"
   fi
 }
 
@@ -630,6 +761,91 @@ on_exit() {
 # Main
 # ---------------------------------------------------------------------------
 
+# Already on Postgres: there is nothing to provision and nothing to migrate.
+# The enterprise image reads the very same store config the community image
+# did, so step 2 collapses to a no-op and the run is a plain image swap.
+configure_existing_postgres() {
+  EXISTING_POSTGRES="yes"
+  MIGRATE_POSTGRES="no"
+
+  # DSN first — detect_postgres_service prefers the host it names.
+  POSTGRES_DSN=$(detect_store_dsn)
+  if [[ -z "$POSTGRES_DSN" ]] || [[ "$POSTGRES_DSN" == "null" ]]; then
+    POSTGRES_DSN=$(detect_store_dsn_from_compose)
+  fi
+  if [[ "$POSTGRES_DSN" == "null" ]]; then
+    POSTGRES_DSN=""
+  fi
+
+  POSTGRES_SERVICE=$(detect_postgres_service)
+  if [[ -n "$POSTGRES_SERVICE" ]]; then
+    POSTGRES_DEPENDS_CONDITION=$(detect_postgres_depends_condition)
+  fi
+
+  echo "Step 2: Postgres migration not needed — this deployment already runs on"
+  echo "        Postgres. Its store configuration is reused as-is and left"
+  echo "        untouched; no database is created and no data is moved."
+  if [[ -n "$POSTGRES_SERVICE" ]]; then
+    echo "        Postgres service: $POSTGRES_SERVICE (in $COMPOSE_FILE)"
+  else
+    echo "        Postgres service: managed outside $COMPOSE_FILE"
+  fi
+}
+
+configure_sqlite_store() {
+  MIGRATE_POSTGRES=$(read_yes_no "Step 2: Migrate storage from SQLite to Postgres? (recommended)" "n")
+  [[ "$MIGRATE_POSTGRES" == "yes" ]] || return 0
+
+  # The override would otherwise merge into a service of the same name and
+  # quietly rewrite its image and credentials.
+  local existing
+  existing=$(yq eval '.services | has("postgres")' "$COMPOSE_FILE")
+  if [[ "$existing" == "true" ]]; then
+    echo "" > /dev/stderr
+    echo "$COMPOSE_FILE already defines a service named 'postgres', but config.yaml" > /dev/stderr
+    echo "still has server.store.engine: sqlite. This script would add its own" > /dev/stderr
+    echo "'postgres' service and Compose would merge the two." > /dev/stderr
+    echo "" > /dev/stderr
+    echo "Point server.store.engine at that Postgres yourself, or rename the service," > /dev/stderr
+    echo "then re-run." > /dev/stderr
+    exit 1
+  fi
+
+  echo ""
+  echo "  ⚠  Data will be migrated from SQLite to Postgres. The SQLite store"
+  echo "     will be backed up automatically. To fully revert later, restore"
+  echo "     that backup and delete docker-compose.override.yml +"
+  echo "     config.yaml.enterprise."
+  local confirm
+  confirm=$(read_yes_no "  Continue?" "y")
+  if [[ "$confirm" != "yes" ]]; then
+    MIGRATE_POSTGRES="no"
+    echo "  Skipping Postgres migration."
+    return 0
+  fi
+
+  POSTGRES_PASSWORD=$(rand_password)
+  POSTGRES_SERVICE="postgres"
+  POSTGRES_DEPENDS_CONDITION="service_healthy"
+  POSTGRES_DSN="host=postgres user=netbird password=${POSTGRES_PASSWORD} dbname=netbird port=5432 sslmode=disable"
+}
+
+# mysql, or something this script has never seen. Swapping the images is still
+# valid; touching the store is not.
+configure_unsupported_store() {
+  MIGRATE_POSTGRES="no"
+  echo "  ⚠  server.store.engine is '$STORE_ENGINE'. This script only migrates"
+  echo "     SQLite to Postgres, and traffic flow requires Postgres, so both are"
+  echo "     unavailable here. The store configuration will be left untouched."
+  echo ""
+  local proceed
+  proceed=$(read_yes_no "Step 2 skipped. Continue with the image swap only?" "n")
+  if [[ "$proceed" != "yes" ]]; then
+    echo "Aborted."
+    exit 0
+  fi
+}
+
 init_migration() {
   DOCKER_COMPOSE_COMMAND=$(check_docker_compose)
   check_yq
@@ -679,12 +895,15 @@ init_migration() {
     exit 1
   fi
 
+  STORE_ENGINE=$(detect_store_engine)
+
   echo "Detected existing deployment:"
   echo "  Combined service: $COMBINED_SERVICE"
   echo "  Dashboard:        $DASHBOARD_SERVICE"
   echo "  config.yaml:      $CONFIG_YAML_HOST"
   echo "  Data volume:      $DATA_VOLUME"
   echo "  Network:          $COMPOSE_NETWORK"
+  echo "  Store engine:     $STORE_ENGINE"
   echo ""
 
   require_eula_acceptance
@@ -703,28 +922,17 @@ init_migration() {
   echo "Step 1: Image swap (community → Enterprise). License key required."
   NB_LICENSE_KEY=$(read_secret "  License key")
 
-  # Step 2 — optional
+  # Step 2 — what this does depends on what the deployment already stores in.
   echo ""
-  MIGRATE_POSTGRES=$(read_yes_no "Step 2: Migrate storage from SQLite to Postgres? (recommended)" "n")
-  if [[ "$MIGRATE_POSTGRES" == "yes" ]]; then
-    echo ""
-    echo "  ⚠  Data will be migrated from SQLite to Postgres. The SQLite store"
-    echo "     will be backed up automatically. To fully revert later, restore"
-    echo "     that backup and delete docker-compose.override.yml +"
-    echo "     config.yaml.enterprise."
-    local confirm
-    confirm=$(read_yes_no "  Continue?" "y")
-    if [[ "$confirm" != "yes" ]]; then
-      MIGRATE_POSTGRES="no"
-      echo "  Skipping Postgres migration."
-    else
-      POSTGRES_PASSWORD=$(rand_password)
-    fi
-  fi
+  case "$STORE_ENGINE" in
+    postgres) configure_existing_postgres ;;
+    sqlite) configure_sqlite_store ;;
+    *) configure_unsupported_store ;;
+  esac
 
   # Step 3 — optional, only if Postgres is on (flow requires Postgres)
   echo ""
-  if [[ "$MIGRATE_POSTGRES" == "yes" ]]; then
+  if [[ "$MIGRATE_POSTGRES" == "yes" ]] || [[ "$EXISTING_POSTGRES" == "yes" ]]; then
     ENABLE_FLOW=$(read_yes_no "Step 3: Enable traffic flow? (requires Postgres)" "n")
     if [[ "$ENABLE_FLOW" == "yes" ]]; then
       # Auth secret MUST match server.authSecret from config.yaml
@@ -748,12 +956,46 @@ init_migration() {
         echo "Could not read server.store.encryptionKey from $CONFIG_YAML_HOST." > /dev/stderr
         exit 1
       fi
+
+      # flow-enricher talks to Postgres directly, so this is the one place an
+      # existing deployment's DSN is actually needed — and the one place a host
+      # that only works from inside the server container shows up.
+      while :; do
+        local dsn_problem=""
+        if [[ -z "$POSTGRES_DSN" ]]; then
+          dsn_problem="No DSN could be read from $CONFIG_YAML_HOST or from the $COMBINED_SERVICE environment."
+        elif ! dsn_host_reachable "$POSTGRES_DSN"; then
+          dsn_problem="Its host '$(dsn_host "$POSTGRES_DSN")' only resolves inside the server container."
+        fi
+        [[ -n "$dsn_problem" ]] || break
+
+        echo ""
+        echo "  The flow enricher reaches Postgres from a container of its own."
+        echo "  $dsn_problem"
+        echo "  Enter a DSN reachable from other containers, or press Ctrl-C to abort."
+        POSTGRES_DSN=$(read_required "  Postgres DSN (host=… user=… password=… dbname=… port=5432 sslmode=disable)")
+      done
+
+      # Only where the operator owns Postgres: a DSN entered above may name a
+      # different host. The sqlite path creates its own service, nothing to find.
+      if [[ "$EXISTING_POSTGRES" == "yes" ]]; then
+        POSTGRES_SERVICE=$(detect_postgres_service)
+        if [[ -n "$POSTGRES_SERVICE" ]]; then
+          POSTGRES_DEPENDS_CONDITION=$(detect_postgres_depends_condition)
+        fi
+      fi
     fi
   else
     ENABLE_FLOW="no"
     echo "Step 3 (traffic flow) skipped — requires Postgres."
   fi
 
+  # config.yaml.enterprise only exists to hold changes; without any there is
+  # nothing to generate and the server keeps running on its own config.yaml.
+  if [[ "$MIGRATE_POSTGRES" == "yes" ]] || [[ "$ENABLE_FLOW" == "yes" ]]; then
+    ENTERPRISE_CONFIG="yes"
+  fi
+
   check_data_directory
   check_stale_postgres_volume
 }
@@ -771,7 +1013,7 @@ apply_changes() {
     sed -i.bak '/NETBIRD_LICENSE_SERVER_BASE_URL/d' "$OVERRIDE_FILE" && rm -f "$OVERRIDE_FILE.bak"
   fi
 
-  if [[ "$MIGRATE_POSTGRES" == "yes" ]]; then
+  if [[ "$ENTERPRISE_CONFIG" == "yes" ]]; then
     echo "Writing $ENTERPRISE_CONFIG_FILE ..."
     install -m 600 /dev/null "$ENTERPRISE_CONFIG_FILE"
     render_enterprise_config
@@ -807,6 +1049,9 @@ apply_changes() {
       echo "POSTGRES_PASSWORD=${POSTGRES_PASSWORD}"
     fi
     if [[ "$ENABLE_FLOW" == "yes" ]]; then
+      # Own variable name rather than NB_STORE_ENGINE_POSTGRES_DSN so that a
+      # deployment already setting that one keeps its own value.
+      echo "NB_ENTERPRISE_POSTGRES_DSN=$(env_value "$POSTGRES_DSN")"
       echo "NB_FLOW_AUTH_SECRET=${NB_FLOW_AUTH_SECRET}"
       echo "NETBIRD_ENCRYPTION_KEY=${NETBIRD_ENCRYPTION_KEY}"
     fi
@@ -868,14 +1113,19 @@ print_summary() {
   echo " Summary"
   echo "──────────────────────────────────────────────────────────────────────"
   echo "  Images:           swapped to enterprise"
-  [[ "$MIGRATE_POSTGRES" == "yes" ]] && echo "  Storage:          Postgres (data migrated from SQLite)"
-  [[ "$MIGRATE_POSTGRES" != "yes" ]] && echo "  Storage:          SQLite (unchanged)"
+  if [[ "$MIGRATE_POSTGRES" == "yes" ]]; then
+    echo "  Storage:          Postgres (data migrated from SQLite)"
+  elif [[ "$EXISTING_POSTGRES" == "yes" ]]; then
+    echo "  Storage:          Postgres (pre-existing, configuration unchanged)"
+  else
+    echo "  Storage:          $STORE_ENGINE (unchanged)"
+  fi
   [[ "$ENABLE_FLOW" == "yes" ]] && echo "  Traffic flow:     enabled"
   [[ "$ENABLE_FLOW" != "yes" ]] && echo "  Traffic flow:     disabled"
   echo ""
   echo "  Generated files (next to your docker-compose.yml):"
   echo "    $OVERRIDE_FILE"
-  [[ "$MIGRATE_POSTGRES" == "yes" ]] && echo "    $ENTERPRISE_CONFIG_FILE"
+  [[ "$ENTERPRISE_CONFIG" == "yes" ]] && echo "    $ENTERPRISE_CONFIG_FILE"
   echo "    .env  (license key + secrets, mode 600)"
   [[ "$ENV_EXISTED" == "yes" ]] && [[ -f "$ENV_BACKUP" ]] && echo "    $ENV_BACKUP  (.env as it was before this run)"
   [[ "$MIGRATE_POSTGRES" == "yes" ]] && echo "    backups/sqlite-pre-enterprise-*/  (SQLite backup)"
@@ -899,7 +1149,11 @@ print_summary() {
   else
     echo "  $DOCKER_COMPOSE_COMMAND down"
   fi
-  echo "  rm -f $OVERRIDE_FILE $ENTERPRISE_CONFIG_FILE"
+  if [[ "$ENTERPRISE_CONFIG" == "yes" ]]; then
+    echo "  rm -f $OVERRIDE_FILE $ENTERPRISE_CONFIG_FILE"
+  else
+    echo "  rm -f $OVERRIDE_FILE"
+  fi
   if [[ "$ENV_EXISTED" == "yes" ]] && [[ -f "$ENV_BACKUP" ]]; then
     echo "  mv $ENV_BACKUP .env   # restores .env as it was before this run"
   elif [[ "$ENV_EXISTED" == "no" ]]; then

management/internals/modules/peers/ephemeral/manager/ephemeral.go

@@ -216,13 +216,52 @@ func (e *EphemeralManager) cleanup(ctx context.Context) {
 
 	for accountID, peerIDs := range peerIDsPerAccount {
 		log.WithContext(ctx).Debugf("cleanup: deleting %d ephemeral peers for account %s: %s", len(peerIDs), accountID, peerIDs)
-		err := e.peersManager.DeletePeers(ctx, accountID, peerIDs, activity.SystemInitiator, true)
+		skipped, err := e.peersManager.DeletePeers(ctx, accountID, peerIDs, activity.SystemInitiator, true)
 		if err != nil {
 			log.WithContext(ctx).Errorf("failed to delete ephemeral peers: %s", err)
 			e.metrics.CountCleanupError()
+		}
+		if len(skipped) > 0 {
+			// A skipped peer was not deleted: it is still connected in the
+			// store, was seen too recently, or its deletion failed. None of
+			// that says whether a disconnect will ever be observed for it
+			// again, so schedule another attempt instead of dropping it, or
+			// it is never collected.
+			log.WithContext(ctx).Debugf("cleanup: requeueing %d skipped ephemeral peers for account %s: %s", len(skipped), accountID, skipped)
+			e.requeuePeers(ctx, accountID, skipped)
+		}
+		e.metrics.CountPeersCleaned(int64(len(peerIDs) - len(skipped)))
+	}
+}
+
+// requeuePeers puts peers whose deletion was skipped back on the list with a
+// fresh deadline. A peer that reconnected and disconnected in the meantime is
+// already listed again and keeps its existing entry.
+func (e *EphemeralManager) requeuePeers(ctx context.Context, accountID string, peerIDs []string) {
+	e.peersLock.Lock()
+	defer e.peersLock.Unlock()
+
+	added := 0
+	for _, id := range peerIDs {
+		if e.isPeerOnList(id) {
 			continue
 		}
-		e.metrics.CountPeersCleaned(int64(len(peerIDs)))
+		e.addPeer(accountID, id, e.newDeadLine())
+		added++
+	}
+	if added == 0 {
+		return
+	}
+	e.metrics.AddPending(int64(added))
+
+	if e.timer == nil {
+		delay := e.headPeer.deadline.Sub(timeNow()) + e.cleanupWindow
+		if delay < 0 {
+			delay = 0
+		}
+		e.timer = time.AfterFunc(delay, func() {
+			e.cleanup(ctx)
+		})
 	}
 }
 

management/internals/modules/peers/ephemeral/manager/ephemeral_test.go

@@ -2,14 +2,15 @@ package manager
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"sync"
 	"testing"
 	"time"
 
-	"go.uber.org/mock/gomock"
 	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
+	"go.uber.org/mock/gomock"
 
 	nbdns "github.com/netbirdio/netbird/dns"
 	"github.com/netbirdio/netbird/management/internals/modules/peers"
@@ -104,11 +105,11 @@ func TestNewManager(t *testing.T) {
 	// Expect DeletePeers to be called for ephemeral peers
 	peersManager.EXPECT().
 		DeletePeers(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), true).
-		DoAndReturn(func(ctx context.Context, accountID string, peerIDs []string, userID string, checkConnected bool) error {
+		DoAndReturn(func(ctx context.Context, accountID string, peerIDs []string, userID string, checkConnected bool) ([]string, error) {
 			for _, peerID := range peerIDs {
 				delete(store.account.Peers, peerID)
 			}
-			return nil
+			return nil, nil
 		}).
 		AnyTimes()
 
@@ -142,11 +143,11 @@ func TestNewManagerPeerConnected(t *testing.T) {
 	// Expect DeletePeers to be called for ephemeral peers (except the connected one)
 	peersManager.EXPECT().
 		DeletePeers(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), true).
-		DoAndReturn(func(ctx context.Context, accountID string, peerIDs []string, userID string, checkConnected bool) error {
+		DoAndReturn(func(ctx context.Context, accountID string, peerIDs []string, userID string, checkConnected bool) ([]string, error) {
 			for _, peerID := range peerIDs {
 				delete(store.account.Peers, peerID)
 			}
-			return nil
+			return nil, nil
 		}).
 		AnyTimes()
 
@@ -183,11 +184,11 @@ func TestNewManagerPeerDisconnected(t *testing.T) {
 	// Expect DeletePeers to be called for the one disconnected peer
 	peersManager.EXPECT().
 		DeletePeers(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), true).
-		DoAndReturn(func(ctx context.Context, accountID string, peerIDs []string, userID string, checkConnected bool) error {
+		DoAndReturn(func(ctx context.Context, accountID string, peerIDs []string, userID string, checkConnected bool) ([]string, error) {
 			for _, peerID := range peerIDs {
 				delete(store.account.Peers, peerID)
 			}
-			return nil
+			return nil, nil
 		}).
 		AnyTimes()
 
@@ -240,16 +241,16 @@ func TestCleanupSchedulingBehaviorIsBatched(t *testing.T) {
 	// Set up expectation that DeletePeers will be called once with all peer IDs
 	peersManager.EXPECT().
 		DeletePeers(gomock.Any(), account.Id, gomock.Any(), gomock.Any(), true).
-		DoAndReturn(func(ctx context.Context, accountID string, peerIDs []string, userID string, checkConnected bool) error {
+		DoAndReturn(func(ctx context.Context, accountID string, peerIDs []string, userID string, checkConnected bool) ([]string, error) {
 			// Simulate the actual deletion behavior
 			for _, peerID := range peerIDs {
 				err := mockAM.DeletePeer(ctx, accountID, peerID, userID)
 				if err != nil {
-					return err
+					return nil, err
 				}
 			}
 			mockAM.BufferUpdateAccountPeers(ctx, accountID, types.UpdateReason{})
-			return nil
+			return nil, nil
 		}).
 		Times(1)
 
@@ -276,6 +277,94 @@ func TestCleanupSchedulingBehaviorIsBatched(t *testing.T) {
 	assert.Equal(t, ephemeralPeers, mockAM.GetDeletePeerCalls(), "should have deleted all peers")
 }
 
+// TestCleanupRequeuesVetoedPeers covers a peer whose deletion is vetoed (the
+// store still reports it connected, or it was seen too recently): it must be
+// scheduled for another attempt rather than dropped from the list, or it is
+// never collected once the veto clears.
+func TestCleanupRequeuesVetoedPeers(t *testing.T) {
+	t.Cleanup(func() {
+		timeNow = time.Now
+	})
+	startTime := time.Now()
+	timeNow = func() time.Time {
+		return startTime
+	}
+
+	mockStore := &MockStore{}
+	seedPeers(mockStore, 0, 1)
+	ctrl := gomock.NewController(t)
+	peersManager := peers.NewMockManager(ctrl)
+
+	// The first attempt vetoes the peer, the second deletes it.
+	first := peersManager.EXPECT().
+		DeletePeers(gomock.Any(), gomock.Any(), []string{"ephemeral_peer_0"}, gomock.Any(), true).
+		Return([]string{"ephemeral_peer_0"}, nil)
+	peersManager.EXPECT().
+		DeletePeers(gomock.Any(), gomock.Any(), []string{"ephemeral_peer_0"}, gomock.Any(), true).
+		After(first).
+		DoAndReturn(func(_ context.Context, _ string, peerIDs []string, _ string, _ bool) ([]string, error) {
+			for _, peerID := range peerIDs {
+				delete(mockStore.account.Peers, peerID)
+			}
+			return nil, nil
+		})
+
+	mgr := NewEphemeralManager(mockStore, peersManager)
+	mgr.loadEphemeralPeers(context.Background())
+
+	startTime = startTime.Add(ephemeral.EphemeralLifeTime + time.Second)
+	mgr.cleanup(context.Background())
+	assert.Len(t, mockStore.account.Peers, 1, "vetoed peer must not be deleted yet")
+
+	startTime = startTime.Add(ephemeral.EphemeralLifeTime + time.Second)
+	mgr.cleanup(context.Background())
+	assert.Len(t, mockStore.account.Peers, 0, "vetoed peer should be retried and deleted once the veto clears")
+}
+
+// TestCleanupRequeuesFailedDeletes covers a peer whose deletion attempt errors
+// (DeletePeers reports it as skipped alongside the aggregate error): it must be
+// scheduled for another attempt rather than dropped from the list, or a
+// transient store failure leaks it forever.
+func TestCleanupRequeuesFailedDeletes(t *testing.T) {
+	t.Cleanup(func() {
+		timeNow = time.Now
+	})
+	startTime := time.Now()
+	timeNow = func() time.Time {
+		return startTime
+	}
+
+	mockStore := &MockStore{}
+	seedPeers(mockStore, 0, 1)
+	ctrl := gomock.NewController(t)
+	peersManager := peers.NewMockManager(ctrl)
+
+	// The first attempt fails, the second deletes the peer.
+	first := peersManager.EXPECT().
+		DeletePeers(gomock.Any(), gomock.Any(), []string{"ephemeral_peer_0"}, gomock.Any(), true).
+		Return([]string{"ephemeral_peer_0"}, errors.New("transient store failure"))
+	peersManager.EXPECT().
+		DeletePeers(gomock.Any(), gomock.Any(), []string{"ephemeral_peer_0"}, gomock.Any(), true).
+		After(first).
+		DoAndReturn(func(_ context.Context, _ string, peerIDs []string, _ string, _ bool) ([]string, error) {
+			for _, peerID := range peerIDs {
+				delete(mockStore.account.Peers, peerID)
+			}
+			return nil, nil
+		})
+
+	mgr := NewEphemeralManager(mockStore, peersManager)
+	mgr.loadEphemeralPeers(context.Background())
+
+	startTime = startTime.Add(ephemeral.EphemeralLifeTime + time.Second)
+	mgr.cleanup(context.Background())
+	assert.Len(t, mockStore.account.Peers, 1, "peer whose deletion failed must still exist")
+
+	startTime = startTime.Add(ephemeral.EphemeralLifeTime + time.Second)
+	mgr.cleanup(context.Background())
+	assert.Len(t, mockStore.account.Peers, 0, "peer whose deletion failed should be retried and deleted")
+}
+
 func seedPeers(store *MockStore, numberOfPeers int, numberOfEphemeralPeers int) {
 	store.account = newAccountWithId(context.Background(), "my account", "", "", false)
 

management/internals/modules/peers/manager.go

@@ -8,9 +8,11 @@ import (
 	"net"
 	"time"
 
+	"github.com/hashicorp/go-multierror"
 	"github.com/rs/xid"
 	log "github.com/sirupsen/logrus"
 
+	nberrors "github.com/netbirdio/netbird/client/errors"
 	"github.com/netbirdio/netbird/management/internals/controllers/network_map"
 	"github.com/netbirdio/netbird/management/internals/modules/peers/ephemeral"
 	"github.com/netbirdio/netbird/management/server/account"
@@ -30,7 +32,12 @@ type Manager interface {
 	GetPeerAccountID(ctx context.Context, peerID string) (string, error)
 	GetAllPeers(ctx context.Context, accountID, userID string) ([]*peer.Peer, error)
 	GetPeersByGroupIDs(ctx context.Context, accountID string, groupsIDs []string) ([]*peer.Peer, error)
-	DeletePeers(ctx context.Context, accountID string, peerIDs []string, userID string, checkConnected bool) error
+	// DeletePeers removes the given peers along with their group memberships and
+	// policies. Every peer that was not deleted is returned in skipped, so the
+	// caller can retry it later: with checkConnected, a peer that is still
+	// connected or was seen too recently is left in place, and a peer whose
+	// deletion failed is skipped with the failure aggregated into err.
+	DeletePeers(ctx context.Context, accountID string, peerIDs []string, userID string, checkConnected bool) (skipped []string, err error)
 	SetNetworkMapController(networkMapController network_map.Controller)
 	SetIntegratedPeerValidator(integratedPeerValidator integrated_validator.IntegratedValidator)
 	SetAccountManager(accountManager account.Manager)
@@ -128,91 +135,153 @@ func (m *managerImpl) GetPeerWithGroups(ctx context.Context, accountID, peerID s
 	return p, groups, nil
 }
 
-func (m *managerImpl) DeletePeers(ctx context.Context, accountID string, peerIDs []string, userID string, checkConnected bool) error {
+// deletePeerOutcome is the per-peer result of a DeletePeers pass.
+type deletePeerOutcome int
+
+const (
+	// peerMissing means the peer no longer exists, so nothing changed.
+	peerMissing deletePeerOutcome = iota
+	// peerVetoed means the peer cannot be deleted yet: it is still connected or
+	// was seen too recently.
+	peerVetoed
+	// peerDeleted means the peer and its objects were removed.
+	peerDeleted
+)
+
+func (m *managerImpl) DeletePeers(ctx context.Context, accountID string, peerIDs []string, userID string, checkConnected bool) ([]string, error) {
 	settings, err := m.store.GetAccountSettings(ctx, store.LockingStrengthNone, accountID)
 	if err != nil {
-		return err
+		return peerIDs, err
 	}
 	dnsDomain := m.networkMapController.GetDNSDomain(settings)
 
+	var skipped []string
+	var merr *multierror.Error
+	deletedAny := false
 	for _, peerID := range peerIDs {
-		var eventsToStore []func()
-		err = m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
-			peer, err := transaction.GetPeerByID(ctx, store.LockingStrengthNone, accountID, peerID)
-			if err != nil {
-				if e, ok := status.FromError(err); ok && e.Type() == status.NotFound {
-					log.WithContext(ctx).Tracef("DeletePeers: peer %s not found, skipping", peerID)
-					return nil
-				}
-				return err
-			}
-
-			if checkConnected && (peer.Status.Connected || peer.Status.LastSeen.After(time.Now().Add(-(ephemeral.EphemeralLifeTime - 10*time.Second)))) {
-				log.WithContext(ctx).Tracef("DeletePeers: peer %s skipped (connected=%t, lastSeen=%s, threshold=%s, ephemeral=%t)",
-					peerID, peer.Status.Connected,
-					peer.Status.LastSeen.Format(time.RFC3339),
-					time.Now().Add(-(ephemeral.EphemeralLifeTime - 10*time.Second)).Format(time.RFC3339),
-					peer.Ephemeral)
-				return nil
-			}
-
-			if err := transaction.RemovePeerFromAllGroups(ctx, peerID); err != nil {
-				return fmt.Errorf("failed to remove peer %s from groups", peerID)
-			}
-
-			peerPolicyRules, err := transaction.GetPolicyRulesByResourceID(ctx, store.LockingStrengthNone, accountID, peerID)
-			if err != nil {
-				return err
-			}
-			for _, rule := range peerPolicyRules {
-				policy, err := transaction.GetPolicyByID(ctx, store.LockingStrengthNone, accountID, rule.PolicyID)
-				if err != nil {
-					return err
-				}
-
-				err = transaction.DeletePolicy(ctx, accountID, rule.PolicyID)
-				if err != nil {
-					return err
-				}
-
-				eventsToStore = append(eventsToStore, func() {
-					m.accountManager.StoreEvent(ctx, userID, peer.ID, accountID, activity.PolicyRemoved, policy.EventMeta())
-				})
-			}
-
-			if err = transaction.DeletePeer(ctx, accountID, peerID); err != nil {
-				return err
-			}
-
-			log.WithContext(ctx).Debugf("DeletePeers: deleted peer %s", peerID)
-
-			if !(peer.ProxyMeta.Embedded || peer.Meta.KernelVersion == "wasm") {
-				eventsToStore = append(eventsToStore, func() {
-					m.accountManager.StoreEvent(ctx, userID, peer.ID, accountID, activity.PeerRemovedByUser, peer.EventMeta(dnsDomain))
-				})
-			}
-
-			return nil
-		})
+		outcome, events, err := m.deleteSinglePeer(ctx, accountID, peerID, userID, checkConnected, dnsDomain)
 		if err != nil {
-			log.WithContext(ctx).Errorf("DeletePeers: failed to delete peer %s: %v", peerID, err)
+			merr = multierror.Append(merr, fmt.Errorf("delete peer %s: %w", peerID, err))
+			skipped = append(skipped, peerID)
 			continue
 		}
 
-		if m.integratedPeerValidator != nil {
-			if err = m.integratedPeerValidator.PeerDeleted(ctx, accountID, peerID, settings.Extra); err != nil {
-				log.WithContext(ctx).Errorf("failed to delete peer %s from integrated validator: %v", peerID, err)
-			}
-		}
-
-		for _, event := range eventsToStore {
-			event()
+		switch outcome {
+		case peerVetoed:
+			skipped = append(skipped, peerID)
+		case peerDeleted:
+			deletedAny = true
+			m.notifyPeerDeleted(ctx, accountID, peerID, settings.Extra, events)
+		case peerMissing:
 		}
 	}
 
-	m.accountManager.UpdateAccountPeers(ctx, accountID, types.UpdateReason{Resource: types.UpdateResourcePeer, Operation: types.UpdateOperationDelete})
+	// Skipped or missing peers changed nothing, so an update would push an
+	// identical map to every peer in the account.
+	if deletedAny {
+		m.accountManager.UpdateAccountPeers(ctx, accountID, types.UpdateReason{Resource: types.UpdateResourcePeer, Operation: types.UpdateOperationDelete})
+	}
 
-	return nil
+	return skipped, nberrors.FormatErrorOrNil(merr)
+}
+
+// deleteSinglePeer deletes one peer along with its group memberships and
+// policies in a single transaction. With checkConnected, a peer that is still
+// connected or was seen too recently is left untouched and reported as vetoed.
+// The returned events must be stored by the caller once the deletion is final.
+func (m *managerImpl) deleteSinglePeer(ctx context.Context, accountID, peerID, userID string, checkConnected bool, dnsDomain string) (deletePeerOutcome, []func(), error) {
+	outcome := peerMissing
+	var eventsToStore []func()
+	err := m.store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
+		outcome = peerMissing
+		eventsToStore = nil
+
+		p, err := transaction.GetPeerByID(ctx, store.LockingStrengthNone, accountID, peerID)
+		if err != nil {
+			if e, ok := status.FromError(err); ok && e.Type() == status.NotFound {
+				log.WithContext(ctx).Tracef("DeletePeers: peer %s not found, skipping", peerID)
+				return nil
+			}
+			return err
+		}
+
+		if checkConnected && (p.Status.Connected || p.Status.LastSeen.After(time.Now().Add(-(ephemeral.EphemeralLifeTime - 10*time.Second)))) {
+			log.WithContext(ctx).Tracef("DeletePeers: peer %s skipped (connected=%t, lastSeen=%s, threshold=%s, ephemeral=%t)",
+				peerID, p.Status.Connected,
+				p.Status.LastSeen.Format(time.RFC3339),
+				time.Now().Add(-(ephemeral.EphemeralLifeTime - 10*time.Second)).Format(time.RFC3339),
+				p.Ephemeral)
+			outcome = peerVetoed
+			return nil
+		}
+
+		eventsToStore, err = m.deletePeerObjects(ctx, transaction, accountID, userID, dnsDomain, p)
+		if err != nil {
+			return err
+		}
+		outcome = peerDeleted
+		return nil
+	})
+	if err != nil {
+		return outcome, nil, err
+	}
+	return outcome, eventsToStore, nil
+}
+
+// deletePeerObjects removes the peer's group memberships, its policies and the
+// peer itself within the given transaction, returning the activity events to
+// store once the transaction commits.
+func (m *managerImpl) deletePeerObjects(ctx context.Context, transaction store.Store, accountID, userID, dnsDomain string, p *peer.Peer) ([]func(), error) {
+	if err := transaction.RemovePeerFromAllGroups(ctx, p.ID); err != nil {
+		return nil, fmt.Errorf("remove peer %s from groups: %w", p.ID, err)
+	}
+
+	var eventsToStore []func()
+	peerPolicyRules, err := transaction.GetPolicyRulesByResourceID(ctx, store.LockingStrengthNone, accountID, p.ID)
+	if err != nil {
+		return nil, err
+	}
+	for _, rule := range peerPolicyRules {
+		policy, err := transaction.GetPolicyByID(ctx, store.LockingStrengthNone, accountID, rule.PolicyID)
+		if err != nil {
+			return nil, err
+		}
+
+		if err := transaction.DeletePolicy(ctx, accountID, rule.PolicyID); err != nil {
+			return nil, err
+		}
+
+		eventsToStore = append(eventsToStore, func() {
+			m.accountManager.StoreEvent(ctx, userID, p.ID, accountID, activity.PolicyRemoved, policy.EventMeta())
+		})
+	}
+
+	if err := transaction.DeletePeer(ctx, accountID, p.ID); err != nil {
+		return nil, err
+	}
+
+	log.WithContext(ctx).Debugf("DeletePeers: deleted peer %s", p.ID)
+
+	if !(p.ProxyMeta.Embedded || p.Meta.KernelVersion == "wasm") {
+		eventsToStore = append(eventsToStore, func() {
+			m.accountManager.StoreEvent(ctx, userID, p.ID, accountID, activity.PeerRemovedByUser, p.EventMeta(dnsDomain))
+		})
+	}
+
+	return eventsToStore, nil
+}
+
+// notifyPeerDeleted reports a completed deletion to the integrated validator and
+// stores the deletion's activity events.
+func (m *managerImpl) notifyPeerDeleted(ctx context.Context, accountID, peerID string, extraSettings *types.ExtraSettings, events []func()) {
+	if m.integratedPeerValidator != nil {
+		if err := m.integratedPeerValidator.PeerDeleted(ctx, accountID, peerID, extraSettings); err != nil {
+			log.WithContext(ctx).Errorf("failed to delete peer %s from integrated validator: %v", peerID, err)
+		}
+	}
+	for _, event := range events {
+		event()
+	}
 }
 
 func (m *managerImpl) GetPeerID(ctx context.Context, peerKey string) (string, error) {

management/internals/modules/peers/manager_mock.go

@@ -61,11 +61,12 @@ func (mr *MockManagerMockRecorder) CreateProxyPeer(ctx, accountID, peerKey, clus
 }
 
 // DeletePeers mocks base method.
-func (m *MockManager) DeletePeers(ctx context.Context, accountID string, peerIDs []string, userID string, checkConnected bool) error {
+func (m *MockManager) DeletePeers(ctx context.Context, accountID string, peerIDs []string, userID string, checkConnected bool) ([]string, error) {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "DeletePeers", ctx, accountID, peerIDs, userID, checkConnected)
-	ret0, _ := ret[0].(error)
-	return ret0
+	ret0, _ := ret[0].([]string)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
 }
 
 // DeletePeers indicates an expected call of DeletePeers.