)]}'
{"zuul_registry/main.py":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"654945165e419fcd7ef73ee4d70a38245485334f","unresolved":false,"context_lines":[{"line_number":354,"context_line":"                # return an error to the client so it can try again."},{"line_number":355,"context_line":"                for layer in data[\u0027layers\u0027]:"},{"line_number":356,"context_line":"                    self.log.info(\"Clean layer %s\" % layer[\u0027digest\u0027])"},{"line_number":357,"context_line":"                    self.storage.delete_blob(namespace, layer[\u0027digest\u0027])"},{"line_number":358,"context_line":"                raise cherrypy.HTTPError(400, msg)"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        if changed:"}],"source_content_type":"text/x-python","patch_set":1,"id":"12f4a623_e04ba84c","line":357,"updated":"2021-09-03 15:34:24.000000000","message":"Having said that, I don\u0027t think we should delete the layer; we have no indication that there\u0027s anything wrong with that blob.  It was uploaded with a content digest and we validated it already.  It\u0027s the manifest that\u0027s wrong.","commit_id":"e43629a8d508dc6564887b1e6e92909c2d886452"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"b0393332171ea5aa9da8d039c64c45132cbf3d4c","unresolved":true,"context_lines":[{"line_number":354,"context_line":"                # return an error to the client so it can try again."},{"line_number":355,"context_line":"                for layer in data[\u0027layers\u0027]:"},{"line_number":356,"context_line":"                    self.log.info(\"Clean layer %s\" % layer[\u0027digest\u0027])"},{"line_number":357,"context_line":"                    self.storage.delete_blob(namespace, layer[\u0027digest\u0027])"},{"line_number":358,"context_line":"                raise cherrypy.HTTPError(400, msg)"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        if changed:"}],"source_content_type":"text/x-python","patch_set":1,"id":"74bb948c_e75aa981","line":357,"updated":"2021-09-03 15:17:54.000000000","message":"I think there is a possible race between simultaneous uploads for the same image and deleting these blobs. In the storage layer we would check a simple lock mechanism to ensure we don\u0027t have multiple writers at the same time; however, I worry that simple mechanism is too simple to negotiate for these deletions.\n\nThis race may manifest as the blobs already being deleted when we try to delete them here (because the other upload is tripping over the same verification problem). In the case of filesystem storage we do check that the path exists before attempting to delete it, but there is a small period of time where a context switch could happen between checking existence and attempting the delete which would result in an error being raised.\n\nThe other way I think this can show up is if one uploader fails the strict check and the other does not. In that case we could potentially delete the upload out from under the successfully verified uploader.\n\nThat said if the docker protocol mandates that the layers all be uploaded prior to the manifest (somewhat implied by the size checking above) then we would avoid this race entirely as any subsequent uploads would have stopped prior to any manifest checking. It would be good if someone else can double check this.","commit_id":"e43629a8d508dc6564887b1e6e92909c2d886452"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"c1763f38a2f404970c0e7c8e97d9eb59a183ff8a","unresolved":false,"context_lines":[{"line_number":354,"context_line":"                # return an error to the client so it can try again."},{"line_number":355,"context_line":"                for layer in data[\u0027layers\u0027]:"},{"line_number":356,"context_line":"                    self.log.info(\"Clean layer %s\" % layer[\u0027digest\u0027])"},{"line_number":357,"context_line":"                    self.storage.delete_blob(namespace, layer[\u0027digest\u0027])"},{"line_number":358,"context_line":"                raise cherrypy.HTTPError(400, msg)"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        if changed:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7221f7cf_5ef2dbe2","line":357,"updated":"2021-09-03 15:31:29.000000000","message":"Manifest upload should only happen after the layers are uploaded, per https://docs.docker.com/registry/spec/api/#pushing-an-image","commit_id":"e43629a8d508dc6564887b1e6e92909c2d886452"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"d83510f6191ccaa3036a7b180e85019304d12a5b","unresolved":false,"context_lines":[{"line_number":354,"context_line":"                # return an error to the client so it can try again."},{"line_number":355,"context_line":"                for layer in data[\u0027layers\u0027]:"},{"line_number":356,"context_line":"                    self.log.info(\"Clean layer %s\" % layer[\u0027digest\u0027])"},{"line_number":357,"context_line":"                    self.storage.delete_blob(namespace, layer[\u0027digest\u0027])"},{"line_number":358,"context_line":"                raise cherrypy.HTTPError(400, msg)"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        if changed:"}],"source_content_type":"text/x-python","patch_set":1,"id":"f47599ea_921ae6d6","line":357,"in_reply_to":"7221f7cf_5ef2dbe2","updated":"2021-09-03 15:33:40.000000000","message":"In that case I think we avoid this race entirely because additional uploads would have stopped executing before they tried to write the manifest.","commit_id":"e43629a8d508dc6564887b1e6e92909c2d886452"}]}