mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-25 13:10:03 +00:00
Reproduce partial recovery when guest metadata cannot be imported (#3244)
**Reproduce partial recovery when guest metadata cannot be imported** Cover false-success metadata failures, whole-file rollback and restored absence, plus HTTP failure audit and unchanged runtime controls. Contract-Neutral: Adds regression controls for the existing configuration recovery transaction; no runtime, archive format, API or access contract changes. **Commit guest metadata with configuration recovery** Stage guest metadata with the archive's settings and credentials, publish the live cache only after a successful commit, and serialize it with pending identity snapshots. Restore newly created targets and an original moved aside before a failed replacement. Preserve normalization, scope, archive compatibility and ordinary metadata-writer ownership. Contract-Neutral: Repairs the existing all-or-nothing configuration recovery behaviour; archive formats, API response shape, tenant authority and normal metadata mutation contracts are unchanged. **Keep recovery backups and legacy settings inside the import transaction** The full preservation controls exposed backup writes outside staging. Stage node and API-token backups, delay pruning until every section commits, and include the legacy settings-file update. Preserve normal non-import save behaviour and successful import backup retention; verify prior backups and settings survive failed recovery. Contract-Neutral: Completes failure preservation for the existing configuration import transaction without changing archive formats, API response shape, tenant scope or ordinary save and backup-retention contracts. --- Fleet candidate `20261025T063939Z-core-runtime` from the `core-runtime` lane. Reviewed tip `a16f19c370e3d390eec3fd49566528e43221721f` on `main`. [How Pulse handles triage](https://github.com/rcourtman/Pulse/blob/main/docs/AI_TRANSPARENCY.md)
This commit is contained in:
@@ -0,0 +1,119 @@
|
||||
package configapi
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"reflect"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/config"
|
||||
)
|
||||
|
||||
func TestHandleImportConfigMetadataFailureDoesNotApplyOrSucceed(t *testing.T) {
|
||||
for _, failure := range []string{"metadata_directory", "closed_store"} {
|
||||
t.Run(failure, func(t *testing.T) {
|
||||
const passphrase = "synthetic-import-validation-passphrase"
|
||||
cfg := &config.Config{DataPath: t.TempDir(), PublicURL: "https://keep.invalid", AuthUser: "keep-admin", AuthPass: "synthetic-auth-hash"}
|
||||
p := config.NewConfigPersistence(cfg.DataPath)
|
||||
settings := config.DefaultSystemSettings()
|
||||
settings.PublicURL = cfg.PublicURL
|
||||
if err := p.SaveSystemSettings(*settings); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := p.SaveAPITokens([]config.APITokenRecord{{ID: "keep-token", Hash: "synthetic-token-hash", OrgID: "default"}}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(cfg.DataPath, ".env"), []byte("AUTO_UPDATE_ENABLED=true\nPOLLING_INTERVAL=77\n"), 0600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
store := p.GetGuestMetadataStore()
|
||||
if err := store.Set("keep:101", &config.GuestMetadata{Description: "Retained operator metadata"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
metadataBefore := store.GetAll()
|
||||
if failure == "closed_store" {
|
||||
if err := store.Close(time.Second); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
} else {
|
||||
metadataPath := filepath.Join(cfg.DataPath, "guest_metadata.json")
|
||||
if err := os.Remove(metadataPath); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.Mkdir(metadataPath, 0700); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
before := metadataImportFiles(t, cfg.DataPath)
|
||||
beforeConfig := cfg.DeepCopy()
|
||||
reloads, audits := 0, 0
|
||||
h := &ConfigHandlers{defaultConfig: cfg, defaultPersistence: p, reloadFunc: func() error { reloads++; return nil }}
|
||||
h.SetRuntimeDependencies(RuntimeDependencies{AuditEvent: func(_, event, _, _, _ string, success bool, _ string) {
|
||||
audits++
|
||||
if event != "config_imported" || success {
|
||||
t.Error("failed metadata import produced a success audit")
|
||||
}
|
||||
}})
|
||||
plaintext, err := json.Marshal(map[string]any{
|
||||
"version": "4.4", "nodes": map[string]any{}, "alerts": map[string]any{},
|
||||
"email": map[string]any{}, "webhooks": nil, "system": map[string]any{},
|
||||
"guestMetadata": map[string]any{"imported:202": map[string]string{"description": failure}},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
request := ImportConfigRequest{Data: encryptedImportValidationFixture(t, plaintext, passphrase), Passphrase: passphrase}
|
||||
body, err := json.Marshal(request)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
rec := httptest.NewRecorder()
|
||||
h.HandleImportConfig(rec, httptest.NewRequest(http.MethodPost, "/api/config/import", bytes.NewReader(body)))
|
||||
if rec.Code != http.StatusBadRequest || rec.Body.String() != "Failed to import configuration. Verify the backup file and passphrase are correct.\n" {
|
||||
t.Fatalf("failed metadata import response = %d %q", rec.Code, rec.Body.String())
|
||||
}
|
||||
if reloads != 0 || audits != 1 || !reflect.DeepEqual(beforeConfig, cfg.DeepCopy()) {
|
||||
t.Error("failed import changed runtime or triggered apply")
|
||||
}
|
||||
if !reflect.DeepEqual(before, metadataImportFiles(t, cfg.DataPath)) || !reflect.DeepEqual(metadataBefore, store.GetAll()) {
|
||||
t.Error("failed import changed persisted or live state")
|
||||
}
|
||||
readback, err := json.Marshal(map[string]any{"request": request, "status": rec.Code, "contentType": rec.Header().Get("Content-Type"), "body": rec.Body.String()})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Logf("SYNTHETIC_METADATA_IMPORT_REJECTION_READBACK %s", readback)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func metadataImportFiles(t *testing.T, dir string) map[string]string {
|
||||
t.Helper()
|
||||
files := make(map[string]string)
|
||||
if err := filepath.WalkDir(dir, func(path string, entry os.DirEntry, err error) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
info, err := entry.Info()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
files[path] = info.Mode().String()
|
||||
if !entry.IsDir() {
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
files[path] += string(data)
|
||||
}
|
||||
return nil
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return files
|
||||
}
|
||||
@@ -258,15 +258,13 @@ func (c *ConfigPersistence) ImportConfig(encryptedData string, passphrase string
|
||||
}
|
||||
}
|
||||
|
||||
if err := tx.Commit(); err != nil {
|
||||
if err := c.GetGuestMetadataStore().commitImport(tx, exportData.GuestMetadata); err != nil {
|
||||
return fmt.Errorf("failed to commit import transaction: %w", err)
|
||||
}
|
||||
committed = true
|
||||
|
||||
// Import guest metadata into the active persistence scope.
|
||||
if err := c.GetGuestMetadataStore().ReplaceAll(exportData.GuestMetadata); err != nil {
|
||||
log.Warn().Err(err).Msg("Failed to import guest metadata")
|
||||
}
|
||||
c.mu.Lock()
|
||||
c.cleanupOldBackups(c.nodesFile + ".backup-*")
|
||||
c.mu.Unlock()
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -0,0 +1,178 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/notifications"
|
||||
)
|
||||
|
||||
func metadataImportArchive(t *testing.T, metadata map[string]*GuestMetadata) string {
|
||||
t.Helper()
|
||||
settings := DefaultSystemSettings()
|
||||
settings.PublicURL = "https://imported.invalid"
|
||||
data, err := json.Marshal(ExportData{
|
||||
Version: "4.4", System: *settings,
|
||||
Email: notifications.EmailConfig{SMTPHost: "imported.invalid", Password: "synthetic-imported-secret"},
|
||||
APITokens: []APITokenRecord{{ID: "imported-token", Hash: "synthetic-imported-hash", OrgID: "default"}},
|
||||
GuestMetadata: metadata,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return encryptImportValidationData(t, data)
|
||||
}
|
||||
|
||||
func TestImportConfigMetadataFailurePreservesState(t *testing.T) {
|
||||
for _, failure := range []string{"metadata_directory", "closed_store", "later_destination", "later_destination_without_metadata_file"} {
|
||||
t.Run(failure, func(t *testing.T) {
|
||||
cp := seedImportValidationState(t)
|
||||
seedMetadataImportRecoveryArtifacts(t, cp)
|
||||
store := cp.GetGuestMetadataStore()
|
||||
metadataBefore := store.GetAll()
|
||||
metadataFile := filepath.Join(cp.configDir, "guest_metadata.json")
|
||||
var blocker string
|
||||
switch failure {
|
||||
case "metadata_directory":
|
||||
blocker = metadataFile
|
||||
case "closed_store":
|
||||
if err := store.Close(time.Second); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
case "later_destination_without_metadata_file":
|
||||
if err := os.Remove(metadataFile); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
blocker = cp.systemFile
|
||||
case "later_destination":
|
||||
blocker = cp.systemFile
|
||||
}
|
||||
if blocker != "" {
|
||||
if err := os.Remove(blocker); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.Mkdir(blocker, 0700); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(blocker, "keep"), []byte("retained blocker"), 0600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
before := importValidationFiles(t, cp.configDir)
|
||||
err := cp.ImportConfig(metadataImportArchive(t, map[string]*GuestMetadata{
|
||||
"imported:202": {Description: "Imported metadata", Tags: []string{"imported"}},
|
||||
}), "synthetic-import-validation-passphrase")
|
||||
if err == nil {
|
||||
t.Error("failed metadata recovery was reported as successful")
|
||||
}
|
||||
if !reflect.DeepEqual(before, importValidationFiles(t, cp.configDir)) {
|
||||
t.Error("failed import changed files, credentials or transaction artifacts")
|
||||
}
|
||||
if !reflect.DeepEqual(metadataBefore, store.GetAll()) {
|
||||
t.Error("failed import replaced live metadata")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestImportConfigCommitsMetadataWithConfiguration(t *testing.T) {
|
||||
for _, inventory := range []string{"nil", "empty", "populated"} {
|
||||
t.Run(inventory, func(t *testing.T) {
|
||||
cp := seedImportValidationState(t)
|
||||
seedMetadataImportRecoveryArtifacts(t, cp)
|
||||
nodesBefore, err := os.ReadFile(cp.nodesFile)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
tokensBefore, err := os.ReadFile(cp.apiTokensFile)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var metadata map[string]*GuestMetadata
|
||||
expected := map[string]*GuestMetadata{}
|
||||
if inventory == "empty" {
|
||||
metadata = map[string]*GuestMetadata{}
|
||||
}
|
||||
if inventory == "populated" {
|
||||
metadata = map[string]*GuestMetadata{
|
||||
"imported:202": {ID: "wrong-id", Description: "Imported description", Notes: []string{"retained note"}},
|
||||
"null-entry": nil,
|
||||
}
|
||||
expected["imported:202"] = &GuestMetadata{ID: "imported:202", Description: "Imported description", Notes: []string{"retained note"}, Tags: []string{}}
|
||||
}
|
||||
if err := cp.ImportConfig(metadataImportArchive(t, metadata), "synthetic-import-validation-passphrase"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for path, want := range map[string][]byte{cp.nodesFile + ".backup": nodesBefore, cp.apiTokensFile + ".backup": tokensBefore} {
|
||||
if got, err := os.ReadFile(path); err != nil || string(got) != string(want) {
|
||||
t.Error("successful import did not retain its previous configuration backup")
|
||||
}
|
||||
}
|
||||
backups, err := filepath.Glob(cp.nodesFile + ".backup-*")
|
||||
if err != nil || len(backups) != 10 {
|
||||
t.Fatal("successful import did not apply the existing ten-backup retention")
|
||||
}
|
||||
env, err := os.ReadFile(filepath.Join(cp.configDir, ".env"))
|
||||
if err != nil || strings.Contains(string(env), "POLLING_INTERVAL=") || !strings.Contains(string(env), "AUTO_UPDATE_ENABLED=false") || !strings.Contains(string(env), "KEEP_SETTING=retained") {
|
||||
t.Fatal("successful import did not commit the existing legacy settings-file update")
|
||||
}
|
||||
if !reflect.DeepEqual(cp.GetGuestMetadataStore().GetAll(), expected) {
|
||||
t.Error("successful import did not publish normalized metadata")
|
||||
}
|
||||
reloaded := NewGuestMetadataStore(cp.configDir, nil)
|
||||
if !reflect.DeepEqual(reloaded.GetAll(), expected) {
|
||||
t.Error("fresh metadata reader does not match the imported live state")
|
||||
}
|
||||
if err := cp.GetGuestMetadataStore().Load(); err != nil {
|
||||
t.Fatalf("committed metadata cannot be reloaded: %v", err)
|
||||
}
|
||||
settings, err := cp.LoadSystemSettings()
|
||||
if err != nil || settings.PublicURL != "https://imported.invalid" {
|
||||
t.Fatal("successful import did not persist settings")
|
||||
}
|
||||
tokens, err := cp.LoadAPITokens()
|
||||
if err != nil || len(tokens) != 1 || tokens[0].ID != "imported-token" {
|
||||
t.Fatal("successful import did not persist credentials")
|
||||
}
|
||||
info, err := os.Stat(filepath.Join(cp.configDir, "guest_metadata.json"))
|
||||
if err != nil || info.Mode().Perm() != 0600 {
|
||||
t.Fatal("imported metadata is not a private file")
|
||||
}
|
||||
for path := range importValidationFiles(t, cp.configDir) {
|
||||
if strings.HasPrefix(filepath.Base(path), ".import-") || strings.Contains(filepath.Base(path), ".import-backup-") {
|
||||
t.Error("successful import retained transaction staging")
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func seedMetadataImportRecoveryArtifacts(t *testing.T, cp *ConfigPersistence) {
|
||||
t.Helper()
|
||||
for path, data := range map[string]string{
|
||||
filepath.Join(cp.configDir, ".env"): "UPDATE_CHANNEL=stable\nAUTO_UPDATE_ENABLED=true\nPOLLING_INTERVAL=77\nKEEP_SETTING=retained\n",
|
||||
cp.nodesFile + ".backup": "prior node backup",
|
||||
cp.apiTokensFile + ".backup": "prior token backup",
|
||||
} {
|
||||
if err := os.WriteFile(path, []byte(data), 0600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
// More than the normal retention bound makes premature pruning visible.
|
||||
for i := 0; i < 11; i++ {
|
||||
path := fmt.Sprintf("%s.backup-20260101-%06d", cp.nodesFile, i)
|
||||
if err := os.WriteFile(path, []byte(fmt.Sprintf("retained backup %d", i)), 0600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
stamp := time.Date(2000, 1, 1, 0, i, 0, 0, time.UTC)
|
||||
if err := os.Chtimes(path, stamp, stamp); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,52 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"path/filepath"
|
||||
)
|
||||
|
||||
// commitImport includes guest metadata in the same file transaction as settings
|
||||
// and credentials. Keep the store's writer and mutation locks through commit so
|
||||
// an older background snapshot cannot overwrite the imported file, and readers
|
||||
// cannot observe imported metadata before its configuration has committed.
|
||||
func (s *GuestMetadataStore) commitImport(tx *importTransaction, metadata map[string]*GuestMetadata) error {
|
||||
s.writeMu.Lock()
|
||||
defer s.writeMu.Unlock()
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
if s.closed.Load() {
|
||||
return ErrGuestMetadataStoreClosed
|
||||
}
|
||||
|
||||
working := make(map[string]*GuestMetadata, len(metadata))
|
||||
for id, meta := range metadata {
|
||||
if meta == nil {
|
||||
continue
|
||||
}
|
||||
clone := cloneGuestMetadata(meta)
|
||||
clone.ID = id
|
||||
if clone.Tags == nil {
|
||||
clone.Tags = []string{}
|
||||
}
|
||||
working[id] = clone
|
||||
}
|
||||
data, err := json.Marshal(working)
|
||||
if err != nil {
|
||||
return fmt.Errorf("serialize imported guest metadata: %w", err)
|
||||
}
|
||||
if err := tx.StageFile(filepath.Join(s.dataPath, "guest_metadata.json"), data, 0600); err != nil {
|
||||
return fmt.Errorf("stage imported guest metadata: %w", err)
|
||||
}
|
||||
if err := tx.Commit(); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Publish only after every file committed. A pending writer will now see
|
||||
// that this revision is durable instead of replaying a pre-import snapshot.
|
||||
s.metadata = working
|
||||
s.revision++
|
||||
s.attemptedRevision = s.revision
|
||||
s.lastWriteErr = nil
|
||||
return nil
|
||||
}
|
||||
@@ -0,0 +1,134 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"reflect"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
type importBlockedMetadataFS struct {
|
||||
FileSystem
|
||||
once sync.Once
|
||||
entered chan struct{}
|
||||
release chan struct{}
|
||||
}
|
||||
|
||||
func (f *importBlockedMetadataFS) WriteFile(path string, data []byte, perm os.FileMode) error {
|
||||
if filepath.Base(path) == "guest_metadata.json.tmp" {
|
||||
f.once.Do(func() { close(f.entered); <-f.release })
|
||||
}
|
||||
return f.FileSystem.WriteFile(path, data, perm)
|
||||
}
|
||||
|
||||
func TestGuestMetadataImportOrdersPendingSnapshotsAndCopiesInput(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
fs := &importBlockedMetadataFS{FileSystem: defaultFileSystem{}, entered: make(chan struct{}), release: make(chan struct{})}
|
||||
store := NewGuestMetadataStore(dir, fs)
|
||||
var releaseOnce sync.Once
|
||||
release := func() { releaseOnce.Do(func() { close(fs.release) }) }
|
||||
t.Cleanup(func() {
|
||||
release()
|
||||
if err := store.Close(time.Second); err != nil {
|
||||
t.Error(err)
|
||||
}
|
||||
})
|
||||
if err := store.SetAsync("old:101", &GuestMetadata{Description: "Old snapshot"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
select {
|
||||
case <-fs.entered:
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("pending metadata writer did not enter")
|
||||
}
|
||||
if err := store.RememberIdentity("old:101", "Old guest", "qemu"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
tx, err := newImportTransaction(dir)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer tx.Cleanup()
|
||||
settings := filepath.Join(dir, "system.json")
|
||||
if err := os.WriteFile(settings, []byte("old settings"), 0600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := tx.StageFile(settings, []byte("imported settings"), 0600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
metadata := map[string]*GuestMetadata{"new:202": {ID: "wrong", Description: "Imported snapshot", Tags: []string{"imported"}, Notes: []string{"note"}}}
|
||||
done := make(chan error, 1)
|
||||
go func() { done <- store.commitImport(tx, metadata) }()
|
||||
select {
|
||||
case err := <-done:
|
||||
t.Fatalf("import overtook an in-flight metadata snapshot: %v", err)
|
||||
case <-time.After(50 * time.Millisecond):
|
||||
// A bounded negative observation, followed by the positive joined proof.
|
||||
}
|
||||
if data, err := os.ReadFile(settings); err != nil || string(data) != "old settings" {
|
||||
t.Fatal("configuration committed while the old metadata write was still in flight")
|
||||
}
|
||||
release()
|
||||
select {
|
||||
case err := <-done:
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("import did not complete after the prior snapshot")
|
||||
}
|
||||
if !store.WaitForPendingWrites(time.Second) {
|
||||
t.Fatal("pending writer did not drain")
|
||||
}
|
||||
expected := map[string]*GuestMetadata{"new:202": {ID: "new:202", Description: "Imported snapshot", Tags: []string{"imported"}, Notes: []string{"note"}}}
|
||||
metadata["new:202"].Tags[0] = "caller mutation"
|
||||
metadata["new:202"].Notes[0] = "caller mutation"
|
||||
if !reflect.DeepEqual(store.GetAll(), expected) {
|
||||
t.Fatal("import retained old records or caller-owned slices")
|
||||
}
|
||||
data, err := os.ReadFile(filepath.Join(dir, "guest_metadata.json"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var persisted map[string]*GuestMetadata
|
||||
if err := json.Unmarshal(data, &persisted); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !reflect.DeepEqual(persisted, expected) {
|
||||
t.Fatal("pending snapshot overwrote the imported metadata file")
|
||||
}
|
||||
if data, err := os.ReadFile(settings); err != nil || string(data) != "imported settings" {
|
||||
t.Fatal("metadata import did not commit its settings")
|
||||
}
|
||||
if err := store.Load(); err != nil {
|
||||
t.Fatalf("committed revision was left pending: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGuestMetadataImportStageFailurePreservesMemory(t *testing.T) {
|
||||
cp := seedImportValidationState(t)
|
||||
store := cp.GetGuestMetadataStore()
|
||||
before := store.GetAll()
|
||||
tx, err := newImportTransaction(cp.configDir)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// An occupied staging pathname is an actual filesystem failure before
|
||||
// commit, not a simulated successful persistence result.
|
||||
if err := os.Remove(tx.stagingDir); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(tx.stagingDir, []byte("block staging"), 0600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer tx.Cleanup()
|
||||
if err := store.commitImport(tx, map[string]*GuestMetadata{"new:202": {Description: "Do not publish"}}); err == nil {
|
||||
t.Fatal("metadata staging failure was accepted")
|
||||
}
|
||||
if !reflect.DeepEqual(before, store.GetAll()) || tx.committed {
|
||||
t.Fatal("staging failure changed live metadata or committed configuration")
|
||||
}
|
||||
}
|
||||
@@ -170,6 +170,12 @@ func (tx *importTransaction) Commit() error {
|
||||
continue
|
||||
}
|
||||
tx.backups[target] = ""
|
||||
} else {
|
||||
// This target did not exist before the import. Restoring its
|
||||
// absence matters just as much as restoring an overwritten file.
|
||||
if err := os.Remove(target); err != nil && !os.IsNotExist(err) {
|
||||
joinRestoreErr(fmt.Errorf("remove new target %s during restore: %w", target, err))
|
||||
}
|
||||
}
|
||||
}
|
||||
return restoreErr
|
||||
@@ -203,11 +209,12 @@ func (tx *importTransaction) Commit() error {
|
||||
return failWithRestore(fmt.Errorf("stat destination %s: %w", target, err))
|
||||
}
|
||||
|
||||
// Track the target before replacement: a failed rename after moving
|
||||
// the original into a backup must restore that original too.
|
||||
applied = append(applied, target)
|
||||
if err := os.Rename(stagedPath, target); err != nil {
|
||||
return failWithRestore(fmt.Errorf("apply staged file to %s: %w", target, err))
|
||||
}
|
||||
|
||||
applied = append(applied, target)
|
||||
}
|
||||
|
||||
// Successful commit: remove backups. Keep success behavior, but surface cleanup failures in logs.
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// Failed replacement must restore both overwritten files and the absence of
|
||||
// files first created by this transaction, before returning to its caller.
|
||||
func TestImportTransactionFailedCommitRestoresEveryTarget(t *testing.T) {
|
||||
for _, failure := range []string{"later_directory", "missing_staged_existing", "missing_staged_new"} {
|
||||
t.Run(failure, func(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
existing := filepath.Join(dir, "a-existing")
|
||||
created := filepath.Join(dir, "b-new")
|
||||
blocked := filepath.Join(dir, "z-directory")
|
||||
if err := os.WriteFile(existing, []byte("original"), 0640); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.Mkdir(blocked, 0700); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
tx, err := newImportTransaction(dir)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer tx.Cleanup()
|
||||
for _, target := range []string{existing, created, blocked} {
|
||||
if err := tx.StageFile(target, []byte("replacement"), 0600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
if failure != "later_directory" {
|
||||
target := existing
|
||||
if failure == "missing_staged_new" {
|
||||
target = created
|
||||
}
|
||||
if err := os.Remove(tx.staged[target]); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
if err := tx.Commit(); err == nil {
|
||||
t.Fatal("expected failed commit")
|
||||
}
|
||||
assertRestored := func() {
|
||||
t.Helper()
|
||||
data, err := os.ReadFile(existing)
|
||||
if err != nil || string(data) != "original" {
|
||||
t.Error("failed commit did not restore the original file")
|
||||
}
|
||||
info, err := os.Stat(existing)
|
||||
if err != nil || info.Mode().Perm() != 0640 {
|
||||
t.Error("failed commit did not restore original permissions")
|
||||
}
|
||||
if _, err := os.Stat(created); !os.IsNotExist(err) {
|
||||
t.Error("failed commit left a newly created configuration file")
|
||||
}
|
||||
if info, err := os.Stat(blocked); err != nil || !info.IsDir() {
|
||||
t.Error("failed commit changed the blocking directory")
|
||||
}
|
||||
}
|
||||
assertRestored()
|
||||
tx.Rollback()
|
||||
assertRestored()
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -473,6 +473,16 @@ func (c *ConfigPersistence) writeConfigFileLocked(path string, data []byte, perm
|
||||
return nil
|
||||
}
|
||||
|
||||
// writeConfigBackupLocked preserves ordinary backup writes, but makes them
|
||||
// part of an active import instead of changing recovery files before commit.
|
||||
// Caller must hold c.mu.
|
||||
func (c *ConfigPersistence) writeConfigBackupLocked(path string, data []byte, perm os.FileMode) error {
|
||||
if c.tx != nil {
|
||||
return c.tx.StageFile(path, data, perm)
|
||||
}
|
||||
return c.fs.WriteFile(path, data, perm)
|
||||
}
|
||||
|
||||
// preserveCorruptConfigFileLocked moves a config file that failed to parse or
|
||||
// decrypt aside to <path>.corrupt-<timestamp>. Loaders return empty data with
|
||||
// a nil error for such files, so read-modify-write savers would otherwise
|
||||
@@ -782,7 +792,7 @@ func (c *ConfigPersistence) persistAPITokensLocked(tokens []APITokenRecord) erro
|
||||
|
||||
// Backup previous state (best effort).
|
||||
if existing, err := c.fs.ReadFile(c.apiTokensFile); err == nil && len(existing) > 0 {
|
||||
if err := c.fs.WriteFile(c.apiTokensFile+".backup", existing, 0600); err != nil {
|
||||
if err := c.writeConfigBackupLocked(c.apiTokensFile+".backup", existing, 0600); err != nil {
|
||||
log.Warn().Err(err).Msg("Failed to create API token backup file")
|
||||
}
|
||||
}
|
||||
@@ -1634,7 +1644,7 @@ func (c *ConfigPersistence) saveNodesConfig(pveInstances []PVEInstance, pbsInsta
|
||||
// Create timestamped backup
|
||||
timestampedBackup := fmt.Sprintf("%s.backup-%s", c.nodesFile, time.Now().Format("20060102-150405"))
|
||||
if backupData, err := c.fs.ReadFile(c.nodesFile); err == nil {
|
||||
if err := c.fs.WriteFile(timestampedBackup, backupData, 0600); err != nil {
|
||||
if err := c.writeConfigBackupLocked(timestampedBackup, backupData, 0600); err != nil {
|
||||
log.Warn().Err(err).Msg("Failed to create timestamped backup of nodes config")
|
||||
} else {
|
||||
log.Info().Str("backup", timestampedBackup).Msg("Created timestamped backup of nodes config")
|
||||
@@ -1644,13 +1654,16 @@ func (c *ConfigPersistence) saveNodesConfig(pveInstances []PVEInstance, pbsInsta
|
||||
// Also maintain a "latest" backup for quick recovery
|
||||
latestBackup := c.nodesFile + ".backup"
|
||||
if backupData, err := c.fs.ReadFile(c.nodesFile); err == nil {
|
||||
if err := c.fs.WriteFile(latestBackup, backupData, 0600); err != nil {
|
||||
if err := c.writeConfigBackupLocked(latestBackup, backupData, 0600); err != nil {
|
||||
log.Warn().Err(err).Msg("Failed to create latest backup of nodes config")
|
||||
}
|
||||
}
|
||||
|
||||
// Clean up old timestamped backups (keep last 10)
|
||||
c.cleanupOldBackups(c.nodesFile + ".backup-*")
|
||||
// Imports prune only after the entire archive commits. A failed
|
||||
// recovery must not delete an older, still usable backup.
|
||||
if c.tx == nil {
|
||||
c.cleanupOldBackups(c.nodesFile + ".backup-*")
|
||||
}
|
||||
}
|
||||
|
||||
// Encrypt if crypto manager is available
|
||||
@@ -2037,6 +2050,9 @@ func (c *ConfigPersistence) saveSystemSettingsLocked(settings SystemSettings) er
|
||||
// Also update the .env file if it exists
|
||||
envFile := filepath.Join(c.configDir, ".env")
|
||||
if err := c.updateEnvFile(envFile, settings); err != nil {
|
||||
if c.tx != nil {
|
||||
return fmt.Errorf("stage imported environment settings: %w", err)
|
||||
}
|
||||
log.Warn().Err(err).Msg("Failed to update .env file")
|
||||
// Don't fail the operation if .env update fails
|
||||
}
|
||||
@@ -3877,6 +3893,9 @@ func (c *ConfigPersistence) updateEnvFile(envFile string, settings SystemSetting
|
||||
perm = 0600
|
||||
}
|
||||
}
|
||||
if c.tx != nil {
|
||||
return c.writeConfigFileLocked(envFile, []byte(content), perm)
|
||||
}
|
||||
if err := c.fs.WriteFile(tempFile, []byte(content), perm); err != nil {
|
||||
return fmt.Errorf("write temp env file %s: %w", tempFile, err)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user