fix puppet tests and add CI workers (#16529)

for #16059

.github/workflows/test-puppet.yml

@@ -0,0 +1,54 @@
+name: Test Puppet
+
+on:
+  push:
+    branches:
+      - main
+      - patch-*
+  pull_request:
+    paths:
+      - 'ee/tools/puppet/fleetdm/*.*'
+      - '.github/workflows/test-puppet.yml'
+  workflow_dispatch: # Manual
+
+# This allows a subsequently queued workflow run to interrupt previous runs
+concurrency:
+  group: ${{ github.workflow }}-${{ github.head_ref || github.run_id}}
+  cancel-in-progress: true
+
+defaults:
+  run:
+    # fail-fast using bash -eo pipefail. See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#exit-codes-and-error-action-preference
+    shell: bash
+
+permissions:
+  contents: read
+
+jobs:
+  test-puppet:
+    runs-on: macos-latest
+    steps:
+    - name: Install Puppet Development Kit
+      run: brew install --cask puppetlabs/puppet/pdk 
+
+    - name: Checkout Code
+      uses: actions/checkout@629c2de402a417ea7690ca6ce3f33229e27606a5 # v2
+      with:
+        fetch-depth: 0
+
+    - name: Install Ruby Gems
+      working-directory: ./ee/tools/puppet/fleetdm/
+      run: /opt/puppetlabs/pdk/bin/pdk bundle install
+
+    - name: Run Tests
+      working-directory: ./ee/tools/puppet/fleetdm/
+      run: /opt/puppetlabs/pdk/bin/pdk test unit
+
+    - name: Run Rubocop
+      working-directory: ./ee/tools/puppet/fleetdm/
+      run: /opt/puppetlabs/pdk/bin/pdk bundle exec rubocop
+
+    - name: Run Linter
+      working-directory: ./ee/tools/puppet/fleetdm/
+      run: /opt/puppetlabs/pdk/bin/pdk bundle exec puppet-lint .
+      

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

@@ -14,7 +14,7 @@ Puppet::Functions.create_function(:"fleetdm::command_xml") do
   end
 
   def command_xml(uuid, xml_data)
-    env = closure_scope['server_facts']['environment']
+    env = closure_scope['environment']
     client = Puppet::Util::FleetClient.instance
     response = client.send_mdm_command(uuid, xml_data, env)
 

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

@@ -16,11 +16,11 @@ Puppet::Functions.create_function(:"fleetdm::preassign_profile") do
     preassign_profile_response = { 'error' => '', 'resource_changed' => false }
 
     client = Puppet::Util::FleetClient.instance
-    env = closure_scope['server_facts']['environment']
+    env = closure_scope['environment']
     run_identifier = "#{closure_scope.catalog.catalog_uuid}-#{Puppet[:node_name_value]}"
 
     # initiate the pre-assignment process with fleet server
-    client_resp = client.preassign_profile(run_identifier, host_uuid, template, group, ensure_profile, closure_scope['environment'])
+    client_resp = client.preassign_profile(run_identifier, host_uuid, template, group, ensure_profile, env)
     if check_error(client_resp)
       return handle_error("Error pre-assigning profile #{profile_identifier} (ensure #{ensure_profile})", client_resp['error'], template, preassign_profile_response)
     end

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

@@ -29,7 +29,7 @@ Puppet::Functions.create_function(:"fleetdm::release_device") do
       </plist>
     COMMAND_TEMPLATE
 
-    env = closure_scope['server_facts']['environment']
+    env = closure_scope['environment']
     client = Puppet::Util::FleetClient.instance
     response = client.send_mdm_command(uuid, command_xml, env)
 

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

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'spec_helper'
+require_relative '../../lib/puppet/util/fleet_client.rb'
 
 describe 'fleetdm::profile' do
   let(:fleet_client_mock) { instance_double('Puppet::Util::FleetClient') }
@@ -11,14 +12,10 @@ describe 'fleetdm::profile' do
   let(:catalog_uuid) { '827a74c8-cf98-44da-9ff7-18c5e4bee41e' }
   let(:run_identifier) { "#{catalog_uuid}-#{node_name}" }
   let(:host_response) { { 'host' => { 'id' => 1 } } }
-  let(:params) do
-    { 'template' => template, 'group' => group }
-  end
+  let(:rspec_puppet_env) { 'rp_env' }
 
   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(:instance) { fleet_client_mock }
+    allow(Puppet::Util::FleetClient).to receive(:instance).and_return(fleet_client_mock)
     allow(SecureRandom).to receive(:uuid).and_return(catalog_uuid)
   end
 
@@ -26,24 +23,10 @@ describe 'fleetdm::profile' do
     context "on #{os}" do
       let(:facts) { os_facts.merge({}) }
 
-      it 'compiles' do
-        uuid = os_facts[:system_profiler]['hardware_uuid']
-        expect(fleet_client_mock)
-          .to receive(:get_host_by_identifier)
-          .with(uuid, 'production')
-          .and_return({ 'error' => '', 'body' => host_response })
-        expect(fleet_client_mock)
-          .to receive(:get_host_profiles)
-          .with(host_response['host']['id'], 'production')
-          .and_return({ 'error' => '', 'body' => { 'profiles' => [] } })
-        expect(fleet_client_mock)
-          .to receive(:preassign_profile)
-          .with(run_identifier, uuid, template, group, 'present', 'production')
-          .and_return({ 'error' => '' })
-        is_expected.to compile
-      end
-
       context 'noop' do
+        let(:params) do
+          { 'template' => template, 'group' => group }
+        end
         let(:facts) { { 'clientnoop' => true } }
 
         it 'does not send a request in noop mode' do
@@ -75,6 +58,54 @@ describe 'fleetdm::profile' do
         it { is_expected.to compile.and_raise_error(%r{'ensure' expects a match for Enum\['absent', 'present'\]}) }
       end
 
+      # FIXME(roberto): for some reason I don't understand, the first call to
+      # any method that uses Puppet::Util::FleetClient always uses the real
+      # class instead of the double, but all subsequent calls do.
+      #
+      # My theory is that this class is pre-loaded by some mechanism that I
+      # haven't figured out how to tweak. This hack is a "dummy" test that does
+      # nothing and the only purpose is to cleanup that pre-loaded version.
+      context 'clean preloaded mock' do
+        let(:params) do
+          { 'template' => template, 'group' => group }
+        end
+
+        it 'compiles' do
+          expect(fleet_client_mock)
+            .not_to receive(:get_host_by_identifier)
+          expect(fleet_client_mock)
+            .not_to receive(:get_host_profiles)
+          expect(fleet_client_mock)
+            .not_to receive(:preassign_profile)
+
+          is_expected.to compile
+        end
+      end
+
+      context 'with different template' do
+        let(:params) do
+          { 'template' => 'foo', 'group' => group }
+        end
+
+        it 'compiles 2' do
+          uuid = os_facts[:system_profiler]['hardware_uuid']
+          expect(fleet_client_mock)
+            .to receive(:get_host_by_identifier)
+            .with(uuid, rspec_puppet_env)
+            .and_return({ 'error' => '', 'body' => host_response })
+          expect(fleet_client_mock)
+            .to receive(:get_host_profiles)
+            .with(host_response['host']['id'], rspec_puppet_env)
+            .and_return({ 'error' => '', 'body' => { 'profiles' => [] } })
+          expect(fleet_client_mock)
+            .to receive(:preassign_profile)
+            .with(run_identifier, uuid, 'foo', group, 'present', rspec_puppet_env)
+            .and_return({ 'error' => '' })
+
+          is_expected.to compile
+        end
+      end
+
       context 'without group' do
         let(:params) do
           { 'template' => template }
@@ -82,23 +113,24 @@ describe 'fleetdm::profile' do
 
         it 'compiles' do
           uuid = os_facts[:system_profiler]['hardware_uuid']
+          expect(fleet_client_mock)
+            .to receive(:preassign_profile)
+            .with(run_identifier, uuid, template, 'default', 'present', rspec_puppet_env)
+            .and_return({ 'error' => '' })
           expect(fleet_client_mock)
             .to receive(:get_host_by_identifier)
-            .with(uuid, 'production')
+            .with(uuid, rspec_puppet_env)
             .and_return({ 'error' => '', 'body' => host_response })
           expect(fleet_client_mock)
             .to receive(:get_host_profiles)
-            .with(host_response['host']['id'], 'production')
+            .with(host_response['host']['id'], rspec_puppet_env)
             .and_return({ 'error' => '', 'body' => { 'profiles' => [] } })
-          expect(fleet_client_mock)
-            .to receive(:preassign_profile)
-            .with(run_identifier, uuid, template, 'default', 'present', 'production')
-            .and_return({ 'error' => '' })
           is_expected.to compile
         end
       end
 
       context 'ensure => absent' do
+        let(:facts) { os_facts.merge({}) }
         let(:params) do
           { 'template' => template, 'ensure' => 'absent' }
         end
@@ -107,15 +139,15 @@ describe 'fleetdm::profile' do
           uuid = os_facts[:system_profiler]['hardware_uuid']
           expect(fleet_client_mock)
             .to receive(:get_host_by_identifier)
-            .with(uuid, 'production')
+            .with(uuid, rspec_puppet_env)
             .and_return({ 'error' => '', 'body' => host_response })
           expect(fleet_client_mock)
             .to receive(:get_host_profiles)
-            .with(host_response['host']['id'], 'production')
+            .with(host_response['host']['id'], rspec_puppet_env)
             .and_return({ 'error' => '', 'body' => { 'profiles' => [] } })
           expect(fleet_client_mock)
             .to receive(:preassign_profile)
-            .with(run_identifier, uuid, template, 'default', 'absent', 'production')
+            .with(run_identifier, uuid, template, 'default', 'absent', rspec_puppet_env)
             .and_return({ 'error' => '' })
           is_expected.to compile
         end

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

@@ -1,12 +1,14 @@
 # frozen_string_literal: true
 
 require 'spec_helper'
+require_relative '../../lib/puppet/reports/fleetdm.rb'
 
 describe 'Puppet::Util::FleetClient' do
   let(:client) { Puppet::Util::FleetClient.instance }
   let(:host) { 'https://test.example.com' }
   let(:token) { 'supersecret' }
   let(:identifier) { 'test_ident' }
+  let(:rspec_puppet_env) { 'rp_env' }
 
   before(:each) do
     stub_const(
@@ -64,7 +66,7 @@ describe 'Puppet::Util::FleetClient' do
           request_body: { 'external_host_identifier' => identifier },
           response: instance_double(Net::HTTPSuccess, code: 204, body: nil),
         )
-        client.match_profiles(identifier, 'production')
+        client.match_profiles(identifier, rspec_puppet_env)
       end
 
       it { expect(result['body']).to eq({}) }
@@ -83,7 +85,7 @@ describe 'Puppet::Util::FleetClient' do
             body: body.to_json,
           ),
         )
