Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(maint) Pass the appropriate Puppetfile path to module loader #1203

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.mkd
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Unreleased
- (CODEMGMT-1421) Add skeleton for deploy_spec option [#1189](https://github.com/puppetlabs/r10k/pull/1189)
- (RK-369) Make module deploys run the postrun command if any environments were updated. [#982](https://github.com/puppetlabs/r10k/issues/982)
- Add support for Github App auth token. This allows r10k to authenticate under strict SSO/2FA guidelines that cannot utilize machine users for code deployment. [#1180](https://github.com/puppetlabs/r10k/pull/1180)
- Restore the ability to load a Puppetfile from a relative `basedir`. [#1202](https://github.com/puppetlabs/r10k/pull/1202)
- Restore the ability to load a Puppetfile from a relative `basedir`. [#1202](https://github.com/puppetlabs/r10k/pull/1202), [#1203](https://github.com/puppetlabs/r10k/pull/1203)

3.10.0
------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@

#Init
master_certname = on(master, puppet('config', 'print', 'certname')).stdout.rstrip
moduledir = on(master, puppet('config', 'print', 'environmentpath')).stdout.strip + '/production/modules'
environments_path = on(master, puppet('config', 'print', 'environmentpath')).stdout.strip
moduledir = File.join(environments_path, 'production', 'modules')
puppetfile_path = File.join(environments_path, 'production', 'Puppetfile')
git_remote_environments_path = '/root/environments'
last_commit = git_last_commit(master, git_remote_environments_path)
r10k_fqp = get_r10k_fqp(master)
Expand Down Expand Up @@ -76,7 +78,7 @@ class { 'motd':
end

step 'Use r10k to Purge Unmanaged Modules'
on(master, "#{r10k_fqp} puppetfile purge --puppetfile #{remote_puppetfile_path} --moduledir #{moduledir} --verbose debug --trace")
on(master, "#{r10k_fqp} puppetfile purge --puppetfile #{puppetfile_path} --moduledir #{moduledir} --verbose debug --trace")

#Agent will fail because r10k will purge the "motd" module
agents.each do |agent|
Expand Down
15 changes: 11 additions & 4 deletions lib/r10k/module_loader/puppetfile.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
require 'r10k/logging'

module R10K
module ModuleLoader
class Puppetfile

include R10K::Logging

DEFAULT_MODULEDIR = 'modules'
DEFAULT_PUPPETFILE_NAME = 'Puppetfile'
DEFAULT_FORGE_API = 'forgeapi.puppetlabs.com'

attr_accessor :default_branch_override, :environment
attr_reader :modules, :moduledir,
attr_reader :modules, :moduledir, :puppetfile_path,
:managed_directories, :desired_contents, :purge_exclusions

# @param basedir [String] The path that contains the moduledir &
Expand All @@ -30,7 +34,7 @@ def initialize(basedir:,

@basedir = cleanpath(basedir)
@moduledir = resolve_path(@basedir, moduledir)
@puppetfile = resolve_path(@basedir, puppetfile)
@puppetfile_path = resolve_path(@basedir, puppetfile)
@forge = forge
@overrides = overrides
@environment = environment
Expand All @@ -41,11 +45,14 @@ def initialize(basedir:,
@managed_directories = []
@desired_contents = []
@purge_exclusions = []

logger.info _("Using Puppetfile '%{puppetfile}'") % {puppetfile: @puppetfile_path}
logger.debug _("Using moduledir '%{moduledir}'") % {moduledir: @moduledir}
end

def load
dsl = R10K::ModuleLoader::Puppetfile::DSL.new(self)
dsl.instance_eval(puppetfile_content(@puppetfile), @puppetfile)
dsl.instance_eval(puppetfile_content(@puppetfile_path), @puppetfile_path)

validate_no_duplicate_names(@modules)
@modules
Expand All @@ -64,7 +71,7 @@ def load
}

rescue SyntaxError, LoadError, ArgumentError, NameError => e
raise R10K::Error.wrap(e, _("Failed to evaluate %{path}") % {path: @puppetfile})
raise R10K::Error.wrap(e, _("Failed to evaluate %{path}") % {path: @puppetfile_path})
end


Expand Down
21 changes: 10 additions & 11 deletions lib/r10k/puppetfile.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ class Puppetfile
# @return [String] The base directory that contains the Puppetfile
attr_reader :basedir

# @!attrbute [r] puppetfile_path
# @return [String] The path to the Puppetfile
attr_reader :puppetfile_path

# @!attribute [r] environment
# @return [R10K::Environment] Optional R10K::Environment that this Puppetfile belongs to.
attr_reader :environment
Expand Down Expand Up @@ -68,21 +64,20 @@ def initialize(basedir, options_or_moduledir = nil, deprecated_path_arg = nil, d

@force = deprecated_force_arg || options.delete(:force) || false
@moduledir = deprecated_moduledir_arg || options.delete(:moduledir) || File.join(basedir, 'modules')
@puppetfile_name = deprecated_name_arg || options.delete(:puppetfile_name) || 'Puppetfile'
@puppetfile_path = deprecated_path_arg || options.delete(:puppetfile_path) || File.join(basedir, @puppetfile_name)
puppetfile_name = deprecated_name_arg || options.delete(:puppetfile_name) || 'Puppetfile'
puppetfile_path = deprecated_path_arg || options.delete(:puppetfile_path)
@puppetfile = puppetfile_path || puppetfile_name
@environment = options.delete(:environment)

@overrides = options.delete(:overrides) || {}
@default_branch_override = @overrides.dig(:environments, :default_branch_override)

logger.info _("Using Puppetfile '%{puppetfile}'") % {puppetfile: @puppetfile_path}

@forge = 'forgeapi.puppetlabs.com'

@loader = ::R10K::ModuleLoader::Puppetfile.new(
basedir: @basedir,
moduledir: @moduledir,
puppetfile: @puppetfile_name,
puppetfile: @puppetfile,
forge: @forge,
overrides: @overrides,
environment: @environment
Expand All @@ -106,8 +101,8 @@ def load(default_branch_override = nil)
if self.loaded?
return @loaded_content
else
if !File.readable?(@puppetfile_path)
justinstoller marked this conversation as resolved.
Show resolved Hide resolved
logger.debug _("Puppetfile %{path} missing or unreadable") % {path: @puppetfile_path.inspect}
if !File.readable?(puppetfile_path)
logger.debug _("Puppetfile %{path} missing or unreadable") % {path: puppetfile_path.inspect}
else
self.load!(default_branch_override)
end
Expand Down Expand Up @@ -156,6 +151,10 @@ def moduledir
@loader.moduledir
end

def puppetfile_path
@loader.puppetfile_path
end

def environment=(env)
@loader.environment = env
@environment = env
Expand Down
6 changes: 3 additions & 3 deletions spec/unit/module_loader/puppetfile_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@
it 'respects absolute paths' do
absolute_options = options.merge({puppetfile: '/opt/puppetlabs/special/Puppetfile'})
puppetfile = R10K::ModuleLoader::Puppetfile.new(**absolute_options)
expect(puppetfile.instance_variable_get(:@puppetfile)).to eq('/opt/puppetlabs/special/Puppetfile')
expect(puppetfile.instance_variable_get(:@puppetfile_path)).to eq('/opt/puppetlabs/special/Puppetfile')
end

it 'roots the Puppetfile in the basepath if a relative path is specified' do
relative_options = options.merge({puppetfile: 'Puppetfile.global'})
puppetfile = R10K::ModuleLoader::Puppetfile.new(**relative_options)
expect(puppetfile.instance_variable_get(:@puppetfile)).to eq('/test/basedir/env/Puppetfile.global')
expect(puppetfile.instance_variable_get(:@puppetfile_path)).to eq('/test/basedir/env/Puppetfile.global')
end
end

Expand All @@ -68,7 +68,7 @@
end

it 'has a Puppetfile rooted in the basedir' do
expect(subject.instance_variable_get(:@puppetfile)).to eq('/test/basedir/Puppetfile')
expect(subject.instance_variable_get(:@puppetfile_path)).to eq('/test/basedir/Puppetfile')
end

it 'uses the public forge' do
Expand Down
21 changes: 20 additions & 1 deletion spec/unit/puppetfile_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,33 @@
)
end

describe "a custom puppetfile Puppetfile.r10k" do
describe "a custom puppetfile_name" do
it "is the basedir joined with '/Puppetfile.r10k' path" do
expect(subject.puppetfile_path).to eq '/some/nonexistent/basedir/Puppetfile.r10k'
end
end

end

describe R10K::Puppetfile do

describe "a custom relative puppetfile_path" do
it "is the basedir joined with the puppetfile_path" do
relative_subject = described_class.new('/some/nonexistent/basedir',
{puppetfile_path: 'relative/Puppetfile'})
expect(relative_subject.puppetfile_path).to eq '/some/nonexistent/basedir/relative/Puppetfile'
end
end

describe "a custom absolute puppetfile_path" do
it "is the puppetfile_path as given" do
absolute_subject = described_class.new('/some/nonexistent/basedir',
{puppetfile_path: '/some/absolute/custom/Puppetfile'})
expect(absolute_subject.puppetfile_path).to eq '/some/absolute/custom/Puppetfile'
end
end
end

describe R10K::Puppetfile do

subject do
Expand Down