support HEAD http request on named node endpoint #1218

jeremymv2 · 2017-04-05T18:38:03Z

This PR adds support for the HEAD HTTP verb on the named node endpoint.
Since node objects can be quite large, querying for their existence via GET can be expensive.

The benefits:

less expensive checks for node existence
potential optimization to incorporate into Chef::ChefFS

Existing node:

/Devel/ChefProject/chef-server/dev (jeremymv2/node_http_head $%)$ knife raw -m HEAD "/nodes/foo" -VV
INFO: Using configuration from /Users/jmiller/Devel/ChefProject/chef-server/dev/.chef/knife.rb
DEBUG: Chef::HTTP calling Chef::HTTP::JSONOutput#handle_request
DEBUG: Chef::HTTP calling Chef::HTTP::CookieManager#handle_request
DEBUG: Chef::HTTP calling Chef::HTTP::Decompressor#handle_request
DEBUG: Chef::HTTP calling Chef::HTTP::Authenticator#handle_request
DEBUG: Signing the request as admin
DEBUG: Chef::HTTP calling Chef::HTTP::RemoteRequestID#handle_request
DEBUG: Initiating HEAD to https://api.chef-server.dev/organizations/test/nodes/foo
DEBUG: ---- HTTP Request Header Data: ----
DEBUG: Content-Type: application/json
DEBUG: Accept: application/json
DEBUG: Accept-Encoding: gzip;q=1.0,deflate;q=0.6,identity;q=0.3
DEBUG: X-Ops-Server-API-Version: 1
DEBUG: X-OPS-SIGN: algorithm=sha1;version=1.1;
DEBUG: X-OPS-USERID: admin
DEBUG: X-OPS-TIMESTAMP: 2017-04-05T18:32:26Z
DEBUG: X-OPS-CONTENT-HASH: 2jmj7l5rSw0yVb/vlWAYkK/YBwk=
DEBUG: X-OPS-AUTHORIZATION-1: wyHnZ29GHvHD6Q0ACgnSqroxw9PjQJFCEsA1+s4S/6EHFTnijkTjqDQDg38h
DEBUG: X-OPS-AUTHORIZATION-2: xTWWrLtS8/Ax07kZAUleGna83/WXdDeoaBRmug0XmN9Uucn180b0AN25imGG
DEBUG: X-OPS-AUTHORIZATION-3: 5JGWETXe5ZtSR5+IaBPyIRezJu8CGwjXTrowBRgxqS0p9oZ3vyK/odDgxTzV
DEBUG: X-OPS-AUTHORIZATION-4: pYrTpWgt0ZvzzJrEoG2VdBgpOtCgSMPfeRpRWNem0wyI/VyFZ1pvlsCPNOAj
DEBUG: X-OPS-AUTHORIZATION-5: WxPiiOjhpCEep3OyQoynn1iu6egd35pOpCIvXNab6X7PBJlTD8NDi/CuApwN
DEBUG: X-OPS-AUTHORIZATION-6: nvhnQSaypa0Ilu7gaasTE1UDpc6qhSt6E8RS75ZlXg==
DEBUG: HOST: api.chef-server.dev:443
DEBUG: X-REMOTE-REQUEST-ID: 740ffd42-4ac4-495d-b1aa-02af91fb83d0
DEBUG: ---- End HTTP Request Header Data ----
DEBUG: ---- HTTP Status and Header Data: ----
DEBUG: HTTP 1.1 200 OK
DEBUG: server: openresty/1.11.2.1
DEBUG: date: Wed, 05 Apr 2017 18:32:26 GMT
DEBUG: content-type: application/json
DEBUG: content-length: 0
DEBUG: connection: close
DEBUG: x-ops-server-api-version: {"min_version":"0","max_version":"1","request_version":"1","response_version":"1"}
DEBUG: x-ops-api-info: flavor=cs;version=12.0.0;oc_erchef=12.14.0-7467208
DEBUG: ---- End HTTP Status/Header Data ----
DEBUG: Chef::HTTP calling Chef::HTTP::RemoteRequestID#handle_response
DEBUG: Chef::HTTP calling Chef::HTTP::Authenticator#handle_response
DEBUG: Chef::HTTP calling Chef::HTTP::Decompressor#handle_response
DEBUG: Chef::HTTP calling Chef::HTTP::CookieManager#handle_response
DEBUG: Chef::HTTP calling Chef::HTTP::JSONOutput#handle_response
null
~/Devel/ChefProject/chef-server/dev (jeremymv2/node_http_head $%)$

