(CAT-1483) - Enhancement of validation for apt::source parameters

Author: Ramesh7

REFERENCE.md

@@ -970,17 +970,18 @@ Default value: `present`
 
 ##### <a name="-apt--source--release"></a>`release`
 
-Data type: `Optional[String]`
+Data type: `Optional[String[1]]`
 
 Specifies a distribution of the Apt repository.
 
 Default value: `undef`
 
 ##### <a name="-apt--source--repos"></a>`repos`
 
-Data type: `String`
+Data type: `String[1]`
 
-Specifies a component of the Apt repository.
+Specifies a component of the Apt repository, Valid options: main, contrib, non-free, universe, multiverse and restricted.
+Default 'main'.
 
 Default value: `'main'`
 

manifests/source.pp

@@ -23,7 +23,8 @@
 #   Specifies a distribution of the Apt repository.
 #
 # @param repos
-#   Specifies a component of the Apt repository.
+#   Specifies a component of the Apt repository, Valid options: main, contrib, non-free, universe, multiverse and restricted.
+#   Default 'main'.
 #
 # @param include
 #   Configures include options. Valid options: a hash of available keys.
@@ -68,8 +69,8 @@
   Optional[String] $location                    = undef,
   String $comment                               = $name,
   String $ensure                                = present,
-  Optional[String] $release                     = undef,
-  String $repos                                 = 'main',
+  Optional[String[1]] $release                  = undef,
+  String[1] $repos                              = 'main',
   Variant[Hash] $include                        = {},
   Optional[Variant[String, Hash]] $key          = undef,
   Optional[Stdlib::AbsolutePath] $keyring       = undef,
@@ -90,6 +91,8 @@
     } else {
       fail('os.distro.codename fact not available: release parameter required')
     }
+  } elsif $release !~ Regexp(/^[a-z-]+$/) {
+    fail('release parameter must be set to a valid release name')
   } else {
     $_release = $release
   }
@@ -131,6 +134,12 @@
     }
   }
 
+  $repos.split(' ').each | $repo | {
+    if $repo !~ Regexp(/^(main|contrib|non-free|universe|multiverse|restricted)$/) {
+      fail('repos parameter must be set to a valid component name, or a comma-separated list of valid component names')
+    }
+  }
+
   $header = epp('apt/_header.epp')
 
   if $architecture {

spec/defines/source_compat_spec.rb

@@ -41,7 +41,7 @@
         'comment' => 'foo',
         'location' => 'http://debian.mirror.iweb.ca/debian/',
         'release' => 'sid',
-        'repos' => 'testing',
+        'repos' => 'main',
         'include' => { 'src' => false },
         'key' => id,
         'pin' => '10',
@@ -51,7 +51,7 @@
     end
 
     it {
-      expect(subject).to contain_apt__setting('list-my_source').with_content(%r{# foo\ndeb \[arch=x86_64 trusted=yes\] http://debian.mirror.iweb.ca/debian/ sid testing\n})
+      expect(subject).to contain_apt__setting('list-my_source').with_content(%r{# foo\ndeb \[arch=x86_64 trusted=yes\] http://debian.mirror.iweb.ca/debian/ sid main\n})
                                                                .without_content(%r{deb-src})
     }
 

spec/defines/source_spec.rb

@@ -65,13 +65,39 @@
       }
     end
 
+    context 'with invalid repos' do
+      let :params do
+        {
+          comment: 'foo',
+          location: 'http://debian.mirror.iweb.ca/debian/',
+          release: 'sid',
+          repos: 'test',
+        }
+      end
+
+      it { is_expected.to raise_error(Puppet::Error, %r{repos parameter must be set to a valid component name, or a comma-separated list of valid component names}) }
+    end
+
+    context 'with empty string for repos' do
+      let :params do
+        {
+          comment: 'foo',
+          location: 'http://debian.mirror.iweb.ca/debian/',
+          release: 'sid',
+          repos: '',
+        }
+      end
+
+      it { is_expected.to raise_error(Puppet::Error, %r{parameter 'repos' expects a String\[1\] value, got String}) }
+    end
+
     context 'with simple key' do
       let :params do
         {
           comment: 'foo',
           location: 'http://debian.mirror.iweb.ca/debian/',
           release: 'sid',
-          repos: 'testing',
+          repos: 'main',
           key: id,
           pin: '10',
           architecture: 'x86_64',
@@ -80,7 +106,7 @@
       end
 
       it {
-        expect(subject).to contain_apt__setting('list-my_source').with(ensure: 'present').with_content(%r{# foo\ndeb \[arch=x86_64 trusted=yes\] http://debian.mirror.iweb.ca/debian/ sid testing\n})
+        expect(subject).to contain_apt__setting('list-my_source').with(ensure: 'present').with_content(%r{# foo\ndeb \[arch=x86_64 trusted=yes\] http://debian.mirror.iweb.ca/debian/ sid main\n})
                                                                  .without_content(%r{deb-src})
       }
 
@@ -102,7 +128,7 @@
           comment: 'foo',
           location: 'http://debian.mirror.iweb.ca/debian/',
           release: 'sid',
-          repos: 'testing',
+          repos: 'main',
           key: {
             'ensure' => 'refreshed',
             'id' => id,
@@ -118,7 +144,7 @@
       end
 
       it {
-        expect(subject).to contain_apt__setting('list-my_source').with(ensure: 'present').with_content(%r{# foo\ndeb \[arch=x86_64 trusted=yes\] http://debian.mirror.iweb.ca/debian/ sid testing\n})
+        expect(subject).to contain_apt__setting('list-my_source').with(ensure: 'present').with_content(%r{# foo\ndeb \[arch=x86_64 trusted=yes\] http://debian.mirror.iweb.ca/debian/ sid main\n})
                                                                  .without_content(%r{deb-src})
       }
 
@@ -421,10 +447,22 @@
       end
     end
 
+    context 'with release is not empty string' do
+      let(:params) { { location: 'hello.there', release: 'test' } }
+
+      it { is_expected.to contain_apt__setting('list-my_source').with_content(%r{hello\.there test main}) }
+    end
+
+    context 'with release invalid string' do
+      let(:params) { { location: 'hello.there', release: '/' } }
+
+      it { is_expected.to raise_error(Puppet::Error, %r{release parameter must be set to a valid release name}) }
+    end
+
     context 'with release is empty string' do
       let(:params) { { location: 'hello.there', release: '' } }
 
-      it { is_expected.to contain_apt__setting('list-my_source').with_content(%r{hello\.there  main}) }
+      it { is_expected.to raise_error(Puppet::Error, %r{expects a value of type Undef or String\[1\], got String}) }
     end
 
     context 'with invalid pin' do