improve matching resiliency of puppet endpoints (#12402)

this PR modifies the `external_host_identifier` parameter that's used to
match hosts to Puppet runs to use an identifier that's *unique per run*
(instead of an identifier that's *unique per host*)

this has the adventage to:

1. allow for concurrent Puppet runs that don't interfere with each
other.
2. allow for failed/orphaned Puppet runs to not interfere with new runs
(the keys will eventually get expired)

all the existent behavior should be preserved.

> Note: I have verified that the value that the reporter gets is the one
  associated with the right puppet run, even if multiple runs happen
  simultaneously.

ee/tools/puppet/fleetdm/lib/puppet/functions/fleetdm/preassign_profile.rb

@@ -14,7 +14,8 @@ Puppet::Functions.create_function(:"fleetdm::preassign_profile") do
     host = call_function('lookup', 'fleetdm::host')
     token = call_function('lookup', 'fleetdm::token')
     client = Puppet::Util::FleetClient.new(host, token)
-    response = client.preassign_profile(host_uuid, template, group)
+    run_identifier = "#{closure_scope.catalog.catalog_uuid}-#{Puppet[:node_name_value]}"
+    response = client.preassign_profile(run_identifier, host_uuid, template, group)
 
     if response['error'].empty?
       Puppet.info("successfully pre-assigned profile #{profile_identifier}")

ee/tools/puppet/fleetdm/lib/puppet/reports/fleetdm.rb

@@ -17,7 +17,8 @@ Puppet::Reports.register_report(:fleetdm) do
     token = Puppet::Pops::Lookup.lookup('fleetdm::token', nil, '', false, nil, lookup_invocation)
 
     client = Puppet::Util::FleetClient.new(host, token)
-    response = client.match_profiles
+    run_identifier = "#{catalog_uuid}-#{node_name}"
+    response = client.match_profiles(run_identifier)
 
     if response['error'].empty?
       Puppet.info("successfully matched #{node_name} with a team containing configuration profiles")

ee/tools/puppet/fleetdm/lib/puppet/util/fleet_client.rb

@@ -15,15 +15,16 @@ module Puppet::Util
     # Pre-assigns a profile to a host. Note that the profile assignment is not
     # effective until the sibling `match_profiles` method is called.
     #
+    # @param run_identifier [String] Used to identify this run during profile matching.
     # @param uuid [String] The host uuid.
     # @param profile_xml [String] Raw XML with the configuration profile.
     # @param group [String] Used to construct a team name.
     # @return [Hash] The response status code, headers, and body.
-    def preassign_profile(uuid, profile_xml, group)
+    def preassign_profile(run_identifier, uuid, profile_xml, group)
       post(
         '/api/latest/fleet/mdm/apple/profiles/preassign',
         {
-          'external_host_identifier' => Puppet[:node_name_value],
+          'external_host_identifier' => run_identifier,
           'host_uuid' => uuid,
           'profile' => Base64.strict_encode64(profile_xml),
           'group' => group,
@@ -37,11 +38,13 @@ module Puppet::Util
     # It uses `Puppet[:node_name_value]` as the `external_host_identifier`,
     # which is unique per Puppet host.
     #
+    # @param run_identifier [String] Used to identify this run to match
+    # pre-assigned profiles.
     # @return [Hash] The response status code, headers, and body.
-    def match_profiles
+    def match_profiles(run_identifier)
       post('/api/latest/fleet/mdm/apple/profiles/match',
   {
-    'external_host_identifier' => Puppet[:node_name_value],
+    'external_host_identifier' => run_identifier,
   })
     end
 

ee/tools/puppet/fleetdm/metadata.json

@@ -1,6 +1,6 @@
 {
   "name": "root-fleetdm",
-  "version": "0.1.1",
+  "version": "0.1.2",
   "author": "Fleet Device Management Inc",
   "summary": "",
   "license": "proprietary",

ee/tools/puppet/fleetdm/spec/defines/profile_spec.rb

@@ -7,7 +7,9 @@ describe 'fleetdm::profile' do
   let(:title) { 'namevar' }
   let(:template) { 'test-template' }
   let(:group) { 'group' }
-  let(:node) { 'testhost.example.com' }
+  let(:node_name) { Puppet[:node_name_value] }
+  let(:catalog_uuid) { '827a74c8-cf98-44da-9ff7-18c5e4bee41e' }
+  let(:run_identifier) { "#{catalog_uuid}-#{node_name}" }
   let(:params) do
     { 'template' => template, 'group' => group }
   end
@@ -16,15 +18,16 @@ describe 'fleetdm::profile' do
     fleet_client_class = class_spy('Puppet::Util::FleetClient')
     stub_const('Puppet::Util::FleetClient', fleet_client_class)
     allow(fleet_client_class).to receive(:new).with('https://example.com', 'test_token') { fleet_client_mock }
+    allow(SecureRandom).to receive(:uuid).and_return(catalog_uuid)
   end
 
   on_supported_os.each do |os, os_facts|
     context "on #{os}" do
-      let(:facts) { os_facts }
+      let(:facts) { os_facts.merge({}) }
 
       it 'compiles' do
         uuid = os_facts[:system_profiler]['hardware_uuid']
-        expect(fleet_client_mock).to receive(:preassign_profile).with(uuid, template, group)
+        expect(fleet_client_mock).to receive(:preassign_profile).with(run_identifier, uuid, template, group).and_return({ 'error' => '' })
         is_expected.to compile
       end
 
@@ -59,7 +62,7 @@ describe 'fleetdm::profile' do
 
         it 'compiles' do
           uuid = os_facts[:system_profiler]['hardware_uuid']
-          expect(fleet_client_mock).to receive(:preassign_profile).with(uuid, template, 'default')
+          expect(fleet_client_mock).to receive(:preassign_profile).with(run_identifier, uuid, template, 'default').and_return({ 'error' => '' })
           is_expected.to compile
         end
       end

ee/tools/puppet/fleetdm/spec/functions/fleet_client_spec.rb

@@ -7,11 +7,9 @@ describe 'Puppet::Util::FleetClient' do
 
   it 'handles POST with 204 responses' do
     response = Net::HTTPSuccess.new(1.0, '204', 'OK')
-    expect_any_instance_of(Net::HTTP).to receive(:request) { response }
-    expect(response).to receive(:body) { nil }
+    expect_any_instance_of(Net::HTTP).to receive(:request) { response } # rubocop:disable RSpec/AnyInstance
 
     result = client.post('/example')
-    expect(result[:status]).to be(204)
     expect(result[:body]).to be(nil)
   end
 end

ee/tools/puppet/fleetdm/spec/functions/preassign_profile_spec.rb

@@ -7,22 +7,27 @@ describe 'fleetdm::preassign_profile' do
   let(:device_uuid) { 'device-uuid' }
   let(:template) { 'template' }
   let(:group) { 'group' }
+  let(:node_name) { Puppet[:node_name_value] }
+  let(:catalog_uuid) { '827a74c8-cf98-44da-9ff7-18c5e4bee41e' }
+  let(:run_identifier) { "#{catalog_uuid}-#{node_name}" }
+  let(:profile_identifier) { 'test.example.com' }
 
   before(:each) do
     fleet_client_class = class_spy('Puppet::Util::FleetClient')
     stub_const('Puppet::Util::FleetClient', fleet_client_class)
     allow(fleet_client_class).to receive(:new).with('https://example.com', 'test_token') { fleet_client_mock }
+    allow(SecureRandom).to receive(:uuid).and_return(catalog_uuid)
   end
 
   it { is_expected.to run.with_params(nil).and_raise_error(StandardError) }
 
   it 'performs an API call to Fleet with the right parameters' do
-    expect(fleet_client_mock).to receive(:preassign_profile).with(device_uuid, template, group)
-    is_expected.to run.with_params(device_uuid, template, group)
+    expect(fleet_client_mock).to receive(:preassign_profile).with(run_identifier, device_uuid, template, group).and_return({ 'error' => '' })
+    is_expected.to run.with_params(profile_identifier, device_uuid, template, group)
   end
 
   it 'has a default value if group is not provided' do
-    expect(fleet_client_mock).to receive(:preassign_profile).with(device_uuid, template, 'default')
-    is_expected.to run.with_params(device_uuid, template)
+    expect(fleet_client_mock).to receive(:preassign_profile).with(run_identifier, device_uuid, template, 'default').and_return({ 'error' => '' })
+    is_expected.to run.with_params(profile_identifier, device_uuid, template)
   end
 end

ee/tools/puppet/fleetdm/spec/functions/release_device_spec.rb

@@ -16,7 +16,7 @@ describe 'fleetdm::release_device' do
   it { is_expected.to run.with_params(nil).and_raise_error(StandardError) }
 
   it 'performs an API call to Fleet' do
-    expect(fleet_client_mock).to receive(:send_mdm_command).with(device_uuid, %r{DeviceConfigured})
+    expect(fleet_client_mock).to receive(:send_mdm_command).with(device_uuid, %r{DeviceConfigured}).and_return({ 'error' => '' })
     is_expected.to run.with_params(device_uuid)
   end
 end