From 9444c1238a98f1f0cdc5b7f79e8c7d7c0a476f3f Mon Sep 17 00:00:00 2001 From: Craig Gumbley Date: Fri, 9 Dec 2022 21:24:44 +0000 Subject: [PATCH 1/3] (MAINT) Revert hardening changes --- manifests/kube_addons.pp | 8 +++----- manifests/kubeadm_init.pp | 8 ++++---- manifests/kubeadm_join.pp | 7 ++----- manifests/packages.pp | 7 ++----- manifests/service.pp | 6 ++---- manifests/wait_for_default_sa.pp | 6 ++++-- 6 files changed, 17 insertions(+), 25 deletions(-) diff --git a/manifests/kube_addons.pp b/manifests/kube_addons.pp index a050a1fe..a0d5a15c 100644 --- a/manifests/kube_addons.pp +++ b/manifests/kube_addons.pp @@ -91,12 +91,10 @@ } if $schedule_on_controller { - $schedule_command = ['kubectl', 'taint', 'nodes', $node_name, 'node-role.kubernetes.io/master-'] - $schedule_onlyif = "kubectl describe nodes ${node_name} | tr -s ' ' | grep 'Taints: node-role.kubernetes.io/master:NoSchedule'" - + $safe_node_name = shell_escape($node_name) exec { 'schedule on controller': - command => $schedule_command, - onlyif => $schedule_onlyif, + command => "kubectl taint nodes ${safe_node_name} node-role.kubernetes.io/master-", + onlyif => "kubectl describe nodes ${safe_node_name} | tr -s ' ' | grep 'Taints: node-role.kubernetes.io/master:NoSchedule'", } } diff --git a/manifests/kubeadm_init.pp b/manifests/kubeadm_init.pp index 8d81a5b3..0c8352d2 100644 --- a/manifests/kubeadm_init.pp +++ b/manifests/kubeadm_init.pp @@ -15,15 +15,15 @@ skip_phases => $skip_phases, }) - $exec_init = ['kubeadm', 'init', $kubeadm_init_flags] - $unless_init = "kubectl get nodes | grep ${node_name}" + $safe_node_name = shell_escape($node_name) + exec { 'kubeadm init': - command => $exec_init, + command => "kubeadm init ${kubeadm_init_flags}", environment => $env, path => $path, logoutput => true, timeout => 0, - unless => $unless_init, + unless => "kubectl get nodes | grep ${safe_node_name}", } # This prevents a known race condition https://github.com/kubernetes/kubernetes/issues/66689 diff --git a/manifests/kubeadm_join.pp b/manifests/kubeadm_join.pp index a2d25c89..030d8edf 100644 --- a/manifests/kubeadm_join.pp +++ b/manifests/kubeadm_join.pp @@ -43,15 +43,12 @@ } } - $exec_join = ['kubeadm', 'join', $kubeadm_join_flags] - $unless_join = "kubectl get nodes | grep ${node_name}" - exec { 'kubeadm join': - command => $exec_join, + command => "kubeadm join ${kubeadm_join_flags}", environment => $env, path => $path, logoutput => true, timeout => 0, - unless => $unless_join, + unless => "kubectl get nodes | grep ${node_name}", } } diff --git a/manifests/packages.pp b/manifests/packages.pp index 3f14b9df..e54371c4 100644 --- a/manifests/packages.pp +++ b/manifests/packages.pp @@ -60,13 +60,10 @@ $kube_packages = ['kubelet', 'kubectl', 'kubeadm'] if $disable_swap { - $command = ['swapoff', '-a'] - $unless = [['awk', '"{ if (NR > 1) exit 1}"', '/proc/swaps']] - exec { 'disable swap': path => ['/usr/sbin/', '/usr/bin', '/bin', '/sbin'], - command => $command, - unless => $unless, + command => 'swapoff -a', + unless => "awk '{ if (NR > 1) exit 1}' /proc/swaps", } file_line { 'remove swap in /etc/fstab': ensure => absent, diff --git a/manifests/service.pp b/manifests/service.pp index 6418d382..7479dfb0 100644 --- a/manifests/service.pp +++ b/manifests/service.pp @@ -20,11 +20,9 @@ ensure => directory, } - $exec_reload = ['systemctl', 'daemon-reload'] - exec { 'kubernetes-systemd-reload': path => '/bin', - command => $exec_reload, + command =>'systemctl daemon-reload', refreshonly => true, } @@ -107,7 +105,7 @@ if $etcd_install_method == 'wget' { exec { 'systemctl-daemon-reload-etcd': path => '/usr/bin:/bin:/usr/sbin:/sbin', - command => $exec_reload, + command => 'systemctl daemon-reload', refreshonly => true, subscribe => File['/etc/systemd/system/etcd.service'], notify => Service['etcd'], diff --git a/manifests/wait_for_default_sa.pp b/manifests/wait_for_default_sa.pp index c6b7f43d..14710447 100644 --- a/manifests/wait_for_default_sa.pp +++ b/manifests/wait_for_default_sa.pp @@ -10,8 +10,10 @@ $safe_namespace = shell_escape($namespace) # This prevents a known race condition https://github.com/kubernetes/kubernetes/issues/66689 - $cmd = ['kubectl', '-n', $safe_namespace, 'get', 'serviceaccount', 'default', '-o', 'name'] - $unless_cmd = [['kubectl', '-n', $safe_namespace, 'get', 'serviceaccount', 'default', '-o', 'name']] + $cmd = "kubectl -n ${safe_namespace} get serviceaccount default -o name" + $unless_cmd = [ + "kubectl -n ${safe_namespace} get serviceaccount default -o name" + ] exec { "wait for default serviceaccount creation in ${safe_namespace}": command => $cmd, From fe6c101c2c64ccf0722343024217cda8ffe64112 Mon Sep 17 00:00:00 2001 From: Craig Gumbley Date: Mon, 12 Dec 2022 10:55:33 +0000 Subject: [PATCH 2/3] (MAINT) Remove additional variables This commit builds on the work that @deric submitted in a previous pr. In a few other manifests there were variables set that added no value. They have been reduced back to literal entries in the exec resources. This commit applies those changes to this PR to keep things consistent. --- manifests/kube_addons.pp | 47 ++++++++---------------- manifests/service.pp | 2 +- manifests/wait_for_default_sa.pp | 9 +---- spec/classes/kube_addons_spec.rb | 2 +- spec/defines/kubeadm_init_spec.rb | 6 +-- spec/defines/kubeadm_join_spec.rb | 8 ++-- spec/defines/wait_for_default_sa_spec.rb | 4 +- 7 files changed, 28 insertions(+), 50 deletions(-) diff --git a/manifests/kube_addons.pp b/manifests/kube_addons.pp index a0d5a15c..83742d41 100644 --- a/manifests/kube_addons.pp +++ b/manifests/kube_addons.pp @@ -27,37 +27,26 @@ $exec_onlyif = 'kubectl get nodes' if $cni_rbac_binding { - $binding_command = ['kubectl', 'apply', '-f', $cni_rbac_binding] - $binding_unless = 'kubectl get clusterrole | grep calico' - exec { 'Install calico rbac bindings': environment => $env, - command => $binding_command, + command => ['kubectl', 'apply', '-f', $cni_rbac_binding], onlyif => $exec_onlyif, - unless => $binding_unless, + unless => 'kubectl get clusterrole | grep calico', } } if $cni_network_provider { if $cni_provider == 'calico-tigera' { if $cni_network_preinstall { - $preinstall_command = ['kubectl', 'apply', '-f', $cni_network_preinstall] - $preinstall_unless = 'kubectl -n tigera-operator get deployments | egrep "^tigera-operator"' - exec { 'Install cni network (preinstall)': - command => $preinstall_command, + command => ['kubectl', 'apply', '-f', $cni_network_preinstall], onlyif => $exec_onlyif, - unless => $preinstall_unless, + unless => 'kubectl -n tigera-operator get deployments | egrep "^tigera-operator"', environment => $env, before => Exec['Install cni network provider'], } } - # Removing Calico_installation_path variable as it doesnt seem to apport any extra value here. - $calico_installation_path = '/etc/kubernetes/calico-installation.yaml' - $path_command = 'kubectl apply -f /etc/kubernetes/calico-installation.yaml' - $path_unless = 'kubectl -n calico-system get daemonset | egrep "^calico-node"' - - file { $calico_installation_path: + file { '/etc/kubernetes/calico-installation.yaml': ensure => 'present', group => 'root', mode => '0400', @@ -66,25 +55,22 @@ source => $cni_network_provider, } -> file_line { 'Configure calico ipPools.cidr': ensure => present, - path => $calico_installation_path, + path => '/etc/kubernetes/calico-installation.yaml', match => ' cidr:', line => " cidr: ${cni_pod_cidr}", multiple => false, replace => true, } -> exec { 'Install cni network provider': - command => $path_command, + command => 'kubectl apply -f /etc/kubernetes/calico-installation.yaml', onlyif => $exec_onlyif, - unless => $path_unless, + unless => 'kubectl -n calico-system get daemonset | egrep "^calico-node"', environment => $env, } } else { - $provider_command = ['kubectl', 'apply', '-f', $cni_network_provider] - $provider_unless = 'kubectl -n kube-system get daemonset | egrep "(flannel|weave|calico-node|cilium)"' - exec { 'Install cni network provider': - command => $provider_command, + command => ['kubectl', 'apply', '-f', $cni_network_provider], onlyif => $exec_onlyif, - unless => $provider_unless, + unless => 'kubectl -n kube-system get daemonset | egrep "(flannel|weave|calico-node|cilium)"', environment => $env, } } @@ -99,16 +85,13 @@ } if $install_dashboard { - $dashboard_command = ['kubectl', 'apply', '-f', $dashboard_url] - $dashboard_unless = [ - 'kubectl get pods --field-selector="status.phase=Running" -n kubernetes-dashboard | grep kubernetes-dashboard-', - 'kubectl get pods --field-selector="status.phase=Running" -n kube-system | grep kubernetes-dashboard-' - ] - exec { 'Install Kubernetes dashboard': - command => $dashboard_command, + command => ['kubectl', 'apply', '-f', $dashboard_url], onlyif => $exec_onlyif, - unless => $dashboard_unless, + unless => [ + 'kubectl get pods --field-selector="status.phase=Running" -n kubernetes-dashboard | grep kubernetes-dashboard-', + 'kubectl get pods --field-selector="status.phase=Running" -n kube-system | grep kubernetes-dashboard-', + ], environment => $env, } } diff --git a/manifests/service.pp b/manifests/service.pp index 7479dfb0..507acf58 100644 --- a/manifests/service.pp +++ b/manifests/service.pp @@ -22,7 +22,7 @@ exec { 'kubernetes-systemd-reload': path => '/bin', - command =>'systemctl daemon-reload', + command => 'systemctl daemon-reload', refreshonly => true, } diff --git a/manifests/wait_for_default_sa.pp b/manifests/wait_for_default_sa.pp index 14710447..614481bf 100644 --- a/manifests/wait_for_default_sa.pp +++ b/manifests/wait_for_default_sa.pp @@ -10,14 +10,9 @@ $safe_namespace = shell_escape($namespace) # This prevents a known race condition https://github.com/kubernetes/kubernetes/issues/66689 - $cmd = "kubectl -n ${safe_namespace} get serviceaccount default -o name" - $unless_cmd = [ - "kubectl -n ${safe_namespace} get serviceaccount default -o name" - ] - exec { "wait for default serviceaccount creation in ${safe_namespace}": - command => $cmd, - unless => $unless_cmd, + command => "kubectl -n ${safe_namespace} get serviceaccount default -o name", + unless => ["kubectl -n ${safe_namespace} get serviceaccount default -o name"], path => $path, environment => $env, timeout => $timeout, diff --git a/spec/classes/kube_addons_spec.rb b/spec/classes/kube_addons_spec.rb index c3f13343..ad67817b 100644 --- a/spec/classes/kube_addons_spec.rb +++ b/spec/classes/kube_addons_spec.rb @@ -64,7 +64,7 @@ when 'calico-tigera' it { is_expected.to contain_exec('Install cni network (preinstall)').with({ 'command': ['kubectl', 'apply', '-f', 'https://foo.test/tigera-operator'], - 'onlyif': ['kubectl get nodes'], + 'onlyif': 'kubectl get nodes', }) } it { is_expected.to contain_file('/etc/kubernetes/calico-installation.yaml')} diff --git a/spec/defines/kubeadm_init_spec.rb b/spec/defines/kubeadm_init_spec.rb index 821756e9..1fcdddde 100644 --- a/spec/defines/kubeadm_init_spec.rb +++ b/spec/defines/kubeadm_init_spec.rb @@ -29,7 +29,7 @@ } end it { is_expected.to compile.with_all_deps } - it { is_expected.to contain_exec('kubeadm init').with_command(["kubeadm", "init", "--config '/etc/kubernetes/config.yaml'"])} + it { is_expected.to contain_exec('kubeadm init').with_command("kubeadm init --config '/etc/kubernetes/config.yaml'")} it { is_expected.to contain_kubernetes__wait_for_default_sa('default')} end @@ -44,7 +44,7 @@ } end it { is_expected.to compile.with_all_deps } - it { is_expected.to contain_exec('kubeadm init').with_command(["kubeadm", "init", "--config '/etc/kubernetes/config.yaml' --dry-run"])} + it { is_expected.to contain_exec('kubeadm init').with_command("kubeadm init --config '/etc/kubernetes/config.yaml' --dry-run")} it { is_expected.to contain_kubernetes__wait_for_default_sa('default')} end @@ -59,7 +59,7 @@ } end it { is_expected.to compile.with_all_deps } - it { is_expected.to contain_exec('kubeadm init').with_command(["kubeadm", "init", "--config '/etc/kubernetes/config.yaml' --ignore-preflight-errors='foo,bar'"])} + it { is_expected.to contain_exec('kubeadm init').with_command("kubeadm init --config '/etc/kubernetes/config.yaml' --ignore-preflight-errors='foo,bar'")} it { is_expected.to contain_kubernetes__wait_for_default_sa('default')} end end diff --git a/spec/defines/kubeadm_join_spec.rb b/spec/defines/kubeadm_join_spec.rb index 856567b7..329f422f 100644 --- a/spec/defines/kubeadm_join_spec.rb +++ b/spec/defines/kubeadm_join_spec.rb @@ -39,7 +39,7 @@ end it { is_expected.to compile.with_all_deps } - it { is_expected.to contain_exec('kubeadm join').with_command(["kubeadm", "join", "'10.0.0.1:6443' --discovery-token 'token' --discovery-token-ca-cert-hash 'sha256:hash' --node-name 'kube-node' --token 'token'"])} + it { is_expected.to contain_exec('kubeadm join').with_command("kubeadm join '10.0.0.1:6443' --discovery-token 'token' --discovery-token-ca-cert-hash 'sha256:hash' --node-name 'kube-node' --token 'token'")} end context 'with kubernetes_version => 1.12.3 and controller_address => 10.0.0.1:6443' do @@ -48,7 +48,7 @@ end it { is_expected.to compile.with_all_deps } - it { is_expected.to contain_exec('kubeadm join').with_command(["kubeadm", "join", "--config '/etc/kubernetes/config.yaml'"])} + it { is_expected.to contain_exec('kubeadm join').with_command("kubeadm join --config '/etc/kubernetes/config.yaml'")} end context 'with kubernetes_version => 1.12.3 and ignore_preflight_errors => [foo, bar]' do @@ -60,7 +60,7 @@ end it { is_expected.to compile.with_all_deps } - it { is_expected.to contain_exec('kubeadm join').with_command(["kubeadm", "join", "--config '/etc/kubernetes/config.yaml' --ignore-preflight-errors 'foo,bar'"])} + it { is_expected.to contain_exec('kubeadm join').with_command("kubeadm join --config '/etc/kubernetes/config.yaml' --ignore-preflight-errors 'foo,bar'")} end context 'with kubernetes_version => 1.12.3 and discovery_file => /etc/kubernetes/admin.conf' do @@ -72,6 +72,6 @@ end it { is_expected.to compile.with_all_deps } - it { is_expected.to contain_exec('kubeadm join').with_command(["kubeadm", "join", "--discovery-file '/etc/kubernetes/admin.conf'"])} + it { is_expected.to contain_exec('kubeadm join').with_command("kubeadm join --discovery-file '/etc/kubernetes/admin.conf'")} end end diff --git a/spec/defines/wait_for_default_sa_spec.rb b/spec/defines/wait_for_default_sa_spec.rb index 9bbf3743..3bb8f760 100644 --- a/spec/defines/wait_for_default_sa_spec.rb +++ b/spec/defines/wait_for_default_sa_spec.rb @@ -27,7 +27,7 @@ end it { is_expected.to compile.with_all_deps } it { is_expected.to contain_exec('wait for default serviceaccount creation in default') - .with_command(['kubectl', '-n', 'default', 'get', 'serviceaccount', 'default', '-o', 'name'])} + .with_command('kubectl -n default get serviceaccount default -o name')} end context 'with namespace foo and path /bar' do @@ -39,7 +39,7 @@ end it { is_expected.to compile.with_all_deps } it { is_expected.to contain_exec('wait for default serviceaccount creation in foo') - .with_command(['kubectl', '-n', 'foo', 'get', 'serviceaccount', 'default', '-o', 'name']) + .with_command('kubectl -n foo get serviceaccount default -o name') .with_path(['/bar'])} end end From 4bdf2a86ce581b6709569df7e4327f8906aa717b Mon Sep 17 00:00:00 2001 From: Craig Gumbley Date: Mon, 12 Dec 2022 12:44:58 +0000 Subject: [PATCH 3/3] (MAINT) Remove safe_node_name --- manifests/kube_addons.pp | 5 ++--- manifests/kubeadm_init.pp | 4 +--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/manifests/kube_addons.pp b/manifests/kube_addons.pp index 83742d41..6d0684f9 100644 --- a/manifests/kube_addons.pp +++ b/manifests/kube_addons.pp @@ -77,10 +77,9 @@ } if $schedule_on_controller { - $safe_node_name = shell_escape($node_name) exec { 'schedule on controller': - command => "kubectl taint nodes ${safe_node_name} node-role.kubernetes.io/master-", - onlyif => "kubectl describe nodes ${safe_node_name} | tr -s ' ' | grep 'Taints: node-role.kubernetes.io/master:NoSchedule'", + command => "kubectl taint nodes ${node_name} node-role.kubernetes.io/master-", + onlyif => "kubectl describe nodes ${node_name} | tr -s ' ' | grep 'Taints: node-role.kubernetes.io/master:NoSchedule'", } } diff --git a/manifests/kubeadm_init.pp b/manifests/kubeadm_init.pp index 0c8352d2..a2172d1a 100644 --- a/manifests/kubeadm_init.pp +++ b/manifests/kubeadm_init.pp @@ -15,15 +15,13 @@ skip_phases => $skip_phases, }) - $safe_node_name = shell_escape($node_name) - exec { 'kubeadm init': command => "kubeadm init ${kubeadm_init_flags}", environment => $env, path => $path, logoutput => true, timeout => 0, - unless => "kubectl get nodes | grep ${safe_node_name}", + unless => "kubectl get nodes | grep ${node_name}", } # This prevents a known race condition https://github.com/kubernetes/kubernetes/issues/66689