Chef::ApiClient.update supports V1.

5 files changed, 296 insertions, 22 deletions

diff --git a/lib/chef/api_client.rb b/lib/chef/api_client.rb
index 5ad7a581a2..f3a83d7bcb 100644
--- a/lib/chef/api_client.rb
+++ b/lib/chef/api_client.rb
@@ -1,7 +1,7 @@
 #
-# Author:: Adam Jacob (<adam@opscode.com>)
-# Author:: Nuo Yan (<nuo@opscode.com>)
-# Copyright:: Copyright (c) 2008 Opscode, Inc.
+# Author:: Adam Jacob (<adam@chef.io>)
+# Author:: Nuo Yan (<nuo@chef.io>)
+# Copyright:: Copyright (c) 2015 Chef Software, Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -213,11 +213,11 @@ class Chef
     # Save this client via the REST API, returns a hash including the private key
     def save
       begin
-        http_api.put("clients/#{name}", { :name => self.name, :admin => self.admin, :validator => self.validator})
+        update
       rescue Net::HTTPServerException => e
         # If that fails, go ahead and try and update it
         if e.response.code == "404"
-          http_api.post("clients", {:name => self.name, :admin => self.admin, :validator => self.validator })
+          create
         else
           raise e
         end
@@ -234,6 +234,30 @@ class Chef
       self
     end
 
