Add knife upload tests; print warnings/errors on undeleteable things

10 files changed, 341 insertions, 126 deletions

diff --git a/lib/chef/chef_fs/file_system.rb b/lib/chef/chef_fs/file_system.rb
index 26183538c5..581ac2be29 100644
--- a/lib/chef/chef_fs/file_system.rb
+++ b/lib/chef/chef_fs/file_system.rb
@@ -17,6 +17,8 @@
 #
 
 require 'chef/chef_fs/path_utils'
+require 'chef/chef_fs/file_system/operation_skipped_error'
+require 'chef/chef_fs/file_system/operation_not_allowed_error'
 
 class Chef
   module ChefFS
@@ -115,14 +117,18 @@ class Chef
       #
       def self.copy_to(pattern, src_root, dest_root, recurse_depth, options, ui)
         found_result = false
+        error = false
         list_pairs(pattern, src_root, dest_root) do |src, dest|
           found_result = true
           new_dest_parent = get_or_create_parent(dest, options, ui)
-          copy_entries(src, dest, new_dest_parent, recurse_depth, options, ui)
+          child_error = copy_entries(src, dest, new_dest_parent, recurse_depth, options, ui)
+          error ||= child_error
         end
         if !found_result && pattern.exact_path
           ui.error "#{pattern}: No such file or directory on remote or local"
+          error = true
         end
+        error
       end
 
       # Yield entries for children that are in either +a_root+ or +b_root+, with
@@ -228,115 +234,126 @@ class Chef
         # exist.
         # Will need to decide how that works with checksums, though.
 
-        if !src_entry.exists?
-          if options[:purge]
-            # If we would not have uploaded it, we will not purge it.
-            if src_entry.parent.can_have_child?(dest_entry.name, dest_entry.dir?)
-              if options[:dry_run]
-                ui.output "Would delete #{dest_entry.path_for_printing}"
+        error = false
+        begin
+          if !src_entry.exists?
+            if options[:purge]
+              # If we would not have uploaded it, we will not purge it.
+              if src_entry.parent.can_have_child?(dest_entry.name, dest_entry.dir?)
+                if options[:dry_run]
+                  ui.output "Would delete #{dest_entry.path_for_printing}"
+                else
+                  dest_entry.delete(true)
+                  ui.output "Deleted extra entry #{dest_entry.path_for_printing} (purge is on)"
+                end
               else
-                dest_entry.delete(true)
-                ui.output "Deleted extra entry #{dest_entry.path_for_printing} (purge is on)"
+                Chef::Log.info("Not deleting extra entry #{dest_entry.path_for_printing} (purge is off)")
               end
-            else
-              Chef::Log.info("Not deleting extra entry #{dest_entry.path_for_printing} (purge is off)")
             end
-          end
 
-        elsif !dest_entry.exists?
-          if new_dest_parent.can_have_child?(src_entry.name, src_entry.dir?)
-            # If the entry can do a copy directly from filesystem, do that.
-            if new_dest_parent.respond_to?(:create_child_from)
-              if options[:dry_run]
-                ui.output "Would create #{dest_entry.path_for_printing}"
-              else
-                new_dest_parent.create_child_from(src_entry)
-                ui.output "Created #{dest_entry.path_for_printing}"
+          elsif !dest_entry.exists?
+            if new_dest_parent.can_have_child?(src_entry.name, src_entry.dir?)
+              # If the entry can do a copy directly from filesystem, do that.
+              if new_dest_parent.respond_to?(:create_child_from)
+                if options[:dry_run]
+                  ui.output "Would create #{dest_entry.path_for_printing}"
+                else
+                  new_dest_parent.create_child_from(src_entry)
+                  ui.output "Created #{dest_entry.path_for_printing}"
+                end
+                return
               end
-              return
-            end
 
-            if src_entry.dir?
-              if options[:dry_run]
-                ui.output "Would create #{dest_entry.path_for_printing}"
-                new_dest_dir = new_dest_parent.child(src_entry.name)
-              else
-                new_dest_dir = new_dest_parent.create_child(src_entry.name, nil)
-                ui.output "Created #{dest_entry.path_for_printing}/"
-              end
-              # Directory creation is recursive.
-              if recurse_depth != 0
-                src_entry.children.each do |src_child|
-                  new_dest_child = new_dest_dir.child(src_child.name)
-                  copy_entries(src_child, new_dest_child, new_dest_dir, recurse_depth ? recurse_depth - 1 : recurse_depth, options, ui)
+              if src_entry.dir?
+                if options[:dry_run]
+                  ui.output "Would create #{dest_entry.path_for_printing}"
+                  new_dest_dir = new_dest_parent.child(src_entry.name)
+                else
+                  new_dest_dir = new_dest_parent.create_child(src_entry.name, nil)
+                  ui.output "Created #{dest_entry.path_for_printing}/"
+                end
+                # Directory creation is recursive.
+                if recurse_depth != 0
+                  src_entry.children.each do |src_child|
+                    new_dest_child = new_dest_dir.child(src_child.name)
+                    child_error = copy_entries(src_child, new_dest_child, new_dest_dir, recurse_depth ? recurse_depth - 1 : recurse_depth, options, ui)
+                    error ||= child_error
+                  end
                 end
