search

phylum-dev / cli

Command line interface for the Phylum API
https://phylum.io
GNU General Public License v3.0
103 stars 11 forks source link

`analyze` command output breaks `phylum-ci` #1131

Closed maxrake closed 1 year ago

maxrake commented 1 year ago

Overview

1123 introduced a breaking change to phylum-ci by changing the format of the JSON output returned by the phylum analyze --verbose --json command.

The change from using get_job_status (the /policy/evaluate endpoint) to get_job_status_raw (the /policy/evaluate/raw endpoint) breaks phylum-ci, which previously ripped out all the custom markdown report generation logic in favor of using the report output directly from the CLI's --verbose --json output when switching to policy based operation.

How To Reproduce

Steps to reproduce this behavior:

  1. Ensure the phylum CLI v5.3.1-rc1 is installed
  2. Force analysis with phylum-ci in a repo protected with Phylum: phylum-ci -afvv
  3. See error:
Full Output (click to expand)

``` ../cli  16  16 on  main via 🦕 via 🦀 v1.70.0 took 12s ❯ phylum --version phylum v5.3.0 ../cli  16  16 on  main via 🦕 via 🦀 v1.70.0 ❯ phylum-ci --version phylum-ci 0.30.1 ../cli  16  16 on  main via 🦕 via 🦀 v1.70.0 ❯ phylum-ci -afvv Logging initialized to level 10 (DEBUG) Phylum CLI version not specified Found installed Phylum CLI version: v5.3.0 Minimum supported Phylum CLI version required for install: v5.2.0 Making request to GitHub API URL: https://api.github.com/repos/phylum-dev/cli/releases 57 GitHub API requests remaining until window resets at: Wed Jun 21 18:17:29 2023 Using Phylum CLI version: v5.3.0 No CI environment detected Confirming pre-requisites ... `git` binary found on the PATH Existing .git directory found at the current working directory All pre-requisites met Project name not provided as argument. Checking the `.phylum_project` file ... Project name provided in `.phylum_project` file: cli Attempting to create a Phylum project with the name: cli ... Existing Phylum CLI instance found: v5.3.0 at /Users/maxrake/.local/bin/phylum Using Phylum CLI instance: v5.3.0 at /Users/maxrake/.local/bin/phylum Using Phylum group: integrations Project cli already exists. Continuing with it ... No valid lockfiles were provided as arguments. Checking the `.phylum_project` file ... Lockfile(s) provided in `.phylum_project` file: [PosixPath('Cargo.lock')] Valid provided lockfiles: [Cargo.lock] Lockfiles in use: [Cargo.lock] Forced analysis specified with flag or otherwise set. Proceeding with analysis ... Label to use for analysis: No-CI_main_eea395c Considering all current dependencies ... 595 unique current dependencies Performing analysis. This may take a few seconds. Using analysis command: /Users/maxrake/.local/bin/phylum analyze --label No-CI_main_eea395c --project cli --group integrations --verbose --json --base /var/folders/gh/wnf14j7n4q34y2t36hq2jz800000gn/T/base_geuk8zrt.json /Users/maxrake/dev/phylum/localdev/cli/Cargo.lock The analysis is complete and there were NO failures Analysis output: ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ Phylum OSS Supply Chain Risk Analysis - SUCCESS ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ The Phylum risk analysis is complete and has passed the active policy. View this project in the Phylum UI (https://app.phylum.io/projects/d84b0bb3-fb64-43cb-a473-dc96b54db662?group=integrations&label=No-CI_main_eea395c) Return code: 0 ../cli  16  16 on  main via 🦕 via 🦀 v1.70.0 took 11s ❯ phylum update --prerelease phylum-cli installer OK Copied completions to /Users/maxrake/.local/share/phylum/completions Installing extension npm... ✅ Extension npm already installed, nothing to do Installing extension pip... ✅ Extension pip already installed, nothing to do Installing extension poetry... ✅ Extension poetry already installed, nothing to do Installing extension yarn... ✅ Extension yarn already installed, nothing to do OK Installed default extensions OK Completions are enabled for bash. OK Completions are enabled for zsh. OK Successfully installed phylum. Source your /Users/maxrake/.zshrc file, add /Users/maxrake/.local/bin to your $PATH variable, or log in to a new terminal in order to make `phylum` available. ✅ Successfully updated to v5.3.1-rc1! ../cli  16  16 on  main via 🦕 via 🦀 v1.70.0 took 7s ❯ phylum-ci -afvv Logging initialized to level 10 (DEBUG) Phylum CLI version not specified Found installed Phylum CLI version: v5.3.1-rc1 Minimum supported Phylum CLI version required for install: v5.2.0 Making request to GitHub API URL: https://api.github.com/repos/phylum-dev/cli/releases 55 GitHub API requests remaining until window resets at: Wed Jun 21 18:17:30 2023 Using Phylum CLI version: v5.3.1-rc1 No CI environment detected Confirming pre-requisites ... `git` binary found on the PATH Existing .git directory found at the current working directory All pre-requisites met Project name not provided as argument. Checking the `.phylum_project` file ... Project name provided in `.phylum_project` file: cli Attempting to create a Phylum project with the name: cli ... Existing Phylum CLI instance found: v5.3.1-rc1 at /Users/maxrake/.local/bin/phylum Using Phylum CLI instance: v5.3.1-rc1 at /Users/maxrake/.local/bin/phylum Using Phylum group: integrations Project cli already exists. Continuing with it ... No valid lockfiles were provided as arguments. Checking the `.phylum_project` file ... Lockfile(s) provided in `.phylum_project` file: [PosixPath('Cargo.lock')] Valid provided lockfiles: [Cargo.lock] Lockfiles in use: [Cargo.lock] Forced analysis specified with flag or otherwise set. Proceeding with analysis ... Label to use for analysis: No-CI_main_eea395c Considering all current dependencies ... 595 unique current dependencies Performing analysis. This may take a few seconds. Using analysis command: /Users/maxrake/.local/bin/phylum analyze --label No-CI_main_eea395c --project cli --group integrations --verbose --json --base /var/folders/gh/wnf14j7n4q34y2t36hq2jz800000gn/T/base_kar6w6vs.json /Users/maxrake/dev/phylum/localdev/cli/Cargo.lock ╭─────────────────────────────── Traceback (most recent call last) ────────────────────────────────╮ │ /Users/maxrake/.local/bin/phylum-ci:8 in │ │ │ │ 5 from phylum.ci.cli import script_main │ │ 6 if __name__ == '__main__': │ │ 7 │ sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0]) │ │ ❱ 8 │ sys.exit(script_main()) │ │ 9 │ │ │ │ ╭─────────────────────────────────────────── locals ───────────────────────────────────────────╮ │ │ │ re = │ │ │ │ script_main = │ │ │ │ sys = │ │ │ ╰──────────────────────────────────────────────────────────────────────────────────────────────╯ │ │ │ │ /Users/maxrake/.local/pipx/venvs/phylum/lib/python3.11/site-packages/phylum/ci/cli.py:223 in scr │ │ │ │ 220 def script_main() -> None: │ │ 221 │ """Script entry point.""" │ │ 222 │ # The only point of this function is to ensure the proper exit code is set when call │ │ ❱ 223 │ sys.exit(main()) │ │ 224 │ │ 225 │ │ 226 if __name__ == "__main__": │ │ │ │ /Users/maxrake/.local/pipx/venvs/phylum/lib/python3.11/site-packages/phylum/ci/cli.py:208 in mai │ │ │ │ 205 │ LOG.info("Label to use for analysis: %s", ci_env.phylum_label) │ │ 206 │ │ │ 207 │ # Analyze current project lockfile(s) with Phylum CLI │ │ ❱ 208 │ ci_env.analyze() │ │ 209 │ │ │ 210 │ # Output the results of the analysis │ │ 211 │ ci_env.post_output() │ │ │ │ ╭─────────────────────────────────────────── locals ───────────────────────────────────────────╮ │ │ │ args = None │ │ │ │ ci_env = │ │ │ │ parsed_args = Namespace(verbose=2, quiet=0, lockfile=None, all_deps=True, │ │ │ │ force_analysis=True, token=None, project=None, group=None, │ │ │ │ version='v5.3.1-rc1', target='aarch64-apple-darwin', uri=None, │ │ │ │ force_install=False) │ │ │ │ remainder_args = [] │ │ │ ╰──────────────────────────────────────────────────────────────────────────────────────────────╯ │ │ │ │ /Users/maxrake/.local/pipx/venvs/phylum/lib/python3.11/site-packages/phylum/logger.py:231 in tra │ │ │ │ 228 │ │ │ │ transient=True, │ │ 229 │ │ │ ) as progress: │ │ 230 │ │ │ │ task = progress.add_task(desc, total=None) │ │ ❱ 231 │ │ │ │ result = func(*args, **kwargs) │ │ 232 │ │ │ │ progress.stop_task(task) │ │ 233 │ │ │ return result │ │ 234 │ │ │ │ ╭──────────────────────────── locals ────────────────────────────╮ │ │ │ args = (,) │ │ │ │ desc = 'Analyzing dependencies with Phylum' │ │ │ │ func = │ │ │ │ kwargs = {} │ │ │ │ progress = │ │ │ │ task = 0 │ │ │ ╰────────────────────────────────────────────────────────────────╯ │ │ │ │ /Users/maxrake/.local/pipx/venvs/phylum/lib/python3.11/site-packages/phylum/ci/ci_base.py:459 in │ │ │ │ 456 │ │ │ │ │ │ If the command was expected to succeed, please report this as a │ │ bug.""" │ │ 457 │ │ │ │ │ raise PhylumCalledProcessError(err, textwrap.dedent(msg)) from err │ │ 458 │ │ │ │ ❱ 459 │ │ analysis = JobPolicyEvalResult(**json.loads(analysis_result)) │ │ 460 │ │ self._analysis_report = analysis.report │ │ 461 │ │ │ │ 462 │ │ # The logic below would make for a good match statement, which was introduced in │ │ Python 3.10 │ │ │ │ ╭─────────────────────────────────────────── locals ───────────────────────────────────────────╮ │ │ │ analysis_result = '{\n "is_failure": false,\n "incomplete_packages_count": 0,\n "help": │ │ │ │ "The Phylum'+215 │ │ │ │ base_fd = │ │ │ │ base_pkgs = [] │ │ │ │ cmd = [ │ │ │ │ │ '/Users/maxrake/.local/bin/phylum', │ │ │ │ │ 'analyze', │ │ │ │ │ '--label', │ │ │ │ │ 'No-CI_main_eea395c', │ │ │ │ │ '--project', │ │ │ │ │ 'cli', │ │ │ │ │ '--group', │ │ │ │ │ 'integrations', │ │ │ │ │ '--verbose', │ │ │ │ │ '--json', │ │ │ │ │ ... +3 │ │ │ │ ] │ │ │ │ self = │ │ │ │ total_packages = { │ │ │ │ │ PackageDescriptor( │ │ │ │ │ │ name='anstyle-parse', │ │ │ │ │ │ version='0.2.0', │ │ │ │ │ │ type='cargo' │ │ │ │ │ ), │ │ │ │ │ PackageDescriptor(name='dirs', version='5.0.1', type='cargo'), │ │ │ │ │ PackageDescriptor(name='group', version='0.13.0', type='cargo'), │ │ │ │ │ PackageDescriptor(name='serde', version='1.0.164', type='cargo'), │ │ │ │ │ PackageDescriptor( │ │ │ │ │ │ name='serde_derive', │ │ │ │ │ │ version='1.0.164', │ │ │ │ │ │ type='cargo' │ │ │ │ │ ), │ │ │ │ │ PackageDescriptor(name='serde_repr', version='0.1.9', type='cargo'), │ │ │ │ │ PackageDescriptor( │ │ │ │ │ │ name='tokio-rustls', │ │ │ │ │ │ version='0.24.1', │ │ │ │ │ │ type='cargo' │ │ │ │ │ ), │ │ │ │ │ PackageDescriptor( │ │ │ │ │ │ name='windows_x86_64_msvc', │ │ │ │ │ │ version='0.42.2', │ │ │ │ │ │ type='cargo' │ │ │ │ │ ), │ │ │ │ │ PackageDescriptor(name='term', version='0.7.0', type='cargo'), │ │ │ │ │ PackageDescriptor(name='hmac', version='0.12.1', type='cargo'), │ │ │ │ │ ... +585 │ │ │ │ } │ │ │ ╰──────────────────────────────────────────────────────────────────────────────────────────────╯ │ ╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ TypeError: JobPolicyEvalResult.__init__() got an unexpected keyword argument 'incomplete_packages_count' ```

