Restructure based on feedback

cmd/manager/main.go

@@ -18,7 +18,6 @@ package main
 
 import (
 	"crypto/tls"
-	"crypto/x509"
 	"flag"
 	"fmt"
 	"log"
@@ -27,7 +26,6 @@ import (
 	"path/filepath"
 	"time"
 
-	"github.com/go-logr/logr"
 	"github.com/spf13/pflag"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -46,6 +44,7 @@ import (
 	corecontrollers "github.com/operator-framework/catalogd/internal/controllers/core"
 	"github.com/operator-framework/catalogd/internal/features"
 	"github.com/operator-framework/catalogd/internal/garbagecollection"
+	"github.com/operator-framework/catalogd/internal/httputil"
 	catalogdmetrics "github.com/operator-framework/catalogd/internal/metrics"
 	"github.com/operator-framework/catalogd/internal/serverutil"
 	"github.com/operator-framework/catalogd/internal/source"
@@ -179,7 +178,7 @@ func main() {
 		os.Exit(1)
 	}
 
-	certPool, err := newCertPool(caCertDir, ctrl.Log.WithName("cert-pool"))
+	certPool, err := httputil.NewCertPool(caCertDir, ctrl.Log.WithName("cert-pool"))
 	if err != nil {
 		setupLog.Error(err, "unable to create CA certificate pool")
 		os.Exit(1)
@@ -279,50 +278,3 @@ func podNamespace() string {
 	}
 	return string(namespace)
 }
-
-// Should share code from operator-controller.
-// see: https://issues.redhat.com/browse/OPRUN-3535
-func newCertPool(caDir string, log logr.Logger) (*x509.CertPool, error) {
-	caCertPool, err := x509.SystemCertPool()
-	if err != nil {
-		return nil, err
-	}
-	if caDir == "" {
-		return caCertPool, nil
-	}
-
-	dirEntries, err := os.ReadDir(caDir)
-	if err != nil {
-		return nil, err
-	}
-	count := 0
-
-	for _, e := range dirEntries {
-		file := filepath.Join(caDir, e.Name())
-		// These might be symlinks pointing to directories, so use Stat() to resolve
-		fi, err := os.Stat(file)
-		if err != nil {
-			return nil, err
-		}
-		if fi.IsDir() {
-			log.Info("skip directory", "name", e.Name())
-			continue
-		}
-		log.Info("load certificate", "name", e.Name())
-		data, err := os.ReadFile(file)
-		if err != nil {
-			return nil, fmt.Errorf("error reading cert file %q: %w", file, err)
-		}
-
-		if ok := caCertPool.AppendCertsFromPEM(data); ok {
-			count++
-		}
-	}
-
-	// Found no certs!
-	if count == 0 {
-		return nil, fmt.Errorf("no certificates found in %q", caDir)
-	}
-
-	return caCertPool, nil
-}

internal/httputil/certutil.go

@@ -0,0 +1,57 @@
+package httputil
+
+import (
+	"crypto/x509"
+	"fmt"
+	"os"
+	"path/filepath"
+
+	"github.com/go-logr/logr"
+)
+
+// Should share code from operator-controller.
+// see: https://issues.redhat.com/browse/OPRUN-3535
+func NewCertPool(caDir string, log logr.Logger) (*x509.CertPool, error) {
+	caCertPool, err := x509.SystemCertPool()
+	if err != nil {
+		return nil, err
+	}
+	if caDir == "" {
+		return caCertPool, nil
+	}
+
+	dirEntries, err := os.ReadDir(caDir)
+	if err != nil {
+		return nil, err
+	}
+	count := 0
+
+	for _, e := range dirEntries {
+		file := filepath.Join(caDir, e.Name())
+		// These might be symlinks pointing to directories, so use Stat() to resolve
+		fi, err := os.Stat(file)
+		if err != nil {
+			return nil, err
+		}
+		if fi.IsDir() {
+			log.Info("skip directory", "name", e.Name())
+			continue
+		}
+		log.Info("load certificate", "name", e.Name())
+		data, err := os.ReadFile(file)
+		if err != nil {
+			return nil, fmt.Errorf("error reading cert file %q: %w", file, err)
+		}
+
+		if ok := caCertPool.AppendCertsFromPEM(data); ok {
+			count++
+		}
+	}
+
+	// Found no certs!
+	if count == 0 {
+		return nil, fmt.Errorf("no certificates found in %q", caDir)
+	}
+
+	return caCertPool, nil
+}

cmd/manager/main_test.go → internal/httputil/main_test.go

@@ -1,4 +1,4 @@
-package main
+package httputil
 
 import (
 	"crypto/rand"
@@ -15,7 +15,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func Test_newCertPool(t *testing.T) {
+func TestNewCertPool(t *testing.T) {
 	t.Parallel()
 
 	// set up our CA certificate
@@ -60,14 +60,14 @@ func Test_newCertPool(t *testing.T) {
 	})
 	require.NoError(t, err)
 
-	_, err = newCertPool("testdata/newCertPool", testr.New(t))
+	_, err = NewCertPool("testdata/newCertPool", testr.New(t))
 	require.NoError(t, err)
 }
 
 func Test_newCertPool_empty(t *testing.T) {
 	err := os.MkdirAll("testdata/newCertPoolEmpty", 0700)
 	require.NoError(t, err)
 
-	_, err = newCertPool("testdata/newCertPoolEmpty", testr.New(t))
+	_, err = NewCertPool("testdata/newCertPoolEmpty", testr.New(t))
 	require.EqualError(t, err, `no certificates found in "testdata/newCertPoolEmpty"`)
 }

internal/source/image_registry_client.go

@@ -63,7 +63,6 @@ func (i *ImageRegistry) Unpack(ctx context.Context, catalog *catalogdv1alpha1.Cl
 	if i.CertPool != nil {
 		tlsTransport.TLSClientConfig.RootCAs = i.CertPool
 	}
-	remoteOpts = append(remoteOpts, remote.WithTransport(tlsTransport))
 
 	if catalog.Spec.Source.Image.PullSecret != "" {
 		chainOpts := k8schain.Options{
@@ -89,6 +88,8 @@ func (i *ImageRegistry) Unpack(ctx context.Context, catalog *catalogdv1alpha1.Cl
 		}
 		insecureTransport.TLSClientConfig.InsecureSkipVerify = true // nolint:gosec
 		remoteOpts = append(remoteOpts, remote.WithTransport(insecureTransport))
+	} else {
+		remoteOpts = append(remoteOpts, remote.WithTransport(tlsTransport))
 	}
 
 	digest, isDigest := imgRef.(name.Digest)