Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions docs/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -620,8 +620,12 @@ The command classifies every `HMCConfig` field for this report:
Unset the per-profile exports and configure those settings in individual TOML profiles.
An exported setting would otherwise override every profile's own credentials, trust,
attribution or limits. The refusal also covers settings this read-only report does not use.
The command also refuses global connection options
(`--host`, `--user`, `--password`, `--verify-ssl`, `--profile`), which would be ignored.
The command also refuses global connection options, which would be ignored:
`--host`, `--user` (`-u`), `--password` (`-p`), `--verify-ssl`/`--no-verify-ssl`,
`--trust-system-store`/`--no-trust-system-store`, `--ca-bundle`, `--cert-fingerprint`
and `--profile`. The error names only the supplied options using their canonical long
names. A global `--profile` before `report` is refused; `--profile` after
`report utilization` selects profiles for the survey.

When profiles fail, the `fleet` row's `notes` says how many and which: a system that only a
failed profile manages is missing from the fleet totals. It also says how many systems
Expand Down
54 changes: 54 additions & 0 deletions docs/workflow/specs/2026-11-04-report-option-diagnostics-design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# Report utilization option diagnostics

## Problem

Issue #1892: global TLS options are refused, but the error lists five unrelated
options and the guide omits TLS options. The existing refusal is correct.

## Scope

One unit: replace report.py's fixed diagnostic list with sorted canonical names
from `current_options().command_line_options`, hyphenating underscores as serve.py
does. Keep report as the diagnostic owner; no shared helper or caller migration.
Update `docs/cli.md` and `tests/app/test_report_cli.py`; no ADR is needed.
The operator excludes acceptance behavior/config policy (owner: outside issue
operator). Preserve #1891's 19 refused and two global environment settings,
validation order, exit status, profile loading and survey behavior.

### Failure model

- Actors and deployments: local CLI operators and CI CLI tests on supported Python versions.
- Invariants and assets: name only supplied global option identities; disclose no
argument values; refusal still precedes profile loading and output creation.
- Accepted failure classes: original alias spelling is not retained by the existing
canonical option set; report canonical long names, including positive names for
negative boolean switches, following serve.py. Earlier validation errors remain first.
- Covered elsewhere: CLI parsing and option-source classification belong to app.py;
environment classification and configuration policy remain outside this issue.

## Success

- C1: When execution reaches the global-option refusal, its diagnostic lists exactly
the supplied members of the eight-option set, sorted and hyphenated, without values.
That set is host, user, password, verify_ssl, trust_system_store, ca_bundle,
cert_fingerprint and profile. Repeated flags appear once; aliases use canonical names.
- C2: The guide lists those eight global options, the `-u`/`-p` aliases and both boolean
polarities; distinguishes the accepted report-local `--profile` from global `--profile`.
- C3: Existing refusal and accepted report behavior remain unchanged.

## Validation

- C1/C3, Mode: focused-test. Parameterize the public `cli.app` runner in
`tests/app/test_report_cli.py` over each global option, aliases and negative booleans;
include mixed and repeated flags, assert exact canonical list and absence of values,
exit 2, no profile inventory/load/survey and no output. Existing report-local profile
and no-global success tests cover accepted paths. Red: TLS-only input omits its name
and includes unrelated names. Green command:
`uv run --no-sync pytest --no-cov tests/app/test_report_cli.py -q`.
Verify tests bite by temporarily restoring the old fixed list, then restore the fix.
- C2, Mode: task-test-not-applicable. Guide prose has no executable consumer for its
option list; manually compare it with app.py's eight callback parameters and aliases,
without adding a prose snapshot test.
- Integration: `just verify` and `uv run --no-sync prek run --all-files`; CI checks the
supported Python matrix. No live HMC operation is needed. Revert this isolated diff
to roll back; the command changes no data format or persistent state.
8 changes: 6 additions & 2 deletions src/hmcpctl/cli_commands/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -415,10 +415,14 @@ def report_utilization(
"report utilization logs on to each profile's own host with that profile's "
f"credentials and TLS setting; unset {', '.join(exported)}"
)
if current_options().command_line_options:
command_line_options = current_options().command_line_options
if command_line_options:
options = ", ".join(
f"--{name.replace('_', '-')}" for name in sorted(command_line_options)
)
usage_error(
"report utilization connects to each profile's own host; drop the global "
"--host, --user, --password, --verify-ssl and --profile options"
f"{options} options"
)
try:
selected = _selected_profiles(profiles)
Expand Down
49 changes: 41 additions & 8 deletions tests/app/test_report_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,15 +328,48 @@ async def fake(profiles, open_client, *, concurrency, hmc_timeout):
assert failure["notes"] == "'=cmd|x"


def test_root_connection_option_is_refused(configured, tmp_path) -> None:
result = RUNNER.invoke(
cli.app,
["--profile", "hmc-a", "report", "utilization", "--csv",
str(tmp_path / "r.csv")],
) # fmt: skip
@pytest.mark.parametrize(
("args", "expected"),
[
(["--host", "other.example.test"], "--host"),
(["--user", "other-user"], "--user"),
(["-u", "other-user"], "--user"),
(["--password", "private-value"], "--password"),
(["-p", "private-value"], "--password"),
(["--verify-ssl"], "--verify-ssl"),
(["--no-verify-ssl"], "--verify-ssl"),
(["--trust-system-store"], "--trust-system-store"),
(["--no-trust-system-store"], "--trust-system-store"),
(["--ca-bundle", "other-ca.pem"], "--ca-bundle"),
(["--cert-fingerprint", "AA:BB:CC"], "--cert-fingerprint"),
(["--profile", "hmc-a"], "--profile"),
(["--host", "first.example.test", "--host", "second.example.test"], "--host"),
(
["--profile", "hmc-a", "--no-trust-system-store", "--password", "private-value",
"--ca-bundle", "other-ca.pem"],
"--ca-bundle, --password, --profile, --trust-system-store",
),
],
) # fmt: skip
def test_root_connection_option_is_refused(
configured, tmp_path, monkeypatch, args, expected
) -> None:
def unexpected(*args, **kwargs):
pytest.fail("refused report must not inspect profiles or survey")

assert result.exit_code == 2
assert "drop the global" in result.stderr
monkeypatch.setattr(report, "config_inventory", unexpected)
monkeypatch.setattr(report, "load_profile", unexpected)
monkeypatch.setattr(report, "survey_fleet", unexpected)
out = tmp_path / "r.csv"
result = RUNNER.invoke(cli.app, [*args, "report", "utilization", "--csv", str(out)])

assert result.exit_code == 2, result.output
error = " ".join(result.stderr.split("Error: ", 1)[1].split())
assert error == (
"report utilization connects to each profile's own host; drop the global "
f"{expected} options"
)
assert not out.exists()


def test_write_failure_keeps_failure_lines_and_leaves_no_file(
Expand Down