(PUP-9407) Disable filebuckets by default

Previously puppet stored a copy of any file it overwrote or deleted in its local
filebucket. However, the local filebucket grows unbounded over time and a cron
job or tidy resource must be used to delete old files.

Change the `backup` parameter's default value to false. The old behavior
can be reenabled by adding a resource default:

    File { backup => 'puppet' }

Author: joshcooper

acceptance/tests/ticket_1334_clientbucket_corrupted.rb

@@ -14,7 +14,7 @@
     end
 
     managed_content = "managed\n"
-    manifest = "file { '#{tmpfile}': content => '#{managed_content}', }"
+    manifest = "file { '#{tmpfile}': content => '#{managed_content}', backup => 'puppet' }"
 
     step 'create unmanaged file' do
       create_remote_file(agent, tmpfile, unmanaged_content)

acceptance/tests/ticket_6541_invalid_filebucket_files.rb

@@ -15,7 +15,7 @@
   apply_manifest_on(agent, manifest)
 
   step "overwrite file, causing zero-length file to be backed up"
-  manifest = "file { '#{target}': content => 'some text' }"
+  manifest = "file { '#{target}': content => 'some text', backup => 'puppet' }"
   apply_manifest_on(agent, manifest)
 
   test_name "verify invalid hashes should not change the file"
@@ -40,9 +40,9 @@
 
   test_name "verify that an empty file can be retrieved from the filebucket"
   if fips_mode
-    manifest = "file { '#{target}': content => '{sha256}e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' }"
+    manifest = "file { '#{target}': content => '{sha256}e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855', backup => 'puppet' }"
   else
-    manifest = "file { '#{target}': content => '{md5}d41d8cd98f00b204e9800998ecf8427e' }"
+    manifest = "file { '#{target}': content => '{md5}d41d8cd98f00b204e9800998ecf8427e', backup => 'puppet' }"
   end
 
   apply_manifest_on(agent, manifest) do

lib/puppet/type/file.rb

@@ -83,13 +83,11 @@ def self.title_patterns
         use copy the file in the same directory with that value as the extension
         of the backup. (A value of `true` is a synonym for `.puppet-bak`.)
       * If set to any other string, Puppet will try to back up to a filebucket
-        with that title. See the `filebucket` resource type for more details.
-        (This is the preferred method for backup, since it can be centralized
-        and queried.)
+        with that title. Puppet automatically creates a **local** filebucket
+        named `puppet` if one doesn't already exist. See the `filebucket` resource
+        type for more details.
 
-      Default value: `puppet`, which backs up to a filebucket of the same name.
-      (Puppet automatically creates a **local** filebucket named `puppet` if one
-      doesn't already exist.)
+      Default value: `false`
 
       Backing up to a local filebucket isn't particularly useful. If you want
       to make organized use of backups, you will generally want to use the
@@ -125,7 +123,7 @@ def self.title_patterns
         - Restrict the directory to a maximum size after which the oldest items are removed.
     EOT
 
-    defaultto "puppet"
+    defaultto false
 
     munge do |value|
       # I don't really know how this is happening.

spec/unit/type/file/content_spec.rb

@@ -7,7 +7,7 @@
   let(:filename) { tmpfile('testfile') }
   let(:environment) { Puppet::Node::Environment.create(:testing, []) }
   let(:catalog) { Puppet::Resource::Catalog.new(:test, environment) }
-  let(:resource) { Puppet::Type.type(:file).new :path => filename, :catalog => catalog }
+  let(:resource) { Puppet::Type.type(:file).new :path => filename, :catalog => catalog, :backup => 'puppet' }
 
   before do
     File.open(filename, 'w') {|f| f.write "initial file content"}

spec/unit/type/file_spec.rb

@@ -133,6 +133,10 @@
   end
 
   describe "the backup parameter" do
+    it 'should be disabled by default' do
+      expect(file[:backup]).to eq(nil)
+    end
+
     [false, 'false', :false].each do |value|
       it "should disable backup if the value is #{value.inspect}" do
         file[:backup] = value
@@ -928,7 +932,7 @@
     end
 
     it "should fail if it can't backup the file" do
-      # Default: file[:backup] = true
+      file[:backup] = true
       allow(file).to receive(:stat).and_return(double('stat', :ftype => 'file'))
       allow(file).to receive(:perform_backup).and_return(false)
 
@@ -937,7 +941,7 @@
 
     describe "backing up directories" do
       it "should not backup directories if backup is true and force is false" do
-        # Default: file[:backup] = true
+        file[:backup] = true
         file[:force] = false
         allow(file).to receive(:stat).and_return(double('stat', :ftype => 'directory'))
 
@@ -947,7 +951,7 @@
       end
 
       it "should backup directories if backup is true and force is true" do
-        # Default: file[:backup] = true
+        file[:backup] = true
         file[:force] = true
         allow(file).to receive(:stat).and_return(double('stat', :ftype => 'directory'))
 
@@ -974,7 +978,7 @@
     end
 
     it "should remove a directory if backup is true and force is true" do
-      # Default: file[:backup] = true
+      file[:backup] = true
       file[:force] = true
       allow(file).to receive(:stat).and_return(double('stat', :ftype => 'directory'))
 
@@ -1006,6 +1010,7 @@
     end
 
     it "should fail if the file is not a directory, link, file, fifo, socket, or is unknown" do
+      file[:backup] = 'puppet'
       allow(file).to receive(:stat).and_return(double('stat', :ftype => 'blockSpecial'))
 
       expect(file).to receive(:warning).with("Could not back up file of type blockSpecial")

spec/unit/util/backups_spec.rb

@@ -16,8 +16,6 @@
     let(:path) { make_absolute('/no/such/file') }
 
     it "should noop if the file does not exist" do
-      file = Puppet::Type.type(:file).new(:name => path)
-
       expect(file).not_to receive(:bucket)
       expect(Puppet::FileSystem).to receive(:exist?).with(path).and_return(false)