#Hey Erik, can I ping some of your brain

1 messages · Page 1 of 1 (latest)

stoic plank
#

I'm not sure how I should / if I should still rely on

sock, cleanup, err := sshforward.MountSSHSocket(ctx, caller, sshforward.SocketOpt{
        ID:   sshID,
        UID:  uid,
        GID:  gid,
        Mode: 0700,
    })
``` to mount the socket for the git tags implementation with the git subprocess.

Is the `LLBID` equivalent to `SSHID`, meaning that I can call LLBID ?

I've changed the git implementation to:
```go
var authSock *core.Socket
    if args.SSHAuthSocket.Valid {
      // existing
    } else {
        socketStore, err := parent.Sockets(ctx)
        if err != nil {
            return nil, fmt.Errorf("failed to get socket store: %w", err)
        }

        clientMetadata, err := engine.ClientMetadataFromContext(ctx)
        if err != nil {
            return nil, err
        }

        if err := socketStore.AddUnixSocket(authSock, clientMetadata.ClientID, clientMetadata.SSHAuthSocketPath); err != nil {
            return nil, fmt.Errorf("failed to add unix socket to store: %w", err)
        }
    }
return &core.GitRepository{
        Query:         parent,
        URL:           args.URL,
        KeepGitDir:    args.KeepGitDir,
        SSHKnownHosts: args.SSHKnownHosts,
        SSHAuthSocket: authSock,
        Services:      svcs,
        Platform:      parent.Platform(),
    }, nil

I was relying on the HostPath too, but i can get it with a new accessor function from the storedSocket

It's more about SSHID and if I can still rely on MountSSHSocket to forward it or there is a better change

merry jetty
#

And happy to help! Continue to ask away if I can clarify or review anything else

stoic plank
# stoic plank I'm not sure how I should / if I should still rely on ``` sock, cleanup, err :=...

I am getting an error on:

