fix: normalize a blank --project/--domain at the option, not in init

Follow-up to the previous commit, which normalized the blank inside
`CLIConfig.init`. That was too low in the stack: `flyte get secret`,
`flyte create secret` and `flyte delete secret` all call `init` with a
literal empty project/domain to select the *org-level* scope, and the
normalization rewrote it to None, which then fell back to the config
file. The listing would have silently changed scope, and `--cluster-pool`
would have started failing outright, since `Secret._resolve_scope`
rejects a request that carries both a cluster pool and a project/domain.

Only a value typed on the command line is ambiguous, so the
normalization moves to a click callback on the shared PROJECT/DOMAIN
options (and on the two `flyte rerun` declares itself, which bypass
those). A caller that passes project="" in Python now means it.

fixes FLYTE-SDK-3A

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

Author: EngHabu

src/flyte/cli/_common.py

@@ -37,11 +37,39 @@
 PREFERRED_ACCENT_COLOR = "bold #FFD700"
 HEADER_STYLE = f"{PREFERRED_ACCENT_COLOR} on black"
 
+
+def blank_option_to_none(_ctx: click.Context, _param: click.Parameter, value: str | None) -> str | None:
+    """
+    Normalize a blank command-line value to None, i.e. "the flag was not given".
+
+    `flyte run --project "$PROJECT"` with `PROJECT` unset hands click an empty string. Without
+    this the blank counts as an explicit value, overrides the config file, and travels to the
+    backend as `id is required` (FLYTE-SDK-3A).
+
+    This deliberately lives on the click option rather than in `CLIConfig.init`: only a value
+    typed on the command line is ambiguous. A caller that passes `project=""` in Python means it
+    -- `flyte get/create/delete secret` use the empty string as the org-level scope sentinel --
+    and must not have it rewritten.
+
+    Args:
+        _ctx: The click context, unused.
+        _param: The parameter being processed, unused.
+        value: The raw command-line value.
+
+    Returns:
+        The stripped value, or None if it was absent or blank.
+    """
+    from flyte._initialize import blank_to_none
+
+    return blank_to_none(value)
+
+
 PROJECT_OPTION = click.Option(
     param_decls=["-p", "--project"],
     required=False,
     type=str,
     default=None,
+    callback=blank_option_to_none,
     help="Project to which this command applies.",
     show_default=True,
 )
@@ -51,6 +79,7 @@
     required=False,
     type=str,
     default=None,
+    callback=blank_option_to_none,
     help="Domain to which this command applies.",
     show_default=True,
 )
@@ -134,17 +163,15 @@ def init(
         images: tuple[str, ...] | None = None,
         sync_local_sys_paths: bool = True,
     ):
-        from flyte._initialize import blank_to_none
         from flyte.config._config import TaskConfig
 
         api_key = os.getenv("FLYTE_API_KEY")
 
