Repository: vmware/tern (python)

Issue:
Split "License" into "License" and "Copyright"
**Description**
There is a difference between the License name and the actual Copyright text. This means that the properties in the Package class need to change and the retrieval methods need to change

**Implementation**
1. Add "copyright" to the Package class
2. Add a method of retrieval for "copyright" text

**References**
Some places in the code base to look for reference implementations if any

**Super Issues**
#239


Pull request description:
Allow for Copyright text metadata
This PR resolves #245 

It contains changes to the Package object to include a 'copyright' property.
This also changes the listing in base.yml. It would report a warning for any
package manager listing that does not include a copyright listing. General
functionality is not affected.

Patch:
diff --git a/tern/classes/package.py b/tern/classes/package.py
index edab9ae..36289c9 100644
--- a/tern/classes/package.py
+++ b/tern/classes/package.py
@@ -4,6 +4,7 @@
 # SPDX-License-Identifier: BSD-2-Clause
 #
 
+from tern.classes.notice import Notice
 from tern.classes.origins import Origins
 from tern.utils.general import prop_names
 
@@ -13,7 +14,8 @@ class Package:
     attributes:
         name: package name
         version: package version
-        pkg_license: package license
+        pkg_license: package license that is declared
+        copyright: copyright text
         src_url: package source url
         origins: a list of NoticeOrigin objects
 
@@ -25,6 +27,7 @@ class Package:
         self.__name = name
         self.__version = ''
         self.__pkg_license = ''
+        self.__copyright = ''
         self.__src_url = ''
         self.__origins = Origins()
 
@@ -45,8 +48,16 @@ class Package:
         return self.__pkg_license
 
     @pkg_license.setter
-    def pkg_license(self, pkg_license):  
-        self.__pkg_license =pkg_license
+    def pkg_license(self, pkg_license):
+        self.__pkg_license = pkg_license
+
+    @property
+    def copyright(self):
+        return self.__copyright
+
+    @copyright.setter
+    def copyright(self, text):
+        self.__copyright = text
 
     @property
     def src_url(self):
@@ -83,11 +94,25 @@ class Package:
             pkg_dict.update({'origins': self.origins.to_dict()})
         return pkg_dict
 
+    def __fill_properties(self, package_dict):
+        '''Check to see if the dictionary keys have all the properties
+        listed. If not then put a Notice object in the list of Origins
+        package_dict should not contain 'name' '''
+        for key, prop in prop_names(self):
+            if prop not in ('name', 'origins'):
+                if prop not in package_dict.keys():
+                    self.origins.add_notice_to_origins(
+                        self.name, Notice(
+                            "No metadata for key: {}".format(prop), 'warning'))
+                else:
+                    self.__dict__[key] = package_dict[prop]
+
     def fill(self, package_dict):
         '''The package dict looks like this:
             name: <name>
             version: <version>
-            pkg_license: <packagelicense string>
+            pkg_license: <package license string>
+            copyright: <package copyright text>
             src_url: <source url>
         the way to use this method is to instantiate the class with the
         name and then give it a package dictionary to fill in the rest
@@ -95,9 +120,7 @@ class Package:
         the object, false if not'''
         success = True
         if self.name == package_dict['name']:
-            self.version = package_dict['version']
-            self.pkg_license = package_dict['pkg_license']
-            self.src_url = package_dict['src_url']
+            self.__fill_properties(package_dict)
         else:
             success = False
         return success
diff --git a/tern/command_lib/base.yml b/tern/command_lib/base.yml
index 7a02e6e..f14faea 100644
--- a/tern/command_lib/base.yml
+++ b/tern/command_lib/base.yml
@@ -73,7 +73,7 @@ dpkg:
           - "pkgs=`dpkg --get-selections | cut -f1 -d':' | awk '{print $1}'`"
           - "for p in $pkgs; do dpkg -l $p | awk 'NR>5 {print $3}'; done"
     delimiter: "\n"
-  licenses:
+  copyrights:
     invoke:
       1:
         container:
diff --git a/tern/command_lib/command_lib.py b/tern/command_lib/command_lib.py
index 5f73aac..b634794 100644
--- a/tern/command_lib/command_lib.py
+++ b/tern/command_lib/command_lib.py
@@ -22,7 +22,8 @@ from tern.report import errors
 # base image command library
 base_file = pkg_resources.resource_filename('tern', 'command_lib/base.yml')
 # general snippets in command library
-snippet_file = pkg_resources.resource_filename('tern', 'command_lib/snippets.yml')
+snippet_file = pkg_resources.resource_filename('tern',
+                                               'command_lib/snippets.yml')
 # command library
 command_lib = {'base': {}, 'snippets': {}}
 with open(os.path.abspath(base_file)) as f:
