From f3d3ea5280b8eea467cd11fb5d8ffa6a40e01254 Mon Sep 17 00:00:00 2001 From: Tom Hayward Date: Thu, 8 Jul 2021 17:16:53 -0700 Subject: [PATCH] fix: discover mounted geoip db files (#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 | 12 ++-- internal/ingress/controller/config/config.go | 2 +- internal/ingress/controller/controller.go | 2 +- internal/nginx/maxmind.go | 14 +++- internal/nginx/maxmind_test.go | 75 ++++++++++++++++++++ test/e2e/settings/geoip2.go | 32 +++++++++ 6 files changed, 127 insertions(+), 10 deletions(-) create mode 100644 internal/nginx/maxmind_test.go diff --git a/cmd/nginx/flags.go b/cmd/nginx/flags.go index 1abe7d9e3f..b125719fdb 100644 --- a/cmd/nginx/flags.go +++ b/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 diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index f8e79e66e1..b96e267721 100644 --- a/internal/ingress/controller/config/config.go +++ b/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 diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 08ef2d2402..66fbd752de 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -105,7 +105,7 @@ type Configuration struct { ValidationWebhookKeyPath string GlobalExternalAuth *ngx_config.GlobalExternalAuth - MaxmindEditionFiles []string + MaxmindEditionFiles *[]string MonitorMaxBatchSize int diff --git a/internal/nginx/maxmind.go b/internal/nginx/maxmind.go index c764be1615..df9fd22316 100644 --- a/internal/nginx/maxmind.go +++ b/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,7 +223,7 @@ 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 @@ -225,3 +231,5 @@ func fileExists(filePath string) bool { return !info.IsDir() } + +var fileExists = _fileExists diff --git a/internal/nginx/maxmind_test.go b/internal/nginx/maxmind_test.go new file mode 100644 index 0000000000..ed78c32a16 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/test/e2e/settings/geoip2.go b/test/e2e/settings/geoip2.go index 37f99f2169..cec35f4591 100644 --- a/test/e2e/settings/geoip2.go +++ b/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")