Even if phylum-ci is updated to accept the new json output format, the report it expects is still missing:

TypeError: JobPolicyEvalResult.__init__() missing 1 required positional argument: 'report'

Expected Behavior

Additional Context

Several options are possible for proceeding, but discussion here first might help to shape the decision:

  1. Put custom report generation logic back into phylum-ci but with the information provided by the new CLI json output
  2. Update the CLI to provide the pre-generated report from the /policy/evaluate endpoint again
  3. Start making API calls directly in phylum-ci instead of relying on the CLI
  4. Update the API to add the report and output results to the /policy/evaluate/raw endpoint so that it acts as a superset of the /policy/evaluate endpoint
  5. Use the CLI's analyze output, eschewing formatted markdown entirely
  6. Update the CLI analyze output so that it can also be used as markdown
  7. Something else?

Any solution that only uses the new /policy/evaluate/raw endpoint will also require updates to phylum-ci to ensure the change from incomplete_count to incomplete_packages_count is addressed since that is another breaking change.

maxrake commented 1 year ago

Options 2 and 4 are prefered but will require changes in either cli or api, respectively.

Option 1 is undesirable b/c the discussion at the time of switching to policy based operation was that report generation was best done in one place (API now) and used for consistency between the GitHub App and all other integrations.

Option 3 will require a lot of work to decouple from CLI calls.

Options 5 and 6 break with the consistent integration output format and re-introduces multiple locations where a report format is defined/created.

maxrake commented 1 year ago

UPDATE: The team decided to pursue option 7.

The extension API was updated to allow for all the options to the analyze command. The --label option was the only one missing. With that change a custom extension is possible, to allow for replicating the previous behavior. This extension will live in the phylum-ci repository and be installed such that it is available for all uses of the phylum-ci entrypoint and integrations that make use of it.