From e3b813e27edac4cad573c0a1788cb60d41d9e535 Mon Sep 17 00:00:00 2001 From: "Chandan Kumar (raukadah)" Date: Fri, 4 Jul 2025 17:57:30 +0530 Subject: [PATCH] Drop Code related to OperationNotPermitted exception The following exception was added in initial import of watcher code base[1]. In each of the controller REST APIs, it was called with a flag stating request was coming from top level resources apis. But this exception and code was not used anywhere in the rest api. It seems to be a dead code. So, it needs to be cleaned up. Note: In audit_template, under patchapi, this exception was used for not removal goal from audit template. Since this cr drops this exception, It replace the same with NotAuthorized exception keeping status code same. Links: [1]. https://github.com/openstack/watcher/commit/d14e057da15e1b6636c8a277bfc397a926e01569#diff-6d510a275605e20ba8b435157062da2b749265a88a3cfd6d90abb7e8e5feac2aR235 Closes-Bug: #2115968 Change-Id: I82a5e4a7a51726b3a89257c84a75157fbfcb82eb Signed-off-by: Chandan Kumar (raukadah) --- ...-not-permitted-exception-14e49b35a3ca00d1.yaml | 9 +++++++++ watcher/api/controllers/v1/action.py | 7 ------- watcher/api/controllers/v1/action_plan.py | 10 ---------- watcher/api/controllers/v1/audit.py | 13 ------------- watcher/api/controllers/v1/audit_template.py | 15 +-------------- watcher/api/controllers/v1/data_model.py | 6 ------ watcher/api/controllers/v1/goal.py | 7 ------- watcher/api/controllers/v1/scoring_engine.py | 8 -------- watcher/api/controllers/v1/service.py | 7 ------- watcher/api/controllers/v1/strategy.py | 7 ------- watcher/common/exception.py | 4 ---- watcher/tests/api/v1/test_audit_templates.py | 2 +- 12 files changed, 11 insertions(+), 84 deletions(-) create mode 100644 releasenotes/notes/drop-operation-not-permitted-exception-14e49b35a3ca00d1.yaml diff --git a/releasenotes/notes/drop-operation-not-permitted-exception-14e49b35a3ca00d1.yaml b/releasenotes/notes/drop-operation-not-permitted-exception-14e49b35a3ca00d1.yaml new file mode 100644 index 000000000..9cdeb6272 --- /dev/null +++ b/releasenotes/notes/drop-operation-not-permitted-exception-14e49b35a3ca00d1.yaml @@ -0,0 +1,9 @@ +--- +other: + - | + Removed unused ``OperationNotPermitted`` exception that was dead code + since the initial import of the Watcher codebase. This exception was + not used anywhere in the REST API controllers except for preventing + goal removal from audit templates. The functionality has been replaced + with the standard ``wsme.exc.ClientSideError`` exception, which provides + the appropriate 400 Bad Request response behavior. diff --git a/watcher/api/controllers/v1/action.py b/watcher/api/controllers/v1/action.py index 54971cb96..163975b0f 100644 --- a/watcher/api/controllers/v1/action.py +++ b/watcher/api/controllers/v1/action.py @@ -230,10 +230,6 @@ class ActionsController(rest.RestController): def __init__(self): super(ActionsController, self).__init__() - from_actions = False - """A flag to indicate if the requests to this controller are coming - from the top-level resource Actions.""" - _custom_actions = { 'detail': ['GET'], } @@ -351,9 +347,6 @@ class ActionsController(rest.RestController): :param action_uuid: UUID of a action. """ - if self.from_actions: - raise exception.OperationNotPermitted - context = pecan.request.context action = api_utils.get_resource('Action', action_uuid) policy.enforce(context, 'action:get', action, action='action:get') diff --git a/watcher/api/controllers/v1/action_plan.py b/watcher/api/controllers/v1/action_plan.py index a0e7db92e..8c77cac20 100644 --- a/watcher/api/controllers/v1/action_plan.py +++ b/watcher/api/controllers/v1/action_plan.py @@ -337,10 +337,6 @@ class ActionPlansController(rest.RestController): super(ActionPlansController, self).__init__() self.applier_client = rpcapi.ApplierAPI() - from_actionsPlans = False - """A flag to indicate if the requests to this controller are coming - from the top-level resource ActionPlan.""" - _custom_actions = { 'start': ['POST'], 'detail': ['GET'] @@ -450,9 +446,6 @@ class ActionPlansController(rest.RestController): :param action_plan_uuid: UUID of a action plan. """ - if self.from_actionsPlans: - raise exception.OperationNotPermitted - context = pecan.request.context action_plan = api_utils.get_resource('ActionPlan', action_plan_uuid) policy.enforce( @@ -492,9 +485,6 @@ class ActionPlansController(rest.RestController): :param action_plan_uuid: UUID of a action plan. :param patch: a json PATCH document to apply to this action plan. """ - if self.from_actionsPlans: - raise exception.OperationNotPermitted - context = pecan.request.context action_plan_to_update = api_utils.get_resource( 'ActionPlan', action_plan_uuid, eager=True) diff --git a/watcher/api/controllers/v1/audit.py b/watcher/api/controllers/v1/audit.py index 4ef21acec..e58b26b57 100644 --- a/watcher/api/controllers/v1/audit.py +++ b/watcher/api/controllers/v1/audit.py @@ -479,10 +479,6 @@ class AuditsController(rest.RestController): super(AuditsController, self).__init__() self.dc_client = rpcapi.DecisionEngineAPI() - from_audits = False - """A flag to indicate if the requests to this controller are coming - from the top-level resource Audits.""" - _custom_actions = { 'detail': ['GET'], } @@ -594,9 +590,6 @@ class AuditsController(rest.RestController): :param audit: UUID or name of an audit. """ - if self.from_audits: - raise exception.OperationNotPermitted - context = pecan.request.context rpc_audit = api_utils.get_resource('Audit', audit) policy.enforce(context, 'audit:get', rpc_audit, action='audit:get') @@ -615,9 +608,6 @@ class AuditsController(rest.RestController): action='audit:create') audit = audit_p.as_audit(context) - if self.from_audits: - raise exception.OperationNotPermitted - strategy_uuid = audit.strategy_uuid no_schema = True if strategy_uuid is not None: @@ -673,9 +663,6 @@ class AuditsController(rest.RestController): :param audit: UUID or name of an audit. :param patch: a json PATCH document to apply to this audit. """ - if self.from_audits: - raise exception.OperationNotPermitted - context = pecan.request.context audit_to_update = api_utils.get_resource( 'Audit', audit, eager=True) diff --git a/watcher/api/controllers/v1/audit_template.py b/watcher/api/controllers/v1/audit_template.py index 1d6de34c7..461a164ec 100644 --- a/watcher/api/controllers/v1/audit_template.py +++ b/watcher/api/controllers/v1/audit_template.py @@ -208,7 +208,7 @@ class AuditTemplatePatchType(types.JsonPatchType): if patch.path == "/goal" and patch.op != "remove": AuditTemplatePatchType._validate_goal(patch) elif patch.path == "/goal" and patch.op == "remove": - raise exception.OperationNotPermitted( + raise wsme.exc.ClientSideError( _("Cannot remove 'goal' attribute " "from an audit template")) if patch.path == "/strategy": @@ -479,10 +479,6 @@ class AuditTemplatesController(rest.RestController): def __init__(self): super(AuditTemplatesController, self).__init__() - from_audit_templates = False - """A flag to indicate if the requests to this controller are coming - from the top-level resource AuditTemplates.""" - _custom_actions = { 'detail': ['GET'], } @@ -606,9 +602,6 @@ class AuditTemplatesController(rest.RestController): :param audit_template: UUID or name of an audit template. """ - if self.from_audit_templates: - raise exception.OperationNotPermitted - context = pecan.request.context rpc_audit_template = api_utils.get_resource('AuditTemplate', audit_template) @@ -626,9 +619,6 @@ class AuditTemplatesController(rest.RestController): :param audit_template_postdata: the audit template POST data from the request body. """ - if self.from_audit_templates: - raise exception.OperationNotPermitted - context = pecan.request.context policy.enforce(context, 'audit_template:create', action='audit_template:create') @@ -654,9 +644,6 @@ class AuditTemplatesController(rest.RestController): :param template_uuid: UUID of a audit template. :param patch: a json PATCH document to apply to this audit template. """ - if self.from_audit_templates: - raise exception.OperationNotPermitted - context = pecan.request.context audit_template_to_update = api_utils.get_resource('AuditTemplate', audit_template) diff --git a/watcher/api/controllers/v1/data_model.py b/watcher/api/controllers/v1/data_model.py index 5a039e891..a3c794ade 100644 --- a/watcher/api/controllers/v1/data_model.py +++ b/watcher/api/controllers/v1/data_model.py @@ -36,10 +36,6 @@ class DataModelController(rest.RestController): def __init__(self): super(DataModelController, self).__init__() - from_data_model = False - """A flag to indicate if the requests to this controller are coming - from the top-level resource DataModel.""" - @wsme_pecan.wsexpose(wtypes.text, wtypes.text, types.uuid) def get_all(self, data_model_type='compute', audit_uuid=None): """Retrieve information about the given data model. @@ -53,8 +49,6 @@ class DataModelController(rest.RestController): """ if not utils.allow_list_datamodel(): raise exception.NotAcceptable - if self.from_data_model: - raise exception.OperationNotPermitted allowed_data_model_type = [ 'compute', ] diff --git a/watcher/api/controllers/v1/goal.py b/watcher/api/controllers/v1/goal.py index 11371eeee..cdd9d7c3b 100644 --- a/watcher/api/controllers/v1/goal.py +++ b/watcher/api/controllers/v1/goal.py @@ -157,10 +157,6 @@ class GoalsController(rest.RestController): def __init__(self): super(GoalsController, self).__init__() - from_goals = False - """A flag to indicate if the requests to this controller are coming - from the top-level resource Goals.""" - _custom_actions = { 'detail': ['GET'], } @@ -232,9 +228,6 @@ class GoalsController(rest.RestController): :param goal: UUID or name of the goal. """ - if self.from_goals: - raise exception.OperationNotPermitted - context = pecan.request.context rpc_goal = api_utils.get_resource('Goal', goal) policy.enforce(context, 'goal:get', rpc_goal, action='goal:get') diff --git a/watcher/api/controllers/v1/scoring_engine.py b/watcher/api/controllers/v1/scoring_engine.py index 7846335e2..06f5c0322 100644 --- a/watcher/api/controllers/v1/scoring_engine.py +++ b/watcher/api/controllers/v1/scoring_engine.py @@ -149,10 +149,6 @@ class ScoringEngineController(rest.RestController): def __init__(self): super(ScoringEngineController, self).__init__() - from_scoring_engines = False - """A flag to indicate if the requests to this controller are coming - from the top-level resource Scoring Engines.""" - _custom_actions = { 'detail': ['GET'], } @@ -240,10 +236,6 @@ class ScoringEngineController(rest.RestController): context = pecan.request.context policy.enforce(context, 'scoring_engine:get', action='scoring_engine:get') - - if self.from_scoring_engines: - raise exception.OperationNotPermitted - rpc_scoring_engine = api_utils.get_resource( 'ScoringEngine', scoring_engine) diff --git a/watcher/api/controllers/v1/service.py b/watcher/api/controllers/v1/service.py index 1ea7ae91f..1199c58cc 100644 --- a/watcher/api/controllers/v1/service.py +++ b/watcher/api/controllers/v1/service.py @@ -179,10 +179,6 @@ class ServicesController(rest.RestController): def __init__(self): super(ServicesController, self).__init__() - from_services = False - """A flag to indicate if the requests to this controller are coming - from the top-level resource Services.""" - _custom_actions = { 'detail': ['GET'], } @@ -253,9 +249,6 @@ class ServicesController(rest.RestController): :param service: ID or name of the service. """ - if self.from_services: - raise exception.OperationNotPermitted - context = pecan.request.context rpc_service = api_utils.get_resource('Service', service) policy.enforce(context, 'service:get', rpc_service, diff --git a/watcher/api/controllers/v1/strategy.py b/watcher/api/controllers/v1/strategy.py index 063c16377..90754226d 100644 --- a/watcher/api/controllers/v1/strategy.py +++ b/watcher/api/controllers/v1/strategy.py @@ -200,10 +200,6 @@ class StrategiesController(rest.RestController): def __init__(self): super(StrategiesController, self).__init__() - from_strategies = False - """A flag to indicate if the requests to this controller are coming - from the top-level resource Strategies.""" - _custom_actions = { 'detail': ['GET'], 'state': ['GET'], @@ -327,9 +323,6 @@ class StrategiesController(rest.RestController): :param strategy: UUID or name of the strategy. """ - if self.from_strategies: - raise exception.OperationNotPermitted - context = pecan.request.context rpc_strategy = api_utils.get_resource('Strategy', strategy) policy.enforce(context, 'strategy:get', rpc_strategy, diff --git a/watcher/common/exception.py b/watcher/common/exception.py index 405e4cd77..d246f16f9 100644 --- a/watcher/common/exception.py +++ b/watcher/common/exception.py @@ -127,10 +127,6 @@ class PolicyNotAuthorized(NotAuthorized): msg_fmt = _("Policy doesn't allow %(action)s to be performed.") -class OperationNotPermitted(NotAuthorized): - msg_fmt = _("Operation not permitted") - - class Invalid(WatcherException, ValueError): msg_fmt = _("Unacceptable parameters") code = HTTPStatus.BAD_REQUEST diff --git a/watcher/tests/api/v1/test_audit_templates.py b/watcher/tests/api/v1/test_audit_templates.py index 7b138e664..639f55108 100644 --- a/watcher/tests/api/v1/test_audit_templates.py +++ b/watcher/tests/api/v1/test_audit_templates.py @@ -505,7 +505,7 @@ class TestPatch(FunctionalTestWithSetup): '/audit_templates/%s' % self.audit_template.uuid, [{'path': '/goal', 'op': 'remove'}], expect_errors=True) - self.assertEqual(HTTPStatus.FORBIDDEN, response.status_code) + self.assertEqual(HTTPStatus.BAD_REQUEST, response.status_code) self.assertEqual('application/json', response.content_type) self.assertTrue(response.json['error_message'])