Non-existent node:

~/Devel/ChefProject/chef-server/dev (jeremymv2/node_http_head $%)$ knife raw -m HEAD "/nodes/bar" -VV
INFO: Using configuration from /Users/jmiller/Devel/ChefProject/chef-server/dev/.chef/knife.rb
DEBUG: Chef::HTTP calling Chef::HTTP::JSONOutput#handle_request
DEBUG: Chef::HTTP calling Chef::HTTP::CookieManager#handle_request
DEBUG: Chef::HTTP calling Chef::HTTP::Decompressor#handle_request
DEBUG: Chef::HTTP calling Chef::HTTP::Authenticator#handle_request
DEBUG: Signing the request as admin
DEBUG: Chef::HTTP calling Chef::HTTP::RemoteRequestID#handle_request
DEBUG: Initiating HEAD to https://api.chef-server.dev/organizations/test/nodes/bar
DEBUG: ---- HTTP Request Header Data: ----
DEBUG: Content-Type: application/json
DEBUG: Accept: application/json
DEBUG: Accept-Encoding: gzip;q=1.0,deflate;q=0.6,identity;q=0.3
DEBUG: X-Ops-Server-API-Version: 1
DEBUG: X-OPS-SIGN: algorithm=sha1;version=1.1;
DEBUG: X-OPS-USERID: admin
DEBUG: X-OPS-TIMESTAMP: 2017-04-05T18:33:14Z
DEBUG: X-OPS-CONTENT-HASH: 2jmj7l5rSw0yVb/vlWAYkK/YBwk=
DEBUG: X-OPS-AUTHORIZATION-1: UKXK4zIpcQ4zpIRSV5ne9pxOKY2+9llxya9LcWAI1Dp6yxaZmoYYIi8GS+KF
DEBUG: X-OPS-AUTHORIZATION-2: ReWxpMG7TK4DnnhTR+dx6mbu5Hoe4fcGg71Gbzwwr4sZZFn7ArDzjenMBHjg
DEBUG: X-OPS-AUTHORIZATION-3: BHZqRDGTsBDYxyGU2+kXDpmWvWbtJwXO7ZAWLpy4v8zjUkL4tyteQcFjbKVU
DEBUG: X-OPS-AUTHORIZATION-4: lE33vukZkcRT+MQ9WTNpcSx/xGWucYrwBAZZeWSdMFp/RAgWZSbEUS6kEPRL
DEBUG: X-OPS-AUTHORIZATION-5: 9ipwcUv9pWeV+2PgdZ05zblzPEWv6ipkMNIfwD0i9Kqw3KiW/jsn2m5w/sd3
DEBUG: X-OPS-AUTHORIZATION-6: 96C+zVJnRWUxSU2VvZgO1ETekR34HcN6zsHim4raAw==
DEBUG: HOST: api.chef-server.dev:443
DEBUG: X-REMOTE-REQUEST-ID: ceeb40b5-f01f-48db-815f-9dea1764da34
DEBUG: ---- End HTTP Request Header Data ----
DEBUG: ---- HTTP Status and Header Data: ----
DEBUG: HTTP 1.1 404 Object Not Found
DEBUG: server: openresty/1.11.2.1
DEBUG: date: Wed, 05 Apr 2017 18:33:14 GMT
DEBUG: content-length: 34
DEBUG: connection: close
DEBUG: x-ops-server-api-version: {"min_version":"0","max_version":"1","request_version":"1","response_version":"1"}
DEBUG: x-ops-api-info: flavor=cs;version=12.0.0;oc_erchef=12.14.0-7467208
DEBUG: ---- End HTTP Status/Header Data ----
DEBUG: Chef::HTTP calling Chef::HTTP::RemoteRequestID#handle_response
DEBUG: Chef::HTTP calling Chef::HTTP::Authenticator#handle_response
DEBUG: Chef::HTTP calling Chef::HTTP::Decompressor#handle_response
DEBUG: Chef::HTTP calling Chef::HTTP::CookieManager#handle_response
DEBUG: Chef::HTTP calling Chef::HTTP::JSONOutput#handle_response
DEBUG: Expected JSON response, but got content-type ''
INFO: HTTP Request Returned 404 Object Not Found:
ERROR: Server responded with error 404 "Object Not Found"
~/Devel/ChefProject/chef-server/dev (jeremymv2/node_http_head $%)$

