Skip to content

Commit

Permalink
fix rubocop offenses, add .rubocop_todo.yml, reactivate rubocop in th…
Browse files Browse the repository at this point in the history
…e CI
  • Loading branch information
SimonHoenscheid committed Mar 1, 2023
1 parent 0b39cdb commit 07cb801
Show file tree
Hide file tree
Showing 43 changed files with 493 additions and 456 deletions.
2 changes: 0 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,3 @@ jobs:
puppet:
name: Puppet
uses: voxpupuli/gha-puppet/.github/workflows/basic.yml@v1
with:
rubocop: false
2 changes: 2 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
---
inherit_from: .rubocop_todo.yml

# Managed by modulesync - DO NOT EDIT
# https://voxpupuli.org/docs/updating-files-managed-with-modulesync/

Expand Down
23 changes: 23 additions & 0 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2023-03-01 07:21:23 UTC using RuboCop version 1.22.3.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.

# Offense count: 34
# Configuration parameters: .
# SupportedStyles: have_received, receive
RSpec/MessageSpies:
EnforcedStyle: receive

# Offense count: 17
# Configuration parameters: AllowSubject.
RSpec/MultipleMemoizedHelpers:
Max: 8

# Offense count: 9
RSpec/StubbedMock:
Exclude:
- 'spec/unit/puppet/provider/kubectl_apply_resource/kubectl_spec.rb'
4 changes: 2 additions & 2 deletions lib/puppet/functions/k8s/format_url.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ def k8s_format_binary(url, components)
kernel_ext = 'zip'
kernel_ext = 'tar.gz' if kernel == 'linux'

components = Hash[components.map { |k, v| [k.to_sym, v] }].merge(
components = components.transform_keys(&:to_sym).merge(
arch: arch,
underscore_arch_suffix: underscore_arch_suffix,
dash_arch_suffix: dash_arch_suffix,
k3s_arch: k3s_arch,
kernel: kernel,
kernel_ext: kernel_ext,
kernel_ext: kernel_ext
)

url % components
Expand Down
1 change: 1 addition & 0 deletions lib/puppet/provider/kubeconfig/ruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ def credentials_valid?

return false unless user['name'] == resource[:user]
return false unless user['user']

if resource[:client_cert]
if resource[:embed_certs] == :true
wanted = Base64.strict_encode64(File.read(resource[:client_cert]))
Expand Down
4 changes: 2 additions & 2 deletions lib/puppet/provider/kubectl_apply/file.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# frozen_string_literal: true

require File.expand_path('../../../util/k8s', __FILE__)
require File.expand_path('../../util/k8s', __dir__)

# Applies resources as YAML files on disk
Puppet::Type.type(:kubectl_apply).provide(:file) do
Expand Down Expand Up @@ -54,7 +54,7 @@ def file_get
return nil unless File.exist? resource[:file]

JSON.parse(File.read(resource[:file]))
rescue
rescue StandardError
{}
end
end
2 changes: 1 addition & 1 deletion lib/puppet/provider/kubectl_apply/kubectl.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# frozen_string_literal: true

require File.expand_path('../../../util/k8s', __FILE__)
require File.expand_path('../../util/k8s', __dir__)
require 'tempfile'

# Applies resources as data in a Kubernetes cluster
Expand Down
30 changes: 8 additions & 22 deletions lib/puppet/type/kubeconfig.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,7 @@ def retrieve
desc 'An arbitrary path used as the identity of the resource.'

validate do |value|
unless Puppet::Util.absolute_path?(value)
raise Puppet::Error, "File paths must be fully qualified, not '#{value}'"
end
raise Puppet::Error, "File paths must be fully qualified, not '#{value}'" unless Puppet::Util.absolute_path?(value)
end
end

Expand Down Expand Up @@ -105,27 +103,21 @@ def retrieve
desc 'The path to a CA certificate to include in the kubeconfig'

validate do |value|
unless Puppet::Util.absolute_path?(value)
raise Puppet::Error, "File paths must be fully qualified, not '#{value}'"
end
raise Puppet::Error, "File paths must be fully qualified, not '#{value}'" unless Puppet::Util.absolute_path?(value)
end
end
newparam(:client_cert) do
desc 'The path to a client certificate to include in the kubeconfig'

validate do |value|
unless Puppet::Util.absolute_path?(value)
raise Puppet::Error, "File paths must be fully qualified, not '#{value}'"
end
raise Puppet::Error, "File paths must be fully qualified, not '#{value}'" unless Puppet::Util.absolute_path?(value)
end
end
newparam(:client_key) do
desc 'The path to a client key to include in the kubeconfig'

validate do |value|
unless Puppet::Util.absolute_path?(value)
raise Puppet::Error, "File paths must be fully qualified, not '#{value}'"
end
raise Puppet::Error, "File paths must be fully qualified, not '#{value}'" unless Puppet::Util.absolute_path?(value)
end
end

Expand All @@ -137,9 +129,7 @@ def retrieve
desc 'The path to a file containing an authentication token'

validate do |value|
unless Puppet::Util.absolute_path?(value)
raise Puppet::Error, "File paths must be fully qualified, not '#{value}'"
end
raise Puppet::Error, "File paths must be fully qualified, not '#{value}'" unless Puppet::Util.absolute_path?(value)
end
end

Expand All @@ -151,12 +141,8 @@ def retrieve
end

validate do
if self[:token] && self[:token_file]
raise Puppet::Error, "Can't specify both token and token_file for the same kubeconfig entry"
end
unless self[:path]
raise(Puppet::Error, 'path is a required attribute')
end
raise Puppet::Error, "Can't specify both token and token_file for the same kubeconfig entry" if self[:token] && self[:token_file]
raise(Puppet::Error, 'path is a required attribute') unless self[:path]
end

# Ensure the file or directory exists
Expand All @@ -172,7 +158,7 @@ def retrieve
self[:client_key],
].compact
end
req += [ self[:token_file] ].compact
req += [self[:token_file]].compact

