containers / buildah

A tool that facilitates building OCI images.
https://buildah.io
Apache License 2.0
7.44k stars 785 forks source link

FR: Support ssh:// protocol #4032

Open bendem opened 2 years ago

bendem commented 2 years ago

Currently, it is possible to build an image from a git repository by prefixing the context argument with git:// or postfixing with .git. Sadly, this uses the raw git protocol without authentication. Most users want the https protocol or git over ssh (git+ssh).

Since the clone is delegated to the git command, you need to specify either ssh:// or git+ssh:// to use that protocol. Neither of those prefixes is recognized by buildah.

I have added the relevant lines in the code (types.go, strings.HasPrefix(url, "git+ssh://") in the two places required), but I'm not able to comprehend the code further. It looks from my testing like the cloning happens in an isolated namespace which doesn't have access either to ~/.ssh or to the SSH_AUTH_SOCK, whether --ssh is used or not.

Would love to hear what others think about this and whether it would be possible to allow building images from ssh repositories.

> buildah build --debug 'git+ssh://git@github.com/containers/buildah.git'
DEBU[0000] cloning "git+ssh://git@github.com/containers/buildah.git" to "/var/tmp/buildah1505040640"
error prepping temporary context directory: cloning "git+ssh://git@github.com/containers/buildah.git" to "/var/tmp/buildah1505040640":
Cloning into '/var/tmp/buildah1505040640'...
Bad owner or permissions on /etc/ssh/ssh_config.d/05-redhat.conf
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
: exit status 128
DEBU[0000] exit status 128
Version:         1.27.0-dev
Go Version:      go1.17.7
Image Spec:      1.0.2-dev
Runtime Spec:    1.0.2-dev
CNI Spec:        1.0.0
libcni Version:  v1.1.1
image Version:   5.21.2-dev
Git Commit:      fb28761c
Built:           Fri Jun  3 14:31:44 2022
OS/Arch:         linux/amd64
BuildPlatform:   linux/amd64
rhatdan commented 2 years ago

@nalind @flouthoc PTAL

github-actions[bot] commented 2 years ago

A friendly reminder that this issue had no activity for 30 days.

flouthoc commented 2 years ago

@bendem Does your patch works when you run with sudo or root user ? I think issues is with running git from unshared session.

bendem commented 2 years ago

Indeed, it does work when using root. The git clone works ~, It's failing due to iptables errors, but I'm guessing that's an entirely different problem~ (iptables error is fixed by --network=host because WSL).

bendem commented 2 years ago

Patch in case people want it:

From a0eb30551cb4b910523392a851a72816e409aadf Mon Sep 17 00:00:00 2001
From: Benjamin Demarteau <benjamin.demarteau@liege.be>
Date: Mon, 11 Jul 2022 10:23:40 +0200
Subject: [PATCH] support building ssh:// urls

---
 define/types.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/define/types.go b/define/types.go
index 07d90081..1e412a92 100644
--- a/define/types.go
+++ b/define/types.go
@@ -117,6 +117,7 @@ func TempDirForURL(dir, prefix, url string) (name string, subdir string, err err
        if !strings.HasPrefix(url, "http://") &&
                !strings.HasPrefix(url, "https://") &&
                !strings.HasPrefix(url, "git://") &&
+               !strings.HasPrefix(url, "ssh://") &&
                !strings.HasPrefix(url, "github.com/") &&
                url != "-" {
                return "", "", nil
@@ -129,7 +130,7 @@ func TempDirForURL(dir, prefix, url string) (name string, subdir string, err err
        if err != nil {
                return "", "", fmt.Errorf("error parsing url %q: %w", url, err)
        }
-       if strings.HasPrefix(url, "git://") || strings.HasSuffix(urlParsed.Path, ".git") {
+       if strings.HasPrefix(url, "git://") || strings.HasPrefix(url, "ssh://") || strings.HasSuffix(urlParsed.Path, ".git") {
                combinedOutput, gitSubDir, err := cloneToDirectory(url, name)
                if err != nil {
                        if err2 := os.RemoveAll(name); err2 != nil {
--
2.31.1
bendem commented 2 years ago

For reference, docker checks URLs a bit different and thus doesn't support git+ssh either. URL has to start with git@, git:// or being an http url ending with .git + ref/subfolder.

https://github.com/docker/cli/blob/8dd94d382409b7384d210a2ed84b52f0d4a12995/vendor/github.com/docker/docker/builder/remotecontext/urlutil/urlutil.go#L79

rhatdan commented 2 years ago

Please open a PR with your patch.

bendem commented 2 years ago

The change is both incomplete and trivial. I'm unable to make it work rootless or add tests. Feel free to take it as your own.

If you really want, I can open a PR, but I won't be able to take it any further than this so it will just sit there.

flouthoc commented 2 years ago

I think issue is more like that user inside unshare session cannot access the global ssh config and altering file on host just for making it work for buildah rootless looks weird to me, I'll try playing with this patch @bendem does this feature works on docker ? But even if it works its not a surprise since everything is accessed from root user there which is same when buildah is invoked from root user.

bendem commented 2 years ago

It only works in docker if your git server uses git as the connecting user since you the only URL format that will use git+ssh is git@$server[#ref][:directory].

i.e. docker build -t buildah 'git@github.com:containers/buildah.git#main:/contrib/docker'

ref: https://github.com/docker/cli/blob/master/vendor/github.com/docker/docker/builder/remotecontext/urlutil/urlutil.go#L78

bendem commented 2 years ago

Opened https://github.com/docker/buildx/issues/1209

bendem commented 2 years ago

Docker side mentions me that + in urls is deprecated (no source) and that they support (doesn't actually work) ssh://, which makes more sense in my opinion too.

github-actions[bot] commented 2 years ago

A friendly reminder that this issue had no activity for 30 days.

rhatdan commented 2 years ago

@bendem @flouthoc What is going on with this issue?

bendem commented 2 years ago

Nothing, I don't have the knowledge to implement this, but it's still an open feature request.

rhatdan commented 2 years ago

@flouthoc WDYT?

github-actions[bot] commented 2 years ago

A friendly reminder that this issue had no activity for 30 days.

bendem commented 2 years ago

Still valid.

flouthoc commented 2 years ago

I think issue is more like that user inside unshare session cannot access the global ssh config and altering file on host just for making it work for buildah rootless looks weird to me, I'll try playing with this patch @bendem does this feature works on docker ? But even if it works its not a surprise since everything is accessed from root user there which is same when buildah is invoked from root user.

This is blocked because of this reason but lets wait for reply on docker issue first and see how it goes there: https://github.com/docker/buildx/issues/1209

github-actions[bot] commented 1 year ago

A friendly reminder that this issue had no activity for 30 days.

rhatdan commented 1 year ago

Looks like not much is happening here.

github-actions[bot] commented 1 year ago

A friendly reminder that this issue had no activity for 30 days.

rhatdan commented 1 year ago

@flouthoc PTAL

github-actions[bot] commented 1 year ago

A friendly reminder that this issue had no activity for 30 days.

github-actions[bot] commented 1 year ago

A friendly reminder that this issue had no activity for 30 days.