refactor!: remove --config-path from CLI (#844)

Signed-off-by: Trong Nhan Mai <[email protected]>

Author: Trong Nhan Mai

docs/source/pages/cli_usage/command_analyze.rst

@@ -21,7 +21,7 @@ Usage
 
     usage: ./run_macaron.sh analyze
         [-h] [-sbom SBOM_PATH] [-purl PURL] [-rp REPO_PATH] [-b BRANCH]
-        [-d DIGEST] [-pe PROVENANCE_EXPECTATION] [-c CONFIG_PATH]
+        [-d DIGEST] [-pe PROVENANCE_EXPECTATION]
         [--skip-deps] [-g TEMPLATE_PATH]
 
 -------
@@ -62,10 +62,6 @@ Options
 
     The path to the provenance file in in-toto format.
 
-.. option:: -c CONFIG_PATH, --config-path CONFIG_PATH
-
-    The path to the user configuration.
-
 .. option:: --skip-deps
 
     Skip automatic dependency analysis.

scripts/release_scripts/run_macaron.sh

@@ -320,10 +320,6 @@ if [[ $command == "analyze" ]]; then
                 arg_prov_file="$2"
                 shift
                 ;;
-            -c|--config-path)
-                arg_config_path="$2"
-                shift
-                ;;
             -g|--template-path)
                 arg_template_path="$2"
                 shift
@@ -414,16 +410,6 @@ if [[ -n "${arg_template_path:-}" ]]; then
     mount_file "-g/--template-path" "$template_path" "$template_path_in_container" "ro,Z"
 fi
 
-# Determine the config path to be mounted into ${MACARON_WORKSPACE}/config/${file_name}
-if [[ -n "${arg_config_path:-}" ]]; then
-    config_path="${arg_config_path}"
-    file_name="$(basename "${config_path}")"
-    config_path_in_container="${MACARON_WORKSPACE}/config/${file_name}"
-
-    argv_command+=("--config-path" "$config_path_in_container")
-    mount_file "-c/--config-path" "$config_path" "$config_path_in_container" "ro,Z"
-fi
-
 # Determine the sbom path to be mounted into ${MACARON_WORKSPACE}/sbom/${file_name}
 if [[ -n "${arg_sbom_path:-}" ]]; then
     sbom_path="${arg_sbom_path}"

src/macaron/__main__.py

@@ -18,7 +18,6 @@
 from macaron.config.global_config import global_config
 from macaron.errors import ConfigurationError
 from macaron.output_reporter.reporter import HTMLReporter, JSONReporter, PolicyReporter
-from macaron.parsers.yaml.loader import YamlLoader
 from macaron.policy_engine.policy_engine import run_policy_engine, show_prelude
 from macaron.slsa_analyzer.analyzer import Analyzer
 from macaron.slsa_analyzer.git_service import GIT_SERVICES
@@ -32,22 +31,14 @@
 
 def analyze_slsa_levels_single(analyzer_single_args: argparse.Namespace) -> None:
     """Run the SLSA checks against a single target repository."""
-    if not (analyzer_single_args.repo_path or analyzer_single_args.package_url or analyzer_single_args.config_path):
-        # We don't mention --config-path as a possible option in this log message as it going to be move soon.
-        # See: https://github.com/oracle/macaron/issues/417
+    if not (analyzer_single_args.repo_path or analyzer_single_args.package_url):
         logger.error(
             """Analysis target missing. Please provide a package url (PURL) and/or repo path.
             Examples of a PURL can be seen at https://github.com/package-url/purl-spec:
             pkg:github/micronaut-projects/micronaut-core."""
         )
         sys.exit(os.EX_USAGE)
 
-    if analyzer_single_args.config_path and (analyzer_single_args.package_url or analyzer_single_args.repo_path):
-        # TODO: revisit when the config-path option is moved.
-        # See: https://github.com/oracle/macaron/issues/417
-        logger.error("Cannot provide both config path and (package url (PURL) and/or repo path).")
-        sys.exit(os.EX_USAGE)
-
     # Set provenance expectation path.
     if analyzer_single_args.provenance_expectation is not None:
         if not os.path.exists(analyzer_single_args.provenance_expectation):
@@ -89,55 +80,45 @@ def analyze_slsa_levels_single(analyzer_single_args: argparse.Namespace) -> None
     analyzer.reporters.append(JSONReporter())
 
     run_config = {}
