)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"e63632a78edcb67c8a7fd76da5e0ca907f8d7804","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"183f18f7_61744487","updated":"2022-02-23 06:40:49.000000000","message":"\u003e Patch Set 4: Code-Review-1\n\u003e \n\u003e (1 comment)\n\u003e \n\u003e I think this is good, but we should add a comment about L7 currently only being valid for HTTP in the API reference. This would clarify that the Octavia LBaaSv2 API currently doesn\u0027t support L7 for protocols other than HTTP based.\n\nMy latest patchset moved the validation from the API to the amphora driver.\nDo you think we still need to update the api-ref to mention that Octavia (the amphora driver) doesn\u0027t support L7 policies for non-HTTP protocols?\n","commit_id":"45b0a507a150721389833e33789b4cfdbc1a9590"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"3de088c5e5496e8ee38ca6cd25430919f33cb3eb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"ec51436c_e4abc994","updated":"2022-03-01 15:46:02.000000000","message":"I think this is good enough as-is. We can revisit the api-ref discussion at the PTG.","commit_id":"45b0a507a150721389833e33789b4cfdbc1a9590"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"c37b021b417c855a8d1a258cefe4d0b48f2fdc12","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"1d3871bb_6043c6ef","updated":"2022-02-23 06:41:10.000000000","message":"recheck","commit_id":"45b0a507a150721389833e33789b4cfdbc1a9590"}],"octavia/api/v2/controllers/l7policy.py":[{"author":{"_account_id":6469,"name":"Carlos Gonçalves","display_name":"Carlos Goncalves","email":"cgoncalves@redhat.com","username":"cgoncalves"},"change_message_id":"85a1da4503e60eddc6e20e3a1a730a20fac60f8e","unresolved":false,"context_lines":[{"line_number":97,"context_line":"    def _validate_create_l7policy(self, lock_session, l7policy_dict):"},{"line_number":98,"context_line":"        listener \u003d self._get_db_listener(lock_session,"},{"line_number":99,"context_line":"                                         l7policy_dict[\u0027listener_id\u0027])"},{"line_number":100,"context_line":"        if listener.protocol in (constants.PROTOCOL_UDP,"},{"line_number":101,"context_line":"                                 constants.PROTOCOL_TCP):"},{"line_number":102,"context_line":"            raise exceptions.ValidationException(detail\u003d_("},{"line_number":103,"context_line":"                \"%s protocol listener does not support L7 policies.\""}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_9eaed97e","line":100,"updated":"2020-07-10 14:38:17.000000000","message":"+ HTTPS? HTTPS is HTTPS passthrough so the URL is encrypted. We could get the server name present in the ClientHello packet if the SNI extension is enabled and so apply a subset of the L7 rules but I\u0027m not sure we can do that today in the amphora provider at least.","commit_id":"801bf965148ff304755ad335f372402852e7ad74"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"a7e4d26da2b654c63675cf77da673e62fe5c089d","unresolved":false,"context_lines":[{"line_number":97,"context_line":"    def _validate_create_l7policy(self, lock_session, l7policy_dict):"},{"line_number":98,"context_line":"        listener \u003d self._get_db_listener(lock_session,"},{"line_number":99,"context_line":"                                         l7policy_dict[\u0027listener_id\u0027])"},{"line_number":100,"context_line":"        if listener.protocol in (constants.PROTOCOL_UDP,"},{"line_number":101,"context_line":"                                 constants.PROTOCOL_TCP):"},{"line_number":102,"context_line":"            raise exceptions.ValidationException(detail\u003d_("},{"line_number":103,"context_line":"                \"%s protocol listener does not support L7 policies.\""}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_910a351d","line":100,"in_reply_to":"bf51134e_80ff85ff","updated":"2020-07-13 14:22:39.000000000","message":"IIRC it would require the HTTPS connection to be terminated and the payload decrypted, which is out of bounds for a \u0027passthrough\u0027 connection.","commit_id":"801bf965148ff304755ad335f372402852e7ad74"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"b3fa9e5bd9cc33dc7e1c0e5b0543e7625e7a386b","unresolved":false,"context_lines":[{"line_number":97,"context_line":"    def _validate_create_l7policy(self, lock_session, l7policy_dict):"},{"line_number":98,"context_line":"        listener \u003d self._get_db_listener(lock_session,"},{"line_number":99,"context_line":"                                         l7policy_dict[\u0027listener_id\u0027])"},{"line_number":100,"context_line":"        if listener.protocol in (constants.PROTOCOL_UDP,"},{"line_number":101,"context_line":"                                 constants.PROTOCOL_TCP):"},{"line_number":102,"context_line":"            raise exceptions.ValidationException(detail\u003d_("},{"line_number":103,"context_line":"                \"%s protocol listener does not support L7 policies.\""}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_718a011b","line":100,"in_reply_to":"bf51134e_910a351d","updated":"2020-07-13 14:44:57.000000000","message":"The api-ref says:\n\n\"The HTTPS protocol is HTTPS pass-through. For most providers, this is treated as a TCP protocol. Some advanced providers may support HTTPS session persistence features by using the session ID.\" https://docs.openstack.org/api-ref/load-balancer/v2/index.html#protocol-combinations-listener-pool\n\nSo my guess is that if a provider can support session persistence for HTTPS pass-through, it can also support some L7Rules on unencrypted data (certificate or SSL headers).","commit_id":"801bf965148ff304755ad335f372402852e7ad74"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"61f996510437c2003f294dff3cb69d2fbd26940a","unresolved":false,"context_lines":[{"line_number":97,"context_line":"    def _validate_create_l7policy(self, lock_session, l7policy_dict):"},{"line_number":98,"context_line":"        listener \u003d self._get_db_listener(lock_session,"},{"line_number":99,"context_line":"                                         l7policy_dict[\u0027listener_id\u0027])"},{"line_number":100,"context_line":"        if listener.protocol in (constants.PROTOCOL_UDP,"},{"line_number":101,"context_line":"                                 constants.PROTOCOL_TCP):"},{"line_number":102,"context_line":"            raise exceptions.ValidationException(detail\u003d_("},{"line_number":103,"context_line":"                \"%s protocol listener does not support L7 policies.\""}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_80ff85ff","line":100,"in_reply_to":"bf51134e_9eaed97e","updated":"2020-07-13 11:34:05.000000000","message":"Done.\nBut I\u0027m wondering if a 3rd party provider can implement l7 policies in a pass-through https LB.","commit_id":"801bf965148ff304755ad335f372402852e7ad74"},{"author":{"_account_id":6469,"name":"Carlos Gonçalves","display_name":"Carlos Goncalves","email":"cgoncalves@redhat.com","username":"cgoncalves"},"change_message_id":"407cc4b95c4be0b05e779ef1d3c555d5e59ac9b7","unresolved":false,"context_lines":[{"line_number":97,"context_line":"    def _validate_create_l7policy(self, lock_session, l7policy_dict):"},{"line_number":98,"context_line":"        listener \u003d self._get_db_listener(lock_session,"},{"line_number":99,"context_line":"                                         l7policy_dict[\u0027listener_id\u0027])"},{"line_number":100,"context_line":"        if (listener.protocol not in"},{"line_number":101,"context_line":"                constants.VALID_L7POLICY_LISTENER_PROTOCOLS):"},{"line_number":102,"context_line":"            raise exceptions.ValidationException(detail\u003d_("},{"line_number":103,"context_line":"                \"%s protocol listener does not support L7 policies.\""}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_c26d8199","line":100,"updated":"2020-07-29 07:36:24.000000000","message":"Putting more thought into this, and following-up to my earlier comment, this is a limitation to the amphora provider. Other providers may be able to do L7 policing (e.g. server name present in the ClientHello packet). So, it would be unfair to disallow others to support L7 policy on HTTPS listeners just because the amphora provider cannot do it today.\n\nBest would be to extend the amphora provider driver to validate this scenario and raise UnsupportedOptionError. For example, see how the it validates pool algorithms in https://github.com/openstack/octavia/blob/955bb8840616d96ed74de3086f8959ad4190a472/octavia/api/drivers/amphora_driver/v1/driver.py#L154","commit_id":"e86aaa1806223c6498654ad658e3b42501431290"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"2b3a040b19580d2624cb4f8d29771af11a11fa7f","unresolved":false,"context_lines":[{"line_number":97,"context_line":"    def _validate_create_l7policy(self, lock_session, l7policy_dict):"},{"line_number":98,"context_line":"        listener \u003d self._get_db_listener(lock_session,"},{"line_number":99,"context_line":"                                         l7policy_dict[\u0027listener_id\u0027])"},{"line_number":100,"context_line":"        if (listener.protocol not in"},{"line_number":101,"context_line":"                constants.VALID_L7POLICY_LISTENER_PROTOCOLS):"},{"line_number":102,"context_line":"            raise exceptions.ValidationException(detail\u003d_("},{"line_number":103,"context_line":"                \"%s protocol listener does not support L7 policies.\""}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_ead1e5cd","line":100,"in_reply_to":"9f560f44_c26d8199","updated":"2020-08-25 20:55:14.000000000","message":"This is a fair point and thank you for thinking of the drivers needs.\nI think this is a case where we need to decide what the API documents and supports vs. future features.\nWe know there are L7 cases for TLS, etc. in fact our engine supports them, it\u0027s just that feature hasn\u0027t been implemented yet.\nLooking at the L7 rules we support, I think the only ones that would match for non-http protocols would be the SSL_* rule types, but we also don\u0027t support TLS offload for TCP flows.\nCurrently we don\u0027t test these in any way either (API tests, etc.).\nSo, I think we also need to update the API documentation and be more explicit about the current capability of the API.\nWith that documentation, I am ok adding this, then in the future allowing a new feature that enables L7 processing of non-HTTP listeners.","commit_id":"e86aaa1806223c6498654ad658e3b42501431290"}],"octavia/tests/functional/api/v2/test_l7policy.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"fdcbf4fdc07c28a19164420a4b92a551191896fd","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    constants.PROTOCOL_HTTPS,"},{"line_number":32,"context_line":"    constants.PROTOCOL_TCP,"},{"line_number":33,"context_line":"    constants.PROTOCOL_UDP,"},{"line_number":34,"context_line":")"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"class TestL7Policy(base.BaseAPITest):"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_f032880f","line":34,"updated":"2020-07-13 18:53:42.000000000","message":"Is it worth putting in the constants file and using in l7policy.py too?","commit_id":"01b43d5d63e9bd68c780393ecb05d16aea17894e"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"d2652f6195e86b19f3b829a9fa2da80188a3ae88","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    constants.PROTOCOL_HTTPS,"},{"line_number":32,"context_line":"    constants.PROTOCOL_TCP,"},{"line_number":33,"context_line":"    constants.PROTOCOL_UDP,"},{"line_number":34,"context_line":")"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"class TestL7Policy(base.BaseAPITest):"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_2029dcf1","line":34,"in_reply_to":"bf51134e_54284b31","updated":"2020-07-21 13:49:47.000000000","message":"Done. (I added a list of valid protocols)","commit_id":"01b43d5d63e9bd68c780393ecb05d16aea17894e"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"261920ca8d9654457d64da2acaf668f42eaa0936","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    constants.PROTOCOL_HTTPS,"},{"line_number":32,"context_line":"    constants.PROTOCOL_TCP,"},{"line_number":33,"context_line":"    constants.PROTOCOL_UDP,"},{"line_number":34,"context_line":")"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"class TestL7Policy(base.BaseAPITest):"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_54284b31","line":34,"in_reply_to":"bf51134e_f032880f","updated":"2020-07-16 18:57:50.000000000","message":"+1\nI wonder if a \"valid\" list may be better/shorter in the long run. Just thinking about SCTP, etc.","commit_id":"01b43d5d63e9bd68c780393ecb05d16aea17894e"}],"releasenotes/notes/validate-protocols-for-l7policies-83d678171f13136a.yaml":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"cd2314b32f911046923263435add8999dbd232ed","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Validate that the creation of L7 policies is compatible with the protocol"},{"line_number":5,"context_line":"    of the listener. L7 policies are not allowed for HTTPS, TCP or UDP protocol"},{"line_number":6,"context_line":"    listeners."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"bf51134e_e2fdec08","line":4,"range":{"start_line":4,"start_character":46,"end_line":4,"end_character":48},"updated":"2020-07-23 21:13:32.000000000","message":"nit: are","commit_id":"904b68e20c97030972871d6af86103970d81ee4d"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"cd2314b32f911046923263435add8999dbd232ed","unresolved":false,"context_lines":[{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Validate that the creation of L7 policies is compatible with the protocol"},{"line_number":5,"context_line":"    of the listener. L7 policies are not allowed for HTTPS, TCP or UDP protocol"},{"line_number":6,"context_line":"    listeners."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"bf51134e_b706941f","line":6,"range":{"start_line":5,"start_character":21,"end_line":6,"end_character":14},"updated":"2020-07-23 21:13:32.000000000","message":"small nit if you have to update, but since the code is checking the positive, maybe list the supported protocols here, \"L7 policies are allowed for HTTP and Terminated HTTPS, but not for HTTPS, TCP or UDP protocol listeners.\"","commit_id":"904b68e20c97030972871d6af86103970d81ee4d"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"247fc79feaa05c58f984e6496bc9b47557a51af5","unresolved":false,"context_lines":[{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Validate that the creation of L7 policies is compatible with the protocol"},{"line_number":5,"context_line":"    of the listener. L7 policies are not allowed for HTTPS, TCP or UDP protocol"},{"line_number":6,"context_line":"    listeners."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"bf51134e_58492430","line":6,"range":{"start_line":5,"start_character":21,"end_line":6,"end_character":14},"in_reply_to":"bf51134e_b706941f","updated":"2020-07-24 06:49:44.000000000","message":"Done","commit_id":"904b68e20c97030972871d6af86103970d81ee4d"}]}