Refactoring duplicated code into a separate module. Also making CLI options more informative

5 files changed, 166 insertions, 171 deletions

diff --git a/lib/chef/dsl/data_query.rb b/lib/chef/dsl/data_query.rb
index 3dafbca6bf..e36784271a 100644
--- a/lib/chef/dsl/data_query.rb
+++ b/lib/chef/dsl/data_query.rb
@@ -20,6 +20,7 @@ require 'chef/search/query'
 require 'chef/data_bag'
 require 'chef/data_bag_item'
 require 'chef/encrypted_data_bag_item'
+require 'chef/encrypted_data_bag_item/check_encrypted'
 
 class Chef
   module DSL
@@ -28,6 +29,7 @@ class Chef
     # Provides DSL for querying data from the chef-server via search or data
     # bag.
     module DataQuery
+      include Chef::EncryptedDataBagItem::CheckEncrypted
 
       def search(*args, &block)
         # If you pass a block, or have at least the start argument, do raw result parsing
@@ -78,35 +80,6 @@ class Chef
         raise
       end
 
-      private
-
-      # Tries to autodetect if the item's raw hash appears to be encrypted.
-      def encrypted?(raw_data)
-        data = raw_data.reject { |k, _| k == "id" } # Remove the "id" key.
-        # Assume hashes containing only the "id" key are not encrypted.
-        # Otherwise, remove the keys that don't appear to be encrypted and compare
-        # the result with the hash. If some entry has been removed, then some entry
-        # doesn't appear to be encrypted and we assume the entire hash is not encrypted.
-        data.empty? ? false : data.reject { |_, v| !looks_like_encrypted?(v) } == data
-      end
-
-      # Checks if data looks like it has been encrypted by
-      # Chef::EncryptedDataBagItem::Encryptor::VersionXEncryptor. Returns
-      # true only when there is an exact match between the VersionXEncryptor
-      # keys and the hash's keys.
-      def looks_like_encrypted?(data)
-        return false unless data.is_a?(Hash) && data.has_key?("version")
-        case data["version"]
-        when 1
-          Chef::EncryptedDataBagItem::Encryptor::Version1Encryptor.encryptor_keys.sort == data.keys.sort
-        when 2
-          Chef::EncryptedDataBagItem::Encryptor::Version2Encryptor.encryptor_keys.sort == data.keys.sort
-        when 3
-          Chef::EncryptedDataBagItem::Encryptor::Version3Encryptor.encryptor_keys.sort == data.keys.sort
-        else
-          false # version means something else... assume not encrypted.
-        end
-      end
     end
   end
 end
diff --git a/lib/chef/encrypted_data_bag_item/check_encrypted.rb b/lib/chef/encrypted_data_bag_item/check_encrypted.rb
new file mode 100644
index 0000000000..b7cb5841b3
--- /dev/null
+++ b/lib/chef/encrypted_data_bag_item/check_encrypted.rb
@@ -0,0 +1,56 @@
+#
+# Author:: Tyler Ball (<tball@getchef.com>)
+# Copyright:: Copyright (c) 2010-2014 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/encrypted_data_bag_item/encryptor'
+
+class Chef::EncryptedDataBagItem
+  # Common code for checking if a data bag appears encrypted
+  module CheckEncrypted
+
+    # Tries to autodetect if the item's raw hash appears to be encrypted.
+    def encrypted?(raw_data)
+      data = raw_data.reject { |k, _| k == "id" } # Remove the "id" key.
+      # Assume hashes containing only the "id" key are not encrypted.
+      # Otherwise, remove the keys that don't appear to be encrypted and compare
+      # the result with the hash. If some entry has been removed, then some entry
+      # doesn't appear to be encrypted and we assume the entire hash is not encrypted.
+      data.empty? ? false : data.reject { |_, v| !looks_like_encrypted?(v) } == data
+    end
+
+    private
+
+    # Checks if data looks like it has been encrypted by
+    # Chef::EncryptedDataBagItem::Encryptor::VersionXEncryptor. Returns
+    # true only when there is an exact match between the VersionXEncryptor
+    # keys and the hash's keys.
+    def looks_like_encrypted?(data)
+      return false unless data.is_a?(Hash) && data.has_key?("version")
+      case data["version"]
+        when 1
+          Chef::EncryptedDataBagItem::Encryptor::Version1Encryptor.encryptor_keys.sort == data.keys.sort
+        when 2
+          Chef::EncryptedDataBagItem::Encryptor::Version2Encryptor.encryptor_keys.sort == data.keys.sort
+        when 3
+          Chef::EncryptedDataBagItem::Encryptor::Version3Encryptor.encryptor_keys.sort == data.keys.sort
+        else
+          false # version means something else... assume not encrypted.
+      end
+    end
+
+  end
+end
diff --git a/lib/chef/knife/data_bag_secret_options.rb b/lib/chef/knife/data_bag_secret_options.rb
index 8b9c947bac..b692fc767c 100644
--- a/lib/chef/knife/data_bag_secret_options.rb
+++ b/lib/chef/knife/data_bag_secret_options.rb
@@ -18,27 +18,29 @@
 
 require 'mixlib/cli'
 require 'chef/config'
