Allow artifact base names for S3 uploads to differ from GL canonical names

Related: #4268
Signed-off-by: Tobias Wolf <wolf@b1-systems.de>
On-behalf-of: SAP <tobias.wolf@sap.com>

Author: NotTheEvilOne

src/gardenlinux/s3/__main__.py

@@ -9,11 +9,6 @@
 
 from .s3_artifacts import S3Artifacts
 
-_ARGS_ACTION_ALLOWED = [
-    "download-artifacts-from-bucket",
-    "upload-artifacts-to-bucket",
-]
-
 
 def main() -> None:
     """
@@ -25,17 +20,22 @@ def main() -> None:
     parser = argparse.ArgumentParser()
 
     parser.add_argument("--bucket", dest="bucket")
-    parser.add_argument("--cname", required=False, dest="cname")
     parser.add_argument("--path", required=False, dest="path")
     parser.add_argument("--dry-run", action="store_true")
 
-    parser.add_argument("action", nargs="?", choices=_ARGS_ACTION_ALLOWED)
+    subparsers = parser.add_subparsers(dest="action")
+
+    download_parser = subparsers.add_parser("download-artifacts-from-bucket")
+    download_parser.add_argument("--cname", required=False, dest="cname")
+
+    upload_parser = subparsers.add_parser("upload-artifacts-to-bucket")
+    upload_parser.add_argument("--artifact-name", required=False, dest="artifact_name")
 
     args = parser.parse_args()
 
     if args.action == "download-artifacts-from-bucket":
         S3Artifacts(args.bucket).download_to_directory(args.cname, args.path)
     elif args.action == "upload-artifacts-to-bucket":
         S3Artifacts(args.bucket).upload_from_directory(
-            args.cname, args.path, dry_run=args.dry_run
+            args.artifact_name, args.path, dry_run=args.dry_run
         )

src/gardenlinux/s3/s3_artifacts.py

@@ -98,15 +98,15 @@ def download_to_directory(
 
     def upload_from_directory(
         self,
-        cname: str,
+        base_name: str,
         artifacts_dir: PathLike[str] | str,
         delete_before_push: bool = False,
         dry_run: bool = False,
     ) -> None:
         """
         Pushes S3 artifacts to the underlying bucket.
 
-        :param cname:              Canonical name of the GardenLinux S3 artifacts
+        :param base_name:          Base name of the GardenLinux S3 artifacts
         :param artifacts_dir:      Path of the image artifacts
         :param delete_before_push: True to delete objects before upload
 
