)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"eb378dd2a82921cac43eb30bcc95a4de90d32a24","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"209fee4f_c21b0a85","updated":"2024-01-25 11:03:13.000000000","message":"I\u0027d rather suggest that we use a instance property, instead of adding an argument to the check method. We also have set_rules where _need_check_rule is enabled. We already have similar knobs such as self.suppress_default_change_warnings .\n\nAlso, I\u0027m wondering if it makes sense to add separate interfaces for undefined check and cyclic check. IIUC what you want to disable is only undefined check. Is that correct ?","commit_id":"1253b7a558efc02db8f5c953d8ffcb4dbafc2499"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e00bc52e4595895a283730c986516014a6b06ae0","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"8a400916_f999b912","in_reply_to":"209fee4f_c21b0a85","updated":"2024-01-25 13:00:11.000000000","message":"Hi Takashi. Please check [0] for more info. In a nutshell, in Neutron we create a role enforcer only for three roles (admin, srv and API). The problem is that we also load the default Neutron policy file, if any of these roles has been modified. Along with that, the other rules are also loaded but discarded by the enforcer. In this method, if new rules are loaded, they will be checked. In case of having undefined checks, we\u0027ll have WARNING messages in the log that could confuse the users.\n\nI tried a different approach in [1]. The problem is that I realize we not only accept one single policy file but also policy.d; this is considered in \"Enforcer.load_rules\" but not in my patch. Actually I think that re-implemeting locally in neutron-lib a policy parser and groomer could clash with further new implementations of \"Enforcer\".\n\nSo the point of this patch is to avoid this log.warning message printed in the logs, instead of, in neutron-lib, to re-implement a big chunk of the policy parsing code.\n\nCould be possible a third alternative? To separate, in oslo.policy, the parsing methods to have an isolated method and accessible from outside. That could be used by n-lib to parse, groom and create a temporary limited configuration.\n\n[0]https://bugs.launchpad.net/neutron/+bug/2048198\n[1]https://review.opendev.org/c/openstack/neutron-lib/+/906539","commit_id":"1253b7a558efc02db8f5c953d8ffcb4dbafc2499"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"8c58de352b1c1e49ea85712773ed9e16e1dcc19e","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b94978b8_97008b73","in_reply_to":"8a400916_f999b912","updated":"2024-01-30 11:59:20.000000000","message":"I understand that problem and agree with adding a flag to skip checking undefined rules. My suggestion is more about the way to implement the flag. See https://review.opendev.org/c/openstack/oslo.policy/+/907196 . With this you can call `_ROLE_ENFORCER.skip_undefined_check \u003d False` before `_ROLE_ENFORCER.load_rules(True)` .","commit_id":"1253b7a558efc02db8f5c953d8ffcb4dbafc2499"}],"oslo_policy/policy.py":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"8c58de352b1c1e49ea85712773ed9e16e1dcc19e","unresolved":true,"context_lines":[{"line_number":993,"context_line":"            ``True``.  Note: for rules using the \"case\" expression, this"},{"line_number":994,"context_line":"            ``True`` value will be the specified string from the expression."},{"line_number":995,"context_line":"        \"\"\""},{"line_number":996,"context_line":"        self.load_rules()"},{"line_number":997,"context_line":""},{"line_number":998,"context_line":"        if isinstance(creds, context.RequestContext):"},{"line_number":999,"context_line":"            creds \u003d self._map_context_attributes_into_creds(creds)"}],"source_content_type":"text/x-python","patch_set":1,"id":"79ea2aff_d4cfeee2","line":996,"updated":"2024-01-30 11:59:20.000000000","message":"A problem with the current version is that it may work for initial load but does not work for automatic reload upon policy file change (this enforce is called from authorize which is callsed by neutron-lib)\n\nIf we add the argument to load_rules then we also have to add the same to enforce, but I think adding the single flag as a class property is a much simpler way. See the link in the other comment.","commit_id":"1253b7a558efc02db8f5c953d8ffcb4dbafc2499"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"a22e7f5ab17a3d3ced6038dc7ff1e3399dd5caa7","unresolved":true,"context_lines":[{"line_number":993,"context_line":"            ``True``.  Note: for rules using the \"case\" expression, this"},{"line_number":994,"context_line":"            ``True`` value will be the specified string from the expression."},{"line_number":995,"context_line":"        \"\"\""},{"line_number":996,"context_line":"        self.load_rules()"},{"line_number":997,"context_line":""},{"line_number":998,"context_line":"        if isinstance(creds, context.RequestContext):"},{"line_number":999,"context_line":"            creds \u003d self._map_context_attributes_into_creds(creds)"}],"source_content_type":"text/x-python","patch_set":1,"id":"f50692ea_efd6e5d2","line":996,"in_reply_to":"79ea2aff_d4cfeee2","updated":"2024-01-30 12:00:28.000000000","message":"Adding -1 to highlight the missing argument in enforce (and then probably authorize) which I mentioned above.","commit_id":"1253b7a558efc02db8f5c953d8ffcb4dbafc2499"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"491e0966bda02f9ce18096899cd237aa1f8f7b80","unresolved":false,"context_lines":[{"line_number":993,"context_line":"            ``True``.  Note: for rules using the \"case\" expression, this"},{"line_number":994,"context_line":"            ``True`` value will be the specified string from the expression."},{"line_number":995,"context_line":"        \"\"\""},{"line_number":996,"context_line":"        self.load_rules()"},{"line_number":997,"context_line":""},{"line_number":998,"context_line":"        if isinstance(creds, context.RequestContext):"},{"line_number":999,"context_line":"            creds \u003d self._map_context_attributes_into_creds(creds)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1283b5c7_49a25e19","line":996,"in_reply_to":"f50692ea_efd6e5d2","updated":"2024-01-30 12:10:27.000000000","message":"Thanks for the alternative proposed: https://review.opendev.org/c/openstack/oslo.policy/+/907196","commit_id":"1253b7a558efc02db8f5c953d8ffcb4dbafc2499"}]}