Store the list of deb packages. #401

a-martynovich · 2019-09-05T12:23:28Z

rptrchv · 2019-09-06T10:36:16Z

backend/backend/settings/dev.py

@@ -22,6 +22,7 @@
    'django_extensions'
 ]

-IS_DEV = True
+IS_MTLS_API=IS_API=IS_DASH=IS_DEV = True


backend/backend/settings/dev.py

rptrchv · 2019-09-06T10:40:10Z

backend/device_registry/models.py

@@ -39,6 +40,18 @@ def from_db_value(self, value, expression, connection, context):
        return value


+class DebPackage(models.Model):
+    class Distro(Enum):


Usually in Django they do this ^ way. What's the advantage of your approach?

I want the field value to be converted to Enum class, this way we can have some type checking later when we work with deb packages in a separate worker which will scan them for CVEs.

can you give me some example of such type checking that requires enum?

What's the advantage of your approach?
can you give me some example of such type checking that requires enum?

This is self-documented, has less code and will fail quickly (before even getting to the db backend) if we do package.distro = SomeOtherEnum.Other instead of package.distro = DebPackage.Distro.Raspbian.

backend/device_registry/models.py

rptrchv · 2019-09-06T10:53:23Z

@a-martynovich I recommend to re-create migrations for the task at the very end of working on it in order to reduce their number. I can help with this if you need

backend/device_registry/models.py

a-martynovich · 2019-09-06T11:07:00Z

@a-martynovich I recommend to re-create migrations for the task at the very end of working on it in order to reduce their number. I can help with this if you need

It's easy, I'll do this myself together with the improvements you've requested.
Also I forgot that we need to store deb package OS version and arch.

backend/device_registry/api_views.py

backend/device_registry/tests/test_api.py

rptrchv · 2019-09-06T11:12:55Z

backend/device_registry/models.py

+        return self.deb_packages.filter(name__in=self.INSECURE_SERVICES)
+
+    def set_deb_packages(self, packages):
+        packages_set = set((p['name'], p['version']) for p in packages)


IMO there's no need to transmit {'name': 'xxx', 'version': 'yyy'}. ('xxx', 'yyy') is much better for bandwith

This will only reduce the size of this message by 2. But then we will lose flexibility. If we decide to change this message in any way (and we will eventually), i.e. add, remove or change fields, we will break compatibility with older message.
Also the size of this message is no more than 100Kbytes. I don't think we should worry about bandwidth here.

#391

Fix test_ping_writes_packages.

Implement deb_packages as an m2m field.

Remove unused import in test_api. Don't allow missing fields in deb_packages inside ping message. Add unique constraint to DebPackage model.

Merge migrations.

vpetersson requested a review from rptrchv September 5, 2019 12:24