)]}'
{"ceilometer/api/controllers/v2/alarms.py":[{"author":{"_account_id":1669,"name":"Julien Danjou","display_name":"jd","email":"julien@danjou.info","username":"jdanjou"},"change_message_id":"c1eadeb7891f85b9b22ae4b9f01260939a036e79","unresolved":false,"context_lines":[{"line_number":401,"context_line":"                        url[1] \u003d netloc"},{"line_number":402,"context_line":"                        actions[index] \u003d urlparse.urlunsplit(url)"},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"    def create_trust_id(self):"},{"line_number":405,"context_line":"        admin_client \u003d keystone_client.get_v3_client()"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"        trustee_user_id \u003d admin_client.auth_ref.user_id"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_bc9c4e5e","line":404,"updated":"2015-05-26 06:18:02.000000000","message":"Looks like a @staticmethod","commit_id":"a25ef259999d8710c1163fcc6d8d4cc3d9dcd254"},{"author":{"_account_id":7385,"name":"Thomas Herve","email":"therve@redhat.com","username":"therve"},"change_message_id":"479ea2de2d3c8dccf07cb5c8c93c3ef4643b5151","unresolved":false,"context_lines":[{"line_number":401,"context_line":"                        url[1] \u003d netloc"},{"line_number":402,"context_line":"                        actions[index] \u003d urlparse.urlunsplit(url)"},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"    def create_trust_id(self):"},{"line_number":405,"context_line":"        admin_client \u003d keystone_client.get_v3_client()"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"        trustee_user_id \u003d admin_client.auth_ref.user_id"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_bc70cedd","line":404,"in_reply_to":"7a016987_bc9c4e5e","updated":"2015-05-26 06:38:34.000000000","message":"Good idea. I actually made a function and moved it out of the way, let me know if it works for you.","commit_id":"a25ef259999d8710c1163fcc6d8d4cc3d9dcd254"},{"author":{"_account_id":12606,"name":"Miguel Grinberg","email":"miguel.grinberg@gmail.com","username":"miguelgrinberg"},"change_message_id":"66b40f0743e9338a0106da57943ce22e89030c20","unresolved":false,"context_lines":[{"line_number":383,"context_line":"    def update_actions(self):"},{"line_number":384,"context_line":"        trustor_user_id \u003d pecan.request.headers.get(\u0027X-User-Id\u0027)"},{"line_number":385,"context_line":"        trustor_project_id \u003d pecan.request.headers.get(\u0027X-Project-Id\u0027)"},{"line_number":386,"context_line":"        roles \u003d pecan.request.headers.get(\u0027X-Roles\u0027, \u0027\u0027)"},{"line_number":387,"context_line":"        if roles:"},{"line_number":388,"context_line":"            roles \u003d roles.split(\u0027,\u0027)"},{"line_number":389,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a016987_05c9b3ac","line":386,"updated":"2015-05-26 14:49:53.000000000","message":"We have a configurable role list for trusts on the heat side, but it seems that list will not make it here. Not sure this is really a problem, but it seems there is no way to restrict the scope of the trust, since we never send this header.","commit_id":"1b4e96c5eb083248618ea272dc11a3cb17e1b345"},{"author":{"_account_id":7385,"name":"Thomas Herve","email":"therve@redhat.com","username":"therve"},"change_message_id":"5e2e514752c56aba17f848c92f74bd8d99887a5e","unresolved":false,"context_lines":[{"line_number":383,"context_line":"    def update_actions(self):"},{"line_number":384,"context_line":"        trustor_user_id \u003d pecan.request.headers.get(\u0027X-User-Id\u0027)"},{"line_number":385,"context_line":"        trustor_project_id \u003d pecan.request.headers.get(\u0027X-Project-Id\u0027)"},{"line_number":386,"context_line":"        roles \u003d pecan.request.headers.get(\u0027X-Roles\u0027, \u0027\u0027)"},{"line_number":387,"context_line":"        if roles:"},{"line_number":388,"context_line":"            roles \u003d roles.split(\u0027,\u0027)"},{"line_number":389,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a016987_c01f0908","line":386,"in_reply_to":"7a016987_05c9b3ac","updated":"2015-05-26 15:01:44.000000000","message":"We could add such a configurable value in ceilometer, presumably.","commit_id":"1b4e96c5eb083248618ea272dc11a3cb17e1b345"},{"author":{"_account_id":12606,"name":"Miguel Grinberg","email":"miguel.grinberg@gmail.com","username":"miguelgrinberg"},"change_message_id":"8cb1c703c5d09ce39dad800064bc61dfc9b482be","unresolved":false,"context_lines":[{"line_number":383,"context_line":"    def update_actions(self):"},{"line_number":384,"context_line":"        trustor_user_id \u003d pecan.request.headers.get(\u0027X-User-Id\u0027)"},{"line_number":385,"context_line":"        trustor_project_id \u003d pecan.request.headers.get(\u0027X-Project-Id\u0027)"},{"line_number":386,"context_line":"        roles \u003d pecan.request.headers.get(\u0027X-Roles\u0027, \u0027\u0027)"},{"line_number":387,"context_line":"        if roles:"},{"line_number":388,"context_line":"            roles \u003d roles.split(\u0027,\u0027)"},{"line_number":389,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a016987_7e51150e","line":386,"in_reply_to":"7a016987_c01f0908","updated":"2015-05-26 17:19:46.000000000","message":"What do you think about the idea of heat passing its own role list in the X-Roles header when it creates the alarm?","commit_id":"1b4e96c5eb083248618ea272dc11a3cb17e1b345"},{"author":{"_account_id":12606,"name":"Miguel Grinberg","email":"miguel.grinberg@gmail.com","username":"miguelgrinberg"},"change_message_id":"66b40f0743e9338a0106da57943ce22e89030c20","unresolved":false,"context_lines":[{"line_number":398,"context_line":"                        # We have a trust action without a trust ID, create it"},{"line_number":399,"context_line":"                        trust_id \u003d keystone_client.create_trust_id("},{"line_number":400,"context_line":"                            trustor_user_id, trustor_project_id, roles,"},{"line_number":401,"context_line":"                            auth_plugin)"},{"line_number":402,"context_line":"                        netloc \u003d \u0027%s@%s\u0027 % (trust_id, url.netloc)"},{"line_number":403,"context_line":"                        url \u003d list(url)"},{"line_number":404,"context_line":"                        url[1] \u003d netloc"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a016987_056d33cc","line":401,"updated":"2015-05-26 14:49:53.000000000","message":"Wouldn\u0027t it be more efficient to use a single trust for all the actions?","commit_id":"1b4e96c5eb083248618ea272dc11a3cb17e1b345"},{"author":{"_account_id":7385,"name":"Thomas Herve","email":"therve@redhat.com","username":"therve"},"change_message_id":"5e2e514752c56aba17f848c92f74bd8d99887a5e","unresolved":false,"context_lines":[{"line_number":398,"context_line":"                        # We have a trust action without a trust ID, create it"},{"line_number":399,"context_line":"                        trust_id \u003d keystone_client.create_trust_id("},{"line_number":400,"context_line":"                            trustor_user_id, trustor_project_id, roles,"},{"line_number":401,"context_line":"                            auth_plugin)"},{"line_number":402,"context_line":"                        netloc \u003d \u0027%s@%s\u0027 % (trust_id, url.netloc)"},{"line_number":403,"context_line":"                        url \u003d list(url)"},{"line_number":404,"context_line":"                        url[1] \u003d netloc"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a016987_00c9a1ac","line":401,"in_reply_to":"7a016987_056d33cc","updated":"2015-05-26 15:01:44.000000000","message":"It\u0027d be more efficient, but having different trusts allows for more flexibility. Not sure it\u0027s worth it.","commit_id":"1b4e96c5eb083248618ea272dc11a3cb17e1b345"},{"author":{"_account_id":12606,"name":"Miguel Grinberg","email":"miguel.grinberg@gmail.com","username":"miguelgrinberg"},"change_message_id":"66b40f0743e9338a0106da57943ce22e89030c20","unresolved":false,"context_lines":[{"line_number":402,"context_line":"                        netloc \u003d \u0027%s@%s\u0027 % (trust_id, url.netloc)"},{"line_number":403,"context_line":"                        url \u003d list(url)"},{"line_number":404,"context_line":"                        url[1] \u003d netloc"},{"line_number":405,"context_line":"                        actions[index] \u003d urlparse.urlunsplit(url)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"Alarm.add_attributes(**{\"%s_rule\" % ext.name: ext.plugin"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a016987_c5cddb87","line":405,"updated":"2015-05-26 14:49:53.000000000","message":"I\u0027m wondering if it isn\u0027t a security issue to insert the trust id in the action URLs, which in turn will be written to the database. If the ceilometer service user credentials are known then having the trust id would allow full impersonation of the originating user and project. Or am I missing something?","commit_id":"1b4e96c5eb083248618ea272dc11a3cb17e1b345"},{"author":{"_account_id":12606,"name":"Miguel Grinberg","email":"miguel.grinberg@gmail.com","username":"miguelgrinberg"},"change_message_id":"8cb1c703c5d09ce39dad800064bc61dfc9b482be","unresolved":false,"context_lines":[{"line_number":402,"context_line":"                        netloc \u003d \u0027%s@%s\u0027 % (trust_id, url.netloc)"},{"line_number":403,"context_line":"                        url \u003d list(url)"},{"line_number":404,"context_line":"                        url[1] \u003d netloc"},{"line_number":405,"context_line":"                        actions[index] \u003d urlparse.urlunsplit(url)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"Alarm.add_attributes(**{\"%s_rule\" % ext.name: ext.plugin"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a016987_7ed7d523","line":405,"in_reply_to":"7a016987_c04a6927","updated":"2015-05-26 17:19:46.000000000","message":"I\u0027m not sure there is anything we can do about it, but on the heat side we encrypt the trust ids. Something to think about, I agree that in theory it is not a security issue to expose the trust alone.","commit_id":"1b4e96c5eb083248618ea272dc11a3cb17e1b345"},{"author":{"_account_id":7385,"name":"Thomas Herve","email":"therve@redhat.com","username":"therve"},"change_message_id":"5e2e514752c56aba17f848c92f74bd8d99887a5e","unresolved":false,"context_lines":[{"line_number":402,"context_line":"                        netloc \u003d \u0027%s@%s\u0027 % (trust_id, url.netloc)"},{"line_number":403,"context_line":"                        url \u003d list(url)"},{"line_number":404,"context_line":"                        url[1] \u003d netloc"},{"line_number":405,"context_line":"                        actions[index] \u003d urlparse.urlunsplit(url)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"Alarm.add_attributes(**{\"%s_rule\" % ext.name: ext.plugin"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a016987_c04a6927","line":405,"in_reply_to":"7a016987_c5cddb87","updated":"2015-05-26 15:01:44.000000000","message":"If you have ceilometer service user credentials, you have admin access, so the trust is not a security issue. If you have a specific concern, we can think about it.","commit_id":"1b4e96c5eb083248618ea272dc11a3cb17e1b345"},{"author":{"_account_id":12606,"name":"Miguel Grinberg","email":"miguel.grinberg@gmail.com","username":"miguelgrinberg"},"change_message_id":"941aefa44d6b24e16cadc3f73a80e02a4e7cef8b","unresolved":false,"context_lines":[{"line_number":385,"context_line":"    def _is_trust_url(url):"},{"line_number":386,"context_line":"        return url.scheme in (\u0027trust+http\u0027, \u0027trust+https\u0027)"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":"    def update_actions(self, old_action\u003dNone):"},{"line_number":389,"context_line":"        trustor_user_id \u003d pecan.request.headers.get(\u0027X-User-Id\u0027)"},{"line_number":390,"context_line":"        trustor_project_id \u003d pecan.request.headers.get(\u0027X-Project-Id\u0027)"},{"line_number":391,"context_line":"        roles \u003d pecan.request.headers.get(\u0027X-Roles\u0027, \u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a016987_18490bd1","line":388,"updated":"2015-05-26 23:20:48.000000000","message":"nit: old_action should be more appropriately called old_alarm, or dict_old_alarm, or something of that sort, I think.","commit_id":"2db3816fa72823ab78f44c6f014a21617e35cf4c"},{"author":{"_account_id":7385,"name":"Thomas Herve","email":"therve@redhat.com","username":"therve"},"change_message_id":"7a0ffd191164371afd55d01812dab40d2b86da5c","unresolved":false,"context_lines":[{"line_number":385,"context_line":"    def _is_trust_url(url):"},{"line_number":386,"context_line":"        return url.scheme in (\u0027trust+http\u0027, \u0027trust+https\u0027)"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":"    def update_actions(self, old_action\u003dNone):"},{"line_number":389,"context_line":"        trustor_user_id \u003d pecan.request.headers.get(\u0027X-User-Id\u0027)"},{"line_number":390,"context_line":"        trustor_project_id \u003d pecan.request.headers.get(\u0027X-Project-Id\u0027)"},{"line_number":391,"context_line":"        roles \u003d pecan.request.headers.get(\u0027X-Roles\u0027, \u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a016987_52ce3a84","line":388,"in_reply_to":"7a016987_18490bd1","updated":"2015-05-27 03:58:17.000000000","message":"Done","commit_id":"2db3816fa72823ab78f44c6f014a21617e35cf4c"},{"author":{"_account_id":12606,"name":"Miguel Grinberg","email":"miguel.grinberg@gmail.com","username":"miguelgrinberg"},"change_message_id":"941aefa44d6b24e16cadc3f73a80e02a4e7cef8b","unresolved":false,"context_lines":[{"line_number":410,"context_line":"                        actions[index] \u003d urlparse.urlunsplit(url)"},{"line_number":411,"context_line":"        if old_action:"},{"line_number":412,"context_line":"            for key in (\u0027ok_actions\u0027, \u0027alarm_actions\u0027,"},{"line_number":413,"context_line":"                        \u0027insufficient_data_actions\u0027):"},{"line_number":414,"context_line":"                for action in old_action[key]:"},{"line_number":415,"context_line":"                    url \u003d netutils.urlsplit(action)"},{"line_number":416,"context_line":"                    if (self._is_trust_url(url) and url.password and"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a016987_58542353","line":413,"updated":"2015-05-26 23:20:48.000000000","message":"another code cleanup nit: the code to delete the trusts is duplicated, here it uses a dict as source, and below in delete_actions() it uses an Alarm object. Probably a good idea to have a single method to delete that is invoked from both places, given that conversions between object and dict are accessible.","commit_id":"2db3816fa72823ab78f44c6f014a21617e35cf4c"},{"author":{"_account_id":7385,"name":"Thomas Herve","email":"therve@redhat.com","username":"therve"},"change_message_id":"7a0ffd191164371afd55d01812dab40d2b86da5c","unresolved":false,"context_lines":[{"line_number":410,"context_line":"                        actions[index] \u003d urlparse.urlunsplit(url)"},{"line_number":411,"context_line":"        if old_action:"},{"line_number":412,"context_line":"            for key in (\u0027ok_actions\u0027, \u0027alarm_actions\u0027,"},{"line_number":413,"context_line":"                        \u0027insufficient_data_actions\u0027):"},{"line_number":414,"context_line":"                for action in old_action[key]:"},{"line_number":415,"context_line":"                    url \u003d netutils.urlsplit(action)"},{"line_number":416,"context_line":"                    if (self._is_trust_url(url) and url.password and"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a016987_92aa7284","line":413,"in_reply_to":"7a016987_58542353","updated":"2015-05-27 03:58:17.000000000","message":"I changed it to an object, but I can\u0027t really change to use the same method, as you need to compare if the data changed in this case.","commit_id":"2db3816fa72823ab78f44c6f014a21617e35cf4c"}],"ceilometer/keystone_client.py":[{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"3d537eba7b6b0080a75aee3cd2be93b8e61c16c2","unresolved":false,"context_lines":[{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    session \u003d ks_session.Session.construct({"},{"line_number":72,"context_line":"        \u0027cacert\u0027: cfg.CONF.service_credentials.os_cacert,"},{"line_number":73,"context_line":"        \u0027insecure\u0027: cfg.CONF.service_credentials.insecure})"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    client \u003d ks_client_v3.Client(session\u003dsession, auth\u003dauth_plugin)"},{"line_number":76,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_fdbb4e0d","line":73,"updated":"2015-07-08 13:05:39.000000000","message":"Using cfg.CONF.service_credentials.XXX here looks suspicious to me,\n\nks_client_v3.Client(session\u003dsession, auth\u003dauth_plugin) will reuse the configuration options of the auth_plugin, [keystone_authtoken] section when the session will use configuration option from [service_credentials].\n\nI guess we should to do that instead:\n\n  session \u003d ks_session.Session.construct(auth\u003dauth_plugin)\n  client \u003d ks_client_v3.Client(session\u003dsession)\n\nI will check verify","commit_id":"cfd9b746e1037e04f07ae699b2d498e6a19f1499"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"dc5684d5eb779c2b90bd79f4c54a46d2c40b5ceb","unresolved":false,"context_lines":[{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    session \u003d ks_session.Session.construct({"},{"line_number":72,"context_line":"        \u0027cacert\u0027: cfg.CONF.service_credentials.os_cacert,"},{"line_number":73,"context_line":"        \u0027insecure\u0027: cfg.CONF.service_credentials.insecure})"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    client \u003d ks_client_v3.Client(session\u003dsession, auth\u003dauth_plugin)"},{"line_number":76,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_6c278f12","line":73,"in_reply_to":"ba3cc151_fdbb4e0d","updated":"2015-07-08 14:06:28.000000000","message":"I got it, [keystone_authtoken] options that are for \u0027python-requests\u0027 are not part of those present in the \u0027auth_plugin\u0027.","commit_id":"cfd9b746e1037e04f07ae699b2d498e6a19f1499"}]}