many: add GPG key resolving and source

internal/test/helpers.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
-	"fmt"
 	"io"
 	"net"
 	"net/http"
@@ -18,8 +17,6 @@ import (
 	"github.com/BurntSushi/toml"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/uuid"
-	"github.com/osbuild/images/pkg/arch"
-	"github.com/osbuild/images/pkg/distro"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -196,17 +193,6 @@ func Ignore(what string) cmp.Option {
 	return cmp.FilterPath(func(p cmp.Path) bool { return p.String() == what }, cmp.Ignore())
 }
 
-// CompareImageType considers two image type objects equal if and only if the names of their distro/arch/imagetype
-// are. The thinking is that the objects are static, and resolving by these three keys should always give equivalent
-// objects. Whether we actually have object equality, is an implementation detail, so we don't want to rely on that.
-func CompareImageTypes() cmp.Option {
-	return cmp.Comparer(func(x, y distro.ImageType) bool {
-		return x.Name() == y.Name() &&
-			x.Arch().Name() == y.Arch().Name() &&
-			x.Arch().Distro().Name() == y.Arch().Distro().Name()
-	})
-}
-
 // Create a temporary repository
 func SetUpTemporaryRepository() (string, error) {
 	dir, err := os.MkdirTemp("/tmp", "osbuild-composer-test-")
@@ -232,19 +218,3 @@ func SetUpTemporaryRepository() (string, error) {
 func TearDownTemporaryRepository(dir string) error {
 	return os.RemoveAll(dir)
 }
-
-// GenerateCIArtifactName generates a new identifier for CI artifacts which is based
-// on environment variables specified by Jenkins
-// note: in case of migration to sth else like Github Actions, change it to whatever variables GH Action provides
-func GenerateCIArtifactName(prefix string) (string, error) {
-	distroCode := os.Getenv("DISTRO_CODE")
-	branchName := os.Getenv("BRANCH_NAME")
-	buildId := os.Getenv("BUILD_ID")
-	if branchName == "" || buildId == "" || distroCode == "" {
-		return "", fmt.Errorf("The environment variables must specify BRANCH_NAME, BUILD_ID, and DISTRO_CODE")
-	}
-
-	arch := arch.Current().String()
-
-	return fmt.Sprintf("%s%s-%s-%s-%s", prefix, distroCode, arch, branchName, buildId), nil
-}

internal/testarch/ci_artifact.go

@@ -0,0 +1,24 @@
+package testarch
+
+import (
+	"fmt"
+	"os"
+
+	"github.com/osbuild/images/pkg/arch"
+)
+
+// GenerateCIArtifactName generates a new identifier for CI artifacts which is based
+// on environment variables specified by Jenkins
+// note: in case of migration to sth else like Github Actions, change it to whatever variables GH Action provides
+func GenerateCIArtifactName(prefix string) (string, error) {
+	distroCode := os.Getenv("DISTRO_CODE")
+	branchName := os.Getenv("BRANCH_NAME")
+	buildId := os.Getenv("BUILD_ID")
+	if branchName == "" || buildId == "" || distroCode == "" {
+		return "", fmt.Errorf("The environment variables must specify BRANCH_NAME, BUILD_ID, and DISTRO_CODE")
+	}
+
+	archStr := arch.Current().String()
+
+	return fmt.Sprintf("%s%s-%s-%s-%s", prefix, distroCode, archStr, branchName, buildId), nil
+}

internal/testdistro/compare.go

@@ -0,0 +1,17 @@
+package testdistro
+
+import (
+	"github.com/google/go-cmp/cmp"
+	"github.com/osbuild/images/pkg/distro"
+)
+
+// CompareImageTypes considers two image type objects equal if and only if the names of their distro/arch/imagetype
+// are. The thinking is that the objects are static, and resolving by these three keys should always give equivalent
+// objects. Whether we actually have object equality, is an implementation detail, so we don't want to rely on that.
+func CompareImageTypes() cmp.Option {
+	return cmp.Comparer(func(x, y distro.ImageType) bool {
+		return x.Name() == y.Name() &&
+			x.Arch().Name() == y.Arch().Name() &&
+			x.Arch().Distro().Name() == y.Arch().Distro().Name()
+	})
+}

pkg/osbuild/curl_source.go

@@ -2,10 +2,16 @@ package osbuild
 
 import (
 	"bytes"
+	"context"
+	"crypto/sha256"
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
+	"net/http"
+	"net/url"
 	"regexp"
 
+	"github.com/osbuild/images/pkg/remotefile"
 	"github.com/osbuild/images/pkg/rpmmd"
 )
 
@@ -81,6 +87,37 @@ type URLSecrets struct {
 	Name string `json:"name"`
 }
 
+var resolveDoer remotefile.Doer = &http.Client{}
+
+// ValidURL checks if a URL string is valid and has a scheme and host.
+func IsValidURL(urlStr string) bool {
+	u, err := url.Parse(urlStr)
+	return err == nil && u.Scheme != "" && u.Host != ""
+}
+
+// ResolveAddURLs downloads each URL via the remotefile package, computes the
+// checksum, and adds a new item to the source.
+func (source *CurlSource) ResolveAddURLs(ctx context.Context, urls ...string) error {
+	if len(urls) == 0 {
+		return nil
+	}
+
+	resolver := remotefile.NewResolver(ctx, remotefile.WithDoer(resolveDoer))
+	resolver.Add(urls...)
+	specs, err := resolver.Finish()
+	if err != nil {
+		return err
+	}
+
+	for _, spec := range specs {
+		sum := sha256.Sum256(spec.Content)
+		checksum := "sha256:" + hex.EncodeToString(sum[:])
+		source.Items[checksum] = URL(spec.URL)
+	}
+
+	return nil
+}
+
 // Unmarshal method for CurlSource for handling the CurlSourceItem interface:
 // Tries each of the implementations until it finds the one that works.
 func (cs *CurlSource) UnmarshalJSON(data []byte) (err error) {

pkg/osbuild/curl_source_test.go

@@ -1,12 +1,41 @@
 package osbuild
 
 import (
+	"bytes"
+	"context"
+	"crypto/sha256"
+	"encoding/hex"
+	"errors"
+	"io"
+	"net/http"
 	"testing"
 
+	"github.com/osbuild/images/internal/test"
+	"github.com/osbuild/images/pkg/remotefile"
 	"github.com/osbuild/images/pkg/rpmmd"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
+// mockDoer is a remotefile.Doer that returns predefined bodies and status codes per URL.
+type mockDoer struct {
+	responses map[string]struct {
+		body   []byte
+		status int
+	}
+}
+
+func (m *mockDoer) Do(req *http.Request) (*http.Response, error) {
+	r, ok := m.responses[req.URL.String()]
+	if !ok {
+		return &http.Response{StatusCode: http.StatusNotFound, Body: io.NopCloser(bytes.NewReader(nil))}, nil
+	}
+	return &http.Response{
+		StatusCode: r.status,
+		Body:       io.NopCloser(bytes.NewReader(r.body)),
+	}, nil
+}
+
 func TestPackageSourceValidation(t *testing.T) {
 	assert := assert.New(t)
 
@@ -132,3 +161,100 @@ func TestPackageSourceValidation(t *testing.T) {
 		}
 	}
 }
+
+func TestResolveAddURLs(t *testing.T) {
+	type response struct {
+		body   []byte
+		status int
+	}
+	type wantItem struct {
+		body []byte
+		url  string
+	}
+	cases := []struct {
+		name      string
+		responses map[string]response
+		urls      []string
+		wantErr   error
+		wantItems []wantItem
+	}{
+		{
+			name:      "empty URLs",
+			responses: map[string]response{},
+			urls:      nil,
+			wantErr:   nil,
+			wantItems: nil,
+		},
+		{
+			name: "single URL",
+			responses: map[string]response{
+				"https://example.com/key1": {body: []byte("key1\n"), status: http.StatusOK},
+			},
+			urls:      []string{"https://example.com/key1"},
+			wantErr:   nil,
+			wantItems: []wantItem{{body: []byte("key1\n"), url: "https://example.com/key1"}},
+		},
+		{
+			name: "multiple URLs",
+			responses: map[string]response{
+				"https://example.com/key1": {body: []byte("key1\n"), status: http.StatusOK},
+				"https://example.com/key2": {body: []byte("key2\n"), status: http.StatusOK},
+			},
+			urls:    []string{"https://example.com/key1", "https://example.com/key2"},
+			wantErr: nil,
+			wantItems: []wantItem{
+				{body: []byte("key1\n"), url: "https://example.com/key1"},
+				{body: []byte("key2\n"), url: "https://example.com/key2"},
+			},
+		},
+		{
+			name:      "resolve error",
+			responses: map[string]response{},
+			urls:      []string{"https://example.com/notfound"},
+			wantErr:   errors.New("failed to resolve remote files"),
+		},
+		{
+			name: "non-OK status",
+			responses: map[string]response{
+				"https://example.com/error": {body: []byte("error"), status: http.StatusInternalServerError},
+			},
+			urls:    []string{"https://example.com/error"},
+			wantErr: errors.New("unexpected status 500"),
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			doer := &mockDoer{
+				responses: make(map[string]struct {
+					body   []byte
+					status int
+				}),
+			}
+			for u, r := range tc.responses {
+				doer.responses[u] = struct {
+					body   []byte
+					status int
+				}{body: r.body, status: r.status}
+			}
+			var doerIf remotefile.Doer = doer
+			test.MockGlobal(t, &resolveDoer, doerIf)
+			source := NewCurlSource()
+			err := source.ResolveAddURLs(context.Background(), tc.urls...)
+			if tc.wantErr != nil {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), tc.wantErr.Error())
+				assert.Empty(t, source.Items)
+				return
+			}
+			require.NoError(t, err)
+			require.Len(t, source.Items, len(tc.wantItems))
+			for _, wi := range tc.wantItems {
+				sum := sha256.Sum256(wi.body)
+				checksum := "sha256:" + hex.EncodeToString(sum[:])
+				item, ok := source.Items[checksum].(URL)
+				require.True(t, ok, "item for %s", checksum)
+				assert.Equal(t, wi.url, string(item))
+			}
+		})
+	}
+}

pkg/osbuild/rpm_stage.go

@@ -3,6 +3,7 @@ package osbuild
 import (
 	"fmt"
 	"slices"
+	"strings"
 
 	"github.com/osbuild/images/internal/common"
 	"github.com/osbuild/images/pkg/depsolvednf"
@@ -160,6 +161,9 @@ func pkgRefs(pkgs rpmmd.PackageList) FilesInputRef {
 // only the GPG keys needed for a specific set of packages, rather than
 // importing all keys from all configured repositories.
 //
+// Strings which are not valid ASCII armored GPG keys are removed from the
+// list before returning. This includes keys which are entered as URLs.
+//
 // Returns an error if:
 // - Any package has a nil Repo pointer (indicates a bug in depsolving)
 // - Any package requires GPG checking but its repo has no GPG keys configured
@@ -169,8 +173,7 @@ func pkgRefs(pkgs rpmmd.PackageList) FilesInputRef {
 // NOTE: Currently collects keys even for packages/repos with CheckGPG=false.
 // This could be changed if importing unused keys is not desirable.
 func GPGKeysForPackages(pkgs rpmmd.PackageList) ([]string, error) {
-	keyMap := make(map[string]bool)
-	var gpgKeys []string
+	var keys []string
 	for _, pkg := range pkgs {
 		if pkg.Repo == nil {
 			return nil, fmt.Errorf("package %q has nil Repo pointer. This is a bug in depsolving.", pkg.Name)
@@ -181,14 +184,17 @@ func GPGKeysForPackages(pkgs rpmmd.PackageList) ([]string, error) {
 				pkg.Name, pkg.Repo.Id)
 		}
 		for _, key := range pkg.Repo.GPGKeys {
-			if !keyMap[key] {
-				gpgKeys = append(gpgKeys, key)
-				keyMap[key] = true
+			if strings.HasPrefix(key, "-----BEGIN PGP") {
+				keys = append(keys, key)
 			}
 		}
 	}
-	slices.Sort(gpgKeys)
-	return gpgKeys, nil
+	slices.Sort(keys)
+	keys = slices.Compact(keys)
+	if len(keys) == 0 {
+		return nil, nil
+	}
+	return keys, nil
 }
 
 // GenRPMStagesFromTransactions creates RPM stages for each transaction.

pkg/osbuild/rpm_stage_test.go

@@ -216,11 +216,16 @@ func TestGPGKeysForPackages(t *testing.T) {
 	key2 := "-----BEGIN PGP PUBLIC KEY BLOCK-----\nkey2\n-----END PGP PUBLIC KEY BLOCK-----"
 	keyA := "-----BEGIN PGP PUBLIC KEY BLOCK-----\nkeyA\n-----END PGP PUBLIC KEY BLOCK-----"
 	keyB := "-----BEGIN PGP PUBLIC KEY BLOCK-----\nkeyB\n-----END PGP PUBLIC KEY BLOCK-----"
+	keyURL1 := "http://example.com/key1.gpg"
+	keyURL2 := "http://example.com/key2.gpg"
 
 	repoWithKey1 := &rpmmd.RepoConfig{GPGKeys: []string{key1}}
 	repoWithKey2 := &rpmmd.RepoConfig{GPGKeys: []string{key2}}
 	repoWithMultipleKeys := &rpmmd.RepoConfig{GPGKeys: []string{keyA, keyB}}
 	repoNoKeys := &rpmmd.RepoConfig{GPGKeys: nil}
+	repoWithOnlyURLKeys := &rpmmd.RepoConfig{GPGKeys: []string{keyURL1, keyURL2}}
+	repoWithInvalidKey := &rpmmd.RepoConfig{GPGKeys: []string{"invalid-key"}}
+	repoWithMixedKeys := &rpmmd.RepoConfig{GPGKeys: []string{key1, keyURL1}}
 
 	tests := map[string]struct {
 		pkgs        rpmmd.PackageList
@@ -266,6 +271,24 @@ func TestGPGKeysForPackages(t *testing.T) {
 			},
 			expected: []string{keyA, keyB},
 		},
+		"repo-with-url-keys": {
+			pkgs: rpmmd.PackageList{
+				{Name: "pkg1", Repo: repoWithOnlyURLKeys, CheckGPG: true},
+			},
+			expected: nil,
+		},
+		"repo-with-invalid-keys": {
+			pkgs: rpmmd.PackageList{
+				{Name: "pkg1", Repo: repoWithInvalidKey, CheckGPG: true},
+			},
+			expected: nil,
+		},
+		"repo-with-mixed-keys": {
+			pkgs: rpmmd.PackageList{
+				{Name: "pkg1", Repo: repoWithMixedKeys, CheckGPG: true},
+			},
+			expected: []string{key1},
+		},
 		// Error cases
 		"error-checkgpg-true-no-keys": {
 			pkgs: rpmmd.PackageList{