@@ -30,8 +31,8 @@ with open(os.path.abspath(base_file)) as f:
 with open(os.path.abspath(snippet_file)) as f:
     command_lib['snippets'] = yaml.safe_load(f)
 # list of package information keys that the command library can accomodate
-base_keys = {'names', 'versions', 'licenses', 'src_urls', 'srcs'}
-package_keys = {'name', 'version', 'src_url', 'license', 'src'}
+base_keys = {'names', 'versions', 'licenses', 'copyrights', 'src_urls', 'srcs'}
+package_keys = {'name', 'version', 'license', 'copyright', 'src_url', 'src'}
 
 # global logger
 logger = logging.getLogger(constants.logger_name)
diff --git a/tern/command_lib/snippets.yml b/tern/command_lib/snippets.yml
index 30a8911..aa35194 100644
--- a/tern/command_lib/snippets.yml
+++ b/tern/command_lib/snippets.yml
@@ -11,41 +11,8 @@ apt-get:
   remove: 'purge' # subcommand to remove a package
   ignore: # list of subcommands that don't add or remove packages
     - 'update' 
-  packages:
-    - name: default
-      version:
-        # either a version number or a script snippet
-        invoke:
-          1:
-            container:
-              - "dpkg -l {package} | awk 'NR>5 {print $3}'"
-      src_url:
-        invoke:
-          1:
-            container:
-              - "apt-get download --print-uris {package}"
-      license:
-        invoke:
-          1:
-            container:
-              - "cat /usr/share/doc/{package}/copyright"
-      deps:
-        invoke:
-          1:
-            container:
-              - "apt-cache depends --installed --no-suggests {package} | awk 'NR>1 {print $2}'"
-        delimiter: "\n"
-      src:
-        # copy_in: used to copy into the running container
-        copy_in:
-            - from: 'config/{image}/{tag}/sources.list'
-              to: '/etc/apt/sources.list'
-            - from: 'config/image/apt_get_sources.sh'
-              to: '/'
-        invoke:
-            1:
-              container:
-                - './apt_get_sources.sh {package}'
+  packages: 'dpkg' # refer to base.yml's method of collection
+
 tyum:
   install: 'install'
   remove: 'remove'
@@ -65,31 +32,7 @@ tdnf:
 apk:
   install: 'add'
   remove: 'del'
-  # ignore:
-  packages:
-    - name: default
-      version:
-        invoke:
-          1:
-            container:
-                # use double quotes when using awk
-              - "apk info {package} 2>/dev/null | head -1 | awk '{print $1}'"
-      license:
-        invoke:
-          1:
-            container:
-              - 'apk -a info {package} 2>/dev/null | tail -2 | head -1'
-      src_url:
-        invoke:
-          1:
-            container:
-              - 'apk info {package} 2>/dev/null | head -5 | tail -1'
-      deps:
-        invoke:
-          1:
-            container:
-              - 'apk -R info python 2>/dev/null | tail -n +2 | head -n -1 | cut -f2 -d":" | cut -f1 -d"."'
-        delimiter: "\n"
+  packages: 'apk'
 
 pacman:
   install: '-Syu'
diff --git a/tern/common.py b/tern/common.py
index 62906f8..f5bf197 100644
--- a/tern/common.py
+++ b/tern/common.py
@@ -81,7 +81,54 @@ def get_base_bin():
     return binary
 
 