-              end
-            else
-              if options[:dry_run]
-                ui.output "Would create #{dest_entry.path_for_printing}"
               else
-                new_dest_parent.create_child(src_entry.name, src_entry.read)
-                ui.output "Created #{dest_entry.path_for_printing}"
+                if options[:dry_run]
+                  ui.output "Would create #{dest_entry.path_for_printing}"
+                else
+                  return if new_dest_parent.create_child(src_entry.name, src_entry.read) == :skipped
+                  ui.output "Created #{dest_entry.path_for_printing}"
+                end
               end
             end
-          end
 
-        else
-          # Both exist.
-
-          # If the entry can do a copy directly, do that.
-          if dest_entry.respond_to?(:copy_from)
-            if options[:force] || compare(src_entry, dest_entry)[0] == false
-              if options[:dry_run]
-                ui.output "Would update #{dest_entry.path_for_printing}"
-              else
-                dest_entry.copy_from(src_entry)
-                ui.output "Updated #{dest_entry.path_for_printing}"
-              end
-            end
-            return
-          end
+          else
+            # Both exist.
 
-          # If they are different types, log an error.
-          if src_entry.dir?
-            if dest_entry.dir?
-              # If both are directories, recurse into their children
-              if recurse_depth != 0
-                child_pairs(src_entry, dest_entry).each do |src_child, dest_child|
-                  copy_entries(src_child, dest_child, dest_entry, recurse_depth ? recurse_depth - 1 : recurse_depth, options, ui)
+            # If the entry can do a copy directly, do that.
+            if dest_entry.respond_to?(:copy_from)
+              if options[:force] || compare(src_entry, dest_entry)[0] == false
+                if options[:dry_run]
+                  ui.output "Would update #{dest_entry.path_for_printing}"
+                else
+                  dest_entry.copy_from(src_entry)
+                  ui.output "Updated #{dest_entry.path_for_printing}"
                 end
               end
-            else
-              # If they are different types.
-              ui.error("File #{dest_entry.path_for_printing} is a directory while file #{dest_entry.path_for_printing} is a regular file\n")
               return
             end
-          else
-            if dest_entry.dir?
-              ui.error("File #{dest_entry.path_for_printing} is a directory while file #{dest_entry.path_for_printing} is a regular file\n")
-              return
-            else
 
-              # Both are files!  Copy them unless we're sure they are the same.
-              if options[:force]
-                should_copy = true
-                src_value = nil
+            # If they are different types, log an error.
+            if src_entry.dir?
+              if dest_entry.dir?
+                # If both are directories, recurse into their children
+                if recurse_depth != 0
+                  child_pairs(src_entry, dest_entry).each do |src_child, dest_child|
+                    child_error = copy_entries(src_child, dest_child, dest_entry, recurse_depth ? recurse_depth - 1 : recurse_depth, options, ui)
+                    error ||= child_error
+                  end
+                end
               else
-                are_same, src_value, dest_value = compare(src_entry, dest_entry)
-                should_copy = !are_same
+                # If they are different types.
+                ui.error("File #{dest_entry.path_for_printing} is a directory while file #{dest_entry.path_for_printing} is a regular file\n")
+                return
               end
-              if should_copy
-                if options[:dry_run]
-                  ui.output "Would update #{dest_entry.path_for_printing}"
+            else
+              if dest_entry.dir?
+                ui.error("File #{dest_entry.path_for_printing} is a directory while file #{dest_entry.path_for_printing} is a regular file\n")
+                return
+              else
+
+                # Both are files!  Copy them unless we're sure they are the same.
+                if options[:force]
+                  should_copy = true
+                  src_value = nil
                 else
