diff --git a/Dockerfile b/Dockerfile index 8d7ba56c..396b40f8 100644 --- a/Dockerfile +++ b/Dockerfile @@ -204,7 +204,7 @@ ENV VPN_SERVICE_PROVIDER=pia \ UPDATER_VPN_SERVICE_PROVIDERS= \ # Public IP PUBLICIP_FILE="/tmp/gluetun/ip" \ - PUBLICIP_PERIOD=12h \ + PUBLICIP_ENABLED=on \ PUBLICIP_API=ipinfo \ PUBLICIP_API_TOKEN= \ # Storage diff --git a/internal/configuration/settings/errors.go b/internal/configuration/settings/errors.go index 87de8866..12026978 100644 --- a/internal/configuration/settings/errors.go +++ b/internal/configuration/settings/errors.go @@ -30,7 +30,6 @@ var ( ErrPortForwardingEnabled = errors.New("port forwarding cannot be enabled") ErrPortForwardingUserEmpty = errors.New("port forwarding username is empty") ErrPortForwardingPasswordEmpty = errors.New("port forwarding password is empty") - ErrPublicIPPeriodTooShort = errors.New("public IP address check period is too short") ErrRegionNotValid = errors.New("the region specified is not valid") ErrServerAddressNotValid = errors.New("server listening address is not valid") ErrSystemPGIDNotValid = errors.New("process group id is not valid") diff --git a/internal/configuration/settings/publicip.go b/internal/configuration/settings/publicip.go index 74360296..5e2c30ed 100644 --- a/internal/configuration/settings/publicip.go +++ b/internal/configuration/settings/publicip.go @@ -3,7 +3,6 @@ package settings import ( "fmt" "path/filepath" - "time" "github.com/qdm12/gluetun/internal/publicip/api" "github.com/qdm12/gosettings" @@ -13,11 +12,9 @@ import ( // PublicIP contains settings for port forwarding. type PublicIP struct { - // Period is the period to get the public IP address. - // It can be set to 0 to disable periodic checking. - // It cannot be nil for the internal state. - // TODO change to value and add enabled field - Period *time.Duration + // Enabled is set to true to fetch the public ip address + // information on VPN connection. It defaults to true. + Enabled *bool // IPFilepath is the public IP address status file path // to use. It can be the empty string to indicate not // to write to a file. It cannot be nil for the @@ -48,12 +45,6 @@ func (p PublicIP) UpdateWith(partialUpdate PublicIP) (updatedSettings PublicIP, } func (p PublicIP) validate() (err error) { - const minPeriod = 5 * time.Second - if *p.Period < minPeriod { - return fmt.Errorf("%w: %s must be at least %s", - ErrPublicIPPeriodTooShort, p.Period, minPeriod) - } - if *p.IPFilepath != "" { // optional _, err := filepath.Abs(*p.IPFilepath) if err != nil { @@ -71,7 +62,7 @@ func (p PublicIP) validate() (err error) { func (p *PublicIP) copy() (copied PublicIP) { return PublicIP{ - Period: gosettings.CopyPointer(p.Period), + Enabled: gosettings.CopyPointer(p.Enabled), IPFilepath: gosettings.CopyPointer(p.IPFilepath), API: p.API, APIToken: gosettings.CopyPointer(p.APIToken), @@ -79,15 +70,14 @@ func (p *PublicIP) copy() (copied PublicIP) { } func (p *PublicIP) overrideWith(other PublicIP) { - p.Period = gosettings.OverrideWithPointer(p.Period, other.Period) + p.Enabled = gosettings.OverrideWithPointer(p.Enabled, other.Enabled) p.IPFilepath = gosettings.OverrideWithPointer(p.IPFilepath, other.IPFilepath) p.API = gosettings.OverrideWithComparable(p.API, other.API) p.APIToken = gosettings.OverrideWithPointer(p.APIToken, other.APIToken) } func (p *PublicIP) setDefaults() { - const defaultPeriod = 12 * time.Hour - p.Period = gosettings.DefaultPointer(p.Period, defaultPeriod) + p.Enabled = gosettings.DefaultPointer(p.Enabled, true) p.IPFilepath = gosettings.DefaultPointer(p.IPFilepath, "/tmp/gluetun/ip") p.API = gosettings.DefaultComparable(p.API, "ipinfo") p.APIToken = gosettings.DefaultPointer(p.APIToken, "") @@ -98,19 +88,12 @@ func (p PublicIP) String() string { } func (p PublicIP) toLinesNode() (node *gotree.Node) { + if !*p.Enabled { + return gotree.New("Public IP settings: disabled") + } + node = gotree.New("Public IP settings:") - if *p.Period == 0 { - node.Appendf("Enabled: no") - return node - } - - updatePeriod := "disabled" - if *p.Period > 0 { - updatePeriod = "every " + p.Period.String() - } - node.Appendf("Fetching: %s", updatePeriod) - if *p.IPFilepath != "" { node.Appendf("IP file path: %s", *p.IPFilepath) } @@ -124,8 +107,8 @@ func (p PublicIP) toLinesNode() (node *gotree.Node) { return node } -func (p *PublicIP) read(r *reader.Reader) (err error) { - p.Period, err = r.DurationPtr("PUBLICIP_PERIOD") +func (p *PublicIP) read(r *reader.Reader, warner Warner) (err error) { + p.Enabled, err = readPublicIPEnabled(r, warner) if err != nil { return err } @@ -136,3 +119,23 @@ func (p *PublicIP) read(r *reader.Reader) (err error) { p.APIToken = r.Get("PUBLICIP_API_TOKEN") return nil } + +func readPublicIPEnabled(r *reader.Reader, warner Warner) ( + enabled *bool, err error) { + periodPtr, err := r.DurationPtr("PUBLICIP_PERIOD") // Retro-compatibility + if err != nil { + return nil, err + } else if periodPtr == nil { + return r.BoolPtr("PUBLICIP_ENABLED") + } + + if *periodPtr == 0 { + warner.Warn("please replace PUBLICIP_PERIOD=0 with PUBLICIP_ENABLED=no") + return ptrTo(false), nil + } + + warner.Warn("PUBLICIP_PERIOD is no longer used. " + + "It is assumed from its non-zero value you want PUBLICIP_ENABLED=yes. " + + "Please migrate to use PUBLICIP_ENABLED only in the future.") + return ptrTo(true), nil +} diff --git a/internal/configuration/settings/settings.go b/internal/configuration/settings/settings.go index 7ae41c3a..b26632ff 100644 --- a/internal/configuration/settings/settings.go +++ b/internal/configuration/settings/settings.go @@ -197,14 +197,16 @@ func (s *Settings) Read(r *reader.Reader, warner Warner) (err error) { "health": s.Health.Read, "http proxy": s.HTTPProxy.read, "log": s.Log.read, - "public ip": s.PublicIP.read, - "shadowsocks": s.Shadowsocks.read, - "storage": s.Storage.read, - "system": s.System.read, - "updater": s.Updater.read, - "version": s.Version.read, - "VPN": s.VPN.read, - "profiling": s.Pprof.Read, + "public ip": func(r *reader.Reader) error { + return s.PublicIP.read(r, warner) + }, + "shadowsocks": s.Shadowsocks.read, + "storage": s.Storage.read, + "system": s.System.read, + "updater": s.Updater.read, + "version": s.Version.read, + "VPN": s.VPN.read, + "profiling": s.Pprof.Read, } for name, read := range readFunctions { diff --git a/internal/configuration/settings/settings_test.go b/internal/configuration/settings/settings_test.go index 5c3350fc..6f1592be 100644 --- a/internal/configuration/settings/settings_test.go +++ b/internal/configuration/settings/settings_test.go @@ -78,7 +78,6 @@ func Test_Settings_String(t *testing.T) { | ├── Process UID: 1000 | └── Process GID: 1000 ├── Public IP settings: -| ├── Fetching: every 12h0m0s | ├── IP file path: /tmp/gluetun/ip | └── Public IP data API: ipinfo └── Version settings: diff --git a/internal/portforward/service/start.go b/internal/portforward/service/start.go index b6bc7726..e8f1d017 100644 --- a/internal/portforward/service/start.go +++ b/internal/portforward/service/start.go @@ -79,9 +79,12 @@ func (s *Service) Start(ctx context.Context) (runError <-chan error, err error) keepPortDoneCh := make(chan struct{}) s.keepPortDoneCh = keepPortDoneCh + readyCh := make(chan struct{}) go func(ctx context.Context, portForwarder PortForwarder, - obj utils.PortForwardObjects, runError chan<- error, doneCh chan<- struct{}) { + obj utils.PortForwardObjects, readyCh chan<- struct{}, + runError chan<- error, doneCh chan<- struct{}) { defer close(doneCh) + close(readyCh) err = portForwarder.KeepPortForward(ctx, obj) crashed := ctx.Err() == nil if !crashed { // stopped by Stop call @@ -91,7 +94,8 @@ func (s *Service) Start(ctx context.Context) (runError <-chan error, err error) defer s.startStopMutex.Unlock() _ = s.cleanup() runError <- err - }(keepPortCtx, s.settings.PortForwarder, obj, runErrorCh, keepPortDoneCh) + }(keepPortCtx, s.settings.PortForwarder, obj, readyCh, runErrorCh, keepPortDoneCh) + <-readyCh return runErrorCh, nil } diff --git a/internal/publicip/loop.go b/internal/publicip/loop.go index 9741502d..c50c5ce4 100644 --- a/internal/publicip/loop.go +++ b/internal/publicip/loop.go @@ -76,14 +76,8 @@ func (l *Loop) run(runCtx context.Context, runDone chan<- struct{}, updateTrigger <-chan settings.PublicIP, updatedResult chan<- error) { defer close(runDone) - timer := time.NewTimer(time.Hour) - defer timer.Stop() - _ = timer.Stop() - timerIsReadyToReset := true - lastFetch := time.Unix(0, 0) - for { - singleRunCtx := runCtx + var singleRunCtx context.Context var singleRunResult chan<- error select { case <-runCtx.Done(): @@ -91,26 +85,17 @@ func (l *Loop) run(runCtx context.Context, runDone chan<- struct{}, case singleRunCtx = <-runTrigger: // Note singleRunCtx is canceled if runCtx is canceled. singleRunResult = runResult - case <-timer.C: - timerIsReadyToReset = true case partialUpdate := <-updateTrigger: var err error - timerIsReadyToReset, err = l.update(partialUpdate, lastFetch, timer, timerIsReadyToReset) + err = l.update(partialUpdate) updatedResult <- err continue } - lastFetch = l.timeNow() - timerIsReadyToReset = l.updateTimer(*l.settings.Period, lastFetch, timer, timerIsReadyToReset) - result, err := l.fetcher.FetchInfo(singleRunCtx, netip.Addr{}) if err != nil { err = fmt.Errorf("fetching information: %w", err) - if singleRunResult != nil { - singleRunResult <- err - } else { - l.logger.Error(err.Error()) - } + singleRunResult <- err continue } @@ -128,11 +113,7 @@ func (l *Loop) run(runCtx context.Context, runDone chan<- struct{}, err = fmt.Errorf("persisting public ip address: %w", err) } - if singleRunResult != nil { - singleRunResult <- err - } else if err != nil { - l.logger.Error(err.Error()) - } + singleRunResult <- err } } diff --git a/internal/publicip/update.go b/internal/publicip/update.go index 656c56c0..f117a0a2 100644 --- a/internal/publicip/update.go +++ b/internal/publicip/update.go @@ -3,25 +3,16 @@ package publicip import ( "fmt" "os" - "time" "github.com/qdm12/gluetun/internal/configuration/settings" ) -func (l *Loop) update(partialUpdate settings.PublicIP, - lastTick time.Time, timer *time.Timer, timerIsReadyToReset bool) ( - newTimerIsReadyToReset bool, err error) { - newTimerIsReadyToReset = timerIsReadyToReset +func (l *Loop) update(partialUpdate settings.PublicIP) (err error) { // No need to lock the mutex since it can only be written // in the code below in this goroutine. updatedSettings, err := l.settings.UpdateWith(partialUpdate) if err != nil { - return newTimerIsReadyToReset, err - } - - if *l.settings.Period != *updatedSettings.Period { - newTimerIsReadyToReset = l.updateTimer(*updatedSettings.Period, lastTick, - timer, timerIsReadyToReset) + return err } if *l.settings.IPFilepath != *updatedSettings.IPFilepath { @@ -30,17 +21,17 @@ func (l *Loop) update(partialUpdate settings.PublicIP, err = persistPublicIP(*updatedSettings.IPFilepath, l.ipData.IP.String(), l.puid, l.pgid) if err != nil { - return newTimerIsReadyToReset, fmt.Errorf("persisting ip data: %w", err) + return fmt.Errorf("persisting ip data: %w", err) } case *updatedSettings.IPFilepath == "": err = os.Remove(*l.settings.IPFilepath) if err != nil { - return newTimerIsReadyToReset, fmt.Errorf("removing ip data file path: %w", err) + return fmt.Errorf("removing ip data file path: %w", err) } default: err = os.Rename(*l.settings.IPFilepath, *updatedSettings.IPFilepath) if err != nil { - return newTimerIsReadyToReset, fmt.Errorf("renaming ip data file path: %w", err) + return fmt.Errorf("renaming ip data file path: %w", err) } } } @@ -49,34 +40,5 @@ func (l *Loop) update(partialUpdate settings.PublicIP, l.settings = updatedSettings l.settingsMutex.Unlock() - return newTimerIsReadyToReset, nil -} - -func (l *Loop) updateTimer(period time.Duration, lastFetch time.Time, - timer *time.Timer, timerIsReadyToReset bool) (newTimerIsReadyToReset bool) { - disableTimer := period == 0 - if disableTimer { - if !timer.Stop() { - <-timer.C - } - return true - } - - if !timerIsReadyToReset { - if !timer.Stop() { - <-timer.C - } - } - - var waited time.Duration - if lastFetch.UnixNano() > 0 { - waited = l.timeNow().Sub(lastFetch) - } - leftToWait := period - waited - if leftToWait <= 0 { - leftToWait = time.Nanosecond - } - - timer.Reset(leftToWait) - return false + return nil }