fix: discover mounted geoip db files (kubernetes#7228)

* fix: discover mounted geoip db files

* add test

* fix runtime reload of config.MaxmindEditionFiles

* add e2e test

* log missing geoip2 db

cmd/nginx/flags.go

@@ -308,15 +308,17 @@ https://blog.maxmind.com/2019/12/18/significant-changes-to-accessing-and-using-g
 	}
 
 	var err error
-	if (nginx.MaxmindLicenseKey != "" || nginx.MaxmindMirror != "") && nginx.MaxmindEditionIDs != "" {
+	if nginx.MaxmindEditionIDs != "" {
 		if err = nginx.ValidateGeoLite2DBEditions(); err != nil {
 			return false, nil, err
 		}
-		klog.InfoS("downloading maxmind GeoIP2 databases")
-		if err = nginx.DownloadGeoLite2DB(nginx.MaxmindRetriesCount, nginx.MaxmindRetriesTimeout); err != nil {
-			klog.ErrorS(err, "unexpected error downloading GeoIP2 database")
+		if nginx.MaxmindLicenseKey != "" || nginx.MaxmindMirror != "" {
+			klog.InfoS("downloading maxmind GeoIP2 databases")
+			if err = nginx.DownloadGeoLite2DB(nginx.MaxmindRetriesCount, nginx.MaxmindRetriesTimeout); err != nil {
+				klog.ErrorS(err, "unexpected error downloading GeoIP2 database")
+			}
 		}
-		config.MaxmindEditionFiles = nginx.MaxmindEditionFiles
+		config.MaxmindEditionFiles = &nginx.MaxmindEditionFiles
 	}
 
 	return false, config, err

internal/ingress/controller/config/config.go

@@ -925,7 +925,7 @@ type TemplateConfig struct {
 	ListenPorts              *ListenPorts
 	PublishService           *apiv1.Service
 	EnableMetrics            bool
-	MaxmindEditionFiles      []string
+	MaxmindEditionFiles      *[]string
 	MonitorMaxBatchSize      int
 
 	PID        string

internal/ingress/controller/controller.go

@@ -105,7 +105,7 @@ type Configuration struct {
 	ValidationWebhookKeyPath  string
 
 	GlobalExternalAuth  *ngx_config.GlobalExternalAuth
-	MaxmindEditionFiles []string
+	MaxmindEditionFiles *[]string
 
 	MonitorMaxBatchSize int
 

internal/nginx/maxmind.go

@@ -64,12 +64,19 @@ const (
 
 // GeoLite2DBExists checks if the required databases for
 // the GeoIP2 NGINX module are present in the filesystem
+// and indexes the discovered databases for iteration in
+// the config.
 func GeoLite2DBExists() bool {
+	files := []string{}
 	for _, dbName := range strings.Split(MaxmindEditionIDs, ",") {
-		if !fileExists(path.Join(geoIPPath, dbName+dbExtension)) {
+		filename := dbName + dbExtension
+		if !fileExists(path.Join(geoIPPath, filename)) {
+			klog.Error(filename, " not found")
 			return false
 		}
+		files = append(files, filename)
 	}
+	MaxmindEditionFiles = files
 
 	return true
 }
@@ -101,7 +108,6 @@ func DownloadGeoLite2DB(attempts int, period time.Duration) error {
 			if dlError != nil {
 				break
 			}
-			MaxmindEditionFiles = append(MaxmindEditionFiles, dbName+dbExtension)
 		}
 
 		lastErr = dlError
@@ -217,11 +223,13 @@ func ValidateGeoLite2DBEditions() error {
 	return nil
 }
 
-func fileExists(filePath string) bool {
+func _fileExists(filePath string) bool {
 	info, err := os.Stat(filePath)
 	if os.IsNotExist(err) {
 		return false
 	}
 
 	return !info.IsDir()
 }
+
+var fileExists = _fileExists

internal/nginx/maxmind_test.go

@@ -0,0 +1,75 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package nginx
+
+import (
+	"reflect"
+	"testing"
+)
+
+func resetForTesting() {
+	fileExists = _fileExists
+	MaxmindLicenseKey = ""
+	MaxmindEditionIDs = ""
+	MaxmindEditionFiles = []string{}
+	MaxmindMirror = ""
+}
+
+func TestGeoLite2DBExists(t *testing.T) {
+	tests := []struct {
+		name      string
+		setup     func()
+		want      bool
+		wantFiles []string
+	}{
+		{
+			name:      "empty",
+			wantFiles: []string{},
+		},
+		{
+			name: "existing files",
+			setup: func() {
+				MaxmindEditionIDs = "GeoLite2-City,GeoLite2-ASN"
+				fileExists = func(string) bool {
+					return true
+				}
+			},
+			want:      true,
+			wantFiles: []string{"GeoLite2-City.mmdb", "GeoLite2-ASN.mmdb"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			resetForTesting()
+			// mimics assignment in flags.go
+			config := &MaxmindEditionFiles
+
+			if tt.setup != nil {
+				tt.setup()
+			}
+			if got := GeoLite2DBExists(); got != tt.want {
+				t.Errorf("GeoLite2DBExists() = %v, want %v", got, tt.want)
+			}
+			if !reflect.DeepEqual(MaxmindEditionFiles, tt.wantFiles) {
+				t.Errorf("nginx.MaxmindEditionFiles = %v, want %v", MaxmindEditionFiles, tt.wantFiles)
+			}
+			if !reflect.DeepEqual(*config, tt.wantFiles) {
+				t.Errorf("config.MaxmindEditionFiles = %v, want %v", *config, tt.wantFiles)
+			}
+		})
+	}
+}

test/e2e/settings/geoip2.go

@@ -17,15 +17,23 @@ limitations under the License.
 package settings
 
 import (
+	"context"
+	"fmt"
+	"path/filepath"
 	"strings"
 
 	"net/http"
 
 	"github.com/onsi/ginkgo"
+	"github.com/stretchr/testify/assert"
 
+	appsv1 "k8s.io/api/apps/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/ingress-nginx/test/e2e/framework"
 )
 
+const testdataURL = "https://github.com/maxmind/MaxMind-DB/blob/5a0be1c0320490b8e4379dbd5295a18a648ff156/test-data/GeoLite2-Country-Test.mmdb?raw=true"
+
 var _ = framework.DescribeSetting("Geoip2", func() {
 	f := framework.NewDefaultFramework("geoip2")
 
@@ -35,6 +43,30 @@ var _ = framework.DescribeSetting("Geoip2", func() {
 		f.NewEchoDeployment()
 	})
 
+	ginkgo.It("should include geoip2 line in config when enabled and db file exists", func() {
+		edition := "GeoLite2-Country"
+
+		err := f.UpdateIngressControllerDeployment(func(deployment *appsv1.Deployment) error {
+			args := deployment.Spec.Template.Spec.Containers[0].Args
+			args = append(args, "--maxmind-edition-ids="+edition)
+			deployment.Spec.Template.Spec.Containers[0].Args = args
+			_, err := f.KubeClientSet.AppsV1().Deployments(f.Namespace).Update(context.TODO(), deployment, metav1.UpdateOptions{})
+			return err
+		})
+		assert.Nil(ginkgo.GinkgoT(), err, "updating ingress controller deployment flags")
+
+		filename := fmt.Sprintf("/etc/nginx/geoip/%s.mmdb", edition)
+		exec, err := f.ExecIngressPod(fmt.Sprintf(`sh -c "mkdir -p '%s' && wget -O '%s' '%s' 2>&1"`, filepath.Dir(filename), filename, testdataURL))
+		framework.Logf(exec)
+		assert.Nil(ginkgo.GinkgoT(), err, fmt.Sprintln("error downloading test geoip2 db", filename))
+
+		f.UpdateNginxConfigMapData("use-geoip2", "true")
+		f.WaitForNginxConfiguration(
+			func(cfg string) bool {
+				return strings.Contains(cfg, fmt.Sprintf("geoip2 %s", filename))
+			})
+	})
+
 	ginkgo.It("should only allow requests from specific countries", func() {
 		ginkgo.Skip("GeoIP test are temporarily disabled")