fix: restore DNS during invalid-device uninstall

This commit is contained in:
Dev Scribe
2026-07-28 16:12:36 +07:00
committed by Cuong Manh Le
parent d29e7d131e
commit b79098658a
5 changed files with 118 additions and 39 deletions
+45 -30
View File
@@ -451,18 +451,7 @@ func run(appCallback *AppCallback, stopCh chan struct{}) {
p.onStopped = append(p.onStopped, func() { p.onStopped = append(p.onStopped, func() {
// restore static DNS settings or DHCP // restore static DNS settings or DHCP
p.resetDNS(false, true) p.resetDNS(false, true)
// Iterate over all physical interfaces and restore static DNS if a saved static config exists. restoreSavedStaticDNS("", false)
withEachPhysicalInterfaces("", "restore static DNS", func(i *net.Interface) error {
file := savedStaticDnsSettingsFilePath(i)
if _, err := os.Stat(file); err == nil {
if err := restoreDNS(i); err != nil {
mainLog.Load().Error().Err(err).Msgf("Could not restore static DNS on interface %s", i.Name)
} else {
mainLog.Load().Debug().Msgf("Restored static DNS on interface %s successfully", i.Name)
}
}
return nil
})
}) })
close(waitCh) close(waitCh)
@@ -826,7 +815,7 @@ func processLogAndCacheFlags(v *viper.Viper, cfg *ctrld.Config) {
} }
func netInterface(ifaceName string) (*net.Interface, error) { func netInterface(ifaceName string) (*net.Interface, error) {
if ifaceName == "auto" { if ifaceName == autoIface {
ifaceName = defaultIfaceName() ifaceName = defaultIfaceName()
} }
var iface *net.Interface var iface *net.Interface
@@ -1112,23 +1101,7 @@ func uninstall(p *prog, s service.Service) {
} }
// restore static DNS settings or DHCP // restore static DNS settings or DHCP
p.resetDNS(false, true) p.resetDNS(false, true)
restoreSavedStaticDNS(p.runningIface, true)
// Iterate over all physical interfaces and restore DNS if a saved static config exists.
withEachPhysicalInterfaces(p.runningIface, "restore static DNS", func(i *net.Interface) error {
file := savedStaticDnsSettingsFilePath(i)
if _, err := os.Stat(file); err == nil {
if err := restoreDNS(i); err != nil {
mainLog.Load().Error().Err(err).Msgf("Could not restore static DNS on interface %s", i.Name)
} else {
mainLog.Load().Debug().Msgf("Restored static DNS on interface %s successfully", i.Name)
err = os.Remove(file)
if err != nil {
mainLog.Load().Debug().Err(err).Msgf("Could not remove saved static DNS file for interface %s", i.Name)
}
}
}
return nil
})
if router.Name() != "" { if router.Name() != "" {
mainLog.Load().Debug().Msg("Router cleanup") mainLog.Load().Debug().Msg("Router cleanup")
@@ -1141,6 +1114,26 @@ func uninstall(p *prog, s service.Service) {
} }
} }
// restoreSavedStaticDNS restores DNS from saved static config files on physical interfaces.
func restoreSavedStaticDNS(excludeIfaceName string, removeSaved bool) {
withEachPhysicalInterfaces(excludeIfaceName, "restore static DNS", func(i *net.Interface) error {
file := savedStaticDnsSettingsFilePath(i)
if _, err := os.Stat(file); err == nil {
if err := restoreDNS(i); err != nil {
mainLog.Load().Error().Err(err).Msgf("Could not restore static DNS on interface %s", i.Name)
} else {
mainLog.Load().Debug().Msgf("Restored static DNS on interface %s successfully", i.Name)
if removeSaved {
if err := os.Remove(file); err != nil {
mainLog.Load().Debug().Err(err).Msgf("Could not remove saved static DNS file for interface %s", i.Name)
}
}
}
}
return nil
})
}
func validateConfig(cfg *ctrld.Config) error { func validateConfig(cfg *ctrld.Config) error {
if err := ctrld.ValidateConfig(validator.New(), cfg); err != nil { if err := ctrld.ValidateConfig(validator.New(), cfg); err != nil {
var ve validator.ValidationErrors var ve validator.ValidationErrors
@@ -2069,6 +2062,23 @@ func doValidateCdRemoteConfig(cdUID string, fatal bool) error {
return nil return nil
} }
// ensureRunningIfaceForInvalidUninstall populates p.runningIface before the
// invalid-device self-uninstall resets DNS. This path can run early during
// service startup (e.g. right after a reboot) before the running interface is
// otherwise known. resetDNS, via resetDNSForRunningIface, silently skips DNS
// restoration when p.runningIface is empty, which would leave the OS pointed at
// ctrld's local listener after the service is removed. See issue-556.
func ensureRunningIfaceForInvalidUninstall(p *prog, s service.Service) {
if iface == "" {
iface = autoIface
}
p.preRun()
if ir := runningIface(s); ir != nil {
p.runningIface = ir.Name
p.requiredMultiNICsConfig = ir.All
}
}
// uninstallInvalidCdUID performs self-uninstallation because the ControlD device does not exist. // uninstallInvalidCdUID performs self-uninstallation because the ControlD device does not exist.
func uninstallInvalidCdUID(p *prog, logger zerolog.Logger, doStop bool) bool { func uninstallInvalidCdUID(p *prog, logger zerolog.Logger, doStop bool) bool {
s, err := newService(p, svcConfig) s, err := newService(p, svcConfig)
@@ -2076,8 +2086,13 @@ func uninstallInvalidCdUID(p *prog, logger zerolog.Logger, doStop bool) bool {
logger.Warn().Err(err).Msg("failed to create new service") logger.Warn().Err(err).Msg("failed to create new service")
return false return false
} }
ensureRunningIfaceForInvalidUninstall(p, s)
// restore static DNS settings or DHCP // restore static DNS settings or DHCP
p.resetDNS(false, true) p.resetDNS(false, true)
// The invalid-device path may run early during service startup before runningIface
// is known. Restore every saved static DNS file so uninstalling does not leave the
// OS pointed at ctrld's local listener after the service is removed.
restoreSavedStaticDNS("", true)
tasks := []task{{s.Uninstall, true, "Uninstall"}} tasks := []task{{s.Uninstall, true, "Uninstall"}}
if doTasks(tasks) { if doTasks(tasks) {
+8 -8
View File
@@ -399,7 +399,7 @@ NOTE: running "ctrld start" without any arguments will start already installed c
reportSetDnsOk := func(sockDir string) { reportSetDnsOk := func(sockDir string) {
if cc := newSocketControlClient(ctx, s, sockDir); cc != nil { if cc := newSocketControlClient(ctx, s, sockDir); cc != nil {
if resp, _ := cc.post(ifacePath, nil); resp != nil && resp.StatusCode == http.StatusOK { if resp, _ := cc.post(ifacePath, nil); resp != nil && resp.StatusCode == http.StatusOK {
if iface == "auto" { if iface == autoIface {
iface = defaultIfaceName() iface = defaultIfaceName()
} }
res := &ifaceResponse{} res := &ifaceResponse{}
@@ -748,7 +748,7 @@ NOTE: running "ctrld start" without any arguments will start already installed c
startCmd.Run(cmd, args) startCmd.Run(cmd, args)
}, },
} }
startCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", "auto", `Update DNS setting for iface, "auto" means the default interface gateway`) startCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", autoIface, `Update DNS setting for iface, "auto" means the default interface gateway`)
startCmdAlias.Flags().AddFlagSet(startCmd.Flags()) startCmdAlias.Flags().AddFlagSet(startCmd.Flags())
rootCmd.AddCommand(startCmdAlias) rootCmd.AddCommand(startCmdAlias)
@@ -833,7 +833,7 @@ func initStopCmd() *cobra.Command {
stopCmd.Run(cmd, args) stopCmd.Run(cmd, args)
}, },
} }
stopCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", "auto", `Reset DNS setting for iface, "auto" means the default interface gateway`) stopCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", autoIface, `Reset DNS setting for iface, "auto" means the default interface gateway`)
stopCmdAlias.Flags().AddFlagSet(stopCmd.Flags()) stopCmdAlias.Flags().AddFlagSet(stopCmd.Flags())
rootCmd.AddCommand(stopCmdAlias) rootCmd.AddCommand(stopCmdAlias)
@@ -865,7 +865,7 @@ func initRestartCmd() *cobra.Command {
return return
} }
if iface == "" { if iface == "" {
iface = "auto" iface = autoIface
} }
p.preRun() p.preRun()
if ir := runningIface(s); ir != nil { if ir := runningIface(s); ir != nil {
@@ -1111,7 +1111,7 @@ NOTE: Uninstalling will set DNS to values provided by DHCP.`,
return return
} }
if iface == "" { if iface == "" {
iface = "auto" iface = autoIface
} }
p.preRun() p.preRun()
if ir := runningIface(s); ir != nil { if ir := runningIface(s); ir != nil {
@@ -1207,7 +1207,7 @@ NOTE: Uninstalling will set DNS to values provided by DHCP.`,
uninstallCmd.Run(cmd, args) uninstallCmd.Run(cmd, args)
}, },
} }
uninstallCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", "auto", `Reset DNS setting for iface, "auto" means the default interface gateway`) uninstallCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", autoIface, `Reset DNS setting for iface, "auto" means the default interface gateway`)
uninstallCmdAlias.Flags().AddFlagSet(uninstallCmd.Flags()) uninstallCmdAlias.Flags().AddFlagSet(uninstallCmd.Flags())
rootCmd.AddCommand(uninstallCmdAlias) rootCmd.AddCommand(uninstallCmdAlias)
@@ -1400,7 +1400,7 @@ func initUpgradeCmd() *cobra.Command {
return return
} }
if iface == "" { if iface == "" {
iface = "auto" iface = autoIface
} }
p.preRun() p.preRun()
if ir := runningIface(s); ir != nil { if ir := runningIface(s); ir != nil {
@@ -1595,7 +1595,7 @@ func onlyInterceptFlags(args []string) bool {
} else { } else {
return false return false
} }
case arg == "--iface=auto" || arg == "--iface" || arg == "auto": case arg == "--iface="+autoIface || arg == "--iface" || arg == autoIface:
// Auto-added by startCmdAlias or its value; safe to ignore. // Auto-added by startCmdAlias or its value; safe to ignore.
continue continue
default: default:
+3
View File
@@ -56,6 +56,9 @@ const (
cdOrgFlagName = "cd-org" cdOrgFlagName = "cd-org"
customHostnameFlagName = "custom-hostname" customHostnameFlagName = "custom-hostname"
nextdnsFlagName = "nextdns" nextdnsFlagName = "nextdns"
// autoIface is the sentinel --iface value meaning "use the default gateway interface".
autoIface = "auto"
) )
func init() { func init() {
+1 -1
View File
@@ -394,7 +394,7 @@ func preserveBoundListeners(newListeners, curListeners map[string]*ctrld.Listene
} }
func (p *prog) preRun() { func (p *prog) preRun() {
if iface == "auto" { if iface == autoIface {
iface = defaultIfaceName() iface = defaultIfaceName()
p.requiredMultiNICsConfig = requiredMultiNICsConfig() p.requiredMultiNICsConfig = requiredMultiNICsConfig()
} }
+61
View File
@@ -0,0 +1,61 @@
package cli
import "testing"
// Test_ensureRunningIfaceForInvalidUninstall is a regression test for issue-556:
// after a reboot, the invalid-device self-uninstall path could run before the
// running interface was known. Because resetDNS (via resetDNSForRunningIface)
// silently skips DNS restoration when p.runningIface is empty, the OS was left
// pointed at ctrld's local listener with no internet after the service was
// removed. ensureRunningIfaceForInvalidUninstall must populate p.runningIface
// before resetDNS runs.
func Test_ensureRunningIfaceForInvalidUninstall(t *testing.T) {
// preRun mutates the package-level iface global; restore it after the test.
origIface := iface
t.Cleanup(func() { iface = origIface })
// newService needs the package service config; it is safe to build here
// because ensureRunningIfaceForInvalidUninstall only queries the (absent)
// control socket via runningIface, which returns nil when ctrld is not
// running, and performs no DNS or service mutation.
s, err := newService(&prog{}, svcConfig)
if err != nil {
t.Fatalf("newService: %v", err)
}
t.Run("iface flag already resolved but not yet copied", func(t *testing.T) {
iface = "eth-test"
p := &prog{}
// Precondition mirrors the buggy post-reboot state: an empty running
// interface would make resetDNS skip restoration entirely.
if p.runningIface != "" {
t.Fatalf("precondition: runningIface = %q, want empty", p.runningIface)
}
ensureRunningIfaceForInvalidUninstall(p, s)
if p.runningIface == "" {
t.Fatal("runningIface still empty after prepare: resetDNS would skip DNS " +
"restoration and leave the OS pointed at ctrld's local listener")
}
if p.runningIface != "eth-test" {
t.Fatalf("runningIface = %q, want the resolved iface %q", p.runningIface, "eth-test")
}
})
t.Run("iface unset falls back to auto-detected interface", func(t *testing.T) {
iface = ""
p := &prog{}
ensureRunningIfaceForInvalidUninstall(p, s)
// With iface unset the prep resolves "auto" to the default interface
// (defaultIfaceName never returns empty on the supported platforms), so
// resetDNS has a concrete interface to restore.
if p.runningIface == "" {
t.Fatal("runningIface still empty after prepare with iface unset: " +
"resetDNS would skip DNS restoration")
}
})
}