Merge pull request 'feat(webhook): sourcehut: submit SSH URL for private repository (or when pre-filled)' (#6445) from oliverpool/forgejo:sourcehut-refactor into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6445
Reviewed-by: Gusted <gusted@noreply.codeberg.org>

release-notes/6445.md

@@ -0,0 +1 @@
+feat: webhook: sourcehut: submit SSH URL for private repository or when pre-filled

services/webhook/sourcehut/builds.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"fmt"
 	"html/template"
+	"io"
 	"io/fs"
 	"net/http"
 	"strings"
@@ -189,11 +190,11 @@ func (pc sourcehutConvertor) Package(_ *api.PackagePayload) (graphqlPayload[buil
 	return graphqlPayload[buildsVariables]{}, shared.ErrPayloadTypeNotSupported
 }
 
-// mustBuildManifest adjusts the manifest to submit to the builds service
+// newPayload opens and adjusts the manifest to submit to the builds service
 //
 // in case of an error the Error field will be set, to be visible by the end-user under recent deliveries
 func (pc sourcehutConvertor) newPayload(repo *api.Repository, commitID, ref, note string, trusted bool) (graphqlPayload[buildsVariables], error) {
-	manifest, err := pc.buildManifest(repo, commitID, ref)
+	manifest, err := pc.constructManifest(repo, commitID, ref)
 	if err != nil {
 		if len(manifest) == 0 {
 			return graphqlPayload[buildsVariables]{}, err
@@ -238,9 +239,9 @@ func (pc sourcehutConvertor) newPayload(repo *api.Repository, commitID, ref, not
 	}, nil
 }
 
-// buildManifest adjusts the manifest to submit to the builds service
+// constructManifest opens and adjusts the manifest to submit to the builds service
 // in case of an error the []byte might contain an error that can be displayed to the user
-func (pc sourcehutConvertor) buildManifest(repo *api.Repository, commitID, gitRef string) ([]byte, error) {
+func (pc sourcehutConvertor) constructManifest(repo *api.Repository, commitID, gitRef string) ([]byte, error) {
 	gitRepo, err := gitrepo.OpenRepository(pc.ctx, repo)
 	if err != nil {
 		msg := "could not open repository"
@@ -265,6 +266,10 @@ func (pc sourcehutConvertor) buildManifest(repo *api.Repository, commitID, gitRe
 	}
 	defer r.Close()
 
+	return adjustManifest(repo, commitID, gitRef, r, pc.meta.ManifestPath)
+}
+
+func adjustManifest(repo *api.Repository, commitID, gitRef string, r io.Reader, path string) ([]byte, error) {
 	// reference: https://man.sr.ht/builds.sr.ht/manifest.md
 	var manifest struct {
 		Sources     []string          `yaml:"sources"`
@@ -273,7 +278,7 @@ func (pc sourcehutConvertor) buildManifest(repo *api.Repository, commitID, gitRe
 		Rest map[string]yaml.Node `yaml:",inline"`
 	}
 	if err := yaml.NewDecoder(r).Decode(&manifest); err != nil {
-		msg := fmt.Sprintf("could not decode manifest %q", pc.meta.ManifestPath)
+		msg := fmt.Sprintf("could not decode manifest %q", path)
 		return []byte(msg), fmt.Errorf(msg+": %w", err)
 	}
 
@@ -284,16 +289,21 @@ func (pc sourcehutConvertor) buildManifest(repo *api.Repository, commitID, gitRe
 	manifest.Environment["BUILD_SUBMITTER_URL"] = setting.AppURL
 	manifest.Environment["GIT_REF"] = gitRef
 
-	source := repo.CloneURL + "#" + commitID
 	found := false
 	for i, s := range manifest.Sources {
-		if s == repo.CloneURL {
+		if s == repo.CloneURL || s == repo.SSHURL {
-			manifest.Sources[i] = source
+			manifest.Sources[i] = s + "#" + commitID
 			found = true
 			break
 		}
 	}
 	if !found {
+		source := repo.CloneURL
+		if repo.Private || setting.Repository.DisableHTTPGit {
+			// default to ssh for private repos or when git clone is disabled over http
+			source = repo.SSHURL
+		}
+		source += "#" + commitID
 		manifest.Sources = append(manifest.Sources, source)
 	}
 

services/webhook/sourcehut/builds_test.go

@@ -5,6 +5,7 @@ package sourcehut
 
 import (
 	"context"
+	"strings"
 	"testing"
 
 	webhook_model "code.gitea.io/gitea/models/webhook"
@@ -384,3 +385,134 @@ func TestSourcehutJSONPayload(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, "json test", body.Variables.Note)
 }
+
+func TestSourcehutAdjustManifest(t *testing.T) {
+	defer test.MockVariableValue(&setting.AppURL, "https://example.forgejo.org/")()
+	t.Run("without sources", func(t *testing.T) {
+		repo := &api.Repository{
+			CloneURL: "http://localhost:3000/testdata/repo.git",
+		}
+
+		manifest, err := adjustManifest(repo, "58771003157b81abc6bf41df0c5db4147a3e3c83", "refs/heads/main", strings.NewReader(`image: alpine/edge
+tasks:
+    - say-hello: |
+        echo hello
+    - say-world: echo world`), ".build.yml")
+
+		require.NoError(t, err)
+		assert.Equal(t, `sources:
+    - http://localhost:3000/testdata/repo.git#58771003157b81abc6bf41df0c5db4147a3e3c83
+environment:
+    BUILD_SUBMITTER: forgejo
+    BUILD_SUBMITTER_URL: https://example.forgejo.org/
+    GIT_REF: refs/heads/main
+image: alpine/edge
+tasks:
+    - say-hello: |
+        echo hello
+    - say-world: echo world
+`, string(manifest))
+	})
+
+	t.Run("with other sources", func(t *testing.T) {
+		repo := &api.Repository{
+			CloneURL: "http://localhost:3000/testdata/repo.git",
+		}
+
+		manifest, err := adjustManifest(repo, "58771003157b81abc6bf41df0c5db4147a3e3c83", "refs/heads/main", strings.NewReader(`image: alpine/edge
+sources:
+- http://other.example.conm/repo.git
+tasks:
+    - hello: echo world`), ".build.yml")
+
+		require.NoError(t, err)
+		assert.Equal(t, `sources:
+    - http://other.example.conm/repo.git
+    - http://localhost:3000/testdata/repo.git#58771003157b81abc6bf41df0c5db4147a3e3c83
+environment:
+    BUILD_SUBMITTER: forgejo
+    BUILD_SUBMITTER_URL: https://example.forgejo.org/
+    GIT_REF: refs/heads/main
+image: alpine/edge
+tasks:
+    - hello: echo world
+`, string(manifest))
+	})
+
+	t.Run("with same source", func(t *testing.T) {
+		repo := &api.Repository{
+			CloneURL: "http://localhost:3000/testdata/repo.git",
+		}
+
+		manifest, err := adjustManifest(repo, "58771003157b81abc6bf41df0c5db4147a3e3c83", "refs/heads/main", strings.NewReader(`image: alpine/edge
+sources:
+- http://localhost:3000/testdata/repo.git
+- http://other.example.conm/repo.git
+tasks:
+    - hello: echo world`), ".build.yml")
+
+		require.NoError(t, err)
+		assert.Equal(t, `sources:
+    - http://localhost:3000/testdata/repo.git#58771003157b81abc6bf41df0c5db4147a3e3c83
+    - http://other.example.conm/repo.git
+environment:
+    BUILD_SUBMITTER: forgejo
+    BUILD_SUBMITTER_URL: https://example.forgejo.org/
+    GIT_REF: refs/heads/main
+image: alpine/edge
+tasks:
+    - hello: echo world
+`, string(manifest))
+	})
+
+	t.Run("with ssh source", func(t *testing.T) {
+		repo := &api.Repository{
+			CloneURL: "http://localhost:3000/testdata/repo.git",
+			SSHURL:   "git@localhost:testdata/repo.git",
+		}
+
+		manifest, err := adjustManifest(repo, "58771003157b81abc6bf41df0c5db4147a3e3c83", "refs/heads/main", strings.NewReader(`image: alpine/edge
+sources:
+- git@localhost:testdata/repo.git
+- http://other.example.conm/repo.git
+tasks:
+    - hello: echo world`), ".build.yml")
+
+		require.NoError(t, err)
+		assert.Equal(t, `sources:
+    - git@localhost:testdata/repo.git#58771003157b81abc6bf41df0c5db4147a3e3c83
+    - http://other.example.conm/repo.git
+environment:
+    BUILD_SUBMITTER: forgejo
+    BUILD_SUBMITTER_URL: https://example.forgejo.org/
+    GIT_REF: refs/heads/main
+image: alpine/edge
+tasks:
+    - hello: echo world
+`, string(manifest))
+	})
+
+	t.Run("private without source", func(t *testing.T) {
+		repo := &api.Repository{
+			CloneURL: "http://localhost:3000/testdata/repo.git",
+			SSHURL:   "git@localhost:testdata/repo.git",
+			Private:  true,
+		}
+
+		manifest, err := adjustManifest(repo, "58771003157b81abc6bf41df0c5db4147a3e3c83", "refs/heads/main", strings.NewReader(`image: alpine/edge
+tasks:
+    - hello: echo world`), ".build.yml")
+
+		require.NoError(t, err)
+		assert.Equal(t, `sources:
+    - git@localhost:testdata/repo.git#58771003157b81abc6bf41df0c5db4147a3e3c83
+environment:
+    BUILD_SUBMITTER: forgejo
+    BUILD_SUBMITTER_URL: https://example.forgejo.org/
+    GIT_REF: refs/heads/main
+image: alpine/edge
+tasks:
+    - hello: echo world
+`, string(manifest))
+	})
+}