fix: migration loop caused by phantom config_v5_migration
Three layered causes: 1. config_v5_migration had no executor phase — structurally impossible to mark complete. 2. StateManager.loadConfig unmarshaled into typed config.Config, but install template writes version as JSON number while Config types it as string — migration completion never persisted to disk. 3. readConfigVersion only parsed float64, so normalized string read 0 and re-triggered. Fix: StateManager is map-based (immune to field-type drift). Executor has real config_v5 phase (bump + mark complete). parseConfigVersion accepts both number and string. validateMigration uses MkdirAll then stat (fixes false 'state dir not found' on Windows).
This commit is contained in:
parent
c2d683d63b
commit
9c8e08e079
4 changed files with 275 additions and 47 deletions
|
|
@ -272,13 +272,29 @@ func readConfigVersion(configPath string) (string, int, error) {
|
|||
if version, ok := config["agent_version"].(string); ok {
|
||||
agentVersion = version
|
||||
}
|
||||
if version, ok := config["version"].(float64); ok {
|
||||
cfgVersion = int(version)
|
||||
}
|
||||
// The `version` field has historically been serialized both as a JSON number
|
||||
// (install template writes `"version": 5`) and as a string (config.Config types
|
||||
// it as a string; the agent rewrites it that way on save). Detection must read
|
||||
// both, or the version gate misfires and re-triggers migrations forever.
|
||||
cfgVersion = parseConfigVersion(config["version"])
|
||||
|
||||
return agentVersion, cfgVersion, nil
|
||||
}
|
||||
|
||||
// parseConfigVersion coerces the config `version` field to an int regardless of
|
||||
// whether it was stored as a JSON number or a string.
|
||||
func parseConfigVersion(v interface{}) int {
|
||||
switch t := v.(type) {
|
||||
case float64:
|
||||
return int(t)
|
||||
case string:
|
||||
if n, err := strconv.Atoi(strings.TrimSpace(t)); err == nil {
|
||||
return n
|
||||
}
|
||||
}
|
||||
return 0
|
||||
}
|
||||
|
||||
// determineRequiredMigrations determines what migrations are needed
|
||||
func determineRequiredMigrations(detection *MigrationDetection, config *FileDetectionConfig) []string {
|
||||
var migrations []string
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ import (
|
|||
"github.com/Fimeg/RedFlag/agent/internal/common"
|
||||
"github.com/Fimeg/RedFlag/agent/internal/event"
|
||||
"github.com/Fimeg/RedFlag/agent/internal/models"
|
||||
"github.com/Fimeg/RedFlag/agent/internal/version"
|
||||
"github.com/gofrs/uuid/v5"
|
||||
)
|
||||
|
||||
|
|
@ -155,6 +156,28 @@ func (e *MigrationExecutor) ExecuteMigration() (*MigrationResult, error) {
|
|||
}
|
||||
}
|
||||
|
||||
// Phase 3b: config v5 schema migration.
|
||||
// The actual schema reshaping is performed lazily by the config package on load
|
||||
// (mergeConfigPreservingDefaults + migrateConfig). This phase's job is to record
|
||||
// completion and bump the on-disk config `version` so detection converges; without
|
||||
// it, config_v5_migration has no handler, is never marked complete, and re-triggers
|
||||
// on every boot (§11.10 State Machine Exhaustiveness).
|
||||
if contains(e.plan.Detection.RequiredMigrations, "config_v5_migration") {
|
||||
targetConfigVersion := version.ExtractConfigVersionFromAgent(e.plan.TargetVersion)
|
||||
if err := e.stateManager.BumpConfigVersion(targetConfigVersion, e.plan.TargetVersion); err != nil {
|
||||
// Non-fatal: the config package also self-heals the version on load. Marking
|
||||
// completion below is what actually breaks the re-trigger loop.
|
||||
fmt.Printf("[MIGRATION] Warning: Failed to bump config version: %v\n", err)
|
||||
} else {
|
||||
e.result.AppliedChanges = append(e.result.AppliedChanges,
|
||||
fmt.Sprintf("Bumped config schema version to %s", targetConfigVersion))
|
||||
}
|
||||
|
||||
if err := e.stateManager.MarkMigrationCompleted("config_v5_migration", e.plan.BackupPath, e.plan.TargetVersion); err != nil {
|
||||
fmt.Printf("[MIGRATION] Warning: Failed to mark config v5 migration as completed: %v\n", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Phase 4: Docker secrets migration (if available)
|
||||
if contains(e.plan.Detection.RequiredMigrations, "docker_secrets_migration") {
|
||||
if e.plan.Detection.DockerDetection == nil {
|
||||
|
|
@ -320,11 +343,18 @@ func (e *MigrationExecutor) applySecurityHardening() error {
|
|||
func (e *MigrationExecutor) validateMigration() error {
|
||||
fmt.Printf("[MIGRATION] Validating migration...\n")
|
||||
|
||||
// Check that new directories exist
|
||||
// Ensure new directories exist. These dirs are created on demand elsewhere
|
||||
// (install script, GetAgentStateDir callers) and are not always materialized by
|
||||
// a non-legacy migration, so a bare os.Stat here produced a false "not found" on
|
||||
// Windows. Create-then-verify: MkdirAll is idempotent, and only a stat that still
|
||||
// fails after a successful MkdirAll is a real failure.
|
||||
newDirectories := []string{e.plan.Config.NewConfigPath, e.plan.Config.NewStatePath}
|
||||
for _, newDir := range newDirectories {
|
||||
if err := os.MkdirAll(newDir, 0755); err != nil {
|
||||
return fmt.Errorf("failed to ensure new directory %s: %w", newDir, err)
|
||||
}
|
||||
if _, err := os.Stat(newDir); err != nil {
|
||||
return fmt.Errorf("new directory %s not found: %w", newDir, err)
|
||||
return fmt.Errorf("new directory %s not found after create: %w", newDir, err)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
140
agent/internal/migration/executor_config_v5_test.go
Normal file
140
agent/internal/migration/executor_config_v5_test.go
Normal file
|
|
@ -0,0 +1,140 @@
|
|||
package migration
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestConfigV5MigrationMarksCompleteAndConverges proves the fix for the Windows
|
||||
// re-trigger loop: config_v5_migration must be recorded as completed and the
|
||||
// on-disk config version must be bumped, so a second detection pass does not
|
||||
// re-add it. Before the fix, config_v5_migration had no executor phase and was
|
||||
// never marked complete, so it re-triggered on every boot.
|
||||
func TestConfigV5MigrationMarksCompleteAndConverges(t *testing.T) {
|
||||
tmp := t.TempDir()
|
||||
configDir := filepath.Join(tmp, "agent")
|
||||
stateDir := filepath.Join(tmp, "state")
|
||||
if err := os.MkdirAll(configDir, 0755); err != nil {
|
||||
t.Fatalf("mkdir config dir: %v", err)
|
||||
}
|
||||
configPath := filepath.Join(configDir, "config.json")
|
||||
|
||||
// Seed a config at an old schema version with no migration state.
|
||||
seed := map[string]interface{}{
|
||||
"version": 4,
|
||||
"agent_version": "0.2.6.5",
|
||||
"agent_id": "7ba67463-b796-4b20-be7d-780d37f2b239",
|
||||
"token": "x",
|
||||
}
|
||||
writeJSON(t, configPath, seed)
|
||||
|
||||
detectionCfg := &FileDetectionConfig{
|
||||
OldConfigPath: filepath.Join(tmp, "old-config"),
|
||||
OldStatePath: filepath.Join(tmp, "old-state"),
|
||||
NewConfigPath: configDir,
|
||||
NewStatePath: stateDir,
|
||||
BackupDirPattern: filepath.Join(tmp, "backups", "%s"),
|
||||
}
|
||||
|
||||
detection := &MigrationDetection{
|
||||
CurrentAgentVersion: "0.2.6.5",
|
||||
CurrentConfigVersion: 4,
|
||||
RequiresMigration: true,
|
||||
RequiredMigrations: []string{"config_v5_migration"},
|
||||
Inventory: &AgentFileInventory{},
|
||||
}
|
||||
|
||||
plan := &MigrationPlan{
|
||||
Detection: detection,
|
||||
TargetVersion: "0.2.6.7",
|
||||
Config: detectionCfg,
|
||||
BackupPath: filepath.Join(tmp, "backup"),
|
||||
}
|
||||
|
||||
executor := NewMigrationExecutor(plan, configPath)
|
||||
result, err := executor.ExecuteMigration()
|
||||
if err != nil {
|
||||
t.Fatalf("ExecuteMigration returned error: %v", err)
|
||||
}
|
||||
if !result.Success {
|
||||
t.Fatalf("migration did not succeed: %+v", result.Errors)
|
||||
}
|
||||
|
||||
// 1. config_v5_migration must now be recorded as completed.
|
||||
sm := NewStateManager(configPath)
|
||||
completed, err := sm.IsMigrationCompleted("config_v5_migration")
|
||||
if err != nil {
|
||||
t.Fatalf("IsMigrationCompleted error: %v", err)
|
||||
}
|
||||
if !completed {
|
||||
t.Fatal("config_v5_migration was not marked complete after migration")
|
||||
}
|
||||
|
||||
// 2. The on-disk config version must be bumped to the target schema (7).
|
||||
// It is normalized to a string, the type config.Config.Version expects.
|
||||
got := readJSON(t, configPath)
|
||||
if v, _ := got["version"].(string); v != "7" {
|
||||
t.Fatalf("config version not bumped: got %v (%T) want \"7\"", got["version"], got["version"])
|
||||
}
|
||||
|
||||
// 3. A fresh detection pass must NOT re-add config_v5_migration (convergence).
|
||||
migrations := determineRequiredMigrations(detection, detectionCfg)
|
||||
for _, m := range migrations {
|
||||
if m == "config_v5_migration" {
|
||||
t.Fatalf("config_v5_migration re-added after completion; migrations=%v", migrations)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateMigrationCreatesMissingDirs proves validateMigration no longer
|
||||
// fails when the new state/config directories do not yet exist — it creates
|
||||
// them rather than reporting a false "not found" (the Windows symptom).
|
||||
func TestValidateMigrationCreatesMissingDirs(t *testing.T) {
|
||||
tmp := t.TempDir()
|
||||
configDir := filepath.Join(tmp, "agent") // intentionally not pre-created
|
||||
stateDir := filepath.Join(tmp, "agent", "state") // intentionally not pre-created
|
||||
|
||||
plan := &MigrationPlan{
|
||||
Detection: &MigrationDetection{Inventory: &AgentFileInventory{}},
|
||||
Config: &FileDetectionConfig{
|
||||
NewConfigPath: configDir,
|
||||
NewStatePath: stateDir,
|
||||
},
|
||||
}
|
||||
executor := NewMigrationExecutor(plan, filepath.Join(configDir, "config.json"))
|
||||
|
||||
if err := executor.validateMigration(); err != nil {
|
||||
t.Fatalf("validateMigration failed on missing dirs: %v", err)
|
||||
}
|
||||
for _, d := range []string{configDir, stateDir} {
|
||||
if _, err := os.Stat(d); err != nil {
|
||||
t.Fatalf("expected dir %s to exist after validation: %v", d, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func writeJSON(t *testing.T, path string, v map[string]interface{}) {
|
||||
t.Helper()
|
||||
data, err := json.MarshalIndent(v, "", " ")
|
||||
if err != nil {
|
||||
t.Fatalf("marshal: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(path, data, 0644); err != nil {
|
||||
t.Fatalf("write %s: %v", path, err)
|
||||
}
|
||||
}
|
||||
|
||||
func readJSON(t *testing.T, path string) map[string]interface{} {
|
||||
t.Helper()
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
t.Fatalf("read %s: %v", path, err)
|
||||
}
|
||||
var v map[string]interface{}
|
||||
if err := json.Unmarshal(data, &v); err != nil {
|
||||
t.Fatalf("unmarshal %s: %v", path, err)
|
||||
}
|
||||
return v
|
||||
}
|
||||
|
|
@ -11,7 +11,15 @@ import (
|
|||
|
||||
// MigrationState is imported from config package to avoid duplication
|
||||
|
||||
// StateManager manages migration state persistence
|
||||
// StateManager manages migration state persistence.
|
||||
//
|
||||
// It operates on the config file as a generic JSON map rather than unmarshaling
|
||||
// into the typed config.Config. This is deliberate: StateManager only owns the
|
||||
// `migration_state`, `version`, and `agent_version` keys, and must stay robust
|
||||
// against field-type drift elsewhere in config.json. In particular the install
|
||||
// template writes `"version"` as a JSON number while config.Config types it as a
|
||||
// string — unmarshaling the whole struct here would fail and silently prevent
|
||||
// migration completion from ever persisting (the Windows re-trigger loop).
|
||||
type StateManager struct {
|
||||
configPath string
|
||||
}
|
||||
|
|
@ -25,52 +33,54 @@ func NewStateManager(configPath string) *StateManager {
|
|||
|
||||
// LoadState loads migration state from config file
|
||||
func (sm *StateManager) LoadState() (*config.MigrationState, error) {
|
||||
// Load config to get migration state
|
||||
cfg, err := sm.loadConfig()
|
||||
raw, err := sm.loadConfigMap()
|
||||
if err != nil {
|
||||
if os.IsNotExist(err) {
|
||||
// Fresh install - no migration state yet
|
||||
return &config.MigrationState{
|
||||
LastCompleted: make(map[string]time.Time),
|
||||
AgentVersion: "",
|
||||
ConfigVersion: "",
|
||||
Timestamp: time.Now().UTC(),
|
||||
Success: false,
|
||||
CompletedMigrations: []string{},
|
||||
}, nil
|
||||
return newEmptyState(), nil
|
||||
}
|
||||
return nil, fmt.Errorf("failed to load config: %w", err)
|
||||
}
|
||||
|
||||
// Check if migration state exists in config
|
||||
if cfg.MigrationState == nil {
|
||||
return &config.MigrationState{
|
||||
LastCompleted: make(map[string]time.Time),
|
||||
AgentVersion: cfg.AgentVersion,
|
||||
ConfigVersion: cfg.Version,
|
||||
Timestamp: time.Now().UTC(),
|
||||
Success: false,
|
||||
CompletedMigrations: []string{},
|
||||
}, nil
|
||||
msRaw, ok := raw["migration_state"]
|
||||
if !ok || msRaw == nil {
|
||||
// No migration state recorded yet - seed from what we can read.
|
||||
state := newEmptyState()
|
||||
if v, ok := raw["agent_version"].(string); ok {
|
||||
state.AgentVersion = v
|
||||
}
|
||||
if v, ok := raw["version"].(string); ok {
|
||||
state.ConfigVersion = v
|
||||
}
|
||||
return state, nil
|
||||
}
|
||||
|
||||
return cfg.MigrationState, nil
|
||||
// Re-marshal the migration_state subtree and decode into the typed struct.
|
||||
data, err := json.Marshal(msRaw)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to re-marshal migration state: %w", err)
|
||||
}
|
||||
var state config.MigrationState
|
||||
if err := json.Unmarshal(data, &state); err != nil {
|
||||
return nil, fmt.Errorf("failed to decode migration state: %w", err)
|
||||
}
|
||||
if state.LastCompleted == nil {
|
||||
state.LastCompleted = make(map[string]time.Time)
|
||||
}
|
||||
return &state, nil
|
||||
}
|
||||
|
||||
// SaveState saves migration state to config file
|
||||
func (sm *StateManager) SaveState(state *config.MigrationState) error {
|
||||
// Load current config
|
||||
cfg, err := sm.loadConfig()
|
||||
raw, err := sm.loadConfigMap()
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to load config for state save: %w", err)
|
||||
}
|
||||
|
||||
// Update migration state
|
||||
cfg.MigrationState = state
|
||||
state.Timestamp = time.Now().UTC()
|
||||
raw["migration_state"] = state
|
||||
|
||||
// Save config with updated state
|
||||
return sm.saveConfig(cfg)
|
||||
return sm.saveConfigMap(raw)
|
||||
}
|
||||
|
||||
// IsMigrationCompleted checks if a specific migration was completed
|
||||
|
|
@ -107,8 +117,6 @@ func (sm *StateManager) MarkMigrationCompleted(migrationType string, rollbackPat
|
|||
for _, completed := range state.CompletedMigrations {
|
||||
if completed == migrationType {
|
||||
found = true
|
||||
// Update timestamp
|
||||
state.LastCompleted[migrationType] = time.Now().UTC()
|
||||
break
|
||||
}
|
||||
}
|
||||
|
|
@ -117,6 +125,9 @@ func (sm *StateManager) MarkMigrationCompleted(migrationType string, rollbackPat
|
|||
state.CompletedMigrations = append(state.CompletedMigrations, migrationType)
|
||||
}
|
||||
|
||||
if state.LastCompleted == nil {
|
||||
state.LastCompleted = make(map[string]time.Time)
|
||||
}
|
||||
state.LastCompleted[migrationType] = time.Now().UTC()
|
||||
state.AgentVersion = agentVersion
|
||||
state.Success = true
|
||||
|
|
@ -127,6 +138,25 @@ func (sm *StateManager) MarkMigrationCompleted(migrationType string, rollbackPat
|
|||
return sm.SaveState(state)
|
||||
}
|
||||
|
||||
// BumpConfigVersion persists the config schema version (and agent version) to the
|
||||
// config file so migration detection converges on the next boot. Without this, a
|
||||
// config-schema migration whose actual schema work is done lazily on load (by the
|
||||
// config package's mergeConfigPreservingDefaults/migrateConfig) leaves the on-disk
|
||||
// `version` field stale, and detection keeps re-triggering the same migration.
|
||||
func (sm *StateManager) BumpConfigVersion(targetConfigVersion, agentVersion string) error {
|
||||
raw, err := sm.loadConfigMap()
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to load config for version bump: %w", err)
|
||||
}
|
||||
|
||||
raw["version"] = targetConfigVersion
|
||||
if agentVersion != "" {
|
||||
raw["agent_version"] = agentVersion
|
||||
}
|
||||
|
||||
return sm.saveConfigMap(raw)
|
||||
}
|
||||
|
||||
// CleanupOldDirectories removes old migration directories after successful migration
|
||||
func (sm *StateManager) CleanupOldDirectories() error {
|
||||
oldDirs := []string{
|
||||
|
|
@ -146,27 +176,39 @@ func (sm *StateManager) CleanupOldDirectories() error {
|
|||
return nil
|
||||
}
|
||||
|
||||
// loadConfig loads configuration from file
|
||||
func (sm *StateManager) loadConfig() (*config.Config, error) {
|
||||
// newEmptyState returns a zero-value migration state ready for use.
|
||||
func newEmptyState() *config.MigrationState {
|
||||
return &config.MigrationState{
|
||||
LastCompleted: make(map[string]time.Time),
|
||||
AgentVersion: "",
|
||||
ConfigVersion: "",
|
||||
Timestamp: time.Now().UTC(),
|
||||
Success: false,
|
||||
CompletedMigrations: []string{},
|
||||
}
|
||||
}
|
||||
|
||||
// loadConfigMap reads the config file as a generic JSON object. Returns an error
|
||||
// satisfying os.IsNotExist when the file is absent so callers can treat a fresh
|
||||
// install distinctly.
|
||||
func (sm *StateManager) loadConfigMap() (map[string]interface{}, error) {
|
||||
data, err := os.ReadFile(sm.configPath)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
var cfg config.Config
|
||||
if err := json.Unmarshal(data, &cfg); err != nil {
|
||||
return nil, err
|
||||
raw := make(map[string]interface{})
|
||||
if err := json.Unmarshal(data, &raw); err != nil {
|
||||
return nil, fmt.Errorf("failed to parse config json: %w", err)
|
||||
}
|
||||
|
||||
return &cfg, nil
|
||||
return raw, nil
|
||||
}
|
||||
|
||||
// saveConfig saves configuration to file
|
||||
func (sm *StateManager) saveConfig(cfg *config.Config) error {
|
||||
data, err := json.MarshalIndent(cfg, "", " ")
|
||||
// saveConfigMap writes the config map back, preserving every key it didn't touch.
|
||||
func (sm *StateManager) saveConfigMap(raw map[string]interface{}) error {
|
||||
data, err := json.MarshalIndent(raw, "", " ")
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return os.WriteFile(sm.configPath, data, 0644)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue