From 191b498e65e5caef30b77fa2ad91bc77b5106ce8 Mon Sep 17 00:00:00 2001 From: david22swan Date: Tue, 30 Sep 2025 11:17:15 +0100 Subject: [PATCH 1/5] (CAT-2453) Bump Rubocop versions in order to remove rexml The version of rexml brought in by the current rubocop versions has a security vulnerability. Updating to these rubocop versions will remove their dependency entirely. --- Gemfile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index d9c2169b..84454d98 100644 --- a/Gemfile +++ b/Gemfile @@ -19,9 +19,9 @@ group :tests do # since the Resource API runs inside the puppetserver, test against the JRuby versions we ship # these require special dependencies to have everything load properly # rubocop 1.48 supports JRuby 9.3+, which includes coverage for versions we support - gem 'rubocop', '~> 1.70.0', require: false - gem 'rubocop-rspec', '~> 2.20.0', require: false - gem 'rubocop-performance', '~> 1.17.1', require: false + gem 'rubocop', '~> 1.73.0', require: false + gem 'rubocop-rspec', '~> 3.5.0', require: false + gem 'rubocop-performance', '~> 1.24.0', require: false end group :development do From b22210faa76376d117174d5720cd973c53029afe Mon Sep 17 00:00:00 2001 From: david22swan Date: Tue, 30 Sep 2025 11:23:07 +0100 Subject: [PATCH 2/5] (RUBOCOP) Autocorrect changes --- spec/puppet/resource_api/glue_spec.rb | 2 +- .../resource_api/transport/wrapper_spec.rb | 2 +- spec/puppet/resource_api/transport_spec.rb | 4 +- spec/puppet/resource_api_spec.rb | 38 +++++++++---------- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/spec/puppet/resource_api/glue_spec.rb b/spec/puppet/resource_api/glue_spec.rb index 1b1c5fba..21c5665a 100644 --- a/spec/puppet/resource_api/glue_spec.rb +++ b/spec/puppet/resource_api/glue_spec.rb @@ -136,7 +136,7 @@ it { expect(value <=> 'string').to eq(-1) } # Avoid this test on jruby 1.7, where it is hitting a implementation inconsistency and `'string' <=> value` returns `nil` - it('compares to a string', j17_exclude: true) { expect('string' <=> value).to eq 1 } + it('compares to a string', :j17_exclude) { expect('string' <=> value).to eq 1 } it { expect(value <=> described_class[b: 'b', c: 'c']).to eq 0 } it { expect(value <=> described_class[d: 'd']).to eq(-1) } it { expect(value <=> described_class[a: 'a']).to eq 1 } diff --git a/spec/puppet/resource_api/transport/wrapper_spec.rb b/spec/puppet/resource_api/transport/wrapper_spec.rb index b8d67b02..be79fac2 100644 --- a/spec/puppet/resource_api/transport/wrapper_spec.rb +++ b/spec/puppet/resource_api/transport/wrapper_spec.rb @@ -6,7 +6,7 @@ require 'puppet/resource_api/transport/wrapper' require_relative '../../../fixtures/test_module/lib/puppet/transport/test_device' -RSpec.describe Puppet::ResourceApi::Transport::Wrapper, agent_test: true do +RSpec.describe Puppet::ResourceApi::Transport::Wrapper, :agent_test do describe '#initialize(name, url_or_config)' do before do module Puppet::Transport diff --git a/spec/puppet/resource_api/transport_spec.rb b/spec/puppet/resource_api/transport_spec.rb index 49e18d94..fe50b978 100644 --- a/spec/puppet/resource_api/transport_spec.rb +++ b/spec/puppet/resource_api/transport_spec.rb @@ -178,7 +178,7 @@ def change_environment(name = nil) end end - describe '#connect(name, connection_info)', agent_test: true do + describe '#connect(name, connection_info)', :agent_test do let(:name) { 'test_target' } let(:schema) do { @@ -274,7 +274,7 @@ class Wibble; end end end - describe '#validate(name, connection_info)', agent_test: true do + describe '#validate(name, connection_info)', :agent_test do context 'when the transport does not exist' do it { expect { described_class.send(:validate, 'wibble', {}) }.to raise_error LoadError, %r{(no such file to load|cannot load such file) -- puppet/transport/schema/wibble} } end diff --git a/spec/puppet/resource_api_spec.rb b/spec/puppet/resource_api_spec.rb index 3f126c77..cbb273c8 100644 --- a/spec/puppet/resource_api_spec.rb +++ b/spec/puppet/resource_api_spec.rb @@ -38,7 +38,7 @@ subject(:type) { Puppet::Type.type(:minimal) } it { is_expected.not_to be_nil } - it { is_expected.to be_respond_to :instances } + it { is_expected.to respond_to :instances } it { expect(type.apply_to).to eq(:host) } end @@ -64,7 +64,7 @@ subject(:type) { Puppet::Type.type(:both) } it { is_expected.not_to be_nil } - it { is_expected.to be_respond_to :instances } + it { is_expected.to respond_to :instances } it { expect(type.instance_variable_get(:@doc)).to eq 'the docs' } end end @@ -279,7 +279,7 @@ def extract_values(function) # rubocop:enable Lint/BooleanSymbol end - context 'with a basic provider', agent_test: true do + context 'with a basic provider', :agent_test do let(:provider_class) do Class.new do def get(_context) @@ -322,7 +322,7 @@ def set(_context, _changes); end test_enum: 'a', test_variant_pattern: '0xAEF123FF', test_url: 'http://example.com' } end - context 'with a bad provider', agent_test: true do + context 'with a bad provider', :agent_test do before do stub_const('Puppet::Provider::TypeCheck', Module.new) stub_const('Puppet::Provider::TypeCheck::TypeCheck', provider_class) @@ -408,7 +408,7 @@ def set(_context, _changes); end it('has the secret value is set correctly') { expect(instance[:secret]).to be_a Puppet::Pops::Types::PSensitiveType::Sensitive } - context 'with a basic provider', agent_test: true do + context 'with a basic provider', :agent_test do let(:provider_class) do Class.new do def get(_context) @@ -471,7 +471,7 @@ def set(_context, _changes); end end end - context 'when registering a type that is ensurable', agent_test: true do + context 'when registering a type that is ensurable', :agent_test do context 'when ensurable is correctly declared' do let(:definition) do { @@ -787,7 +787,7 @@ def set(_context, _changes); end end end - context 'when registering a type with an `init_only` attribute', agent_test: true do + context 'when registering a type with an `init_only` attribute', :agent_test do let(:definition) do { name: 'init_behaviour', @@ -913,7 +913,7 @@ def set(_context, _changes); end end end - context 'when registering a type with an `read_only` attribute', agent_test: true do + context 'when registering a type with an `read_only` attribute', :agent_test do let(:definition) do { name: 'read_only_behaviour', @@ -1022,7 +1022,7 @@ def set(_context, _changes); end end end - describe 'a provider that does not return the namevar', agent_test: true do + describe 'a provider that does not return the namevar', :agent_test do subject(:instance) { Puppet::Type.type(:not_name_namevar) } let(:provider_class) do @@ -1048,7 +1048,7 @@ def set(_context, changes) end end end - context 'when registering a type with multiple namevars', agent_test: true do + context 'when registering a type with multiple namevars', :agent_test do let(:name) { 'multiple' } let(:title_patterns) { nil } let(:definition) do @@ -1332,7 +1332,7 @@ def set(_context, _changes); end end end - context 'when registering a type with a mandatory boolean value', agent_test: true do + context 'when registering a type with a mandatory boolean value', :agent_test do let(:provider_class) do Class.new do def get(_context) @@ -1418,7 +1418,7 @@ def set(_context, _changes); end end end - describe '#load_provider', agent_test: true do + describe '#load_provider', :agent_test do before { described_class.register_type(definition) } context 'when loading a non-existing provider' do @@ -1507,7 +1507,7 @@ class OtherDevice; end end end - context 'with a provider that does canonicalization', agent_test: true do + context 'with a provider that does canonicalization', :agent_test do let(:definition) do { name: 'canonicalizer', @@ -1714,7 +1714,7 @@ def set(_context, changes) end context 'with a provider that has a custom - generate', agent_test: true do + generate', :agent_test do let(:definition) do { name: 'generator', @@ -1798,7 +1798,7 @@ def set(_context, changes) end end - context 'with a provider that does not need canonicalization', agent_test: true do + context 'with a provider that does not need canonicalization', :agent_test do let(:definition) do { name: 'passthrough', @@ -1927,7 +1927,7 @@ def set(_context, changes) end end - context 'with a provider that does custom insync', agent_test: true do + context 'with a provider that does custom insync', :agent_test do let(:provider_class) do Class.new do def insync?(_context, _title, _attribute_name, _is_hash, _should_hash) @@ -2052,7 +2052,7 @@ def set(_context, changes) end end - context 'with a `remote_resource` provider', agent_test: true do + context 'with a `remote_resource` provider', :agent_test do let(:definition) do { name: 'remoter', @@ -2123,7 +2123,7 @@ class Wibble; end end end - context 'with a `supports_noop` provider', agent_test: true do + context 'with a `supports_noop` provider', :agent_test do let(:definition) do { name: 'test_noop_support', @@ -2176,7 +2176,7 @@ def set(_context, _changes, noop: false); end end end - context 'with a `simple_get_filter` provider', agent_test: true do + context 'with a `simple_get_filter` provider', :agent_test do let(:definition) do { name: 'test_get_filter', From 9118232bd74bb8f7b43848fef84c8be6b632e46c Mon Sep 17 00:00:00 2001 From: david22swan Date: Tue, 30 Sep 2025 11:29:58 +0100 Subject: [PATCH 3/5] (RUBOCOP) Unsafe autocorrections --- spec/integration/resource_api_spec.rb | 4 +--- spec/puppet/resource_api/property_spec.rb | 3 +-- spec/puppet/resource_api/simple_provider_spec.rb | 3 +-- spec/puppet/resource_api/transport/wrapper_spec.rb | 3 +-- 4 files changed, 4 insertions(+), 9 deletions(-) diff --git a/spec/integration/resource_api_spec.rb b/spec/integration/resource_api_spec.rb index 1a25cc8c..c958dd53 100644 --- a/spec/integration/resource_api_spec.rb +++ b/spec/integration/resource_api_spec.rb @@ -239,9 +239,7 @@ def get(_context) end before do - allow(catalog).to receive(:resource).and_return nil - allow(catalog).to receive(:alias).and_return nil - allow(catalog).to receive(:host_config?).and_return true + allow(catalog).to receive_messages(resource: nil, alias: nil, host_config?: true) end it('flushes') { expect { instance.flush }.not_to raise_exception } diff --git a/spec/puppet/resource_api/property_spec.rb b/spec/puppet/resource_api/property_spec.rb index fbf5a9e9..74cf88ff 100644 --- a/spec/puppet/resource_api/property_spec.rb +++ b/spec/puppet/resource_api/property_spec.rb @@ -203,8 +203,7 @@ allow(resource).to receive(:rsapi_canonicalized_target_state) allow(resource).to receive(:rsapi_current_state) allow(resource).to receive(:rsapi_title) - allow(referrable_type_custom_insync).to receive(:context).and_return(context) - allow(referrable_type_custom_insync).to receive(:my_provider).and_return(test_provider_with_insync) + allow(referrable_type_custom_insync).to receive_messages(context: context, my_provider: test_provider_with_insync) end context 'when the property is not rsapi_custom_insync_trigger' do diff --git a/spec/puppet/resource_api/simple_provider_spec.rb b/spec/puppet/resource_api/simple_provider_spec.rb index d8420334..32e2f3ed 100644 --- a/spec/puppet/resource_api/simple_provider_spec.rb +++ b/spec/puppet/resource_api/simple_provider_spec.rb @@ -445,8 +445,7 @@ def delete(context, _name); end before do allow(context).to receive(:updating).with('title').and_yield allow(type_def).to receive(:feature?).with('simple_get_filter') - allow(type_def).to receive(:ensurable?).and_return(false) - allow(type_def).to receive(:namevars).and_return([:name]) + allow(type_def).to receive_messages(ensurable?: false, namevars: [:name]) end it { expect { provider.set(context, changes) }.to raise_error(/SimpleProvider cannot be used with a Type that is not ensurable/) } diff --git a/spec/puppet/resource_api/transport/wrapper_spec.rb b/spec/puppet/resource_api/transport/wrapper_spec.rb index be79fac2..9c51722c 100644 --- a/spec/puppet/resource_api/transport/wrapper_spec.rb +++ b/spec/puppet/resource_api/transport/wrapper_spec.rb @@ -64,8 +64,7 @@ class SomethingSomethingDarkside; end let(:transport) { instance_double(Puppet::Transport::TestDevice, 'transport') } it 'returns the facts provided by the transport' do - allow(Puppet::ResourceApi::Transport).to receive(:connect).and_return(transport) - allow(Puppet::ResourceApi::Transport).to receive(:list).and_return(schema: :dummy) + allow(Puppet::ResourceApi::Transport).to receive_messages(connect: transport, list: { schema: :dummy }) allow(Puppet::ResourceApi::PuppetContext).to receive(:new).and_return(context) allow(transport).to receive(:facts).with(context).and_return(facts) From 64d5c1d41c66eba11a82f6eedbdf03e8cb7f7c6f Mon Sep 17 00:00:00 2001 From: david22swan Date: Tue, 30 Sep 2025 13:35:30 +0100 Subject: [PATCH 4/5] (maint) Correct .gitignore --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 60d17fb7..c945837a 100644 --- a/.gitignore +++ b/.gitignore @@ -14,4 +14,4 @@ vendor/ .rspec_status # puppetlabs_spec_helper automatic fixtures -spec/fixtures/modules/test_module +spec/fixtures/modules/ From 2eb46cf4b9036ded36e58225a99d03f469dfef51 Mon Sep 17 00:00:00 2001 From: david22swan Date: Tue, 30 Sep 2025 15:05:54 +0100 Subject: [PATCH 5/5] (CAT-2453) Implement rubocop plugins that have been moved out These plugins where previously included in the base rubocop setup but have since been moved out. --- .rubocop.yml | 7 ++++++- .rubocop_todo.yml | 11 ++++++++--- Gemfile | 3 +++ spec/puppet/resource_api_spec.rb | 2 +- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 75e3806c..7d18adbe 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,7 +1,12 @@ --- inherit_from: .rubocop_todo.yml -require: rubocop-rspec +plugins: + - rubocop-rspec + - rubocop-rspec_rails + - rubocop-performance + - rubocop-factory_bot + - rubocop-capybara AllCops: NewCops: enable TargetRubyVersion: '3.1' diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 7b1e3193..034b9317 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2025-03-10 20:03:26 UTC using RuboCop version 1.70.0. +# on 2025-09-30 14:08:02 UTC using RuboCop version 1.73.2. # 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 @@ -24,6 +24,13 @@ Lint/EmptyClass: - 'spec/puppet/resource_api/transport_spec.rb' - 'spec/puppet/resource_api_spec.rb' +# Offense count: 5 +# Configuration parameters: MinSize. +Performance/CollectionLiteralInLoop: + Exclude: + - 'lib/puppet/resource_api.rb' + - 'spec/puppet/resource_api/data_type_handling_spec.rb' + # Offense count: 25 # Configuration parameters: IgnoredMetadata. RSpec/DescribeClass: @@ -70,8 +77,6 @@ RSpec/SubjectStub: # Offense count: 24 # This cop supports unsafe autocorrection (--autocorrect-all). -# Configuration parameters: EnforcedStyle. -# SupportedStyles: constant, string RSpec/VerifiedDoubleReference: Exclude: - 'spec/integration/resource_api_spec.rb' diff --git a/Gemfile b/Gemfile index 84454d98..e877b35c 100644 --- a/Gemfile +++ b/Gemfile @@ -22,6 +22,9 @@ group :tests do gem 'rubocop', '~> 1.73.0', require: false gem 'rubocop-rspec', '~> 3.5.0', require: false gem 'rubocop-performance', '~> 1.24.0', require: false + gem 'rubocop-rspec_rails', '~> 2.31.0', require: false + gem 'rubocop-factory_bot', '~> 2.27.0', require: false + gem 'rubocop-capybara', '~> 2.22.0', require: false end group :development do diff --git a/spec/puppet/resource_api_spec.rb b/spec/puppet/resource_api_spec.rb index cbb273c8..5717c9fa 100644 --- a/spec/puppet/resource_api_spec.rb +++ b/spec/puppet/resource_api_spec.rb @@ -43,7 +43,7 @@ end describe Puppet::Provider do - it('has a module prepared for the provider') { expect(described_class.const_get('Minimal', false).name).to eq 'Puppet::Provider::Minimal' } + it('has a module prepared for the provider') { expect(described_class.const_get(:Minimal, false).name).to eq 'Puppet::Provider::Minimal' } end end