fix rebase issues, address review comments

Signed-off-by: Bryce Palmer <[email protected]>

cmd/manager/main.go

@@ -18,6 +18,7 @@ package main
 
 import (
 	"flag"
+	"net/http"
 	"os"
 
 	"github.com/spf13/pflag"
@@ -35,6 +36,7 @@ import (
 	rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
 
 	operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
+	"github.com/operator-framework/operator-controller/internal/catalogmetadata/cache"
 	catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client"
 	"github.com/operator-framework/operator-controller/internal/controllers"
 	"github.com/operator-framework/operator-controller/pkg/features"
@@ -56,14 +58,18 @@ func init() {
 }
 
 func main() {
-	var metricsAddr string
-	var enableLeaderElection bool
-	var probeAddr string
+	var (
+		metricsAddr          string
+		enableLeaderElection bool
+		probeAddr            string
+		cachePath            string
+	)
 	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
 	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
 	flag.BoolVar(&enableLeaderElection, "leader-elect", false,
 		"Enable leader election for controller manager. "+
 			"Enabling this will ensure there is only one active controller manager.")
+	flag.StringVar(&cachePath, "cache-path", "/var/cache", "The local directory path used for filesystem based caching")
 	opts := zap.Options{
 		Development: true,
 	}
@@ -100,7 +106,7 @@ func main() {
 	}
 
 	cl := mgr.GetClient()
-	catalogClient := catalogclient.New(cl)
+	catalogClient := catalogclient.New(cl, cache.NewFilesystemCache(cachePath, http.DefaultClient))
 
 	if err = (&controllers.OperatorReconciler{
 		Client: cl,

config/manager/manager.yaml

@@ -72,6 +72,9 @@ spec:
         image: controller:latest
         imagePullPolicy: IfNotPresent
         name: manager
+        volumeMounts:
+          - name: cache
+            mountPath: /var/cache
         securityContext:
           allowPrivilegeEscalation: false
           capabilities:
@@ -97,3 +100,6 @@ spec:
             memory: 64Mi
       serviceAccountName: controller-manager
       terminationGracePeriodSeconds: 10
+      volumes:
+        - name: cache
+          emptyDir: {}

go.mod

@@ -9,8 +9,8 @@ require (
 	github.com/onsi/ginkgo/v2 v2.12.1
 	github.com/onsi/gomega v1.27.10
 	github.com/operator-framework/api v0.17.4-0.20230223191600-0131a6301e42
-	github.com/operator-framework/deppy v0.0.1
 	github.com/operator-framework/catalogd v0.7.0
+	github.com/operator-framework/deppy v0.0.1
 	github.com/operator-framework/operator-registry v1.28.0
 	github.com/operator-framework/rukpak v0.13.0
 	github.com/spf13/pflag v1.0.5

go.sum

@@ -702,8 +702,8 @@ github.com/opencontainers/selinux v1.8.0/go.mod h1:RScLhm78qiWa2gbVCcGkC7tCGdgk3
 github.com/opencontainers/selinux v1.8.2/go.mod h1:MUIHuUEvKB1wtJjQdOyYRgOnLD2xAPP8dBsCoU0KuF8=
 github.com/operator-framework/api v0.17.4-0.20230223191600-0131a6301e42 h1:d/Pnr19TnmIq3zQ6ebewC+5jt5zqYbRkvYd37YZENQY=
 github.com/operator-framework/api v0.17.4-0.20230223191600-0131a6301e42/go.mod h1:l/cuwtPxkVUY7fzYgdust2m9tlmb8I4pOvbsUufRb24=
-github.com/operator-framework/catalogd v0.6.0 h1:dSZ54MVSHJ8hcoV7OCRxnk3x4O3ramlyPvvz0vsKYdk=
-github.com/operator-framework/catalogd v0.6.0/go.mod h1:I0n086a4a+nP1YZy742IrPaWvOlWu0Mj6qA6j4K96Vg=
+github.com/operator-framework/catalogd v0.7.0 h1:L0uesxq+r59rGubtxMoVtIShKn7gSSSLqxpWLfwpAaw=
+github.com/operator-framework/catalogd v0.7.0/go.mod h1:tVhaenJVFTHHgdJ0Pju7U4G3epeoZfUWWM1J5nPISPQ=
 github.com/operator-framework/deppy v0.0.1 h1:PLTtaFGwktPhKuKZkfUruTimrWpyaO3tghbsjs0uMjc=
 github.com/operator-framework/deppy v0.0.1/go.mod h1:EV6vnxRodSFRn2TFztfxFhMPGh5QufOhn3tpIP1Z8cc=
 github.com/operator-framework/operator-registry v1.28.0 h1:vtmd2WgJxkx7vuuOxW4k5Le/oo0SfonSeJVMU3rKIfk=

internal/catalogmetadata/cache/cache.go

@@ -24,11 +24,11 @@ var _ client.Fetcher = &filesystemCache{}
 //   - IF cached it will verify the cache is up to date. If it is up to date it will return
 //     the cached contents, if not it will fetch the new contents from the catalogd HTTP
 //     server and update the cached contents.
-func NewFilesystemCache(cachePath string, tripper http.RoundTripper) client.Fetcher {
+func NewFilesystemCache(cachePath string, client *http.Client) client.Fetcher {
 	return &filesystemCache{
 		cachePath:              cachePath,
 		mutex:                  sync.RWMutex{},
-		tripper:                tripper,
+		client:                 client,
 		cacheDataByCatalogName: map[string]cacheData{},
 	}
 }
@@ -49,7 +49,7 @@ type cacheData struct {
 type filesystemCache struct {
 	mutex                  sync.RWMutex
 	cachePath              string
-	tripper                http.RoundTripper
+	client                 *http.Client
 	cacheDataByCatalogName map[string]cacheData
 }
 
@@ -96,19 +96,14 @@ func (fsc *filesystemCache) FetchCatalogContents(ctx context.Context, catalog *c
 		return nil, fmt.Errorf("error forming request: %s", err)
 	}
 
-	resp, err := fsc.tripper.RoundTrip(req)
+	resp, err := fsc.client.Do(req)
 	if err != nil {
 		return nil, fmt.Errorf("error performing request: %s", err)
 	}
 	defer resp.Body.Close()
 
 	switch resp.StatusCode {
 	case http.StatusOK:
-		contents, err := io.ReadAll(resp.Body)
-		if err != nil {
-			return nil, fmt.Errorf("error reading response body: %s", err)
-		}
-
 		fsc.mutex.Lock()
 		defer fsc.mutex.Unlock()
 
@@ -128,8 +123,13 @@ func (fsc *filesystemCache) FetchCatalogContents(ctx context.Context, catalog *c
 			return nil, fmt.Errorf("error creating cache directory for Catalog %q: %s", catalog.Name, err)
 		}
 
-		if err = os.WriteFile(cacheFilePath, contents, os.ModePerm); err != nil {
-			return nil, fmt.Errorf("error caching response: %s", err)
+		file, err := os.Create(cacheFilePath)
+		if err != nil {
+			return nil, fmt.Errorf("error creating cache file for Catalog %q: %s", catalog.Name, err)
+		}
+
+		if _, err := io.Copy(file, resp.Body); err != nil {
+			return nil, fmt.Errorf("error writing contents to cache for Catalog %q: %s", catalog.Name, err)
 		}
 
 		fsc.cacheDataByCatalogName[catalog.Name] = cacheData{

internal/catalogmetadata/cache/cache_test.go

@@ -220,7 +220,9 @@ func TestClient(t *testing.T) {
 				ctx := context.Background()
 				cacheDir := t.TempDir()
 				tt.tripper.content = tt.contents
-				c := cache.NewFilesystemCache(cacheDir, tt.tripper)
+				httpClient := http.DefaultClient
+				httpClient.Transport = tt.tripper
+				c := cache.NewFilesystemCache(cacheDir, httpClient)
 
 				rc, err := c.FetchCatalogContents(ctx, tt.catalog)
 				if !tt.wantErr {

test/operator-framework-e2e/operator_framework_test.go

@@ -200,10 +200,6 @@ var _ = Describe("Operator Framework E2E for plain+v0 bundles", func() {
 		Expect(err).ToNot(HaveOccurred())
 
 		By("creating a Catalog CR and verifying the creation of respective packages and bundle metadata")
-		bundleVersions := make([]string, len(bundleInfo.bundles))
-		for i, bundle := range bundleInfo.bundles {
-			bundleVersions[i] = bundle.bundleVersion
-		}
 		operatorCatalog, err = createCatalogCheckResources(operatorCatalog, catalogDInfo)
 		Expect(err).ToNot(HaveOccurred())
 
@@ -336,10 +332,6 @@ var _ = Describe("Operator Framework E2E for registry+v1 bundles", func() {
 		Expect(err).ToNot(HaveOccurred())
 
 		By("creating a Catalog CR and verifying the creation of respective packages and bundle metadata")
-		bundleVersions := make([]string, len(bundleInfo.bundles))
-		for i, bundle := range bundleInfo.bundles {
-			bundleVersions[i] = bundle.bundleVersion
-		}
 		operatorCatalog, err = createCatalogCheckResources(operatorCatalog, catalogDInfo)
 		Expect(err).ToNot(HaveOccurred())