HEAD and GET inconsistencies in Keystone

Bug #1334368 reported by Morgan Fainberg
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Fix Released
Medium
Morgan Fainberg
Icehouse
Fix Released
Medium
Morgan Fainberg
openstack-api-site
Fix Released
Medium
Diane Fleming
tempest
Fix Released
Undecided
Morgan Fainberg

Bug Description

While trying to convert Keystone to gate/check under mod_wsgi, it was noticed that occasionally a few HEAD calls were returning HTTP 200 where under eventlet they consistently return HTTP 204.

This is an inconsistency within Keystone. Based upon the RFC, HEAD should be identitcal to GET except that there is no body returned. Apache + MOD_WSGI in some cases converts a HEAD request to a GET request to the back-end wsgi application to avoid issues where the headers cannot be built to be sent as part of the response (this can occur when no content is returned from the wsgi app).

This situation shows that Keystone should likely never build specific HEAD request methods and have HEAD simply call to the controller GET handler, the wsgi-layer should then simply remove the response body.

This will help to simplify Keystone's code as well as mkae the API responses more consistent.

Example Error in Gate:

2014-06-25 05:20:37.820 | tempest.api.identity.admin.v3.test_trusts.TrustsV3TestJSON.test_trust_expire[gate,smoke]
2014-06-25 05:20:37.820 | ----------------------------------------------------------------------------------------
2014-06-25 05:20:37.820 |
2014-06-25 05:20:37.820 | Captured traceback:
2014-06-25 05:20:37.820 | ~~~~~~~~~~~~~~~~~~~
2014-06-25 05:20:37.820 | Traceback (most recent call last):
2014-06-25 05:20:37.820 | File "tempest/api/identity/admin/v3/test_trusts.py", line 241, in test_trust_expire
2014-06-25 05:20:37.820 | self.check_trust_roles()
2014-06-25 05:20:37.820 | File "tempest/api/identity/admin/v3/test_trusts.py", line 173, in check_trust_roles
2014-06-25 05:20:37.821 | self.assertEqual('204', resp['status'])
2014-06-25 05:20:37.821 | File "/usr/local/lib/python2.7/dist-packages/testtools/testcase.py", line 321, in assertEqual
2014-06-25 05:20:37.821 | self.assertThat(observed, matcher, message)
2014-06-25 05:20:37.821 | File "/usr/local/lib/python2.7/dist-packages/testtools/testcase.py", line 406, in assertThat
2014-06-25 05:20:37.821 | raise mismatch_error
2014-06-25 05:20:37.821 | MismatchError: '204' != '200'

This is likely going to require changes to Keystone, Keystoneclient, Tempest, and possibly services that consume data from keystone.

Changed in keystone:
importance: Undecided → Medium
status: New → Triaged
assignee: nobody → Morgan Fainberg (mdrnstm)
Revision history for this message
Haneef Ali (haneef) wrote :

Morgon,
I verified this and Apache converts HEAD to GET.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (master)

Fix proposed to branch: master
Review: https://review.openstack.org/104026

Changed in keystone:
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (stable/icehouse)

Fix proposed to branch: stable/icehouse
Review: https://review.openstack.org/104606

Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

Added to Tempest as it requires tempest changes to complete.

Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

Proposed fix for branch: stable/icehouse (keystone)

Review: https://review.openstack.org/#/c/104606/

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to tempest (master)

Fix proposed to branch: master
Review: https://review.openstack.org/104673

Changed in tempest:
assignee: nobody → Morgan Fainberg (mdrnstm)
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to tempest (master)

Reviewed: https://review.openstack.org/104610
Committed: https://git.openstack.org/cgit/openstack/tempest/commit/?id=59a9d7a8d4f9620cdcf783b9ef94a99acbe20873
Submitter: Jenkins
Branch: master

commit 59a9d7a8d4f9620cdcf783b9ef94a99acbe20873
Author: Morgan Fainberg <email address hidden>
Date: Thu Jul 3 09:23:18 2014 -0700

    Add a skip for bug #1334368

    Trust checking for 204 response is incorrect to support httpd based
    deployment because mod_wsgi could translate a HEAD call to GET.
    According to HTTP spec HEAD needs to respond with the same
    response as GET.

    This disables the trust check for HTTP 204 for
    changeId: I13ce159cbe9739d4bf5d321fc4bd069245f32734

    Once the changes to Icehouse and Juno (master) are accepted, this
    can be modified to look for HTTP 200 and re-enabled.

    Change-Id: I5e7d7dab2fc1432888bf8c691cae9f2109ac2fec
    Partial-Bug: #1334368

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (master)

Reviewed: https://review.openstack.org/104026
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=82101a36d348ad0c3de10f455878828f4263ffe2
Submitter: Jenkins
Branch: master

commit 82101a36d348ad0c3de10f455878828f4263ffe2
Author: Morgan Fainberg <email address hidden>
Date: Tue Jul 1 16:55:11 2014 -0700

    HEAD responses should return same status as GET

    According to the HTTP spec, a HEAD request should return the same
    status and headers as the GET request (including content-type and
    content-length). The HEAD request simply strips out the body and
    returns no body. Any case where HEAD routing returned a different
    status code than GET, now returns the same status and headers.

    Any case where HEAD was supported where GET was not supported now
    supports both GET and HEAD.

    The wsgi.render_response code now handles HEAD appropriately and
    will maintain headers while enforcing no body data is returned.

    The bulk of this change is to support the same behavior between
    deploying Keystone under eventlet and under HTTPD + mod_wsgi. In
    the case of deploying under HTTPD + mod_wsgi, there are cases
    where mod_wsgi will turn a HEAD request into a GET request to
    ensure that the proper response is rendered. With these changes
    all HEAD responses will respond in the same manner under either
    eventlet or mod_wsgi.

    Change-Id: I13ce159cbe9739d4bf5d321fc4bd069245f32734
    Closes-Bug: #1334368

