Make the partition= tag optional.

Since we might use ToT release tools to sign a package
generated by an older build, we make the new
`partition=` tag optional. This also means we need to be
careful to use non-greedy regex matching.

Bug: 153133823
Test: python3 -m unittest
Test: input with and without the new `partition=` tag
Test: new test_ReadApkCerts_WithWithoutOptionalFields
Change-Id: Ic57efd34e745ad302ae17150c6f2318f0b4524cb

tools/releasetools/common.py

@@ -1786,8 +1786,8 @@ def ReadApkCerts(tf_zip):
       continue
     m = re.match(
         r'^name="(?P<NAME>.*)"\s+certificate="(?P<CERT>.*)"\s+'
-        r'private_key="(?P<PRIVKEY>.*?)"(\s+compressed="(?P<COMPRESSED>.*)")?'
+        r'private_key="(?P<PRIVKEY>.*?)"(\s+compressed="(?P<COMPRESSED>.*?)")?'
-        r'(\s+partition="(?P<PARTITION>.*)")?$',
+        r'(\s+partition="(?P<PARTITION>.*?)")?$',
         line)
     if not m:
       continue

tools/releasetools/merge_target_files.py

@@ -117,14 +117,15 @@ OPTIONS.keep_tmp = False
 
 PARTITION_ITEM_PATTERN = re.compile(r'^([A-Z_]+)/\*$')
 
-# In apexkeys.txt or apkcerts.txt, we may find partition tags on the various
+# In apexkeys.txt or apkcerts.txt, we will find partition tags on each entry in
-# entries in the file. We use these partition tags to filter the entries in
+# the file. We use these partition tags to filter the entries in those files
-# those files from the two different target files packages to produce a merged
+# from the two different target files packages to produce a merged apexkeys.txt
-# apexkeys.txt or apkcerts.txt file. A partition tag (e.g., for the product
+# or apkcerts.txt file. A partition tag (e.g., for the product partition) looks
-# partition) looks like this: 'partition="_PRODUCT"' or 'partition="product".
+# like this: 'partition="product"'. We use the group syntax grab the value of
-# We use the group syntax grab the value of the tag.
+# the tag. We use non-greedy matching in case there are other fields on the
+# same line.
 
-PARTITION_TAG_PATTERN = re.compile(r'partition="(.*)"')
+PARTITION_TAG_PATTERN = re.compile(r'partition="(.*?)"')
 
 # The sorting algorithm for apexkeys.txt and apkcerts.txt does not include the
 # ".apex" or ".apk" suffix, so we use the following pattern to extract a key.

tools/releasetools/sign_target_files_apks.py

@@ -1082,8 +1082,8 @@ def ReadApexKeysInfo(tf_zip):
         r'public_key="(?P<PAYLOAD_PUBLIC_KEY>.*)"\s+'
         r'private_key="(?P<PAYLOAD_PRIVATE_KEY>.*)"\s+'
         r'container_certificate="(?P<CONTAINER_CERT>.*)"\s+'
-        r'container_private_key="(?P<CONTAINER_PRIVATE_KEY>.*)"\s+'
+        r'container_private_key="(?P<CONTAINER_PRIVATE_KEY>.*?)"'
-        r'partition="(?P<PARTITION>.*)"$',
+        r'(\s+partition="(?P<PARTITION>.*?)")?$',
         line)
     if not matches:
       continue

tools/releasetools/test_common.py

@@ -710,6 +710,25 @@ class CommonApkUtilsTest(test_utils.ReleaseToolsTestCase):
       'Compressed4.apk' : 'certs/compressed4',
   }
 
+  # Test parsing with no optional fields, both optional fields, and only the
+  # partition optional field.
+  APKCERTS_TXT4 = (
+      'name="RecoveryLocalizer.apk" certificate="certs/devkey.x509.pem"'
+      ' private_key="certs/devkey.pk8"\n'
+      'name="Settings.apk"'
+      ' certificate="build/make/target/product/security/platform.x509.pem"'
+      ' private_key="build/make/target/product/security/platform.pk8"'
+      ' compressed="gz" partition="system"\n'
+      'name="TV.apk" certificate="PRESIGNED" private_key=""'
+      ' partition="product"\n'
+  )
+
+  APKCERTS_CERTMAP4 = {
+      'RecoveryLocalizer.apk' : 'certs/devkey',
+      'Settings.apk' : 'build/make/target/product/security/platform',
+      'TV.apk' : 'PRESIGNED',
+  }
+
   def setUp(self):
     self.testdata_dir = test_utils.get_testdata_dir()
 
@@ -786,6 +805,14 @@ class CommonApkUtilsTest(test_utils.ReleaseToolsTestCase):
     with zipfile.ZipFile(target_files, 'r') as input_zip:
       self.assertRaises(ValueError, common.ReadApkCerts, input_zip)
 
+  def test_ReadApkCerts_WithWithoutOptionalFields(self):
+    target_files = self._write_apkcerts_txt(self.APKCERTS_TXT4)
+    with zipfile.ZipFile(target_files, 'r') as input_zip:
+      certmap, ext = common.ReadApkCerts(input_zip)
+
+    self.assertDictEqual(self.APKCERTS_CERTMAP4, certmap)
+    self.assertIsNone(ext)
+
   def test_ExtractPublicKey(self):
     cert = os.path.join(self.testdata_dir, 'testkey.x509.pem')
     pubkey = os.path.join(self.testdata_dir, 'testkey.pubkey.pem')

tools/releasetools/test_sign_target_files_apks.py

@@ -35,9 +35,13 @@ class SignTargetFilesApksTest(test_utils.ReleaseToolsTestCase):
   <signer signature="{}"><seinfo value="media"/></signer>
 </policy>"""
 
+  # Note that we test one apex with the partition tag, and another without to
+  # make sure that new OTA tools can process an older target files package that
+  # does not include the partition tag.
+
   # pylint: disable=line-too-long
   APEX_KEYS_TXT = """name="apex.apexd_test.apex" public_key="system/apex/apexd/apexd_testdata/com.android.apex.test_package.avbpubkey" private_key="system/apex/apexd/apexd_testdata/com.android.apex.test_package.pem" container_certificate="build/make/target/product/security/testkey.x509.pem" container_private_key="build/make/target/product/security/testkey.pk8" partition="system"
-name="apex.apexd_test_different_app.apex" public_key="system/apex/apexd/apexd_testdata/com.android.apex.test_package_2.avbpubkey" private_key="system/apex/apexd/apexd_testdata/com.android.apex.test_package_2.pem" container_certificate="build/make/target/product/security/testkey.x509.pem" container_private_key="build/make/target/product/security/testkey.pk8" partition="system"
+name="apex.apexd_test_different_app.apex" public_key="system/apex/apexd/apexd_testdata/com.android.apex.test_package_2.avbpubkey" private_key="system/apex/apexd/apexd_testdata/com.android.apex.test_package_2.pem" container_certificate="build/make/target/product/security/testkey.x509.pem" container_private_key="build/make/target/product/security/testkey.pk8"
 """
 
   def setUp(self):