Logs for a successful query:

==> /var/log/opscode/nginx/access.log <==
192.168.33.1 - - [05/Apr/2017:14:35:12 -0400]  "HEAD /organizations/test/nodes/foo HTTP/1.1" 200 "0.013" 0 "-" "Chef Knife/12.18.31 (ruby-2.3.1-p112; ohai-8.23.0; x86_64-darwin14; +https://chef.io)" "127.0.0.1:8000" "200" "0.012" "12.18.31" "algorithm=sha1;version=1.1;" "admin" "2017-04-05T18:35:12Z" "2jmj7l5rSw0yVb/vlWAYkK/YBwk=" 1071

==> /var/log/opscode/opscode-erchef/requests.log.1 <==
2017-04-05T18:35:12Z oc_erchef@api method=HEAD; path=/organizations/test/nodes/foo; status=200; req_id=g3IAA2QADW9jX2VyY2hlZkBhcGkCAANAJwEAAAEAAAAA; org_name=test; couchdb_groups=false; couchdb_organizations=false; couchdb_containers=false; couchdb_acls=false; 503_mode=false; couchdb_associations=false; couchdb_association_requests=false; req_time=10; rdbms_time=2; rdbms_count=4; authz_time=4; authz_count=1; user=admin; req_api_version=1;

==> /var/log/opscode/oc_bifrost/requests.log.1 <==
2017-04-05T18:35:12Z [email protected] method=GET; path=/objects/cbcfda915a4f56e22b9141099391cd2c/acl/read/actors/faeeb8cdf9da7fa8dee11a80f32fcf61; status=200; requestor_id=faeeb8cdf9da7fa8dee11a80f32fcf61; req_time=1; rdbms.bifrost_db.has_permission_time=0; rdbms.bifrost_db.has_permission_count=1; rdbms.bifrost_db.exists_time=0; rdbms.bifrost_db.exists_count=1;

stevendanna

Nice work! Overall this looks good to me. I've left some inline comments for your consideration.

The travis tests are likely failing because this same functionality has not yet been added to chef-zero. To ensure those two code bases stay in sync, we require that functionality be added in both places OR that a specific exception is made in the test config for chef-zero. In this case, I would recommend that we implement the chef-zero functionality.

I'd eventually like to see us support HEAD against all named resources. We should make sure we get something in a backlog to make the rest of the endpoints consistent.

Should we consider doing an RFC for this? My assumption is that the RFC process would land on requiring this for all named endpoints; however, there is nothing that says we have to ship the contents of an RFC "all at once" provided we do it in a sane way.

stevendanna · 2017-04-06T11:41:19Z

src/oc_erchef/apps/oc_chef_wm/src/chef_wm_named_node.erl

