Merge pull request #42 from salesforce/plaird/allowlist

renovate the allowlist for dupe checking

.gitignore

@@ -5,3 +5,6 @@ bazel-*
 #   git config --global user.email '#####+username@users.noreply.github.com'
 # but dont check it in
 setGitOSSUser.sh
+
+# python
+__pycache__

tools/springboot/BUILD

@@ -16,7 +16,7 @@ exports_files([
     "verify_conflict.py",
     "write_gitinfo_properties.sh",
     "write_manifest.sh",
-    "whitelist.txt",
+    "allowlist.txt",
 ])
 
 py_test(

tools/springboot/allowlist.txt

@@ -0,0 +1,47 @@
+#
+# Copyright (c) 2019-2020, salesforce.com, inc.
+# All rights reserved.
+# Licensed under the BSD 3-Clause license.
+# For full license text, see LICENSE.txt file in the repo root  or https://opensource.org/licenses/BSD-3-Clause
+#
+
+# For a class to be ignored for dupe checking, every jar that it may appear must
+# be listed below. It is not enough to list just one of the jars.
+
+
+# TODO This is absurd that this is hard coded into the rule itself.
+# See Issue 29 https://github.com/salesforce/bazel-springboot-rule/issues/29 as the fix for this.
+# Hi guys, these are Salesforce specific target names and jar versions. Enjoy. ಠ╭╮ಠ
+
+grpc-context-1.24.0.jar
+grpc-core-1.24.0.jar
+grpc-api-1.24.0.jar
+grpc-stub-1.24.0.jar
+grpc-protobuf-1.24.0.jar
+grpc-protobuf-lite-1.24.0.jar
+
+proto-google-common-protos-1.12.0.jar
+protobuf-java-3.5.1.jar
+protobuf-java-util-3.5.1.jar
+protobuf-java-3.9.0.jar
+protobuf-java-util-3.9.0.jar
+
+libannotations_proto-speed.jar
+libany_proto-speed.jar
+libapi.jar
+libcode_proto-speed.jar
+libcontext.jar
+libcore.jar
+libdescriptor_proto-speed.jar
+libduration_proto-speed.jar
+libempty_proto-speed.jar
+liberror_details_proto-speed.jar
+libhttp_proto-speed.jar
+libprotobuf.jar
+libprotobuf_java.jar
+libprotobuf_java_util.jar
+libprotobuf-lite.jar
+librpc_protos-speed.jar
+libstatus_proto-speed.jar
+libstub.jar
+libtimestamp_proto-speed.jar

tools/springboot/run_tests.sh

@@ -0,0 +1 @@
+python3 -m unittest tests/verify_conflict_test.py 

tools/springboot/springboot.bzl

@@ -212,7 +212,7 @@ def springboot(name, java_library, boot_app_class, deps, fail_on_duplicate_class
         tools = [
             "@bazel_springboot_rule//tools/springboot:springboot_pkg.sh",
             "@bazel_springboot_rule//tools/springboot:verify_conflict.py",
-            "@bazel_springboot_rule//tools/springboot:whitelist.txt",
+            "@bazel_springboot_rule//tools/springboot:allowlist.txt",
             "@bazel_tools//tools/jdk:singlejar",
         ],
         tags = tags,

tools/springboot/springboot_pkg.sh

@@ -127,7 +127,7 @@ unzip -l $RULEDIR/$APPJAR >> $CLASSLIST_FILENAME 2>/dev/null
 
 if [[ $VERIFY_DUPE == "verify" ]]; then
     # This python script parses the jarfile to check for duplicate classes
-    python external/bazel_springboot_rule/tools/springboot/verify_conflict.py $CLASSLIST_FILENAME external/bazel_springboot_rule/tools/springboot/whitelist.txt
+    python external/bazel_springboot_rule/tools/springboot/verify_conflict.py $CLASSLIST_FILENAME external/bazel_springboot_rule/tools/springboot/allowlist.txt
     returnCode=$?
 
     if [[ $returnCode -eq 1 ]]; then

tools/springboot/tests/verify_conflict_test.py

@@ -8,14 +8,14 @@
 
 FAKE_CONT1 = "This is some class bytecode"
 FAKE_CONT2 = "This is some other class bytecode"
-WHITELIST_PATH = ""
+ALLOWLIST_PATH = ""
 
 class TestVerifyConflict(unittest.TestCase):
 
     def setUp(self):
-        global WHITELIST_PATH
+        global ALLOWLIST_PATH
         self.tempdir = tempfile.mkdtemp("TestVerifyConflict")
-        WHITELIST_PATH = self._write_whitelist_file()
+        ALLOWLIST_PATH = self._write_allowlist_file()
 
     def tearDown(self):
         shutil.rmtree(self.tempdir)
@@ -29,7 +29,7 @@ def test_single_jar_with_two_unique_class(self):
         self._run("jar tf %s" % jar_file)
 
         index_file_path = self._write_index_file([jar_file])
-        verify_conflict.run(index_file_path, WHITELIST_PATH)
+        verify_conflict.run(index_file_path, ALLOWLIST_PATH)
 
     def test_two_jars_with_duplicate_class__same_content(self):
         classes_dir1 = self._create_fake_class("MyClass.class", "classes1",
@@ -44,7 +44,7 @@ def test_two_jars_with_duplicate_class__same_content(self):
 
         index_file_path = self._write_index_file([jar_file1, jar_file2])
 
-        verify_conflict.run(index_file_path, WHITELIST_PATH)
+        verify_conflict.run(index_file_path, ALLOWLIST_PATH)
 
     def test_two_jars_with_duplicate_class__different_content(self):
         classes_dir1 = self._create_fake_class("MyClass.class", "classes1",
@@ -57,11 +57,11 @@ def test_two_jars_with_duplicate_class__different_content(self):
         index_file_path = self._write_index_file([jar_file1, jar_file2])
 
         with self.assertRaises(Exception) as ctx:
-            verify_conflict.run(index_file_path, WHITELIST_PATH)
+            verify_conflict.run(index_file_path, ALLOWLIST_PATH)
 
         self.assertIn("Found duplicate classes", str(ctx.exception))
 
-    def test_two_jars_with_duplicate_class_whitelisted__different_content(self):
+    def test_two_jars_with_duplicate_class_allowlisted__different_content(self):
         classes_dir1 = self._create_fake_class("MyClass.class", "classes1",
                                                "com/salesforce", FAKE_CONT1)
         classes_dir2 = self._create_fake_class("MyClass.class", "classes2",
@@ -71,7 +71,7 @@ def test_two_jars_with_duplicate_class_whitelisted__different_content(self):
 
         index_file_path = self._write_index_file([jar_file1, jar_file2])
 
-        verify_conflict.run(index_file_path, WHITELIST_PATH)
+        verify_conflict.run(index_file_path, ALLOWLIST_PATH)
 
     def _create_jar(self, name, classes_dir):
         jar_file = os.path.join(self.tempdir, name)
@@ -88,7 +88,7 @@ def _create_fake_class(self, name, root_dir, package, content):
         with open(class_file, "w") as f:
             f.write(content)
         return classes_dir
-            
+
     def _write_index_file(self, jar_files):
         index_file_path = os.path.join(self.tempdir, "classes.txt")
         print(index_file_path)
@@ -98,20 +98,19 @@ def _write_index_file(self, jar_files):
                 f.write(self._run("unzip -l %s" % jar_file))
         return index_file_path
 
-    def _write_whitelist_file(self):
-        whitelist_file_path = os.path.join(self.tempdir, "whitelist.txt")
-        with open(whitelist_file_path, "w") as f:
+    def _write_allowlist_file(self):
+        allowlist_file_path = os.path.join(self.tempdir, "allowlist.txt")
+        with open(allowlist_file_path, "w") as f:
                 f.write(self.tempdir+ "/myjar20.jar\n")
                 f.write(self.tempdir+ "/myjar21.jar\n")
-        return whitelist_file_path
+        return allowlist_file_path
 
     def _run(self, cmd, cwd=None):
         if not cwd:
             cwd = self.tempdir
         output = subprocess.Popen(cmd, cwd=cwd, shell=True, stdout=subprocess.PIPE).stdout.read()
         return output
-        
+
 
 if __name__ == '__main__':
     unittest.main()
-

tools/springboot/verify_conflict.py

@@ -4,15 +4,16 @@
 import sys
 import zipfile
 
-def _check_for_duplicate_classes(class_path_to_jar_paths, whitelisted_jars):
+def _check_for_duplicate_classes(class_path_to_jar_paths, allowlisted_jars):
     """
     Prints error message and returns True if duplicate classes were found,
     false otherwise.
 
-    Jars in the whitelisted_jars list are excluded from the check.
+    Jars in the allowlisted_jars list are excluded from the check.
     """
 
     found_duplicates = False
+    allowlist_violation_jars = set()
 
     for class_path, jars in class_path_to_jar_paths.items():
         if len(jars) > 1:
@@ -27,14 +28,27 @@ def _check_for_duplicate_classes(class_path_to_jar_paths, whitelisted_jars):
                 jar_path_and_md5s.append((jar_path, digest,))
                 if previous_digest is not None:
                     if previous_digest != digest:
-                        if os.path.basename(jar_path) not in whitelisted_jars or os.path.basename(previous_jar_path) not in whitelisted_jars:
+                        jar_base = os.path.basename(jar_path)
+                        prev_jar_base = os.path.basename(previous_jar_path)
+                        # we fail as a dupe if both jars are not in the allowlist
+                        # we could be nicer and only fail if one of the jars is not in the allowlist?
+                        if jar_base not in allowlisted_jars:
+                            allowlist_violation_jars.add(jar_base)
+                            all_hash_digests_match = False
+                            found_duplicates = True
+                        if prev_jar_base not in allowlisted_jars:
+                            allowlist_violation_jars.add(prev_jar_base)
                             all_hash_digests_match = False
                             found_duplicates = True
                 previous_digest = digest
                 previous_jar_path = jar_path
             if not all_hash_digests_match:
                 print("The class [%s] was found in multiple jars:\n%s\n\n" % (class_path, '\n'.join((str(t) for t in jar_path_and_md5s))))
 
+    print("Consider adding these jars to the allowlist.txt file:")
+    for allowlist_candidate in allowlist_violation_jars:
+        print(allowlist_candidate)
+
     return found_duplicates
 
 JARNAME_PREFIX = "Jarname: "
@@ -76,28 +90,28 @@ def _parse_classes_index_file(filename):
 
     return class_path_to_jar_paths
 
-def _parse_whitelisted_jars_file(whitelist_file):
+def _parse_allowlisted_jars_file(allowlist_file):
     """
-    Reads the whitelist.txt file and returns the jars as a set.
-    """        
-    whitelisted_jars = set()
-
-    with open(whitelist_file, "r") as lines:
+    Reads the allowlist.txt file and returns the jars as a set.
+    """
+    allowlisted_jars = set()
 
+    with open(allowlist_file, "r") as lines:
         for line in lines:
             line = line.strip()
             if len(line) == 0 or line.startswith("#"):
                 continue
-            # cannot use the whole jar path as it is different for generated jars on SFCI and mac
-            # this logic might need to change if two jars with the same name are part of the whitelist
-            whitelisted_jars.add(os.path.basename(line))
+            # cannot use the whole jar path as it is different for generated jars on linux and mac
+            # this logic might need to change if two jars with the same name are part of the allowlist
+            jar = os.path.basename(line)
+            allowlisted_jars.add(jar)
 
-    return whitelisted_jars
+    return allowlisted_jars
 
-def run(classes_index_file_path, whitelisted_jar_path):
-    whitelisted_jars = _parse_whitelisted_jars_file(whitelisted_jar_path)
+def run(classes_index_file_path, allowlisted_jar_path):
+    allowlisted_jars = _parse_allowlisted_jars_file(allowlisted_jar_path)
     class_path_to_jar_paths = _parse_classes_index_file(classes_index_file_path)
-    found_duplicates = _check_for_duplicate_classes(class_path_to_jar_paths, whitelisted_jars)
+    found_duplicates = _check_for_duplicate_classes(class_path_to_jar_paths, allowlisted_jars)
     if found_duplicates:
         raise Exception("Found duplicate classes in the packaged springboot jar")
 if __name__ == "__main__":