From cf49d4c945ddb00861a90a4d5fa55719409275fa Mon Sep 17 00:00:00 2001
From: Sascha Doering <s.doering@syseleven.de>
Date: Mon, 16 Jul 2018 15:58:49 +0200
Subject: [PATCH] Add parameters to upstream and upstreammembers

Add parameters to nginx::resource::upstream and
nginx::resource::upstream::member which allows more configuration on
upstreams as before. The only thing that we broke is that the
members of an upstream must now be passed as a hash rather than an
array. This also makes the sorting of keepalive to the end no
longer necessary because there is now a parameter for it. And
values for a nginx::resource::upstream::member can now be set as
default for all members of an upstream or individually for each
member inside the members hash. Of course, the explicit
specification overrides the defaults. In general the changes have
made more parameters available to nginx::resource::upstream and
nginx::resource::upstream::member. In addition, one of the two
templates for nginx::resource::upstream::member was disposed since
it is no longer needed.

Fixes GH-1222
---
 README.md                               |  40 +-
 manifests/resource/upstream.pp          | 149 ++++---
 manifests/resource/upstream/member.pp   |  70 +++-
 spec/classes/nginx_spec.rb              |   2 +-
 spec/defines/resource_upstream_spec.rb  | 529 +++++++++++++++++++-----
 templates/upstream/upstream_footer.erb  |  53 ++-
 templates/upstream/upstream_header.erb  |   9 +-
 templates/upstream/upstream_member.erb  |  13 +-
 templates/upstream/upstream_members.erb |   2 -
 types/size.pp                           |   1 +
 types/time.pp                           |   1 +
 types/upstreamleasttime.pp              |   1 +
 types/upstreamleasttimehttp.pp          |   1 +
 types/upstreamleasttimestream.pp        |   1 +
 types/upstreammemberdefaults.pp         |  15 +
 types/upstreammemberserver.pp           |   1 +
 types/upstreamsticky.pp                 |  28 ++
 types/upstreamzone.pp                   |   1 +
 18 files changed, 719 insertions(+), 198 deletions(-)
 delete mode 100644 templates/upstream/upstream_members.erb
 create mode 100644 types/size.pp
 create mode 100644 types/time.pp
 create mode 100644 types/upstreamleasttime.pp
 create mode 100644 types/upstreamleasttimehttp.pp
 create mode 100644 types/upstreamleasttimestream.pp
 create mode 100644 types/upstreammemberdefaults.pp
 create mode 100644 types/upstreammemberserver.pp
 create mode 100644 types/upstreamsticky.pp
 create mode 100644 types/upstreamzone.pp

