)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"857daac99e258ab5c0ab1b870776cb4cc8d59700","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"51a9fca6_8886deb0","updated":"2023-11-29 17:11:24.000000000","message":"LGTM, but you said you add another test case for \"DNS name only\".","commit_id":"2b9582f39267aff263364e2de7dfaefe4b31258b"},{"author":{"_account_id":6577,"name":"Sergey Kraynev","email":"sergejyit@gmail.com","username":"skraynev"},"change_message_id":"918a12491a8eb2f3de98b41e4c82a11434e411db","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"81ba95f1_6abbe3f9","updated":"2023-11-23 06:18:50.000000000","message":"Thank you for the fast fix.\nI have only important suggestion about extending docs with expectations about SSL certificates. In the bug you mentioned, that:\n```\nWe strive to be a standards based project, so the correct behavior should track back to standards documentation. In this case we are talking about the x.509 (ISO/IEC 9594-8:2020) ITU-T recommendation[1]\n```\n\nSo could you please add note about this requirement to docs?\n\nas I understand, https://docs.openstack.org/octavia/latest/admin/guides/certificates.html - is not a good candidate for it.\nSo this note potentially could be placed here: https://docs.openstack.org/octavia/latest/user/guides/basic-cookbook.html#basic-tls-terminated-listener \n\n\np.s. could this PR be backport candidate?","commit_id":"2b9582f39267aff263364e2de7dfaefe4b31258b"},{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"a1f09386ff0954f81699066502c8bf7c45da5c3a","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d3d65ea4_f82e4cfa","in_reply_to":"7a85739e_5ccab23c","updated":"2023-11-29 17:07:43.000000000","message":"The documentation mentions PKCS #12, which is a container for \"X.509 public key certificates, X.509 private keys, X.509 CRLs, generic data\"[1]. So I guess X.509 is implied in the documentation already.\n\nMaybe we can change the cookbook to say \"X.509 compliant certificate\" the first time it mentions the certificate in order to make it very clear. \n\n[1]: https://en.wikipedia.org/wiki/PKCS_12","commit_id":"2b9582f39267aff263364e2de7dfaefe4b31258b"},{"author":{"_account_id":6577,"name":"Sergey Kraynev","email":"sergejyit@gmail.com","username":"skraynev"},"change_message_id":"da98c9dfba59d5fe04fe57dc309aaadda903942a","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a78c7535_0dc0a0bd","in_reply_to":"81ba95f1_6abbe3f9","updated":"2023-11-23 06:20:07.000000000","message":"I mean, that without this note it could be not obvious, that certificate have to match specific requirements.","commit_id":"2b9582f39267aff263364e2de7dfaefe4b31258b"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"647b0fa0f23dd05214fa39b11296bcdb6b986efa","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f964aaac_5a913486","in_reply_to":"a78c7535_0dc0a0bd","updated":"2023-11-28 22:25:17.000000000","message":"I kind of disagree on this point. There are no specific requirements here beyond the standards that define what a certificate is. I don\u0027t think we should copy the whole x.509/SSL/TLS standard(s) into the Octavia docs.\n\nWhat do others think?\n\nAs for backport candidate, yes, I think this is possible to backport.","commit_id":"2b9582f39267aff263364e2de7dfaefe4b31258b"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"0f326d9c669e6d640d205b88ba830cae88792c94","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e6a886d7_07fa3641","in_reply_to":"d3d65ea4_f82e4cfa","updated":"2023-12-01 00:49:35.000000000","message":"The feature is called \"TLS\" and the documentation also notes that this feature is for TLS termination. The TLS RFCs all state that the certificate must be x.509 certificates (in fact it is more specific that they must be x.509v3).\nSee: https://datatracker.ietf.org/doc/html/rfc2246#section-7.4.2\n\nQuote from the RFC:\ncertificate_list\n       This is a sequence (chain) of X.509v3 certificates.\n       \nThis is a requirement of the protocol and has nothing to do with HAProxy nor Octavia.\nI still don\u0027t think we should say anything about x.509 in our docs as it\u0027s not anything specific about Octavia, it\u0027s a detail of the protocol itself.","commit_id":"2b9582f39267aff263364e2de7dfaefe4b31258b"},{"author":{"_account_id":6577,"name":"Sergey Kraynev","email":"sergejyit@gmail.com","username":"skraynev"},"change_message_id":"28b68b004626a85f1ee9679d84df4fa3db8e0690","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7e085a6f_98db184f","in_reply_to":"e6a886d7_07fa3641","updated":"2023-12-01 07:27:27.000000000","message":"heh. ok. I suppose, that with explicit note will be better :)\nHowever your logic is also valid and I could reference on it in future. thank you","commit_id":"2b9582f39267aff263364e2de7dfaefe4b31258b"},{"author":{"_account_id":6577,"name":"Sergey Kraynev","email":"sergejyit@gmail.com","username":"skraynev"},"change_message_id":"cda121c7abc33e34f8ac717d09588195a28bb1ef","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7a85739e_5ccab23c","in_reply_to":"f964aaac_5a913486","updated":"2023-11-29 03:30:07.000000000","message":"Hello, Michael. \nMy point was not about copy pasting all standards or requirements, sorry if it looked so.\nThe original issue, which I try to solve was mentioned in bug report:\n- octavia use haproxy and it allows to use \"incorrect\" according x509 standards.\n- there is no any notion about fact, that octavia follows x509 standards.\n\nSo I suggested to add couple words about last point with reference. It will remove any questions, like: should I use x509 standard for octavia, if it uses haproxy under the hood.","commit_id":"2b9582f39267aff263364e2de7dfaefe4b31258b"},{"author":{"_account_id":31664,"name":"Omer Schwartz","email":"oschwart@redhat.com","username":"oschwart"},"change_message_id":"daa1a49840a3fbf58e16cc06a3b252deef316536","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"11fd514d_c34e256c","updated":"2024-01-05 19:33:57.000000000","message":"Looks good to me","commit_id":"73cdee503ff1cb3419d8db0295b20b2b6ddf30c0"}],"octavia/tests/functional/api/v2/test_listener.py":[{"author":{"_account_id":6577,"name":"Sergey Kraynev","email":"sergejyit@gmail.com","username":"skraynev"},"change_message_id":"918a12491a8eb2f3de98b41e4c82a11434e411db","unresolved":true,"context_lines":[{"line_number":1044,"context_line":"        self.assertEqual(constants.CLIENT_AUTH_NONE,"},{"line_number":1045,"context_line":"                         listener_api.get(\u0027client_authentication\u0027))"},{"line_number":1046,"context_line":""},{"line_number":1047,"context_line":"    def test_create_tls_with_no_subject(self):"},{"line_number":1048,"context_line":"        tls_cert_mock \u003d mock.MagicMock()"},{"line_number":1049,"context_line":"        tls_cert_mock.get_certificate.return_value \u003d ("},{"line_number":1050,"context_line":"            sample_certs.NOCN_NOSUBALT_CRT)"}],"source_content_type":"text/x-python","patch_set":2,"id":"970de54f_ff5d079a","line":1047,"updated":"2023-11-23 06:18:50.000000000","message":"As I understand this test uses certificate without both CN and DNS names, but do we have couple tests with happy paths?\n- cert with only CN\n- cert with only DNS names\n- cert with both CN and DNS names\n\nI suppose, that at least one already used in tests, but I am not sure about other cases.","commit_id":"2b9582f39267aff263364e2de7dfaefe4b31258b"},{"author":{"_account_id":6577,"name":"Sergey Kraynev","email":"sergejyit@gmail.com","username":"skraynev"},"change_message_id":"cda121c7abc33e34f8ac717d09588195a28bb1ef","unresolved":true,"context_lines":[{"line_number":1044,"context_line":"        self.assertEqual(constants.CLIENT_AUTH_NONE,"},{"line_number":1045,"context_line":"                         listener_api.get(\u0027client_authentication\u0027))"},{"line_number":1046,"context_line":""},{"line_number":1047,"context_line":"    def test_create_tls_with_no_subject(self):"},{"line_number":1048,"context_line":"        tls_cert_mock \u003d mock.MagicMock()"},{"line_number":1049,"context_line":"        tls_cert_mock.get_certificate.return_value \u003d ("},{"line_number":1050,"context_line":"            sample_certs.NOCN_NOSUBALT_CRT)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ed39090e_a3ce9d1b","line":1047,"in_reply_to":"0077d59d_13931c32","updated":"2023-11-29 03:30:07.000000000","message":"awesome. I actually forgot to check tempest. \nSo if it\u0027s covered there, I am ok without your plan. \nthank you","commit_id":"2b9582f39267aff263364e2de7dfaefe4b31258b"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"647b0fa0f23dd05214fa39b11296bcdb6b986efa","unresolved":true,"context_lines":[{"line_number":1044,"context_line":"        self.assertEqual(constants.CLIENT_AUTH_NONE,"},{"line_number":1045,"context_line":"                         listener_api.get(\u0027client_authentication\u0027))"},{"line_number":1046,"context_line":""},{"line_number":1047,"context_line":"    def test_create_tls_with_no_subject(self):"},{"line_number":1048,"context_line":"        tls_cert_mock \u003d mock.MagicMock()"},{"line_number":1049,"context_line":"        tls_cert_mock.get_certificate.return_value \u003d ("},{"line_number":1050,"context_line":"            sample_certs.NOCN_NOSUBALT_CRT)"}],"source_content_type":"text/x-python","patch_set":2,"id":"0077d59d_13931c32","line":1047,"in_reply_to":"970de54f_ff5d079a","updated":"2023-11-28 22:25:17.000000000","message":"I think path #1 and #3 are covered in the tempest tests which use live certs and code paths. Digging in the tests I don\u0027t see a test with DNS names only. I will add one for that.","commit_id":"2b9582f39267aff263364e2de7dfaefe4b31258b"},{"author":{"_account_id":6577,"name":"Sergey Kraynev","email":"sergejyit@gmail.com","username":"skraynev"},"change_message_id":"28b68b004626a85f1ee9679d84df4fa3db8e0690","unresolved":false,"context_lines":[{"line_number":1044,"context_line":"        self.assertEqual(constants.CLIENT_AUTH_NONE,"},{"line_number":1045,"context_line":"                         listener_api.get(\u0027client_authentication\u0027))"},{"line_number":1046,"context_line":""},{"line_number":1047,"context_line":"    def test_create_tls_with_no_subject(self):"},{"line_number":1048,"context_line":"        tls_cert_mock \u003d mock.MagicMock()"},{"line_number":1049,"context_line":"        tls_cert_mock.get_certificate.return_value \u003d ("},{"line_number":1050,"context_line":"            sample_certs.NOCN_NOSUBALT_CRT)"}],"source_content_type":"text/x-python","patch_set":2,"id":"11aedd3d_16d6343d","line":1047,"in_reply_to":"ed39090e_a3ce9d1b","updated":"2023-12-01 07:27:27.000000000","message":"Acknowledged","commit_id":"2b9582f39267aff263364e2de7dfaefe4b31258b"}]}