+require 'chef/encrypted_data_bag_item/check_encrypted'
 
 class Chef
   class Knife
     module DataBagSecretOptions
       include Mixlib::CLI
+      include Chef::EncryptedDataBagItem::CheckEncrypted
 
       def self.included(base)
         base.option :secret,
                :short => "-s SECRET",
                :long  => "--secret ",
-               :description => "The secret key to use to encrypt data bag item values.  Can also be defaulted in your knife.rb with the key 'secret'",
+               :description => "The secret key to use to encrypt data bag item values.  Can also be defaulted in your config with the key 'secret'",
                :proc => Proc.new { |s| Chef::Config[:knife][:secret] = s }
 
         base.option :secret_file,
                :long => "--secret-file SECRET_FILE",
-               :description => "A file containing the secret key to use to encrypt data bag item values.  Can also be defaulted in your knife.rb with the key 'secret_file'",
+               :description => "A file containing the secret key to use to encrypt data bag item values.  Can also be defaulted in your config with the key 'secret_file'",
                :proc => Proc.new { |sf| Chef::Config[:knife][:secret_file] = sf }
 
         base.option :encrypt,
                :long => "--encrypt",
-               :description => "If 'secret' or 'secret_file' is present in your knife.rb, then encrypt data bags using it",
+               :description => "If 'secret' or 'secret_file' is present in your config, then encrypt data bags using it",
                :boolean => true,
                :default => false
       end
@@ -99,41 +101,12 @@ class Chef
         knife_config[:secret_file] || Chef::Config[:secret_file]
       end
 
-      # TODO duplicated from data_query.rb, also needs test coverage when it is extracted
-      # Tries to autodetect if the item's raw hash appears to be encrypted.
-      def encrypted?(raw_data)
-        data = raw_data.reject { |k, _| k == "id" } # Remove the "id" key.
-        # Assume hashes containing only the "id" key are not encrypted.
-        # Otherwise, remove the keys that don't appear to be encrypted and compare
-        # the result with the hash. If some entry has been removed, then some entry
-        # doesn't appear to be encrypted and we assume the entire hash is not encrypted.
-        data.empty? ? false : data.reject { |_, v| !looks_like_encrypted?(v) } == data
-      end
-
       private
 
       def knife_config
         Chef::Config.key?(:knife) ? Chef::Config[:knife] : {}
       end
 
-      # Checks if data looks like it has been encrypted by
-      # Chef::EncryptedDataBagItem::Encryptor::VersionXEncryptor. Returns
-      # true only when there is an exact match between the VersionXEncryptor
-      # keys and the hash's keys.
-      def looks_like_encrypted?(data)
-        return false unless data.is_a?(Hash) && data.has_key?("version")
-        case data["version"]
-          when 1
-            Chef::EncryptedDataBagItem::Encryptor::Version1Encryptor.encryptor_keys.sort == data.keys.sort
-          when 2
-            Chef::EncryptedDataBagItem::Encryptor::Version2Encryptor.encryptor_keys.sort == data.keys.sort
-          when 3
-            Chef::EncryptedDataBagItem::Encryptor::Version3Encryptor.encryptor_keys.sort == data.keys.sort
-          else
-            false # version means something else... assume not encrypted.
-        end
-      end
-
     end
   end
 end
