cirruslabs / orchard

Orchestrator for running Tart Virtual Machines on a cluster of Apple Silicon devices
Other
200 stars 17 forks source link

Fix #221 by bumping Go. #223

Closed timpeeters closed 1 week ago

timpeeters commented 1 week ago

Fixes #221

CLAassistant commented 1 week ago

CLA assistant check
All committers have signed the CLA.

edigaryev commented 1 week ago

Hi Tim 👋

We'd probably need to bump the Golang version used to build the Orchard too, see https://github.com/cirruslabs/orchard/pull/222.

I'm happy to merge this if you cherry-pick the changes from that PR.

timpeeters commented 1 week ago

@edigaryev thanks, I've now cherry-picked your change.

edigaryev commented 1 week ago

It's weird, but I'm still not seeing the LC_UUID in the binary produced by Golang 1.23.3.

You can check this yourself by downloading the binaries artifact here and running dwarfdump -u dist/orchard_darwin_arm64_v8.0/orchard.

timpeeters commented 1 week ago

Hmm strange, I'll have a look here as well. Nevertheless, I've successfully tested a locally built binary using Golang 1.23.3 and it did no longer result in the "no route to host" error.

edigaryev commented 1 week ago

It's weird, but I'm still not seeing the LC_UUID in the binary produced by Golang 1.23.3.

Oh, I now see why:

Just give an update: This is fixed on tip (to be released in Go 1.24), by generating LC_UUID by default.

For previous releases (Go 1.22 and 1.23), we did a simpler backport, which requires a Go linker flag (-B gobuildid) to generate the UUID. Compared to -linkmode=external, this does not require a C toolchain installation (and therefore easier to work with in a cross compilation setting). This will be included in the next Go 1.22 and 1.23 minor releases (early November).

@timpeeters we'll probably need to update the .goreleaser.yml as follows:

diff --git a/.goreleaser.yml b/.goreleaser.yml
index ad5a8ea..b53d35c 100644
--- a/.goreleaser.yml
+++ b/.goreleaser.yml
@@ -10,6 +10,7 @@ builds:
     ldflags: >
       -X github.com/cirruslabs/orchard/internal/version.Version={{.Version}}
       -X github.com/cirruslabs/orchard/internal/version.Commit={{.ShortCommit}}
+      -B gobuildid
     env:
       - CGO_ENABLED=0
     goos:
timpeeters commented 1 week ago

Ok, I've just added the flag. Waiting for the build now.

edigaryev commented 1 week ago

Looks like it's there now:

% dwarfdump -u ./orchard
UUID: 2D8094E1-BEB3-3878-C9A1-922885726958 (arm64) ./orchard
hblockx commented 6 days ago

but isn't that only solving the "issue" by addressing the permission at the end of binary-calling? The same issue is appearing when trying to build images with tart, as actually the tart-binary is the one which needs the permission, as it is using a bridge-network to access a vm and this now seems to trigger the local-networking permission? In gitlab there is an open issue regarding that topic: https://gitlab.com/gitlab-org/gitlab-runner/-/issues/38262

This fix is not solving the issue for tart in general...

edigaryev commented 6 days ago

tart-binary is the one which needs the permission, as it is using a bridge-network to access a vm

Tart does not access its own VMs in any way over the network. So it doesn't need the "Local Network" permission to operate correctly.

The only exception to this is when doing clone/pull against OCI registries hosted on IP-address ranges of directly connected networks. But in this case, you can simply approve the permission in the UI.

hblockx commented 6 days ago

In case of gitLab-Runner and packer-builds, which binary actually should deal with the local-networking permission? It also kinda feels wrong that the gitlab runner is asking for it, although it is packer / tart accessing a vm? I thought tart accesses the VM via using the SSH command and the need of the permission can be provided top-down, so the top-level-caller may provide it for calling binaries which actually need the permission... (which then would result in the calling gitlab runner, orchard, or terminal which is calling the binary)

edigaryev commented 6 days ago

It also kinda feels wrong that the gitlab runner is asking for it, although it is packer / tart accessing a vm?

Here's a quote from TN3179: Understanding local network privacy (macOS considerations):

When a process performs a local network operation, macOS tries to track down the responsible code. For example, if your app spawns a helper tool and the helper tool performs a local network operation, macOS considers the app to be the responsible code.

It's also interesting that Apple states that these restrictions do not apply to:

Any daemon started by launchd

The exception for launchd daemons doesn’t apply to launchd agents. If you’re building a launchd agent, see the discussion below.

Yet in reality they still apply 🤷

Unless your launchd daemon omits the UserName and runs directly as root.