Merge lp:~danilo/linaro-license-protection/staging-publish into lp:~linaro-automation/linaro-license-protection/trunk

[Данило Шеган]

This adds an option to do publishing to staging as well, which includes
a trivial sanitization: the file is truncated and the base of the filename
is written as the contents instead.

Some side-refactoring, and mostly all things are tested.

[lp:~danilo/linaro-license-protection/staging-publish]

115. By Данило Шеган on 2012-08-22

Lint fixes.

[Stevan Radaković (stevanr)]

Looks good.
As I've mentioned, we should probably increase the list of accepted files but we can do it 'nother time.
Approve +1.

review: Approve (code)

=== modified file 'scripts/publish_to_snapshots.py'
--- scripts/publish_to_snapshots.py	2012-08-03 10:38:06 +0000
+++ scripts/publish_to_snapshots.py	2012-08-22 19:44:18 +0000
@@ -1,26 +1,12 @@
 #!/usr/bin/env python
 # Move artifacts from a temporary location to a permanent location on s.l.o
 
+import argparse
+import fnmatch
 import os
+import os.path
+import shutil
 import sys
-import shutil
-import argparse
-
-parser = argparse.ArgumentParser()
-parser.add_argument("-t", "--job-type", dest="job_type",
-                    help="Specify the job type (Ex: android/kernel-hwpack)")
-parser.add_argument(
-    "-j", "--job-name", dest="job_name",
-    help=("Specify the job name which resulted the archive to "
-          "be stored. Ex: ${JOB_NAME} should be specified for "
-          "android/ubuntu-{hwpacks,images,sysroots}/binaries and for"
-          "kernel-hwpack ${KERNEL_JOB_NAME}"))
-parser.add_argument(
-        "-n", "--build-num", dest="build_num", type=int,
-        help=("Specify the job build number for android/"
-              "ubuntu-{hwpacks,images,sysroots}/binaries"))
-parser.add_argument("-m", "--manifest", dest="manifest", action='store_true',
-                    help="Optional parameter to generate MANIFEST file")
 
 uploads_path = '/srv/snapshots.linaro.org/uploads/'
 target_path = '/srv/snapshots.linaro.org/www/'
@@ -39,20 +25,87 @@
     ]
 
 
+def setup_parser():
+    """Set up the argument parser for publish_to_snapshots script."""
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
+        "-s", "--staging", dest="staging", default=False,
+        action='store_true',
+        help=("Perform sanitization on the file to not expose any"
+              "potentially sensitive data.  Used for staging deployment."))
+    parser.add_argument(
+        "-t", "--job-type", dest="job_type",
+        help="Specify the job type (Ex: android/kernel-hwpack)")
+    parser.add_argument(
+        "-j", "--job-name", dest="job_name",
+        help=("Specify the job name which resulted the archive to "
+              "be stored. Ex: ${JOB_NAME} should be specified for "
+              "android/ubuntu-{hwpacks,images,sysroots}/binaries and for"
+              "kernel-hwpack ${KERNEL_JOB_NAME}"))
+    parser.add_argument(
+            "-n", "--build-num", dest="build_num", type=int,
+            help=("Specify the job build number for android/"
+                  "ubuntu-{hwpacks,images,sysroots}/binaries"))
+    parser.add_argument(
+        "-m", "--manifest", dest="manifest",
+        action='store_true',
+        help="Optional parameter to generate MANIFEST file")
+    return parser
+
+
+class PublisherArgumentException(Exception):
+    """There was a problem with one of the publisher arguments."""
+    pass
+
+
 class SnapshotsPublisher(object):
 
