From 60594f3320278d1ff0f6bf3b226d75b96c6eda88 Mon Sep 17 00:00:00 2001 From: Tom Beech Date: Wed, 27 Jan 2021 14:41:11 -0800 Subject: [PATCH] (GH-2549) Use new default modulepath This updates Bolt to use the new default modulepath `['modules']` and append the `.modules/` directory to the end of both the default modulepath and configured modulepath. !feature * **Update default modulepath** ([#2549](https://github.com/puppetlabs/bolt/issues/2549)) Bolt's default modulepath is now `['modules']` instead of `['modules', 'site', 'site-modules']`. Bolt will also automatically append the project's `.modules/` directory to all modulepaths, whether a project uses the default modulepath or a configured modulepath. --- lib/bolt/config.rb | 12 +++-------- lib/bolt/project.rb | 9 +-------- lib/bolt/project_manager.rb | 2 +- lib/bolt/project_manager/module_migrator.rb | 18 ++++++++++++----- spec/bolt/cli_spec.rb | 2 +- spec/bolt/config_spec.rb | 4 ++-- .../project_manager/module_migrator_spec.rb | 8 +++----- spec/bolt/project_spec.rb | 20 ------------------- 8 files changed, 24 insertions(+), 51 deletions(-) diff --git a/lib/bolt/config.rb b/lib/bolt/config.rb index 334f3356a1..6744403c39 100644 --- a/lib/bolt/config.rb +++ b/lib/bolt/config.rb @@ -335,7 +335,7 @@ def validate ) end - if @project.modules && @data['modulepath']&.include?(@project.managed_moduledir.to_s) + if @data['modulepath']&.include?(@project.managed_moduledir.to_s) raise Bolt::ValidationError, "Found invalid path in modulepath: #{@project.managed_moduledir}. This path "\ "is automatically appended to the modulepath and cannot be configured." @@ -372,17 +372,11 @@ def puppetfile end def modulepath - path = @data['modulepath'] || @project.modulepath - - if @project.modules.any? - path + [@project.managed_moduledir.to_s] - else - path - end + (@data['modulepath'] || @project.modulepath) + [@project.managed_moduledir.to_s] end def modulepath=(value) - @data['modulepath'] = value + @data['modulepath'] = Array(value) end def plugin_cache diff --git a/lib/bolt/project.rb b/lib/bolt/project.rb index 9c51eec2e4..584c03b994 100644 --- a/lib/bolt/project.rb +++ b/lib/bolt/project.rb @@ -114,6 +114,7 @@ def initialize(data, path, type = 'option') @backup_dir = @path + '.bolt-bak' @plugin_cache_file = @path + '.plugin_cache.json' @plan_cache_file = @path + '.plan_cache.json' + @modulepath = [(@path + 'modules').to_s] if (tc = Bolt::Config::INVENTORY_OPTIONS.keys & data.keys).any? Bolt::Logger.warn( @@ -124,14 +125,6 @@ def initialize(data, path, type = 'option') @data = data.slice(*Bolt::Config::PROJECT_OPTIONS) - # If the 'modules' key is present in the project configuration file, - # use the new, shorter modulepath. - @modulepath = if @data.key?('modules') - [(@path + 'modules').to_s] - else - [(@path + 'modules').to_s, (@path + 'site-modules').to_s, (@path + 'site').to_s] - end - validate if project_file? end diff --git a/lib/bolt/project_manager.rb b/lib/bolt/project_manager.rb index 602f495946..bde8efffc0 100644 --- a/lib/bolt/project_manager.rb +++ b/lib/bolt/project_manager.rb @@ -195,7 +195,7 @@ def migrate migrator.migrate( @config.project, - @config.modulepath + @config.modulepath[0...-1] ) end end diff --git a/lib/bolt/project_manager/module_migrator.rb b/lib/bolt/project_manager/module_migrator.rb index ee95d46643..a703ffbf23 100644 --- a/lib/bolt/project_manager/module_migrator.rb +++ b/lib/bolt/project_manager/module_migrator.rb @@ -6,19 +6,20 @@ module Bolt class ProjectManager class ModuleMigrator < Migrator def migrate(project, configured_modulepath) - return true unless project.modules.empty? + return true if project.managed_moduledir.exist? @outputter.print_message "Migrating project modules\n\n" config = project.project_file puppetfile = project.puppetfile managed_moduledir = project.managed_moduledir - modulepath = [(project.path + 'modules').to_s, + new_modulepath = [(project.path + 'modules').to_s] + old_modulepath = [(project.path + 'modules').to_s, (project.path + 'site-modules').to_s, (project.path + 'site').to_s] # Notify user to manually migrate modules if using non-default modulepath - if configured_modulepath != modulepath + if configured_modulepath != new_modulepath && configured_modulepath != old_modulepath @outputter.print_action_step( "Project has a non-default configured modulepath, unable to automatically "\ "migrate project modules. To migrate project modules manually, see "\ @@ -27,14 +28,21 @@ def migrate(project, configured_modulepath) true # Migrate modules from Puppetfile elsif File.exist?(puppetfile) - migrate_modules_from_puppetfile(config, puppetfile, managed_moduledir, modulepath) + migrate_modules_from_puppetfile(config, puppetfile, managed_moduledir, old_modulepath) # Migrate modules to updated modulepath else - consolidate_modules(modulepath) + consolidate_modules(old_modulepath) update_project_config([], config) end end + # Returns true if the project's modules can be migrated, false otherwise. + # To migrate a project's modules, the current project must use either + # the old or new default modulepath. + def migrateable? + # TODO + end + # Migrates modules by reading a Puppetfile and prompting the user for # which ones are direct dependencies for the project. Once the user has # selected the direct dependencies, this will resolve the modules, write a diff --git a/spec/bolt/cli_spec.rb b/spec/bolt/cli_spec.rb index 8a92d1eed5..674eefb537 100644 --- a/spec/bolt/cli_spec.rb +++ b/spec/bolt/cli_spec.rb @@ -2435,7 +2435,7 @@ def stub_config(file_content = {}) it 'reads modulepath' do cli = Bolt::CLI.new(%w[command run uptime] + config_flags) cli.parse - expect(cli.config.modulepath).to eq(modulepath) + expect(cli.config.modulepath).to include(*modulepath) end it 'reads concurrency' do diff --git a/spec/bolt/config_spec.rb b/spec/bolt/config_spec.rb index 08fd614d79..febe7d8d52 100644 --- a/spec/bolt/config_spec.rb +++ b/spec/bolt/config_spec.rb @@ -41,13 +41,13 @@ it "treats relative modulepath as relative to project" do module_dirs = %w[site modules] config = Bolt::Config.new(project, 'modulepath' => module_dirs.join(File::PATH_SEPARATOR)) - expect(config.modulepath).to eq(module_dirs.map { |dir| (project.path + dir).to_s }) + expect(config.modulepath).to include(*module_dirs.map { |dir| (project.path + dir).to_s }) end it "accepts an array for modulepath" do module_dirs = %w[site modules] config = Bolt::Config.new(project, 'modulepath' => module_dirs) - expect(config.modulepath).to eq(module_dirs.map { |dir| (project.path + dir).to_s }) + expect(config.modulepath).to include(*module_dirs.map { |dir| (project.path + dir).to_s }) end it 'modifies concurrency if ulimit is low', :ssh do diff --git a/spec/bolt/project_manager/module_migrator_spec.rb b/spec/bolt/project_manager/module_migrator_spec.rb index 9e779d92dd..4459875d68 100644 --- a/spec/bolt/project_manager/module_migrator_spec.rb +++ b/spec/bolt/project_manager/module_migrator_spec.rb @@ -45,17 +45,15 @@ def make_moduledirs allow(outputter).to receive(:stop_spin) end - context 'with modules configured' do - let(:project_config) { { 'modules' => [] } } - + context 'with a managed moduledir' do it 'does not migrate' do + FileUtils.mkdir(@tmpdir + '.modules') expect(migrate).to be(true) - expect(File.exist?(project.managed_moduledir)).to be(false) end end context 'with a non-default modulepath' do - let(:modulepath) { [(@tmpdir + 'modules').to_s] } + let(:modulepath) { [(@tmpdir + 'mymodules').to_s] } it 'does not migrate' do migrate diff --git a/spec/bolt/project_spec.rb b/spec/bolt/project_spec.rb index f2f3f34b8f..afbc2224f3 100644 --- a/spec/bolt/project_spec.rb +++ b/spec/bolt/project_spec.rb @@ -232,24 +232,4 @@ Bolt::Project.create_project('myproject', 'user') end end - - describe '#modulepath' do - context 'with modules set' do - let(:project_config) { { 'modules' => [] } } - - it 'returns the new default modulepath' do - expect(project.modulepath).to match_array([(project.path + 'modules').to_s]) - end - end - - context 'without modules set' do - it 'returns the old default modulepath if modules is not set' do - expect(project.modulepath).to match_array([ - (project.path + 'modules').to_s, - (project.path + 'site-modules').to_s, - (project.path + 'site').to_s - ]) - end - end - end end