From 997a4f722cfc87890f5375f6d3c89c0e8cf1a62a Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Thu, 23 Jul 2026 16:23:36 -0700 Subject: [PATCH 1/5] Validate PsrpOperator parameters after rendering command, powershell, cmdlet, arguments and parameters are template fields, rendered after __init__ runs. The constructor validated their combination and derived task_id from cmdlet, all reading the un-rendered Jinja expressions. Move the validation into execute(), which runs after rendering. This drops the cmdlet-derived task_id default (a construction-time read of a template field that cannot move): task_id must now be passed explicitly when using cmdlet. related: #70296 Signed-off-by: 1fanwang <1fannnw@gmail.com> --- providers/microsoft/psrp/docs/changelog.rst | 11 ++++++++++ .../microsoft/psrp/operators/psrp.py | 20 +++++++++---------- .../microsoft/psrp/operators/test_psrp.py | 11 ++++------ .../validate_operators_init_exemptions.txt | 1 - 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/providers/microsoft/psrp/docs/changelog.rst b/providers/microsoft/psrp/docs/changelog.rst index cf1576c90c878..3aa1eafa5f56b 100644 --- a/providers/microsoft/psrp/docs/changelog.rst +++ b/providers/microsoft/psrp/docs/changelog.rst @@ -27,6 +27,17 @@ Changelog --------- +Main +.... + +Breaking changes +~~~~~~~~~~~~~~~~~ + +* ``PsrpOperator`` no longer derives ``task_id`` from ``cmdlet``. ``cmdlet`` is a template field, + so validating and reading it in ``__init__`` operated on the un-rendered Jinja expression; + the parameter validation now runs in ``execute()``. Pass ``task_id`` explicitly when using + ``cmdlet``. + 3.2.6 ..... diff --git a/providers/microsoft/psrp/src/airflow/providers/microsoft/psrp/operators/psrp.py b/providers/microsoft/psrp/src/airflow/providers/microsoft/psrp/operators/psrp.py index 4b64dc6dac578..94921b306ff24 100644 --- a/providers/microsoft/psrp/src/airflow/providers/microsoft/psrp/operators/psrp.py +++ b/providers/microsoft/psrp/src/airflow/providers/microsoft/psrp/operators/psrp.py @@ -56,8 +56,7 @@ class PsrpOperator(BaseOperator): :param command: command to execute on remote host. (templated) :param powershell: powershell to execute on remote host. (templated) :param cmdlet: - cmdlet to execute on remote host (templated). Also used as the default - value for `task_id`. + cmdlet to execute on remote host (templated). :param arguments: When using the `cmdlet` or `powershell` option, use `arguments` to provide arguments (templated). @@ -106,15 +105,6 @@ def __init__( psrp_session_init: Command | None = None, **kwargs, ) -> None: - args = {command, powershell, cmdlet} - if not exactly_one(*args): - raise ValueError("Must provide exactly one of 'command', 'powershell', or 'cmdlet'") - if arguments and not (powershell or cmdlet): - raise ValueError("Arguments only allowed with 'powershell' or 'cmdlet'") - if parameters and not (powershell or cmdlet): - raise ValueError("Parameters only allowed with 'powershell' or 'cmdlet'") - if cmdlet: - kwargs.setdefault("task_id", cmdlet) super().__init__(**kwargs) self.conn_id = psrp_conn_id self.command = command @@ -128,6 +118,14 @@ def __init__( self.psrp_session_init = psrp_session_init def execute(self, context: Context) -> list[Any] | None: + # command/powershell/cmdlet/arguments/parameters are template fields; validate their + # combination here, after rendering, rather than in __init__. + if not exactly_one(*{self.command, self.powershell, self.cmdlet}): + raise ValueError("Must provide exactly one of 'command', 'powershell', or 'cmdlet'") + if self.arguments and not (self.powershell or self.cmdlet): + raise ValueError("Arguments only allowed with 'powershell' or 'cmdlet'") + if self.parameters and not (self.powershell or self.cmdlet): + raise ValueError("Parameters only allowed with 'powershell' or 'cmdlet'") with ( PsrpHook( self.conn_id, diff --git a/providers/microsoft/psrp/tests/unit/microsoft/psrp/operators/test_psrp.py b/providers/microsoft/psrp/tests/unit/microsoft/psrp/operators/test_psrp.py index 8d2fb922d6ed2..4e028f49a9714 100644 --- a/providers/microsoft/psrp/tests/unit/microsoft/psrp/operators/test_psrp.py +++ b/providers/microsoft/psrp/tests/unit/microsoft/psrp/operators/test_psrp.py @@ -41,12 +41,9 @@ class ExecuteParameter(NamedTuple): class TestPsrpOperator: def test_no_command_or_powershell(self): exception_msg = "Must provide exactly one of 'command', 'powershell', or 'cmdlet'" + op = PsrpOperator(task_id="test_task_id", psrp_conn_id=CONNECTION_ID) with pytest.raises(ValueError, match=exception_msg): - PsrpOperator(task_id="test_task_id", psrp_conn_id=CONNECTION_ID) - - def test_cmdlet_task_id_default(self): - operator = PsrpOperator(cmdlet="Invoke-Foo", psrp_conn_id=CONNECTION_ID) - assert operator.task_id == "Invoke-Foo" + op.execute(None) @pytest.mark.parametrize("do_xcom_push", [True, False]) @pytest.mark.parametrize( @@ -116,14 +113,14 @@ def test_execute(self, hook_impl, parameter, had_errors, rc, do_xcom_push): assert ps.mock_calls == expected_ps_calls def test_securestring_sandboxed(self): - op = PsrpOperator(psrp_conn_id=CONNECTION_ID, cmdlet="test") + op = PsrpOperator(task_id="test_task_id", psrp_conn_id=CONNECTION_ID, cmdlet="test") template = op.get_template_env().from_string("{{ 'foo' | securestring }}") with pytest.raises(AirflowException): template.render() @patch.object(BaseOperator, "get_template_env") def test_securestring_native(self, get_template_env): - op = PsrpOperator(psrp_conn_id=CONNECTION_ID, cmdlet="test") + op = PsrpOperator(task_id="test_task_id", psrp_conn_id=CONNECTION_ID, cmdlet="test") get_template_env.return_value = NativeEnvironment() template = op.get_template_env().from_string("{{ 'foo' | securestring }}") rendered = template.render() diff --git a/scripts/ci/prek/validate_operators_init_exemptions.txt b/scripts/ci/prek/validate_operators_init_exemptions.txt index 01fd5bc56dbb7..3f46eb21a5dcc 100644 --- a/scripts/ci/prek/validate_operators_init_exemptions.txt +++ b/scripts/ci/prek/validate_operators_init_exemptions.txt @@ -76,7 +76,6 @@ providers/google/src/airflow/providers/google/marketing_platform/operators/campa providers/microsoft/azure/src/airflow/providers/microsoft/azure/sensors/compute.py::AzureVirtualMachineStateSensor providers/microsoft/azure/src/airflow/providers/microsoft/azure/transfers/gcs_to_wasb.py::GCSToAzureBlobStorageOperator providers/microsoft/azure/src/airflow/providers/microsoft/azure/transfers/oracle_to_azure_data_lake.py::OracleToAzureDataLakeOperator -providers/microsoft/psrp/src/airflow/providers/microsoft/psrp/operators/psrp.py::PsrpOperator providers/neo4j/src/airflow/providers/neo4j/operators/neo4j.py::Neo4jOperator providers/oracle/src/airflow/providers/oracle/transfers/oracle_to_oracle.py::OracleToOracleOperator providers/papermill/src/airflow/providers/papermill/operators/papermill.py::PapermillOperator From f755696348d98d6806bcc33581f59a5edeee4c90 Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Thu, 23 Jul 2026 23:11:42 -0700 Subject: [PATCH 2/5] Tighten PsrpOperator validation comment Signed-off-by: 1fanwang <1fannnw@gmail.com> --- .../src/airflow/providers/microsoft/psrp/operators/psrp.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/providers/microsoft/psrp/src/airflow/providers/microsoft/psrp/operators/psrp.py b/providers/microsoft/psrp/src/airflow/providers/microsoft/psrp/operators/psrp.py index 94921b306ff24..000a7a63c1cf1 100644 --- a/providers/microsoft/psrp/src/airflow/providers/microsoft/psrp/operators/psrp.py +++ b/providers/microsoft/psrp/src/airflow/providers/microsoft/psrp/operators/psrp.py @@ -118,8 +118,7 @@ def __init__( self.psrp_session_init = psrp_session_init def execute(self, context: Context) -> list[Any] | None: - # command/powershell/cmdlet/arguments/parameters are template fields; validate their - # combination here, after rendering, rather than in __init__. + # These are template fields; validate their combination after rendering, not in __init__. if not exactly_one(*{self.command, self.powershell, self.cmdlet}): raise ValueError("Must provide exactly one of 'command', 'powershell', or 'cmdlet'") if self.arguments and not (self.powershell or self.cmdlet): From 7c652dc22d884e4f5aed46bbeb95b61ceec09090 Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Fri, 24 Jul 2026 11:33:10 -0700 Subject: [PATCH 3/5] Drop narrating comment from PsrpOperator Signed-off-by: 1fanwang <1fannnw@gmail.com> --- .../psrp/src/airflow/providers/microsoft/psrp/operators/psrp.py | 1 - 1 file changed, 1 deletion(-) diff --git a/providers/microsoft/psrp/src/airflow/providers/microsoft/psrp/operators/psrp.py b/providers/microsoft/psrp/src/airflow/providers/microsoft/psrp/operators/psrp.py index 000a7a63c1cf1..511f6ac45991c 100644 --- a/providers/microsoft/psrp/src/airflow/providers/microsoft/psrp/operators/psrp.py +++ b/providers/microsoft/psrp/src/airflow/providers/microsoft/psrp/operators/psrp.py @@ -118,7 +118,6 @@ def __init__( self.psrp_session_init = psrp_session_init def execute(self, context: Context) -> list[Any] | None: - # These are template fields; validate their combination after rendering, not in __init__. if not exactly_one(*{self.command, self.powershell, self.cmdlet}): raise ValueError("Must provide exactly one of 'command', 'powershell', or 'cmdlet'") if self.arguments and not (self.powershell or self.cmdlet): From e10bff0b540d0fd69bba55b46d7befe1fb8f0aff Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Fri, 24 Jul 2026 11:36:16 -0700 Subject: [PATCH 4/5] Cover PsrpOperator arguments/parameters validation at execute Signed-off-by: 1fanwang <1fannnw@gmail.com> --- .../tests/unit/microsoft/psrp/operators/test_psrp.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/providers/microsoft/psrp/tests/unit/microsoft/psrp/operators/test_psrp.py b/providers/microsoft/psrp/tests/unit/microsoft/psrp/operators/test_psrp.py index 4e028f49a9714..e96b95a42983e 100644 --- a/providers/microsoft/psrp/tests/unit/microsoft/psrp/operators/test_psrp.py +++ b/providers/microsoft/psrp/tests/unit/microsoft/psrp/operators/test_psrp.py @@ -45,6 +45,18 @@ def test_no_command_or_powershell(self): with pytest.raises(ValueError, match=exception_msg): op.execute(None) + @pytest.mark.parametrize( + ("kwargs", "match"), + [ + ({"command": "hostname", "arguments": ["x"]}, "Arguments only allowed"), + ({"command": "hostname", "parameters": {"k": "v"}}, "Parameters only allowed"), + ], + ) + def test_arguments_and_parameters_require_powershell_or_cmdlet(self, kwargs, match): + op = PsrpOperator(task_id="test_task_id", psrp_conn_id=CONNECTION_ID, **kwargs) + with pytest.raises(ValueError, match=match): + op.execute(None) + @pytest.mark.parametrize("do_xcom_push", [True, False]) @pytest.mark.parametrize( ("had_errors", "rc"), [(False, 0), (False, None), (True, None), (False, 1), (True, 1)] From 2afaa93c2c73bf408205301062590f78b0689cf9 Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Sat, 25 Jul 2026 00:54:37 -0700 Subject: [PATCH 5/5] Add render-then-execute regression test for PsrpOperator validation command is a template field, so validating it in __init__ checked the raw Jinja expression, not the rendered value. Add a test that renders a command resolving to an empty string, then executes: it fails on the pre-fix source (the empty command slips past __init__ and reaches execution) and passes with validation in execute(). Signed-off-by: 1fanwang <1fannnw@gmail.com> --- .../tests/unit/microsoft/psrp/operators/test_psrp.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/providers/microsoft/psrp/tests/unit/microsoft/psrp/operators/test_psrp.py b/providers/microsoft/psrp/tests/unit/microsoft/psrp/operators/test_psrp.py index e96b95a42983e..26fbdfdd84a1a 100644 --- a/providers/microsoft/psrp/tests/unit/microsoft/psrp/operators/test_psrp.py +++ b/providers/microsoft/psrp/tests/unit/microsoft/psrp/operators/test_psrp.py @@ -57,6 +57,16 @@ def test_arguments_and_parameters_require_powershell_or_cmdlet(self, kwargs, mat with pytest.raises(ValueError, match=match): op.execute(None) + def test_command_validated_after_rendering(self): + # command is a template field: __init__-time validation saw the raw Jinja (truthy), + # so an expression that renders to an empty string slipped through. The check must + # run on the rendered value in execute(). + op = PsrpOperator(task_id="test", psrp_conn_id=CONNECTION_ID, command="{{ '' }}") + op.render_template_fields({}) + assert op.command == "" + with pytest.raises(ValueError, match="exactly one"): + op.execute(None) + @pytest.mark.parametrize("do_xcom_push", [True, False]) @pytest.mark.parametrize( ("had_errors", "rc"), [(False, 0), (False, None), (True, None), (False, 1), (True, 1)]