From 248da466ec33cb5c19a386f67544e8bdd00a7df0 Mon Sep 17 00:00:00 2001 From: Reid Vandewiele Date: Wed, 18 Aug 2021 09:04:20 -0700 Subject: [PATCH 1/3] Update test for upgrade plan Test for when a primary, and a compiler are both given, but no replica is given --- spec/fixtures/plans/trusted-compiler.json | 13 ++++++++ spec/fixtures/plans/trusted-primary.json | 13 ++++++++ spec/plans/upgrade_spec.rb | 37 ++++++++++++++++++----- 3 files changed, 56 insertions(+), 7 deletions(-) create mode 100644 spec/fixtures/plans/trusted-compiler.json create mode 100644 spec/fixtures/plans/trusted-primary.json diff --git a/spec/fixtures/plans/trusted-compiler.json b/spec/fixtures/plans/trusted-compiler.json new file mode 100644 index 00000000..e677fcb5 --- /dev/null +++ b/spec/fixtures/plans/trusted-compiler.json @@ -0,0 +1,13 @@ +{ + "certname" : "compiler", + "extensions" : { + "pp_auth_role" : "pe_compiler", + "1.3.6.1.4.1.34380.1.3.13" : "pe_compiler", + "1.3.6.1.4.1.34380.1.1.9813" : "A" + }, + "dns-alt-names" : [ + "pe-server-d8b317-0.us-west1-a.c.davidsand.internal", + "pe-server-d8b317-0.us-west1-a.c.davidsand.internal", + "puppet" + ] +} diff --git a/spec/fixtures/plans/trusted-primary.json b/spec/fixtures/plans/trusted-primary.json new file mode 100644 index 00000000..792cb076 --- /dev/null +++ b/spec/fixtures/plans/trusted-primary.json @@ -0,0 +1,13 @@ +{ + "certname" : "primary", + "extensions" : { + "1.3.6.1.4.1.34380.1.3.39" : "true", + "1.3.6.1.4.1.34380.1.1.9812" : "puppet/primary", + "1.3.6.1.4.1.34380.1.1.9813" : "A" + }, + "dns-alt-names" : [ + "pe-server-d8b317-0.us-west1-a.c.davidsand.internal", + "pe-server-d8b317-0.us-west1-a.c.davidsand.internal", + "puppet" + ] +} diff --git a/spec/plans/upgrade_spec.rb b/spec/plans/upgrade_spec.rb index 71dae11c..cb1e3493 100644 --- a/spec/plans/upgrade_spec.rb +++ b/spec/plans/upgrade_spec.rb @@ -4,20 +4,43 @@ # Include the BoltSpec library functions include BoltSpec::Plans - let(:trustedjson) do - JSON.parse File.read(File.expand_path(File.join(fixtures, 'plans', 'trusted_facts.json'))) - end - - it 'minimum variables to run' do + def allow_standard_non_returning_calls allow_apply allow_any_task allow_any_plan allow_any_command allow_out_message + end + + let(:trusted_primary) do + JSON.parse File.read(File.expand_path(File.join(fixtures, 'plans', 'trusted-primary.json'))) + end + + let(:trusted_compiler) do + JSON.parse File.read(File.expand_path(File.join(fixtures, 'plans', 'trusted-compiler.json'))) + end + + it 'minimum variables to run' do + allow_standard_non_returning_calls + + expect_task('peadm::read_file').always_return({ 'content' => 'mock' }) + expect_task('peadm::cert_data').return_for_targets('primary' => trusted_primary) + + expect(run_plan('peadm::upgrade', + 'primary_host' => 'primary', + 'version' => '2019.8.6')).to be_ok + end + + it 'runs with a primary, compilers, but no replica' do + allow_standard_non_returning_calls - expect_task('peadm::cert_data').return_for_targets('primary' => trustedjson) expect_task('peadm::read_file').always_return({ 'content' => 'mock' }) + expect_task('peadm::cert_data').return_for_targets('primary' => trusted_primary, + 'compiler' => trusted_compiler) - expect(run_plan('peadm::upgrade', 'primary_host' => 'primary', 'version' => '2019.8.6')).to be_ok + expect(run_plan('peadm::upgrade', + 'primary_host' => 'primary', + 'compiler_hosts' => 'compiler', + 'version' => '2019.8.6')).to be_ok end end From 8445f33ecda6b155675e17155dc8ebb7b1e8f7f3 Mon Sep 17 00:00:00 2001 From: Reid Vandewiele Date: Wed, 18 Aug 2021 08:31:51 -0700 Subject: [PATCH 2/3] Use Target object as key in cert_exts hash Previously, the $target.name was used as the hash key. This was originally done to work around a bug in an older version of Bolt; it is not necessary to use the .name string as the hash key. Using .name makes it more difficult to deal with when there are no actual Target objects in an array. So let's stop doing that. --- plans/upgrade.pp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/plans/upgrade.pp b/plans/upgrade.pp index 5c0832aa..04920e96 100644 --- a/plans/upgrade.pp +++ b/plans/upgrade.pp @@ -81,7 +81,7 @@ # Gather certificate extension information from all systems $cert_extensions = run_task('peadm::cert_data', $all_targets).reduce({}) |$memo,$result| { - $memo + { $result.target.name => $result['extensions'] } + $memo + { $result.target => $result['extensions'] } } $convert_targets = $cert_extensions.filter |$name,$exts| { @@ -106,13 +106,13 @@ # Determine which compilers are associated with which DR group $compiler_m1_targets = $compiler_targets.filter |$target| { - ($cert_extensions[$target.name][peadm::oid('peadm_availability_group')] - == $cert_extensions[$primary_target[0].name][peadm::oid('peadm_availability_group')]) + ($cert_extensions[$target][peadm::oid('peadm_availability_group')] + == $cert_extensions[$primary_target[0]][peadm::oid('peadm_availability_group')]) } $compiler_m2_targets = $compiler_targets.filter |$target| { - ($cert_extensions[$target.name][peadm::oid('peadm_availability_group')] - == $cert_extensions[$replica_target[0].name][peadm::oid('peadm_availability_group')]) + ($cert_extensions[$target][peadm::oid('peadm_availability_group')] + == $cert_extensions[$replica_target[0]][peadm::oid('peadm_availability_group')]) } $primary_target.peadm::fail_on_transport('pcp') From 8ef9bd52f22b588c2f150e80f1e62110ab819245 Mon Sep 17 00:00:00 2001 From: Reid Vandewiele Date: Wed, 18 Aug 2021 08:41:26 -0700 Subject: [PATCH 3/3] Switch to using .dig for checking cert exts This fixes the situation where there is no replica, so computing the second compiler group cannot be done due to hitting an array indexing issue. By switching to dig, undef will simply be returned in that situation. --- plans/upgrade.pp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/plans/upgrade.pp b/plans/upgrade.pp index 04920e96..c8733947 100644 --- a/plans/upgrade.pp +++ b/plans/upgrade.pp @@ -95,7 +95,8 @@ # Ensure needed trusted facts are available if $cert_extensions.any |$_,$cert| { - [peadm::oid('peadm_role'), 'pp_auth_role'].all |$ext| { $cert[$ext] == undef } + [peadm::oid('peadm_role'), 'pp_auth_role'].all |$ext| { $cert[$ext] == undef } or + $cert[peadm::oid('peadm_availability_group')] == undef } { fail_plan(@(HEREDOC/L)) Required trusted facts are not present; upgrade cannot be completed. If \ @@ -106,13 +107,13 @@ # Determine which compilers are associated with which DR group $compiler_m1_targets = $compiler_targets.filter |$target| { - ($cert_extensions[$target][peadm::oid('peadm_availability_group')] - == $cert_extensions[$primary_target[0]][peadm::oid('peadm_availability_group')]) + ($cert_extensions.dig($target, peadm::oid('peadm_availability_group')) + == $cert_extensions.dig($primary_target[0], peadm::oid('peadm_availability_group'))) } $compiler_m2_targets = $compiler_targets.filter |$target| { - ($cert_extensions[$target][peadm::oid('peadm_availability_group')] - == $cert_extensions[$replica_target[0]][peadm::oid('peadm_availability_group')]) + ($cert_extensions.dig($target, peadm::oid('peadm_availability_group')) + == $cert_extensions.dig($replica_target[0], peadm::oid('peadm_availability_group'))) } $primary_target.peadm::fail_on_transport('pcp')