diff --git a/backend/copr_backend/background_worker_build.py b/backend/copr_backend/background_worker_build.py index ae20ff6ac..11bc40436 100644 --- a/backend/copr_backend/background_worker_build.py +++ b/backend/copr_backend/background_worker_build.py @@ -8,6 +8,7 @@ import os import shutil import statistics +import tarfile import time import json import shlex @@ -581,6 +582,22 @@ def _transfer_log_file(self): except SSHConnectionError as exc: return "Stopped following builder for broken SSH: {}".format(exc) + def _archive_uploaded_logs(self): + uploaded_logs_dir = os.path.join(self.job.results_dir, "uploaded-logs") + if not os.path.isdir(uploaded_logs_dir): + return + + tarball_path = os.path.join(self.job.results_dir, "uploaded-logs.tar.gz") + try: + with tarfile.open(tarball_path, "w:gz") as tar: + for entry in sorted(os.listdir(uploaded_logs_dir)): + tar.add(os.path.join(uploaded_logs_dir, entry), arcname=entry) + except OSError as ex: + self.log.error("Unable to archive uploaded logs: %s", ex) + return + + shutil.rmtree(uploaded_logs_dir, ignore_errors=True) + def _compress_logs(self): """ Compress builder-live.log, backend.log, and fedora-review.log using gzip. @@ -1106,6 +1123,7 @@ def handle_task(self): self._drop_host() if self.job: self._mark_finished() + self._archive_uploaded_logs() self._compress_logs() else: self.log.error("No job object from Frontend") diff --git a/backend/tests/test_background_worker_build.py b/backend/tests/test_background_worker_build.py index 53eb16149..2d069f285 100644 --- a/backend/tests/test_background_worker_build.py +++ b/backend/tests/test_background_worker_build.py @@ -11,6 +11,7 @@ import os import shutil import subprocess +import tarfile import time import tempfile from unittest import mock @@ -895,3 +896,55 @@ def test_buildjob_chroot_dir(f_build_rpm_case): job_dict = copy.deepcopy(testlib.VALID_SUBPROJECT_PRM_JOB) job = BuildJob(job_dict, worker.opts) assert job.chroot_dir.endswith("copr-pull-requests:pr:3568/fedora-40-x86_64") + +def test_archive_uploaded_logs_creates_tarball(f_build_rpm_case): + config = f_build_rpm_case + worker = config.bw + worker.job = _get_rpm_job_object(worker.opts) + + uploaded_logs_dir = os.path.join(worker.job.results_dir, "uploaded-logs") + os.makedirs(uploaded_logs_dir) + with open(os.path.join(uploaded_logs_dir, "builder-live.log"), "w", + encoding="utf-8") as handle: + handle.write("live log") + with open(os.path.join(uploaded_logs_dir, "notes.txt.gz"), "wb") as handle: + handle.write(b"gz data") + + worker._archive_uploaded_logs() + + tarball_path = os.path.join(worker.job.results_dir, "uploaded-logs.tar.gz") + assert os.path.exists(tarball_path) + assert not os.path.exists(uploaded_logs_dir) + with tarfile.open(tarball_path, "r:gz") as tar: + names = sorted(tar.getnames()) + assert names == ["builder-live.log", "notes.txt.gz"] + +def test_archive_uploaded_logs_noop_without_dir(f_build_rpm_case): + config = f_build_rpm_case + worker = config.bw + worker.job = _get_rpm_job_object(worker.opts) + os.makedirs(worker.job.results_dir, exist_ok=True) + + worker._archive_uploaded_logs() + + tarball_path = os.path.join(worker.job.results_dir, "uploaded-logs.tar.gz") + assert not os.path.exists(tarball_path) + +def test_archive_uploaded_logs_tar_error(f_build_rpm_case, caplog): + config = f_build_rpm_case + worker = config.bw + worker.job = _get_rpm_job_object(worker.opts) + + uploaded_logs_dir = os.path.join(worker.job.results_dir, "uploaded-logs") + os.makedirs(uploaded_logs_dir) + with open(os.path.join(uploaded_logs_dir, "builder-live.log"), "w", + encoding="utf-8") as handle: + handle.write("live log") + + with mock.patch("copr_backend.background_worker_build.tarfile.open", + side_effect=OSError("disk full")): + worker._archive_uploaded_logs() + + assert_logs_exist(["Unable to archive uploaded logs"], caplog) + # the raw directory is left in place, nothing crashed + assert os.path.isdir(uploaded_logs_dir) diff --git a/beaker-tests/Sanity/copr-cli-basic-operations/runtest-rpm-upload.sh b/beaker-tests/Sanity/copr-cli-basic-operations/runtest-rpm-upload.sh index a4201c28b..0d0948d19 100755 --- a/beaker-tests/Sanity/copr-cli-basic-operations/runtest-rpm-upload.sh +++ b/beaker-tests/Sanity/copr-cli-basic-operations/runtest-rpm-upload.sh @@ -23,6 +23,7 @@ source "$HERE/config" source "$HERE/helpers" PACKAGE=copr-rpm-upload-sanity-test +PACKAGE_MULTI=copr-rpm-upload-multi-sanity-test # Build a throwaway binary RPM locally build_local_rpm() @@ -49,6 +50,41 @@ EOF find "$workdir" -name '*.rpm' } +# Build a throwaway package with a sub-package (-> multiple binary RPMs) plus +# its srpm, to exercise the multi-file "uploadrpm" scenario (RPM array + +# optional accompanying srpm) +build_local_rpms_with_subpackage_and_srpm() +{ + local workdir + workdir=$(mktemp -d) + cat > "$workdir/$PACKAGE_MULTI.spec" <&2 + find "$workdir" -name '*.rpm' +} + rlJournalStart rlPhaseStartSetup setup_checks @@ -93,8 +129,73 @@ rlJournalStart $PROJECT $RPM_PATH" 1 "Upload with wrong SHA256 should fail" rlPhaseEnd + rlPhaseStartTest "multi-file uploadrpm: RPM array + --name + --srpm + --logs" + if [[ $FRONTEND_URL == "https://copr.stg.fedoraproject.org" ]]; then + rlLog "Skipping, RPM uploads are not enabled for the Fedora Copr instance" + exit 0 + fi + + rlRun "RPM_PATHS=(\$(build_local_rpms_with_subpackage_and_srpm))" \ + 0 "Building local test RPMs (main + sub-package + srpm)" + + SRPM_PATH= + BINARY_RPMS=() + for _path in "${RPM_PATHS[@]}"; do + case "$_path" in + *.src.rpm) SRPM_PATH=$_path ;; + *) BINARY_RPMS+=("$_path") ;; + esac + done + rlAssertExists "$SRPM_PATH" + rlRun "test ${#BINARY_RPMS[@]} -eq 2" 0 \ + "Expecting 2 binary RPMs (main package + sub-package)" + + # a handful of throwaway log/text files to upload alongside the + # RPMs, covering all four supported extensions + LOG1=$(mktemp --suffix=.log) + echo "fake builder-live log" > "$LOG1" + LOG2=$(mktemp --suffix=.log.gz) + echo "fake gzipped log" | gzip > "$LOG2" + LOG3=$(mktemp --suffix=.txt) + echo "fake notes" > "$LOG3" + LOG4=$(mktemp --suffix=.txt.gz) + echo "fake gzipped notes" | gzip > "$LOG4" + + # one SHA256 checksum per binary RPM, in the same order as + # BINARY_RPMS, to exercise the multi-RPM checksum verification + CHECKSUMS=() + for _rpm in "${BINARY_RPMS[@]}"; do + CHECKSUMS+=("$(sha256sum "$_rpm" | cut -d' ' -f1)") + done + + # --name is required here since more than one RPM is uploaded and + # the package name can't be reliably guessed from the filenames + rlRun -s "copr-cli uploadrpm --nowait --chroot $CHROOT \ + --name $PACKAGE_MULTI \ + --srpm $SRPM_PATH \ + --logs $LOG1 $LOG2 $LOG3 $LOG4 \ + --sha256 ${CHECKSUMS[*]} \ + $PROJECT ${BINARY_RPMS[*]}" + rlRun "parse_build_id" + rlRun "copr watch-build $BUILD_ID" + + # verify both the main package and its sub-package are installable + rlRun "dnf install -y --disablerepo='*' \ + --enablerepo=\"copr:${FRONTEND_PUBLIC_HOST}:$(repo_owner):${PROJECTNAME}\" \ + $PACKAGE_MULTI $PACKAGE_MULTI-subpkg" + rlAssertRpm "$PACKAGE_MULTI" + rlAssertRpm "$PACKAGE_MULTI-subpkg" + + # the uploaded logs must be auto-compressed into a single tarball + # and downloadable the same way as regular build logs (never via + # Pulp -- they only ever live on the backend filesystem) + DOWNLOAD_DEST=$(mktemp -d) + rlRun "copr-cli download-build --dest $DOWNLOAD_DEST --logs $BUILD_ID" + rlRun "find $DOWNLOAD_DEST -name 'uploaded-logs.tar.gz'" + rlPhaseEnd + rlPhaseStartCleanup - rlRun "dnf -y remove $PACKAGE" + rlRun "dnf -y remove $PACKAGE $PACKAGE_MULTI $PACKAGE_MULTI-subpkg" rlRun "dnf -y copr remove $DNF_COPR_ID/$PROJECT" cleanProject rlPhaseEnd diff --git a/cli/copr_cli/main.py b/cli/copr_cli/main.py index f9d50cc4b..3de7ab8d4 100644 --- a/cli/copr_cli/main.py +++ b/cli/copr_cli/main.py @@ -464,17 +464,31 @@ def action_upload_rpm(self, args): username, projectname, project_dirname = self.parse_dirname(args.copr_repo) buildopts = buildopts_from_args(args) - if not os.path.exists(args.rpm): - raise CoprException("File {0} not found".format(args.rpm)) + if len(args.rpms) > 1 and not args.pkgname: + raise CoprException( + "--name is required when uploading more than one RPM " + "(it can't be reliably guessed).") + + all_paths = list(args.rpms) + if args.srpm: + all_paths.append(args.srpm) + if args.logs: + all_paths.extend(args.logs) + + for path in all_paths: + if not os.path.exists(path): + raise CoprException("File {0} not found".format(path)) - progress_callback = get_progress_callback(os.path.getsize(args.rpm)) + total_size = sum(os.path.getsize(path) for path in all_paths) + progress_callback = get_progress_callback(total_size) buildopts["progress_callback"] = progress_callback - print('Uploading package {0}'.format(args.rpm)) + print('Uploading package(s) {0}'.format(', '.join(all_paths))) try: build = self.client.build_proxy.create_from_rpm_upload( ownername=username, projectname=projectname, project_dirname=project_dirname, buildopts=buildopts, - path=args.rpm, + paths=args.rpms, name=args.pkgname, + srpm_path=args.srpm, log_paths=args.logs, sha256=getattr(args, "sha256", None)) finally: if progress_callback: @@ -884,6 +898,8 @@ def action_download_build(self, args): if args.logs: cmd.extend(["-A", "*.log.gz"]) + # tarball of client-uploaded logs for "uploadrpm" builds + cmd.extend(["-A", "uploaded-logs.tar.gz"]) if args.review: cmd.extend([ @@ -1739,13 +1755,30 @@ def setup_parser(): # create the parser for the "uploadrpm" command parser_upload_rpm = subparsers.add_parser( "uploadrpm", parents=[parser_build_parent], - help="Publish an already-built local RPM directly to a specified copr, " - "skipping the SRPM build phase entirely") + help="Publish one or more already-built local RPMs directly to a " + "specified copr, skipping the SRPM build phase entirely") + parser_upload_rpm.add_argument( + "rpms", nargs="+", + help="Local path(s) to the already-built .rpm file(s) to publish") + parser_upload_rpm.add_argument( + "--name", dest="pkgname", required=False, + help=("Package name. Optional when uploading a single RPM " + "(guessed from its filename), required when uploading " + "more than one RPM.")) + parser_upload_rpm.add_argument( + "--srpm", dest="srpm", metavar="SRPM", required=False, + help=("Optional local path to an accompanying .src.rpm/.nosrc.rpm " + "file, published alongside the uploaded RPM(s)")) parser_upload_rpm.add_argument( - "rpm", help="Local path to the already-built .rpm file to publish") + "--logs", dest="logs", nargs="+", metavar="LOG", required=False, + help=("Optional local path(s) to .log/.log.gz/.txt/.txt.gz " + "file(s), auto-compressed into a single tarball and " + "stored on the backend filesystem")) parser_upload_rpm.add_argument( - "--sha256", help="Expected SHA256 hex digest of the uploaded file; " - "the server rejects the build on mismatch") + "--sha256", nargs="+", metavar="SHA256", required=False, + help=("Optional expected SHA256 hex digest(s) of the uploaded " + "RPM(s), one per RPM in the same order as the RPM " + "arguments; the server rejects the build on mismatch")) parser_upload_rpm.set_defaults(func="action_upload_rpm") # create the parser for the "buildpypi" command diff --git a/cli/man/copr-cli.1.asciidoc b/cli/man/copr-cli.1.asciidoc index 41566c6a9..4ddac6752 100644 --- a/cli/man/copr-cli.1.asciidoc +++ b/cli/man/copr-cli.1.asciidoc @@ -268,16 +268,29 @@ This limitation comes from the COPR API. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ usage: copr-cli uploadrpm [-h] [-r, --chroot CHROOTS] [--memory MEMORY] [--timeout TIMEOUT] [--nowait] - [--background] - copr_repo RPM + [--background] [--name NAME] [--srpm SRPM] [--logs LOG [LOG ...]] + copr_repo RPMS [RPMS ...] -Publish an already-built local RPM directly to a project, skipping the SRPM -build phase entirely. +Publish one or more already-built local RPMs directly to a project, skipping +the SRPM build phase entirely. For the arguments, see `copr-cli build` command above. -RPM:: -Local path to the already-built .rpm file to publish. +RPMS:: +Local path(s) to the already-built .rpm file(s) to publish. + +--name NAME:: +Package name. Optional when uploading a single RPM (guessed from its +filename), required when uploading more than one RPM. + +--srpm SRPM:: +Optional local path to an accompanying .src.rpm/.nosrc.rpm file, published +alongside the uploaded RPM(s). + +--logs LOG [LOG ...]:: +Optional local path(s) to .log/.log.gz/.txt/.txt.gz file(s). They are +auto-compressed into a single tarball and stored to Copr, downloadable the same way +as regular build logs e.g. via `copr-cli download-build --logs`. `copr-cli buildpypi [options]` diff --git a/cli/man/copr-cli.cheat b/cli/man/copr-cli.cheat index a1e089f3b..7a2123f87 100644 --- a/cli/man/copr-cli.cheat +++ b/cli/man/copr-cli.cheat @@ -34,5 +34,13 @@ copr-cli buildpypi test-project --packagename pydantic-ai --with-deps --chroot f # to publish an already-built local RPM directly, skipping the SRPM build copr-cli uploadrpm test-project ~/packages/package-1.6-1.fc28.x86_64.rpm +# to publish multiple built RPMs (plus their srpm and build logs) directly, +# skipping the SRPM build; --name is required with more than one RPM +copr-cli uploadrpm test-project --name package \ + --srpm ~/packages/package-1.6-1.fc28.src.rpm \ # optional + --logs ~/packages/build.log ~/packages/build.txt \ + ~/packages/package-1.6-1.fc28.x86_64.rpm \ + ~/packages/package-devel-1.6-1.fc28.x86_64.rpm + # to regenerate repository metadata for a project copr-cli regenerate-repos test-project diff --git a/cli/tests/test_cli.py b/cli/tests/test_cli.py index e7b0a9dcc..e42a40aa4 100644 --- a/cli/tests/test_cli.py +++ b/cli/tests/test_cli.py @@ -1,4 +1,5 @@ # pylint: disable=too-many-positional-arguments +# pylint: disable=too-many-lines import os import argparse @@ -695,6 +696,59 @@ def test_create_upload_rpm(watch_builds, _config_from_file, assert "Build was added to foo" in stdout assert not watch_builds.called create_from_rpm_upload.assert_called_once() + _args, kwargs = create_from_rpm_upload.call_args + assert kwargs["paths"] == [rpm_file.name] + assert kwargs["name"] is None + assert kwargs["srpm_path"] is None + assert kwargs["log_paths"] is None + + +@mock.patch('copr.v3.proxies.build.BuildProxy.create_from_rpm_upload') +@mock.patch('copr_cli.main.config_from_file', return_value=mock_config) +@mock.patch('copr_cli.main.Commands._watch_builds') +def test_create_upload_rpm_multi_requires_name( + watch_builds, _config_from_file, create_from_rpm_upload, capsys): + with tempfile.NamedTemporaryFile(suffix=".rpm") as rpm1, \ + tempfile.NamedTemporaryFile(suffix=".rpm") as rpm2: + with pytest.raises(SystemExit): + main.main(argv=[ + "uploadrpm", "--nowait", "copr_name", rpm1.name, rpm2.name, + ]) + + _stdout, stderr = capsys.readouterr() + assert "--name is required" in stderr + assert not watch_builds.called + create_from_rpm_upload.assert_not_called() + + +@mock.patch('copr.v3.proxies.build.BuildProxy.create_from_rpm_upload') +@mock.patch('copr_cli.main.config_from_file', return_value=mock_config) +@mock.patch('copr_cli.main.Commands._watch_builds') +def test_create_upload_rpm_multi_with_srpm_logs( + watch_builds, _config_from_file, create_from_rpm_upload, capsys): + create_from_rpm_upload.return_value = Munch(projectname="foo", id=123) + + with tempfile.NamedTemporaryFile(suffix=".rpm") as rpm1, \ + tempfile.NamedTemporaryFile(suffix=".rpm") as rpm2, \ + tempfile.NamedTemporaryFile(suffix=".src.rpm") as srpm, \ + tempfile.NamedTemporaryFile(suffix=".log") as log1, \ + tempfile.NamedTemporaryFile(suffix=".log.gz") as log2: + main.main(argv=[ + "uploadrpm", "--nowait", "copr_name", rpm1.name, rpm2.name, + "--name", "hello", + "--srpm", srpm.name, + "--logs", log1.name, log2.name, + ]) + + stdout, _stderr = capsys.readouterr() + assert "Build was added to foo" in stdout + assert not watch_builds.called + create_from_rpm_upload.assert_called_once() + _args, kwargs = create_from_rpm_upload.call_args + assert kwargs["paths"] == [rpm1.name, rpm2.name] + assert kwargs["name"] == "hello" + assert kwargs["srpm_path"] == srpm.name + assert kwargs["log_paths"] == [log1.name, log2.name] @mock.patch('copr.v3.proxies.build.BuildProxy.check_before_build') diff --git a/common/copr_common/enums.py b/common/copr_common/enums.py index a753caffe..8f422999e 100644 --- a/common/copr_common/enums.py +++ b/common/copr_common/enums.py @@ -118,7 +118,7 @@ class BuildSourceEnum(metaclass=EnumType): "scm": 8, # type, clone_url, committish, subdirectory, spec, srpm_build_method "custom": 9, # user-provided script to build sources "distgit": 10, # distgit_instance, package_name, committish - "rpm_upload": 11, # tmp, files -- pre-built RPMs uploaded directly, no SRPM build + "rpm_upload": 11, # logfiles, and prebuilt srpm and rpms uploaded directly } diff --git a/frontend/coprs_frontend/coprs/forms.py b/frontend/coprs_frontend/coprs/forms.py index df7fc26f1..d02c2ee38 100644 --- a/frontend/coprs_frontend/coprs/forms.py +++ b/frontend/coprs_frontend/coprs/forms.py @@ -307,6 +307,66 @@ def __call__(self, form, field): raise wtforms.ValidationError(self.message) +class LogValidator: + """ + Validate that every uploaded file looks like a log/text file + (.log, .log.gz, .txt, or .txt.gz) with a usable filename. + """ + def __init__(self, message=None): + if not message: + message = ("You can only upload .log, .log.gz, .txt, and " + ".txt.gz files") + self.message = message + + def __call__(self, form, field): + for file_storage in field.data or []: + if not file_storage.filename: + raise wtforms.ValidationError(self.message) + + filename = file_storage.filename.lower() + if not filename.endswith((".log", ".log.gz", ".txt", ".txt.gz")): + raise wtforms.ValidationError(self.message) + + +class Sha256Validator: + """ + Validate that every given value looks like a SHA256 hex digest (64 + hex characters). + """ + def __init__(self, message=None): + if not message: + message = "SHA256 checksums must be 64 hexadecimal characters" + self.message = message + + def __call__(self, form, field): + for value in field.data or []: + if not re.match(r"^[0-9a-fA-F]{64}$", value): + raise wtforms.ValidationError(self.message) + + +class SrpmUploadValidator: + """ + Validate that the (optional, single) uploaded file looks like a + .src.rpm / .nosrc.rpm file with a usable filename. + """ + def __init__(self, message=None): + if not message: + message = "You can upload at most one .src.rpm or .nosrc.rpm file" + self.message = message + + def __call__(self, form, field): + file_storage = field.data + if not file_storage: + return + + if not file_storage.filename: + raise wtforms.ValidationError(self.message) + + filename = file_storage.filename.lower() + if not filename.endswith((".src.rpm", ".nosrc.rpm")): + raise wtforms.ValidationError(self.message) + + class CoprUniqueNameValidator(object): def __init__(self, message=None, user=None, group=None, exist_ok=False): @@ -1502,7 +1562,17 @@ def __new__(cls, active_chroots): form.pkgs = MultipleFileField('rpms', validators=[ FileRequired(), RpmValidator()]) - form.sha256 = wtforms.StringField('sha256') + form.srpm = FileField('srpm', validators=[ + wtforms.validators.Optional(), + SrpmUploadValidator()]) + form.logs = MultipleFileField('logs', validators=[ + wtforms.validators.Optional(), + LogValidator()]) + form.name = wtforms.StringField('name', validators=[ + wtforms.validators.Optional()]) + form.sha256 = wtforms.StringField('sha256', validators=[ + wtforms.validators.Optional(), + Sha256Validator()]) return form diff --git a/frontend/coprs_frontend/coprs/logic/builds_logic.py b/frontend/coprs_frontend/coprs/logic/builds_logic.py index f6102b5bf..ac38b5ecf 100644 --- a/frontend/coprs_frontend/coprs/logic/builds_logic.py +++ b/frontend/coprs_frontend/coprs/logic/builds_logic.py @@ -749,48 +749,79 @@ def create_new_from_upload(cls, user, copr, form_field, orig_filename, return build @classmethod - def _save_uploaded_rpms(cls, form_files, expected_sha256=None): - """ - Save each uploaded file into a fresh STORAGE_DIR tmp directory. - - :return: (tmp_name, filenames) - :raises BadRequest: if there isn't exactly one uploaded file, or if - its filename is invalid / not a ".rpm", or if expected_sha256 + def _sanitize_uploaded_filename(cls, form_file, allowed_suffixes, reject_suffixes=()): + sanitized = secure_filename(form_file.filename) + if (not sanitized or not sanitized.endswith(allowed_suffixes) or + sanitized.endswith(reject_suffixes)): + raise BadRequest( + f"Uploaded filename '{form_file.filename}' is invalid " + "or could not be safely sanitized to a valid filename.") + return sanitized + + @classmethod + def _save_uploaded_files(cls, rpm_files, srpm_file=None, log_files=None, + expected_sha256s=None): + """ + :param rpm_files: list of uploaded ".rpm" file objects + :param srpm_file: optional src.rpm + :param log_files: optional list logs and txts + :param expected_sha256s: optional list of expected SHA256 hex + digests, one per uploaded RPM, in the same order as rpm_files + :return: (tmp_name, rpm_filenames, srpm_filename, log_filenames) + :raises BadRequest: if there isn't at least one uploaded RPM file, + if any filename is invalid, if the number of expected_sha256s + doesn't match the number of rpm_files, or if a checksum doesn't match :raises InsufficientStorage """ - if len(form_files) != 1: - # multi-RPM upload may be added later + if not rpm_files: + raise BadRequest("At least one .rpm file has to be uploaded.") + + if expected_sha256s and len(expected_sha256s) != len(rpm_files): raise BadRequest( - "Only one RPM can be uploaded per call for now " - "(multi-RPM upload may be added later).") - - sanitized_names = [] - for form_file in form_files: - sanitized = secure_filename(form_file.filename) - if (not sanitized or not sanitized.endswith(".rpm") or - sanitized.endswith((".src.rpm", ".nosrc.rpm"))): - raise BadRequest( - f"Uploaded filename '{form_file.filename}' is invalid " - "or could not be safely sanitized to a valid .rpm filename.") - sanitized_names.append(sanitized) + f"Got {len(expected_sha256s)} --sha256 checksum(s) for " + f"{len(rpm_files)} uploaded RPM(s); provide one checksum " + "per RPM, in the same order.") + + rpm_names = [ + cls._sanitize_uploaded_filename(rpm_file, (".rpm",), (".src.rpm", ".nosrc.rpm")) + for rpm_file in rpm_files + ] + + srpm_name = None + if srpm_file: + srpm_name = cls._sanitize_uploaded_filename( + srpm_file, (".src.rpm", ".nosrc.rpm")) + + log_names = [ + cls._sanitize_uploaded_filename( + log_file, (".log", ".log.gz", ".txt", ".txt.gz")) + for log_file in (log_files or []) + ] tmp = None try: tmp = tempfile.mkdtemp(dir=app.config["STORAGE_DIR"]) tmp_name = os.path.basename(tmp) - filenames = [] - for form_file, filename in zip(form_files, sanitized_names): + + sha256s = expected_sha256s or [None] * len(rpm_files) + for rpm_file, filename, expected_sha256 in zip(rpm_files, rpm_names, sha256s): file_path = os.path.join(tmp, filename) - save_form_file_field_to(form_file, file_path) + save_form_file_field_to(rpm_file, file_path) if expected_sha256: actual = sha256_of_file(file_path) if expected_sha256.lower() != actual.lower(): raise BadRequest( f"SHA256 mismatch for '{filename}': " f"expected {expected_sha256}, got {actual}") - filenames.append(filename) - return tmp_name, filenames + + if srpm_file: + save_form_file_field_to(srpm_file, os.path.join(tmp, srpm_name)) + + for log_file, filename in zip(log_files or [], log_names): + save_form_file_field_to(log_file, os.path.join(tmp, filename)) + + return tmp_name, rpm_names, srpm_name, log_names except (OSError, BadRequest): if tmp: shutil.rmtree(tmp) @@ -815,12 +846,37 @@ def _find_or_create_rpm_upload_package(cls, user, copr, pkg_name, source_json): return package # pylint: disable=too-many-arguments + @classmethod + def _resolve_rpm_upload_pkg_name(cls, name, rpm_filenames): + """ + Determine the package name for a "direct RPM upload" build: use + the explicitly given name if any, guess it from the (single) + uploaded RPM's filename otherwise, or fail if that's not possible. + + :raises BadRequest + """ + if name: + return name + + if len(rpm_filenames) == 1: + pkg_name = helpers.parse_package_name(rpm_filenames[0]) + if pkg_name: + return pkg_name + + raise BadRequest( + f"Can not derive a package name from the uploaded " + f"filename '{rpm_filenames[0]}'") + + raise BadRequest( + "A package name is required when uploading more than " + "one RPM (it can't be reliably guessed).") + @classmethod def create_new_from_rpm_upload(cls, user, copr, chroot_names, form_files, *, - copr_dirname=None, background=False, - timeout=None, after_build_id=None, - with_build_id=None, - expected_sha256=None): + name=None, srpm_form_file=None, + log_form_files=None, copr_dirname=None, + expected_sha256s=None, + **build_options): """ Create a build that publishes built RPMs directly for one or more chroots, skipping the SRPM build and dist-git import phases @@ -829,35 +885,57 @@ def create_new_from_rpm_upload(cls, user, copr, chroot_names, form_files, *, :type user: models.User :type copr: models.Copr :param chroot_names: names of the chroots to publish into - :param form_files: list of uploaded-file objects + :param form_files: list of uploaded ".rpm" file objects (at least one) + :param name: explicit package name. Optional (guessed from the + uploaded filename) when exactly one RPM is uploaded, but + required when more than one RPM is uploaded because the name + can't be reliably guessed from a single filename anymore. + :param srpm_form_file: optional uploaded ".src.rpm"/".nosrc.rpm" + file object, published alongside the RPM(s) + :param log_form_files: optional list of uploaded + ".log"/".log.gz"/".txt"/".txt.gz" file objects, auto-compressed + into a single tarball and kept on the backend filesystem only + :param expected_sha256s: optional list of expected SHA256 hex + digests, one per uploaded RPM, in the same order as form_files + :param build_options: background, timeout, after_build_id, + with_build_id :return: models.Build """ + # pylint: disable=too-many-locals coprs_logic.CoprsLogic.raise_if_unfinished_blocking_action( copr, "Can't build while there is an operation in progress: {action}") users_logic.UsersLogic.raise_if_cant_build_in_copr( user, copr, "You don't have permissions to build in this copr.") - tmp_name, filenames = cls._save_uploaded_rpms(form_files, expected_sha256) + tmp_name, rpm_filenames, srpm_filename, log_filenames = \ + cls._save_uploaded_files(form_files, srpm_form_file, log_form_files, + expected_sha256s) try: - pkg_name = helpers.parse_package_name(filenames[0]) - if not pkg_name: - raise BadRequest( - f"Can not derive a package name from the uploaded " - f"filename '{filenames[0]}'") - - source_json = json.dumps({"tmp": tmp_name, "files": filenames}) + pkg_name = cls._resolve_rpm_upload_pkg_name(name, rpm_filenames) + source_json = json.dumps({ + "tmp": tmp_name, + "rpms": rpm_filenames, + "srpm": srpm_filename, + "logs": log_filenames, + }) package = cls._find_or_create_rpm_upload_package( user, copr, pkg_name, source_json) - batch = cls.setup_batch(after_build_id, with_build_id, user) + batch = cls.setup_batch( + build_options.get("after_build_id"), + build_options.get("with_build_id"), user) copr_dir = None if copr_dirname: copr_dir = coprs_logic.CoprDirsLogic.get_or_create(copr, copr_dirname) + pkgs_display = list(rpm_filenames) + if srpm_filename: + pkgs_display.append(srpm_filename) + build = models.Build( user=user, - pkgs=", ".join(filenames), + pkgs=", ".join(pkgs_display), copr=copr, copr_dir=copr_dir, package=package, @@ -865,9 +943,9 @@ def create_new_from_rpm_upload(cls, user, copr, chroot_names, form_files, *, source_json=source_json, source_status=StatusEnum("succeeded"), submitted_on=int(time.time()), - is_background=bool(background), + is_background=bool(build_options.get("background")), batch=batch, - timeout=timeout or app.config["DEFAULT_BUILD_TIMEOUT"], + timeout=build_options.get("timeout") or app.config["DEFAULT_BUILD_TIMEOUT"], ) db.session.add(build) diff --git a/frontend/coprs_frontend/coprs/templates/coprs/detail/_describe_source.html b/frontend/coprs_frontend/coprs/templates/coprs/detail/_describe_source.html index 7e6827329..72edd2350 100644 --- a/frontend/coprs_frontend/coprs/templates/coprs/detail/_describe_source.html +++ b/frontend/coprs_frontend/coprs/templates/coprs/detail/_describe_source.html @@ -75,11 +75,23 @@ {% endif %} {% if source_type_text == "rpm_upload" %} + {% if source_json_dict.get("srpm") %} +
Uploaded SRPM:
+
{{ source_json_dict["srpm"] }}
+ {% endif %} + {% if source_json_dict.get("rpms") %}
Uploaded RPMs:
- {% for filename in source_json_dict.get("files", []) %} + {% for filename in source_json_dict.get("rpms", []) %} +
{{ filename }}
+ {% endfor %} + {% endif %} + {% if source_json_dict.get("logs") %} +
Uploaded logs:
+ {% for filename in source_json_dict.get("logs", []) %}
{{ filename }}
{% endfor %} {% endif %} + {% endif %} {% if source_type_text == "distgit" %} {% for info in ["distgit", "committish", "clone_url"] %} diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py index 9c484039d..e47da0e2d 100644 --- a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py +++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py @@ -335,10 +335,14 @@ def post(self): "Direct RPM upload is not enabled on this Copr instance") copr = get_copr() - data = get_form_compatible_data(preserve=["chroots", "exclude_chroots"]) + data = get_form_compatible_data( + preserve=["chroots", "exclude_chroots", "sha256"]) # pylint: disable-next=not-callable form = forms.BuildFormRpmUploadFactory(copr.active_chroots)(data, meta={'csrf': False}) form.pkgs.data = flask.request.files.getlist("pkgs") + form.srpm.data = flask.request.files.get("srpm") + form.logs.data = flask.request.files.getlist("logs") + form.sha256.data = data.getlist("sha256") if not form.validate_on_submit(): raise BadRequest(f"Bad request parameters: {form.errors}") @@ -348,12 +352,15 @@ def post(self): build = BuildsLogic.create_new_from_rpm_upload( flask.g.user, copr, form.selected_chroots, form.pkgs.data, + name=form.name.data or None, + srpm_form_file=form.srpm.data, + log_form_files=form.logs.data, copr_dirname=form.project_dirname.data, background=form.background.data, timeout=form.timeout.data, after_build_id=form.after_build_id.data, with_build_id=form.with_build_id.data, - expected_sha256=form.sha256.data or None, + expected_sha256s=form.sha256.data or None, ) db.session.commit() return to_dict(build) diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/schema/schemas.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/schema/schemas.py index 39ea438b7..e9cf648d3 100644 --- a/frontend/coprs_frontend/coprs/views/apiv3_ns/schema/schemas.py +++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/schema/schemas.py @@ -781,9 +781,26 @@ class CreateBuildRpmUpload(_BuildDataCommon, _BuildOptionsBase, InputSchema): description="application/x-rpm files to publish directly, " "skipping the SRPM build phase entirely", ) - sha256: String = String( - description="Expected SHA256 hex digest of the uploaded file; " - "the build is rejected on mismatch", + name: String = String( + description="Package name. Optional when uploading a single RPM " + "(guessed from its filename), required when uploading " + "more than one RPM.", + ) + srpm: Raw = Raw( + description="Optional accompanying .src.rpm/.nosrc.rpm file " + "(at most one), published alongside the uploaded RPM(s)", + ) + logs: List = List( + Raw, + description="Optional .log/.log.gz/.txt/.txt.gz files, " + "auto-compressed into a single tarball and stored on " + "the backend filesystem", + ) + sha256: List = List( + String, + description="Optional expected SHA256 hex digest(s), one per " + "uploaded RPM in the same order as pkgs; the build is " + "rejected on mismatch", ) diff --git a/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py b/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py index 2651a0e6a..b3447c2ad 100755 --- a/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py +++ b/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py @@ -212,11 +212,22 @@ def get_build_record(task, for_backend=False): # in copr-rpmbuild requires "source_type" to be absent source_data = task.build.source_json_dict base_url = app.config["PUBLIC_COPR_BASE_URL"] + tmp = source_data.get("tmp") build_record["prebuilt_rpm_urls"] = [ - f"{base_url}/tmp/{source_data.get('tmp')}/{filename}" - for filename in source_data.get("files", []) + f"{base_url}/tmp/{tmp}/{filename}" + for filename in source_data.get("rpms", []) ] + if source_data.get("srpm"): + build_record["prebuilt_srpm_url"] = \ + f"{base_url}/tmp/{tmp}/{source_data['srpm']}" + + if source_data.get("logs"): + build_record["prebuilt_log_urls"] = [ + f"{base_url}/tmp/{tmp}/{filename}" + for filename in source_data["logs"] + ] + return build_record diff --git a/frontend/coprs_frontend/tests/test_apiv3/test_builds.py b/frontend/coprs_frontend/tests/test_apiv3/test_builds.py index c2d07420c..c75c27d7d 100644 --- a/frontend/coprs_frontend/tests/test_apiv3/test_builds.py +++ b/frontend/coprs_frontend/tests/test_apiv3/test_builds.py @@ -298,7 +298,7 @@ def test_rpm_upload_creates_pending_chroot(self): assert build_chroot.status == StatusEnum("pending") source_data = json.loads(build.source_json) - assert source_data["files"] == ["hello-2.8-1.fc43.x86_64.rpm"] + assert source_data["rpms"] == ["hello-2.8-1.fc43.x86_64.rpm"] # no Action is queued for this build type -- the pending BuildChroot # is picked up and dispatched to a real builder like any other build @@ -325,26 +325,9 @@ def test_rpm_upload_multiple_chroots(self): @pytest.mark.usefixtures("f_users", "f_users_api", "f_coprs", "f_mock_chroots", "f_db") - def test_rpm_upload_rejects_multiple_files(self): - # only one RPM can be uploaded per call for now - user = self.models.User.query.filter_by(username="user2").first() - content = { - "ownername": "user2", - "projectname": "foocopr", - "chroots": "fedora-17-x86_64", - "pkgs": [ - _fake_rpm_file("hello-2.8-1.fc43.x86_64.rpm"), - _fake_rpm_file("hello-debuginfo-2.8-1.fc43.x86_64.rpm"), - ], - } - response = self.post_api3_with_auth_multipart( - "/api_3/build/create/rpm-upload", content, user) - assert response.status_code == 400 - assert self.models.Build.query.first() is None - - @pytest.mark.usefixtures("f_users", "f_users_api", "f_coprs", - "f_mock_chroots", "f_db") - def test_rpm_upload_rejects_srpm(self): + def test_rpm_upload_rejects_srpm_in_pkgs(self): + # a src.rpm posted through the "pkgs" field (rather than the + # dedicated "srpm" field) is still rejected user = self.models.User.query.filter_by(username="user2").first() content = { "ownername": "user2", @@ -359,35 +342,25 @@ def test_rpm_upload_rejects_srpm(self): @pytest.mark.usefixtures("f_users", "f_users_api", "f_coprs", "f_mock_chroots", "f_db") - def test_rpm_upload_sha256_match(self): + def test_rpm_upload_sha256_count_mismatch(self): + # two RPMs but only one checksum -- ambiguous, must be rejected user = self.models.User.query.filter_by(username="user2").first() content = { "ownername": "user2", "projectname": "foocopr", "chroots": "fedora-17-x86_64", - "pkgs": _fake_rpm_file("hello-2.8-1.fc43.x86_64.rpm"), + "pkgs": [ + _fake_rpm_file("hello-2.8-1.fc43.x86_64.rpm"), + _fake_rpm_file("hello-debuginfo-2.8-1.fc43.x86_64.rpm", + content=b"other rpm bytes"), + ], + "name": "hello", "sha256": "dae37be1717e714967b78e21ea9fdf00928a7652687d462f3ad631cde43d1a3d", } - response = self.post_api3_with_auth_multipart( - "/api_3/build/create/rpm-upload", content, user) - assert response.status_code == 200 - assert self.models.Build.query.first() is not None - - @pytest.mark.usefixtures("f_users", "f_users_api", "f_coprs", - "f_mock_chroots", "f_db") - def test_rpm_upload_sha256_mismatch(self): - user = self.models.User.query.filter_by(username="user2").first() - content = { - "ownername": "user2", - "projectname": "foocopr", - "chroots": "fedora-17-x86_64", - "pkgs": _fake_rpm_file("hello-2.8-1.fc43.x86_64.rpm"), - "sha256": "0000000000000000000000000000000000000000000000000000000000000000", - } response = self.post_api3_with_auth_multipart( "/api_3/build/create/rpm-upload", content, user) assert response.status_code == 400 - assert "SHA256 mismatch" in response.json["error"] + assert "checksum(s)" in response.json["error"] assert self.models.Build.query.first() is None @pytest.mark.usefixtures("f_users", "f_users_api", "f_coprs", diff --git a/frontend/coprs_frontend/tests/test_forms.py b/frontend/coprs_frontend/tests/test_forms.py index 8c66f8c53..a2e31a10f 100644 --- a/frontend/coprs_frontend/tests/test_forms.py +++ b/frontend/coprs_frontend/tests/test_forms.py @@ -10,6 +10,8 @@ CoprFormFactory, CreateModuleForm, RpmValidator, + LogValidator, + Sha256Validator, REGEX_BOOTSTRAP_IMAGE, REGEX_CHROOT_DENYLIST, ) @@ -140,6 +142,68 @@ def test_rejects_empty_filename(self): validator(None, self._field("")) +class TestLogValidator: + @staticmethod + def _field(filename): + file_storage = mock.Mock() + file_storage.filename = filename + field = mock.Mock() + field.data = [file_storage] + return field + + @pytest.mark.parametrize("filename", [ + "builder-live.log", + "backend.log.gz", + "notes.txt", + "notes.txt.gz", + ]) + def test_valid_log(self, filename): + validator = LogValidator() + validator(None, self._field(filename)) + + def test_rejects_non_log(self): + validator = LogValidator() + with pytest.raises(wtforms.ValidationError): + validator(None, self._field("hello.rpm")) + + def test_rejects_missing_filename(self): + validator = LogValidator() + with pytest.raises(wtforms.ValidationError): + validator(None, self._field(None)) + + def test_rejects_empty_filename(self): + validator = LogValidator() + with pytest.raises(wtforms.ValidationError): + validator(None, self._field("")) + + +class TestSha256Validator: + @staticmethod + def _field(values): + field = mock.Mock() + field.data = values + return field + + def test_valid_checksums(self): + validator = Sha256Validator() + validator(None, self._field(["a" * 64, "F" * 64])) + + def test_empty_is_valid(self): + validator = Sha256Validator() + validator(None, self._field([])) + validator(None, self._field(None)) + + def test_rejects_wrong_length(self): + validator = Sha256Validator() + with pytest.raises(wtforms.ValidationError): + validator(None, self._field(["a" * 63])) + + def test_rejects_non_hex(self): + validator = Sha256Validator() + with pytest.raises(wtforms.ValidationError): + validator(None, self._field(["g" * 64])) + + def test_form_regexes(): assert re.match(REGEX_BOOTSTRAP_IMAGE, "fedora:33") assert re.match(REGEX_BOOTSTRAP_IMAGE, "fedora") diff --git a/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py b/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py index 9ab7b0c40..0c19ee26a 100644 --- a/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py +++ b/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py @@ -487,8 +487,12 @@ def test_rpm_upload_ignores_unknown_chroots(self): assert {bch.name for bch in build.build_chroots} == {"fedora-18-x86_64"} @pytest.mark.usefixtures("f_users", "f_coprs", "f_mock_chroots", "f_db") - def test_rpm_upload_rejects_multiple_files(self): - # only one RPM can be uploaded per call for now + def test_rpm_upload_multi_requires_name(self): + # name can't be reliably guessed when more than one RPM is + # uploaded, so --name is required in that case + storage_dir = self.app.config["STORAGE_DIR"] + before = set(os.listdir(storage_dir)) + rpm_files = [ self._fake_rpm_file("hello-2.8-1.fc18.x86_64.rpm"), self._fake_rpm_file("hello-devel-2.8-1.fc18.x86_64.rpm"), @@ -496,10 +500,55 @@ def test_rpm_upload_rejects_multiple_files(self): with pytest.raises(BadRequest) as error: BuildsLogic.create_new_from_rpm_upload( self.u1, self.c1, ["fedora-18-x86_64"], rpm_files) - assert "Only one RPM can be uploaded per call" in str(error.value) + assert "A package name is required" in str(error.value) assert self.models.Build.query.first() is None for rpm_file in rpm_files: - rpm_file.save.assert_not_called() + rpm_file.save.assert_called_once() + + # the files were saved before the name check, make sure the tmp + # dir got cleaned up again once the check failed + after = set(os.listdir(storage_dir)) + assert after == before + + @pytest.mark.usefixtures("f_users", "f_coprs", "f_mock_chroots", "f_db") + def test_rpm_upload_multiple_files_with_name(self): + rpm_files = [ + self._fake_rpm_file("hello-2.8-1.fc18.x86_64.rpm"), + self._fake_rpm_file("hello-devel-2.8-1.fc18.x86_64.rpm"), + ] + build = BuildsLogic.create_new_from_rpm_upload( + self.u1, self.c1, ["fedora-18-x86_64"], rpm_files, name="hello") + self.db.session.commit() + assert build.package.name == "hello" + assert build.source_json_dict["rpms"] == [ + "hello-2.8-1.fc18.x86_64.rpm", "hello-devel-2.8-1.fc18.x86_64.rpm"] + + @pytest.mark.usefixtures("f_users", "f_coprs", "f_mock_chroots", "f_db") + def test_rpm_upload_with_srpm(self): + rpm_file = self._fake_rpm_file("hello-2.8-1.fc18.x86_64.rpm") + srpm_file = self._fake_rpm_file("hello-2.8-1.fc18.src.rpm") + build = BuildsLogic.create_new_from_rpm_upload( + self.u1, self.c1, ["fedora-18-x86_64"], [rpm_file], + srpm_form_file=srpm_file) + self.db.session.commit() + assert build.source_json_dict["srpm"] == "hello-2.8-1.fc18.src.rpm" + srpm_file.save.assert_called_once() + + @pytest.mark.usefixtures("f_users", "f_coprs", "f_mock_chroots", "f_db") + def test_rpm_upload_with_logs(self): + rpm_file = self._fake_rpm_file("hello-2.8-1.fc18.x86_64.rpm") + log_files = [ + self._fake_rpm_file("builder-live.log", content=b"log contents"), + self._fake_rpm_file("backend.log.gz", content=b"gz contents"), + ] + build = BuildsLogic.create_new_from_rpm_upload( + self.u1, self.c1, ["fedora-18-x86_64"], [rpm_file], + log_form_files=log_files) + self.db.session.commit() + assert build.source_json_dict["logs"] == [ + "builder-live.log", "backend.log.gz"] + for log_file in log_files: + log_file.save.assert_called_once() @pytest.mark.usefixtures("f_users", "f_coprs", "f_mock_chroots", "f_db") def test_rpm_upload_rejects_unsanitizable_name(self): diff --git a/frontend/coprs_frontend/tests/test_views/test_backend_ns/test_backend_general.py b/frontend/coprs_frontend/tests/test_views/test_backend_ns/test_backend_general.py index 421a59726..13b875f46 100644 --- a/frontend/coprs_frontend/tests/test_views/test_backend_ns/test_backend_general.py +++ b/frontend/coprs_frontend/tests/test_views/test_backend_ns/test_backend_general.py @@ -71,6 +71,44 @@ def test_rpm_upload_prebuilt_rpm_urls(self, f_users, f_coprs, f_mock_chroots, f_ base_url = app.config["PUBLIC_COPR_BASE_URL"] assert data["prebuilt_rpm_urls"] == [f"{base_url}/tmp/{tmp}/{filename}"] + def test_rpm_upload_prebuilt_srpm_and_log_urls(self, f_users, f_coprs, + f_mock_chroots, f_db): + """ + An optional accompanying srpm and/or uploaded log files also get + translated into their own public tmp URLs for the Builder. + """ + rpm_filename = "hello-2.8-1.fc18.x86_64.rpm" + srpm_filename = "hello-2.8-1.fc18.src.rpm" + log_filenames = ["builder-live.log", "backend.log.gz"] + + rpm_file = FileStorage(stream=BytesIO(b"fake rpm bytes"), + filename=rpm_filename, + content_type="application/x-rpm") + srpm_file = FileStorage(stream=BytesIO(b"fake srpm bytes"), + filename=srpm_filename, + content_type="application/x-rpm") + log_files = [ + FileStorage(stream=BytesIO(b"log data"), filename=name) + for name in log_filenames + ] + + build = BuildsLogic.create_new_from_rpm_upload( + self.u1, self.c1, ["fedora-18-x86_64"], [rpm_file], + srpm_form_file=srpm_file, log_form_files=log_files) + self.db.session.commit() + + build_chroot = build.build_chroots[0] + task_id = f"{build.id}-{build_chroot.name}" + r = self.tc.get(f"/backend/get-build-task/{task_id}", + headers=self.auth_header).data + data = json.loads(r.decode("utf-8")) + + tmp = build.source_json_dict["tmp"] + base_url = app.config["PUBLIC_COPR_BASE_URL"] + assert data["prebuilt_srpm_url"] == f"{base_url}/tmp/{tmp}/{srpm_filename}" + assert data["prebuilt_log_urls"] == [ + f"{base_url}/tmp/{tmp}/{name}" for name in log_filenames] + class TestWaitingBuilds(CoprsTestCase): diff --git a/python/copr/test/client_v3/test_builds.py b/python/copr/test/client_v3/test_builds.py index 9acdb26e0..ceead0f16 100644 --- a/python/copr/test/client_v3/test_builds.py +++ b/python/copr/test/client_v3/test_builds.py @@ -1,7 +1,9 @@ import tempfile +import pytest from requests import Response from copr.v3 import Client, BuildProxy +from copr.v3.exceptions import CoprValidationException from copr.v3.requests import Request from copr.test import config_location, mock @@ -44,7 +46,7 @@ def test_build_rpm_upload(send): mock_client = Client.create_from_config_file(config_location) with tempfile.NamedTemporaryFile(suffix=".rpm") as rpm_file: mock_client.build_proxy.create_from_rpm_upload( - "praiskup", "ping", rpm_file.name, + "praiskup", "ping", paths=[rpm_file.name], buildopts={"chroots": ["fedora-40-x86_64"]}, ) assert len(send.call_args_list) == 1 @@ -54,5 +56,34 @@ def test_build_rpm_upload(send): assert args['endpoint'] == '/build/create/rpm-upload' assert args['data'] == { 'ownername': 'praiskup', 'projectname': 'ping', - 'project_dirname': None, 'chroots': ['fedora-40-x86_64'], - 'sha256': None} + 'project_dirname': None, 'name': None, + 'chroots': ['fedora-40-x86_64'], 'sha256': None} + + +@mock.patch('copr.v3.proxies.Request.send') +def test_build_rpm_upload_multi(send): + mock_client = Client.create_from_config_file(config_location) + with tempfile.NamedTemporaryFile(suffix=".rpm") as rpm1, \ + tempfile.NamedTemporaryFile(suffix=".rpm") as rpm2, \ + tempfile.NamedTemporaryFile(suffix=".src.rpm") as srpm, \ + tempfile.NamedTemporaryFile(suffix=".log") as log: + mock_client.build_proxy.create_from_rpm_upload( + "praiskup", "ping", paths=[rpm1.name, rpm2.name], + name="ping", srpm_path=srpm.name, log_paths=[log.name], + buildopts={"chroots": ["fedora-40-x86_64"]}, + ) + assert len(send.call_args_list) == 1 + call = send.call_args_list[0] + args = call[1] + assert args['method'] == 'POST' + assert args['endpoint'] == '/build/create/rpm-upload' + assert args['data'] == { + 'ownername': 'praiskup', 'projectname': 'ping', + 'project_dirname': None, 'name': 'ping', + 'chroots': ['fedora-40-x86_64'], 'sha256': None} + + +def test_build_rpm_upload_requires_paths(): + mock_client = Client.create_from_config_file(config_location) + with pytest.raises(CoprValidationException): + mock_client.build_proxy.create_from_rpm_upload("praiskup", "ping") diff --git a/python/copr/test/client_v3/test_requests.py b/python/copr/test/client_v3/test_requests.py index 97275451e..840a7d92f 100644 --- a/python/copr/test/client_v3/test_requests.py +++ b/python/copr/test/client_v3/test_requests.py @@ -1,6 +1,7 @@ from requests import Response +from requests_toolbelt.multipart.encoder import MultipartEncoderMonitor from copr.test import mock -from copr.v3.requests import Request, munchify +from copr.v3.requests import FileRequest, Request, munchify class TestResponse(object): @@ -35,3 +36,43 @@ def test_send(self, request): args, kwargs = request.call_args assert kwargs["method"] == "GET" assert kwargs["url"] == "http://copr/api_3/foo" + + +class TestFileRequest(object): + def test_request_params_dict_files(self): + # a single file per field name -- the traditional, backward + # compatible shape + req = FileRequest( + api_base_url="http://copr/api_3", + files={"pkgs": ("f.rpm", b"data", "application/x-rpm")}, + ) + # pylint: disable-next=protected-access + params = req._request_params(endpoint="foo", method="POST", data={"a": 1}) + + assert isinstance(params["data"], MultipartEncoderMonitor) + assert params["json"] is None + fields = params["data"].encoder.fields + assert fields["pkgs"] == ("f.rpm", b"data", "application/x-rpm") + assert fields["json"][0] == "json" + + def test_request_params_list_files(self): + # multiple files under the same field name (e.g. several RPMs + # uploaded via "pkgs") require a list-of-tuples instead of a dict, + # since a dict can't hold duplicate keys + req = FileRequest( + api_base_url="http://copr/api_3", + files=[ + ("pkgs", ("f1.rpm", b"data1", "application/x-rpm")), + ("pkgs", ("f2.rpm", b"data2", "application/x-rpm")), + ("srpm", ("f.src.rpm", b"data3", "application/x-rpm")), + ], + ) + # pylint: disable-next=protected-access + params = req._request_params(endpoint="foo", method="POST", data={"a": 1}) + + assert isinstance(params["data"], MultipartEncoderMonitor) + fields = params["data"].encoder.fields + names = [name for name, _value in fields] + assert names.count("pkgs") == 2 + assert names.count("srpm") == 1 + assert names.count("json") == 1 diff --git a/python/copr/v3/proxies/build.py b/python/copr/v3/proxies/build.py index 00ccc8962..498684052 100644 --- a/python/copr/v3/proxies/build.py +++ b/python/copr/v3/proxies/build.py @@ -149,36 +149,68 @@ def create_from_file(self, ownername, projectname, path, buildopts=None, project } return self._create(endpoint, data, files=files, buildopts=buildopts) - def create_from_rpm_upload(self, ownername, projectname, path, *, - buildopts=None, project_dirname=None, - sha256=None): + def create_from_rpm_upload(self, ownername, projectname, *, + paths=None, name=None, srpm_path=None, + log_paths=None, buildopts=None, + project_dirname=None, sha256=None): """ - Publish an already-built local RPM file directly, skipping the SRPM - build and dist-git import phases entirely. + Publish one or more already-built local RPM files directly, + skipping the SRPM build and dist-git import phases entirely. :param str ownername: :param str projectname: - :param str path: local path to the already-built .rpm file + :param list paths: local path(s) to the already-built .rpm file(s) + to publish + :param str name: package name. Optional (guessed from the + filename) when uploading a single RPM, required when + uploading more than one RPM + :param str srpm_path: optional local path to an accompanying + .src.rpm/.nosrc.rpm file, published alongside the RPM(s) + :param list log_paths: optional local path(s) to + .log/.log.gz/.txt/.txt.gz file(s), auto-compressed into a + single tarball and stored on the backend :param buildopts: http://python-copr.readthedocs.io/en/latest/client_v3/build_options.html :param str project_dirname: - :param str sha256: expected SHA256 hex digest of the uploaded file + :param list sha256: optional expected SHA256 hex digest(s), one + per uploaded RPM, in the same order as `paths` :return: Munch """ + if not paths: + raise CoprValidationException("'paths' has to be provided") + endpoint = "/build/create/rpm-upload" - # the file must stay open for the whole request (sent by _create() - # below), so it can't be wrapped in a local "with" block here - # pylint: disable-next=consider-using-with - f = open(path, "rb") + # the files must stay open for the whole request (sent by + # _create() below), so they can't be wrapped in local "with" + # blocks here + # pylint: disable=consider-using-with + files = [] + for path in paths: + files.append(( + "pkgs", + (os.path.basename(path), open(path, "rb"), "application/x-rpm"), + )) + + if srpm_path: + files.append(( + "srpm", + (os.path.basename(srpm_path), open(srpm_path, "rb"), + "application/x-rpm"), + )) + + for log_path in log_paths or []: + files.append(( + "logs", + (os.path.basename(log_path), open(log_path, "rb"), + "text/plain"), + )) data = { "ownername": ownername, "projectname": projectname, "project_dirname": project_dirname, + "name": name, "sha256": sha256, } - files = { - "pkgs": (os.path.basename(f.name), f, "application/x-rpm"), - } return self._create(endpoint, data, files=files, buildopts=buildopts) def check_before_build(self, ownername, projectname, diff --git a/python/copr/v3/requests.py b/python/copr/v3/requests.py index 5fb1e172a..91d9ed756 100644 --- a/python/copr/v3/requests.py +++ b/python/copr/v3/requests.py @@ -108,8 +108,14 @@ def __init__(self, files=None, progress_callback=None, **kwargs): def _request_params(self, *args, **kwargs): params = super(FileRequest, self)._request_params(*args, **kwargs) - data = self.files or {} - data["json"] = ("json", json.dumps(params["json"]), "application/json") + json_field = ("json", json.dumps(params["json"]), "application/json") + if isinstance(self.files, list): + # A list-of-tuples is needed (instead of a dict) whenever more + # than one file has to be sent under the same field name + data = list(self.files) + [("json", json_field)] + else: + data = dict(self.files or {}) + data["json"] = json_field callback = self.progress_callback or (lambda x: x) m = MultipartEncoder(data) diff --git a/rpmbuild/main.py b/rpmbuild/main.py index 184435e70..424ab891a 100755 --- a/rpmbuild/main.py +++ b/rpmbuild/main.py @@ -280,6 +280,15 @@ def build_rpm_upload(task, config): "chroot '{2}'".format( os.path.basename(rpm_path), hdr["arch"], task["chroot"])) + if task.get("prebuilt_srpm_url"): + download_file(task["prebuilt_srpm_url"], resultdir) + + if task.get("prebuilt_log_urls"): + uploaded_logs_dir = os.path.join(resultdir, "uploaded-logs") + os.makedirs(uploaded_logs_dir, exist_ok=True) + for url in task["prebuilt_log_urls"]: + download_file(url, uploaded_logs_dir) + with open(os.path.join(resultdir, "success"), "w", encoding="utf-8") as success: success.write("done") diff --git a/rpmbuild/tests/test_build_rpm_upload.py b/rpmbuild/tests/test_build_rpm_upload.py index 3fb04b307..aa8877ad0 100644 --- a/rpmbuild/tests/test_build_rpm_upload.py +++ b/rpmbuild/tests/test_build_rpm_upload.py @@ -120,6 +120,55 @@ def test_build_rpm_upload_download_failure(self, mc_download, mc_get_header.assert_not_called() mc_run_automation_tools.assert_not_called() + @mock.patch("main.run_automation_tools") + @mock.patch("main.get_rpm_header") + @mock.patch("main.download_file") + def test_build_rpm_upload_with_srpm(self, mc_download, mc_get_header, + mc_run_automation_tools): + mc_download.side_effect = self._fake_download_file + mc_get_header.return_value = _fake_header("x86_64") + + task = dict(self.task, prebuilt_srpm_url=( + "https://copr.example.com/tmp/abc/hello-2.8-1.fc40.src.rpm")) + + build_rpm_upload(task, self.config) + + mc_download.assert_any_call( + task["prebuilt_srpm_url"], self.resultdir) + assert os.path.exists( + os.path.join(self.resultdir, "hello-2.8-1.fc40.src.rpm")) + mc_run_automation_tools.assert_called_once() + + @mock.patch("main.run_automation_tools") + @mock.patch("main.get_rpm_header") + @mock.patch("main.download_file") + def test_build_rpm_upload_with_logs(self, mc_download, mc_get_header, + mc_run_automation_tools): + mc_download.side_effect = self._fake_download_file + mc_get_header.return_value = _fake_header("x86_64") + + log_urls = [ + "https://copr.example.com/tmp/abc/builder-live.log", + "https://copr.example.com/tmp/abc/backend.log.gz", + "https://copr.example.com/tmp/abc/notes.txt", + "https://copr.example.com/tmp/abc/notes.txt.gz", + ] + task = dict(self.task, prebuilt_log_urls=log_urls) + + build_rpm_upload(task, self.config) + + # the builder only downloads the plain files into a dedicated + # subdirectory -- it's Copr Backend's job to tar them up once + # transferred, see _archive_uploaded_logs() in background_worker_build + uploaded_logs_dir = os.path.join(self.resultdir, "uploaded-logs") + assert sorted(os.listdir(uploaded_logs_dir)) == sorted( + os.path.basename(url) for url in log_urls) + assert not os.path.exists( + os.path.join(self.resultdir, "uploaded-logs.tar.gz")) + for url in log_urls: + mc_download.assert_any_call(url, uploaded_logs_dir) + mc_run_automation_tools.assert_called_once() + class TestBuildRpmDispatch(TestCase): """