Merge pull request #3381 from chef/lcg/warn-cb-self-dep

warn on cookbook self-deps

5 files changed, 55 insertions, 4 deletions

diff --git a/lib/chef/cookbook/metadata.rb b/lib/chef/cookbook/metadata.rb
index 4f12275a82..01a98fda39 100644
--- a/lib/chef/cookbook/metadata.rb
+++ b/lib/chef/cookbook/metadata.rb
@@ -286,9 +286,13 @@ class Chef
       # === Returns
       # versions<Array>:: Returns the list of versions for the platform
       def depends(cookbook, *version_args)
-        version = new_args_format(:depends, cookbook, version_args)
-        constraint = validate_version_constraint(:depends, cookbook, version)
-        @dependencies[cookbook] = constraint.to_s
+        if cookbook == name
+          Chef::Log.warn "Ignoring self-dependency in cookbook #{name}, please remove it (in the future this will be fatal)."
+        else
+          version = new_args_format(:depends, cookbook, version_args)
+          constraint = validate_version_constraint(:depends, cookbook, version)
+          @dependencies[cookbook] = constraint.to_s
+        end
         @dependencies[cookbook]
       end
 
diff --git a/lib/chef/knife.rb b/lib/chef/knife.rb
index 4c59f831de..71215e7fbf 100644
--- a/lib/chef/knife.rb
+++ b/lib/chef/knife.rb
@@ -359,7 +359,7 @@ class Chef
 
       case Chef::Config[:verbosity]
       when 0, nil
-        Chef::Config[:log_level] = :error
+        Chef::Config[:log_level] = :warn
       when 1
         Chef::Config[:log_level] = :info
       else
@@ -401,6 +401,8 @@ class Chef
     end
 
     def configure_chef
+      # knife needs to send logger output to STDERR by default
+      Chef::Config[:log_location] = STDERR
       config_loader = self.class.load_config(config[:config_file])
       config[:config_file] = config_loader.config_location
 
diff --git a/spec/integration/knife/upload_spec.rb b/spec/integration/knife/upload_spec.rb
index cef4f54e97..826ecec364 100644
--- a/spec/integration/knife/upload_spec.rb
+++ b/spec/integration/knife/upload_spec.rb
@@ -154,6 +154,24 @@ EOM
           end
         end
 
+        context 'when cookbook metadata has a self-dependency' do
+          before do
+            file 'cookbooks/x/metadata.rb', "name 'x'; version '1.0.0'; depends 'x'"
+          end
+
+          it "should warn", :chef_lt_13_only do
+            knife('upload /cookbooks').should_succeed(
+              stdout: "Updated /cookbooks/x\n",
+              stderr: "WARN: Ignoring self-dependency in cookbook x, please remove it (in the future this will be fatal).\n"
+            )
+            knife('diff --name-status /').should_succeed ''
+          end
+          it "should fail in Chef 13", :chef_gte_13_only do
+            knife('upload /cookbooks').should_fail ''
+            # FIXME: include the error message here
+          end
+        end
+
         context 'as well as one extra copy of each thing' do
           before do
             file 'clients/y.json', { 'public_key' => ChefZero::PUBLIC_KEY }
diff --git a/spec/unit/cookbook/metadata_spec.rb b/spec/unit/cookbook/metadata_spec.rb
index 760ae5dd2a..d2954726e8 100644
--- a/spec/unit/cookbook/metadata_spec.rb
+++ b/spec/unit/cookbook/metadata_spec.rb
@@ -304,6 +304,21 @@ describe Chef::Cookbook::Metadata do
         end
       end
     end
+
+    it "strips out self-dependencies", :chef_lt_13_only do
+      metadata.name('foo')
+      expect(Chef::Log).to receive(:warn).with(
+        "Ignoring self-dependency in cookbook foo, please remove it (in the future this will be fatal)."
+      )
+      metadata.depends('foo')
+      expect(metadata.dependencies).to eql({})
+    end
+
+    it "errors on self-dependencies", :chef_gte_13_only do
+      metadata.name('foo')
+      expect { metadata.depends('foo') }.to raise_error
+      # FIXME: add the error type
+    end
   end
 
   describe "attribute groupings" do
diff --git a/spec/unit/knife_spec.rb b/spec/unit/knife_spec.rb
index 7cb16aae2a..7aab16ce99 100644
--- a/spec/unit/knife_spec.rb
+++ b/spec/unit/knife_spec.rb
@@ -252,6 +252,18 @@ describe Chef::Knife do
                                         :default => "default-value")
       end
 
+      it "sets the default log_location to STDERR for Chef::Log warnings" do
+        knife_command = KnifeSpecs::TestYourself.new([])
+        knife_command.configure_chef
+        expect(Chef::Config[:log_location]).to eq(STDERR)
+      end
+
+      it "sets the default log_level to warn so we can issue Chef::Log.warn" do
+        knife_command = KnifeSpecs::TestYourself.new([])
+        knife_command.configure_chef
+        expect(Chef::Config[:log_level]).to eql(:warn)
+      end
+
       it "prefers the default value if no config or command line value is present" do
         knife_command = KnifeSpecs::TestYourself.new([]) #empty argv
         knife_command.configure_chef