+    # Updates the client via the REST API
+    def update
+      # NOTE: API V1 dropped support for updating client keys via update (aka PUT),
+      # but this code never supported key updating in the first place. Since
+      # it was never implemented, we will simply ignore that functionality
+      # as it is being deprecated.
+      # Delete this comment after V0 support is dropped.
+      payload = { :name => name }
+      payload[:validator] = validator unless validator.nil?
+      # this field is ignored in API V1, but left for backwards-compat,
+      # can remove after OSC 11 support is finished?
+      payload[:admin] = admin unless admin.nil?
+
+      begin
+        new_client = chef_rest_v1.put("clients/#{name}", payload)
+      rescue Net::HTTPServerException => e
+        # rescue API V0 if 406 and the server supports V0
+        raise e unless handle_version_http_exception(e, SUPPORTED_API_VERSIONS[0], SUPPORTED_API_VERSIONS[-1])
+        new_client = chef_rest_v0.put("clients/#{name}", payload)
+      end
+
+      new_client
+    end
+
     # Create the client via the REST API
     def create
       payload = {
@@ -247,8 +271,8 @@ class Chef
         # try API V1
         raise Chef::Exceptions::InvalidClientAttribute, "You cannot set both public_key and create_key for create." if create_key && public_key
 
-        payload[:public_key] = public_key if public_key
-        payload[:create_key] = create_key if create_key
+        payload[:public_key] = public_key unless public_key.nil?
+        payload[:create_key] = create_key unless create_key.nil?
 
         new_client = chef_rest_v1.post("clients", payload)
 
@@ -267,7 +291,7 @@ class Chef
 
           # under API V0, a key pair will always be created unless public_key is
           # passed on initial POST
-          payload[:public_key] = public_key if public_key
+          payload[:public_key] = public_key unless public_key.nil?
 
           new_client = chef_rest_v0.post("clients", payload)
       end
diff --git a/spec/support/shared/unit/api_versioning.rb b/spec/support/shared/unit/api_versioning.rb
new file mode 100644
index 0000000000..ffa424a78d
--- /dev/null
+++ b/spec/support/shared/unit/api_versioning.rb
@@ -0,0 +1,33 @@
+#
+# Author:: Tyler Cloke (<tyler@chef.io>)
+# Copyright:: Copyright (c) 2015 Chef Software, 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.
+#
+
+shared_examples_for "version handling" do
+  before do
+    allow(rest_v1).to receive(http_verb).and_raise(exception_406)
+  end
+
+  context "when the server does not support the min or max server API version that Chef::User supports" do
+    before do
+      allow(object).to receive(:handle_version_http_exception).and_return(false)
+    end
+
+    it "raises the original exception" do
+      expect{ object.send(method) }.to raise_error(exception_406)
+    end
+  end # when the server does not support the min or max server API version that Chef::User supports
+end # version handling
diff --git a/spec/support/shared/unit/knife_shared.rb b/spec/support/shared/unit/knife_shared.rb
new file mode 100644
index 0000000000..8c9010f3cf
--- /dev/null
+++ b/spec/support/shared/unit/knife_shared.rb
@@ -0,0 +1,40 @@
+#
+# Author:: Tyler Cloke (<tyler@chef.io>)
+# Copyright:: Copyright (c) 2015 Chef Software, 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.
+#
+
+
+shared_examples_for "mandatory field missing" do
+  context "when field is nil" do
+    before do
+      knife.name_args = name_args
+    end
+
+    it "exits 1" do
+      expect { knife.run }.to raise_error(SystemExit)
+    end
+
+    it "prints the usage" do
+      expect(knife).to receive(:show_usage)
+      expect { knife.run }.to raise_error(SystemExit)
+    end
+
+    it "prints a relevant error message" do
+      expect { knife.run }.to raise_error(SystemExit)
+      expect(stderr.string).to match /You must specify a #{fieldname}/
+    end
+  end
+end
diff --git a/spec/support/shared/unit/user_and_client_shared.rb b/spec/support/shared/unit/user_and_client_shared.rb
new file mode 100644
index 0000000000..36a7a7aeff
--- /dev/null
+++ b/spec/support/shared/unit/user_and_client_shared.rb
@@ -0,0 +1,114 @@
+#
+# Author:: Tyler Cloke (<tyler@chef.io>)
+# Copyright:: Copyright (c) 2015 Chef Software, 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.
+#
+
+shared_examples_for "user or client create" do
+
+  context "when server API V1 is valid on the Chef Server receiving the request" do
+
+    it "creates a new object via the API" do
+      expect(rest_v1).to receive(:post).with(url, payload).and_return({})
+      object.create
+    end
+
+    it "creates a new object via the API with a public_key when it exists" do
+      object.public_key "some_public_key"
+      expect(rest_v1).to receive(:post).with(url, payload.merge({:public_key => "some_public_key"})).and_return({})
+      object.create
+    end
+
+    context "raise error when create_key and public_key are both set" do
+
+      before do
+        object.public_key "key"
+        object.create_key true
+      end
+
+      it "rasies the proper error" do
+        expect { object.create }.to raise_error(error)
+      end
+    end
+
+    context "when create_key == true" do
+      before do
+        object.create_key true
+      end
+
+      it "creates a new object via the API with create_key" do
+        expect(rest_v1).to receive(:post).with(url, payload.merge({:create_key => true})).and_return({})
+        object.create
+      end
+    end
+
+    context "when chef_key is returned by the server" do
+      let(:chef_key) {
+        {
+          "chef_key" => {
+            "public_key" => "some_public_key"
+          }
+        }
+      }
+
+      it "puts the public key into the objectr returned by create" do
+        expect(rest_v1).to receive(:post).with(url, payload).and_return(payload.merge(chef_key))
+        new_object = object.create
+        expect(new_object.public_key).to eq("some_public_key")
+      end
+
+      context "when private_key is returned in chef_key" do
+        let(:chef_key) {
+          {
+            "chef_key" => {
+              "public_key" => "some_public_key",
+              "private_key" => "some_private_key"
+            }
+          }
+        }
+
+        it "puts the private key into the object returned by create" do
+          expect(rest_v1).to receive(:post).with(url, payload).and_return(payload.merge(chef_key))
+          new_object = object.create
+          expect(new_object.private_key).to eq("some_private_key")
+        end
+      end
+    end # when chef_key is returned by the server
+
+  end # when server API V1 is valid on the Chef Server receiving the request
+
+  context "when server API V1 is not valid on the Chef Server receiving the request" do
+
+    context "when the server supports API V0" do
+      before do
+        allow(object).to receive(:handle_version_http_exception).and_return(true)
+        allow(rest_v1).to receive(:post).and_raise(exception_406)
+      end
+
+      it "creates a new object via the API" do
+        expect(rest_v0).to receive(:post).with(url, payload).and_return({})
+        object.create
+      end
+
+      it "creates a new object via the API with a public_key when it exists" do
+        object.public_key "some_public_key"
+        expect(rest_v0).to receive(:post).with(url, payload.merge({:public_key => "some_public_key"})).and_return({})
+        object.create
+      end
+
+    end # when the server supports API V0
+  end # when server API V1 is not valid on the Chef Server receiving the request
+
+end
diff --git a/spec/unit/api_client_spec.rb b/spec/unit/api_client_spec.rb
index d39a90946b..b903c0e3c7 100644
--- a/spec/unit/api_client_spec.rb
+++ b/spec/unit/api_client_spec.rb
@@ -384,27 +384,24 @@ describe Chef::ApiClient do
   describe "Versioned API Interactions" do
     let(:response_406) { OpenStruct.new(:code => '406') }
     let(:exception_406) { Net::HTTPServerException.new("406 Not Acceptable", response_406) }
+    let(:payload)  {
+      {
+        :name => "some_name",
+        :validator => true,
+        :admin => true
+      }
+    }
 
-    before (:each) do
+    before do
       @client = Chef::ApiClient.new
       allow(@client).to receive(:chef_rest_v0).and_return(double('chef rest root v0 object'))
       allow(@client).to receive(:chef_rest_v1).and_return(double('chef rest root v1 object'))
+      @client.name "some_name"
+      @client.validator true
+      @client.admin true
     end
 
     describe "create" do
-      let(:payload)  {
-        {
-          :name => "some_name",
-          :validator => true,
-          :admin => true
-        }
-      }
-
-      before do
-        @client.name "some_name"
-        @client.validator true
-        @client.admin true
-      end
 
       # from spec/support/shared/unit/user_and_client_shared.rb
       it_should_behave_like "user or client create" do
@@ -427,5 +424,71 @@ describe Chef::ApiClient do
 
     end # create
 
+    describe "update" do
+      context "when a valid client is defined" do
+
+        shared_examples_for "client updating" do
+          it "updates the client" do
+            expect(rest). to receive(:put).with("clients/some_name", payload)
+            @client.update
+          end
+
+          context "when only the name field exists" do
+
+            before do
+              # needed since there is no way to set to nil via code
+              @client.instance_variable_set(:@validator, nil)
+              @client.instance_variable_set(:@admin, nil)
+            end
+
+            after do
+              @client.validator true
+              @client.admin true
+            end
+
+            it "updates the client with only the name" do
+              expect(rest). to receive(:put).with("clients/some_name", {:name => "some_name"})
+              @client.update
+            end
+          end
+
+        end
+
+        context "when API V1 is supported by the server" do
+
+          it_should_behave_like "client updating" do
+            let(:rest) { @client.chef_rest_v1 }
+          end
+
+        end # when API V1 is supported by the server
+
+        context "when API V1 is not supported by the server" do
+          context "when no version is supported" do
+            # from spec/support/shared/unit/api_versioning.rb
+            it_should_behave_like "version handling" do
+              let(:object)    { @client }
+              let(:method)    { :create }
+              let(:http_verb) { :post }
+              let(:rest_v1)   { @client.chef_rest_v1 }
+            end
+          end # when no version is supported
+
+          context "when API V0 is supported" do
+
+            before do
+              allow(@client.chef_rest_v1).to receive(:put).and_raise(exception_406)
+              allow(@client).to receive(:handle_version_http_exception).and_return(true)
+            end
+
+            it_should_behave_like "client updating" do
+              let(:rest) { @client.chef_rest_v0 }
+            end
+
+          end
+
+        end # when API V1 is not supported by the server
+      end # when a valid client is defined
+    end # update
+
   end
 end