diff options
| -rw-r--r-- | Gemfile | 4 | ||||
| -rw-r--r-- | Gemfile.lock | 25 | ||||
| -rw-r--r-- | lib/chef/http.rb | 39 | ||||
| -rw-r--r-- | lib/chef/http/api_versions.rb | 6 | ||||
| -rw-r--r-- | lib/chef/http/authenticator.rb | 21 | ||||
| -rw-r--r-- | lib/chef/mixin/versioned_api.rb | 17 | ||||
| -rw-r--r-- | lib/chef/server_api_versions.rb | 4 | ||||
| -rw-r--r-- | omnibus/config/software/chef.rb | 2 | ||||
| -rw-r--r-- | spec/spec_helper.rb | 3 | ||||
| -rw-r--r-- | spec/unit/http/api_versions_spec.rb | 12 | ||||
| -rw-r--r-- | spec/unit/http/authenticator_spec.rb | 20 | ||||
| -rw-r--r-- | spec/unit/mixin/versioned_api_spec.rb | 21 |
12 files changed, 147 insertions, 27 deletions
@@ -22,8 +22,8 @@ gem "cheffish" # required for rspec tests group(:omnibus_package) do gem "appbundler" gem "rb-readline" - # CVE-2016-4658 https://github.com/sparklemotion/nokogiri/issues/1615 - gem "nokogiri", ">= 1.7.1" + # nokogiri has no ruby-2.4 version for windows so it cannot go into our Gemfile.lock + # gem "nokogiri", ">= 1.7.1" end group(:omnibus_package, :pry) do diff --git a/Gemfile.lock b/Gemfile.lock index 9029de0096..d5652b17eb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,6 +1,6 @@ GIT remote: https://github.com/chef/chef-server - revision: aa0ee4ec5538ef5f4842b3ea0bd089714c8d2c72 + revision: 40c201d434ec14fe8985259b4fed9f81f3176691 specs: oc-chef-pedant (2.2.0) activesupport (>= 4.2.7.1, < 6.0) @@ -54,19 +54,19 @@ GIT GIT remote: https://github.com/poise/halite.git - revision: b0f1372ea7710e47b52c8c843597d21aaed5ebf6 + revision: fc1f42ac129865ffe08b7fb47fd366abe9dbf29c specs: - halite (1.4.1.pre) + halite (1.5.1.pre) bundler chef (>= 12.0, < 14.0) - stove (~> 4.0) + stove (~> 5.0) thor GIT remote: https://github.com/poise/poise-boiler.git - revision: dedd08087d65830e7c7dd0d6c8daa964cfb33999 + revision: d547f77ca7335af7943f681e1d0dd68c825cf429 specs: - poise-boiler (1.13.3.pre) + poise-boiler (1.14.1.pre) bundler chefspec (~> 5.0) codeclimate-test-reporter (~> 0.4) @@ -205,13 +205,13 @@ GEM mixlib-cli (~> 1.4) artifactory (2.8.1) ast (2.3.0) - aws-sdk (2.8.13) - aws-sdk-resources (= 2.8.13) - aws-sdk-core (2.8.13) + aws-sdk (2.8.14) + aws-sdk-resources (= 2.8.14) + aws-sdk-core (2.8.14) aws-sigv4 (~> 1.0) jmespath (~> 1.0) - aws-sdk-resources (2.8.13) - aws-sdk-core (= 2.8.13) + aws-sdk-resources (2.8.14) + aws-sdk-core (= 2.8.14) aws-sigv4 (1.0.0) backports (3.7.0) binding_of_caller (0.7.2) @@ -477,7 +477,7 @@ GEM net-ssh (>= 2.7, < 5.0) net-telnet sfl - stove (4.1.1) + stove (5.0.0) chef-api (~> 0.5) logify (~> 0.2) syslog-logger (1.6.8) @@ -581,7 +581,6 @@ DEPENDENCIES knife-windows mixlib-install netrc - nokogiri (>= 1.7.1) oc-chef-pedant! octokit ohai! diff --git a/lib/chef/http.rb b/lib/chef/http.rb index 12acae953c..c741dcca97 100644 --- a/lib/chef/http.rb +++ b/lib/chef/http.rb @@ -142,13 +142,26 @@ class Chef # Makes an HTTP request to +path+ with the given +method+, +headers+, and # +data+ (if applicable). def request(method, path, headers = {}, data = false) + http_attempts ||= 0 url = create_url(path) method, url, headers, data = apply_request_middleware(method, url, headers, data) response, rest_request, return_value = send_http_request(method, url, headers, data) response, rest_request, return_value = apply_response_middleware(response, rest_request, return_value) + response.error! unless success_response?(response) return_value + + rescue Net::HTTPServerException => e + http_attempts += 1 + response = e.response + if response.kind_of?(Net::HTTPNotAcceptable) && version_retries - http_attempts > 0 + Chef::Log.debug("Negotiating protocol version with #{url}, retry #{http_attempts}/#{version_retries}") + sleep(http_retry_delay) + retry + else + raise + end rescue Exception => exception log_failed_request(response, return_value) unless response.nil? @@ -159,6 +172,7 @@ class Chef end def streaming_request_with_progress(path, headers = {}, &progress_block) + http_attempts ||= 0 url = create_url(path) response, rest_request, return_value = nil, nil, nil tempfile = nil @@ -177,6 +191,16 @@ class Chef response.error! end tempfile + rescue Net::HTTPServerException => e + http_attempts += 1 + response = e.response + if response.kind_of?(Net::HTTPNotAcceptable) && version_retries - http_attempts > 0 + Chef::Log.debug("Negotiating protocol version with #{url}, retry #{http_attempts}/#{version_retries}") + sleep(http_retry_delay) + retry + else + raise + end rescue Exception => e log_failed_request(response, return_value) unless response.nil? if e.respond_to?(:chef_rest_request=) @@ -195,6 +219,7 @@ class Chef # @yield [tempfile] block to process the tempfile # @yieldparams [tempfile<Tempfile>] tempfile def streaming_request(path, headers = {}) + http_attempts ||= 0 url = create_url(path) response, rest_request, return_value = nil, nil, nil tempfile = nil @@ -222,6 +247,16 @@ class Chef end end tempfile + rescue Net::HTTPServerException => e + http_attempts += 1 + response = e.response + if response.kind_of?(Net::HTTPNotAcceptable) && version_retries - http_attempts > 0 + Chef::Log.debug("Negotiating protocol version with #{url}, retry #{http_attempts}/#{version_retries}") + sleep(http_retry_delay) + retry + else + raise + end rescue Exception => e log_failed_request(response, return_value) unless response.nil? if e.respond_to?(:chef_rest_request=) @@ -413,6 +448,10 @@ class Chef end end + def version_retries + @version_retries ||= options[:version_class].possible_requests + end + # @api private def http_retry_delay config[:http_retry_delay] diff --git a/lib/chef/http/api_versions.rb b/lib/chef/http/api_versions.rb index e164da262d..674d8f85a7 100644 --- a/lib/chef/http/api_versions.rb +++ b/lib/chef/http/api_versions.rb @@ -16,6 +16,7 @@ # require "chef/server_api_versions" +require "chef/json_compat" class Chef class HTTP @@ -31,8 +32,11 @@ class Chef end def handle_response(http_response, rest_request, return_value) + if http_response.code == "406" + ServerAPIVersions.instance.reset! + end if http_response.key?("x-ops-server-api-version") - ServerAPIVersions.instance.set_versions(http_response["x-ops-server-api-version"]) + ServerAPIVersions.instance.set_versions(JSONCompat.parse(http_response["x-ops-server-api-version"])) end [http_response, rest_request, return_value] end diff --git a/lib/chef/http/authenticator.rb b/lib/chef/http/authenticator.rb index 84065bf816..65367af107 100644 --- a/lib/chef/http/authenticator.rb +++ b/lib/chef/http/authenticator.rb @@ -30,6 +30,8 @@ class Chef attr_reader :raw_key attr_reader :attr_names attr_reader :auth_credentials + attr_reader :version_class + attr_reader :api_version attr_accessor :sign_request @@ -39,15 +41,12 @@ class Chef @signing_key_filename = opts[:signing_key_filename] @key = load_signing_key(opts[:signing_key_filename], opts[:raw_key]) @auth_credentials = AuthCredentials.new(opts[:client_name], @key) - if opts[:api_version] - @api_version = opts[:api_version] - else - @api_version = DEFAULT_SERVER_API_VERSION - end + @version_class = opts[:version_class] + @api_version = opts[:api_version] end def handle_request(method, url, headers = {}, data = false) - headers["X-Ops-Server-API-Version"] = @api_version + headers["X-Ops-Server-API-Version"] = request_version headers.merge!(authentication_headers(method, url, data, headers)) if sign_requests? [method, url, headers, data] end @@ -64,6 +63,16 @@ class Chef [http_response, rest_request, return_value] end + def request_version + if version_class + version_class.best_request_version + elsif api_version + api_version + else + DEFAULT_SERVER_API_VERSION + end + end + def sign_requests? auth_credentials.sign_requests? && @sign_request end diff --git a/lib/chef/mixin/versioned_api.rb b/lib/chef/mixin/versioned_api.rb index 9c2f2f4cdb..17c9838d29 100644 --- a/lib/chef/mixin/versioned_api.rb +++ b/lib/chef/mixin/versioned_api.rb @@ -40,11 +40,15 @@ class Chef end def versioned_api_class + get_class_for(:max_server_version) + end + + def get_class_for(type) versioned_interfaces.select do |klass| version = klass.send(:minimum_api_version) # min and max versions will be nil if we've not made a request to the server yet, # in which case we'll just start with the highest version and see what happens - ServerAPIVersions.instance.min_server_version.nil? || (version >= ServerAPIVersions.instance.min_server_version && version <= ServerAPIVersions.instance.max_server_version) + ServerAPIVersions.instance.min_server_version.nil? || (version >= ServerAPIVersions.instance.min_server_version && version <= ServerAPIVersions.instance.send(type)) end .sort { |a, b| a.send(:minimum_api_version) <=> b.send(:minimum_api_version) } .last @@ -59,6 +63,17 @@ class Chef module_eval(str, __FILE__, line_no) end + # When teeing up an HTTP request, we need to be able to ask which API version we should use. + # Something in Net::HTTP seems to expect to strip headers, so we return this as a string. + def best_request_version + klass = get_class_for(:max_server_version) + klass.minimum_api_version.to_s + end + + def possible_requests + versioned_interfaces.length + end + def new(*args) object = versioned_api_class.allocate object.send(:initialize, *args) diff --git a/lib/chef/server_api_versions.rb b/lib/chef/server_api_versions.rb index 68dfd5ac90..2a4d0e6a5b 100644 --- a/lib/chef/server_api_versions.rb +++ b/lib/chef/server_api_versions.rb @@ -26,11 +26,11 @@ class Chef end def min_server_version - !@versions.nil? ? @versions["min_version"] : nil + !@versions.nil? ? Integer(@versions["min_version"]) : nil end def max_server_version - !@versions.nil? ? @versions["max_version"] : nil + !@versions.nil? ? Integer(@versions["max_version"]) : nil end def reset! diff --git a/omnibus/config/software/chef.rb b/omnibus/config/software/chef.rb index 4f7319bda1..c0af403b20 100644 --- a/omnibus/config/software/chef.rb +++ b/omnibus/config/software/chef.rb @@ -39,7 +39,7 @@ dependency "bundler" # Worst offenders first to take best advantage of cache: dependency "chef-gem-ffi-yajl" dependency "chef-gem-ohai" -dependency "chef-gem-nokogiri" +dependency "chef-gem-nokogiri" unless windows? dependency "chef-gem-libyajl2" dependency "chef-gem-ruby-prof" dependency "chef-gem-byebug" diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 50ad2a3fb6..8e1faa3060 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -70,6 +70,9 @@ require "chef/chef_fs/file_system_cache" require "chef/api_client_v1" +require "chef/mixin/versioned_api" +require "chef/server_api_versions" + if ENV["CHEF_FIPS"] == "1" Chef::Config.init_openssl end diff --git a/spec/unit/http/api_versions_spec.rb b/spec/unit/http/api_versions_spec.rb index 79c97a1b69..91d46763c2 100644 --- a/spec/unit/http/api_versions_spec.rb +++ b/spec/unit/http/api_versions_spec.rb @@ -38,13 +38,14 @@ describe Chef::HTTP::APIVersions do let(:response_body) { "Thanks for checking in." } let(:response_headers) do { - "x-ops-server-api-version" => { "min_version" => 0, "max_version" => 2 }, + "x-ops-server-api-version" => { "min_version" => 0, "max_version" => 2 }.to_json, } end let(:response) do m = double("HttpResponse", :body => response_body) allow(m).to receive(:key?).with("x-ops-server-api-version").and_return(true) + allow(m).to receive(:code).and_return(return_value) allow(m).to receive(:[]) do |key| response_headers[key] end @@ -66,4 +67,13 @@ describe Chef::HTTP::APIVersions do run_api_version_handler expect(Chef::ServerAPIVersions.instance.min_server_version).to eq(0) end + + context "with an unacceptable api version" do + let (:return_value) { "406" } + it "resets the list of supported versions" do + Chef::ServerAPIVersions.instance.set_versions({ "min_version" => 1, "max_version" => 3 }) + run_api_version_handler + expect(Chef::ServerAPIVersions.instance.min_server_version).to eq(0) + end + end end diff --git a/spec/unit/http/authenticator_spec.rb b/spec/unit/http/authenticator_spec.rb index 7fd2bdc821..5de39523cf 100644 --- a/spec/unit/http/authenticator_spec.rb +++ b/spec/unit/http/authenticator_spec.rb @@ -38,6 +38,26 @@ describe Chef::HTTP::Authenticator do to include({ "X-Ops-Server-API-Version" => Chef::HTTP::Authenticator::DEFAULT_SERVER_API_VERSION }) end + context "when version_class is provided" do + class V0Class; extend Chef::Mixin::VersionedAPI; minimum_api_version 0; end + class V2Class; extend Chef::Mixin::VersionedAPI; minimum_api_version 2; end + + class AuthFactoryClass + extend Chef::Mixin::VersionedAPIFactory + add_versioned_api_class V0Class + add_versioned_api_class V2Class + end + + let(:class_instance) { Chef::HTTP::Authenticator.new({ version_class: AuthFactoryClass }) } + + it "uses it to select the correct http version" do + Chef::ServerAPIVersions.instance.reset! + expect(AuthFactoryClass).to receive(:best_request_version).and_call_original + expect(class_instance.handle_request(method, url, headers, data)[2]). + to include({ "X-Ops-Server-API-Version" => "2" }) + end + end + context "when api_version is set to something other than the default" do let(:class_instance) { Chef::HTTP::Authenticator.new({ :api_version => "-10" }) } diff --git a/spec/unit/mixin/versioned_api_spec.rb b/spec/unit/mixin/versioned_api_spec.rb index 4f2418ca24..e8b65158c4 100644 --- a/spec/unit/mixin/versioned_api_spec.rb +++ b/spec/unit/mixin/versioned_api_spec.rb @@ -87,6 +87,27 @@ describe Chef::Mixin::VersionedAPIFactory do end end + describe "#best_request_version" do + it "returns a String" do + factory_class.add_versioned_api_class V2Class + expect(factory_class.best_request_version).to be_a(String) + end + + it "returns the most relevant version" do + factory_class.add_versioned_api_class V2Class + factory_class.add_versioned_api_class V3Class + expect(factory_class.best_request_version).to eq("3") + end + end + + describe "#possible_requests" do + it "returns the number of registered classes" do + factory_class.add_versioned_api_class V2Class + factory_class.add_versioned_api_class V3Class + expect(factory_class.possible_requests).to eq(2) + end + end + describe "#new" do it "creates an instance of the versioned class" do factory_class.add_versioned_api_class V2Class |