From c1794c4793a52a455e212373eaf668cd5a722eb2 Mon Sep 17 00:00:00 2001 From: Tony Vu Date: Thu, 12 Aug 2021 14:40:58 -0700 Subject: [PATCH] (CODEMGMT-1457) Flip `deploy-spec` setting to `exclude-spec` Previous to this change, r10k would default not deploying the spec dir for modules; this change flips that, switching to using `exclude-spec` and keeping the original default behavior of deploying the spec dir. The setting `exclude-spec` can be used in the r10k conf, Puppetfile, or CLI. --- lib/r10k/action/deploy/environment.rb | 4 ++-- lib/r10k/action/deploy/module.rb | 4 ++-- lib/r10k/action/runner.rb | 4 ++-- lib/r10k/cli/deploy.rb | 2 +- lib/r10k/module/base.rb | 8 ++++---- lib/r10k/settings.rb | 4 ++-- spec/unit/action/deploy/environment_spec.rb | 4 ++-- spec/unit/action/deploy/module_spec.rb | 4 ++-- spec/unit/module/base_spec.rb | 12 ++++++------ spec/unit/module/forge_spec.rb | 4 ++-- spec/unit/module/git_spec.rb | 4 ++-- spec/unit/module/svn_spec.rb | 4 ++-- spec/unit/module_spec.rb | 12 ++++++------ spec/unit/settings_spec.rb | 12 ++++++------ 14 files changed, 41 insertions(+), 41 deletions(-) diff --git a/lib/r10k/action/deploy/environment.rb b/lib/r10k/action/deploy/environment.rb index 5d1798626..707c4b991 100644 --- a/lib/r10k/action/deploy/environment.rb +++ b/lib/r10k/action/deploy/environment.rb @@ -44,7 +44,7 @@ def initialize(opts, argv, settings = {}) preload_environments: true }, modules: { - deploy_spec: settings.dig(:deploy, :deploy_spec), + exclude_spec: settings.dig(:deploy, :exclude_spec), requested_modules: [], deploy_modules: @modules, force: !@no_force, # force here is used to make it easier to reason about @@ -238,7 +238,7 @@ def allowed_initialize_opts modules: :self, cachedir: :self, 'no-force': :self, - 'deploy-spec': :self, + 'exclude-spec': :self, 'generate-types': :self, 'puppet-path': :self, 'puppet-conf': :self, diff --git a/lib/r10k/action/deploy/module.rb b/lib/r10k/action/deploy/module.rb index ef46db66a..af7c19e5a 100644 --- a/lib/r10k/action/deploy/module.rb +++ b/lib/r10k/action/deploy/module.rb @@ -38,7 +38,7 @@ def initialize(opts, argv, settings = {}) generate_types: @generate_types }, modules: { - deploy_spec: settings.dig(:deploy, :deploy_spec), + exclude_spec: settings.dig(:deploy, :exclude_spec), requested_modules: @argv.map.to_a, # force here is used to make it easier to reason about force: !@no_force @@ -118,7 +118,7 @@ def visit_environment(environment) def allowed_initialize_opts super.merge(environment: true, cachedir: :self, - 'deploy-spec': :self, + 'exclude-spec': :self, 'no-force': :self, 'generate-types': :self, 'puppet-path': :self, diff --git a/lib/r10k/action/runner.rb b/lib/r10k/action/runner.rb index edd029f05..2a21e2f05 100644 --- a/lib/r10k/action/runner.rb +++ b/lib/r10k/action/runner.rb @@ -44,12 +44,12 @@ def setup_settings overrides = {} overrides[:cachedir] = @opts[:cachedir] if @opts.key?(:cachedir) - if @opts.key?(:'puppet-path') || @opts.key?(:'generate-types') || @opts.key?(:'deploy-spec') || @opts.key?(:'puppet-conf') + if @opts.key?(:'puppet-path') || @opts.key?(:'generate-types') || @opts.key?(:'exclude-spec') || @opts.key?(:'puppet-conf') overrides[:deploy] = {} overrides[:deploy][:puppet_path] = @opts[:'puppet-path'] if @opts.key?(:'puppet-path') overrides[:deploy][:puppet_conf] = @opts[:'puppet-conf'] if @opts.key?(:'puppet-conf') overrides[:deploy][:generate_types] = @opts[:'generate-types'] if @opts.key?(:'generate-types') - overrides[:deploy][:deploy_spec] = @opts[:'deploy-spec'] if @opts.key?(:'deploy-spec') + overrides[:deploy][:exclude_spec] = @opts[:'exclude-spec'] if @opts.key?(:'exclude-spec') end with_overrides = config_settings.merge(overrides) do |key, oldval, newval| diff --git a/lib/r10k/cli/deploy.rb b/lib/r10k/cli/deploy.rb index 0deb8c096..2de853131 100644 --- a/lib/r10k/cli/deploy.rb +++ b/lib/r10k/cli/deploy.rb @@ -24,7 +24,7 @@ def self.command option nil, :cachedir, 'Specify a cachedir, overriding the value in config', argument: :required flag nil, :'no-force', 'Prevent the overwriting of local module modifications' flag nil, :'generate-types', 'Run `puppet generate types` after updating an environment' - flag nil, :'deploy-spec', 'Deploy the spec dir alongside other module directories' + flag nil, :'exclude-spec', 'Exclude the module\'s spec dir from deployment' option nil, :'puppet-path', 'Path to puppet executable', argument: :required do |value, cmd| unless File.executable? value $stderr.puts "The specified puppet executable #{value} is not executable." diff --git a/lib/r10k/module/base.rb b/lib/r10k/module/base.rb index fbf4efacc..09aea6452 100644 --- a/lib/r10k/module/base.rb +++ b/lib/r10k/module/base.rb @@ -57,8 +57,8 @@ def initialize(title, dirname, args, environment=nil) @environment = environment @overrides = args.delete(:overrides) || {} @spec_deletable = true - @deploy_spec = args.delete(:deploy_spec) - @deploy_spec = @overrides[:modules].delete(:deploy_spec) if @overrides.dig(:modules, :deploy_spec) + @exclude_spec = args.delete(:exclude_spec) + @exclude_spec = @overrides[:modules].delete(:exclude_spec) if @overrides.dig(:modules, :exclude_spec) @origin = 'external' # Expect Puppetfile or R10k::Environment to set this to a specific value @requested_modules = @overrides.dig(:modules, :requested_modules) || [] @@ -71,9 +71,9 @@ def full_path path.to_s end - # Delete the spec dir unless @deploy_spec has been set to true or @spec_deletable is false + # Delete the spec dir unless @exclude_spec has been set to true or @spec_deletable is false def maybe_delete_spec_dir - unless @deploy_spec + if @exclude_spec if @spec_deletable delete_spec_dir else diff --git a/lib/r10k/settings.rb b/lib/r10k/settings.rb index 3cd73c579..3415cee78 100644 --- a/lib/r10k/settings.rb +++ b/lib/r10k/settings.rb @@ -195,12 +195,12 @@ def self.deploy_settings end end }), - Definition.new(:deploy_spec, { + Definition.new(:exclude_spec, { :desc => "Whether or not to deploy the spec dir of a module. Defaults to false.", :default => false, :validate => lambda do |value| unless !!value == value - raise ArgumentError, "`deploy_spec` can only be a boolean value, not '#{value}'" + raise ArgumentError, "`exclude_spec` can only be a boolean value, not '#{value}'" end end })]) diff --git a/spec/unit/action/deploy/environment_spec.rb b/spec/unit/action/deploy/environment_spec.rb index 509461e24..04751aa7e 100644 --- a/spec/unit/action/deploy/environment_spec.rb +++ b/spec/unit/action/deploy/environment_spec.rb @@ -59,8 +59,8 @@ described_class.new({ 'github-app-key': '/nonexistent' }, [], {}) end - it 'can accept a deploy-spec option' do - described_class.new({ :'deploy-spec' => true }, [], {}) + it 'can accept a exclude-spec option' do + described_class.new({ :'exclude-spec' => true }, [], {}) end describe "initializing errors" do diff --git a/spec/unit/action/deploy/module_spec.rb b/spec/unit/action/deploy/module_spec.rb index 012057742..4a62261b9 100644 --- a/spec/unit/action/deploy/module_spec.rb +++ b/spec/unit/action/deploy/module_spec.rb @@ -54,8 +54,8 @@ described_class.new({ 'github-app-key': '/nonexistent' }, [], {}) end - it 'can accept a deploy-spec option' do - described_class.new({ :'deploy-spec' => true }, [], {}) + it 'can accept a exclude-spec option' do + described_class.new({ :'exclude-spec' => true }, [], {}) end end diff --git a/spec/unit/module/base_spec.rb b/spec/unit/module/base_spec.rb index dd2051499..7f2c5979c 100644 --- a/spec/unit/module/base_spec.rb +++ b/spec/unit/module/base_spec.rb @@ -42,27 +42,27 @@ allow(logger).to receive(:info).with(any_args) end - it 'removes the spec directory' do + it 'does not remove the spec directory by default' do FileUtils.mkdir_p(spec_path) m = described_class.new(title, dirname, {}) m.maybe_delete_spec_dir - expect(Dir.exist?(spec_path)).to eq false + expect(Dir.exist?(spec_path)).to eq true end it 'detects a symlink and deletes the target' do Dir.mkdir(dirname + module_name) target_dir = Dir.mktmpdir FileUtils.ln_s(target_dir, spec_path) - m = described_class.new(title, dirname, {}) + m = described_class.new(title, dirname, {exclude_spec: true}) m.maybe_delete_spec_dir expect(Dir.exist?(target_dir)).to eq false end - it 'does not remove the spec directory if deploy_spec is set' do + it 'removes the spec directory if exclude_spec is set' do FileUtils.mkdir_p(spec_path) - m = described_class.new(title, dirname, {deploy_spec: true}) + m = described_class.new(title, dirname, {exclude_spec: true}) m.maybe_delete_spec_dir - expect(Dir.exist?(spec_path)).to eq true + expect(Dir.exist?(spec_path)).to eq false end it 'does not remove the spec directory if spec_deletable is false' do diff --git a/spec/unit/module/forge_spec.rb b/spec/unit/module/forge_spec.rb index 78287425d..1637778d2 100644 --- a/spec/unit/module/forge_spec.rb +++ b/spec/unit/module/forge_spec.rb @@ -175,12 +175,12 @@ let(:spec_path) { dirname + module_name + 'spec' } subject { described_class.new(title, dirname, {}) } - it 'defaults to deleting the spec dir' do + it 'defaults to keeping the spec dir' do FileUtils.mkdir_p(spec_path) expect(subject).to receive(:status).and_return(:absent) expect(subject).to receive(:install) subject.sync - expect(Dir.exist?(spec_path)).to eq false + expect(Dir.exist?(spec_path)).to eq true end end diff --git a/spec/unit/module/git_spec.rb b/spec/unit/module/git_spec.rb index 9523254a9..7fbef3f23 100644 --- a/spec/unit/module/git_spec.rb +++ b/spec/unit/module/git_spec.rb @@ -97,12 +97,12 @@ let(:spec_path) { dirname + module_name + 'spec' } subject { described_class.new(title, dirname, {}) } - it 'defaults to deleting the spec dir' do + it 'defaults to keeping the spec dir' do FileUtils.mkdir_p(spec_path) allow(mock_repo).to receive(:resolve).with('master').and_return('abc123') allow(mock_repo).to receive(:sync) subject.sync - expect(Dir.exist?(spec_path)).to eq false + expect(Dir.exist?(spec_path)).to eq true end end diff --git a/spec/unit/module/svn_spec.rb b/spec/unit/module/svn_spec.rb index 90d317b66..1cb9a1c10 100644 --- a/spec/unit/module/svn_spec.rb +++ b/spec/unit/module/svn_spec.rb @@ -127,13 +127,13 @@ let(:spec_path) { dirname + module_name + 'spec' } subject { described_class.new(title, dirname, {}) } - it 'gets deleted by default' do + it 'is kept by default' do FileUtils.mkdir_p(spec_path) expect(subject).to receive(:status).and_return(:absent) expect(subject).to receive(:install).and_return(nil) subject.sync - expect(Dir.exist?(spec_path)).to eq false + expect(Dir.exist?(spec_path)).to eq true end end diff --git a/spec/unit/module_spec.rb b/spec/unit/module_spec.rb index 5ef076c11..b882a0b16 100644 --- a/spec/unit/module_spec.rb +++ b/spec/unit/module_spec.rb @@ -91,15 +91,15 @@ expect(obj).to be_a_kind_of(R10K::Module::Base) }.not_to raise_error end - describe 'the deploy_spec setting' do - it 'sets the deploy_spec instance variable' do - obj = R10K::Module.new(name, '/modulepath', options.merge({deploy_spec: true})) - expect(obj.instance_variable_get("@deploy_spec")).to eq(true) + describe 'the exclude_spec setting' do + it 'sets the exclude_spec instance variable' do + obj = R10K::Module.new(name, '/modulepath', options.merge({exclude_spec: true})) + expect(obj.instance_variable_get("@exclude_spec")).to eq(true) end it 'can be overridden by the overrides map' do - options = options.merge({deploy_spec: false, overrides: {modules: {deploy_spec: true}}}) + options = options.merge({exclude_spec: false, overrides: {modules: {exclude_spec: true}}}) obj = R10K::Module.new(name, '/modulepath', options) - expect(obj.instance_variable_get("@deploy_spec")).to eq(true) + expect(obj.instance_variable_get("@exclude_spec")).to eq(true) end end end diff --git a/spec/unit/settings_spec.rb b/spec/unit/settings_spec.rb index 8c2a54bf7..e03786b40 100644 --- a/spec/unit/settings_spec.rb +++ b/spec/unit/settings_spec.rb @@ -95,21 +95,21 @@ describe "deploy settings" do subject { described_class.deploy_settings } - describe 'deploy_spec' do + describe 'exclude_spec' do it 'is false by default' do - expect(subject.evaluate({})[:deploy_spec]).to eq(false) + expect(subject.evaluate({})[:exclude_spec]).to eq(false) end it 'can be set to true' do - expect(subject.evaluate({"deploy_spec" => true})[:deploy_spec]).to eq(true) + expect(subject.evaluate({"exclude_spec" => true})[:exclude_spec]).to eq(true) end it "raises an error for non-boolean values" do expect { - subject.evaluate({"deploy_spec" => 'invalid_string'}) + subject.evaluate({"exclude_spec" => 'invalid_string'}) }.to raise_error do |err| expect(err.message).to match(/Validation failed for 'deploy' settings group/) expect(err.errors.size).to eq 1 - expect(err.errors[:deploy_spec]).to be_a_kind_of(ArgumentError) - expect(err.errors[:deploy_spec].message).to match(/`deploy_spec` can only be a boolean value, not 'invalid_string'/) + expect(err.errors[:exclude_spec]).to be_a_kind_of(ArgumentError) + expect(err.errors[:exclude_spec].message).to match(/`exclude_spec` can only be a boolean value, not 'invalid_string'/) end end end