diff --git a/spec/unit/dsl/data_query_spec.rb b/spec/unit/dsl/data_query_spec.rb
index 8a985437b7..78cd5569e8 100644
--- a/spec/unit/dsl/data_query_spec.rb
+++ b/spec/unit/dsl/data_query_spec.rb
@@ -86,123 +86,21 @@ describe Chef::DSL::DataQuery do
     end
 
     context "when the item is encrypted" do
-      let(:default_secret) { "abc123SECRET" }
-
-      let(:encoded_data) { Chef::EncryptedDataBagItem.encrypt_data_bag_item(raw_data, default_secret) }
-
-      let(:item) do
-        item = Chef::DataBagItem.new
-        item.data_bag(bag_name)
-        item.raw_data = encoded_data
-        item
-      end
+      let(:secret) { "abc123SECRET" }
+      let(:enc_data_bag) { double("Chef::EncryptedDataBagItem") }
 
       before do
         allow( Chef::DataBagItem ).to receive(:load).with(bag_name, item_name).and_return(item)
+        expect(language).to receive(:encrypted?).and_return(true)
+        expect( Chef::EncryptedDataBagItem ).to receive(:load_secret).and_return(secret)
       end
 
-      shared_examples_for "encryption detected" do
-        let(:encoded_data) do
-          Chef::Config[:data_bag_encrypt_version] = version
-          Chef::EncryptedDataBagItem.encrypt_data_bag_item(raw_data, default_secret)
-        end
-
-        before do
-          allow( Chef::EncryptedDataBagItem ).to receive(:load_secret).and_return(default_secret)
-        end
-
-        it "detects encrypted data bag" do
-          expect( encryptor ).to receive(:encryptor_keys).at_least(:once).and_call_original
-          expect( Chef::Log ).to receive(:debug).with(/Data bag item looks encrypted/)
-          language.data_bag_item(bag_name, item_name)
-        end
-      end
-
-      context "when encryption version is 1" do
-        include_examples "encryption detected" do
-          let(:version) { 1 }
-          let(:encryptor) { Chef::EncryptedDataBagItem::Encryptor::Version1Encryptor }
-        end
-      end
-
-      context "when encryption version is 2" do
-        include_examples "encryption detected" do
-          let(:version) { 2 }
-          let(:encryptor) { Chef::EncryptedDataBagItem::Encryptor::Version2Encryptor }
-        end
+      it "detects encrypted data bag" do
+        expect( Chef::EncryptedDataBagItem ).to receive(:new).with(raw_data, secret).and_return(enc_data_bag)
+        expect( Chef::Log ).to receive(:debug).with(/Data bag item looks encrypted/)
+        expect(language.data_bag_item(bag_name, item_name)).to eq(enc_data_bag)
       end
 
-      context "when encryption version is 3", :ruby_20_only do
-        include_examples "encryption detected" do
-          let(:version) { 3 }
-          let(:encryptor) { Chef::EncryptedDataBagItem::Encryptor::Version3Encryptor }
-        end
-      end
-
-      shared_examples_for "an encrypted data bag item" do
-        it "returns an encrypted data bag item" do
-          expect( language.data_bag_item(bag_name, item_name, secret) ).to be_a_kind_of(Chef::EncryptedDataBagItem)
-        end
-
-        it "decrypts the contents of the data bag item" do
-          expect( language.data_bag_item(bag_name, item_name, secret).to_hash ).to eql raw_data
-        end
-      end
-
-      context "when a secret is supplied" do
-        include_examples "an encrypted data bag item" do
-          let(:secret) { default_secret }
-        end
-      end
-
-      context "when a secret is not supplied" do
-        before do
-          allow( Chef::Config ).to receive(:[]).and_call_original
-          expect( Chef::Config ).to receive(:[]).with(:encrypted_data_bag_secret).and_return(path)
-          expect( Chef::EncryptedDataBagItem ).to receive(:load_secret).and_call_original
-        end
-
-        context "when a secret is located at Chef::Config[:encrypted_data_bag_secret]" do
-          let(:path) { "/tmp/my_secret" }
-
-          before do
-            expect( File ).to receive(:exist?).with(path).and_return(true)
-            expect( IO ).to receive(:read).with(path).and_return(default_secret)
-          end
-
-          include_examples "an encrypted data bag item" do
-            let(:secret) { nil }
-          end
-        end
-
-        shared_examples_for "no secret file" do
-          it "should fail to load the data bag item" do
-            expect( Chef::Log ).to receive(:error).with(/Failed to load secret for encrypted data bag item/)
-            expect( Chef::Log ).to receive(:error).with(/Failed to load data bag item/)
-            expect{ language.data_bag_item(bag_name, item_name) }.to raise_error(error_type, error_message)
-          end
-        end
-
-        context "when Chef::Config[:encrypted_data_bag_secret] is not configured" do
-          include_examples "no secret file" do
-            let(:path) { nil }
-            let(:error_type) { ArgumentError }
-            let(:error_message) { /No secret specified and no secret found/ }
-          end
-        end
-
-        context "when Chef::Config[:encrypted_data_bag_secret] does not exist" do
-          include_examples "no secret file" do
-            before do
-              expect( File ).to receive(:exist?).with(path).and_return(false)
-            end
-
-            let(:path) { "/tmp/my_secret" }
-            let(:error_type) { Errno::ENOENT }
-            let(:error_message) { /file not found/ }
-          end
-        end
-      end
     end
   end
 end