+    # Files that need no sanitization even when publishing to staging.
+    STAGING_ACCEPTED_FILES = [
+        'BUILD-INFO.txt',
+        'EULA.txt',
+        'OPEN-EULA.txt',
+        '*.EULA.txt.*',
+        ]
+
+    def __init__(self, argument_parser=None):
+        """Allow moving files around for publishing on snapshots.l.o."""
+        self.argument_parser = argument_parser
+
+    @classmethod
+    def is_accepted_for_staging(cls, filename):
+        """Is filename is in a list of globs in STAGING_ACCEPTED_FILES?"""
+        filename = os.path.basename(filename)
+        for accepted_names in cls.STAGING_ACCEPTED_FILES:
+            if fnmatch.fnmatch(filename, accepted_names):
+                return True
+        return False
+
+    @classmethod
+    def sanitize_file(cls, file_path):
+        """This truncates the file and fills it with its own filename."""
+        assert not cls.is_accepted_for_staging(file_path)
+        base_file_name = os.path.basename(file_path)
+        protected_file = open(file_path, "w")
+        # Nice property of this is that we are also saving on disk space
+        # needed.
+        protected_file.truncate()
+        # To help distinguish files more easily when they are downloaded,
+        # we write out the base file name as the contents.
+        protected_file.write(base_file_name)
+        protected_file.close()
+
     def validate_args(self, args):
         # Validate that all the required information
         # is passed on the command line
         if (args.job_type == None or args.job_name == None or
             args.build_num == None):
-            parser.error(
+            raise PublisherArgumentException(
                 "\nYou must specify job-type, job-name and build-num")
-            return FAIL
 
         if (args.job_type not in acceptable_job_types):
-            parser.error("Invalid job type")
-            return FAIL
+            raise PublisherArgumentException("Invalid job type")
+        return True
 
     def jobname_to_target_subdir(self, args, jobname):
         ret_val = None
@@ -202,7 +255,7 @@
             os.chdir(orig_dir)
             return FAIL
 
-    def move_dir_content(self, src_dir, dest_dir):
+    def move_dir_content(self, src_dir, dest_dir, sanitize=False):
         filelist = os.listdir(src_dir)
         try:
             for file in filelist:
@@ -214,6 +267,11 @@
                         continue
                     else:
                         os.remove(dest)
+                if sanitize and not self.is_accepted_for_staging(src):
+                    # Perform the sanitization before moving the file
+                    # into place.
+                    print "Sanitizing contents of '%s'." % src
+                    self.sanitize_file(src)
                 print "Moving the src '", src, "'to dest'", dest, "'"
                 shutil.move(src, dest)
         except shutil.Error:
@@ -231,7 +289,8 @@
                 if not os.path.isdir(target_dir_path):
                     raise OSError
 
-            self.move_dir_content(build_dir_path, target_dir_path)
+            self.move_dir_content(build_dir_path, target_dir_path,
+                                  sanitize=args.staging)
 
             if (args.job_type == "android" or
                 args.job_type == "ubuntu-hwpacks" or
@@ -265,9 +324,13 @@
 
 
 def main():
-    publisher = SnapshotsPublisher()
-    args = parser.parse_args()
-    publisher.validate_args(args)
+    argument_parser = setup_parser()
+    publisher = SnapshotsPublisher(argument_parser)
+    args = argument_parser.parse_args()
+    try:
+        publisher.validate_args(args)
+    except PublisherArgumentException as exception:
+        argument_parser.error(exception.message)
     try:
         build_dir_path, target_dir_path = publisher.validate_paths(
             args, uploads_path, target_path)

=== modified file 'tests/test_publish_to_snapshots.py'
--- tests/test_publish_to_snapshots.py	2012-08-03 10:38:06 +0000
+++ tests/test_publish_to_snapshots.py	2012-08-22 19:44:18 +0000
@@ -4,10 +4,13 @@
 import sys
 import shutil
 import tempfile
-import argparse
 from StringIO import StringIO
 from testtools import TestCase
-from scripts.publish_to_snapshots import SnapshotsPublisher
+from scripts.publish_to_snapshots import (
+    PublisherArgumentException,
+    SnapshotsPublisher,
+    setup_parser,
+    )
 
 
 class TestSnapshotsPublisher(TestCase):
@@ -18,13 +21,8 @@
     orig_dir = os.getcwd()
 
     def setUp(self):
-        self.parser = argparse.ArgumentParser()
-        self.parser.add_argument("-t", "--job-type", dest="job_type")
-        self.parser.add_argument("-j", "--job-name", dest="job_name")
-        self.parser.add_argument("-n", "--build-num", dest="build_num",
-                                 type=int)
-        self.parser.add_argument("-m", "--manifest", dest="manifest",
-                                 action='store_true')
+        self.parser = setup_parser()
+
         if not os.path.isdir(self.uploads_path):
             os.mkdir(self.uploads_path)
 
@@ -72,21 +70,18 @@
             ['-t', 'binaries', '-j', 'dummy_job_name', '-n', '1'])
         self.publisher.validate_args(param)
 
+        # Staging parameter is accepted as well.
+        param = self.parser.parse_args(
+            ['--staging', '-t', 'binaries', '-j', 'dummy_job_name', '-n', '1'])
+        self.publisher.validate_args(param)
+
     def test_validate_args_invalid_job_type(self):
-        orig_stderr = sys.stderr
-        stderr = sys.stderr = StringIO()
         self.publisher = SnapshotsPublisher()
         param = self.parser.parse_args(
             ['-t', 'invalid_job_type', '-j', 'dummy_job_name', '-n', '1'])
-        try:
-            self.publisher.validate_args(param)
-        except SystemExit, err:
-            self.assertEqual(err.code, 2, "Expected result")
-        finally:
-            sys.stderr = orig_stderr
-
-        stderr.seek(0)
-        self.assertIn("Invalid job type", stderr.read())
+        self.assertRaisesRegexp(
+            PublisherArgumentException, "Invalid job type",
+            self.publisher.validate_args, param)
 
     def test_validate_args_run_invalid_argument(self):
         orig_stderr = sys.stderr
@@ -120,21 +115,13 @@
                       stderr.read())
 
     def test_validate_args_run_none_values(self):
