From 30480a12de4a1584cc74a9c80e771febd98f7fe3 Mon Sep 17 00:00:00 2001 From: OpenStack Release Bot Date: Fri, 4 Oct 2024 10:40:45 +0000 Subject: [PATCH 1/5] Update .gitreview for stable/2024.2 Change-Id: If6b98d77d6ecdbaf0460e52b5a7725e116e2d4b1 --- .gitreview | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitreview b/.gitreview index 19b5cb4f..2b4d2a0d 100644 --- a/.gitreview +++ b/.gitreview @@ -2,3 +2,4 @@ host=review.opendev.org port=29418 project=openstack/puppet-openstacklib.git +defaultbranch=stable/2024.2 From b3292c3df6e3766cc262a468bfb93e88b597015c Mon Sep 17 00:00:00 2001 From: OpenStack Release Bot Date: Fri, 4 Oct 2024 10:40:46 +0000 Subject: [PATCH 2/5] Update TOX_CONSTRAINTS_FILE for stable/2024.2 Update the URL to the upper-constraints file to point to the redirect rule on releases.openstack.org so that anyone working on this branch will switch to the correct upper-constraints list automatically when the requirements repository branches. Until the requirements repository has as stable/2024.2 branch, tests will continue to use the upper-constraints list on master. Change-Id: I7918c2b3593fe76ed4d050c3c7f1e224004eea89 --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 5bf76d23..7e063274 100644 --- a/tox.ini +++ b/tox.ini @@ -8,6 +8,6 @@ ignore_basepython_conflict = True basepython = python3 [testenv:releasenotes] -deps = -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} +deps = -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/2024.2} -r{toxinidir}/doc/requirements.txt commands = sphinx-build -a -E -W -d releasenotes/build/doctrees -b html releasenotes/source releasenotes/build/html From a1f3e5a7702ff12f3dc8af9a4399020542d4d6aa Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Tue, 18 Feb 2025 09:42:29 +0900 Subject: [PATCH 3/5] Replace remaining use of legacy facts ... because these are no longer available in Puppet 8. Change-Id: Ic1f11b7e12d35f7381d621c6f28636a34ca14e0e (cherry picked from commit ee48d7a366f2bfb820c7f5d30919b20d8f19d329) --- spec/acceptance/mysql_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/acceptance/mysql_spec.rb b/spec/acceptance/mysql_spec.rb index 8286782f..4b3b4ca4 100644 --- a/spec/acceptance/mysql_spec.rb +++ b/spec/acceptance/mysql_spec.rb @@ -10,7 +10,7 @@ class { 'mysql::server': } - $charset = $::operatingsystem ? { + $charset = $facts['os']['name'] ? { 'Ubuntu' => 'utf8mb3', default => 'utf8', } From 0b584c8a461d01b7fce2fbe87491dd6c82c5eb87 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Sun, 6 Apr 2025 23:07:59 +0900 Subject: [PATCH 4/5] Replace deprecated File.exists? File.exists? has been deprecated since Ruby 2.1, in favor of File.exist? . Change-Id: I2ca024bd0c808e026a9f6f8e33842e8a501b9a10 (cherry picked from commit 458510026db1b2dbf277f79e47aae0039deebd47) --- lib/puppet/provider/openstack/auth.rb | 4 +-- spec/unit/provider/openstack/auth_spec.rb | 34 +++++++++++------------ 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/lib/puppet/provider/openstack/auth.rb b/lib/puppet/provider/openstack/auth.rb index 5a181da0..677c9547 100644 --- a/lib/puppet/provider/openstack/auth.rb +++ b/lib/puppet/provider/openstack/auth.rb @@ -19,7 +19,7 @@ def get_os_vars_from_env end def get_os_vars_from_cloudsfile(scope) - cloudsfile = clouds_filenames.detect { |f| File.exists? f} + cloudsfile = clouds_filenames.detect { |f| File.exist? f} unless cloudsfile.nil? { 'OS_CLOUD' => scope, @@ -32,7 +32,7 @@ def get_os_vars_from_cloudsfile(scope) def get_os_vars_from_rcfile(filename) env = {} - rcfile = [filename, '/root/openrc'].detect { |f| File.exists? f } + rcfile = [filename, '/root/openrc'].detect { |f| File.exist? f } unless rcfile.nil? File.open(rcfile).readlines.delete_if{|l| l=~ /^#|^$/ }.each do |line| # we only care about the OS_ vars from the file LP#1699950 diff --git a/spec/unit/provider/openstack/auth_spec.rb b/spec/unit/provider/openstack/auth_spec.rb index 6f33d35e..2dc0b994 100644 --- a/spec/unit/provider/openstack/auth_spec.rb +++ b/spec/unit/provider/openstack/auth_spec.rb @@ -88,7 +88,7 @@ class Puppet::Provider::Openstack::AuthTester < Puppet::Provider::Openstack describe '#get_os_vars_from_cloudsfile' do context 'with a clouds.yaml present' do it 'provides a hash' do - expect(File).to receive(:exists?).with('/etc/openstack/puppet/clouds.yaml').and_return(true) + expect(File).to receive(:exist?).with('/etc/openstack/puppet/clouds.yaml').and_return(true) response = klass.get_os_vars_from_cloudsfile('project') expect(response).to eq({ @@ -100,8 +100,8 @@ class Puppet::Provider::Openstack::AuthTester < Puppet::Provider::Openstack context 'with a admin-clouds.yaml present' do it 'provides a hash' do - expect(File).to receive(:exists?).with('/etc/openstack/puppet/clouds.yaml').and_return(false) - expect(File).to receive(:exists?).with('/etc/openstack/puppet/admin-clouds.yaml').and_return(true) + expect(File).to receive(:exist?).with('/etc/openstack/puppet/clouds.yaml').and_return(false) + expect(File).to receive(:exist?).with('/etc/openstack/puppet/admin-clouds.yaml').and_return(true) response = klass.get_os_vars_from_cloudsfile('project') expect(response).to eq({ @@ -113,8 +113,8 @@ class Puppet::Provider::Openstack::AuthTester < Puppet::Provider::Openstack context 'with a clouds.yaml not present' do it 'provides an empty hash' do - expect(File).to receive(:exists?).with('/etc/openstack/puppet/clouds.yaml').and_return(false) - expect(File).to receive(:exists?).with('/etc/openstack/puppet/admin-clouds.yaml').and_return(false) + expect(File).to receive(:exist?).with('/etc/openstack/puppet/clouds.yaml').and_return(false) + expect(File).to receive(:exist?).with('/etc/openstack/puppet/admin-clouds.yaml').and_return(false) response = klass.get_os_vars_from_cloudsfile('project') expect(response).to eq({}) @@ -127,7 +127,7 @@ class Puppet::Provider::Openstack::AuthTester < Puppet::Provider::Openstack it 'provides a hash' do content = "export OS_USERNAME='test'\nexport OS_PASSWORD='abc123'\nexport OS_PROJECT_NAME='test'\nexport OS_AUTH_URL='http://127.0.0.1:5000'" filename = 'file' - expect(File).to receive(:exists?).with('file').and_return(true) + expect(File).to receive(:exist?).with('file').and_return(true) expect(File).to receive(:open).with('file').and_return(StringIO.new(content)) response = klass.get_os_vars_from_rcfile(filename) @@ -143,7 +143,7 @@ class Puppet::Provider::Openstack::AuthTester < Puppet::Provider::Openstack it 'provides a hash' do content = "export OS_USERNAME='test'\nexport OS_PASSWORD='abc123'\nexport OS_PROJECT_NAME='test'\nexport OS_AUTH_URL='http://127.0.0.1:5000'\n_openstack() {\n foo\n} " filename = 'file' - expect(File).to receive(:exists?).with('file').and_return(true) + expect(File).to receive(:exist?).with('file').and_return(true) expect(File).to receive(:open).with('file').and_return(StringIO.new(content)) response = klass.get_os_vars_from_rcfile(filename) @@ -158,7 +158,7 @@ class Puppet::Provider::Openstack::AuthTester < Puppet::Provider::Openstack context 'with an empty file' do it 'provides an empty hash' do filename = 'file' - expect(File).to receive(:exists?).with(filename).and_return(true) + expect(File).to receive(:exist?).with(filename).and_return(true) expect(File).to receive(:open).with(filename).and_return(StringIO.new("")) response = klass.get_os_vars_from_rcfile(filename) @@ -172,8 +172,8 @@ class Puppet::Provider::Openstack::AuthTester < Puppet::Provider::Openstack content = "export OS_USERNAME='user'\nexport OS_PASSWORD='secret'\nexport OS_PROJECT_NAME='project'\nexport OS_AUTH_URL='http://127.0.0.1:5000'" filename = '/root/openrc' - expect(File).to receive(:exists?).with("#{ENV['HOME']}/openrc").and_return(false) - expect(File).to receive(:exists?).with(filename).and_return(true) + expect(File).to receive(:exist?).with("#{ENV['HOME']}/openrc").and_return(false) + expect(File).to receive(:exist?).with(filename).and_return(true) expect(File).to receive(:open).with(filename).and_return(StringIO.new(content)) expect(klass.get_os_vars_from_rcfile("#{ENV['HOME']}/openrc")).to eq({ @@ -252,7 +252,7 @@ class Puppet::Provider::Openstack::AuthTester expect(klass).to receive(:get_os_vars_from_env) .and_return({ 'OS_USERNAME' => 'incompleteusername', 'OS_AUTH_URL' => 'incompleteauthurl' }) - expect(File).to receive(:exists?).with('/etc/openstack/puppet/clouds.yaml').and_return(true) + expect(File).to receive(:exist?).with('/etc/openstack/puppet/clouds.yaml').and_return(true) expect(klass).to receive(:openstack) .with('project', 'list', '--quiet', '--format', 'csv', ['--long']) .and_return('"ID","Name","Description","Enabled" @@ -275,9 +275,9 @@ class Puppet::Provider::Openstack::AuthTester .and_return({ 'OS_USERNAME' => 'incompleteusername', 'OS_AUTH_URL' => 'incompleteauthurl' }) content = "export OS_USERNAME='test'\nexport OS_PASSWORD='abc123'\nexport OS_PROJECT_NAME='test'\nexport OS_AUTH_URL='http://127.0.0.1:5000'\nexport OS_NOT_VALID='notvalid'" - expect(File).to receive(:exists?).with("/etc/openstack/puppet/clouds.yaml").and_return(false) - expect(File).to receive(:exists?).with("/etc/openstack/puppet/admin-clouds.yaml").and_return(false) - expect(File).to receive(:exists?).with("#{ENV['HOME']}/openrc").and_return(true) + expect(File).to receive(:exist?).with("/etc/openstack/puppet/clouds.yaml").and_return(false) + expect(File).to receive(:exist?).with("/etc/openstack/puppet/admin-clouds.yaml").and_return(false) + expect(File).to receive(:exist?).with("#{ENV['HOME']}/openrc").and_return(true) expect(File).to receive(:open).with("#{ENV['HOME']}/openrc").and_return(StringIO.new(content)) expect(klass).to receive(:openstack) .with('project', 'list', '--quiet', '--format', 'csv', ['--long']) @@ -302,9 +302,9 @@ class Puppet::Provider::Openstack::AuthTester expect(klass).to receive(:get_os_vars_from_env) .and_return({ 'OS_TOKEN' => 'incomplete' }) content = "export OS_TOKEN='test'\nexport OS_ENDPOINT='abc123'\nexport OS_NOT_VALID='notvalid'\n" - expect(File).to receive(:exists?).with("/etc/openstack/puppet/clouds.yaml").and_return(false) - expect(File).to receive(:exists?).with("/etc/openstack/puppet/admin-clouds.yaml").and_return(false) - expect(File).to receive(:exists?).with("#{ENV['HOME']}/openrc").and_return(true) + expect(File).to receive(:exist?).with("/etc/openstack/puppet/clouds.yaml").and_return(false) + expect(File).to receive(:exist?).with("/etc/openstack/puppet/admin-clouds.yaml").and_return(false) + expect(File).to receive(:exist?).with("#{ENV['HOME']}/openrc").and_return(true) expect(File).to receive(:open).with("#{ENV['HOME']}/openrc").and_return(StringIO.new(content)) expect(klass).to receive(:openstack) .with('project', 'list', '--quiet', '--format', 'csv', ['--long']) From 98e64c6dad3b6657458f3d8510e13841305bcfd0 Mon Sep 17 00:00:00 2001 From: Tobias Urdin Date: Thu, 6 Nov 2025 16:53:24 +0100 Subject: [PATCH 5/5] Replace Puppet::Util::withenv with custom version ... that filters out OS_* environment variables from the existing copied ENV and then set the passed environment variables to ENV and yield to the openstack CLI call. This has been a problem for a very long this where if you source OS_* environment in your shell and try to run Puppet the openstack CLI calls executed by the Puppet modules will pick up these environment variables causing side effects such as `openstack token issue` commands failing to test password for keystone_user resources causing a `openstack uset set` command even though the password has not changed. Conflicts: spec/unit/provider/openstack_spec.rb Change-Id: Ic4f9d7f7e8faf5ba5caaade49f10789aa8dba864 Signed-off-by: Tobias Urdin (cherry picked from commit 864f02dda63a0bb566643302b561b2dc04eb8530) (cherry picked from commit 2e85937d7503ad44156c7fd9f209d25aa0a8c8cb) (cherry picked from commit 12557ed183275edadd72c9942eef9f7fdc761f78) --- lib/puppet/provider/openstack.rb | 19 +++++++- .../notes/os-withenv-3ca466fde75f6441.yaml | 6 +++ spec/unit/provider/openstack_spec.rb | 43 ++++++++++++++++++- 3 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/os-withenv-3ca466fde75f6441.yaml diff --git a/lib/puppet/provider/openstack.rb b/lib/puppet/provider/openstack.rb index 5c261e3d..9a403fe3 100644 --- a/lib/puppet/provider/openstack.rb +++ b/lib/puppet/provider/openstack.rb @@ -78,6 +78,23 @@ def self.request_without_retry(&block) rc end + # Copy of Puppet::Util::withenv but that filters out + # env variables starting with OS_ from the existing + # environment. + # + # @param hash [Hash] Hash of environment variables + def self.os_withenv(hash) + saved = ENV.to_hash + begin + cleaned_env = ENV.to_hash.reject { |k, _| k.start_with?('OS_') } + ENV.replace(cleaned_env) + ENV.merge!(hash.transform_keys(&:to_s)) + yield + ensure + ENV.replace(saved) + end + end + # Returns an array of hashes, where the keys are the downcased CSV headers # with underscores instead of spaces # @@ -87,7 +104,7 @@ def self.request(service, action, properties, credentials=nil, options={}) env = credentials ? credentials.to_env : {} no_retry = options[:no_retry_exception_msgs] - Puppet::Util.withenv(env) do + os_withenv(env) do rv = nil end_time = current_time + request_timeout start_time = current_time diff --git a/releasenotes/notes/os-withenv-3ca466fde75f6441.yaml b/releasenotes/notes/os-withenv-3ca466fde75f6441.yaml new file mode 100644 index 00000000..e381ca63 --- /dev/null +++ b/releasenotes/notes/os-withenv-3ca466fde75f6441.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + The ``Puppet::Provider::Openstack.request`` now filters out all external + ``OS_*`` environment variables to prevent collisions with environment + variables in the shell where Puppet is running. diff --git a/spec/unit/provider/openstack_spec.rb b/spec/unit/provider/openstack_spec.rb index 81b636aa..37aa005c 100644 --- a/spec/unit/provider/openstack_spec.rb +++ b/spec/unit/provider/openstack_spec.rb @@ -80,7 +80,7 @@ end it 'uses provided credentials' do - expect(Puppet::Util).to receive(:withenv).with(credentials.to_env) + expect(provider.class).to receive(:os_withenv).with(credentials.to_env) Puppet::Provider::Openstack.request('project', 'list', ['--long'], credentials) end @@ -210,4 +210,45 @@ end end end + + describe '#os_withenv' do + around do |example| + @original_env = ENV.to_hash + example.run + ENV.replace(@original_env) + end + + it 'removes environment variables starting with OS_' do + ENV['OS_FOO'] = 'should be removed' + ENV['OTHER'] = 'should stay' + + Puppet::Provider::Openstack.os_withenv({}) do + expect(ENV.key?('OS_FOO')).to be false + expect(ENV['OTHER']).to eq('should stay') + end + end + + it 'merges the given environment variables' do + Puppet::Provider::Openstack.os_withenv({'MY_VAR' => '123'}) do + expect(ENV['MY_VAR']).to eq('123') + end + end + + it 'restores the original environment after the block' do + original = ENV.to_hash + Puppet::Provider::Openstack.os_withenv({'TEMP_VAR' => 'test'}) do + ENV['INSIDE_BLOCK'] = 'yep' + end + + expect(ENV.key?('TEMP_VAR')).to be false + expect(ENV.key?('INSIDE_BLOCK')).to be false + expect(ENV.to_hash).to eq(original) + end + + it 'handles string and symbol keys in the input hash' do + Puppet::Provider::Openstack.os_withenv({:FOO => 'bar'}) do + expect(ENV['FOO']).to eq('bar') + end + end + end end