diff options
author | Tyler Cloke <tylercloke@gmail.com> | 2015-05-04 14:30:06 -0700 |
---|---|---|
committer | Tyler Cloke <tylercloke@gmail.com> | 2015-05-04 14:30:06 -0700 |
commit | 20559c2206c5a7f7e1512f6146593dc34a6101fc (patch) | |
tree | 92768bd2da446ddc6fec8e347835d6f88aa396c5 | |
parent | c6c7329ff54851390e0e84d0b681ee8396b3f34c (diff) | |
parent | 81a0a4b1b8b615749953c0154c127e900a7121b9 (diff) | |
download | chef-20559c2206c5a7f7e1512f6146593dc34a6101fc.tar.gz |
Merge pull request #3319 from chef/tc/api-version
Implemented X-Ops-Server-API-Version in Chef requests
-rw-r--r-- | CHANGELOG.md | 1 | ||||
-rw-r--r-- | lib/chef/formatters/error_inspectors/api_error_formatting.rb | 17 | ||||
-rw-r--r-- | lib/chef/formatters/error_inspectors/cookbook_resolve_error_inspector.rb | 2 | ||||
-rw-r--r-- | lib/chef/formatters/error_inspectors/cookbook_sync_error_inspector.rb | 2 | ||||
-rw-r--r-- | lib/chef/formatters/error_inspectors/node_load_error_inspector.rb | 2 | ||||
-rw-r--r-- | lib/chef/formatters/error_inspectors/registration_error_inspector.rb | 4 | ||||
-rw-r--r-- | lib/chef/formatters/error_inspectors/run_list_expansion_error_inspector.rb | 2 | ||||
-rw-r--r-- | lib/chef/http/authenticator.rb | 3 | ||||
-rw-r--r-- | lib/chef/knife.rb | 8 | ||||
-rw-r--r-- | spec/unit/formatters/error_inspectors/api_error_formatting_spec.rb | 75 | ||||
-rw-r--r-- | spec/unit/http/authenticator_spec.rb | 69 | ||||
-rw-r--r-- | spec/unit/knife_spec.rb | 13 | ||||
-rw-r--r-- | spec/unit/rest_spec.rb | 30 |
13 files changed, 207 insertions, 21 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 15d093da29..462d1c11ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ * [Issue #3316](https://github.com/chef/chef/issues/3316) Fix idempotency issues with the `windows_package` resource * [pr#3295](https://github.com/chef/chef/pull/3295): Stop mutating `new_resource.checksum` in file providers. Fixes some ChecksumMismatch exceptions like [issue#3168](https://github.com/chef/chef/issues/3168) * [pr#3320] Sanitize non-UTF8 characters in the node data before doing node.save(). Works around many UTF8 exception issues reported on node.save(). +* Implemented X-Ops-Server-API-Version with a API version of 0, as well as error handling when the Chef server does not support the API version that the client supports. ## 12.3.0 diff --git a/lib/chef/formatters/error_inspectors/api_error_formatting.rb b/lib/chef/formatters/error_inspectors/api_error_formatting.rb index 652d478b40..ec25f4e903 100644 --- a/lib/chef/formatters/error_inspectors/api_error_formatting.rb +++ b/lib/chef/formatters/error_inspectors/api_error_formatting.rb @@ -16,6 +16,8 @@ # limitations under the License. # +require 'chef/http/authenticator' + class Chef module Formatters @@ -65,6 +67,21 @@ E error_description.section("Server Response:",format_rest_error) end + def describe_406_error(error_description, response) + if Chef::JSONCompat.from_json(response.body)["error"] == "invalid-x-ops-server-api-version" + min_version = Chef::JSONCompat.from_json(response.body)["min_version"] + max_version = Chef::JSONCompat.from_json(response.body)["max_version"] + error_description.section("Incompatible server API version:",<<-E) +This version of Chef is not supported by the Chef server you sent this request to +This version of Chef requires a server API version of #{Chef::HTTP::Authenticator::SERVER_API_VERSION} +The Chef server you sent the request to supports a min API version of #{min_version} and a max API version of #{max_version} +Please either update your Chef client or server to be a compatible set +E + else + describe_http_error(error_description) + end + end + def describe_500_error(error_description) error_description.section("Unknown Server Error:",<<-E) The server had a fatal error attempting to load the node data. diff --git a/lib/chef/formatters/error_inspectors/cookbook_resolve_error_inspector.rb b/lib/chef/formatters/error_inspectors/cookbook_resolve_error_inspector.rb index aa5eb8485d..e011fa9d9b 100644 --- a/lib/chef/formatters/error_inspectors/cookbook_resolve_error_inspector.rb +++ b/lib/chef/formatters/error_inspectors/cookbook_resolve_error_inspector.rb @@ -72,6 +72,8 @@ E describe_500_error(error_description) when Net::HTTPBadGateway, Net::HTTPServiceUnavailable describe_503_error(error_description) + when Net::HTTPNotAcceptable + describe_406_error(error_description, response) else describe_http_error(error_description) end diff --git a/lib/chef/formatters/error_inspectors/cookbook_sync_error_inspector.rb b/lib/chef/formatters/error_inspectors/cookbook_sync_error_inspector.rb index 0cb849a17f..971dbd664e 100644 --- a/lib/chef/formatters/error_inspectors/cookbook_sync_error_inspector.rb +++ b/lib/chef/formatters/error_inspectors/cookbook_sync_error_inspector.rb @@ -67,6 +67,8 @@ class Chef describe_500_error(error_description) when Net::HTTPBadGateway, Net::HTTPServiceUnavailable, Net::HTTPGatewayTimeOut describe_503_error(error_description) + when Net::HTTPNotAcceptable + describe_406_error(error_description, response) else describe_http_error(error_description) end diff --git a/lib/chef/formatters/error_inspectors/node_load_error_inspector.rb b/lib/chef/formatters/error_inspectors/node_load_error_inspector.rb index e257ee30c0..d81a9f7cc8 100644 --- a/lib/chef/formatters/error_inspectors/node_load_error_inspector.rb +++ b/lib/chef/formatters/error_inspectors/node_load_error_inspector.rb @@ -84,6 +84,8 @@ E describe_500_error(error_description) when Net::HTTPBadGateway, Net::HTTPServiceUnavailable describe_503_error(error_description) + when Net::HTTPNotAcceptable + describe_406_error(error_description, response) else describe_http_error(error_description) end diff --git a/lib/chef/formatters/error_inspectors/registration_error_inspector.rb b/lib/chef/formatters/error_inspectors/registration_error_inspector.rb index f31b348278..dbd23f4a52 100644 --- a/lib/chef/formatters/error_inspectors/registration_error_inspector.rb +++ b/lib/chef/formatters/error_inspectors/registration_error_inspector.rb @@ -9,6 +9,8 @@ class Chef # TODO: Lots of duplication with the node_load_error_inspector, just # slightly tweaked to talk about validation keys instead of other keys. class RegistrationErrorInspector + include APIErrorFormatting + attr_reader :exception attr_reader :node_name attr_reader :config @@ -94,6 +96,8 @@ E error_description.section("Relevant Config Settings:",<<-E) chef_server_url "#{server_url}" E + when Net::HTTPNotAcceptable + describe_406_error(error_description, response) when Net::HTTPInternalServerError error_description.section("Unknown Server Error:",<<-E) The server had a fatal error attempting to load the node data. diff --git a/lib/chef/formatters/error_inspectors/run_list_expansion_error_inspector.rb b/lib/chef/formatters/error_inspectors/run_list_expansion_error_inspector.rb index ac19a983af..818228276e 100644 --- a/lib/chef/formatters/error_inspectors/run_list_expansion_error_inspector.rb +++ b/lib/chef/formatters/error_inspectors/run_list_expansion_error_inspector.rb @@ -98,6 +98,8 @@ E error_description.section("Possible Causes:",<<-E) * Your client (#{username}) may have misconfigured authorization permissions. E + when Net::HTTPNotAcceptable + describe_406_error(error_description, response) when Net::HTTPInternalServerError error_description.section("Unknown Server Error:",<<-E) The server had a fatal error attempting to load a role. diff --git a/lib/chef/http/authenticator.rb b/lib/chef/http/authenticator.rb index 4255f18cbd..4ec35add34 100644 --- a/lib/chef/http/authenticator.rb +++ b/lib/chef/http/authenticator.rb @@ -24,6 +24,8 @@ class Chef class HTTP class Authenticator + SERVER_API_VERSION = "0" + attr_reader :signing_key_filename attr_reader :raw_key attr_reader :attr_names @@ -41,6 +43,7 @@ class Chef def handle_request(method, url, headers={}, data=false) headers.merge!(authentication_headers(method, url, data)) if sign_requests? + headers.merge!({'X-Ops-Server-API-Version' => SERVER_API_VERSION}) [method, url, headers, data] end diff --git a/lib/chef/knife.rb b/lib/chef/knife.rb index 2e0694aebc..4c59f831de 100644 --- a/lib/chef/knife.rb +++ b/lib/chef/knife.rb @@ -27,6 +27,7 @@ require 'chef/knife/core/subcommand_loader' require 'chef/knife/core/ui' require 'chef/local_mode' require 'chef/rest' +require 'chef/http/authenticator' require 'pp' class Chef @@ -483,6 +484,13 @@ class Chef when Net::HTTPServiceUnavailable ui.error "Service temporarily unavailable" ui.info "Response: #{format_rest_error(response)}" + when Net::HTTPNotAcceptable + min_version = Chef::JSONCompat.from_json(response.body)["min_version"] + max_version = Chef::JSONCompat.from_json(response.body)["max_version"] + ui.error "The version of Chef that Knife is using is not supported by the Chef server you sent this request to" + ui.info "This version of Chef requires a server API version of #{Chef::HTTP::Authenticator::SERVER_API_VERSION}" + ui.info "The Chef server you sent the request to supports a min API verson of #{min_version} and a max API version of #{max_version}" + ui.info "Please either update your Chef client or server to be a compatible set" else ui.error response.message ui.info "Response: #{format_rest_error(response)}" diff --git a/spec/unit/formatters/error_inspectors/api_error_formatting_spec.rb b/spec/unit/formatters/error_inspectors/api_error_formatting_spec.rb new file mode 100644 index 0000000000..4b3b8bff44 --- /dev/null +++ b/spec/unit/formatters/error_inspectors/api_error_formatting_spec.rb @@ -0,0 +1,75 @@ +# +# 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. +# + +require 'spec_helper' +require 'chef/formatters/error_inspectors/api_error_formatting' + +describe Chef::Formatters::APIErrorFormatting do + let(:class_instance) { (Class.new { include Chef::Formatters::APIErrorFormatting }).new } + let(:error_description) { instance_double(Chef::Formatters::ErrorDescription) } + let(:response) { double("response") } + before do + allow(response).to receive(:body) + end + + + context "when describe_406_error is called" do + context "when response.body['error'] == 'invalid-x-ops-server-api-version'" do + let(:min_version) { "2" } + let(:max_version) { "5" } + let(:return_hash) { + { + "error" => "invalid-x-ops-server-api-version", + "min_version" => min_version, + "max_version" => max_version + } + } + + before do + allow(Chef::JSONCompat).to receive(:from_json).and_return(return_hash) + end + + it "prints an error about client and server API version incompatibility with a min API version" do + expect(error_description).to receive(:section).with("Incompatible server API version:",/a min API version of #{min_version}/) + class_instance.describe_406_error(error_description, response) + end + + it "prints an error about client and server API version incompatibility with a max API version" do + expect(error_description).to receive(:section).with("Incompatible server API version:",/a max API version of #{max_version}/) + class_instance.describe_406_error(error_description, response) + end + end + + context "when response.body['error'] != 'invalid-x-ops-server-api-version'" do + let(:return_hash) { + { + "error" => "some-other-error" + } + } + + before do + allow(Chef::JSONCompat).to receive(:from_json).and_return(return_hash) + end + + it "forwards the error_description to describe_http_error" do + expect(class_instance).to receive(:describe_http_error).with(error_description) + class_instance.describe_406_error(error_description, response) + end + end + end +end diff --git a/spec/unit/http/authenticator_spec.rb b/spec/unit/http/authenticator_spec.rb new file mode 100644 index 0000000000..82d38d6c2b --- /dev/null +++ b/spec/unit/http/authenticator_spec.rb @@ -0,0 +1,69 @@ +# +# 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. +# + +require 'spec_helper' +require 'chef/http/authenticator' + +describe Chef::HTTP::Authenticator do + let(:class_instance) { Chef::HTTP::Authenticator.new } + let(:method) { double("method") } + let(:url) { double("url") } + let(:headers) { Hash.new } + let(:data) { double("data") } + + before do + allow(class_instance).to receive(:authentication_headers).and_return({}) + end + + context "when handle_request is called" do + shared_examples_for "merging the server API version into the headers" do + it "merges X-Ops-Server-API-Version into the headers" do + # headers returned + expect(class_instance.handle_request(method, url, headers, data)[2]). + to include({'X-Ops-Server-API-Version' => Chef::HTTP::Authenticator::SERVER_API_VERSION}) + end + end + + context "when !sign_requests?" do + before do + allow(class_instance).to receive(:sign_requests?).and_return(false) + end + + it_behaves_like "merging the server API version into the headers" + + it "authentication_headers is not called" do + expect(class_instance).to_not receive(:authentication_headers) + class_instance.handle_request(method, url, headers, data) + end + + end + + context "when sign_requests?" do + before do + allow(class_instance).to receive(:sign_requests?).and_return(true) + end + + it_behaves_like "merging the server API version into the headers" + + it "calls authentication_headers with the proper input" do + expect(class_instance).to receive(:authentication_headers).with(method, url, data).and_return({}) + class_instance.handle_request(method, url, headers, data) + end + end + end +end diff --git a/spec/unit/knife_spec.rb b/spec/unit/knife_spec.rb index b748232081..7cb16aae2a 100644 --- a/spec/unit/knife_spec.rb +++ b/spec/unit/knife_spec.rb @@ -130,7 +130,8 @@ describe Chef::Knife do "Accept-Encoding"=>"gzip;q=1.0,deflate;q=0.6,identity;q=0.3", 'X-Chef-Version' => Chef::VERSION, "Host"=>"api.opscode.piab", - "X-REMOTE-REQUEST-ID"=>request_id}} + "X-REMOTE-REQUEST-ID"=>request_id, + 'X-Ops-Server-API-Version' => Chef::HTTP::Authenticator::SERVER_API_VERSION}} let(:request_id) {"1234"} @@ -374,6 +375,16 @@ describe Chef::Knife do expect(stderr.string).to match(%r[Response: nothing to see here]) end + it "formats 406s (non-supported API version error) nicely" do + response = Net::HTTPNotAcceptable.new("1.1", "406", "Not Acceptable") + response.instance_variable_set(:@read, true) # I hate you, net/http. + allow(response).to receive(:body).and_return(Chef::JSONCompat.to_json(:error => "sad trombone", :min_version => "0", :max_version => "1")) + allow(knife).to receive(:run).and_raise(Net::HTTPServerException.new("406 Not Acceptable", response)) + knife.run_with_pretty_exceptions + expect(stderr.string).to include('The version of Chef that Knife is using is not supported by the Chef server you sent this request to') + expect(stderr.string).to include("This version of Chef requires a server API version of #{Chef::HTTP::Authenticator::SERVER_API_VERSION}") + end + it "formats 500s nicely" do response = Net::HTTPInternalServerError.new("1.1", "500", "Internal Server Error") response.instance_variable_set(:@read, true) # I hate you, net/http. diff --git a/spec/unit/rest_spec.rb b/spec/unit/rest_spec.rb index 85c9e3df8f..b4f8f336a9 100644 --- a/spec/unit/rest_spec.rb +++ b/spec/unit/rest_spec.rb @@ -69,8 +69,8 @@ describe Chef::REST do rest end - let(:standard_read_headers) {{"Accept"=>"application/json", "Accept"=>"application/json", "Accept-Encoding"=>"gzip;q=1.0,deflate;q=0.6,identity;q=0.3", "X-REMOTE-REQUEST-ID"=>request_id}} - let(:standard_write_headers) {{"Accept"=>"application/json", "Content-Type"=>"application/json", "Accept"=>"application/json", "Accept-Encoding"=>"gzip;q=1.0,deflate;q=0.6,identity;q=0.3", "X-REMOTE-REQUEST-ID"=>request_id}} + let(:standard_read_headers) {{"Accept"=>"application/json", "Accept"=>"application/json", "Accept-Encoding"=>"gzip;q=1.0,deflate;q=0.6,identity;q=0.3", "X-REMOTE-REQUEST-ID"=>request_id, 'X-Ops-Server-API-Version' => Chef::HTTP::Authenticator::SERVER_API_VERSION}} + let(:standard_write_headers) {{"Accept"=>"application/json", "Content-Type"=>"application/json", "Accept"=>"application/json", "Accept-Encoding"=>"gzip;q=1.0,deflate;q=0.6,identity;q=0.3", "X-REMOTE-REQUEST-ID"=>request_id, 'X-Ops-Server-API-Version' => Chef::HTTP::Authenticator::SERVER_API_VERSION}} before(:each) do Chef::Log.init(log_stringio) @@ -277,19 +277,6 @@ describe Chef::REST do rest end - let(:base_headers) do - { - 'Accept' => 'application/json', - 'X-Chef-Version' => Chef::VERSION, - 'Accept-Encoding' => Chef::REST::RESTRequest::ENCODING_GZIP_DEFLATE, - 'X-REMOTE-REQUEST-ID' => request_id - } - end - - let (:req_with_body_headers) do - base_headers.merge("Content-Type" => "application/json", "Content-Length" => '13') - end - before(:each) do Chef::Config[:ssl_client_cert] = nil Chef::Config[:ssl_client_key] = nil @@ -304,7 +291,8 @@ describe Chef::REST do 'X-Chef-Version' => Chef::VERSION, 'Accept-Encoding' => Chef::REST::RESTRequest::ENCODING_GZIP_DEFLATE, 'Host' => host_header, - 'X-REMOTE-REQUEST-ID' => request_id + 'X-REMOTE-REQUEST-ID' => request_id, + 'X-Ops-Server-API-Version' => Chef::HTTP::Authenticator::SERVER_API_VERSION } end @@ -548,7 +536,7 @@ describe Chef::REST do end end end - end + end # as JSON API requests context "when streaming downloads to a tempfile" do let!(:tempfile) { Tempfile.open("chef-rspec-rest_spec-line-@{__LINE__}--") } @@ -586,7 +574,8 @@ describe Chef::REST do 'X-Chef-Version' => Chef::VERSION, 'Accept-Encoding' => Chef::REST::RESTRequest::ENCODING_GZIP_DEFLATE, 'Host' => host_header, - 'X-REMOTE-REQUEST-ID'=> request_id + 'X-REMOTE-REQUEST-ID'=> request_id, + 'X-Ops-Server-API-Version' => Chef::HTTP::Authenticator::SERVER_API_VERSION } expect(Net::HTTP::Get).to receive(:new).with("/?foo=bar", expected_headers).and_return(request_mock) rest.streaming_request(url, {}) @@ -597,7 +586,8 @@ describe Chef::REST do 'X-Chef-Version' => Chef::VERSION, 'Accept-Encoding' => Chef::REST::RESTRequest::ENCODING_GZIP_DEFLATE, 'Host' => host_header, - 'X-REMOTE-REQUEST-ID'=> request_id + 'X-REMOTE-REQUEST-ID'=> request_id, + 'X-Ops-Server-API-Version' => Chef::HTTP::Authenticator::SERVER_API_VERSION } expect(Net::HTTP::Get).to receive(:new).with("/?foo=bar", expected_headers).and_return(request_mock) rest.streaming_request(url, {}) @@ -695,7 +685,7 @@ describe Chef::REST do expect(block_called).to be_truthy end end - end + end # when making REST requests context "when following redirects" do let(:rest) do |