Replace source and dest parameters

Split `source` parameter into `source_socket_file`, `source_host` and
`source_port` parameters.  Rename `dest` parameter to `target_`
parameters.

By splitting the parameters, it's much easier to do proper input
validation and build the socket parameters passed to the binary
correctly.  For instance, IP addresses are supposed to be surrounded by
square brackets but hostnames should not.

See https://github.com/Tarsnap/spiped/blob/5c13832aeecdad8a655dadcf5413cc504ad99e49/libcperciva/util/sock.c#L253

Replace erb with epp template.

README.md

@@ -46,19 +46,21 @@ On `redis-host`, we would define a server tunnel:
 
 ```puppet
 spiped::tunnel::server { 'redis':
-  source => '0.0.0.0:1234',
-  dest   => '/var/run/redis.sock',
-  secret => 'hunter2',
+  source_host        => '0.0.0.0',
+  source_port        => 1234,
+  target_socket_file => '/var/run/redis.sock',
+  secret             => 'hunter2',
 }
 ```
 
 On clients, we would define a client tunnel:
 
 ```puppet
 spiped::tunnel::client { 'redis':
-  source => '/var/run/redis.sock',
-  dest   => 'redis-host:1234',
-  secret => 'hunter2',
+  source_socket_file => '/var/run/redis.sock',
+  target_host        => 'redis-host'
+  target_port        => 1234',
+  secret             => 'hunter2',
 }
 ```
 

manifests/tunnel.pp

@@ -1,12 +1,46 @@
 define spiped::tunnel(
-  $type,
-  $source,
-  $dest,
-  $secret,
+  Enum['client','server'] $type,
+  Variant[Sensitive[String[1]],String[1]] $secret,
+
+  Optional[Stdlib::Unixpath] $source_socket_file = undef,
+  Optional[Stdlib::Host]     $source_host = undef,
+  Optional[Stdlib::Port]     $source_port = undef,
+
+  Optional[Stdlib::Unixpath] $target_socket_file = undef,
+  Optional[Stdlib::Host]     $target_host = undef,
+  Optional[Stdlib::Port]     $target_port = undef,
 ) {
   assert_private()
   include spiped
 
+  if $source_socket_file {
+    if $source_host { fail('`source_host` must not be specified when using `source_socket_file`') }
+    if $source_port { fail('`source_port` must not be specified when using `source_socket_file`') }
+    $source_socket = $source_socket_file
+  } else {
+    unless $source_host and $source_port { fail('either `source_socket_file` or `source_host` and `source_port must be specified') }
+    $source_socket = $source_host ? {
+      # lint:ignore:unquoted_string_in_selector
+      Stdlib::IP::Address => "[${source_host}]:${source_port}",
+      Stdlib::Fqdn        => "${source_host}:${source_port}",
+      # lint:endignore
+    }
+  }
+
+  if $target_socket_file {
+    if $target_host { fail('`target_host` must not be specified when using `target_socket_file`') }
+    if $target_port { fail('`target_port` must not be specified when using `target_socket_file`') }
+    $target_socket = $target_socket_file
+  } else {
+    unless $target_host and $target_port { fail('either `target_socket_file` or `target_host` and `target_port must be specified') }
+    $target_socket = $target_host ? {
+      # lint:ignore:unquoted_string_in_selector
+      Stdlib::IP::Address => "[${target_host}]:${target_port}",
+      Stdlib::Fqdn        => "${target_host}:${target_port}",
+      # lint:endignore
+    }
+  }
+
   $keyfile = "/etc/spiped/${title}.key"
 
   file { $keyfile:
@@ -24,7 +58,16 @@
   }
 
   systemd::unit_file { "spiped-${title}.service":
-    content => template('spiped/service.erb'),
+    content => epp(
+      'spiped/service.epp',
+      {
+        'name'          => $title,
+        'type'          => $type,
+        'keyfile'       => $keyfile,
+        'source_socket' => $source_socket,
+        'target_socket' => $target_socket,
+      },
+    ),
   }
 
   service { "spiped-${title}":

manifests/tunnel/client.pp

@@ -1,12 +1,23 @@
 define spiped::tunnel::client(
-  $source,
-  $dest,
-  $secret,
-) {
+  Variant[Sensitive[String[1]],String[1]] $secret,
+
+  Optional[Stdlib::Unixpath] $source_socket_file = undef,
+  Optional[Stdlib::Host]     $source_host = undef,
+  Optional[Stdlib::Port]     $source_port = undef,
+
+  Optional[Stdlib::Unixpath] $target_socket_file = undef,
+  Optional[Stdlib::Host]     $target_host = undef,
+  Optional[Stdlib::Port]     $target_port = undef,
+)
+{
   spiped::tunnel { $title:
-    type   => 'client',
-    source => $source,
-    dest   => $dest,
-    secret => $secret;
+    type               => 'client',
+    source_socket_file => $source_socket_file,
+    source_host        => $source_host,
+    source_port        => $source_port,
+    target_socket_file => $target_socket_file,
+    target_host        => $target_host,
+    target_port        => $target_port,
+    secret             => $secret,
   }
 }

manifests/tunnel/server.pp

@@ -1,12 +1,23 @@
 define spiped::tunnel::server(
-  $source,
-  $dest,
-  $secret,
-) {
+  Variant[Sensitive[String[1]],String[1]] $secret,
+
+  Optional[Stdlib::Unixpath] $source_socket_file = undef,
+  Optional[Stdlib::Host]     $source_host = undef,
+  Optional[Stdlib::Port]     $source_port = undef,
+
+  Optional[Stdlib::Unixpath] $target_socket_file = undef,
+  Optional[Stdlib::Host]     $target_host = undef,
+  Optional[Stdlib::Port]     $target_port = undef,
+)
+{
   spiped::tunnel { $title:
-    type   => 'server',
-    source => $source,
-    dest   => $dest,
-    secret => $secret;
+    type               => 'server',
+    source_socket_file => $source_socket_file,
+    source_host        => $source_host,
+    source_port        => $source_port,
+    target_socket_file => $target_socket_file,
+    target_host        => $target_host,
+    target_port        => $target_port,
+    secret             => $secret,
   }
 }

metadata.json

@@ -8,7 +8,7 @@
     "project_page": "https://github.com/chriskuehl/puppet-spiped",
     "issues_url": "https://github.com/chriskuehl/puppet-spiped/issues",
     "dependencies": [
-        { "name": "puppetlabs/stdlib", "version_requirement": ">= 2.3.1 < 7.0.0" },
+        { "name": "puppetlabs/stdlib", "version_requirement": ">= 4.25.0 < 7.0.0" },
         { "name": "camptocamp/systemd", "version_requirement": ">= 1.1.0 < 3.0.0" }
     ],
     "operatingsystem_support": [
@@ -38,5 +38,11 @@
           "18.04"
         ]
       }
+    ],
+    "requirements": [
+      {
+        "name": "puppet",
+        "version_requirement": ">= 5.5.8 < 7.0.0"
+      }
     ]
 }

