From 13b08bbad71e411184198ca011f13333615bfab0 Mon Sep 17 00:00:00 2001 From: Steve Traylen Date: Wed, 22 May 2024 15:40:09 +0200 Subject: [PATCH] Allow ports parameters as Stdlib::Ports Currently the ports parameters must be specified as one of: * `'1234'` * `'1234,5678'` * `['1234']` * `['1234','5678']` Allow it to also be specified as the `Stdlib::Port` type. e.g. * `1234` * `[1234]` * `[1234,5678]` To encourage this direction the documentation has been updated to use `Stdlib::Port` types. The old formats are still supported. Motivation here is we were happily using v4 of this module with integer specifications for ports and this is better anyway. Some future version could drop the old stringified ports. --- README.md | 30 ++++++------- REFERENCE.md | 26 ++++++++--- manifests/balancermember.pp | 6 +-- manifests/frontend.pp | 4 +- manifests/listen.pp | 4 +- spec/acceptance/basic_spec.rb | 30 ++++++++++++- spec/defines/balancermember_spec.rb | 42 +++++++++++++++--- spec/defines/frontend_spec.rb | 59 +++++++++++++++++-------- spec/type_aliases/haproxy_ports_spec.rb | 22 +++++++++ templates/fragments/_bind.epp | 6 ++- types/ports.pp | 3 ++ 11 files changed, 173 insertions(+), 59 deletions(-) create mode 100644 spec/type_aliases/haproxy_ports_spec.rb create mode 100644 types/ports.pp diff --git a/README.md b/README.md index 288fa0f7..c6c6bc34 100644 --- a/README.md +++ b/README.md @@ -43,20 +43,20 @@ node 'haproxy-server' { haproxy::listen { 'puppet00': collect_exported => false, ipaddress => $facts['networking']['ip'], - ports => '8140', + ports => [8140], } haproxy::balancermember { 'server00': listening_service => 'puppet00', server_names => 'server00.example.com', ipaddresses => '10.0.0.10', - ports => '8140', + ports => [8140], options => 'check', } haproxy::balancermember { 'server01': listening_service => 'puppet00', server_names => 'server01.example.com', ipaddresses => '10.0.0.11', - ports => '8140', + ports => [8140], options => 'check', } } @@ -153,7 +153,7 @@ To export the resource for a balancermember and collect it on a single HAProxy l ~~~puppet haproxy::listen { 'puppet00': ipaddress => $facts['networking']['ip'], - ports => '8140', + ports => [8140], mode => 'tcp', options => { 'option' => [ @@ -213,7 +213,7 @@ Then create the resource for multiple balancermembers at once: ~~~puppet haproxy::balancermember { 'haproxy': listening_service => 'puppet00', - ports => '8140', + ports => 8140, server_names => ['server01', 'server02'], ipaddresses => ['192.168.56.200', '192.168.56.201'], options => 'check', @@ -231,7 +231,7 @@ node 'haproxy-server' { include ::haproxy haproxy::listen { 'puppet00': ipaddress => $facts['networking']['ip'], - ports => '8140', + ports => 8140, } } @@ -240,7 +240,7 @@ node /^server\d+/ { listening_service => 'puppet00', server_names => $facts['networking']['hostname'], ipaddresses => $facts['networking']['ip'], - ports => '8140', + ports => 8140, options => 'check', } } @@ -255,7 +255,7 @@ This example routes traffic from port 8140 to all balancermembers added to a bac ~~~puppet haproxy::frontend { 'puppet00': ipaddress => $facts['networking']['ip'], - ports => '8140', + ports => 8140, mode => 'tcp', bind_options => 'accept-proxy', options => { @@ -274,7 +274,7 @@ If option order is important, pass an array of hashes to the `options` parameter ~~~puppet haproxy::frontend { 'puppet00': ipaddress => $facts['networking']['ip'], - ports => '8140', + ports => [8140], mode => 'tcp', bind_options => 'accept-proxy', options => [ @@ -353,7 +353,7 @@ haproxy::resolver { 'puppet00': # Setup the balancermember to use the resolver for DNS resolution haproxy::balancermember { 'haproxy': listening_service => 'puppet00', - ports => '8140', + ports => 8140, server_names => ['server01', 'server02'], ipaddresses => ['server01', 'server02'], options => 'check resolvers puppet00 resolve-prefer ipv4', @@ -399,7 +399,7 @@ class and uses `haproxy::instance` to add an additional instance called instance => 'haproxy', collect_exported => false, ipaddress => $facts['networking']['ip'], - ports => '8800', + ports => 8800, } haproxy::instance { 'beta': } @@ -413,7 +413,7 @@ class and uses `haproxy::instance` to add an additional instance called instance => 'beta', collect_exported => false, ipaddress => $facts['networking']['ip'], - ports => '9900', + ports => 9900, } ~~~ @@ -432,7 +432,7 @@ The second uses a custom package. instance => 'group1', collect_exported => false, ipaddress => $facts['networking']['ip'], - ports => '8800', + ports => 8800, } haproxy::instance { 'group2': } -> @@ -446,7 +446,7 @@ The second uses a custom package. instance => 'group2', collect_exported => false, ipaddress => $facts['networking']['ip'], - ports => '9900', + ports => 9900, } ~~~ @@ -481,7 +481,7 @@ Or expressed using `haproxy::frontend`: ~~~puppet haproxy::frontend { 'ft_allapps': ipaddress => '0.0.0.0', - ports => '80', + ports => ['80'], mode => 'http', options => { 'use_backend' => '%[req.hdr(host),lower,map(/etc/haproxy/domains-to-backends.map,bk_default)]' diff --git a/REFERENCE.md b/REFERENCE.md index 3aada6ab..cf18b5d8 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -55,6 +55,10 @@ file on an haproxy load balancer. * [`haproxy::sort_bind`](#haproxy--sort_bind) * [`haproxy::validate_ip_addr`](#haproxy--validate_ip_addr) +### Data types + +* [`Haproxy::Ports`](#Haproxy--Ports): Port or list of ports for haproxy. Supports `,` seperated list of ports also. + ## Classes ### `haproxy` @@ -485,7 +489,7 @@ Exporting the resource for a balancer member: @@haproxy::balancermember { 'haproxy': listening_service => 'puppet00', - ports => '8140', + ports => [8140], server_names => $::hostname, ipaddresses => $::ipaddress, options => 'check', @@ -505,7 +509,7 @@ pass to export the resources if you know the members in advance): haproxy::balancermember { 'haproxy': listening_service => 'puppet00', - ports => '8140', + ports => 8140, server_names => ['server01', 'server02'], ipaddresses => ['192.168.56.200', '192.168.56.201'], options => 'check', @@ -566,7 +570,7 @@ The haproxy service's instance name (or, the title of the ##### `ports` -Data type: `Optional[Variant[Array, String]]` +Data type: `Optional[Haproxy::Ports]` An array or commas-separated list of ports for which the balancer member will accept connections from the load balancer. Note that cookie values @@ -777,7 +781,7 @@ Exporting the resource for a balancer member: haproxy::frontend { 'puppet00': ipaddress => $::ipaddress, - ports => '18140', + ports => [18140], mode => 'tcp', bind_options => 'accept-proxy', options => { @@ -821,7 +825,7 @@ Default value: `$name` ##### `ports` -Data type: `Optional[Variant[Array, String]]` +Data type: `Optional[Haproxy::Ports]` Ports on which the proxy will listen for connections on the ip address specified in the ipaddress parameter. Accepts either a single @@ -1286,7 +1290,7 @@ load balancer server. ```puppet haproxy::listen { 'puppet00': ipaddress => $::ipaddress, - ports => '18140', + ports => [18140], mode => 'tcp', options => { 'option' => [ @@ -1327,7 +1331,7 @@ Default value: `$name` ##### `ports` -Data type: `Optional[Variant[Array, String]]` +Data type: `Optional[Haproxy::Ports]` Ports on which the proxy will listen for connections on the ip address specified in the ipaddress parameter. Accepts either a single @@ -2085,3 +2089,11 @@ Data type: `String` +## Data types + +### `Haproxy::Ports` + +Port or list of ports for haproxy. Supports `,` seperated list of ports also. + +Alias of `Variant[Array[Variant[Pattern[/^[0-9]+$/],Stdlib::Port],0], Pattern[/^[0-9,]+$/], Stdlib::Port]` + diff --git a/manifests/balancermember.pp b/manifests/balancermember.pp index a4e6df0b..3a8ba0e7 100644 --- a/manifests/balancermember.pp +++ b/manifests/balancermember.pp @@ -90,7 +90,7 @@ # # @@haproxy::balancermember { 'haproxy': # listening_service => 'puppet00', -# ports => '8140', +# ports => [8140], # server_names => $::hostname, # ipaddresses => $::ipaddress, # options => 'check', @@ -108,7 +108,7 @@ # # haproxy::balancermember { 'haproxy': # listening_service => 'puppet00', -# ports => '8140', +# ports => 8140, # server_names => ['server01', 'server02'], # ipaddresses => ['192.168.56.200', '192.168.56.201'], # options => 'check', @@ -137,7 +137,7 @@ define haproxy::balancermember ( String $listening_service, Enum['server', 'default-server', 'server-template'] $type = 'server', - Optional[Variant[Array, String]] $ports = undef, + Optional[Haproxy::Ports] $ports = undef, Optional[Variant[String, Stdlib::Port]] $port = undef, Variant[String[1], Array] $server_names = $facts['networking']['hostname'], Variant[String, Array] $ipaddresses = $facts['networking']['ip'], diff --git a/manifests/frontend.pp b/manifests/frontend.pp index 1f3fcc69..89a43754 100644 --- a/manifests/frontend.pp +++ b/manifests/frontend.pp @@ -73,7 +73,7 @@ # # haproxy::frontend { 'puppet00': # ipaddress => $::ipaddress, -# ports => '18140', +# ports => [18140], # mode => 'tcp', # bind_options => 'accept-proxy', # options => { @@ -91,7 +91,7 @@ # Gary Larizza # define haproxy::frontend ( - Optional[Variant[Array, String]] $ports = undef, + Optional[Haproxy::Ports] $ports = undef, Optional[Variant[String, Array]] $ipaddress = undef, Optional[Hash] $bind = undef, Optional[Enum['tcp', 'http', 'health']] $mode = undef, diff --git a/manifests/listen.pp b/manifests/listen.pp index 0b73609c..6866476d 100644 --- a/manifests/listen.pp +++ b/manifests/listen.pp @@ -78,7 +78,7 @@ # @example # haproxy::listen { 'puppet00': # ipaddress => $::ipaddress, -# ports => '18140', +# ports => [18140], # mode => 'tcp', # options => { # 'option' => [ @@ -94,7 +94,7 @@ # Gary Larizza # define haproxy::listen ( - Optional[Variant[Array, String]] $ports = undef, + Optional[Haproxy::Ports] $ports = undef, Optional[Variant[String, Array]] $ipaddress = undef, Optional[Hash] $bind = undef, Optional[Enum['tcp', 'http', 'health']] $mode = undef, diff --git a/spec/acceptance/basic_spec.rb b/spec/acceptance/basic_spec.rb index 6aa79043..48e64200 100644 --- a/spec/acceptance/basic_spec.rb +++ b/spec/acceptance/basic_spec.rb @@ -38,6 +38,32 @@ class { 'haproxy': describe 'configuring haproxy load balancing' do describe 'multiple ports' do + pp_two = <<-PUPPETCODE + class { 'haproxy': } + haproxy::listen { 'stats': + ipaddress => '127.0.0.1', + ports => [9090, 9091], + mode => 'http', + options => { 'stats' => ['uri /','auth puppet:puppet'], }, + } + PUPPETCODE + it 'is able to listen on an array of ports' do + retry_on_error_matching do + apply_manifest(pp_two, catch_failures: true) + end + end + + ['9090', '9091'].each do |port| + it "port #{port} has stats listening on each port" do + run_shell("/usr/bin/curl -u puppet:puppet localhost:#{port}") do |r| + expect(r.stdout).to contain %r{HAProxy} + expect(r.exit_code).to eq 0 + end + end + end + end + + describe 'multiple ports as strings' do pp_two = <<-PUPPETCODE class { 'haproxy': } haproxy::listen { 'stats': @@ -71,7 +97,7 @@ class { 'haproxy::globals': class { 'haproxy': } haproxy::listen { 'stats': ipaddress => '127.0.0.1', - ports => ['9090','9091'], + ports => [9090,9091], mode => 'http', options => { 'stats' => ['uri /','auth puppet:puppet'], }, } @@ -101,7 +127,7 @@ class { 'haproxy::globals': class { 'haproxy': } haproxy::listen { 'stats': ipaddress => '127.0.0.1', - ports => ['9091'], + ports => [9091], mode => 'http', } haproxy::backend { 'servers': diff --git a/spec/defines/balancermember_spec.rb b/spec/defines/balancermember_spec.rb index 02d8eeb9..f9b51331 100644 --- a/spec/defines/balancermember_spec.rb +++ b/spec/defines/balancermember_spec.rb @@ -23,7 +23,7 @@ { name: 'tyler', listening_service: 'croy', - ports: '18140', + ports: 18_140, options: 'check' } end @@ -35,6 +35,20 @@ 'content' => " server dero 1.1.1.1:18140 check\n", ) } + + context 'with stringy ports' do + let(:params) do + super().merge(ports: '18140') + end + + it { + is_expected.to contain_concat__fragment('haproxy-croy_balancermember_tyler').with( + 'order' => '20-croy-01-tyler', + 'target' => '/etc/haproxy/haproxy.cfg', + 'content' => " server dero 1.1.1.1:18140 check\n", + ) + } + end end context 'with multiple balancermember options' do @@ -42,7 +56,7 @@ { name: 'tyler', listening_service: 'croy', - ports: '18140', + ports: 18_140, options: ['check', 'close'] } end @@ -61,7 +75,7 @@ { name: 'tyler', listening_service: 'croy', - ports: '18140', + ports: 18_140, options: ['check', 'close'], define_cookies: true } @@ -81,7 +95,7 @@ { name: 'tyler', listening_service: 'croy', - ports: '18140', + ports: 18_140, options: ['check', 'close'], verifyhost: true } @@ -101,7 +115,7 @@ { name: 'tyler', listening_service: 'croy', - ports: '18140', + ports: 18_140, server_names: ['server01', 'server02'], ipaddresses: ['192.168.56.200', '192.168.56.201'], options: ['check'] @@ -122,7 +136,7 @@ { name: 'tyler', listening_service: 'croy', - ports: ['18140', '18150'], + ports: [18_140, 18_150], server_names: ['server01', 'server02'], ipaddresses: ['192.168.56.200', '192.168.56.201'], options: ['check'] @@ -136,6 +150,20 @@ 'content' => " server server01 192.168.56.200:18140 check\n server server01 192.168.56.200:18150 check\n server server02 192.168.56.201:18140 check\n server server02 192.168.56.201:18150 check\n", # rubocop:disable Layout/LineLength ) } + + context 'with stringy ports' do + let(:params) do + super().merge(ports: ['18140', '18150']) + end + + it { + is_expected.to contain_concat__fragment('haproxy-croy_balancermember_tyler').with( + 'order' => '20-croy-01-tyler', + 'target' => '/etc/haproxy/haproxy.cfg', + 'content' => " server server01 192.168.56.200:18140 check\n server server01 192.168.56.200:18150 check\n server server02 192.168.56.201:18140 check\n server server02 192.168.56.201:18150 check\n", # rubocop:disable Layout/LineLength + ) + } + end end context 'with multiple servers and no port' do @@ -184,7 +212,7 @@ { name: 'tyler', listening_service: 'croy', - ports: '18140', + ports: 18_140, options: 'check', weight: '100' } diff --git a/spec/defines/frontend_spec.rb b/spec/defines/frontend_spec.rb index e994e5ab..d3731557 100644 --- a/spec/defines/frontend_spec.rb +++ b/spec/defines/frontend_spec.rb @@ -22,7 +22,7 @@ { name: 'croy', ipaddress: '1.1.1.1', - ports: '18140' + ports: 18_140 } end @@ -33,6 +33,19 @@ 'content' => "\nfrontend croy\n bind 1.1.1.1:18140 \n option tcplog\n", ) } + context 'with stringy port' do + let(:params) do + super().merge(ports: '18140') + end + + it { + is_expected.to contain_concat__fragment('haproxy-croy_frontend_block').with( + 'order' => '15-croy-00', + 'target' => '/etc/haproxy/haproxy.cfg', + 'content' => "\nfrontend croy\n bind 1.1.1.1:18140 \n option tcplog\n", + ) + } + end end # C9948 C9947 @@ -41,7 +54,7 @@ { name: 'apache', ipaddress: '23.23.23.23', - ports: ['80', '443'] + ports: [80, 443] } end @@ -52,25 +65,33 @@ 'content' => "\nfrontend apache\n bind 23.23.23.23:80 \n bind 23.23.23.23:443 \n option tcplog\n", ) } - end - - # C9948 - context 'when a comma-separated list of ports is provided' do - let(:params) do - { - name: 'apache', - ipaddress: '23.23.23.23', - ports: '80,443' + context 'with stringy port' do + let(:params) do + super().merge(ports: ['80', '443']) + end + + it { + is_expected.to contain_concat__fragment('haproxy-apache_frontend_block').with( + 'order' => '15-apache-00', + 'target' => '/etc/haproxy/haproxy.cfg', + 'content' => "\nfrontend apache\n bind 23.23.23.23:80 \n bind 23.23.23.23:443 \n option tcplog\n", + ) + } + end + # C9948 + context 'with a comma-seperated list of ports' do + let(:params) do + super().merge(ports: '80,443') + end + + it { + is_expected.to contain_concat__fragment('haproxy-apache_frontend_block').with( + 'order' => '15-apache-00', + 'target' => '/etc/haproxy/haproxy.cfg', + 'content' => "\nfrontend apache\n bind 23.23.23.23:80 \n bind 23.23.23.23:443 \n option tcplog\n", + ) } end - - it { - is_expected.to contain_concat__fragment('haproxy-apache_frontend_block').with( - 'order' => '15-apache-00', - 'target' => '/etc/haproxy/haproxy.cfg', - 'content' => "\nfrontend apache\n bind 23.23.23.23:80 \n bind 23.23.23.23:443 \n option tcplog\n", - ) - } end # C9971 diff --git a/spec/type_aliases/haproxy_ports_spec.rb b/spec/type_aliases/haproxy_ports_spec.rb new file mode 100644 index 00000000..7bfdd995 --- /dev/null +++ b/spec/type_aliases/haproxy_ports_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe 'Haproxy::Ports' do + # Sensible port declarations + it { is_expected.to allow_value(1234) } + it { is_expected.to allow_value([1234]) } + it { is_expected.to allow_value([1234, 5678]) } + it { is_expected.to allow_value([]) } + + # Bad multi port declaration - consider droping their support + # in the future. + it { is_expected.to allow_value('1234') } + it { is_expected.to allow_value('1234,5678') } + it { is_expected.to allow_value(['1234']) } + it { is_expected.to allow_value(['1234', '4567']) } + + # Disallowed + it { is_expected.not_to allow_value('') } + # These cause errors already in current rspec tests so disallow here as well. + it { is_expected.not_to allow_value(['1234,5678']) } + it { is_expected.not_to allow_value('1234, 5678') } +end diff --git a/templates/fragments/_bind.epp b/templates/fragments/_bind.epp index 3dca3cec..b28bba1c 100644 --- a/templates/fragments/_bind.epp +++ b/templates/fragments/_bind.epp @@ -4,9 +4,11 @@ <%- } -%> <% } else { -%> <%- Array($ipaddress.flatten).unique.each |$virtual_ip| { -%> - <%- if String(type($ports, 'generalized')).index('Array') == 0 { -%> + <%- if $ports =~ Array { -%> <%- $ports_as_array = $ports -%> - <%- } elsif String(type($ports, 'generalized')).index('String') == 0 { -%> + <%- } elsif $ports =~ Stdlib::Port { -%> + <%- $ports_as_array = Array($ports,true) -%> + <%- } elsif $ports =~ String { -%> <%- $ports_as_array = Array($ports.split(",")) -%> <%- } else { -%> <%- $ports_as_array = [] -%> diff --git a/types/ports.pp b/types/ports.pp new file mode 100644 index 00000000..fb2907f9 --- /dev/null +++ b/types/ports.pp @@ -0,0 +1,3 @@ +# @summary Port or list of ports for haproxy. Supports `,` seperated list of ports also. +# +type Haproxy::Ports = Variant[Array[Variant[Pattern[/^[0-9]+$/],Stdlib::Port],0], Pattern[/^[0-9,]+$/], Stdlib::Port]