Merge pull request #602 from deric/namespace

Validate namespace parameter as DNS subdomain name

Author: chelnak

manifests/wait_for_default_sa.pp

@@ -1,18 +1,21 @@
 # == kubernetes::wait_for_default_sa
+#
+# @param namespace
+#  Namespace name must be a valid DNS name (max. 63 characters)
+#  see https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/#namespaces-and-dns
+#
 define kubernetes::wait_for_default_sa (
-  String $namespace            = $title,
-  Array $path                  = $kubernetes::default_path,
-  Optional[Integer] $timeout   = undef,
-  Optional[Integer] $tries     = $kubernetes::wait_for_default_sa_tries,
-  Optional[Integer] $try_sleep = $kubernetes::wait_for_default_sa_try_sleep,
-  Optional[Array] $env         = $kubernetes::environment,
+  Kubernetes::Namespace $namespace = $title,
+  Array $path                      = $kubernetes::default_path,
+  Optional[Integer] $timeout       = undef,
+  Optional[Integer] $tries         = $kubernetes::wait_for_default_sa_tries,
+  Optional[Integer] $try_sleep     = $kubernetes::wait_for_default_sa_try_sleep,
+  Optional[Array] $env             = $kubernetes::environment,
 ) {
-  $safe_namespace = shell_escape($namespace)
-
   # This prevents a known race condition https://github.com/kubernetes/kubernetes/issues/66689
-  exec { "wait for default serviceaccount creation in ${safe_namespace}":
-    command     => "kubectl -n ${safe_namespace} get serviceaccount default -o name",
-    unless      => ["kubectl -n ${safe_namespace} get serviceaccount default -o name"],
+  exec { "wait for default serviceaccount creation in ${namespace}":
+    command     => "kubectl -n ${namespace} get serviceaccount default -o name",
+    unless      => "kubectl -n ${namespace} get serviceaccount default -o name",
     path        => $path,
     environment => $env,
     timeout     => $timeout,

spec/defines/wait_for_default_sa_spec.rb

@@ -42,4 +42,41 @@
           .with_command('kubectl -n foo get serviceaccount default -o name')
           .with_path(['/bar'])}
   end
+
+  describe 'namespace naming' do
+    tests = [
+      ['01010', true],
+      ['abc', true],
+      ['A0c', true],
+      ['A0c-', false],
+      ['-A0c', false],
+      ['A-0c', true],
+      ['o123456701234567012345670123456701234567012345670123456701234567', false],
+      ['o12345670123456701234567012345670123456701234567012345670123456', true],
+      ['', false],
+      ['a', true],
+      ['0--0', true],
+      ["A0c\nA0c", false],
+      ['host;rm -rf /', false]
+    ]
+
+    tests.each do |namespace, expected|
+      context "with namespace #{namespace}" do
+        let(:params) do
+          {
+            'namespace' => namespace,
+          }
+        end
+
+        if expected
+          it { is_expected.to compile.with_all_deps }
+          it { is_expected.to contain_exec("wait for default serviceaccount creation in #{namespace}")
+                .with_command("kubectl -n #{namespace} get serviceaccount default -o name")
+          }
+        else
+          it { is_expected.to raise_error(/parameter 'namespace' expects a match for Kubernetes::Namespace/) }
+        end
+      end
+    end
+  end
 end

types/namespace.pp

@@ -0,0 +1,3 @@
+# namespace should conform to RFC 1123
+# source https://stackoverflow.com/a/20945961/334831
+type Kubernetes::Namespace = Pattern['\A(?!-)[a-zA-Z0-9-]{1,63}(?<!-)\z']