-                  src_value = src_entry.read if src_value.nil?
-                  dest_entry.write(src_value)
-                  ui.output "Updated #{dest_entry.path_for_printing}"
+                  are_same, src_value, dest_value = compare(src_entry, dest_entry)
+                  should_copy = !are_same
+                end
+                if should_copy
+                  if options[:dry_run]
+                    ui.output "Would update #{dest_entry.path_for_printing}"
+                  else
+                    src_value = src_entry.read if src_value.nil?
+                    dest_entry.write(src_value)
+                    ui.output "Updated #{dest_entry.path_for_printing}"
+                  end
                 end
               end
             end
           end
+        rescue OperationSkippedError
+          # If it was simply skipped, a warning has already been printed.
+        rescue OperationNotAllowedError => e
+          ui.error e.message
+          error = true
         end
+        error
       end
 
       def self.get_or_create_parent(entry, options, ui)
diff --git a/lib/chef/chef_fs/file_system/base_fs_object.rb b/lib/chef/chef_fs/file_system/base_fs_object.rb
index 88391adafe..3fbb0c939a 100644
--- a/lib/chef/chef_fs/file_system/base_fs_object.rb
+++ b/lib/chef/chef_fs/file_system/base_fs_object.rb
@@ -17,6 +17,7 @@
 #
 
 require 'chef/chef_fs/path_utils'
+require 'chef/chef_fs/file_system/operation_not_allowed_error'
 
 class Chef
   module ChefFS
@@ -39,42 +40,6 @@ class Chef
         attr_reader :parent
         attr_reader :path
 
-        def root
-          parent ? parent.root : self
-        end
-
-        def path_for_printing
-          if parent
-            parent_path = parent.path_for_printing
-            if parent_path == '.'
-              name
-            else
-              Chef::ChefFS::PathUtils::join(parent.path_for_printing, name)
-            end
-          else
-            name
-          end
-        end
-
-        def dir?
-          false
-        end
-
-        def exists?
-          true
-        end
-
-        def child(name)
-          NonexistentFSObject.new(name, self)
-        end
-
-        # Override can_have_child? to report whether a given file *could* be added
-        # to this directory.  (Some directories can't have subdirs, some can only have .json
-        # files, etc.)
-        def can_have_child?(name, is_dir)
-          false
-        end
-
         # Override this if you have a special comparison algorithm that can tell
         # you whether this entry is the same as another--either a quicker or a
         # more reliable one.  Callers will use this to decide whether to upload,
@@ -112,11 +77,98 @@ class Chef
           nil
         end
 
+        # Override can_have_child? to report whether a given file *could* be added
+        # to this directory.  (Some directories can't have subdirs, some can only have .json
+        # files, etc.)
+        def can_have_child?(name, is_dir)
+          false
+        end
+
+        # Get a child of this entry with the given name.  This MUST always
+        # return a child, even if it is NonexistentFSObject.  Overriders should
+        # take caution not to do expensive network requests to get the list of
+        # children to fulfill this request, unless absolutely necessary here; it
+        # is intended as a quick way to traverse a hierarchy.
+        #
+        # For example, knife show /data_bags/x/y.json will call
+        # root.child('data_bags').child('x').child('y.json'), which can then
+        # directly perform a network request to retrieve the y.json data bag.  No
+        # network request was necessary to retrieve
+        def child(name)
+          NonexistentFSObject.new(name, self)
+        end
+
+        # Override children to report your *actual* list of children as an array.
+        def children
+          raise NotFoundError, "Nonexistent #{path_for_printing}" if !exists?
+          []
+        end
+
+        # Expand this entry into a chef object (Chef::Role, ::Node, etc.)
         def chef_object
-          raise Chef::ChefFS::FileSystem::NotFoundError, "Nonexistent #{path_for_printing}" if !exists?
+          raise NotFoundError, "Nonexistent #{path_for_printing}" if !exists?
           nil
         end
 