spec/acceptance/spiped_spec.rb

@@ -7,8 +7,10 @@
     it 'setups spiped server idempotently' do
       pp = %(
       spiped::tunnel::server { 'redis':
-        source => '[0.0.0.0]:16379',
-        dest   => '127.0.0.1:6379',
+        source_host => '0.0.0.0',
+        source_port => 16379,
+        target_host => '127.0.0.1',
+        target_port => 6379,
         secret => 'hunter2',
       }
       )
@@ -21,8 +23,10 @@
     it 'setups spiped client idempotently' do
       pp = %(
       spiped::tunnel::client { 'redis':
-        source => '[127.0.0.1]:1234',
-        dest   => "#{server_ip}:16379",
+        source_host => '127.0.0.1',
+        source_port => 1234,
+        target_host => #{server_ip},
+        target_port => 16379
         secret => 'hunter2',
       }
       )

spec/defines/spiped_tunnel_client_spec.rb

@@ -4,8 +4,9 @@
   let(:title) { 'redis' }
   let(:params) do
     {
-      source: '/var/run/redis.sock',
-      dest:   'redis-host:1234',
+      source_socket_file: '/var/run/redis.sock',
+      target_host: 'redis-host',
+      target_port: 1234,
       secret: 'hunter2'
     }
   end
@@ -17,9 +18,9 @@
       it { is_expected.to compile }
       it {
         is_expected.to contain_spiped__tunnel('redis').with(
-          type: 'client',
-          source: '/var/run/redis.sock',
-          dest:   'redis-host:1234',
+          source_socket_file: '/var/run/redis.sock',
+          target_host: 'redis-host',
+          target_port: 1234,
           secret: 'hunter2'
         )
       }
@@ -46,7 +47,7 @@
           expect(content).to include('Description=spiped tunnel (redis)')
         end
         it 'ExecStart' do
-          expect(content).to include('ExecStart=/usr/bin/spiped -e -D -g -F -k \'/etc/spiped/redis.key\' -p /dev/null -s \'/var/run/redis.sock\' -t \'redis-host:1234\'')
+          expect(content).to include('ExecStart=/usr/bin/spiped -e -D -g -F -k \'/etc/spiped/redis.key\' -s \'/var/run/redis.sock\' -t \'redis-host:1234\'')
         end
       end
 