-    if analyzer_single_args.config_path:
-        # Get user config from yaml file
-        loaded_config = YamlLoader.load(analyzer_single_args.config_path)
-        if loaded_config is None:
-            logger.error("The input yaml config at %s is invalid.", analyzer_single_args.config_path)
-            sys.exit(os.EX_DATAERR)
-        else:
-            run_config = loaded_config
-    else:
-        repo_path = analyzer_single_args.repo_path
-        purl = analyzer_single_args.package_url
-        branch = analyzer_single_args.branch
-        digest = analyzer_single_args.digest
-
-        if repo_path and purl:
-            # To provide the purl together with the repository path, the user must specify the commit digest unless the
-            # purl has a version.
-            try:
-                purl_object = PackageURL.from_string(purl)
-            except ValueError as error:
-                logger.debug("Could not parse PURL: %s", error)
-                sys.exit(os.EX_USAGE)
-            if not (purl_object.version or digest):
-                logger.error(
-                    "Please provide the commit digest for the repo at %s that matches to the PURL string %s. Or "
-                    "include the version in the PURL",
-                    repo_path,
-                    purl,
-                )
-                sys.exit(os.EX_USAGE)
+    repo_path = analyzer_single_args.repo_path
+    purl = analyzer_single_args.package_url
+    branch = analyzer_single_args.branch
+    digest = analyzer_single_args.digest
+
+    if repo_path and purl:
+        # To provide the purl together with the repository path, the user must specify the commit digest unless the
+        # purl has a version.
+        try:
+            purl_object = PackageURL.from_string(purl)
+        except ValueError as error:
+            logger.debug("Could not parse PURL: %s", error)
+            sys.exit(os.EX_USAGE)
+        if not (purl_object.version or digest):
+            logger.error(
+                "Please provide the commit digest for the repo at %s that matches to the PURL string %s. Or "
+                "include the version in the PURL",
+                repo_path,
+                purl,
+            )
+            sys.exit(os.EX_USAGE)
 