req
end
Expand Down
30 changes: 9 additions & 21 deletions lib/puppet/type/kubectl_apply.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,7 @@ def retrieve
desc 'The name of the resource'

validate do |value|
unless value.match? %r{^([a-z0-9][a-z0-9.:-]{0,251}[a-z0-9]|[a-z0-9])$}
raise Puppet::Error, 'Resource name must be valid'
end
raise Puppet::Error, 'Resource name must be valid' unless value.match? %r{^([a-z0-9][a-z0-9.:-]{0,251}[a-z0-9]|[a-z0-9])$}
end
end

Expand All @@ -86,28 +84,22 @@ def retrieve
desc 'The namespace the resource is contained in'

validate do |value|
unless value.match? %r{^[a-z0-9.-]{0,253}$}
raise Puppet::Error, 'Namespace must be valid'
end
raise Puppet::Error, 'Namespace must be valid' unless value.match? %r{^[a-z0-9.-]{0,253}$}
end
end

newparam(:kubeconfig) do
desc 'The kubeconfig file to use for handling the resource'

validate do |value|
unless Puppet::Util.absolute_path?(value)
raise Puppet::Error, 'Kubeconfig path must be fully qualified'
end
raise Puppet::Error, 'Kubeconfig path must be fully qualified' unless Puppet::Util.absolute_path?(value)
end
end
newparam(:file) do
desc 'The local file for the resource'

validate do |value|
unless Puppet::Util.absolute_path?(value)
raise Puppet::Error, 'File path must be fully qualified'
end
raise Puppet::Error, 'File path must be fully qualified' unless Puppet::Util.absolute_path?(value)
end
end

Expand Down Expand Up @@ -139,13 +131,9 @@ def retrieve
defaultto({})

validate do |value|
unless value.is_a? Hash
raise Puppet::Error, 'Content must be a valid content hash'
end
raise Puppet::Error, 'Content must be a valid content hash' unless value.is_a? Hash

if ['apiVersion', 'kind'].any? { |key| value.key? key }
raise Puppet::Error, "Can't specify apiVersion or kind in content"
end
raise Puppet::Error, "Can't specify apiVersion or kind in content" if %w[apiVersion kind].any? { |key| value.key? key }
end
end

Expand All @@ -157,10 +145,10 @@ def retrieve
end

autorequire(:kubeconfig) do
[ self[:kubeconfig] ]
[self[:kubeconfig]]
end
autorequire(:service) do
[ 'k8s-apiserver' ]
['k8s-apiserver']
end
autorequire(:file) do
[
Expand All @@ -169,7 +157,7 @@ def retrieve
]
end
autorequire(:k8s__binary) do
[ 'kubectl' ]
['kubectl']
end

def nice_name
Expand Down
8 changes: 3 additions & 5 deletions lib/puppet/util/k8s.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ def self.content_diff(local, content)
next if value.include? v

if v.is_a? Hash
diff ||= value.select { |ov| ov.is_a? Hash }
.none? do |ov|
diff ||= value.select { |ov| ov.is_a? Hash }.
none? do |ov|
v_copy = Marshal.load(Marshal.dump(v))
delete_merge.call(v_copy, ov)