+        # Create a child of this entry with the given name and contents.  If
+        # contents is nil, create a directory.
+        #
+        # NOTE: create_child_from is an optional method that can also be added to
+        # your entry class, and will be called without actually reading the
+        # file_contents.  This is used for knife upload /cookbooks/cookbookname.
+        def create_child(name, file_contents)
+          raise NotFoundError, "Nonexistent #{path_for_printing}" if !exists?
+          raise OperationNotAllowedError.new(:create_child), "#{path_for_printing} cannot have a child created under it."
+        end
+
+        # Delete this item, possibly recursively.  Entries MUST NOT delete a
+        # directory unless recurse is true.
+        def delete(recurse)
+          raise NotFoundError, "Nonexistent #{path_for_printing}" if !exists?
+          raise OperationNotAllowedError.new(:delete), "#{path_for_printing} cannot be deleted."
+        end
+
+        # Ask whether this entry is a directory.  If not, it is a file.
+        def dir?
+          false
+        end
+
+        # Ask whether this entry exists.
+        def exists?
+          true
+        end
+
+        # Printable path, generally used to distinguish paths in one root from
+        # paths in another.
+        def path_for_printing
+          if parent
+            parent_path = parent.path_for_printing
+            if parent_path == '.'
+              name
+            else
+              Chef::ChefFS::PathUtils::join(parent.path_for_printing, name)
+            end
+          else
+            name
+          end
+        end
+
+        def root
+          parent ? parent.root : self
+        end
+
+        # Read the contents of this file entry.
+        def read
+          raise NotFoundError, "Nonexistent #{path_for_printing}" if !exists?
+          raise OperationNotAllowedError.new(:read), "#{path_for_printing} cannot be read."
+        end
+
+        # Write the contents of this file entry.
+        def write(file_contents)
+          raise NotFoundError, "Nonexistent #{path_for_printing}" if !exists?
+          raise OperationNotAllowedError.new(:write), "#{path_for_printing} cannot be updated."
+        end
+
         # Important directory attributes: name, parent, path, root
         # Overridable attributes: dir?, child(name), path_for_printing
         # Abstract: read, write, delete, children, can_have_child?, create_child, compare_to
@@ -124,3 +176,5 @@ class Chef
     end
   end
 end
+
+require 'chef/chef_fs/file_system/nonexistent_fs_object'
diff --git a/lib/chef/chef_fs/file_system/chef_server_root_dir.rb b/lib/chef/chef_fs/file_system/chef_server_root_dir.rb
index d3c217d11c..4f1256d664 100644
--- a/lib/chef/chef_fs/file_system/chef_server_root_dir.rb
+++ b/lib/chef/chef_fs/file_system/chef_server_root_dir.rb
@@ -21,6 +21,7 @@ require 'chef/chef_fs/file_system/rest_list_dir'
 require 'chef/chef_fs/file_system/cookbooks_dir'
 require 'chef/chef_fs/file_system/data_bags_dir'
 require 'chef/chef_fs/file_system/nodes_dir'
+require 'chef/chef_fs/file_system/environments_dir'
 
 class Chef
   module ChefFS
@@ -63,7 +64,7 @@ class Chef
             result = [
               CookbooksDir.new(self),
               DataBagsDir.new(self),
-              RestListDir.new("environments", self),
+              EnvironmentsDir.new(self),
               RestListDir.new("roles", self)
             ]
             if repo_mode == 'everything'
diff --git a/lib/chef/chef_fs/file_system/cookbooks_dir.rb b/lib/chef/chef_fs/file_system/cookbooks_dir.rb
index 9249b42aaa..f44d6b5ee2 100644
--- a/lib/chef/chef_fs/file_system/cookbooks_dir.rb
+++ b/lib/chef/chef_fs/file_system/cookbooks_dir.rb
@@ -46,7 +46,18 @@ class Chef
           # pieces.
           begin
             uploader = Chef::CookbookUploader.new(other_cookbook_version, other.parent.file_path)
-            uploader.upload_cookbooks
+            # Work around the fact that CookbookUploader doesn't understand chef_repo_path (yet)
+            old_cookbook_path = Chef::Config.cookbook_path
+            Chef::Config.cookbook_path = other.parent.file_path if !Chef::Config.cookbook_path
+            begin
+              if uploader.respond_to?(:upload_cookbook)
+                uploader.upload_cookbook
+              else
+                uploader.upload_cookbooks
+              end
+            ensure
+              Chef::Config.cookbook_path = old_cookbook_path
+            end
           rescue Net::HTTPServerException => e
             case e.response.code
             when "409"