@@ -87,8 +88,13 @@ auth_info(Req, #base_state{chef_db_context = DbContext,
    end.

 to_json(Req, #base_state{resource_state = NodeState} = State) ->
-    #node_state{chef_node = Node} = NodeState,
-    {chef_db_compression:decompress(Node#chef_node.serialized_object), Req, State}.
+    case wrq:method(Req) of


I don't think we need to do this now since simply having the HEAD request will improve performance for the intended use case; however, if we extend this to more endpoints, we might consider that this code currently still pulls the entire node object from the database when all we need for the HEAD request (as implemented) is the authz_id.

We could create an alternate fetch method that only pulls the authz_id to avoid transferring the entire object from the database over to erchef for no reason. Alternatively, if we are transferring the data, we could potentially add some useful information to this HEAD response, such as a checksum of the object or something similar.

jeremymv2 · 2017-04-06T14:54:48Z

Thanks @stevendanna for the review.

I have an implementation ready for PR to add HEAD support to chef-zero so I can get that in as a place-holder. I'll link here when it's in.

I'm in agreement that proposing an RFC for transparency and feedback would be an ideal way to move forward. Assuming there are no objections, I'll go ahead and submit that proposal.

jeremymv2 · 2017-04-06T20:37:17Z

Chef-RFC: chef-boneyard/chef-rfc#263

stevendanna · 2017-05-01T10:31:23Z

@jeremymv2 Thanks for getting that RFC through. Where does this stand, I think the remaining TODO is the chef-zero implementation, do you need any assistance there?

jeremymv2 · 2017-05-01T14:30:16Z

Thanks @stevendanna for reaching out! I had begun the work on chef-zero then put it on ice until the RFC went through. I'll thaw it off an get it wrapped up this week.

jeremymv2 · 2017-05-18T19:16:24Z

Back to it.. here's the work in progress for the chef-zero implementation.
chef/chef-zero#264

stevendanna · 2017-07-19T23:48:31Z

@jeremymv2 I think we are good to go now, mind rebasing this?

jeremymv2 · 2018-03-29T16:21:04Z

@markan what do you think about us getting this in for the next release?

jeremymv2 · 2018-06-28T18:19:47Z

@btm @nsdavidson I've rebased and we have a happy Travis

nsdavidson · 2018-06-28T19:37:25Z

@stevendanna @markan Can y'all take another peek at this?

markan

LGTM

markan · 2018-07-02T20:03:02Z

Hold on merging for a bit; I'm trying to stabilize the build right now.

Signed-off-by: Jeremy J. Miller <[email protected]>

stevendanna reviewed Apr 6, 2017

View reviewed changes

jeremymv2 mentioned this pull request Apr 6, 2017

server http head chef-boneyard/chef-rfc#263

Closed

stevendanna assigned jeremymv2 May 18, 2017

stevendanna self-assigned this Nov 27, 2017

jeremymv2 force-pushed the jeremymv2/node_http_head branch from 6d1a870 to 445a545 Compare March 29, 2018 16:00

jeremymv2 requested a review from a team March 29, 2018 16:00

jeremymv2 force-pushed the jeremymv2/node_http_head branch from 445a545 to fe6596e Compare April 9, 2018 23:22

jeremymv2 closed this May 30, 2018

jeremymv2 deleted the jeremymv2/node_http_head branch May 30, 2018 02:06

jeremymv2 restored the jeremymv2/node_http_head branch June 26, 2018 16:15

nsdavidson reopened this Jun 28, 2018

jeremymv2 force-pushed the jeremymv2/node_http_head branch from fe6596e to 45a3b57 Compare June 28, 2018 18:06

nsdavidson approved these changes Jun 28, 2018

View reviewed changes

jeremymv2 force-pushed the jeremymv2/node_http_head branch from 45a3b57 to 6e0a9fe Compare June 29, 2018 22:51

markan approved these changes Jul 2, 2018

View reviewed changes

jeremymv2 force-pushed the jeremymv2/node_http_head branch from 6e0a9fe to 88b88bd Compare July 9, 2018 12:43

http head for /nodes/xxx endpoint

6174f5f

Signed-off-by: Jeremy J. Miller <[email protected]>

jeremymv2 force-pushed the jeremymv2/node_http_head branch from 88b88bd to 6174f5f Compare July 11, 2018 23:15

markan merged commit 651ef2a into chef:master Jul 12, 2018

jeremymv2 deleted the jeremymv2/node_http_head branch July 12, 2018 15:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support HEAD http request on named node endpoint #1218

support HEAD http request on named node endpoint #1218

jeremymv2 commented Apr 5, 2017

stevendanna left a comment

stevendanna Apr 6, 2017

jeremymv2 commented Apr 6, 2017

jeremymv2 commented Apr 6, 2017

stevendanna commented May 1, 2017

jeremymv2 commented May 1, 2017

jeremymv2 commented May 18, 2017

stevendanna commented Jul 19, 2017

jeremymv2 commented Mar 29, 2018

jeremymv2 commented Jun 28, 2018

nsdavidson commented Jun 28, 2018

markan left a comment

markan commented Jul 2, 2018

support HEAD http request on named node endpoint #1218

support HEAD http request on named node endpoint #1218

Conversation

jeremymv2 commented Apr 5, 2017

stevendanna left a comment

Choose a reason for hiding this comment

stevendanna Apr 6, 2017

Choose a reason for hiding this comment

jeremymv2 commented Apr 6, 2017

jeremymv2 commented Apr 6, 2017

stevendanna commented May 1, 2017

jeremymv2 commented May 1, 2017

jeremymv2 commented May 18, 2017

stevendanna commented Jul 19, 2017

jeremymv2 commented Mar 29, 2018

jeremymv2 commented Jun 28, 2018

nsdavidson commented Jun 28, 2018

markan left a comment

Choose a reason for hiding this comment

markan commented Jul 2, 2018