mirror of
https://github.com/Control-D-Inc/ctrld.git
synced 2026-07-16 13:17:19 +02:00
fix: restore DNS during invalid-device uninstall
This commit is contained in:
committed by
Cuong Manh Le
parent
5ef0a59081
commit
265573c744
+45
-30
@@ -451,18 +451,7 @@ func run(appCallback *AppCallback, stopCh chan struct{}) {
|
||||
p.onStopped = append(p.onStopped, func() {
|
||||
// restore static DNS settings or DHCP
|
||||
p.resetDNS(false, true)
|
||||
// Iterate over all physical interfaces and restore static DNS if a saved static config exists.
|
||||
withEachPhysicalInterfaces("", "restore static DNS", func(i *net.Interface) error {
|
||||
file := ctrld.SavedStaticDnsSettingsFilePath(i)
|
||||
if _, err := os.Stat(file); err == nil {
|
||||
if err := restoreDNS(i); err != nil {
|
||||
p.Error().Err(err).Msgf("Could not restore static dns on interface %s", i.Name)
|
||||
} else {
|
||||
p.Debug().Msgf("Restored static dns on interface %s successfully", i.Name)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
})
|
||||
restoreSavedStaticDNS("", false)
|
||||
})
|
||||
|
||||
close(waitCh)
|
||||
@@ -886,7 +875,7 @@ func processLogAndCacheFlags() {
|
||||
|
||||
// netInterface returns the network interface by name
|
||||
func netInterface(ifaceName string) (*net.Interface, error) {
|
||||
if ifaceName == "auto" {
|
||||
if ifaceName == autoIface {
|
||||
ifaceName = defaultIfaceName()
|
||||
}
|
||||
var iface *net.Interface
|
||||
@@ -1142,29 +1131,33 @@ func uninstall(p *prog, s service.Service) {
|
||||
if doTasks(tasks) {
|
||||
// restore static DNS settings or DHCP
|
||||
p.resetDNS(false, 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 := ctrld.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
|
||||
})
|
||||
restoreSavedStaticDNS(p.runningIface, true)
|
||||
|
||||
mainLog.Load().Notice().Msg("Service uninstalled")
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
// 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 := ctrld.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 {
|
||||
mainLog.Load().Debug().Msg("Validating configuration")
|
||||
|
||||
@@ -2052,6 +2045,23 @@ func doValidateCdRemoteConfig(cdUID string, fatal bool) error {
|
||||
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.
|
||||
func uninstallInvalidCdUID(p *prog, logger *ctrld.Logger, doStop bool) bool {
|
||||
svcCmd := NewServiceCommand()
|
||||
@@ -2060,8 +2070,13 @@ func uninstallInvalidCdUID(p *prog, logger *ctrld.Logger, doStop bool) bool {
|
||||
logger.Warn().Err(err).Msg("Failed to create new service")
|
||||
return false
|
||||
}
|
||||
ensureRunningIfaceForInvalidUninstall(p, s)
|
||||
// restore static DNS settings or DHCP
|
||||
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"}}
|
||||
if doTasks(tasks) {
|
||||
|
||||
@@ -305,7 +305,7 @@ func onlyInterceptFlags(args []string) bool {
|
||||
} else {
|
||||
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.
|
||||
continue
|
||||
default:
|
||||
|
||||
@@ -32,7 +32,7 @@ func (sc *ServiceCommand) Restart(cmd *cobra.Command, args []string) error {
|
||||
|
||||
p.cfg = &cfg
|
||||
if iface == "" {
|
||||
iface = "auto"
|
||||
iface = autoIface
|
||||
}
|
||||
p.preRun()
|
||||
if ir := runningIface(s); ir != nil {
|
||||
|
||||
@@ -124,7 +124,7 @@ func (sc *ServiceCommand) Start(cmd *cobra.Command, args []string) error {
|
||||
reportSetDnsOk := func(sockDir string) {
|
||||
if cc := newSocketControlClient(ctx, s, sockDir); cc != nil {
|
||||
if resp, _ := cc.post(ifacePath, nil); resp != nil && resp.StatusCode == http.StatusOK {
|
||||
if iface == "auto" {
|
||||
if iface == autoIface {
|
||||
iface = defaultIfaceName()
|
||||
}
|
||||
res := &ifaceResponse{}
|
||||
@@ -456,7 +456,7 @@ NOTE: running "ctrld start" without any arguments will start already installed c
|
||||
return startCmd.RunE(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())
|
||||
|
||||
return startCmd, startCmdAlias
|
||||
|
||||
@@ -24,7 +24,7 @@ func (sc *ServiceCommand) Stop(cmd *cobra.Command, args []string) error {
|
||||
|
||||
p.cfg = &cfg
|
||||
if iface == "" {
|
||||
iface = "auto"
|
||||
iface = autoIface
|
||||
}
|
||||
p.preRun()
|
||||
if ir := runningIface(s); ir != nil {
|
||||
|
||||
@@ -26,7 +26,7 @@ func (sc *ServiceCommand) Uninstall(cmd *cobra.Command, args []string) error {
|
||||
|
||||
p.cfg = &cfg
|
||||
if iface == "" {
|
||||
iface = "auto"
|
||||
iface = autoIface
|
||||
}
|
||||
p.preRun()
|
||||
if ir := runningIface(s); ir != nil {
|
||||
|
||||
@@ -55,7 +55,7 @@ func (uc *UpgradeCommand) Upgrade(cmd *cobra.Command, args []string) error {
|
||||
}
|
||||
|
||||
if iface == "" {
|
||||
iface = "auto"
|
||||
iface = autoIface
|
||||
}
|
||||
p.preRun()
|
||||
if ir := runningIface(s); ir != nil {
|
||||
|
||||
@@ -64,6 +64,9 @@ const (
|
||||
cdOrgFlagName = "cd-org"
|
||||
customHostnameFlagName = "custom-hostname"
|
||||
nextdnsFlagName = "nextdns"
|
||||
|
||||
// autoIface is the sentinel --iface value meaning "use the default gateway interface".
|
||||
autoIface = "auto"
|
||||
)
|
||||
|
||||
// init initializes the default logger before any CLI commands are executed
|
||||
|
||||
+1
-1
@@ -384,7 +384,7 @@ func preserveBoundListeners(newListeners, curListeners map[string]*ctrld.Listene
|
||||
}
|
||||
|
||||
func (p *prog) preRun() {
|
||||
if iface == "auto" {
|
||||
if iface == autoIface {
|
||||
iface = defaultIfaceName()
|
||||
p.requiredMultiNICsConfig = requiredMultiNICsConfig()
|
||||
}
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
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.
|
||||
sc := NewServiceCommand()
|
||||
s, err := sc.newService(&prog{}, sc.createServiceConfig())
|
||||
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")
|
||||
}
|
||||
})
|
||||
}
|
||||
Reference in New Issue
Block a user