)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6e2e99607c895e5878a6233c354be254feddb83d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6258f42b_c620eede","updated":"2025-12-05 21:39:19.000000000","message":"I\u0027d like you to think about going a different direction with this using current glance functionality.\n\nThere\u0027s a schema file that can be extended by operators.  The default is here:\nhttps://opendev.org/openstack/glance/src/branch/master/etc/schema-image.json\n(though if the deployment tooling doesn\u0027t make it available to glance-api, you won\u0027t see it).\n\nYou could modify that file for your cloud and make the properties you want to require as \u0027required\u0027.  Then that schema will be used by the RequestDeserializer to validate the user\u0027s image-create request.  Roughly, the path is:\nhttps://opendev.org/openstack/glance/src/commit/955d086873b9368090134aa01d5226f9b9fa19df/glance/api/v2/router.py#L38-L41\nhttps://opendev.org/openstack/glance/src/commit/955d086873b9368090134aa01d5226f9b9fa19df/glance/api/v2/router.py#L397\nhttps://opendev.org/openstack/glance/src/commit/955d086873b9368090134aa01d5226f9b9fa19df/glance/api/v2/images.py#L1279\nhttps://opendev.org/openstack/glance/src/commit/955d086873b9368090134aa01d5226f9b9fa19df/glance/api/v2/images.py#L2098-L2107\nhttps://opendev.org/openstack/glance/src/commit/955d086873b9368090134aa01d5226f9b9fa19df/glance/api/v2/images.py#L1300\n\nThe advantage is that these properties will show up in the image-schema-get response, so they are discoverable by your users.\n\nI don\u0027t know how widely used this functionality is, so instead of contributing code, you could maybe contribute some tests to make sure it works correctly (I think there are some functional tests you could adapt to verify that operator required properties are handled correctly), and some docs to make it clear to an operator how to make this work.\n\nThe main disadvantage I see with your current patch is that the required properties aren\u0027t discoverable programatically, whereas they are if you take advantage of glance\u0027s configurable schema functionality.","commit_id":"746a252e6c6a7679b8516efb98a10259fd65653e"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"7519e9a25f3289c8f94764784e64e20a0b84bb5c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3f8326c1_9f060bc7","updated":"2025-12-05 16:36:28.000000000","message":"masahito.muroi@lycorp.co.jp","commit_id":"746a252e6c6a7679b8516efb98a10259fd65653e"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"5f05b7b2ceb33c04eba6c4a29d7ce108c6319f22","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"71026029_4e0be430","in_reply_to":"6258f42b_c620eede","updated":"2025-12-07 15:50:12.000000000","message":"Hi Brian,\n\nThank you for the insightful feedback. I have investigated your suggestion to use schema-image.json instead of adding a new config option.\n\nI confirmed that adding os_tag to the schema file makes it discoverable via the GET /v2/schemas/image API, which addresses the discoverability concern perfectly.\n\nHowever, I found that the current schema loading logic cannot enforce the property as \"required\". The current load_custom_properties implementation treats the JSON file strictly as a flat dictionary of properties. It ignores top-level keys like required, so I cannot enforce the presence of os_tag just by modifying the file.\n\nSolution in this Patch Set: To solve this while maintaining the benefits of your suggestion, I have updated the patch to enhance the schema loading logic itself.\n\nThe changes in glance/api/v2/images.py:\n- Update load_custom_proaperties to support a structured JSON format: {\"properties\": {...}, \"required\": [...]}.\n- Pass the required list to the PermissiveSchema constructor to enable validation.\n- Maintain full backward compatibility by detecting the format; existing flat JSON files will continue to work as before.\n\nThis approach achieves both \"discoverability\" (via API) and \"enforcement\" (via validation) without adding ad-hoc configuration options.\n\nI believe this is the right direction for the project. Could you please review the updated implementation?","commit_id":"746a252e6c6a7679b8516efb98a10259fd65653e"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"2ee0613f7d8f65d1c832db6ad8a1d3b4947bc3b2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"32f648fd_03e2496f","updated":"2025-12-08 01:14:18.000000000","message":"I have tested this patch by adding extended schema format and the `required` for `os_tag` worked as expected.\nHere is sample for added schema-image.json and CLI result for new image creation.\n\nIt returned `400 Bad Request: Provided object does not match schema image: os_tag; is a required property`.\nHowever, as you can see the command response, it contains bunch of un-escaped HTML entities and this makes hard to understand the error message itself for user. This should be fixed in separated but fix commit.\n\n```\n# cat /etc/glance/schema-image.json\n{\n  \"properties\": {\n    \"os_tag\": {\n      \"type\": \"string\",\n      \"description\": \"Operating system tag (Mandatory)\",\n      \"maxLength\": 255\n    }\n  },\n  \"required\": [\"os_tag\"]\n}\n```\n\n```\n# openstack image create --disk-format raw --file tmpfile mtanino-required-test1\nBadRequestException: 400: Client Error for url: http://glance.test/v2/images, : 400 Bad Request: Provided object does not match schema \u0026#x27;image\u0026#x27;: \u0026#x27;os_tag\u0026#x27; is a required property: Failed validating \u0026#x27;required\u0026#x27; in schema:: {\u0026#x27;name\u0026#x27;: \u0026#x27;image\u0026#x27;,: \u0026#x27;properties\u0026#x27;: {\u0026#x27;id\u0026#x27;: {\u0026#x27;type\u0026#x27;: \u0026#x27;string\u0026#x27;,: \u0026#x27;description\u0026#x27;: \u0026#x27;An identifier for the image\u0026#x27;,: \u0026#x27;pattern\u0026#x27;: \u0026#x27;^([0-9a-fA-F]){8}-([0-9a-fA-F]){4}-([0-9a-fA-F]){4}-([0-9a-fA-F]){4}-([0-9a-fA-F]){12}$\u0026#x27;},: \u0026#x27;name\u0026#x27;: {\u0026#x27;type\u0026#x27;: [\u0026#x27;null\u0026#x27;, \u0026#x27;string\u0026#x27;],: \u0026#x27;description\u0026#x27;: \u0026#x27;Descriptive name for the \u0026#x27;: \u0026#x27;image\u0026#x27;,\n....\n\u0026#x27;owner_specified.openstack.object\u0026#x27;: \u0026#x27;images/mtanino-required-test1\u0026#x27;}\n```","commit_id":"8653053eb223e47c5e3591fec64307ba85a3a696"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"d981885f14fe1592f5115aa4851424244ac62fd8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"32c1bc58_01be291e","updated":"2026-01-05 15:10:29.000000000","message":"A suggestion inline, and to nitpick:\n\nThe following tests:\n\n    test_flat_format_without_required\n    test_extended_format_with_required\n    test_extended_format_with_multiple_required\n    test_extended_format_without_required_key\n    test_extended_format_with_empty_required\n    \nCould be name test_get_schema_* for consistency.","commit_id":"b89c6f7e9ada62bc927749a234853e221d8d0989"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"53ef293b4bf3adc5393b0d5adc7a615dc4086919","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"2a7b157e_3a7b64fd","updated":"2025-12-11 13:55:12.000000000","message":"I think this looks good, but I want to look over the tests a bit more.","commit_id":"b89c6f7e9ada62bc927749a234853e221d8d0989"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"3a7d7dcf606ebb0cf920dd892b037e29953db673","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"b8eb3d07_d61e23f3","in_reply_to":"2a7b157e_3a7b64fd","updated":"2025-12-11 14:17:24.000000000","message":"Thanks for your review. Sure, please take a time for the review.","commit_id":"b89c6f7e9ada62bc927749a234853e221d8d0989"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"906e1393008d2f0ccb396a51602ae69ee8a43523","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":6,"id":"eb6cecee_e1331433","in_reply_to":"b8eb3d07_d61e23f3","updated":"2025-12-18 23:57:52.000000000","message":"Hi rosmaita,\n\nJust a gentle ping on this. Did you have a chance to look over the tests? If you have any questions or if there are any additional test cases you\u0027d like me to add, please let me know.\n\nI\u0027d love to address any concerns before the holidays if possible.","commit_id":"b89c6f7e9ada62bc927749a234853e221d8d0989"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"b58bf58c69671a768c9ce8770e35506b6e00890d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"54491f96_69a06b7b","updated":"2026-01-06 15:32:20.000000000","message":"Please check updated commit with including release note.","commit_id":"9534380978c00e9d44ecb0b5a29d459c638d5827"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"e4d006d9997811ba8a27910b37045291db29a23e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"dca0bc8a_376699bd","updated":"2026-01-06 14:30:09.000000000","message":"Thanks!\n\nI forgot to ask you to add release notes regarding this change, mentioning that users can now use the extended format, and that backwards compatibility is maintained. Can you please do that? Sorry for forgetting about it during my previous review.","commit_id":"9534380978c00e9d44ecb0b5a29d459c638d5827"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"b58bf58c69671a768c9ce8770e35506b6e00890d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"c6224e34_354062c8","in_reply_to":"dca0bc8a_376699bd","updated":"2026-01-06 15:32:20.000000000","message":"@cyril@redhat.com\nThanks. I just added release notes. Please check it.","commit_id":"9534380978c00e9d44ecb0b5a29d459c638d5827"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"32c589e8cb33af0872176554b48b8a1c1a3d1293","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"642b6cb8_8a08223c","updated":"2026-01-27 15:43:39.000000000","message":"Apologies for the delay in getting back to this.  Really nice work!","commit_id":"c322704ba748be3a561f00eb54c8b4c3749f47de"},{"author":{"_account_id":33451,"name":"Yushiro Furukawa","display_name":"Yushiro Furukawa","email":"yushiro.furukawa@lycorp.co.jp","username":"yushiro2"},"change_message_id":"3b4024c1abb810342813baeff022fa0120791e59","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"6371995b_739ebf16","updated":"2026-01-13 00:52:55.000000000","message":"LGTM","commit_id":"c322704ba748be3a561f00eb54c8b4c3749f47de"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"5bb45d1d1bd931f70bf51133ef39416e8e416d01","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"2fde0535_90a1b00d","updated":"2026-01-28 00:42:27.000000000","message":"recheck","commit_id":"c322704ba748be3a561f00eb54c8b4c3749f47de"},{"author":{"_account_id":8878,"name":"Masahito Muroi","email":"masahito.muroi@linecorp.com","username":"masa"},"change_message_id":"ef8e50368bd6e4e1254b2a7af72b0491bbde80ab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"6a7da2c2_9812a170","updated":"2026-01-28 14:16:24.000000000","message":"recheck tempest-integrated-storage-import","commit_id":"c322704ba748be3a561f00eb54c8b4c3749f47de"}],"glance/tests/unit/v2/test_images_resource.py":[{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"d981885f14fe1592f5115aa4851424244ac62fd8","unresolved":true,"context_lines":[{"line_number":6465,"context_line":"        self.assertFalse("},{"line_number":6466,"context_line":"            schema.properties[\u0027architecture\u0027].get(\u0027is_base\u0027, True))"},{"line_number":6467,"context_line":""},{"line_number":6468,"context_line":"    def test_extended_format_with_multiple_required(self):"},{"line_number":6469,"context_line":"        \"\"\"Test extended format with multiple required fields\"\"\""},{"line_number":6470,"context_line":"        custom_properties \u003d {"},{"line_number":6471,"context_line":"            \u0027properties\u0027: {"}],"source_content_type":"text/x-python","patch_set":6,"id":"897dcabc_5ff940e3","line":6468,"range":{"start_line":6468,"start_character":8,"end_line":6468,"end_character":51},"updated":"2026-01-05 15:10:29.000000000","message":"I think this is basically the same test as test_extended_format_with_required, but the required field has 2 entries rather than 1? I think you could remove the test above and just keep this one.","commit_id":"b89c6f7e9ada62bc927749a234853e221d8d0989"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"b1a183c5a61308e849e728a6473a49135dab314f","unresolved":false,"context_lines":[{"line_number":6465,"context_line":"        self.assertFalse("},{"line_number":6466,"context_line":"            schema.properties[\u0027architecture\u0027].get(\u0027is_base\u0027, True))"},{"line_number":6467,"context_line":""},{"line_number":6468,"context_line":"    def test_extended_format_with_multiple_required(self):"},{"line_number":6469,"context_line":"        \"\"\"Test extended format with multiple required fields\"\"\""},{"line_number":6470,"context_line":"        custom_properties \u003d {"},{"line_number":6471,"context_line":"            \u0027properties\u0027: {"}],"source_content_type":"text/x-python","patch_set":6,"id":"9876d9ab_b25b8bde","line":6468,"range":{"start_line":6468,"start_character":8,"end_line":6468,"end_character":51},"in_reply_to":"897dcabc_5ff940e3","updated":"2026-01-06 10:38:47.000000000","message":"@cyril@redhat.com\n\nThank you for your review. I\u0027ve addressed all your comments.\nPlease review again whenever you have time.","commit_id":"b89c6f7e9ada62bc927749a234853e221d8d0989"}]}
