Merge ~suligap/snapstore-client:airgap-related-bugfixes into snapstore-client:master

[Przemysław Suliga]

Fixes for remote airgap snap uploads

- missing utf-8 encoding of assertions payload
- ignore missing revisions when uploading snap files
- missing package_type field in various API calls

Tested this locally against an airgap proxy built from recent snapstore-snap master.

[Simon Davy (bloodearnest)]

LGTM. If you think snap-store-proxy has not had attention, the client is worse :(

review: Approve

diff --git a/snapstore_client/logic/push.py b/snapstore_client/logic/push.py
index abd06e8..1a91bbb 100644
--- a/snapstore_client/logic/push.py
+++ b/snapstore_client/logic/push.py
@@ -40,6 +40,7 @@ def _push_ident(store, password, downloaded_map):
     snap_details = downloaded_map['snap']
     push_details = {
         'snap_id': snap_id,
+        'package_type': 'snap',
         'snap_name': snap_details['name'],
         'private': False,  # If you're pushing to your own proxy
         'stores': ['ubuntu'],  # ditto
@@ -84,6 +85,7 @@ def _push_revs(store, password, downloaded_map):
         # create the revision
         rev = {
             'snap_id': snap_id,
+            'package_type': 'snap',
             'revision': instance['revision'],
             'version': instance['version'],
             'confinement': instance['confinement'],
@@ -118,10 +120,19 @@ def _push_channelmap(store, password, downloaded_map, force_push=False):
 
     snap_id = downloaded_map['snap-id']
 
+    filter_payload = {
+        'filters': [
+            {
+                'series': '16',
+                'package_type': 'snap',
+                'snap_id': snap_id,
+            },
+        ],
+    }
     # Check if we have an existing channel map
     existing = requests.post(
         store_url + '/channelmaps/filter',
-        json={'filters': [{'series': '16', 'snap_id': snap_id}]},
+        json=filter_payload,
         auth=requests.auth.HTTPBasicAuth(USERNAME, password))
     if existing.status_code != 200:
         raise pushException("Error retrieving current channelmap")
@@ -140,6 +151,7 @@ def _push_channelmap(store, password, downloaded_map, force_push=False):
             'developer_id': downloaded_map['snap']['publisher']['id'],
             'release_requests': [
                 {
+                    'package_type': 'snap',
                     'snap_id': snap_id,
                     'channel': [
                         track,
@@ -204,7 +216,7 @@ def _add_assertion_to_service(store, password, list_assertions):
             continue
         response = requests.post(
             store_url + '/v1/assertions',
-            '\n'.join(assertion),
+            '\n'.join(assertion).encode('utf-8'),
             headers={'Content-Type': 'application/x.ubuntu.assertion'},
             auth=requests.auth.HTTPBasicAuth(USERNAME, password))
         if response.status_code != 201:
@@ -278,10 +290,21 @@ def _push(store, tar_file, password, force_channel_map=False):
             snap_name, instance['revision'])
         snap_path = str(json_path.parent / snap_file_name)
         assert_path = str(json_path.parent / assert_file_name)
-        _push_file_to_nginx_cache(
-            store, password, snap_path,
-            downloaded_map['snap-id'], instance['revision'])
-        _push_assertions(store, password, assert_path)
+        try:
+            _push_file_to_nginx_cache(
+                store, password, snap_path,
+                downloaded_map['snap-id'], instance['revision'],
+            )
+        except FileNotFoundError as exc:
+            logger.warning(
+                'Skipping revision %s (%s/%s) as %s not found',
+                instance['revision'],
+                instance['channel']['name'],
+                instance['channel']['architecture'],
+                exc.filename,
+            )
+        else:
+            _push_assertions(store, password, assert_path)
 
 
 def push_snap(args):
diff --git a/snapstore_client/logic/tests/test-snap-assert.assert b/snapstore_client/logic/tests/test-snap-assert.assert
index b6b2ebd..4eda67a 100644
--- a/snapstore_client/logic/tests/test-snap-assert.assert
+++ b/snapstore_client/logic/tests/test-snap-assert.assert
@@ -42,7 +42,7 @@ oG3Ie3WOHrVjCLXIdYslpL1O4nadqR6Xv58pHj6k
 type: account
 authority-id: canonical
 account-id: 1RgHGj7Zp3nyycrGa5sODDbZznzJAwAX
-display-name: Tom Wardill
+display-name: Tom Wardill (Ω)
 timestamp: 2018-02-06T09:22:49.118291Z
 username: twom
 validation: unproven
diff --git a/snapstore_client/logic/tests/test_push.py b/snapstore_client/logic/tests/test_push.py
index 7fedbbd..cda240c 100644
--- a/snapstore_client/logic/tests/test_push.py
+++ b/snapstore_client/logic/tests/test_push.py
@@ -37,9 +37,15 @@ class pushTests(TestCase):
 
         _push_ident(self.store, 'test', self.snap_map)
 
+        request = responses.calls[0].request
+        payload = json.loads(request.body.decode('utf-8'))
+
         self.assertEqual(
-            'Basic YWRtaW46dGVzdA==',
-            responses.calls[0].request.headers['Authorization'])
+            payload['snaps'][0]['package_type'], 'snap',
+        )
+        self.assertEqual(
+            'Basic YWRtaW46dGVzdA==', request.headers['Authorization'],
+        )
 
     @responses.activate
     def test_push_ident_failed(self):
@@ -56,9 +62,13 @@ class pushTests(TestCase):
 
         _push_revs(self.store, 'test', self.snap_map)
 
+        request = responses.calls[0].request
+        payload = json.loads(request.body.decode('utf-8'))
+
+        self.assertEqual(payload[0]['package_type'], 'snap')
         self.assertEqual(
-            'Basic YWRtaW46dGVzdA==',
-            responses.calls[0].request.headers['Authorization'])
+            'Basic YWRtaW46dGVzdA==', request.headers['Authorization'],
+        )
 
     @responses.activate
     def test_push_revs_unexpected_status_code(self):
@@ -85,9 +95,30 @@ class pushTests(TestCase):
 
         _push_channelmap(self.store, 'test', self.snap_map)
 
+        filter_request = responses.calls[0].request
+        filter_payload = json.loads(filter_request.body.decode('utf-8'))
         self.assertEqual(
-            'Basic YWRtaW46dGVzdA==',
-            responses.calls[0].request.headers['Authorization'])
+            filter_payload['filters'],
+            [
+                {
+                    'series': '16',
+                    'package_type': 'snap',
+                    'snap_id': self.snap_map['snap-id'],
+                },
+            ],
+        )
+        self.assertEqual(
+            'Basic YWRtaW46dGVzdA==', filter_request.headers['Authorization'],
+        )
+
+        chanmap_update_request = responses.calls[1].request
+        chanmap_update_payload = json.loads(
+            chanmap_update_request.body.decode('utf-8'),
+        )
+        self.assertEqual(
+            chanmap_update_payload['release_requests'][0]['package_type'],
+            'snap',
+        )
 
     @responses.activate
     def test_push_map_failed(self):
@@ -144,6 +175,10 @@ class pushTests(TestCase):
         split_assertions = _split_assertions(self.snap_assert)
         _add_assertion_to_service(self.store, 'test', split_assertions)
 
+        self.assertIn(
+            'display-name: Tom Wardill (Ω)',
+            responses.calls[0].request.body.decode('utf-8'),
+        )
         self.assertEqual(
             'Basic YWRtaW46dGVzdA==',
             responses.calls[0].request.headers['Authorization'])