-        # We need to use empty strings when the input values are of None type. This is because this dictionary will be
-        # passed into the Configuration instance, where the existing values in Configuration.options are replaced by
-        # whatever we assign it here. Technically, the data in ``Configuration`` class are not limited to only strings.
-        # Therefore, it could be cases where the ``purl`` field is initialized as an empty string in the constructor
-        # of the Configuration class, but if `` analyzer_single_args.package_url`` is None, the ``purl`` field is set
-        # to None in the Configuration instance.
-        # This inconsistency could cause potential issues when Macaron handles those inputs.
-        # TODO: improve the implementation of ``Configuration`` class to avoid such inconsistencies.
-        run_config = {
-            "target": {
-                "id": purl or repo_path or "",
-                "purl": purl or "",
-                "path": repo_path or "",
-                "branch": branch or "",
-                "digest": digest or "",
-            },
-            "dependencies": [],
+    # We need to use empty strings when the input values are of None type. This is because this dictionary will be
+    # passed into the Configuration instance, where the existing values in Configuration.options are replaced by
+    # whatever we assign it here. Technically, the data in ``Configuration`` class are not limited to only strings.
+    # Therefore, it could be cases where the ``purl`` field is initialized as an empty string in the constructor
+    # of the Configuration class, but if `` analyzer_single_args.package_url`` is None, the ``purl`` field is set
+    # to None in the Configuration instance.
+    # This inconsistency could cause potential issues when Macaron handles those inputs.
+    # TODO: improve the implementation of ``Configuration`` class to avoid such inconsistencies.
+    run_config = {
+        "target": {
+            "id": purl or repo_path or "",
+            "purl": purl or "",
+            "path": repo_path or "",
+            "branch": branch or "",
+            "digest": digest or "",
         }
+    }
 
     prov_payload = None
     if analyzer_single_args.provenance_file:
@@ -325,15 +306,6 @@ def main(argv: list[str] | None = None) -> None:
     # Use Macaron to analyze one single repository.
     single_analyze_parser = sub_parser.add_parser(name="analyze")
 
-    # We make the mutually exclusive usage of --config-path and --repo-path optional
-    # so that the user can provide the --package-url separately while keeping the current behavior of Macaron.
-    # Note that if the user provides both --package-url and --config-path, we will still raise an error,
-    # which is handled within the ``analyze_slsa_levels_single`` method.
-    # When we remove the --config-path option, we can remove this group and instead add all relevant
-    # options in the analyze command through ``single_analyze_parser``.
-    # See: https://github.com/oracle/macaron/issues/417
-    group = single_analyze_parser.add_mutually_exclusive_group(required=False)
-
     single_analyze_parser.add_argument(
         "-sbom",
         "--sbom-path",
@@ -343,7 +315,7 @@ def main(argv: list[str] | None = None) -> None:
         help=("The path to the SBOM of the analysis target."),
     )
 
-    group.add_argument(
+    single_analyze_parser.add_argument(
         "-rp",
         "--repo-path",
         required=False,
@@ -398,15 +370,6 @@ def main(argv: list[str] | None = None) -> None:
         help=("The path to the provenance file in in-toto format."),
     )
 
-    group.add_argument(
-        "-c",
-        "--config-path",
-        required=False,
-        type=str,
-        default="",
-        help=("The path to the user configuration."),
-    )
-
     single_analyze_parser.add_argument(
         "--skip-deps",
         required=False,

src/macaron/config/README.md

@@ -1,20 +1,13 @@
-# Copyright (c) 2022 - 2023, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2022 - 2024, Oracle and/or its affiliates. All rights reserved.
 # Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl/.
 
 """This module contains the Configuration class for the target analyzed repository."""
 
 import logging
-import os
 from typing import Any
 
-import yamale
-from yamale.schema import Schema
-
 logger: logging.Logger = logging.getLogger(__name__)
 
-_SCHEMA_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "target_config_schema.yaml")
-
-TARGET_CONFIG_SCHEMA: Schema = yamale.make_schema(_SCHEMA_DIR)
 """The schema for the target configuration yaml file."""
 
 

src/macaron/config/target_config.py

@@ -31,7 +31,6 @@
 from macaron.output_reporter.scm import SCMStatus
 from macaron.repo_finder.repo_finder import find_repo
 from macaron.repo_finder.repo_validator import find_valid_repository_url
-from macaron.slsa_analyzer.git_url import get_repo_full_name_from_url
 
 logger: logging.Logger = logging.getLogger(__name__)
 
@@ -259,46 +258,26 @@ def add_latest_version(
                 logger.error("Could not parse dependency version number: %s", error)
 
     @staticmethod
-    def merge_configs(
-        config_deps: list[Configuration], resolved_deps: dict[str, DependencyInfo]
-    ) -> list[Configuration]:
-        """Merge the resolved dependencies into the manual config dependencies.
-
-        Manual configuration entries are prioritized over the automatically resolved dependencies.
+    def to_configs(resolved_deps: dict[str, DependencyInfo]) -> list[Configuration]:
+        """Convert the resolved dependencies into the format used by the Analyzer.
 
         Parameters
         ----------
-        config_deps : list[Configuration]
-            Dependencies defined in the configuration file.
         resolved_deps : dict[str, DependencyInfo]
             The automatically resolved dependencies.
 
         Returns
         -------
         list[Configuration]
-            The result list contains the merged dependencies.
+            The dependency list to be used by the Analyzer.
         """
-        merged_deps: list[Configuration] = []
-        if config_deps:
-            for dep in config_deps:
-                dep.set_value("available", SCMStatus.AVAILABLE)
-                merged_deps.append(dep)
-
         if not resolved_deps:
-            return merged_deps
+            return []
+
+        config_list: list[Configuration] = []
 
         for key, value in resolved_deps.items():
-            duplicate = False
-            if config_deps:
-                for m_dep in config_deps:
-                    m_repo = get_repo_full_name_from_url(m_dep.get_value("path"))
-                    a_repo = get_repo_full_name_from_url(value.get("url", ""))
-                    if m_repo and m_repo == a_repo:
-                        duplicate = True
-                        break
-                if duplicate:
-                    continue
-            merged_deps.append(
+            config_list.append(
                 Configuration(
                     {
                         "id": key,
@@ -312,7 +291,7 @@ def merge_configs(
                 )
             )
 
-        return merged_deps
+        return config_list
 
     @staticmethod
     def tool_valid(tool: str) -> bool:

src/macaron/config/target_config_schema.yaml

@@ -155,7 +155,6 @@ def run(
             The return status code.
         """
         main_config = Configuration(user_config.get("target", {}))
-        deps_config: list[Configuration] = [Configuration(dep) for dep in user_config.get("dependencies", [])]
         deps_resolved: dict[str, DependencyInfo] = {}
 
         # Get a single session once for the whole analysis.
@@ -194,7 +193,7 @@ def run(
                     deps_resolved = DependencyAnalyzer.resolve_dependencies(main_record.context, sbom_path)
 
                 # Merge the automatically resolved dependencies with the manual configuration.
-                deps_config = DependencyAnalyzer.merge_configs(deps_config, deps_resolved)
+                deps_config = DependencyAnalyzer.to_configs(deps_resolved)
 
                 # Create a report instance with the record of the main repo.
                 report = Report(main_record)