Merge pull request #8940 from joshcooper/legacy_facts_11662

(PUP-11662) Allow legacy facts to be excluded

Author: tvpartytonight

lib/puppet/defaults.rb

@@ -1712,6 +1712,14 @@ def self.initialize_default_settings!(settings)
         new configurations, where you want to fix the broken configuration
         rather than reverting to a known-good one.",
     },
+    :include_legacy_facts => {
+      :type       => :boolean,
+      :default    => true,
+      :desc       => "Whether to include legacy facts when requesting a catalog. This
+        option can be set to false provided all puppet manifests, hiera.yaml and hiera
+        configuration layers no longer access legacy facts, such as `$osfamily`, and
+        instead access structured facts, such as `$facts['os']['family']`."
+    },
     :fact_name_length_soft_limit => {
       :default    => 2560,
       :type       => :integer,

lib/puppet/indirector/facts/facter.rb

@@ -35,8 +35,12 @@ def find(request)
     result = if request.options[:resolve_options]
                raise(Puppet::Error, _("puppet facts show requires version 4.0.40 or greater of Facter.")) unless Facter.respond_to?(:resolve)
                find_with_options(request)
-             else
+             elsif Puppet[:include_legacy_facts]
+               # to_hash returns both structured and legacy facts
                Puppet::Node::Facts.new(request.key, Puppet.runtime[:facter].to_hash)
+             else
+               # resolve does not return legacy facts unless requested
+               Puppet::Node::Facts.new(request.key, Puppet.runtime[:facter].resolve(''))
              end
 
     result.add_local_facts unless request.options[:resolve_options]

spec/integration/application/agent_spec.rb

@@ -897,4 +897,103 @@ def copy_fixtures(sources, dest)
       end
     end
   end
+
+  context "legacy facts" do
+    let(:mod_dir) { tmpdir('module_dir') }
+    let(:custom_dir) { File.join(mod_dir, 'lib') }
+    let(:external_dir) { File.join(mod_dir, 'facts.d') }
+
+    before(:each) do
+      # don't stub facter behavior, since we're relying on
+      # `Facter.resolve` to omit legacy facts
+      Puppet::Node::Facts.indirection.terminus_class = :facter
+
+      facter_dir = File.join(custom_dir, 'facter')
+      FileUtils.mkdir_p(facter_dir)
+      File.write(File.join(facter_dir, 'custom.rb'), <<~END)
+        Facter.add(:custom) { setcode { 'a custom value' } }
+      END
+
+      FileUtils.mkdir_p(external_dir)
+      File.write(File.join(external_dir, 'external.json'), <<~END)
+        {"external":"an external value"}
+      END
+
+      # avoid pluginsync'ing contents
+      FileUtils.mkdir_p(Puppet[:vardir])
+      FileUtils.cp_r(custom_dir, Puppet[:vardir])
+      FileUtils.cp_r(external_dir, Puppet[:vardir])
+    end
+
+    def mounts
+      {
+        # the server needs to provide metadata that matches what the agent has
+        # so that the agent doesn't delete them during pluginsync
+        file_metadatas: -> (req, res) {
+          path = case req.path
+                 when /pluginfacts/
+                   external_dir
+                 when /plugins/
+                   custom_dir
+                 else
+                   raise "Unknown mount #{req.path}"
+                 end
+          request = Puppet::FileServing::Metadata.indirection.request(
+            :search, path, nil, recurse: true
+          )
+          data = Puppet::FileServing::Metadata.indirection.terminus(:file).search(request)
+          res.body = formatter.render(data)
+          res['Content-Type'] = formatter.mime
+        },
+        catalog: -> (req, res) {
+          data = CGI.unescape(req.query['facts'])
+          facts = Puppet::Node::Facts.convert_from('json', data)
+          node.fact_merge(facts)
+
+          catalog = compile_to_catalog(<<~MANIFEST, node)
+            notify { "legacy $osfamily": }
+            notify { "custom ${facts['custom']}": }
+            notify { "external ${facts['external']}": }
+          MANIFEST
+
+          res.body = formatter.render(catalog)
+          res['Content-Type'] = formatter.mime
+        }
+      }
+    end
+
+    it "includes legacy facts by default" do
+      server.start_server(mounts: mounts) do |port|
+        Puppet[:serverport] = port
+
+        agent.command_line.args << '--test'
+        expect {
+          agent.run
+        }.to exit_with(2)
+          .and output(
+            match(/defined 'message' as 'legacy [A-Za-z]+'/)
+              .and match(/defined 'message' as 'custom a custom value'/)
+              .and match(/defined 'message' as 'external an external value'/)
+          ).to_stdout
+      end
+    end
+
+    it "can exclude legacy facts" do
+      server.start_server(mounts: mounts) do |port|
+        Puppet[:serverport] = port
+        Puppet[:include_legacy_facts] = false
+
+        agent.command_line.args << '--test'
+        expect {
+          agent.run
+        }.to exit_with(2)
+          .and output(
+            match(/defined 'message' as 'legacy '/)
+              .and match(/defined 'message' as 'custom a custom value'/)
+              .and match(/defined 'message' as 'external an external value'/)
+            ).to_stdout
+          .and output(/Warning: Unknown variable: 'osfamily'/).to_stderr
+      end
+    end
+  end
 end

spec/unit/indirector/facts/facter_spec.rb

@@ -83,6 +83,16 @@
 
       @facter.find(@request)
     end
+
+    it "can exclude legacy facts" do
+      Puppet[:include_legacy_facts] = false
+
+      facts = Puppet::Node::Facts.new("foo")
+      expect(Puppet::Node::Facts).to receive(:new).and_return(facts)
+      expect(Puppet.runtime[:facter]).to receive(:resolve).with('')
+
+      @facter.find(@request)
+    end
   end
 
   it 'should fail when saving facts' do