diff options
author | tylercloke <tylercloke@gmail.com> | 2015-06-04 12:23:55 -0700 |
---|---|---|
committer | tylercloke <tylercloke@gmail.com> | 2015-06-05 13:47:44 -0700 |
commit | 58048d42caeadc290876a02aaec7eec6f305920a (patch) | |
tree | ea9743fc860b69f2c4c20bb26b7f58a5d2f6f44f | |
parent | 4f2400c8cce57e68f14cb9d4b756ab82b23dd69d (diff) | |
download | chef-58048d42caeadc290876a02aaec7eec6f305920a.tar.gz |
Better API version error handling helper code.
Renamed Chef::Mixin::ApiVersionRequestHandling.handle_version_http_exception -> server_client_api_version_intersection and made it do much more useful / sane things. See comments for details.
-rw-r--r-- | lib/chef/api_client.rb | 8 | ||||
-rw-r--r-- | lib/chef/knife/user_create.rb | 9 | ||||
-rw-r--r-- | lib/chef/knife/user_delete.rb | 8 | ||||
-rw-r--r-- | lib/chef/knife/user_edit.rb | 8 | ||||
-rw-r--r-- | lib/chef/knife/user_reregister.rb | 7 | ||||
-rw-r--r-- | lib/chef/knife/user_show.rb | 7 | ||||
-rw-r--r-- | lib/chef/mixin/api_version_request_handling.rb | 57 | ||||
-rw-r--r-- | lib/chef/user.rb | 8 | ||||
-rw-r--r-- | spec/support/shared/unit/api_versioning.rb | 2 | ||||
-rw-r--r-- | spec/support/shared/unit/user_and_client_shared.rb | 2 | ||||
-rw-r--r-- | spec/unit/api_client_spec.rb | 2 | ||||
-rw-r--r-- | spec/unit/mixin/api_version_request_handling_spec.rb | 127 | ||||
-rw-r--r-- | spec/unit/user_spec.rb | 8 |
13 files changed, 191 insertions, 62 deletions
diff --git a/lib/chef/api_client.rb b/lib/chef/api_client.rb index 3b777b3574..ad31fb7d7b 100644 --- a/lib/chef/api_client.rb +++ b/lib/chef/api_client.rb @@ -32,7 +32,7 @@ class Chef include Chef::Mixin::FromFile include Chef::Mixin::ParamsValidate - include Chef::ApiVersionRequestHandling + include Chef::Mixin::ApiVersionRequestHandling SUPPORTED_API_VERSIONS = [0,1] @@ -272,7 +272,8 @@ class Chef 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]) + supported_versions = server_client_api_version_intersection(e, SUPPORTED_API_VERSIONS) + raise e unless supported_versions && supported_versions.include?(0) new_client = chef_rest_v0.put("clients/#{name}", payload) end @@ -308,7 +309,8 @@ class Chef 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]) + supported_versions = server_client_api_version_intersection(e, SUPPORTED_API_VERSIONS) + raise e unless supported_versions && supported_versions.include?(0) # under API V0, a key pair will always be created unless public_key is # passed on initial POST diff --git a/lib/chef/knife/user_create.rb b/lib/chef/knife/user_create.rb index 5332e863ae..e73f6be8b6 100644 --- a/lib/chef/knife/user_create.rb +++ b/lib/chef/knife/user_create.rb @@ -78,11 +78,13 @@ knife user create for Open Source 11 Server is being deprecated. Open Source 11 Server user commands now live under the knife osc_user namespace. For backwards compatibility, we will forward this request to knife osc_user create. If you are using an Open Source 11 Server, please use that command to avoid this warning. -If you are not using an Open Source Chef 11 Server install, please read knife user create --help for proper usage. EOF end def run_osc_11_user_create + # run osc_user_create with our input + ARGV.delete("user") + ARGV.unshift("osc_user") Chef::Knife.run(ARGV, Chef::Application::Knife.options) end @@ -93,12 +95,7 @@ EOF # If only 1 arg is passed, assume OSC 11 case. if @name_args.length == 1 ui.warn(osc_11_warning) - - # run osc_user_create with our input - ARGV.delete("user") - ARGV.unshift("osc_user") run_osc_11_user_create - else # EC / CS 12 user create test_mandatory_field(@name_args[0], "username") diff --git a/lib/chef/knife/user_delete.rb b/lib/chef/knife/user_delete.rb index 3a46e33f84..803be6b90c 100644 --- a/lib/chef/knife/user_delete.rb +++ b/lib/chef/knife/user_delete.rb @@ -41,6 +41,9 @@ EOF end def run_osc_11_user_delete + # run osc_user_delete with our input + ARGV.delete("user") + ARGV.unshift("osc_user") Chef::Knife.run(ARGV, Chef::Application::Knife.options) end @@ -82,12 +85,7 @@ EOF # OSC 11 case if object.username.nil? ui.warn(osc_11_warning) - - # run osc_user_delete with our input - ARGV.delete("user") - ARGV.unshift("osc_user") run_osc_11_user_delete - else # proceed with EC / CS delete delete_object(@user_name) end diff --git a/lib/chef/knife/user_edit.rb b/lib/chef/knife/user_edit.rb index 5bed51a1cc..dd2fc02743 100644 --- a/lib/chef/knife/user_edit.rb +++ b/lib/chef/knife/user_edit.rb @@ -41,6 +41,9 @@ EOF end def run_osc_11_user_edit + # run osc_user_create with our input + ARGV.delete("user") + ARGV.unshift("osc_user") Chef::Knife.run(ARGV, Chef::Application::Knife.options) end @@ -62,12 +65,7 @@ EOF # forward to deprecated command if original_user["username"].nil? ui.warn(osc_11_warning) - - # run osc_user_create with our input - ARGV.delete("user") - ARGV.unshift("osc_user") run_osc_11_user_edit - else # EC / CS 12 user create edited_user = edit_data(original_user) if original_user != edited_user diff --git a/lib/chef/knife/user_reregister.rb b/lib/chef/knife/user_reregister.rb index c2b39761cc..eab2245025 100644 --- a/lib/chef/knife/user_reregister.rb +++ b/lib/chef/knife/user_reregister.rb @@ -41,6 +41,9 @@ EOF end def run_osc_11_user_reregister + # run osc_user_edit with our input + ARGV.delete("user") + ARGV.unshift("osc_user") Chef::Knife.run(ARGV, Chef::Application::Knife.options) end @@ -67,10 +70,6 @@ EOF # forward to deprecated command if user.username.nil? ui.warn(osc_11_warning) - - # run osc_user_edit with our input - ARGV.delete("user") - ARGV.unshift("osc_user") run_osc_11_user_reregister else # EC / CS 12 case user.reregister diff --git a/lib/chef/knife/user_show.rb b/lib/chef/knife/user_show.rb index d0e9f64f53..f5e81e9972 100644 --- a/lib/chef/knife/user_show.rb +++ b/lib/chef/knife/user_show.rb @@ -43,6 +43,9 @@ EOF end def run_osc_11_user_show + # run osc_user_edit with our input + ARGV.delete("user") + ARGV.unshift("osc_user") Chef::Knife.run(ARGV, Chef::Application::Knife.options) end @@ -64,10 +67,6 @@ EOF # forward to deprecated command if user.username.nil? ui.warn(osc_11_warning) - - # run osc_user_edit with our input - ARGV.delete("user") - ARGV.unshift("osc_user") run_osc_11_user_show else output(format_for_display(user)) diff --git a/lib/chef/mixin/api_version_request_handling.rb b/lib/chef/mixin/api_version_request_handling.rb index 851bcb4968..20ab3bf452 100644 --- a/lib/chef/mixin/api_version_request_handling.rb +++ b/lib/chef/mixin/api_version_request_handling.rb @@ -17,43 +17,50 @@ # class Chef - module ApiVersionRequestHandling - # takes in an http exception, and a min and max supported API version and - # handles all the versioning cases - # - # it will return false if there was a non-versioning related error - # or the server and the client are not compatible - # - # if the server does not support versioning, then it will return true, and you - # can assume API v0 is safe to send - def handle_version_http_exception(exception, min_client_supported_version, max_client_supported_version) - # only rescue 406 Unacceptable with proper header - return false if exception.response.code != "406" || exception.response["x-ops-server-api-version"].nil? + module Mixin + module ApiVersionRequestHandling + # Input: + # exeception: + # Net::HTTPServerException that may or may not contain the x-ops-server-api-version header + # supported_client_versions: + # An array of Integers that represent the API versions the client supports. + # + # Output: + # nil: + # If the execption was not a 406 or the server does not support versioning + # Array of length zero: + # If there was no intersection between supported client versions and supported server versions + # Arrary of Integers: + # If there was an intersection of supported versions, the array returns will contain that intersection + def server_client_api_version_intersection(exception, supported_client_versions) + # return empty array unless 406 Unacceptable with proper header + return nil if exception.response.code != "406" || exception.response["x-ops-server-api-version"].nil? + + # intersection of versions the server and client support, will be of length zero if no intersection + server_supported_client_versions = Array.new - # if the version header doesn't exist, just assume API v0 - if exception.response["x-ops-server-api-version"] header = Chef::JSONCompat.from_json(exception.response["x-ops-server-api-version"]) min_server_version = Integer(header["min_version"]) max_server_version = Integer(header["max_version"]) - # if the min API version the server supports is greater than the min version the client supports - # and the max API version the server supports is less than the max version the client supports - if min_server_version > min_client_supported_version || max_server_version < max_client_supported_version - # if it had x-ops-server-api-version header, return false - return false + supported_client_versions.each do |version| + if version >= min_server_version && version <= max_server_version + server_supported_client_versions.push(version) + end end + server_supported_client_versions end - true - end - def reregister_only_v0_supported_error_msg(max_version, min_version) -<<-EOH + def reregister_only_v0_supported_error_msg(max_version, min_version) + <<-EOH The reregister command only supports server API version 0. The server that received the request supports a min version of #{min_version} and a max version of #{max_version}. User keys are now managed via the key rotation commmands. -Please refer to the documentation on how to manage your keys via the key rotation commands. +Please refer to the documentation on how to manage your keys via the key rotation commands: +https://docs.chef.io/server_security.html#key-rotation EOH - end + end + end end end diff --git a/lib/chef/user.rb b/lib/chef/user.rb index 1b5f454099..717deb63c3 100644 --- a/lib/chef/user.rb +++ b/lib/chef/user.rb @@ -36,7 +36,7 @@ class Chef include Chef::Mixin::FromFile include Chef::Mixin::ParamsValidate - include Chef::ApiVersionRequestHandling + include Chef::Mixin::ApiVersionRequestHandling SUPPORTED_API_VERSIONS = [0,1] @@ -169,7 +169,8 @@ class Chef end 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]) + supported_versions = server_client_api_version_intersection(e, SUPPORTED_API_VERSIONS) + raise e unless supported_versions && supported_versions.include?(0) payload = { :username => @username, :display_name => @display_name, @@ -212,7 +213,8 @@ class Chef raise e end else # for other types of errors, test for API versioning errors right away - raise e unless handle_version_http_exception(e, SUPPORTED_API_VERSIONS[0], SUPPORTED_API_VERSIONS[-1]) + supported_versions = server_client_api_version_intersection(e, SUPPORTED_API_VERSIONS) + raise e unless supported_versions && supported_versions.include?(0) end updated_user = chef_root_rest_v0.put("users/#{username}", payload) end diff --git a/spec/support/shared/unit/api_versioning.rb b/spec/support/shared/unit/api_versioning.rb index 61fdfe7faf..a4f353de60 100644 --- a/spec/support/shared/unit/api_versioning.rb +++ b/spec/support/shared/unit/api_versioning.rb @@ -28,7 +28,7 @@ shared_examples_for "version handling" do 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) + allow(object).to receive(:server_client_api_version_intersection).and_return([]) end it "raises the original exception" do diff --git a/spec/support/shared/unit/user_and_client_shared.rb b/spec/support/shared/unit/user_and_client_shared.rb index b0f3902922..bc5ffa07c2 100644 --- a/spec/support/shared/unit/user_and_client_shared.rb +++ b/spec/support/shared/unit/user_and_client_shared.rb @@ -93,7 +93,7 @@ shared_examples_for "user or client create" do context "when the server supports API V0" do before do - allow(object).to receive(:handle_version_http_exception).and_return(true) + allow(object).to receive(:server_client_api_version_intersection).and_return([0]) allow(rest_v1).to receive(:post).and_raise(exception_406) end diff --git a/spec/unit/api_client_spec.rb b/spec/unit/api_client_spec.rb index a95597d40e..ba0eca3284 100644 --- a/spec/unit/api_client_spec.rb +++ b/spec/unit/api_client_spec.rb @@ -487,7 +487,7 @@ describe Chef::ApiClient 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) + allow(@client).to receive(:server_client_api_version_intersection).and_return([0]) end it_should_behave_like "client updating" do diff --git a/spec/unit/mixin/api_version_request_handling_spec.rb b/spec/unit/mixin/api_version_request_handling_spec.rb new file mode 100644 index 0000000000..cc5340e424 --- /dev/null +++ b/spec/unit/mixin/api_version_request_handling_spec.rb @@ -0,0 +1,127 @@ +# +# Author:: Tyler Cloke (tyler@chef.io) +# Copyright:: Copyright 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. +# + +require 'spec_helper' + +describe Chef::Mixin::ApiVersionRequestHandling do + let(:dummy_class) { Class.new { include Chef::Mixin::ApiVersionRequestHandling } } + let(:object) { dummy_class.new } + + describe ".server_client_api_version_intersection" do + let(:default_supported_client_versions) { [0,1,2] } + + + context "when the response code is not 406" do + let(:response) { OpenStruct.new(:code => '405') } + let(:exception) { Net::HTTPServerException.new("405 Something Else", response) } + + it "returns nil" do + expect(object.server_client_api_version_intersection(exception, default_supported_client_versions)). + to be_nil + end + + end # when the response code is not 406 + + context "when the response code is 406" do + let(:response) { OpenStruct.new(:code => '406') } + let(:exception) { Net::HTTPServerException.new("406 Not Acceptable", response) } + + context "when x-ops-server-api-version header does not exist" do + it "returns nil" do + expect(object.server_client_api_version_intersection(exception, default_supported_client_versions)). + to be_nil + end + end # when x-ops-server-api-version header does not exist + + context "when x-ops-server-api-version header exists" do + let(:min_server_version) { 2 } + let(:max_server_version) { 4 } + let(:return_hash) { + { + "min_version" => min_server_version, + "max_version" => max_server_version + } + } + + before(:each) do + allow(response).to receive(:[]).with('x-ops-server-api-version').and_return(Chef::JSONCompat.to_json(return_hash)) + end + + context "when there is no intersection between client and server versions" do + shared_examples_for "no intersection between client and server versions" do + it "return an array" do + expect(object.server_client_api_version_intersection(exception, supported_client_versions)). + to be_a_kind_of(Array) + end + + it "returns an empty array" do + expect(object.server_client_api_version_intersection(exception, supported_client_versions).length). + to eq(0) + end + + end + + context "when all the versions are higher than the max" do + it_should_behave_like "no intersection between client and server versions" do + let(:supported_client_versions) { [5,6,7] } + end + end + + context "when all the versions are lower than the min" do + it_should_behave_like "no intersection between client and server versions" do + let(:supported_client_versions) { [0,1] } + end + end + + end # when there is no intersection between client and server versions + + context "when there is an intersection between client and server versions" do + context "when multiple versions intersect" do + let(:supported_client_versions) { [1,2,3,4,5] } + + it "includes all of the intersection" do + expect(object.server_client_api_version_intersection(exception, supported_client_versions)). + to eq([2,3,4]) + end + end # when multiple versions intersect + + context "when only the min client version intersects" do + let(:supported_client_versions) { [0,1,2] } + + it "includes the intersection" do + expect(object.server_client_api_version_intersection(exception, supported_client_versions)). + to eq([2]) + end + end # when only the min client version intersects + + context "when only the max client version intersects" do + let(:supported_client_versions) { [4,5,6] } + + it "includes the intersection" do + expect(object.server_client_api_version_intersection(exception, supported_client_versions)). + to eq([4]) + end + end # when only the max client version intersects + + end # when there is an intersection between client and server versions + + end # when x-ops-server-api-version header exists + end # when the response code is 406 + + end # .server_client_api_version_intersection +end # Chef::Mixin::ApiVersionRequestHandling diff --git a/spec/unit/user_spec.rb b/spec/unit/user_spec.rb index 15d9591712..57822df7e3 100644 --- a/spec/unit/user_spec.rb +++ b/spec/unit/user_spec.rb @@ -390,8 +390,8 @@ describe Chef::User do @user.update end - it "does not call handle_version_http_exception, since we know to proceed with V0 in this case" do - expect(@user).to_not receive(:handle_version_http_exception) + it "does not call server_client_api_version_intersection, since we know to proceed with V0 in this case" do + expect(@user).to_not receive(:server_client_api_version_intersection) allow(@user.chef_root_rest_v0).to receive(:put).and_return({}) @user.update end @@ -428,7 +428,7 @@ describe Chef::User do context "when the server supports API V0" do before do - allow(@user).to receive(:handle_version_http_exception).and_return(true) + allow(@user).to receive(:server_client_api_version_intersection).and_return([0]) allow(@user.chef_root_rest_v1).to receive(:put).and_raise(exception_406) end @@ -491,7 +491,7 @@ describe Chef::User do context "when handling API V0" do before do - allow(@user).to receive(:handle_version_http_exception).and_return(true) + allow(@user).to receive(:server_client_api_version_intersection).and_return([0]) allow(@user.chef_root_rest_v1).to receive(:post).and_raise(exception_406) end |