Expand All @@ -43,9 +43,7 @@ def self.content_diff(local, content)
hash1.delete(key) if hash1.key?(key) && (hash1[key].nil? || (hash1[key].respond_to?(:empty?) && hash1[key].empty?))
end
hash1.dup.each_pair do |key, value|
if value.nil? && !hash2.key?(key)
hash1.delete key
end
hash1.delete key if value.nil? && !hash2.key?(key)
end

hash1
Expand Down
4 changes: 2 additions & 2 deletions spec/classes/k8s_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

it { is_expected.to compile }

[ 'node', 'server' ].each do |role|
%w[node server].each do |role|
context "with role #{role}" do
let(:params) do
{
Expand All @@ -24,7 +24,7 @@
context 'With dual-stack' do
it { is_expected.to compile }

[ 'node', 'server' ].each do |role|
%w[node server].each do |role|
context "with role #{role}" do
let(:params) do
{
Expand Down
38 changes: 21 additions & 17 deletions spec/classes/node/kube_proxy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
describe 'k8s::node::kube_proxy' do
let(:pre_condition) do
<<~PUPPET
function assert_private() {}
function assert_private() {}
include ::k8s
class { '::k8s::node':
manage_kubelet => false,
manage_proxy => false,
}
include ::k8s
class { '::k8s::node':
manage_kubelet => false,
manage_proxy => false,
}
PUPPET
end

Expand All @@ -20,13 +20,15 @@ class { '::k8s::node':
let(:facts) { os_facts }

it { is_expected.to compile }

it do
sysconf = '/etc/sysconfig'
sysconf = '/etc/default' if os_facts['os']['family'] == 'Debian'

is_expected.to contain_file(File.join(sysconf, 'kube-proxy'))
.that_notifies('Service[kube-proxy]')
is_expected.to contain_file(File.join(sysconf, 'kube-proxy')).
that_notifies('Service[kube-proxy]')
end

it { is_expected.to contain_systemd__unit_file('kube-proxy.service').that_notifies('Service[kube-proxy]') }
it { is_expected.to contain_service('kube-proxy') }

Expand All @@ -41,11 +43,11 @@ class { '::k8s::node':
end

it do
is_expected.to contain_kubeconfig('/srv/kubernetes/kube-proxy.kubeconf')
.with_ca_cert('/tmp/ca.pem')
.with_client_cert('/tmp/cert.pem')
.with_client_key('/tmp/key.pem')
.that_notifies('Service[kube-proxy]')
is_expected.to contain_kubeconfig('/srv/kubernetes/kube-proxy.kubeconf').
with_ca_cert('/tmp/ca.pem').
with_client_cert('/tmp/cert.pem').
with_client_key('/tmp/key.pem').
that_notifies('Service[kube-proxy]')
end
end

Expand All @@ -59,10 +61,10 @@ class { '::k8s::node':
end

it do
is_expected.to contain_kubeconfig('/srv/kubernetes/kube-proxy.kubeconf')
.with_ca_cert('/tmp/ca.pem')
.with_token('blah')
.that_notifies('Service[kube-proxy]')
is_expected.to contain_kubeconfig('/srv/kubernetes/kube-proxy.kubeconf').
with_ca_cert('/tmp/ca.pem').
with_token('blah').
that_notifies('Service[kube-proxy]')
end
end

Expand All @@ -75,12 +77,14 @@ class { '::k8s::node':

it { is_expected.not_to contain_kubeconfig('/srv/kubernetes/kube-proxy.kubeconf') }
it { is_expected.to contain_k8s__binary('kube-proxy').with_ensure('absent') }

it do
sysconf = '/etc/sysconfig'
sysconf = '/etc/default' if os_facts['os']['family'] == 'Debian'

is_expected.to contain_file(File.join(sysconf, 'kube-proxy')).with_ensure('absent')
end

it { is_expected.to contain_systemd__unit_file('kube-proxy.service').with_ensure('absent') }
it { is_expected.to contain_service('kube-proxy').with_ensure('stopped').with_enable(false) }
end
Expand Down
12 changes: 6 additions & 6 deletions spec/classes/node/kubectl_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
describe 'k8s::node::kubectl' do
let(:pre_condition) do
<<~PUPPET
function assert_private() {}
function assert_private() {}
include ::k8s
class { '::k8s::node':
manage_kubelet => false,
manage_proxy => false,
}
include ::k8s
class { '::k8s::node':
manage_kubelet => false,
manage_proxy => false,
}
PUPPET
end

Expand Down
Loading

0 comments on commit 07cb801

Please sign in to comment.