diff --git a/spec/unit/encrypted_data_bag_item/check_encrypted_spec.rb b/spec/unit/encrypted_data_bag_item/check_encrypted_spec.rb
new file mode 100644
index 0000000000..1da5efb36e
--- /dev/null
+++ b/spec/unit/encrypted_data_bag_item/check_encrypted_spec.rb
@@ -0,0 +1,95 @@
+#
+# Author:: Tyler Ball (<tball@getchef.com>)
+# Copyright:: Copyright (c) 2010-2014 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 'spec_helper'
+require 'chef/encrypted_data_bag_item/check_encrypted'
+
+class CheckEncryptedTester
+  include Chef::EncryptedDataBagItem::CheckEncrypted
+end
+
+describe Chef::EncryptedDataBagItem::CheckEncrypted do
+
+  let(:tester) { CheckEncryptedTester.new }
+
+  it "detects the item is not encrypted when the data is empty" do
+    expect(tester.encrypted?({})).to eq(false)
+  end
+
+  it "detects the item is not encrypted when the data only contains an id" do
+    expect(tester.encrypted?({id: "foo"})).to eq(false)
+  end
+
+  context "when the item is encrypted" do
+
+    let(:default_secret) { "abc123SECRET" }
+    let(:item_name) { "item_name" }
+    let(:raw_data) {{
+        "id" => item_name,
+        "greeting" => "hello",
+        "nested" => {
+            "a1" => [1, 2, 3],
+            "a2" => { "b1" => true }
+        }
+    }}
+
+    let(:version) { 1 }
+    let(:encoded_data) do
+      Chef::Config[:data_bag_encrypt_version] = version
+      Chef::EncryptedDataBagItem.encrypt_data_bag_item(raw_data, default_secret)
+    end
+
+    it "does not detect encryption when the item version is unknown" do
+      # It shouldn't be possible for someone to normally encrypt an item with an unknown version - they would have to
+      # do something funky like encrypting it and then manually changing the version
+      modified_encoded_data = encoded_data
+      modified_encoded_data["greeting"]["version"] = 4
+      expect(tester.encrypted?(modified_encoded_data)).to eq(false)
+    end
+
+    shared_examples_for "encryption detected" do
+      it "detects encrypted data bag" do
+        expect( encryptor ).to receive(:encryptor_keys).at_least(:once).and_call_original
+        expect(tester.encrypted?(encoded_data)).to eq(true)
+      end
+    end
+
+    context "when encryption version is 1" do
+      include_examples "encryption detected" do
+        let(:version) { 1 }
+        let(:encryptor) { Chef::EncryptedDataBagItem::Encryptor::Version1Encryptor }
+      end
+    end
+
+    context "when encryption version is 2" do
+      include_examples "encryption detected" do
+        let(:version) { 2 }
+        let(:encryptor) { Chef::EncryptedDataBagItem::Encryptor::Version2Encryptor }
+      end
+    end
+
+    context "when encryption version is 3", :ruby_20_only do
+      include_examples "encryption detected" do
+        let(:version) { 3 }
+        let(:encryptor) { Chef::EncryptedDataBagItem::Encryptor::Version3Encryptor }
+      end
+    end
+
+  end
+
+end