From 04476035b358a56630abed989e934ac55537af44 Mon Sep 17 00:00:00 2001 From: cv-dote <74953312+cv-dote@users.noreply.github.com> Date: Mon, 4 May 2026 14:18:13 +0100 Subject: [PATCH 1/5] Add PackageValidator for pre-built zip submission --- src/rpdk/core/package_validator.py | 250 ++++++++++++++++++++++ tests/test_package_validator.py | 329 +++++++++++++++++++++++++++++ 2 files changed, 579 insertions(+) create mode 100644 src/rpdk/core/package_validator.py create mode 100644 tests/test_package_validator.py diff --git a/src/rpdk/core/package_validator.py b/src/rpdk/core/package_validator.py new file mode 100644 index 00000000..6250e58d --- /dev/null +++ b/src/rpdk/core/package_validator.py @@ -0,0 +1,250 @@ +"""Validation utilities for pre-built schema handler packages. + +This module supports the ``cfn submit --package `` workflow, in which a +user submits a zip file that was already built (for example, by a previous +``cfn submit --dry-run`` or by a CI pipeline). The validator opens the zip, +extracts the minimum metadata required to call ``RegisterType``, and reports +any problems via :class:`rpdk.core.exceptions.InvalidProjectError`. + +The validator does not mutate the zip or the working directory, and does not +require a ``.rpdk-config`` in the current working directory. +""" +import json +import logging +import zipfile +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Dict + +from .exceptions import InvalidProjectError + +LOG = logging.getLogger(__name__) + +SETTINGS_FILENAME = ".rpdk-config" +SCHEMA_FILENAME = "schema.json" +METADATA_FILENAME = ".cfn_metadata.json" + +ARTIFACT_TYPE_RESOURCE = "RESOURCE" +ARTIFACT_TYPE_MODULE = "MODULE" +ARTIFACT_TYPE_HOOK = "HOOK" +SUPPORTED_ARTIFACT_TYPES = frozenset( + {ARTIFACT_TYPE_RESOURCE, ARTIFACT_TYPE_MODULE, ARTIFACT_TYPE_HOOK} +) + + +@dataclass(frozen=True) +class PackageMetadata: + """Minimal information about a pre-built package, extracted from its zip. + + Attributes: + path: Filesystem path to the original zip file. The caller is + expected to re-open this path as a binary file object when it is + ready to upload the bytes. + type_name: The ``typeName`` declared in the zip's ``.rpdk-config`` + entry (for example ``"Acme::Example::Widget"``). + artifact_type: One of ``"RESOURCE"``, ``"MODULE"`` or ``"HOOK"``. + """ + + path: Path + type_name: str + artifact_type: str + + +class PackageValidator: + """Validate a pre-built schema handler package zip and extract metadata. + + The validator never writes to the filesystem and never consults the + current working directory. All validation is performed against the zip + file whose path is given to the constructor. + + Usage:: + + metadata = PackageValidator(Path("./my-type.zip")).validate() + # metadata.type_name and metadata.artifact_type are ready to be + # passed to the CloudFormation RegisterType API. + + On any validation failure, :class:`rpdk.core.exceptions.InvalidProjectError` + is raised. The exception message is expected to name the offending input + (file path, missing entry, invalid field) so that the CLI can surface a + clear error to the user. + """ + + def __init__(self, path: Path) -> None: + self.path = path + + def validate(self) -> PackageMetadata: + """Run the full validation pipeline and return a :class:`PackageMetadata`. + + The pipeline steps, in order: + + 1. Open the zip archive (existence + format check). + 2. Read and parse the ``.rpdk-config`` entry inside the zip. + 3. Resolve the ``artifact_type`` field (with a RESOURCE fallback). + 4. Assert that all required entries for the resolved artifact type + are present (currently: ``schema.json``). + 5. Opportunistically log metadata from ``.cfn_metadata.json`` if it + is present. + 6. Return a :class:`PackageMetadata` built from the zip contents. + """ + with self._open_zip() as zip_file: + settings = self._read_settings(zip_file) + artifact_type = self._resolve_artifact_type(settings) + type_name = self._require_type_name(settings) + self._assert_required_entries(zip_file, artifact_type) + self._log_metadata_if_present(zip_file) + + return PackageMetadata( + path=self.path, + type_name=type_name, + artifact_type=artifact_type, + ) + + # ------------------------------------------------------------------ + # Internal helpers. Each step of the validation pipeline is split + # into a small private method so that error messages stay close to + # the check that produced them and the control flow in ``validate`` + # remains easy to read. + # ------------------------------------------------------------------ + + def _open_zip(self) -> zipfile.ZipFile: + """Open ``self.path`` as a zip archive. + + Raises: + InvalidProjectError: If the path does not exist, is not a file, + or is not a readable zip archive. The error message always + names the offending path. + """ + if not self.path.is_file(): + raise InvalidProjectError(f"Package file '{self.path}' does not exist.") + try: + return zipfile.ZipFile(self.path, mode="r") + except zipfile.BadZipFile as exc: + raise InvalidProjectError( + f"Package file '{self.path}' is not a valid zip archive." + ) from exc + + def _read_settings(self, zip_file: zipfile.ZipFile) -> Dict[str, Any]: + """Read and parse the ``.rpdk-config`` entry inside the zip. + + Raises: + InvalidProjectError: If the zip does not contain ``.rpdk-config`` + or the entry cannot be parsed as JSON. + """ + if SETTINGS_FILENAME not in zip_file.namelist(): + raise InvalidProjectError( + f"Package file '{self.path}' is missing required entry " + f"'{SETTINGS_FILENAME}'." + ) + try: + with zip_file.open(SETTINGS_FILENAME, mode="r") as entry: + raw = entry.read() + return json.loads(raw.decode("utf-8")) + except (json.JSONDecodeError, UnicodeDecodeError) as exc: + raise InvalidProjectError( + f"Settings file '{SETTINGS_FILENAME}' in package " + f"'{self.path}' is not valid JSON." + ) from exc + + def _resolve_artifact_type(self, settings: Dict[str, Any]) -> str: + """Return the artifact type declared in ``settings``. + + Falls back to :data:`ARTIFACT_TYPE_RESOURCE` for backward + compatibility when the field is absent (legacy packages written + before the CLI started emitting ``artifact_type``). + + Raises: + InvalidProjectError: If ``artifact_type`` is present but is not + one of :data:`SUPPORTED_ARTIFACT_TYPES`. + """ + if "artifact_type" not in settings: + return ARTIFACT_TYPE_RESOURCE + + artifact_type = settings["artifact_type"] + if artifact_type not in SUPPORTED_ARTIFACT_TYPES: + raise InvalidProjectError( + f"Settings file '{SETTINGS_FILENAME}' in package " + f"'{self.path}' declares an unsupported artifact_type " + f"{artifact_type!r}. Expected one of " + f"{sorted(SUPPORTED_ARTIFACT_TYPES)}." + ) + return artifact_type + + def _require_type_name(self, settings: Dict[str, Any]) -> str: + """Return the ``typeName`` field from ``settings``. + + Raises: + InvalidProjectError: If ``typeName`` is missing or empty. + """ + type_name = settings.get("typeName") + if not type_name: + raise InvalidProjectError( + f"Settings file '{SETTINGS_FILENAME}' in package " + f"'{self.path}' is missing required field 'typeName'." + ) + return type_name + + def _assert_required_entries( + self, zip_file: zipfile.ZipFile, artifact_type: str + ) -> None: + """Verify that every entry required for ``artifact_type`` is present. + + Currently every supported artifact type (RESOURCE / MODULE / HOOK) + is required to ship a ``schema.json`` entry. The ``artifact_type`` + parameter is kept in the signature so that the validator can + diverge per-type without changing its caller. + + Raises: + InvalidProjectError: If a required entry is missing. + """ + del artifact_type # currently unused; future-proofed in signature + if SCHEMA_FILENAME not in zip_file.namelist(): + raise InvalidProjectError( + f"Package file '{self.path}' is missing required entry " + f"'{SCHEMA_FILENAME}'." + ) + + def _log_metadata_if_present(self, zip_file: zipfile.ZipFile) -> None: + """Log metadata about the package if ``.cfn_metadata.json`` is present. + + The metadata file is informational only. Parse failures or missing + fields must not abort validation, so problems are logged at WARNING + and execution continues. + """ + if METADATA_FILENAME not in zip_file.namelist(): + return + + try: + with zip_file.open(METADATA_FILENAME, mode="r") as entry: + raw = entry.read() + metadata = json.loads(raw.decode("utf-8")) + except (json.JSONDecodeError, UnicodeDecodeError) as exc: + LOG.warning( + "Package '%s' has a '%s' entry that could not be parsed as JSON: %s", + self.path, + METADATA_FILENAME, + exc, + ) + return + + if not isinstance(metadata, dict): + LOG.warning( + "Package '%s' has a '%s' entry that is not a JSON object; ignoring.", + self.path, + METADATA_FILENAME, + ) + return + + cli_version = metadata.get("cli-version") + if cli_version is None: + LOG.warning( + "Package '%s' has a '%s' entry without a 'cli-version' field.", + self.path, + METADATA_FILENAME, + ) + return + + LOG.info( + "Package '%s' was built with cfn-cli version %s.", + self.path, + cli_version, + ) diff --git a/tests/test_package_validator.py b/tests/test_package_validator.py new file mode 100644 index 00000000..701ba9da --- /dev/null +++ b/tests/test_package_validator.py @@ -0,0 +1,329 @@ +# Tests for rpdk.core.package_validator +# +# pylint: disable=protected-access,redefined-outer-name +"""Unit tests for :mod:`rpdk.core.package_validator`. + +These tests cover the ``cfn submit --package`` workflow: validating that a +pre-built zip file declares a supported artifact type and ships the entries +that ``RegisterType`` needs. +""" +import json +import logging +import zipfile +from dataclasses import FrozenInstanceError +from pathlib import Path +from typing import Mapping, Optional + +import pytest + +from rpdk.core.exceptions import InvalidProjectError +from rpdk.core.package_validator import ( + ARTIFACT_TYPE_HOOK, + ARTIFACT_TYPE_MODULE, + ARTIFACT_TYPE_RESOURCE, + METADATA_FILENAME, + SCHEMA_FILENAME, + SETTINGS_FILENAME, + PackageMetadata, + PackageValidator, +) + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _write_zip(path: Path, entries: Optional[Mapping[str, object]] = None) -> Path: + """Write a zip at ``path`` with ``entries``. Values may be str, bytes, or + pre-serialized JSON via a helper. + + A value of ``None`` means "do not write this entry". ``entries=None`` or + omitted writes an empty zip. + """ + entries = entries or {} + with zipfile.ZipFile(path, mode="w") as zf: + for name, content in entries.items(): + if content is None: + continue + if isinstance(content, bytes): + zf.writestr(name, content) + else: + zf.writestr(name, content) + return path + + +def _valid_settings( + artifact_type: str = ARTIFACT_TYPE_RESOURCE, + type_name: str = "Acme::Example::Widget", +) -> str: + return json.dumps({"typeName": type_name, "artifact_type": artifact_type}) + + +def _schema(type_name: str = "Acme::Example::Widget") -> str: + return json.dumps({"typeName": type_name}) + + +# --------------------------------------------------------------------------- +# PackageMetadata +# --------------------------------------------------------------------------- + + +def test_package_metadata_is_frozen(): + meta = PackageMetadata( + path=Path("/tmp/x.zip"), + type_name="A::B::C", + artifact_type=ARTIFACT_TYPE_RESOURCE, + ) + with pytest.raises(FrozenInstanceError): + meta.type_name = "X::Y::Z" # type: ignore[misc] + + +# --------------------------------------------------------------------------- +# validate() happy paths +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "artifact_type,type_name", + [ + (ARTIFACT_TYPE_RESOURCE, "Acme::Example::Widget"), + (ARTIFACT_TYPE_MODULE, "Acme::Example::Thing::MODULE"), + (ARTIFACT_TYPE_HOOK, "Acme::Example::HookGuard"), + ], +) +def test_validate_returns_metadata_for_each_artifact_type( + tmp_path, artifact_type, type_name +): + pkg = _write_zip( + tmp_path / f"{artifact_type.lower()}.zip", + { + SETTINGS_FILENAME: _valid_settings(artifact_type, type_name), + SCHEMA_FILENAME: _schema(type_name), + }, + ) + + result = PackageValidator(pkg).validate() + + assert result == PackageMetadata( + path=pkg, type_name=type_name, artifact_type=artifact_type + ) + + +def test_validate_treats_missing_artifact_type_as_resource(tmp_path): + """Legacy packages were written without ``artifact_type``. They must be + accepted and treated as RESOURCE for backward compatibility. + """ + pkg = _write_zip( + tmp_path / "legacy.zip", + { + SETTINGS_FILENAME: json.dumps({"typeName": "Legacy::Example::Thing"}), + SCHEMA_FILENAME: _schema("Legacy::Example::Thing"), + }, + ) + + result = PackageValidator(pkg).validate() + + assert result.artifact_type == ARTIFACT_TYPE_RESOURCE + assert result.type_name == "Legacy::Example::Thing" + + +# --------------------------------------------------------------------------- +# validate() error paths +# --------------------------------------------------------------------------- + + +def test_validate_rejects_missing_path(tmp_path): + missing = tmp_path / "does-not-exist.zip" + + with pytest.raises(InvalidProjectError, match="does not exist"): + PackageValidator(missing).validate() + + +def test_validate_rejects_non_zip_file(tmp_path): + non_zip = tmp_path / "not-a-zip.bin" + non_zip.write_bytes(b"these bytes are not a zip archive") + + with pytest.raises(InvalidProjectError, match="not a valid zip archive"): + PackageValidator(non_zip).validate() + + +def test_validate_rejects_missing_rpdk_config(tmp_path): + pkg = _write_zip( + tmp_path / "no-settings.zip", + {SCHEMA_FILENAME: _schema()}, + ) + + with pytest.raises( + InvalidProjectError, match=r"missing required entry '\.rpdk-config'" + ): + PackageValidator(pkg).validate() + + +def test_validate_rejects_invalid_json_in_settings(tmp_path): + pkg = _write_zip( + tmp_path / "bad-json.zip", + { + SETTINGS_FILENAME: "{not json", + SCHEMA_FILENAME: _schema(), + }, + ) + + with pytest.raises(InvalidProjectError, match="is not valid JSON"): + PackageValidator(pkg).validate() + + +def test_validate_rejects_unknown_artifact_type(tmp_path): + pkg = _write_zip( + tmp_path / "bad-artifact.zip", + { + SETTINGS_FILENAME: json.dumps( + {"typeName": "A::B::C", "artifact_type": "FOO"} + ), + SCHEMA_FILENAME: _schema("A::B::C"), + }, + ) + + with pytest.raises( + InvalidProjectError, + match=r"unsupported artifact_type 'FOO'", + ): + PackageValidator(pkg).validate() + + +def test_validate_rejects_missing_type_name(tmp_path): + pkg = _write_zip( + tmp_path / "no-type-name.zip", + { + SETTINGS_FILENAME: json.dumps({"artifact_type": ARTIFACT_TYPE_RESOURCE}), + SCHEMA_FILENAME: _schema(), + }, + ) + + with pytest.raises( + InvalidProjectError, + match=r"missing required field 'typeName'", + ): + PackageValidator(pkg).validate() + + +def test_validate_rejects_missing_schema_json(tmp_path): + pkg = _write_zip( + tmp_path / "no-schema.zip", + {SETTINGS_FILENAME: _valid_settings()}, + ) + + with pytest.raises( + InvalidProjectError, + match=r"missing required entry 'schema\.json'", + ): + PackageValidator(pkg).validate() + + +# --------------------------------------------------------------------------- +# Metadata file - best-effort, never fatal +# --------------------------------------------------------------------------- + + +def test_validate_logs_cli_version_when_metadata_present(tmp_path, caplog): + pkg = _write_zip( + tmp_path / "with-metadata.zip", + { + SETTINGS_FILENAME: _valid_settings(), + SCHEMA_FILENAME: _schema(), + METADATA_FILENAME: json.dumps({"cli-version": "1.2.3"}), + }, + ) + + with caplog.at_level(logging.INFO, logger="rpdk.core.package_validator"): + PackageValidator(pkg).validate() + + logged = [ + record.getMessage() + for record in caplog.records + if record.name == "rpdk.core.package_validator" + ] + assert any("1.2.3" in message for message in logged), logged + + +def test_validate_tolerates_broken_metadata_json(tmp_path, caplog): + pkg = _write_zip( + tmp_path / "broken-metadata.zip", + { + SETTINGS_FILENAME: _valid_settings(), + SCHEMA_FILENAME: _schema(), + METADATA_FILENAME: "{not json", + }, + ) + + # should not raise + with caplog.at_level(logging.WARNING, logger="rpdk.core.package_validator"): + result = PackageValidator(pkg).validate() + + assert result.type_name == "Acme::Example::Widget" + assert any( + "could not be parsed as JSON" in record.getMessage() + for record in caplog.records + if record.name == "rpdk.core.package_validator" + ) + + +def test_validate_tolerates_metadata_without_cli_version(tmp_path, caplog): + pkg = _write_zip( + tmp_path / "metadata-without-version.zip", + { + SETTINGS_FILENAME: _valid_settings(), + SCHEMA_FILENAME: _schema(), + METADATA_FILENAME: json.dumps({"other-field": "value"}), + }, + ) + + with caplog.at_level(logging.WARNING, logger="rpdk.core.package_validator"): + PackageValidator(pkg).validate() + + assert any( + "without a 'cli-version' field" in record.getMessage() + for record in caplog.records + if record.name == "rpdk.core.package_validator" + ) + + +def test_validate_tolerates_metadata_that_is_not_a_json_object(tmp_path, caplog): + pkg = _write_zip( + tmp_path / "metadata-list.zip", + { + SETTINGS_FILENAME: _valid_settings(), + SCHEMA_FILENAME: _schema(), + METADATA_FILENAME: json.dumps(["list", "instead", "of", "object"]), + }, + ) + + with caplog.at_level(logging.WARNING, logger="rpdk.core.package_validator"): + PackageValidator(pkg).validate() + + assert any( + "not a JSON object" in record.getMessage() + for record in caplog.records + if record.name == "rpdk.core.package_validator" + ) + + +def test_validate_succeeds_silently_when_metadata_absent(tmp_path, caplog): + pkg = _write_zip( + tmp_path / "no-metadata.zip", + { + SETTINGS_FILENAME: _valid_settings(), + SCHEMA_FILENAME: _schema(), + }, + ) + + with caplog.at_level(logging.DEBUG, logger="rpdk.core.package_validator"): + PackageValidator(pkg).validate() + + # No log record from this module should be emitted in the "happy, no metadata" + # case. Other loggers are not our concern. + logs_from_module = [ + record + for record in caplog.records + if record.name == "rpdk.core.package_validator" + ] + assert logs_from_module == [] From fcc023023936fd37804bb4b32deb2fa56621d2d9 Mon Sep 17 00:00:00 2001 From: cv-dote <74953312+cv-dote@users.noreply.github.com> Date: Mon, 4 May 2026 14:46:15 +0100 Subject: [PATCH 2/5] Add Project.from_package classmethod for pre-built zip submission --- src/rpdk/core/project.py | 24 ++++++++++++++ tests/test_project.py | 71 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/src/rpdk/core/project.py b/src/rpdk/core/project.py index 5df758ea..de13c6f8 100644 --- a/src/rpdk/core/project.py +++ b/src/rpdk/core/project.py @@ -703,6 +703,30 @@ def _add_modules_content_to_zip(self, zip_file): def _validate_fragments(template_fragment): template_fragment.validate_fragments() + @classmethod + def from_package(cls, metadata): + """Build a minimal :class:`Project` from a :class:`PackageMetadata`. + + Used by the ``cfn submit --package`` workflow, in which the caller + already has a fully-built zip and only needs to call ``_upload``. + Unlike :meth:`load`, this constructor does not read ``.rpdk-config`` + from the current working directory; every value that ``_upload`` + depends on comes from ``metadata`` or from the caller's CLI + arguments. + + The returned instance has ``schema = {}`` on purpose: this keeps + ``_upload`` out of its automatic-role branch (which would require a + role template on disk). + + :param metadata: result of + :meth:`rpdk.core.package_validator.PackageValidator.validate`. + """ + project = cls() + project.type_name = metadata.type_name + project.artifact_type = metadata.artifact_type + project.schema = {} + return project + def submit( self, dry_run, diff --git a/tests/test_project.py b/tests/test_project.py index 63016414..2fa1e0c4 100644 --- a/tests/test_project.py +++ b/tests/test_project.py @@ -30,6 +30,7 @@ InvalidProjectError, SpecValidationError, ) +from rpdk.core.package_validator import PackageMetadata from rpdk.core.plugin_base import LanguagePlugin from rpdk.core.project import ( CANARY_DEPENDENCY_FILE_NAME, @@ -3922,3 +3923,73 @@ def test_create_template_file_with_nested_add_patch_inputs(mock_yaml_dump, proje ) assert args[0] == expected_template_data_create assert kwargs + + +# --------------------------------------------------------------------------- +# Project.from_package (cfn submit --package workflow) +# --------------------------------------------------------------------------- + + +def _package_metadata( + artifact_type=ARTIFACT_TYPE_RESOURCE, + type_name="Acme::Example::Widget", + path="/tmp/fake-package.zip", +): + """Return a PackageMetadata for a pre-built package test fixture.""" + return PackageMetadata( + path=Path(path), type_name=type_name, artifact_type=artifact_type + ) + + +@pytest.mark.parametrize( + "artifact_type,type_name,expected_hyphenated", + [ + ("RESOURCE", "Acme::Example::Widget", "acme-example-widget"), + ( + "MODULE", + "Acme::Example::Thing::MODULE", + "acme-example-thing-module", + ), + ("HOOK", "Acme::Example::HookGuard", "acme-example-hookguard"), + ], +) +def test_from_package_copies_metadata(artifact_type, type_name, expected_hyphenated): + metadata = _package_metadata(artifact_type=artifact_type, type_name=type_name) + + project = Project.from_package(metadata) + + assert project.artifact_type == artifact_type + assert project.type_name == type_name + assert project.hypenated_name == expected_hyphenated + + +def test_from_package_sets_empty_schema_so_upload_skips_role_branch(): + # _upload uses ``"handlers" in self.schema`` as the gate for auto-creating + # an execution role template from disk. from_package intentionally sets + # ``schema = {}`` so that branch is never taken (the caller must supply + # --role-arn or --no-role instead). + project = Project.from_package(_package_metadata()) + + assert project.schema == {} + assert "handlers" not in project.schema + + +def test_from_package_does_not_call_load(tmp_path, monkeypatch): + """``from_package`` must not depend on a ``.rpdk-config`` in CWD. + Running it from an empty tmp_path must succeed even though no such + file exists there. + """ + monkeypatch.chdir(tmp_path) + assert not (tmp_path / ".rpdk-config").exists() + + # Also guard against accidental calls to load()/load_settings(). + with patch.object(Project, "load", autospec=True) as mock_load, patch.object( + Project, "load_settings", autospec=True + ) as mock_load_settings: + project = Project.from_package(_package_metadata()) + + assert mock_load.call_count == 0 + assert mock_load_settings.call_count == 0 + # And the built project still has the fields _upload needs. + assert project.type_name == "Acme::Example::Widget" + assert project.artifact_type == "RESOURCE" From b3b3c28534c2e44bf90a7144513d9661f204b799 Mon Sep 17 00:00:00 2001 From: cv-dote <74953312+cv-dote@users.noreply.github.com> Date: Mon, 4 May 2026 15:24:01 +0100 Subject: [PATCH 3/5] Add --package option to cfn submit --- src/rpdk/core/submit.py | 72 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/src/rpdk/core/submit.py b/src/rpdk/core/submit.py index 8d6acefd..7d0dff53 100644 --- a/src/rpdk/core/submit.py +++ b/src/rpdk/core/submit.py @@ -3,13 +3,22 @@ Projects can be created via the 'init' sub command. """ import logging +from pathlib import Path +from .exceptions import InvalidProjectError +from .package_validator import PackageValidator from .project import Project LOG = logging.getLogger(__name__) def submit(args): + # cfn submit --package : skip the packaging workflow and upload the + # pre-built zip as-is (see --package on the subparser). + if args.package: + _submit_existing_package(args) + return + project = Project() project.load() # Use CLI override opposed to config file if use-docker or no-docker switch used @@ -27,6 +36,59 @@ def submit(args): ) +def _validate_package_flag_combinations(args): + """Reject flag combinations that do not apply to ``--package``. + + ``--dry-run``, ``--use-docker`` and ``--no-docker`` all configure the + packaging step. When ``--package`` is used, the zip is already built, + so combining them is almost certainly a user mistake. + + Raises: + InvalidProjectError: If any of the packaging-only flags are set. + """ + conflicts = [] + if args.dry_run: + conflicts.append("--dry-run") + if args.use_docker: + conflicts.append("--use-docker") + if args.no_docker: + conflicts.append("--no-docker") + if conflicts: + raise InvalidProjectError( + f"--package cannot be combined with {', '.join(conflicts)}." + ) + + +def _submit_existing_package(args): + """Upload a pre-built schema handler package without repackaging. + + The flow is: + + 1. Reject flag combinations that don't make sense with ``--package``. + 2. Validate the zip's structure and read metadata out of it. + 3. Build a minimal :class:`Project` via :meth:`Project.from_package` + (no ``.rpdk-config`` in CWD is required). + 4. Stream the zip bytes straight into ``Project._upload``. + """ + _validate_package_flag_combinations(args) + + package_path = Path(args.package) + metadata = PackageValidator(package_path).validate() + project = Project.from_package(metadata) + + with open(package_path, "rb") as fileobj: + # pylint: disable=protected-access + project._upload( + fileobj, + args.endpoint_url, + args.region, + args.role_arn, + args.use_role, + args.set_default, + args.profile, + ) + + def setup_subparser(subparsers, parents): # see docstring of this file parser = subparsers.add_parser("submit", description=__doc__, parents=parents) @@ -43,6 +105,16 @@ def setup_subparser(subparsers, parents): help="If registration is successful, set submitted version to the default.", ) parser.add_argument("--profile", help="AWS profile to use.") + parser.add_argument( + "--package", + "-p", + metavar="PATH", + help=( + "Submit a pre-built schema handler package (zip) instead of " + "building one from the current project. Cannot be combined with " + "--dry-run, --use-docker, or --no-docker." + ), + ) role_group = parser.add_mutually_exclusive_group() role_group.add_argument( "--role-arn", From 8626c1719744b19facd037c8653ecfabdae04282 Mon Sep 17 00:00:00 2001 From: cv-dote <74953312+cv-dote@users.noreply.github.com> Date: Mon, 4 May 2026 15:42:50 +0100 Subject: [PATCH 4/5] Add unit tests for --package submit workflow --- tests/test_submit.py | 335 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 335 insertions(+) create mode 100644 tests/test_submit.py diff --git a/tests/test_submit.py b/tests/test_submit.py new file mode 100644 index 00000000..554192aa --- /dev/null +++ b/tests/test_submit.py @@ -0,0 +1,335 @@ +# Tests for rpdk.core.submit (the ``cfn submit`` sub-command). +# +# pylint: disable=protected-access,redefined-outer-name +"""Unit tests for :mod:`rpdk.core.submit`, covering both the classic +packaging workflow and the ``--package`` (pre-built zip) workflow added +by the submit-existing-zip feature. +""" +import argparse +import json +import zipfile +from pathlib import Path +from unittest.mock import ANY, patch + +import pytest + +from rpdk.core.exceptions import InvalidProjectError +from rpdk.core.package_validator import ( + ARTIFACT_TYPE_HOOK, + ARTIFACT_TYPE_MODULE, + ARTIFACT_TYPE_RESOURCE, + SCHEMA_FILENAME, + SETTINGS_FILENAME, +) +from rpdk.core.submit import ( + _submit_existing_package, + _validate_package_flag_combinations, + setup_subparser, + submit, +) + +# --------------------------------------------------------------------------- +# Helpers and fixtures +# --------------------------------------------------------------------------- + + +def _build_submit_parser(): + root = argparse.ArgumentParser() + subs = root.add_subparsers(dest="command") + setup_subparser(subs, []) + return root + + +@pytest.fixture +def submit_parser(): + return _build_submit_parser() + + +def _make_valid_zip( + path: Path, artifact_type=ARTIFACT_TYPE_RESOURCE, type_name="Acme::Example::Widget" +) -> Path: + with zipfile.ZipFile(path, mode="w") as zf: + zf.writestr( + SETTINGS_FILENAME, + json.dumps({"typeName": type_name, "artifact_type": artifact_type}), + ) + zf.writestr(SCHEMA_FILENAME, json.dumps({"typeName": type_name})) + return path + + +def _default_args(**overrides): + """argparse.Namespace matching what the real parser would produce.""" + base = { + "package": None, + "dry_run": False, + "endpoint_url": None, + "region": None, + "profile": None, + "role_arn": None, + "use_role": True, + "set_default": False, + "use_docker": False, + "no_docker": False, + } + base.update(overrides) + return argparse.Namespace(**base) + + +# --------------------------------------------------------------------------- +# CLI argument parsing +# --------------------------------------------------------------------------- + + +def test_parser_accepts_package_long_form(submit_parser): + args = submit_parser.parse_args(["submit", "--package", "foo.zip"]) + assert args.package == "foo.zip" + + +def test_parser_accepts_package_short_form(submit_parser): + args = submit_parser.parse_args(["submit", "-p", "foo.zip"]) + assert args.package == "foo.zip" + + +def test_parser_defaults_package_to_none(submit_parser): + args = submit_parser.parse_args(["submit"]) + assert args.package is None + + +def test_submit_help_mentions_package_option(submit_parser): + submit_sub = submit_parser._subparsers._actions[-1].choices["submit"] + help_text = submit_sub.format_help() + assert "--package" in help_text + assert "-p" in help_text + assert "pre-built" in help_text + # Restriction is explained in help so users can discover it. + assert "--dry-run" in help_text + assert "--use-docker" in help_text + + +# --------------------------------------------------------------------------- +# Flag combination validation +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "flag,expected_name", + [ + ("dry_run", "--dry-run"), + ("use_docker", "--use-docker"), + ("no_docker", "--no-docker"), + ], +) +def test_validate_rejects_conflicting_flag(flag, expected_name): + args = _default_args(package="foo.zip", **{flag: True}) + + with pytest.raises(InvalidProjectError) as exc_info: + _validate_package_flag_combinations(args) + + assert "--package" in str(exc_info.value) + assert expected_name in str(exc_info.value) + + +def test_validate_reports_all_conflicting_flags_at_once(): + args = _default_args( + package="foo.zip", dry_run=True, use_docker=True, no_docker=False + ) + + with pytest.raises(InvalidProjectError) as exc_info: + _validate_package_flag_combinations(args) + + message = str(exc_info.value) + assert "--dry-run" in message + assert "--use-docker" in message + + +def test_validate_accepts_package_with_only_safe_flags(): + args = _default_args( + package="foo.zip", + endpoint_url="https://example", + region="us-east-1", + role_arn="arn:aws:iam::123:role/X", + use_role=True, + set_default=True, + profile="default", + ) + + # Should not raise. + _validate_package_flag_combinations(args) + + +# --------------------------------------------------------------------------- +# submit() dispatch +# --------------------------------------------------------------------------- + + +def test_submit_without_package_runs_classic_workflow(): + args = _default_args(package=None) + + with patch("rpdk.core.submit.Project") as mock_project_cls, patch( + "rpdk.core.submit._submit_existing_package" + ) as mock_existing: + project = mock_project_cls.return_value + project.settings = {} + submit(args) + + assert project.load.called + assert project.submit.called + assert mock_existing.called is False + + +def test_submit_with_package_delegates_to_existing_package_path(): + args = _default_args(package="foo.zip") + + with patch("rpdk.core.submit._submit_existing_package") as mock_existing, patch( + "rpdk.core.submit.Project" + ) as mock_project_cls: + submit(args) + + mock_existing.assert_called_once_with(args) + # Classic workflow must not run. + assert not mock_project_cls.return_value.load.called + assert not mock_project_cls.return_value.submit.called + + +# --------------------------------------------------------------------------- +# _submit_existing_package integration +# --------------------------------------------------------------------------- + + +def test_existing_package_rejects_missing_file(tmp_path): + missing = tmp_path / "nothing-here.zip" + args = _default_args(package=str(missing)) + + with pytest.raises(InvalidProjectError, match="does not exist"): + _submit_existing_package(args) + + +def test_existing_package_rejects_conflicting_flag(tmp_path): + pkg = _make_valid_zip(tmp_path / "valid.zip") + args = _default_args(package=str(pkg), dry_run=True) + + with pytest.raises(InvalidProjectError, match="--dry-run"): + _submit_existing_package(args) + + +def test_existing_package_calls_upload_with_cli_options(tmp_path): + pkg = _make_valid_zip( + tmp_path / "valid.zip", + artifact_type=ARTIFACT_TYPE_RESOURCE, + type_name="Acme::Example::Widget", + ) + args = _default_args( + package=str(pkg), + endpoint_url="https://cfn.example", + region="eu-west-1", + role_arn="arn:aws:iam::123:role/X", + use_role=True, + set_default=True, + profile="myprofile", + ) + + # The file object passed to _upload is opened inside a ``with`` block in + # the production code, so by the time the test inspects the mock the + # file has already been closed. Capture the bytes inside the fake + # _upload so we can assert on them afterwards. + captured_bytes = {} + + def fake_upload(self, fileobj, *_args, **_kwargs): + captured_bytes["data"] = fileobj.read() + + with patch( + "rpdk.core.project.Project._upload", + autospec=True, + side_effect=fake_upload, + ) as mock_upload: + _submit_existing_package(args) + + call_args = mock_upload.call_args + project_self = call_args.args[0] + + assert project_self.type_name == "Acme::Example::Widget" + assert project_self.artifact_type == ARTIFACT_TYPE_RESOURCE + # schema must be empty so the auto-role branch never fires + assert project_self.schema == {} + + # CLI options are forwarded positionally (endpoint_url, region, role_arn, + # use_role, set_default, profile) exactly as Project.submit does today. + remaining = call_args.args[2:] + assert remaining == ( + "https://cfn.example", + "eu-west-1", + "arn:aws:iam::123:role/X", + True, + True, + "myprofile", + ) + + # The fileobj passed to _upload must stream back exactly the bytes of + # the zip on disk. + assert captured_bytes["data"] == pkg.read_bytes() + + +def test_existing_package_does_not_invoke_project_load(tmp_path, monkeypatch): + """The --package path must not depend on a ``.rpdk-config`` in CWD.""" + pkg = _make_valid_zip(tmp_path / "valid.zip") + # Put us in a directory that has no .rpdk-config at all. + empty_cwd = tmp_path / "other-dir" + empty_cwd.mkdir() + monkeypatch.chdir(empty_cwd) + assert not (empty_cwd / ".rpdk-config").exists() + + args = _default_args(package=str(pkg), use_role=False) + + with patch("rpdk.core.project.Project.load", autospec=True) as mock_load, patch( + "rpdk.core.project.Project._upload", autospec=True + ): + _submit_existing_package(args) + + assert mock_load.call_count == 0 + + +@pytest.mark.parametrize( + "artifact_type,type_name", + [ + (ARTIFACT_TYPE_RESOURCE, "Acme::Example::Widget"), + (ARTIFACT_TYPE_MODULE, "Acme::Example::Thing::MODULE"), + (ARTIFACT_TYPE_HOOK, "Acme::Example::HookGuard"), + ], +) +def test_existing_package_preserves_artifact_type_and_type_name( + tmp_path, artifact_type, type_name +): + pkg = _make_valid_zip( + tmp_path / f"{artifact_type.lower()}.zip", + artifact_type=artifact_type, + type_name=type_name, + ) + args = _default_args(package=str(pkg), use_role=False) + + with patch("rpdk.core.project.Project._upload", autospec=True) as mock_upload: + _submit_existing_package(args) + + project_self = mock_upload.call_args.args[0] + assert project_self.artifact_type == artifact_type + assert project_self.type_name == type_name + + +def test_existing_package_surfaces_upload_errors(tmp_path): + pkg = _make_valid_zip(tmp_path / "valid.zip") + args = _default_args(package=str(pkg), use_role=False) + + class FakeApiError(RuntimeError): + pass + + with patch( + "rpdk.core.project.Project._upload", + autospec=True, + side_effect=FakeApiError("Unknown CloudFormation error"), + ): + with pytest.raises(FakeApiError, match="CloudFormation"): + _submit_existing_package(args) + + +# ANY is imported above because some future assertions may need it; keep the +# import from being flagged as unused. +_ = ANY From 7848f0a4ce90b4dcb9a5208abf0f7d37d8dbff61 Mon Sep 17 00:00:00 2001 From: cv-dote <74953312+cv-dote@users.noreply.github.com> Date: Mon, 4 May 2026 15:56:08 +0100 Subject: [PATCH 5/5] Document --package option in submit command --- README.md | 1 + doc_source/resource-type-cli-submit.md | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/README.md b/README.md index 849194cc..d3c012e9 100644 --- a/README.md +++ b/README.md @@ -44,6 +44,7 @@ To register a resource provider, module, or hook in your account, use the `submi cfn submit cfn submit --dry-run #prepares schema handler package without submitting for registration cfn submit --set-default # if successfully registered, set submitted version to be the new default version +cfn submit --package ./my-type.zip --region us-east-1 --no-role # register a pre-built package (cannot be combined with --dry-run, --use-docker, or --no-docker) ``` ### Command: package diff --git a/doc_source/resource-type-cli-submit.md b/doc_source/resource-type-cli-submit.md index 0add620c..b2702ac5 100755 --- a/doc_source/resource-type-cli-submit.md +++ b/doc_source/resource-type-cli-submit.md @@ -27,6 +27,7 @@ The user registering the extension must be able to access the schema handler pac [--role-arn ] [--no-role] [--set-default] +[--package ] ``` ## Options @@ -67,6 +68,30 @@ You can't specify both `--role-arn` and `--no-role` arguments\. Upon successful registration of the type version, sets the current type version as the default version\. +`--package `, `-p ` + +Submit a pre-built schema handler package \(zip\) at the given path instead of building one from the current project\. The CloudFormation CLI validates the zip, reads the artifact type and `typeName` out of its `.rpdk-config` entry, and uploads the zip as-is\. This lets you build once and register the same artifact across multiple regions, or register a zip built by a third party, without rebuilding\. + +When `--package` is used, the CloudFormation CLI does not read `.rpdk-config` from the current working directory, so this option works even outside a CloudFormation CLI project\. + +`--package` cannot be combined with `--dry-run`, `--use-docker`, or `--no-docker`: those options configure the packaging step that `--package` skips\. + +Because the pre-built zip does not carry a role template, you must either specify `--role-arn` or pass `--no-role` together with `--package`\. + +## Examples + +Register a pre-built package that you built earlier with `cfn submit --dry-run`: + +``` +cfn submit --package ./my-type.zip --region us-east-1 --no-role +``` + +Register the same zip in a second region using an existing execution role: + +``` +cfn submit --package ./my-type.zip --region ap-northeast-1 --role-arn arn:aws:iam::123456789012:role/MyExecutionRole +``` + ## Output Extension registration is an asynchronous operation\. You can use the supplied registration token to track the progress of your extension registration request using the [DescribeTypeRegistration](https://docs.aws.amazon.com/AWSCloudFormation/latest/APIReference/API_DescribeTypeRegistration.html) action of the CloudFormation API\.