)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ae956ff4ed15c8996fb74e3899e3fc1ac03f0d1b","unresolved":false,"context_lines":[{"line_number":7,"context_line":"[placement] Use own set_middleware_defaults"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The CORS middleware needs to have its defaults set to something"},{"line_number":10,"context_line":"reaonable. Placement had been using some code in nova to do so."},{"line_number":11,"context_line":"This change duplicates that code into the placement WSGI app"},{"line_number":12,"context_line":"which had been calling it."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5f7c97a3_1a3e7db5","line":10,"range":{"start_line":10,"start_character":0,"end_line":10,"end_character":9},"updated":"2018-08-01 12:46:37.000000000","message":"nit: reasonable","commit_id":"45dbccd1fae91a42a69292cb9e708ed14de0a90f"}],"nova/api/openstack/placement/wsgi.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"769566019c1b7d81344fe1604fb484c3d4f294b7","unresolved":false,"context_lines":[{"line_number":69,"context_line":"              default_config_files\u003ddefault_config_files)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"def _set_middleware_defaults():"},{"line_number":73,"context_line":"    \"\"\"Update default configuration options for oslo.middleware.\"\"\""},{"line_number":74,"context_line":"    cors.set_defaults("},{"line_number":75,"context_line":"        allow_headers\u003d[\u0027X-Auth-Token\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_c6285e86","line":72,"updated":"2018-07-30 19:47:45.000000000","message":"Check.\n\nhttps://github.com/openstack/nova/blob/d7a87fad357dc635212ce96609dbc2b5c8a67871/nova/common/config.py","commit_id":"45dbccd1fae91a42a69292cb9e708ed14de0a90f"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"84d4a1d800e197f07ffee3a9278fb81b562d5a7b","unresolved":false,"context_lines":[{"line_number":72,"context_line":"def _set_middleware_defaults():"},{"line_number":73,"context_line":"    \"\"\"Update default configuration options for oslo.middleware.\"\"\""},{"line_number":74,"context_line":"    cors.set_defaults("},{"line_number":75,"context_line":"        allow_headers\u003d[\u0027X-Auth-Token\u0027,"},{"line_number":76,"context_line":"                       \u0027X-Openstack-Request-Id\u0027,"},{"line_number":77,"context_line":"                       \u0027X-Identity-Status\u0027,"},{"line_number":78,"context_line":"                       \u0027X-Roles\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_461a0e18","line":75,"updated":"2018-07-30 20:01:37.000000000","message":"We probably want openstack-api-version here and in expose as well.\n\nPresumably the reason this hasn\u0027t come up before is that few people use CORS with the nova or placement apis.","commit_id":"45dbccd1fae91a42a69292cb9e708ed14de0a90f"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"6db141d2190fa4e7f2de2804a4a9eb5962031403","unresolved":false,"context_lines":[{"line_number":72,"context_line":"def _set_middleware_defaults():"},{"line_number":73,"context_line":"    \"\"\"Update default configuration options for oslo.middleware.\"\"\""},{"line_number":74,"context_line":"    cors.set_defaults("},{"line_number":75,"context_line":"        allow_headers\u003d[\u0027X-Auth-Token\u0027,"},{"line_number":76,"context_line":"                       \u0027X-Openstack-Request-Id\u0027,"},{"line_number":77,"context_line":"                       \u0027X-Identity-Status\u0027,"},{"line_number":78,"context_line":"                       \u0027X-Roles\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_f9540a58","line":75,"in_reply_to":"5f7c97a3_0224f05d","updated":"2018-07-31 14:50:37.000000000","message":"\u003e \u003e pre-flight OPTIONS checks\n \u003e \n \u003e How hard is that? (Forgive my ignorance; I had to go look up what\n \u003e CORS even is yesterday when I reviewed this, and I still don\u0027t\n \u003e really get it.)\n\nThe difficulty isn\u0027t so much in creating some tests but more in two things:\n\n* without a web browser in the mix there\u0027s no good way to _really_ know that the behavior is happening as desired. All you can really do is validate some headers when making some requests.\n\n \u003e \u003e but we don\u0027t seem to have a history of\n \u003e \u003e doing that kind of thing.\n \u003e \n \u003e Who are you, and what have you done with the Chris Dent who always\n \u003e argues the *other* side of the historical-momentum debate?\n\nI figured since it was the week after feature freeze I\u0027d put that guy away for a little while. Don\u0027t worry, you\u0027ll see him again.\n\n] \u003e Unless it\u0027s just way more difficult than it\u0027s worth, we should do\n \u003e it. For now, can we get a bug opened (two, I guess - one for nova\n \u003e and one for placement) and/or add a TODO in here?\n\nA bug is probably ideal, I\u0027ll make one.\n\nNote that we do have nova/tests/functional/api/openstack/placement/gabbits/cors.yaml and nova/tests/functional/api/openstack/placement/gabbits/non-cors.yaml\nbut they aren\u0027t testing this code, they are testing the middleware is present with it\u0027s own defaults (not the defaults set by this set_defaults).\n\nblargh!","commit_id":"45dbccd1fae91a42a69292cb9e708ed14de0a90f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"25cde1ae7c192fe043ce9e78ef2074c9d28a222a","unresolved":false,"context_lines":[{"line_number":72,"context_line":"def _set_middleware_defaults():"},{"line_number":73,"context_line":"    \"\"\"Update default configuration options for oslo.middleware.\"\"\""},{"line_number":74,"context_line":"    cors.set_defaults("},{"line_number":75,"context_line":"        allow_headers\u003d[\u0027X-Auth-Token\u0027,"},{"line_number":76,"context_line":"                       \u0027X-Openstack-Request-Id\u0027,"},{"line_number":77,"context_line":"                       \u0027X-Identity-Status\u0027,"},{"line_number":78,"context_line":"                       \u0027X-Roles\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_b171dead","line":75,"in_reply_to":"5f7c97a3_461a0e18","updated":"2018-07-30 20:51:35.000000000","message":"oo, yeah, so what do we need to add, test-wise, to actually exercise this patch?","commit_id":"45dbccd1fae91a42a69292cb9e708ed14de0a90f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"7805f98f60c6f8394b084d020f64bf2154909f94","unresolved":false,"context_lines":[{"line_number":72,"context_line":"def _set_middleware_defaults():"},{"line_number":73,"context_line":"    \"\"\"Update default configuration options for oslo.middleware.\"\"\""},{"line_number":74,"context_line":"    cors.set_defaults("},{"line_number":75,"context_line":"        allow_headers\u003d[\u0027X-Auth-Token\u0027,"},{"line_number":76,"context_line":"                       \u0027X-Openstack-Request-Id\u0027,"},{"line_number":77,"context_line":"                       \u0027X-Identity-Status\u0027,"},{"line_number":78,"context_line":"                       \u0027X-Roles\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_0224f05d","line":75,"in_reply_to":"5f7c97a3_8526d321","updated":"2018-07-31 13:47:31.000000000","message":"\u003e We could write tempest or tempest-plugin -based\n \u003e tests to do check for the presence of the headers by doing\n \u003e pre-flight OPTIONS checks\n\nHow hard is that? (Forgive my ignorance; I had to go look up what CORS even is yesterday when I reviewed this, and I still don\u0027t really get it.)\n\n \u003e but we don\u0027t seem to have a history of\n \u003e doing that kind of thing.\n\nWho are you, and what have you done with the Chris Dent who always argues the *other* side of the historical-momentum debate?\n\nUnless it\u0027s just way more difficult than it\u0027s worth, we should do it. For now, can we get a bug opened (two, I guess - one for nova and one for placement) and/or add a TODO in here?","commit_id":"45dbccd1fae91a42a69292cb9e708ed14de0a90f"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"5e68a1f0504abc3dce7e775335ac0d5f1cad8069","unresolved":false,"context_lines":[{"line_number":72,"context_line":"def _set_middleware_defaults():"},{"line_number":73,"context_line":"    \"\"\"Update default configuration options for oslo.middleware.\"\"\""},{"line_number":74,"context_line":"    cors.set_defaults("},{"line_number":75,"context_line":"        allow_headers\u003d[\u0027X-Auth-Token\u0027,"},{"line_number":76,"context_line":"                       \u0027X-Openstack-Request-Id\u0027,"},{"line_number":77,"context_line":"                       \u0027X-Identity-Status\u0027,"},{"line_number":78,"context_line":"                       \u0027X-Roles\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_8526d321","line":75,"in_reply_to":"5f7c97a3_b171dead","updated":"2018-07-31 08:20:32.000000000","message":"It\u0027s weird in two ways:\n\n* None of our functional tests exercise any of this file because it is not used for bringing up the placement wsgi app that is used by the wsgi-intercept in those tests.\n* The integration jobs (the ones that start up a devstack) do exercise this file, but as far as I\u0027m aware none of them are set up to exercise CORS. We could write tempest or tempest-plugin -based tests to do check for the presence of the headers by doing pre-flight OPTIONS checks but we don\u0027t seem to have a history of doing that kind of thing.\n\nWe should probably set a flag to \"do this better\" but it\u0027s undertaking to do right, and I agree with your assessment that this is okay as-is because it is moving some existing code.","commit_id":"45dbccd1fae91a42a69292cb9e708ed14de0a90f"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"e10eccc15317ced1276db896cfb4d988ee58d572","unresolved":false,"context_lines":[{"line_number":72,"context_line":"def _set_middleware_defaults():"},{"line_number":73,"context_line":"    \"\"\"Update default configuration options for oslo.middleware.\"\"\""},{"line_number":74,"context_line":"    cors.set_defaults("},{"line_number":75,"context_line":"        allow_headers\u003d[\u0027X-Auth-Token\u0027,"},{"line_number":76,"context_line":"                       \u0027X-Openstack-Request-Id\u0027,"},{"line_number":77,"context_line":"                       \u0027X-Identity-Status\u0027,"},{"line_number":78,"context_line":"                       \u0027X-Roles\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_6a2ed642","line":75,"in_reply_to":"5f7c97a3_f9540a58","updated":"2018-07-31 15:57:25.000000000","message":"https://bugs.launchpad.net/nova/+bug/1784664","commit_id":"45dbccd1fae91a42a69292cb9e708ed14de0a90f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"769566019c1b7d81344fe1604fb484c3d4f294b7","unresolved":false,"context_lines":[{"line_number":87,"context_line":"                       \u0027PUT\u0027,"},{"line_number":88,"context_line":"                       \u0027POST\u0027,"},{"line_number":89,"context_line":"                       \u0027DELETE\u0027,"},{"line_number":90,"context_line":"                       \u0027PATCH\u0027]"},{"line_number":91,"context_line":"    )"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_a6fa020e","line":90,"range":{"start_line":90,"start_character":24,"end_line":90,"end_character":29},"updated":"2018-07-30 19:47:45.000000000","message":"Is this needed?","commit_id":"45dbccd1fae91a42a69292cb9e708ed14de0a90f"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"84d4a1d800e197f07ffee3a9278fb81b562d5a7b","unresolved":false,"context_lines":[{"line_number":87,"context_line":"                       \u0027PUT\u0027,"},{"line_number":88,"context_line":"                       \u0027POST\u0027,"},{"line_number":89,"context_line":"                       \u0027DELETE\u0027,"},{"line_number":90,"context_line":"                       \u0027PATCH\u0027]"},{"line_number":91,"context_line":"    )"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_860b46d2","line":90,"range":{"start_line":90,"start_character":24,"end_line":90,"end_character":29},"in_reply_to":"5f7c97a3_a6fa020e","updated":"2018-07-30 20:01:37.000000000","message":"It\u0027s a totally legit method to exist and for CORS to allow. I didn\u0027t want to create a dependency between the methods listed here and the methods we actually use. If we\u0027re allow PUT, POST, DELETE over CORS then PATCH is no additional risk.","commit_id":"45dbccd1fae91a42a69292cb9e708ed14de0a90f"}]}