-def add_base_packages(image_layer, binary, shell):  # pylint: disable=too-many-locals
+def collate_list_metadata(shell, listing):
+    '''Given the shell and the listing for the package manager, collect
+    metadata that gets returned as a list'''
+    pkg_dict = {}
+    msgs = ''
+    warnings = ''
+    for item in command_lib.base_keys:
+        if item in listing.keys():
+            items, msg = command_lib.get_pkg_attr_list(shell, listing[item])
+            msgs = msgs + msg
+            pkg_dict.update({item: items})
+        else:
+            warnings = warnings + errors.no_listing_for_base_key.format(
+                listing_key=item)
+    return pkg_dict, msgs, warnings
+
+
+def convert_to_pkg_dicts(pkg_dict):
+    '''The pkg_dict is what gets returned after collecting individual
+    metadata as a list. It looks like this if property collected:
+        {'names': [....], 'versions': [...], 'licenses': [...], ....}
+    Convert these into a package dictionary expected by the Package
+    Object'''
+    mapping = {'name': 'names',
+               'version': 'versions',
+               'pkg_license': 'licenses',
+               'copyright': 'copyrights',
+               'src_url': 'src_urls'}
+    pkg_list = []
+    len_names = len(pkg_dict['names'])
+    # make a list of keys that correspond with package property names
+    new_dict = {}
+    for key, value in mapping.items():
+        if value in pkg_dict.keys():
+            if len(pkg_dict[value]) == len_names:
+                new_dict.update({key: pkg_dict[value]})
+            else:
+                logger.warning("Inconsistent lengths for key: %s", value)
+    # convert each of the keys into package dictionaries
+    for index, _ in enumerate(new_dict['name']):
+        a_pkg = {}
+        for key, value in new_dict.items():
+            a_pkg.update({key: value[index]})
+        pkg_list.append(a_pkg)
+    return pkg_list
+
+
+def add_base_packages(image_layer, binary, shell):
     '''Given the image layer, the binary to invoke and shell:
         1. get the listing from the base.yml
         2. Invoke any commands against the base layer
@@ -107,27 +154,18 @@ def add_base_packages(image_layer, binary, shell):  # pylint: disable=too-many-l
         if not shell:
             shell = constants.shell
         # get all the packages in the base layer
-        names, n_msg = command_lib.get_pkg_attr_list(shell, listing['names'])
-        versions, v_msg = command_lib.get_pkg_attr_list(
-            shell, listing['versions'])
-        licenses, l_msg = command_lib.get_pkg_attr_list(
-            shell, listing['licenses'])
-        src_urls, u_msg = command_lib.get_pkg_attr_list(
-            shell, listing['src_urls'])
-        # add a notice to the image if something went wrong
-        invoke_msg = n_msg + v_msg + l_msg + u_msg
+        pkg_dict, invoke_msg, warnings = collate_list_metadata(shell, listing)
         if invoke_msg:
             image_layer.origins.add_notice_to_origins(
                 origin_layer, Notice(invoke_msg, 'error'))
-        if names and len(names) > 1:
-            for index, name in enumerate(names):
-                pkg = Package(name)
-                if len(versions) == len(names):
-                    pkg.version = versions[index]
-                if len(licenses) == len(names):
-                    pkg.license = licenses[index]
-                if len(src_urls) == len(names):
-                    pkg.src_url = src_urls[index]
+        if warnings:
+            image_layer.origins.add_notice_to_origins(
+                origin_command_lib, Notice(warnings, 'warning'))
+        if 'names' in pkg_dict and len(pkg_dict['names']) > 1:
+            pkg_list = convert_to_pkg_dicts(pkg_dict)
+            for pkg_dict in pkg_list:
+                pkg = Package(pkg_dict['name'])
+                pkg.fill(pkg_dict)
                 image_layer.add_package(pkg)
     # if there is no listing add a notice
     else:
diff --git a/tern/report/content.py b/tern/report/content.py
index 0c9b0f9..2afe265 100644
--- a/tern/report/content.py
+++ b/tern/report/content.py
@@ -18,7 +18,7 @@ def print_invoke_list(info_dict, info):
     package information.
     info_dict: get_base_listing or get_command_listing from
     command_lib/command_lib.py
-    info: the required key (niames, versions, licenses, etc)'''
+    see command_lib/command_lib.py for a list of supported info items'''
     report = ''
     if 'invoke' in info_dict[info]:
         report = report + info + ':\n'
@@ -39,10 +39,9 @@ def print_base_invoke(key):
     the command_lib/base.yml'''
     info = command_lib.get_base_listing(key)
     report = ''
-    report = report + print_invoke_list(info, 'names')
-    report = report + print_invoke_list(info, 'versions')
-    report = report + print_invoke_list(info, 'licenses')
-    report = report + print_invoke_list(info, 'src_urls')
+    for item in command_lib.base_keys:
+        if item in info.keys():
+            report = report + print_invoke_list(info, item)
     report = report + '\n'
     return report
 
@@ -74,7 +73,9 @@ def print_package(pkg_obj, prefix):
         package_url=pkg_obj.src_url)
     notes = notes + prefix + formats.package_license.format(
         package_license=pkg_obj.pkg_license)
-    notes = notes + '\n'
+    notes = notes + prefix + formats.package_copyright.format(
+        package_copyright=pkg_obj.copyright)
+    notes = notes + '\n\n'
     return notes
 
 
diff --git a/tern/report/formats.py b/tern/report/formats.py
index 8a296a6..d9afc1b 100644
--- a/tern/report/formats.py
+++ b/tern/report/formats.py
@@ -41,7 +41,8 @@ invoke_on_host = '''\ton host:\n'''
 package_name = '''Package: {package_name}\n'''
 package_version = '''Version: {package_version}\n'''
 package_url = '''Project URL: {package_url}\n'''
-package_license = '''License: {package_license}\n\n'''
+package_license = '''License: {package_license}\n'''
+package_copyright = '''Copyright Text: {package_copyright}\n'''
 # notes
 package_notes = '''Errors: {package_info_retrieval_errors}\n''' \
     '''Improvements: {package_info_reporting_improvements}\n'''