Changed in keystone:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (stable/icehouse)

Reviewed: https://review.openstack.org/104606
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=6f8c444d33ad26a23a546309cb0a281255478ce6
Submitter: Jenkins
Branch: stable/icehouse

commit 6f8c444d33ad26a23a546309cb0a281255478ce6
Author: Morgan Fainberg <email address hidden>
Date: Tue Jul 1 16:55:11 2014 -0700

    HEAD responses should return same status as GET

    According to the HTTP spec, a HEAD request should return the same
    status and headers as the GET request (including content-type and
    content-length). The HEAD request simply strips out the body and
    returns no body. Any case where HEAD routing returned a different
    status code than GET, now returns the same status and headers.

    Any case where HEAD was supported where GET was not supported now
    supports both GET and HEAD.

    The wsgi.render_response code now handles HEAD appropriately and
    will maintain headers while enforcing no body data is returned.

    The bulk of this change is to support the same behavior between
    deploying Keystone under eventlet and under HTTPD + mod_wsgi. In
    the case of deploying under HTTPD + mod_wsgi, there are cases
    where mod_wsgi will turn a HEAD request into a GET request to
    ensure that the proper response is rendered. With these changes
    all HEAD responses will respond in the same manner under either
    eventlet or mod_wsgi.

    Change-Id: I13ce159cbe9739d4bf5d321fc4bd069245f32734
    Closes-Bug: #1334368
    (cherry picked from commit 82101a36d348ad0c3de10f455878828f4263ffe2)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to tempest (master)

Reviewed: https://review.openstack.org/104673
Committed: https://git.openstack.org/cgit/openstack/tempest/commit/?id=883311d64fd749315ee3639bff6c730e86026ac5
Submitter: Jenkins
Branch: master

commit 883311d64fd749315ee3639bff6c730e86026ac5
Author: Morgan Fainberg <email address hidden>
Date: Thu Jul 3 13:13:10 2014 -0700

    Re-enable 'check_trust_roles'

    Re-enable the 'check_trust_roles' method. This can be
    merged after the patches for change id
    I13ce159cbe9739d4bf5d321fc4bd069245f32734 are merged
    (master and stable/icehouse for Keystone).

    This changeset updates the new location for the check_trust_role
    HTTP status validation (in services/identity/v3/json/identity_client.py).
    Previously this was located in identity/admin/v3/test_trusts.py.

    Commented code indicating the above location change occurred has been
    removed.

    Change-Id: If1b7f18d7a357f4b3a4b478e300a17f2cc4a6159
    Closes-Bug: #1334368

Changed in tempest:
status: In Progress → Fix Released
Changed in keystone:
milestone: none → juno-2
status: Fix Committed → Fix Released
Chuck Short (zulcss)
tags: added: in-stable-icehouse
Revision history for this message
Gabriel Assis Bezerra (gabriel-bezerra) wrote :
Dolph Mathews (dolph)
tags: added: identity-api
Anne Gentle (annegentle)
Changed in openstack-api-site:
status: New → Confirmed
importance: Undecided → Medium
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to identity-api (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/124243

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to keystone (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/124244

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on keystone (master)

Change abandoned by Nathan Kinder (<email address hidden>) on branch: master
Review: https://review.openstack.org/124244
Reason: As Steve pointed out, this change is uneccesarry as head() in test_v3 is already checking for a response status of 204.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to identity-api (master)

Reviewed: https://review.openstack.org/124243
Committed: https://git.openstack.org/cgit/openstack/identity-api/commit/?id=363af890511e8cb047ea07c4e4a381680366941f
Submitter: Jenkins
Branch: master

commit 363af890511e8cb047ea07c4e4a381680366941f
Author: Nathan Kinder <email address hidden>
Date: Thu Sep 25 18:34:54 2014 -0700

    Correct response status for HEAD requests

    The response status for some HEAD calls was changed to follow the
    HTTP specification a while back. Specifically, any call that supports
    both GET and HEAD methods should return the same response code. The
    API docs still list all successfull HEAD requests with a response code
    of 204, which is not correct in all cases.

    This patch adjusts the expected response status for HEAD calls to
    match the actual implementation.

    Change-Id: I6f518ebbe00a0b2860ca5db0d10b93f313abd488
    Related-bug: #1334368

Thierry Carrez (ttx)
Changed in keystone:
milestone: juno-2 → 2014.2
Revision history for this message
Diane Fleming (diane-fleming) wrote :
Changed in openstack-api-site:
assignee: nobody → Diane Fleming (diane-fleming)
milestone: none → kilo
Revision history for this message
Diane Fleming (diane-fleming) wrote :
Changed in openstack-api-site:
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to api-site (master)

Reviewed: https://review.openstack.org/185469
Committed: https://git.openstack.org/cgit/openstack/api-site/commit/?id=b28a90ce696a1826daf243fc3f64c0f1292bddea
Submitter: Jenkins
Branch: master

commit b28a90ce696a1826daf243fc3f64c0f1292bddea
Author: Diane Fleming <email address hidden>
Date: Mon May 25 18:11:27 2015 -0500

    Update HEAD methods to return 200 return code

    Change-Id: Ia102d5d3e8d86ae1429e3d57bc6c98ad3e2c9072
    Closes-Bug: #1334368

Changed in openstack-api-site:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to tempest (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/479286

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.