Skip to content

Commit

Permalink
refactor!: remove --config-path from CLI (#844)
Browse files Browse the repository at this point in the history
Signed-off-by: Trong Nhan Mai <[email protected]>
  • Loading branch information
tromai committed Sep 16, 2024
1 parent edfe06e commit dc6f276
Show file tree
Hide file tree
Showing 28 changed files with 147 additions and 459 deletions.
6 changes: 1 addition & 5 deletions docs/source/pages/cli_usage/command_analyze.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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]
-------
Expand Down Expand Up @@ -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.
Expand Down
14 changes: 0 additions & 14 deletions scripts/release_scripts/run_macaron.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}"
Expand Down
115 changes: 39 additions & 76 deletions src/macaron/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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",
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
31 changes: 0 additions & 31 deletions src/macaron/config/README.md

This file was deleted.

9 changes: 1 addition & 8 deletions src/macaron/config/target_config.py
Original file line number Diff line number Diff line change
@@ -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."""


Expand Down
22 changes: 0 additions & 22 deletions src/macaron/config/target_config_schema.yaml

This file was deleted.

37 changes: 8 additions & 29 deletions src/macaron/dependency_analyzer/cyclonedx.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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,
Expand All @@ -312,7 +291,7 @@ def merge_configs(
)
)

return merged_deps
return config_list

@staticmethod
def tool_valid(tool: str) -> bool:
Expand Down
3 changes: 1 addition & 2 deletions src/macaron/slsa_analyzer/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down

This file was deleted.

This file was deleted.

Loading

0 comments on commit dc6f276

Please sign in to comment.