gitea-admin/proxy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: complete reset recovery safety

Browse Source
This commit is contained in:
proxyctl coordinator
2026-07-17 11:25:09 +08:00
parent 9e2418f45e
commit 4cf9731db4
9 changed files with 339 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/app/maintenance.go
+33 -6
View File
@@ -94,10 +94,12 @@ func (m *Maintenance) Doctor(ctx context.Context, applicationChecks []target.Che
}
type ResetTargetResult struct {
TargetID string
Inspection target.Inspection
Skipped bool
Err error
TargetID string
Inspection target.Inspection
Skipped bool
OperationErr error
LogErr error
Err error
}
type ResetResult struct {
Cancelled bool
@@ -105,6 +107,23 @@ type ResetResult struct {
Err error
}
// ResetConfirmation performs only read-only inspection and returns every
// warning required by Targets that a reset would attempt to disable.
func (m *Maintenance) ResetConfirmation(ctx context.Context) string {
active := activeFromState(m.state)
var warnings []string
for _, adapter := range m.targets {
inspection := adapter.Inspect(ctx, active)
if inspection.State != target.Enabled && inspection.State != target.Drifted {
continue
}
if warning := adapter.Confirmation(false); warning != "" {
warnings = append(warnings, adapter.ID()+": "+warning)
}
}
return strings.Join(warnings, "\n")
}
func (m *Maintenance) Reset(ctx context.Context, confirmed bool) ResetResult {
if !confirmed {
return ResetResult{Cancelled: true}
@@ -138,10 +157,18 @@ func (m *Maintenance) Reset(ctx context.Context, confirmed bool) ResetResult {
result.Err = errors.Join(result.Err, fmt.Errorf("reset %s Target: %w", adapter.ID(), redactError(operation.Err)))
}
detail = RedactLogDetail(detail)
operationErr := redactError(operation.Err)
var logErr error
if m.logger != nil {
_ = m.logger.Write(LogEntry{Time: time.Now(), Operation: "reset", Target: adapter.ID(), Result: outcome, Detail: detail})
if err := m.logger.Write(LogEntry{Time: time.Now(), Operation: "reset", Target: adapter.ID(), Result: outcome, Detail: detail}); err != nil {
logErr = redactError(err)
result.Err = errors.Join(result.Err, fmt.Errorf("record reset %s Target operation log: %w", adapter.ID(), logErr))
}
}
result.Targets = append(result.Targets, ResetTargetResult{TargetID: adapter.ID(), Inspection: finalInspection, Err: redactError(operation.Err)})
result.Targets = append(result.Targets, ResetTargetResult{
TargetID: adapter.ID(), Inspection: finalInspection,
OperationErr: operationErr, LogErr: logErr, Err: errors.Join(operationErr, logErr),
})
}
return result
}
internal/app/maintenance_test.go
+42 -3
View File
@@ -19,6 +19,7 @@ type maintenanceTarget struct {
checks []target.Check
disableErr error
disableCalls int
confirmation string
}
func (f *maintenanceTarget) ID() string { return f.id }
@@ -26,7 +27,7 @@ func (f *maintenanceTarget) Description() target.Description {
return target.Description{Name: strings.ToUpper(f.id)}
}
func (f *maintenanceTarget) Availability(context.Context) target.Availability { return f.available }
func (f *maintenanceTarget) Confirmation(bool) string { return "" }
func (f *maintenanceTarget) Confirmation(bool) string { return f.confirmation }
func (f *maintenanceTarget) Inspect(context.Context, *profile.Profile) target.Inspection {
return f.inspection
}
@@ -73,11 +74,14 @@ func (l *recordingLock) Lock(context.Context) (func(), error) {
return func() { l.held-- }, nil
}
type recordingLog struct{ entries []app.LogEntry }
type recordingLog struct {
entries []app.LogEntry
err error
}
func (l *recordingLog) Write(entry app.LogEntry) error {
l.entries = append(l.entries, entry)
return nil
return l.err
}
func TestDoctorReportsReadOnlyChecksEvidenceAndRedactsCredentials(t *testing.T) {
@@ -149,6 +153,41 @@ func TestResetFailureUsesFreshInspectionForResultAndRedactedLog(t *testing.T) {
}
}
func TestResetConfirmationAggregatesWarningsOnlyForManagedCandidatesWithoutMutation(t *testing.T) {
managed := &maintenanceTarget{id: "managed", inspection: target.Inspection{State: target.Enabled, Evidence: "managed"}}
drifted := &maintenanceTarget{id: "drifted", inspection: target.Inspection{State: target.Drifted, Evidence: "recovery metadata remains"}}
external := &maintenanceTarget{id: "external", inspection: target.Inspection{State: target.External, Evidence: "administrator owned"}}
managed.confirmation = "managed warning"
drifted.confirmation = "drifted warning"
external.confirmation = "must not be shown"
maintenance := app.NewMaintenance(profile.State{}, []target.Target{managed, drifted, external}, &recordingLock{}, nil)
confirmation := maintenance.ResetConfirmation(context.Background())
for _, want := range []string{"managed: managed warning", "drifted: drifted warning"} {
if !strings.Contains(confirmation, want) {
t.Errorf("confirmation missing %q: %q", want, confirmation)
}
}
if strings.Contains(confirmation, "must not be shown") || managed.disableCalls+drifted.disableCalls+external.disableCalls != 0 {
t.Fatalf("confirmation=%q mutation calls=%d/%d/%d", confirmation, managed.disableCalls, drifted.disableCalls, external.disableCalls)
}
}
func TestResetLoggerFailureIsReportedWithoutUndoingCompletedConfiguration(t *testing.T) {
adapter := &maintenanceTarget{id: "managed", inspection: target.Inspection{State: target.Enabled, Evidence: "managed"}}
logger := &recordingLog{err: errors.New("operation log disk full")}
result := app.NewMaintenance(profile.State{}, []target.Target{adapter}, &recordingLock{}, logger).Reset(context.Background(), true)
if result.Err == nil || !strings.Contains(result.Err.Error(), "operation log disk full") || len(result.Targets) != 1 {
t.Fatalf("reset result = %#v", result)
}
item := result.Targets[0]
if item.Inspection.State != target.Disabled || item.OperationErr != nil || item.LogErr == nil || item.Err == nil || adapter.disableCalls != 1 {
t.Fatalf("logger failure outcome=%#v disable calls=%d", item, adapter.disableCalls)
}
}
func TestResetRequiresConfirmationAndReportsTruthfulPartialResults(t *testing.T) {
good := &maintenanceTarget{id: "good", available: target.Availability{Available: true}, inspection: target.Inspection{State: target.Enabled, Evidence: "managed"}}
failure := errors.New("remove https://alice:secret@proxy.example failed: permission denied")
internal/dockerclient/dockerclient.go
+26
View File
@@ -358,6 +358,19 @@ func (a *Adapter) disable() error {
if err != nil {
return err
}
if err := validateManagedProxyStrings(defaults); err != nil {
return err
}
currentFingerprint := fingerprintCurrent(config)
if currentFingerprint == fingerprintOriginal(metadata) {
if err := a.files.Remove(a.configuration.RecoveryFile); err != nil && !errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("clean Docker client recovery metadata for already-restored proxy defaults: %w", err)
}
return nil
}
if currentFingerprint != metadata.Fingerprint {
return errors.New("Docker client managed proxy defaults changed after proxyctl applied them; refusing to overwrite current configuration or discard recovery metadata")
}
for _, key := range managedKeys {
prior := metadata.Original[key]
if prior.Present {
@@ -533,6 +546,19 @@ func fingerprintDefaults(values map[string]string) string {
return fingerprintSnapshot(snapshot)
}
func fingerprintOriginal(metadata recovery) string {
snapshot := make(map[string]managedValue, len(managedKeys))
for _, key := range managedKeys {
prior := metadata.Original[key]
value := managedValue{Present: prior.Present}
if prior.Present && json.Unmarshal(prior.Value, &value.Value) == nil {
value.Valid = true
}
snapshot[key] = value
}
return fingerprintSnapshot(snapshot)
}
func fingerprintCurrent(config map[string]json.RawMessage) string {
snapshot := make(map[string]managedValue, len(managedKeys))
proxies, _, proxyErr := childObject(config, "proxies")
internal/dockerclient/dockerclient_test.go
+55
View File
@@ -13,6 +13,7 @@ import (
"testing"
"time"
"proxyctl/internal/app"
"proxyctl/internal/dockerclient"
"proxyctl/internal/profile"
"proxyctl/internal/target"
@@ -202,6 +203,56 @@ func TestEnableSafelyMergesAndDisableRestoresPriorDefaults(t *testing.T) {
}
}
func TestResetDriftedDockerClientRejectsAdministratorManagedKeyEditsWithoutWritesOrCleanup(t *testing.T) {
files := &countingFS{FileSystem: dockerclient.OSFileSystem{}}
a := newAdapterWithFS(t, files)
writeConfig(t, a.ConfigPath(), `{"auths":{"keep":{}},"proxies":{"default":{"httpProxy":"http://prior.example:3128"}}}`)
if result := a.SetDesired(context.Background(), true, activeProfile()); result.Err != nil {
t.Fatal(result.Err)
}
updateDefaultKey(t, a.ConfigPath(), "httpProxy", json.RawMessage(`"http://administrator.example:3128"`))
configBefore := readFile(t, a.ConfigPath())
metadataBefore := readFile(t, a.RecoveryPath())
writesBefore := files.writes
reset := app.NewMaintenance(profile.State{Profiles: []profile.Profile{*activeProfile()}, ActiveProfile: activeProfile().Name}, []target.Target{a}, immediateLock{}, nil).Reset(context.Background(), true)
if reset.Err == nil || len(reset.Targets) != 1 || reset.Targets[0].Inspection.State != target.Drifted || reset.Targets[0].Err == nil {
t.Fatalf("conflicting reset = %#v", reset)
}
if !strings.Contains(reset.Targets[0].Err.Error(), "changed after proxyctl applied") || files.writes != writesBefore {
t.Fatalf("conflict outcome=%#v writes=%d->%d", reset.Targets[0], writesBefore, files.writes)
}
if got := readFile(t, a.ConfigPath()); got != configBefore {
t.Fatalf("conflict rewrote config:\n%s", got)
}
if got := readFile(t, a.RecoveryPath()); got != metadataBefore {
t.Fatalf("conflict changed metadata:\n%s", got)
}
}
func TestResetDriftedDockerClientAlreadyAtOriginalOnlyCleansMetadata(t *testing.T) {
files := &countingFS{FileSystem: dockerclient.OSFileSystem{}}
a := newAdapterWithFS(t, files)
original := `{"auths":{"keep":{}},"proxies":{"default":{"httpProxy":"http://prior.example:3128","custom":{"keep":true}}}}`
writeConfig(t, a.ConfigPath(), original)
if result := a.SetDesired(context.Background(), true, activeProfile()); result.Err != nil {
t.Fatal(result.Err)
}
writeConfig(t, a.ConfigPath(), original)
writesBefore := files.writes
reset := app.NewMaintenance(profile.State{Profiles: []profile.Profile{*activeProfile()}, ActiveProfile: activeProfile().Name}, []target.Target{a}, immediateLock{}, nil).Reset(context.Background(), true)
if reset.Err != nil || len(reset.Targets) != 1 || reset.Targets[0].Inspection.State != target.External {
t.Fatalf("already-original reset = %#v", reset)
}
if files.writes != writesBefore || readFile(t, a.ConfigPath()) != original {
t.Fatalf("already-original reset wrote config: writes=%d->%d", writesBefore, files.writes)
}
if _, err := os.Stat(a.RecoveryPath()); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("recovery metadata remains: %v", err)
}
}
func TestInspectionDistinguishesAllStatesAndManagedValueEditsDrift(t *testing.T) {
a := newAdapter(t, nil)
assertState(t, a, target.Disabled)
@@ -563,6 +614,10 @@ func assertState(t *testing.T, a *dockerclient.Adapter, want target.State) {
}
}
type immediateLock struct{}
func (immediateLock) Lock(context.Context) (func(), error) { return func() {}, nil }
type failingFS struct {
dockerclient.FileSystem
permission, failConfig, leak, restoreLeak, failRecoveryOnce bool
internal/dockerdaemon/dockerdaemon.go
+6
View File
@@ -348,6 +348,12 @@ func (a *Adapter) disable(ctx context.Context) error {
alreadyOriginal := metadata.OriginalExists && currentErr == nil && fingerprint(current) == fingerprint(metadata.Original)
alreadyAbsent := !metadata.OriginalExists && errors.Is(currentErr, fs.ErrNotExist)
if alreadyOriginal || alreadyAbsent {
// Recovery metadata proves proxyctl previously changed the effective
// daemon configuration. Even when the file has already been restored,
// complete the service effects before relinquishing that recovery record.
if err := a.reloadRestartInspect(ctx); err != nil {
return err
}
if err := a.files.Remove(a.configuration.RecoveryPath); err != nil && !errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("clean Docker daemon recovery metadata for already-restored configuration: %w", err)
}
internal/dockerdaemon/dockerdaemon_test.go
+70 -32
View File
@@ -23,6 +23,7 @@ type recorder struct{ events []string }
type fakeRunner struct {
recorder *recorder
show string
active string
fail map[string]error
}
@@ -36,7 +37,7 @@ func (r *fakeRunner) Run(_ context.Context, name string, args ...string) ([]byte
return []byte(r.show), nil
}
if strings.Contains(command, " is-active ") {
return []byte("active\n"), nil
return []byte(r.active), nil
}
return nil, nil
}
@@ -73,7 +74,7 @@ func fixture(t *testing.T) (*dockerdaemon.Adapter, *recorder, string, string, *f
daemonPath := filepath.Join(directory, "etc", "docker", "daemon.json")
recoveryPath := filepath.Join(directory, "state", "docker-daemon.json")
record := &recorder{}
runner := &fakeRunner{recorder: record, show: "LoadState=loaded\nFragmentPath=/lib/systemd/system/docker.service\n", fail: map[string]error{}}
runner := &fakeRunner{recorder: record, show: "LoadState=loaded\nFragmentPath=/lib/systemd/system/docker.service\n", active: "active\n", fail: map[string]error{}}
adapter := dockerdaemon.New(dockerdaemon.Configuration{DaemonConfigPath: daemonPath, RecoveryPath: recoveryPath, TrustedRoot: directory}, recordingFS{recorder: record}, runner)
return adapter, record, daemonPath, recoveryPath, runner
}
@@ -429,9 +430,75 @@ func TestDockerDaemonDisableAndResetRejectDriftWithoutMutatingDaemonOrRecovery(t
}
}
func TestDockerDaemonResetCleansAlreadyRestoredRecoveryWithoutDaemonWriteOrSystemctl(t *testing.T) {
func TestDockerDaemonResetAlreadyRestoredReloadsRestartsAndInspectsBeforeMetadataCleanup(t *testing.T) {
a, record, daemonPath, recoveryPath, _ := fixture(t)
original := []byte("{\n \"log-level\": \"debug\",\n \"proxies\": {\"http-proxy\": \"http://prior.example:3128\"}\n}\n")
prepareAlreadyRestoredDaemon(t, a, daemonPath, original)
record.events = nil
reset := app.NewMaintenance(profile.State{Profiles: []profile.Profile{*activeProfile()}, ActiveProfile: activeProfile().Name}, []target.Target{a}, immediateLock{}, nil).Reset(context.Background(), true)
if reset.Err != nil || len(reset.Targets) != 1 || reset.Targets[0].Err != nil || reset.Targets[0].Inspection.State != target.External {
t.Fatalf("already-restored reset = %#v", reset)
}
assertSubsequence(t, record.events, []string{"systemctl daemon-reload", "systemctl restart docker.service", "systemctl is-active docker.service", "remove docker-daemon.json"})
if contains(record.events, "write daemon.json") {
t.Fatalf("already-restored reset rewrote daemon config: %#v", record.events)
}
if got, err := os.ReadFile(daemonPath); err != nil || !reflect.DeepEqual(got, original) {
t.Fatalf("already-restored daemon configuration changed to %q: %v", got, err)
}
if _, err := os.Stat(recoveryPath); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("recovery metadata remains: %v", err)
}
}
func TestDockerDaemonAlreadyRestoredPreservesMetadataAndReportsFreshDriftOnEveryServiceFailure(t *testing.T) {
failures := []struct {
name string
command string
active string
}{
{name: "daemon reload command", command: "systemctl daemon-reload"},
{name: "restart command", command: "systemctl restart docker.service"},
{name: "active inspection command", command: "systemctl is-active docker.service"},
{name: "inactive service state", active: "failed\n"},
}
for _, failure := range failures {
t.Run(failure.name, func(t *testing.T) {
a, record, daemonPath, recoveryPath, runner := fixture(t)
original := []byte("{\n \"log-level\": \"debug\"\n}\n")
prepareAlreadyRestoredDaemon(t, a, daemonPath, original)
metadataBefore, err := os.ReadFile(recoveryPath)
if err != nil {
t.Fatal(err)
}
if failure.command != "" {
runner.fail[failure.command] = errors.New("service failure")
}
if failure.active != "" {
runner.active = failure.active
}
record.events = nil
reset := app.NewMaintenance(profile.State{Profiles: []profile.Profile{*activeProfile()}, ActiveProfile: activeProfile().Name}, []target.Target{a}, immediateLock{}, nil).Reset(context.Background(), true)
if reset.Err == nil || len(reset.Targets) != 1 || reset.Targets[0].Inspection.State != target.Drifted {
t.Fatalf("failed already-restored reset = %#v", reset)
}
if contains(record.events, "write daemon.json") || contains(record.events, "remove docker-daemon.json") {
t.Fatalf("service failure mutated configuration or metadata: %#v", record.events)
}
if got, err := os.ReadFile(daemonPath); err != nil || !reflect.DeepEqual(got, original) {
t.Fatalf("daemon config changed to %q: %v", got, err)
}
if got, err := os.ReadFile(recoveryPath); err != nil || !reflect.DeepEqual(got, metadataBefore) {
t.Fatalf("metadata changed to %q: %v", got, err)
}
})
}
}
func prepareAlreadyRestoredDaemon(t *testing.T, a *dockerdaemon.Adapter, daemonPath string, original []byte) {
t.Helper()
if err := os.MkdirAll(filepath.Dir(daemonPath), 0o700); err != nil {
t.Fatal(err)
}
@@ -444,35 +511,6 @@ func TestDockerDaemonResetCleansAlreadyRestoredRecoveryWithoutDaemonWriteOrSyste
if err := os.WriteFile(daemonPath, original, 0o640); err != nil {
t.Fatal(err)
}
record.events = nil
reset := app.NewMaintenance(profile.State{Profiles: []profile.Profile{*activeProfile()}, ActiveProfile: activeProfile().Name}, []target.Target{a}, immediateLock{}, nil).Reset(context.Background(), true)
if reset.Err != nil || len(reset.Targets) != 1 || reset.Targets[0].Err != nil || reset.Targets[0].Inspection.State != target.External {
t.Fatalf("already-restored reset = %#v", reset)
}
for _, event := range record.events {
if event == "write daemon.json" || strings.Contains(event, "daemon-reload") || strings.Contains(event, " restart ") {
t.Fatalf("already-restored reset changed daemon or systemd state: %#v", record.events)
}
}
if !contains(record.events, "remove docker-daemon.json") {
t.Fatalf("already-restored reset did not clean recovery metadata: %#v", record.events)
}
if got, err := os.ReadFile(daemonPath); err != nil || !reflect.DeepEqual(got, original) {
t.Fatalf("already-restored daemon configuration changed to %q: %v", got, err)
}
if info, err := os.Stat(daemonPath); err != nil || info.Mode().Perm() != 0o640 {
t.Fatalf("already-restored daemon metadata changed: info=%v err=%v", info, err)
}
if _, err := os.Stat(recoveryPath); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("recovery metadata remains: %v", err)
}
record.events = nil
retry := a.SetDesired(context.Background(), false, nil)
if retry.Err != nil || retry.Inspection.State != target.External || len(mutationEvents(record.events)) != 0 {
t.Fatalf("already-restored cleanup retry = %#v events=%#v", retry, record.events)
}
}
func TestDockerDaemonPreservesAndRestoresExistingConfiguration(t *testing.T) {