mgr/orchestrator: reject spec-file input for oauth2-proxy apply

Align oauth2-proxy apply handler with other service-specific commands
(mds, rgw, nfs) by rejecting -i (inbuf) usage upfront.

Remove spec-file parsing path and construct OAuth2ProxySpec from
command-line args. Validation is delegated to spec.validate().

Update tests accordingly.

Fixes: https://tracker.ceph.com/issues/76372
Signed-off-by: Shubha Jain <SHUBHA.JAIN1@ibm.com>

diff --git a/src/pybind/mgr/orchestrator/module.py b/src/pybind/mgr/orchestrator/module.py
index 74f00c18caddbb02c092c63a056b68ab6627ef2d..0eda0cebc1fec01014e800a8ab8a00295ea3dc43 100644 (file)
--- a/src/pybind/mgr/orchestrator/module.py
+++ b/src/pybind/mgr/orchestrator/module.py
@@ -2106,38 +2106,14 @@ Usage:
                             no_overwrite: bool = False,
                             inbuf: Optional[str] = None) -> HandleCommandResult:
         """Add a cluster gateway service (cephadm only)"""
-        missing_oauth2_proxy_config = (
-            'Missing required configuration for oauth2-proxy. Please provide a spec '
-            'file with required fields: provider_display_name, oidc_issuer_url, '
-            'client_id, client_secret.'
-        )
-        if not inbuf or not inbuf.strip():
-            raise OrchestratorError(missing_oauth2_proxy_config)
-
-        try:
-            spec_data = yaml.safe_load(inbuf)
-        except (OSError, yaml.YAMLError):
-            raise OrchestratorValidationError('oauth2-proxy spec file must be valid YAML')
-
-        if not spec_data:
-            raise OrchestratorError(missing_oauth2_proxy_config)
-        if not isinstance(spec_data, dict):
-            raise OrchestratorValidationError(
-                'oauth2-proxy spec file must contain a single YAML object'
-            )
-
-        spec = ServiceSpec.from_json(spec_data)
-        if not isinstance(spec, OAuth2ProxySpec):
-            raise OrchestratorValidationError(
-                'oauth2-proxy spec file must define service_type: oauth2-proxy'
-            )
+        if inbuf:
+            raise OrchestratorValidationError('unrecognized command -i; -h or --help for usage')
 
-        if https_address is not None:
-            spec.https_address = https_address
-        if placement is not None:
-            spec.placement = PlacementSpec.from_string(placement)
-        if unmanaged:
-            spec.unmanaged = unmanaged
+        spec = OAuth2ProxySpec(
+            placement=PlacementSpec.from_string(placement),
+            unmanaged=unmanaged,
+            https_address=https_address,
+        )
         spec.preview_only = dry_run
 
         spec.validate()  # force any validation exceptions to be caught correctly

diff --git a/src/pybind/mgr/orchestrator/tests/test_orchestrator.py b/src/pybind/mgr/orchestrator/tests/test_orchestrator.py
index 67fbb59f08dbfca09139ddf9f4a3f57464629893..ecd901abbe4ddca888e87a878f34edc561cf4c34 100644 (file)
--- a/src/pybind/mgr/orchestrator/tests/test_orchestrator.py
+++ b/src/pybind/mgr/orchestrator/tests/test_orchestrator.py
@@ -359,18 +359,17 @@ class TestApplyOAuth2Proxy:
     def setup_method(self):
         self.m = OrchestratorCli('orchestrator', 0, 0)
 
-    def test_missing_spec_raises_clear_error(self, mock_apply_misc):
+    def test_missing_required_fields_raises_error(self, mock_apply_misc):
         res = self.m._apply_oauth2_proxy()
 
         assert res.retval != 0
         assert (
-            'Missing required configuration for oauth2-proxy. Please provide a spec file '
-            'with required fields: provider_display_name, oidc_issuer_url, client_id, '
-            'client_secret.'
+            'Missing required fields for oauth2-proxy: provider_display_name, '
+            'oidc_issuer_url, client_id, client_secret.'
         ) in res.stderr
         mock_apply_misc.assert_not_called()
 
-    def test_missing_required_fields_raises_combined_error(self, mock_apply_misc):
+    def test_inbuf_with_missing_fields_is_rejected(self, mock_apply_misc):
         res = self.m._apply_oauth2_proxy(inbuf=textwrap.dedent("""
             service_type: oauth2-proxy
             spec:
@@ -381,13 +380,11 @@ class TestApplyOAuth2Proxy:
 
         assert res.retval != 0
         assert (
-            'Missing required fields for oauth2-proxy: provider_display_name.'
+            'unrecognized command -i; -h or --help for usage'
         ) in res.stderr
         mock_apply_misc.assert_not_called()
 
-    def test_valid_spec_is_applied(self, mock_apply_misc):
-        mock_apply_misc.return_value = HandleCommandResult(retval=0, stdout="Success")
-
+    def test_inbuf_with_valid_spec_is_rejected(self, mock_apply_misc):
         res = self.m._apply_oauth2_proxy(inbuf=textwrap.dedent("""
             service_type: oauth2-proxy
             spec:
@@ -397,8 +394,11 @@ class TestApplyOAuth2Proxy:
               client_secret: "oauth-secret"
             """).strip())
 
-        assert res.retval == 0
-        mock_apply_misc.assert_called_once()
+        assert res.retval != 0
+        assert (
+            'unrecognized command -i; -h or --help for usage'
+        ) in res.stderr
+        mock_apply_misc.assert_not_called()
 
 
 @mock.patch("orchestrator.module.OrchestratorCli._apply_misc")