ansible-role-nginx-config icon indicating copy to clipboard operation
ansible-role-nginx-config copied to clipboard

set_real_ip_from (ngx_http_realip_module) should be a list

Open glbyers opened this issue 1 year ago • 1 comments

Describe the bug

In the http/modules.j2 template, the realip macro assumes that set_real_ip_from can only ever be a single value. In reality, set_real_ip_from can (and likely often is) be defined multiple times.

To reproduce

Steps to reproduce the behavior:

  1. Define a nginx_config_http_template_enable some like;
nginx_config_http_template_enable:
  - template_file: http/default.conf.j2
    deployment_location: /etc/nginx/conf.d/default.conf
    backup: false
    config:
      realip:
        ## BUG: set_real_ip_from in http/modules.j2 only allows a single value.
        set_real_ip_from:
          - 127.127.127.127/32
          - 10.10.10.10/32
          - 192.192.192.192/32
        real_ip_header: X-Forwarded-For
  1. Deploy the Ansible NGINX configuration role using playbook.yml
  2. Inspect set_real_ip_from value in /etc/nginx/conf.d/default.conf

Expected behavior

set_real_ip_from should allow multiple values like many other config items do.

Your environment

  • nginxinc.nginx_core 0.8.0
  • ansible-core 2.14
  • Jinja 3.1.3
  • Any deployment platform

Additional context

The macro realip in http/modules.j2 is defined as follows;

{# NGINX HTTP RealIP -- ngx_http_realip_module #}
{% macro realip(realip) %}
{% if realip['set_real_ip_from'] is defined %}
set_real_ip_from {{ realip['set_real_ip_from'] }};
{% endif %}
{% if realip['real_ip_header'] is defined %}
real_ip_header {{ realip['real_ip_header'] }};
{% endif %}
{% if realip['real_ip_recursive'] is defined and realip['real_ip_recursive'] is boolean %}
real_ip_recursive {{ realip['real_ip_recursive'] | ternary('on', 'off') }};
{% endif %}

I think this should be;

{# NGINX HTTP RealIP -- ngx_http_realip_module #}
{% macro realip(realip) %}
{% if realip['set_real_ip_from'] is defined %}
{% for set_real_ip_from in realip['set_real_ip_from'] if realip['set_real_ip_from'] is not string %}
set_real_ip_from {{ set_real_ip_from }};
{% else %}
set_real_ip_from {{ realip['set_real_ip_from'] }};
{% endfor %}
{% endif %}
{% if realip['real_ip_header'] is defined %}
real_ip_header {{ realip['real_ip_header'] }};
{% endif %}
{% if realip['real_ip_recursive'] is defined and realip['real_ip_recursive'] is boolean %}
real_ip_recursive {{ realip['real_ip_recursive'] | ternary('on', 'off') }};
{% endif %}

glbyers avatar Mar 19 '24 03:03 glbyers

You are absolutely right! Could you please open a PR?

alessfg avatar Mar 20 '24 14:03 alessfg