diff options
author | Lamont Granquist <lamont@scriptkiddie.org> | 2020-07-28 14:04:49 -0700 |
---|---|---|
committer | Lamont Granquist <lamont@scriptkiddie.org> | 2020-07-28 14:07:57 -0700 |
commit | d9b9ea1a737772cf0d3c3a3064057b60c47a9549 (patch) | |
tree | 781db4d737bdaef44b3e304a628b23eae8491741 | |
parent | d8e128634615246695f5ff82d7d415db89fc8708 (diff) | |
download | chef-d9b9ea1a737772cf0d3c3a3064057b60c47a9549.tar.gz |
Fix protocol negotiation for unversioned objects
We generally always do protocol negotiation on the get of the node
object and the Chef::Node is an unversioned API, so we need to
test that we backoff the API version to the highest version the server
offers and then retry again.
Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>
-rw-r--r-- | lib/chef/http.rb | 12 | ||||
-rw-r--r-- | lib/chef/http/authenticator.rb | 2 | ||||
-rw-r--r-- | lib/chef/server_api_versions.rb | 4 | ||||
-rw-r--r-- | spec/unit/server_api_spec.rb | 31 |
4 files changed, 35 insertions, 14 deletions
diff --git a/lib/chef/http.rb b/lib/chef/http.rb index eed2a41815..436e5d14eb 100644 --- a/lib/chef/http.rb +++ b/lib/chef/http.rb @@ -154,7 +154,7 @@ class Chef rescue Net::HTTPClientException => e http_attempts += 1 response = e.response - if response.is_a?(Net::HTTPNotAcceptable) && version_retries - http_attempts > 0 + if response.is_a?(Net::HTTPNotAcceptable) && version_retries - http_attempts >= 0 Chef::Log.trace("Negotiating protocol version with #{url}, retry #{http_attempts}/#{version_retries}") retry else @@ -193,7 +193,7 @@ class Chef rescue Net::HTTPClientException => e http_attempts += 1 response = e.response - if response.is_a?(Net::HTTPNotAcceptable) && version_retries - http_attempts > 0 + if response.is_a?(Net::HTTPNotAcceptable) && version_retries - http_attempts >= 0 Chef::Log.trace("Negotiating protocol version with #{url}, retry #{http_attempts}/#{version_retries}") retry else @@ -249,7 +249,7 @@ class Chef rescue Net::HTTPClientException => e http_attempts += 1 response = e.response - if response.is_a?(Net::HTTPNotAcceptable) && version_retries - http_attempts > 0 + if response.is_a?(Net::HTTPNotAcceptable) && version_retries - http_attempts >= 0 Chef::Log.trace("Negotiating protocol version with #{url}, retry #{http_attempts}/#{version_retries}") retry else @@ -470,11 +470,7 @@ class Chef end def version_retries - @version_retries ||= if options[:version_class] - options[:version_class].possible_requests - else - 0 - end + @version_retries ||= options[:version_class]&.possible_requests || 1 end # @api private diff --git a/lib/chef/http/authenticator.rb b/lib/chef/http/authenticator.rb index e65e93d2ac..4a29fcea33 100644 --- a/lib/chef/http/authenticator.rb +++ b/lib/chef/http/authenticator.rb @@ -68,6 +68,8 @@ class Chef version_class.best_request_version elsif api_version api_version + elsif Chef::ServerAPIVersions.instance.negotiated? + Chef::ServerAPIVersions.instance.max_server_version.to_s else DEFAULT_SERVER_API_VERSION end diff --git a/lib/chef/server_api_versions.rb b/lib/chef/server_api_versions.rb index f550fc1d66..30e2802ef0 100644 --- a/lib/chef/server_api_versions.rb +++ b/lib/chef/server_api_versions.rb @@ -51,6 +51,10 @@ class Chef @unversioned end + def negotiated? + !@versions.nil? || unversioned? + end + def reset! @versions = nil @unversioned = false diff --git a/spec/unit/server_api_spec.rb b/spec/unit/server_api_spec.rb index 1b5d729383..5d78c4bd9b 100644 --- a/spec/unit/server_api_spec.rb +++ b/spec/unit/server_api_spec.rb @@ -116,17 +116,36 @@ describe Chef::ServerAPI do it "500 on a get retries and gets correctly " do WebMock.disable_net_connect! - get_body = { bar: "baz" } headers = { "Accept" => "application/json", "Accept-Encoding" => "gzip;q=1.0,deflate;q=0.6,identity;q=0.3", "Host" => "chef.example.com:4000", "X-Chef-Version" => Chef::VERSION, "X-Ops-Sign" => "algorithm=sha1;version=1.1;", "X-Ops-Userid" => "silent-bob" } stub_request(:get, "http://chef.example.com:4000/foo").with(headers: headers).to_return(status: [500, "Internal Server Error"]) stub_request(:get, "http://chef.example.com:4000/foo").with(headers: headers).to_return(status: 200, body: "", headers: {}) client.get("foo") end - end - it "does not retry a 406 Not Acceptable" do - WebMock.disable_net_connect! - stub_request(:get, "http://chef.example.com:4000/foo").to_return(status: [406, "Not Acceptable"]) - expect { client.get("foo") }.to raise_error(Net::HTTPServerException) + it "406 on a post does protocol negotiation" do + WebMock.disable_net_connect! + post_body = { bar: "baz" } + body_406 = '{"error":"invalid-x-ops-server-api-version","message":"Specified version 2 not supported","min_version":0,"max_version":1}' + stub_request(:post, "http://chef.example.com:4000/foo").with(body: post_body.to_json, headers: { "X-Ops-Server-Api-Version" => "2" }).to_return(status: [406, "Not Acceptable"], body: body_406 ) + stub_request(:post, "http://chef.example.com:4000/foo").with(body: post_body.to_json, headers: { "X-Ops-Server-Api-Version" => "0" }).to_return(status: 200, body: "", headers: {}) + client.post("foo", post_body) + end + + it "406 on a put does protocol negotiation" do + WebMock.disable_net_connect! + put_body = { bar: "baz" } + body_406 = '{"error":"invalid-x-ops-server-api-version","message":"Specified version 2 not supported","min_version":0,"max_version":1}' + stub_request(:put, "http://chef.example.com:4000/foo").with(body: put_body.to_json, headers: { "X-Ops-Server-Api-Version" => "2" }).to_return(status: [406, "Not Acceptable"], body: body_406 ) + stub_request(:put, "http://chef.example.com:4000/foo").with(body: put_body.to_json, headers: { "X-Ops-Server-Api-Version" => "0" }).to_return(status: 200, body: "", headers: {}) + client.put("foo", put_body) + end + + it "406 on a get does protocol negotiation" do + WebMock.disable_net_connect! + body_406 = '{"error":"invalid-x-ops-server-api-version","message":"Specified version 2 not supported","min_version":0,"max_version":1}' + stub_request(:get, "http://chef.example.com:4000/foo").with(headers: { "X-Ops-Server-Api-Version" => "2" }).to_return(status: [406, "Not Acceptable"], body: body_406 ) + stub_request(:get, "http://chef.example.com:4000/foo").with(headers: { "X-Ops-Server-Api-Version" => "0" }).to_return(status: 200, body: "", headers: {}) + client.get("foo") + end end end |