)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"0e54c86887a3208ee5c59247c402ec6be4deaa90","unresolved":false,"context_lines":[{"line_number":11,"context_line":"configuration option is set to True in Nova\u0027s glance configuration."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I0ae2dbf66241207a425bf7d0fc02a4d2e2dea409"},{"line_number":14,"context_line":"Depends-On: I8d7f43fb4c0573ac3681147eac213b369bbbcb3b"},{"line_number":15,"context_line":"Depends-On: I872b50932f7611584661efc604c8e5d4324fae9b"},{"line_number":16,"context_line":"Implements: blueprint nova-validate-certificates"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"9f436f4f_f9054712","line":14,"range":{"start_line":14,"start_character":12,"end_line":14,"end_character":53},"updated":"2017-08-11 21:12:23.000000000","message":"note that this will fail all of the tests until the next release of cursive occurs, and it will be important for the requirements file to be updated to require the new version","commit_id":"9bc90a806f8f10abe5625c2d9a71df667b2819ad"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3869cb1c630cc742f1fe906cf4ecbf7172a0e9ac","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Implement certificate_utils"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Implement the certificate_utils module. The module\u0027s verify_certificate"},{"line_number":10,"context_line":"method should only be called if the enable_certiciate_validation"},{"line_number":11,"context_line":"configuration option is set to True in Nova\u0027s glance configuration."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I0ae2dbf66241207a425bf7d0fc02a4d2e2dea409"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":24,"id":"7f96bb07_c2f3b009","line":10,"range":{"start_line":10,"start_character":43,"end_line":10,"end_character":53},"updated":"2018-01-15 20:13:55.000000000","message":"certificate","commit_id":"053b68784b82d4c787b5e45825c85a241634e316"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"92b17e9fea9e11ef1f23e896d01ae561e45fa91d","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Implement certificate_utils"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Implement the certificate_utils module. The module\u0027s verify_certificate"},{"line_number":10,"context_line":"method should only be called if the enable_certiciate_validation"},{"line_number":11,"context_line":"configuration option is set to True in Nova\u0027s glance configuration."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I0ae2dbf66241207a425bf7d0fc02a4d2e2dea409"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":24,"id":"7f96bb07_1e6490e3","line":10,"range":{"start_line":10,"start_character":43,"end_line":10,"end_character":53},"in_reply_to":"7f96bb07_c2f3b009","updated":"2018-01-16 14:51:25.000000000","message":"Done","commit_id":"053b68784b82d4c787b5e45825c85a241634e316"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"e1040e9541e9bf9a9ad1fbfe2ba9b7613b4a9e7d","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Implement certificate_utils"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Implement the certificate_utils module. The module\u0027s verify_certificate"},{"line_number":10,"context_line":"method should only be called if the enable_certificate_validation"},{"line_number":11,"context_line":"configuration option is set to True in Nova\u0027s glance configuration."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I0ae2dbf66241207a425bf7d0fc02a4d2e2dea409"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":43,"id":"bf659307_a0b5a5cf","line":10,"updated":"2018-04-06 15:00:07.000000000","message":"that\u0027s not the only time you\u0027re calling it, see nova/image/glance.py changes","commit_id":"6276d017d893521820be03d84fccffc05787e188"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"4d9932b1e89244e96ec4eb3febb07a38d0da3b2f","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Implement certificate_utils"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Implement the certificate_utils module. The module\u0027s verify_certificate"},{"line_number":10,"context_line":"method should only be called if the enable_certificate_validation"},{"line_number":11,"context_line":"configuration option is set to True in Nova\u0027s glance configuration."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I0ae2dbf66241207a425bf7d0fc02a4d2e2dea409"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":43,"id":"bf659307_c972337d","line":10,"in_reply_to":"bf659307_a0b5a5cf","updated":"2018-04-06 15:33:39.000000000","message":"Thanks, Matthew. Forgot to update the commit message.","commit_id":"6276d017d893521820be03d84fccffc05787e188"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a2d54c5c89004a1315309c900a4f6d67071edd7d","unresolved":false,"context_lines":[{"line_number":11,"context_line":"triggered by one of two ways:"},{"line_number":12,"context_line":"    1) The enable_certificate_validation configuration option is set to"},{"line_number":13,"context_line":"       True in Nova\u0027s glance configuration"},{"line_number":14,"context_line":"    2) A list of trusted certificate IDs is provided"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I0ae2dbf66241207a425bf7d0fc02a4d2e2dea409"},{"line_number":17,"context_line":"Depends-On: I872b50932f7611584661efc604c8e5d4324fae9b"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":56,"id":"bf659307_9d7a8a21","line":14,"updated":"2018-04-13 16:00:40.000000000","message":"Now that I\u0027m getting closer to approval on the stuff that\u0027s here, I\u0027m realizing you\u0027ve really got two patches here. The commit message describes one patch, which is all the glance download plumbing to honor the trusted ids, if passed, and the second is all the libvirt changes to use it (which this message completely ignores).\n\nI think those should be split into two pieces.","commit_id":"75e2fd5cb3570608b7bf731c6cb55a5dda8c4011"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9a4d1aa69b8a4b14fa6150bf9f53c97d8805c0f8","unresolved":false,"context_lines":[{"line_number":14,"context_line":"    2) A list of trusted certificate IDs is provided"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I0ae2dbf66241207a425bf7d0fc02a4d2e2dea409"},{"line_number":17,"context_line":"Depends-On: I872b50932f7611584661efc604c8e5d4324fae9b"},{"line_number":18,"context_line":"Implements: blueprint nova-validate-certificates"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":56,"id":"bf659307_7d6e962f","line":17,"updated":"2018-04-13 16:03:12.000000000","message":"This also should go away (and never should have been here).","commit_id":"75e2fd5cb3570608b7bf731c6cb55a5dda8c4011"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"278bbcc7ef2c25b855c7dab3a5762d2b40226240","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Implement certificate_utils"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Implement the certificate_utils module. The module\u0027s verify_certificate"},{"line_number":10,"context_line":"method can be applied to the creation or rebuild of an instance. It is"},{"line_number":11,"context_line":"triggered by one of two ways:"},{"line_number":12,"context_line":"    1) The enable_certificate_validation configuration option is set to"},{"line_number":13,"context_line":"       True in Nova\u0027s glance configuration"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":57,"id":"bf659307_55ccc287","line":10,"range":{"start_line":10,"start_character":29,"end_line":10,"end_character":63},"updated":"2018-04-13 20:59:18.000000000","message":"From what I can tell, cert validation will occur any time the image is fetched for the instance including: create, rebuild, rescue, migrate. That\u0027s explained in the spec already but just wanted to point it out here.","commit_id":"783a0e8201d5f05819bd8910fe66519d44b003cf"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bab5dd76f25174b70cd811446f3d246024ae3290","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Implement the certificate_utils module. The module\u0027s verify_certificate"},{"line_number":10,"context_line":"method can be applied to the creation or rebuild of an instance. It is"},{"line_number":11,"context_line":"triggered by one of two ways:"},{"line_number":12,"context_line":"    1) The enable_certificate_validation configuration option is set to"},{"line_number":13,"context_line":"       True in Nova\u0027s glance configuration"},{"line_number":14,"context_line":"    2) A list of trusted certificate IDs is provided"},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":76,"id":"5f7c97a3_339f08b5","line":12,"range":{"start_line":12,"start_character":11,"end_line":12,"end_character":40},"updated":"2018-06-12 13:28:28.000000000","message":"Should this be verify_glance_signatures?\n\nGiven: https://review.openstack.org/#/c/479949/76/nova/image/glance.py@338","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"ad3ec32143815af952ba2b4c38273c4ddceddcd2","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Implement the certificate_utils module. The module\u0027s verify_certificate"},{"line_number":10,"context_line":"method can be applied to the creation or rebuild of an instance. It is"},{"line_number":11,"context_line":"triggered by one of two ways:"},{"line_number":12,"context_line":"    1) The enable_certificate_validation configuration option is set to"},{"line_number":13,"context_line":"       True in Nova\u0027s glance configuration"},{"line_number":14,"context_line":"    2) A list of trusted certificate IDs is provided"},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":76,"id":"5f7c97a3_9c9869c0","line":12,"range":{"start_line":12,"start_character":11,"end_line":12,"end_character":40},"in_reply_to":"5f7c97a3_339f08b5","updated":"2018-06-12 16:11:10.000000000","message":"I\u0027ll note that it has to be set to true alongside the verify_glance_signatures option.","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"}],"nova/compute/manager.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a2d54c5c89004a1315309c900a4f6d67071edd7d","unresolved":false,"context_lines":[{"line_number":2251,"context_line":"                        exception.CertificateValidationFailed,"},{"line_number":2252,"context_line":"                        exception.UnexpectedDeletingTaskStateError)):"},{"line_number":2253,"context_line":"                    LOG.exception(\u0027Instance failed to spawn\u0027,"},{"line_number":2254,"context_line":"                                  instance\u003dinstance)"},{"line_number":2255,"context_line":"                # Make sure the async call finishes"},{"line_number":2256,"context_line":"                if network_info is not None:"},{"line_number":2257,"context_line":"                    network_info.wait(do_raise\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":56,"id":"bf659307_7de8d6e0","line":2254,"updated":"2018-04-13 16:00:40.000000000","message":"I actually think this should go away too. AFAICT, before my recent change we must have been logging this exception twice, here and in the place I removed above. I was going to complain about your adding this here, but I actually think we should just remove this whole block (unrelated to this patch).","commit_id":"75e2fd5cb3570608b7bf731c6cb55a5dda8c4011"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6e73166285929230b75147d1f79ee0280f1c09d3","unresolved":false,"context_lines":[{"line_number":2093,"context_line":"                exception.InvalidDiskInfo,"},{"line_number":2094,"context_line":"                exception.InvalidDiskFormat,"},{"line_number":2095,"context_line":"                cursive_exception.SignatureVerificationError,"},{"line_number":2096,"context_line":"                exception.CertificateValidationFailed,"},{"line_number":2097,"context_line":"                exception.VolumeEncryptionNotSupported,"},{"line_number":2098,"context_line":"                exception.InvalidInput) as e:"},{"line_number":2099,"context_line":"            self._notify_about_instance_usage(context, instance,"}],"source_content_type":"text/x-python","patch_set":57,"id":"bf659307_aeb39609","line":2096,"updated":"2018-04-13 20:45:47.000000000","message":"Note to self: this is for instance create, we\u0027ll abort the build if cert validation fails.","commit_id":"783a0e8201d5f05819bd8910fe66519d44b003cf"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6e73166285929230b75147d1f79ee0280f1c09d3","unresolved":false,"context_lines":[{"line_number":2248,"context_line":"            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":2249,"context_line":"                if not isinstance(exc, ("},{"line_number":2250,"context_line":"                        exception.InstanceNotFound,"},{"line_number":2251,"context_line":"                        exception.CertificateValidationFailed,"},{"line_number":2252,"context_line":"                        exception.UnexpectedDeletingTaskStateError)):"},{"line_number":2253,"context_line":"                    LOG.exception(\u0027Instance failed to spawn\u0027,"},{"line_number":2254,"context_line":"                                  instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":57,"id":"bf659307_7281dcfa","line":2251,"updated":"2018-04-13 20:45:47.000000000","message":"Note to self: this will just avoid logging the exception if the cert validation failed.","commit_id":"783a0e8201d5f05819bd8910fe66519d44b003cf"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ae515b1cb5deeba1446a8db5e2664d3a1bccf4fe","unresolved":false,"context_lines":[{"line_number":2248,"context_line":"            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":2249,"context_line":"                if not isinstance(exc, ("},{"line_number":2250,"context_line":"                        exception.InstanceNotFound,"},{"line_number":2251,"context_line":"                        exception.CertificateValidationFailed,"},{"line_number":2252,"context_line":"                        exception.UnexpectedDeletingTaskStateError)):"},{"line_number":2253,"context_line":"                    LOG.exception(\u0027Instance failed to spawn\u0027,"},{"line_number":2254,"context_line":"                                  instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":58,"id":"bf659307_1ba51b9f","line":2251,"updated":"2018-04-13 22:54:14.000000000","message":"TODO-in-the-future-not-part-of-this-patch-series: add exception.SignatureVerificationFailed here too because we\u0027re currently logging tracebacks for failed signature verifications. See comment for details:\n\nhttps://review.openstack.org/#/c/515210","commit_id":"4f5c3cb37ce3173869be3d326078272da400e7b3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4664dcb328a9b6deb324966b0bee1c7de8eaee1a","unresolved":false,"context_lines":[{"line_number":2082,"context_line":"                exception.InvalidDiskInfo,"},{"line_number":2083,"context_line":"                exception.InvalidDiskFormat,"},{"line_number":2084,"context_line":"                cursive_exception.SignatureVerificationError,"},{"line_number":2085,"context_line":"                exception.CertificateValidationFailed,"},{"line_number":2086,"context_line":"                exception.VolumeEncryptionNotSupported,"},{"line_number":2087,"context_line":"                exception.InvalidInput,"},{"line_number":2088,"context_line":"                # TODO(mriedem): We should be validating RequestedVRamTooHigh"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_332f08f7","line":2085,"updated":"2018-06-12 13:30:59.000000000","message":"This is likely better saved for the next patch in the series which plumbs the calls through from the driver.spawn(), but it\u0027s not a big deal. It\u0027s easy to forget to add this either way.","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"ad3ec32143815af952ba2b4c38273c4ddceddcd2","unresolved":false,"context_lines":[{"line_number":2082,"context_line":"                exception.InvalidDiskInfo,"},{"line_number":2083,"context_line":"                exception.InvalidDiskFormat,"},{"line_number":2084,"context_line":"                cursive_exception.SignatureVerificationError,"},{"line_number":2085,"context_line":"                exception.CertificateValidationFailed,"},{"line_number":2086,"context_line":"                exception.VolumeEncryptionNotSupported,"},{"line_number":2087,"context_line":"                exception.InvalidInput,"},{"line_number":2088,"context_line":"                # TODO(mriedem): We should be validating RequestedVRamTooHigh"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_5c8f31cc","line":2085,"in_reply_to":"5f7c97a3_332f08f7","updated":"2018-06-12 16:11:10.000000000","message":"I\u0027ll move it to the next patch for clarity.","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"79b64462431d65efdbb1d59e5d428c8efc7e806f","unresolved":false,"context_lines":[{"line_number":2082,"context_line":"                exception.InvalidDiskInfo,"},{"line_number":2083,"context_line":"                exception.InvalidDiskFormat,"},{"line_number":2084,"context_line":"                cursive_exception.SignatureVerificationError,"},{"line_number":2085,"context_line":"                exception.CertificateValidationFailed,"},{"line_number":2086,"context_line":"                exception.VolumeEncryptionNotSupported,"},{"line_number":2087,"context_line":"                exception.InvalidInput,"},{"line_number":2088,"context_line":"                # TODO(mriedem): We should be validating RequestedVRamTooHigh"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_4c93fa4f","line":2085,"in_reply_to":"5f7c97a3_5c8f31cc","updated":"2018-06-12 18:17:06.000000000","message":"Done: https://review.openstack.org/#/c/561262/26/nova/compute/manager.py@2098","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bab5dd76f25174b70cd811446f3d246024ae3290","unresolved":false,"context_lines":[{"line_number":2240,"context_line":"            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":2241,"context_line":"                if not isinstance(exc, ("},{"line_number":2242,"context_line":"                        exception.InstanceNotFound,"},{"line_number":2243,"context_line":"                        exception.CertificateValidationFailed,"},{"line_number":2244,"context_line":"                        exception.UnexpectedDeletingTaskStateError)):"},{"line_number":2245,"context_line":"                    LOG.exception(\u0027Instance failed to spawn\u0027,"},{"line_number":2246,"context_line":"                                  instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_332ce897","line":2243,"updated":"2018-06-12 13:28:28.000000000","message":"How would we get here? Image validation is done during driver.spawn right? When we\u0027re here, we\u0027re waiting for block device mapping (boot from volume / volume attach) and networking to complete, neither of which is going to be doing image validation.","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"ad3ec32143815af952ba2b4c38273c4ddceddcd2","unresolved":false,"context_lines":[{"line_number":2240,"context_line":"            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":2241,"context_line":"                if not isinstance(exc, ("},{"line_number":2242,"context_line":"                        exception.InstanceNotFound,"},{"line_number":2243,"context_line":"                        exception.CertificateValidationFailed,"},{"line_number":2244,"context_line":"                        exception.UnexpectedDeletingTaskStateError)):"},{"line_number":2245,"context_line":"                    LOG.exception(\u0027Instance failed to spawn\u0027,"},{"line_number":2246,"context_line":"                                  instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_dcb4a184","line":2243,"in_reply_to":"5f7c97a3_332ce897","updated":"2018-06-12 16:11:10.000000000","message":"I\u0027ll remove it.","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"}],"nova/exception.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"171045f30e4ab27b362727702aec3925c0562dbd","unresolved":false,"context_lines":[{"line_number":2296,"context_line":""},{"line_number":2297,"context_line":"class CertificateValidationFailed(NovaException):"},{"line_number":2298,"context_line":"    msg_fmt \u003d _(\"Image signature certificate validation failed for \""},{"line_number":2299,"context_line":"                \"certificate: %(cert_uuid)s. %(reason)s\")"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_b3c1d882","line":2299,"range":{"start_line":2299,"start_character":44,"end_line":2299,"end_character":55},"updated":"2018-06-12 13:30:52.000000000","message":"nit: sometimes, people don\u0027t like providing reasons directly because it could be a translation problem. But meh, I\u0027m fine with that.","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"ad3ec32143815af952ba2b4c38273c4ddceddcd2","unresolved":false,"context_lines":[{"line_number":2296,"context_line":""},{"line_number":2297,"context_line":"class CertificateValidationFailed(NovaException):"},{"line_number":2298,"context_line":"    msg_fmt \u003d _(\"Image signature certificate validation failed for \""},{"line_number":2299,"context_line":"                \"certificate: %(cert_uuid)s. %(reason)s\")"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_dc37c1fd","line":2299,"range":{"start_line":2299,"start_character":44,"end_line":2299,"end_character":55},"in_reply_to":"5f7c97a3_b3c1d882","updated":"2018-06-12 16:11:10.000000000","message":"I\u0027ll leave it as is.","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"}],"nova/image/api.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"525d231ba6f1bd4b88bdd625fc24a430464c5b80","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        return session.delete(context, image_id)"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def download(self, context, id_or_uri, data\u003dNone, dest_path\u003dNone,"},{"line_number":156,"context_line":"                 **kwargs):"},{"line_number":157,"context_line":"        \"\"\"Transfer image bits from Glance or a known source location to the"},{"line_number":158,"context_line":"        supplied destination filepath."},{"line_number":159,"context_line":""}],"source_content_type":"text/x-python","patch_set":37,"id":"df7087c5_2f2c1998","line":156,"range":{"start_line":156,"start_character":17,"end_line":156,"end_character":25},"updated":"2018-03-20 12:18:29.000000000","message":"Surely this needs to be trusted_certs\u003dNone here, so we match the implementation?","commit_id":"e7c874a33fa8a2069084a81d2ef2712005547989"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"6104ffb8c153bbeae4591f443c511489a5082b81","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        return session.delete(context, image_id)"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def download(self, context, id_or_uri, data\u003dNone, dest_path\u003dNone,"},{"line_number":156,"context_line":"                 **kwargs):"},{"line_number":157,"context_line":"        \"\"\"Transfer image bits from Glance or a known source location to the"},{"line_number":158,"context_line":"        supplied destination filepath."},{"line_number":159,"context_line":""}],"source_content_type":"text/x-python","patch_set":37,"id":"df7087c5_ad1010aa","line":156,"range":{"start_line":156,"start_character":17,"end_line":156,"end_character":25},"in_reply_to":"df7087c5_2f2c1998","updated":"2018-03-20 18:23:26.000000000","message":"Will be fixed in patch 39","commit_id":"e7c874a33fa8a2069084a81d2ef2712005547989"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"525d231ba6f1bd4b88bdd625fc24a430464c5b80","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        :param id_or_uri: A UUID identifier or an image URI to look up image"},{"line_number":163,"context_line":"                          information for."},{"line_number":164,"context_line":"        :param data: A file object to use in downloading image data."},{"line_number":165,"context_line":"        :param dest_path: Filepath to transfer image bits to."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        Note that because of the poor design of the"},{"line_number":168,"context_line":"        `glance.ImageService.download` method, the function returns different"}],"source_content_type":"text/x-python","patch_set":37,"id":"df7087c5_6f31b1ec","line":165,"updated":"2018-03-20 12:18:29.000000000","message":"Need to add the new param in here, however we add it.","commit_id":"e7c874a33fa8a2069084a81d2ef2712005547989"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"6104ffb8c153bbeae4591f443c511489a5082b81","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        :param id_or_uri: A UUID identifier or an image URI to look up image"},{"line_number":163,"context_line":"                          information for."},{"line_number":164,"context_line":"        :param data: A file object to use in downloading image data."},{"line_number":165,"context_line":"        :param dest_path: Filepath to transfer image bits to."},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"        Note that because of the poor design of the"},{"line_number":168,"context_line":"        `glance.ImageService.download` method, the function returns different"}],"source_content_type":"text/x-python","patch_set":37,"id":"df7087c5_4d27f413","line":165,"in_reply_to":"df7087c5_6f31b1ec","updated":"2018-03-20 18:23:26.000000000","message":"Will be fixed in patch 39","commit_id":"e7c874a33fa8a2069084a81d2ef2712005547989"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c8be0d6b8b3c0d61b200d341aafbd5905b1cef3a","unresolved":false,"context_lines":[{"line_number":163,"context_line":"                          information for."},{"line_number":164,"context_line":"        :param data: A file object to use in downloading image data."},{"line_number":165,"context_line":"        :param dest_path: Filepath to transfer image bits to."},{"line_number":166,"context_line":"        :param trusted_certs: List of trusted image certificate IDs."},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        Note that because of the poor design of the"},{"line_number":169,"context_line":"        `glance.ImageService.download` method, the function returns different"}],"source_content_type":"text/x-python","patch_set":72,"id":"5f7c97a3_63f000ff","line":166,"updated":"2018-06-04 14:33:07.000000000","message":"According to the actual glance image API implementation, this is wrong:\n\nhttps://review.openstack.org/#/c/479949/72/nova/image/glance.py@375\n\nAlso see my questions about usage in the libvirt driver in the next patch in the series:\n\nhttps://review.openstack.org/#/c/561262/17/nova/virt/libvirt/driver.py","commit_id":"d8e292f9cd4d52efff5692ad600a02f21b7af65b"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"b25ae9598c67b85407d8828e98a873bc40e29e7e","unresolved":false,"context_lines":[{"line_number":163,"context_line":"                          information for."},{"line_number":164,"context_line":"        :param data: A file object to use in downloading image data."},{"line_number":165,"context_line":"        :param dest_path: Filepath to transfer image bits to."},{"line_number":166,"context_line":"        :param trusted_certs: List of trusted image certificate IDs."},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        Note that because of the poor design of the"},{"line_number":169,"context_line":"        `glance.ImageService.download` method, the function returns different"}],"source_content_type":"text/x-python","patch_set":72,"id":"5f7c97a3_ac2e05a9","line":166,"in_reply_to":"5f7c97a3_03290c9a","updated":"2018-06-05 10:36:16.000000000","message":"I agree that the doc string isn\u0027t correct.  I\u0027ve fixed it in the latest patch.","commit_id":"d8e292f9cd4d52efff5692ad600a02f21b7af65b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ce379f472b86598b566777b245ce82bffb5cfdb6","unresolved":false,"context_lines":[{"line_number":163,"context_line":"                          information for."},{"line_number":164,"context_line":"        :param data: A file object to use in downloading image data."},{"line_number":165,"context_line":"        :param dest_path: Filepath to transfer image bits to."},{"line_number":166,"context_line":"        :param trusted_certs: List of trusted image certificate IDs."},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        Note that because of the poor design of the"},{"line_number":169,"context_line":"        `glance.ImageService.download` method, the function returns different"}],"source_content_type":"text/x-python","patch_set":72,"id":"5f7c97a3_2f1662f3","line":166,"in_reply_to":"5f7c97a3_03290c9a","updated":"2018-06-04 22:46:41.000000000","message":"I\u0027ll drop the -1, the -1 is more deserved on the libvirt change next in the series which passes a list of string IDs in one call here, which is clearly broken. The docstring can be updated in a follow up. Having said that, I haven\u0027t gone through the rest of this in detail yet.","commit_id":"d8e292f9cd4d52efff5692ad600a02f21b7af65b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"40539f9b774533c610f938760bf2bfc2742c8819","unresolved":false,"context_lines":[{"line_number":163,"context_line":"                          information for."},{"line_number":164,"context_line":"        :param data: A file object to use in downloading image data."},{"line_number":165,"context_line":"        :param dest_path: Filepath to transfer image bits to."},{"line_number":166,"context_line":"        :param trusted_certs: List of trusted image certificate IDs."},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        Note that because of the poor design of the"},{"line_number":169,"context_line":"        `glance.ImageService.download` method, the function returns different"}],"source_content_type":"text/x-python","patch_set":72,"id":"5f7c97a3_03290c9a","line":166,"in_reply_to":"5f7c97a3_63f000ff","updated":"2018-06-04 14:53:06.000000000","message":"To clarify what @mriedem is saying... it looks like trusted_certs is expected to be an objects.TrustedCerts object, not a list of trusted image certificate IDs...\n\nthat said, I\u0027m still +2 on the patch. I don\u0027t believe that is worth -1\u0027ing the patch over.","commit_id":"d8e292f9cd4d52efff5692ad600a02f21b7af65b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"171045f30e4ab27b362727702aec3925c0562dbd","unresolved":false,"context_lines":[{"line_number":163,"context_line":"                          information for."},{"line_number":164,"context_line":"        :param data: A file object to use in downloading image data."},{"line_number":165,"context_line":"        :param dest_path: Filepath to transfer image bits to."},{"line_number":166,"context_line":"        :param trusted_certs: A \u0027nova.objects.trusted_certs.TrustedCerts\u0027"},{"line_number":167,"context_line":"                              object with a list of trusted image certificate"},{"line_number":168,"context_line":"                              IDs."},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"        Note that because of the poor design of the"},{"line_number":171,"context_line":"        `glance.ImageService.download` method, the function returns different"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_d356d4ce","line":168,"range":{"start_line":166,"start_character":30,"end_line":168,"end_character":34},"updated":"2018-06-12 13:30:52.000000000","message":"for the moment, I don\u0027t see where you pass it, but fine, I trust you.","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c23d94b7b48aea36e76fed7543410c746136879e","unresolved":false,"context_lines":[{"line_number":163,"context_line":"                          information for."},{"line_number":164,"context_line":"        :param data: A file object to use in downloading image data."},{"line_number":165,"context_line":"        :param dest_path: Filepath to transfer image bits to."},{"line_number":166,"context_line":"        :param trusted_certs: A \u0027nova.objects.trusted_certs.TrustedCerts\u0027"},{"line_number":167,"context_line":"                              object with a list of trusted image certificate"},{"line_number":168,"context_line":"                              IDs."},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"        Note that because of the poor design of the"},{"line_number":171,"context_line":"        `glance.ImageService.download` method, the function returns different"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_f382f0c6","line":168,"range":{"start_line":166,"start_character":30,"end_line":168,"end_character":34},"in_reply_to":"5f7c97a3_d356d4ce","updated":"2018-06-12 13:35:47.000000000","message":"driver.spawn (next patch in the series) plumbs that through to call this","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"}],"nova/image/glance.py":[{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"0e54c86887a3208ee5c59247c402ec6be4deaa90","unresolved":false,"context_lines":[{"line_number":335,"context_line":"                        certificate_utils.verify_certificate("},{"line_number":336,"context_line":"                            context\u003dcontext,"},{"line_number":337,"context_line":"                            certificate_uuid\u003dimg_sig_cert_uuid,"},{"line_number":338,"context_line":"                            trusted_certificate_uuids\u003dtrusted_cert_uuids)"},{"line_number":339,"context_line":"                        LOG.info("},{"line_number":340,"context_line":"                            _LI(\u0027Image signature certificate validation \u0027"},{"line_number":341,"context_line":"                                \u0027succeeded for certificate: %s\u0027),"}],"source_content_type":"text/x-python","patch_set":11,"id":"9f436f4f_d4c92aac","line":338,"range":{"start_line":338,"start_character":54,"end_line":338,"end_character":72},"updated":"2017-08-11 21:12:23.000000000","message":"Can we pass this directly to cursive?\n\nFrom what I understand, this is a \"TrustedCerts\" object, and the list of trusted_cert_uuids needs to be pulled out of the TrustedCerts object before it can be passed to cursive.","commit_id":"9bc90a806f8f10abe5625c2d9a71df667b2819ad"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"4277826705f18d07bf90dc889189142064d5e6c6","unresolved":false,"context_lines":[{"line_number":335,"context_line":"                        certificate_utils.verify_certificate("},{"line_number":336,"context_line":"                            context\u003dcontext,"},{"line_number":337,"context_line":"                            certificate_uuid\u003dimg_sig_cert_uuid,"},{"line_number":338,"context_line":"                            trusted_certificate_uuids\u003dtrusted_cert_uuids)"},{"line_number":339,"context_line":"                        LOG.info("},{"line_number":340,"context_line":"                            _LI(\u0027Image signature certificate validation \u0027"},{"line_number":341,"context_line":"                                \u0027succeeded for certificate: %s\u0027),"}],"source_content_type":"text/x-python","patch_set":11,"id":"9f436f4f_9c0b9b7b","line":338,"range":{"start_line":338,"start_character":54,"end_line":338,"end_character":72},"in_reply_to":"9f436f4f_d4c92aac","updated":"2017-08-15 01:49:32.000000000","message":"Thanks for pointing this out, Brianna. Fixed in patch 12.","commit_id":"9bc90a806f8f10abe5625c2d9a71df667b2819ad"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"0e54c86887a3208ee5c59247c402ec6be4deaa90","unresolved":false,"context_lines":[{"line_number":337,"context_line":"                            certificate_uuid\u003dimg_sig_cert_uuid,"},{"line_number":338,"context_line":"                            trusted_certificate_uuids\u003dtrusted_cert_uuids)"},{"line_number":339,"context_line":"                        LOG.info("},{"line_number":340,"context_line":"                            _LI(\u0027Image signature certificate validation \u0027"},{"line_number":341,"context_line":"                                \u0027succeeded for certificate: %s\u0027),"},{"line_number":342,"context_line":"                            img_sig_cert_uuid)"},{"line_number":343,"context_line":"                    except cursive_exception.SignatureVerificationError:"}],"source_content_type":"text/x-python","patch_set":11,"id":"9f436f4f_94fa82ba","line":340,"range":{"start_line":340,"start_character":28,"end_line":340,"end_character":31},"updated":"2017-08-11 21:12:23.000000000","message":"this should also be removed","commit_id":"9bc90a806f8f10abe5625c2d9a71df667b2819ad"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"0e54c86887a3208ee5c59247c402ec6be4deaa90","unresolved":false,"context_lines":[{"line_number":343,"context_line":"                    except cursive_exception.SignatureVerificationError:"},{"line_number":344,"context_line":"                        with excutils.save_and_reraise_exception():"},{"line_number":345,"context_line":"                            LOG.error("},{"line_number":346,"context_line":"                                _LE(\u0027Image signature certificate validation \u0027"},{"line_number":347,"context_line":"                                    \u0027failed for certificate: %s\u0027),"},{"line_number":348,"context_line":"                                img_sig_cert_uuid)"},{"line_number":349,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":11,"id":"9f436f4f_b4110607","line":346,"range":{"start_line":346,"start_character":32,"end_line":346,"end_character":35},"updated":"2017-08-11 21:12:23.000000000","message":"log message translations no longer occur, so this should be removed.","commit_id":"9bc90a806f8f10abe5625c2d9a71df667b2819ad"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"4277826705f18d07bf90dc889189142064d5e6c6","unresolved":false,"context_lines":[{"line_number":343,"context_line":"                    except cursive_exception.SignatureVerificationError:"},{"line_number":344,"context_line":"                        with excutils.save_and_reraise_exception():"},{"line_number":345,"context_line":"                            LOG.error("},{"line_number":346,"context_line":"                                _LE(\u0027Image signature certificate validation \u0027"},{"line_number":347,"context_line":"                                    \u0027failed for certificate: %s\u0027),"},{"line_number":348,"context_line":"                                img_sig_cert_uuid)"},{"line_number":349,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":11,"id":"9f436f4f_1c744bfd","line":346,"range":{"start_line":346,"start_character":32,"end_line":346,"end_character":35},"in_reply_to":"9f436f4f_b4110607","updated":"2017-08-15 01:49:32.000000000","message":"Fixed in patch 12.","commit_id":"9bc90a806f8f10abe5625c2d9a71df667b2819ad"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"0e54c86887a3208ee5c59247c402ec6be4deaa90","unresolved":false,"context_lines":[{"line_number":348,"context_line":"                                img_sig_cert_uuid)"},{"line_number":349,"context_line":"                else:"},{"line_number":350,"context_line":"                    raise cursive_exception.SignatureVerificationError("},{"line_number":351,"context_line":"                        message\u003d\u0027No image signature certificate to validate\u0027)"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"        close_file \u003d False"},{"line_number":354,"context_line":"        if data is None and dst_path:"}],"source_content_type":"text/x-python","patch_set":11,"id":"9f436f4f_284fff34","line":351,"range":{"start_line":351,"start_character":33,"end_line":351,"end_character":75},"updated":"2017-08-11 21:12:23.000000000","message":"I think this should be re-worded so that it is more obvious how to fix it (since this message is displayed to the user in Horizon if the instance fails to boot with no trusted cert uuids provided).  Currently, this is what would be displayed to the user: \"Error: Failed to perform requested operation on instance \"this-is-the-instance-name\", the instance has an error status: Please try again later [Error: Build of instance 68b71045-c46a-4a9b-9cd6-cee4cc9b4951 aborted: No image signature certificate to validate]. \"  Maybe something like this instead:\n\nNo trusted certificate IDs provided, even though certificate validation is enabled.  Unable to validate the certificate used to verify the image signature.","commit_id":"9bc90a806f8f10abe5625c2d9a71df667b2819ad"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"4277826705f18d07bf90dc889189142064d5e6c6","unresolved":false,"context_lines":[{"line_number":348,"context_line":"                                img_sig_cert_uuid)"},{"line_number":349,"context_line":"                else:"},{"line_number":350,"context_line":"                    raise cursive_exception.SignatureVerificationError("},{"line_number":351,"context_line":"                        message\u003d\u0027No image signature certificate to validate\u0027)"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"        close_file \u003d False"},{"line_number":354,"context_line":"        if data is None and dst_path:"}],"source_content_type":"text/x-python","patch_set":11,"id":"9f436f4f_5c577359","line":351,"range":{"start_line":351,"start_character":33,"end_line":351,"end_character":75},"in_reply_to":"9f436f4f_284fff34","updated":"2017-08-15 01:49:32.000000000","message":"Reworded in patch 12.","commit_id":"9bc90a806f8f10abe5625c2d9a71df667b2819ad"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"c06fd9c57f8ae70c3e1f17493e814bcf2fddc1ff","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"import cryptography"},{"line_number":30,"context_line":"from cursive import exception as cursive_exception"},{"line_number":31,"context_line":"from cursive import certificate_utils"},{"line_number":32,"context_line":"from cursive import signature_utils"},{"line_number":33,"context_line":"import glanceclient"},{"line_number":34,"context_line":"import glanceclient.exc"}],"source_content_type":"text/x-python","patch_set":14,"id":"7f515b1d_16801646","line":31,"range":{"start_line":31,"start_character":20,"end_line":31,"end_character":37},"updated":"2017-10-05 20:39:31.000000000","message":"This line should be moved up one (c comes before e)","commit_id":"595e13e94ac22cc77740a4830d014291981da7ad"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"6529634f220e80ded34bfe8148489ade3cad01ac","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"import cryptography"},{"line_number":30,"context_line":"from cursive import exception as cursive_exception"},{"line_number":31,"context_line":"from cursive import certificate_utils"},{"line_number":32,"context_line":"from cursive import signature_utils"},{"line_number":33,"context_line":"import glanceclient"},{"line_number":34,"context_line":"import glanceclient.exc"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f4e5783_f2b4a337","line":31,"range":{"start_line":31,"start_character":20,"end_line":31,"end_character":37},"in_reply_to":"7f515b1d_16801646","updated":"2017-10-17 15:16:25.000000000","message":"Thanks, Brianna. This will be done in the next patch.","commit_id":"595e13e94ac22cc77740a4830d014291981da7ad"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"ae26df08a090de629ac287cde93160237017acba","unresolved":false,"context_lines":[{"line_number":353,"context_line":"                        certificate_utils.verify_certificate("},{"line_number":354,"context_line":"                            context\u003dcontext,"},{"line_number":355,"context_line":"                            certificate_uuid\u003dimg_sig_cert_uuid,"},{"line_number":356,"context_line":"                            trusted_certificate_uuids\u003dtrusted_certs)"},{"line_number":357,"context_line":"                        LOG.info(\u0027Image signature certificate validation \u0027"},{"line_number":358,"context_line":"                                 \u0027succeeded for certificate: %s\u0027,"},{"line_number":359,"context_line":"                                 img_sig_cert_uuid)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_be8c2dd7","line":356,"range":{"start_line":356,"start_character":54,"end_line":356,"end_character":67},"updated":"2017-10-24 19:04:17.000000000","message":"I think this needs to be trusted_certs.ids","commit_id":"500e9a8c0213dbcf1001addc8e53e3b3d6bb5504"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"bb05d30af4fb2e24b9ca0fa7128b761676f69a91","unresolved":false,"context_lines":[{"line_number":353,"context_line":"                        certificate_utils.verify_certificate("},{"line_number":354,"context_line":"                            context\u003dcontext,"},{"line_number":355,"context_line":"                            certificate_uuid\u003dimg_sig_cert_uuid,"},{"line_number":356,"context_line":"                            trusted_certificate_uuids\u003dtrusted_certs)"},{"line_number":357,"context_line":"                        LOG.info(\u0027Image signature certificate validation \u0027"},{"line_number":358,"context_line":"                                 \u0027succeeded for certificate: %s\u0027,"},{"line_number":359,"context_line":"                                 img_sig_cert_uuid)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_07d9b4f1","line":356,"range":{"start_line":356,"start_character":54,"end_line":356,"end_character":67},"in_reply_to":"3f4b6375_be8c2dd7","updated":"2017-10-24 21:10:19.000000000","message":"Good catch, Brianna. Done in patch 21.","commit_id":"500e9a8c0213dbcf1001addc8e53e3b3d6bb5504"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3869cb1c630cc742f1fe906cf4ecbf7172a0e9ac","unresolved":false,"context_lines":[{"line_number":368,"context_line":"                        LOG.error(\u0027Image signature certificate validation \u0027"},{"line_number":369,"context_line":"                                  \u0027failed for certificate: %s\u0027,"},{"line_number":370,"context_line":"                                  img_sig_cert_uuid)"},{"line_number":371,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":372,"context_line":"                raise cursive_exception.SignatureVerificationError("},{"line_number":373,"context_line":"                    message\u003d\u0027Image signature certificate validation enabled, \u0027"},{"line_number":374,"context_line":"                            \u0027but no trusted certificate IDs were provided. \u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"7f96bb07_a246a420","line":371,"range":{"start_line":371,"start_character":12,"end_line":371,"end_character":59},"updated":"2018-01-15 20:13:55.000000000","message":"note to reviewers... \n\ntrusted_certs will contain CONF.glance.default_trusted_certificate_ids if the user did not pass their own trusted cert IDs on server boot/rebuild. See line 915 from the REST API patch here:\n\nhttps://review.openstack.org/#/c/486204/28/nova/compute/api.py\n\nso this block here will be executed IFF:\n\nCONF.glance.default_trusted_certificate_ids is empty\nAND\nCONF.glance.enabled_certificate_validation is True\nAND\nthe user did not pass anything in trusted_image_certificates parameter to boot request.","commit_id":"053b68784b82d4c787b5e45825c85a241634e316"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"011c5857a8bb82c320cea572d70678ef8128f0aa","unresolved":false,"context_lines":[{"line_number":293,"context_line":"        if not any(check(mode) for check in (stat.S_ISFIFO, stat.S_ISSOCK)):"},{"line_number":294,"context_line":"            os.fsync(fileno)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"    def download(self, context, image_id, data\u003dNone, dst_path\u003dNone, **kwargs):"},{"line_number":297,"context_line":"        \"\"\"Calls out to Glance for data and writes data.\"\"\""},{"line_number":298,"context_line":"        if CONF.glance.allowed_direct_url_schemes and dst_path is not None:"},{"line_number":299,"context_line":"            image \u003d self.show(context, image_id, include_locations\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f93b717_92b8e02a","line":296,"range":{"start_line":296,"start_character":68,"end_line":296,"end_character":76},"updated":"2018-01-25 22:58:15.000000000","message":"Why are you doing this instead of making trusted_certs a proper argument?","commit_id":"fce54e9eb0b9160eb877e60d7cc9b8be51720f35"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"ed49a68b35088834d953fc1fcce0adee91afe6e6","unresolved":false,"context_lines":[{"line_number":293,"context_line":"        if not any(check(mode) for check in (stat.S_ISFIFO, stat.S_ISSOCK)):"},{"line_number":294,"context_line":"            os.fsync(fileno)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"    def download(self, context, image_id, data\u003dNone, dst_path\u003dNone, **kwargs):"},{"line_number":297,"context_line":"        \"\"\"Calls out to Glance for data and writes data.\"\"\""},{"line_number":298,"context_line":"        if CONF.glance.allowed_direct_url_schemes and dst_path is not None:"},{"line_number":299,"context_line":"            image \u003d self.show(context, image_id, include_locations\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f93b717_fe6c8b09","line":296,"range":{"start_line":296,"start_character":68,"end_line":296,"end_character":76},"in_reply_to":"5f93b717_92b8e02a","updated":"2018-01-26 02:47:04.000000000","message":"Didn\u0027t want to change arguments in case download() was called elsewhere without trusted_certs, but setting trusted_certs\u003dNone by default should make that irrelevant","commit_id":"fce54e9eb0b9160eb877e60d7cc9b8be51720f35"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"c0e3fe5c360a90cb7af39b8dc4ec022fad2e9d40","unresolved":false,"context_lines":[{"line_number":293,"context_line":"        if not any(check(mode) for check in (stat.S_ISFIFO, stat.S_ISSOCK)):"},{"line_number":294,"context_line":"            os.fsync(fileno)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"    def download(self, context, image_id, data\u003dNone, dst_path\u003dNone, **kwargs):"},{"line_number":297,"context_line":"        \"\"\"Calls out to Glance for data and writes data.\"\"\""},{"line_number":298,"context_line":"        if CONF.glance.allowed_direct_url_schemes and dst_path is not None:"},{"line_number":299,"context_line":"            image \u003d self.show(context, image_id, include_locations\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f93b717_090fbfe1","line":296,"range":{"start_line":296,"start_character":68,"end_line":296,"end_character":76},"in_reply_to":"5f93b717_fe6c8b09","updated":"2018-01-26 11:52:06.000000000","message":"Done","commit_id":"fce54e9eb0b9160eb877e60d7cc9b8be51720f35"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"011c5857a8bb82c320cea572d70678ef8128f0aa","unresolved":false,"context_lines":[{"line_number":320,"context_line":""},{"line_number":321,"context_line":"        # Verify image signature if feature is enabled or trusted"},{"line_number":322,"context_line":"        # certificates were provided"},{"line_number":323,"context_line":"        trusted_certs \u003d kwargs.get(\u0027trusted_certs\u0027, None)"},{"line_number":324,"context_line":"        if trusted_certs or CONF.glance.verify_glance_signatures:"},{"line_number":325,"context_line":"            image_meta_dict \u003d self.show(context, image_id,"},{"line_number":326,"context_line":"                                        include_locations\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f93b717_b2b5dc33","line":323,"updated":"2018-01-25 22:58:15.000000000","message":":(\n\nPython has a construct for making this exactly as optional as you\u0027ve done here. Why not use it?","commit_id":"fce54e9eb0b9160eb877e60d7cc9b8be51720f35"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"35e31e5c825b7327e06a06c6d084ec04c6e9fa48","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        # Verify image signature if feature is enabled or trusted"},{"line_number":322,"context_line":"        # certificates were provided"},{"line_number":323,"context_line":"        trusted_certs \u003d kwargs.get(\u0027trusted_certs\u0027, None)"},{"line_number":324,"context_line":"        if trusted_certs or CONF.glance.verify_glance_signatures:"},{"line_number":325,"context_line":"            image_meta_dict \u003d self.show(context, image_id,"},{"line_number":326,"context_line":"                                        include_locations\u003dFalse)"},{"line_number":327,"context_line":"            image_meta \u003d objects.ImageMeta.from_dict(image_meta_dict)"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f93b717_d8ae87e6","line":324,"range":{"start_line":324,"start_character":25,"end_line":324,"end_character":27},"updated":"2018-01-26 00:34:59.000000000","message":"Is it ever a valid case for trusted_certs to be set and CONF.glance.verify_glance_signatures \u003d False? I\u0027d have thought not and that this \"if trusted_certs\" should only live on L352.\n\nIf the above case is valid, then you need to handle the combination where trusted_certs \u003d truthy and CONF.glance.verify_glance_signatures is falsey. Because as-is, this is going to try to do signature verification even if the config option is False, if trusted_certs were provided.","commit_id":"fce54e9eb0b9160eb877e60d7cc9b8be51720f35"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0d50d1d2d4c5b30ee47d33040cd2ca73b918cd1c","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        # Verify image signature if feature is enabled or trusted"},{"line_number":322,"context_line":"        # certificates were provided"},{"line_number":323,"context_line":"        trusted_certs \u003d kwargs.get(\u0027trusted_certs\u0027, None)"},{"line_number":324,"context_line":"        if trusted_certs or CONF.glance.verify_glance_signatures:"},{"line_number":325,"context_line":"            image_meta_dict \u003d self.show(context, image_id,"},{"line_number":326,"context_line":"                                        include_locations\u003dFalse)"},{"line_number":327,"context_line":"            image_meta \u003d objects.ImageMeta.from_dict(image_meta_dict)"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f93b717_a4d7d2ed","line":324,"range":{"start_line":324,"start_character":25,"end_line":324,"end_character":27},"in_reply_to":"5f93b717_3ebe7365","updated":"2018-01-26 03:57:36.000000000","message":"I see, thanks.","commit_id":"fce54e9eb0b9160eb877e60d7cc9b8be51720f35"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"ed49a68b35088834d953fc1fcce0adee91afe6e6","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        # Verify image signature if feature is enabled or trusted"},{"line_number":322,"context_line":"        # certificates were provided"},{"line_number":323,"context_line":"        trusted_certs \u003d kwargs.get(\u0027trusted_certs\u0027, None)"},{"line_number":324,"context_line":"        if trusted_certs or CONF.glance.verify_glance_signatures:"},{"line_number":325,"context_line":"            image_meta_dict \u003d self.show(context, image_id,"},{"line_number":326,"context_line":"                                        include_locations\u003dFalse)"},{"line_number":327,"context_line":"            image_meta \u003d objects.ImageMeta.from_dict(image_meta_dict)"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f93b717_3ebe7365","line":324,"range":{"start_line":324,"start_character":25,"end_line":324,"end_character":27},"in_reply_to":"5f93b717_d8ae87e6","updated":"2018-01-26 02:47:04.000000000","message":"Yes, we intentionally made it so that trusted_certs can be set and CONF.glance.verify_glance_signatures \u003d False. In this case, certificate validation will still occur. See line 17 of https://etherpad.openstack.org/p/queens-nova-certificate-validation","commit_id":"fce54e9eb0b9160eb877e60d7cc9b8be51720f35"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"011c5857a8bb82c320cea572d70678ef8128f0aa","unresolved":false,"context_lines":[{"line_number":378,"context_line":"                         \u0027certificate validation will be performed jointly, \u0027"},{"line_number":379,"context_line":"                         \u0027so a list of trusted image certificate IDs will \u0027"},{"line_number":380,"context_line":"                         \u0027need to be provided in order to validate a signed \u0027"},{"line_number":381,"context_line":"                         \u0027image.\u0027)"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"        close_file \u003d False"},{"line_number":384,"context_line":"        if data is None and dst_path:"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f93b717_52e9f817","line":381,"updated":"2018-01-25 22:58:15.000000000","message":"This is a super long log message.Couldn\u0027t we just say what happened without the full explanation of what may come in the future? Also, isn\u0027t this kindof a warning-level thing since you\u0027re basically saying the operator messed up?","commit_id":"fce54e9eb0b9160eb877e60d7cc9b8be51720f35"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"c0e3fe5c360a90cb7af39b8dc4ec022fad2e9d40","unresolved":false,"context_lines":[{"line_number":378,"context_line":"                         \u0027certificate validation will be performed jointly, \u0027"},{"line_number":379,"context_line":"                         \u0027so a list of trusted image certificate IDs will \u0027"},{"line_number":380,"context_line":"                         \u0027need to be provided in order to validate a signed \u0027"},{"line_number":381,"context_line":"                         \u0027image.\u0027)"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"        close_file \u003d False"},{"line_number":384,"context_line":"        if data is None and dst_path:"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f93b717_c921d74f","line":381,"in_reply_to":"5f93b717_1ebdb764","updated":"2018-01-26 11:52:06.000000000","message":"Done","commit_id":"fce54e9eb0b9160eb877e60d7cc9b8be51720f35"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"ed49a68b35088834d953fc1fcce0adee91afe6e6","unresolved":false,"context_lines":[{"line_number":378,"context_line":"                         \u0027certificate validation will be performed jointly, \u0027"},{"line_number":379,"context_line":"                         \u0027so a list of trusted image certificate IDs will \u0027"},{"line_number":380,"context_line":"                         \u0027need to be provided in order to validate a signed \u0027"},{"line_number":381,"context_line":"                         \u0027image.\u0027)"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"        close_file \u003d False"},{"line_number":384,"context_line":"        if data is None and dst_path:"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f93b717_1ebdb764","line":381,"in_reply_to":"5f93b717_52e9f817","updated":"2018-01-26 02:47:04.000000000","message":"We can change to a warning and remove the \"In the future...\" part","commit_id":"fce54e9eb0b9160eb877e60d7cc9b8be51720f35"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"525d231ba6f1bd4b88bdd625fc24a430464c5b80","unresolved":false,"context_lines":[{"line_number":302,"context_line":"            os.fsync(fileno)"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"    def download(self, context, image_id, data\u003dNone, dst_path\u003dNone,"},{"line_number":305,"context_line":"                 trusted_certs\u003dNone):"},{"line_number":306,"context_line":"        \"\"\"Calls out to Glance for data and writes data.\"\"\""},{"line_number":307,"context_line":"        if CONF.glance.allowed_direct_url_schemes and dst_path is not None:"},{"line_number":308,"context_line":"            image \u003d self.show(context, image_id, include_locations\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":37,"id":"df7087c5_2f51790f","line":305,"updated":"2018-03-20 12:18:29.000000000","message":"This seems better, just need to do that in all the other places too.","commit_id":"e7c874a33fa8a2069084a81d2ef2712005547989"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"6104ffb8c153bbeae4591f443c511489a5082b81","unresolved":false,"context_lines":[{"line_number":302,"context_line":"            os.fsync(fileno)"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"    def download(self, context, image_id, data\u003dNone, dst_path\u003dNone,"},{"line_number":305,"context_line":"                 trusted_certs\u003dNone):"},{"line_number":306,"context_line":"        \"\"\"Calls out to Glance for data and writes data.\"\"\""},{"line_number":307,"context_line":"        if CONF.glance.allowed_direct_url_schemes and dst_path is not None:"},{"line_number":308,"context_line":"            image \u003d self.show(context, image_id, include_locations\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":37,"id":"df7087c5_6d22f803","line":305,"in_reply_to":"df7087c5_2f51790f","updated":"2018-03-20 18:23:26.000000000","message":"Will be fixed in patch 39","commit_id":"e7c874a33fa8a2069084a81d2ef2712005547989"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"525d231ba6f1bd4b88bdd625fc24a430464c5b80","unresolved":false,"context_lines":[{"line_number":384,"context_line":"                            \u0027Unable to validate the certificate used to \u0027"},{"line_number":385,"context_line":"                            \u0027verify the image signature.\u0027)"},{"line_number":386,"context_line":"            else:"},{"line_number":387,"context_line":"                LOG.warning(\u0027Image signature verification succeeded, but \u0027"},{"line_number":388,"context_line":"                            \u0027certificate validation was not performed. A list \u0027"},{"line_number":389,"context_line":"                            \u0027of trusted image certificate IDs must be \u0027"},{"line_number":390,"context_line":"                            \u0027provided in order to validate an image \u0027"}],"source_content_type":"text/x-python","patch_set":37,"id":"df7087c5_ef5aa128","line":387,"updated":"2018-03-20 12:18:29.000000000","message":"Seems fixed to me.","commit_id":"e7c874a33fa8a2069084a81d2ef2712005547989"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"6104ffb8c153bbeae4591f443c511489a5082b81","unresolved":false,"context_lines":[{"line_number":384,"context_line":"                            \u0027Unable to validate the certificate used to \u0027"},{"line_number":385,"context_line":"                            \u0027verify the image signature.\u0027)"},{"line_number":386,"context_line":"            else:"},{"line_number":387,"context_line":"                LOG.warning(\u0027Image signature verification succeeded, but \u0027"},{"line_number":388,"context_line":"                            \u0027certificate validation was not performed. A list \u0027"},{"line_number":389,"context_line":"                            \u0027of trusted image certificate IDs must be \u0027"},{"line_number":390,"context_line":"                            \u0027provided in order to validate an image \u0027"}],"source_content_type":"text/x-python","patch_set":37,"id":"df7087c5_4d6cd435","line":387,"in_reply_to":"df7087c5_ef5aa128","updated":"2018-03-20 18:23:26.000000000","message":"This only gets displayed if the user enables image signature verification without providing a list of trusted image certificates","commit_id":"e7c874a33fa8a2069084a81d2ef2712005547989"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"96efedf15baab4c7660e43e3d8b77bf91c1001af","unresolved":false,"context_lines":[{"line_number":369,"context_line":"                        context\u003dcontext,"},{"line_number":370,"context_line":"                        certificate_uuid\u003dimg_sig_cert_uuid,"},{"line_number":371,"context_line":"                        trusted_certificate_uuids\u003dtrusted_certs.ids)"},{"line_number":372,"context_line":"                    LOG.info(\u0027Image signature certificate validation \u0027"},{"line_number":373,"context_line":"                             \u0027succeeded for certificate: %s\u0027,"},{"line_number":374,"context_line":"                             img_sig_cert_uuid)"},{"line_number":375,"context_line":"                except cursive_exception.SignatureVerificationError:"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_4c6a031a","line":372,"updated":"2018-04-03 14:19:04.000000000","message":"I wonder if it\u0027s too verbose to log at info level for \"everything worked, don\u0027t worry\" ?\n\nThis should probably be debug, IMHO.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"f2c10d8e748d18f397c35b1089cc93555bbe6e24","unresolved":false,"context_lines":[{"line_number":369,"context_line":"                        context\u003dcontext,"},{"line_number":370,"context_line":"                        certificate_uuid\u003dimg_sig_cert_uuid,"},{"line_number":371,"context_line":"                        trusted_certificate_uuids\u003dtrusted_certs.ids)"},{"line_number":372,"context_line":"                    LOG.info(\u0027Image signature certificate validation \u0027"},{"line_number":373,"context_line":"                             \u0027succeeded for certificate: %s\u0027,"},{"line_number":374,"context_line":"                             img_sig_cert_uuid)"},{"line_number":375,"context_line":"                except cursive_exception.SignatureVerificationError:"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_4f10b54e","line":372,"in_reply_to":"bf659307_4c6a031a","updated":"2018-04-03 15:19:33.000000000","message":"Agreed. Good point. This will be fixed in the next patch.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"96efedf15baab4c7660e43e3d8b77bf91c1001af","unresolved":false,"context_lines":[{"line_number":373,"context_line":"                             \u0027succeeded for certificate: %s\u0027,"},{"line_number":374,"context_line":"                             img_sig_cert_uuid)"},{"line_number":375,"context_line":"                except cursive_exception.SignatureVerificationError:"},{"line_number":376,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":377,"context_line":"                        LOG.error(\u0027Image signature certificate validation \u0027"},{"line_number":378,"context_line":"                                  \u0027failed for certificate: %s\u0027,"},{"line_number":379,"context_line":"                                  img_sig_cert_uuid)"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_8ceb3b76","line":376,"updated":"2018-04-03 14:19:04.000000000","message":"If I\u0027m an admin and I have something going on with this stuff, I think i expect to be able to set enable_certificate_validation\u003dFalse and disable it. But by my reading here, even if I disable this in config, if people have trusted_certs in their image, I will fail to boot them, right?  I\u0027m sure the reasoning there is that you\u0027re assuming that people with trusted certs would rather not boot a thing, than boot it unverified, but I think that it\u0027s odd to not have a circuit breaker that the admin can flip to *actually* disable it, no?\n\nI think it\u0027s not unlikely to expect that the admin is providing all the images, with the trusted_certs bit, but while trying to get it to work, she needs to be able to enable and disable it at will, and currently that means modifying images.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"1a910e8af454febf7328711d2345ae94151ad31c","unresolved":false,"context_lines":[{"line_number":373,"context_line":"                             \u0027succeeded for certificate: %s\u0027,"},{"line_number":374,"context_line":"                             img_sig_cert_uuid)"},{"line_number":375,"context_line":"                except cursive_exception.SignatureVerificationError:"},{"line_number":376,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":377,"context_line":"                        LOG.error(\u0027Image signature certificate validation \u0027"},{"line_number":378,"context_line":"                                  \u0027failed for certificate: %s\u0027,"},{"line_number":379,"context_line":"                                  img_sig_cert_uuid)"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_592aa67e","line":376,"in_reply_to":"bf659307_68623ab0","updated":"2018-04-04 18:25:42.000000000","message":"I\u0027ll add a note to explain our decision and the idea of adding a soft check in the fugure","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"f2c10d8e748d18f397c35b1089cc93555bbe6e24","unresolved":false,"context_lines":[{"line_number":373,"context_line":"                             \u0027succeeded for certificate: %s\u0027,"},{"line_number":374,"context_line":"                             img_sig_cert_uuid)"},{"line_number":375,"context_line":"                except cursive_exception.SignatureVerificationError:"},{"line_number":376,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":377,"context_line":"                        LOG.error(\u0027Image signature certificate validation \u0027"},{"line_number":378,"context_line":"                                  \u0027failed for certificate: %s\u0027,"},{"line_number":379,"context_line":"                                  img_sig_cert_uuid)"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_ef52e9dd","line":376,"in_reply_to":"bf659307_8ceb3b76","updated":"2018-04-03 15:19:33.000000000","message":"\u003e But by my reading here, even if I disable this in config, if people have trusted_certs in their image, I will fail to boot them, right?\n\nCorrect. We included this reasoning in the spec:\n\nIt is possible a silent fail scenario could occur if (1) nova is not configured to conduct certificate validation, and (2) the user provides trusted certificate IDs expecting certificate validation to occur. In this case, nova would not conduct certificate validation and would boot the instance, causing the user to believe certificate validation succeeded even though it never happened. To prevent this from happening, the boot workflow will be updated to conduct signature and certificate verification if trusted certificate IDs are associated with the instance data. This matches the user’s expectations and prevents a silent fail scenario from ever occurring. Note here that this override only occurs if the user specifies certificate IDs. The default list of certificate IDs is only used if the feature is enabled and therefore will never trigger the override.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9c9e121ede0bd79d5de454ccc14ece1f82b14968","unresolved":false,"context_lines":[{"line_number":373,"context_line":"                             \u0027succeeded for certificate: %s\u0027,"},{"line_number":374,"context_line":"                             img_sig_cert_uuid)"},{"line_number":375,"context_line":"                except cursive_exception.SignatureVerificationError:"},{"line_number":376,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":377,"context_line":"                        LOG.error(\u0027Image signature certificate validation \u0027"},{"line_number":378,"context_line":"                                  \u0027failed for certificate: %s\u0027,"},{"line_number":379,"context_line":"                                  img_sig_cert_uuid)"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_68623ab0","line":376,"in_reply_to":"bf659307_950a5a1d","updated":"2018-04-04 09:32:41.000000000","message":"Good point, it is odd not to make this a \"soft\" check somehow. We could always add that later I guess.\n\nI guess this might make certain upgrades bumpy, but I don\u0027t see much of an alternative.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4cee0bb348b4ba292e1b82e011d92a917e2401e5","unresolved":false,"context_lines":[{"line_number":373,"context_line":"                             \u0027succeeded for certificate: %s\u0027,"},{"line_number":374,"context_line":"                             img_sig_cert_uuid)"},{"line_number":375,"context_line":"                except cursive_exception.SignatureVerificationError:"},{"line_number":376,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":377,"context_line":"                        LOG.error(\u0027Image signature certificate validation \u0027"},{"line_number":378,"context_line":"                                  \u0027failed for certificate: %s\u0027,"},{"line_number":379,"context_line":"                                  img_sig_cert_uuid)"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_950a5a1d","line":376,"in_reply_to":"bf659307_ef52e9dd","updated":"2018-04-03 15:28:32.000000000","message":"Yeah, and I get that we want to provide safety for the user, it just seems odd to not provide the operator a way to turn it off completely. It\u0027d be trivial for an operator to modify one line of code and cause this not to do the thing you want, so it\u0027s not like this is un-circumventable.\n\nAnyway, the spec says it, so that\u0027s fine, but it seems weird to me.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"96efedf15baab4c7660e43e3d8b77bf91c1001af","unresolved":false,"context_lines":[{"line_number":384,"context_line":"                            \u0027Unable to validate the certificate used to \u0027"},{"line_number":385,"context_line":"                            \u0027verify the image signature.\u0027)"},{"line_number":386,"context_line":"            else:"},{"line_number":387,"context_line":"                LOG.warning(\u0027Image signature verification succeeded, but \u0027"},{"line_number":388,"context_line":"                            \u0027certificate validation was not performed. A list \u0027"},{"line_number":389,"context_line":"                            \u0027of trusted image certificate IDs must be \u0027"},{"line_number":390,"context_line":"                            \u0027provided in order to validate an image \u0027"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_cc7e13e3","line":387,"updated":"2018-04-03 14:19:04.000000000","message":"Isn\u0027t this going to warn on every single image download until the end of time? For people that don\u0027t want to use this feature, they\u0027ll just get warning spam in their logs all the time right?","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4cee0bb348b4ba292e1b82e011d92a917e2401e5","unresolved":false,"context_lines":[{"line_number":384,"context_line":"                            \u0027Unable to validate the certificate used to \u0027"},{"line_number":385,"context_line":"                            \u0027verify the image signature.\u0027)"},{"line_number":386,"context_line":"            else:"},{"line_number":387,"context_line":"                LOG.warning(\u0027Image signature verification succeeded, but \u0027"},{"line_number":388,"context_line":"                            \u0027certificate validation was not performed. A list \u0027"},{"line_number":389,"context_line":"                            \u0027of trusted image certificate IDs must be \u0027"},{"line_number":390,"context_line":"                            \u0027provided in order to validate an image \u0027"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_75a30601","line":387,"in_reply_to":"bf659307_0f5ddd13","updated":"2018-04-03 15:28:32.000000000","message":"Okay right, but if I just want signature validation but not certificate validation (i.e. what I have today), I get log spam all the time yeah?","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"d484fe5aeb6bcef9bf6619badb7c9fd53600bc45","unresolved":false,"context_lines":[{"line_number":384,"context_line":"                            \u0027Unable to validate the certificate used to \u0027"},{"line_number":385,"context_line":"                            \u0027verify the image signature.\u0027)"},{"line_number":386,"context_line":"            else:"},{"line_number":387,"context_line":"                LOG.warning(\u0027Image signature verification succeeded, but \u0027"},{"line_number":388,"context_line":"                            \u0027certificate validation was not performed. A list \u0027"},{"line_number":389,"context_line":"                            \u0027of trusted image certificate IDs must be \u0027"},{"line_number":390,"context_line":"                            \u0027provided in order to validate an image \u0027"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_f505d614","line":387,"in_reply_to":"bf659307_75a30601","updated":"2018-04-03 16:00:02.000000000","message":"Correct. Maybe we could change to a debug log message?","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"f2c10d8e748d18f397c35b1089cc93555bbe6e24","unresolved":false,"context_lines":[{"line_number":384,"context_line":"                            \u0027Unable to validate the certificate used to \u0027"},{"line_number":385,"context_line":"                            \u0027verify the image signature.\u0027)"},{"line_number":386,"context_line":"            else:"},{"line_number":387,"context_line":"                LOG.warning(\u0027Image signature verification succeeded, but \u0027"},{"line_number":388,"context_line":"                            \u0027certificate validation was not performed. A list \u0027"},{"line_number":389,"context_line":"                            \u0027of trusted image certificate IDs must be \u0027"},{"line_number":390,"context_line":"                            \u0027provided in order to validate an image \u0027"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_0f5ddd13","line":387,"in_reply_to":"bf659307_cc7e13e3","updated":"2018-04-03 15:19:33.000000000","message":"People who don\u0027t want to use this feature (certificate validation), but have image signature verification enabled will see this warning. If you have both features disabled, then you should not see this warning.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"13d14da27847c36768b28b6b68c224292c71c383","unresolved":false,"context_lines":[{"line_number":384,"context_line":"                            \u0027Unable to validate the certificate used to \u0027"},{"line_number":385,"context_line":"                            \u0027verify the image signature.\u0027)"},{"line_number":386,"context_line":"            else:"},{"line_number":387,"context_line":"                LOG.warning(\u0027Image signature verification succeeded, but \u0027"},{"line_number":388,"context_line":"                            \u0027certificate validation was not performed. A list \u0027"},{"line_number":389,"context_line":"                            \u0027of trusted image certificate IDs must be \u0027"},{"line_number":390,"context_line":"                            \u0027provided in order to validate an image \u0027"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_133e5fc0","line":387,"in_reply_to":"bf659307_f505d614","updated":"2018-04-03 18:02:26.000000000","message":"Sounds reasonable.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bdfab2326b27d87ce0a2f23e64c54f0fc365c14a","unresolved":false,"context_lines":[{"line_number":377,"context_line":"                              \u0027succeeded for certificate: %s\u0027,"},{"line_number":378,"context_line":"                              img_sig_cert_uuid)"},{"line_number":379,"context_line":"                except cursive_exception.SignatureVerificationError:"},{"line_number":380,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":381,"context_line":"                        LOG.error(\u0027Image signature certificate validation \u0027"},{"line_number":382,"context_line":"                                  \u0027failed for certificate: %s\u0027,"},{"line_number":383,"context_line":"                                  img_sig_cert_uuid)"}],"source_content_type":"text/x-python","patch_set":43,"id":"bf659307_b1ae5d96","line":380,"updated":"2018-04-05 14:16:25.000000000","message":"This will cause a huge stack trace in the logs if it happens. Since this is user-controlled, this should just be a warning and a cleanup of the process right? I assume you need to raise here, but catch SignatureVerificationError somewhere above this to avoid the explosion. Perhaps best to translate this cursive error into something Nova-y. I see that the existing verification code also raises, but I imagine it\u0027s because nobody looked at the logs so that probably also needs fixing.\n\nhttp://logs.openstack.org/10/515210/29/check/barbican-simple-crypto-devstack-tempest/4e92c1a/logs/screen-n-cpu.txt.gz#_Apr_04_19_01_15_844613","commit_id":"6276d017d893521820be03d84fccffc05787e188"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"20c79fa324768fcb07a7909a86b3284404d91131","unresolved":false,"context_lines":[{"line_number":377,"context_line":"                              \u0027succeeded for certificate: %s\u0027,"},{"line_number":378,"context_line":"                              img_sig_cert_uuid)"},{"line_number":379,"context_line":"                except cursive_exception.SignatureVerificationError:"},{"line_number":380,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":381,"context_line":"                        LOG.error(\u0027Image signature certificate validation \u0027"},{"line_number":382,"context_line":"                                  \u0027failed for certificate: %s\u0027,"},{"line_number":383,"context_line":"                                  img_sig_cert_uuid)"}],"source_content_type":"text/x-python","patch_set":43,"id":"bf659307_746bcc1e","line":380,"in_reply_to":"bf659307_b1ae5d96","updated":"2018-04-05 21:16:11.000000000","message":"Warning and cleanup is sufficient. Are you looking for a very generic NovaException?\n\nAlso, do you want the exceptions in the signature verification code fixed in this patch? Or save it for another patch?","commit_id":"6276d017d893521820be03d84fccffc05787e188"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bdfab2326b27d87ce0a2f23e64c54f0fc365c14a","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                                  \u0027failed for certificate: %s\u0027,"},{"line_number":383,"context_line":"                                  img_sig_cert_uuid)"},{"line_number":384,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":385,"context_line":"                raise cursive_exception.SignatureVerificationError("},{"line_number":386,"context_line":"                    message\u003d\u0027Image signature certificate validation enabled, \u0027"},{"line_number":387,"context_line":"                            \u0027but no trusted certificate IDs were provided. \u0027"},{"line_number":388,"context_line":"                            \u0027Unable to validate the certificate used to \u0027"}],"source_content_type":"text/x-python","patch_set":43,"id":"bf659307_11c4e93f","line":385,"updated":"2018-04-05 14:16:25.000000000","message":"I imagine this also creates a traceback, unnecessarily. Probably need to warn here as well and raise whatever common exception you use above.","commit_id":"6276d017d893521820be03d84fccffc05787e188"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c5e99f2983aef2389fa529921c2780c47c3113f7","unresolved":false,"context_lines":[{"line_number":380,"context_line":"                    LOG.warning(\u0027Image signature certificate validation \u0027"},{"line_number":381,"context_line":"                                \u0027failed for certificate: %s\u0027,"},{"line_number":382,"context_line":"                                img_sig_cert_uuid)"},{"line_number":383,"context_line":"                    raise exception.CertificateValidationFailed("},{"line_number":384,"context_line":"                        cert_uuid\u003dimg_sig_cert_uuid)"},{"line_number":385,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":386,"context_line":"                msg \u003d \u0027Image signature certificate validation enabled, but \u0027 \\"}],"source_content_type":"text/x-python","patch_set":44,"id":"bf659307_300467c6","line":383,"updated":"2018-04-06 23:11:41.000000000","message":"Where are you catching this now? The thing we need to avoid is a stack trace in the logs. I see you\u0027re translating it to something generic, but we need to catch it and just fail the build like normal without spamming the logs.\n\nApologies if I missed it, but I don\u0027t think I see it.","commit_id":"9db34af604788a52c38ab3a02aa978c0e2c149b9"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"5a1d8d862560e834203559e1cc057896e3708978","unresolved":false,"context_lines":[{"line_number":380,"context_line":"                    LOG.warning(\u0027Image signature certificate validation \u0027"},{"line_number":381,"context_line":"                                \u0027failed for certificate: %s\u0027,"},{"line_number":382,"context_line":"                                img_sig_cert_uuid)"},{"line_number":383,"context_line":"                    raise exception.CertificateValidationFailed("},{"line_number":384,"context_line":"                        cert_uuid\u003dimg_sig_cert_uuid)"},{"line_number":385,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":386,"context_line":"                msg \u003d \u0027Image signature certificate validation enabled, but \u0027 \\"}],"source_content_type":"text/x-python","patch_set":44,"id":"bf659307_832acf8d","line":383,"in_reply_to":"bf659307_300467c6","updated":"2018-04-07 02:24:20.000000000","message":"Sorry, I\u0027m a bit confused. If we just want to avoid a stack trace in the logs and make sure that the error message can still be seen in the logs, warnings without raising any sort of exception should be sufficient, right?","commit_id":"9db34af604788a52c38ab3a02aa978c0e2c149b9"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"3525779428190b31289bbe5d11c90044be36efcc","unresolved":false,"context_lines":[{"line_number":380,"context_line":"                    LOG.warning(\u0027Image signature certificate validation \u0027"},{"line_number":381,"context_line":"                                \u0027failed for certificate: %s\u0027,"},{"line_number":382,"context_line":"                                img_sig_cert_uuid)"},{"line_number":383,"context_line":"                    raise exception.CertificateValidationFailed("},{"line_number":384,"context_line":"                        cert_uuid\u003dimg_sig_cert_uuid)"},{"line_number":385,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":386,"context_line":"                msg \u003d \u0027Image signature certificate validation enabled, but \u0027 \\"}],"source_content_type":"text/x-python","patch_set":44,"id":"bf659307_e0913af3","line":383,"in_reply_to":"bf659307_832acf8d","updated":"2018-04-08 04:48:07.000000000","message":"Added in https://review.openstack.org/#/c/479949/44..47/nova/virt/libvirt/driver.py","commit_id":"9db34af604788a52c38ab3a02aa978c0e2c149b9"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6e73166285929230b75147d1f79ee0280f1c09d3","unresolved":false,"context_lines":[{"line_number":390,"context_line":"                LOG.warning(msg)"},{"line_number":391,"context_line":"                raise exception.CertificateValidationFailed(msg)"},{"line_number":392,"context_line":"            else:"},{"line_number":393,"context_line":"                LOG.debug(\u0027Image signature verification succeeded, but \u0027"},{"line_number":394,"context_line":"                          \u0027certificate validation was not performed. A list \u0027"},{"line_number":395,"context_line":"                          \u0027of trusted image certificate IDs must be provided \u0027"},{"line_number":396,"context_line":"                          \u0027in order to validate an image certificate.\u0027)"}],"source_content_type":"text/x-python","patch_set":57,"id":"bf659307_9246f07a","line":393,"updated":"2018-04-13 20:45:47.000000000","message":"If I\u0027m understanding this correctly, image signature verification has not yet succeeded at this point. On L353, there\u0027s \"get_verifier\" but later on the signature is verified. So this log message would create confusion if the signature verification later fails and you get a \"succeeded\" and a \"failed\" log. So I\u0027d just remove the \"Image signature verification succeeded, but\" part.","commit_id":"783a0e8201d5f05819bd8910fe66519d44b003cf"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"c84948aa273119214ac71a32718a046b5d20ec2b","unresolved":false,"context_lines":[{"line_number":390,"context_line":"                LOG.warning(msg)"},{"line_number":391,"context_line":"                raise exception.CertificateValidationFailed(msg)"},{"line_number":392,"context_line":"            else:"},{"line_number":393,"context_line":"                LOG.debug(\u0027Image signature verification succeeded, but \u0027"},{"line_number":394,"context_line":"                          \u0027certificate validation was not performed. A list \u0027"},{"line_number":395,"context_line":"                          \u0027of trusted image certificate IDs must be provided \u0027"},{"line_number":396,"context_line":"                          \u0027in order to validate an image certificate.\u0027)"}],"source_content_type":"text/x-python","patch_set":57,"id":"bf659307_757b067a","line":393,"in_reply_to":"bf659307_9246f07a","updated":"2018-04-13 21:02:27.000000000","message":"Doh! Good catch. I\u0027ll fix this.","commit_id":"783a0e8201d5f05819bd8910fe66519d44b003cf"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6e73166285929230b75147d1f79ee0280f1c09d3","unresolved":false,"context_lines":[{"line_number":400,"context_line":"            data \u003d open(dst_path, \u0027wb\u0027)"},{"line_number":401,"context_line":"            close_file \u003d True"},{"line_number":402,"context_line":""},{"line_number":403,"context_line":"        if data is None:"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"            # Perform image signature verification"},{"line_number":406,"context_line":"            if verifier:"},{"line_number":407,"context_line":"                try:"},{"line_number":408,"context_line":"                    for chunk in image_chunks:"},{"line_number":409,"context_line":"                        verifier.update(chunk)"},{"line_number":410,"context_line":"                    verifier.verify()"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"                    LOG.info(\u0027Image signature verification succeeded \u0027"},{"line_number":413,"context_line":"                             \u0027for image: %s\u0027, image_id)"},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"                except cryptography.exceptions.InvalidSignature:"},{"line_number":416,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":417,"context_line":"                        LOG.error(\u0027Image signature verification failed \u0027"},{"line_number":418,"context_line":"                                  \u0027for image: %s\u0027, image_id)"},{"line_number":419,"context_line":"            return image_chunks"},{"line_number":420,"context_line":"        else:"},{"line_number":421,"context_line":"            try:"},{"line_number":422,"context_line":"                for chunk in image_chunks:"},{"line_number":423,"context_line":"                    if verifier:"},{"line_number":424,"context_line":"                        verifier.update(chunk)"},{"line_number":425,"context_line":"                    data.write(chunk)"},{"line_number":426,"context_line":"                if verifier:"},{"line_number":427,"context_line":"                    verifier.verify()"},{"line_number":428,"context_line":"                    LOG.info(\u0027Image signature verification succeeded \u0027"},{"line_number":429,"context_line":"                             \u0027for image %s\u0027, image_id)"},{"line_number":430,"context_line":"            except cryptography.exceptions.InvalidSignature:"},{"line_number":431,"context_line":"                data.truncate(0)"},{"line_number":432,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":433,"context_line":"                    LOG.error(\u0027Image signature verification failed \u0027"},{"line_number":434,"context_line":"                              \u0027for image: %s\u0027, image_id)"},{"line_number":435,"context_line":"            except Exception as ex:"},{"line_number":436,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":437,"context_line":"                    LOG.error(\"Error writing to %(path)s: %(exception)s\","},{"line_number":438,"context_line":"                              {\u0027path\u0027: dst_path, \u0027exception\u0027: ex})"},{"line_number":439,"context_line":"            finally:"},{"line_number":440,"context_line":"                if close_file:"},{"line_number":441,"context_line":"                    # Ensure that the data is pushed all the way down to"},{"line_number":442,"context_line":"                    # persistent storage. This ensures that in the event of a"},{"line_number":443,"context_line":"                    # subsequent host crash we don\u0027t have running instances"},{"line_number":444,"context_line":"                    # using a corrupt backing file."},{"line_number":445,"context_line":"                    data.flush()"},{"line_number":446,"context_line":"                    self._safe_fsync(data)"},{"line_number":447,"context_line":"                    data.close()"},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"    def create(self, context, image_meta, data\u003dNone):"},{"line_number":450,"context_line":"        \"\"\"Store the image data and return the new image object.\"\"\""}],"source_content_type":"text/x-python","patch_set":57,"id":"bf659307_f5195679","line":447,"range":{"start_line":403,"start_character":8,"end_line":447,"end_character":32},"updated":"2018-04-13 20:45:47.000000000","message":"This is the rest of the signature verification stuff.","commit_id":"783a0e8201d5f05819bd8910fe66519d44b003cf"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"561b5b3793b166668e4ced16d8be3a32e92f32a9","unresolved":false,"context_lines":[{"line_number":390,"context_line":"                LOG.warning(msg)"},{"line_number":391,"context_line":"                raise exception.CertificateValidationFailed(msg)"},{"line_number":392,"context_line":"            else:"},{"line_number":393,"context_line":"                LOG.debug(\u0027Image signature verification succeeded, but \u0027"},{"line_number":394,"context_line":"                          \u0027certificate validation was not performed. A list \u0027"},{"line_number":395,"context_line":"                          \u0027of trusted image certificate IDs must be provided \u0027"},{"line_number":396,"context_line":"                          \u0027in order to validate an image certificate.\u0027)"}],"source_content_type":"text/x-python","patch_set":62,"id":"9f6a8fd7_c3a54cb7","line":393,"range":{"start_line":393,"start_character":26,"end_line":393,"end_character":72},"updated":"2018-04-16 15:11:06.000000000","message":"This part should be removed. Signature verification hasn\u0027t been performed yet at this point. If you log this and if signature verification fails later on L418 or L434, you\u0027ll get a log that says that signature verification both succeeded and failed.","commit_id":"0fb58969f1fb90663455dd3c5d3d5e1f292d844b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"561b5b3793b166668e4ced16d8be3a32e92f32a9","unresolved":false,"context_lines":[{"line_number":415,"context_line":"                except cryptography.exceptions.InvalidSignature:"},{"line_number":416,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":417,"context_line":"                        LOG.error(\u0027Image signature verification failed \u0027"},{"line_number":418,"context_line":"                                  \u0027for image: %s\u0027, image_id)"},{"line_number":419,"context_line":"            return image_chunks"},{"line_number":420,"context_line":"        else:"},{"line_number":421,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":62,"id":"9f6a8fd7_43913c52","line":418,"updated":"2018-04-16 15:11:06.000000000","message":"Signature verification can fail here.","commit_id":"0fb58969f1fb90663455dd3c5d3d5e1f292d844b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"561b5b3793b166668e4ced16d8be3a32e92f32a9","unresolved":false,"context_lines":[{"line_number":431,"context_line":"                data.truncate(0)"},{"line_number":432,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":433,"context_line":"                    LOG.error(\u0027Image signature verification failed \u0027"},{"line_number":434,"context_line":"                              \u0027for image: %s\u0027, image_id)"},{"line_number":435,"context_line":"            except Exception as ex:"},{"line_number":436,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":437,"context_line":"                    LOG.error(\"Error writing to %(path)s: %(exception)s\","}],"source_content_type":"text/x-python","patch_set":62,"id":"9f6a8fd7_e39df066","line":434,"updated":"2018-04-16 15:11:06.000000000","message":"Signature verification can fail here too,","commit_id":"0fb58969f1fb90663455dd3c5d3d5e1f292d844b"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"f2dca72cb2626b95067c807e1cae9761c0941f8b","unresolved":false,"context_lines":[{"line_number":383,"context_line":"                    raise exception.CertificateValidationFailed("},{"line_number":384,"context_line":"                        cert_uuid\u003dimg_sig_cert_uuid, reason\u003de.message)"},{"line_number":385,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":386,"context_line":"                msg \u003d \u0027Image signature certificate validation enabled, but \u0027 \\"},{"line_number":387,"context_line":"                      \u0027no trusted certificate IDs were provided. Unable to \u0027 \\"},{"line_number":388,"context_line":"                      \u0027validate the certificate used to verify the image \u0027 \\"},{"line_number":389,"context_line":"                      \u0027signature.\u0027"}],"source_content_type":"text/x-python","patch_set":66,"id":"9f6a8fd7_01d212d3","line":386,"range":{"start_line":386,"start_character":77,"end_line":386,"end_character":78},"updated":"2018-04-19 04:07:33.000000000","message":"It is not necessary. You can remove it.","commit_id":"b59369a6eb2fe522faf07d842ecc35d59ac26200"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"74060363a2fd5ec8db6a9dd74ab1c922c217391c","unresolved":false,"context_lines":[{"line_number":383,"context_line":"                    raise exception.CertificateValidationFailed("},{"line_number":384,"context_line":"                        cert_uuid\u003dimg_sig_cert_uuid, reason\u003de.message)"},{"line_number":385,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":386,"context_line":"                msg \u003d \u0027Image signature certificate validation enabled, but \u0027 \\"},{"line_number":387,"context_line":"                      \u0027no trusted certificate IDs were provided. Unable to \u0027 \\"},{"line_number":388,"context_line":"                      \u0027validate the certificate used to verify the image \u0027 \\"},{"line_number":389,"context_line":"                      \u0027signature.\u0027"}],"source_content_type":"text/x-python","patch_set":66,"id":"9f6a8fd7_a48d747c","line":386,"range":{"start_line":386,"start_character":77,"end_line":386,"end_character":78},"in_reply_to":"9f6a8fd7_01d212d3","updated":"2018-04-19 04:30:14.000000000","message":"Are you sure about that? flake8 doesn\u0027t seem to like that:\n\n  Running flake8 on all files\n  ./nova/image/glance.py:387:23: E111 indentation is not a multiple of four\n  ./nova/image/glance.py:387:23: E113 unexpected indentation\n  ./nova/image/glance.py:387:23: E901 IndentationError: unexpected indent\n  ./nova/image/glance.py:388:23: E111 indentation is not a multiple of four\n  ./nova/image/glance.py:389:23: E111 indentation is not a multiple of four\n\nIf we want to move the backslash, we need to wrap the entire string in parentheses.","commit_id":"b59369a6eb2fe522faf07d842ecc35d59ac26200"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"8f28c4a27a5a8bc6de2170040d8e41842d835869","unresolved":false,"context_lines":[{"line_number":383,"context_line":"                    raise exception.CertificateValidationFailed("},{"line_number":384,"context_line":"                        cert_uuid\u003dimg_sig_cert_uuid, reason\u003de.message)"},{"line_number":385,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":386,"context_line":"                msg \u003d \u0027Image signature certificate validation enabled, but \u0027 \\"},{"line_number":387,"context_line":"                      \u0027no trusted certificate IDs were provided. Unable to \u0027 \\"},{"line_number":388,"context_line":"                      \u0027validate the certificate used to verify the image \u0027 \\"},{"line_number":389,"context_line":"                      \u0027signature.\u0027"}],"source_content_type":"text/x-python","patch_set":66,"id":"9f6a8fd7_aa72ffab","line":386,"range":{"start_line":386,"start_character":77,"end_line":386,"end_character":78},"in_reply_to":"9f6a8fd7_47055e45","updated":"2018-04-20 02:10:17.000000000","message":"No problem, the parentheses helped. Thanks for the review!","commit_id":"b59369a6eb2fe522faf07d842ecc35d59ac26200"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"3a17b60c9cb7df04628669d95572f4656d3b988f","unresolved":false,"context_lines":[{"line_number":383,"context_line":"                    raise exception.CertificateValidationFailed("},{"line_number":384,"context_line":"                        cert_uuid\u003dimg_sig_cert_uuid, reason\u003de.message)"},{"line_number":385,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":386,"context_line":"                msg \u003d \u0027Image signature certificate validation enabled, but \u0027 \\"},{"line_number":387,"context_line":"                      \u0027no trusted certificate IDs were provided. Unable to \u0027 \\"},{"line_number":388,"context_line":"                      \u0027validate the certificate used to verify the image \u0027 \\"},{"line_number":389,"context_line":"                      \u0027signature.\u0027"}],"source_content_type":"text/x-python","patch_set":66,"id":"9f6a8fd7_47055e45","line":386,"range":{"start_line":386,"start_character":77,"end_line":386,"end_character":78},"in_reply_to":"9f6a8fd7_a48d747c","updated":"2018-04-20 01:52:26.000000000","message":"Sorry. That was my misunderstanding.\nI wanted to say that it is better to avoid using \u0027\\\u0027.","commit_id":"b59369a6eb2fe522faf07d842ecc35d59ac26200"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"f2dca72cb2626b95067c807e1cae9761c0941f8b","unresolved":false,"context_lines":[{"line_number":384,"context_line":"                        cert_uuid\u003dimg_sig_cert_uuid, reason\u003de.message)"},{"line_number":385,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":386,"context_line":"                msg \u003d \u0027Image signature certificate validation enabled, but \u0027 \\"},{"line_number":387,"context_line":"                      \u0027no trusted certificate IDs were provided. Unable to \u0027 \\"},{"line_number":388,"context_line":"                      \u0027validate the certificate used to verify the image \u0027 \\"},{"line_number":389,"context_line":"                      \u0027signature.\u0027"},{"line_number":390,"context_line":"                LOG.warning(msg)"}],"source_content_type":"text/x-python","patch_set":66,"id":"9f6a8fd7_21d5d6b9","line":387,"range":{"start_line":387,"start_character":77,"end_line":387,"end_character":78},"updated":"2018-04-19 04:07:33.000000000","message":"ditto","commit_id":"b59369a6eb2fe522faf07d842ecc35d59ac26200"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"f2dca72cb2626b95067c807e1cae9761c0941f8b","unresolved":false,"context_lines":[{"line_number":385,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":386,"context_line":"                msg \u003d \u0027Image signature certificate validation enabled, but \u0027 \\"},{"line_number":387,"context_line":"                      \u0027no trusted certificate IDs were provided. Unable to \u0027 \\"},{"line_number":388,"context_line":"                      \u0027validate the certificate used to verify the image \u0027 \\"},{"line_number":389,"context_line":"                      \u0027signature.\u0027"},{"line_number":390,"context_line":"                LOG.warning(msg)"},{"line_number":391,"context_line":"                raise exception.CertificateValidationFailed(msg)"}],"source_content_type":"text/x-python","patch_set":66,"id":"9f6a8fd7_c1dbbaea","line":388,"range":{"start_line":388,"start_character":75,"end_line":388,"end_character":76},"updated":"2018-04-19 04:07:33.000000000","message":"ditto","commit_id":"b59369a6eb2fe522faf07d842ecc35d59ac26200"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"5c434bc76fbce7244e31a09ecf020e0e4aec7800","unresolved":false,"context_lines":[{"line_number":383,"context_line":"                    raise exception.CertificateValidationFailed("},{"line_number":384,"context_line":"                        cert_uuid\u003dimg_sig_cert_uuid, reason\u003de.message)"},{"line_number":385,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":386,"context_line":"                msg \u003d (\u0027Image signature certificate validation enabled, but \u0027"},{"line_number":387,"context_line":"                       \u0027no trusted certificate IDs were provided. Unable to \u0027"},{"line_number":388,"context_line":"                       \u0027validate the certificate used to verify the image \u0027"},{"line_number":389,"context_line":"                       \u0027signature.\u0027)"},{"line_number":390,"context_line":"                LOG.warning(msg)"},{"line_number":391,"context_line":"                raise exception.CertificateValidationFailed(msg)"},{"line_number":392,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":67,"id":"9f6a8fd7_678742b7","line":389,"range":{"start_line":386,"start_character":22,"end_line":389,"end_character":36},"updated":"2018-04-20 01:53:57.000000000","message":"Nice","commit_id":"3d9dbc4dbc56be204281d3007fd3bb162fbf8270"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bab5dd76f25174b70cd811446f3d246024ae3290","unresolved":false,"context_lines":[{"line_number":335,"context_line":""},{"line_number":336,"context_line":"        # Verify image signature if feature is enabled or trusted"},{"line_number":337,"context_line":"        # certificates were provided"},{"line_number":338,"context_line":"        if trusted_certs or CONF.glance.verify_glance_signatures:"},{"line_number":339,"context_line":"            image_meta_dict \u003d self.show(context, image_id,"},{"line_number":340,"context_line":"                                        include_locations\u003dFalse)"},{"line_number":341,"context_line":"            image_meta \u003d objects.ImageMeta.from_dict(image_meta_dict)"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_b36558e3","line":338,"updated":"2018-06-12 13:28:28.000000000","message":"nit: this entire conditional block should be put into a private method since the download() method is getting too big now, but that refactor could happen in a follow up change.","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"ad3ec32143815af952ba2b4c38273c4ddceddcd2","unresolved":false,"context_lines":[{"line_number":335,"context_line":""},{"line_number":336,"context_line":"        # Verify image signature if feature is enabled or trusted"},{"line_number":337,"context_line":"        # certificates were provided"},{"line_number":338,"context_line":"        if trusted_certs or CONF.glance.verify_glance_signatures:"},{"line_number":339,"context_line":"            image_meta_dict \u003d self.show(context, image_id,"},{"line_number":340,"context_line":"                                        include_locations\u003dFalse)"},{"line_number":341,"context_line":"            image_meta \u003d objects.ImageMeta.from_dict(image_meta_dict)"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_a798aebf","line":338,"in_reply_to":"5f7c97a3_b36558e3","updated":"2018-06-12 16:11:10.000000000","message":"Done","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bab5dd76f25174b70cd811446f3d246024ae3290","unresolved":false,"context_lines":[{"line_number":381,"context_line":"                                \u0027failed for certificate: %s\u0027,"},{"line_number":382,"context_line":"                                img_sig_cert_uuid)"},{"line_number":383,"context_line":"                    raise exception.CertificateValidationFailed("},{"line_number":384,"context_line":"                        cert_uuid\u003dimg_sig_cert_uuid, reason\u003de.message)"},{"line_number":385,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":386,"context_line":"                msg \u003d (\u0027Image signature certificate validation enabled, but \u0027"},{"line_number":387,"context_line":"                       \u0027no trusted certificate IDs were provided. Unable to \u0027"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_13ea0c0e","line":384,"range":{"start_line":384,"start_character":60,"end_line":384,"end_character":69},"updated":"2018-06-12 13:28:28.000000000","message":"This would be better as:\n\nsix.text_type(e)\n\nIn case that \u0027message\u0027 field ever changes.","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"ad3ec32143815af952ba2b4c38273c4ddceddcd2","unresolved":false,"context_lines":[{"line_number":381,"context_line":"                                \u0027failed for certificate: %s\u0027,"},{"line_number":382,"context_line":"                                img_sig_cert_uuid)"},{"line_number":383,"context_line":"                    raise exception.CertificateValidationFailed("},{"line_number":384,"context_line":"                        cert_uuid\u003dimg_sig_cert_uuid, reason\u003de.message)"},{"line_number":385,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":386,"context_line":"                msg \u003d (\u0027Image signature certificate validation enabled, but \u0027"},{"line_number":387,"context_line":"                       \u0027no trusted certificate IDs were provided. Unable to \u0027"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_c7d84a80","line":384,"range":{"start_line":384,"start_character":60,"end_line":384,"end_character":69},"in_reply_to":"5f7c97a3_13ea0c0e","updated":"2018-06-12 16:11:10.000000000","message":"Done","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bab5dd76f25174b70cd811446f3d246024ae3290","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                                img_sig_cert_uuid)"},{"line_number":383,"context_line":"                    raise exception.CertificateValidationFailed("},{"line_number":384,"context_line":"                        cert_uuid\u003dimg_sig_cert_uuid, reason\u003de.message)"},{"line_number":385,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":386,"context_line":"                msg \u003d (\u0027Image signature certificate validation enabled, but \u0027"},{"line_number":387,"context_line":"                       \u0027no trusted certificate IDs were provided. Unable to \u0027"},{"line_number":388,"context_line":"                       \u0027validate the certificate used to verify the image \u0027"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_d3c814a9","line":385,"updated":"2018-06-12 13:28:28.000000000","message":"Why wouldn\u0027t we check default_trusted_certificate_ids here? I believe the answer is that the API will use default_trusted_certificate_ids if the user didn\u0027t provide trusted_certs, but please confirm.","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"171045f30e4ab27b362727702aec3925c0562dbd","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                                img_sig_cert_uuid)"},{"line_number":383,"context_line":"                    raise exception.CertificateValidationFailed("},{"line_number":384,"context_line":"                        cert_uuid\u003dimg_sig_cert_uuid, reason\u003de.message)"},{"line_number":385,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":386,"context_line":"                msg \u003d (\u0027Image signature certificate validation enabled, but \u0027"},{"line_number":387,"context_line":"                       \u0027no trusted certificate IDs were provided. Unable to \u0027"},{"line_number":388,"context_line":"                       \u0027validate the certificate used to verify the image \u0027"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_b3c79845","line":385,"range":{"start_line":385,"start_character":17,"end_line":385,"end_character":59},"updated":"2018-06-12 13:30:52.000000000","message":"here, you\u0027re explicity asking people who use an already deprecated config option to stop using it.\nThat\u0027s acceptable, but you at least need to write a release note asking folks to stop using before they upgrade to Rocky.","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"ad3ec32143815af952ba2b4c38273c4ddceddcd2","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                                img_sig_cert_uuid)"},{"line_number":383,"context_line":"                    raise exception.CertificateValidationFailed("},{"line_number":384,"context_line":"                        cert_uuid\u003dimg_sig_cert_uuid, reason\u003de.message)"},{"line_number":385,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":386,"context_line":"                msg \u003d (\u0027Image signature certificate validation enabled, but \u0027"},{"line_number":387,"context_line":"                       \u0027no trusted certificate IDs were provided. Unable to \u0027"},{"line_number":388,"context_line":"                       \u0027validate the certificate used to verify the image \u0027"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_473bda9a","line":385,"range":{"start_line":385,"start_character":17,"end_line":385,"end_character":59},"in_reply_to":"5f7c97a3_16b99aca","updated":"2018-06-12 16:11:10.000000000","message":"I replied to the mailing list thread, and also added a check for default_trusted_certificate_ids here.","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"434d2bee7cf98c99061076fc63a67e8cc628cc86","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                                img_sig_cert_uuid)"},{"line_number":383,"context_line":"                    raise exception.CertificateValidationFailed("},{"line_number":384,"context_line":"                        cert_uuid\u003dimg_sig_cert_uuid, reason\u003de.message)"},{"line_number":385,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":386,"context_line":"                msg \u003d (\u0027Image signature certificate validation enabled, but \u0027"},{"line_number":387,"context_line":"                       \u0027no trusted certificate IDs were provided. Unable to \u0027"},{"line_number":388,"context_line":"                       \u0027validate the certificate used to verify the image \u0027"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_16b99aca","line":385,"range":{"start_line":385,"start_character":17,"end_line":385,"end_character":59},"in_reply_to":"5f7c97a3_53e1649a","updated":"2018-06-12 14:01:25.000000000","message":"http://lists.openstack.org/pipermail/openstack-dev/2018-June/131410.html","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c23d94b7b48aea36e76fed7543410c746136879e","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                                img_sig_cert_uuid)"},{"line_number":383,"context_line":"                    raise exception.CertificateValidationFailed("},{"line_number":384,"context_line":"                        cert_uuid\u003dimg_sig_cert_uuid, reason\u003de.message)"},{"line_number":385,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":386,"context_line":"                msg \u003d (\u0027Image signature certificate validation enabled, but \u0027"},{"line_number":387,"context_line":"                       \u0027no trusted certificate IDs were provided. Unable to \u0027"},{"line_number":388,"context_line":"                       \u0027validate the certificate used to verify the image \u0027"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_53e1649a","line":385,"range":{"start_line":385,"start_character":17,"end_line":385,"end_character":59},"in_reply_to":"5f7c97a3_b3c79845","updated":"2018-06-12 13:35:47.000000000","message":"enable_certificate_validation is a weird transition option, read the help on it:\n\nhttps://docs.openstack.org/nova/latest/configuration/config.html#glance.enable_certificate_validation\n\nI can\u0027t say I totally understand it, but as I noted above, I believe the flow is for the API to pass the default trusted certs (if configured) when image signature verification is enabled - see the API change later in the series for that.","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"171045f30e4ab27b362727702aec3925c0562dbd","unresolved":false,"context_lines":[{"line_number":388,"context_line":"                       \u0027validate the certificate used to verify the image \u0027"},{"line_number":389,"context_line":"                       \u0027signature.\u0027)"},{"line_number":390,"context_line":"                LOG.warning(msg)"},{"line_number":391,"context_line":"                raise exception.CertificateValidationFailed(msg)"},{"line_number":392,"context_line":"            else:"},{"line_number":393,"context_line":"                LOG.debug(\u0027Certificate validation was not performed. A list \u0027"},{"line_number":394,"context_line":"                          \u0027of trusted image certificate IDs must be provided \u0027"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_33e148d1","line":391,"updated":"2018-06-12 13:30:52.000000000","message":"here, you now provide a exception if folks use it. That could be an upgrade issue, so please at least provide the reno file or just don\u0027t raise an exception here.","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c23d94b7b48aea36e76fed7543410c746136879e","unresolved":false,"context_lines":[{"line_number":388,"context_line":"                       \u0027validate the certificate used to verify the image \u0027"},{"line_number":389,"context_line":"                       \u0027signature.\u0027)"},{"line_number":390,"context_line":"                LOG.warning(msg)"},{"line_number":391,"context_line":"                raise exception.CertificateValidationFailed(msg)"},{"line_number":392,"context_line":"            else:"},{"line_number":393,"context_line":"                LOG.debug(\u0027Certificate validation was not performed. A list \u0027"},{"line_number":394,"context_line":"                          \u0027of trusted image certificate IDs must be provided \u0027"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_73f58050","line":391,"in_reply_to":"5f7c97a3_33e148d1","updated":"2018-06-12 13:35:47.000000000","message":"Yeah I wondered about this too...my only thinking is the API change at the end is used to turn this on, and you shouldn\u0027t configure any of your computes to use this until the API is ready for it, but I\u0027m a bit unclear on the upgrade dance myself.","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"79b64462431d65efdbb1d59e5d428c8efc7e806f","unresolved":false,"context_lines":[{"line_number":423,"context_line":"            # We may want to consider making this a \"soft\" check in the future."},{"line_number":424,"context_line":"            if trusted_certs:"},{"line_number":425,"context_line":"                _verify_certs(context, img_sig_cert_uuid, trusted_certs)"},{"line_number":426,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":427,"context_line":"                if CONF.glance.default_trusted_certificate_ids:"},{"line_number":428,"context_line":"                    trusted_certs \u003d objects.TrustedCerts("},{"line_number":429,"context_line":"                        ids\u003dCONF.glance.default_trusted_certificate_ids)"},{"line_number":430,"context_line":"                    _verify_certs(context, img_sig_cert_uuid, trusted_certs)"}],"source_content_type":"text/x-python","patch_set":77,"id":"5f7c97a3_2ca23eaf","line":427,"range":{"start_line":426,"start_character":12,"end_line":427,"end_character":63},"updated":"2018-06-12 18:17:06.000000000","message":"I think it would be cleaner if we handled this up above, something like this:\n\nif not trusted_certs and \\\n       CONF.glance.enable_certificate_validation and \\\n       CONF.glance.default_trusted_certificate_ids:\n    trusted_certs \u003d CONF.glance.default_trusted_certificate_ids\n\nThen you get the trusted_certs conditionals and checking down in here for free.","commit_id":"016e987d57b78a0eb2d1d7b16510dcc384e117bf"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"79b64462431d65efdbb1d59e5d428c8efc7e806f","unresolved":false,"context_lines":[{"line_number":425,"context_line":"                _verify_certs(context, img_sig_cert_uuid, trusted_certs)"},{"line_number":426,"context_line":"            elif CONF.glance.enable_certificate_validation:"},{"line_number":427,"context_line":"                if CONF.glance.default_trusted_certificate_ids:"},{"line_number":428,"context_line":"                    trusted_certs \u003d objects.TrustedCerts("},{"line_number":429,"context_line":"                        ids\u003dCONF.glance.default_trusted_certificate_ids)"},{"line_number":430,"context_line":"                    _verify_certs(context, img_sig_cert_uuid, trusted_certs)"},{"line_number":431,"context_line":"                else:"},{"line_number":432,"context_line":"                    msg \u003d (\u0027Image signature certificate validation enabled, \u0027"},{"line_number":433,"context_line":"                           \u0027but no trusted certificate IDs were provided. \u0027"}],"source_content_type":"text/x-python","patch_set":77,"id":"5f7c97a3_2cb9deca","line":430,"range":{"start_line":428,"start_character":20,"end_line":430,"end_character":76},"updated":"2018-06-12 18:17:06.000000000","message":"We don\u0027t have a test for this scenario.","commit_id":"016e987d57b78a0eb2d1d7b16510dcc384e117bf"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"79b64462431d65efdbb1d59e5d428c8efc7e806f","unresolved":false,"context_lines":[{"line_number":5457,"context_line":"        self._test_build_and_run_spawn_exceptions("},{"line_number":5458,"context_line":"            cursive_exception.SignatureVerificationError(reason\u003d\"\"))"},{"line_number":5459,"context_line":""},{"line_number":5460,"context_line":"    def test_build_and_run_certificate_validation_error(self):"},{"line_number":5461,"context_line":"        self._test_build_and_run_spawn_exceptions("},{"line_number":5462,"context_line":"            exception.CertificateValidationFailed(cert_uuid\u003d\u0027trusted-cert-id\u0027,"},{"line_number":5463,"context_line":"                                                  reason\u003d\"\"))"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_cc9e2a53","line":5460,"updated":"2018-06-12 18:17:06.000000000","message":"Done: https://review.openstack.org/#/c/561262/26/nova/tests/unit/compute/test_compute_mgr.py@5503","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"}],"nova/tests/unit/image/test_glance.py":[{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"3cd63fb704fe1698be8b46314b86e29929afffa2","unresolved":false,"context_lines":[{"line_number":961,"context_line":"            )"},{"line_number":962,"context_line":""},{"line_number":963,"context_line":"    def setUp(self):"},{"line_number":964,"context_line":"        super(TestDownloadSignatureVerification, self).setUp()"},{"line_number":965,"context_line":"        self.flags(verify_glance_signatures\u003dTrue, group\u003d\u0027glance\u0027)"},{"line_number":966,"context_line":"        self.flags(enable_certificate_validation\u003dTrue, group\u003d\u0027glance\u0027)"},{"line_number":967,"context_line":"        self.fake_img_props \u003d {"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f4e5783_c82aa22a","line":964,"range":{"start_line":964,"start_character":14,"end_line":964,"end_character":47},"updated":"2017-10-13 21:26:04.000000000","message":"Should this be \"TestDownloadCertificateValidation\" instead?","commit_id":"ec7b392cf30ef7fdd2ca713797bb40e421a53d21"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"6529634f220e80ded34bfe8148489ade3cad01ac","unresolved":false,"context_lines":[{"line_number":961,"context_line":"            )"},{"line_number":962,"context_line":""},{"line_number":963,"context_line":"    def setUp(self):"},{"line_number":964,"context_line":"        super(TestDownloadSignatureVerification, self).setUp()"},{"line_number":965,"context_line":"        self.flags(verify_glance_signatures\u003dTrue, group\u003d\u0027glance\u0027)"},{"line_number":966,"context_line":"        self.flags(enable_certificate_validation\u003dTrue, group\u003d\u0027glance\u0027)"},{"line_number":967,"context_line":"        self.fake_img_props \u003d {"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f4e5783_25f15302","line":964,"range":{"start_line":964,"start_character":14,"end_line":964,"end_character":47},"in_reply_to":"5f4e5783_c82aa22a","updated":"2017-10-17 15:16:25.000000000","message":"Good catch. Will fix in next patch update.","commit_id":"ec7b392cf30ef7fdd2ca713797bb40e421a53d21"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3869cb1c630cc742f1fe906cf4ecbf7172a0e9ac","unresolved":false,"context_lines":[{"line_number":971,"context_line":"                                                     mock_show,"},{"line_number":972,"context_line":"                                                     mock_log):"},{"line_number":973,"context_line":"        service \u003d glance.GlanceImageServiceV2(self.client)"},{"line_number":974,"context_line":"        mock_get_verifier.return_value \u003d self.MockVerifier()"},{"line_number":975,"context_line":"        mock_verify_certificate.return_value \u003d self.MockVerifyCertificate()"},{"line_number":976,"context_line":"        mock_show.return_value \u003d self.fake_img_props"},{"line_number":977,"context_line":"        fake_cert \u003d uuids.img_sig_cert_uuid"}],"source_content_type":"text/x-python","patch_set":24,"id":"7f96bb07_421c8011","line":974,"range":{"start_line":974,"start_character":41,"end_line":974,"end_character":46},"updated":"2018-01-15 20:13:55.000000000","message":"don\u0027t need the \"self.\" here... MockVerifier is already in local scope and will be found.","commit_id":"053b68784b82d4c787b5e45825c85a241634e316"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"71e75c34cedbe268700bad0db0414330dfbe1202","unresolved":false,"context_lines":[{"line_number":971,"context_line":"                                                     mock_show,"},{"line_number":972,"context_line":"                                                     mock_log):"},{"line_number":973,"context_line":"        service \u003d glance.GlanceImageServiceV2(self.client)"},{"line_number":974,"context_line":"        mock_get_verifier.return_value \u003d self.MockVerifier()"},{"line_number":975,"context_line":"        mock_verify_certificate.return_value \u003d self.MockVerifyCertificate()"},{"line_number":976,"context_line":"        mock_show.return_value \u003d self.fake_img_props"},{"line_number":977,"context_line":"        fake_cert \u003d uuids.img_sig_cert_uuid"}],"source_content_type":"text/x-python","patch_set":24,"id":"7f96bb07_573e5d8c","line":974,"range":{"start_line":974,"start_character":41,"end_line":974,"end_character":46},"in_reply_to":"7f96bb07_23acd332","updated":"2018-01-16 17:25:03.000000000","message":"ok, meh then, please ignore me :) It\u0027s definitely shorter than saying TestDownloadCertificateValidation.MockVerifier() ;)","commit_id":"053b68784b82d4c787b5e45825c85a241634e316"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"92b17e9fea9e11ef1f23e896d01ae561e45fa91d","unresolved":false,"context_lines":[{"line_number":971,"context_line":"                                                     mock_show,"},{"line_number":972,"context_line":"                                                     mock_log):"},{"line_number":973,"context_line":"        service \u003d glance.GlanceImageServiceV2(self.client)"},{"line_number":974,"context_line":"        mock_get_verifier.return_value \u003d self.MockVerifier()"},{"line_number":975,"context_line":"        mock_verify_certificate.return_value \u003d self.MockVerifyCertificate()"},{"line_number":976,"context_line":"        mock_show.return_value \u003d self.fake_img_props"},{"line_number":977,"context_line":"        fake_cert \u003d uuids.img_sig_cert_uuid"}],"source_content_type":"text/x-python","patch_set":24,"id":"7f96bb07_23acd332","line":974,"range":{"start_line":974,"start_character":41,"end_line":974,"end_character":46},"in_reply_to":"7f96bb07_421c8011","updated":"2018-01-16 14:51:25.000000000","message":"Hmm are you sure? I get flake8 errors when I run `tox -e pep8` without the \"self.\"","commit_id":"053b68784b82d4c787b5e45825c85a241634e316"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3869cb1c630cc742f1fe906cf4ecbf7172a0e9ac","unresolved":false,"context_lines":[{"line_number":972,"context_line":"                                                     mock_log):"},{"line_number":973,"context_line":"        service \u003d glance.GlanceImageServiceV2(self.client)"},{"line_number":974,"context_line":"        mock_get_verifier.return_value \u003d self.MockVerifier()"},{"line_number":975,"context_line":"        mock_verify_certificate.return_value \u003d self.MockVerifyCertificate()"},{"line_number":976,"context_line":"        mock_show.return_value \u003d self.fake_img_props"},{"line_number":977,"context_line":"        fake_cert \u003d uuids.img_sig_cert_uuid"},{"line_number":978,"context_line":"        fake_trusted_certs \u003d objects.TrustedCerts(ids\u003d[fake_cert])"}],"source_content_type":"text/x-python","patch_set":24,"id":"7f96bb07_02d4381f","line":975,"range":{"start_line":975,"start_character":47,"end_line":975,"end_character":52},"updated":"2018-01-15 20:13:55.000000000","message":"ditto","commit_id":"053b68784b82d4c787b5e45825c85a241634e316"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"525d231ba6f1bd4b88bdd625fc24a430464c5b80","unresolved":false,"context_lines":[{"line_number":840,"context_line":"            img_signature\u003d\u0027signature\u0027,"},{"line_number":841,"context_line":"            img_signature_key_type\u003d\u0027RSA-PSS\u0027"},{"line_number":842,"context_line":"        )"},{"line_number":843,"context_line":"        mock_log.warning.assert_called_once_with(mock.ANY)"},{"line_number":844,"context_line":""},{"line_number":845,"context_line":"    @mock.patch.object(six.moves.builtins, \u0027open\u0027)"},{"line_number":846,"context_line":"    @mock.patch(\u0027nova.image.glance.LOG\u0027)"}],"source_content_type":"text/x-python","patch_set":37,"id":"df7087c5_8f68c5ba","line":843,"updated":"2018-03-20 12:18:29.000000000","message":"I didn\u0027t see you remove an info message, why do we only check for warning now? I sort of expected to see warning and info checked, if we are expecting an additional log message now.","commit_id":"e7c874a33fa8a2069084a81d2ef2712005547989"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"6104ffb8c153bbeae4591f443c511489a5082b81","unresolved":false,"context_lines":[{"line_number":840,"context_line":"            img_signature\u003d\u0027signature\u0027,"},{"line_number":841,"context_line":"            img_signature_key_type\u003d\u0027RSA-PSS\u0027"},{"line_number":842,"context_line":"        )"},{"line_number":843,"context_line":"        mock_log.warning.assert_called_once_with(mock.ANY)"},{"line_number":844,"context_line":""},{"line_number":845,"context_line":"    @mock.patch.object(six.moves.builtins, \u0027open\u0027)"},{"line_number":846,"context_line":"    @mock.patch(\u0027nova.image.glance.LOG\u0027)"}],"source_content_type":"text/x-python","patch_set":37,"id":"df7087c5_2d43a0b1","line":843,"in_reply_to":"df7087c5_8f68c5ba","updated":"2018-03-20 18:23:26.000000000","message":"This test tests signature verification with certificate validation. In other words, this test will not pass a list of trusted image certificates to download() (e.g., trusted_certs\u003dNone). This results in signature verification still occurring, but a warning being displayed that \"Image signature verification succeeded, but certificate validation was not performed...\"","commit_id":"e7c874a33fa8a2069084a81d2ef2712005547989"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"525d231ba6f1bd4b88bdd625fc24a430464c5b80","unresolved":false,"context_lines":[{"line_number":868,"context_line":"            img_signature\u003d\u0027signature\u0027,"},{"line_number":869,"context_line":"            img_signature_key_type\u003d\u0027RSA-PSS\u0027"},{"line_number":870,"context_line":"        )"},{"line_number":871,"context_line":"        mock_log.warning.assert_called_once_with(mock.ANY)"},{"line_number":872,"context_line":"        self.assertEqual(len(self.fake_img_data), mock_dest.write.call_count)"},{"line_number":873,"context_line":"        self.assertTrue(mock_dest.close.called)"},{"line_number":874,"context_line":"        mock_fsync.assert_called_once_with(mock_dest)"}],"source_content_type":"text/x-python","patch_set":37,"id":"df7087c5_af6589e3","line":871,"updated":"2018-03-20 12:18:29.000000000","message":"as above, this seems confusing to me?","commit_id":"e7c874a33fa8a2069084a81d2ef2712005547989"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6e73166285929230b75147d1f79ee0280f1c09d3","unresolved":false,"context_lines":[{"line_number":842,"context_line":"            img_signature\u003d\u0027signature\u0027,"},{"line_number":843,"context_line":"            img_signature_key_type\u003d\u0027RSA-PSS\u0027"},{"line_number":844,"context_line":"        )"},{"line_number":845,"context_line":"        mock_log.debug.assert_called_once_with(mock.ANY)"},{"line_number":846,"context_line":""},{"line_number":847,"context_line":"    @mock.patch.object(six.moves.builtins, \u0027open\u0027)"},{"line_number":848,"context_line":"    @mock.patch(\u0027nova.image.glance.LOG\u0027)"}],"source_content_type":"text/x-python","patch_set":57,"id":"bf659307_d57a1286","line":845,"updated":"2018-04-13 20:45:47.000000000","message":"I think you needed to add this debug statement and not delete the info log assert, because the info log assert was for the signature verification phase.","commit_id":"783a0e8201d5f05819bd8910fe66519d44b003cf"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6e73166285929230b75147d1f79ee0280f1c09d3","unresolved":false,"context_lines":[{"line_number":870,"context_line":"            img_signature\u003d\u0027signature\u0027,"},{"line_number":871,"context_line":"            img_signature_key_type\u003d\u0027RSA-PSS\u0027"},{"line_number":872,"context_line":"        )"},{"line_number":873,"context_line":"        mock_log.debug.assert_called_once_with(mock.ANY)"},{"line_number":874,"context_line":"        self.assertEqual(len(self.fake_img_data), mock_dest.write.call_count)"},{"line_number":875,"context_line":"        self.assertTrue(mock_dest.close.called)"},{"line_number":876,"context_line":"        mock_fsync.assert_called_once_with(mock_dest)"}],"source_content_type":"text/x-python","patch_set":57,"id":"bf659307_756d46ca","line":873,"updated":"2018-04-13 20:45:47.000000000","message":"Same.","commit_id":"783a0e8201d5f05819bd8910fe66519d44b003cf"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"561b5b3793b166668e4ced16d8be3a32e92f32a9","unresolved":false,"context_lines":[{"line_number":842,"context_line":"            img_signature\u003d\u0027signature\u0027,"},{"line_number":843,"context_line":"            img_signature_key_type\u003d\u0027RSA-PSS\u0027"},{"line_number":844,"context_line":"        )"},{"line_number":845,"context_line":"        mock_log.debug.assert_called_once_with(mock.ANY)"},{"line_number":846,"context_line":""},{"line_number":847,"context_line":"    @mock.patch.object(six.moves.builtins, \u0027open\u0027)"},{"line_number":848,"context_line":"    @mock.patch(\u0027nova.image.glance.LOG\u0027)"}],"source_content_type":"text/x-python","patch_set":62,"id":"9f6a8fd7_43e37c06","line":845,"updated":"2018-04-16 15:11:06.000000000","message":"The assert for mock_log.info shouldn\u0027t have been removed -- it\u0027s for the image verification step.","commit_id":"0fb58969f1fb90663455dd3c5d3d5e1f292d844b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"561b5b3793b166668e4ced16d8be3a32e92f32a9","unresolved":false,"context_lines":[{"line_number":870,"context_line":"            img_signature\u003d\u0027signature\u0027,"},{"line_number":871,"context_line":"            img_signature_key_type\u003d\u0027RSA-PSS\u0027"},{"line_number":872,"context_line":"        )"},{"line_number":873,"context_line":"        mock_log.debug.assert_called_once_with(mock.ANY)"},{"line_number":874,"context_line":"        self.assertEqual(len(self.fake_img_data), mock_dest.write.call_count)"},{"line_number":875,"context_line":"        self.assertTrue(mock_dest.close.called)"},{"line_number":876,"context_line":"        mock_fsync.assert_called_once_with(mock_dest)"}],"source_content_type":"text/x-python","patch_set":62,"id":"9f6a8fd7_03ed84ee","line":873,"updated":"2018-04-16 15:11:06.000000000","message":"Same.","commit_id":"0fb58969f1fb90663455dd3c5d3d5e1f292d844b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d3f550ba84a5e1bd9a7f2e0e7ebd5a02bc19ff72","unresolved":false,"context_lines":[{"line_number":969,"context_line":"            return"},{"line_number":970,"context_line":""},{"line_number":971,"context_line":"        def verify(self):"},{"line_number":972,"context_line":"            return"},{"line_number":973,"context_line":""},{"line_number":974,"context_line":"    def setUp(self):"},{"line_number":975,"context_line":"        super(TestDownloadCertificateValidation, self).setUp()"}],"source_content_type":"text/x-python","patch_set":72,"id":"5f7c97a3_4110c100","line":972,"updated":"2018-05-29 16:16:20.000000000","message":"little bit odd that the MockVerifier returns True for the verify() method but the MockVerifyCertificate returns None for the verify() method...","commit_id":"d8e292f9cd4d52efff5692ad600a02f21b7af65b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d3f550ba84a5e1bd9a7f2e0e7ebd5a02bc19ff72","unresolved":false,"context_lines":[{"line_number":1099,"context_line":"                          service.download,"},{"line_number":1100,"context_line":"                          context\u003dNone, image_id\u003dNone,"},{"line_number":1101,"context_line":"                          data\u003dNone, dst_path\u003dNone)"},{"line_number":1102,"context_line":"        mock_log.warning.assert_called_once_with(mock.ANY)"},{"line_number":1103,"context_line":""},{"line_number":1104,"context_line":""},{"line_number":1105,"context_line":"class TestIsImageAvailable(test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":72,"id":"5f7c97a3_21d90555","line":1102,"updated":"2018-05-29 16:16:20.000000000","message":"good work on these unit tests. not sure whether the DEBUG level log mocks were necessary, but perhaps a previous reviewer asked for those. In any case, nice work.","commit_id":"d8e292f9cd4d52efff5692ad600a02f21b7af65b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6288aa9bc6883f21267f6a1c4e4fd107d61ac703","unresolved":false,"context_lines":[{"line_number":842,"context_line":"            img_signature\u003d\u0027signature\u0027,"},{"line_number":843,"context_line":"            img_signature_key_type\u003d\u0027RSA-PSS\u0027"},{"line_number":844,"context_line":"        )"},{"line_number":845,"context_line":"        mock_log.debug.assert_called_once_with(mock.ANY)"},{"line_number":846,"context_line":"        mock_log.info.assert_called_once_with(mock.ANY, mock.ANY)"},{"line_number":847,"context_line":""},{"line_number":848,"context_line":"    @mock.patch.object(six.moves.builtins, \u0027open\u0027)"}],"source_content_type":"text/x-python","patch_set":75,"id":"5f7c97a3_b247ada3","line":845,"updated":"2018-06-11 15:51:41.000000000","message":"nit: I think this assert has a really small value.","commit_id":"91b03fcca14ce1f5ff33cb013facf92b5c7fd83b"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"cb74e67ef2dd129165b458efe1a8398986141b81","unresolved":false,"context_lines":[{"line_number":842,"context_line":"            img_signature\u003d\u0027signature\u0027,"},{"line_number":843,"context_line":"            img_signature_key_type\u003d\u0027RSA-PSS\u0027"},{"line_number":844,"context_line":"        )"},{"line_number":845,"context_line":"        mock_log.debug.assert_called_once_with(mock.ANY)"},{"line_number":846,"context_line":"        mock_log.info.assert_called_once_with(mock.ANY, mock.ANY)"},{"line_number":847,"context_line":""},{"line_number":848,"context_line":"    @mock.patch.object(six.moves.builtins, \u0027open\u0027)"}],"source_content_type":"text/x-python","patch_set":75,"id":"5f7c97a3_2657db5c","line":845,"in_reply_to":"5f7c97a3_b247ada3","updated":"2018-06-11 18:13:33.000000000","message":"Agreed.  I\u0027ve added a check for what was logged.","commit_id":"91b03fcca14ce1f5ff33cb013facf92b5c7fd83b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6288aa9bc6883f21267f6a1c4e4fd107d61ac703","unresolved":false,"context_lines":[{"line_number":957,"context_line":"    certificate validation is enabled."},{"line_number":958,"context_line":"    \"\"\""},{"line_number":959,"context_line":""},{"line_number":960,"context_line":"    class MockVerifier(object):"},{"line_number":961,"context_line":"        def update(self, data):"},{"line_number":962,"context_line":"            return"},{"line_number":963,"context_line":""},{"line_number":964,"context_line":"        def verify(self):"},{"line_number":965,"context_line":"            return True"},{"line_number":966,"context_line":""},{"line_number":967,"context_line":"    class MockVerifyCertificate(object):"},{"line_number":968,"context_line":"        def update(self, data):"},{"line_number":969,"context_line":"            return"},{"line_number":970,"context_line":""},{"line_number":971,"context_line":"        def verify(self):"},{"line_number":972,"context_line":"            return"},{"line_number":973,"context_line":""},{"line_number":974,"context_line":"    def setUp(self):"},{"line_number":975,"context_line":"        super(TestDownloadCertificateValidation, self).setUp()"}],"source_content_type":"text/x-python","patch_set":75,"id":"5f7c97a3_35570704","line":972,"range":{"start_line":960,"start_character":0,"end_line":972,"end_character":18},"updated":"2018-06-11 15:51:41.000000000","message":"You can drop these an all usage of them from the test, the default Mock behavior exactly what you written here.","commit_id":"91b03fcca14ce1f5ff33cb013facf92b5c7fd83b"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"cb74e67ef2dd129165b458efe1a8398986141b81","unresolved":false,"context_lines":[{"line_number":957,"context_line":"    certificate validation is enabled."},{"line_number":958,"context_line":"    \"\"\""},{"line_number":959,"context_line":""},{"line_number":960,"context_line":"    class MockVerifier(object):"},{"line_number":961,"context_line":"        def update(self, data):"},{"line_number":962,"context_line":"            return"},{"line_number":963,"context_line":""},{"line_number":964,"context_line":"        def verify(self):"},{"line_number":965,"context_line":"            return True"},{"line_number":966,"context_line":""},{"line_number":967,"context_line":"    class MockVerifyCertificate(object):"},{"line_number":968,"context_line":"        def update(self, data):"},{"line_number":969,"context_line":"            return"},{"line_number":970,"context_line":""},{"line_number":971,"context_line":"        def verify(self):"},{"line_number":972,"context_line":"            return"},{"line_number":973,"context_line":""},{"line_number":974,"context_line":"    def setUp(self):"},{"line_number":975,"context_line":"        super(TestDownloadCertificateValidation, self).setUp()"}],"source_content_type":"text/x-python","patch_set":75,"id":"5f7c97a3_069f3f50","line":972,"range":{"start_line":960,"start_character":0,"end_line":972,"end_character":18},"in_reply_to":"5f7c97a3_35570704","updated":"2018-06-11 18:13:33.000000000","message":"Done","commit_id":"91b03fcca14ce1f5ff33cb013facf92b5c7fd83b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6288aa9bc6883f21267f6a1c4e4fd107d61ac703","unresolved":false,"context_lines":[{"line_number":997,"context_line":"                                                     mock_show,"},{"line_number":998,"context_line":"                                                     mock_log):"},{"line_number":999,"context_line":"        service \u003d glance.GlanceImageServiceV2(self.client)"},{"line_number":1000,"context_line":"        mock_get_verifier.return_value \u003d self.MockVerifier()"},{"line_number":1001,"context_line":"        mock_verify_certificate.return_value \u003d self.MockVerifyCertificate()"},{"line_number":1002,"context_line":"        mock_show.return_value \u003d self.fake_img_props"},{"line_number":1003,"context_line":"        fake_cert \u003d uuids.img_sig_cert_uuid"},{"line_number":1004,"context_line":"        fake_trusted_certs \u003d objects.TrustedCerts(ids\u003d[fake_cert])"}],"source_content_type":"text/x-python","patch_set":75,"id":"5f7c97a3_156bebb8","line":1001,"range":{"start_line":1000,"start_character":0,"end_line":1001,"end_character":75},"updated":"2018-06-11 15:51:41.000000000","message":"you can drop these lines","commit_id":"91b03fcca14ce1f5ff33cb013facf92b5c7fd83b"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"cb74e67ef2dd129165b458efe1a8398986141b81","unresolved":false,"context_lines":[{"line_number":997,"context_line":"                                                     mock_show,"},{"line_number":998,"context_line":"                                                     mock_log):"},{"line_number":999,"context_line":"        service \u003d glance.GlanceImageServiceV2(self.client)"},{"line_number":1000,"context_line":"        mock_get_verifier.return_value \u003d self.MockVerifier()"},{"line_number":1001,"context_line":"        mock_verify_certificate.return_value \u003d self.MockVerifyCertificate()"},{"line_number":1002,"context_line":"        mock_show.return_value \u003d self.fake_img_props"},{"line_number":1003,"context_line":"        fake_cert \u003d uuids.img_sig_cert_uuid"},{"line_number":1004,"context_line":"        fake_trusted_certs \u003d objects.TrustedCerts(ids\u003d[fake_cert])"}],"source_content_type":"text/x-python","patch_set":75,"id":"5f7c97a3_a6a76ba8","line":1001,"range":{"start_line":1000,"start_character":0,"end_line":1001,"end_character":75},"in_reply_to":"5f7c97a3_156bebb8","updated":"2018-06-11 18:13:33.000000000","message":"Done","commit_id":"91b03fcca14ce1f5ff33cb013facf92b5c7fd83b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6288aa9bc6883f21267f6a1c4e4fd107d61ac703","unresolved":false,"context_lines":[{"line_number":1032,"context_line":"                                                mock_log):"},{"line_number":1033,"context_line":"        self.flags(enable_certificate_validation\u003dFalse, group\u003d\u0027glance\u0027)"},{"line_number":1034,"context_line":"        service \u003d glance.GlanceImageServiceV2(self.client)"},{"line_number":1035,"context_line":"        mock_get_verifier.return_value \u003d self.MockVerifier()"},{"line_number":1036,"context_line":"        mock_verify_certificate.return_value \u003d self.MockVerifyCertificate()"},{"line_number":1037,"context_line":"        mock_show.return_value \u003d self.fake_img_props"},{"line_number":1038,"context_line":"        fake_cert \u003d uuids.img_sig_cert_uuid"},{"line_number":1039,"context_line":"        fake_trusted_certs \u003d objects.TrustedCerts(ids\u003d[fake_cert])"}],"source_content_type":"text/x-python","patch_set":75,"id":"5f7c97a3_d56813bb","line":1036,"range":{"start_line":1035,"start_character":0,"end_line":1036,"end_character":75},"updated":"2018-06-11 15:51:41.000000000","message":"ditto","commit_id":"91b03fcca14ce1f5ff33cb013facf92b5c7fd83b"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"cb74e67ef2dd129165b458efe1a8398986141b81","unresolved":false,"context_lines":[{"line_number":1032,"context_line":"                                                mock_log):"},{"line_number":1033,"context_line":"        self.flags(enable_certificate_validation\u003dFalse, group\u003d\u0027glance\u0027)"},{"line_number":1034,"context_line":"        service \u003d glance.GlanceImageServiceV2(self.client)"},{"line_number":1035,"context_line":"        mock_get_verifier.return_value \u003d self.MockVerifier()"},{"line_number":1036,"context_line":"        mock_verify_certificate.return_value \u003d self.MockVerifyCertificate()"},{"line_number":1037,"context_line":"        mock_show.return_value \u003d self.fake_img_props"},{"line_number":1038,"context_line":"        fake_cert \u003d uuids.img_sig_cert_uuid"},{"line_number":1039,"context_line":"        fake_trusted_certs \u003d objects.TrustedCerts(ids\u003d[fake_cert])"}],"source_content_type":"text/x-python","patch_set":75,"id":"5f7c97a3_c6a4e7a5","line":1036,"range":{"start_line":1035,"start_character":0,"end_line":1036,"end_character":75},"in_reply_to":"5f7c97a3_d56813bb","updated":"2018-06-11 18:13:33.000000000","message":"Done","commit_id":"91b03fcca14ce1f5ff33cb013facf92b5c7fd83b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6288aa9bc6883f21267f6a1c4e4fd107d61ac703","unresolved":false,"context_lines":[{"line_number":1066,"context_line":"                                                mock_show,"},{"line_number":1067,"context_line":"                                                mock_log):"},{"line_number":1068,"context_line":"        service \u003d glance.GlanceImageServiceV2(self.client)"},{"line_number":1069,"context_line":"        mock_get_verifier.return_value \u003d self.MockVerifier()"},{"line_number":1070,"context_line":"        mock_verify_certificate.side_effect \u003d \\"},{"line_number":1071,"context_line":"            cursive_exception.SignatureVerificationError("},{"line_number":1072,"context_line":"                reason\u003d\u0027Invalid certificate.\u0027"}],"source_content_type":"text/x-python","patch_set":75,"id":"5f7c97a3_955e1b11","line":1069,"range":{"start_line":1069,"start_character":0,"end_line":1069,"end_character":60},"updated":"2018-06-11 15:51:41.000000000","message":"ditto","commit_id":"91b03fcca14ce1f5ff33cb013facf92b5c7fd83b"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"cb74e67ef2dd129165b458efe1a8398986141b81","unresolved":false,"context_lines":[{"line_number":1066,"context_line":"                                                mock_show,"},{"line_number":1067,"context_line":"                                                mock_log):"},{"line_number":1068,"context_line":"        service \u003d glance.GlanceImageServiceV2(self.client)"},{"line_number":1069,"context_line":"        mock_get_verifier.return_value \u003d self.MockVerifier()"},{"line_number":1070,"context_line":"        mock_verify_certificate.side_effect \u003d \\"},{"line_number":1071,"context_line":"            cursive_exception.SignatureVerificationError("},{"line_number":1072,"context_line":"                reason\u003d\u0027Invalid certificate.\u0027"}],"source_content_type":"text/x-python","patch_set":75,"id":"5f7c97a3_66b713da","line":1069,"range":{"start_line":1069,"start_character":0,"end_line":1069,"end_character":60},"in_reply_to":"5f7c97a3_955e1b11","updated":"2018-06-11 18:13:33.000000000","message":"Done","commit_id":"91b03fcca14ce1f5ff33cb013facf92b5c7fd83b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6288aa9bc6883f21267f6a1c4e4fd107d61ac703","unresolved":false,"context_lines":[{"line_number":1093,"context_line":"        # validation will be skipped."},{"line_number":1094,"context_line":"        self.flags(verify_glance_signatures\u003dTrue, group\u003d\u0027glance\u0027)"},{"line_number":1095,"context_line":"        service \u003d glance.GlanceImageServiceV2(self.client)"},{"line_number":1096,"context_line":"        mock_get_verifier.return_value \u003d self.MockVerifier()"},{"line_number":1097,"context_line":"        mock_show.return_value \u003d self.fake_img_props"},{"line_number":1098,"context_line":"        self.assertRaises(exception.CertificateValidationFailed,"},{"line_number":1099,"context_line":"                          service.download,"}],"source_content_type":"text/x-python","patch_set":75,"id":"5f7c97a3_b55b1720","line":1096,"range":{"start_line":1096,"start_character":0,"end_line":1096,"end_character":60},"updated":"2018-06-11 15:51:41.000000000","message":"ditto","commit_id":"91b03fcca14ce1f5ff33cb013facf92b5c7fd83b"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"cb74e67ef2dd129165b458efe1a8398986141b81","unresolved":false,"context_lines":[{"line_number":1093,"context_line":"        # validation will be skipped."},{"line_number":1094,"context_line":"        self.flags(verify_glance_signatures\u003dTrue, group\u003d\u0027glance\u0027)"},{"line_number":1095,"context_line":"        service \u003d glance.GlanceImageServiceV2(self.client)"},{"line_number":1096,"context_line":"        mock_get_verifier.return_value \u003d self.MockVerifier()"},{"line_number":1097,"context_line":"        mock_show.return_value \u003d self.fake_img_props"},{"line_number":1098,"context_line":"        self.assertRaises(exception.CertificateValidationFailed,"},{"line_number":1099,"context_line":"                          service.download,"}],"source_content_type":"text/x-python","patch_set":75,"id":"5f7c97a3_86bcafbb","line":1096,"range":{"start_line":1096,"start_character":0,"end_line":1096,"end_character":60},"in_reply_to":"5f7c97a3_b55b1720","updated":"2018-06-11 18:13:33.000000000","message":"Done","commit_id":"91b03fcca14ce1f5ff33cb013facf92b5c7fd83b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6288aa9bc6883f21267f6a1c4e4fd107d61ac703","unresolved":false,"context_lines":[{"line_number":1099,"context_line":"                          service.download,"},{"line_number":1100,"context_line":"                          context\u003dNone, image_id\u003dNone,"},{"line_number":1101,"context_line":"                          data\u003dNone, dst_path\u003dNone)"},{"line_number":1102,"context_line":"        mock_log.warning.assert_called_once_with(mock.ANY)"},{"line_number":1103,"context_line":""},{"line_number":1104,"context_line":""},{"line_number":1105,"context_line":"class TestIsImageAvailable(test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":75,"id":"5f7c97a3_1579ab80","line":1102,"updated":"2018-06-11 15:51:41.000000000","message":"nit: I don\u0027t feel having an assert on logging with mock.ANY matcher give us too much value. Either I would check what is logged or didn\u0027t include these asserts.","commit_id":"91b03fcca14ce1f5ff33cb013facf92b5c7fd83b"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"cb74e67ef2dd129165b458efe1a8398986141b81","unresolved":false,"context_lines":[{"line_number":1099,"context_line":"                          service.download,"},{"line_number":1100,"context_line":"                          context\u003dNone, image_id\u003dNone,"},{"line_number":1101,"context_line":"                          data\u003dNone, dst_path\u003dNone)"},{"line_number":1102,"context_line":"        mock_log.warning.assert_called_once_with(mock.ANY)"},{"line_number":1103,"context_line":""},{"line_number":1104,"context_line":""},{"line_number":1105,"context_line":"class TestIsImageAvailable(test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":75,"id":"5f7c97a3_c66e272b","line":1102,"in_reply_to":"5f7c97a3_1579ab80","updated":"2018-06-11 18:13:33.000000000","message":"Agreed.  I have added checks for what is logged.","commit_id":"91b03fcca14ce1f5ff33cb013facf92b5c7fd83b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bab5dd76f25174b70cd811446f3d246024ae3290","unresolved":false,"context_lines":[{"line_number":842,"context_line":"            img_signature\u003d\u0027signature\u0027,"},{"line_number":843,"context_line":"            img_signature_key_type\u003d\u0027RSA-PSS\u0027"},{"line_number":844,"context_line":"        )"},{"line_number":845,"context_line":"        msg \u003d (\u0027Certificate validation was not performed. A list of \u0027"},{"line_number":846,"context_line":"               \u0027trusted image certificate IDs must be provided in \u0027"},{"line_number":847,"context_line":"               \u0027order to validate an image certificate.\u0027)"},{"line_number":848,"context_line":"        mock_log.debug.assert_called_once_with(msg)"},{"line_number":849,"context_line":"        msg \u003d (\u0027Image signature verification succeeded for image: %s\u0027)"},{"line_number":850,"context_line":"        mock_log.info.assert_called_once_with(msg, None)"},{"line_number":851,"context_line":""}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_73c1004e","line":848,"range":{"start_line":845,"start_character":8,"end_line":848,"end_character":51},"updated":"2018-06-12 13:28:28.000000000","message":"So we get this because trusted_certs is None and enable_certificate_validation is False - would be nice to have a comment for that in the test.","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"ad3ec32143815af952ba2b4c38273c4ddceddcd2","unresolved":false,"context_lines":[{"line_number":842,"context_line":"            img_signature\u003d\u0027signature\u0027,"},{"line_number":843,"context_line":"            img_signature_key_type\u003d\u0027RSA-PSS\u0027"},{"line_number":844,"context_line":"        )"},{"line_number":845,"context_line":"        msg \u003d (\u0027Certificate validation was not performed. A list of \u0027"},{"line_number":846,"context_line":"               \u0027trusted image certificate IDs must be provided in \u0027"},{"line_number":847,"context_line":"               \u0027order to validate an image certificate.\u0027)"},{"line_number":848,"context_line":"        mock_log.debug.assert_called_once_with(msg)"},{"line_number":849,"context_line":"        msg \u003d (\u0027Image signature verification succeeded for image: %s\u0027)"},{"line_number":850,"context_line":"        mock_log.info.assert_called_once_with(msg, None)"},{"line_number":851,"context_line":""}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_07008265","line":848,"range":{"start_line":845,"start_character":8,"end_line":848,"end_character":51},"in_reply_to":"5f7c97a3_73c1004e","updated":"2018-06-12 16:11:10.000000000","message":"Done","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e544b9330f4f8929b3e0b80b7fa66e402eb7cb8f","unresolved":false,"context_lines":[{"line_number":847,"context_line":"               \u0027order to validate an image certificate.\u0027)"},{"line_number":848,"context_line":"        mock_log.debug.assert_called_once_with(msg)"},{"line_number":849,"context_line":"        msg \u003d (\u0027Image signature verification succeeded for image: %s\u0027)"},{"line_number":850,"context_line":"        mock_log.info.assert_called_once_with(msg, None)"},{"line_number":851,"context_line":""},{"line_number":852,"context_line":"    @mock.patch.object(six.moves.builtins, \u0027open\u0027)"},{"line_number":853,"context_line":"    @mock.patch(\u0027nova.image.glance.LOG\u0027)"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_b4e1c7d2","line":850,"range":{"start_line":850,"start_character":51,"end_line":850,"end_character":55},"updated":"2018-06-12 09:13:12.000000000","message":"nit: this looks weird but it is just the image_id that is set to None at L835","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"},{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"ad3ec32143815af952ba2b4c38273c4ddceddcd2","unresolved":false,"context_lines":[{"line_number":847,"context_line":"               \u0027order to validate an image certificate.\u0027)"},{"line_number":848,"context_line":"        mock_log.debug.assert_called_once_with(msg)"},{"line_number":849,"context_line":"        msg \u003d (\u0027Image signature verification succeeded for image: %s\u0027)"},{"line_number":850,"context_line":"        mock_log.info.assert_called_once_with(msg, None)"},{"line_number":851,"context_line":""},{"line_number":852,"context_line":"    @mock.patch.object(six.moves.builtins, \u0027open\u0027)"},{"line_number":853,"context_line":"    @mock.patch(\u0027nova.image.glance.LOG\u0027)"}],"source_content_type":"text/x-python","patch_set":76,"id":"5f7c97a3_a7faee71","line":850,"range":{"start_line":850,"start_character":51,"end_line":850,"end_character":55},"in_reply_to":"5f7c97a3_b4e1c7d2","updated":"2018-06-12 16:11:10.000000000","message":"Done","commit_id":"11f906d489c90cab168ce898af1bf8269d95be18"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3869cb1c630cc742f1fe906cf4ecbf7172a0e9ac","unresolved":false,"context_lines":[{"line_number":727,"context_line":""},{"line_number":728,"context_line":""},{"line_number":729,"context_line":"@ddt.ddt"},{"line_number":730,"context_line":"class LibvirtConnTestCase(test.TestCase,"},{"line_number":731,"context_line":"                          test_diagnostics.DiagnosticsComparisonMixin):"},{"line_number":732,"context_line":""},{"line_number":733,"context_line":"    REQUIRES_LOCKING \u003d True"}],"source_content_type":"text/x-python","patch_set":24,"id":"7f96bb07_8295a842","line":730,"range":{"start_line":730,"start_character":0,"end_line":730,"end_character":39},"updated":"2018-01-15 20:13:55.000000000","message":"Definitely big -1 on this... this adds a non-insignificant amount of runtime to a very large number of unit tests. There should be no reason to make this switch. You will want to mock out any DB accesses that the unit tests make.","commit_id":"053b68784b82d4c787b5e45825c85a241634e316"}],"nova/virt/images.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"525d231ba6f1bd4b88bdd625fc24a430464c5b80","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        raise exception.ImageUnacceptable(image_id\u003dsource, reason\u003dmsg)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"def fetch(context, image_href, path, **kwargs):"},{"line_number":145,"context_line":"    with fileutils.remove_path_on_error(path):"},{"line_number":146,"context_line":"        IMAGE_API.download(context, image_href, dest_path\u003dpath, **kwargs)"},{"line_number":147,"context_line":""}],"source_content_type":"text/x-python","patch_set":37,"id":"df7087c5_0fe83514","line":144,"updated":"2018-03-20 12:18:29.000000000","message":"Seems like we should have the real argument added here?","commit_id":"e7c874a33fa8a2069084a81d2ef2712005547989"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"6104ffb8c153bbeae4591f443c511489a5082b81","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        raise exception.ImageUnacceptable(image_id\u003dsource, reason\u003dmsg)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"def fetch(context, image_href, path, **kwargs):"},{"line_number":145,"context_line":"    with fileutils.remove_path_on_error(path):"},{"line_number":146,"context_line":"        IMAGE_API.download(context, image_href, dest_path\u003dpath, **kwargs)"},{"line_number":147,"context_line":""}],"source_content_type":"text/x-python","patch_set":37,"id":"df7087c5_6d675853","line":144,"in_reply_to":"df7087c5_0fe83514","updated":"2018-03-20 18:23:26.000000000","message":"Will be fixed in patch 39","commit_id":"e7c874a33fa8a2069084a81d2ef2712005547989"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d105936c586d305c795dc95df0c030e76c0a8a17","unresolved":false,"context_lines":[{"line_number":153,"context_line":""},{"line_number":154,"context_line":"def fetch_to_raw(context, image_href, path, trusted_certs\u003dNone):"},{"line_number":155,"context_line":"    path_tmp \u003d \"%s.part\" % path"},{"line_number":156,"context_line":"    fetch(context, image_href, path_tmp, trusted_certs\u003dtrusted_certs)"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    with fileutils.remove_path_on_error(path_tmp):"},{"line_number":159,"context_line":"        data \u003d qemu_img_info(path_tmp)"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_2c955633","line":156,"updated":"2018-04-03 09:11:20.000000000","message":"Nit: you don\u0027t need trusted_certs\u003dtrusted_certs here, and if you just passed the argument normally it would simplify a bunch of the unit tests. Personally I am going to block the patch on that though.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"f2c10d8e748d18f397c35b1089cc93555bbe6e24","unresolved":false,"context_lines":[{"line_number":153,"context_line":""},{"line_number":154,"context_line":"def fetch_to_raw(context, image_href, path, trusted_certs\u003dNone):"},{"line_number":155,"context_line":"    path_tmp \u003d \"%s.part\" % path"},{"line_number":156,"context_line":"    fetch(context, image_href, path_tmp, trusted_certs\u003dtrusted_certs)"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    with fileutils.remove_path_on_error(path_tmp):"},{"line_number":159,"context_line":"        data \u003d qemu_img_info(path_tmp)"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_ac62bfac","line":156,"in_reply_to":"bf659307_2c955633","updated":"2018-04-03 15:19:33.000000000","message":"Thanks, John. This will be fixed in the next patch.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":7012,"name":"Brianna Poulos","email":"Brianna.Poulos@jhuapl.edu","username":"brianna-poulos"},"change_message_id":"3cd63fb704fe1698be8b46314b86e29929afffa2","unresolved":false,"context_lines":[{"line_number":3216,"context_line":"            nova.privsep.path.chown(image(\u0027disk\u0027).path, uid\u003duid)"},{"line_number":3217,"context_line":""},{"line_number":3218,"context_line":"        trusted_certs \u003d None"},{"line_number":3219,"context_line":"        if instance.trusted_certs:"},{"line_number":3220,"context_line":"            trusted_certs \u003d instance.trusted_certs"},{"line_number":3221,"context_line":""},{"line_number":3222,"context_line":"        self._create_and_inject_local_root("},{"line_number":3223,"context_line":"            context, instance,"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f4e5783_689cce69","line":3220,"range":{"start_line":3219,"start_character":8,"end_line":3220,"end_character":50},"updated":"2017-10-13 21:26:04.000000000","message":"This is causing some of the tests to fail.  You might need to surround it with a try/catch block, or check to see if the attribute exists before trying to retrieve it.","commit_id":"ec7b392cf30ef7fdd2ca713797bb40e421a53d21"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3869cb1c630cc742f1fe906cf4ecbf7172a0e9ac","unresolved":false,"context_lines":[{"line_number":3238,"context_line":"            uid \u003d pwd.getpwnam(\u0027root\u0027).pw_uid"},{"line_number":3239,"context_line":"            nova.privsep.path.chown(image(\u0027disk\u0027).path, uid\u003duid)"},{"line_number":3240,"context_line":""},{"line_number":3241,"context_line":"        try:"},{"line_number":3242,"context_line":"            trusted_certs \u003d instance.trusted_certs"},{"line_number":3243,"context_line":"        except exception.NovaException:"},{"line_number":3244,"context_line":"            trusted_certs \u003d None"},{"line_number":3245,"context_line":""},{"line_number":3246,"context_line":"        self._create_and_inject_local_root("},{"line_number":3247,"context_line":"            context, instance,"}],"source_content_type":"text/x-python","patch_set":24,"id":"7f96bb07_6273bc4e","line":3244,"range":{"start_line":3241,"start_character":8,"end_line":3244,"end_character":32},"updated":"2018-01-15 20:13:55.000000000","message":"please do the following instead of an open-ended try/except NovaException:\n\n trusted_certs \u003d None\n if \u0027trusted_certs\u0027 in instance:\n     trusted_certs \u003d instance.trusted_certs","commit_id":"053b68784b82d4c787b5e45825c85a241634e316"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"011c5857a8bb82c320cea572d70678ef8128f0aa","unresolved":false,"context_lines":[{"line_number":3385,"context_line":"            self._try_fetch_image_cache(backend, fetch_func, context,"},{"line_number":3386,"context_line":"                                        root_fname, disk_images[\u0027image_id\u0027],"},{"line_number":3387,"context_line":"                                        instance, size, fallback_from_host,"},{"line_number":3388,"context_line":"                                        **kwargs)"},{"line_number":3389,"context_line":""},{"line_number":3390,"context_line":"            if need_inject:"},{"line_number":3391,"context_line":"                self._inject_data(backend, instance, injection_info)"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f93b717_32f8ece3","line":3388,"range":{"start_line":3388,"start_character":39,"end_line":3388,"end_character":48},"updated":"2018-01-25 22:58:15.000000000","message":"This is an anti-pattern. You\u0027re basically establishing an API between L3289 and the deeeeep layers of image code way below. Why are you doing this? It makes it so much harder to debug/fix later.","commit_id":"fce54e9eb0b9160eb877e60d7cc9b8be51720f35"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"ed49a68b35088834d953fc1fcce0adee91afe6e6","unresolved":false,"context_lines":[{"line_number":3385,"context_line":"            self._try_fetch_image_cache(backend, fetch_func, context,"},{"line_number":3386,"context_line":"                                        root_fname, disk_images[\u0027image_id\u0027],"},{"line_number":3387,"context_line":"                                        instance, size, fallback_from_host,"},{"line_number":3388,"context_line":"                                        **kwargs)"},{"line_number":3389,"context_line":""},{"line_number":3390,"context_line":"            if need_inject:"},{"line_number":3391,"context_line":"                self._inject_data(backend, instance, injection_info)"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f93b717_7e071b1e","line":3388,"range":{"start_line":3388,"start_character":39,"end_line":3388,"end_character":48},"in_reply_to":"5f93b717_32f8ece3","updated":"2018-01-26 02:47:04.000000000","message":"I followed the stack trace for booting an image to determine where to pass trusted_certs, and this was part of it. I\u0027m not sure how else to tell image.cache() to use trusted_certs","commit_id":"fce54e9eb0b9160eb877e60d7cc9b8be51720f35"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"525d231ba6f1bd4b88bdd625fc24a430464c5b80","unresolved":false,"context_lines":[{"line_number":3437,"context_line":"    def _create_and_inject_local_root(self, context, instance,"},{"line_number":3438,"context_line":"                                      booted_from_volume, suffix, disk_images,"},{"line_number":3439,"context_line":"                                      injection_info, fallback_from_host,"},{"line_number":3440,"context_line":"                                      **kwargs):"},{"line_number":3441,"context_line":"        # File injection only if needed"},{"line_number":3442,"context_line":"        need_inject \u003d (not configdrive.required_by(instance) and"},{"line_number":3443,"context_line":"                       injection_info is not None and"}],"source_content_type":"text/x-python","patch_set":37,"id":"df7087c5_cff1dd03","line":3440,"updated":"2018-03-20 12:18:29.000000000","message":"Same thing here, seems we should add a real argument. No need for it to be optional I believe.","commit_id":"e7c874a33fa8a2069084a81d2ef2712005547989"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"6104ffb8c153bbeae4591f443c511489a5082b81","unresolved":false,"context_lines":[{"line_number":3437,"context_line":"    def _create_and_inject_local_root(self, context, instance,"},{"line_number":3438,"context_line":"                                      booted_from_volume, suffix, disk_images,"},{"line_number":3439,"context_line":"                                      injection_info, fallback_from_host,"},{"line_number":3440,"context_line":"                                      **kwargs):"},{"line_number":3441,"context_line":"        # File injection only if needed"},{"line_number":3442,"context_line":"        need_inject \u003d (not configdrive.required_by(instance) and"},{"line_number":3443,"context_line":"                       injection_info is not None and"}],"source_content_type":"text/x-python","patch_set":37,"id":"df7087c5_0d76dc07","line":3440,"in_reply_to":"df7087c5_cff1dd03","updated":"2018-03-20 18:23:26.000000000","message":"Will be fixed in patch 39","commit_id":"e7c874a33fa8a2069084a81d2ef2712005547989"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d105936c586d305c795dc95df0c030e76c0a8a17","unresolved":false,"context_lines":[{"line_number":3525,"context_line":"    def _create_and_inject_local_root(self, context, instance,"},{"line_number":3526,"context_line":"                                      booted_from_volume, suffix, disk_images,"},{"line_number":3527,"context_line":"                                      injection_info, fallback_from_host,"},{"line_number":3528,"context_line":"                                      trusted_certs\u003dNone):"},{"line_number":3529,"context_line":"        # File injection only if needed"},{"line_number":3530,"context_line":"        need_inject \u003d (not configdrive.required_by(instance) and"},{"line_number":3531,"context_line":"                       injection_info is not None and"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_ac3c062f","line":3528,"updated":"2018-04-03 09:11:20.000000000","message":"Nit: does it really need to be optional here, I think that just confuses things.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"96efedf15baab4c7660e43e3d8b77bf91c1001af","unresolved":false,"context_lines":[{"line_number":3525,"context_line":"    def _create_and_inject_local_root(self, context, instance,"},{"line_number":3526,"context_line":"                                      booted_from_volume, suffix, disk_images,"},{"line_number":3527,"context_line":"                                      injection_info, fallback_from_host,"},{"line_number":3528,"context_line":"                                      trusted_certs\u003dNone):"},{"line_number":3529,"context_line":"        # File injection only if needed"},{"line_number":3530,"context_line":"        need_inject \u003d (not configdrive.required_by(instance) and"},{"line_number":3531,"context_line":"                       injection_info is not None and"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_cc9a9312","line":3528,"in_reply_to":"bf659307_ac3c062f","updated":"2018-04-03 14:19:04.000000000","message":"Agreed, and I think I commented about a similar thing in the past on this set.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"f2c10d8e748d18f397c35b1089cc93555bbe6e24","unresolved":false,"context_lines":[{"line_number":3525,"context_line":"    def _create_and_inject_local_root(self, context, instance,"},{"line_number":3526,"context_line":"                                      booted_from_volume, suffix, disk_images,"},{"line_number":3527,"context_line":"                                      injection_info, fallback_from_host,"},{"line_number":3528,"context_line":"                                      trusted_certs\u003dNone):"},{"line_number":3529,"context_line":"        # File injection only if needed"},{"line_number":3530,"context_line":"        need_inject \u003d (not configdrive.required_by(instance) and"},{"line_number":3531,"context_line":"                       injection_info is not None and"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_8f29ed37","line":3528,"in_reply_to":"bf659307_cc9a9312","updated":"2018-04-03 15:19:33.000000000","message":"Thanks, John and Dan. Good point. We had made it optional in case certificate validation wasn\u0027t going to be used, but that None should be passed in for the trusted_certs argument in that case anyway. This will be fixed in the next patch.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d105936c586d305c795dc95df0c030e76c0a8a17","unresolved":false,"context_lines":[{"line_number":3557,"context_line":"            self._try_fetch_image_cache(backend, fetch_func, context,"},{"line_number":3558,"context_line":"                                        root_fname, disk_images[\u0027image_id\u0027],"},{"line_number":3559,"context_line":"                                        instance, size, fallback_from_host,"},{"line_number":3560,"context_line":"                                        trusted_certs\u003dtrusted_certs)"},{"line_number":3561,"context_line":""},{"line_number":3562,"context_line":"            if need_inject:"},{"line_number":3563,"context_line":"                self._inject_data(backend, instance, injection_info)"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_0c1732a6","line":3560,"updated":"2018-04-03 09:11:20.000000000","message":"Ni:t Not sure why you are doing trusted_certs\u003dtrusted_certs here, it does complicate the testing somewhat.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"f2c10d8e748d18f397c35b1089cc93555bbe6e24","unresolved":false,"context_lines":[{"line_number":3557,"context_line":"            self._try_fetch_image_cache(backend, fetch_func, context,"},{"line_number":3558,"context_line":"                                        root_fname, disk_images[\u0027image_id\u0027],"},{"line_number":3559,"context_line":"                                        instance, size, fallback_from_host,"},{"line_number":3560,"context_line":"                                        trusted_certs\u003dtrusted_certs)"},{"line_number":3561,"context_line":""},{"line_number":3562,"context_line":"            if need_inject:"},{"line_number":3563,"context_line":"                self._inject_data(backend, instance, injection_info)"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_6c5827f9","line":3560,"in_reply_to":"bf659307_0c1732a6","updated":"2018-04-03 15:19:33.000000000","message":"Thanks, John. This will be fixed in the next patch.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d105936c586d305c795dc95df0c030e76c0a8a17","unresolved":false,"context_lines":[{"line_number":7386,"context_line":"                         fallback_from_host\u003dNone, trusted_certs\u003dNone):"},{"line_number":7387,"context_line":"        try:"},{"line_number":7388,"context_line":"            libvirt_utils.fetch_image(context, path, image_id,"},{"line_number":7389,"context_line":"                                      trusted_certs\u003dtrusted_certs)"},{"line_number":7390,"context_line":"        except exception.ImageNotFound:"},{"line_number":7391,"context_line":"            if not fallback_from_host:"},{"line_number":7392,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_cccffa26","line":7389,"updated":"2018-04-03 09:11:20.000000000","message":"Nit: same thing here really.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"f2c10d8e748d18f397c35b1089cc93555bbe6e24","unresolved":false,"context_lines":[{"line_number":7386,"context_line":"                         fallback_from_host\u003dNone, trusted_certs\u003dNone):"},{"line_number":7387,"context_line":"        try:"},{"line_number":7388,"context_line":"            libvirt_utils.fetch_image(context, path, image_id,"},{"line_number":7389,"context_line":"                                      trusted_certs\u003dtrusted_certs)"},{"line_number":7390,"context_line":"        except exception.ImageNotFound:"},{"line_number":7391,"context_line":"            if not fallback_from_host:"},{"line_number":7392,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_8c5b3bf4","line":7389,"in_reply_to":"bf659307_cccffa26","updated":"2018-04-03 15:19:33.000000000","message":"Thanks, John. This will be fixed in the next patch.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9c9e121ede0bd79d5de454ccc14ece1f82b14968","unresolved":false,"context_lines":[{"line_number":3458,"context_line":""},{"line_number":3459,"context_line":"        trusted_certs \u003d None"},{"line_number":3460,"context_line":"        if \u0027trusted_certs\u0027 in instance:"},{"line_number":3461,"context_line":"            trusted_certs \u003d instance.trusted_certs"},{"line_number":3462,"context_line":""},{"line_number":3463,"context_line":"        self._create_and_inject_local_root("},{"line_number":3464,"context_line":"            context, instance,"}],"source_content_type":"text/x-python","patch_set":42,"id":"bf659307_c8c26653","line":3461,"updated":"2018-04-04 09:32:41.000000000","message":"I don\u0027t actually see a unit test where you pass an instance that has trusted_certs set. (You are testing the case where it is not set).","commit_id":"9f0d426da0af744d350bb4be4455ed39f36d76e6"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"1a910e8af454febf7328711d2345ae94151ad31c","unresolved":false,"context_lines":[{"line_number":3458,"context_line":""},{"line_number":3459,"context_line":"        trusted_certs \u003d None"},{"line_number":3460,"context_line":"        if \u0027trusted_certs\u0027 in instance:"},{"line_number":3461,"context_line":"            trusted_certs \u003d instance.trusted_certs"},{"line_number":3462,"context_line":""},{"line_number":3463,"context_line":"        self._create_and_inject_local_root("},{"line_number":3464,"context_line":"            context, instance,"}],"source_content_type":"text/x-python","patch_set":42,"id":"bf659307_cf57c857","line":3461,"in_reply_to":"bf659307_c8c26653","updated":"2018-04-04 18:25:42.000000000","message":"Updated tests in test_utils.py and test_driver.py to use not-None trusted_certs arguments. Also fixed _create_images_and_backing so that we\u0027re actually passing trusted_certs to _try_fetch_image_cache.","commit_id":"9f0d426da0af744d350bb4be4455ed39f36d76e6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"63e4a2db76cb8a1cb4226c4b5a7d747bffb53070","unresolved":false,"context_lines":[{"line_number":7743,"context_line":""},{"line_number":7744,"context_line":"                trusted_certs \u003d None"},{"line_number":7745,"context_line":"                if \u0027trusted_certs\u0027 in instance:"},{"line_number":7746,"context_line":"                    trusted_certs \u003d instance.trusted_certs.ids"},{"line_number":7747,"context_line":""},{"line_number":7748,"context_line":"                if cache_name.startswith(\u0027ephemeral\u0027):"},{"line_number":7749,"context_line":"                    # The argument \u0027size\u0027 is used by image.cache to"}],"source_content_type":"text/x-python","patch_set":43,"id":"bf659307_2c33eb64","line":7746,"updated":"2018-04-04 18:14:54.000000000","message":"So fixing the test caught this? :)","commit_id":"6276d017d893521820be03d84fccffc05787e188"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"1a910e8af454febf7328711d2345ae94151ad31c","unresolved":false,"context_lines":[{"line_number":7743,"context_line":""},{"line_number":7744,"context_line":"                trusted_certs \u003d None"},{"line_number":7745,"context_line":"                if \u0027trusted_certs\u0027 in instance:"},{"line_number":7746,"context_line":"                    trusted_certs \u003d instance.trusted_certs.ids"},{"line_number":7747,"context_line":""},{"line_number":7748,"context_line":"                if cache_name.startswith(\u0027ephemeral\u0027):"},{"line_number":7749,"context_line":"                    # The argument \u0027size\u0027 is used by image.cache to"}],"source_content_type":"text/x-python","patch_set":43,"id":"bf659307_6c4a4384","line":7746,"in_reply_to":"bf659307_2c33eb64","updated":"2018-04-04 18:25:42.000000000","message":"Yes :-) Yay, tests","commit_id":"6276d017d893521820be03d84fccffc05787e188"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d7134a6569773f9d8cd9f3dbd97a1ff53f785872","unresolved":false,"context_lines":[{"line_number":7743,"context_line":""},{"line_number":7744,"context_line":"                trusted_certs \u003d None"},{"line_number":7745,"context_line":"                if \u0027trusted_certs\u0027 in instance:"},{"line_number":7746,"context_line":"                    trusted_certs \u003d instance.trusted_certs.ids"},{"line_number":7747,"context_line":""},{"line_number":7748,"context_line":"                if cache_name.startswith(\u0027ephemeral\u0027):"},{"line_number":7749,"context_line":"                    # The argument \u0027size\u0027 is used by image.cache to"}],"source_content_type":"text/x-python","patch_set":43,"id":"bf659307_ed5ded48","line":7746,"in_reply_to":"bf659307_6c4a4384","updated":"2018-04-04 20:40:22.000000000","message":"Yeah. So, given that, is there a tempest/devstack patch that tests this for real from the outside? Those can depends-on the API patch above this and demonstrate that this works (i.e. we should be able to see some of your log messages to make sure it\u0027s working).\n\nIf not, is there anything that would block that from being doable?","commit_id":"6276d017d893521820be03d84fccffc05787e188"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"706ed8e96b4453c1e3ee5d6d43c03d84eb05913f","unresolved":false,"context_lines":[{"line_number":7743,"context_line":""},{"line_number":7744,"context_line":"                trusted_certs \u003d None"},{"line_number":7745,"context_line":"                if \u0027trusted_certs\u0027 in instance:"},{"line_number":7746,"context_line":"                    trusted_certs \u003d instance.trusted_certs.ids"},{"line_number":7747,"context_line":""},{"line_number":7748,"context_line":"                if cache_name.startswith(\u0027ephemeral\u0027):"},{"line_number":7749,"context_line":"                    # The argument \u0027size\u0027 is used by image.cache to"}],"source_content_type":"text/x-python","patch_set":43,"id":"bf659307_357c8771","line":7746,"in_reply_to":"bf659307_ed5ded48","updated":"2018-04-04 22:05:14.000000000","message":"Yes, there\u0027s a tempest patch:\nhttps://review.openstack.org/#/c/526485/\n\nAnd a barbican-tempest-plugin patch to show that everything works for real from the outside:\nhttps://review.openstack.org/#/c/515210/","commit_id":"6276d017d893521820be03d84fccffc05787e188"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f199e1684a90c9157f54910536bf590fa4b53150","unresolved":false,"context_lines":[{"line_number":7501,"context_line":"                                     host\u003dfallback_from_host,"},{"line_number":7502,"context_line":"                                     receive\u003dTrue)"},{"line_number":7503,"context_line":"        except exception.CertificateValidationFailed:"},{"line_number":7504,"context_line":"            LOG.warning(\"Image signature certificate validation failed\")"},{"line_number":7505,"context_line":""},{"line_number":7506,"context_line":"    def _fetch_instance_kernel_ramdisk(self, context, instance,"},{"line_number":7507,"context_line":"                                       fallback_from_host\u003dNone):"}],"source_content_type":"text/x-python","patch_set":47,"id":"bf659307_3b82f1c9","line":7504,"updated":"2018-04-09 13:46:23.000000000","message":"These are now logging twice, once in the glance client code, and once here. Example from your logs:\n\nhttp://logs.openstack.org/10/515210/30/check/barbican-simple-crypto-devstack-tempest/9fb6df2/logs/screen-n-cpu.txt.gz?level\u003dWARNING#_Apr_08_00_22_28_530173\n\nAlso, don\u0027t you need to fail here somewhere? Or can this try_.. method not succeed? It seems like it fails \"if not fllback_from_host\" so I would imagine you should replicate some of that logic here because it means we didn\u0027t get an image.\n\nAlso, what happens if we _do_ fallback to copying the image from the original host on L7500? shouldn\u0027t we verify that certificate, and if it fails, refuse to use it?","commit_id":"ed916e1719a3a54830e91c73664d9b4641a93c23"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"63f18c07b4816b1431bad16f982bcf713a188b08","unresolved":false,"context_lines":[{"line_number":7501,"context_line":"                                     host\u003dfallback_from_host,"},{"line_number":7502,"context_line":"                                     receive\u003dTrue)"},{"line_number":7503,"context_line":"        except exception.CertificateValidationFailed:"},{"line_number":7504,"context_line":"            LOG.warning(\"Image signature certificate validation failed\")"},{"line_number":7505,"context_line":""},{"line_number":7506,"context_line":"    def _fetch_instance_kernel_ramdisk(self, context, instance,"},{"line_number":7507,"context_line":"                                       fallback_from_host\u003dNone):"}],"source_content_type":"text/x-python","patch_set":47,"id":"bf659307_82a29099","line":7504,"in_reply_to":"bf659307_05469c7d","updated":"2018-04-10 14:31:05.000000000","message":"Raising an exception does not result in a stack trace. Raising it and never catching it, or doing a LOG.exception() gives you a stack trace. What I want is for you to raise it to break out of here and catch it to trigger the proper BuildAbort somewhere appropriate.\n\nAlso note that we were stack tracing on buildabort until my patch yesterday, which may have made it a little more confusing, so rebase on master and figure out how/where to do that catch and translate.","commit_id":"ed916e1719a3a54830e91c73664d9b4641a93c23"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"4c83aa86bb98846768c1c3c8ea4a0a0b80bab573","unresolved":false,"context_lines":[{"line_number":7501,"context_line":"                                     host\u003dfallback_from_host,"},{"line_number":7502,"context_line":"                                     receive\u003dTrue)"},{"line_number":7503,"context_line":"        except exception.CertificateValidationFailed:"},{"line_number":7504,"context_line":"            LOG.warning(\"Image signature certificate validation failed\")"},{"line_number":7505,"context_line":""},{"line_number":7506,"context_line":"    def _fetch_instance_kernel_ramdisk(self, context, instance,"},{"line_number":7507,"context_line":"                                       fallback_from_host\u003dNone):"}],"source_content_type":"text/x-python","patch_set":47,"id":"bf659307_05469c7d","line":7504,"in_reply_to":"bf659307_3b82f1c9","updated":"2018-04-10 00:58:20.000000000","message":"I\u0027d like to keep the logging in the glance client code. I\u0027m okay with not logging here.\n\nI\u0027m unsure how we should be failing here. I\u0027d like to raise an exception, but that would result in a stacktrace.\n\nIf we _do_ fallback to copying the image from the original host (L7500), then we should probably be calling IMAGE_API.download() afterwards. That\u0027s not in the current codebase, though. If it should be added, it should probably be in a different patch.","commit_id":"ed916e1719a3a54830e91c73664d9b4641a93c23"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"f2f03560a10fc40d63c6b5080f4d500215411a4e","unresolved":false,"context_lines":[{"line_number":7501,"context_line":"                                     host\u003dfallback_from_host,"},{"line_number":7502,"context_line":"                                     receive\u003dTrue)"},{"line_number":7503,"context_line":"        except exception.CertificateValidationFailed:"},{"line_number":7504,"context_line":"            LOG.warning(\"Image signature certificate validation failed\")"},{"line_number":7505,"context_line":""},{"line_number":7506,"context_line":"    def _fetch_instance_kernel_ramdisk(self, context, instance,"},{"line_number":7507,"context_line":"                                       fallback_from_host\u003dNone):"}],"source_content_type":"text/x-python","patch_set":47,"id":"bf659307_971e3e5d","line":7504,"in_reply_to":"bf659307_6473c181","updated":"2018-04-11 02:10:13.000000000","message":"We\u0027ve rebased, but there still seems to be a stack trace on BuildAbort:\nhttp://logs.openstack.org/10/515210/30/check/barbican-simple-crypto-devstack-tempest/806227a/logs/screen-n-cpu.txt.gz#_Apr_11_02_01_52_351328","commit_id":"ed916e1719a3a54830e91c73664d9b4641a93c23"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"c4e8cf16f54d08d82a298ae3b0fe2b385f6026aa","unresolved":false,"context_lines":[{"line_number":7501,"context_line":"                                     host\u003dfallback_from_host,"},{"line_number":7502,"context_line":"                                     receive\u003dTrue)"},{"line_number":7503,"context_line":"        except exception.CertificateValidationFailed:"},{"line_number":7504,"context_line":"            LOG.warning(\"Image signature certificate validation failed\")"},{"line_number":7505,"context_line":""},{"line_number":7506,"context_line":"    def _fetch_instance_kernel_ramdisk(self, context, instance,"},{"line_number":7507,"context_line":"                                       fallback_from_host\u003dNone):"}],"source_content_type":"text/x-python","patch_set":47,"id":"bf659307_6473c181","line":7504,"in_reply_to":"bf659307_82a29099","updated":"2018-04-10 20:01:39.000000000","message":"Got it. I\u0027ll rebase.\n\nIt looks nova.compute.manager._build_resources() is the best place to catch the exception. Should we still be catching and re-raising in nova.virt.libvirt.driver\u0027s _try_fetch_image and _try_fetch_image_cache?","commit_id":"ed916e1719a3a54830e91c73664d9b4641a93c23"}],"nova/virt/libvirt/utils.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"525d231ba6f1bd4b88bdd625fc24a430464c5b80","unresolved":false,"context_lines":[{"line_number":376,"context_line":"            \u0027used\u0027: used}"},{"line_number":377,"context_line":""},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"def fetch_image(context, target, image_id, **kwargs):"},{"line_number":380,"context_line":"    \"\"\"Grab image.\"\"\""},{"line_number":381,"context_line":"    images.fetch_to_raw(context, image_id, target, **kwargs)"},{"line_number":382,"context_line":""}],"source_content_type":"text/x-python","patch_set":37,"id":"df7087c5_4fe5cd37","line":379,"updated":"2018-03-20 12:18:29.000000000","message":"Same comments here, should be an explicitly named argument.","commit_id":"e7c874a33fa8a2069084a81d2ef2712005547989"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"6104ffb8c153bbeae4591f443c511489a5082b81","unresolved":false,"context_lines":[{"line_number":376,"context_line":"            \u0027used\u0027: used}"},{"line_number":377,"context_line":""},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"def fetch_image(context, target, image_id, **kwargs):"},{"line_number":380,"context_line":"    \"\"\"Grab image.\"\"\""},{"line_number":381,"context_line":"    images.fetch_to_raw(context, image_id, target, **kwargs)"},{"line_number":382,"context_line":""}],"source_content_type":"text/x-python","patch_set":37,"id":"df7087c5_ed64c84b","line":379,"in_reply_to":"df7087c5_4fe5cd37","updated":"2018-03-20 18:23:26.000000000","message":"Will be fixed in patch 39","commit_id":"e7c874a33fa8a2069084a81d2ef2712005547989"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d105936c586d305c795dc95df0c030e76c0a8a17","unresolved":false,"context_lines":[{"line_number":376,"context_line":"            \u0027used\u0027: used}"},{"line_number":377,"context_line":""},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"def fetch_image(context, target, image_id, trusted_certs\u003dNone):"},{"line_number":380,"context_line":"    \"\"\"Grab image.\"\"\""},{"line_number":381,"context_line":"    images.fetch_to_raw(context, image_id, target, trusted_certs\u003dtrusted_certs)"},{"line_number":382,"context_line":""}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_6cdeeeda","line":379,"updated":"2018-04-03 09:11:20.000000000","message":"Can we get away with it not being optional? just to make things easier?","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"f2c10d8e748d18f397c35b1089cc93555bbe6e24","unresolved":false,"context_lines":[{"line_number":376,"context_line":"            \u0027used\u0027: used}"},{"line_number":377,"context_line":""},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"def fetch_image(context, target, image_id, trusted_certs\u003dNone):"},{"line_number":380,"context_line":"    \"\"\"Grab image.\"\"\""},{"line_number":381,"context_line":"    images.fetch_to_raw(context, image_id, target, trusted_certs\u003dtrusted_certs)"},{"line_number":382,"context_line":""}],"source_content_type":"text/x-python","patch_set":40,"id":"bf659307_12bf40aa","line":379,"in_reply_to":"bf659307_6cdeeeda","updated":"2018-04-03 15:19:33.000000000","message":"This calls images.fetch_to_raw(), which calls images.fetch(), which calls glance.download().\n\nglance.download() takes in an optional trusted_certs argument (None by default). We need images.fetch() to pass in a trusted_certs argument in order for glance.download() to not use None by default. If we convert all of the trusted_certs arguments from optional to real in images and utils, then we\u0027ll need to add some logic within the methods to check for whether trusted_certs is None.","commit_id":"305601783baec29f79dea9eced024f1806f10ef1"}]}