diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b656d1c7..02c0986d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -66,6 +66,22 @@ repos: "-sn", # Don't display the score "--rcfile=.pylintrc", # Link to config file ] + additional_dependencies: + [ + bodhi-client, + feedparser, + google-api-python-client, + gssapi, + koji, + nitrate, + oauth2client, + pytest, + python-bugzilla, + python-dateutil, + requests, + requests-gssapi, + tenacity, + ] - repo: https://github.com/codespell-project/codespell rev: v2.4.1 diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..cb0b4617 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,48 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Project + +`did` is a Python CLI that gathers status-report data ("What did you do last week, month, year?") by querying many issue/code-review/wiki backends and aggregating the results into a single report. Distributed as a Python package and as Fedora/EPEL RPMs (via `did.spec` + Packit). + +## Common commands + +Tests use a temp `DID_DIR` and run in parallel (`pytest-xdist`). The `Makefile` wires the canonical invocations: + +- `make test` — full suite (`pytest -n auto tests`) +- `make smoke` — fast CLI smoke (`tests/test_cli.py`) +- `make coverage` — coverage to `cov_html/` +- Single test: `DID_DIR=$(mktemp -d) pytest tests/unit/test_stats.py::TestStatsGroup::test_name -n0` +- Lint/format (matches CI): `pre-commit run --all-files` — runs autopep8, isort, flake8 (with `flake8-pytest-style`), pylint, codespell, mypy, and assorted file hooks. Config in `.pre-commit-config.yaml`. +- Install dev extras: `pip install -e '.[all]'` (or pick a backend extra: `bugzilla`, `jira`, `google`, `koji`, `redmine`, `rt`, `nitrate`, `bodhi`, `tests`, `mypy`, `docs`). +- Docs: `make docs` (Sphinx, also built on Read the Docs via `.readthedocs.yaml`). +- RPM: `make rpm` / `make srpm` (uses `did.spec`; Packit config in `.packit.yaml`). +- Version: parsed by `setup.py` from `did.spec`'s `Version:` and `Release:` lines — bump there, not in `setup.py`. + +## Architecture + +Three layers, all rooted in `did/`: + +1. **CLI + config (`did/cli.py`, `did/base.py`)** — `cli.py` parses args and orchestrates the run. `base.py` owns the `Config`, `Date`, and `User` objects plus the date-range arithmetic (`this/last week|month|quarter|year`, `WEEKDAY_MAP`, etc.). Config comes from `~/.did/config` (override with `DID_DIR` / `DID_CONFIG`) and is sectioned: a `[general]` section sets defaults; every other section is one plugin instance whose `type = ` selects the backend. + +2. **Stats core (`did/stats.py`)** — defines `Stats` and `StatsGroup`. A `StatsGroup` is the per-config-section container; `Stats` subclasses are the individual queries (e.g. "issues created"). `UserStats` fans groups out across configured users. Plugin work runs through a `ThreadPoolExecutor`, so plugin code must be thread-safe and surface its own errors (`Stats.error`) rather than crashing the run. + +3. **Plugins (`did/plugins/*.py`)** — one file per backend (Jira, Bugzilla, GitHub, GitLab, Gerrit, Koji, Bodhi, Google, Confluence, Sentry, Trello, Pagure, Forgejo, Phabricator, Redmine, RT, Trac, Nitrate, Hyperkitty, public-inbox, git, wiki, plus `header`/`footer`). Each plugin defines a `StatsGroup` subclass and one or more `Stats` subclasses; the plugin module is loaded by name from the config's `type = ...`. Backend-specific extras live in `setup.py`'s `extras_require` — keep the two in sync when adding a plugin. + +### Adding a plugin + +Create `did/plugins/.py` with a `Stats(StatsGroup)` plus the individual `Stats` subclasses, mirror the structure of a similar existing plugin (e.g. `github.py` for REST, `jira.py` for auth-heavy, `git.py` for local), add any new third-party dep to `extras_require` in `setup.py`, document the config section in `docs/plugins/`, and add unit coverage under `tests/unit/plugins/`. + +## Tests + +- `tests/unit/` — pure unit tests (`test_base.py`, `test_cli.py`, `test_stats.py`, `test_utils.py`, `plugins/`). These are what runs in the GitHub Actions `pre-commit.yml` / unit workflow. +- `tests/basic/` and the FMF metadata (`*.fmf`, `.fmf/`, `plans/`) drive `tmt`-based integration tests run downstream in Testing Farm via Packit — not part of `pytest`. Don't add Python tests under `tests/basic/`. +- `conftest.py` and `pytest.ini` set the shared fixtures and discovery rules. + +## Conventions worth knowing + +- Style is enforced by pre-commit (autopep8 `--aggressive --aggressive --max-line-length=88`, isort, flake8). Don't hand-format; let the hook do it. +- Python 3.9+ is supported (see classifiers in `setup.py`); type hints use `Optional[...]` and `from __future__ import annotations` is used selectively — match the file you're editing. +- Logging goes through `did.utils.log`; user-facing output uses the report formatter, not `print`. +- Network/credentials in plugins: read tokens via `token_file` config keys (see existing plugins) rather than embedding them; GSSAPI is used for several Red Hat-internal backends. diff --git a/did/base.py b/did/base.py index 3971e78c..a2f8d63d 100644 --- a/did/base.py +++ b/did/base.py @@ -7,9 +7,12 @@ import locale import os import re +import shlex +import subprocess import sys from configparser import NoOptionError, NoSectionError from datetime import timedelta +from functools import lru_cache from typing import Iterator, Optional, Union from dateutil.relativedelta import FR as FRIDAY @@ -529,21 +532,63 @@ def alias(self, aliases: Optional[str], stats: Optional[str]) -> None: log.info("Using login alias '%s' for '%s'", login, stats) +@lru_cache(maxsize=None) +def _run_token_command(command: str) -> str: + """ + Run `command` and return its stripped stdout. + + The command line is parsed with `shlex.split` and executed without + a shell, so config files cannot inject shell metacharacters. A + 30-second timeout protects against secret managers that block on + interactive prompts (e.g. an expired ``op`` session). Non-zero + exit, missing binary or timeout each raise `ConfigError`; stdout + is never logged because it is the secret. + + Results are memoized for the lifetime of the process so that + multiple config sections sharing the same command string only + invoke the external tool once per run. Failures are not cached. + """ + try: + result = subprocess.run( + shlex.split(command), + capture_output=True, text=True, + timeout=30, check=True, + ) + except FileNotFoundError as exc: + raise ConfigError( + f"Token command not found: {exc.filename}") from exc + except subprocess.TimeoutExpired as exc: + raise ConfigError( + f"Token command timed out after {exc.timeout}s: {command}" + ) from exc + except subprocess.CalledProcessError as exc: + raise ConfigError( + f"Token command failed (exit {exc.returncode}): " + f"{exc.stderr.strip()}") from exc + return result.stdout.strip() + + def get_token( config: dict[str, str], token_key: str = "token", - token_file_key: str = "token_file") -> Optional[str]: + token_file_key: str = "token_file", + token_command_key: str = "token_command") -> Optional[str]: """ - Extract the authentication token from config or token file + Extract the authentication token from config, file, or command. - Returns the contents of `config[token_key]`, or the file contents of - `config[token_file_key]` if no `config[token]` exists. If neither - keys exist, `None` is returned. + Returns the contents of `config[token_key]`, the file contents of + `config[token_file_key]`, or the stdout of + `config[token_command_key]`, whichever is set. If none are set, + returns `None`. Sometimes you want to be able to store a token in a file rather than - in the your plain config file. Use this function to support a system - wide mechanism to retrieve tokens or secrets either directly from - the config file as plain text or from an outsourced file. + in your plain config file, or fetch it from a password manager such + as BitWarden (``bw get password did-jira``) or 1Password + (``op read op://Personal/Jira/token``). Use this function to support + a system-wide mechanism to retrieve tokens or secrets. + + Precedence when more than one key is set: ``token`` > ``token_file`` + > ``token_command``. :param config: A configuration dictionary. @@ -553,12 +598,15 @@ def get_token( :param token_file_key: The dict entry to look for when the token is supposed to be read from file. + :param token_command_key: + The dict entry to look for when the token is produced by running + an external command. :returns: The stripped token or `None` if no or only empty entries were found in the `config` dict. """ - token = None + token: Optional[str] = None if token_key in config: token = str(config[token_key]).strip() @@ -566,8 +614,7 @@ def get_token( file_path = os.path.expanduser(config[token_file_key]) with open(file_path, encoding="utf-8") as token_file: token = token_file.read().strip() + elif token_command_key in config: + token = _run_token_command(str(config[token_command_key]).strip()) - if token == "": - token = None - - return token + return token or None diff --git a/did/plugins/confluence.py b/did/plugins/confluence.py index aec9a851..8ef85c4d 100644 --- a/did/plugins/confluence.py +++ b/did/plugins/confluence.py @@ -43,14 +43,21 @@ token_expiration = 30 Notes: -Either ``token`` or ``token_file`` has to be defined. +One of ``token``, ``token_file`` or ``token_command`` has to be defined. token Token string directly included in the config. - Has a higher priority over ``token_file``. + Has a higher priority over ``token_file`` and ``token_command``. token_file Path to the file where the token is stored. + Has a higher priority over ``token_command``. + +token_command + Shell-style command line whose stdout is used as the token, e.g. + ``bw get password did-confluence`` or + ``op read op://Personal/Confluence/token``. The command is parsed + with ``shlex`` and executed without a shell. token_expiration Print warning if token with provided ``token_name`` expires within diff --git a/did/plugins/github.py b/did/plugins/github.py index 754dea63..793b108b 100644 --- a/did/plugins/github.py +++ b/did/plugins/github.py @@ -203,7 +203,11 @@ def request(self, url): log.warning("Sleeping now for %s.", listed(sleep_time, 'second')) time.sleep(sleep_time) continue - raise ReportError(f"GitHub query failed: {response.text}") + try: + message = json.loads(response.text).get("message", response.text) + except (json.JSONDecodeError, AttributeError): + message = response.text + raise ReportError(f"GitHub query failed: {message}") # all good! break diff --git a/did/plugins/jira.py b/did/plugins/jira.py index 1edca27d..c5f73dfd 100644 --- a/did/plugins/jira.py +++ b/did/plugins/jira.py @@ -13,14 +13,21 @@ token_name = did-token Notes: -Either ``token`` or ``token_file`` has to be defined. +One of ``token``, ``token_file`` or ``token_command`` has to be defined. token Token string directly included in the config. - Has a higher priority over ``token_file``. + Has a higher priority over ``token_file`` and ``token_command``. token_file Path to the file where the token is stored. + Has a higher priority over ``token_command``. + +token_command + Shell-style command line whose stdout is used as the token, e.g. + ``bw get password did-jira`` or + ``op read op://Personal/Jira/token``. The command is parsed with + ``shlex`` and executed without a shell. token_expiration Print warning if token with provided ``token_name`` expires within @@ -71,9 +78,10 @@ token_file = ~/.did/jira_api_token api_version = 3 -Keys ``auth_username``, ``token`` and ``token_file`` are -used for Jira Cloud with ``basic`` authentication. Either ``token`` or -``token_file`` must be provided, ``token`` has a higher priority. +Keys ``auth_username``, ``token``, ``token_file`` and ``token_command`` +are used for Jira Cloud with ``basic`` authentication. One of ``token``, +``token_file`` or ``token_command`` must be provided; ``token`` has the +highest priority, then ``token_file``, then ``token_command``. For Jira Server/Data Center with ``basic`` authentication, use ``auth_password`` or ``auth_password_file`` instead. diff --git a/did/stats.py b/did/stats.py index d1aca7cf..6ff9fca6 100644 --- a/did/stats.py +++ b/did/stats.py @@ -179,7 +179,7 @@ def check(self) -> None: try: f.result() except did.base.ReportError as error: - log.error("Skipping %s due to %s", f, error) + log.error("%s", error) sys.stdout.flush() sys.stderr.flush() diff --git a/did/utils.py b/did/utils.py index 9aca2fc3..bfb13aa5 100644 --- a/did/utils.py +++ b/did/utils.py @@ -521,7 +521,7 @@ def color( light_code = (1 if light else 0) # Starting and finishing sequence start = f"\033[{light_code}{text_color_code}{background_code}m" - finish = "\033[1;m" + finish = "\033[0m" return "".join([start, text, finish]) diff --git a/tests/github/issues.sh b/tests/github/issues.sh index 9432eda9..8379a303 100755 --- a/tests/github/issues.sh +++ b/tests/github/issues.sh @@ -14,17 +14,15 @@ rlJournalStart rlPhaseStartTest "Issues Created" rlRun -s "$did ./config-default.ini --gh-issues-created $YEAR_2022" - rlAssertGrep "Issues created on gh: 31$" $rlRun_LOG + rlAssertGrep "Issues created on gh: 30$" $rlRun_LOG rlAssertGrep "teemtee/tmt#1737 - Introduce a new step for cleanup tasks" $rlRun_LOG - rlAssertGrep "teemtee/fmf#149 - Checkout of the default branch fails" $rlRun_LOG rlAssertGrep "packit/packit-service#1645 - Manually trigger internal jobs" $rlRun_LOG rlPhaseEnd rlPhaseStartTest "Issues Created (org:teemtee)" rlRun -s "$did ./config-org.ini --gh-issues-created $YEAR_2022" - rlAssertGrep "Issues created on gh: 30$" $rlRun_LOG + rlAssertGrep "Issues created on gh: 29$" $rlRun_LOG rlAssertGrep "teemtee/tmt#1737 - Introduce a new step for cleanup tasks" $rlRun_LOG - rlAssertGrep "teemtee/fmf#149 - Checkout of the default branch fails" $rlRun_LOG rlAssertNotGrep "packit/packit-service#1645 - Manually trigger internal jobs" $rlRun_LOG rlPhaseEnd @@ -38,9 +36,8 @@ rlJournalStart rlPhaseStartTest "Issues Created (repo:teemtee/fmf)" rlRun -s "$did ./config-repo.ini --gh-issues-created $YEAR_2022" - rlAssertGrep "Issues created on gh: 2$" $rlRun_LOG + rlAssertGrep "Issues created on gh: 1$" $rlRun_LOG rlAssertNotGrep "teemtee/tmt#1737 - Introduce a new step for cleanup tasks" $rlRun_LOG - rlAssertGrep "teemtee/fmf#149 - Checkout of the default branch fails" $rlRun_LOG rlAssertNotGrep "packit/packit-service#1645 - Manually trigger internal jobs" $rlRun_LOG rlPhaseEnd @@ -64,7 +61,7 @@ rlJournalStart rlPhaseStartTest "Issues Closed" rlRun -s "$did ./config-default.ini --gh-issues-closed $YEAR_2022" - rlAssertGrep "Issues closed on gh: 17$" $rlRun_LOG + rlAssertGrep "Issues closed on gh: 16$" $rlRun_LOG rlAssertGrep "teemtee/fmf#014 - Define a way how to undefine an attribute" $rlRun_LOG rlAssertGrep "teemtee/tmt#991 - Incompatible environment variable name" $rlRun_LOG rlAssertGrep "psss/did#269 - Invalid plugin type 'google'" $rlRun_LOG @@ -72,7 +69,7 @@ rlJournalStart rlPhaseStartTest "Issues Closed (org:teemtee)" rlRun -s "$did ./config-org.ini --gh-issues-closed $YEAR_2022" - rlAssertGrep "Issues closed on gh: 15$" $rlRun_LOG + rlAssertGrep "Issues closed on gh: 14$" $rlRun_LOG rlAssertGrep "teemtee/fmf#014 - Define a way how to undefine an attribute" $rlRun_LOG rlAssertGrep "teemtee/tmt#991 - Incompatible environment variable name" $rlRun_LOG rlAssertNotGrep "psss/did#269 - Invalid plugin type 'google'" $rlRun_LOG @@ -80,7 +77,7 @@ rlJournalStart rlPhaseStartTest "Issues Closed (repo:teemtee/fmf)" rlRun -s "$did ./config-repo.ini --gh-issues-closed $YEAR_2022" - rlAssertGrep "Issues closed on gh: 3$" $rlRun_LOG + rlAssertGrep "Issues closed on gh: 2$" $rlRun_LOG rlAssertGrep "teemtee/fmf#014 - Define a way how to undefine an attribute" $rlRun_LOG rlAssertNotGrep "teemtee/tmt#991 - Incompatible environment variable name" $rlRun_LOG rlAssertNotGrep "psss/did#269 - Invalid plugin type 'google'" $rlRun_LOG diff --git a/tests/github/pulls.sh b/tests/github/pulls.sh index 33deb060..aaf06034 100755 --- a/tests/github/pulls.sh +++ b/tests/github/pulls.sh @@ -13,10 +13,9 @@ rlJournalStart rlPhaseStartTest "Pull Requests Created" rlRun -s "$did ./config-default.ini --gh-pull-requests-created $YEAR_2022" - rlAssertGrep "Pull requests created on gh: 94$" $rlRun_LOG + rlAssertGrep "Pull requests created on gh: 93$" $rlRun_LOG rlAssertGrep "teemtee/tmt#1750 - Include the new web link in verbose" $rlRun_LOG rlAssertGrep "teemtee/fmf#170 - Implement a directive for disabling inheritance" $rlRun_LOG - rlAssertGrep "teemtee/try#002 - Check logs for test with a hash sign in" $rlRun_LOG rlAssertGrep "psss/did#275 - Speed up local testing" $rlRun_LOG rlAssertGrep "psss/python-nitrate#039 - Enable basic sanity" $rlRun_LOG rlAssertGrep "packit/packit.dev#399 - Update \`tmt\` examples" $rlRun_LOG @@ -24,10 +23,9 @@ rlJournalStart rlPhaseStartTest "Pull Requests Created (org:teemtee)" rlRun -s "$did ./config-org.ini --gh-pull-requests-created $YEAR_2022" - rlAssertGrep "Pull requests created on gh: 85$" $rlRun_LOG + rlAssertGrep "Pull requests created on gh: 84$" $rlRun_LOG rlAssertGrep "teemtee/tmt#1750 - Include the new web link in verbose" $rlRun_LOG rlAssertGrep "teemtee/fmf#170 - Implement a directive for disabling inheritance" $rlRun_LOG - rlAssertGrep "teemtee/try#002 - Check logs for test with a hash sign in" $rlRun_LOG rlAssertNotGrep "psss/did#275 - Speed up local testing" $rlRun_LOG rlAssertNotGrep "psss/python-nitrate#039 - Enable basic sanity" $rlRun_LOG rlAssertNotGrep "packit/packit.dev#399 - Update \`tmt\` examples" $rlRun_LOG @@ -35,10 +33,9 @@ rlJournalStart rlPhaseStartTest "Pull Requests Created (org:teemtee,packit)" rlRun -s "$did ./config-more.ini --gh-pull-requests-created $YEAR_2022" - rlAssertGrep "Pull requests created on gh: 86$" $rlRun_LOG + rlAssertGrep "Pull requests created on gh: 85$" $rlRun_LOG rlAssertGrep "teemtee/tmt#1750 - Include the new web link in verbose" $rlRun_LOG rlAssertGrep "teemtee/fmf#170 - Implement a directive for disabling inheritance" $rlRun_LOG - rlAssertGrep "teemtee/try#002 - Check logs for test with a hash sign in" $rlRun_LOG rlAssertNotGrep "psss/did#275 - Speed up local testing" $rlRun_LOG rlAssertNotGrep "psss/python-nitrate#039 - Enable basic sanity" $rlRun_LOG rlAssertGrep "packit/packit.dev#399 - Update \`tmt\` examples" $rlRun_LOG diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py index c07930ea..65fa976c 100644 --- a/tests/unit/test_base.py +++ b/tests/unit/test_base.py @@ -303,6 +303,12 @@ def test_reporterror_exception() -> None: class TestGetToken(unittest.TestCase): """ Tests for the `get_token` function """ + def setUp(self) -> None: + # Clear the per-process token-command cache so memoized results + # from previous tests do not bleed into this one. + # pylint: disable=protected-access + did.base._run_token_command.cache_clear() + @contextmanager def get_token_as_file(self, token: str) -> Iterator[str]: """ @@ -359,13 +365,17 @@ def test_get_token_file_empty(self) -> None: assert did.base.get_token(config) is None def test_get_token_precedence(self) -> None: - """ Test plain token precedence over file one """ + """ token wins over token_file, which wins over command """ token_plain = str(uuid4()) token_in_file = str(uuid4()) - filename: str with self.get_token_as_file(token_in_file) as filename: - config = {"token_file": filename, "token": token_plain} - assert did.base.get_token(config) == token_plain + assert did.base.get_token( + {"token": token_plain, + "token_file": filename, + "token_command": "printf nope"}) == token_plain + assert did.base.get_token( + {"token_file": filename, + "token_command": "printf nope"}) == token_in_file def test_get_token_file_different_name(self) -> None: """ Test getting a token from a file under different name """ @@ -374,3 +384,31 @@ def test_get_token_file_different_name(self) -> None: config = {"mytoken_file": filename} assert did.base.get_token( config, token_file_key="mytoken_file") == token_in_file + + def test_get_token_command(self) -> None: + """ Test getting a token from the stdout of a command """ + token = str(uuid4()) + config = {"token_command": f"printf %s {token}"} + assert did.base.get_token(config) == token + + def test_get_token_command_failure(self) -> None: + """ Non-zero exit from token command must raise ConfigError """ + config = {"token_command": "false"} + with pytest.raises(did.base.ConfigError): + did.base.get_token(config) + + def test_get_token_command_missing_binary(self) -> None: + """ Missing binary in token command must raise ConfigError """ + config = {"token_command": "did-no-such-binary-xyz"} + with pytest.raises(did.base.ConfigError): + did.base.get_token(config) + + def test_get_token_command_memoized(self) -> None: + """ Repeated calls with same command run the subprocess once """ + token = str(uuid4()) + config = {"token_command": f"printf %s {token}"} + with patch("did.base.subprocess.run", + wraps=did.base.subprocess.run) as run_spy: + assert did.base.get_token(config) == token + assert did.base.get_token(config) == token + assert run_spy.call_count == 1 diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 6e999092..18ed22ca 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -238,7 +238,7 @@ def test_color_function_exists() -> None: # No color sets res = did.utils.color( "text", text_color=None, background=None, light=False, enabled=True) - assert res == "\033[0mtext\033[1;m" + assert res == "\033[0mtext\033[0m" # Disabled res = did.utils.color( "text", text_color=None, background=None, light=False, enabled=False) @@ -256,11 +256,11 @@ def test_color_function_exists() -> None: # Known color res = did.utils.color( "text", text_color="red", background=None, light=False, enabled=True) - assert res == "\033[0;31mtext\033[1;m" + assert res == "\033[0;31mtext\033[0m" # Light version res = did.utils.color( "text", text_color="red", background=None, light=True, enabled=True) - assert res == "\033[1;31mtext\033[1;m" + assert res == "\033[1;31mtext\033[0m" # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # strtobool