diff --git a/tern/report/report.py b/tern/report/report.py
index a6070b4..83ac3f8 100644
--- a/tern/report/report.py
+++ b/tern/report/report.py
@@ -308,7 +308,8 @@ def generate_report(args, *images):
 
 def generate_verbose(is_summary, images):
     '''Generate a verbose report'''
-    report = formats.disclaimer.format(version_info=general.get_git_rev_or_version())
+    report = formats.disclaimer.format(
+        version_info=general.get_git_rev_or_version())
     if is_summary:
         logger.debug('Creating a summary of components in image...')
         for image in images:
@@ -323,7 +324,8 @@ def generate_verbose(is_summary, images):
 def generate_yaml(images):
     '''Generate a yaml report'''
     logger.debug('Creating YAML report...')
-    report = formats.disclaimer_yaml.format(version_info=general.get_git_rev_or_version())
+    report = formats.disclaimer_yaml.format(
+        version_info=general.get_git_rev_or_version())
     for image in images:
         report = report + content.print_yaml_report(image)
     return report
@@ -343,7 +345,7 @@ def check_docker_daemon():
         docker.from_env()
     except IOError as error:
         logger.error('Docker daemon is not running: %s',
-            error.output.decode('utf-8'))
+                     error.output.decode('utf-8'))
         sys.exit()
 
 


Test patch:
diff --git a/tests/test_class_package.py b/tests/test_class_package.py
index aa59a42..847c6b4 100644
--- a/tests/test_class_package.py
+++ b/tests/test_class_package.py
@@ -17,6 +17,7 @@ class TestClassPackage(unittest.TestCase):
         self.p1 = Package('p1')
         self.p1.version = '1.0'
         self.p1.pkg_license = 'Apache 2.0'
+        self.p1.copyright = 'All Rights Reserved'
         self.p1.src_url = 'github.com'
 
         self.p2 = Package('p2')
@@ -30,6 +31,7 @@ class TestClassPackage(unittest.TestCase):
         self.assertFalse(self.p2.version)
         self.assertFalse(self.p2.src_url)
         self.assertFalse(self.p2.pkg_license)
+        self.assertFalse(self.p2.copyright)
 
     def testSetters(self):
         self.assertRaises(AttributeError, setattr, self.p2, 'name', 'y')
@@ -37,6 +39,8 @@ class TestClassPackage(unittest.TestCase):
         self.assertEqual(self.p2.version, '2.0')
         self.p2.pkg_license = 'Apache 2.0'
         self.assertEqual(self.p2.pkg_license, 'Apache 2.0')
+        self.p2.copyright = "All Rights Reserved"
+        self.assertEqual(self.p2.copyright, 'All Rights Reserved')
         self.p2.src_url = 'github.com'
         self.assertEqual(self.p2.src_url, 'github.com')
 
@@ -44,6 +48,7 @@ class TestClassPackage(unittest.TestCase):
         self.assertEqual(self.p1.name, 'p1')
         self.assertEqual(self.p1.version, '1.0')
         self.assertEqual(self.p1.pkg_license, 'Apache 2.0')
+        self.assertEqual(self.p1.copyright, 'All Rights Reserved')
         self.assertEqual(self.p1.src_url, 'github.com')
 
     def testToDict(self):
@@ -51,6 +56,7 @@ class TestClassPackage(unittest.TestCase):
         self.assertEqual(a_dict['name'], 'p1')
         self.assertEqual(a_dict['version'], '1.0')
         self.assertEqual(a_dict['pkg_license'], 'Apache 2.0')
+        self.assertEqual(a_dict['copyright'], 'All Rights Reserved')
         self.assertEqual(a_dict['src_url'], 'github.com')
         self.assertFalse(a_dict['origins'])
 
@@ -78,6 +84,26 @@ class TestClassPackage(unittest.TestCase):
         p.version = '2.0'
         self.assertTrue(self.p2.is_equal(p))
 
+    def testFill(self):
+        p_dict = {'name': 'p1',
+                  'version': '1.0',
+                  'pkg_license': 'Apache 2.0'}
+        p = Package('p1')
+        p.fill(p_dict)
+        self.assertEqual(p.name, 'p1')
+        self.assertEqual(p.version, '1.0')
+        self.assertEqual(p.pkg_license, 'Apache 2.0')
+        self.assertFalse(p.copyright)
+        self.assertFalse(p.src_url)
+        self.assertEqual(len(p.origins.origins), 1)
+        self.assertEqual(p.origins.origins[0].origin_str, 'p1')
+        self.assertEqual(len(p.origins.origins[0].notices), 2)
+        self.assertEqual(p.origins.origins[0].notices[0].message,
+                         "No metadata for key: copyright")
+        self.assertEqual(p.origins.origins[0].notices[0].level, 'warning')
+        self.assertEqual(p.origins.origins[0].notices[1].message,
+                         "No metadata for key: src_url")
+
 
 if __name__ == '__main__':
     unittest.main()