Manual review required for azure-iot-edge snap

store-requests
1

Snap: https://snapcraft.io/azure-iot-edge

I attempted to publish a new revision (#9) for this snap as part of an initial effort to bring it up-to-date with the latest releases for Azure IoT Edge. The auto review failed indicating:

human review required due to ‘allow-installation’ constraint (bool) declaration-snap-v2_plugs_installation (workload-sockets, system-files)

The updated snap does use the system-files interface to expose sockets at /var/run/iotedge similar to what is done when installing the existing .deb packages for Azure IoT Edge.

In combination with the request for manual review / approval to use the system-files interface, can the snap also get an alias for the azure-iot-edge.iotedge command to enable using ‘iotedge’ from the CLI?

1 Like
2

Currently the snap has the following for this system-files declaration:

  workload-sockets:
    interface: system-files
    write:
    - /run
    - /var/run
    - /var/run/iotedge
    - /var/run/iotedge/mgmt.sock
    - /var/run/iotedge/workload.sock

Unfortunately providing write access to all of /run seems far too privileged and I suspect may potentially allow the snap to escalate privileges. Instead the snap should expose these sockets at $SNAP_COMMON which would map to /var/run/snap/azure-iot-edge/common which is the standard location for such things in snaps.

Regarding the alias, +1 from me - there doesn’t seem to be any obvious conflict and this is the standard name for this tool.

3

Thank you for the quick response, Alex. One of the primary consumers of these sockets is a separate component that is only delivered as a Docker container. AFAIK, a socket under $SNAP_COMMON would not be accessible to a Docker container, would it? The requested write access can be scoped to just the /var/run/iotedge folder and files.

4

Hi @alexmurray,

I worked on this snap, and want to chime in to second what Micah said. I think the /run/ & /var/run are not necessary and I’m not sure how they found their way in (maybe my fault, if so, sorry about that Micah & Alex). However, we ran into significant issues trying to interface with the workload in the Docker container when exposing the mgmt.sock & workload.sock files only within $SNAP_COMMON, hence the system-files request. If absolutely necessary we can revisit, but my impression is that the latter 3 meet the criteria for system-files (i.e. clearly owned by the iotedge snap, not reasonably likely to conflict with another application on a system).

For all the other socket-based communication (between this and another azure snap) we do use the correct paradigm :slight_smile:

5

@alexclewontin and @alexmurry I’ve pushed a new revision (#10) that should have system-files scoped to /run/iotedge and /var/run/iotedge.

6

@micah1 thanks for the update - ok this sounds much more reasonable.

+1 from me for use of and auto-connect of a system-files instance named run-iotedge for azure-iot-edge.

Can other @reviewers please vote too?

7

So one thing I just noticed about the current system-files declaration - if you declare /foo then you don’t need to declare /foo/bar since this is already covered by /foo - as such can you please change the interface declaration to:

run-iotedge:
    interface: system-files
    write:
    - /run/iotedge
    - /var/run/iotedge
8

+1 from my side on @alexmurray’s suggestion above and using auto-connect of run-iotedge for azure-iot-edge

9

@alexmurray, @0xnishit, @alexclewontin pushed a new revision (#11) that applies Alex’s suggestion.

10

Sure, +2 for, 0 against. granting auto-connect and use of system-files instance name workload-sockets to have a write access to /run/iotedge, and /var/run/iotedge. This is now live

11

hi @micah1 Kindly rename your plug from workload-sockets to run-iotedge as the workload-sockets name is too generic. I have granted this name run-iotedge of the plug and not the workload-sockets

Thanks

12

@micah1 ping,

Can you please rename the iface reference? The declaration has been granted so as soon as you upload a new revision, the automated review should pass.

13

Ping @micah1 - can you please update the snap for the new interface name, then it should pass automated review.

14

Ping @micah1, can you please rename your plug from workload-sockets to run-iotedge

thanks

15

@micah1 ping, are you planning to rename the interface reference as we suggested?

We are still monitoring this request to make sure your snap properly works. We then much appreciate if you can either update your snap so it passes automated review, or tell us if you don’t have time to move forward with this so we can clean our review queue. Thanks!

16

@micah1,

I see the plug was renamed to run-iotedge and latest revisions of azure-iot-edge have been successfully published so I am removing this request from our review queue.

Feel free to write here again if there is any further question.

Thanks!