Azure / iot-identity-service

Source of the Azure IoT Identity Service and related services.
MIT License
37 stars 46 forks source link

many: allow build-time setting of the socket directory #478

Closed alexclewontin closed 1 year ago

alexclewontin commented 1 year ago

A proposal for allowing build-time setting of the directory the *.sock files are created in. This follows the model of @micahl's patches doing something similar for usernames.

I mostly identified locations with a search. I did leave a couple of /run/aziot/ paths untouched when it was obvious they were part of a test (i.e. lines 133 & 148 of key/aziot-keyd-config/src/lib.rs or lines 579, 580, 597, and 600 in cert/aziot-certd-config/src/lib.rs). I didn't touch any CI or E2E pieces.

Motivations:

This is part of our joint initiative to support the identity service in a snap package. We've done some initial PoC work to show that it works. As it stands right now, the way we can make it work is by making a carveout in confinement to allow the services to create their sockets to listen on at the normal location (/run/aziot/*.sock). This works, and is fine.

However, because of the way snaps handle systemd socket-activated services (which the identity services already have the ability to run as), we cannot leverage them at this path (they need to be at a path prefixed by $SNAP_DATA or $SNAP_COMMON, which are /var/snap/azure-iot-identity/current/ or /var/snap/azure-iot-identity/common/ respectively). It's worth noting that today, we can actually already sort of do this because when systemd handles the socket, the services get passed an open FD, not associated with an particular path. However, the aziotctl tool needs to know where these sockets live, even when managed by systemd.

Additionally, the truly idiomatic, "snappy" way would be to share them to other snaps as part of a content interface (much less privileged than the other way of carving out a highly privileged system-files interface to /run/aziot). This has the advantage of providing us with an extra set of event-triggered hook scripts that run when the content interface connects & disconnects, which can give us some additional flexibility in dropping in config files, registering consumers with the identity service, ensuring consistent config between the identity service & the edge agent, etc.

Where precisely I would override these paths to is a little up in the air. My initial idea would be to somewhere in /var/snap/azure-iot-identity/common/ but where within that file structure isn't particularly important to me.

Understand that changing may have carry-on effects on client compatibility, if those clients have a specific expectation about where to find the socket. My initial thinking is that at least at first, consumers of the snapped iot-identity service will primarily be other snaps, and so adapting those consumers to understand the new path will just be part of packaging them as snaps.

alexclewontin commented 1 year ago

@microsoft-github-policy-service agree [company="Canonical"]

alexclewontin commented 1 year ago

@microsoft-github-policy-service agree company="Canonical"

arsing commented 1 year ago

What will you override them to?

micahl commented 1 year ago

@alexclewontin, thanks for opening the PR per discussion. Can you also add in the description some of the motivations (e.g., was it support the "snappy" way and enable socket activation)?

alexclewontin commented 1 year ago

@arsing @micahl Added context above!

micahl commented 1 year ago

@alexclewontin would moving the sockets also be needed to enable all the "aziotctl system 's" to work or was that a separate thing? this change would commit consumers of the sockets to also be snapped. i agree that's probably OK since we also provide the regular .deb or .rpm that someone could use if needed.

@arsing would you have any concerns with making some strings set at build-time such as user names and socket paths?

alexclewontin commented 1 year ago

@micahl this is a separate thing. To be clear, I'd classify this more as a "nice-to-have" thing: non-critical to make the services run in a snap, but definitely better than alternative implementations.

micahl commented 1 year ago

@alexclewontin you should probably add an entry for the SOCKET_DIR to the aziotctl/.cartgo/config.toml for when someone builds directly with cargo.

I'm in favor of using the idomatic "snappy" way for a snap and leaving the existing behavior as-is for all others. We will take the stance that customers shouldn't mix and match the snap with the .deb or .rpm packages. @arsing let me know if you see any potential problems.

alexclewontin commented 1 year ago

@micahl added .cargo/config.toml & rebased onto latest

alexclewontin commented 1 year ago

Incorporated this into https://github.com/Azure/iot-identity-service/pull/479, so closing here