-        client.match_profiles(identifier, 'production')
+        client.match_profiles(identifier, rspec_puppet_env)
       end
 
       let(:body) do

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

@@ -13,6 +13,7 @@ describe 'fleetdm::preassign_profile' do
   let(:run_identifier) { "#{catalog_uuid}-#{node_name}" }
   let(:profile_identifier) { 'test.example.com' }
   let(:host_response) { { 'host' => { 'id' => 1 } } }
+  let(:rspec_puppet_env) { 'rp_env' }
 
   before(:each) do
     fleet_client_class = class_spy('Puppet::Util::FleetClient')
@@ -26,15 +27,15 @@ describe 'fleetdm::preassign_profile' do
   it 'performs an API call to Fleet with the right parameters' do
     expect(fleet_client_mock)
       .to receive(:get_host_by_identifier)
-      .with(device_uuid, 'production')
+      .with(device_uuid, rspec_puppet_env)
       .and_return({ 'error' => '', 'body' => host_response })
     expect(fleet_client_mock)
       .to receive(:get_host_profiles)
-      .with(host_response['host']['id'], 'production')
+      .with(host_response['host']['id'], rspec_puppet_env)
       .and_return({ 'error' => '', 'body' => { 'profiles' => [] } })
     expect(fleet_client_mock)
       .to receive(:preassign_profile)