@@ -115,35 +115,32 @@ def upload_from_directory(
 
         artifacts_dir = Path(artifacts_dir)
 
-        cname_object = CName(cname)
-
         if not artifacts_dir.is_dir():
             raise RuntimeError(f"Artifacts directory given is invalid: {artifacts_dir}")
 
-        release_file = artifacts_dir.joinpath(f"{cname}.release")
-        release_timestamp = stat(release_file).st_ctime
-
-        cname_object.load_from_release_file(release_file)
+        release_file = artifacts_dir.joinpath(f"{base_name}.release")
 
-        if cname_object.arch is None:
-            raise RuntimeError(
-                "Architecture could not be determined from GardenLinux canonical name or release file"
-            )
+        cname_object = CName.new_from_release_file(release_file)
 
         if cname_object.version_and_commit_id is None:
             raise RuntimeError(
-                "Version information could not be determined from GardenLinux canonical name or release file"
+                "Version information could not be determined from release file"
             )
 
+        arch = cname_object.arch
         feature_list = cname_object.feature_set
-        requirements_file = artifacts_dir.joinpath(f"{cname}.requirements")
+        release_timestamp = stat(release_file).st_ctime
+        requirements_file = artifacts_dir.joinpath(f"{base_name}.requirements")
         require_uefi = None
         secureboot = None
 
         if requirements_file.exists():
             requirements_config = ConfigParser(allow_unnamed_section=True)
             requirements_config.read(requirements_file)
 
+            if requirements_config.has_option(UNNAMED_SECTION, "arch"):
+                arch = requirements_config.get(UNNAMED_SECTION, "arch")
+
             if requirements_config.has_option(UNNAMED_SECTION, "uefi"):
                 require_uefi = requirements_config.getboolean(UNNAMED_SECTION, "uefi")
 
@@ -152,16 +149,25 @@ def upload_from_directory(
                     UNNAMED_SECTION, "secureboot"
                 )
 
+        if arch is None:
+            raise RuntimeError(
+                "Architecture could not be determined from release or requirements file"
+            )
+
         if require_uefi is None:
             require_uefi = "_usi" in feature_list
 
         if secureboot is None:
             secureboot = "_trustedboot" in feature_list
 
-        commit_hash = cname_object.commit_hash
+        # RegEx for S3 supported characters
+        re_object = re.compile("[^a-zA-Z0-9\\s+\\-=.\\_:/@]")
+
+        arch = re_object.sub("+", arch)
+        commit_id_or_hash = cname_object.commit_hash
 
-        if commit_hash is None:
-            commit_hash = ""
+        if commit_id_or_hash is None:
+            commit_id_or_hash = cname_object.commit_id
 
         version_epoch = str(cname_object.version_epoch)
 
@@ -170,9 +176,9 @@ def upload_from_directory(
 
         metadata = {
             "platform": cname_object.feature_set_platform,
-            "architecture": cname_object.arch,
+            "architecture": arch,
             "base_image": None,
-            "build_committish": commit_hash,
+            "build_committish": commit_id_or_hash,
             "build_timestamp": datetime.fromtimestamp(release_timestamp).isoformat(),
             "gardenlinux_epoch": {version_epoch},
             "logs": None,
@@ -181,7 +187,7 @@ def upload_from_directory(
             "secureboot": secureboot,
             "published_image_metadata": None,
             "s3_bucket": self._bucket.name,
-            "s3_key": f"meta/singles/{cname}",
+            "s3_key": f"meta/singles/{base_name}",
             "test_result": None,
             "version": cname_object.version,
             "paths": [],
@@ -192,39 +198,34 @@ def upload_from_directory(
         if platform_variant is not None:
             metadata["platform_variant"] = platform_variant
 
-        re_object = re.compile("[^a-zA-Z0-9\\s+\\-=.\\_:/@]")
+        base_name_length = len(base_name)
 
         for artifact in artifacts_dir.iterdir():
-            if not artifact.match(f"{cname}*"):
+            if not artifact.match(f"{base_name}*"):
                 continue
 
-            if not artifact.name.startswith(cname):
-                raise RuntimeError(
-                    f"Artifact name '{artifact.name}' does not start with cname '{cname}'"
-                )
-
-            s3_key = f"objects/{cname}/{artifact.name}"
+            s3_key = f"objects/{base_name}/{artifact.name}"
 
             with artifact.open("rb") as fp:
                 md5sum = file_digest(fp, "md5").hexdigest()
                 sha256sum = file_digest(fp, "sha256").hexdigest()
 
-            suffix = artifact.name[len(cname) :]
+            suffixes = "".join(artifact.name)[1 + base_name_length :]
 
             artifact_metadata = {
                 "name": artifact.name,
                 "s3_bucket_name": self._bucket.name,
                 "s3_key": s3_key,
-                "suffix": suffix,
+                "suffix": re_object.sub("+", suffixes),
                 "md5sum": md5sum,
                 "sha256sum": sha256sum,
             }
 
             s3_tags = {
-                "architecture": re_object.sub("+", cname_object.arch),
+                "architecture": arch,
                 "platform": re_object.sub("+", cname_object.platform),
                 "version": re_object.sub("+", cname_object.version),  # type: ignore[arg-type]
-                "committish": commit_hash,
+                "committish": commit_id_or_hash,
                 "md5sum": md5sum,
                 "sha256sum": sha256sum,
             }
@@ -246,13 +247,15 @@ def upload_from_directory(
         else:
             if delete_before_push:
                 self._bucket.delete_objects(
-                    Delete={"Objects": [{"Key": f"meta/singles/{cname}"}]}
+                    Delete={"Objects": [{"Key": f"meta/singles/{base_name}"}]}
                 )
 
             with TemporaryFile(mode="wb+") as fp:
                 fp.write(yaml.dump(metadata).encode("utf-8"))
                 fp.seek(0)
 
                 self._bucket.upload_fileobj(
-                    fp, f"meta/singles/{cname}", ExtraArgs={"ContentType": "text/yaml"}
+                    fp,
+                    f"meta/singles/{base_name}",
+                    ExtraArgs={"ContentType": "text/yaml"},
                 )

tests/s3/conftest.py

@@ -27,12 +27,12 @@ def make_cname(
     flavor: str = "container",
     arch: str = "amd64",
     version: str = "1234.1",
-    commit: str = "abc123",
+    commit: str = "abc123long",
 ) -> str:
     """
     Helper function to build cname. Can be used to customized the cname.
     """
-    return f"{flavor}-{arch}-{version}-{commit}"
+    return f"{flavor}-{arch}-{version}-{commit[:8]}"
 
 
 # Helpers to compute digests for fake files

tests/s3/test_main.py

@@ -15,11 +15,11 @@
                 "__main__.py",
                 "--bucket",
                 "test-bucket",
-                "--cname",
-                "test-cname",
                 "--path",
                 "some/path",
                 "download-artifacts-from-bucket",
+                "--cname",
+                "test-cname",
             ],
             "download_to_directory",
             ["test-cname", "some/path"],
@@ -30,11 +30,11 @@
                 "__main__.py",
                 "--bucket",
                 "test-bucket",
-                "--cname",
-                "test-cname",
                 "--path",
                 "some/path",
                 "upload-artifacts-to-bucket",
+                "--artifact-name",
+                "test-cname",
             ],
             "upload_from_directory",
             ["test-cname", "some/path"],

tests/s3/test_s3_artifacts.py

@@ -13,7 +13,7 @@
 RELEASE_DATA = """
 GARDENLINUX_CNAME="container-amd64-1234.1"
 GARDENLINUX_VERSION=1234.1
-GARDENLINUX_COMMIT_ID="abc123"
+GARDENLINUX_COMMIT_ID="abc123lo"
 GARDENLINUX_COMMIT_ID_LONG="abc123long"
 GARDENLINUX_FEATURES="_usi,_trustedboot"
 GARDENLINUX_FEATURES_ELEMENTS=
@@ -174,9 +174,9 @@ def test_upload_from_directory_invalid_dir_raises(s3_setup: S3Env) -> None:
         artifacts.upload_from_directory(env.cname, "/invalid/path")
 
 
-def test_upload_from_directory_version_mismatch_raises(s3_setup: S3Env) -> None:
+def test_upload_from_directory_version_mismatch(s3_setup: S3Env) -> None:
     """
-    RuntimeError if version in release file does not match cname.
+    Validate that the release file may contain a different version not matching the artifact name.
     """
     # Arrange
     env = s3_setup
@@ -186,8 +186,7 @@ def test_upload_from_directory_version_mismatch_raises(s3_setup: S3Env) -> None:
     artifacts = S3Artifacts(env.bucket_name)
 
     # Act / Assert
-    with pytest.raises(RuntimeError, match="failed consistency check"):
-        artifacts.upload_from_directory(env.cname, env.tmp_path)
+    artifacts.upload_from_directory(env.cname, env.tmp_path)
 
 
 def test_upload_from_directory_succeeds_because_of_release_file(
@@ -226,8 +225,10 @@ def test_upload_from_directory_invalid_artifact_name(s3_setup: S3Env) -> None:
     assert len(list(bucket.objects.filter(Prefix=f"meta/singles/{env.cname}"))) == 1
 
 
-def test_upload_from_directory_commit_mismatch_raises(s3_setup: S3Env) -> None:
-    """Raise RuntimeError when commit ID is not matching with cname."""
+def test_upload_from_directory_commit_mismatch(s3_setup: S3Env) -> None:
+    """
+    Validate that the release file may contain a different commit hash not matching the artifact name.
+    """
     # Arrange
     env = s3_setup
     release_path = env.tmp_path / f"{env.cname}.release"
@@ -236,8 +237,7 @@ def test_upload_from_directory_commit_mismatch_raises(s3_setup: S3Env) -> None:
     artifacts = S3Artifacts(env.bucket_name)
 
     # Act / Assert
-    with pytest.raises(RuntimeError, match="failed consistency check"):
-        artifacts.upload_from_directory(env.cname, env.tmp_path)
+    artifacts.upload_from_directory(env.cname, env.tmp_path)
 
 
 def test_upload_from_directory_with_platform_variant(s3_setup: S3Env) -> None: