From 7b311d12f3f2c989d9ab0811f41ee642acdab06b Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Tue, 16 Jul 2024 18:13:46 +0200 Subject: [PATCH 01/29] cachi2: conversion function to cachi2 params Conversion function that transforms remote_source into cachi2 params STONEBLD-2586 Signed-off-by: Martin Basti --- atomic_reactor/utils/cachi2.py | 46 ++++++++++++++++++ tests/utils/test_cachi2.py | 85 ++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+) create mode 100644 atomic_reactor/utils/cachi2.py create mode 100644 tests/utils/test_cachi2.py diff --git a/atomic_reactor/utils/cachi2.py b/atomic_reactor/utils/cachi2.py new file mode 100644 index 000000000..3bbe3f2f4 --- /dev/null +++ b/atomic_reactor/utils/cachi2.py @@ -0,0 +1,46 @@ +# Copyright (c) 2024 Red Hat, Inc +# All rights reserved. +# +# This software may be modified and distributed under the terms +# of the BSD license. See the LICENSE file for details. + +""" +Utils to help to integrate with cachi2 CLI tool +""" + +from typing import Any, Dict + + +def remote_source_to_cachi2(remote_source: Dict[str, Any]) -> Dict[str, Any]: + """Converts remote source into cachi2 expected params. + + Remote sources were orignally designed for cachito. Cachi2 is not a direct + fork but has lot of similarities. + However, some parameters must be updated to be compatible with cachi2. + + Removed flags (OSBS process them): + * include-git-dir + * remove-unsafe-symlinks + + Removed pkg-managers (OSBS process them): + * git-submodule + + """ + removed_flags = {"include-git-dir", "remove-unsafe-symlinks"} + removed_pkg_managers = {"git-submodule"} + + cachi2_flags = sorted( + set(remote_source.get("flags", [])) - removed_flags + ) + cachi2_packages = [] + + for pkg_manager in remote_source["pkg_managers"]: + if pkg_manager in removed_pkg_managers: + continue + + packages = remote_source.get("packages", {}).get(pkg_manager, []) + packages = packages or [{"path": "."}] + for pkg in packages: + cachi2_packages.append({"type": pkg_manager, **pkg}) + + return {"packages": cachi2_packages, "flags": cachi2_flags} diff --git a/tests/utils/test_cachi2.py b/tests/utils/test_cachi2.py new file mode 100644 index 000000000..fd5191f72 --- /dev/null +++ b/tests/utils/test_cachi2.py @@ -0,0 +1,85 @@ +""" +Copyright (c) 2024 Red Hat, Inc +All rights reserved. + +This software may be modified and distributed under the terms +of the BSD license. See the LICENSE file for details. +""" + +from atomic_reactor.utils.cachi2 import remote_source_to_cachi2 + +import pytest + + +@pytest.mark.parametrize(('input_remote_source', 'expected_cachi2'), [ + pytest.param( + {"pkg_managers": ["gomod"]}, + {"flags": [], "packages": [{"path": ".", "type": "gomod"}]}, + id="pkg_manager_plain", + ), + pytest.param( + {"pkg_managers": ["gomod"], "packages": {"gomod": [{"path": "operator"}]}}, + {"flags": [], "packages": [{"path": "operator", "type": "gomod"}]}, + id="pkg_manager_single_path" + ), + pytest.param( + {"pkg_managers": ["gomod"], "packages": {"gomod": [ + {"path": "."}, {"path": "operator"} + ]}}, + {"flags": [], "packages": [ + {"path": ".", "type": "gomod"}, {"path": "operator", "type": "gomod"} + ]}, + id="pkg_manager_multiple_paths" + ), + pytest.param( + {"pkg_managers": ["pip"], "packages": { + "pip": [{ + "path": "src/web", + "requirements_files": ["requirements.txt", "requirements-extras.txt"], + "requirements_build_files": [ + "requirements-build.txt", "requirements-build-extras.txt" + ] + }, { + "path": "src/workers" + }] + }}, + {"flags": [], "packages": [ + { + "path": "src/web", "type": "pip", + "requirements_files": ["requirements.txt", "requirements-extras.txt"], + "requirements_build_files": [ + "requirements-build.txt", "requirements-build-extras.txt" + ] + }, + {"path": "src/workers", "type": "pip"}]}, + id="pip_extra_options" + ), + pytest.param( + {"pkg_managers": ["gomod", "npm"], "packages": {"gomod": [{"path": "operator"}]}}, + {"flags": [], "packages": [ + {"path": "operator", "type": "gomod"}, {"path": ".", "type": "npm"} + ]}, + id="mixed_pkg_managers" + ), + pytest.param( + {"pkg_managers": ["gomod"], "flags": ["gomod-vendor"]}, + {"flags": ["gomod-vendor"], "packages": [{"path": ".", "type": "gomod"}]}, + id="pkg_manager_with_flags", + ), + pytest.param( + {"pkg_managers": ["gomod"], "flags": [ + "remove-unsafe-symlinks", "gomod-vendor", "include-git-dir" + ]}, + {"flags": ["gomod-vendor"], "packages": [{"path": ".", "type": "gomod"}]}, + id="unsupported_flags", + ), + pytest.param( + {"pkg_managers": ["git-submodule"]}, + {"flags": [], "packages": []}, + id="unsupported_git_submodule", + ), +]) +def test_remote_source_to_cachi2_conversion(input_remote_source, expected_cachi2): + """Test conversion of remote_source (cachito) configuration from container yaml + into cachi2 params""" + assert remote_source_to_cachi2(input_remote_source) == expected_cachi2 From da568f3fda55987f81514b73c5e4dfe1cb40c1ed Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Thu, 15 Aug 2024 18:01:54 +0200 Subject: [PATCH 02/29] cachi2: generate ICM form SBOM Cachi2 doesn't generate ICM as Cachito 1, for backward compatibility generate ICM from SBOM Unfortunatelly cachi2 provides only flat structure, so dependencies are part of the flat structure they are not listed separatelly. STONEBLD-2582 Signed-off-by: Martin Basti --- atomic_reactor/utils/cachi2.py | 24 +++++++++++++++++ tests/utils/test_cachi2.py | 49 +++++++++++++++++++++++++++++++++- 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/atomic_reactor/utils/cachi2.py b/atomic_reactor/utils/cachi2.py index 3bbe3f2f4..f6491301a 100644 --- a/atomic_reactor/utils/cachi2.py +++ b/atomic_reactor/utils/cachi2.py @@ -44,3 +44,27 @@ def remote_source_to_cachi2(remote_source: Dict[str, Any]) -> Dict[str, Any]: cachi2_packages.append({"type": pkg_manager, **pkg}) return {"packages": cachi2_packages, "flags": cachi2_flags} + + +def convert_SBOM_to_ICM(sbom: Dict[str, Any]) -> Dict[str, Any]: + """Function converts cachi2 SBOM into ICM + + Unfortunately cachi2 doesn't provide all details about dependencies + and sources, so the ICM can contain only flat structure of everything + """ + icm = { + "metadata": { + "icm_spec": ( + "https://raw.githubusercontent.com/containerbuildsystem/atomic-reactor/" + "f4abcfdaf8247a6b074f94fa84f3846f82d781c6/atomic_reactor/schemas/" + "content_manifest.json" + ), + "icm_version": 1, + "image_layer_index": -1 + }, + "image_contents": [], + } + icm["image_contents"] = [ + {"purl": comp["purl"]} for comp in sbom["components"] # type: ignore + ] + return icm diff --git a/tests/utils/test_cachi2.py b/tests/utils/test_cachi2.py index fd5191f72..44ce18bd6 100644 --- a/tests/utils/test_cachi2.py +++ b/tests/utils/test_cachi2.py @@ -6,7 +6,10 @@ of the BSD license. See the LICENSE file for details. """ -from atomic_reactor.utils.cachi2 import remote_source_to_cachi2 +from atomic_reactor.utils.cachi2 import ( + convert_SBOM_to_ICM, + remote_source_to_cachi2 +) import pytest @@ -83,3 +86,47 @@ def test_remote_source_to_cachi2_conversion(input_remote_source, expected_cachi2 """Test conversion of remote_source (cachito) configuration from container yaml into cachi2 params""" assert remote_source_to_cachi2(input_remote_source) == expected_cachi2 + + +@pytest.mark.parametrize(('sbom', 'expected_icm'), [ + pytest.param( + { + "bomFormat": "CycloneDX", + "components": [{ + "name": "unsafe", + "purl": "pkg:golang/unsafe?type=package", + "properties": [{ + "name": "cachi2:found_by", + "value": "cachi2", + }], + "type": "library", + }], + "metadata": { + "tools": [{ + "vendor": "red hat", + "name": "cachi2" + }] + }, + "specVersion": "1.4", + "version": 1 + }, + { + "image_contents": [ + {"purl": "pkg:golang/unsafe?type=package"}, + ], + "metadata": { + "icm_spec": ( + "https://raw.githubusercontent.com/containerbuildsystem/atomic-reactor/" + "f4abcfdaf8247a6b074f94fa84f3846f82d781c6/atomic_reactor/" + "schemas/content_manifest.json" + ), + "icm_version": 1, + "image_layer_index": -1 + } + }, + id="easy", + ), +]) +def test_convert_SBOM_to_ICM(sbom, expected_icm): + """Test conversion from cachi2 SBOM into ICM format""" + assert convert_SBOM_to_ICM(sbom) == expected_icm From f18230642bc13c903ed05a51020902c761b3e20b Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Tue, 24 Sep 2024 11:27:43 +0200 Subject: [PATCH 03/29] cachi2: add init task Init task contains plugins which are responsible for fetching data and validation. It split from prebuild task. Prebuild task compared to init task, does changes to dockerfile. This is prerequisite for having replacebale cachito task with cachi2 task in future, as this task has to be between init and prebuild. Parameter --platforms-result has been moved to init task from prebuild task, as init task now runs check_and_set_platforms plugin. STONEBLD-2587 Signed-off-by: Martin Basti --- atomic_reactor/cli/parser.py | 11 ++++- atomic_reactor/cli/task.py | 17 +++++-- atomic_reactor/tasks/binary.py | 47 +++++++++++-------- tekton/pipelines/binary-container.yaml | 39 ++++++++++++++-- tekton/tasks/binary-container-init.yaml | 49 ++++++++++++++++++++ tekton/tasks/binary-container-prebuild.yaml | 7 +-- tests/cli/test_parser.py | 20 ++++++-- tests/cli/test_task.py | 11 +++-- tests/tasks/test_plugin_based.py | 51 +++++++++++++++++---- 9 files changed, 202 insertions(+), 50 deletions(-) create mode 100644 tekton/tasks/binary-container-init.yaml diff --git a/atomic_reactor/cli/parser.py b/atomic_reactor/cli/parser.py index c5072aeeb..c86c38385 100644 --- a/atomic_reactor/cli/parser.py +++ b/atomic_reactor/cli/parser.py @@ -78,14 +78,21 @@ def parse_args(args: Optional[Sequence[str]] = None) -> dict: ) clone.set_defaults(func=task.clone) + binary_container_init = tasks.add_parser( + "binary-container-init", + help="binary container pre-build step", + description="Execute binary container pre-build steps.", + ) + binary_container_init.set_defaults(func=task.binary_container_init) + binary_container_init.add_argument("--platforms-result", metavar="FILE", default=None, + help="file to write final platforms result") + binary_container_prebuild = tasks.add_parser( "binary-container-prebuild", help="binary container pre-build step", description="Execute binary container pre-build steps.", ) binary_container_prebuild.set_defaults(func=task.binary_container_prebuild) - binary_container_prebuild.add_argument("--platforms-result", metavar="FILE", default=None, - help="file to write final platforms result") binary_container_build = tasks.add_parser( "binary-container-build", diff --git a/atomic_reactor/cli/task.py b/atomic_reactor/cli/task.py index 15b844f54..8281be89e 100644 --- a/atomic_reactor/cli/task.py +++ b/atomic_reactor/cli/task.py @@ -6,7 +6,8 @@ of the BSD license. See the LICENSE file for details. """ from atomic_reactor.tasks.binary import (BinaryExitTask, BinaryPostBuildTask, BinaryPreBuildTask, - PreBuildTaskParams, BinaryExitTaskParams) + BinaryInitTask, + InitTaskParams, BinaryExitTaskParams) from atomic_reactor.tasks.binary_container_build import BinaryBuildTask, BinaryBuildTaskParams from atomic_reactor.tasks.clone import CloneTask from atomic_reactor.tasks.common import TaskParams @@ -44,14 +45,24 @@ def clone(task_args: dict): return task.run() +def binary_container_init(task_args: dict): + """Run binary container pre-build steps. + + :param task_args: CLI arguments for a binary-container-init task + """ + params = InitTaskParams.from_cli_args(task_args) + task = BinaryInitTask(params) + return task.run() + + def binary_container_prebuild(task_args: dict): """Run binary container pre-build steps. :param task_args: CLI arguments for a binary-container-prebuild task """ - params = PreBuildTaskParams.from_cli_args(task_args) + params = TaskParams.from_cli_args(task_args) task = BinaryPreBuildTask(params) - return task.run() + return task.run(init_build_dirs=True) def binary_container_build(task_args: dict): diff --git a/atomic_reactor/tasks/binary.py b/atomic_reactor/tasks/binary.py index 1491d9a5a..c7ad97d58 100644 --- a/atomic_reactor/tasks/binary.py +++ b/atomic_reactor/tasks/binary.py @@ -20,8 +20,8 @@ @dataclass(frozen=True) -class PreBuildTaskParams(TaskParams): - """Binary container prebuild task parameters""" +class InitTaskParams(TaskParams): + """Binary container init task parameters""" platforms_result: Optional[str] @@ -31,10 +31,10 @@ class BinaryExitTaskParams(TaskParams): annotations_result: Optional[str] -class BinaryPreBuildTask(plugin_based.PluginBasedTask[PreBuildTaskParams]): +class BinaryInitTask(plugin_based.PluginBasedTask[InitTaskParams]): """Binary container pre-build task.""" - task_name = 'binary_container_prebuild' + task_name = 'binary_container_init_build' plugins_conf = [ {"name": "distgit_fetch_artefacts"}, {"name": "check_and_set_platforms"}, @@ -44,23 +44,9 @@ class BinaryPreBuildTask(plugin_based.PluginBasedTask[PreBuildTaskParams]): {"name": "check_base_image"}, {"name": "koji_parent"}, {"name": "resolve_composes"}, - {"name": "flatpak_update_dockerfile"}, - {"name": "bump_release"}, - {"name": "add_flatpak_labels"}, - {"name": "add_labels_in_dockerfile"}, - {"name": "resolve_remote_source"}, {"name": "pin_operator_digest"}, - {"name": "add_help"}, {"name": "fetch_maven_artifacts"}, - {"name": "add_image_content_manifest"}, - {"name": "add_dockerfile"}, - {"name": "inject_yum_repos"}, - {"name": "add_filesystem"}, - {"name": "change_from_in_dockerfile"}, - {"name": "hide_files"}, - {"name": "distribution_scope"}, - {"name": "add_buildargs_in_dockerfile"}, - {"name": "tag_from_config"}, + ] def prepare_workflow(self) -> inner.DockerBuildWorkflow: @@ -82,6 +68,29 @@ def prepare_workflow(self) -> inner.DockerBuildWorkflow: return workflow +class BinaryPreBuildTask(plugin_based.PluginBasedTask[TaskParams]): + """Binary container pre-build task.""" + + task_name = 'binary_container_prebuild' + plugins_conf = [ + {"name": "flatpak_update_dockerfile"}, + {"name": "bump_release"}, + {"name": "add_flatpak_labels"}, + {"name": "add_labels_in_dockerfile"}, + {"name": "resolve_remote_source"}, + {"name": "add_help"}, + {"name": "add_image_content_manifest"}, + {"name": "add_dockerfile"}, + {"name": "inject_yum_repos"}, + {"name": "add_filesystem"}, + {"name": "change_from_in_dockerfile"}, + {"name": "hide_files"}, + {"name": "distribution_scope"}, + {"name": "add_buildargs_in_dockerfile"}, + {"name": "tag_from_config"}, + ] + + class BinaryPostBuildTask(plugin_based.PluginBasedTask[TaskParams]): """Binary container post-build task.""" diff --git a/tekton/pipelines/binary-container.yaml b/tekton/pipelines/binary-container.yaml index 5adb6a085..5a74c2a24 100644 --- a/tekton/pipelines/binary-container.yaml +++ b/tekton/pipelines/binary-container.yaml @@ -1,7 +1,7 @@ apiVersion: tekton.dev/v1beta1 kind: Pipeline metadata: - name: binary-container-0-1 # dot is not allowed in the name + name: binary-container-0-2 # dot is not allowed in the name spec: params: - name: osbs-image @@ -34,7 +34,7 @@ spec: - name: task_build_result_aarch64 value: $(tasks.binary-container-build-aarch64.results.task_result) - name: platforms_result - value: $(tasks.binary-container-prebuild.results.platforms_result) + value: $(tasks.binary-container-init.results.platforms_result) - name: annotations value: $(finally.binary-container-exit.results.annotations) @@ -66,11 +66,42 @@ spec: value: '$(params.user-params)' timeout: "0" - - name: binary-container-prebuild + - name: binary-container-init runAfter: - clone taskRef: - name: binary-container-prebuild-0-1 + name: binary-container-init-0-1 + workspaces: + - name: ws-build-dir + workspace: ws-container + subPath: build-dir + - name: ws-context-dir + workspace: ws-container + subPath: context-dir + - name: ws-home-dir + workspace: ws-home-dir + - name: ws-registries-secret + workspace: ws-registries-secret + - name: ws-koji-secret + workspace: ws-koji-secret + - name: ws-reactor-config-map + workspace: ws-reactor-config-map + - name: ws-autobot-keytab + workspace: ws-autobot-keytab + params: + - name: osbs-image + value: "$(params.osbs-image)" + - name: pipeline-run-name + value: "$(context.pipelineRun.name)" + - name: user-params + value: '$(params.user-params)' + timeout: "0" + + - name: binary-container-prebuild + runAfter: + - binary-container-init + taskRef: + name: binary-container-prebuild-0-2 workspaces: - name: ws-build-dir workspace: ws-container diff --git a/tekton/tasks/binary-container-init.yaml b/tekton/tasks/binary-container-init.yaml new file mode 100644 index 000000000..a54d9de5b --- /dev/null +++ b/tekton/tasks/binary-container-init.yaml @@ -0,0 +1,49 @@ +apiVersion: tekton.dev/v1beta1 +kind: Task +metadata: + name: binary-container-init-0-1 # dot is not allowed in the name +spec: + description: >- + OSBS init task for building binary container image + params: + - name: osbs-image + description: The location of the OSBS builder image (FQDN pullspec) + type: string + - name: pipeline-run-name + type: string + description: PipelineRun name to reference current PipelineRun + - name: user-params + type: string + description: User parameters in JSON format + + workspaces: + - name: ws-build-dir + - name: ws-context-dir + - name: ws-home-dir + - name: ws-registries-secret # access with $(workspaces.ws-registries-secret.path)/token + - name: ws-koji-secret # access with $(workspaces.ws-koji-secret.path)/token + - name: ws-reactor-config-map + - name: ws-autobot-keytab + + results: + - name: platforms_result + + stepTemplate: + env: + - name: HOME + value: $(workspaces.ws-home-dir.path) + + steps: + - name: binary-container-init + image: $(params.osbs-image) + workingDir: $(workspaces.ws-home-dir.path) + resources: + requests: + memory: 512Mi + cpu: 250m + limits: + memory: 1Gi + cpu: 395m + script: | + set -x + atomic-reactor -v task --user-params='$(params.user-params)' --build-dir=$(workspaces.ws-build-dir.path) --context-dir=$(workspaces.ws-context-dir.path) --config-file=$(workspaces.ws-reactor-config-map.path)/config.yaml --namespace=$(context.taskRun.namespace) --pipeline-run-name="$(params.pipeline-run-name)" binary-container-init --platforms-result=$(results.platforms_result.path) diff --git a/tekton/tasks/binary-container-prebuild.yaml b/tekton/tasks/binary-container-prebuild.yaml index 122657a8d..1f634d0ff 100644 --- a/tekton/tasks/binary-container-prebuild.yaml +++ b/tekton/tasks/binary-container-prebuild.yaml @@ -1,7 +1,7 @@ apiVersion: tekton.dev/v1beta1 kind: Task metadata: - name: binary-container-prebuild-0-1 # dot is not allowed in the name + name: binary-container-prebuild-0-2 # dot is not allowed in the name spec: description: >- OSBS prebuild task for building binary container image @@ -25,9 +25,6 @@ spec: - name: ws-reactor-config-map - name: ws-autobot-keytab - results: - - name: platforms_result - stepTemplate: env: - name: HOME @@ -46,4 +43,4 @@ spec: cpu: 395m script: | set -x - atomic-reactor -v task --user-params='$(params.user-params)' --build-dir=$(workspaces.ws-build-dir.path) --context-dir=$(workspaces.ws-context-dir.path) --config-file=$(workspaces.ws-reactor-config-map.path)/config.yaml --namespace=$(context.taskRun.namespace) --pipeline-run-name="$(params.pipeline-run-name)" binary-container-prebuild --platforms-result=$(results.platforms_result.path) + atomic-reactor -v task --user-params='$(params.user-params)' --build-dir=$(workspaces.ws-build-dir.path) --context-dir=$(workspaces.ws-context-dir.path) --config-file=$(workspaces.ws-reactor-config-map.path)/config.yaml --namespace=$(context.taskRun.namespace) --pipeline-run-name="$(params.pipeline-run-name)" binary-container-prebuild diff --git a/tests/cli/test_parser.py b/tests/cli/test_parser.py index c20c24724..6caebd9fc 100644 --- a/tests/cli/test_parser.py +++ b/tests/cli/test_parser.py @@ -41,7 +41,7 @@ **EXPECTED_ARGS, "platform": "x86_64", } -EXPECTED_ARGS_CONTAINER_PREBUILD = { +EXPECTED_ARGS_CONTAINER_INIT = { **EXPECTED_ARGS, "platforms_result": None, } @@ -79,9 +79,13 @@ def test_parse_args_version(capsys): ["task", *REQUIRED_COMMON_ARGS, "clone"], {**EXPECTED_ARGS, "func": task.clone}, ), + ( + ["task", *REQUIRED_COMMON_ARGS, "binary-container-init"], + {**EXPECTED_ARGS_CONTAINER_INIT, "func": task.binary_container_init}, + ), ( ["task", *REQUIRED_COMMON_ARGS, "binary-container-prebuild"], - {**EXPECTED_ARGS_CONTAINER_PREBUILD, "func": task.binary_container_prebuild}, + {**EXPECTED_ARGS, "func": task.binary_container_prebuild}, ), ( ["task", *REQUIRED_COMMON_ARGS, "binary-container-build", @@ -107,10 +111,16 @@ def test_parse_args_version(capsys): ["task", *REQUIRED_COMMON_ARGS, "--config-file=config.yaml", "clone"], {**EXPECTED_ARGS, "config_file": "config.yaml", "func": task.clone}, ), + ( + ["task", *REQUIRED_COMMON_ARGS, "--config-file=config.yaml", + "binary-container-init"], + {**EXPECTED_ARGS_CONTAINER_INIT, "config_file": "config.yaml", + "func": task.binary_container_init}, + ), ( ["task", *REQUIRED_COMMON_ARGS, "--config-file=config.yaml", "binary-container-prebuild"], - {**EXPECTED_ARGS_CONTAINER_PREBUILD, "config_file": "config.yaml", + {**EXPECTED_ARGS, "config_file": "config.yaml", "func": task.binary_container_prebuild}, ), ( @@ -132,10 +142,10 @@ def test_parse_args_version(capsys): "func": task.binary_container_postbuild}, ), ( - ["task", *REQUIRED_COMMON_ARGS, "binary-container-prebuild", + ["task", *REQUIRED_COMMON_ARGS, "binary-container-init", "--platforms-result=platforms_file"], {**EXPECTED_ARGS, "platforms_result": "platforms_file", - "func": task.binary_container_prebuild}, + "func": task.binary_container_init}, ), ( ["task", *REQUIRED_COMMON_ARGS, "--config-file=config.yaml", "binary-container-exit", diff --git a/tests/cli/test_task.py b/tests/cli/test_task.py index 0c39cf7da..16d03a803 100644 --- a/tests/cli/test_task.py +++ b/tests/cli/test_task.py @@ -28,7 +28,7 @@ "config_file": "reactor-config-map.yaml", "user_params": '{"some_param": "some_value"}', } -PRE_TASK_ARGS = { +INIT_TASK_ARGS = { **TASK_ARGS, "platforms_result": 'platform_result', } @@ -60,9 +60,14 @@ def test_source_build(): assert task.source_container_build(TASK_ARGS) == TASK_RESULT +def test_binary_container_init(): + mock(binary.BinaryInitTask, task_args=INIT_TASK_ARGS) + assert task.binary_container_init(INIT_TASK_ARGS) == TASK_RESULT + + def test_binary_container_prebuild(): - mock(binary.BinaryPreBuildTask, task_args=PRE_TASK_ARGS) - assert task.binary_container_prebuild(PRE_TASK_ARGS) == TASK_RESULT + mock(binary.BinaryPreBuildTask, task_args=TASK_ARGS) + assert task.binary_container_prebuild(TASK_ARGS) == TASK_RESULT def test_binary_container_build(): diff --git a/tests/tasks/test_plugin_based.py b/tests/tasks/test_plugin_based.py index ccb7c5b5d..f951e201c 100644 --- a/tests/tasks/test_plugin_based.py +++ b/tests/tasks/test_plugin_based.py @@ -17,7 +17,7 @@ from atomic_reactor.inner import ImageBuildWorkflowData from atomic_reactor.plugin import TaskCanceledException, PluginFailedException from atomic_reactor.tasks.common import TaskParams -from atomic_reactor.tasks.binary import PreBuildTaskParams, BinaryPreBuildTask +from atomic_reactor.tasks.binary import InitTaskParams, BinaryPreBuildTask, BinaryInitTask from atomic_reactor.util import DockerfileImages from atomic_reactor import inner, dirs @@ -194,18 +194,51 @@ def _cancel_build(*args, **kwargs): assert {} == wf_data.plugins_results +def test_ensure_workflow_data_is_saved_init_task( + build_dir, dummy_source, tmpdir +): + context_dir = tmpdir.join("context_dir").mkdir() + params = InitTaskParams(build_dir=str(build_dir), + config_file="config.yaml", + context_dir=str(context_dir), + namespace="test-namespace", + pipeline_run_name='test-pipeline-run', + user_params={}, + task_result='results', + platforms_result='platforms_result') + (flexmock(params) + .should_receive("source") + .and_return(dummy_source)) + + task = BinaryInitTask(params) + + (flexmock(plugin_based.inner.DockerBuildWorkflow) + .should_receive("build_container_image") + .once()) + + task.run() + time.sleep(1) + assert context_dir.join("workflow.json").exists() + + wf_data = ImageBuildWorkflowData() + wf_data.load_from_dir(ContextDir(Path(context_dir))) + # As long as the data is loaded successfully, just check some + # attributes to check the data. + assert DockerfileImages() == wf_data.dockerfile_images + assert {} == wf_data.plugins_results + + def test_ensure_workflow_data_is_saved_prebuild_task( build_dir, dummy_source, tmpdir ): context_dir = tmpdir.join("context_dir").mkdir() - params = PreBuildTaskParams(build_dir=str(build_dir), - config_file="config.yaml", - context_dir=str(context_dir), - namespace="test-namespace", - pipeline_run_name='test-pipeline-run', - user_params={}, - task_result='results', - platforms_result='platforms_result') + params = TaskParams(build_dir=str(build_dir), + config_file="config.yaml", + context_dir=str(context_dir), + namespace="test-namespace", + pipeline_run_name='test-pipeline-run', + user_params={}, + task_result='results') (flexmock(params) .should_receive("source") .and_return(dummy_source)) From 2ad58b0357b299609af1958c3bf3cb32f102b4e0 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Tue, 24 Sep 2024 22:30:33 +0200 Subject: [PATCH 04/29] cachi2: separate cachito task Separating resolve_remote_sources into a standalone cachito task in binary build, to allow us replacement with cachi2 in future. STONEBLD-2587 Signed-off-by: Martin Basti --- atomic_reactor/cli/parser.py | 7 ++++ atomic_reactor/cli/task.py | 12 +++++- atomic_reactor/tasks/binary.py | 10 ++++- tekton/pipelines/binary-container.yaml | 33 +++++++++++++++- tekton/tasks/binary-container-cachito.yaml | 46 ++++++++++++++++++++++ tests/cli/test_parser.py | 10 +++++ tests/cli/test_task.py | 5 +++ tests/tasks/test_plugin_based.py | 11 ++++-- 8 files changed, 127 insertions(+), 7 deletions(-) create mode 100644 tekton/tasks/binary-container-cachito.yaml diff --git a/atomic_reactor/cli/parser.py b/atomic_reactor/cli/parser.py index c86c38385..e488634ad 100644 --- a/atomic_reactor/cli/parser.py +++ b/atomic_reactor/cli/parser.py @@ -87,6 +87,13 @@ def parse_args(args: Optional[Sequence[str]] = None) -> dict: binary_container_init.add_argument("--platforms-result", metavar="FILE", default=None, help="file to write final platforms result") + binary_container_cachito = tasks.add_parser( + "binary-container-cachito", + help="binary container cachito step", + description="Execute binary container cachito steps.", + ) + binary_container_cachito.set_defaults(func=task.binary_container_cachito) + binary_container_prebuild = tasks.add_parser( "binary-container-prebuild", help="binary container pre-build step", diff --git a/atomic_reactor/cli/task.py b/atomic_reactor/cli/task.py index 8281be89e..4e6d871b3 100644 --- a/atomic_reactor/cli/task.py +++ b/atomic_reactor/cli/task.py @@ -6,7 +6,7 @@ of the BSD license. See the LICENSE file for details. """ from atomic_reactor.tasks.binary import (BinaryExitTask, BinaryPostBuildTask, BinaryPreBuildTask, - BinaryInitTask, + BinaryInitTask, BinaryCachitoTask, InitTaskParams, BinaryExitTaskParams) from atomic_reactor.tasks.binary_container_build import BinaryBuildTask, BinaryBuildTaskParams from atomic_reactor.tasks.clone import CloneTask @@ -55,6 +55,16 @@ def binary_container_init(task_args: dict): return task.run() +def binary_container_cachito(task_args: dict): + """Run binary container Cachito steps. + + :param task_args: CLI arguments for a binary-container-cachito task + """ + params = TaskParams.from_cli_args(task_args) + task = BinaryCachitoTask(params) + return task.run(init_build_dirs=True) + + def binary_container_prebuild(task_args: dict): """Run binary container pre-build steps. diff --git a/atomic_reactor/tasks/binary.py b/atomic_reactor/tasks/binary.py index c7ad97d58..88f285afd 100644 --- a/atomic_reactor/tasks/binary.py +++ b/atomic_reactor/tasks/binary.py @@ -68,6 +68,15 @@ def prepare_workflow(self) -> inner.DockerBuildWorkflow: return workflow +class BinaryCachitoTask(plugin_based.PluginBasedTask[TaskParams]): + """Binary container Cachito task.""" + + task_name = 'binary_container_cachito' + plugins_conf = [ + {"name": "resolve_remote_source"}, + ] + + class BinaryPreBuildTask(plugin_based.PluginBasedTask[TaskParams]): """Binary container pre-build task.""" @@ -77,7 +86,6 @@ class BinaryPreBuildTask(plugin_based.PluginBasedTask[TaskParams]): {"name": "bump_release"}, {"name": "add_flatpak_labels"}, {"name": "add_labels_in_dockerfile"}, - {"name": "resolve_remote_source"}, {"name": "add_help"}, {"name": "add_image_content_manifest"}, {"name": "add_dockerfile"}, diff --git a/tekton/pipelines/binary-container.yaml b/tekton/pipelines/binary-container.yaml index 5a74c2a24..7afcd2a95 100644 --- a/tekton/pipelines/binary-container.yaml +++ b/tekton/pipelines/binary-container.yaml @@ -97,9 +97,40 @@ spec: value: '$(params.user-params)' timeout: "0" - - name: binary-container-prebuild + - name: binary-container-cachito runAfter: - binary-container-init + taskRef: + name: binary-container-cachito-0-1 + workspaces: + - name: ws-build-dir + workspace: ws-container + subPath: build-dir + - name: ws-context-dir + workspace: ws-container + subPath: context-dir + - name: ws-home-dir + workspace: ws-home-dir + - name: ws-registries-secret + workspace: ws-registries-secret + - name: ws-koji-secret + workspace: ws-koji-secret + - name: ws-reactor-config-map + workspace: ws-reactor-config-map + - name: ws-autobot-keytab + workspace: ws-autobot-keytab + params: + - name: osbs-image + value: "$(params.osbs-image)" + - name: pipeline-run-name + value: "$(context.pipelineRun.name)" + - name: user-params + value: '$(params.user-params)' + timeout: "0" + + - name: binary-container-prebuild + runAfter: + - binary-container-cachito taskRef: name: binary-container-prebuild-0-2 workspaces: diff --git a/tekton/tasks/binary-container-cachito.yaml b/tekton/tasks/binary-container-cachito.yaml new file mode 100644 index 000000000..5ad07664d --- /dev/null +++ b/tekton/tasks/binary-container-cachito.yaml @@ -0,0 +1,46 @@ +apiVersion: tekton.dev/v1beta1 +kind: Task +metadata: + name: binary-container-cachito-0-1 # dot is not allowed in the name +spec: + description: >- + OSBS cachito task for building binary container image + params: + - name: osbs-image + description: The location of the OSBS builder image (FQDN pullspec) + type: string + - name: pipeline-run-name + type: string + description: PipelineRun name to reference current PipelineRun + - name: user-params + type: string + description: User parameters in JSON format + + workspaces: + - name: ws-build-dir + - name: ws-context-dir + - name: ws-home-dir + - name: ws-registries-secret # access with $(workspaces.ws-registries-secret.path)/token + - name: ws-koji-secret # access with $(workspaces.ws-koji-secret.path)/token + - name: ws-reactor-config-map + - name: ws-autobot-keytab + + stepTemplate: + env: + - name: HOME + value: $(workspaces.ws-home-dir.path) + + steps: + - name: binary-container-cachito + image: $(params.osbs-image) + workingDir: $(workspaces.ws-home-dir.path) + resources: + requests: + memory: 512Mi + cpu: 250m + limits: + memory: 1Gi + cpu: 395m + script: | + set -x + atomic-reactor -v task --user-params='$(params.user-params)' --build-dir=$(workspaces.ws-build-dir.path) --context-dir=$(workspaces.ws-context-dir.path) --config-file=$(workspaces.ws-reactor-config-map.path)/config.yaml --namespace=$(context.taskRun.namespace) --pipeline-run-name="$(params.pipeline-run-name)" binary-container-cachito diff --git a/tests/cli/test_parser.py b/tests/cli/test_parser.py index 6caebd9fc..8b3c66683 100644 --- a/tests/cli/test_parser.py +++ b/tests/cli/test_parser.py @@ -83,6 +83,10 @@ def test_parse_args_version(capsys): ["task", *REQUIRED_COMMON_ARGS, "binary-container-init"], {**EXPECTED_ARGS_CONTAINER_INIT, "func": task.binary_container_init}, ), + ( + ["task", *REQUIRED_COMMON_ARGS, "binary-container-cachito"], + {**EXPECTED_ARGS, "func": task.binary_container_cachito}, + ), ( ["task", *REQUIRED_COMMON_ARGS, "binary-container-prebuild"], {**EXPECTED_ARGS, "func": task.binary_container_prebuild}, @@ -117,6 +121,12 @@ def test_parse_args_version(capsys): {**EXPECTED_ARGS_CONTAINER_INIT, "config_file": "config.yaml", "func": task.binary_container_init}, ), + ( + ["task", *REQUIRED_COMMON_ARGS, "--config-file=config.yaml", + "binary-container-cachito"], + {**EXPECTED_ARGS, "config_file": "config.yaml", + "func": task.binary_container_cachito}, + ), ( ["task", *REQUIRED_COMMON_ARGS, "--config-file=config.yaml", "binary-container-prebuild"], diff --git a/tests/cli/test_task.py b/tests/cli/test_task.py index 16d03a803..952a3d4c6 100644 --- a/tests/cli/test_task.py +++ b/tests/cli/test_task.py @@ -65,6 +65,11 @@ def test_binary_container_init(): assert task.binary_container_init(INIT_TASK_ARGS) == TASK_RESULT +def test_binary_container_cachito(): + mock(binary.BinaryCachitoTask, task_args=TASK_ARGS) + assert task.binary_container_cachito(TASK_ARGS) == TASK_RESULT + + def test_binary_container_prebuild(): mock(binary.BinaryPreBuildTask, task_args=TASK_ARGS) assert task.binary_container_prebuild(TASK_ARGS) == TASK_RESULT diff --git a/tests/tasks/test_plugin_based.py b/tests/tasks/test_plugin_based.py index f951e201c..2e24d2cad 100644 --- a/tests/tasks/test_plugin_based.py +++ b/tests/tasks/test_plugin_based.py @@ -17,7 +17,9 @@ from atomic_reactor.inner import ImageBuildWorkflowData from atomic_reactor.plugin import TaskCanceledException, PluginFailedException from atomic_reactor.tasks.common import TaskParams -from atomic_reactor.tasks.binary import InitTaskParams, BinaryPreBuildTask, BinaryInitTask +from atomic_reactor.tasks.binary import ( + InitTaskParams, BinaryPreBuildTask, BinaryInitTask, BinaryCachitoTask, +) from atomic_reactor.util import DockerfileImages from atomic_reactor import inner, dirs @@ -228,8 +230,9 @@ def test_ensure_workflow_data_is_saved_init_task( assert {} == wf_data.plugins_results -def test_ensure_workflow_data_is_saved_prebuild_task( - build_dir, dummy_source, tmpdir +@pytest.mark.parametrize("taskfunc", [BinaryCachitoTask, BinaryPreBuildTask]) +def test_ensure_workflow_data_is_saved_standard_task( + build_dir, dummy_source, tmpdir, taskfunc ): context_dir = tmpdir.join("context_dir").mkdir() params = TaskParams(build_dir=str(build_dir), @@ -243,7 +246,7 @@ def test_ensure_workflow_data_is_saved_prebuild_task( .should_receive("source") .and_return(dummy_source)) - task = BinaryPreBuildTask(params) + task = taskfunc(params) (flexmock(plugin_based.inner.DockerBuildWorkflow) .should_receive("build_container_image") From 451d09d13e8cc853b4e66ea55cefd9707c243825 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Tue, 8 Oct 2024 17:13:57 +0200 Subject: [PATCH 05/29] DO NOt MERGE TO MAIN: Allow to run tasks from feature branch This commit must be removed before merging to main branch. Signed-off-by: Martin Basti --- tekton/pipelines/binary-container.yaml | 27 +++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/tekton/pipelines/binary-container.yaml b/tekton/pipelines/binary-container.yaml index 7afcd2a95..af2d17142 100644 --- a/tekton/pipelines/binary-container.yaml +++ b/tekton/pipelines/binary-container.yaml @@ -70,7 +70,14 @@ spec: runAfter: - clone taskRef: - name: binary-container-init-0-1 + resolver: git + params: + - name: url + value: https://github.com/containerbuildsystem/atomic-reactor.git + - name: revision + value: feature_cachi2 + - name: pathInRepo + value: tekton/tasks/binary-container-init.yaml workspaces: - name: ws-build-dir workspace: ws-container @@ -101,7 +108,14 @@ spec: runAfter: - binary-container-init taskRef: - name: binary-container-cachito-0-1 + resolver: git + params: + - name: url + value: https://github.com/containerbuildsystem/atomic-reactor.git + - name: revision + value: feature_cachi2 + - name: pathInRepo + value: tekton/tasks/binary-container-cachito.yaml workspaces: - name: ws-build-dir workspace: ws-container @@ -132,7 +146,14 @@ spec: runAfter: - binary-container-cachito taskRef: - name: binary-container-prebuild-0-2 + resolver: git + params: + - name: url + value: https://github.com/containerbuildsystem/atomic-reactor.git + - name: revision + value: feature_cachi2 + - name: pathInRepo + value: tekton/tasks/binary-container-prebuild.yaml workspaces: - name: ws-build-dir workspace: ws-container From 49dcf2f98f66fd90f7f37110aa0be336f57fb1c1 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Tue, 20 Aug 2024 14:26:48 +0200 Subject: [PATCH 06/29] cachi2: generate remote-source.json Cachi2 doesn't provide remote-source.json but for backward compatibility OSBS must provide it. STONEBLD-2585 Signed-off-by: Martin Basti --- atomic_reactor/utils/cachi2.py | 83 +++++++++- requirements-devel.txt | 2 + requirements.in | 1 + requirements.txt | 2 + tests/utils/test_cachi2.py | 293 ++++++++++++++++++++++++++++++++- 5 files changed, 379 insertions(+), 2 deletions(-) diff --git a/atomic_reactor/utils/cachi2.py b/atomic_reactor/utils/cachi2.py index f6491301a..cc6f2c540 100644 --- a/atomic_reactor/utils/cachi2.py +++ b/atomic_reactor/utils/cachi2.py @@ -8,7 +8,9 @@ Utils to help to integrate with cachi2 CLI tool """ -from typing import Any, Dict +from typing import Any, Callable, Dict, Optional, Tuple + +from packageurl import PackageURL def remote_source_to_cachi2(remote_source: Dict[str, Any]) -> Dict[str, Any]: @@ -68,3 +70,82 @@ def convert_SBOM_to_ICM(sbom: Dict[str, Any]) -> Dict[str, Any]: {"purl": comp["purl"]} for comp in sbom["components"] # type: ignore ] return icm + + +def gen_dependency_from_sbom_component(sbom_dep: Dict[str, Any]) -> Dict[str, Optional[str]]: + """Generate a single request.json dependency from a SBOM component + + Dependency type is derived from purl. + Version is decided on how Cachito would do it. + """ + # we need to detect type from purl, this is just heuristics, + # we cannot reliably construct type from purl + purl = PackageURL.from_string(sbom_dep["purl"]) + heuristic_type = purl.type or "unknown" # for unknown types, reuse what's in purl type + # types supported by cachito/cachi2 + purl_type_matchers: Tuple[Tuple[Callable[[PackageURL], bool], str], ...] = ( + (lambda p: p.type == "golang" and p.qualifiers.get("type", "") == "module", "gomod"), + (lambda p: p.type == "golang", "go-package"), + (lambda p: p.type == "npm", "npm"), + (lambda p: p.type == "pypi", "pip"), + (lambda p: p.type == "rpm", "rpm"), + (lambda p: p.type == "gem", "rubygems"), + (lambda p: p.type == "cargo", "cargo"), + ) + + for matcher, request_type in purl_type_matchers: + if matcher(purl): + heuristic_type = request_type + break + + version = ( + # for non-registry dependencies cachito uses URL as version + purl.qualifiers.get("vcs_url") or + purl.qualifiers.get("download_url") or + # for local dependencies Cachito uses path as version + (f"./{purl.subpath}" if purl.subpath and purl.type == "golang" else None) or + (f"file:{purl.subpath}" if purl.subpath and purl.type != "golang" else None) or + # version is mainly for dependencies from pkg registries + sbom_dep.get("version") + # returns None if version cannot be determined + ) + + res = { + "name": sbom_dep["name"], + "replaces": None, # it's always None, replacements aren't supported by cachi2 + "type": heuristic_type, + "version": version, + } + + # dev package definition + # currently only NPM + if any(p["name"] == "cdx:npm:package:development" and p["value"] == "true" + for p in sbom_dep.get("properties", [])): + res["dev"] = True + + return res + + +def generate_request_json( + remote_source: Dict[str, Any], remote_source_sbom: Dict[str, Any], + remote_source_env_json: Dict[str, Dict[str, str]], +) -> Dict[str, Any]: + """Generates Cachito like request.json + + Cachito does provide request.json, for backward compatibility + as some tools are depending on it, we have to generate also request.json from cachi2 + """ + + res = { + "dependencies": [ + gen_dependency_from_sbom_component(dep) + for dep in remote_source_sbom["components"] + ], + "pkg_managers": remote_source.get("pkg_managers", []), + "ref": remote_source["ref"], + "repo": remote_source["repo"], + "environment_variables": {key: val["value"] for key, val in remote_source_env_json.items()}, + "flags": remote_source.get("flags", []), + "packages": [], # this will be always empty cachi2 doesn't provide nested deps + } + return res diff --git a/requirements-devel.txt b/requirements-devel.txt index 75ccb0aac..265827d15 100644 --- a/requirements-devel.txt +++ b/requirements-devel.txt @@ -128,6 +128,8 @@ osbs-client @ git+https://github.com/containerbuildsystem/osbs-client@628b0707b5 # via -r requirements.in otel-extensions==0.2.5 # via -r otel-requirements.in +packageurl-python==0.15.6 + # via -r requirements.in packaging==23.1 # via # hatchling diff --git a/requirements.in b/requirements.in index d1a0c2559..6ef7f5ac1 100644 --- a/requirements.in +++ b/requirements.in @@ -15,3 +15,4 @@ PyGObject reflink urllib3 < 2.0 setuptools<=70.3.0 +packageurl-python >= 0.15.6 diff --git a/requirements.txt b/requirements.txt index 3899009a4..20e93b7fd 100644 --- a/requirements.txt +++ b/requirements.txt @@ -115,6 +115,8 @@ osbs-client @ git+https://github.com/containerbuildsystem/osbs-client@628b0707b5 # via -r requirements.in otel-extensions==0.2.5 # via -r otel-requirements.in +packageurl-python==0.15.6 + # via -r requirements.in packaging==23.1 # via hatchling paramiko==3.4.1 diff --git a/tests/utils/test_cachi2.py b/tests/utils/test_cachi2.py index 44ce18bd6..363db0df8 100644 --- a/tests/utils/test_cachi2.py +++ b/tests/utils/test_cachi2.py @@ -8,7 +8,9 @@ from atomic_reactor.utils.cachi2 import ( convert_SBOM_to_ICM, - remote_source_to_cachi2 + remote_source_to_cachi2, + gen_dependency_from_sbom_component, + generate_request_json, ) import pytest @@ -130,3 +132,292 @@ def test_remote_source_to_cachi2_conversion(input_remote_source, expected_cachi2 def test_convert_SBOM_to_ICM(sbom, expected_icm): """Test conversion from cachi2 SBOM into ICM format""" assert convert_SBOM_to_ICM(sbom) == expected_icm + + +@pytest.mark.parametrize(("sbom_comp", "expected"), [ + pytest.param( + { + "name": "github.com/cachito-testing/testmodule", + "version": "v1.0.0", + "purl": "pkg:golang/github.com/cachito-testing/testmodule@v1.0.0?type=module", + "type": "library" + }, + { + "name": "github.com/cachito-testing/testmodule", + "replaces": None, + "type": "gomod", + "version": "v1.0.0" + }, + id="type_gomod" + ), + pytest.param( + { + "name": "github.com/cachito-testing", + "version": "v1.0.0", + "purl": "pkg:golang/github.com/cachito-testing@v1.0.0", + "type": "library" + }, + { + "name": "github.com/cachito-testing", + "replaces": None, + "type": "go-package", + "version": "v1.0.0" + }, + id="type_go-package" + ), + pytest.param( + { + "name": "cachito-npm-without-deps", + "purl": ("pkg:npm/cachito-npm-without-deps?" + "vcs_url=git%2Bhttps://github.com/cachito-testing/" + "cachito-npm-without-deps.git%402f0ce1d7b1f8b35572d919428b965285a69583f6"), + "type": "library" + }, + { + "name": "cachito-npm-without-deps", + "replaces": None, + "type": "npm", + "version": ( + "git+https://github.com/cachito-testing/cachito-npm-without-deps.git@" + "2f0ce1d7b1f8b35572d919428b965285a69583f6"), + }, + id="type_npm" + ), + pytest.param( + { + "name": "aiowsgi", + "version": "0.8", + "purl": "pkg:pypi/aiowsgi@0.8", + "type": "library" + }, + { + "name": "aiowsgi", + "replaces": None, + "type": "pip", + "version": "0.8" + }, + id="type_pip" + ), + pytest.param( + { + "name": "validate_url", + "version": "1.0.5", + "purl": "pkg:gem/validate_url@1.0.5", + "type": "library" + }, + { + "name": "validate_url", + "replaces": None, + "type": "rubygems", + "version": "1.0.5" + }, + id="type_rubygem" + ), + pytest.param( + { + "name": "cachito-testing", + "version": "1.0.0-5", + "purl": "pkg:rpm/cachito-testing@1.0.0-5", + "type": "library" + }, + { + "name": "cachito-testing", + "replaces": None, + "type": "rpm", + "version": "1.0.0-5" + }, + id="type_rpm" + ), + pytest.param( + { + "name": "github.com/cachito-testing", + "version": "v1.0.0", + "purl": "pkg:somethingnew/github.com/cachito-testing@v1.0.0", + "type": "library" + }, + { + "name": "github.com/cachito-testing", + "replaces": None, + "type": "somethingnew", + "version": "v1.0.0" + }, + id="type_somethingnew" + ), + pytest.param( + { + "name": "github.com/cachito-testing", + "version": "v1.0.0", + "purl": "pkg:golang/github.com/cachito-testing#path", + "type": "library" + }, + { + "name": "github.com/cachito-testing", + "replaces": None, + "type": "go-package", + "version": "./path" + }, + id="version_golang_path" + ), + pytest.param( + { + # vcs_url has priority + "name": "cachito-npm-without-deps", + "purl": ("pkg:npm/cachito-npm-without-deps?" + "vcs_url=git%2Bhttps://github.com/cachito-testing/" + "cachito-npm-without-deps.git%402f0ce1d7b1f8b35572d919428b965285a69583f6&" + "download_url=https://example.com/pkg#path"), + "type": "library", + "version": "1.2.3" + }, + { + "name": "cachito-npm-without-deps", + "replaces": None, + "type": "npm", + "version": ( + "git+https://github.com/cachito-testing/cachito-npm-without-deps.git@" + "2f0ce1d7b1f8b35572d919428b965285a69583f6"), + }, + id="version_vsc_url" + ), + pytest.param( + { + # download_url has priority + "name": "cachito-npm-without-deps", + "purl": ("pkg:npm/cachito-npm-without-deps?" + "download_url=https://example.com/pkg#path"), + "type": "library", + "version": "1.2.3" + }, + { + "name": "cachito-npm-without-deps", + "replaces": None, + "type": "npm", + "version": "https://example.com/pkg", + }, + id="version_download_url" + ), + pytest.param( + { + # path has priority + "name": "cachito-npm-without-deps", + "purl": "pkg:npm/cachito-npm-without-deps#path", + "type": "library", + "version": "1.0.0" + }, + { + "name": "cachito-npm-without-deps", + "replaces": None, + "type": "npm", + "version": "file:path", + }, + id="version_path" + ), + pytest.param( + { + "name": "cachito-npm-without-deps", + "purl": "pkg:npm/cachito-npm-without-deps", + "type": "library", + "version": "1.0.0" + }, + { + "name": "cachito-npm-without-deps", + "replaces": None, + "type": "npm", + "version": "1.0.0", + }, + id="version_version" + ), + pytest.param( + { + "name": "cachito-npm-without-deps", + "purl": "pkg:npm/cachito-npm-without-deps", + "type": "library", + }, + { + "name": "cachito-npm-without-deps", + "replaces": None, + "type": "npm", + "version": None, + }, + id="version_missing" + ), + pytest.param( + { + "name": "cachito-npm-without-deps", + "purl": "pkg:npm/cachito-npm-without-deps", + "type": "library", + "properties": [ + { + "name": "cdx:npm:package:development", + "value": "true" + } + ], + }, + { + "name": "cachito-npm-without-deps", + "replaces": None, + "type": "npm", + "version": None, + "dev": True, + }, + id="npm_dev" + ), +]) +def test_gen_dependency_from_sbom_component(sbom_comp, expected): + """Test generating request.json dependency from sbom component""" + assert gen_dependency_from_sbom_component(sbom_comp) == expected + + +def test_generate_request_json(): + """Test generating request.json from cachi2 SBOM and OSBS metadata""" + remote_source = { + "repo": "https://example.com/org/repo.git", + "ref": "7d669deedc3fd0e9199213e1f66056bb9f388394", + "pkg_managers": ["gomod"], + "flags": ["gomod-vendor-check"] + } + + remote_source_sbom = { + "bomFormat": "CycloneDX", + "components": [ + { + "name": "bytes", + "purl": "pkg:golang/bytes?type=package", + "properties": [ + { + "name": "cachi2:found_by", + "value": "cachi2" + } + ], + "type": "library" + }, + ], + } + + remote_source_env_json = { + "GOCACHE": { + "kind": "path", + "value": "deps/gomod" + }, + } + + expected = { + "dependencies": [ + { + "name": "bytes", + "replaces": None, + "type": "go-package", + "version": None, + }, + ], + "pkg_managers": ["gomod"], + "repo": "https://example.com/org/repo.git", + "ref": "7d669deedc3fd0e9199213e1f66056bb9f388394", + "environment_variables": {"GOCACHE": "deps/gomod"}, + "flags": ["gomod-vendor-check"], + "packages": [], + } + + assert generate_request_json( + remote_source, remote_source_sbom, remote_source_env_json + ) == expected From dd03090e4726d6e400f8d013f9e718c218e68818 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Thu, 31 Oct 2024 14:50:31 +0100 Subject: [PATCH 07/29] cachi2: replace rubygems with bundler Rubygems pkg_manager is named bundler in cachi, name must be converted. STONEBLD-2919 Signed-off-by: Martin Basti --- atomic_reactor/utils/cachi2.py | 7 +++++++ tests/utils/test_cachi2.py | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/atomic_reactor/utils/cachi2.py b/atomic_reactor/utils/cachi2.py index cc6f2c540..d9e1ee444 100644 --- a/atomic_reactor/utils/cachi2.py +++ b/atomic_reactor/utils/cachi2.py @@ -28,6 +28,10 @@ def remote_source_to_cachi2(remote_source: Dict[str, Any]) -> Dict[str, Any]: * git-submodule """ + pkg_managers_map = { + "rubygems": "bundler" # renamed in cachi2 + } + removed_flags = {"include-git-dir", "remove-unsafe-symlinks"} removed_pkg_managers = {"git-submodule"} @@ -40,6 +44,9 @@ def remote_source_to_cachi2(remote_source: Dict[str, Any]) -> Dict[str, Any]: if pkg_manager in removed_pkg_managers: continue + # if pkg manager has different name in cachi2 update it + pkg_manager = pkg_managers_map.get(pkg_manager, pkg_manager) + packages = remote_source.get("packages", {}).get(pkg_manager, []) packages = packages or [{"path": "."}] for pkg in packages: diff --git a/tests/utils/test_cachi2.py b/tests/utils/test_cachi2.py index 363db0df8..25e0e99d6 100644 --- a/tests/utils/test_cachi2.py +++ b/tests/utils/test_cachi2.py @@ -22,6 +22,11 @@ {"flags": [], "packages": [{"path": ".", "type": "gomod"}]}, id="pkg_manager_plain", ), + pytest.param( + {"pkg_managers": ["rubygems"]}, + {"flags": [], "packages": [{"path": ".", "type": "bundler"}]}, + id="pkg_rubygems_to_bundler", + ), pytest.param( {"pkg_managers": ["gomod"], "packages": {"gomod": [{"path": "operator"}]}}, {"flags": [], "packages": [{"path": "operator", "type": "gomod"}]}, From 180b93fd55a8b32c0a7e5053cdf9c641aff6f126 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Thu, 31 Oct 2024 14:54:29 +0100 Subject: [PATCH 08/29] cachi2: support empty pkg_manager Empty package manager (not defined) should deafult to gomod, to keep compatibility in behavior with Cachito STONEBLD-2921 Signed-off-by: Martin Basti --- atomic_reactor/utils/cachi2.py | 7 ++++++- tests/utils/test_cachi2.py | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/atomic_reactor/utils/cachi2.py b/atomic_reactor/utils/cachi2.py index d9e1ee444..0191f3599 100644 --- a/atomic_reactor/utils/cachi2.py +++ b/atomic_reactor/utils/cachi2.py @@ -40,7 +40,12 @@ def remote_source_to_cachi2(remote_source: Dict[str, Any]) -> Dict[str, Any]: ) cachi2_packages = [] - for pkg_manager in remote_source["pkg_managers"]: + pkg_managers = remote_source.get("pkg_managers") + if pkg_managers is None: + # Cachito behavior, missing pkg_managers means to use gomod + pkg_managers = ["gomod"] + + for pkg_manager in pkg_managers: if pkg_manager in removed_pkg_managers: continue diff --git a/tests/utils/test_cachi2.py b/tests/utils/test_cachi2.py index 25e0e99d6..b8db18f30 100644 --- a/tests/utils/test_cachi2.py +++ b/tests/utils/test_cachi2.py @@ -27,6 +27,11 @@ {"flags": [], "packages": [{"path": ".", "type": "bundler"}]}, id="pkg_rubygems_to_bundler", ), + pytest.param( + {}, + {"flags": [], "packages": [{"path": ".", "type": "gomod"}]}, + id="pkg_manager_missing", + ), pytest.param( {"pkg_managers": ["gomod"], "packages": {"gomod": [{"path": "operator"}]}}, {"flags": [], "packages": [{"path": "operator", "type": "gomod"}]}, From 60412c88fdcd198baa94f2cf7eafb41cf79c19d4 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Mon, 14 Oct 2024 18:08:52 +0200 Subject: [PATCH 09/29] cachi2: init task/step Add init task/step responsible for creating directory structure, cloning remote sources and preparing options file for cachi2 run. Signed-off-by: Martin Basti --- atomic_reactor/cli/parser.py | 7 + atomic_reactor/cli/task.py | 11 + atomic_reactor/constants.py | 8 + atomic_reactor/plugins/cachi2_init.py | 154 +++++++++++ atomic_reactor/tasks/binary.py | 14 +- tekton/tasks/binary-container-cachi2.yaml | 56 ++++ tests/cli/test_parser.py | 4 + tests/cli/test_task.py | 5 + tests/plugins/test_cachi2_init.py | 311 ++++++++++++++++++++++ tests/tasks/test_plugin_based.py | 3 +- tests/utils/test_cachi2.py | 5 + 11 files changed, 576 insertions(+), 2 deletions(-) create mode 100644 atomic_reactor/plugins/cachi2_init.py create mode 100644 tekton/tasks/binary-container-cachi2.yaml create mode 100644 tests/plugins/test_cachi2_init.py diff --git a/atomic_reactor/cli/parser.py b/atomic_reactor/cli/parser.py index e488634ad..a38cd88ba 100644 --- a/atomic_reactor/cli/parser.py +++ b/atomic_reactor/cli/parser.py @@ -94,6 +94,13 @@ def parse_args(args: Optional[Sequence[str]] = None) -> dict: ) binary_container_cachito.set_defaults(func=task.binary_container_cachito) + binary_container_cachi2_init = tasks.add_parser( + "binary-container-cachi2-init", + help="binary container cachi2 init step", + description="Execute binary container cachi2 init step.", + ) + binary_container_cachi2_init.set_defaults(func=task.binary_container_cachi2_init) + binary_container_prebuild = tasks.add_parser( "binary-container-prebuild", help="binary container pre-build step", diff --git a/atomic_reactor/cli/task.py b/atomic_reactor/cli/task.py index 4e6d871b3..d9003a5dd 100644 --- a/atomic_reactor/cli/task.py +++ b/atomic_reactor/cli/task.py @@ -7,6 +7,7 @@ """ from atomic_reactor.tasks.binary import (BinaryExitTask, BinaryPostBuildTask, BinaryPreBuildTask, BinaryInitTask, BinaryCachitoTask, + BinaryCachi2InitTask, InitTaskParams, BinaryExitTaskParams) from atomic_reactor.tasks.binary_container_build import BinaryBuildTask, BinaryBuildTaskParams from atomic_reactor.tasks.clone import CloneTask @@ -65,6 +66,16 @@ def binary_container_cachito(task_args: dict): return task.run(init_build_dirs=True) +def binary_container_cachi2_init(task_args: dict): + """Run binary container Cachi2 init step. + + :param task_args: CLI arguments for a binary-container-cachi2-init task + """ + params = TaskParams.from_cli_args(task_args) + task = BinaryCachi2InitTask(params) + return task.run(init_build_dirs=True) + + def binary_container_prebuild(task_args: dict): """Run binary container pre-build steps. diff --git a/atomic_reactor/constants.py b/atomic_reactor/constants.py index 40897173d..0eb4877db 100644 --- a/atomic_reactor/constants.py +++ b/atomic_reactor/constants.py @@ -112,6 +112,7 @@ PLUGIN_FLATPAK_CREATE_OCI = 'flatpak_create_oci' PLUGIN_GENERATE_SBOM = 'generate_sbom' PLUGIN_RPMQA = 'all_rpm_packages' +PLUGIN_CACHI2_INIT = "cachi2_init" # some shared dict keys for build metadata that gets recorded with koji. # for consistency of metadata in historical builds, these values basically cannot change. @@ -197,6 +198,13 @@ REMOTE_SOURCE_JSON_ENV_FILENAME = 'remote-source.env.json' ICM_JSON_FILENAME = 'icm-{}.json' +# Cachi2 constants +CACHI2_BUILD_DIR = "_cachi2_remote_sources" +CACHI2_BUILD_APP_DIR = "app" +CACHI2_PKG_OPTIONS_FILE = "cachi2_pkg_options.json" +CACHI2_FOR_OUTPUT_DIR_OPT_FILE = "cachi2_for_output_dir_opt.txt" +CACHI2_SINGLE_REMOTE_SOURCE_NAME = "remote-source" + # koji osbs_build metadata KOJI_KIND_IMAGE_BUILD = 'container_build' KOJI_KIND_IMAGE_SOURCE_BUILD = 'source_container_build' diff --git a/atomic_reactor/plugins/cachi2_init.py b/atomic_reactor/plugins/cachi2_init.py new file mode 100644 index 000000000..2d2a13fa2 --- /dev/null +++ b/atomic_reactor/plugins/cachi2_init.py @@ -0,0 +1,154 @@ +""" +Copyright (c) 2024 Red Hat, Inc +All rights reserved. + +This software may be modified and distributed under the terms +of the BSD license. See the LICENSE file for details. +""" +import json +from collections import Counter +from typing import Any, Optional, List, Dict +from pathlib import Path + +from osbs.utils import clone_git_repo + +from atomic_reactor.constants import ( + PLUGIN_CACHI2_INIT, + CACHI2_BUILD_DIR, + CACHI2_BUILD_APP_DIR, + CACHI2_PKG_OPTIONS_FILE, + CACHI2_FOR_OUTPUT_DIR_OPT_FILE, + CACHI2_SINGLE_REMOTE_SOURCE_NAME, + REMOTE_SOURCE_DIR, +) +from atomic_reactor.plugin import Plugin +from atomic_reactor.util import map_to_user_params +from atomic_reactor.utils.cachi2 import remote_source_to_cachi2 + + +class Cachi2InitPlugin(Plugin): + """Initiate remote sources for Cachi2 + + This plugin will read the remote_sources configuration from + container.yaml in the git repository, clone them and prepare + params for Cachi2 execution. + """ + + key = PLUGIN_CACHI2_INIT + is_allowed_to_fail = False + + args_from_user_params = map_to_user_params("dependency_replacements") + + def __init__(self, workflow, dependency_replacements=None): + """ + :param workflow: DockerBuildWorkflow instance + :param dependency_replacements: list, dependencies for the cachito fetched artifact to + be replaced. Must be of the form pkg_manager:name:version[:new_name] + """ + super(Cachi2InitPlugin, self).__init__(workflow) + self._osbs = None + self.dependency_replacements = dependency_replacements + self.single_remote_source_params = self.workflow.source.config.remote_source + self.multiple_remote_sources_params = self.workflow.source.config.remote_sources + self.remote_sources_root_path = self.workflow.build_dir.path / CACHI2_BUILD_DIR + + def run(self) -> Optional[List[Dict[str, Any]]]: + if (not self.workflow.conf.allow_multiple_remote_sources + and self.multiple_remote_sources_params): + raise ValueError('Multiple remote sources are not enabled, ' + 'use single remote source in container.yaml') + + if not (self.single_remote_source_params or self.multiple_remote_sources_params): + self.log.info('Aborting plugin execution: missing remote source configuration') + return None + + if self.multiple_remote_sources_params: + self.verify_multiple_remote_sources_names_are_unique() + + if self.dependency_replacements: + raise ValueError('Dependency replacements are not supported by Cachi2') + + processed_remote_sources = self.process_remote_sources() + + return processed_remote_sources + + def process_remote_sources(self) -> List[Dict[str, Any]]: + """Process remote source requests and return information about the processed sources.""" + + remote_sources = self.multiple_remote_sources_params + if self.single_remote_source_params: + remote_sources = [{ + "name": "", # name single remote source with generic name + "remote_source": self.single_remote_source_params}] + + processed_remote_sources = [] + + self.remote_sources_root_path.mkdir() + + for remote_source in remote_sources: + # single source doesn't have name, fake it for cachi2_run task + source_name = ( + remote_source["name"] if self.multiple_remote_sources_params + else CACHI2_SINGLE_REMOTE_SOURCE_NAME + ) + self.log.info("Initializing remote source %s", source_name) + source_path = self.remote_sources_root_path / source_name + source_path.mkdir() + + remote_source_data = remote_source["remote_source"] + + self.write_cachi2_pkg_options( + remote_source_data, + source_path / CACHI2_PKG_OPTIONS_FILE) + self.write_cachi2_for_output_dir( + source_name, + source_path / CACHI2_FOR_OUTPUT_DIR_OPT_FILE) + + source_path_app = source_path / CACHI2_BUILD_APP_DIR + source_path_app.mkdir() + + self.clone_remote_source( + remote_source_data["repo"], + source_path_app, + remote_source_data["ref"] + ) + + processed_remote_sources.append({ + "source_path": str(source_path), + **remote_source, + }) + + return processed_remote_sources + + def clone_remote_source(self, repo: str, target_dir: Path, commit: str): + self.log.debug("Cloning %s at %s into %s", repo, commit, target_dir) + clone_git_repo( + repo, + target_dir, + commit + ) + + def verify_multiple_remote_sources_names_are_unique(self): + names = [remote_source['name'] for remote_source in self.multiple_remote_sources_params] + duplicate_names = [name for name, count in Counter(names).items() if count > 1] + if duplicate_names: + raise ValueError(f'Provided remote sources parameters contain ' + f'non unique names: {duplicate_names}') + + def write_cachi2_pkg_options(self, remote_source: Dict[str, Any], path: Path): + """Write cachi2 package options into file""" + with path.open("w") as f: + json.dump(remote_source_to_cachi2(remote_source), f) + + def write_cachi2_for_output_dir(self, remote_source_name: str, path: Path): + """Write value for --for-output-dir cachi2 option. + + This must be path inside container so users have the right paths to + use it within image build + """ + value = Path(REMOTE_SOURCE_DIR) + if self.multiple_remote_sources_params: + value = value / remote_source_name + + with open(path, 'w') as f: + f.write(str(value)) diff --git a/atomic_reactor/tasks/binary.py b/atomic_reactor/tasks/binary.py index 88f285afd..3b7e9b076 100644 --- a/atomic_reactor/tasks/binary.py +++ b/atomic_reactor/tasks/binary.py @@ -12,7 +12,10 @@ from dataclasses import dataclass from atomic_reactor import inner -from atomic_reactor.constants import DOCKERFILE_FILENAME +from atomic_reactor.constants import ( + PLUGIN_CACHI2_INIT, + DOCKERFILE_FILENAME, +) from atomic_reactor.tasks import plugin_based from atomic_reactor.tasks.common import TaskParams @@ -77,6 +80,15 @@ class BinaryCachitoTask(plugin_based.PluginBasedTask[TaskParams]): ] +class BinaryCachi2InitTask(plugin_based.PluginBasedTask[TaskParams]): + """Binary container Cachi2 init task.""" + + task_name = 'binary_container_cachi2_init' + plugins_conf = [ + {"name": PLUGIN_CACHI2_INIT}, + ] + + class BinaryPreBuildTask(plugin_based.PluginBasedTask[TaskParams]): """Binary container pre-build task.""" diff --git a/tekton/tasks/binary-container-cachi2.yaml b/tekton/tasks/binary-container-cachi2.yaml new file mode 100644 index 000000000..99a5ddd39 --- /dev/null +++ b/tekton/tasks/binary-container-cachi2.yaml @@ -0,0 +1,56 @@ +apiVersion: tekton.dev/v1beta1 +kind: Task +metadata: + name: binary-container-cachi2-0-1 # dot is not allowed in the name +spec: + description: >- + OSBS cachi2 task for building binary container image + params: + - name: osbs-image + description: The location of the OSBS builder image (FQDN pullspec) + type: string + - name: pipeline-run-name + type: string + description: PipelineRun name to reference current PipelineRun + - name: user-params + type: string + description: User parameters in JSON format + + workspaces: + - name: ws-build-dir + - name: ws-context-dir + - name: ws-home-dir + - name: ws-registries-secret # access with $(workspaces.ws-registries-secret.path)/token + - name: ws-koji-secret # access with $(workspaces.ws-koji-secret.path)/token + - name: ws-reactor-config-map + - name: ws-autobot-keytab + + stepTemplate: + env: + - name: HOME + value: $(workspaces.ws-home-dir.path) + + steps: + - name: binary-container-cachi2-init + image: $(params.osbs-image) + workingDir: $(workspaces.ws-home-dir.path) + env: + - name: USER_PARAMS + value: $(params.user-params) + resources: + requests: + memory: 512Mi + cpu: 250m + limits: + memory: 1Gi + cpu: 395m + script: | + set -x + atomic-reactor -v task \ + --user-params="${USER_PARAMS}" \ + --build-dir="$(workspaces.ws-build-dir.path)" \ + --context-dir="$(workspaces.ws-context-dir.path)" \ + --config-file="$(workspaces.ws-reactor-config-map.path)/config.yaml" \ + --namespace="$(context.taskRun.namespace)" \ + --pipeline-run-name="$(params.pipeline-run-name)" \ + binary-container-cachi2-init diff --git a/tests/cli/test_parser.py b/tests/cli/test_parser.py index 8b3c66683..7fe6eb0a5 100644 --- a/tests/cli/test_parser.py +++ b/tests/cli/test_parser.py @@ -87,6 +87,10 @@ def test_parse_args_version(capsys): ["task", *REQUIRED_COMMON_ARGS, "binary-container-cachito"], {**EXPECTED_ARGS, "func": task.binary_container_cachito}, ), + ( + ["task", *REQUIRED_COMMON_ARGS, "binary-container-cachi2-init"], + {**EXPECTED_ARGS, "func": task.binary_container_cachi2_init}, + ), ( ["task", *REQUIRED_COMMON_ARGS, "binary-container-prebuild"], {**EXPECTED_ARGS, "func": task.binary_container_prebuild}, diff --git a/tests/cli/test_task.py b/tests/cli/test_task.py index 952a3d4c6..b2ef7d0df 100644 --- a/tests/cli/test_task.py +++ b/tests/cli/test_task.py @@ -70,6 +70,11 @@ def test_binary_container_cachito(): assert task.binary_container_cachito(TASK_ARGS) == TASK_RESULT +def test_binary_container_cachi2_init(): + mock(binary.BinaryCachi2InitTask, task_args=TASK_ARGS) + assert task.binary_container_cachi2_init(TASK_ARGS) == TASK_RESULT + + def test_binary_container_prebuild(): mock(binary.BinaryPreBuildTask, task_args=TASK_ARGS) assert task.binary_container_prebuild(TASK_ARGS) == TASK_RESULT diff --git a/tests/plugins/test_cachi2_init.py b/tests/plugins/test_cachi2_init.py new file mode 100644 index 000000000..1a305d765 --- /dev/null +++ b/tests/plugins/test_cachi2_init.py @@ -0,0 +1,311 @@ +""" +Copyright (c) 2024 Red Hat, Inc +All rights reserved. + +This software may be modified and distributed under the terms +of the BSD license. See the LICENSE file for details. +""" + +from textwrap import dedent +import os.path +import json +from typing import Dict + +import pytest +import yaml +from flexmock import flexmock + +from atomic_reactor.constants import ( + CACHI2_BUILD_DIR, + CACHI2_SINGLE_REMOTE_SOURCE_NAME, + CACHI2_BUILD_APP_DIR, + CACHI2_FOR_OUTPUT_DIR_OPT_FILE, + CACHI2_PKG_OPTIONS_FILE, +) + +from atomic_reactor.inner import DockerBuildWorkflow + +from atomic_reactor.plugin import PluginFailedException +from atomic_reactor.plugins.cachi2_init import ( + Cachi2InitPlugin, +) +from atomic_reactor.source import SourceConfig +from tests.mock_env import MockEnv + +from tests.stubs import StubSource + + +REMOTE_SOURCE_REPO = 'https://git.example.com/team/repo.git' +REMOTE_SOURCE_REF = 'b55c00f45ec3dfee0c766cea3d395d6e21cc2e5a' +SECOND_REMOTE_SOURCE_REPO = 'https://git.example.com/other-team/other-repo.git' +SECOND_REMOTE_SOURCE_REF = 'd55c00f45ec3dfee0c766cea3d395d6e21cc2e5c' + +MOCKED_CLONE_FILE = "clone.txt" + + +def mock_reactor_config(workflow, data=None): + config = yaml.safe_load(data) + workflow.conf.conf = config + + +def mock_repo_config(workflow, data=None): + if data is None: + data = dedent("""\ + remote_source: + repo: {} + ref: {} + """.format(REMOTE_SOURCE_REPO, REMOTE_SOURCE_REF)) + + workflow._tmpdir.joinpath('container.yaml').write_text(data, "utf-8") + + # The repo config is read when SourceConfig is initialized. Force + # reloading here to make usage easier. + workflow.source.config = SourceConfig(str(workflow._tmpdir)) + + +@pytest.fixture +def mocked_cachi2_init(): + def clone_f(repo, target_dir, ref): + with open(target_dir / "clone.txt", "w") as f: + f.write(f"{repo}:{ref}") + f.flush() + + mocked = flexmock(Cachi2InitPlugin) + mocked.should_receive('clone_remote_source').replace_with(clone_f) + return Cachi2InitPlugin + + +@pytest.fixture +def workflow(workflow: DockerBuildWorkflow, source_dir): + # Stash the tmpdir in workflow so it can be used later + workflow._tmpdir = source_dir + + class MockSource(StubSource): + + def __init__(self, workdir): + super(MockSource, self).__init__() + self.workdir = workdir + self.path = workdir + + workflow.source = MockSource(str(source_dir)) + + mock_repo_config(workflow) + + workflow.build_dir.init_build_dirs(["x86_64", "ppc64le"], workflow.source) + + return workflow + + +def assert_cachi2_init_files( + workflow, + remote_source_name: str, + expected_pkg_opt: Dict, + expected_for_output: str): + + cachi2_build_path = workflow.build_dir.path / CACHI2_BUILD_DIR + assert os.path.exists(cachi2_build_path) + + remote_source_path = cachi2_build_path / remote_source_name + assert os.path.exists(remote_source_path) + + clone_path = remote_source_path / CACHI2_BUILD_APP_DIR / MOCKED_CLONE_FILE + assert os.path.exists(clone_path) + + cachi2_pkg_opt_path = remote_source_path / CACHI2_PKG_OPTIONS_FILE + assert os.path.exists(cachi2_pkg_opt_path) + with open(cachi2_pkg_opt_path, "r") as f: + assert json.load(f) == expected_pkg_opt + + cachi2_for_output_opt_path = remote_source_path / CACHI2_FOR_OUTPUT_DIR_OPT_FILE + assert os.path.exists(cachi2_for_output_opt_path) + with open(cachi2_for_output_opt_path, "r") as f: + assert f.read() == expected_for_output + + +def test_single_remote_source_initialization(workflow, mocked_cachi2_init): + """Tests initialization or repos for single remote source""" + result = mocked_cachi2_init(workflow).run() + + assert_cachi2_init_files( + workflow, + CACHI2_SINGLE_REMOTE_SOURCE_NAME, + {"packages": [{"path": ".", "type": "gomod"}], "flags": []}, + '/remote-source') + + assert result == [{ + "name": "", + "source_path": str( + workflow.build_dir.path / CACHI2_BUILD_DIR / CACHI2_SINGLE_REMOTE_SOURCE_NAME), + "remote_source": { + "repo": REMOTE_SOURCE_REPO, + "ref": REMOTE_SOURCE_REF, + } + }] + + +def test_multi_remote_source_initialization(workflow, mocked_cachi2_init): + """Tests initialization or repos for multiple remote sources""" + + first_remote_source_name = "first" + second_remote_source_name = "second" + + remote_source_config = dedent( + f"""\ + remote_sources: + - name: {first_remote_source_name} + remote_source: + repo: {REMOTE_SOURCE_REPO} + ref: {REMOTE_SOURCE_REF} + flags: + - gomod-vendor + pkg_managers: + - gomod + - name: {second_remote_source_name} + remote_source: + repo: {SECOND_REMOTE_SOURCE_REPO} + ref: {SECOND_REMOTE_SOURCE_REF} + """ + ) + + mock_repo_config(workflow, remote_source_config) + + reactor_config = dedent("""\ + allow_multiple_remote_sources: True + """) + mock_reactor_config(workflow, reactor_config) + + result = mocked_cachi2_init(workflow).run() + + assert_cachi2_init_files( + workflow, + first_remote_source_name, + { + "packages": [{"path": ".", "type": "gomod"}], + "flags": ["gomod-vendor"] + }, + f'/remote-source/{first_remote_source_name}') + assert_cachi2_init_files( + workflow, + second_remote_source_name, + {"packages": [{"path": ".", "type": "gomod"}], "flags": []}, + f'/remote-source/{second_remote_source_name}') + + assert result == [{ + "name": first_remote_source_name, + "source_path": str(workflow.build_dir.path / CACHI2_BUILD_DIR / first_remote_source_name), + "remote_source": { + "repo": REMOTE_SOURCE_REPO, + "ref": REMOTE_SOURCE_REF, + "pkg_managers": ["gomod"], + "flags": ["gomod-vendor"], + } + }, { + "name": second_remote_source_name, + "source_path": str(workflow.build_dir.path / CACHI2_BUILD_DIR / second_remote_source_name), + "remote_source": { + "repo": SECOND_REMOTE_SOURCE_REPO, + "ref": SECOND_REMOTE_SOURCE_REF, + } + }] + + +def test_no_fail_when_missing_cachito_config(workflow, mocked_cachi2_init): + """Cachi2 is not dependent on cachito config""" + reactor_config = dedent("""\ + version: 1 + """) + mock_reactor_config(workflow, reactor_config) + + result = mocked_cachi2_init(workflow).run() + assert result + + +def test_ignore_when_missing_remote_source_config(workflow, mocked_cachi2_init): + """Plugin should just skip when remote source is not configured""" + remote_source_config = dedent("""---""") + mock_repo_config(workflow, remote_source_config) + result = mocked_cachi2_init(workflow).run() + assert result is None + + +def test_disallowed_multiple_remote_sources(workflow): + first_remote_source_name = 'gomod' + + container_yaml_config = dedent( + f"""\ + remote_sources: + - name: {first_remote_source_name} + remote_source: + repo: {REMOTE_SOURCE_REPO} + ref: {REMOTE_SOURCE_REF} + """ + ) + + reactor_config = dedent("""\ + version: 1 + allow_multiple_remote_sources: false + """) + mock_repo_config(workflow, data=container_yaml_config) + mock_reactor_config(workflow, reactor_config) + + err_msg = ( + "Multiple remote sources are not enabled, " + "use single remote source in container.yaml" + ) + result = run_plugin_with_args(workflow, expect_result=False, expect_error=err_msg) + assert result is None + + +def test_multiple_remote_sources_non_unique_names(workflow): + container_yaml_config = dedent("""\ + remote_sources: + - name: same + remote_source: + repo: https://git.example.com/team/repo.git + ref: a55c00f45ec3dfee0c766cea3d395d6e21cc2e5a + - name: same + remote_source: + repo: https://git.example.com/team/repo.git + ref: a55c00f45ec3dfee0c766cea3d395d6e21cc2e5a + - name: bit-different + remote_source: + repo: https://git.example.com/team/repo.git + ref: a55c00f45ec3dfee0c766cea3d395d6e21cc2e5a + """) + mock_repo_config(workflow, data=container_yaml_config) + + reactor_config = dedent("""\ + allow_multiple_remote_sources: True + """) + mock_reactor_config(workflow, reactor_config) + + err_msg = ( + r"Provided remote sources parameters contain non unique names: \['same'\]" + ) + result = run_plugin_with_args(workflow, expect_result=False, expect_error=err_msg) + assert result is None + + +def test_dependency_replacements(workflow): + run_plugin_with_args(workflow, dependency_replacements={"dep": "something"}, + expect_error="Dependency replacements are not supported by Cachi2") + + +def run_plugin_with_args(workflow, dependency_replacements=None, expect_error=None, + expect_result=True, expected_plugin_results=None): + runner = (MockEnv(workflow) + .for_plugin(Cachi2InitPlugin.key) + .set_plugin_args({"dependency_replacements": dependency_replacements}) + .create_runner()) + + if expect_error: + with pytest.raises(PluginFailedException, match=expect_error): + runner.run() + return + + results = runner.run()[Cachi2InitPlugin.key] + + if expect_result: + assert results == expected_plugin_results + + return results diff --git a/tests/tasks/test_plugin_based.py b/tests/tasks/test_plugin_based.py index 2e24d2cad..1cd599468 100644 --- a/tests/tasks/test_plugin_based.py +++ b/tests/tasks/test_plugin_based.py @@ -19,6 +19,7 @@ from atomic_reactor.tasks.common import TaskParams from atomic_reactor.tasks.binary import ( InitTaskParams, BinaryPreBuildTask, BinaryInitTask, BinaryCachitoTask, + BinaryCachi2InitTask ) from atomic_reactor.util import DockerfileImages @@ -230,7 +231,7 @@ def test_ensure_workflow_data_is_saved_init_task( assert {} == wf_data.plugins_results -@pytest.mark.parametrize("taskfunc", [BinaryCachitoTask, BinaryPreBuildTask]) +@pytest.mark.parametrize("taskfunc", [BinaryCachitoTask, BinaryCachi2InitTask, BinaryPreBuildTask]) def test_ensure_workflow_data_is_saved_standard_task( build_dir, dummy_source, tmpdir, taskfunc ): diff --git a/tests/utils/test_cachi2.py b/tests/utils/test_cachi2.py index b8db18f30..bc744f61e 100644 --- a/tests/utils/test_cachi2.py +++ b/tests/utils/test_cachi2.py @@ -93,6 +93,11 @@ {"flags": [], "packages": []}, id="unsupported_git_submodule", ), + pytest.param( + {"pkg_managers": []}, + {"flags": [], "packages": []}, + id="empty_package_managers", + ), ]) def test_remote_source_to_cachi2_conversion(input_remote_source, expected_cachi2): """Test conversion of remote_source (cachito) configuration from container yaml From d71ca523df4910c5df04994788300ca6d2124aa1 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Mon, 14 Oct 2024 19:17:51 +0200 Subject: [PATCH 10/29] Add Checkton Shellcheck for tekton github action Signed-off-by: Martin Basti --- .github/workflows/linters.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.github/workflows/linters.yaml b/.github/workflows/linters.yaml index 835651b7d..b22b9ffb3 100644 --- a/.github/workflows/linters.yaml +++ b/.github/workflows/linters.yaml @@ -100,10 +100,26 @@ jobs: steps: - name: Check out repo uses: actions/checkout@v4 + with: + fetch-depth: 0 - name: Run ShellCheck uses: containerbuildsystem/actions/shellcheck@master + # ShellCheck for tekton + - name: Run Checkton + id: checkton + uses: chmeliik/checkton@v0.2.2 + with: + fail-on-findings: false + + - name: Upload SARIF file + uses: github/codeql-action/upload-sarif@v3 + with: + sarif_file: ${{ steps.checkton.outputs.sarif }} + # Avoid clashing with ShellCheck + category: checkton + tekton-lint: name: tekton-lint runs-on: ubuntu-latest From a602095e484bd24063bd69a8fa52324bfcf019a8 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Mon, 14 Oct 2024 20:21:26 +0200 Subject: [PATCH 11/29] cachi2: run cachi2 Add cachi2 step that: - fetches deps - create env file and env json - make source archives - remove git (option to keep git will be added later) - merge sboms into single one Signed-off-by: Martin Basti --- tekton/tasks/binary-container-cachi2.yaml | 70 +++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/tekton/tasks/binary-container-cachi2.yaml b/tekton/tasks/binary-container-cachi2.yaml index 99a5ddd39..2c4663a7d 100644 --- a/tekton/tasks/binary-container-cachi2.yaml +++ b/tekton/tasks/binary-container-cachi2.yaml @@ -9,12 +9,18 @@ spec: - name: osbs-image description: The location of the OSBS builder image (FQDN pullspec) type: string + - name: cachi2-image + description: The location of the Cachi2 image (FQDN pullspec) + type: string - name: pipeline-run-name type: string description: PipelineRun name to reference current PipelineRun - name: user-params type: string description: User parameters in JSON format + - name: log-level + description: Set cachi2 log level (debug, info, warning, error) + default: "info" workspaces: - name: ws-build-dir @@ -54,3 +60,67 @@ spec: --namespace="$(context.taskRun.namespace)" \ --pipeline-run-name="$(params.pipeline-run-name)" \ binary-container-cachi2-init + - name: binary-container-cachi2-run + image: $(params.cachi2-image) + env: + - name: LOG_LEVEL + value: $(params.log-level) + workingDir: $(workspaces.ws-home-dir.path) + resources: + requests: + memory: 512Mi + cpu: 250m + limits: + memory: 1Gi + cpu: 395m + script: | + #!/usr/bin/bash + set -eux + CACHI2_DIR="$(workspaces.ws-build-dir.path)/_cachi2_remote_sources" + + if [ ! -d "$CACHI2_DIR" ]; then + echo "Skipping step: remote sources not found" + exit 0 + fi + + SBOMS=() + + # Process each remote source + for REMOTE_SOURCE_PATH in "${CACHI2_DIR}"/* + do + + pushd "${REMOTE_SOURCE_PATH}" + + FOR_OUTPUT_DIR="$(cat cachi2_for_output_dir_opt.txt)" + + cachi2 --log-level="$LOG_LEVEL" fetch-deps \ + --source="${REMOTE_SOURCE_PATH}/app/" \ + --output="${REMOTE_SOURCE_PATH}" \ + "$(cat cachi2_pkg_options.json)" + + SBOMS+=("${REMOTE_SOURCE_PATH}/bom.json") + + cachi2 --log-level="$LOG_LEVEL" generate-env "${REMOTE_SOURCE_PATH}" \ + --format json \ + --for-output-dir="${FOR_OUTPUT_DIR}" \ + --output "${REMOTE_SOURCE_PATH}/cachi2.env.json" + + rm -fr app/.git/ # remove git directory + + # create source archive before injecting files + tar -czf remote-source.tar.gz app/ deps/ + + cachi2 --log-level="$LOG_LEVEL" inject-files "${REMOTE_SOURCE_PATH}" \ + --for-output-dir="${FOR_OUTPUT_DIR}" + + popd + done + + if [ "${#SBOMS[@]}" -gt 1 ]; then + # merge multiple sboms into single one + cachi2 --log-level="$LOG_LEVEL" merge-sboms "${SBOMS[@]}" \ + --output "${CACHI2_DIR}/bom.json" + else + # single SBOM is the final SBOM + cp "${SBOMS[0]}" "${CACHI2_DIR}/bom.json" + fi From f0721071bd81ec84637c9f349b1d833acbb69ba9 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Fri, 18 Oct 2024 18:11:55 +0200 Subject: [PATCH 12/29] fix(cachi2): processing of env vars Fixing format of envvars, Cachi2 have slightly different format Signed-off-by: Martin Basti --- atomic_reactor/utils/cachi2.py | 6 +++--- tests/utils/test_cachi2.py | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/atomic_reactor/utils/cachi2.py b/atomic_reactor/utils/cachi2.py index 0191f3599..43740b045 100644 --- a/atomic_reactor/utils/cachi2.py +++ b/atomic_reactor/utils/cachi2.py @@ -8,7 +8,7 @@ Utils to help to integrate with cachi2 CLI tool """ -from typing import Any, Callable, Dict, Optional, Tuple +from typing import Any, Callable, Dict, Optional, Tuple, List from packageurl import PackageURL @@ -140,7 +140,7 @@ def gen_dependency_from_sbom_component(sbom_dep: Dict[str, Any]) -> Dict[str, Op def generate_request_json( remote_source: Dict[str, Any], remote_source_sbom: Dict[str, Any], - remote_source_env_json: Dict[str, Dict[str, str]], + remote_source_env_json: List[Dict[str, str]], ) -> Dict[str, Any]: """Generates Cachito like request.json @@ -156,7 +156,7 @@ def generate_request_json( "pkg_managers": remote_source.get("pkg_managers", []), "ref": remote_source["ref"], "repo": remote_source["repo"], - "environment_variables": {key: val["value"] for key, val in remote_source_env_json.items()}, + "environment_variables": {env['name']: env["value"] for env in remote_source_env_json}, "flags": remote_source.get("flags", []), "packages": [], # this will be always empty cachi2 doesn't provide nested deps } diff --git a/tests/utils/test_cachi2.py b/tests/utils/test_cachi2.py index bc744f61e..166f93676 100644 --- a/tests/utils/test_cachi2.py +++ b/tests/utils/test_cachi2.py @@ -409,12 +409,12 @@ def test_generate_request_json(): ], } - remote_source_env_json = { - "GOCACHE": { - "kind": "path", - "value": "deps/gomod" + remote_source_env_json = [ + { + "name": "GOCACHE", + "value": "deps/gomod", }, - } + ] expected = { "dependencies": [ From 7cf95df9063d3008650287e4f225784c6c574062 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Fri, 18 Oct 2024 11:33:27 +0200 Subject: [PATCH 13/29] cachi2: postprocess Postprocssing plugin to take cachi2 generated dependencies and generate expected metadata for OSBS and prepare sources into build dirs. Signed-off-by: Martin Basti --- atomic_reactor/cli/parser.py | 7 + atomic_reactor/cli/task.py | 12 +- atomic_reactor/constants.py | 1 + atomic_reactor/plugins/cachi2_postprocess.py | 243 +++++++++ atomic_reactor/tasks/binary.py | 10 + tekton/tasks/binary-container-cachi2.yaml | 23 + tests/cli/test_parser.py | 4 + tests/cli/test_task.py | 5 + tests/plugins/test_cachi2_postprocess.py | 487 +++++++++++++++++++ 9 files changed, 791 insertions(+), 1 deletion(-) create mode 100644 atomic_reactor/plugins/cachi2_postprocess.py create mode 100644 tests/plugins/test_cachi2_postprocess.py diff --git a/atomic_reactor/cli/parser.py b/atomic_reactor/cli/parser.py index a38cd88ba..22ebdbc5f 100644 --- a/atomic_reactor/cli/parser.py +++ b/atomic_reactor/cli/parser.py @@ -101,6 +101,13 @@ def parse_args(args: Optional[Sequence[str]] = None) -> dict: ) binary_container_cachi2_init.set_defaults(func=task.binary_container_cachi2_init) + binary_container_cachi2_postprocess = tasks.add_parser( + "binary-container-cachi2-postprocess", + help="binary container cachi2 init step", + description="Execute binary container cachi2 postprocess step.", + ) + binary_container_cachi2_postprocess.set_defaults(func=task.binary_container_cachi2_postprocess) + binary_container_prebuild = tasks.add_parser( "binary-container-prebuild", help="binary container pre-build step", diff --git a/atomic_reactor/cli/task.py b/atomic_reactor/cli/task.py index d9003a5dd..cc64bae91 100644 --- a/atomic_reactor/cli/task.py +++ b/atomic_reactor/cli/task.py @@ -7,7 +7,7 @@ """ from atomic_reactor.tasks.binary import (BinaryExitTask, BinaryPostBuildTask, BinaryPreBuildTask, BinaryInitTask, BinaryCachitoTask, - BinaryCachi2InitTask, + BinaryCachi2InitTask, BinaryCachi2PostprocessTask, InitTaskParams, BinaryExitTaskParams) from atomic_reactor.tasks.binary_container_build import BinaryBuildTask, BinaryBuildTaskParams from atomic_reactor.tasks.clone import CloneTask @@ -76,6 +76,16 @@ def binary_container_cachi2_init(task_args: dict): return task.run(init_build_dirs=True) +def binary_container_cachi2_postprocess(task_args: dict): + """Run binary container Cachi2 postprocess step. + + :param task_args: CLI arguments for a binary-container-cachi2-postprocess task + """ + params = TaskParams.from_cli_args(task_args) + task = BinaryCachi2PostprocessTask(params) + return task.run(init_build_dirs=True) + + def binary_container_prebuild(task_args: dict): """Run binary container pre-build steps. diff --git a/atomic_reactor/constants.py b/atomic_reactor/constants.py index 0eb4877db..61c96ba6b 100644 --- a/atomic_reactor/constants.py +++ b/atomic_reactor/constants.py @@ -113,6 +113,7 @@ PLUGIN_GENERATE_SBOM = 'generate_sbom' PLUGIN_RPMQA = 'all_rpm_packages' PLUGIN_CACHI2_INIT = "cachi2_init" +PLUGIN_CACHI2_POSTPROCESS = "cachi2_postprocess" # some shared dict keys for build metadata that gets recorded with koji. # for consistency of metadata in historical builds, these values basically cannot change. diff --git a/atomic_reactor/plugins/cachi2_postprocess.py b/atomic_reactor/plugins/cachi2_postprocess.py new file mode 100644 index 000000000..bc0c4da4e --- /dev/null +++ b/atomic_reactor/plugins/cachi2_postprocess.py @@ -0,0 +1,243 @@ +""" +Copyright (c) 2024 Red Hat, Inc +All rights reserved. + +This software may be modified and distributed under the terms +of the BSD license. See the LICENSE file for details. +""" +import functools +import json +import os.path +import shlex +from dataclasses import dataclass +from pathlib import Path +from shutil import copytree +from typing import Any, Optional, List, Dict + +from atomic_reactor.constants import ( + CACHITO_ENV_ARG_ALIAS, + CACHITO_ENV_FILENAME, + PLUGIN_CACHI2_INIT, + PLUGIN_CACHI2_POSTPROCESS, + REMOTE_SOURCE_DIR, + REMOTE_SOURCE_JSON_FILENAME, + REMOTE_SOURCE_TARBALL_FILENAME, + REMOTE_SOURCE_JSON_ENV_FILENAME, +) +from atomic_reactor.dirs import BuildDir +from atomic_reactor.plugin import Plugin + +from atomic_reactor.utils.cachi2 import generate_request_json + + +@dataclass(frozen=True) +class Cachi2RemoteSource: + """Represents a processed remote source. + + name: the name that identifies this remote source (if multiple remote sources were used) + json_data: subset of the JSON representation of the Cachito request (source_request_to_json) + build_args: environment variables for this remote source + tarball_path: the path of the tarball downloaded from Cachito + """ + + name: Optional[str] + json_data: dict + json_env_data: List[Dict[str, str]] + tarball_path: Path + sources_path: Path + + @classmethod + def tarball_filename(cls, name: Optional[str]): + if name: + return f"remote-source-{name}.tar.gz" + else: + return REMOTE_SOURCE_TARBALL_FILENAME + + @classmethod + def json_filename(cls, name: Optional[str]): + if name: + return f"remote-source-{name}.json" + else: + return REMOTE_SOURCE_JSON_FILENAME + + @classmethod + def json_env_filename(cls, name: Optional[str]): + if name: + return f"remote-source-{name}.env.json" + else: + return REMOTE_SOURCE_JSON_ENV_FILENAME + + @property + def build_args(self) -> Dict[str, str]: + + return { + env_var['name']: env_var['value'] + for env_var in self.json_env_data + } + + +class Cachi2PostprocessPlugin(Plugin): + """Postprocess cachi2 results + + This plugin will postprocess cachi2 results and provide required metadata + """ + + key = PLUGIN_CACHI2_POSTPROCESS + is_allowed_to_fail = False + REMOTE_SOURCE = "unpacked_remote_sources" + + def __init__(self, workflow): + """ + :param workflow: DockerBuildWorkflow instance + + """ + super(Cachi2PostprocessPlugin, self).__init__(workflow) + self._osbs = None + self.single_remote_source_params = self.workflow.source.config.remote_source + self.multiple_remote_sources_params = self.workflow.source.config.remote_sources + self.init_plugin_data = self.workflow.data.plugins_results.get(PLUGIN_CACHI2_INIT) + + def run(self) -> Optional[List[Dict[str, Any]]]: + if not self.init_plugin_data: + self.log.info('Aborting plugin execution: no cachi2 data provided') + return None + + if not (self.single_remote_source_params or self.multiple_remote_sources_params): + self.log.info('Aborting plugin execution: missing remote source configuration') + return None + + processed_remote_sources = self.postprocess_remote_sources() + self.inject_remote_sources(processed_remote_sources) + + return [ + self.remote_source_to_output(remote_source) + for remote_source in processed_remote_sources + ] + + def postprocess_remote_sources(self) -> List[Cachi2RemoteSource]: + """Process remote source requests and return information about the processed sources.""" + + processed_remote_sources = [] + + for remote_source in self.init_plugin_data: + + json_env_path = os.path.join(remote_source['source_path'], 'cachi2.env.json') + with open(json_env_path, 'r') as json_f: + json_env_data = json.load(json_f) + + sbom_path = os.path.join(remote_source['source_path'], 'bom.json') + with open(sbom_path, 'r') as sbom_f: + sbom_data = json.load(sbom_f) + + remote_source_obj = Cachi2RemoteSource( + name=remote_source['name'], + tarball_path=Path(remote_source['source_path'], 'remote-source.tar.gz'), + sources_path=Path(remote_source['source_path']), + json_data=generate_request_json( + remote_source['remote_source'], sbom_data, json_env_data), + json_env_data=json_env_data, + ) + processed_remote_sources.append(remote_source_obj) + return processed_remote_sources + + def inject_remote_sources(self, remote_sources: List[Cachi2RemoteSource]) -> None: + """Inject processed remote sources into build dirs and add build args to workflow.""" + inject_sources = functools.partial(self.inject_into_build_dir, remote_sources) + self.workflow.build_dir.for_all_platforms_copy(inject_sources) + + # For single remote_source workflow, inject all build args directly + if self.single_remote_source_params: + self.workflow.data.buildargs.update(remote_sources[0].build_args) + + self.add_general_buildargs() + + def inject_into_build_dir( + self, remote_sources: List[Cachi2RemoteSource], build_dir: BuildDir, + ) -> List[Path]: + """Inject processed remote sources into a build directory. + + For each remote source, create a dedicated directory, unpack the downloaded tarball + into it and inject the configuration files and an environment file. + + Return a list of the newly created directories. + """ + created_dirs = [] + + for remote_source in remote_sources: + dest_dir = build_dir.path.joinpath(self.REMOTE_SOURCE, remote_source.name or "") + + if dest_dir.exists(): + raise RuntimeError( + f"Conflicting path {dest_dir.relative_to(build_dir.path)} already exists " + "in the dist-git repository" + ) + + dest_dir.mkdir(parents=True) + created_dirs.append(dest_dir) + + # copy app and deps generated by cachito into build_dir + # TODO: reflink? + copytree(remote_source.sources_path/'app', dest_dir/'app', symlinks=True) + copytree(remote_source.sources_path/'deps', dest_dir/'deps', symlinks=True) + + # Create cachito.env file with environment variables received from cachito request + self.generate_cachito_env_file(dest_dir, remote_source.build_args) + + return created_dirs + + def remote_source_to_output(self, remote_source: Cachi2RemoteSource) -> Dict[str, Any]: + """Convert a processed remote source to a dict to be used as output of this plugin.""" + + return { + "name": remote_source.name, + "remote_source_json": { + "json": remote_source.json_data, + "filename": Cachi2RemoteSource.json_filename(remote_source.name), + }, + "remote_source_json_env": { + "json": remote_source.json_env_data, + "filename": Cachi2RemoteSource.json_env_filename(remote_source.name), + }, + "remote_source_tarball": { + "filename": Cachi2RemoteSource.tarball_filename(remote_source.name), + "path": str(remote_source.tarball_path), + }, + } + + def generate_cachito_env_file(self, dest_dir: Path, build_args: Dict[str, str]) -> None: + """ + Generate cachito.env file with exported environment variables received from + cachito request. + + :param dest_dir: destination directory for env file + :param build_args: build arguments to set + """ + self.log.info('Creating %s file with environment variables ' + 'received from cachi2', CACHITO_ENV_FILENAME) + + # Use dedicated dir in container build workdir for cachito.env + abs_path = dest_dir / CACHITO_ENV_FILENAME + with open(abs_path, 'w') as f: + f.write('#!/bin/bash\n') + for env_var, value in build_args.items(): + f.write('export {}={}\n'.format(env_var, shlex.quote(value))) + + def add_general_buildargs(self) -> None: + """Adds general build arguments + + To copy the sources into the build image, Dockerfile should contain + COPY $REMOTE_SOURCE $REMOTE_SOURCE_DIR + or COPY $REMOTE_SOURCES $REMOTE_SOURCES_DIR + """ + if self.multiple_remote_sources_params: + args_for_dockerfile_to_add = { + 'REMOTE_SOURCES': self.REMOTE_SOURCE, + 'REMOTE_SOURCES_DIR': REMOTE_SOURCE_DIR, + } + else: + args_for_dockerfile_to_add = { + 'REMOTE_SOURCE': self.REMOTE_SOURCE, + 'REMOTE_SOURCE_DIR': REMOTE_SOURCE_DIR, + CACHITO_ENV_ARG_ALIAS: os.path.join(REMOTE_SOURCE_DIR, CACHITO_ENV_FILENAME), + } + self.workflow.data.buildargs.update(args_for_dockerfile_to_add) diff --git a/atomic_reactor/tasks/binary.py b/atomic_reactor/tasks/binary.py index 3b7e9b076..6289169a4 100644 --- a/atomic_reactor/tasks/binary.py +++ b/atomic_reactor/tasks/binary.py @@ -14,6 +14,7 @@ from atomic_reactor import inner from atomic_reactor.constants import ( PLUGIN_CACHI2_INIT, + PLUGIN_CACHI2_POSTPROCESS, DOCKERFILE_FILENAME, ) from atomic_reactor.tasks import plugin_based @@ -89,6 +90,15 @@ class BinaryCachi2InitTask(plugin_based.PluginBasedTask[TaskParams]): ] +class BinaryCachi2PostprocessTask(plugin_based.PluginBasedTask[TaskParams]): + """Binary container Cachi2 postprocess task.""" + + task_name = 'binary_container_cachi2_postprocess' + plugins_conf = [ + {"name": PLUGIN_CACHI2_POSTPROCESS}, + ] + + class BinaryPreBuildTask(plugin_based.PluginBasedTask[TaskParams]): """Binary container pre-build task.""" diff --git a/tekton/tasks/binary-container-cachi2.yaml b/tekton/tasks/binary-container-cachi2.yaml index 2c4663a7d..5bb865479 100644 --- a/tekton/tasks/binary-container-cachi2.yaml +++ b/tekton/tasks/binary-container-cachi2.yaml @@ -124,3 +124,26 @@ spec: # single SBOM is the final SBOM cp "${SBOMS[0]}" "${CACHI2_DIR}/bom.json" fi + - name: binary-container-cachi2-postprocess + image: $(params.osbs-image) + env: + - name: USER_PARAMS + value: $(params.user-params) + workingDir: $(workspaces.ws-home-dir.path) + resources: + requests: + memory: 512Mi + cpu: 250m + limits: + memory: 1Gi + cpu: 395m + script: | + set -x + atomic-reactor -v task \ + --user-params="${USER_PARAMS}" \ + --build-dir="$(workspaces.ws-build-dir.path)" \ + --context-dir="$(workspaces.ws-context-dir.path)" \ + --config-file="$(workspaces.ws-reactor-config-map.path)/config.yaml" \ + --namespace="$(context.taskRun.namespace)" \ + --pipeline-run-name="$(params.pipeline-run-name)" \ + binary-container-cachi2-postprocess diff --git a/tests/cli/test_parser.py b/tests/cli/test_parser.py index 7fe6eb0a5..8fd8ba0a7 100644 --- a/tests/cli/test_parser.py +++ b/tests/cli/test_parser.py @@ -91,6 +91,10 @@ def test_parse_args_version(capsys): ["task", *REQUIRED_COMMON_ARGS, "binary-container-cachi2-init"], {**EXPECTED_ARGS, "func": task.binary_container_cachi2_init}, ), + ( + ["task", *REQUIRED_COMMON_ARGS, "binary-container-cachi2-postprocess"], + {**EXPECTED_ARGS, "func": task.binary_container_cachi2_postprocess}, + ), ( ["task", *REQUIRED_COMMON_ARGS, "binary-container-prebuild"], {**EXPECTED_ARGS, "func": task.binary_container_prebuild}, diff --git a/tests/cli/test_task.py b/tests/cli/test_task.py index b2ef7d0df..e51427ef7 100644 --- a/tests/cli/test_task.py +++ b/tests/cli/test_task.py @@ -75,6 +75,11 @@ def test_binary_container_cachi2_init(): assert task.binary_container_cachi2_init(TASK_ARGS) == TASK_RESULT +def test_binary_container_cachi2_postprocess(): + mock(binary.BinaryCachi2PostprocessTask, task_args=TASK_ARGS) + assert task.binary_container_cachi2_postprocess(TASK_ARGS) == TASK_RESULT + + def test_binary_container_prebuild(): mock(binary.BinaryPreBuildTask, task_args=TASK_ARGS) assert task.binary_container_prebuild(TASK_ARGS) == TASK_RESULT diff --git a/tests/plugins/test_cachi2_postprocess.py b/tests/plugins/test_cachi2_postprocess.py new file mode 100644 index 000000000..5d5f2ce85 --- /dev/null +++ b/tests/plugins/test_cachi2_postprocess.py @@ -0,0 +1,487 @@ +""" +Copyright (c) 2024 Red Hat, Inc +All rights reserved. + +This software may be modified and distributed under the terms +of the BSD license. See the LICENSE file for details. +""" + +import json +import io +import tarfile +from collections import namedtuple +from pathlib import Path +from textwrap import dedent +from typing import Callable, Dict + +import pytest +import yaml + +from atomic_reactor.dirs import BuildDir +from atomic_reactor.inner import DockerBuildWorkflow +from atomic_reactor.constants import ( + CACHI2_BUILD_DIR, + CACHI2_BUILD_APP_DIR, + CACHI2_SINGLE_REMOTE_SOURCE_NAME, + CACHITO_ENV_ARG_ALIAS, + CACHITO_ENV_FILENAME, + REMOTE_SOURCE_DIR, + REMOTE_SOURCE_TARBALL_FILENAME, + REMOTE_SOURCE_JSON_FILENAME, + REMOTE_SOURCE_JSON_ENV_FILENAME, + PLUGIN_CACHI2_INIT, +) +from atomic_reactor.plugin import PluginFailedException +from atomic_reactor.plugins.cachi2_postprocess import ( + Cachi2PostprocessPlugin, + Cachi2RemoteSource, +) +from atomic_reactor.source import SourceConfig +from atomic_reactor.utils.cachi2 import generate_request_json + +from tests.mock_env import MockEnv +from tests.stubs import StubSource + + +FIRST_REMOTE_SOURCE_NAME = "first" +SECOND_REMOTE_SOURCE_NAME = "second" +REMOTE_SOURCE_REPO = 'https://git.example.com/team/repo.git' +REMOTE_SOURCE_REF = 'b55c00f45ec3dfee0c766cea3d395d6e21cc2e5a' +SECOND_REMOTE_SOURCE_REPO = 'https://git.example.com/other-team/other-repo.git' +SECOND_REMOTE_SOURCE_REF = 'd55c00f45ec3dfee0c766cea3d395d6e21cc2e5c' + + +RemoteSourceInitResult = namedtuple('RemoteSourceInitResult', ['result', 'env_vars', 'sbom']) + + +def mock_cachi2_init_and_run_plugin( + workflow, *args: RemoteSourceInitResult): + + plugin_result = [] + + for arg in args: + plugin_result.append(arg.result) + + source_root_path = Path(arg.result["source_path"]) + source_root_path.mkdir(parents=True) + + app_dir = source_root_path / CACHI2_BUILD_APP_DIR + app_dir.mkdir() + + name = arg.result["name"] or "single source" + with open(app_dir / "app.txt", "w") as f: + f.write(f"test app {name}") + f.flush() + + deps_dir = source_root_path / "deps" + deps_dir.mkdir() + with open(deps_dir / "dep.txt", "w") as f: + f.write(f"dependency for {name}") + f.flush() + + with open(source_root_path / "cachi2.env.json", "w") as f: + json.dump(arg.env_vars, f) + + with open(source_root_path / "bom.json", "w") as f: + json.dump(arg.sbom, f) + + mock_cachi2_output_tarball(source_root_path / "remote-source.tar.gz") + + workflow.data.plugins_results[PLUGIN_CACHI2_INIT] = plugin_result + + +def mock_reactor_config(workflow, data=None): + config = yaml.safe_load(data) + workflow.conf.conf = config + + +def mock_repo_config(workflow, data=None): + if data is None: + data = dedent("""\ + remote_source: + repo: {} + ref: {} + """.format(REMOTE_SOURCE_REPO, REMOTE_SOURCE_REF)) + + workflow._tmpdir.joinpath('container.yaml').write_text(data, "utf-8") + + # The repo config is read when SourceConfig is initialized. Force + # reloading here to make usage easier. + workflow.source.config = SourceConfig(str(workflow._tmpdir)) + + +@pytest.fixture +def workflow(workflow: DockerBuildWorkflow, source_dir): + # Stash the tmpdir in workflow so it can be used later + workflow._tmpdir = source_dir + + class MockSource(StubSource): + + def __init__(self, workdir): + super(MockSource, self).__init__() + self.workdir = workdir + self.path = workdir + + workflow.source = MockSource(str(source_dir)) + + mock_repo_config(workflow) + + workflow.build_dir.init_build_dirs(["x86_64", "ppc64le"], workflow.source) + + return workflow + + +def expected_build_dir(workflow) -> str: + """The primary build_dir that the plugin is expected to work with.""" + return str(workflow.build_dir.any_platform.path) + + +def mock_cachi2_output_tarball(create_at_path) -> str: + """Create a mocked tarball for a remote source at the specified path.""" + create_at_path = Path(create_at_path) + file_content = f"Content of {create_at_path.name}".encode("utf-8") + + readme = tarfile.TarInfo("app/app.txt") + readme.size = len(file_content) + + with tarfile.open(create_at_path, 'w:gz') as tar: + tar.addfile(readme, io.BytesIO(file_content)) + + return str(create_at_path) + + +def check_injected_files(expected_files: Dict[str, str]) -> Callable[[BuildDir], None]: + """Make a callable that checks expected files in a BuildDir.""" + + def check_files(build_dir: BuildDir) -> None: + """Check the presence and content of files in the unpacked_remote_sources directory.""" + unpacked_remote_sources = build_dir.path / Cachi2PostprocessPlugin.REMOTE_SOURCE + + for path, expected_content in expected_files.items(): + abspath = unpacked_remote_sources / path + assert abspath.read_text() == expected_content + + return check_files + + +def test_skip_when_no_results_from_init(workflow): + """Plugin should skip if there are no results from cachi2_init plugin""" + assert run_plugin_with_args(workflow) is None + + +def test_resolve_remote_source_single(workflow): + + remote_source_sbom = { + "bomFormat": "CycloneDX", + "components": [ + { + "name": "bytes", + "purl": "pkg:golang/bytes?type=package", + "properties": [ + { + "name": "cachi2:found_by", + "value": "cachi2" + } + ], + "type": "library" + }, + ], + } + + remote_source_env_json = [ + { + "name": "GOCACHE", + "value": "/remote-source/deps/gomod", + }, + ] + + single_source = { + "name": None, + "source_path": str( + workflow.build_dir.path / CACHI2_BUILD_DIR / CACHI2_SINGLE_REMOTE_SOURCE_NAME), + "remote_source": { + "repo": REMOTE_SOURCE_REPO, + "ref": REMOTE_SOURCE_REF, + } + } + + mock_cachi2_init_and_run_plugin( + workflow, + RemoteSourceInitResult( + single_source, remote_source_env_json, remote_source_sbom + ) + ) + expected_plugin_results = [ + { + "name": None, + "remote_source_json": { + "json": generate_request_json( + single_source["remote_source"], remote_source_sbom, + remote_source_env_json), + "filename": REMOTE_SOURCE_JSON_FILENAME, + }, + "remote_source_json_env": { + "json": remote_source_env_json, + "filename": REMOTE_SOURCE_JSON_ENV_FILENAME, + }, + "remote_source_tarball": { + "filename": REMOTE_SOURCE_TARBALL_FILENAME, + "path": str(Path(single_source["source_path"]) / "remote-source.tar.gz"), + }, + }, + ] + + run_plugin_with_args( + workflow, + expected_plugin_results=expected_plugin_results, + ) + + cachito_env_content = dedent( + """\ + #!/bin/bash + export GOCACHE=/remote-source/deps/gomod + """ + ) + + workflow.build_dir.for_each_platform( + check_injected_files( + { + "cachito.env": cachito_env_content, + "app/app.txt": "test app single source", + "deps/dep.txt": "dependency for single source", + }, + ) + ) + + assert workflow.data.buildargs == { + "GOCACHE": "/remote-source/deps/gomod", + "REMOTE_SOURCE": Cachi2PostprocessPlugin.REMOTE_SOURCE, + "REMOTE_SOURCE_DIR": REMOTE_SOURCE_DIR, + CACHITO_ENV_ARG_ALIAS: str(Path(REMOTE_SOURCE_DIR, CACHITO_ENV_FILENAME)), + } + + +def test_multiple_remote_sources(workflow): + + container_yaml_config = dedent( + f"""\ + remote_sources: + - name: {FIRST_REMOTE_SOURCE_NAME} + remote_source: + repo: {REMOTE_SOURCE_REPO} + ref: {REMOTE_SOURCE_REF} + - name: {SECOND_REMOTE_SOURCE_NAME} + remote_source: + repo: {REMOTE_SOURCE_REPO} + ref: {REMOTE_SOURCE_REF} + """ + ) + + reactor_config = dedent("""\ + version: 1 + allow_multiple_remote_sources: true + """) + + first_remote_source_sbom = { + "bomFormat": "CycloneDX", + "components": [ + { + "name": "bytes", + "purl": "pkg:golang/bytes?type=package", + "properties": [ + { + "name": "cachi2:found_by", + "value": "cachi2" + } + ], + "type": "library" + }, + ], + } + + second_remote_source_sbom = { + "bomFormat": "CycloneDX", + "components": [ + { + "name": "bytes", + "purl": "pkg:pip/bytes?type=package", + "properties": [ + { + "name": "cachi2:found_by", + "value": "cachi2" + } + ], + "type": "library" + }, + ], + } + + first_remote_source_env_json = [ + { + "name": "GOCACHE", + "value": "/remote-source/deps/gomod", + }, + ] + + second_remote_source_env_json = [ + { + "name": "PIP_INDEX", + "value": "/remote-source/deps/somewhere-here", + }, + ] + + first_source = { + "name": FIRST_REMOTE_SOURCE_NAME, + "source_path": str(workflow.build_dir.path / CACHI2_BUILD_DIR / FIRST_REMOTE_SOURCE_NAME), + "remote_source": { + "repo": REMOTE_SOURCE_REPO, + "ref": REMOTE_SOURCE_REF, + "pkg_managers": ["gomod"], + "flags": ["gomod-vendor"], + } + } + + second_source = { + "name": SECOND_REMOTE_SOURCE_NAME, + "source_path": str(workflow.build_dir.path / CACHI2_BUILD_DIR / SECOND_REMOTE_SOURCE_NAME), + "remote_source": { + "repo": SECOND_REMOTE_SOURCE_REPO, + "ref": SECOND_REMOTE_SOURCE_REF, + } + } + + mock_repo_config(workflow, data=container_yaml_config) + mock_reactor_config(workflow, reactor_config) + mock_cachi2_init_and_run_plugin( + workflow, + RemoteSourceInitResult( + first_source, first_remote_source_env_json, first_remote_source_sbom), + RemoteSourceInitResult( + second_source, second_remote_source_env_json, second_remote_source_sbom), + ) + expected_plugin_results = [ + { + "name": FIRST_REMOTE_SOURCE_NAME, + "remote_source_json": { + "json": generate_request_json( + first_source["remote_source"], first_remote_source_sbom, + first_remote_source_env_json), + "filename": "remote-source-first.json", + }, + "remote_source_json_env": { + "json": first_remote_source_env_json, + "filename": "remote-source-first.env.json", + }, + "remote_source_tarball": { + "filename": "remote-source-first.tar.gz", + "path": str(Path(first_source["source_path"]) / "remote-source.tar.gz"), + }, + }, + { + "name": SECOND_REMOTE_SOURCE_NAME, + "remote_source_json": { + "json": generate_request_json( + second_source["remote_source"], second_remote_source_sbom, + second_remote_source_env_json), + "filename": "remote-source-second.json", + }, + "remote_source_json_env": { + "json": second_remote_source_env_json, + "filename": "remote-source-second.env.json", + }, + "remote_source_tarball": { + "filename": "remote-source-second.tar.gz", + "path": str(Path(second_source["source_path"]) / "remote-source.tar.gz"), + }, + }, + ] + + run_plugin_with_args(workflow, expected_plugin_results=expected_plugin_results) + + first_cachito_env = dedent( + """\ + #!/bin/bash + export GOCACHE=/remote-source/deps/gomod + """ + ) + second_cachito_env = dedent( + """\ + #!/bin/bash + export PIP_INDEX=/remote-source/deps/somewhere-here + """ + ) + + workflow.build_dir.for_each_platform( + check_injected_files( + { + f"{FIRST_REMOTE_SOURCE_NAME}/cachito.env": first_cachito_env, + f"{FIRST_REMOTE_SOURCE_NAME}/app/app.txt": f"test app {FIRST_REMOTE_SOURCE_NAME}", + f"{FIRST_REMOTE_SOURCE_NAME}/deps/dep.txt": ( + f"dependency for {FIRST_REMOTE_SOURCE_NAME}"), + f"{SECOND_REMOTE_SOURCE_NAME}/cachito.env": second_cachito_env, + f"{SECOND_REMOTE_SOURCE_NAME}/app/app.txt": f"test app {SECOND_REMOTE_SOURCE_NAME}", + f"{SECOND_REMOTE_SOURCE_NAME}/deps/dep.txt": ( + f"dependency for {SECOND_REMOTE_SOURCE_NAME}"), + }, + ) + ) + + assert workflow.data.buildargs == { + "REMOTE_SOURCES": Cachi2PostprocessPlugin.REMOTE_SOURCE, + "REMOTE_SOURCES_DIR": REMOTE_SOURCE_DIR, + } + + +def run_plugin_with_args(workflow, expect_error=None, + expect_result=True, expected_plugin_results=None): + runner = (MockEnv(workflow) + .for_plugin(Cachi2PostprocessPlugin.key) + .create_runner()) + + if expect_error: + with pytest.raises(PluginFailedException, match=expect_error): + runner.run() + return + + results = runner.run()[Cachi2PostprocessPlugin.key] + + if expect_result: + assert results == expected_plugin_results + + return results + + +def test_inject_remote_sources_dest_already_exists(workflow): + plugin = Cachi2PostprocessPlugin(workflow) + + processed_remote_sources = [ + Cachi2RemoteSource( + name=None, + json_data={}, + json_env_data={}, + tarball_path=Path("/does/not/matter"), + sources_path="/" + ), + ] + + builddir_path = Path(expected_build_dir(workflow)) + builddir_path.joinpath(Cachi2PostprocessPlugin.REMOTE_SOURCE).mkdir() + + err_msg = "Conflicting path unpacked_remote_sources already exists" + with pytest.raises(RuntimeError, match=err_msg): + plugin.inject_remote_sources(processed_remote_sources) + + +def test_generate_cachito_env_file_shell_quoting(workflow): + plugin = Cachi2PostprocessPlugin(workflow) + + dest_dir = Path(expected_build_dir(workflow)) + plugin.generate_cachito_env_file(dest_dir, {"foo": "somefile; rm -rf ~"}) + + cachito_env = dest_dir / "cachito.env" + assert cachito_env.read_text() == dedent( + """\ + #!/bin/bash + export foo='somefile; rm -rf ~' + """ + ) From f894e7cef9137e958df51bec938ac129bdf9c616 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Wed, 6 Nov 2024 13:32:27 +0100 Subject: [PATCH 14/29] cachi2: use reflink if possible use reflink to copy sources into build dir to save space Signed-off-by: Martin Basti --- atomic_reactor/plugins/cachi2_postprocess.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/atomic_reactor/plugins/cachi2_postprocess.py b/atomic_reactor/plugins/cachi2_postprocess.py index bc0c4da4e..4f18e1a82 100644 --- a/atomic_reactor/plugins/cachi2_postprocess.py +++ b/atomic_reactor/plugins/cachi2_postprocess.py @@ -9,11 +9,14 @@ import json import os.path import shlex +import shutil from dataclasses import dataclass from pathlib import Path from shutil import copytree from typing import Any, Optional, List, Dict +import reflink + from atomic_reactor.constants import ( CACHITO_ENV_ARG_ALIAS, CACHITO_ENV_FILENAME, @@ -24,7 +27,7 @@ REMOTE_SOURCE_TARBALL_FILENAME, REMOTE_SOURCE_JSON_ENV_FILENAME, ) -from atomic_reactor.dirs import BuildDir +from atomic_reactor.dirs import BuildDir, reflink_copy from atomic_reactor.plugin import Plugin from atomic_reactor.utils.cachi2 import generate_request_json @@ -176,9 +179,18 @@ def inject_into_build_dir( created_dirs.append(dest_dir) # copy app and deps generated by cachito into build_dir - # TODO: reflink? - copytree(remote_source.sources_path/'app', dest_dir/'app', symlinks=True) - copytree(remote_source.sources_path/'deps', dest_dir/'deps', symlinks=True) + copy_method = shutil.copy2 + if reflink.supported_at(dest_dir): + copy_method = reflink_copy + self.log.debug( + "copy method used for cachi2 build_dir_injecting: %s", copy_method.__name__) + + copytree( + remote_source.sources_path/'app', dest_dir/'app', + symlinks=True, copy_function=copy_method) + copytree( + remote_source.sources_path/'deps', dest_dir/'deps', + symlinks=True, copy_function=copy_method) # Create cachito.env file with environment variables received from cachito request self.generate_cachito_env_file(dest_dir, remote_source.build_args) From 4cba9f31df42f18aa61bbf05e6e2314a13eddc78 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Wed, 6 Nov 2024 20:30:16 +0100 Subject: [PATCH 15/29] cachi2: update add_image_content_manifest plugin Update add_image_content_manifest to process cachi2 results. ICM must be generated from cachi2 SBOM in this case. Signed-off-by: Martin Basti --- .../plugins/add_image_content_manifest.py | 13 ++++ .../test_add_image_content_manifest.py | 74 ++++++++++++++++++- 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/atomic_reactor/plugins/add_image_content_manifest.py b/atomic_reactor/plugins/add_image_content_manifest.py index 5232dfca3..5b55c5731 100644 --- a/atomic_reactor/plugins/add_image_content_manifest.py +++ b/atomic_reactor/plugins/add_image_content_manifest.py @@ -16,8 +16,10 @@ from atomic_reactor.constants import (IMAGE_BUILD_INFO_DIR, INSPECT_ROOTFS, INSPECT_ROOTFS_LAYERS, + CACHI2_BUILD_DIR, PLUGIN_ADD_IMAGE_CONTENT_MANIFEST, PLUGIN_FETCH_MAVEN_KEY, + PLUGIN_CACHI2_POSTPROCESS, PLUGIN_RESOLVE_REMOTE_SOURCE) from atomic_reactor.config import get_cachito_session from atomic_reactor.dirs import BuildDir @@ -25,6 +27,7 @@ from atomic_reactor.util import (validate_with_schema, read_content_sets, map_to_user_params, allow_path_in_dockerignore) from atomic_reactor.utils.pnc import PNCUtil +from atomic_reactor.utils.cachi2 import convert_SBOM_to_ICM class AddImageContentManifestPlugin(Plugin): @@ -100,6 +103,8 @@ def __init__(self, workflow, destdir=IMAGE_BUILD_INFO_DIR): remote_source_results = wf_data.plugins_results.get(PLUGIN_RESOLVE_REMOTE_SOURCE) or [] self.remote_source_ids = [remote_source['id'] for remote_source in remote_source_results] + self.cachi2_remote_sources = wf_data.plugins_results.get(PLUGIN_CACHI2_POSTPROCESS) or [] + fetch_maven_results = wf_data.plugins_results.get(PLUGIN_FETCH_MAVEN_KEY) or {} self.pnc_artifact_ids = fetch_maven_results.get('pnc_artifact_ids') or [] @@ -130,6 +135,12 @@ def layer_index(self) -> int: return len(inspect[INSPECT_ROOTFS][INSPECT_ROOTFS_LAYERS]) + def _get_cachi2_icm(self) -> dict: + global_sbom_path = self.workflow.build_dir.path/CACHI2_BUILD_DIR/"bom.json" + with open(global_sbom_path, "r") as f: + sbom = json.load(f) + return convert_SBOM_to_ICM(sbom) + @functools.cached_property def _icm_base(self) -> dict: """Create the platform-independent skeleton of the ICM document. @@ -140,6 +151,8 @@ def _icm_base(self) -> dict: if self.remote_source_ids: icm = self.cachito_session.get_image_content_manifest(self.remote_source_ids) + elif self.cachi2_remote_sources: # we doesn't support Cachito and Cachi2 together + icm = self._get_cachi2_icm() if self.pnc_artifact_ids: purl_specs = self.pnc_util.get_artifact_purl_specs(self.pnc_artifact_ids) diff --git a/tests/plugins/test_add_image_content_manifest.py b/tests/plugins/test_add_image_content_manifest.py index d92e1a849..a95804d29 100644 --- a/tests/plugins/test_add_image_content_manifest.py +++ b/tests/plugins/test_add_image_content_manifest.py @@ -21,8 +21,10 @@ from tests.utils.test_cachito import CACHITO_URL, CACHITO_REQUEST_ID from atomic_reactor.constants import ( + CACHI2_BUILD_DIR, INSPECT_ROOTFS, INSPECT_ROOTFS_LAYERS, + PLUGIN_CACHI2_POSTPROCESS, PLUGIN_FETCH_MAVEN_KEY, PLUGIN_RESOLVE_REMOTE_SOURCE, DOCKERIGNORE, @@ -39,6 +41,28 @@ } CACHITO_ICM_URL = '{}/api/v1/content-manifest?requests={}'.format(CACHITO_URL, CACHITO_REQUEST_ID) + +CACHI2_SBOM = { + "bomFormat": "CycloneDX", + "components": [{ + "name": "retrodep", + "purl": "pkg:golang/github.com%2Frelease-engineering%2Fretrodep%2Fv2@v2.0.2", + "properties": [{ + "name": "cachi2:found_by", + "value": "cachi2", + }], + "type": "library", + }], + "metadata": { + "tools": [{ + "vendor": "red hat", + "name": "cachi2" + }] + }, + "specVersion": "1.4", + "version": 1 +} + PNC_ARTIFACT = { 'id': 1234, 'publicUrl': 'http://test.com/artifact.jar', @@ -88,6 +112,28 @@ } ] } + +CACHI2_ICM_DICT = { + 'metadata': { + 'icm_version': 1, + 'icm_spec': ( + 'https://raw.githubusercontent.com/containerbuildsystem/atomic-reactor/' + 'f4abcfdaf8247a6b074f94fa84f3846f82d781c6/atomic_reactor/schemas/' + 'content_manifest.json'), + 'image_layer_index': 1, + }, + 'content_sets': [], + 'image_contents': [ + { + 'purl': + 'pkg:golang/github.com%2Frelease-engineering%2Fretrodep%2Fv2@v2.0.2', + }, + { + 'purl': PNC_ARTIFACT['purl'], + } + ] +} + ICM_JSON = dedent( '''\ { @@ -157,7 +203,8 @@ def mock_get_icm(requests_mock): def mock_env(workflow, df_content, base_layers=0, remote_sources=None, - r_c_m_override=None, pnc_artifacts=True, dockerignore=False): + r_c_m_override=None, pnc_artifacts=True, dockerignore=False, + cachi2_sbom=None): if base_layers > 0: inspection_data = { @@ -206,6 +253,19 @@ def mock_env(workflow, df_content, base_layers=0, remote_sources=None, platforms = list(CONTENT_SETS.keys()) workflow.build_dir.init_build_dirs(platforms, workflow.source) + if cachi2_sbom: + env.set_plugin_result( + PLUGIN_CACHI2_POSTPROCESS, + {"plugin": "did run, real value doesn't matter"} + ) + + # save cachi2 SBOM which is source for ICM + path = workflow.build_dir.path/CACHI2_BUILD_DIR/"bom.json" + path.parent.mkdir() + with open(path, "w") as f: + json.dump(cachi2_sbom, f) + f.flush() + return env.create_runner() @@ -239,6 +299,7 @@ def check_in_build_dir(build_dir): @pytest.mark.parametrize('manifest_file_exists', [True, False]) @pytest.mark.parametrize('content_sets', [True, False]) +@pytest.mark.parametrize('cachi2', [True, False]) @pytest.mark.parametrize( ('df_content, expected_df, base_layers, manifest_file'), [ ( @@ -288,13 +349,18 @@ def check_in_build_dir(build_dir): ), ]) def test_add_image_content_manifest(workflow, requests_mock, - manifest_file_exists, content_sets, + manifest_file_exists, content_sets, cachi2, df_content, expected_df, base_layers, manifest_file, ): mock_get_icm(requests_mock) mock_content_sets_config(workflow.source.path, empty=(not content_sets)) - runner = mock_env(workflow, df_content, base_layers, remote_sources=REMOTE_SOURCES) + if cachi2: + runner_opts = {"cachi2_sbom": CACHI2_SBOM} + else: + runner_opts = {"remote_sources": REMOTE_SOURCES} + + runner = mock_env(workflow, df_content, base_layers, **runner_opts) if manifest_file_exists: workflow.build_dir.any_platform.path.joinpath(manifest_file).touch() @@ -304,7 +370,7 @@ def test_add_image_content_manifest(workflow, requests_mock, runner.run() return - expected_output = deepcopy(ICM_DICT) + expected_output = deepcopy(CACHI2_ICM_DICT if cachi2 else ICM_DICT) expected_output['metadata']['image_layer_index'] = base_layers if base_layers else 0 runner.run() From 59efc9bc3b8dabac695789be21a952fc66d5d35f Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Thu, 7 Nov 2024 16:00:24 +0100 Subject: [PATCH 16/29] cachi2: update generate_sbom plugin Update generate_sbom to process cachi2 results. SBOM must be combined from cachi2 SBOM stored in build_dir in this case. Signed-off-by: Martin Basti --- atomic_reactor/plugins/generate_sbom.py | 14 +++++++++- tests/plugins/test_generate_sbom.py | 37 +++++++++++++++++++++---- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/atomic_reactor/plugins/generate_sbom.py b/atomic_reactor/plugins/generate_sbom.py index 0a24cd968..30385c16d 100644 --- a/atomic_reactor/plugins/generate_sbom.py +++ b/atomic_reactor/plugins/generate_sbom.py @@ -13,13 +13,15 @@ from typing import Any, Dict, List, Optional from atomic_reactor.constants import (PLUGIN_GENERATE_SBOM, + PLUGIN_CACHI2_POSTPROCESS, PLUGIN_RPMQA, PLUGIN_RESOLVE_REMOTE_SOURCE, SBOM_SCHEMA_PATH, PLUGIN_FETCH_MAVEN_KEY, INSPECT_CONFIG, KOJI_BTYPE_ICM, - ICM_JSON_FILENAME) + ICM_JSON_FILENAME, + CACHI2_BUILD_DIR) from atomic_reactor.config import get_cachito_session, get_koji_session from atomic_reactor.utils import retries from atomic_reactor.utils.cachito import CachitoAPI @@ -92,6 +94,8 @@ def __init__(self, workflow): remote_source_results = wf_data.plugins_results.get(PLUGIN_RESOLVE_REMOTE_SOURCE) or [] self.remote_source_ids = [remote_source['id'] for remote_source in remote_source_results] + self.cachi2_remote_sources = wf_data.plugins_results.get(PLUGIN_CACHI2_POSTPROCESS) or [] + self.rpm_components = wf_data.plugins_results.get(PLUGIN_RPMQA) or {} fetch_maven_results = wf_data.plugins_results.get(PLUGIN_FETCH_MAVEN_KEY) or {} @@ -131,6 +135,12 @@ def fetch_url_or_koji_check(self) -> None: if read_fetch_artifacts_url(self.workflow): self.incompleteness_reasons.add("fetch url is used") + def get_cachi2_sbom(self) -> dict: + """Get SBOM from cachi2 results""" + global_sbom_path = self.workflow.build_dir.path/CACHI2_BUILD_DIR/"bom.json" + with open(global_sbom_path, "r") as f: + return json.load(f) + def add_parent_missing_sbom_reason(self, nvr: str) -> None: self.incompleteness_reasons.add(f"parent build '{nvr}' is missing SBOM") @@ -331,6 +341,8 @@ def run(self) -> Dict[str, Any]: if self.remote_source_ids: remote_sources_sbom = self.cachito_session.get_sbom(self.remote_source_ids) remote_souces_components = remote_sources_sbom['components'] + elif self.cachi2_remote_sources: # Cachi2 and Cachito are not supported to be used together + remote_souces_components = self.get_cachi2_sbom()['components'] # add components from cachito, rpms, pnc for platform in self.all_platforms: diff --git a/tests/plugins/test_generate_sbom.py b/tests/plugins/test_generate_sbom.py index ecef5dbb4..86b9d6074 100644 --- a/tests/plugins/test_generate_sbom.py +++ b/tests/plugins/test_generate_sbom.py @@ -32,6 +32,8 @@ REPO_FETCH_ARTIFACTS_KOJI, REPO_FETCH_ARTIFACTS_URL, PLUGIN_CHECK_AND_SET_PLATFORMS_KEY, + PLUGIN_CACHI2_POSTPROCESS, + CACHI2_BUILD_DIR, ) from atomic_reactor.plugin import PluginFailedException from atomic_reactor.plugins.generate_sbom import GenerateSbomPlugin @@ -1065,7 +1067,7 @@ def teardown_function(*args): sys.modules.pop(GenerateSbomPlugin.key, None) -def mock_env(workflow, df_images): +def mock_env(workflow, df_images, cachi2=False): tmp_dir = tempfile.mkdtemp() dockerconfig_contents = {"auths": {LOCALHOST_REGISTRY: {"username": "user", "email": "test@example.com", @@ -1099,12 +1101,20 @@ def mock_env(workflow, df_images): .for_plugin(GenerateSbomPlugin.key) .set_reactor_config(r_c_m) .set_dockerfile_images(df_images) - .set_plugin_result(PLUGIN_RESOLVE_REMOTE_SOURCE, deepcopy(REMOTE_SOURCES)) .set_plugin_result(PLUGIN_FETCH_MAVEN_KEY, {'sbom_components': deepcopy(PNC_SBOM_COMPONENTS)}) .set_plugin_result(PLUGIN_RPMQA, deepcopy(RPM_SBOM_COMPONENTS)) ) + if cachi2: + # Note: using CACHITO_SBOM_JSON here, as the fields are almost the same as + # for Cachi2; I don't want to die from mocking everything again, just to + # make test pass for extra property "found_by: cachi2" added by cachi2 + # this provides good tests + mock_cachi2_sbom(workflow, deepcopy(CACHITO_SBOM_JSON)) + else: + env.set_plugin_result(PLUGIN_RESOLVE_REMOTE_SOURCE, deepcopy(REMOTE_SOURCES)) + all_inspects = [(EMPTY_SBOM_IMAGE_LABELS, EMPTY_SBOM_IMAGE_NAME), (MISSING_LABEL_IMAGE_LABELS, MISSING_LABEL_IMAGE_NAME), (BUILDING_IMAGE_LABELS, BUILDING_IMAGE_NAME), @@ -1131,6 +1141,19 @@ def mock_get_sbom_cachito(requests_mock): requests_mock.register_uri('GET', CACHITO_SBOM_URL, json=CACHITO_SBOM_JSON) +def mock_cachi2_sbom(workflow, cachi2_sbom: dict): + workflow.data.plugins_results[PLUGIN_CACHI2_POSTPROCESS] = { + "plugin": "did run, real value doesn't matter" + } + + # save cachi2 SBOM which is source for ICM + path = workflow.build_dir.path/CACHI2_BUILD_DIR/"bom.json" + path.parent.mkdir() + with open(path, "w") as f: + json.dump(cachi2_sbom, f) + f.flush() + + def mock_build_icm_urls(requests_mock): all_sboms = [(EMPTY_SBOM_KOJI_BUILD, EMPTY_SBOM_BUILD_SBOM_JSON), (BASE_WITH_SBOM_KOJI_BUILD, BASE_WITH_SBOM_BUILD_SBOM_JSON), @@ -1311,12 +1334,16 @@ def koji_session(): INCOMPLETE_CACHE_URL_KOJI, ), ]) +@pytest.mark.parametrize('cachi2', [True, False]) def test_sbom(workflow, requests_mock, koji_session, df_images, use_cache, use_fetch_url, - use_fetch_koji, expected_components, expected_incomplete): - mock_get_sbom_cachito(requests_mock) + use_fetch_koji, expected_components, expected_incomplete, cachi2): + + if not cachi2: + mock_get_sbom_cachito(requests_mock) + mock_build_icm_urls(requests_mock) - runner = mock_env(workflow, df_images) + runner = mock_env(workflow, df_images, cachi2=cachi2) workflow.data.tag_conf.add_unique_image(UNIQUE_IMAGE) def check_cosign_run(args): From 6d0868737419cb84e86a601375689ad9461c82c8 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Thu, 7 Nov 2024 19:28:10 +0100 Subject: [PATCH 17/29] cachi2: update koji_import plugin Update koji_import to process cachi2 results. Signed-off-by: Martin Basti --- atomic_reactor/plugins/koji_import.py | 118 ++++++++++++++++++-------- tests/plugins/test_koji_import.py | 62 ++++++++++++-- 2 files changed, 139 insertions(+), 41 deletions(-) diff --git a/atomic_reactor/plugins/koji_import.py b/atomic_reactor/plugins/koji_import.py index 9dd190d95..f84d1cdce 100644 --- a/atomic_reactor/plugins/koji_import.py +++ b/atomic_reactor/plugins/koji_import.py @@ -36,6 +36,7 @@ from atomic_reactor.plugins.fetch_sources import PLUGIN_FETCH_SOURCES_KEY from atomic_reactor.constants import ( + PLUGIN_CACHI2_POSTPROCESS, PLUGIN_EXPORT_OPERATOR_MANIFESTS_KEY, PLUGIN_KOJI_IMPORT_PLUGIN_KEY, PLUGIN_KOJI_IMPORT_SOURCE_CONTAINER_PLUGIN_KEY, @@ -303,41 +304,76 @@ def set_pnc_build_metadata(self, extra): if pnc_build_metadata: extra['image']['pnc'] = pnc_build_metadata - def set_remote_sources_metadata(self, extra): - remote_source_result = self.workflow.data.plugins_results.get( - PLUGIN_RESOLVE_REMOTE_SOURCE - ) - if remote_source_result: - if self.workflow.conf.allow_multiple_remote_sources: - remote_sources_image_metadata = [ - {"name": remote_source["name"], "url": remote_source["url"].rstrip('/download')} - for remote_source in remote_source_result - ] - extra["image"]["remote_sources"] = remote_sources_image_metadata - - remote_sources_typeinfo_metadata = [ - { - "name": remote_source["name"], - "url": remote_source["url"].rstrip('/download'), - "archives": [ - remote_source["remote_source_json"]["filename"], - remote_source["remote_source_tarball"]["filename"], - remote_source["remote_source_json_env"]["filename"], - remote_source["remote_source_json_config"]["filename"], - ], - } - for remote_source in remote_source_result - ] - else: - extra["image"]["remote_source_url"] = remote_source_result[0]["url"] - remote_sources_typeinfo_metadata = { - "remote_source_url": remote_source_result[0]["url"] - } + def set_remote_sources_metadata_cachito(self, remote_source_result, extra): + if self.workflow.conf.allow_multiple_remote_sources: + remote_sources_image_metadata = [ + {"name": remote_source["name"], "url": remote_source["url"].rstrip('/download')} + for remote_source in remote_source_result + ] + extra["image"]["remote_sources"] = remote_sources_image_metadata - remote_source_typeinfo = { - KOJI_BTYPE_REMOTE_SOURCES: remote_sources_typeinfo_metadata, + remote_sources_typeinfo_metadata = [ + { + "name": remote_source["name"], + "url": remote_source["url"].rstrip('/download'), + "archives": [ + remote_source["remote_source_json"]["filename"], + remote_source["remote_source_tarball"]["filename"], + remote_source["remote_source_json_env"]["filename"], + remote_source["remote_source_json_config"]["filename"], + ], + } + for remote_source in remote_source_result + ] + else: + extra["image"]["remote_source_url"] = remote_source_result[0]["url"] + remote_sources_typeinfo_metadata = { + "remote_source_url": remote_source_result[0]["url"] } - extra.setdefault("typeinfo", {}).update(remote_source_typeinfo) + + remote_source_typeinfo = { + KOJI_BTYPE_REMOTE_SOURCES: remote_sources_typeinfo_metadata, + } + extra.setdefault("typeinfo", {}).update(remote_source_typeinfo) + + def set_remote_sources_metadata_cachi2(self, remote_source_result, extra): + remote_sources_typeinfo_metadata = [] + if self.workflow.conf.allow_multiple_remote_sources: + remote_sources_image_metadata = [ + {"name": remote_source["name"]} + for remote_source in remote_source_result + ] + extra["image"]["remote_sources"] = remote_sources_image_metadata + + remote_sources_typeinfo_metadata = [ + { + "name": remote_source["name"], + "archives": [ + remote_source["remote_source_json"]["filename"], + remote_source["remote_source_tarball"]["filename"], + remote_source["remote_source_json_env"]["filename"], + ], + } + for remote_source in remote_source_result + ] + + remote_source_typeinfo = { + KOJI_BTYPE_REMOTE_SOURCES: remote_sources_typeinfo_metadata, + } + extra.setdefault("typeinfo", {}).update(remote_source_typeinfo) + + def set_remote_sources_metadata(self, extra): + func_map = { + PLUGIN_RESOLVE_REMOTE_SOURCE: self.set_remote_sources_metadata_cachito, + PLUGIN_CACHI2_POSTPROCESS: self.set_remote_sources_metadata_cachi2, + } + for plugin_name, func in func_map.items(): + remote_source_result = self.workflow.data.plugins_results.get( + plugin_name + ) + if remote_source_result: + func(remote_source_result, extra) + break def set_remote_source_file_metadata(self, extra): maven_url_sources_metadata_results = self.workflow.data.plugins_results.get( @@ -613,9 +649,21 @@ def _filesystem_koji_task_id(self) -> Optional[int]: def _collect_remote_sources(self) -> Iterable[ArtifactOutputInfo]: wf_data = self.workflow.data + + remote_source_keys = [ + "remote_source_json", "remote_source_json_env", "remote_source_json_config", + ] + # a list of metadata describing the remote sources. plugin_results: List[Dict[str, Any]] plugin_results = wf_data.plugins_results.get(PLUGIN_RESOLVE_REMOTE_SOURCE) or [] + if not plugin_results: + # Cachi2 + plugin_results = wf_data.plugins_results.get(PLUGIN_CACHI2_POSTPROCESS) or [] + remote_source_keys = [ + "remote_source_json", "remote_source_json_env", + ] + tmpdir = tempfile.mkdtemp() for remote_source in plugin_results: @@ -624,9 +672,7 @@ def _collect_remote_sources(self) -> Iterable[ArtifactOutputInfo]: dest_filename = remote_source_tarball['filename'] yield local_filename, dest_filename, KOJI_BTYPE_REMOTE_SOURCES, None - for source_key in ( - "remote_source_json", "remote_source_json_env", "remote_source_json_config", - ): + for source_key in remote_source_keys: data_json = remote_source[source_key] data_json_filename = data_json['filename'] file_path = os.path.join(tmpdir, data_json_filename) diff --git a/tests/plugins/test_koji_import.py b/tests/plugins/test_koji_import.py index c5cb5f00c..d44a61202 100644 --- a/tests/plugins/test_koji_import.py +++ b/tests/plugins/test_koji_import.py @@ -7,6 +7,7 @@ """ from collections import namedtuple +from enum import Enum import json from pathlib import Path from typing import Any, Dict @@ -37,6 +38,7 @@ from atomic_reactor.source import GitSource, PathSource from atomic_reactor.constants import (IMAGE_TYPE_DOCKER_ARCHIVE, KOJI_BTYPE_OPERATOR_MANIFESTS, PLUGIN_ADD_FILESYSTEM_KEY, + PLUGIN_CACHI2_POSTPROCESS, PLUGIN_EXPORT_OPERATOR_MANIFESTS_KEY, PLUGIN_MAVEN_URL_SOURCES_METADATA_KEY, PLUGIN_GROUP_MANIFESTS_KEY, PLUGIN_KOJI_PARENT_KEY, @@ -185,6 +187,12 @@ def taskFinished(self, task_id): REGISTRY = 'docker.example.com' +class RemoteSourceKind(Enum): + NONE = 1 + CACHITO = 2 + CACHI2 = 3 + + def fake_subprocess_output(cmd): if cmd.startswith('/bin/rpm'): return FAKE_RPM_OUTPUT @@ -268,7 +276,8 @@ def mock_environment(workflow: DockerBuildWorkflow, source_dir: Path, has_op_appregistry_manifests=False, has_op_bundle_manifests=False, push_operator_manifests_enabled=False, source_build=False, - has_remote_source=False, has_remote_source_file=False, + has_remote_source: RemoteSourceKind = RemoteSourceKind.NONE, + has_remote_source_file=False, has_pnc_build_metadata=False, scratch=False): if session is None: session = MockedClientSession('') @@ -524,7 +533,7 @@ def custom_get(method, url, headers, **kwargs): results = workflow.data.plugins_results results[PLUGIN_EXPORT_OPERATOR_MANIFESTS_KEY] = str(archive_file) - if has_remote_source: + if has_remote_source == RemoteSourceKind.CACHITO: source_path = build_dir_path / REMOTE_SOURCE_TARBALL_FILENAME source_path.write_text('dummy file', 'utf-8') remote_source_result = [ @@ -550,6 +559,28 @@ def custom_get(method, url, headers, **kwargs): } ] workflow.data.plugins_results[PLUGIN_RESOLVE_REMOTE_SOURCE] = remote_source_result + elif has_remote_source == RemoteSourceKind.CACHI2: + source_path = build_dir_path / REMOTE_SOURCE_TARBALL_FILENAME + source_path.write_text('dummy file', 'utf-8') + remote_source_result = [ + { + "name": None, + "url": "https://cachito.com/api/v1/requests/21048/download", + "remote_source_json": { + "filename": REMOTE_SOURCE_JSON_FILENAME, + "json": {"stub": "data"}, + }, + "remote_source_json_env": { + "filename": REMOTE_SOURCE_JSON_ENV_FILENAME, + "json": {"var": {"stub": "data"}}, + }, + "remote_source_tarball": { + "filename": REMOTE_SOURCE_TARBALL_FILENAME, + "path": str(source_path), + }, + } + ] + workflow.data.plugins_results[PLUGIN_CACHI2_POSTPROCESS] = remote_source_result else: workflow.data.plugins_results[PLUGIN_RESOLVE_REMOTE_SOURCE] = None @@ -1977,7 +2008,7 @@ def test_operators_bundle_metadata_csv_modifications( assert extra['image']['operator_manifests'] == expected - @pytest.mark.parametrize('has_remote_source', [True, False]) + @pytest.mark.parametrize('has_remote_source', list(RemoteSourceKind)) @pytest.mark.parametrize('allow_multiple_remote_sources', [True, False]) def test_remote_sources(self, workflow, source_dir, has_remote_source, allow_multiple_remote_sources): @@ -2000,7 +2031,7 @@ def test_remote_sources(self, workflow, source_dir, assert isinstance(extra, dict) # https://github.com/PyCQA/pylint/issues/2186 # pylint: disable=W1655 - if has_remote_source: + if has_remote_source == RemoteSourceKind.CACHITO: if allow_multiple_remote_sources: assert extra['image']['remote_sources'] == [ { @@ -2031,7 +2062,28 @@ def test_remote_sources(self, workflow, source_dir, } assert REMOTE_SOURCE_TARBALL_FILENAME in session.uploaded_files.keys() assert REMOTE_SOURCE_JSON_FILENAME in session.uploaded_files.keys() - + elif has_remote_source == RemoteSourceKind.CACHI2: + if allow_multiple_remote_sources: + assert extra['image']['remote_sources'] == [ + { + 'name': None, + } + ] + assert 'remote-sources' in extra['typeinfo'] + assert extra['typeinfo']['remote-sources'] == [ + { + 'name': None, + 'archives': [ + 'remote-source.json', 'remote-source.tar.gz', + 'remote-source.env.json' + ], + } + ] + assert REMOTE_SOURCE_TARBALL_FILENAME in session.uploaded_files.keys() + assert REMOTE_SOURCE_JSON_FILENAME in session.uploaded_files.keys() + else: + assert REMOTE_SOURCE_TARBALL_FILENAME in session.uploaded_files.keys() + assert REMOTE_SOURCE_JSON_FILENAME in session.uploaded_files.keys() else: assert 'remote_source_url' not in extra['image'] assert REMOTE_SOURCE_TARBALL_FILENAME not in session.uploaded_files.keys() From 8a862a665539f03c1aeac384a479ec9bc88a4198 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Mon, 18 Nov 2024 23:07:37 +0100 Subject: [PATCH 18/29] cachi2: support both Cachito and Cachi2 User/admin may specify which version of remote sources should be used (or used by default): - 1: Cachito (current default) - 2: Cachi2 Build pipeline will then use the right task based on condition using result from the init task. STONEBLD-2591 Signed-off-by: Martin Basti --- atomic_reactor/cli/parser.py | 3 + atomic_reactor/config.py | 5 ++ atomic_reactor/inner.py | 3 + atomic_reactor/plugins/check_user_settings.py | 17 +++++ atomic_reactor/schemas/config.json | 6 ++ atomic_reactor/source.py | 1 + atomic_reactor/tasks/binary.py | 2 + requirements.in | 2 +- requirements.txt | 2 +- tekton/pipelines/binary-container.yaml | 45 ++++++++++++ tekton/tasks/binary-container-init.yaml | 12 +++- tests/cli/test_parser.py | 9 ++- tests/plugins/test_check_user_settings.py | 71 +++++++++++++++++++ tests/stubs.py | 1 + tests/tasks/test_plugin_based.py | 4 +- tests/test_config.py | 13 ++++ tests/test_source.py | 9 +++ 17 files changed, 200 insertions(+), 5 deletions(-) diff --git a/atomic_reactor/cli/parser.py b/atomic_reactor/cli/parser.py index 22ebdbc5f..58a717a64 100644 --- a/atomic_reactor/cli/parser.py +++ b/atomic_reactor/cli/parser.py @@ -86,6 +86,9 @@ def parse_args(args: Optional[Sequence[str]] = None) -> dict: binary_container_init.set_defaults(func=task.binary_container_init) binary_container_init.add_argument("--platforms-result", metavar="FILE", default=None, help="file to write final platforms result") + binary_container_init.add_argument("--remote-sources-version-result", metavar="FILE", + default=None, + help="file to write final remote-sources version result") binary_container_cachito = tasks.add_parser( "binary-container-cachito", diff --git a/atomic_reactor/config.py b/atomic_reactor/config.py index 547444304..58960aba5 100644 --- a/atomic_reactor/config.py +++ b/atomic_reactor/config.py @@ -187,6 +187,7 @@ class ReactorConfigKeys(object): OPERATOR_MANIFESTS_KEY = 'operator_manifests' IMAGE_SIZE_LIMIT_KEY = 'image_size_limit' BUILDER_CA_BUNDLE_KEY = 'builder_ca_bundle' + REMOTE_SOURCES_DEFAULT_VERSION = 'remote_sources_default_version' class ODCSConfig(object): @@ -511,3 +512,7 @@ def image_size_limit(self): @property def builder_ca_bundle(self): return self._get_value(ReactorConfigKeys.BUILDER_CA_BUNDLE_KEY, fallback=None) + + @property + def remote_sources_default_version(self): + return self._get_value(ReactorConfigKeys.REMOTE_SOURCES_DEFAULT_VERSION, fallback=1) diff --git a/atomic_reactor/inner.py b/atomic_reactor/inner.py index d61c264b6..9bf378f59 100644 --- a/atomic_reactor/inner.py +++ b/atomic_reactor/inner.py @@ -462,6 +462,7 @@ def __init__( plugin_files: Optional[List[str]] = None, keep_plugins_running: bool = False, platforms_result: Optional[str] = None, + remote_sources_version_result: Optional[str] = None, annotations_result: Optional[str] = None, ): """ @@ -483,6 +484,7 @@ def __init__( :param bool keep_plugins_running: keep plugins running even if error is raised from previous one. This is passed to ``PluginsRunner`` directly. :param platforms_result: path to platform results for prebuild task + :param remote_sources_version_result: path to remote_sources_version result :param annotations_result: path to annotations result for exit task """ self.context_dir = context_dir @@ -493,6 +495,7 @@ def __init__( self.source = source or DummySource(None, None) self.user_params = user_params or self._default_user_params.copy() self.platforms_result = platforms_result + self.remote_sources_version_result = remote_sources_version_result self.annotations_result = annotations_result self.keep_plugins_running = keep_plugins_running diff --git a/atomic_reactor/plugins/check_user_settings.py b/atomic_reactor/plugins/check_user_settings.py index ccffccc22..5caad36f1 100644 --- a/atomic_reactor/plugins/check_user_settings.py +++ b/atomic_reactor/plugins/check_user_settings.py @@ -109,9 +109,26 @@ def validate_user_config_files(self): read_fetch_artifacts_url(self.workflow) read_content_sets(self.workflow) + def resolve_remote_sources_version(self): + """Resolve which version of remote sources should be used""" + version = self.workflow.source.config.remote_sources_version + if not version: + version = self.workflow.conf.remote_sources_default_version + + self.log.info("Remote sources version to be used: %d", version) + + if self.workflow.remote_sources_version_result: + with open(self.workflow.remote_sources_version_result, 'w') as f: + f.write(f"{version}") + f.flush() + else: + self.log.warning("remote_sources_version_result path is not specified, " + "result won't be written") + def run(self): """ run the plugin """ self.dockerfile_checks() self.validate_user_config_files() + self.resolve_remote_sources_version() diff --git a/atomic_reactor/schemas/config.json b/atomic_reactor/schemas/config.json index 785451db0..99fd051da 100644 --- a/atomic_reactor/schemas/config.json +++ b/atomic_reactor/schemas/config.json @@ -708,6 +708,12 @@ } }, "additionalProperties": false + }, + "remote_sources_default_version": { + "description": "Default version of remote sources resolving which will be used when user doesn't explicitly specify it", + "type": "number", + "minimum": 1, + "maximum": 2 } }, "definitions": { diff --git a/atomic_reactor/source.py b/atomic_reactor/source.py index 4b2d2609b..7d9975238 100644 --- a/atomic_reactor/source.py +++ b/atomic_reactor/source.py @@ -72,6 +72,7 @@ def __init__(self, build_path): self.compose.pop('inherit', None) self.remote_source = self.data.get('remote_source') self.remote_sources = self.data.get('remote_sources') + self.remote_sources_version = self.data.get('remote_sources_version', 1) self.operator_manifests = self.data.get('operator_manifests') self.platforms = self.data.get('platforms') or {'not': [], 'only': []} diff --git a/atomic_reactor/tasks/binary.py b/atomic_reactor/tasks/binary.py index 6289169a4..9d52d192c 100644 --- a/atomic_reactor/tasks/binary.py +++ b/atomic_reactor/tasks/binary.py @@ -27,6 +27,7 @@ class InitTaskParams(TaskParams): """Binary container init task parameters""" platforms_result: Optional[str] + remote_sources_version_result: Optional[str] @dataclass(frozen=True) @@ -68,6 +69,7 @@ def prepare_workflow(self) -> inner.DockerBuildWorkflow: plugins_conf=self.plugins_conf, keep_plugins_running=self.keep_plugins_running, platforms_result=self._params.platforms_result, + remote_sources_version_result=self._params.remote_sources_version_result, ) return workflow diff --git a/requirements.in b/requirements.in index 6ef7f5ac1..9327fafdb 100644 --- a/requirements.in +++ b/requirements.in @@ -6,7 +6,7 @@ jsonschema paramiko>=3.4.0 PyYAML ruamel.yaml -git+https://github.com/containerbuildsystem/osbs-client@628b0707b5f3ebb40399ff8f622db22e4799b8fb#egg=osbs-client +git+https://github.com/containerbuildsystem/osbs-client@541387019135f2bb6fc8054a453641f0e9feb3d7#egg=osbs-client # cannot build cryptography with rust for version >= 3.5 cryptography < 3.5 requests diff --git a/requirements.txt b/requirements.txt index 20e93b7fd..d4fd230a8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -111,7 +111,7 @@ opentelemetry-semantic-conventions==0.41b0 # opentelemetry-sdk opentelemetry-util-http==0.41b0 # via opentelemetry-instrumentation-requests -osbs-client @ git+https://github.com/containerbuildsystem/osbs-client@628b0707b5f3ebb40399ff8f622db22e4799b8fb +osbs-client @ git+https://github.com/containerbuildsystem/osbs-client@541387019135f2bb6fc8054a453641f0e9feb3d7 # via -r requirements.in otel-extensions==0.2.5 # via -r otel-requirements.in diff --git a/tekton/pipelines/binary-container.yaml b/tekton/pipelines/binary-container.yaml index af2d17142..949efee48 100644 --- a/tekton/pipelines/binary-container.yaml +++ b/tekton/pipelines/binary-container.yaml @@ -7,6 +7,9 @@ spec: - name: osbs-image description: The location of the OSBS builder image (FQDN pullspec) type: string + - name: cachi2-image + description: The location of the Cachi2 image (FQDN pullspec) + type: string - name: user-params type: string description: User parameters in JSON format @@ -140,11 +143,53 @@ spec: value: "$(context.pipelineRun.name)" - name: user-params value: '$(params.user-params)' + when: + - input: "$(tasks.binary-container-init.results.remote_sources_version_result)" + operator: in + values: ["1"] + timeout: "0" + + - name: binary-container-cachi2 + runAfter: + - binary-container-init + taskRef: + name: binary-container-cachi2-0-1 + workspaces: + - name: ws-build-dir + workspace: ws-container + subPath: build-dir + - name: ws-context-dir + workspace: ws-container + subPath: context-dir + - name: ws-home-dir + workspace: ws-home-dir + - name: ws-registries-secret + workspace: ws-registries-secret + - name: ws-koji-secret + workspace: ws-koji-secret + - name: ws-reactor-config-map + workspace: ws-reactor-config-map + - name: ws-autobot-keytab + workspace: ws-autobot-keytab + params: + - name: osbs-image + value: "$(params.osbs-image)" + - name: cachi2-image + value: "$(params.cachi2-image)" + - name: pipeline-run-name + value: "$(context.pipelineRun.name)" + - name: user-params + value: '$(params.user-params)' + when: + - input: "$(tasks.binary-container-init.results.remote_sources_version_result)" + operator: in + values: ["2"] timeout: "0" - name: binary-container-prebuild runAfter: - binary-container-cachito + - binary-container-cachi2 taskRef: resolver: git params: diff --git a/tekton/tasks/binary-container-init.yaml b/tekton/tasks/binary-container-init.yaml index a54d9de5b..f067ee4e8 100644 --- a/tekton/tasks/binary-container-init.yaml +++ b/tekton/tasks/binary-container-init.yaml @@ -27,6 +27,7 @@ spec: results: - name: platforms_result + - name: remote_sources_version_result stepTemplate: env: @@ -46,4 +47,13 @@ spec: cpu: 395m script: | set -x - atomic-reactor -v task --user-params='$(params.user-params)' --build-dir=$(workspaces.ws-build-dir.path) --context-dir=$(workspaces.ws-context-dir.path) --config-file=$(workspaces.ws-reactor-config-map.path)/config.yaml --namespace=$(context.taskRun.namespace) --pipeline-run-name="$(params.pipeline-run-name)" binary-container-init --platforms-result=$(results.platforms_result.path) + atomic-reactor -v task \ + --user-params='$(params.user-params)' \ + --build-dir=$(workspaces.ws-build-dir.path) \ + --context-dir=$(workspaces.ws-context-dir.path) \ + --config-file=$(workspaces.ws-reactor-config-map.path)/config.yaml \ + --namespace=$(context.taskRun.namespace) \ + --pipeline-run-name="$(params.pipeline-run-name)" \ + binary-container-init \ + --platforms-result=$(results.platforms_result.path) \ + --remote-sources-version-result="$(results.remote_sources_version_result.path)" diff --git a/tests/cli/test_parser.py b/tests/cli/test_parser.py index 8fd8ba0a7..b4463df19 100644 --- a/tests/cli/test_parser.py +++ b/tests/cli/test_parser.py @@ -44,6 +44,7 @@ EXPECTED_ARGS_CONTAINER_INIT = { **EXPECTED_ARGS, "platforms_result": None, + "remote_sources_version_result": None, } EXPECTED_ARGS_JOB = { "quiet": False, @@ -162,7 +163,13 @@ def test_parse_args_version(capsys): ( ["task", *REQUIRED_COMMON_ARGS, "binary-container-init", "--platforms-result=platforms_file"], - {**EXPECTED_ARGS, "platforms_result": "platforms_file", + {**EXPECTED_ARGS_CONTAINER_INIT, "platforms_result": "platforms_file", + "func": task.binary_container_init}, + ), + ( + ["task", *REQUIRED_COMMON_ARGS, "binary-container-init", + "--remote-sources-version-result=version_file"], + {**EXPECTED_ARGS_CONTAINER_INIT, "remote_sources_version_result": "version_file", "func": task.binary_container_init}, ), ( diff --git a/tests/plugins/test_check_user_settings.py b/tests/plugins/test_check_user_settings.py index 39d700c18..39c921602 100644 --- a/tests/plugins/test_check_user_settings.py +++ b/tests/plugins/test_check_user_settings.py @@ -11,6 +11,7 @@ import pytest from flexmock import flexmock +import yaml from atomic_reactor.plugin import PluginFailedException from atomic_reactor.plugins.check_user_settings import CheckUserSettingsPlugin @@ -59,6 +60,11 @@ def mock_dockerfile_multistage(source_dir: Path, labels, from_scratch=False): ) +def mock_reactor_config(workflow, data=None): + config = yaml.safe_load(data) + workflow.conf.conf = config + + class FakeSource(StubSource): """Fake source for config files validation""" @@ -278,3 +284,68 @@ def test_catch_invalid_content_sets(self, workflow, source_dir): runner = mock_env(workflow, source_dir) with pytest.raises(PluginFailedException, match="validating 'pattern' has failed"): runner.run() + + +class TestRemoteSourceVersion(object): + """Test resolve_remote_sources_version""" + + def test_return_default_version_unconfigured(self, workflow, source_dir, tmpdir): + """Nothing in reactor config nor container yaml""" + result_file = tmpdir / "result.txt" + runner = mock_env(workflow, source_dir) + + workflow.remote_sources_version_result = str(result_file) + runner.run() + + with open(result_file, "r") as f: + assert f.read() == "1" + + def test_return_default_version_configured(self, workflow, source_dir, tmpdir): + """Default version configured in reactor config""" + result_file = tmpdir / "result.txt" + runner = mock_env(workflow, source_dir) + + workflow.remote_sources_version_result = str(result_file) + + mock_reactor_config( + workflow, + dedent(""" + --- + # 30 not valid version, for testing purposes + remote_sources_default_version: 30 + """)) + runner.run() + + with open(result_file, "r") as f: + assert f.read() == "30" + + def test_return_container_yaml_version(self, workflow, source_dir, tmpdir): + """Version explicitly specified by user""" + result_file = tmpdir / "result.txt" + runner = mock_env(workflow, source_dir) + + workflow.source.config.remote_sources_version = 40 # invalid version, for testing purposes + workflow.remote_sources_version_result = str(result_file) + runner.run() + + with open(result_file, "r") as f: + assert f.read() == "40" + + def test_return_container_yaml_version_priority(self, workflow, source_dir, tmpdir): + """Version explicitly specified by user has priority over reactor config""" + result_file = tmpdir / "result.txt" + runner = mock_env(workflow, source_dir) + + mock_reactor_config( + workflow, + dedent(""" + --- + # 50 not valid version, for testing purposes + remote_sources_default_version: 50 + """)) + workflow.source.config.remote_sources_version = 51 # invalid version, for testing purposes + workflow.remote_sources_version_result = str(result_file) + runner.run() + + with open(result_file, "r") as f: + assert f.read() == "51" diff --git a/tests/stubs.py b/tests/stubs.py index 3c9a9d087..a231fccb5 100644 --- a/tests/stubs.py +++ b/tests/stubs.py @@ -12,6 +12,7 @@ class StubConfig(object): image_build_method = None release_env_var = None + remote_sources_version = None class StubSource(object): diff --git a/tests/tasks/test_plugin_based.py b/tests/tasks/test_plugin_based.py index 1cd599468..f41652059 100644 --- a/tests/tasks/test_plugin_based.py +++ b/tests/tasks/test_plugin_based.py @@ -208,7 +208,9 @@ def test_ensure_workflow_data_is_saved_init_task( pipeline_run_name='test-pipeline-run', user_params={}, task_result='results', - platforms_result='platforms_result') + platforms_result='platforms_result', + remote_sources_version_result='version_result', + ) (flexmock(params) .should_receive("source") .and_return(dummy_source)) diff --git a/tests/test_config.py b/tests/test_config.py index 652c104ea..07404c7a7 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1469,6 +1469,19 @@ def test_update_dockerfile_images_from_config(self, tmp_path, images_exist, orga else: assert not dockerfile_images + @pytest.mark.parametrize('param', [ + ("", 1), # default + ("remote_sources_default_version: 2", 2), + ]) + def test_remote_sources_default_version(self, param): + config, expected = param + config += "\n" + REQUIRED_CONFIG + + config_json = read_yaml(config, 'schemas/config.json') + conf = Configuration(raw_config=config_json) + + assert conf.remote_sources_default_version == expected + def test_ensure_odcsconfig_does_not_modify_original_signing_intents(): signing_intents = [{'name': 'release', 'keys': ['R123', 'R234']}] diff --git a/tests/test_source.py b/tests/test_source.py index a69b63b1c..0815ae5e2 100644 --- a/tests/test_source.py +++ b/tests/test_source.py @@ -255,6 +255,11 @@ def _create_source_config(self, tmpdir, yml_config): 'packages': {'npm': [{'path': 'client'}, {'path': 'proxy'}]}, }} + ), ( + """\ + remote_sources_version: 2 + """, + {"remote_sources_version": 2} ), ( """\ operator_manifests: @@ -389,6 +394,10 @@ def test_valid_source_config(self, tmpdir, yml_config, attrs_updated): - module: example.com/go/package """, + """\ + remote_sources_version: 500 # no such version + """, + """\ operator_manifests: {} """, From 614670f72dd25048de750fbc020a880bfa7fd9f2 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Fri, 22 Nov 2024 15:25:53 +0100 Subject: [PATCH 19/29] fix(init task): fix checkton issues Fixing issues reported by shellcheck via checkton. Signed-off-by: Martin Basti --- tekton/tasks/binary-container-init.yaml | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/tekton/tasks/binary-container-init.yaml b/tekton/tasks/binary-container-init.yaml index f067ee4e8..296795ea2 100644 --- a/tekton/tasks/binary-container-init.yaml +++ b/tekton/tasks/binary-container-init.yaml @@ -38,6 +38,9 @@ spec: - name: binary-container-init image: $(params.osbs-image) workingDir: $(workspaces.ws-home-dir.path) + env: + - name: USER_PARAMS + value: $(params.user-params) resources: requests: memory: 512Mi @@ -48,12 +51,12 @@ spec: script: | set -x atomic-reactor -v task \ - --user-params='$(params.user-params)' \ - --build-dir=$(workspaces.ws-build-dir.path) \ - --context-dir=$(workspaces.ws-context-dir.path) \ - --config-file=$(workspaces.ws-reactor-config-map.path)/config.yaml \ - --namespace=$(context.taskRun.namespace) \ + --user-params="${USER_PARAMS}" \ + --build-dir="$(workspaces.ws-build-dir.path)" \ + --context-dir="$(workspaces.ws-context-dir.path)" \ + --config-file="$(workspaces.ws-reactor-config-map.path)/config.yaml" \ + --namespace="$(context.taskRun.namespace)" \ --pipeline-run-name="$(params.pipeline-run-name)" \ binary-container-init \ - --platforms-result=$(results.platforms_result.path) \ - --remote-sources-version-result="$(results.remote_sources_version_result.path)" + --platforms-result="$(results.platforms_result.path)" \ + --remote-sources-version-result="$(results.remote_sources_version_result.path)" \ No newline at end of file From 2b7138aae18c75eb71dbfd27f832cb862517e40c Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Mon, 18 Nov 2024 23:11:36 +0100 Subject: [PATCH 20/29] Do not merge into main: allow to run task from feature branch Remove before merging into main Signed-off-by: Martin Basti --- tekton/pipelines/binary-container.yaml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tekton/pipelines/binary-container.yaml b/tekton/pipelines/binary-container.yaml index 949efee48..bc2614c2b 100644 --- a/tekton/pipelines/binary-container.yaml +++ b/tekton/pipelines/binary-container.yaml @@ -153,7 +153,14 @@ spec: runAfter: - binary-container-init taskRef: - name: binary-container-cachi2-0-1 + resolver: git + params: + - name: url + value: https://github.com/containerbuildsystem/atomic-reactor.git + - name: revision + value: feature_cachi2 + - name: pathInRepo + value: tekton/tasks/binary-container-cachi2.yaml workspaces: - name: ws-build-dir workspace: ws-container From 9aa49c480a884a88d29955fd7675cbb5146db66e Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Tue, 26 Nov 2024 21:18:20 +0100 Subject: [PATCH 21/29] fix(cachi2): single remote source name When single remote source is used, returned name should be None and not an empty string. This si compatible with Cachito behavior. Signed-off-by: Martin Basti --- atomic_reactor/plugins/cachi2_init.py | 2 +- tests/plugins/test_cachi2_init.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/atomic_reactor/plugins/cachi2_init.py b/atomic_reactor/plugins/cachi2_init.py index 2d2a13fa2..5cc99dc62 100644 --- a/atomic_reactor/plugins/cachi2_init.py +++ b/atomic_reactor/plugins/cachi2_init.py @@ -78,7 +78,7 @@ def process_remote_sources(self) -> List[Dict[str, Any]]: remote_sources = self.multiple_remote_sources_params if self.single_remote_source_params: remote_sources = [{ - "name": "", # name single remote source with generic name + "name": None, "remote_source": self.single_remote_source_params}] processed_remote_sources = [] diff --git a/tests/plugins/test_cachi2_init.py b/tests/plugins/test_cachi2_init.py index 1a305d765..0e5e8a9d7 100644 --- a/tests/plugins/test_cachi2_init.py +++ b/tests/plugins/test_cachi2_init.py @@ -133,7 +133,7 @@ def test_single_remote_source_initialization(workflow, mocked_cachi2_init): '/remote-source') assert result == [{ - "name": "", + "name": None, "source_path": str( workflow.build_dir.path / CACHI2_BUILD_DIR / CACHI2_SINGLE_REMOTE_SOURCE_NAME), "remote_source": { From 7055df81af73216b82ba800b30489f9053e0fa2d Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Mon, 25 Nov 2024 20:08:16 +0100 Subject: [PATCH 22/29] cachi2: support clone only mode When users specify empty list as pkg_managers, only cloning should be done. In this case usage of cachi2 must be skipped and required files like env.json and bom.json must be created by OSBS. Signed-off-by: Martin Basti --- atomic_reactor/constants.py | 2 + atomic_reactor/plugins/cachi2_init.py | 44 ++++++++++++++++---- atomic_reactor/utils/cachi2.py | 11 +++++ tekton/tasks/binary-container-cachi2.yaml | 38 +++++++++++------- tests/plugins/test_cachi2_init.py | 49 +++++++++++++++++++++++ tests/utils/test_cachi2.py | 34 ++++++++++++++++ 6 files changed, 156 insertions(+), 22 deletions(-) diff --git a/atomic_reactor/constants.py b/atomic_reactor/constants.py index 61c96ba6b..7ae80148e 100644 --- a/atomic_reactor/constants.py +++ b/atomic_reactor/constants.py @@ -202,9 +202,11 @@ # Cachi2 constants CACHI2_BUILD_DIR = "_cachi2_remote_sources" CACHI2_BUILD_APP_DIR = "app" +CACHI2_ENV_JSON = "cachi2.env.json" CACHI2_PKG_OPTIONS_FILE = "cachi2_pkg_options.json" CACHI2_FOR_OUTPUT_DIR_OPT_FILE = "cachi2_for_output_dir_opt.txt" CACHI2_SINGLE_REMOTE_SOURCE_NAME = "remote-source" +CACHI2_SBOM_JSON = "bom.json" # koji osbs_build metadata KOJI_KIND_IMAGE_BUILD = 'container_build' diff --git a/atomic_reactor/plugins/cachi2_init.py b/atomic_reactor/plugins/cachi2_init.py index 5cc99dc62..485c8c23b 100644 --- a/atomic_reactor/plugins/cachi2_init.py +++ b/atomic_reactor/plugins/cachi2_init.py @@ -19,11 +19,13 @@ CACHI2_PKG_OPTIONS_FILE, CACHI2_FOR_OUTPUT_DIR_OPT_FILE, CACHI2_SINGLE_REMOTE_SOURCE_NAME, + CACHI2_SBOM_JSON, + CACHI2_ENV_JSON, REMOTE_SOURCE_DIR, ) from atomic_reactor.plugin import Plugin from atomic_reactor.util import map_to_user_params -from atomic_reactor.utils.cachi2 import remote_source_to_cachi2 +from atomic_reactor.utils.cachi2 import remote_source_to_cachi2, clone_only class Cachi2InitPlugin(Plugin): @@ -72,6 +74,27 @@ def run(self) -> Optional[List[Dict[str, Any]]]: return processed_remote_sources + def write_metadata(self, remote_source_path: Path): + """Step when OSBS only is doing resolution without cachi2. + + Generate and write SBOM and env file for next plugins + """ + # generate empty SBOM + sbom_path = remote_source_path / CACHI2_SBOM_JSON + with open(sbom_path, "w") as f: + json.dump( + { + "bomFormat": "CycloneDX", + "components": [], + }, + f + ) + + # generate empty envs + env_path = remote_source_path / CACHI2_ENV_JSON + with open(env_path, "w") as f: + json.dump([], f) + def process_remote_sources(self) -> List[Dict[str, Any]]: """Process remote source requests and return information about the processed sources.""" @@ -97,13 +120,6 @@ def process_remote_sources(self) -> List[Dict[str, Any]]: remote_source_data = remote_source["remote_source"] - self.write_cachi2_pkg_options( - remote_source_data, - source_path / CACHI2_PKG_OPTIONS_FILE) - self.write_cachi2_for_output_dir( - source_name, - source_path / CACHI2_FOR_OUTPUT_DIR_OPT_FILE) - source_path_app = source_path / CACHI2_BUILD_APP_DIR source_path_app.mkdir() @@ -113,6 +129,18 @@ def process_remote_sources(self) -> List[Dict[str, Any]]: remote_source_data["ref"] ) + if clone_only(remote_source_data): + # OSBS is doing all work here + self.write_metadata(source_path) + else: + # write cachi2 files only when cachi2 should run + self.write_cachi2_pkg_options( + remote_source_data, + source_path / CACHI2_PKG_OPTIONS_FILE) + self.write_cachi2_for_output_dir( + source_name, + source_path / CACHI2_FOR_OUTPUT_DIR_OPT_FILE) + processed_remote_sources.append({ "source_path": str(source_path), **remote_source, diff --git a/atomic_reactor/utils/cachi2.py b/atomic_reactor/utils/cachi2.py index 43740b045..8f534dc7b 100644 --- a/atomic_reactor/utils/cachi2.py +++ b/atomic_reactor/utils/cachi2.py @@ -161,3 +161,14 @@ def generate_request_json( "packages": [], # this will be always empty cachi2 doesn't provide nested deps } return res + + +def clone_only(remote_source: Dict[str, Any]) -> bool: + """Determine if only cloning is required without cachi2 run""" + + pkg_managers = remote_source.get("pkg_managers") + + if pkg_managers is not None and len(pkg_managers) == 0: + return True + + return False diff --git a/tekton/tasks/binary-container-cachi2.yaml b/tekton/tasks/binary-container-cachi2.yaml index 5bb865479..e00e80728 100644 --- a/tekton/tasks/binary-container-cachi2.yaml +++ b/tekton/tasks/binary-container-cachi2.yaml @@ -77,6 +77,7 @@ spec: #!/usr/bin/bash set -eux CACHI2_DIR="$(workspaces.ws-build-dir.path)/_cachi2_remote_sources" + CACHI2_PKG_OPT_PATH="cachi2_pkg_options.json" if [ ! -d "$CACHI2_DIR" ]; then echo "Skipping step: remote sources not found" @@ -91,27 +92,36 @@ spec: pushd "${REMOTE_SOURCE_PATH}" - FOR_OUTPUT_DIR="$(cat cachi2_for_output_dir_opt.txt)" - - cachi2 --log-level="$LOG_LEVEL" fetch-deps \ - --source="${REMOTE_SOURCE_PATH}/app/" \ - --output="${REMOTE_SOURCE_PATH}" \ - "$(cat cachi2_pkg_options.json)" - + if [ -f "${CACHI2_PKG_OPT_PATH}" ]; then + # only presence of $CACHI2_PKG_OPT_PATH config file means that atomic-reactor wants to run cachi2 + FOR_OUTPUT_DIR="$(cat cachi2_for_output_dir_opt.txt)" + + cachi2 --log-level="$LOG_LEVEL" fetch-deps \ + --source="${REMOTE_SOURCE_PATH}/app/" \ + --output="${REMOTE_SOURCE_PATH}" \ + "$(cat "${CACHI2_PKG_OPT_PATH}")" + + cachi2 --log-level="$LOG_LEVEL" generate-env "${REMOTE_SOURCE_PATH}" \ + --format json \ + --for-output-dir="${FOR_OUTPUT_DIR}" \ + --output "${REMOTE_SOURCE_PATH}/cachi2.env.json" + else + mkdir deps/ # create empty deps/ dir to emulate cachi2 run + fi + + # if SBOM is not generated by cachi2, it's generated by atomic-rector, + # we can rely on presence of this file SBOMS+=("${REMOTE_SOURCE_PATH}/bom.json") - cachi2 --log-level="$LOG_LEVEL" generate-env "${REMOTE_SOURCE_PATH}" \ - --format json \ - --for-output-dir="${FOR_OUTPUT_DIR}" \ - --output "${REMOTE_SOURCE_PATH}/cachi2.env.json" - rm -fr app/.git/ # remove git directory # create source archive before injecting files tar -czf remote-source.tar.gz app/ deps/ - cachi2 --log-level="$LOG_LEVEL" inject-files "${REMOTE_SOURCE_PATH}" \ - --for-output-dir="${FOR_OUTPUT_DIR}" + if [ -f "${CACHI2_PKG_OPT_PATH}" ]; then + cachi2 --log-level="$LOG_LEVEL" inject-files "${REMOTE_SOURCE_PATH}" \ + --for-output-dir="${FOR_OUTPUT_DIR}" + fi popd done diff --git a/tests/plugins/test_cachi2_init.py b/tests/plugins/test_cachi2_init.py index 0e5e8a9d7..4cfb192fa 100644 --- a/tests/plugins/test_cachi2_init.py +++ b/tests/plugins/test_cachi2_init.py @@ -21,6 +21,8 @@ CACHI2_BUILD_APP_DIR, CACHI2_FOR_OUTPUT_DIR_OPT_FILE, CACHI2_PKG_OPTIONS_FILE, + CACHI2_ENV_JSON, + CACHI2_SBOM_JSON, ) from atomic_reactor.inner import DockerBuildWorkflow @@ -143,6 +145,53 @@ def test_single_remote_source_initialization(workflow, mocked_cachi2_init): }] +def test_empty_list_pkg_managers(workflow, mocked_cachi2_init): + """Test if when empty_list of pkg managers is specified, + sbom and env-var are generated and cachi2 config skipped""" + first_remote_source_name = "first" + + remote_source_config = dedent( + f"""\ + remote_sources: + - name: {first_remote_source_name} + remote_source: + repo: {REMOTE_SOURCE_REPO} + ref: {REMOTE_SOURCE_REF} + pkg_managers: [] + """ + ) + + mock_repo_config(workflow, remote_source_config) + + reactor_config = dedent("""\ + allow_multiple_remote_sources: True + """) + mock_reactor_config(workflow, reactor_config) + + result = mocked_cachi2_init(workflow).run() + assert result == [{ + "name": first_remote_source_name, + "source_path": str(workflow.build_dir.path / CACHI2_BUILD_DIR / first_remote_source_name), + "remote_source": { + "repo": REMOTE_SOURCE_REPO, + "ref": REMOTE_SOURCE_REF, + "pkg_managers": [], + } + }] + + source_path = workflow.build_dir.path / CACHI2_BUILD_DIR / first_remote_source_name + + # test if bom and env have been created + assert (source_path / CACHI2_SBOM_JSON).is_file() + assert (source_path / CACHI2_ENV_JSON).is_file() + + # test if clone happened + assert (source_path / CACHI2_BUILD_APP_DIR / MOCKED_CLONE_FILE).is_file() + + # test that cachi2 pkg config file hasn't been generated + assert not (source_path / CACHI2_PKG_OPTIONS_FILE).is_file() + + def test_multi_remote_source_initialization(workflow, mocked_cachi2_init): """Tests initialization or repos for multiple remote sources""" diff --git a/tests/utils/test_cachi2.py b/tests/utils/test_cachi2.py index 166f93676..cf232f54b 100644 --- a/tests/utils/test_cachi2.py +++ b/tests/utils/test_cachi2.py @@ -11,6 +11,7 @@ remote_source_to_cachi2, gen_dependency_from_sbom_component, generate_request_json, + clone_only, ) import pytest @@ -436,3 +437,36 @@ def test_generate_request_json(): assert generate_request_json( remote_source, remote_source_sbom, remote_source_env_json ) == expected + + +@pytest.mark.parametrize('remote_source,expected', [ + pytest.param( + { + "pkg_managers": [] + }, + True, + id="empty_list" + ), + pytest.param( + { + "pkg_managers": ["gomod"] + }, + False, + id="gomod" + ), + pytest.param( + {}, + False, + id="undefined" + ), + pytest.param( + { + "pkg_managers": None + }, + False, + id="explicit_none" + ), +]) +def test_clone_only(remote_source, expected): + """Test if clone_only is evaluate correctly only from empty list of pkg_managers""" + assert clone_only(remote_source) == expected From 3986cdc9b05739b7d7a45b663a4640a58110f2b8 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Thu, 28 Nov 2024 14:30:56 +0100 Subject: [PATCH 23/29] cachi2: bump osbs-client This bump is required to validate allowed pkg_managers also on atomic-reactor side. Signed-off-by: Martin Basti --- requirements.in | 2 +- requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements.in b/requirements.in index 9327fafdb..2377aab21 100644 --- a/requirements.in +++ b/requirements.in @@ -6,7 +6,7 @@ jsonschema paramiko>=3.4.0 PyYAML ruamel.yaml -git+https://github.com/containerbuildsystem/osbs-client@541387019135f2bb6fc8054a453641f0e9feb3d7#egg=osbs-client +git+https://github.com/containerbuildsystem/osbs-client@b59210140d04060640e660c7cdee0ba56648e372#egg=osbs-client # cannot build cryptography with rust for version >= 3.5 cryptography < 3.5 requests diff --git a/requirements.txt b/requirements.txt index d4fd230a8..3a7df5321 100644 --- a/requirements.txt +++ b/requirements.txt @@ -111,7 +111,7 @@ opentelemetry-semantic-conventions==0.41b0 # opentelemetry-sdk opentelemetry-util-http==0.41b0 # via opentelemetry-instrumentation-requests -osbs-client @ git+https://github.com/containerbuildsystem/osbs-client@541387019135f2bb6fc8054a453641f0e9feb3d7 +osbs-client @ git+https://github.com/containerbuildsystem/osbs-client@b59210140d04060640e660c7cdee0ba56648e372 # via -r requirements.in otel-extensions==0.2.5 # via -r otel-requirements.in From c8abb4e839078573ea34dfea65eeee80f5a45489 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Fri, 29 Nov 2024 14:25:47 +0100 Subject: [PATCH 24/29] fix(cachi2): copy all files generated by cachi2 Rubygems package manager contains data also out of deps/ and app/ directory. The whole remote dir must be copied. For rubygems, bundler config file hasn't been copied. Signed-off-by: Martin Basti --- atomic_reactor/plugins/cachi2_postprocess.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/atomic_reactor/plugins/cachi2_postprocess.py b/atomic_reactor/plugins/cachi2_postprocess.py index 4f18e1a82..e7f5faa73 100644 --- a/atomic_reactor/plugins/cachi2_postprocess.py +++ b/atomic_reactor/plugins/cachi2_postprocess.py @@ -186,11 +186,8 @@ def inject_into_build_dir( "copy method used for cachi2 build_dir_injecting: %s", copy_method.__name__) copytree( - remote_source.sources_path/'app', dest_dir/'app', - symlinks=True, copy_function=copy_method) - copytree( - remote_source.sources_path/'deps', dest_dir/'deps', - symlinks=True, copy_function=copy_method) + remote_source.sources_path, dest_dir, + symlinks=True, copy_function=copy_method, dirs_exist_ok=True) # Create cachito.env file with environment variables received from cachito request self.generate_cachito_env_file(dest_dir, remote_source.build_args) From d91453c6d8b1db59f190031b092b46d1730a04fe Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Tue, 3 Dec 2024 15:01:24 +0100 Subject: [PATCH 25/29] fix(request.json): version with subpath We cannot realibly determine if dependency was vendored from SBOM metdata. Keep it safe and export version including git od download URL with subpath. Signed-off-by: Martin Basti --- atomic_reactor/utils/cachi2.py | 11 +++++++++-- tests/utils/test_cachi2.py | 19 +++++++++++++++++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/atomic_reactor/utils/cachi2.py b/atomic_reactor/utils/cachi2.py index 8f534dc7b..c2c7d943b 100644 --- a/atomic_reactor/utils/cachi2.py +++ b/atomic_reactor/utils/cachi2.py @@ -110,18 +110,25 @@ def gen_dependency_from_sbom_component(sbom_dep: Dict[str, Any]) -> Dict[str, Op heuristic_type = request_type break + pkg_dot_path = ("golang", "gem") + version = ( # for non-registry dependencies cachito uses URL as version purl.qualifiers.get("vcs_url") or purl.qualifiers.get("download_url") or # for local dependencies Cachito uses path as version - (f"./{purl.subpath}" if purl.subpath and purl.type == "golang" else None) or - (f"file:{purl.subpath}" if purl.subpath and purl.type != "golang" else None) or + (f"./{purl.subpath}" if purl.subpath and purl.type in pkg_dot_path else None) or + (f"file:{purl.subpath}" if purl.subpath and purl.type not in pkg_dot_path else None) or # version is mainly for dependencies from pkg registries sbom_dep.get("version") # returns None if version cannot be determined ) + if version and purl.subpath and not version.endswith(purl.subpath): + # include subpath into vcs or download url to get exact location of dependency + # used mainly for vendored deps + version = f"{version}#{purl.subpath}" + res = { "name": sbom_dep["name"], "replaces": None, # it's always None, replacements aren't supported by cachi2 diff --git a/tests/utils/test_cachi2.py b/tests/utils/test_cachi2.py index cf232f54b..f8d59ca2a 100644 --- a/tests/utils/test_cachi2.py +++ b/tests/utils/test_cachi2.py @@ -291,7 +291,7 @@ def test_convert_SBOM_to_ICM(sbom, expected_icm): "type": "npm", "version": ( "git+https://github.com/cachito-testing/cachito-npm-without-deps.git@" - "2f0ce1d7b1f8b35572d919428b965285a69583f6"), + "2f0ce1d7b1f8b35572d919428b965285a69583f6#path"), }, id="version_vsc_url" ), @@ -308,7 +308,7 @@ def test_convert_SBOM_to_ICM(sbom, expected_icm): "name": "cachito-npm-without-deps", "replaces": None, "type": "npm", - "version": "https://example.com/pkg", + "version": "https://example.com/pkg#path", }, id="version_download_url" ), @@ -378,6 +378,21 @@ def test_convert_SBOM_to_ICM(sbom, expected_icm): }, id="npm_dev" ), + pytest.param( + { + "name": "validate_url", + "version": "1.0.5", + "purl": "pkg:gem/validate_url#subpath", + "type": "library" + }, + { + "name": "validate_url", + "replaces": None, + "type": "rubygems", + "version": "./subpath" + }, + id="type_rubygem_subpath_only" + ), ]) def test_gen_dependency_from_sbom_component(sbom_comp, expected): """Test generating request.json dependency from sbom component""" From 42462700fd79b68962fa0b4db86ca5f0c347415b Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Thu, 5 Dec 2024 19:27:17 +0100 Subject: [PATCH 26/29] cachi2: allow only relative paths For security reasons, only relative paths within cloned remote source can be specified by users Don't allow to point to symlink pointing out of cloned remote source Signed-off-by: Martin Basti --- atomic_reactor/plugins/cachi2_init.py | 4 +- atomic_reactor/utils/cachi2.py | 44 +++++++++++++ tests/plugins/test_cachi2_init.py | 28 ++++++++ tests/utils/test_cachi2.py | 95 +++++++++++++++++++++++++++ 4 files changed, 170 insertions(+), 1 deletion(-) diff --git a/atomic_reactor/plugins/cachi2_init.py b/atomic_reactor/plugins/cachi2_init.py index 485c8c23b..ad09145ba 100644 --- a/atomic_reactor/plugins/cachi2_init.py +++ b/atomic_reactor/plugins/cachi2_init.py @@ -25,7 +25,7 @@ ) from atomic_reactor.plugin import Plugin from atomic_reactor.util import map_to_user_params -from atomic_reactor.utils.cachi2 import remote_source_to_cachi2, clone_only +from atomic_reactor.utils.cachi2 import remote_source_to_cachi2, clone_only, validate_paths class Cachi2InitPlugin(Plugin): @@ -129,6 +129,8 @@ def process_remote_sources(self) -> List[Dict[str, Any]]: remote_source_data["ref"] ) + validate_paths(source_path_app, remote_source_data.get("packages", {})) + if clone_only(remote_source_data): # OSBS is doing all work here self.write_metadata(source_path) diff --git a/atomic_reactor/utils/cachi2.py b/atomic_reactor/utils/cachi2.py index c2c7d943b..fe11a791c 100644 --- a/atomic_reactor/utils/cachi2.py +++ b/atomic_reactor/utils/cachi2.py @@ -9,10 +9,54 @@ """ from typing import Any, Callable, Dict, Optional, Tuple, List +from pathlib import Path +import os.path from packageurl import PackageURL +def validate_paths(repo_path: Path, remote_sources_packages: dict) -> None: + """Paths must be relative and within cloned repo""" + def is_path_ok(path_string): + path = Path(path_string) + if path.is_absolute(): + return False + + # using real repo to properly resolve and block bad symlinks + full_path = (repo_path/path).resolve() + + # using commonpath to be compatible with py3.8 + if os.path.commonpath((full_path, repo_path)) != str(repo_path): + return False + + return True + + for pkg_mgr, options in remote_sources_packages.items(): + if not options: + continue + + for option in options: + for key, val in option.items(): + if key == "path": + if not is_path_ok(val): + raise ValueError( + f"{pkg_mgr}:{key}: path '{val}' must be relative " + "within remote source repository" + ) + elif ( + pkg_mgr == "pip" and + key in ("requirements_files", "requirements_build_files") + ): + for v in val: + if not is_path_ok(v): + raise ValueError( + f"{pkg_mgr}:{key}: path '{v}' must be relative " + "within remote source repository" + ) + else: + raise ValueError(f"unexpected key '{key}' in '{pkg_mgr}' config") + + def remote_source_to_cachi2(remote_source: Dict[str, Any]) -> Dict[str, Any]: """Converts remote source into cachi2 expected params. diff --git a/tests/plugins/test_cachi2_init.py b/tests/plugins/test_cachi2_init.py index 4cfb192fa..e2dcbba4b 100644 --- a/tests/plugins/test_cachi2_init.py +++ b/tests/plugins/test_cachi2_init.py @@ -335,6 +335,34 @@ def test_multiple_remote_sources_non_unique_names(workflow): assert result is None +def test_path_out_of_repo(workflow, mocked_cachi2_init): + """Should fail when path is outside of repository""" + container_yaml_config = dedent("""\ + remote_sources: + - name: bit-different + remote_source: + repo: https://git.example.com/team/repo.git + ref: a55c00f45ec3dfee0c766cea3d395d6e21cc2e5a + packages: + gomod: + - path: "/out/of/repo" + """) + mock_repo_config(workflow, data=container_yaml_config) + + reactor_config = dedent("""\ + allow_multiple_remote_sources: True + """) + mock_reactor_config(workflow, reactor_config) + + err_msg = ( + "gomod:path: path '/out/of/repo' must be relative within remote source repository" + ) + with pytest.raises(ValueError) as exc_info: + mocked_cachi2_init(workflow).run() + + assert err_msg in str(exc_info) + + def test_dependency_replacements(workflow): run_plugin_with_args(workflow, dependency_replacements={"dep": "something"}, expect_error="Dependency replacements are not supported by Cachi2") diff --git a/tests/utils/test_cachi2.py b/tests/utils/test_cachi2.py index f8d59ca2a..67b4531b1 100644 --- a/tests/utils/test_cachi2.py +++ b/tests/utils/test_cachi2.py @@ -6,12 +6,15 @@ of the BSD license. See the LICENSE file for details. """ +from pathlib import Path + from atomic_reactor.utils.cachi2 import ( convert_SBOM_to_ICM, remote_source_to_cachi2, gen_dependency_from_sbom_component, generate_request_json, clone_only, + validate_paths, ) import pytest @@ -106,6 +109,98 @@ def test_remote_source_to_cachi2_conversion(input_remote_source, expected_cachi2 assert remote_source_to_cachi2(input_remote_source) == expected_cachi2 +@pytest.mark.parametrize("remote_source_packages,expected_err", [ + pytest.param( + {"gomod": [{"path": "/path/to/secret"}]}, + "gomod:path: path '/path/to/secret' must be relative within remote source repository", + id="absolute_path" + ), + pytest.param( + {"gomod": [{"unknown": "/path/to/secret"}]}, + "unexpected key 'unknown' in 'gomod' config", + id="unknown_option" + ), + pytest.param( + {"gomod": [{"path": "path/../../../to/secret"}]}, + ( + "gomod:path: path 'path/../../../to/secret' must be relative " + "within remote source repository" + ), + id="path_traversal" + ), + pytest.param( + { + "pip": [{ + "path": "/src/web", + }] + }, + "pip:path: path '/src/web' must be relative within remote source repository", + id="pip_absolute_path" + ), + pytest.param( + { + "pip": [{ + "requirements_files": ["requirements.txt", "/requirements-extras.txt"], + }] + }, + ( + "pip:requirements_files: path '/requirements-extras.txt' " + "must be relative within remote source repository" + ), + id="pip_requirements_files_absolute_path" + ), + pytest.param( + { + "pip": [{ + "requirements_build_files": [ + "requirements-build.txt", "/requirements-build-extras.txt" + ] + }] + }, + ( + "pip:requirements_build_files: path '/requirements-build-extras.txt' " + "must be relative within remote source repository" + ), + id="pip_requirements_build_files_absolute_path" + ) +]) +def test_remote_source_invalid_paths(tmpdir, remote_source_packages, expected_err): + """Test conversion of remote_source (cachito) configuration from container yaml + into cachi2 params""" + with pytest.raises(ValueError) as exc_info: + validate_paths(Path(tmpdir), remote_source_packages) + + assert str(exc_info.value) == expected_err + + +def test_remote_source_path_to_symlink_out_of_repo(tmpdir): + """If cloned repo contains symlink that points out fo repo, + path in packages shouldn't be allowed""" + tmpdir_path = Path(tmpdir) + + symlink_target = tmpdir_path/"dir_outside" + symlink_target.mkdir() + + cloned = tmpdir_path/'app' + cloned.mkdir() + + symlink = cloned/"symlink" + symlink.symlink_to(symlink_target, target_is_directory=True) + + remote_source_packages = { + "pip": [{ + "path": "symlink", + }] + } + + expected_err = "pip:path: path 'symlink' must be relative within remote source repository" + + with pytest.raises(ValueError) as exc_info: + validate_paths(cloned, remote_source_packages) + + assert str(exc_info.value) == expected_err + + @pytest.mark.parametrize(('sbom', 'expected_icm'), [ pytest.param( { From 9f2ab9e6b09e4fb52d2006d4da137ef8d9861514 Mon Sep 17 00:00:00 2001 From: Martin Basti Date: Wed, 11 Dec 2024 19:10:26 +0100 Subject: [PATCH 27/29] fix(cachi2): set gomod explicitly undefined pkg_managers means gomod, record this explictily in metadata, so this information is not lost in workflow and proper metadata are returned. Signed-off-by: Martin Basti --- atomic_reactor/plugins/cachi2_init.py | 8 +++++++- atomic_reactor/utils/cachi2.py | 20 ++++++++++++++++---- tests/plugins/test_cachi2_init.py | 2 ++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/atomic_reactor/plugins/cachi2_init.py b/atomic_reactor/plugins/cachi2_init.py index ad09145ba..26bebf858 100644 --- a/atomic_reactor/plugins/cachi2_init.py +++ b/atomic_reactor/plugins/cachi2_init.py @@ -25,7 +25,10 @@ ) from atomic_reactor.plugin import Plugin from atomic_reactor.util import map_to_user_params -from atomic_reactor.utils.cachi2 import remote_source_to_cachi2, clone_only, validate_paths +from atomic_reactor.utils.cachi2 import ( + remote_source_to_cachi2, clone_only, validate_paths, + normalize_gomod_pkg_manager +) class Cachi2InitPlugin(Plugin): @@ -114,6 +117,9 @@ def process_remote_sources(self) -> List[Dict[str, Any]]: remote_source["name"] if self.multiple_remote_sources_params else CACHI2_SINGLE_REMOTE_SOURCE_NAME ) + + normalize_gomod_pkg_manager(remote_source['remote_source']) + self.log.info("Initializing remote source %s", source_name) source_path = self.remote_sources_root_path / source_name source_path.mkdir() diff --git a/atomic_reactor/utils/cachi2.py b/atomic_reactor/utils/cachi2.py index fe11a791c..3a829560f 100644 --- a/atomic_reactor/utils/cachi2.py +++ b/atomic_reactor/utils/cachi2.py @@ -57,6 +57,19 @@ def is_path_ok(path_string): raise ValueError(f"unexpected key '{key}' in '{pkg_mgr}' config") +def normalize_gomod_pkg_manager(remote_source: Dict[str, Any]): + """Cachito compatibility, empty/undefined pkg_managers means gomod. + Replace it to explicitly use gomod + + Function does in-place change. + """ + pkg_managers = remote_source.get("pkg_managers") + if pkg_managers is None: + # Cachito behavior, missing pkg_managers means to use gomod + pkg_managers = ["gomod"] + remote_source["pkg_managers"] = pkg_managers + + def remote_source_to_cachi2(remote_source: Dict[str, Any]) -> Dict[str, Any]: """Converts remote source into cachi2 expected params. @@ -84,10 +97,9 @@ def remote_source_to_cachi2(remote_source: Dict[str, Any]) -> Dict[str, Any]: ) cachi2_packages = [] - pkg_managers = remote_source.get("pkg_managers") - if pkg_managers is None: - # Cachito behavior, missing pkg_managers means to use gomod - pkg_managers = ["gomod"] + normalize_gomod_pkg_manager(remote_source) + + pkg_managers = remote_source["pkg_managers"] for pkg_manager in pkg_managers: if pkg_manager in removed_pkg_managers: diff --git a/tests/plugins/test_cachi2_init.py b/tests/plugins/test_cachi2_init.py index e2dcbba4b..0a70b1978 100644 --- a/tests/plugins/test_cachi2_init.py +++ b/tests/plugins/test_cachi2_init.py @@ -141,6 +141,7 @@ def test_single_remote_source_initialization(workflow, mocked_cachi2_init): "remote_source": { "repo": REMOTE_SOURCE_REPO, "ref": REMOTE_SOURCE_REF, + "pkg_managers": ["gomod"], } }] @@ -254,6 +255,7 @@ def test_multi_remote_source_initialization(workflow, mocked_cachi2_init): "remote_source": { "repo": SECOND_REMOTE_SOURCE_REPO, "ref": SECOND_REMOTE_SOURCE_REF, + "pkg_managers": ["gomod"], } }] From f7064c707a5fae256aa3b2d7c81e824ebc171d59 Mon Sep 17 00:00:00 2001 From: Ben Alkov Date: Tue, 17 Dec 2024 12:34:23 -0500 Subject: [PATCH 28/29] feat(utils): implement symlink sandbox for cachi2 What/why: implement detection/removal of unsafe symlinks in repos, specifically covering cachi2 use case: Cachito already does this How: - copypasta `_enforce_sandbox()` and related unit tests from Cachito ("cachito/cachito/workers/tasks/general.py" and "cachito/tests/test_workers/test_tasks/test_general.py", respectively) - add call to `_enforce_sandbox()` - add CLI boolean arg `remove-unsafe-symlinks`, which toggles removing all symlinks which point to location(s) outside of any cloned repository Signed-off-by: Ben Alkov rh-pre-commit.version: 2.3.2 rh-pre-commit.check-secrets: ENABLED --- atomic_reactor/plugins/cachi2_init.py | 3 +- atomic_reactor/utils/cachi2.py | 52 +++++++++++++ tests/utils/test_cachi2.py | 101 ++++++++++++++++++++++++++ 3 files changed, 155 insertions(+), 1 deletion(-) diff --git a/atomic_reactor/plugins/cachi2_init.py b/atomic_reactor/plugins/cachi2_init.py index 26bebf858..bf6d6b4c3 100644 --- a/atomic_reactor/plugins/cachi2_init.py +++ b/atomic_reactor/plugins/cachi2_init.py @@ -27,7 +27,7 @@ from atomic_reactor.util import map_to_user_params from atomic_reactor.utils.cachi2 import ( remote_source_to_cachi2, clone_only, validate_paths, - normalize_gomod_pkg_manager + normalize_gomod_pkg_manager, enforce_sandbox, ) @@ -135,6 +135,7 @@ def process_remote_sources(self) -> List[Dict[str, Any]]: remote_source_data["ref"] ) + enforce_sandbox(source_path_app, remove_unsafe_symlinks=False) validate_paths(source_path_app, remote_source_data.get("packages", {})) if clone_only(remote_source_data): diff --git a/atomic_reactor/utils/cachi2.py b/atomic_reactor/utils/cachi2.py index 3a829560f..5fdc732f2 100644 --- a/atomic_reactor/utils/cachi2.py +++ b/atomic_reactor/utils/cachi2.py @@ -8,12 +8,64 @@ Utils to help to integrate with cachi2 CLI tool """ +import logging + from typing import Any, Callable, Dict, Optional, Tuple, List from pathlib import Path import os.path from packageurl import PackageURL +logger = logging.getLogger(__name__) + + +class SymlinkSandboxError(Exception): + """Found symlink(s) pointing outside the sandbox.""" + + +def enforce_sandbox(repo_root: Path, remove_unsafe_symlinks: bool) -> None: + """ + Check that there are no symlinks that try to leave the cloned repository. + + :param (str | Path) repo_root: absolute path to root of cloned repository + :raises OsbsValidationException: if any symlink points outside of cloned repository + """ + for path_to_dir, subdirs, files in os.walk(repo_root): + dirpath = Path(path_to_dir) + + for entry in subdirs + files: + # the logic in here actually *requires* f-strings with `!r`. using + # `%r` DOES NOT WORK (tested) + # pylint: disable=logging-fstring-interpolation + + # apparently pylint doesn't understand Path + full_path = dirpath / entry # pylint: disable=old-division + + try: + real_path = full_path.resolve() + except RuntimeError as e: + if "Symlink loop from " in str(e): + logger.info(f"Symlink loop from {full_path!r}") + continue + logger.exception("RuntimeError encountered") + raise + + try: + real_path.relative_to(repo_root) + except ValueError as exc: + # Unlike the real path, the full path is always relative to the root + relative_path = str(full_path.relative_to(repo_root)) + if remove_unsafe_symlinks: + full_path.unlink() + logger.warning( + f"The destination of {relative_path!r} is outside of cloned repository. " + "Removing...", + ) + else: + raise SymlinkSandboxError( + f"The destination of {relative_path!r} is outside of cloned repository", + ) from exc + def validate_paths(repo_path: Path, remote_sources_packages: dict) -> None: """Paths must be relative and within cloned repo""" diff --git a/tests/utils/test_cachi2.py b/tests/utils/test_cachi2.py index 67b4531b1..e11885f36 100644 --- a/tests/utils/test_cachi2.py +++ b/tests/utils/test_cachi2.py @@ -6,10 +6,14 @@ of the BSD license. See the LICENSE file for details. """ +import os from pathlib import Path +from typing import Union from atomic_reactor.utils.cachi2 import ( + SymlinkSandboxError, convert_SBOM_to_ICM, + enforce_sandbox, remote_source_to_cachi2, gen_dependency_from_sbom_component, generate_request_json, @@ -19,6 +23,8 @@ import pytest +from unittest import mock + @pytest.mark.parametrize(('input_remote_source', 'expected_cachi2'), [ pytest.param( @@ -580,3 +586,98 @@ def test_generate_request_json(): def test_clone_only(remote_source, expected): """Test if clone_only is evaluate correctly only from empty list of pkg_managers""" assert clone_only(remote_source) == expected + + +class Symlink(str): + """ + Use this to create symlinks via write_file_tree(). + + The value of a Symlink instance is the target path (path to make a symlink to). + """ + + +def write_file_tree(tree_def: dict, rooted_at: Union[str, Path], *, exist_dirs_ok: bool = False): + """ + Write a file tree to disk. + + :param tree_def: Definition of file tree, see usage for intuitive examples + :param rooted_at: Root of file tree, must be an existing directory + :param exist_dirs_ok: If True, existing directories will not cause this function to fail + """ + root = Path(rooted_at) + for entry, value in tree_def.items(): + entry_path = root / entry + if isinstance(value, Symlink): + os.symlink(value, entry_path) + elif isinstance(value, str): + entry_path.write_text(value) + else: + entry_path.mkdir(exist_ok=exist_dirs_ok) + write_file_tree(value, entry_path) + + +@pytest.mark.parametrize( + "file_tree,bad_symlink", + [ + # good + pytest.param({}, None, id="empty-no-symlink"), + pytest.param({"symlink_to_self": Symlink(".")}, None, id="self-symlink-ok"), + pytest.param( + {"subdir": {"symlink_to_parent": Symlink("..")}}, None, id="parent-symlink-ok" + ), + pytest.param( + {"symlink_to_subdir": Symlink("subdir/some_file"), "subdir": {"some_file": "foo"}}, + None, + id="subdir-symlink-ok", + ), + # bad + pytest.param( + {"symlink_to_parent": Symlink("..")}, "symlink_to_parent", id="parent-symlink-bad" + ), + pytest.param({"symlink_to_root": Symlink("/")}, "symlink_to_root", id="root-symlink-bad"), + pytest.param( + {"subdir": {"symlink_to_parent_parent": Symlink("../..")}}, + "subdir/symlink_to_parent_parent", + id="parent-parent-symlink-bad", + ), + pytest.param( + {"subdir": {"symlink_to_root": Symlink("/")}}, + "subdir/symlink_to_root", + id="subdir-root-symlink-bad", + ), + ], +) +def test_enforce_sandbox(file_tree, bad_symlink, tmp_path): + write_file_tree(file_tree, tmp_path) + if bad_symlink: + error = f"The destination of {bad_symlink!r} is outside of cloned repository" + with pytest.raises(SymlinkSandboxError, match=error): + enforce_sandbox(tmp_path, remove_unsafe_symlinks=False) + assert Path(tmp_path / bad_symlink).exists() + enforce_sandbox(tmp_path, remove_unsafe_symlinks=True) + assert not Path(tmp_path / bad_symlink).exists() + else: + enforce_sandbox(tmp_path, remove_unsafe_symlinks=False) + enforce_sandbox(tmp_path, remove_unsafe_symlinks=True) + + +def test_enforce_sandbox_symlink_loop(tmp_path, caplog): + file_tree = {"foo_b": Symlink("foo_a"), "foo_a": Symlink("foo_b")} + write_file_tree(file_tree, tmp_path) + enforce_sandbox(tmp_path, remove_unsafe_symlinks=True) + assert "Symlink loop from " in caplog.text + + +@mock.patch("pathlib.Path.resolve") +def test_enforce_sandbox_runtime_error(mock_resolve, tmp_path): + error = "RuntimeError is triggered" + + def side_effect(): + raise RuntimeError(error) + + mock_resolve.side_effect = side_effect + + file_tree = {"foo_b": Symlink("foo_a"), "foo_a": Symlink("foo_b")} + write_file_tree(file_tree, tmp_path) + with pytest.raises(RuntimeError, match=error): + enforce_sandbox(tmp_path, remove_unsafe_symlinks=True) From 1438a3124a72a6205cb62a55fb9fffe2808aaa49 Mon Sep 17 00:00:00 2001 From: Ben Alkov Date: Tue, 7 Jan 2025 17:14:05 -0500 Subject: [PATCH 29/29] feat(utils): implement `remove-unsafe-symlinks` flag for cachi2 What/why: implement parsing `remove-unsafe-symlinks` flag, which toggles removing all symlinks which point to location(s) outside of any cloned repository (default `False`) How: - parse flag `remove-unsafe-symlinks` from "remote-source" section of 'container.yaml' Signed-off-by: Ben Alkov --- atomic_reactor/plugins/cachi2_init.py | 10 +++++++++- atomic_reactor/utils/cachi2.py | 4 +++- tests/plugins/test_cachi2_init.py | 25 +++++++++++++++++++++++++ tests/utils/test_cachi2.py | 2 +- 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/atomic_reactor/plugins/cachi2_init.py b/atomic_reactor/plugins/cachi2_init.py index bf6d6b4c3..606d77514 100644 --- a/atomic_reactor/plugins/cachi2_init.py +++ b/atomic_reactor/plugins/cachi2_init.py @@ -135,7 +135,15 @@ def process_remote_sources(self) -> List[Dict[str, Any]]: remote_source_data["ref"] ) - enforce_sandbox(source_path_app, remove_unsafe_symlinks=False) + remove_unsafe_symlinks = False + flags = remote_source_data.get("flags", []) + if "remove_unsafe_symlinks" in flags: + remove_unsafe_symlinks = True + + enforce_sandbox( + source_path_app, + remove_unsafe_symlinks, + ) validate_paths(source_path_app, remote_source_data.get("packages", {})) if clone_only(remote_source_data): diff --git a/atomic_reactor/utils/cachi2.py b/atomic_reactor/utils/cachi2.py index 5fdc732f2..d4ec73b5a 100644 --- a/atomic_reactor/utils/cachi2.py +++ b/atomic_reactor/utils/cachi2.py @@ -10,6 +10,7 @@ import logging +import sys from typing import Any, Callable, Dict, Optional, Tuple, List from pathlib import Path import os.path @@ -23,11 +24,12 @@ class SymlinkSandboxError(Exception): """Found symlink(s) pointing outside the sandbox.""" -def enforce_sandbox(repo_root: Path, remove_unsafe_symlinks: bool) -> None: +def enforce_sandbox(repo_root: Path, remove_unsafe_symlinks: bool = False) -> None: """ Check that there are no symlinks that try to leave the cloned repository. :param (str | Path) repo_root: absolute path to root of cloned repository + :param bool remove_unsafe_symlinks: remove unsafe symlinks if any are found :raises OsbsValidationException: if any symlink points outside of cloned repository """ for path_to_dir, subdirs, files in os.walk(repo_root): diff --git a/tests/plugins/test_cachi2_init.py b/tests/plugins/test_cachi2_init.py index 0a70b1978..c3bd0f6e0 100644 --- a/tests/plugins/test_cachi2_init.py +++ b/tests/plugins/test_cachi2_init.py @@ -35,6 +35,7 @@ from tests.mock_env import MockEnv from tests.stubs import StubSource +from tests.utils.test_cachi2 import Symlink, write_file_tree REMOTE_SOURCE_REPO = 'https://git.example.com/team/repo.git' @@ -370,6 +371,30 @@ def test_dependency_replacements(workflow): expect_error="Dependency replacements are not supported by Cachi2") +def test_enforce_sandbox(workflow, mocked_cachi2_init, tmp_path): + """Should remove symlink pointing outside of repository""" + container_yaml_config = dedent("""\ + remote_source: + repo: https://git.example.com/team/repo.git + ref: a55c00f45ec3dfee0c766cea3d395d6e21cc2e5a + packages: + gomod: + - path: "." + remove_unsafe_symlinks: True + """) + mock_repo_config(workflow, data=container_yaml_config) + + write_file_tree({"subdir": {"symlink_to_root": Symlink("/")}}, tmp_path) + + msg = ( + "Removing..." + ) + with pytest.raises(ValueError): + mocked_cachi2_init(workflow).run() + + # assert msg in str(exc_info) + + def run_plugin_with_args(workflow, dependency_replacements=None, expect_error=None, expect_result=True, expected_plugin_results=None): runner = (MockEnv(workflow) diff --git a/tests/utils/test_cachi2.py b/tests/utils/test_cachi2.py index e11885f36..c00c09269 100644 --- a/tests/utils/test_cachi2.py +++ b/tests/utils/test_cachi2.py @@ -180,7 +180,7 @@ def test_remote_source_invalid_paths(tmpdir, remote_source_packages, expected_er def test_remote_source_path_to_symlink_out_of_repo(tmpdir): - """If cloned repo contains symlink that points out fo repo, + """If cloned repo contains symlink that points out of repo, path in packages shouldn't be allowed""" tmpdir_path = Path(tmpdir)