)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":38562,"name":"Richard Cruise","email":"rcruise@redhat.com","username":"rcruise"},"change_message_id":"40c740d8befd278fca4f6a207e9eee869b549a3e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f43b494c_5dd35c3f","updated":"2026-10-01 10:41:05.000000000","message":"I think there\u0027s a few issues here with how the duplicates are found, it seems some rule updates will be given a 400 error when they should succeed\n\nReviewed with help from GPT 5.5","commit_id":"ac0cf5e1dc8ed40b708c9cc06c808f2cf45337f8"}],"octavia/api/v2/controllers/l7rule.py":[{"author":{"_account_id":38562,"name":"Richard Cruise","email":"rcruise@redhat.com","username":"rcruise"},"change_message_id":"40c740d8befd278fca4f6a207e9eee869b549a3e","unresolved":true,"context_lines":[{"line_number":134,"context_line":"        except odb_exceptions.DBError as e:"},{"line_number":135,"context_line":"            raise exceptions.APIException() from e"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    def _validate_l7rule_duplicates(self, lock_session, l7rule):"},{"line_number":138,"context_line":"        if isinstance(l7rule, dict):"},{"line_number":139,"context_line":"            l7rule_dict \u003d l7rule"},{"line_number":140,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"e54b6cc9_4c1131da","line":137,"updated":"2026-10-01 10:41:05.000000000","message":"GPT found an issue which I\u0027ve looked through and I think is valid. Short version, because the l7rule is included with the duplicate check, then updates to anything other than the filter_keys will result in a 400 error. This is probably incorrect\n\nHere\u0027s the full output from AI if it helps:\n```\nRoot Cause\nThe duplicate validation query checks for existing rules matching the proposed signature (type, compare_type, key, value, invert) within the same L7 policy, but it does not exclude the current rule\u0027s ID during update operations. \nOn a PUT request:\n1. new_l7rule contains the current rule\u0027s data including its ID\n2. The validation queries for rules matching the signature without excluding the current ID\n3. It finds the rule itself and incorrectly flags it as a duplicate\n4. Valid metadata-only updates (e.g., changing only admin_state_up) fail with HTTP 400\nConcrete Example\nConsider an existing L7 rule:\n{\n  \"id\": \"rule-123\",\n  \"type\": \"PATH\",\n  \"compare_type\": \"STARTS_WITH\",\n  \"key\": null,\n  \"value\": \"/api\",\n  \"invert\": false,\n  \"admin_state_up\": true,\n  \"tags\": [\"production\"]\n}\nValid metadata-only update request:\n{\n  \"admin_state_up\": false\n}\nWhat happens:\n1. Validation checks for duplicate rules with signature: \n- type\u003dPATH, compare_type\u003dSTARTS_WITH, key\u003dnull, value\u003d/api, invert\u003dfalse\n2. Query finds the existing rule (ID: rule-123) which matches the signature\n3. Since current ID is not excluded, it treats this as a duplicate\n4. Request fails with HTTP 400: \"Found existing duplicate l7rule\"\n\nRecommended Fix\nExclude the current rule\u0027s ID during duplicate validation for update operations:\n1. Query all rules matching the signature in the policy\n2. Reject only if any matching rule has a different ID than the rule being updated\n3. Add functional tests for metadata-only updates (tags-only, admin_state_up-only)\n4. Test with legacy duplicate rows to ensure proper handling\n```","commit_id":"ac0cf5e1dc8ed40b708c9cc06c808f2cf45337f8"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"0daeea3af3f38c48675388a7ea8c7411d5be8136","unresolved":true,"context_lines":[{"line_number":134,"context_line":"        except odb_exceptions.DBError as e:"},{"line_number":135,"context_line":"            raise exceptions.APIException() from e"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    def _validate_l7rule_duplicates(self, lock_session, l7rule):"},{"line_number":138,"context_line":"        if isinstance(l7rule, dict):"},{"line_number":139,"context_line":"            l7rule_dict \u003d l7rule"},{"line_number":140,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"b9e29c05_4de65aea","line":137,"in_reply_to":"e54b6cc9_4c1131da","updated":"2026-10-01 10:51:12.000000000","message":"TLDR: basically \"openstack loadbalancer l7rule set --disable \u003cl7policy_id\u003e \u003cl7rule_id\u003e\" doesn\u0027t work","commit_id":"ac0cf5e1dc8ed40b708c9cc06c808f2cf45337f8"},{"author":{"_account_id":38562,"name":"Richard Cruise","email":"rcruise@redhat.com","username":"rcruise"},"change_message_id":"40c740d8befd278fca4f6a207e9eee869b549a3e","unresolved":true,"context_lines":[{"line_number":147,"context_line":"        }"},{"line_number":148,"context_line":"        filters[\u0027show_deleted\u0027] \u003d False"},{"line_number":149,"context_line":"        filters[\u0027l7policy_id\u0027] \u003d self.l7policy_id"},{"line_number":150,"context_line":"        duplicate_l7rule \u003d self.repositories.l7rule.get("},{"line_number":151,"context_line":"            lock_session, **filters)"},{"line_number":152,"context_line":"        if duplicate_l7rule:"},{"line_number":153,"context_line":"            err \u003d exceptions.InvalidL7Rule("}],"source_content_type":"text/x-python","patch_set":3,"id":"965cb7e1_5a2ce6ef","line":150,"updated":"2026-10-01 10:41:05.000000000","message":"There could be multiple duplicates here given there was no check before. I think a single row get isn\u0027t enough. I\u0027d suggest doing a multi row query, and then filtering out the id of l7rule that you passed in. I think this would also fix the issue noted above\n\nI got GPT to generate a suggestion, although I\u0027d recommend using this as a guide rather than a fix:\n```\nduplicate_l7rules \u003d session.query(models.L7Rule).filter_by(\n    l7policy_id\u003dself.l7policy_id,\n    type\u003dl7rule.type,\n    compare_type\u003dl7rule.compare_type,\n    key\u003dl7rule.key,\n    value\u003dl7rule.value,\n    invert\u003dl7rule.invert,\n).filter(\n    models.L7Rule.provisioning_status !\u003d constants.DELETED,\n    models.L7Rule.id !\u003d l7rule.id,\n).all()\n\nif duplicate_l7rules:\n    err \u003d exceptions.InvalidL7Rule(\n        \"Found existing duplicate l7rules %s\" %\n            [dup_rule.to_dict() for dup_rule in duplicate_l7rules]\n        )\n    raise exceptions.L7RuleValidation(error\u003derr)\n```","commit_id":"ac0cf5e1dc8ed40b708c9cc06c808f2cf45337f8"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"3c158edd9c3583662632245f8d1b91082a7ab911","unresolved":true,"context_lines":[{"line_number":152,"context_line":"        if duplicate_l7rule:"},{"line_number":153,"context_line":"            err \u003d exceptions.InvalidL7Rule("},{"line_number":154,"context_line":"                \"Found existing duplicate l7rule %s\" %"},{"line_number":155,"context_line":"                duplicate_l7rule.to_dict()"},{"line_number":156,"context_line":"            )"},{"line_number":157,"context_line":"            raise exceptions.L7RuleValidation(error\u003derr)"},{"line_number":158,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"235f802a_e7eea41e","line":155,"range":{"start_line":155,"start_character":16,"end_line":155,"end_character":40},"updated":"2026-10-01 10:55:17.000000000","message":"it displays a dict, not really human-friendly IMHO, the ID would be enough, wdyt?","commit_id":"ac0cf5e1dc8ed40b708c9cc06c808f2cf45337f8"}],"octavia/common/validate.py":[{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"3c158edd9c3583662632245f8d1b91082a7ab911","unresolved":true,"context_lines":[{"line_number":125,"context_line":""},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"def l7rules_intersections(l7rules, l7policy_id):"},{"line_number":128,"context_line":"    filter_keys \u003d (\u0027type\u0027, \u0027compare_type\u0027, \u0027key\u0027, \u0027value\u0027, \u0027invert\u0027)"},{"line_number":129,"context_line":"    # find duplicates based on l7 rule options"},{"line_number":130,"context_line":"    uniq_rules \u003d {"},{"line_number":131,"context_line":"        tuple(l7rule.get(key) for key in filter_keys)"}],"source_content_type":"text/x-python","patch_set":3,"id":"c801b801_433656ea","line":128,"range":{"start_line":128,"start_character":19,"end_line":128,"end_character":67},"updated":"2026-10-01 10:55:17.000000000","message":"IMHO we should use the constants defined in octavia_lib","commit_id":"ac0cf5e1dc8ed40b708c9cc06c808f2cf45337f8"}],"octavia/tests/functional/api/v2/test_l7rule.py":[{"author":{"_account_id":38562,"name":"Richard Cruise","email":"rcruise@redhat.com","username":"rcruise"},"change_message_id":"40c740d8befd278fca4f6a207e9eee869b549a3e","unresolved":true,"context_lines":[{"line_number":894,"context_line":"                  \u0027admin_state_up\u0027: True}"},{"line_number":895,"context_line":"        self.post(self.l7rules_path, self._build_body(l7rule), status\u003d403)"},{"line_number":896,"context_line":""},{"line_number":897,"context_line":"    def test_update_duplicates(self):"},{"line_number":898,"context_line":"        l7rule1 \u003d self.create_l7rule("},{"line_number":899,"context_line":"            self.l7policy_id, constants.L7RULE_TYPE_PATH,"},{"line_number":900,"context_line":"            constants.L7RULE_COMPARE_TYPE_STARTS_WITH,"}],"source_content_type":"text/x-python","patch_set":3,"id":"54601b4a_ae459d9c","line":897,"updated":"2026-10-01 10:41:05.000000000","message":"Based on my other comments, I\u0027d recommend adding tests cases for multiple existing duplicates as well as a metadata only update","commit_id":"ac0cf5e1dc8ed40b708c9cc06c808f2cf45337f8"}],"releasenotes/notes/validate-duplicate-l7-rules-3bbd52cf040fb28f.yaml":[{"author":{"_account_id":38562,"name":"Richard Cruise","email":"rcruise@redhat.com","username":"rcruise"},"change_message_id":"40c740d8befd278fca4f6a207e9eee869b549a3e","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Validate creation duplicate l7rules in the same l7policy."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"2fc6c221_d195eab2","line":4,"updated":"2026-10-01 10:41:05.000000000","message":"I\u0027d add a bit more details here, maybe discuss what keys are searched for a duplicate","commit_id":"ac0cf5e1dc8ed40b708c9cc06c808f2cf45337f8"}]}