Merge pull request #23 from safing/fix/intel-geoip

Fix intel/geoip not processing upgrades

intel/geoip/database.go

@@ -59,20 +59,20 @@ func doReload() error {
 
 func openDBs() error {
 	var err error
-	file, err := updates.GetFile("intel/geoip/geoip-city.mmdb")
+	dbCityFile, err = updates.GetFile("intel/geoip/geoip-city.mmdb")
 	if err != nil {
 		return fmt.Errorf("could not get GeoIP City database file: %s", err)
 	}
-	dbCity, err = maxminddb.Open(file.Path())
+	dbCity, err = maxminddb.Open(dbCityFile.Path())
 	if err != nil {
 		return err
 	}
 
-	file, err = updates.GetFile("intel/geoip/geoip-asn.mmdb")
+	dbASNFile, err = updates.GetFile("intel/geoip/geoip-asn.mmdb")
 	if err != nil {
 		return fmt.Errorf("could not get GeoIP ASN database file: %s", err)
 	}
-	dbASN, err = maxminddb.Open(file.Path())
+	dbASN, err = maxminddb.Open(dbASNFile.Path())
 	if err != nil {
 		return err
 	}

intel/geoip/module.go

@@ -4,6 +4,7 @@ import (
 	"context"
 
 	"github.com/safing/portbase/modules"
+	"github.com/safing/portmaster/updates"
 )
 
 var (
@@ -16,9 +17,9 @@ func init() {
 
 func prep() error {
 	return module.RegisterEventHook(
-		"updates",
-		"resource update",
-		"upgrade databases",
+		updates.ModuleName,
+		updates.ResourceUpdateEvent,
+		"Check for GeoIP database updates",
 		upgradeDatabases,
 	)
 }

netenv/main.go

@@ -5,8 +5,8 @@ import (
 )
 
 const (
-	networkChangedEvent      = "network changed"
-	onlineStatusChangedEvent = "online status changed"
+	NetworkChangedEvent      = "network changed"
+	OnlineStatusChangedEvent = "online status changed"
 )
 
 var (
@@ -15,8 +15,8 @@ var (
 
 func init() {
 	module = modules.Register("netenv", nil, start, nil)
-	module.RegisterEvent(networkChangedEvent)
-	module.RegisterEvent(onlineStatusChangedEvent)
+	module.RegisterEvent(NetworkChangedEvent)
+	module.RegisterEvent(OnlineStatusChangedEvent)
 }
 
 func start() error {

netenv/network-change.go

@@ -29,23 +29,21 @@ serviceLoop:
 	for {
 		trigger := false
 
+		timeout := time.Minute
+		if GetOnlineStatus() != StatusOnline {
+			timeout = time.Second
+		}
 		// wait for trigger
-		if GetOnlineStatus() == StatusOnline {
-			select {
-			case <-ctx.Done():
-				return nil
-			case <-networkChangeCheckTrigger:
-			case <-time.After(1 * time.Minute):
-				trigger = true
-			}
-		} else {
-			select {
-			case <-ctx.Done():
-				return nil
-			case <-networkChangeCheckTrigger:
-			case <-time.After(1 * time.Second):
-				trigger = true
-			}
+		select {
+		case <-ctx.Done():
+			return nil
+		case <-networkChangeCheckTrigger:
+			// don't fall through because the online change check
+			// triggers the networkChangeCheck this way. If we would set
+			// trigger == true we would trigger the online check again
+			// resulting in a loop of pointless checks.
+		case <-time.After(timeout):
+			trigger = true
 		}
 
 		// check network for changes
@@ -84,7 +82,7 @@ serviceLoop:
 			if trigger {
 				triggerOnlineStatusInvestigation()
 			}
-			module.TriggerEvent(networkChangedEvent, nil)
+			module.TriggerEvent(NetworkChangedEvent, nil)
 		}
 
 	}

netenv/online-status.go

@@ -156,7 +156,7 @@ func updateOnlineStatus(status OnlineStatus, portalURL, comment string) {
 
 	// trigger event
 	if changed {
-		module.TriggerEvent(onlineStatusChangedEvent, nil)
+		module.TriggerEvent(OnlineStatusChangedEvent, nil)
 		if status == StatusPortal {
 			log.Infof(`network: setting online status to %s at "%s" (%s)`, status, captivePortalURL, comment)
 		} else {
@@ -201,21 +201,17 @@ func triggerOnlineStatusInvestigation() {
 
 func monitorOnlineStatus(ctx context.Context) error {
 	for {
+		timeout := time.Minute
+		if GetOnlineStatus() != StatusOnline {
+			timeout = time.Second
+		}
 		// wait for trigger
-		if GetOnlineStatus() == StatusOnline {
-			select {
-			case <-ctx.Done():
-				return nil
-			case <-onlineStatusInvestigationTrigger:
-			case <-time.After(1 * time.Minute):
-			}
-		} else {
-			select {
-			case <-ctx.Done():
-				return nil
-			case <-onlineStatusInvestigationTrigger:
-			case <-time.After(1 * time.Second):
-			}
+		select {
+		case <-ctx.Done():
+			return nil
+		case <-onlineStatusInvestigationTrigger:
+
+		case <-time.After(timeout):
 		}
 
 		// enable waiting

updates/config.go

@@ -56,7 +56,7 @@ func updateRegistryConfig(_ context.Context, _ interface{}) error {
 
 	if changed {
 		registry.SelectVersions()
-		module.TriggerEvent(eventVersionUpdate, nil)
+		module.TriggerEvent(VersionUpdateEvent, nil)
 	}
 
 	return nil

updates/export.go

@@ -50,8 +50,8 @@ func initVersionExport() (err error) {
 	}
 
 	return module.RegisterEventHook(
-		"updates",
-		eventVersionUpdate,
+		ModuleName,
+		VersionUpdateEvent,
 		"export version status",
 		export,
 	)

updates/get.go

@@ -20,7 +20,7 @@ func GetPlatformFile(identifier string) (*updater.File, error) {
 		return nil, err
 	}
 
-	module.TriggerEvent(eventVersionUpdate, nil)
+	module.TriggerEvent(VersionUpdateEvent, nil)
 	return file, nil
 }
 
@@ -33,6 +33,6 @@ func GetFile(identifier string) (*updater.File, error) {
 		return nil, err
 	}
 
-	module.TriggerEvent(eventVersionUpdate, nil)
+	module.TriggerEvent(VersionUpdateEvent, nil)
 	return file, nil
 }

updates/main.go

@@ -19,8 +19,23 @@ const (
 	releaseChannelStable = "stable"
 	releaseChannelBeta   = "beta"
 
-	eventVersionUpdate  = "active version update"
-	eventResourceUpdate = "resource update"
+	// ModuleName is the name of the update module
+	// and can be used when declaring module dependencies.
+	ModuleName = "updates"
+
+	// VersionUpdateEvent is emitted every time a new
+	// version of a monitored resource is selected.
+	// During module initialization VersionUpdateEvent
+	// is also emitted.
+	VersionUpdateEvent = "active version update"
+
+	// ResourceUpdateEvent is emitted every time the
+	// updater successfully performed a resource update.
+	// ResourceUpdateEvent is emitted even if no new
+	// versions are available. Subscribers are expected
+	// to check if new versions of their resources are
+	// available by checking File.UpgradeAvailable().
+	ResourceUpdateEvent = "resource update"
 )
 
 var (
@@ -29,9 +44,9 @@ var (
 )
 
 func init() {
-	module = modules.Register("updates", registerConfig, start, stop, "base")
-	module.RegisterEvent(eventVersionUpdate)
-	module.RegisterEvent(eventResourceUpdate)
+	module = modules.Register(ModuleName, registerConfig, start, stop, "base")
+	module.RegisterEvent(VersionUpdateEvent)
+	module.RegisterEvent(ResourceUpdateEvent)
 }
 
 func start() error {
@@ -57,7 +72,7 @@ func start() error {
 
 	// create registry
 	registry = &updater.ResourceRegistry{
-		Name: "updates",
+		Name: ModuleName,
 		UpdateURLs: []string{
 			"https://updates.safing.io",
 		},
@@ -83,7 +98,7 @@ func start() error {
 	}
 
 	registry.SelectVersions()
-	module.TriggerEvent(eventVersionUpdate, nil)
+	module.TriggerEvent(VersionUpdateEvent, nil)
 
 	err = initVersionExport()
 	if err != nil {
@@ -96,7 +111,7 @@ func start() error {
 		if err != nil {
 			return fmt.Errorf("updates: failed to update: %s", err)
 		}
-		module.TriggerEvent(eventResourceUpdate, nil)
+		module.TriggerEvent(ResourceUpdateEvent, nil)
 		return nil
 	}).Repeat(24 * time.Hour).MaxDelay(1 * time.Hour).Schedule(time.Now().Add(10 * time.Second))
 

updates/upgrader.go

@@ -37,8 +37,8 @@ var (
 
 func initUpgrader() error {
 	return module.RegisterEventHook(
-		"updates",
-		eventResourceUpdate,
+		ModuleName,
+		ResourceUpdateEvent,
 		"run upgrades",
 		upgrader,
 	)