func (s *gitSchema) git(ctx context.Context, parent *core.Query, args gitArgs) (*core.GitRepository, error) {
    var svcs core.ServiceBindings
    if args.ExperimentalServiceHost.Valid {
        svc, err := args.ExperimentalServiceHost.Value.Load(ctx, s.srv)
        if err != nil {
            return nil, err
        }
        host, err := svc.Self.Hostname(ctx, svc.ID())
        if err != nil {
            return nil, err
        }
        svcs = append(svcs, core.ServiceBinding{
            ID:       svc.ID(),
            Service:  svc.Self,
            Hostname: host,
        })
    }
    var authSock *core.Socket
    if args.SSHAuthSocket.Valid {
        sock, err := args.SSHAuthSocket.Value.Load(ctx, s.srv)
        if err != nil {
            return nil, err
        }
        authSock = sock.Self
    } else {
        socketStore, err := parent.Sockets(ctx)
        if err != nil {
            return nil, fmt.Errorf("failed to get socket store: %w", err)
        }

        clientMetadata, err := engine.ClientMetadataFromContext(ctx)
        if err != nil {
            return nil, err
        }

        if err := socketStore.AddUnixSocket(authSock, clientMetadata.ClientID, clientMetadata.SSHAuthSocketPath); err != nil {
            return nil, fmt.Errorf("failed to add unix socket to store: %w", err)
        }
    }

multi.go:85: 91 : exec dagger --debug call mod-src --mod-src git+ssh://git@bitbucket.org/dagger-modules/private-modules-test.git/top-level@8723e276a45b2e620ba3185cb07dc35e2be5bc86 as-string ERROR [1.7s]

# 
failed to get value for argument "mod-src": failed to get module configuration: failed to get module ref kind: input: moduleSource resolve: failed to resolve git src: select: failed to add unix socket to store: socket must not be nil
merry jetty
#

I think you are providing a nil authSock value to AddUnixSocket

#

Is the idea that if args.SSHAuthSocket.Valid is false, you use the default SSH auth sock on the caller's host?

stoic plank
merry jetty
#

Have you used srv.Select before? You'll want to use that to internally call the query{host{socket}} api to create the socket

#

I can try to find an example of using Select if helpful

stoic plank
#

Thanks, shall be enough 🙏 👼 😍

stoic plank
# merry jetty Have you used `srv.Select` before? You'll want to use that to internally call th...

I am 99% sure I do not mount the socket properly, I am inside the dev engine container, I see the mounted socket which on paper is the proper one from the host (with a sleep on my logic) and try to do git commands with a manually export SSH_AUTH_SOCK, still no luck. Would you be available today for a sync ?

More than the answer, curious to know how you would debug such use-case, I am lacking a component here / a reflex 🙏

merry jetty
stoic plank
merry jetty
#

I'll go wait in lounge for whenever you are ready

merry jetty
#

@stoic plank I fixed it. It was something where the way buildkit normally communicates back to clients to get their sockets is slightly different in this codepath vs. how it does it in other codepaths. The worst part though is that the buildkit code we call out to doesn't log any errors at all so it was impossible to tell what was happening.

I fixed it by just adding our own implementation of MountSSHSocket that fixes the problem and results in us actually being able to log any errors that get hit. So then we don't have to contend with buildkit anymore here.

You okay with me pushing the diff to your PR on top of the debug one you pushed?

stoic plank
merry jetty
#

Basically, in all other cases LLBID is the id to use when buildkit communicates back to these sockets, but for this one codepath it needed to use a different ID. But like I said, the fact that the errors were just totally hidden made it feel like it was better to just use our own code here. Now you just have to call SocketStore.MountSSHSocket and everything else is taken care of without needing to think about any of that

stoic plank
# merry jetty Basically, in all other cases LLBID is the id to use when buildkit communicates ...

One other question:

Last week, the Go sdk (non function) and especially the connect() function was able to retrieve the SSHAuthSocketPath: from the host environment. It seems not to be the case anymore. I do retrieve it on the client side:

🧠 debug: |{ClientID:v7ulniv07p85r6wlwjcd70e93 ClientSecretToken:887dacab-4c02-40fc-a709-a116cd31411b SessionID:tev4yzcoxo25gzzuh5fhnjjlp ClientHostname:scw-hardcore-fermi ClientVersion:dev-bdf32b2e5e760e37aebdb2e41b87db80993c13b3 Labels:map[dagger.io/ci:false dagger.io/client.arch:amd64 dagger.io/client.machine_id:8b215efa45d1a5e12d0719a904bc60798bfd3183430f68c21d9ab1bac06d5c44 dagger.io/client.os:linux dagger.io/client.version:dev-bdf32b2e5e760e37aebdb2e41b87db80993c13b3 dagger.io/git.author.email:erik@sipsma.dev dagger.io/git.author.name:Erik Sipsma dagger.io/git.branch:private-repo dagger.io/git.committer.email:erik@sipsma.dev dagger.io/git.committer.name:Erik Sipsma dagger.io/git.ref:2c2967129175b12dbb80127390e0f2f3378712c8 dagger.io/git.title:rough fix by using our own MountSSHSocket implementation] Interactive:false UpstreamCacheImportConfig:[] UpstreamCacheExportConfig:[] CloudToken: DoNotTrack:false SSHAuthSocketPath:/tmp/4035070020/001/ssh-agent.sock}|

But, inside the core/schema/git, I have a different ClientID now, without the SSHAuthSocketPath set:

level=debug msg="🔥🔥 clientMetadata: |&{ClientID:visxzdpanmtku60bg3re18iuy ClientSecretToken:nqutf1lo4bhrfnqv6n2c2270g SessionID:tev4yzcoxo25gzzuh5fhnjjlp ClientHostname:buildkitsandbox ClientVersion:dev-bdf32b2e5e760e37aebdb2e41b87db80993c13b3 Labels:map[] Interactive:false UpstreamCacheImportConfig:[] UpstreamCacheExportConfig:[] CloudToken: DoNotTrack:false SSHAuthSocketPath:}|\n" client_hostname=buildkitsandbox client_id=visxzdpanmtku60bg3re18iuy session_id=tev4yzcoxo25gzzuh5fhnjjlp

Is it expected ?

#

I was hoping / testing the new refs against all the existing tests :

testOnMultipleVCS(t, func(ctx context.Context, t *testctx.T, tc vcsTestCase) {
        t.Run("git", func(ctx context.Context, t *testctx.T) {
            c := connect(ctx, t)

            ctr := c.Container().From(golangImage).
                WithMountedFile(testCLIBinPath, daggerCliFile(t, c)).
                WithWorkdir("/work").
                With(daggerExec("init")).
                With(daggerExec("install", "--name", "foo", testGitModuleRef(tc, ""))).
                With(daggerExec("install", "--name", "bar", testGitModuleRef(tc, "subdir/dep2")))

            out, err := ctr.With(daggerCallAt("foo", "fn")).Stdout(ctx)
            require.NoError(t, err)
            require.Equal(t, "hi from root hi from dep hi from dep2", strings.TrimSpace(out))

            out, err = ctr.With(daggerCallAt("bar", "fn")).Stdout(ctx)
            require.NoError(t, err)
            require.Equal(t, "hi from dep2", strings.TrimSpace(out))
        })
    })
merry jetty
#

Looks like it’s working as expected. When you run a nested exec like that it’s a different client but in the same session (the session ids are the same).

You would need to mount the ssh auth sock into that nested exec and set the env var pointing to it for it to be used

#

Or you can just run an ssh agent inside the nested exec, that might be easier and more portable if you add some utils for setting it up and such

merry jetty
stoic plank
#

MMmh, I have a small repro showing that in a nested Dagger context, the SSHAuthSocketPath is considered as empty, even though I did set it in the parent container. I might probably be doing something wrong:

output, err := gogitbase.With(goCache(c)).With(mountedPrivateRepoSocket(c)).
        With(daggerExec("init", "--name=test", "--sdk=go", "--source=.")).
        With(daggerExec("install", "ssh://git@gitlab.com/dagger-modules/private/test/more/dagger-test-modules-private.git/py@v0.1.1")).
        File("/work/dagger.json").
        Contents(context.TODO())

with

func mountedPrivateRepoSocket(c *dagger.Client) dagger.WithContainerFunc {
    return func(ctr *dagger.Container) *dagger.Container {
        sock := c.Host().UnixSocket(globalSSHSockPath)
        if sock != nil {
            ctr = ctr.WithUnixSocket("/tmp/unix-socket", sock)
            ctr = ctr.WithEnvVariable("SSH_AUTH_SOCK", "/tmp/unix-socket")
            ctr = ctr.WithEnvVariable("GIT_SSH_COMMAND", "ssh -o StrictHostKeyChecking=no")

        }
        return ctr
    }
}

Am I misunderstanding the mount the unix socket into the nested execs ? To my understanding it is just making sure that the container in which the nested exec is done has the sock and the env var set?

When trying with git, this works, so the issue does not seem to be at the mount of the socket.

Debug shows:

🔥🔥 clientMetadata: |&{ClientID:nx9qz7p99iw93rri6o514c2mn ClientSecretToken:7c0dns698awqgmpfgqgmlc0aj SessionID:r6t7e31sc8pvwr9zabdjrw6xt ClientHostname:buildkitsandbox ClientVersion:dev-703a6bbcbd77d4aca19f103aca53f2f7596e2052 ... SSHAuthSocketPath:}|\n" client_hostn
#

The buildkitSessionID seems to get different in this case:

time="2024-07-23T20:19:40Z" level=debug msg="🥵 to: |unix:///tmp/ssh-XXXX4o5i3Y/agent.2681955#owfxntq1f722e06xr4jbr6xub|&{Socket:0xc004f81cb0 BuildkitSessionID:owfxntq1f722e06xr4jbr6xub HostPath:/tmp/ssh-XXXX4o5i3Y/agent.2681955 HostEndpoint: PortForward:{Frontend:<nil> Backend:0 Protocol:}}|\n"
time="2024-07-23T20:19:40Z" level=debug msg="🥵 to: |//:0#sldi4n8q009ky7xgj32vj3bcx|&{Socket:0xc0037baa30 BuildkitSessionID:sldi4n8q009ky7xgj32vj3bcx HostPath: HostEndpoint: PortForward:{Frontend:<nil> Backend:0 Protocol:}}|\n"
merry jetty
#

Hm, I don't see anything immediately wrong based on that, if I pull down your PR to try locally will that have the changes needed to repro?

#

Am I misunderstanding the mount the unix socket into the nested execs ? To my understanding it is just making sure that the container in which the nested exec is done has the sock and the env var set?
That is correct yes

stoic plank
#

mmh, testing a few things and pushing. Just making sure it's not a dumb issue on my end. I have tests showing that the env seems to remain though ... 🙏

merry jetty
#

Oh you know what, I actually have a guess. I can confirm locally first though if you push the changes. If I'm right I'll explain it all, just don't want to send you down the wrong path with a long explanation that ends up being irrelevant 😄

stoic plank
#

I pushed, and I test with: go test -count=1 -race -run="TestModule/TestDaggerInstall/SSH_Private_GitLab/git/unpinned_gets_pinned" -v ./core/integration/

#

The unsupportedScheme error is because I always create a socket atm, even though the socketPath == "", I'll have to better check that

merry jetty
# 
diff --git a/engine/buildkit/executor.go b/engine/buildkit/executor.go
index fdd6d270c..da51d249c 100644
--- a/engine/buildkit/executor.go
+++ b/engine/buildkit/executor.go
@@ -43,10 +43,11 @@ import (
 )
 
 type ExecutionMetadata struct {
-       ClientID    string
-       SessionID   string
-       SecretToken string
-       Hostname    string
+       ClientID          string
+       SessionID         string
+       SecretToken       string
+       Hostname          string
+       SSHAuthSocketPath string
 
        // Unique (random) ID for this execution.
        // This is used to deduplicate the same execution that gets evaluated multiple times.
diff --git a/engine/buildkit/executor_spec.go b/engine/buildkit/executor_spec.go
index bed18b5b6..9fce57aae 100644
--- a/engine/buildkit/executor_spec.go
+++ b/engine/buildkit/executor_spec.go
@@ -877,6 +877,11 @@ func (w *Worker) setupNestedClient(ctx context.Context, state *execState) (rerr
 
        state.spec.Process.Env = append(state.spec.Process.Env, DaggerSessionTokenEnv+"="+w.execMD.SecretToken)
 
+       // include SSH_AUTH_SOCK if it's set in the exec's env vars
+       if v, ok := state.origEnvMap["SSH_AUTH_SOCK"]; ok {
+               w.execMD.SSHAuthSocketPath = v
+       }
+
        filesyncer, err := client.NewFilesyncer(
                state.rootfsPath,
                strings.TrimPrefix(state.spec.Process.Cwd, "/"),
diff --git a/engine/server/session.go b/engine/server/session.go
index cdc3bb69d..aae4d2151 100644
--- a/engine/server/session.go
+++ b/engine/server/session.go
@@ -726,6 +726,7 @@ func (srv *Server) ServeHTTPToNestedClient(w http.ResponseWriter, r *http.Reques
                        SessionID:         execMD.SessionID,
                        ClientHostname:    execMD.Hostname,
                        Labels:            map[string]string{},
+                       SSHAuthSocketPath: execMD.SSHAuthSocketPath,
                },
                CallID:              execMD.CallID,
                CallerClientID:      execMD.CallerClientID,

This is the diff I think you'll need. It didn't fix the test error when I ran it but think that might be due to what you mentioned above.

The problem is that nested execs are special and served entirely from the engine itself, so that codepath that checks for os.GetEnv("SSH_AUTH_SOCK") in engine/client/client.go won't be used. That's currently only used when the CLI is running in the host directly.

The diff above looks at the nested exec environment variables (before the container has even started) and passes that value through to be set in the clientMetadata if it was there, which should result in the expected behavior I think.

#

I can chat live too if helpful

stoic plank