Skip to content

Commit

Permalink
(RK-390) Remove hardcoded default for git ref
Browse files Browse the repository at this point in the history
Previously r10k fell back to the 'main' ref if no other ref was
specified. With this patch, r10k will no longer fall back to that
ref, and error when no ref is found.

To mitigate breakage relying on the hardcoded ref, a user can
set `default_module_ref` in the r10k config to a ref of their
choice, mimicking the old behavior.
  • Loading branch information
tvpartytonight committed Feb 9, 2022
1 parent 15d0814 commit 4dab311
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 12 deletions.
11 changes: 11 additions & 0 deletions doc/dynamic-environments/configuration.mkd
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,17 @@ deploy:
exclude_spec: true
```

#### default_module_ref

R10k is unable to deploy a git module if no `ref` is specified. A `default_module_ref` can be
set in the r10k config that will become the ref a module uses if not otherwise specified. This
is the lowest priority setting for a module's 'ref'. Read [here](../puppetfile.mkd#git) for more info.

```yaml
deploy:
default_module_ref: main
```

Source options
--------------

Expand Down
4 changes: 4 additions & 0 deletions doc/puppetfile.mkd
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,10 @@ operations when updating the repo, which can speed up install times. When
Module versions can also be specified using `:branch` to track a specific
branch reference.

In r10k 3.x the default branch was hardcoded to `master`; in 4.x that was
removed. A `default_module_ref` can be specified in the r10k config to
to mimick that old behavior, read [here](dynamic-environments/configuration.mkd#default_module_ref) for more info.

#### Examples

```ruby
Expand Down
1 change: 1 addition & 0 deletions lib/r10k/action/deploy/environment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def initialize(opts, argv, settings = {})
environments: {
requested_environments: @argv.map { |arg| arg.gsub(/\W/,'_') },
default_branch_override: @default_branch_override,
default_module_ref: settings.dig(:deploy, :default_module_ref),
generate_types: @generate_types || settings.dig(:deploy, :generate_types) || false,
preload_environments: true,
incremental: @incremental
Expand Down
1 change: 1 addition & 0 deletions lib/r10k/action/deploy/module.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ def initialize(opts, argv, settings = {})
overrides: {
environments: {
requested_environments: requested_env,
default_module_ref: settings.dig(:deploy, :default_module_ref),
generate_types: @generate_types
},
modules: {
Expand Down
6 changes: 3 additions & 3 deletions lib/r10k/module/git.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,15 @@ def initialize(title, dirname, opts, environment=nil)
:commit => :desired_ref,
:ref => :desired_ref,
:git => :remote,
:default_branch => :default_ref,
:default_module_ref => :default_module_ref,
:default_branch => :default_branch,
:default_branch_override => :default_override_ref,
}, :raise_on_unhandled => false)

@default_ref = @default_branch.nil? ? @default_module_ref : @default_branch
force = @overrides[:force]
@force = force == false ? false : true

@desired_ref ||= 'main'

if @desired_ref == :control_branch
if @environment && @environment.respond_to?(:ref)
@desired_ref = @environment.ref
Expand Down
2 changes: 2 additions & 0 deletions lib/r10k/module_loader/puppetfile.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ def initialize(basedir:,
@overrides = overrides
@environment = environment
@environment_name = @environment&.name
@default_module_ref = @overrides.dig(:environments, :default_module_ref)
@default_branch_override = @overrides.dig(:environments, :default_branch_override)
@allow_puppetfile_forge = @overrides.dig(:forge, :allow_puppetfile_override)

Expand Down Expand Up @@ -196,6 +197,7 @@ def parse_module_definition(name, info)
end

info[:overrides] = @overrides
info[:default_module_ref] = @default_module_ref

if @default_branch_override
info[:default_branch_override] = @default_branch_override
Expand Down
4 changes: 4 additions & 0 deletions lib/r10k/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,10 @@ def self.deploy_settings
end,
}),