-        # A blank --project/--domain is what the shell substitutes for an unset variable
-        # (`flyte run --project "$PROJECT"`), so treat it as "not provided" and fall back to
-        # the config file, matching what init_from_config already does with `project or ...`.
-        project = blank_to_none(project)
-        domain = blank_to_none(domain)
-
+        # NOTE: a blank project/domain is *not* normalized here. A blank typed on the command line
+        # is already turned into None by `blank_option_to_none`; what reaches this point as ""
+        # came from a caller that meant it -- `flyte get/create/delete secret` pass the empty
+        # string to select the org-level scope -- so rewriting it here would silently rescope
+        # those commands to the config file's project/domain.
         task_cfg = TaskConfig(
             org=self.org or self.config.task.org,
             project=project if project is not None else self.config.task.project,

src/flyte/cli/_rerun.py

@@ -37,8 +37,20 @@ def _parse_kv(items: Tuple[str, ...], flag: str) -> Optional[Dict[str, str]]:
 
 @click.command("rerun", cls=click.RichCommand)
 @click.argument("run_name", required=True)
-@click.option("-p", "--project", default=None, help="Project for the new run (defaults to config).")
-@click.option("-d", "--domain", default=None, help="Domain for the new run (defaults to config).")
+@click.option(
+    "-p",
+    "--project",
+    default=None,
+    callback=common.blank_option_to_none,
+    help="Project for the new run (defaults to config).",
+)
+@click.option(
+    "-d",
+    "--domain",
+    default=None,
+    callback=common.blank_option_to_none,
+    help="Domain for the new run (defaults to config).",
+)
 @click.option("--name", default=None, help="Name for the new run (a random name is generated if unset).")
 @click.option("-e", "--env", "env", multiple=True, help="Env var KEY=VALUE for the new run. Repeatable.")
 @click.option("--label", "label", multiple=True, help="Label KEY=VALUE for the new run. Repeatable.")

tests/cli/test_blank_project_domain.py

@@ -6,6 +6,11 @@
 "failed to validate project: invalid_argument: id is required". That surfaced as a
 RuntimeSystemError and was reported to Sentry as an SDK crash rather than as the user's
 missing configuration.
+
+The normalization lives on the click option, not in `CLIConfig.init`, because only a value
+typed on the command line is ambiguous. `flyte get/create/delete secret` call `init` with a
+literal `""` to select the org-level scope, and that has to survive -- the second class below
+pins it.
 """
 
 from __future__ import annotations
@@ -15,7 +20,7 @@
 import click
 import pytest
 
-from flyte.cli._common import CLIConfig
+from flyte.cli._common import CLIConfig, CommandBase, blank_option_to_none
 from flyte.config._config import Config, TaskConfig
 
 
@@ -27,46 +32,98 @@ def _make_cli_config(config: Config | None = None) -> CLIConfig:
 CONFIGURED = Config(task=TaskConfig(project="from-config", domain="from-config-domain"))
 
 
-class TestBlankProjectDomain:
+def _invoke(args: list[str], config: Config) -> Config:
+    """Run a `CommandBase` command with `args` and return the Config handed to init_from_config.
+
+    Goes through click so the option callback runs, which is where the blank is normalized.
+    """
+    captured = {}
+
+    @click.command(cls=CommandBase)
+    @click.pass_context
+    def cmd(ctx, project=None, domain=None):
+        CLIConfig(config=config, ctx=ctx).init(project=project, domain=domain)
+
+    with patch("flyte.cli._common.flyte") as mock_flyte:
+        cmd.main(args=args, standalone_mode=False)
+        captured["cfg"] = mock_flyte.init_from_config.call_args[0][0]
+    return captured["cfg"]
+
+
+class TestBlankProjectDomainOnTheCommandLine:
     @pytest.mark.parametrize("blank", ["", "   ", "\t"])
+    def test_blank_cli_values_fall_back_to_config(self, blank):
+        cfg = _invoke(["--project", blank, "--domain", blank], CONFIGURED)
+        assert cfg.task.project == "from-config"
+        assert cfg.task.domain == "from-config-domain"
+
+    def test_explicit_values_still_override_config(self):
+        cfg = _invoke(["--project", "explicit", "--domain", "explicit-domain"], CONFIGURED)
+        assert cfg.task.project == "explicit"
+        assert cfg.task.domain == "explicit-domain"
+
+    def test_padded_values_are_stripped(self):
+        cfg = _invoke(["--project", "  explicit  ", "--domain", "  explicit-domain  "], CONFIGURED)
+        assert cfg.task.project == "explicit"
+        assert cfg.task.domain == "explicit-domain"
+
+    @pytest.mark.parametrize("blank", ["", "   "])
+    def test_blank_on_both_sides_leaves_project_unset(self, blank):
+        """With nothing configured either, project stays None so the guard can raise.
+
+        None is what `require_project_and_domain` reports as "Project must be provided",
+        a user-kind InitializationError that is filtered out of Sentry.
+        """
+        cfg = _invoke(["--project", blank, "--domain", blank], Config())
+        assert cfg.task.project is None
+        assert cfg.task.domain is None
+
+    def test_omitting_the_flags_entirely_falls_back_to_config(self):
+        cfg = _invoke([], CONFIGURED)
+        assert cfg.task.project == "from-config"
+        assert cfg.task.domain == "from-config-domain"
+
+    @pytest.mark.parametrize(
+        "value, expected",
+        [(None, None), ("", None), ("   ", None), ("\t", None), ("proj", "proj"), ("  proj  ", "proj")],
+    )
+    def test_callback_normalization(self, value, expected):
+        assert blank_option_to_none(MagicMock(spec=click.Context), MagicMock(spec=click.Parameter), value) == expected
+
+
+class TestOrgLevelSecretScopeIsPreserved:
+    """`flyte get/create/delete secret` pass project="" to mean "org level", not "unset".
+
+    Normalizing that blank away inside `CLIConfig.init` would make those three commands fall
+    back to the config file's project/domain: the listing would silently change scope, and
+    `--cluster-pool` would start failing outright, since `Secret._resolve_scope` rejects a
+    request that carries both a cluster pool and a project/domain.
+    """
+
     @patch("flyte.cli._common.flyte")
-    def test_blank_cli_values_fall_back_to_config(self, mock_flyte, blank):
+    def test_programmatic_blank_stays_blank(self, mock_flyte):
         cli = _make_cli_config(config=CONFIGURED)
-        cli.init(project=blank, domain=blank)
+        cli.init(project="", domain="")
 
         call_cfg = mock_flyte.init_from_config.call_args[0][0]
-        assert call_cfg.task.project == "from-config"
-        assert call_cfg.task.domain == "from-config-domain"
+        assert call_cfg.task.project == ""
+        assert call_cfg.task.domain == ""
 
     @patch("flyte.cli._common.flyte")
-    def test_explicit_values_still_override_config(self, mock_flyte):
+    def test_cluster_pool_scope_check_still_sees_an_empty_scope(self, mock_flyte):
+        """The empty scope is what keeps the `--cluster-pool` guard from rejecting the request."""
         cli = _make_cli_config(config=CONFIGURED)
-        cli.init(project="explicit", domain="explicit-domain")
+        cli.init(project="", domain="")
 
         call_cfg = mock_flyte.init_from_config.call_args[0][0]
-        assert call_cfg.task.project == "explicit"
-        assert call_cfg.task.domain == "explicit-domain"
+        assert not call_cfg.task.project
+        assert not call_cfg.task.domain
 
     @patch("flyte.cli._common.flyte")
-    def test_padded_values_are_stripped(self, mock_flyte):
+    def test_programmatic_real_values_still_pass_through(self, mock_flyte):
         cli = _make_cli_config(config=CONFIGURED)
-        cli.init(project="  explicit  ", domain="  explicit-domain  ")
+        cli.init(project="explicit", domain="explicit-domain")
 
         call_cfg = mock_flyte.init_from_config.call_args[0][0]
         assert call_cfg.task.project == "explicit"
         assert call_cfg.task.domain == "explicit-domain"
-
-    @pytest.mark.parametrize("blank", ["", "   "])
-    @patch("flyte.cli._common.flyte")
-    def test_blank_on_both_sides_leaves_project_unset(self, mock_flyte, blank):
-        """With nothing configured either, project stays None so the guard can raise.
-
-        None is what `require_project_and_domain` reports as "Project must be provided",
-        a user-kind InitializationError that is filtered out of Sentry.
-        """
-        cli = _make_cli_config()
-        cli.init(project=blank, domain=blank)
-
-        call_cfg = mock_flyte.init_from_config.call_args[0][0]
-        assert call_cfg.task.project is None
-        assert call_cfg.task.domain is None