spec/defines/spiped_tunnel_server_spec.rb

@@ -4,8 +4,9 @@
   let(:title) { 'redis' }
   let(:params) do
     {
-      source: '0.0.0.0:1234',
-      dest:   '/var/run/redis.sock',
+      source_host: '0.0.0.0',
+      source_port: 1234,
+      target_socket_file: '/var/run/redis.sock',
       secret: 'hunter2'
     }
   end
@@ -18,8 +19,9 @@
       it {
         is_expected.to contain_spiped__tunnel('redis').with(
           type: 'server',
-          source: '0.0.0.0:1234',
-          dest:   '/var/run/redis.sock',
+          source_host: '0.0.0.0',
+          source_port: 1234,
+          target_socket_file: '/var/run/redis.sock',
           secret: 'hunter2'
         )
       }
@@ -46,7 +48,7 @@
           expect(content).to include('Description=spiped tunnel (redis)')
         end
         it 'ExecStart' do
-          expect(content).to include('ExecStart=/usr/bin/spiped -d -g -F -k \'/etc/spiped/redis.key\' -p /dev/null -s \'0.0.0.0:1234\' -t \'/var/run/redis.sock\'')
+          expect(content).to include('ExecStart=/usr/bin/spiped -d -g -F -k \'/etc/spiped/redis.key\' -s \'[0.0.0.0]:1234\' -t \'/var/run/redis.sock\'')
         end
       end
 
@@ -58,4 +60,103 @@
       }
     end
   end
+
+  describe 'conflicting parameters' do
+    describe 'source parameters' do
+      context 'source_socket_file and source_host' do
+        let(:params) do
+          {
+            source_socket_file: '/path/to/socket',
+            source_host: '0.0.0.0',
+            target_socket_file: '/var/run/redis.sock',
+            secret: 'hunter2'
+          }
+        end
+
+        it { is_expected.to compile.and_raise_error(%r{`source_host` must not be specified when using `source_socket_file`}) }
+      end
+      context 'source_socket_file and source_port' do
+        let(:params) do
+          {
+            source_socket_file: '/path/to/socket',
+            source_port: 1234,
+            target_socket_file: '/var/run/redis.sock',
+            secret: 'hunter2'
+          }
+        end
+
+        it { is_expected.to compile.and_raise_error(%r{`source_port` must not be specified when using `source_socket_file`}) }
+      end
+      context 'missing source_host' do
+        let(:params) do
+          {
+            source_port: 1234,
+            target_socket_file: '/var/run/redis.sock',
+            secret: 'hunter2'
+          }
+        end
+
+        it { is_expected.to compile.and_raise_error(%r{either `source_socket_file` or `source_host` and `source_port must be specified}) }
+      end
+      context 'missing source_port' do
+        let(:params) do
+          {
+            source_host: '0.0.0.0',
+            target_socket_file: '/var/run/redis.sock',
+            secret: 'hunter2'
+          }
+        end
+
+        it { is_expected.to compile.and_raise_error(%r{either `source_socket_file` or `source_host` and `source_port must be specified}) }
+      end
+    end
+    describe 'target parameters' do
+      context 'target_socket_file and target_host' do
+        let(:params) do
+          {
+            source_socket_file: '/path/to/socket',
+            target_socket_file: '/var/run/redis.sock',
+            target_host: 'redis-host',
+            secret: 'hunter2'
+          }
+        end
+
+        it { is_expected.to compile.and_raise_error(%r{`target_host` must not be specified when using `target_socket_file`}) }
+      end
+      context 'target_socket_file and target_port' do
+        let(:params) do
+          {
+            source_socket_file: '/path/to/socket',
+            target_socket_file: '/var/run/redis.sock',
+            target_port: 1234,
+            secret: 'hunter2'
+          }
+        end
+
+        it { is_expected.to compile.and_raise_error(%r{`target_port` must not be specified when using `target_socket_file`}) }
+      end
+      context 'missing target_host' do
+        let(:params) do
+          {
+            source_socket_file: '/path/to/socket',
+            target_port: 1234,
+            secret: 'hunter2'
+          }
+        end
+
+        it { is_expected.to compile.and_raise_error(%r{either `target_socket_file` or `target_host` and `target_port must be specified}) }
+      end
+      context 'missing target_port' do
+        let(:params) do
+          {
+            source_socket_file: '/path/to/socket',
+            target_host: 'redis-host',
+            secret: 'hunter2'
+          }
+        end
+
+        it { is_expected.to compile.and_raise_error(%r{either `target_socket_file` or `target_host` and `target_port must be specified}) }
+      end
+    end
+  end
 end