From 3106fa3e31dfcf1b21b370d9f5eab0f632aa69ff Mon Sep 17 00:00:00 2001 From: Dan Farnsworth Date: Wed, 13 Nov 2019 11:03:32 -0700 Subject: [PATCH 1/5] Fixes #1134 Add support for `limit_req_zone` in main nginx config and `limit_req` for `nginx::resource::location`. In init.pp `limit_req_zone` can be a String, or an array of String In resource/location.pp `limit_zone` can be a String and should point to a zone defined from `limit_req_zone` in init.pp --- manifests/config.pp | 1 + manifests/init.pp | 1 + manifests/resource/location.pp | 3 +++ templates/conf.d/nginx.conf.erb | 10 ++++++++++ templates/server/location_header.erb | 3 +++ 5 files changed, 18 insertions(+) diff --git a/manifests/config.pp b/manifests/config.pp index 15e2ba40e..6414dcd53 100644 --- a/manifests/config.pp +++ b/manifests/config.pp @@ -28,6 +28,7 @@ $global_owner = $nginx::global_owner $global_group = $nginx::global_group $global_mode = $nginx::global_mode + $limit_req_zone = $nginx::limit_req_zone $log_dir = $nginx::log_dir $log_user = $nginx::log_user $log_group = $nginx::log_group diff --git a/manifests/init.pp b/manifests/init.pp index 1f8136c3d..8c7966089 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -45,6 +45,7 @@ $global_owner = $nginx::params::global_owner, $global_group = $nginx::params::global_group, $global_mode = $nginx::params::global_mode, + Optional[Variant[String, Array[String]]] $limit_req_zone = undef, Stdlib::Absolutepath $log_dir = $nginx::params::log_dir, String[1] $log_user = $nginx::params::log_user, String[1] $log_group = $nginx::params::log_group, diff --git a/manifests/resource/location.pp b/manifests/resource/location.pp index 01fcc1e91..25acbd5d3 100644 --- a/manifests/resource/location.pp +++ b/manifests/resource/location.pp @@ -67,6 +67,8 @@ # [*raw_append*] - A single string, or an array of strings to # append to the location directive (after custom_cfg directives). NOTE: # YOU are responsible for a semicolon on each line that requires one. +# [*limit_zone*] - Apply a limit_req_zone to the location. Expects a string indicating a +# previously defined limit_req_zone in the main nginx configuration # [*location_custom_cfg*] - Expects a hash with custom directives, cannot # be used with other location types (proxy, fastcgi, root, or stub_status) # [*location_cfg_prepend*] - Expects a hash with extra directives to put @@ -214,6 +216,7 @@ Boolean $ssl = false, Boolean $ssl_only = false, Optional[String] $location_alias = undef, + Optional[String] $limit_zone = undef, Optional[Enum['any', 'all']] $location_satisfy = undef, Optional[Array] $location_allow = undef, Optional[Array] $location_deny = undef, diff --git a/templates/conf.d/nginx.conf.erb b/templates/conf.d/nginx.conf.erb index f79cfa0c2..d74f04e15 100644 --- a/templates/conf.d/nginx.conf.erb +++ b/templates/conf.d/nginx.conf.erb @@ -87,6 +87,16 @@ http { error_log <%= @nginx_error_log %> <%= @nginx_error_log_severity %>; <% end -%> +<% if @limit_req_zone -%> + <% if @limit_req_zone.is_a?(Array) -%> + <%- @limit_req_zone.each do |limit_req_zone_item| -%> + limit_req_zone <%= limit_req_zone_item %>; + <%- end -%> + <% else -%> + limit_req_zone <%= @limit_req_zone %>; + <% end -%> +<% end -%> + <% if @sendfile == 'on' -%> sendfile on; <%- if @http_tcp_nopush == 'on' -%> diff --git a/templates/server/location_header.erb b/templates/server/location_header.erb index 85820ed9f..c4450fe6f 100644 --- a/templates/server/location_header.erb +++ b/templates/server/location_header.erb @@ -77,3 +77,6 @@ rewrite <%= rewrite_rule %>; <%- end -%> <% end -%> +<% if @limit_zone -%> + limit_req zone=<%= @limit_zone %>; +<% end -%> From 79b39a3e582ac8ddb728bb8219f6e534a228bdf3 Mon Sep 17 00:00:00 2001 From: Dan Farnsworth Date: Mon, 18 Nov 2019 09:40:03 -0700 Subject: [PATCH 2/5] Prohibit an empty string for $limit_req_zone Co-Authored-By: Tim Meusel --- manifests/init.pp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifests/init.pp b/manifests/init.pp index 8c7966089..3c3a87098 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -45,7 +45,7 @@ $global_owner = $nginx::params::global_owner, $global_group = $nginx::params::global_group, $global_mode = $nginx::params::global_mode, - Optional[Variant[String, Array[String]]] $limit_req_zone = undef, + Optional[Variant[String[1], Array[String[1]]]] $limit_req_zone = undef, Stdlib::Absolutepath $log_dir = $nginx::params::log_dir, String[1] $log_user = $nginx::params::log_user, String[1] $log_group = $nginx::params::log_group, From 894cbc8fe77cbc673edd8329ac5c2040a20074ed Mon Sep 17 00:00:00 2001 From: Dan Farnsworth Date: Mon, 18 Nov 2019 09:40:19 -0700 Subject: [PATCH 3/5] Prohibit an empty string for $limit_zone Co-Authored-By: Tim Meusel --- manifests/resource/location.pp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifests/resource/location.pp b/manifests/resource/location.pp index 25acbd5d3..7703fcd11 100644 --- a/manifests/resource/location.pp +++ b/manifests/resource/location.pp @@ -216,7 +216,7 @@ Boolean $ssl = false, Boolean $ssl_only = false, Optional[String] $location_alias = undef, - Optional[String] $limit_zone = undef, + Optional[String[1]] $limit_zone = undef, Optional[Enum['any', 'all']] $location_satisfy = undef, Optional[Array] $location_allow = undef, Optional[Array] $location_deny = undef, From 2cd91c3ae998cd207a44973339172f8743bcf71e Mon Sep 17 00:00:00 2001 From: Dan Farnsworth Date: Mon, 18 Nov 2019 11:06:18 -0700 Subject: [PATCH 4/5] Cleanup indenting in template for limit_req_zone --- templates/conf.d/nginx.conf.erb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/templates/conf.d/nginx.conf.erb b/templates/conf.d/nginx.conf.erb index d74f04e15..64c1170cf 100644 --- a/templates/conf.d/nginx.conf.erb +++ b/templates/conf.d/nginx.conf.erb @@ -88,13 +88,13 @@ http { <% end -%> <% if @limit_req_zone -%> - <% if @limit_req_zone.is_a?(Array) -%> - <%- @limit_req_zone.each do |limit_req_zone_item| -%> - limit_req_zone <%= limit_req_zone_item %>; - <%- end -%> - <% else -%> - limit_req_zone <%= @limit_req_zone %>; - <% end -%> +<% if @limit_req_zone.is_a?(Array) -%> +<%- @limit_req_zone.each do |limit_req_zone_item| -%> + limit_req_zone <%= limit_req_zone_item %>; +<% end -%> +<% else -%> + limit_req_zone <%= @limit_req_zone %>; +<% end -%> <% end -%> <% if @sendfile == 'on' -%> From a53dd49da63740e302c942c10e0fcdce3a9b3b2e Mon Sep 17 00:00:00 2001 From: Dan Farnsworth Date: Mon, 18 Nov 2019 11:51:31 -0700 Subject: [PATCH 5/5] Add some unit tests --- spec/classes/nginx_spec.rb | 12 ++++++++++++ spec/defines/resource_location_spec.rb | 6 ++++++ 2 files changed, 18 insertions(+) diff --git a/spec/classes/nginx_spec.rb b/spec/classes/nginx_spec.rb index 2d5ece280..fdc4ff68a 100644 --- a/spec/classes/nginx_spec.rb +++ b/spec/classes/nginx_spec.rb @@ -488,6 +488,18 @@ value: 'warn', match: ' error_log /var/log/nginx/error.log warn;' }, + { + title: 'should set limit_req_zone', + attr: 'limit_req_zone', + value: [ + '$binary_remote_addr zone=myzone1:10m rate=5r/s', + '$binary_remote_addr zone=myzone2:10m rate=5r/s' + ], + match: [ + ' limit_req_zone $binary_remote_addr zone=myzone1:10m rate=5r/s;', + ' limit_req_zone $binary_remote_addr zone=myzone2:10m rate=5r/s;' + ] + }, { title: 'should set pid', attr: 'pid', diff --git a/spec/defines/resource_location_spec.rb b/spec/defines/resource_location_spec.rb index 0c0baf3b4..45b849434 100644 --- a/spec/defines/resource_location_spec.rb +++ b/spec/defines/resource_location_spec.rb @@ -100,6 +100,12 @@ value: 'any', match: ' satisfy any;' }, + { + title: 'should set limit_zone', + attr: 'limit_zone', + value: 'myzone1', + match: ' limit_req zone=myzone1;' + }, { title: 'should set expires', attr: 'expires',