Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add all gzip params for http context #367

Closed
wants to merge 7 commits into from
Closed

Add all gzip params for http context #367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7678b4c
Add all gzip params for main http context with blank defaults which t…
Jun 27, 2014
430e5b2
Amend nginx.conf template to allow for string or array data type for …
Jun 27, 2014
6f1d056
Add tests for new gzip params
Jun 27, 2014
8110050
Merge remote-tracking branch 'upstream/master'
Jun 27, 2014
bc2e9c6
Update empty gzip param defaults to use 'undef' data type instead of …
Jun 30, 2014
8e84a98
Update nginx.conf template to check existence of gzip params rather t…
Jun 30, 2014
10ff647
Fix formatting
Jul 11, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions manifests/config.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@
$fastcgi_cache_path = $nginx::params::nx_fastcgi_cache_path,
$fastcgi_cache_use_stale = $nginx::params::nx_fastcgi_cache_use_stale,
$gzip = $nginx::params::nx_gzip,
$gzip_buffers = $nginx::params::nx_gzip_buffers,
$gzip_comp_level = $nginx::params::nx_gzip_comp_level,
$gzip_disable = $nginx::params::nx_gzip_disable,
$gzip_min_length = $nginx::params::nx_gzip_min_length,
$gzip_http_version = $nginx::params::nx_gzip_http_version,
$gzip_proxied = $nginx::params::nx_gzip_proxied,
$gzip_types = $nginx::params::nx_gzip_types,
$gzip_vary = $nginx::params::nx_gzip_vary,
$http_access_log = $nginx::params::nx_http_access_log,
$http_cfg_append = $nginx::params::nx_http_cfg_append,
$http_tcp_nodelay = $nginx::params::nx_http_tcp_nodelay,
Expand Down
16 changes: 16 additions & 0 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@
$fastcgi_cache_path = $nginx::params::nx_fastcgi_cache_path,
$fastcgi_cache_use_stale = $nginx::params::nx_fastcgi_cache_use_stale,
$gzip = $nginx::params::nx_gzip,
$gzip_buffers = $nginx::params::nx_gzip_buffers,
$gzip_comp_level = $nginx::params::nx_gzip_comp_level,
$gzip_disable = $nginx::params::nx_gzip_disable,
$gzip_min_length = $nginx::params::nx_gzip_min_length,
$gzip_http_version = $nginx::params::nx_gzip_http_version,
$gzip_proxied = $nginx::params::nx_gzip_proxied,
$gzip_types = $nginx::params::nx_gzip_types,
$gzip_vary = $nginx::params::nx_gzip_vary,
$http_access_log = $nginx::params::nx_http_access_log,
$http_cfg_append = $nginx::params::nx_http_cfg_append,
$http_tcp_nodelay = $nginx::params::nx_http_tcp_nodelay,
Expand Down Expand Up @@ -202,6 +210,14 @@
fastcgi_cache_path => $fastcgi_cache_path,
fastcgi_cache_use_stale => $fastcgi_cache_use_stale,
gzip => $gzip,
gzip_buffers => $gzip_buffers,
gzip_comp_level => $gzip_comp_level,
gzip_disable => $gzip_disable,
gzip_min_length => $gzip_min_length,
gzip_http_version => $gzip_http_version,
gzip_proxied => $gzip_proxied,
gzip_types => $gzip_types,
gzip_vary => $gzip_vary,
http_access_log => $http_access_log,
http_cfg_append => $http_cfg_append,
http_tcp_nodelay => $http_tcp_nodelay,
Expand Down
8 changes: 8 additions & 0 deletions manifests/params.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@
$nx_http_tcp_nodelay = on
$nx_http_tcp_nopush = off
$nx_gzip = on
$nx_gzip_buffers = undef
$nx_gzip_comp_level = undef
$nx_gzip_disable = 'MSIE [1-6]\.(?!.*SV1)'
$nx_gzip_min_length = undef
$nx_gzip_http_version = undef
$nx_gzip_proxied = undef
$nx_gzip_types = undef
$nx_gzip_vary = undef
$nx_server_tokens = on
$nx_spdy = off
$nx_ssl_stapling = off
Expand Down
54 changes: 54 additions & 0 deletions spec/classes/config_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,60 @@
:value => '123',
:match => ' keepalive_timeout 123;',
},
{
:title => 'should set gzip',
:attr => 'gzip',
:value => 'on',
:match => ' gzip on;',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get a negative test as well? use :value => 'off' and :notmatch => /gzip/

This should be applied to the rest of the parameters too.

},
{
:title => 'should set gzip_buffers',
:attr => 'gzip_buffers',
:value => '32 4k',
:match => ' gzip_buffers 32 4k;',
},
{
:title => 'should set gzip_comp_level',
:attr => 'gzip_comp_level',
:value => '1',
:match => ' gzip_comp_level 1;',
},
{
:title => 'should set gzip_disable',
:attr => 'gzip_disable',
:value => 'MSIE [1-6]\.(?!.*SV1)',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should test with a value different from the default to make sure it's being passed through correctly.

:match => ' gzip_disable MSIE [1-6]\.(?!.*SV1);',
},
{
:title => 'should set gzip_min_length',
:attr => 'gzip_min_length',
:value => '20',
:match => ' gzip_min_length 20;',
},
{
:title => 'should set gzip_http_version',
:attr => 'gzip_http_version',
:value => '1.1',
:match => ' gzip_http_version 1.1;',
},
{
:title => 'should set gzip_proxied',
:attr => 'gzip_proxied',
:value => 'any',
:match => ' gzip_proxied any;',
},
{
:title => 'should set gzip_types',
:attr => 'gzip_types',
:value => ['text/plain','text/html'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get another test where :value is a string?

:match => ' gzip_types text/plain text/html;',
},
{
:title => 'should set gzip_vary',
:attr => 'gzip_vary',
:value => 'on',
:match => ' gzip_vary on;',
}
].each do |param|
context "when #{param[:attr]} is #{param[:value]}" do
let :params do { param[:attr].to_sym => param[:value] } end
Expand Down
25 changes: 23 additions & 2 deletions templates/conf.d/nginx.conf.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,29 @@ http {
tcp_nodelay <%= @http_tcp_nodelay %>;

<% if @gzip == 'on' -%>
gzip on;
gzip_disable "MSIE [1-6]\.(?!.*SV1)";
gzip on;
<% if @gzip_buffers -%>
gzip_buffers <%= @gzip_buffers %>;
<% end -%>
<% if @gzip_comp_level -%>
gzip_comp_level <%= @gzip_comp_level %>;
<% end -%>
gzip_disable <%= @gzip_disable %>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be conditional like the rest so that setting to undef will remove it from the config?

<% if @gzip_min_length -%>
gzip_min_length <%= @gzip_min_length %>;
<% end -%>
<% if @gzip_http_version -%>
gzip_http_version <%= @gzip_http_version %>;
<% end -%>
<% if @gzip_proxied -%>
gzip_proxied <%= @gzip_proxied %>;
<% end -%>
<% if @gzip_types -%>
gzip_types <%= @gzip_types.kind_of?(Array) ? @gzip_types.join(' ') : @gzip_types %>;
<% end -%>
<% if @gzip_vary -%>
gzip_vary <%= @gzip_vary %>;
<% end -%>
<% end -%>

<% if @proxy_cache_path -%>
Expand Down
2 changes: 1 addition & 1 deletion templates/vhost/vhost_header.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ server {
}
<% end -%>
<% if @index_files.count > 0 -%>
index <% Array(@index_files).each do |i| %> <%= i %><% end %>;
index <% Array(@index_files).each do |i| %> <%= i %><% end %>;
<% end -%>
<% if defined? @log_by_lua -%>
log_by_lua '<%= @log_by_lua %>';
Expand Down