This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new b83461fef40 Fix PsrpOperator option checks (#70347)
b83461fef40 is described below
commit b83461fef4086752b4dd518bf68a1be02bb5e7c1
Author: Stefan Wang <[email protected]>
AuthorDate: Thu Aug 13 07:09:06 2026 -0700
Fix PsrpOperator option checks (#70347)
* 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 <[email protected]>
* Tighten PsrpOperator validation comment
Signed-off-by: 1fanwang <[email protected]>
* Drop narrating comment from PsrpOperator
Signed-off-by: 1fanwang <[email protected]>
* Cover PsrpOperator arguments/parameters validation at execute
Signed-off-by: 1fanwang <[email protected]>
* 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 <[email protected]>
* Fix PsrpOperator accepting duplicate command options
exactly_one deduped equal values before counting them, so passing the same
value to two of command/powershell/cmdlet passed validation. Pass the options
positionally so each is counted.
Drop the manual changelog note; the provider release manager regenerates
the changelog from git log.
Signed-off-by: 1fanwang <[email protected]>
* Validate PsrpOperator options by provision, not rendered value
The mutual-exclusivity and arguments/parameters checks are about which
options the Dag author provided, not what the templates render to. Keying them
off rendered truthiness dropped a provided option that rendered to a falsy
value, and let a second option slip through when the first rendered empty.
Check is-not-None (usage) instead, and dispatch the chosen option consistently.
Signed-off-by: 1fanwang <[email protected]>
* Keep PsrpOperator option checks in the constructor
Only the constructor can tell whether an option was passed: with
render_template_as_native_obj a provided field can render to None, so the
same
check in execute() reports a supplied argument as missing. Compare against
None
rather than truthiness, because a provided option can itself be empty.
related: https://github.com/apache/airflow/pull/70505
Signed-off-by: 1fanwang <[email protected]>
* Drop narrating comment from PsrpOperator constructor
Signed-off-by: 1fanwang <[email protected]>
* Keep the cmdlet-derived task_id default
Dropping it changes task identity for Dags that rely on it, so history,
logs, XComs and UI links all move. That is a breaking change and deserves
its own review rather than riding along with a validation fix.
Splitting it out lets the option checks land on their own and unblocks the
burn-down in https://github.com/apache/airflow/issues/70296, since
https://github.com/apache/airflow/pull/70656 was closed in favour of this
PR. The removal follows as a separate change with a provider changelog note.
Signed-off-by: 1fanwang <[email protected]>
---------
Signed-off-by: 1fanwang <[email protected]>
Co-authored-by: Shahar Epstein <[email protected]>
---
.../providers/microsoft/psrp/operators/psrp.py | 11 +++---
.../unit/microsoft/psrp/operators/test_psrp.py | 44 ++++++++++++++++++++--
2 files changed, 45 insertions(+), 10 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 4b64dc6dac5..673c99f4a37 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
@@ -106,12 +106,11 @@ class PsrpOperator(BaseOperator):
psrp_session_init: Command | None = None,
**kwargs,
) -> None:
- args = {command, powershell, cmdlet}
- if not exactly_one(*args):
+ if not exactly_one(command is not None, powershell is not None, cmdlet
is not None):
raise ValueError("Must provide exactly one of 'command',
'powershell', or 'cmdlet'")
- if arguments and not (powershell or cmdlet):
+ if arguments is not None and powershell is None and cmdlet is None:
raise ValueError("Arguments only allowed with 'powershell' or
'cmdlet'")
- if parameters and not (powershell or cmdlet):
+ if parameters is not None and powershell is None and cmdlet is None:
raise ValueError("Parameters only allowed with 'powershell' or
'cmdlet'")
if cmdlet:
kwargs.setdefault("task_id", cmdlet)
@@ -140,10 +139,10 @@ class PsrpOperator(BaseOperator):
):
if self.psrp_session_init is not None:
ps.add_command(self.psrp_session_init)
- if self.command:
+ if self.command is not None:
ps.add_script(f"cmd.exe /c @'\n{self.command}\n'@")
else:
- if self.cmdlet:
+ if self.cmdlet is not None:
ps.add_cmdlet(self.cmdlet)
else:
ps.add_script(self.powershell)
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 8d2fb922d6e..92039ec6f56 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
@@ -39,15 +39,51 @@ class ExecuteParameter(NamedTuple):
class TestPsrpOperator:
- def test_no_command_or_powershell(self):
- exception_msg = "Must provide exactly one of 'command', 'powershell',
or 'cmdlet'"
- with pytest.raises(ValueError, match=exception_msg):
- PsrpOperator(task_id="test_task_id", psrp_conn_id=CONNECTION_ID)
+ @pytest.mark.parametrize(
+ ("kwargs", "match"),
+ [
+ pytest.param({}, "exactly one", id="no-option"),
+ pytest.param({"command": "", "powershell": "Get-Bar"}, "exactly
one", id="two-options-one-empty"),
+ pytest.param({"command": "x", "powershell": "x"}, "exactly one",
id="two-equal-options"),
+ pytest.param(
+ {"command": "hostname", "arguments": ["x"]}, "Arguments only
allowed", id="arguments"
+ ),
+ pytest.param(
+ {"command": "hostname", "parameters": {"k": "v"}}, "Parameters
only allowed", id="parameters"
+ ),
+ ],
+ )
+ def test_invalid_option_combination(self, kwargs, match):
+ with pytest.raises(ValueError, match=match):
+ PsrpOperator(task_id="test_task_id", psrp_conn_id=CONNECTION_ID,
**kwargs)
+
+ @pytest.mark.parametrize(
+ "kwargs",
+ [
+ pytest.param({"command": ""}, id="empty-command"),
+ pytest.param({"powershell": "", "arguments": ["a"]},
id="arguments"),
+ pytest.param({"powershell": "", "parameters": {"k": "v"}},
id="parameters"),
+ ],
+ )
+ def test_empty_option_counts_as_provided(self, kwargs):
+ PsrpOperator(task_id="test_task_id", psrp_conn_id=CONNECTION_ID,
**kwargs)
def test_cmdlet_task_id_default(self):
operator = PsrpOperator(cmdlet="Invoke-Foo",
psrp_conn_id=CONNECTION_ID)
assert operator.task_id == "Invoke-Foo"
+ @patch(f"{PsrpOperator.__module__}.PsrpHook")
+ def test_command_rendering_to_empty_dispatches_as_command(self, hook_impl):
+ op = PsrpOperator(task_id="test", psrp_conn_id=CONNECTION_ID,
command="{{ '' }}")
+ op.render_template_fields({})
+ assert op.command == ""
+ ps = Mock(spec=PowerShell, output=[], had_errors=False,
runspace_pool=Mock(host=Mock(rc=0)))
+ hook_impl.configure_mock(
+
**{"return_value.__enter__.return_value.invoke.return_value.__enter__.return_value":
ps}
+ )
+ op.execute(None)
+ ps.add_script.assert_called_once_with("cmd.exe /c @'\n\n'@")
+
@pytest.mark.parametrize("do_xcom_push", [True, False])
@pytest.mark.parametrize(
("had_errors", "rc"), [(False, 0), (False, None), (True, None),
(False, 1), (True, 1)]