diff --git a/lib/chef/chef_fs/file_system/environments_dir.rb b/lib/chef/chef_fs/file_system/environments_dir.rb
new file mode 100644
index 0000000000..771f26a80e
--- /dev/null
+++ b/lib/chef/chef_fs/file_system/environments_dir.rb
@@ -0,0 +1,61 @@
+#
+# Author:: John Keiser (<jkeiser@opscode.com>)
+# Copyright:: Copyright (c) 2012 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'chef/chef_fs/file_system/base_fs_dir'
+require 'chef/chef_fs/file_system/rest_list_entry'
+require 'chef/chef_fs/file_system/not_found_error'
+require 'chef/chef_fs/file_system/operation_skipped_error'
+
+class Chef
+  module ChefFS
+    module FileSystem
+      class EnvironmentsDir < RestListDir
+        def initialize(parent)
+          super("environments", parent)
+        end
+
+        def _make_child_entry(name, exists = nil)
+          if name == '_default.json'
+            DefaultEnvironmentEntry.new(name, self, exists)
+          else
+            super
+          end
+        end
+
+        class DefaultEnvironmentEntry < RestListEntry
+          def initialize(name, parent, exists = nil)
+            super(name, parent)
+            @exists = exists
+          end
+
+          def delete(recurse)
+            Chef::Log.warn("The default environment (#{name}) cannot be deleted.  Skipping.")
+            raise NotFoundError, "Nonexistent #{path_for_printing}" if !exists?
+            raise OperationSkippedError.new(:delete), "#{path_for_printing} cannot be deleted."
+          end
+
+          def write(file_contents)
+            Chef::Log.warn("The default environment (#{name}) cannot be deleted.  Skipping.")
+            raise NotFoundError, "Nonexistent #{path_for_printing}" if !exists?
+            raise OperationSkippedError.new(:write), "#{path_for_printing} cannot be updated."
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/chef/chef_fs/file_system/nodes_dir.rb b/lib/chef/chef_fs/file_system/nodes_dir.rb
index 4dfbf6d850..5b8f3f48d6 100644
--- a/lib/chef/chef_fs/file_system/nodes_dir.rb
+++ b/lib/chef/chef_fs/file_system/nodes_dir.rb
@@ -29,6 +29,7 @@ class Chef
         end
 
         # Override children to respond to environment
+        # TODO let's not do this mmkay
         def children
           @children ||= begin
             env_api_path = environment ? "environments/#{environment}/#{api_path}" : api_path
diff --git a/lib/chef/chef_fs/file_system/operation_not_allowed_error.rb b/lib/chef/chef_fs/file_system/operation_not_allowed_error.rb
new file mode 100644
index 0000000000..817630ad22
--- /dev/null
+++ b/lib/chef/chef_fs/file_system/operation_not_allowed_error.rb
@@ -0,0 +1,31 @@
+#
+# Author:: John Keiser (<jkeiser@opscode.com>)
+# Copyright:: Copyright (c) 2012 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'chef/chef_fs/file_system/file_system_error'
+
+class Chef
+  module ChefFS
+    module FileSystem
+      class OperationNotAllowedError < FileSystemError
+        def initialize(operation, cause = nil)
+          super(cause)
+        end
+      end
+    end
+  end
+end
diff --git a/lib/chef/chef_fs/file_system/operation_skipped_error.rb b/lib/chef/chef_fs/file_system/operation_skipped_error.rb
new file mode 100644
index 0000000000..f9a900c98e
--- /dev/null
+++ b/lib/chef/chef_fs/file_system/operation_skipped_error.rb
@@ -0,0 +1,31 @@
+#
+# Author:: John Keiser (<jkeiser@opscode.com>)
+# Copyright:: Copyright (c) 2012 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'chef/chef_fs/file_system/operation_not_allowed_error'
+
+class Chef
+  module ChefFS
+    module FileSystem
+      class OperationSkippedError < OperationNotAllowedError
+        def initialize(operation, cause = nil)
+          super(operation, cause)
+        end
+      end
+    end
+  end
+end
diff --git a/lib/chef/knife/download.rb b/lib/chef/knife/download.rb
index 4133b503b6..d4ae363087 100644
--- a/lib/chef/knife/download.rb
+++ b/lib/chef/knife/download.rb
@@ -40,8 +40,12 @@ class Chef
           exit 1
         end
 
+        error = false
         pattern_args.each do |pattern|
-          Chef::ChefFS::FileSystem.copy_to(pattern, chef_fs, local_fs, config[:recurse] ? nil : 1, config, ui)
+          error ||= Chef::ChefFS::FileSystem.copy_to(pattern, chef_fs, local_fs, config[:recurse] ? nil : 1, config, ui)
+        end
+        if error
+          exit 1
         end
       end
     end
diff --git a/lib/chef/knife/upload.rb b/lib/chef/knife/upload.rb
index cca88e84a8..6cdf26e1a0 100644
--- a/lib/chef/knife/upload.rb
+++ b/lib/chef/knife/upload.rb
@@ -40,8 +40,12 @@ class Chef
           exit 1
         end
 
+        error = false
         pattern_args.each do |pattern|
-          Chef::ChefFS::FileSystem.copy_to(pattern, local_fs, chef_fs, config[:recurse] ? nil : 1, config, ui)
+          error ||= Chef::ChefFS::FileSystem.copy_to(pattern, local_fs, chef_fs, config[:recurse] ? nil : 1, config, ui)
+        end
+        if error
+          exit 1
         end
       end
     end