From 3cbe24e18e071f0da9623d0752f7d9c1ee0efa5e Mon Sep 17 00:00:00 2001
From: innyso <mini.inny@gmail.com>
Date: Tue, 10 Jun 2014 00:09:28 +0100
Subject: [PATCH] Adding dash to key_server validate regex

Dashes should be allow when defining domain or url for key_server. Rspec
test cases are included to make sure no malform domain name or url are
used.
---
 lib/puppet/type/apt_key.rb               |  6 +--
 manifests/key.pp                         |  2 +-
 spec/acceptance/apt_key_provider_spec.rb | 32 ++++++++++++
 spec/defines/key_spec.rb                 | 63 +++++++++++++++++++++++-
 4 files changed, 97 insertions(+), 6 deletions(-)

diff --git a/lib/puppet/type/apt_key.rb b/lib/puppet/type/apt_key.rb
index fa7b0c6..e2cb8d9 100644
--- a/lib/puppet/type/apt_key.rb
+++ b/lib/puppet/type/apt_key.rb
@@ -60,10 +60,10 @@ Puppet::Type.newtype(:apt_key) do
   end
 
   newparam(:server) do
-    desc 'The key server to fetch the key from based on the ID.'
+    desc 'The key server to fetch the key from based on the ID. It can either be a domain name or url.'
     defaultto :'keyserver.ubuntu.com'
-    # Need to validate this, preferably through stdlib is_fqdn
-    # but still working on getting to that.
+    
+    newvalues(/\A((hkp|http|https):\/\/)?([a-z\d])([a-z\d-]{0,61}\.)+[a-z\d]+(:\d{2,4})?$/)
   end
 
   newparam(:keyserver_options) do
diff --git a/manifests/key.pp b/manifests/key.pp
index 231992b..8d3bcf0 100644
--- a/manifests/key.pp
+++ b/manifests/key.pp
@@ -69,7 +69,7 @@ define apt::key (
   }
 
   if $key_server {
-    validate_re($key_server,['\A((hkp|http|https):\/\/)?([a-z\d]{0,62}\.)+[a-z\d]+(:\d{2,4})?$'])
+    validate_re($key_server,['\A((hkp|http|https):\/\/)?([a-z\d])([a-z\d-]{0,61}\.)+[a-z\d]+(:\d{2,4})?$'])
   }
 
   if $key_options {
diff --git a/spec/acceptance/apt_key_provider_spec.rb b/spec/acceptance/apt_key_provider_spec.rb
index 9becd25..573452e 100644
--- a/spec/acceptance/apt_key_provider_spec.rb
+++ b/spec/acceptance/apt_key_provider_spec.rb
@@ -192,6 +192,22 @@ ugVIB2pi+8u84f+an4Hml4xlyijgYu05pqNvnLRyJDLd61hviLC8GYU=
       end
     end
 
+    context 'hkp://pgp.mit.edu:80' do
+      it 'works' do
+        pp = <<-EOS
+        apt_key { 'puppetlabs':
+          id     => '#{PUPPETLABS_GPG_KEY_ID}',
+          ensure => 'present',
+          server => 'hkp://pgp.mit.edu:80',
+        }
+        EOS
+
+        apply_manifest(pp, :catch_failures => true)
+        expect(apply_manifest(pp, :catch_failures => true).exit_code).to be_zero
+        shell("apt-key list | grep #{PUPPETLABS_GPG_KEY_ID}")
+      end
+    end
+
     context 'nonexistant.key.server' do
       it 'fails' do
         pp = <<-EOS
@@ -207,6 +223,22 @@ ugVIB2pi+8u84f+an4Hml4xlyijgYu05pqNvnLRyJDLd61hviLC8GYU=
         end
       end
     end
+
+    context 'key server start with dot' do
+      it 'fails' do
+        pp = <<-EOS
+        apt_key { 'puppetlabs':
+          id     => '#{PUPPETLABS_GPG_KEY_ID}',
+          ensure => 'present',
+          server => '.pgp.key.server',
+        }
+        EOS
+
+        apply_manifest(pp, :expect_failures => true) do |r|
+          expect(r.stderr).to match(/Invalid value \".pgp.key.server\"/)
+        end
+      end
+    end
   end
 
   describe 'source =>' do
diff --git a/spec/defines/key_spec.rb b/spec/defines/key_spec.rb
index c20d05d..005c95e 100644
--- a/spec/defines/key_spec.rb
+++ b/spec/defines/key_spec.rb
@@ -169,6 +169,46 @@ describe 'apt::key', :type => :define do
       	end
 			end
 
+      context "domain with dash" do
+        let(:params) do{
+          :key_server => 'p-gp.m-it.edu',
+        } end
+        it "should contain apt::key" do
+          should contain_apt__key(title).with({
+            :key        => title,
+            :ensure     => 'present',
+            :key_server => 'p-gp.m-it.edu',
+          })
+        end
+      end
+
+      context "domain begin with dash" do
+        let(:params) do{
+          :key_server => '-pgp.mit.edu',
+        } end
+        it 'fails' do
+          expect { subject } .to raise_error(/does not match/)
+        end
+      end
+
+      context "domain begin with dot" do
+        let(:params) do{
+          :key_server => '.pgp.mit.edu',
+        } end
+        it 'fails' do
+          expect { subject } .to raise_error(/does not match/)
+        end
+      end
+
+      context "domain end with dot" do
+        let(:params) do{
+          :key_server => "pgp.mit.edu.",
+        } end
+        it 'fails' do
+          expect { subject } .to raise_error(/does not match/)
+        end
+      end
+
       context "url" do
         let (:params) do{
           :key_server => 'hkp://pgp.mit.edu',
@@ -218,7 +258,7 @@ describe 'apt::key', :type => :define do
           expect { subject }.to raise_error(/does not match/)
         end
       end
-      context "malform url" do
+      context "url ending with a dot" do
         let (:params) do{
           :key_server => 'hkp://pgp.mit.edu.'
         } end
@@ -226,6 +266,26 @@ describe 'apt::key', :type => :define do
           expect { subject }.to raise_error(/does not match/)
         end
       end
+      context "url begin with a dash" do
+        let(:params) do{
+          :key_server => "hkp://-pgp.mit.edu",
+        } end
+        it 'fails' do
+          expect { subject }.to raise_error(/does not match/)
+        end
+      end
+      context "url with dash" do
+        let(:params) do{
+          :key_server => 'hkp://p-gp.m-it.edu',
+        } end
+        it "should contain apt::key" do
+          should contain_apt__key(title).with({
+            :key        => title,
+            :ensure     => 'present',
+            :key_server => 'hkp://p-gp.m-it.edu',
+          })
+        end
+      end
       context "exceed characher url" do
         let (:params) do{
           :key_server => 'hkp://pgpiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii.mit.edu'
@@ -234,7 +294,6 @@ describe 'apt::key', :type => :define do
           expect { subject }.to raise_error(/does not match/)
         end
       end
-
 		end
 
     describe 'key_options =>' do
-- 
2.32.3