Definition.new(:default_module_ref, {
:desc => "User defined default for a ref that modules default to when not otherwise specified; r10k itself only initializes this to nil. Only configurable via the r10k.yaml config.",
:default => nil}),

Definition.new(:purge_allowlist, {
:desc => "A list of filename patterns to be excluded from any purge operations. Patterns are matched relative to the root of each deployed environment, if you want a pattern to match recursively you need to use the '**' glob in your pattern. Basic shell style globs are supported.",
:default => [],
Expand Down
4 changes: 2 additions & 2 deletions spec/unit/action/deploy/module_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@

describe 'with modules' do

subject { described_class.new({ config: '/some/nonexistent/path' }, ['mod1', 'mod2'], {}) }
subject { described_class.new({ config: '/some/nonexistent/path' }, ['mod1', 'mod2'], {deploy: {default_module_ref: 'main'}}) }

let(:cache) { instance_double("R10K::Git::Cache", 'sanitized_dirname' => 'foo', 'cached?' => true, 'sync' => true) }
let(:repo) { instance_double("R10K::Git::StatefulRepository", cache: cache, resolve: 'main', tracked_paths: []) }
Expand Down Expand Up @@ -280,7 +280,7 @@
end

describe 'with environments' do
subject { described_class.new({ config: '/some/nonexistent/path', environment: 'first' }, ['mod1'], {}) }
subject { described_class.new({ config: '/some/nonexistent/path', environment: 'first' }, ['mod1'], {deploy: {default_module_ref: 'main'}}) }

let(:cache) { instance_double("R10K::Git::Cache", 'sanitized_dirname' => 'foo', 'cached?' => true, 'sync' => true) }
let(:repo) { instance_double("R10K::Git::StatefulRepository", cache: cache, resolve: 'main', tracked_paths: []) }
Expand Down
30 changes: 23 additions & 7 deletions spec/unit/module/git_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@

describe "properties" do
subject do
described_class.new('boolean', '/moduledir', {:git => 'git://git.example.com/adrienthebo/puppet-boolean'})
described_class.new('boolean', '/moduledir', {:git => 'git://git.example.com/adrienthebo/puppet-boolean',
default_module_ref: "main"})
end

before(:each) do
Expand Down Expand Up @@ -119,7 +120,7 @@
let(:title) { "#{module_org}-#{module_name}" }
let(:dirname) { Pathname.new(Dir.mktmpdir) }
let(:spec_path) { dirname + module_name + 'spec' }
subject { described_class.new(title, dirname, {}) }
subject { described_class.new(title, dirname, {default_module_ref: 'main'}) }

before(:each) do
allow(mock_repo).to receive(:resolve).with('main').and_return('abc123')
Expand Down Expand Up @@ -177,15 +178,30 @@ def test_module(extra_opts, env=nil)
allow(mock_repo).to receive(:head).and_return('abc123')
end

describe "desired ref" do
context "when no desired ref is given" do
it "defaults to main" do
expect(mock_repo).to receive(:resolve).with('main').and_return('abc123')
it "raises an argument error when no refs are supplied" do
expect{test_module({}).properties}.to raise_error(ArgumentError, /unable.*desired ref.*no default/i)
end

describe 'default_module_ref' do

context 'specifying a default_module_ref' do
let(:opts) { { default_module_ref: 'cranberry' } }
it "sets the expected ref to default_module_ref" do
expect(mock_repo).to receive(:resolve).with('cranberry').and_return('def456')
expect(test_module(opts).properties).to include(expected: 'cranberry')
end
end

expect(test_module({}).properties).to include(expected: 'main')
context 'specifying a default_module_ref and default_branch' do
let(:opts) { { default_module_ref: 'cranberry', default_branch: 'orange' } }
it "sets the expected ref to the default_branch" do
expect(mock_repo).to receive(:resolve).with('orange').and_return('def456')
expect(test_module(opts).properties).to include(expected: 'orange')
end
end
end

describe "desired ref" do
context "specifying a static desired branch" do
let(:opts) { { branch: 'banana' } }

Expand Down

0 comments on commit 4dab311

Please sign in to comment.