diff --git a/README.md b/README.md
index 5daf844d7..1ec3ab3c5 100644
--- a/README.md
+++ b/README.md
@@ -53,11 +53,17 @@ nginx::resource::server { 'www.puppetlabs.com':
 
 ```puppet
 nginx::resource::upstream { 'puppet_rack_app':
-  members => [
-    'localhost:3000',
-    'localhost:3001',
-    'localhost:3002',
-  ],
+  members => {
+    'localhost:3000':
+      server => 'localhost'
+      port   => 3000
+    'localhost:3001':
+      server => 'localhost'
+      port   => 3001
+    'localhost:3002':
+      server => 'localhost'
+      port   => 3002
+  },
 }
 
 nginx::resource::server { 'rack.puppetlabs.com':
@@ -137,9 +143,15 @@ nginx::nginx_upstreams:
   'puppet_rack_app':
     ensure: present
     members:
-      - localhost:3000
-      - localhost:3001
-      - localhost:3002
+      'localhost:3000':
+        server: 'localhost'
+        port: 3000
+      'localhost:3001':
+        server: 'localhost'
+        port: 3001
+      'localhost:3002':
+        server: 'localhost'
+        port: 3002
 nginx::nginx_servers:
   'www.puppetlabs.com':
     www_root: '/var/www/www.puppetlabs.com'
@@ -185,9 +197,15 @@ nginx::nginx_upstreams:
   'syslog':
     upstream_context: 'stream'
     members:
-      - '10.0.0.1:514'
-      - '10.0.0.2:514'
-      - '10.0.0.3:514'
+      '10.0.0.1:514'
+        server: '10.0.0.1'
+        port: '514'
+      '10.0.0.2:514'
+        server: '10.0.0.2'
+        port: '514'
+      '10.0.0.3:514'
+        server: '10.0.0.3'
+        port: '514'
 ```
 
 ## Nginx with precompiled Passenger
diff --git a/manifests/resource/upstream.pp b/manifests/resource/upstream.pp
index 91dac6ccd..9da3a21a6 100644
--- a/manifests/resource/upstream.pp
+++ b/manifests/resource/upstream.pp
@@ -3,13 +3,26 @@
 # This definition creates a new upstream proxy entry for NGINX
 #
 # Parameters:
-#   [*members*]               - Array of member URIs for NGINX to connect to. Must follow valid NGINX syntax.
-#                               If omitted, individual members should be defined with nginx::resource::upstream::member
 #   [*ensure*]                - Enables or disables the specified location (present|absent)
-#   [*upstream_cfg_append*]   - Hash of custom directives to put after other directives in upstream
-#   [*upstream_cfg_prepend*]  - It expects a hash with custom directives to put before anything else inside upstream
-#   [*upstream_fail_timeout*] - Set the fail_timeout for the upstream. Default is 10 seconds - As that is what Nginx does normally.
-#   [*upstream_max_fails*]    - Set the max_fails for the upstream. Default is to use nginx default value which is 1.
+#   [*context*]               - Set the type of this upstream (http|stream).
+#   [*members*]               - Hash of member URIs for NGINX to connect to. Must follow valid NGINX syntax.
+#                               If omitted, individual members should be defined with nginx::resource::upstream::member
+#   [*members_tag*]           - Restrict collecting the exported members for this upstream with a tag.
+#   [*member_defaults*]       - Specify default settings added to each member of this upstream.
+#   [*hash*]                  - Activate the hash load balancing method (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#hash).
+#   [*ip_hash*]               - Activate ip_hash for this upstream (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#ip_hash).
+#   [*keepalive*]             - Set the maximum number of idle keepalive connections (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive).
+#   [*least_conn*]            - Activate the least_conn load balancing method (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#least_conn).
+#   [*least_time*]            - Activate the least_time load balancing method (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#least_time).
+#   [*ntlm*]                  - Allow NTLM authentication (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#ntlm).
+#   [*queue_max*]             - Set the maximum number of queued requests (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#queue).
+#   [*queue_timeout*]         - Set the timeout for the queue (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#queue).
+#   [*random*]                - Activate the random load balancing method (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#random).
+#   [*statefile*]             - Specifies a file that keeps the state of the dynamically configurable group (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#state).
+#   [*sticky*]                - Enables session affinity (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#sticky).
+#   [*zone*]                  - Defines the name and optional the size of the shared memory zone (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#zone).
+#   [*cfg_append*]            - Hash of custom directives to put after other directives in upstream
+#   [*cfg_prepend*]           - It expects a hash with custom directives to put before anything else inside upstream
 #
 # Actions:
 #
@@ -18,82 +31,116 @@
 # Sample Usage:
 #  nginx::resource::upstream { 'proxypass':
 #    ensure  => present,
-#    members => [
-#      'localhost:3000',
-#      'localhost:3001',
-#      'localhost:3002',
-#    ],
+#    members => {
+#      'localhost:3001' => {
+#        server => 'localhost',
+#        port   => 3000,
+#      },
+#      'localhost:3002' => {
+#        server => 'localhost',
+#        port   => 3002,
+#      },
+#      'localhost:3003' => {
+#        server => 'localhost',
+#        port   => 3003,
+#      },
+#    },
 #  }
 #
 #  Custom config example to use ip_hash, and 20 keepalive connections
 #  create a hash with any extra custom config you want.
-#  $my_config = {
-#    'ip_hash'   => '',
-#    'keepalive' => '20',
-#  }
 #  nginx::resource::upstream { 'proxypass':
-#    ensure              => present,
-#    members => [
-#      'localhost:3000',
-#      'localhost:3001',
-#      'localhost:3002',
-#    ],
-#    upstream_cfg_prepend => $my_config,
+#    ensure    => present,
+#    members   => {
+#      'localhost:3001' => {
+#        server => 'localhost',
+#        port   => 3000,
+#      },
+#      'localhost:3002' => {
+#        server => 'localhost',
+#        port   => 3002,
+#      },
+#      'localhost:3003' => {
+#        server => 'localhost',
+#        port   => 3003,
+#      },
+#    },
+#    ip_hash   => true,
+#    keepalive => 20,
 #  }
+#
 define nginx::resource::upstream (
-  Optional[Array] $members                  = undef,
-  $members_tag                              = undef,
-  Enum['present', 'absent'] $ensure         = 'present',
-  Optional[Hash] $upstream_cfg_append       = undef,
-  Optional[Hash] $upstream_cfg_prepend      = undef,
-  $upstream_fail_timeout                    = '10s',
-  $upstream_max_fails                       = undef,
-  Enum['http', 'stream'] $upstream_context  = 'http',
+  Enum['present', 'absent']                 $ensure                 = 'present',
+  Enum['http', 'stream']                    $context                = 'http',
+  Optional[Hash]                            $members                = undef,
+  Optional[String]                          $members_tag            = undef,
+  Optional[Nginx::UpstreamMemberDefaults]   $member_defaults        = undef,
+  Optional[String]                          $hash                   = undef,
+  Optional[Boolean]                         $ip_hash                = undef,
+  Optional[Integer[1]]                      $keepalive              = undef,
+  Optional[Boolean]                         $least_conn             = undef,
+  Optional[Nginx::UpstreamLeastTime]        $least_time             = undef,
+  Optional[Boolean]                         $ntlm                   = undef,
+  Optional[Integer]                         $queue_max              = undef,
+  Optional[Nginx::Time]                     $queue_timeout          = undef,
+  Optional[String]                          $random                 = undef,
+  Optional[Stdlib::Unixpath]                $statefile              = undef,
+  Optional[Nginx::UpstreamSticky]           $sticky                 = undef,
+  Optional[Nginx::UpstreamZone]             $zone                   = undef,
+  Optional[Hash]                            $cfg_append             = undef,
+  Optional[Hash]                            $cfg_prepend            = undef,
 ) {
 
   if ! defined(Class['nginx']) {
     fail('You must include the nginx base class before using any defined resources')
   }
 
-  $root_group = $::nginx::root_group
-
-  $ensure_real = $ensure ? {
-    'absent' => absent,
-    default  => present,
+  if $least_time {
+    if $context == 'http' and ! ($least_time =~ Nginx::UpstreamLeastTimeHttp) {
+      fail('The parameter "least_time" does not match the datatype "Nginx::UpstreamLeastTimeHttp"')
+    }
+    if $context == 'stream' and ! ($least_time =~ Nginx::UpstreamLeastTimeStream) {
+      fail('The parameter "least_time" does not match the datatype "Nginx::UpstreamLeastTimeStream"')
+    }
   }
 
-  $conf_dir_real = $upstream_context ? {
-    'stream' => 'conf.stream.d',
-    default  => 'conf.d',
+  $conf_dir = $context ? {
+    'stream' => "${nginx::config::conf_dir}/conf.stream.d",
+    default  => "${nginx::config::conf_dir}/conf.d",
   }
 
-  $conf_dir = "${nginx::config::conf_dir}/${conf_dir_real}"
-
   Concat {
     owner => 'root',
-    group => $root_group,
+    group => $::nginx::root_group,
     mode  => '0644',
   }
 
-  concat { "${nginx::conf_dir}/${conf_dir_real}/${name}-upstream.conf":
-    ensure  => $ensure_real,
+  concat { "${conf_dir}/${name}-upstream.conf":
+    ensure  => $ensure,
     notify  => Class['::nginx::service'],
     require => File[$conf_dir],
   }
 
-  # Uses: $name, $upstream_cfg_prepend
   concat::fragment { "${name}_upstream_header":
-    target  => "${nginx::conf_dir}/${conf_dir_real}/${name}-upstream.conf",
+    target  => "${conf_dir}/${name}-upstream.conf",
     order   => '10',
     content => template('nginx/upstream/upstream_header.erb'),
   }
 
   if $members != undef {
-    # Uses: $members, $upstream_fail_timeout
-    concat::fragment { "${name}_upstream_members":
-      target  => "${nginx::conf_dir}/${conf_dir_real}/${name}-upstream.conf",
-      order   => '50',
-      content => template('nginx/upstream/upstream_members.erb'),
+    $members.each |$member,$values| {
+      $member_values = merge($member_defaults,$values,{'upstream' => $name,'context' => $context})
+
+      if $context == 'stream' and $member_values['route'] {
+        fail('The parameter "route" is not available for upstreams with context "stream"')
+      }
+      if $context == 'stream' and $member_values['state'] and $member_values['state'] == 'drain' {
+        fail('The state "drain" is not available for upstreams with context "stream"')
+      }
+
+      nginx::resource::upstream::member { $member:
+        * => $member_values,
+      }
     }
   } else {
     # Collect exported members:
@@ -105,7 +152,7 @@
   }
 
   concat::fragment { "${name}_upstream_footer":
-    target  => "${nginx::conf_dir}/${conf_dir_real}/${name}-upstream.conf",
+    target  => "${conf_dir}/${name}-upstream.conf",
     order   => '90',
     content => template('nginx/upstream/upstream_footer.erb'),
   }
diff --git a/manifests/resource/upstream/member.pp b/manifests/resource/upstream/member.pp
index 7ce6ea5a9..0c2fd2f36 100644
--- a/manifests/resource/upstream/member.pp
+++ b/manifests/resource/upstream/member.pp
@@ -9,45 +9,79 @@
 #
 #
 # Parameters:
-#   [*ensure*]                  - Enables or disables the specified member (present|absent)
-#   [*upstream*]                - The name of the upstream resource
-#   [*server*]                  - Hostname or IP of the upstream member server
-#   [*port*]                    - Port of the listening service on the upstream member
-#   [*upstream_fail_timeout*]   - Set the fail_timeout for the upstream. Default is 10 seconds
-#
+#   [*upstream*]              - The name of the upstream resource
+#   [*ensure*]                - Enables or disables the specified member (present|absent)
+#   [*context*]               - Set the type of this upstream (http|stream).
+#   [*server*]                - Hostname or IP of the upstream member server
+#   [*port*]                  - Port of the listening service on the upstream member
+#   [*weight*]                - Set the weight for this upstream member
+#   [*max_conns*]             - Set the max_conns for this upstream member
+#   [*max_fails*]             - Set the max_fails for this upstream member
+#   [*fail_timeout*]          - Set the fail_timeout for this upstream member
+#   [*backup*]                - Activate backup for this upstream member
+#   [*resolve*]               - Activate resolve for this upstream member
+#   [*route*]                 - Set the route for this upstream member
+#   [*service*]               - Set the service for this upstream member
+#   [*slow_start*]            - Set the slow_start for this upstream member
+#   [*state*]                 - Set the state for this upstream member
+#   [*params_prepend*]        - prepend a parameter for this upstream member
+#   [*params_append*]         - append a paremeter for this upstream member
+#   [*comment*]               - Add a comment for this upstream member
 #
 # Examples:
 #
 #   Exporting the resource on a upstream member server:
 #
 #   @@nginx::resource::upstream::member { $trusted['certname']:
-#     ensure    => present,
-#     upstream  => 'proxypass',
-#     server    => $facts['networking']['ip'],
-#     port      => 3000,
+#     ensure   => present,
+#     upstream => 'proxypass',
+#     server   => $facts['networking']['ip'],
+#     port     => 3000,
 #   }
 #
 #
 #   Collecting the resource on the NGINX server:
 #
 #   nginx::resource::upstream { 'proxypass':
-#     ensure    => present,
+#     ensure => present,
 #   }
 #
 define nginx::resource::upstream::member (
-  $upstream,
-  $server,
-  Enum['present', 'absent'] $ensure = 'present',
-  Integer $port                     = 80,
-  $upstream_fail_timeout  = '10s',
+  String                                  $upstream,
+  Enum['present', 'absent']               $ensure           = 'present',
+  Enum['http', 'stream']                  $context          = 'http',
+  Optional[Nginx::UpstreamMemberServer]   $server           = $name,
+  Optional[Stdlib::Port]                  $port             = 80,
+  Optional[Integer[1]]                    $weight           = undef,
+  Optional[Integer[1]]                    $max_conns        = undef,
+  Optional[Integer[1]]                    $max_fails        = undef,
+  Optional[Nginx::Time]                   $fail_timeout     = undef,
+  Optional[Boolean]                       $backup           = undef,
+  Optional[Boolean]                       $resolve          = undef,
+  Optional[String]                        $route            = undef,
+  Optional[String]                        $service          = undef,
+  Optional[Nginx::Time]                   $slow_start       = undef,
+  Optional[Enum['drain','down']]          $state            = undef,
+  Optional[String]                        $params_prepend   = undef,
+  Optional[String]                        $params_append    = undef,
+  Optional[String]                        $comment          = undef,
 ) {
   if ! defined(Class['nginx']) {
     fail('You must include the nginx base class before using any defined resources')
   }
 
-  # Uses: $server, $port, $upstream_fail_timeout
+  $conf_dir = $context ? {
+    'stream' => "${nginx::config::conf_dir}/conf.stream.d",
+    default  => "${nginx::config::conf_dir}/conf.d",
+  }
+
+  $_server = ($server =~ Pattern[/^unix:\/([^\/\0]+\/*)*$/]) ? {
+    true  => $server,
+    false => "${server}:${port}",
+  }
+
   concat::fragment { "${upstream}_upstream_member_${name}":
-    target  => "${nginx::conf_dir}/conf.d/${upstream}-upstream.conf",
+    target  => "${conf_dir}/${upstream}-upstream.conf",
     order   => 40,
     content => template('nginx/upstream/upstream_member.erb'),
   }
diff --git a/spec/classes/nginx_spec.rb b/spec/classes/nginx_spec.rb
index 5a8e2d7f6..3610fe0f4 100644
--- a/spec/classes/nginx_spec.rb
+++ b/spec/classes/nginx_spec.rb
@@ -9,7 +9,7 @@
 
       let :params do
         {
-          nginx_upstreams: { 'upstream1' => { 'members' => ['localhost:3000'] } },
+          nginx_upstreams: { 'upstream1' => { 'members' => { 'localhost' => { 'port' => 3000 } } } },
           nginx_servers: { 'test2.local' => { 'www_root' => '/' } },
           nginx_servers_defaults: { 'listen_options' => 'default_server' },
           nginx_locations: { 'test2.local' => { 'server' => 'test2.local', 'www_root' => '/' } },
diff --git a/spec/defines/resource_upstream_spec.rb b/spec/defines/resource_upstream_spec.rb
index 314b93029..ba28411ce 100644
--- a/spec/defines/resource_upstream_spec.rb
+++ b/spec/defines/resource_upstream_spec.rb
@@ -12,7 +12,21 @@
 
       let :default_params do
         {
-          members: ['test']
+          http: {
+            context: 'http',
+            members: { 'member-http' => {} }
+          },
+          stream: {
+            context: 'stream',
+            members: { 'member-stream' => {} }
+          }
+        }
+      end
+
+      let :conf_d_pathes do
+        {
+          http: '/etc/nginx/conf.d',
+          stream: '/etc/nginx/conf.stream.d'
         }
       end
 
@@ -23,117 +37,432 @@
       end
 
       describe 'os-independent items' do
-        describe 'basic assumptions' do
-          let(:params) { default_params }
+        ##
+        ## check that http is the default
+        ##
+        describe 'basic assumptions for default upstreams' do
+          let(:params) { default_params[:http] }
 
-          it { is_expected.to contain_concat("/etc/nginx/conf.d/#{title}-upstream.conf").that_requires('File[/etc/nginx/conf.d]') }
-          it { is_expected.to contain_concat__fragment("#{title}_upstream_header").with_content(%r{upstream #{title}}) }
+          it {
+            is_expected.to contain_concat("/etc/nginx/conf.d/#{title}-upstream.conf").
+              that_requires('File[/etc/nginx/conf.d]')
+          }
+          it {
+            is_expected.to contain_concat__fragment("#{title}_upstream_header").
+              with_content(%r{upstream #{title}}).
+              with(
+                'target' => "/etc/nginx/conf.d/#{title}-upstream.conf",
+                'order'  => 10
+              )
+          }
+          it {
+            is_expected.to contain_concat__fragment("#{title}_upstream_member_#{params[:members].keys[0]}").
+              with(
+                'target' => "/etc/nginx/conf.d/#{title}-upstream.conf",
+                'order'  => 40
+              )
+          }
+          it {
+            is_expected.to contain_concat__fragment("#{title}_upstream_footer").
+              with(
+                'target' => "/etc/nginx/conf.d/#{title}-upstream.conf",
+                'order'  => 90
+              ).
+              with_content("}\n")
+          }
+        end
 
-          it do
-            is_expected.to contain_concat__fragment("#{title}_upstream_header").with(
-              'target' => "/etc/nginx/conf.d/#{title}-upstream.conf",
-              'order'  => 10
-            )
-          end
+        ##
+        ## check http and stream upstreams
+        ##
+        %w[http stream].each do |upstreamcontext|
+          describe "basic assumptions for #{upstreamcontext} upstreams" do
+            let(:params) { default_params[upstreamcontext.to_sym] }
+            let(:conf_d_path) { conf_d_pathes[upstreamcontext.to_sym] }
 
-          it do
-            is_expected.to contain_concat__fragment("#{title}_upstream_members").with(
-              'target' => "/etc/nginx/conf.d/#{title}-upstream.conf",
-              'order'  => 50
-            )
+            it {
+              is_expected.to compile.with_all_deps
+            }
+            it {
+              is_expected.to contain_concat("#{conf_d_path}/#{title}-upstream.conf").
+                that_requires("File[#{conf_d_path}]")
+            }
+            it {
+              is_expected.to contain_concat__fragment("#{title}_upstream_header").
+                with_content(%r{upstream #{title}}).
+                with(
+                  'target' => "#{conf_d_path}/#{title}-upstream.conf",
+                  'order'  => 10
+                )
+            }
+            it {
+              is_expected.to contain_concat__fragment("#{title}_upstream_member_#{params[:members].keys[0]}").
+                with(
+                  'target' => "#{conf_d_path}/#{title}-upstream.conf",
+                  'order'  => 40
+                )
+            }
+            it {
+              is_expected.to contain_concat__fragment("#{title}_upstream_footer").
+                with(
+                  'target' => "#{conf_d_path}/#{title}-upstream.conf",
+                  'order'  => 90
+                ).
+                with_content("}\n")
+            }
           end
 
-          it do
-            is_expected.to contain_concat__fragment("#{title}_upstream_footer").with(
-              'target' => "/etc/nginx/conf.d/#{title}-upstream.conf",
-              'order'  => 90
-            ).with_content("}\n")
-          end
-        end
+          ##
+          ## check the upstream template
+          ##
+          describe 'upstream.conf template content' do
+            ##
+            ## check the default
+            ##
+            context "when only a server is specified in a #{upstreamcontext} upstream" do
+              let(:params) { default_params[upstreamcontext.to_sym] }
+              let(:conf_d_path) { conf_d_pathes[upstreamcontext.to_sym] }
 
-        describe 'upstream.conf template content' do
-          [
-            {
-              title: 'should contain ordered prepended directives',
-              attr: 'upstream_cfg_prepend',
-              fragment: 'header',
-              value: {
-                'test3'     => 'test value 3',
-                'test6'     => { 'subkey1' => %w[subvalue1 subvalue2] },
-                'keepalive' => 'keepalive 1',
-                'test2'     => 'test value 2',
-                'test5'     => { 'subkey1' => 'subvalue1' },
-                'test4'     => ['test value 1', 'test value 2']
-              },
-              match: [
-                '  test2 test value 2;',
-                '  test3 test value 3;',
-                '  test4 test value 1;',
-                '  test4 test value 2;',
-                '  test5 subkey1 subvalue1;',
-                '  test6 subkey1 subvalue1;',
-                '  test6 subkey1 subvalue2;',
-                '  keepalive keepalive 1;'
-              ]
-            },
-            {
-              title: 'should set server',
-              attr: 'members',
-              fragment: 'members',
-              value: %w[test3 test1 test2],
-              match: [
-                '  server     test3  fail_timeout=10s;',
-                '  server     test1  fail_timeout=10s;',
-                '  server     test2  fail_timeout=10s;'
-              ]
-            },
-            {
-              title: 'should contain ordered appended directives',
-              attr: 'upstream_cfg_append',
-              fragment: 'footer',
-              value: {
-                'test3' => 'test value 3',
-                'test6' => { 'subkey1' => %w[subvalue1 subvalue2] },
-                'keepalive' => 'keepalive 1',
-                'test2' => 'test value 2',
-                'test5' => { 'subkey1' => 'subvalue1' },
-                'test4' => ['test value 1', 'test value 2']
-              },
-              match: [
-                '  test2 test value 2;',
-                '  test3 test value 3;',
-                '  test4 test value 1;',
-                '  test4 test value 2;',
-                '  test5 subkey1 subvalue1;',
-                '  test6 subkey1 subvalue1;',
-                '  test6 subkey1 subvalue2;',
-                '  keepalive keepalive 1;'
-              ]
-            }
-          ].each do |param|
-            context "when #{param[:attr]} is #{param[:value]}" do
-              let(:params) { default_params.merge(param[:attr].to_sym => param[:value]) }
-
-              it { is_expected.to contain_concat("/etc/nginx/conf.d/#{title}-upstream.conf").with_mode('0644') }
-              it { is_expected.to contain_concat__fragment("#{title}_upstream_#{param[:fragment]}") }
-              it param[:title] do
-                lines = catalogue.resource('concat::fragment', "#{title}_upstream_#{param[:fragment]}").send(:parameters)[:content].split("\n")
-                expect(lines & Array(param[:match])).to eq(Array(param[:match]))
-                Array(param[:notmatch]).each do |item|
-                  is_expected.to contain_concat__fragment("#{title}_upstream_#{param[:fragment]}").without_content(item)
+              it {
+                is_expected.to compile.with_all_deps
+              }
+              it {
+                is_expected.to contain_concat("#{conf_d_path}/#{title}-upstream.conf").
+                  with_mode('0644')
+              }
+              it {
+                is_expected.to contain_concat__fragment("#{title}_upstream_header").
+                  with_content("# MANAGED BY PUPPET\nupstream #{title} {\n")
+              }
+              it {
+                is_expected.to contain_concat__fragment("#{title}_upstream_member_#{params[:members].keys[0]}").
+                  with_content("  server #{params[:members].keys[0]}:80;\n")
+              }
+              it {
+                is_expected.to contain_concat__fragment("#{title}_upstream_footer").
+                  with_content("}\n")
+              }
+            end
+
+            ##
+            ## check the upstream parameters
+            ##
+            [
+              {
+                value: { hash: '$remote_addr consistent' },
+                match: 'hash $remote_addr consistent'
+              },
+              {
+                value: { keepalive: 20 },
+                match: 'keepalive 20'
+              },
+              {
+                value: { least_conn: true },
+                match: 'least_conn'
+              },
+              {
+                value: { least_conn: false },
+                match: false
+              },
+              {
+                value: { least_time: 'last_byte inflight' },
+                match: 'least_time last_byte inflight'
+              },
+              {
+                value: { least_time: 'header inflight' },
+                match: 'least_time header inflight',
+                fails: { stream: 'The parameter "least_time" does not match the datatype "Nginx::UpstreamLeastTimeStream"' }
+              },
+              {
+                value: { least_time: 'first_byte inflight' },
+                match: 'least_time first_byte inflight',
+                fails: { http: 'The parameter "least_time" does not match the datatype "Nginx::UpstreamLeastTimeHttp"' }
+              },
+              {
+                value: { ntlm: true },
+                match: 'ntlm'
+              },
+              {
+                value: { ntlm: false },
+                match: false
+              },
+              {
+                value: { queue_max: 20 },
+                match: 'queue 20'
+              },
+              {
+                value: { queue_max: 20, queue_timeout: '20s' },
+                match: 'queue 20 timeout=20s'
+              },
+              {
+                value: { random: 'two least_conn' },
+                match: 'random two least_conn'
+              },
+              {
+                value: { statefile: '/var/lib/nginx/state/servers.conf' },
+                match: 'state /var/lib/nginx/state/servers.conf'
+              },
+              {
+                value: { sticky: { cookie: { name: 'srv_id', expires: '1h', domain: '.example.com', httponly: true, secure: true, path: '/' } } },
+                match: 'sticky cookie name=srv_id expires=1h domain=.example.com httponly secure path=/'
+              },
+              {
+                value: { sticky: { route: '$route_cookie $route_uri' } },
+                match: 'sticky route $route_cookie $route_uri'
+              },
+              {
+                value: { sticky: { learn: { create: '$upstream_cookie_examplecookie', lookup: '$cookie_examplecookie', zone: 'client_sessions:1m' } } },
+                match: 'sticky learn create=$upstream_cookie_examplecookie lookup=$cookie_examplecookie zone=client_sessions:1m'
+              },
+              {
+                value: { zone: 'frontend 1M' },
+                match: 'zone frontend 1M'
+              },
+              {
+                value: { zone: 'backend 64k' },
+                match: 'zone backend 64k'
+              },
+              {
+                value: { zone: 'backend' },
+                match: 'zone backend'
+              }
+            ].each do |upstream_parameter|
+              context "when #{upstream_parameter[:value].keys[0]} ist set to #{upstream_parameter[:value]} in #{upstreamcontext} upstream" do
+                let(:params) { default_params[upstreamcontext.to_sym].merge(upstream_parameter[:value]) }
+                let(:conf_d_path) { conf_d_pathes[upstreamcontext.to_sym] }
+
+                if upstream_parameter.key?(:fails) && upstream_parameter[:fails].key?(upstreamcontext.to_sym)
+                  it {
+                    is_expected.to raise_error(Puppet::Error, %r{#{upstream_parameter[:fails][upstreamcontext.to_sym]}})
+                  }
+                  next
+                end
+
+                it {
+                  is_expected.to compile.with_all_deps
+                }
+                it {
+                  is_expected.to contain_concat("#{conf_d_path}/#{title}-upstream.conf").
+                    with_mode('0644')
+                }
+                it {
+                  is_expected.to contain_concat__fragment("#{title}_upstream_header").
+                    with_content("# MANAGED BY PUPPET\nupstream #{title} {\n")
+                }
+                it {
+                  is_expected.to contain_concat__fragment("#{title}_upstream_member_#{params[:members].keys[0]}").
+                    with_content("  server #{params[:members].keys[0]}:80;\n")
+                }
+
+                if upstream_parameter[:match] != false
+                  it {
+                    is_expected.to contain_concat__fragment("#{title}_upstream_footer").
+                      with_content("  #{upstream_parameter[:match]};\n}\n")
+                  }
+                else
+                  it {
+                    is_expected.to contain_concat__fragment("#{title}_upstream_footer").
+                      with_content("}\n")
+                  }
                 end
               end
             end
-          end
 
-          context 'when ensure => absent' do
-            let :params do
-              default_params.merge(
-                ensure: 'absent'
-              )
+            ##
+            ## check the upstream member parameters
+            ##
+            [
+              {
+                value: { unix: { server: 'unix:/tmp/backend3' } },
+                match: 'unix:/tmp/backend3;'
+              },
+              {
+                value: { member1: {} },
+                match: 'member1:80;'
+              },
+              {
+                value: { member1: { server: '127.0.0.1' } },
+                match: '127.0.0.1:80;'
+              },
+              {
+                value: { member1: { server: '127.0.0.1', port: 8080 } },
+                match: '127.0.0.1:8080;'
+              },
+              {
+                value: { member1: { weight: 20 } },
+                match: 'member1:80 weight=20;'
+              },
+              {
+                value: { member1: { max_conns: 20 } },
+                match: 'member1:80 max_conns=20;'
+              },
+              {
+                value: { member1: { max_fails: 20 } },
+                match: 'member1:80 max_fails=20;'
+              },
+              {
+                value: { member1: { fail_timeout: '20s' } },
+                match: 'member1:80 fail_timeout=20s;'
+              },
+              {
+                value: { member1: { backup: true } },
+                match: 'member1:80 backup;'
+              },
+              {
+                value: { member1: { backup: false } },
+                match: 'member1:80;'
+              },
+              {
+                value: { member1: { resolve: true } },
+                match: 'member1:80 resolve;'
+              },
+              {
+                value: { member1: { resolve: false } },
+                match: 'member1:80;'
+              },
+              {
+                value: { member1: { route: 'a' } },
+                match: 'member1:80 route=a;',
+                fails: { stream: 'The parameter "route" is not available for upstreams with context "stream"' }
+              },
+              {
+                value: { member1: { service: 'member1.backend' } },
+                match: 'member1:80 service=member1.backend;'
+              },
+              {
+                value: { member1: { slow_start: '20s' } },
+                match: 'member1:80 slow_start=20s;'
+              },
+              {
+                value: { member1: { state: 'drain' } },
+                match: 'member1:80 drain;',
+                fails: { stream: 'The state "drain" is not available for upstreams with context "stream"' }
+              },
+              {
+                value: { member1: { state: 'down' } },
+                match: 'member1:80 down;'
+              },
+              {
+                value: { member1: { params_prepend: 'member=1', weight: 20 } },
+                match: 'member1:80 member=1 weight=20;'
+              },
+              {
+                value: { member1: { params_append: 'member=1', weight: 20 } },
+                match: 'member1:80 weight=20 member=1;'
+              },
+              {
+                value: { member1: { comment: 'member1' } },
+                match: 'member1:80; # member1'
+              }
+            ].each do |upstream_member_parameter|
+              context "when members ist set to #{upstream_member_parameter[:value]}" do
+                let(:params) { default_params[upstreamcontext.to_sym].merge(members: upstream_member_parameter[:value]) }
+                let(:conf_d_path) { conf_d_pathes[upstreamcontext.to_sym] }
+
+                if upstream_member_parameter.key?(:fails) && upstream_member_parameter[:fails].key?(upstreamcontext.to_sym)
+                  it {
+                    is_expected.to raise_error(Puppet::Error, %r{#{upstream_member_parameter[:fails][upstreamcontext.to_sym]}})
+                  }
+                  next
+                end
+
+                it {
+                  is_expected.to compile.with_all_deps
+                }
+                it {
+                  is_expected.to contain_concat("#{conf_d_path}/#{title}-upstream.conf").
+                    with_mode('0644')
+                }
+                it {
+                  is_expected.to contain_concat__fragment("#{title}_upstream_header").
+                    with_content("# MANAGED BY PUPPET\nupstream #{title} {\n")
+                }
+                it {
+                  is_expected.to contain_concat__fragment("#{title}_upstream_member_#{upstream_member_parameter[:value].keys[0]}").
+                    with_content("  server #{upstream_member_parameter[:match]}\n")
+                }
+                it {
+                  is_expected.to contain_concat__fragment("#{title}_upstream_footer").
+                    with_content("}\n")
+                }
+              end
             end
 
-            it { is_expected.to contain_concat("/etc/nginx/conf.d/#{title}-upstream.conf").with_ensure('absent') }
+            ##
+            ## check cfg_prepend and cfg_append
+            ##
+            [
+              {
+                parameter: 'cfg_prepend',
+                values: {
+                  'k2' => 'v2',
+                  'k5' => { 'k51' => %w[v51 v52] },
+                  'k1' => 'v2',
+                  'k4' => { 'k41' => 'v41' },
+                  'k3' => %w[v31 v32]
+                },
+                match: "  k2 v2;\n  k5 k51 v51;\n  k5 k51 v52;\n  k1 v2;\n  k4 k41 v41;\n  k3 v31;\n  k3 v32;\n",
+                fragment: 'header'
+              },
+              {
+                parameter: 'cfg_append',
+                values: {
+                  'k2' => 'v2',
+                  'k5' => { 'k51' => %w[v51 v52] },
+                  'k1' => 'v2',
+                  'k4' => { 'k41' => 'v41' },
+                  'k3' => %w[v31 v32]
+                },
+                match: "  k2 v2;\n  k5 k51 v51;\n  k5 k51 v52;\n  k1 v2;\n  k4 k41 v41;\n  k3 v31;\n  k3 v32;\n",
+                fragment: 'footer'
+              }
+            ].each do |upstream_cfg_extension|
+              context "when #{upstream_cfg_extension[:parameter]} is set to #{upstream_cfg_extension[:values]} in #{upstreamcontext} upstream" do
+                let(:params) { default_params[upstreamcontext.to_sym].merge(upstream_cfg_extension[:parameter].to_sym => upstream_cfg_extension[:values]) }
+                let(:conf_d_path) { conf_d_pathes[upstreamcontext.to_sym] }
+
+                it {
+                  is_expected.to compile.with_all_deps
+                }
+                it {
+                  is_expected.to contain_concat("#{conf_d_path}/#{title}-upstream.conf").
+                    with_mode('0644')
+                }
+                if upstream_cfg_extension[:fragment] == 'header'
+                  it {
+                    is_expected.to contain_concat__fragment("#{title}_upstream_header").
+                      with_content("# MANAGED BY PUPPET\nupstream #{title} {\n#{upstream_cfg_extension[:match]}")
+                  }
+                else
+                  it {
+                    is_expected.to contain_concat__fragment("#{title}_upstream_header").
+                      with_content("# MANAGED BY PUPPET\nupstream #{title} {\n")
+                  }
+                end
+                it {
+                  is_expected.to contain_concat__fragment("#{title}_upstream_member_#{params[:members].keys[0]}").
+                    with_content("  server #{params[:members].keys[0]}:80;\n")
+                }
+                if upstream_cfg_extension[:fragment] == 'footer'
+                  it {
+                    is_expected.to contain_concat__fragment("#{title}_upstream_footer").
+                      with_content("#{upstream_cfg_extension[:match]}}\n")
+                  }
+                else
+                  it {
+                    is_expected.to contain_concat__fragment("#{title}_upstream_footer").
+                      with_content("}\n")
+                  }
+                end
+              end
+            end
+
+            context 'when ensure => absent' do
+              let(:params) { default_params[upstreamcontext.to_sym].merge(ensure: 'absent') }
+              let(:conf_d_path) { conf_d_pathes[upstreamcontext.to_sym] }
+
+              it { is_expected.to contain_concat("#{conf_d_path}/#{title}-upstream.conf").with_ensure('absent') }
+            end
           end
         end
       end
diff --git a/templates/upstream/upstream_footer.erb b/templates/upstream/upstream_footer.erb
index 33b0bcd30..23bc97eef 100644
--- a/templates/upstream/upstream_footer.erb
+++ b/templates/upstream/upstream_footer.erb
@@ -1,10 +1,45 @@
-<% if @upstream_cfg_append %>
-<%# Slightly less obtuse way to sort but put keepalive at end -%>
-<%
-  @upstream_cfg_append = Hash[@upstream_cfg_append.sort]
-  @upstream_cfg_append['keepalive'] = @upstream_cfg_append.delete('keepalive')
--%>
-  <%- @upstream_cfg_append.each do |key,value| -%>
+<% if @hash -%>
+  hash <%= @hash %>;
+<% end -%>
+<% if @ip_hash -%>
+  ip_hash;
+<% end -%>
+<% if @least_conn -%>
+  least_conn;
+<% end -%>
+<% if @least_time -%>
+  least_time <%= @least_time %>;
+<% end -%>
+<% if @ntlm -%>
+  ntlm;
+<% end -%>
+<% if @random -%>
+  random <%= @random %>;
+<% end -%>
+<% if @statefile -%>
+  state <%= @statefile %>;
+<% end -%>
+<% if @sticky -%>
+  sticky <% @sticky.each do |type,values| %><%= type -%>
+    <%- if type != 'route' -%>
+      <%- values.each do |key,value| -%> <%= key -%>
+        <%- if value != true %>=<%= value -%><% end -%>
+      <%- end -%>;
+    <%- else %> <%= values %>;
+    <%- end -%>
+  <%- end -%>
+<% end -%>
+<% if @zone -%>
+  zone <%= @zone %>;
+<% end -%>
+<% if @keepalive -%>
+  keepalive <%= @keepalive %>;
+<% end -%>
+<% if @queue_max -%>
+  queue <%= @queue_max %><% if @queue_timeout %> timeout=<%= @queue_timeout %><% end %>;
+<% end -%>
+<% if @cfg_append -%>
+  <%- @cfg_append.each do |key,value| -%>
     <%- if value.is_a?(Hash) -%>
       <%- value.each do |subkey,subvalue| -%>
         <%- Array(subvalue).each do |asubvalue| -%>
@@ -13,7 +48,11 @@
       <%- end -%>
     <%- else -%>
       <%- Array(value).each do |asubvalue| -%>
+        <%- if asubvalue != '' -%>
   <%= key %> <%= asubvalue %>;
+        <%- else -%>
+  <%= key %>;
+        <%- end -%>
       <%- end -%>
     <%- end -%>
   <%- end -%>
diff --git a/templates/upstream/upstream_header.erb b/templates/upstream/upstream_header.erb
index 7aed428d2..4829e7326 100644
--- a/templates/upstream/upstream_header.erb
+++ b/templates/upstream/upstream_header.erb
@@ -1,12 +1,7 @@
 # MANAGED BY PUPPET
 upstream <%= @name %> {
-<% if @upstream_cfg_prepend -%>
-<%# Slightly less obtuse way to sort but put keepalive at end -%>
-<%
-  @upstream_cfg_prepend = Hash[@upstream_cfg_prepend.sort]
-  @upstream_cfg_prepend['keepalive'] = @upstream_cfg_prepend.delete('keepalive')
--%>
-  <%- @upstream_cfg_prepend.each do |key,value| -%>
+<% if @cfg_prepend -%>
+  <%- @cfg_prepend.each do |key,value| -%>
     <%- if value.is_a?(Hash) -%>
       <%- value.each do |subkey,subvalue| -%>
         <%- Array(subvalue).each do |asubvalue| -%>
diff --git a/templates/upstream/upstream_member.erb b/templates/upstream/upstream_member.erb
index 5f44b54ee..66f8cdbd5 100644
--- a/templates/upstream/upstream_member.erb
+++ b/templates/upstream/upstream_member.erb
@@ -1 +1,12 @@
-  server     <%= @server %>:<%= @port %>  fail_timeout=<%= @upstream_fail_timeout %><% if @upstream_max_fails -%> max_fails=<%=@upstream_max_fails %><% end %>;
+  server <%= @_server %><% if @params_prepend -%> <%= @params_prepend %><% end -%>
+<%- if @state %> <%= @state %><% end -%>
+<%- if @weight %> weight=<%= @weight %><% end -%>
+<%- if @max_conns %> max_conns=<%= @max_conns %><% end -%>
+<%- if @max_fails %> max_fails=<%= @max_fails %><% end -%>
+<%- if @fail_timeout %> fail_timeout=<%= @fail_timeout %><% end -%>
+<%- if @slow_start %> slow_start=<%= @slow_start %><% end -%>
+<%- if @service %> service=<%= @service %><% end -%>
+<%- if @route %> route=<%= @route %><% end -%>
+<%- if @resolve %> resolve<% end -%>
+<%- if @backup %> backup<% end -%>
+<%- if @params_append %> <%= @params_append %><% end %>;<% if @comment %> # <%= @comment %><% end %>
diff --git a/templates/upstream/upstream_members.erb b/templates/upstream/upstream_members.erb
deleted file mode 100644
index 0f5c30626..000000000
--- a/templates/upstream/upstream_members.erb
+++ /dev/null
@@ -1,2 +0,0 @@
-  <% @members.each do |i| %>
-  server     <%= i %>  fail_timeout=<%= @upstream_fail_timeout %><% if @upstream_max_fails -%> max_fails=<%=@upstream_max_fails %><% end %>;<% end %>
diff --git a/types/size.pp b/types/size.pp
new file mode 100644
index 000000000..580476686
--- /dev/null
+++ b/types/size.pp
@@ -0,0 +1 @@
+type Nginx::Size = Pattern[/^\d+[k|K|m|M]?$/]
diff --git a/types/time.pp b/types/time.pp
new file mode 100644
index 000000000..f6c24edc8
--- /dev/null
+++ b/types/time.pp
@@ -0,0 +1 @@
+type Nginx::Time = Pattern[/^\d+(ms|s|m|h|d|w|M|y)?$/]
diff --git a/types/upstreamleasttime.pp b/types/upstreamleasttime.pp
new file mode 100644
index 000000000..daeab96b8
--- /dev/null
+++ b/types/upstreamleasttime.pp
@@ -0,0 +1 @@
+type Nginx::UpstreamLeastTime = Variant[Nginx::UpstreamLeastTimeHttp,Nginx::UpstreamLeastTimeStream]
diff --git a/types/upstreamleasttimehttp.pp b/types/upstreamleasttimehttp.pp
new file mode 100644
index 000000000..5299f4524
--- /dev/null
+++ b/types/upstreamleasttimehttp.pp
@@ -0,0 +1 @@
+type Nginx::UpstreamLeastTimeHttp = Enum['header','header inflight','last_byte','last_byte inflight']
diff --git a/types/upstreamleasttimestream.pp b/types/upstreamleasttimestream.pp
new file mode 100644
index 000000000..35ff9fcfd
--- /dev/null
+++ b/types/upstreamleasttimestream.pp
@@ -0,0 +1 @@
+type Nginx::UpstreamLeastTimeStream = Enum['connect','connect inflight','first_byte','first_byte inflight','last_byte','last_byte inflight']
diff --git a/types/upstreammemberdefaults.pp b/types/upstreammemberdefaults.pp
new file mode 100644
index 000000000..0bb1ce7c1
--- /dev/null
+++ b/types/upstreammemberdefaults.pp
@@ -0,0 +1,15 @@
+type Nginx::UpstreamMemberDefaults = Struct[{
+  port           => Optional[Stdlib::Port],
+  weight         => Optional[Integer[1]],
+  max_conns      => Optional[Integer[1]],
+  max_fails      => Optional[Integer[1]],
+  fail_timeout   => Optional[Nginx::Time],
+  backup         => Optional[Boolean],
+  resolve        => Optional[Boolean],
+  route          => Optional[String],
+  service        => Optional[String],
+  slow_start     => Optional[Nginx::Time],
+  state          => Optional[Enum['drain','down']],
+  params_prepend => Optional[String],
+  params_append  => Optional[String],
+}]
diff --git a/types/upstreammemberserver.pp b/types/upstreammemberserver.pp
new file mode 100644
index 000000000..81afbfd05
--- /dev/null
+++ b/types/upstreammemberserver.pp
@@ -0,0 +1 @@
+type Nginx::UpstreamMemberServer = Variant[Stdlib::Host,Pattern[/^unix:\/([^\/\0]+\/*)*$/]]
diff --git a/types/upstreamsticky.pp b/types/upstreamsticky.pp
new file mode 100644
index 000000000..f60c3c8a9
--- /dev/null
+++ b/types/upstreamsticky.pp
@@ -0,0 +1,28 @@
+type Nginx::UpstreamSticky = Variant[
+  Hash[
+    Enum['cookie'],
+    Struct[{
+      name     => String,
+      expires  => Optional[Variant[Nginx::Time,Enum['max']]],
+      domain   => Optional[String],
+      httponly => Optional[Boolean],
+      secure   => Optional[Boolean],
+      path     => Optional[String],
+    }]
+  ],
+  Hash[
+    Enum['route'],
+    String
+  ],
+  Hash[
+    Enum['learn'],
+    Struct[{
+      create  => String,
+      lookup  => String,
+      zone    => Nginx::UpstreamZone,
+      timeout => Optional[Nginx::Time],
+      header  => Optional[Boolean],
+      sync    => Optional[Boolean],
+    }]
+  ]
+]
diff --git a/types/upstreamzone.pp b/types/upstreamzone.pp
new file mode 100644
index 000000000..4248bb25a
--- /dev/null
+++ b/types/upstreamzone.pp
@@ -0,0 +1 @@
+type Nginx::UpstreamZone = Pattern[/^[^\s]*( \d+[k|K|m|M])?$/]