Skip to content

Commit

Permalink
Reorganize templates for clearer understanding (#970)
Browse files Browse the repository at this point in the history
Without this change, the structure of the templates for locations are
bit rigid and hard to understand.  Each component of a location uses an
isolated template which means that much of the common logic is hard
coded to a particular location style, the deployment of which is chosen
based on a somewhat arbitrary idea of what it means to be a location,
and in some cases, the module gets it wrong.  In cases where there is
seeming correctness, modifications to a particular selection of logic
are duplicated among the nested templates.

This work is the results of what was necessary for me to understand what
the templates were doing, and to deploy a fastcgi PHP application.  As
such, the templates have been centralized and conditions about their
functionality have been moved into the template to determine if
rendering needs to be taken.  This allows a more complete, and leaves
potential for more complex examples easier to understand and reason
about.

Also here is the addition of a new param fastcgi_index on the location
class.

This work should result in on-disk configurations that are functionally
equivalent to what was in place before, except in places where the
module was masking conflicting options that were not being rendered in
the templates.
  • Loading branch information
zachfi authored and jyaworski committed Nov 14, 2016
1 parent 730a784 commit 50cced9
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 46 deletions.
35 changes: 6 additions & 29 deletions manifests/resource/location.pp
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@
$proxy_hide_header = $::nginx::proxy_hide_header,
$proxy_pass_header = $::nginx::proxy_pass_header,
$fastcgi = undef,
$fastcgi_index = undef,
$fastcgi_param = undef,
$fastcgi_params = "${::nginx::conf_dir}/fastcgi_params",
$fastcgi_script = undef,
Expand Down Expand Up @@ -259,6 +260,9 @@
if ($fastcgi_split_path != undef) {
validate_string($fastcgi_split_path)
}
if ($fastcgi_index != undef) {
validate_string($fastcgi_index)
}
if ($uwsgi != undef) {
validate_string($uwsgi)
}
Expand Down Expand Up @@ -400,25 +404,6 @@
$location_sanitized_tmp = regsubst($location, '\/', '_', 'G')
$location_sanitized = regsubst($location_sanitized_tmp, '\\\\', '_', 'G')

# Use proxy or fastcgi template if $proxy is defined, otherwise use directory template.
if ($proxy != undef) {
$content_real = template('nginx/vhost/locations/proxy.erb')
} elsif ($location_alias != undef) {
$content_real = template('nginx/vhost/locations/alias.erb')
} elsif ($stub_status != undef) {
$content_real = template('nginx/vhost/locations/stub_status.erb')
} elsif ($fastcgi != undef) {
$content_real = template('nginx/vhost/locations/fastcgi.erb')
} elsif ($uwsgi != undef) {
$content_real = template('nginx/vhost/locations/uwsgi.erb')
} elsif ($www_root != undef) {
$content_real = template('nginx/vhost/locations/directory.erb')
} elsif ($try_files != undef) {
$content_real = template('nginx/vhost/locations/try_files.erb')
} else {
$content_real = template('nginx/vhost/locations/empty.erb')
}

if $ensure == present and $fastcgi != undef and !defined(File[$fastcgi_params]) {
file { $fastcgi_params:
ensure => present,
Expand All @@ -441,11 +426,7 @@
if ($ssl_only != true) {
concat::fragment { "${vhost_sanitized}-${priority}-${location_md5}":
target => $config_file,
content => join([
template('nginx/vhost/location_header.erb'),
$content_real,
template('nginx/vhost/location_footer.erb'),
], ''),
content => template('nginx/vhost/location.erb'),
order => $priority,
}
}
Expand All @@ -456,11 +437,7 @@

concat::fragment { "${vhost_sanitized}-${ssl_priority}-${location_md5}-ssl":
target => $config_file,
content => join([
template('nginx/vhost/location_header.erb'),
$content_real,
template('nginx/vhost/location_footer.erb'),
], ''),
content => template('nginx/vhost/location.erb'),
order => $ssl_priority,
}
}
Expand Down
13 changes: 13 additions & 0 deletions templates/vhost/location.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<%= scope.function_template(['nginx/vhost/location_header.erb']) -%>
<%= scope.function_template(['nginx/vhost/locations/alias.erb']) -%>
<%= scope.function_template(['nginx/vhost/locations/stub_status.erb']) -%>
<% if @fastcgi or @uwsgi or @proxy -%>
<%= scope.function_template(['nginx/vhost/locations/proxy.erb']) -%>
<%= scope.function_template(['nginx/vhost/locations/uwsgi.erb']) -%>
<%= scope.function_template(['nginx/vhost/locations/fastcgi.erb']) -%>
<% else -%>
<%= scope.function_template(['nginx/vhost/locations/directory.erb']) -%>
<% end -%>
<%= scope.function_template(['nginx/vhost/locations/try_files.erb']) -%>
<%= scope.function_template(['nginx/vhost/locations/empty.erb']) -%>
<%= scope.function_template(['nginx/vhost/location_footer.erb']) -%>
9 changes: 1 addition & 8 deletions templates/vhost/locations/alias.erb
Original file line number Diff line number Diff line change
@@ -1,10 +1,3 @@
<% if @location_alias -%>
alias <%= @location_alias %>;
<% if defined? @autoindex -%>
autoindex <%= @autoindex %>;
<% end -%>
<% if @index_files and @index_files.count > 0 -%>
index <% Array(@index_files).each do |i| %> <%= i %><% end %>;
<% end -%>
<% if @try_files -%>
try_files<% @try_files.each do |try| -%> <%= try %><% end -%>;
<% end -%>
3 changes: 0 additions & 3 deletions templates/vhost/locations/directory.erb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,3 @@
<% if @index_files and @index_files.count > 0 -%>
index <% Array(@index_files).each do |i| %> <%= i %><% end %>;
<% end -%>
<% if @try_files -%>
try_files<% @try_files.each do |try| -%> <%= try %><% end -%>;
<% end -%>
8 changes: 5 additions & 3 deletions templates/vhost/locations/fastcgi.erb
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
<% if @fastcgi -%>
<% if defined? @www_root -%>
root <%= @www_root %>;
<% end -%>
include <%= @fastcgi_params %>;

fastcgi_pass <%= @fastcgi %>;
<% if @fastcgi_index -%>
fastcgi_index <%= @fastcgi_index %>;
<% end -%>
<% if @fastcgi_split_path -%>
fastcgi_split_path_info <%= @fastcgi_split_path %>;
<% end -%>
<% if @try_files -%>
try_files <% @try_files.each do |try| -%> <%= try %><% end -%>;
<% end -%>
<% if defined? @fastcgi_script -%>
<%-# this setting can be overridden by setting it in the fastcgi_param hash too %>
<%- @fastcgi_param = { 'SCRIPT_FILENAME' => @fastcgi_script }.merge(@fastcgi_param || {}) -%>
Expand All @@ -20,3 +21,4 @@
fastcgi_param <%= sprintf("%-*s", field_width, key) %> <%= val %>;
<%- end -%>
<% end -%>
<% end -%>
2 changes: 2 additions & 0 deletions templates/vhost/locations/proxy.erb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
<% if @proxy -%>
proxy_pass <%= @proxy %>;
proxy_read_timeout <%= @proxy_read_timeout %>;
proxy_connect_timeout <%= @proxy_connect_timeout %>;
Expand Down Expand Up @@ -45,3 +46,4 @@
<% if @proxy_cache_key -%>
proxy_cache_key <%= @proxy_cache_key %>;
<% end -%>
<% end -%>
2 changes: 2 additions & 0 deletions templates/vhost/locations/stub_status.erb
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
<% if @stub_status -%>
stub_status on;
<% end -%>
5 changes: 2 additions & 3 deletions templates/vhost/locations/uwsgi.erb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
<% if @uwsgi -%>
<% if defined? @www_root -%>
root <%= @www_root %>;
<% end -%>
<% if @try_files -%>
try_files<% @try_files.each do |try| -%> <%= try %><% end -%>;
<% end -%>
include <%= @uwsgi_params %>;
uwsgi_pass <%= @uwsgi %>;
Expand All @@ -15,3 +13,4 @@
<% if @uwsgi_read_timeout-%>
uwsgi_read_timeout <%= @uwsgi_read_timeout %>;
<% end -%>
<% end -%>

0 comments on commit 50cced9

Please sign in to comment.