)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fcbf91b6691ee9e0dc08886d3a219eac25de006e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"68a98181_0cba9b32","updated":"2026-02-26 16:04:47.000000000","message":"@kajinamit@oss.nttdata.com I haven\u0027t written tests for this yet because I think maybe this is getting too big. Any thoughts on moving some of the LUKS stuff out into a private `_module.py` or something to keep `format_inspector.py` from inflating too much?","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"b851182ae549165c201df342a0f78477e96385e2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f710af4f_9d941377","updated":"2026-03-06 14:54:35.000000000","message":"Doesn\u0027t the GPT format store a second backup header at the end of the disk/image?\n\nI think this means that just looking at the first block would not be sufficient to determine if an image can be consumed as GPT.","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9e504e49902ea48c6cfc16da712cb804f0ce8e3f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"765eefab_bfc8e2dd","updated":"2026-03-18 13:48:13.000000000","message":"I need to look at this a few more times to be able to grok it fully, so these are more general, high-level comments","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"79b80ae5b524529e0dae320524cd1780326dbaa1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"02b05603_b120372e","in_reply_to":"68a98181_0cba9b32","updated":"2026-03-17 16:22:02.000000000","message":"My initial thought was to split out luks related logics to something like luks_utils but I guess it\u0027s not feasible.\n\nAnother option is renaming format_inspector.py to `format_inspector/__init__.py` and move the all LUKS things to `format_inspector/_luks.py` (or even luks.py) . We can import classes from these files in `__init__` to support the existing import from `__init__`. (And we can follow this for a few other classes such as vhd/vhdx if we want)","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d4f7acb2831dd20b7a27cf5a21a2837dcde58d3b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6d316557_dc27fe1d","in_reply_to":"f710af4f_9d941377","updated":"2026-03-06 15:47:13.000000000","message":"Yeah, but we already don\u0027t capture/inspect it in the GPT path:\n\nhttps://github.com/openstack/oslo.utils/blob/master/oslo_utils/imageutils/format_inspector.py#L1362\n\nThe important part for safety with qemu is that it doesn\u0027t look at a file and guess wrongly about the format, and (based on looking at the code a couple years ago when this all came up) it never does that by looking at backup/footer things. VMDK has a similar concept. If something decides that a thing is GPT, it could look at the backup copy of the descriptor if it is concerned the primary one is damaged or incorrect, but I don\u0027t think we ever risk detecting a thing as GPT because the _last_ block looks like it has a proper header in it.\n\nAlso, while we\u0027re interested in detecting that a thing is GPT, we don\u0027t do a ton of validation (i.e. caring if the backup GPT descriptor has an invalid partition table or something). What we really care about here is that you don\u0027t claim a thing is a LUKS-encrypted bootable raw disk, but there\u0027s really a qcow2 or vmdk hiding inside (each of which could have bad things like a data-file or backing-file) that we didn\u0027t notice because we didn\u0027t unwrap it.","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c60472c4bf81532122a42b797e3229c891464755","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"24c0e5f7_44c66c7a","updated":"2026-03-18 16:49:09.000000000","message":"Note I have not split out the LUKS stuff yet as I\u0027m still not sure what the best path is.","commit_id":"02be1585881f21e6d48a85b068852a7deddf4e67"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0e6db6455beab1612180ec05b684c41bc710110a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"fcbe8566_3790085d","updated":"2026-04-10 18:08:10.000000000","message":"This update is splitting the low-level LUKS stuff out to a helper module and adding tests.","commit_id":"a92df26df328a0b564b9a25a5cd841079c54c3d5"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"94928ccdc1b6a071b4d4a432f1cc3be24f44d45c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"80cde324_6a7d489b","updated":"2026-04-15 14:22:52.000000000","message":"I think there\u0027s a still a TODO open from the previous patchset (assuming you haven\u0027t decided against it)? One additional comment RE: the additional dependency too","commit_id":"a182d3f1e25e18344ec83eff026e49c5289fc5d8"}],"oslo_utils/imageutils/_luks.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"94928ccdc1b6a071b4d4a432f1cc3be24f44d45c","unresolved":true,"context_lines":[{"line_number":194,"context_line":"    :param start_sector: The starting sector number for IV calculation"},{"line_number":195,"context_line":"    :returns: The decrypted data"},{"line_number":196,"context_line":"    \"\"\""},{"line_number":197,"context_line":"    if cipher_name \u003d\u003d \u0027aes\u0027 and cipher_mode.startswith(\u0027xts\u0027):"},{"line_number":198,"context_line":"        # XTS mode - must decrypt sector by sector"},{"line_number":199,"context_line":"        # For XTS, LUKS uses the master key directly if it\u0027s the right size"},{"line_number":200,"context_line":"        # For AES-256-XTS, we need a 512-bit (64-byte) key"}],"source_content_type":"text/x-python","patch_set":7,"id":"86e118dd_7059f6a7","line":197,"updated":"2026-04-15 14:22:52.000000000","message":"Does this still need to split up per https://review.opendev.org/c/openstack/oslo.utils/+/978097/comment/32858a36_3ba550fd/?","commit_id":"a182d3f1e25e18344ec83eff026e49c5289fc5d8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c16d072782a01c18c0d01b36c64859c5f801e211","unresolved":true,"context_lines":[{"line_number":194,"context_line":"    :param start_sector: The starting sector number for IV calculation"},{"line_number":195,"context_line":"    :returns: The decrypted data"},{"line_number":196,"context_line":"    \"\"\""},{"line_number":197,"context_line":"    if cipher_name \u003d\u003d \u0027aes\u0027 and cipher_mode.startswith(\u0027xts\u0027):"},{"line_number":198,"context_line":"        # XTS mode - must decrypt sector by sector"},{"line_number":199,"context_line":"        # For XTS, LUKS uses the master key directly if it\u0027s the right size"},{"line_number":200,"context_line":"        # For AES-256-XTS, we need a 512-bit (64-byte) key"}],"source_content_type":"text/x-python","patch_set":7,"id":"49d9f484_a6e91854","line":197,"in_reply_to":"86e118dd_7059f6a7","updated":"2026-04-15 14:27:22.000000000","message":"Yep it does and still planning on it. Was trying to get the major split done and tests up. Hoping to do this split and the reversion back to isinstance() before next week.","commit_id":"a182d3f1e25e18344ec83eff026e49c5289fc5d8"}],"oslo_utils/imageutils/format_inspector.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9e504e49902ea48c6cfc16da712cb804f0ce8e3f","unresolved":true,"context_lines":[{"line_number":1518,"context_line":"    LUKS_KEY_ENABLED \u003d 0x00AC71F3"},{"line_number":1519,"context_line":""},{"line_number":1520,"context_line":"    def _initialize(self) -\u003e None:"},{"line_number":1521,"context_line":"        # NOTE, this will be a string but we will need it to be bytes later"},{"line_number":1522,"context_line":"        self._passphrase \u003d self._data.get(\u0027luks_passphrase\u0027)"},{"line_number":1523,"context_line":"        self._master_key: bytes | None \u003d None"},{"line_number":1524,"context_line":"        self._key_material_captured \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"fe68077b_adf35ef1","line":1521,"updated":"2026-03-18 13:48:13.000000000","message":"nit: couldn\u0027t we just ask for the user to encode it beforehand and assert the type here/in the safety check?","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ffda4d57097130e24bcc9f584b598a7efc3dd01f","unresolved":true,"context_lines":[{"line_number":1518,"context_line":"    LUKS_KEY_ENABLED \u003d 0x00AC71F3"},{"line_number":1519,"context_line":""},{"line_number":1520,"context_line":"    def _initialize(self) -\u003e None:"},{"line_number":1521,"context_line":"        # NOTE, this will be a string but we will need it to be bytes later"},{"line_number":1522,"context_line":"        self._passphrase \u003d self._data.get(\u0027luks_passphrase\u0027)"},{"line_number":1523,"context_line":"        self._master_key: bytes | None \u003d None"},{"line_number":1524,"context_line":"        self._key_material_captured \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"281b4fa9_1d88a586","line":1521,"in_reply_to":"fe68077b_adf35ef1","updated":"2026-03-18 14:02:04.000000000","message":"So this is on my TODO list to investigate, but I think it needs to be a string. I believe that when prompted for a passphrase during disk setup, the user can enter unicode and the actual bytes used for the password are UTF-8. So if we get a `bytes` here we could only really check that it\u0027s the right /encoding/ if we tried to decode it as UTF-8 which seems less good to me.","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9e504e49902ea48c6cfc16da712cb804f0ce8e3f","unresolved":true,"context_lines":[{"line_number":1583,"context_line":"                    split_key_size \u003d key_bytes * slot[\u0027stripes\u0027]"},{"line_number":1584,"context_line":"                    km_offset \u003d slot[\u0027key_offset\u0027] * self.LUKS_SECTOR_SIZE"},{"line_number":1585,"context_line":""},{"line_number":1586,"context_line":"                    self._trace("},{"line_number":1587,"context_line":"                        \u0027Capturing key material from \u0027"},{"line_number":1588,"context_line":"                        \u0027slot %d: offset\u003d%d size\u003d%d\u0027,"},{"line_number":1589,"context_line":"                        slot_num,"},{"line_number":1590,"context_line":"                        km_offset,"},{"line_number":1591,"context_line":"                        split_key_size,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f8042b5_a5768741","line":1588,"range":{"start_line":1586,"start_character":32,"end_line":1588,"end_character":53},"updated":"2026-03-18 13:48:13.000000000","message":"nit:\n\n```suggestion\n                    self._trace(\n                        \u0027Capturing key material from slot %d: offset\u003d%d size\u003d%d\u0027,\n```\n\nit should fit...","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ffda4d57097130e24bcc9f584b598a7efc3dd01f","unresolved":false,"context_lines":[{"line_number":1583,"context_line":"                    split_key_size \u003d key_bytes * slot[\u0027stripes\u0027]"},{"line_number":1584,"context_line":"                    km_offset \u003d slot[\u0027key_offset\u0027] * self.LUKS_SECTOR_SIZE"},{"line_number":1585,"context_line":""},{"line_number":1586,"context_line":"                    self._trace("},{"line_number":1587,"context_line":"                        \u0027Capturing key material from \u0027"},{"line_number":1588,"context_line":"                        \u0027slot %d: offset\u003d%d size\u003d%d\u0027,"},{"line_number":1589,"context_line":"                        slot_num,"},{"line_number":1590,"context_line":"                        km_offset,"},{"line_number":1591,"context_line":"                        split_key_size,"}],"source_content_type":"text/x-python","patch_set":1,"id":"80c4bfaf_35273b87","line":1588,"range":{"start_line":1586,"start_character":32,"end_line":1588,"end_character":53},"in_reply_to":"5f8042b5_a5768741","updated":"2026-03-18 14:02:04.000000000","message":"Acknowledged","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9e504e49902ea48c6cfc16da712cb804f0ce8e3f","unresolved":true,"context_lines":[{"line_number":1762,"context_line":""},{"line_number":1763,"context_line":"        # Map LUKS hash names to hashlib/cryptography"},{"line_number":1764,"context_line":"        hash_algo_map \u003d {"},{"line_number":1765,"context_line":"            \u0027sha1\u0027: (hashes.SHA1(), hashlib.sha1),  # noqa"},{"line_number":1766,"context_line":"            \u0027sha256\u0027: (hashes.SHA256(), hashlib.sha256),"},{"line_number":1767,"context_line":"            \u0027sha512\u0027: (hashes.SHA512(), hashlib.sha512),"},{"line_number":1768,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":1,"id":"90932ec2_eb3f2009","line":1765,"updated":"2026-03-18 13:48:13.000000000","message":"Could we be more specific about the rule we are ignoring? I\u0027m guessing this is S303?\n\n\n\n```suggestion\n            \u0027sha1\u0027: (hashes.SHA1(), hashlib.sha1),  # noqa: S303\n```","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ffda4d57097130e24bcc9f584b598a7efc3dd01f","unresolved":false,"context_lines":[{"line_number":1762,"context_line":""},{"line_number":1763,"context_line":"        # Map LUKS hash names to hashlib/cryptography"},{"line_number":1764,"context_line":"        hash_algo_map \u003d {"},{"line_number":1765,"context_line":"            \u0027sha1\u0027: (hashes.SHA1(), hashlib.sha1),  # noqa"},{"line_number":1766,"context_line":"            \u0027sha256\u0027: (hashes.SHA256(), hashlib.sha256),"},{"line_number":1767,"context_line":"            \u0027sha512\u0027: (hashes.SHA512(), hashlib.sha512),"},{"line_number":1768,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":1,"id":"6fed5813_db66fd72","line":1765,"in_reply_to":"90932ec2_eb3f2009","updated":"2026-03-18 14:02:04.000000000","message":"Acknowledged","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":33742,"name":"Jonas Schäfer","email":"jonas.schaefer@cloudandheat.com","username":"jssfr"},"change_message_id":"20e08500127c3c8934c223809a70b127fad4e6e6","unresolved":true,"context_lines":[{"line_number":1785,"context_line":"            algorithm\u003dhash_algo,"},{"line_number":1786,"context_line":"            length\u003dkey_bytes,"},{"line_number":1787,"context_line":"            salt\u003dslot[\u0027salt\u0027],"},{"line_number":1788,"context_line":"            iterations\u003dslot[\u0027iterations\u0027],"},{"line_number":1789,"context_line":"            backend\u003ddefault_backend(),"},{"line_number":1790,"context_line":"        )"},{"line_number":1791,"context_line":"        pwd_derived \u003d kdf.derive(passphrase)"}],"source_content_type":"text/x-python","patch_set":1,"id":"b26f16f2_08f5ea3c","line":1788,"updated":"2026-03-04 08:17:59.000000000","message":"Should there be some kind of limit on the iterations which are acceptable? Otherwise this may be a DoS vector of some kind.","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c60472c4bf81532122a42b797e3229c891464755","unresolved":false,"context_lines":[{"line_number":1785,"context_line":"            algorithm\u003dhash_algo,"},{"line_number":1786,"context_line":"            length\u003dkey_bytes,"},{"line_number":1787,"context_line":"            salt\u003dslot[\u0027salt\u0027],"},{"line_number":1788,"context_line":"            iterations\u003dslot[\u0027iterations\u0027],"},{"line_number":1789,"context_line":"            backend\u003ddefault_backend(),"},{"line_number":1790,"context_line":"        )"},{"line_number":1791,"context_line":"        pwd_derived \u003d kdf.derive(passphrase)"}],"source_content_type":"text/x-python","patch_set":1,"id":"cd5b173e_2e6382c0","line":1788,"in_reply_to":"1f84481b_b82fa2d4","updated":"2026-03-18 16:49:09.000000000","message":"Done","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d2e729dee4665f314fe77a61a53ab4cfc8f742f3","unresolved":true,"context_lines":[{"line_number":1785,"context_line":"            algorithm\u003dhash_algo,"},{"line_number":1786,"context_line":"            length\u003dkey_bytes,"},{"line_number":1787,"context_line":"            salt\u003dslot[\u0027salt\u0027],"},{"line_number":1788,"context_line":"            iterations\u003dslot[\u0027iterations\u0027],"},{"line_number":1789,"context_line":"            backend\u003ddefault_backend(),"},{"line_number":1790,"context_line":"        )"},{"line_number":1791,"context_line":"        pwd_derived \u003d kdf.derive(passphrase)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f84481b_b82fa2d4","line":1788,"in_reply_to":"b26f16f2_08f5ea3c","updated":"2026-03-04 16:46:53.000000000","message":"Yep, good call. As noted in the commit message, this part was generated by claude and thus probably has more such things that are worth limiting. I generated this as a PoC to make sure it was doable, but it definitely needs review (by a human) for stuff like this, which I haven\u0027t done yet.\n\nThe LUKS spec does not (AFAICT) specify a maximum number of iterations, and I guess maybe doing so would be a security risk on its own. It does suggest a _minimum_ of 1000 though. I guess we just need do some survey of likely reasonable values and see what existing tools are using for generating images and put an arbitrary cap here.","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":33742,"name":"Jonas Schäfer","email":"jonas.schaefer@cloudandheat.com","username":"jssfr"},"change_message_id":"20e08500127c3c8934c223809a70b127fad4e6e6","unresolved":true,"context_lines":[{"line_number":1827,"context_line":"            algorithm\u003dhash_algo,"},{"line_number":1828,"context_line":"            length\u003dself.LUKS_DIGESTSIZE,"},{"line_number":1829,"context_line":"            salt\u003dheader[\u0027mk_digest_salt\u0027],"},{"line_number":1830,"context_line":"            iterations\u003dheader[\u0027mk_digest_iter\u0027],"},{"line_number":1831,"context_line":"            backend\u003ddefault_backend(),"},{"line_number":1832,"context_line":"        )"},{"line_number":1833,"context_line":"        mk_digest \u003d kdf_verify.derive(master_key_candidate)"}],"source_content_type":"text/x-python","patch_set":1,"id":"c8d27ccb_8d91c3c8","line":1830,"updated":"2026-03-04 08:17:59.000000000","message":"Similarly, here.","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c60472c4bf81532122a42b797e3229c891464755","unresolved":false,"context_lines":[{"line_number":1827,"context_line":"            algorithm\u003dhash_algo,"},{"line_number":1828,"context_line":"            length\u003dself.LUKS_DIGESTSIZE,"},{"line_number":1829,"context_line":"            salt\u003dheader[\u0027mk_digest_salt\u0027],"},{"line_number":1830,"context_line":"            iterations\u003dheader[\u0027mk_digest_iter\u0027],"},{"line_number":1831,"context_line":"            backend\u003ddefault_backend(),"},{"line_number":1832,"context_line":"        )"},{"line_number":1833,"context_line":"        mk_digest \u003d kdf_verify.derive(master_key_candidate)"}],"source_content_type":"text/x-python","patch_set":1,"id":"873b4882_c9343bce","line":1830,"in_reply_to":"c8d27ccb_8d91c3c8","updated":"2026-03-18 16:49:09.000000000","message":"Done","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9e504e49902ea48c6cfc16da712cb804f0ce8e3f","unresolved":true,"context_lines":[{"line_number":1944,"context_line":"                    # Encrypt the sector number to get the IV"},{"line_number":1945,"context_line":"                    iv_cipher \u003d ciphers.Cipher("},{"line_number":1946,"context_line":"                        ciphers.algorithms.AES(iv_key),"},{"line_number":1947,"context_line":"                        ciphers.modes.ECB(),  # noqa"},{"line_number":1948,"context_line":"                        backend\u003ddefault_backend(),"},{"line_number":1949,"context_line":"                    )"},{"line_number":1950,"context_line":"                    iv_enc \u003d iv_cipher.encryptor()"}],"source_content_type":"text/x-python","patch_set":1,"id":"c6a4d17c_b38dbb6e","line":1947,"updated":"2026-03-18 13:48:13.000000000","message":"As above. I guess this is S305?\n\n\n\n```suggestion\n                        ciphers.modes.ECB(),  # noqa: S305\n```","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ffda4d57097130e24bcc9f584b598a7efc3dd01f","unresolved":false,"context_lines":[{"line_number":1944,"context_line":"                    # Encrypt the sector number to get the IV"},{"line_number":1945,"context_line":"                    iv_cipher \u003d ciphers.Cipher("},{"line_number":1946,"context_line":"                        ciphers.algorithms.AES(iv_key),"},{"line_number":1947,"context_line":"                        ciphers.modes.ECB(),  # noqa"},{"line_number":1948,"context_line":"                        backend\u003ddefault_backend(),"},{"line_number":1949,"context_line":"                    )"},{"line_number":1950,"context_line":"                    iv_enc \u003d iv_cipher.encryptor()"}],"source_content_type":"text/x-python","patch_set":1,"id":"caadc1f7_b2869be5","line":1947,"in_reply_to":"c6a4d17c_b38dbb6e","updated":"2026-03-18 14:02:04.000000000","message":"Acknowledged","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9e504e49902ea48c6cfc16da712cb804f0ce8e3f","unresolved":true,"context_lines":[{"line_number":1957,"context_line":""},{"line_number":1958,"context_line":"                cipher \u003d ciphers.Cipher("},{"line_number":1959,"context_line":"                    ciphers.algorithms.AES(key),"},{"line_number":1960,"context_line":"                    ciphers.modes.CBC(iv),  # type: ignore[arg-type]"},{"line_number":1961,"context_line":"                    backend\u003ddefault_backend(),"},{"line_number":1962,"context_line":"                )"},{"line_number":1963,"context_line":"                decryptor \u003d cipher.decryptor()"}],"source_content_type":"text/x-python","patch_set":1,"id":"32858a36_3ba550fd","line":1960,"updated":"2026-03-18 13:48:13.000000000","message":"This is happening because `Cipher` is a generic that has already been defined and bound to `ciphers.modes.XTS` above (on line 1908). If you rename this to e.g. `cipher_cbc` this `type: ignore` won\u0027t be needed.","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ffda4d57097130e24bcc9f584b598a7efc3dd01f","unresolved":true,"context_lines":[{"line_number":1957,"context_line":""},{"line_number":1958,"context_line":"                cipher \u003d ciphers.Cipher("},{"line_number":1959,"context_line":"                    ciphers.algorithms.AES(key),"},{"line_number":1960,"context_line":"                    ciphers.modes.CBC(iv),  # type: ignore[arg-type]"},{"line_number":1961,"context_line":"                    backend\u003ddefault_backend(),"},{"line_number":1962,"context_line":"                )"},{"line_number":1963,"context_line":"                decryptor \u003d cipher.decryptor()"}],"source_content_type":"text/x-python","patch_set":1,"id":"f135a3aa_b41e790a","line":1960,"in_reply_to":"32858a36_3ba550fd","updated":"2026-03-18 14:02:04.000000000","message":"The two variables named `cipher` in the separate scopes that never interact have to be named differently because they end up as different types?","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d3328de0e688d657282f5d026f79d32577244498","unresolved":true,"context_lines":[{"line_number":1957,"context_line":""},{"line_number":1958,"context_line":"                cipher \u003d ciphers.Cipher("},{"line_number":1959,"context_line":"                    ciphers.algorithms.AES(key),"},{"line_number":1960,"context_line":"                    ciphers.modes.CBC(iv),  # type: ignore[arg-type]"},{"line_number":1961,"context_line":"                    backend\u003ddefault_backend(),"},{"line_number":1962,"context_line":"                )"},{"line_number":1963,"context_line":"                decryptor \u003d cipher.decryptor()"}],"source_content_type":"text/x-python","patch_set":1,"id":"71ce181a_74d97b9e","line":1960,"in_reply_to":"bf0703a8_22b58b83","updated":"2026-03-18 14:38:36.000000000","message":"\u003e or (b) split this into e.g. `_decrypt_data_aes_xts` and `_decrypt_data_aes_cbc`.\n\nThis is probably a good idea anyway, so I\u0027ll just do this.","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"aa9cd3b6c058895198c1004394657fbd5ea23a6f","unresolved":true,"context_lines":[{"line_number":1957,"context_line":""},{"line_number":1958,"context_line":"                cipher \u003d ciphers.Cipher("},{"line_number":1959,"context_line":"                    ciphers.algorithms.AES(key),"},{"line_number":1960,"context_line":"                    ciphers.modes.CBC(iv),  # type: ignore[arg-type]"},{"line_number":1961,"context_line":"                    backend\u003ddefault_backend(),"},{"line_number":1962,"context_line":"                )"},{"line_number":1963,"context_line":"                decryptor \u003d cipher.decryptor()"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf0703a8_22b58b83","line":1960,"in_reply_to":"f135a3aa_b41e790a","updated":"2026-03-18 14:33:11.000000000","message":"Unfortunately Python\u0027s scoping rules are weird and unlike e.g. Go variables bleed out of conditionals and loops. mypy can\u0027t be sure of the scope of a variable defined inside such a conditional/loop for effectively the same reason linters like flake8 can\u0027t flag code like this:\n\n```\ndef foo():\n    if False:\n        x \u003d \u0027test\u0027\n    print(x)\n```\n\nThe alternatives to renaming this are (a) to make `cipher` less specific before/as part of its first declaration on line 1908 above:\n\n```\ncipher: ciphers.Cipher[ciphers.modes.CBC | ciphers.modes.XTS]\ncipher \u003d ...\n```\n\nor (b) split this into e.g. `_decrypt_data_aes_xts` and `_decrypt_data_aes_cbc`.","commit_id":"7c729ad0e3f61d6990b412253835f53cc88caa68"}],"requirements.txt":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"94928ccdc1b6a071b4d4a432f1cc3be24f44d45c","unresolved":true,"context_lines":[{"line_number":7,"context_line":"# adding a new feature to oslo.utils means adding a new dependency,"},{"line_number":8,"context_line":"# that is a likely indicator that the feature belongs somewhere else."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"cryptography\u003e\u003d43.0.3  # BSD/Apache-2.0"},{"line_number":11,"context_line":"iso8601\u003e\u003d0.1.11 # MIT"},{"line_number":12,"context_line":"oslo.i18n\u003e\u003d3.15.3 # Apache-2.0"},{"line_number":13,"context_line":"netaddr\u003e\u003d0.10.0 # BSD"}],"source_content_type":"text/plain","patch_set":7,"id":"62277579_81c802a4","line":10,"updated":"2026-04-15 14:22:52.000000000","message":"Given dhellmann\u0027s comment above, I assume this is important enough to warrant making it a main requirements rather than placing it behind an extra?","commit_id":"a182d3f1e25e18344ec83eff026e49c5289fc5d8"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"201e7cf57babad8cf690960b285e876220bae180","unresolved":true,"context_lines":[{"line_number":7,"context_line":"# adding a new feature to oslo.utils means adding a new dependency,"},{"line_number":8,"context_line":"# that is a likely indicator that the feature belongs somewhere else."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"cryptography\u003e\u003d43.0.3  # BSD/Apache-2.0"},{"line_number":11,"context_line":"iso8601\u003e\u003d0.1.11 # MIT"},{"line_number":12,"context_line":"oslo.i18n\u003e\u003d3.15.3 # Apache-2.0"},{"line_number":13,"context_line":"netaddr\u003e\u003d0.10.0 # BSD"}],"source_content_type":"text/plain","patch_set":7,"id":"196640db_a80f1409","line":10,"in_reply_to":"1187e128_914a0109","updated":"2026-04-15 14:55:37.000000000","message":"\u003e Well, I guess I thought that was maybe for things that weren\u0027t in global requirements or were otherwise a bit too obscure.\n\nIt\u0027s more so users like novaclient don\u0027t end up with a bloated deps list. oslo.utils really is [very widely used](https://codesearch.opendev.org/?q\u003doslo.utils\u0026i\u003dnope\u0026literal\u003dnope\u0026files\u003drequirements%5C.txt\u0026excludeFiles\u003d\u0026repos\u003d).\n\n\u003e If the users of this library can require the extra instead of just getting oslo.utils potentially without the ability to do this, then I guess that\u0027s okay.\n\nWho uses this currently? Is it just nova and glance? If so, if they tweak their requirements to replace `oslo.utils\u003e\u003dx.y.z` with e.g. `oslo.utils[image-inspection]\u003e\u003dx.y.z` (or whatever you wanted to call the extra), they should get it transparently. If that\u0027s too painful we can leave this as-is, but I still thought it worth raising given the note.\n\nIf you did do this, you should also add `extras` to `tox.ini` so we pick it up during testing. See nova\u0027s `tox.ini` for examples.\n\n\u003e Obviously for something that is security-sensitive (i.e. you think it\u0027s checking a thing but it\u0027s not because some library isn\u0027t present) I\u0027d rather it be very obviously deterministic.\n\nGood point. However, so long as import as usual (rather than wrapping in a `try-except ImportError`), things should crash and burn as expected if the import is not found?","commit_id":"a182d3f1e25e18344ec83eff026e49c5289fc5d8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ca2fd4a5f07f1c0ec96bdfa856fff01269e81bc0","unresolved":true,"context_lines":[{"line_number":7,"context_line":"# adding a new feature to oslo.utils means adding a new dependency,"},{"line_number":8,"context_line":"# that is a likely indicator that the feature belongs somewhere else."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"cryptography\u003e\u003d43.0.3  # BSD/Apache-2.0"},{"line_number":11,"context_line":"iso8601\u003e\u003d0.1.11 # MIT"},{"line_number":12,"context_line":"oslo.i18n\u003e\u003d3.15.3 # Apache-2.0"},{"line_number":13,"context_line":"netaddr\u003e\u003d0.10.0 # BSD"}],"source_content_type":"text/plain","patch_set":7,"id":"cafa6ed8_7caa008e","line":10,"in_reply_to":"196640db_a80f1409","updated":"2026-04-15 15:08:20.000000000","message":"\u003e It\u0027s more so users like novaclient don\u0027t end up with a bloated deps list. oslo.utils really is [very widely used](https://codesearch.opendev.org/?q\u003doslo.utils\u0026i\u003dnope\u0026literal\u003dnope\u0026files\u003drequirements%5C.txt\u0026excludeFiles\u003d\u0026repos\u003d).\n\nYeah, understood.\n\n\u003e Who uses this currently? Is it just nova and glance?\n\nNova, Glance, Ironic, at least. Cinder is supposed to be using it although they have not fully converted from their initial copies of the glance code (despite me proposing a change for them to). However, with the glance image encryption work slated for the next cycle, I\u0027m sure they\u0027ll need to move.\n\n\u003e If so, if they tweak their requirements to replace `oslo.utils\u003e\u003dx.y.z` with e.g. `oslo.utils[image-inspection]\u003e\u003dx.y.z` (or whatever you wanted to call the extra), they should get it transparently. If that\u0027s too painful we can leave this as-is, but I still thought it worth raising given the note.\n\nAck, that works for pip but presumably it might be a bit more work for the distro packagers as well? Either way, as long as we can make it deterministic, just tell me what you want.\n\n\u003e Good point. However, so long as import as usual (rather than wrapping in a `try-except ImportError`), things should crash and burn as expected if the import is not found?\n\nYes, but I assumed you were expecting me to make it optional with that sort of thing as well if it is an optional dep. Are you thinking that we only import cryptography if the caller passes a passphrase and lights up all those code paths? Or that format_inspector itself isn\u0027t import-able at all if cryptography is missing?\n\nI\u0027ll also note that the glance image encryption stuff will probably be adding things to osc (and maybe glanceclient) to allow easy uploading of encrypted images with the metadata set properly and thus they may be adding that dep there anyway.\n\nBut yeah, as long as we can make it required where it needs to be, just tell me how you want it to be.","commit_id":"a182d3f1e25e18344ec83eff026e49c5289fc5d8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c16d072782a01c18c0d01b36c64859c5f801e211","unresolved":true,"context_lines":[{"line_number":7,"context_line":"# adding a new feature to oslo.utils means adding a new dependency,"},{"line_number":8,"context_line":"# that is a likely indicator that the feature belongs somewhere else."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"cryptography\u003e\u003d43.0.3  # BSD/Apache-2.0"},{"line_number":11,"context_line":"iso8601\u003e\u003d0.1.11 # MIT"},{"line_number":12,"context_line":"oslo.i18n\u003e\u003d3.15.3 # Apache-2.0"},{"line_number":13,"context_line":"netaddr\u003e\u003d0.10.0 # BSD"}],"source_content_type":"text/plain","patch_set":7,"id":"1187e128_914a0109","line":10,"in_reply_to":"62277579_81c802a4","updated":"2026-04-15 14:27:22.000000000","message":"Well, I guess I thought that was maybe for things that weren\u0027t in global requirements or were otherwise a bit too obscure. If the users of this library can require the extra instead of just getting oslo.utils potentially without the ability to do this, then I guess that\u0027s okay. Obviously for something that is security-sensitive (i.e. you think it\u0027s checking a thing but it\u0027s not because some library isn\u0027t present) I\u0027d rather it be very obviously deterministic.","commit_id":"a182d3f1e25e18344ec83eff026e49c5289fc5d8"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d84cc57d6824e8f6fe3380942e89d09895f470c6","unresolved":true,"context_lines":[{"line_number":7,"context_line":"# adding a new feature to oslo.utils means adding a new dependency,"},{"line_number":8,"context_line":"# that is a likely indicator that the feature belongs somewhere else."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"cryptography\u003e\u003d43.0.3  # BSD/Apache-2.0"},{"line_number":11,"context_line":"iso8601\u003e\u003d0.1.11 # MIT"},{"line_number":12,"context_line":"oslo.i18n\u003e\u003d3.15.3 # Apache-2.0"},{"line_number":13,"context_line":"netaddr\u003e\u003d0.10.0 # BSD"}],"source_content_type":"text/plain","patch_set":7,"id":"0ef9d41e_e0b52539","line":10,"in_reply_to":"cafa6ed8_7caa008e","updated":"2026-04-15 17:18:51.000000000","message":"Okay, if you\u0027ve no strong preference then let\u0027s add an extra like `image-inspection` (open to alternative names) and add the `extras` option in `tox.ini`. This will cause anyone importing `oslo_utils.imageutils` without that library installed to explode so if we find that causes more explosions than expected we can change tack (but I suspect that\u0027s unlikely).","commit_id":"a182d3f1e25e18344ec83eff026e49c5289fc5d8"}]}