-        orig_stderr = sys.stderr
-        stderr = sys.stderr = StringIO()
         self.publisher = SnapshotsPublisher()
-        try:
-            param = self.parser.parse_args(
-                ['-t', None, '-j', None, '-n', 0])
-            self.publisher.validate_args(param)
-        except SystemExit, err:
-            self.assertEqual(err.code, 2, "None values are not acceptable")
-        finally:
-            sys.stderr = orig_stderr
-
-        stderr.seek(0)
-        self.assertIn("You must specify job-type, job-name and build-num",
-                      stderr.read())
+        param = self.parser.parse_args(
+            ['-t', None, '-j', None, '-n', 0])
+        self.assertRaisesRegexp(
+            PublisherArgumentException,
+            "You must specify job-type, job-name and build-num",
+            self.publisher.validate_args, param)
 
     def test_validate_paths_invalid_uploads_path(self):
         orig_stdout = sys.stdout
@@ -176,6 +163,123 @@
         stdout.seek(0)
         self.assertIn("Missing target path", stdout.read())
 
+    def test_is_accepted_for_staging_EULA_txt(self):
+        self.assertTrue(
+            SnapshotsPublisher.is_accepted_for_staging("EULA.txt"))
+        self.assertTrue(
+            SnapshotsPublisher.is_accepted_for_staging("/path/to/EULA.txt"))
+        # Full filename should be EULA.txt and nothing should be added to it.
+        self.assertFalse(
+            SnapshotsPublisher.is_accepted_for_staging(
+                "/path/to/EULA.txt.something"))
+
+    def test_is_accepted_for_staging_OPEN_EULA_txt(self):
+        self.assertTrue(
+            SnapshotsPublisher.is_accepted_for_staging(
+                "OPEN-EULA.txt"))
+        self.assertTrue(
+            SnapshotsPublisher.is_accepted_for_staging(
+                "/path/to/OPEN-EULA.txt"))
+
+    def test_is_accepted_for_staging_per_file_EULA(self):
+        self.assertTrue(
+            SnapshotsPublisher.is_accepted_for_staging(
+                "something.tar.gz.EULA.txt.ste"))
+        self.assertTrue(
+            SnapshotsPublisher.is_accepted_for_staging(
+                "/path/to/something.tar.gz.EULA.txt.ste"))
+        # We must have a "theme" for per-file license files in the
+        # EULA-model.
+        self.assertFalse(
+            SnapshotsPublisher.is_accepted_for_staging(
+                "something.tar.gz.EULA.txt"))
+
+    def test_is_accepted_for_staging_build_info(self):
+        self.assertTrue(
+            SnapshotsPublisher.is_accepted_for_staging(
+                "BUILD-INFO.txt"))
+        self.assertTrue(
+            SnapshotsPublisher.is_accepted_for_staging(
+                "/path/to/BUILD-INFO.txt"))
+
+    def test_sanitize_file_assert_on_accepted_files(self):
+        # Since sanitize_file explicitely sanitizes a file,
+        # one needs to ensure outside the function that it's
+        # not being called on one of accepted file names.
+        filename = '/path/to/EULA.txt'
+        self.assertTrue(SnapshotsPublisher.is_accepted_for_staging(filename))
+        self.assertRaises(
+            AssertionError, SnapshotsPublisher.sanitize_file, filename)
+
+    def make_temporary_file(self, data, root=None):
+        """Creates a temporary file and fills it with data.
+
+        Returns the full file path of the new temporary file.
+        """
+        tmp_file_handle, tmp_filename = tempfile.mkstemp(dir=root)
+        tmp_file = os.fdopen(tmp_file_handle, "w")
+        tmp_file.write(data)
+        tmp_file.close()
+        return tmp_filename
+
+    def test_sanitize_file_loses_original_contents(self):
+        original_text = "Some garbage" * 100
+        protected_filename = self.make_temporary_file(original_text)
+
+        SnapshotsPublisher.sanitize_file(protected_filename)
+        new_contents = open(protected_filename).read()
+        self.assertNotEqual(original_text, new_contents)
+        # Clean-up.
+        os.remove(protected_filename)
+
+    def test_sanitize_file_basename_as_contents(self):
+        # It's useful to have an easy way to distinguish files by the content
+        # as well, so we put the basename (filename without a path) in.
+        protected_filename = self.make_temporary_file("Some contents")
+        SnapshotsPublisher.sanitize_file(protected_filename)
+        new_contents = open(protected_filename).read()
+        # Incidentally, the contents are actually the file base name.
+        self.assertEqual(os.path.basename(protected_filename), new_contents)
+        # Clean-up.
+        os.remove(protected_filename)
+
+    def test_move_dir_content_sanitize(self):
+        # A directory containing a file to sanitize is moved with the
+        # data being sanitized first.
+        source_dir = tempfile.mkdtemp()
+        destination_dir = tempfile.mkdtemp()
+        protected_content = "Something secret" * 10
+        protected_file = self.make_temporary_file(protected_content,
+                                                  root=source_dir)
+        publisher = SnapshotsPublisher()
+        publisher.move_dir_content(source_dir, destination_dir, sanitize=True)
+        resulting_file = os.path.join(destination_dir,
+                                      os.path.basename(protected_file))
+        self.assertNotEqual(protected_content,
+                            open(resulting_file).read())
+        shutil.rmtree(source_dir)
+        shutil.rmtree(destination_dir)
+
+    def test_move_dir_content_no_sanitize(self):
+        # A directory containing one of accepted files has it moved
+        # without changes even with sanitization option on.
+        source_dir = tempfile.mkdtemp()
+        destination_dir = tempfile.mkdtemp()
+        allowed_content = "Something public" * 10
+        allowed_file_name = os.path.join(source_dir, "EULA.txt")
+        allowed_file = open(allowed_file_name, "w")
+        allowed_file.write(allowed_content)
+        allowed_file.close()
+
+        publisher = SnapshotsPublisher()
+        publisher.move_dir_content(source_dir, destination_dir, sanitize=True)
+        resulting_file = os.path.join(destination_dir,
+                                      os.path.basename(allowed_file_name))
+        self.assertEqual(allowed_content,
+                         open(resulting_file).read())
+        shutil.rmtree(source_dir)
+        shutil.rmtree(destination_dir)
+
     def test_move_artifacts_kernel_successful_move(self):
         orig_stdout = sys.stdout
         stdout = sys.stdout = StringIO()