Fix wrong picking of the npm package.json

charon/cmd/command.py

@@ -170,12 +170,15 @@ def upload(
         buckets = __get_buckets(targets, conf)
         if npm_archive_type != NpmArchiveType.NOT_NPM:
             logger.info("This is a npm archive")
+            npm_root_path = root_path\
+                if root_path and root_path != "maven-repository" else "package"
             tmp_dir, succeeded = handle_npm_uploading(
                 archive_path,
                 product_key,
                 buckets=buckets,
                 aws_profile=aws_profile,
                 dir_=work_dir,
+                root_path=npm_root_path,
                 gen_sign=contain_signature,
                 key=sign_key,
                 dry_run=dryrun,
@@ -331,12 +334,15 @@ def delete(
         buckets = __get_buckets(targets, conf)
         if npm_archive_type != NpmArchiveType.NOT_NPM:
             logger.info("This is a npm archive")
+            npm_root_path = root_path\
+                if root_path and root_path != "maven-repository" else "package"
             tmp_dir, succeeded = handle_npm_del(
                 archive_path,
                 product_key,
                 buckets=buckets,
                 aws_profile=aws_profile,
                 dir_=work_dir,
+                root_path=npm_root_path,
                 dry_run=dryrun,
                 manifest_bucket_name=manifest_bucket_name
             )

charon/pkgs/npm.py

@@ -73,9 +73,10 @@ def default(self, o):
 def handle_npm_uploading(
         tarball_path: str,
         product: str,
-        buckets: List[Tuple[str, str, str, str]] = None,
+        buckets: List[Tuple[str, str, str, str]],
         aws_profile=None,
         dir_=None,
+        root_path="package",
         do_index=True,
         gen_sign=False,
         key=None,
@@ -103,7 +104,7 @@ def handle_npm_uploading(
         prefix = remove_prefix(bucket[2], "/")
         registry = bucket[3]
         target_dir, valid_paths, package_metadata = _scan_metadata_paths_from_archive(
-            tarball_path, registry, prod=product, dir__=dir_
+            tarball_path, registry, prod=product, dir__=dir_, pkg_root=root_path
         )
         if not os.path.isdir(target_dir):
             logger.error("Error: the extracted target_dir path %s does not exist.", target_dir)
@@ -230,9 +231,10 @@ def handle_npm_uploading(
 def handle_npm_del(
         tarball_path: str,
         product: str,
-        buckets: List[Tuple[str, str, str, str]] = None,
+        buckets: List[Tuple[str, str, str, str]],
         aws_profile=None,
         dir_=None,
+        root_path="package",
         do_index=True,
         dry_run=False,
         manifest_bucket_name=None
@@ -250,7 +252,7 @@ def handle_npm_del(
         Returns the directory used for archive processing and if the rollback is successful
     """
     target_dir, package_name_path, valid_paths = _scan_paths_from_archive(
-        tarball_path, prod=product, dir__=dir_
+        tarball_path, prod=product, dir__=dir_, pkg_root=root_path
     )
 
     valid_dirs = __get_path_tree(valid_paths, target_dir)
@@ -433,11 +435,15 @@ def _gen_npm_package_metadata_for_del(
     return meta_files
 
 
-def _scan_metadata_paths_from_archive(path: str, registry: str, prod="", dir__=None) ->\
-        Tuple[str, list, NPMPackageMetadata]:
+def _scan_metadata_paths_from_archive(
+    path: str, registry: str, prod="", dir__=None, pkg_root="pakage"
+) -> Tuple[str, list, NPMPackageMetadata]:
     tmp_root = mkdtemp(prefix=f"npm-charon-{prod}-", dir=dir__)
     try:
-        _, valid_paths = extract_npm_tarball(path, tmp_root, True, registry)
+        _, valid_paths = extract_npm_tarball(
+            path=path, target_dir=tmp_root, is_for_upload=True,
+            pkg_root=pkg_root, registry=registry
+        )
         if len(valid_paths) > 1:
             version = _scan_for_version(valid_paths[1])
             package = NPMPackageMetadata(version, True)
@@ -447,9 +453,13 @@ def _scan_metadata_paths_from_archive(path: str, registry: str, prod="", dir__=N
         sys.exit(1)
 
 
-def _scan_paths_from_archive(path: str, prod="", dir__=None) -> Tuple[str, str, list]:
+def _scan_paths_from_archive(
+    path: str, prod="", dir__=None, pkg_root="package"
+) -> Tuple[str, str, list]:
     tmp_root = mkdtemp(prefix=f"npm-charon-{prod}-", dir=dir__)
-    package_name_path, valid_paths = extract_npm_tarball(path, tmp_root, False)
+    package_name_path, valid_paths = extract_npm_tarball(
+        path=path, target_dir=tmp_root, is_for_upload=False, pkg_root=pkg_root
+    )
     return tmp_root, package_name_path, valid_paths
 
 

charon/utils/archive.py

@@ -46,8 +46,9 @@ def extract_zip_with_files(zf: ZipFile, target_dir: str, file_suffix: str, debug
     zf.extractall(target_dir, members=filtered)
 
 
-def extract_npm_tarball(path: str, target_dir: str, is_for_upload: bool, registry=DEFAULT_REGISTRY)\
-        -> Tuple[str, list]:
+def extract_npm_tarball(
+    path: str, target_dir: str, is_for_upload: bool, pkg_root="package", registry=DEFAULT_REGISTRY
+) -> Tuple[str, list]:
     """ Extract npm tarball will relocate the tgz file and metadata files.
         * Locate tar path ( e.g.: jquery/-/jquery-7.6.1.tgz or @types/jquery/-/jquery-2.2.3.tgz).
         * Locate version metadata path (e.g.: jquery/7.6.1 or @types/jquery/2.2.3).
@@ -56,30 +57,50 @@ def extract_npm_tarball(path: str, target_dir: str, is_for_upload: bool, registr
     valid_paths = []
     package_name_path = str()
     tgz = tarfile.open(path)
+    pkg_file = None
+    root_pkg_file_exists = True
+    try:
+        root_pkg_path = os.path.join(pkg_root, "package.json")
+        logger.debug(root_pkg_path)
+        pkg_file = tgz.getmember(root_pkg_path)
+        root_pkg_file_exists = pkg_file.isfile()
+    except KeyError:
+        root_pkg_file_exists = False
+        pkg_file = None
     tgz.extractall()
-    for f in tgz:
-        if f.name.endswith("package.json"):
-            version_data, parse_paths = __parse_npm_package_version_paths(f.path)
-            package_name_path = parse_paths[0]
-            os.makedirs(os.path.join(target_dir, parse_paths[0]))
-            tarball_parent_path = os.path.join(target_dir, parse_paths[0], "-")
-            valid_paths.append(os.path.join(tarball_parent_path, _get_tgz_name(path)))
-            version_metadata_parent_path = os.path.join(
-                target_dir, parse_paths[0], parse_paths[1]
+    if not root_pkg_file_exists:
+        logger.info(
+            "Root package.json is not found for archive: %s, will search others",
+            path
+        )
+        for f in tgz:
+            if f.name.endswith("package.json"):
+                logger.info("Found package.json as %s", f.path)
+                pkg_file = f
+                break
+    if pkg_file:
+        version_data, parse_paths = __parse_npm_package_version_paths(pkg_file.path)
+        package_name_path = parse_paths[0]
+        os.makedirs(os.path.join(target_dir, parse_paths[0]))
+        tarball_parent_path = os.path.join(target_dir, parse_paths[0], "-")
+        valid_paths.append(os.path.join(tarball_parent_path, _get_tgz_name(path)))
+        version_metadata_parent_path = os.path.join(
+            target_dir, parse_paths[0], parse_paths[1]
+        )
+        valid_paths.append(os.path.join(version_metadata_parent_path, "package.json"))
+
+        if is_for_upload:
+            tgz_relative_path = "/".join([parse_paths[0], "-", _get_tgz_name(path)])
+            __write_npm_version_dist(
+                path, pkg_file.path, version_data, tgz_relative_path, registry
             )
-            valid_paths.append(os.path.join(version_metadata_parent_path, "package.json"))
-
-            if is_for_upload:
-                tgz_relative_path = "/".join([parse_paths[0], "-", _get_tgz_name(path)])
-                __write_npm_version_dist(path, f.path, version_data, tgz_relative_path, registry)
-
-                os.makedirs(tarball_parent_path)
-                target = os.path.join(tarball_parent_path, os.path.basename(path))
-                shutil.copyfile(path, target)
-                os.makedirs(version_metadata_parent_path)
-                target = os.path.join(version_metadata_parent_path, os.path.basename(f.path))
-                shutil.copyfile(f.path, target)
-            break
+
+            os.makedirs(tarball_parent_path)
+            target = os.path.join(tarball_parent_path, os.path.basename(path))
+            shutil.copyfile(path, target)
+            os.makedirs(version_metadata_parent_path)
+            target = os.path.join(version_metadata_parent_path, os.path.basename(pkg_file.path))
+            shutil.copyfile(pkg_file.path, target)
     return package_name_path, valid_paths
 
 

tests/__init__.py

@@ -0,0 +1,20 @@
+"""
+Copyright (C) 2022 Red Hat, Inc. (https://github.com/Commonjava/charon)
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+         http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+"""
+
+import logging
+
+logging.basicConfig(level=logging.INFO)
+logging.getLogger("charon").setLevel(logging.DEBUG)

tests/test_npm_upload_diff_pkgs.py

@@ -0,0 +1,109 @@
+"""
+Copyright (C) 2022 Red Hat, Inc. (https://github.com/Commonjava/charon)
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+         http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+"""
+import os
+
+from moto import mock_s3
+
+from charon.pkgs.npm import handle_npm_uploading
+from charon.constants import DEFAULT_REGISTRY
+from tests.base import PackageBaseTest
+from tests.commons import TEST_BUCKET
+from tests.constants import INPUTS
+import logging
+
+logger = logging.getLogger(f"charon.tests.{__name__}")
+
+CODE_FRAME_FILES_REDHAT = [
+    "@redhat/code-frame/7.14.5/package.json",
+    "@redhat/code-frame/-/code-frame-7.14.5-multi-pkgs.tgz"
+]
+
+CODE_FRAME_META_REDHAT = "@redhat/code-frame/package.json"
+
+CODE_FRAME_FILES_BABEL = [
+    "@babel/code-frame/7.14.5/package.json",
+    "@babel/code-frame/-/code-frame-7.14.5-no-root-pkg.tgz"
+]
+
+CODE_FRAME_META_BABEL = "@babel/code-frame/package.json"
+
+
+@mock_s3
+class NPMUploadTest(PackageBaseTest):
+
+    def test_npm_uploads_multi_pkgjson_with_root(self):
+        test_tgz = os.path.join(INPUTS, "code-frame-7.14.5-multi-pkgs.tgz")
+        product_7_14_5 = "code-frame-7.14.5"
+        handle_npm_uploading(
+            test_tgz, product_7_14_5,
+            buckets=[('', TEST_BUCKET, '', DEFAULT_REGISTRY)],
+            dir_=self.tempdir, do_index=False
+        )
+        test_bucket = self.mock_s3.Bucket(TEST_BUCKET)
+        objs = list(test_bucket.objects.all())
+        actual_files = [obj.key for obj in objs]
+        logger.debug("actual_files: %s", actual_files)
+        self.assertEqual(5, len(actual_files))
+
+        for f in CODE_FRAME_FILES_REDHAT:
+            self.assertIn(f, actual_files)
+            self.check_product(f, [product_7_14_5])
+        self.assertIn(CODE_FRAME_META_REDHAT, actual_files)
+
+        meta_obj_client = test_bucket.Object(CODE_FRAME_META_REDHAT)
+        meta_content_client = str(meta_obj_client.get()["Body"].read(), "utf-8")
+        self.assertIn("\"name\": \"@redhat/code-frame\"", meta_content_client)
+        self.assertIn("\"description\": \"Generate errors that contain a code frame that point to "
+                      "source locations.\"", meta_content_client)
+        self.assertIn("\"repository\": {\"type\": \"git\", \"url\": "
+                      "\"https://github.com/babel/babel.git\"", meta_content_client)
+        self.assertIn("\"version\": \"7.14.5\"", meta_content_client)
+        self.assertIn("\"versions\": {", meta_content_client)
+        self.assertIn("\"7.14.5\": {\"name\":", meta_content_client)
+        self.assertIn("\"license\": \"MIT\"", meta_content_client)
+        self.assertNotIn("\"dist_tags\":", meta_content_client)
+
+    def test_npm_uploads_multi_pkgjson_with_no_root(self):
+        test_tgz = os.path.join(INPUTS, "code-frame-7.14.5-no-root-pkg.tgz")
+        product_7_14_5 = "code-frame-7.14.5"
+        handle_npm_uploading(
+            test_tgz, product_7_14_5,
+            buckets=[('', TEST_BUCKET, '', DEFAULT_REGISTRY)],
+            dir_=self.tempdir, do_index=False
+        )
+        test_bucket = self.mock_s3.Bucket(TEST_BUCKET)
+        objs = list(test_bucket.objects.all())
+        actual_files = [obj.key for obj in objs]
+        logger.debug("actual_files: %s", actual_files)
+        self.assertEqual(5, len(actual_files))
+
+        for f in CODE_FRAME_FILES_BABEL:
+            self.assertIn(f, actual_files)
+            self.check_product(f, [product_7_14_5])
+        self.assertIn(CODE_FRAME_META_BABEL, actual_files)
+
+        meta_obj_client = test_bucket.Object(CODE_FRAME_META_BABEL)
+        meta_content_client = str(meta_obj_client.get()["Body"].read(), "utf-8")
+        self.assertIn("\"name\": \"@babel/code-frame\"", meta_content_client)
+        self.assertIn("\"description\": \"Generate errors that contain a code frame that point to "
+                      "source locations.\"", meta_content_client)
+        self.assertIn("\"repository\": {\"type\": \"git\", \"url\": "
+                      "\"https://github.com/babel/babel.git\"", meta_content_client)
+        self.assertIn("\"version\": \"7.14.5\"", meta_content_client)
+        self.assertIn("\"versions\": {", meta_content_client)
+        self.assertIn("\"7.14.5\": {\"name\":", meta_content_client)
+        self.assertIn("\"license\": \"MIT\"", meta_content_client)
+        self.assertNotIn("\"dist_tags\":", meta_content_client)