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.

Sign up for GitHub

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

Remove puppet version pin from gemfile #3353

Merged
merged 7 commits into from
Nov 7, 2024
Merged

Remove puppet version pin from gemfile #3353

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f5f43c2
remove puppet version pin from gemfile
alex501212 Oct 22, 2024
335bbb0
update test to return value instead of error
alex501212 Oct 30, 2024
0b92e55
revert last commit
alex501212 Oct 30, 2024
9990d84
remove ruby 2 tests and update lookup spec tests
alex501212 Nov 6, 2024
4b9c2fa
remove puppet7 as test version in apply_spec
alex501212 Nov 6, 2024
7d9ac15
remove some more ruby 2 tests
alex501212 Nov 6, 2024
edf79d6
updated docs
alex501212 Nov 6, 2024
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 .github/workflows/apply.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest, windows-latest]
ruby: [2.7, 3.1]
ruby: [3.1]
runs-on: ${{ matrix.os }}
steps:
- name: Checkout repository
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/bolt_server.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest, windows-latest]
ruby: [2.7, 3.1]
ruby: [3.1]
runs-on: ${{ matrix.os }}
steps:
- name: Checkout repository
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/bolt_spec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest, windows-latest]
ruby: [2.7, 3.1]
ruby: [3.1]
runs-on: ${{ matrix.os }}
steps:
- name: Checkout repository
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/docker_transport.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
ruby: [2.7, 3.1]
ruby: [3.1]
steps:
- name: Checkout repository
uses: actions/checkout@v2
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/linux.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
ruby: [2.7, 3.1]
ruby: [3.1]
steps:
- name: Checkout repository
uses: actions/checkout@v2
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/local_transport.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest, windows-latest]
ruby: [2.7, 3.1]
ruby: [3.1]
runs-on: ${{ matrix.os }}
steps:
- name: Checkout repository
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/modules.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
ruby: [2.7, 3.1]
ruby: [3.1]
steps:
- name: Checkout repository
uses: actions/checkout@v2
Expand All @@ -39,7 +39,7 @@ jobs:
runs-on: windows-latest
strategy:
matrix:
ruby: [2.7, 3.1]
ruby: [3.1]
env:
BOLT_WINDOWS: true
steps:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/orch_transport.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest, windows-latest]
ruby: [2.7, 3.1]
ruby: [3.1]
runs-on: ${{ matrix.os }}
steps:
- name: Checkout repository
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ssh_transport.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
ruby: [2.7, 3.1]
ruby: [3.1]
steps:
- name: Checkout repository
uses: actions/checkout@v2
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/unit.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest, windows-latest]
ruby: [2.7, 3.1]
ruby: [3.1]
runs-on: ${{ matrix.os }}
steps:
- name: Checkout repository
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/windows.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
runs-on: windows-latest
strategy:
matrix:
ruby: [2.7, 3.1]
ruby: [3.1]
env:
WINDOWS_AGENTS: true
steps:
Expand Down Expand Up @@ -58,7 +58,7 @@ jobs:
runs-on: windows-latest
strategy:
matrix:
ruby: [2.7, 3.1]
ruby: [3.1]
env:
BOLT_WINDOWS: true
steps:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/winrm_transport.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
runs-on: windows-latest
strategy:
matrix:
ruby: [2.7, 3.1]
ruby: [3.1]
steps:
- name: Checkout repository
uses: actions/checkout@v2
Expand Down
3 changes: 0 additions & 3 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,3 @@ local_gemfile = File.join(__dir__, 'Gemfile.local')
if File.exist? local_gemfile
eval_gemfile local_gemfile
end

# TODO: remove this pin once we solve PE-35920
gem "puppet", '~> 7.24'
16 changes: 7 additions & 9 deletions documentation/hiera.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,10 @@ Running the plan results in an error like this:
```shell
$ bolt plan run my_project::request --targets server
Starting: plan my_project::request
Finished: plan my_project::example in 0.01 sec
{
"kind": "bolt/pal-error",
"msg": "Interpolations are not supported in lookups outside of an apply block: Undefined variable 'trusted' (file: /Users/bolt/.puppetlabs/bolt/hiera.yaml)",
"details": {
}
}
Interpolation failed with 'application', but compilation continuing;
(file & line not available)
Finished: plan my_project::request in 0.01 sec
Function lookup() did not find a value for the name 'api_key'
```

The plan run fails because the lookup takes place outside of an apply block,
Expand Down Expand Up @@ -218,8 +215,9 @@ apply block.

Interpolations are not well supported in the `plan_hierarchy` hierarchy.
Target-level data such as facts are not available to lookups outside of apply blocks,
so the normal hierarchy interpolation does not work. The `lookup()` function
only interpolates based on the variables currently in scope.
so the normal hierarchy interpolation does not work.
However, the `lookup()` function can perform lookups without error by using default
values provided by Puppet and Hiera when the necessary context is not available.

The following example demonstrates interpolating the `$application` variable
into a `plan_hierarchy` hierarchy:
Expand Down
1 change: 0 additions & 1 deletion spec/integration/apply_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
require 'bolt_spec/run'

TEST_VERSIONS = [
[7, 'puppet7'],
[8, 'puppet8']
].freeze

Expand Down
19 changes: 8 additions & 11 deletions spec/integration/lookup_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,12 @@
end
end

context 'with interpolations' do
context 'when context is not available' do
let(:hiera_config) { File.join(project, 'hiera_interpolations.yaml') }

it 'returns an error' do
it 'returns the default value' do
result = run_cli_json(cli_command + %w[key=test::interpolations])
expect(result).to include(
'kind' => 'bolt/pal-error',
'msg' => /Interpolations are not supported in lookups/
)
expect(result).to eq('test::interpolations data/common.yaml')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to ensure bolt errors here. Returning a success here is a regression.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I completely forgot about #3213 I think you can update the test to validate that the default value is used in the lookup.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this similar to other tests were we add options with a default value to the run_cli_json call?

    it 'accepts a default value' do
      options = { 'default_value' => 'default' }.to_json
      result  = run_cli_json(cli_command + %W[key=foo::bar::baz options=#{options}])
      expect(result).to eq('default')
    end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The core of the issue here is that we wanted to make it clear to users that in plan scope (specifically not in an apply block) there is not going to be the context (for example facts) necessary to resolve a hierachy that relies on that data for resolution. Previously we relied on a behavior in puppet whereby a lookup without necessary context would result in an error. Turns out that was not necessarily the correct assumption (see #3213 ). The path forward here is that a lookup attempted in plan scope (outside of an apply block) should be able to do a lookup without error using the defaults that the underlying puppet/hiera comes up with. We will need to update our tests to codify our expectations about what this behavior should be. Generally, does the test code in these fixtures https://github.com/puppetlabs/bolt/tree/main/spec/fixtures/hiera make sense to you? The idea is that we have some plan code and a bunch of hiera configurations used in the tests to validate assumptions about what lookups will resolve to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would https://github.com/puppetlabs/bolt/blob/main/spec/fixtures/hiera/modules/test/data/common.yaml need updated for this test? I'm not very familar with hiera so i'm wondering if theres any similar tests you could point me to?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests in this file largely cover the expectations for how lookups behave. The data itself in that file (common.yaml) should remain unchanged, the methods we use for doing the lookup (for example in top level plan scope) with the various configuration files we have there and the expected results will need to be refactored to ensure the defaults are expected.

This section of our docs does a decent job of explaining the basics. https://www.puppet.com/docs/bolt/latest/hiera.html You can review that and make sure that the new behavior still tracks with the docs.

end
end

Expand Down Expand Up @@ -159,11 +156,11 @@
let(:plan) { 'test::plan_lookup' }
let(:uri) { 'localhost' }

it 'raises a validation error' do
it 'errors with a missing key' do
result = run_cli_json(cli_command + %W[-t #{uri}])
expect(result).to include(
'kind' => 'bolt/pal-error',
'msg' => /Interpolations are not supported in lookups/
'msg' => "Function lookup() did not find a value for the name 'pop'"
)
end
end
Expand Down Expand Up @@ -307,9 +304,9 @@
context 'with invalid plan_hierarchy' do
let(:hiera_config) { File.join(project, 'plan_hiera_facts.yaml') }

it 'raises a validation error' do
expect { run_cli_json(%w[lookup environment] + opts) }
.to raise_error(Bolt::PAL::PALError, /Interpolations are not supported in lookups/)
it 'returns the default value' do
result = run_cli_json(%w[lookup environment] + opts)
expect(result).to eq("environment data/common.yaml")
end
end

Expand Down
Loading