Usage of GeoIP2 variables CRASH the ingress in case maxmind-license is out of download quota (or download fails)

[ThorstenEngel]

Hi, I use v1.10.1 of the ingress and have GeoIP2 enabled. If I encounter too many reloads (in my case I was upgrading my EKS-Cluster and the ingress), the database download fails. In consequence, usage of GeoIP2 variables (I use them for populating upstream-headers) will lead to an error.

Excerpt from the log:

I0612 17:36:03.967097       7 flags.go:387] "downloading maxmind GeoIP2 databases" 
...

E0612 17:36:04.901483       7 maxmind.go:74] GeoLite2-City.mmdb not found 
W0612 17:36:04.901505       7 store.go:1214] The GeoIP2 feature is enabled but the databases are missing. Disabling 
...
E0612 17:36:06.374855       7 controller.go:205] Unexpected failure reloading the backend: 

------------------------------------------------------------------------------- 
Error: exit status 1 
2024/06/12 17:36:06 [emerg] 25#25: unknown "geoip2_city" variable 
nginx: [emerg] unknown "geoip2_city" variable 

2T17:36:06.375058138Z nginx: configuration file /tmp/nginx/nginx-cfg1060362931 test failed 
...

When the GeoIP2 database download fails, it causes the ingress to disable the feature, leading to errors due to the missing variables. A viable fix for this case (enabled GeoIP2 feature, missing databases lead to disabled GeoIP2 feature) could be default values like empty strings.

Here is my ConfigMap:

kind: ConfigMap
apiVersion: v1
metadata:
  name: upstream-custom-headers
  namespace: ingress-nginx
  labels:
    app.kubernetes.io/name: ingress-nginx
    app.kubernetes.io/part-of: ingress-nginx
data:
  X-Correlation-ID: $request_id
  X-GeoIP-Country-Code: $geoip2_city_country_code
  X-GeoIP-Country-Name: $geoip2_city_country_name
  X-GeoIP-Region: $geoip2_region_name
  X-GeoIP-City: $geoip2_city
  X-GeoIP-Postal-Code: $geoip2_postal_code
  X-GeoIP-Latitude: $geoip2_latitude
  X-GeoIP-Longitude: $geoip2_longitude

It would be great, if you guys could address this. I currently must disable the GeoIP2 feature, as using it might crash my ingresses.

Warm regards,

thorsten

[k8s-ci-robot]

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.

[longwuyuan]

/remove-kind bug

I was recently commenting on a PR related to maxmind and I happened to put the db's tar.gz, in the webroot of a nginx:alpine image based pod, that was adjacent in the same namespace as the controller. Then I could use a existing flag for the controller executable like "--maxmind-mirror=http://adjacentpod" that way I download only once from maxmind and serve it perpetually from that adjacent pod.

https://github.com/kubernetes/ingress-nginx/pull/11435#issuecomment-2156246817

Wondering this flag fits as a solution for the problem you mentioned

[longwuyuan]

Also wondering about the fit of these in your problem https://github.com/ffha/geolite-mirror & https://github.com/clashdev/geolite.clash.dev

[ThorstenEngel]

I think, the ingress-nginx really belongs to the most critical building blocks in a k8s-deployment. So I think, it should be as resilient as possible. A hard dependency of an external source introduces a weakness - which in my eyes should be avoided if possible.

Yes, these solutions are a workaround, but add additional complexity. Thanks for the tip!

[longwuyuan]

Thanks @ThorstenEngel for your update. I agree the controller should be resilient. The default values like you suggested help with the Geoip2 use case.

I am not a developer. My opinion is different though. Download fails based change in the project code, requires maintenance, that is impossible with available resources. It also sets a undesired precedence for other features of the controller, to expect similar granular out-of-scope code.

But please wait for others to comment.

[github-actions[bot]]

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.

[matteovivona]

@ThorstenEngel I don't know if it helps, but I chose to download the database from a secondary source suggested from @longwuyuan, and mount the mmdb with an initContainer

By installing ingress-nginx from Helm I created this initContainer

  extraInitContainers:
    - name: download-geolite2-country
      image: busybox:1.36
      command: ['sh', '-c', 'wget https://git.io/GeoLite2-Country.mmdb --no-check-certificate && mv GeoLite2-Country.mmdb /home/GeoLite2-Country.mmdb']
      volumeMounts:
        - name: geolite2-country
          mountPath: /home

then a volume and volumemounts

  extraVolumes:
    - name: geolite2-country
      emptyDir: {}

  extraVolumeMounts:
    - name: geolite2-country
      mountPath: /etc/ingress-controller/geoip/GeoLite2-Country.mmdb
      subPath: GeoLite2-Country.mmdb

on extraArgs I pass only this option maxmind-edition-ids: GeoLite2-Country without the MaxMind licenses

[ThorstenEngel]

Yes, this would help. I personally switched to AWS Cloudfront headers (they do GeoIP Coding as well, and we use it anyqay). So I have 1 component less to take care of.

[longwuyuan]

@ThorstenEngel can we close this issue ?