inlet/geoip: rename country-database to geo-database

This is a first step to accept another kind of GeoIP database (like
City). This also introduces the way we want to deprecate stuff:
transform the map structure.

akvorado.yaml

@@ -41,7 +41,7 @@ inlet:
   geoip:
     optional: true
     asn-database: /usr/share/GeoIP/GeoLite2-ASN.mmdb
-    country-database: /usr/share/GeoIP/GeoLite2-Country.mmdb
+    geo-database: /usr/share/GeoIP/GeoLite2-Country.mmdb
   snmp:
     workers: 10
   flow:

cmd/config.go

@@ -85,11 +85,7 @@ func (c ConfigRelatedOptions) Parse(out io.Writer, component string, config inte
 		ErrorUnused:      false,
 		Metadata:         &metadata,
 		WeaklyTypedInput: true,
-		MatchName: func(mapKey, fieldName string) bool {
-			key := strings.ToLower(strings.ReplaceAll(mapKey, "-", ""))
-			field := strings.ToLower(fieldName)
-			return key == field
-		},
+		MatchName:        helpers.MapStructureMatchName,
 		DecodeHook: mapstructure.ComposeDecodeHookFunc(
 			defaultHook,
 			zeroSliceHook,

common/helpers/mapstructure.go

@@ -3,7 +3,11 @@
 
 package helpers
 
-import "github.com/mitchellh/mapstructure"
+import (
+	"strings"
+
+	"github.com/mitchellh/mapstructure"
+)
 
 var mapstructureUnmarshallerHookFuncs = []mapstructure.DecodeHookFunc{}
 
@@ -18,3 +22,10 @@ func AddMapstructureUnmarshallerHook(hook mapstructure.DecodeHookFunc) {
 func GetMapStructureUnmarshallerHooks() []mapstructure.DecodeHookFunc {
 	return mapstructureUnmarshallerHookFuncs
 }
+
+// MapStructureMatchName tells if map key and field names are equal.
+func MapStructureMatchName(mapKey, fieldName string) bool {
+	key := strings.ToLower(strings.ReplaceAll(mapKey, "-", ""))
+	field := strings.ToLower(fieldName)
+	return key == field
+}

common/helpers/mapstructure_test.go

@@ -0,0 +1,29 @@
+// SPDX-FileCopyrightText: 2022 Free Mobile
+// SPDX-License-Identifier: AGPL-3.0-only
+
+package helpers
+
+import "testing"
+
+func TestMapStructureMatchName(t *testing.T) {
+	cases := []struct {
+		mapKey    string
+		fieldName string
+		expected  bool
+	}{
+		{"one", "one", true},
+		{"one", "One", true},
+		{"one-two", "OneTwo", true},
+		{"onetwo", "OneTwo", true},
+		{"One-Two", "OneTwo", true},
+		{"two", "one", false},
+	}
+	for _, tc := range cases {
+		got := MapStructureMatchName(tc.mapKey, tc.fieldName)
+		if got && !tc.expected {
+			t.Errorf("%q == %q but expected !=", tc.mapKey, tc.fieldName)
+		} else if !got && tc.expected {
+			t.Errorf("%q != %q but expected ==", tc.mapKey, tc.fieldName)
+		}
+	}
+}

console/data/docs/02-configuration.md

@@ -227,7 +227,7 @@ file format][], one for AS numbers, one for countries. If no database
 is provided, the component is inactive. It accepts the following keys:
 
 - `asn-database` tells the path to the ASN database
-- `country-database` tells the path to the country database
+- `geo-database` tells the path to the geo database (country or city)
 - `optional` makes the presence of the databases optional on start
   (when not present on start, the component is just disabled)
 

console/data/docs/99-changelog.md

@@ -13,6 +13,7 @@ identified with a specific icon:
 
 ## Unreleased
 
+- 💥 *inlet*: `inlet.geoip.country-database` has been renamed to `inlet.geoip.geo-database`
 - 🌱 *inlet*: add counters for GeoIP database hit/miss
 - 🌱 *docker-compose*: disable healthcheck for the conntrack-fixer container
 

inlet/geoip/config.go

@@ -3,12 +3,21 @@
 
 package geoip
 
+import (
+	"fmt"
+	"reflect"
+
+	"akvorado/common/helpers"
+
+	"github.com/mitchellh/mapstructure"
+)
+
 // Configuration describes the configuration for the GeoIP component.
 type Configuration struct {
 	// ASNDatabase defines the path to the ASN database.
 	ASNDatabase string
-	// CountryDatabase defines the path to the country database.
-	CountryDatabase string
+	// GeoDatabase defines the path to the geo database.
+	GeoDatabase string
 	// Optional tells if we need to error if not present on start.
 	Optional bool
 }
@@ -19,3 +28,37 @@ type Configuration struct {
 func DefaultConfiguration() Configuration {
 	return Configuration{}
 }
+
+// ConfigurationUnmarshallerHook normalize GeoIP configuration:
+//  - replace country-database by geo-database
+func ConfigurationUnmarshallerHook() mapstructure.DecodeHookFunc {
+	return func(from, to reflect.Value) (interface{}, error) {
+		if from.Kind() != reflect.Map || from.IsNil() || from.Type().Key().Kind() != reflect.String || to.Type() != reflect.TypeOf(Configuration{}) {
+			return from.Interface(), nil
+		}
+
+		// country-database → geo-database
+		var countryKey *reflect.Value
+		var geoKey *reflect.Value
+		for _, k := range from.MapKeys() {
+			if helpers.MapStructureMatchName(k.String(), "CountryDatabase") {
+				countryKey = &k
+			} else if helpers.MapStructureMatchName(k.String(), "GeoDatabase") {
+				geoKey = &k
+			}
+		}
+		if countryKey != nil && geoKey != nil {
+			return nil, fmt.Errorf("cannot have both %q and %q", countryKey.String(), geoKey.String())
+		}
+		if countryKey != nil {
+			from.SetMapIndex(reflect.ValueOf("geo-database"), from.MapIndex(*countryKey))
+			from.SetMapIndex(*countryKey, reflect.Value{})
+		}
+
+		return from.Interface(), nil
+	}
+}
+
+func init() {
+	helpers.AddMapstructureUnmarshallerHook(ConfigurationUnmarshallerHook())
+}

inlet/geoip/config_test.go

@@ -0,0 +1,91 @@
+// SPDX-FileCopyrightText: 2022 Free Mobile
+// SPDX-License-Identifier: AGPL-3.0-only
+
+package geoip
+
+import (
+	"testing"
+
+	"akvorado/common/helpers"
+
+	"github.com/gin-gonic/gin"
+	"github.com/mitchellh/mapstructure"
+)
+
+func TestConfigurationUnmarshallerHook(t *testing.T) {
+	cases := []struct {
+		Description string
+		Input       gin.H
+		Output      Configuration
+		Error       bool
+	}{
+		{
+			Description: "nil",
+			Input:       nil,
+		}, {
+			Description: "empty",
+			Input:       gin.H{},
+		}, {
+			Description: "no country-database, no geoip-database",
+			Input: gin.H{
+				"asn-database": "something",
+				"optional":     true,
+			},
+			Output: Configuration{
+				ASNDatabase: "something",
+				Optional:    true,
+			},
+		}, {
+			Description: "country-database, no geoip-database",
+			Input: gin.H{
+				"asn-database":     "something",
+				"country-database": "something else",
+			},
+			Output: Configuration{
+				ASNDatabase: "something",
+				GeoDatabase: "something else",
+			},
+		}, {
+			Description: "no country-database, geoip-database",
+			Input: gin.H{
+				"asn-database": "something",
+				"geo-database": "something else",
+			},
+			Output: Configuration{
+				ASNDatabase: "something",
+				GeoDatabase: "something else",
+			},
+		}, {
+			Description: "both country-database, geoip-database",
+			Input: gin.H{
+				"asn-database":     "something",
+				"geo-database":     "something else",
+				"country-database": "another value",
+			},
+			Error: true,
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.Description, func(t *testing.T) {
+			var got Configuration
+			decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
+				Result:      &got,
+				ErrorUnused: true,
+				Metadata:    nil,
+				MatchName:   helpers.MapStructureMatchName,
+				DecodeHook:  ConfigurationUnmarshallerHook(),
+			})
+			if err != nil {
+				t.Fatalf("NewDecoder() error:\n%+v", err)
+			}
+			err = decoder.Decode(tc.Input)
+			if err != nil && !tc.Error {
+				t.Fatalf("Decode() error:\n%+v", err)
+			} else if err == nil && tc.Error {
+				t.Fatal("Decode() did not return an error")
+			} else if diff := helpers.Diff(got, tc.Output); diff != "" {
+				t.Fatalf("Decode() (-got, +want):\n%s", diff)
+			}
+		})
+	}
+}

inlet/geoip/lookup.go

@@ -25,14 +25,14 @@ func (c *Component) LookupASN(ip net.IP) uint32 {
 
 // LookupCountry returns the result of a lookup for country.
 func (c *Component) LookupCountry(ip net.IP) string {
-	countryDB := c.db.country.Load()
-	if countryDB != nil {
-		country, err := countryDB.(*geoip2.Reader).Country(ip)
-		if err == nil && country.Country.IsoCode != "" {
-			c.metrics.databaseHit.WithLabelValues("country").Inc()
-			return country.Country.IsoCode
+	geoDB := c.db.geo.Load()
+	if geoDB != nil {
+		geo, err := geoDB.(*geoip2.Reader).Country(ip)
+		if err == nil && geo.Country.IsoCode != "" {
+			c.metrics.databaseHit.WithLabelValues("geo").Inc()
+			return geo.Country.IsoCode
 		}
-		c.metrics.databaseMiss.WithLabelValues("country").Inc()
+		c.metrics.databaseMiss.WithLabelValues("geo").Inc()
 	}
 	return ""
 }

inlet/geoip/lookup_test.go

@@ -47,12 +47,12 @@ func TestLookup(t *testing.T) {
 	}
 	gotMetrics := r.GetMetrics("akvorado_inlet_geoip_")
 	expectedMetrics := map[string]string{
-		`db_hits_total{database="asn"}`:        "2",
-		`db_hits_total{database="country"}`:    "3",
-		`db_misses_total{database="asn"}`:      "2",
-		`db_misses_total{database="country"}`:  "1",
-		`db_refresh_total{database="asn"}`:     "1",
-		`db_refresh_total{database="country"}`: "1",
+		`db_hits_total{database="asn"}`:    "2",
+		`db_hits_total{database="geo"}`:    "3",
+		`db_misses_total{database="asn"}`:  "2",
+		`db_misses_total{database="geo"}`:  "1",
+		`db_refresh_total{database="asn"}`: "1",
+		`db_refresh_total{database="geo"}`: "1",
 	}
 	if diff := helpers.Diff(gotMetrics, expectedMetrics); diff != "" {
 		t.Fatalf("Metrics (-got, +want):\n%s", diff)

inlet/geoip/root.go

@@ -26,8 +26,8 @@ type Component struct {
 	config Configuration
 
 	db struct {
-		country atomic.Value // *geoip2.Reader
-		asn     atomic.Value // *geoip2.Reader
+		geo atomic.Value // *geoip2.Reader
+		asn atomic.Value // *geoip2.Reader
 	}
 	metrics struct {
 		databaseRefresh *reporter.CounterVec
@@ -48,8 +48,8 @@ func New(r *reporter.Reporter, configuration Configuration, dependencies Depende
 		d:      &dependencies,
 		config: configuration,
 	}
-	if c.config.CountryDatabase != "" {
-		c.config.CountryDatabase = filepath.Clean(c.config.CountryDatabase)
+	if c.config.GeoDatabase != "" {
+		c.config.GeoDatabase = filepath.Clean(c.config.GeoDatabase)
 	}
 	if c.config.ASNDatabase != "" {
 		c.config.ASNDatabase = filepath.Clean(c.config.ASNDatabase)
@@ -106,13 +106,13 @@ func (c *Component) openDatabase(which string, path string, container *atomic.Va
 
 // Start starts the GeoIP component.
 func (c *Component) Start() error {
-	if err := c.openDatabase("country", c.config.CountryDatabase, &c.db.country); err != nil && !c.config.Optional {
+	if err := c.openDatabase("geo", c.config.GeoDatabase, &c.db.geo); err != nil && !c.config.Optional {
 		return err
 	}
 	if err := c.openDatabase("asn", c.config.ASNDatabase, &c.db.asn); err != nil && !c.config.Optional {
 		return err
 	}
-	if c.db.country.Load() == nil && c.db.asn.Load() == nil {
+	if c.db.geo.Load() == nil && c.db.asn.Load() == nil {
 		c.r.Warn().Msg("skipping GeoIP component: no database specified")
 		return nil
 	}
@@ -126,8 +126,8 @@ func (c *Component) Start() error {
 		return fmt.Errorf("cannot setup watcher: %w", err)
 	}
 	dirs := map[string]bool{}
-	if c.config.CountryDatabase != "" {
-		dirs[filepath.Dir(c.config.CountryDatabase)] = true
+	if c.config.GeoDatabase != "" {
+		dirs[filepath.Dir(c.config.GeoDatabase)] = true
 	}
 	if c.config.ASNDatabase != "" {
 		dirs[filepath.Dir(c.config.ASNDatabase)] = true
@@ -155,8 +155,8 @@ func (c *Component) Start() error {
 				if event.Op&(fsnotify.Write|fsnotify.Create) == 0 {
 					continue
 				}
-				if filepath.Clean(event.Name) == c.config.CountryDatabase {
-					c.openDatabase("country", c.config.CountryDatabase, &c.db.country)
+				if filepath.Clean(event.Name) == c.config.GeoDatabase {
+					c.openDatabase("geo", c.config.GeoDatabase, &c.db.geo)
 				}
 				if filepath.Clean(event.Name) == c.config.ASNDatabase {
 					c.openDatabase("asn", c.config.ASNDatabase, &c.db.asn)
@@ -169,7 +169,7 @@ func (c *Component) Start() error {
 
 // Stop stops the GeoIP component.
 func (c *Component) Stop() error {
-	if c.db.country.Load() == nil && c.db.asn.Load() == nil {
+	if c.db.geo.Load() == nil && c.db.asn.Load() == nil {
 		return nil
 	}
 	c.r.Info().Msg("stopping GeoIP component")

inlet/geoip/root_test.go

@@ -36,11 +36,11 @@ func copyFile(src string, dst string) {
 func TestDatabaseRefresh(t *testing.T) {
 	dir := t.TempDir()
 	config := DefaultConfiguration()
-	config.CountryDatabase = filepath.Join(dir, "country.mmdb")
+	config.GeoDatabase = filepath.Join(dir, "country.mmdb")
 	config.ASNDatabase = filepath.Join(dir, "asn.mmdb")
 
 	copyFile(filepath.Join("testdata", "GeoLite2-Country-Test.mmdb"),
-		config.CountryDatabase)
+		config.GeoDatabase)
 	copyFile(filepath.Join("testdata", "GeoLite2-ASN-Test.mmdb"),
 		config.ASNDatabase)
 
@@ -54,8 +54,8 @@ func TestDatabaseRefresh(t *testing.T) {
 	// Check we did load both databases
 	gotMetrics := r.GetMetrics("akvorado_inlet_geoip_db_")
 	expectedMetrics := map[string]string{
-		`refresh_total{database="asn"}`:     "1",
-		`refresh_total{database="country"}`: "1",
+		`refresh_total{database="asn"}`: "1",
+		`refresh_total{database="geo"}`: "1",
 	}
 	if diff := helpers.Diff(gotMetrics, expectedMetrics); diff != "" {
 		t.Fatalf("Metrics (-got, +want):\n%s", diff)
@@ -64,12 +64,12 @@ func TestDatabaseRefresh(t *testing.T) {
 	// Check we can reload the database
 	copyFile(filepath.Join("testdata", "GeoLite2-Country-Test.mmdb"),
 		filepath.Join(dir, "tmp.mmdb"))
-	os.Rename(filepath.Join(dir, "tmp.mmdb"), config.CountryDatabase)
+	os.Rename(filepath.Join(dir, "tmp.mmdb"), config.GeoDatabase)
 	time.Sleep(20 * time.Millisecond)
 	gotMetrics = r.GetMetrics("akvorado_inlet_geoip_db_")
 	expectedMetrics = map[string]string{
-		`refresh_total{database="asn"}`:     "1",
-		`refresh_total{database="country"}`: "2",
+		`refresh_total{database="asn"}`: "1",
+		`refresh_total{database="geo"}`: "2",
 	}
 	if diff := helpers.Diff(gotMetrics, expectedMetrics); diff != "" {
 		t.Fatalf("Metrics (-got, +want):\n%s", diff)
@@ -86,15 +86,15 @@ func TestStartWithoutDatabase(t *testing.T) {
 }
 
 func TestStartWithMissingDatabase(t *testing.T) {
-	countryConfiguration := DefaultConfiguration()
-	countryConfiguration.CountryDatabase = "/i/do/not/exist"
+	geoConfiguration := DefaultConfiguration()
+	geoConfiguration.GeoDatabase = "/i/do/not/exist"
 	asnConfiguration := DefaultConfiguration()
 	asnConfiguration.ASNDatabase = "/i/do/not/exist"
 	cases := []struct {
 		Name   string
 		Config Configuration
 	}{
-		{"Inexisting country database", countryConfiguration},
+		{"Inexisting geo database", geoConfiguration},
 		{"Inexisting ASN database", asnConfiguration},
 	}
 	for _, tc := range cases {

inlet/geoip/tests.go

@@ -25,7 +25,7 @@ func NewMock(t *testing.T, r *reporter.Reporter) *Component {
 	t.Helper()
 	config := DefaultConfiguration()
 	_, src, _, _ := runtime.Caller(0)
-	config.CountryDatabase = filepath.Join(path.Dir(src), "testdata", "GeoLite2-Country-Test.mmdb")
+	config.GeoDatabase = filepath.Join(path.Dir(src), "testdata", "GeoLite2-Country-Test.mmdb")
 	config.ASNDatabase = filepath.Join(path.Dir(src), "testdata", "GeoLite2-ASN-Test.mmdb")
 	c, err := New(r, config, Dependencies{Daemon: daemon.NewMock(t)})
 	if err != nil {

orchestrator/clickhouse/config_test.go

@@ -17,6 +17,7 @@ func TestNetworkNamesUnmarshalHook(t *testing.T) {
 		Description string
 		Input       map[string]interface{}
 		Output      NetworkMap
+		Error       bool
 	}{
 		{
 			Description: "nil",
@@ -61,11 +62,11 @@ func TestNetworkNamesUnmarshalHook(t *testing.T) {
 		}, {
 			Description: "Invalid subnet (1)",
 			Input:       gin.H{"192.0.2.1/38": "customer"},
-			Output:      nil,
+			Error:       true,
 		}, {
 			Description: "Invalid subnet (2)",
 			Input:       gin.H{"192.0.2.1/255.0.255.0": "customer"},
-			Output:      nil,
+			Error:       true,
 		},
 	}
 	for _, tc := range cases {
@@ -75,15 +76,17 @@ func TestNetworkNamesUnmarshalHook(t *testing.T) {
 				Result:      &got,
 				ErrorUnused: true,
 				Metadata:    nil,
-				DecodeHook: mapstructure.ComposeDecodeHookFunc(
-					NetworkMapUnmarshallerHook(),
-				),
+				DecodeHook:  NetworkMapUnmarshallerHook(),
 			})
 			if err != nil {
 				t.Fatalf("NewDecoder() error:\n%+v", err)
 			}
-			decoder.Decode(tc.Input)
-			if diff := helpers.Diff(got, tc.Output); diff != "" {
+			err = decoder.Decode(tc.Input)
+			if err != nil && !tc.Error {
+				t.Fatalf("Decode() error:\n%+v", err)
+			} else if err == nil && tc.Error {
+				t.Fatal("Decode() did not return an error")
+			} else if diff := helpers.Diff(got, tc.Output); diff != "" {
 				t.Fatalf("Decode() (-got, +want):\n%s", diff)
 			}
 		})