-      .with(run_identifier, device_uuid, template, group, ensure_profile, 'production')
+      .with(run_identifier, device_uuid, template, group, ensure_profile, rspec_puppet_env)
       .and_return({ 'error' => '' })
     is_expected.to run.with_params(profile_identifier, device_uuid, template, group, ensure_profile)
   end
@@ -42,15 +43,15 @@ describe 'fleetdm::preassign_profile' do
   it 'has default values for `group` and `ensure`' do
     expect(fleet_client_mock)
       .to receive(:get_host_by_identifier)
-      .with(device_uuid, 'production')
+      .with(device_uuid, rspec_puppet_env)
       .and_return({ 'error' => '', 'body' => host_response })
     expect(fleet_client_mock)
       .to receive(:get_host_profiles)
-      .with(host_response['host']['id'], 'production')
+      .with(host_response['host']['id'], rspec_puppet_env)
       .and_return({ 'error' => '', 'body' => { 'profiles' => [] } })
     expect(fleet_client_mock)
       .to receive(:preassign_profile)
-      .with(run_identifier, device_uuid, template, 'default', 'present', 'production')
+      .with(run_identifier, device_uuid, template, 'default', 'present', rspec_puppet_env)
       .and_return({ 'error' => '' })
     is_expected.to run.with_params(profile_identifier, device_uuid, template)
   end

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

@@ -1,22 +1,32 @@
 # frozen_string_literal: true
 
 require 'spec_helper'
-require 'puppet/util/fleet_client'
+require_relative '../../lib/puppet/util/fleet_client.rb'
 
 describe 'fleetdm::release_device' do
   let(:fleet_client_mock) { instance_double('Puppet::Util::FleetClient') }
   let(:device_uuid) { 'device-uuid' }
+  let(:rspec_puppet_env) { 'rp_env' }
 
   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(:instance) { fleet_client_mock }
+    allow(Puppet::Util::FleetClient).to receive(:instance).and_return(fleet_client_mock)
   end
 
-  it { is_expected.to run.with_params(nil).and_raise_error(StandardError) }
+  on_supported_os.each do |os, os_facts|
+    context "on #{os}" do
+      let(:facts) { os_facts.merge({}) }
 
-  it 'performs an API call to Fleet' do
-    expect(fleet_client_mock).to receive(:send_mdm_command).with(device_uuid, %r{DeviceConfigured}, 'production').and_return({ 'error' => '' })
-    is_expected.to run.with_params(device_uuid)
+      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) { |device_uuid_param, command_param, environment_param|
+          expect(device_uuid_param).to eq(device_uuid)
+          expect(command_param).to include('DeviceConfigured')
+          expect(environment_param).to eq(rspec_puppet_env)
+        }.and_return({ 'error' => '' })
+
+        is_expected.to run.with_params(device_uuid)
+      end
+    end
   end
 end