voxpupuli / puppet-nginx

Puppet Module to manage NGINX on various UNIXes
https://forge.puppet.com/puppet/nginx
MIT License
470 stars 881 forks source link

limit_zone should take Array in addition to String and Undef #1569

Closed ltning closed 10 months ago

ltning commented 1 year ago

Affected Puppet, Ruby, OS and module versions/distributions

How to reproduce (e.g Puppet code you use)

limit_zone:
    - 'remote_ip_20 burst=250'
    - 'request_line_20 burst=50'

What are you seeing

Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Evaluation Error: Error while evaluating a Resource Statement, Nginx::Resource::Location[.....]: parameter 'limit_zone' expects a value of type Undef or String, got Tuple (file: /usr/local/etc/puppet/modules/nginx/manifests/resource/server.pp, line: 637) on node ......

What behaviour did you expect instead

Multiple limit_req statements in resulting nginx configuration

Any additional information you'd like to impart

From https://nginx.org/en/docs/http/ngx_http_limit_req_module.html#limit_req:

 There could be several limit_req directives. For example, the following configuration will limit the processing rate of requests coming from a single IP address and, at the same time, the request processing rate by the virtual server:

    limit_req_zone $binary_remote_addr zone=perip:10m rate=1r/s;
    limit_req_zone $server_name zone=perserver:10m rate=10r/s;

    server {
        ...
        limit_req zone=perip burst=5 nodelay;
        limit_req zone=perserver burst=10;
    }
ltning commented 1 year ago

Suggested patch against 3.3.0:

diff --git a/modules/nginx/manifests/resource/location.pp b/modules/nginx/manifests/resource/location.pp
index 4d7e245c..b7ac3116 100644
--- a/modules/nginx/manifests/resource/location.pp
+++ b/modules/nginx/manifests/resource/location.pp
@@ -265,7 +265,7 @@ define nginx::resource::location (
   Boolean $ssl                                                     = false,
   Boolean $ssl_only                                                = false,
   Optional[String] $location_alias                                 = undef,
-  Optional[String[1]] $limit_zone                                  = undef,
+  Optional[Variant[String[1],Array[String[1],1]]] $limit_zone      = undef,
   Optional[Enum['any', 'all']] $location_satisfy                   = undef,
   Optional[Array] $location_allow                                  = undef,
   Optional[Array] $location_deny                                   = undef,
diff --git a/modules/nginx/templates/server/location_header.erb b/modules/nginx/templates/server/location_header.erb
index 7e526d11..f8c84c81 100644
--- a/modules/nginx/templates/server/location_header.erb
+++ b/modules/nginx/templates/server/location_header.erb
@@ -78,7 +78,13 @@
     <%- end -%>
 <% end -%>
 <% if @limit_zone -%>
+  <%- if @limit_zone.is_a?(Array) then -%>
+    <%- for lz in @limit_zone -%>
+    limit_req zone=<%= lz %>;
+    <%- end -%>
+  <%- else -%>
     limit_req zone=<%= @limit_zone %>;
+  <%- end -%>
 <% end -%>
 <% if @reset_timedout_connection -%>
   reset_timedout_connection <%= @reset_timedout_connection %>;