switch to http.FileServer

Signed-off-by: Anik <anikbhattacharya93@gmail.com>

cmd/manager/main.go

@@ -122,22 +122,19 @@ func main() {
 		os.Exit(1)
 	}
 
-	serverMux := catalogserver.MuxForServer(storageDir)
-	catalogServer := catalogserver.New(storageDir, catalogServerAddr, serverMux)
+	catalogServer := catalogserver.New(storageDir, catalogServerAddr)
 	if err := mgr.Add(catalogServer); err != nil {
 		setupLog.Error(err, "unable to start catalog server")
 		os.Exit(1)
 	}
 
-	if _, err := os.Stat(storageDir); os.IsNotExist(err) {
-		if err := os.MkdirAll(storageDir, 0700); err != nil {
-			setupLog.Error(err, "unable to create storage directory for catalogs")
-		}
+	if err := os.MkdirAll(storageDir, 0700); err != nil {
+		setupLog.Error(err, "unable to create storage directory for catalogs")
 	}
 	if err = (&corecontrollers.CatalogReconciler{
 		Client:   mgr.GetClient(),
 		Unpacker: unpacker,
-		Storage:  storage.NewStorage(storageDir, serverMux),
+		Storage:  storage.NewStorage(storageDir),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "Catalog")
 		os.Exit(1)

pkg/catalogserver/server.go

@@ -2,10 +2,8 @@ package catalogserver
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 	"os"
-	"path/filepath"
 	"time"
 
 	"golang.org/x/sync/errgroup"
@@ -17,24 +15,22 @@ import (
 type Instance struct {
 	Dir             string
 	Addr            string
-	Mux             *http.ServeMux
 	ShutdownTimeout time.Duration
 }
 
 // New returns an Instance of a catalog server that serves
 // the FBC content stored in the given directory on the given
 // http address.
-func New(dir, addr string, mux *http.ServeMux) Instance {
+func New(dir, addr string) Instance {
 	return Instance{
 		Dir:  dir,
 		Addr: addr,
-		Mux:  mux,
 	}
 }
 
 func (s Instance) Start(ctx context.Context) error {
 	server := &http.Server{
-		Handler:           s.Mux,
+		Handler:           ServerHandler(s.Dir),
 		Addr:              s.Addr,
 		ReadHeaderTimeout: 3 * time.Second,
 	}
@@ -64,19 +60,7 @@ func (s Instance) Start(ctx context.Context) error {
 	return server.Shutdown(sc)
 }
 
-func MuxForServer(dir string) *http.ServeMux {
-	m := &http.ServeMux{}
-	m.HandleFunc("/catalogs", func(w http.ResponseWriter, r *http.Request) {
-		files, err := os.ReadDir(dir)
-		if err != nil {
-			w.WriteHeader(http.StatusBadRequest)
-			fmt.Fprintf(w, "error reading catalog store directory: %v", err)
-			return
-		}
-		for _, file := range files {
-			name := file.Name()
-			fmt.Fprintf(w, "%v\n", name[:len(name)-len(filepath.Ext(name))])
-		}
-	})
-	return m
+func ServerHandler(dir string) http.Handler {
+	fs := http.FileServer(http.FS(os.DirFS(dir)))
+	return fs
 }

pkg/controllers/core/catalog_controller_test.go

@@ -56,6 +56,7 @@ var _ = Describe("Catalogd Controller Test", func() {
 		reconciler *core.CatalogReconciler
 		mockSource *MockSource
 		testServer *httptest.Server
+		httpclient *http.Client
 	)
 	BeforeEach(func() {
 		tmpDir, err := os.MkdirTemp(GinkgoT().TempDir(), "cache")
@@ -69,9 +70,10 @@ var _ = Describe("Catalogd Controller Test", func() {
 					v1alpha1.SourceTypeImage: mockSource,
 				},
 			),
-			Storage: storage.NewStorage(tmpDir, http.NewServeMux()),
+			Storage: storage.NewStorage(tmpDir),
 		}
-		testServer = httptest.NewServer(catalogserver.MuxForServer(tmpDir))
+		testServer = httptest.NewServer(catalogserver.ServerHandler(tmpDir))
+		httpclient = &http.Client{}
 	})
 	AfterEach(func() {
 		testServer.Close()
@@ -286,45 +288,51 @@ var _ = Describe("Catalogd Controller Test", func() {
 				})
 
 				When("HTTPServer feature gate is enabled", func() {
+					var httpRequest *http.Request
 					BeforeEach(func() {
 						Expect(features.CatalogdFeatureGate.SetFromMap(map[string]bool{
 							string(features.PackagesBundleMetadataAPIs): false,
 							string(features.CatalogMetadataAPI):         false,
 							string(features.HTTPServer):                 true,
 						})).NotTo(HaveOccurred())
 
+						req, err := http.NewRequest("GET", testServer.URL, nil)
+						Expect(err).To(Not(HaveOccurred()))
+						req.Header.Set("Accept", "text/html")
+						httpRequest = req
+
 						// reconcile
 						res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: catalogKey})
 						Expect(res).To(Equal(ctrl.Result{}))
 						Expect(err).ToNot(HaveOccurred())
 					})
-					It("the catalog should become available at addr/catalogs", func() {
-						res, err := http.Get(testServer.URL + "/catalogs")
+					It("the catalog should become available at server endpoint", func() {
+						resp, err := httpclient.Do(httpRequest)
 						Expect(err).To(Not(HaveOccurred()))
-						catalogs, err := io.ReadAll(res.Body)
-						res.Body.Close()
+						defer resp.Body.Close()
+
+						catalogs, err := io.ReadAll(resp.Body)
 						Expect(err).To(Not(HaveOccurred()))
-						//omitting trailing new line char from response
-						Expect(string(catalogs[:len(catalogs)-1])).To(Equal(catalogKey.Name))
+						Expect(string(catalogs)).To(Equal(fmt.Sprintf(httpResponse, fmt.Sprintf("\n<a href=\"%s\">%s</a>", catalogKey.Name+".json", catalogKey.Name+".json"))))
 					})
 
 					When("the catalog is deleted", func() {
 						BeforeEach(func() {
 							Expect(cl.Delete(ctx, catalog)).Should(Succeed())
 						})
-						It("should not be available at addr/catalogs anymore", func() {
+						It("should not be available at server endpoint anymore", func() {
 							// reconcile
 							res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: catalogKey})
 							Expect(res).To(Equal(ctrl.Result{}))
 							Expect(err).ToNot(HaveOccurred())
 
-							newres, err := http.Get(testServer.URL + "/catalogs")
+							resp, err := httpclient.Do(httpRequest)
 							Expect(err).To(Not(HaveOccurred()))
-							catalogs, err := io.ReadAll(newres.Body)
-							newres.Body.Close()
+							defer resp.Body.Close()
+
+							catalogs, err := io.ReadAll(resp.Body)
 							Expect(err).To(Not(HaveOccurred()))
-							//omitting trailing new line char from response
-							Expect(string(catalogs)).To(Equal(""))
+							Expect(string(catalogs)).To(Equal(fmt.Sprintf(httpResponse, "")))
 						})
 					})
 				})
@@ -598,3 +606,7 @@ name: %s
 entries:
   - name: %s
 `
+
+const httpResponse = `<pre>%s
+</pre>
+`

pkg/storage/storage.go

@@ -3,7 +3,6 @@ package storage
 import (
 	"errors"
 	"fmt"
-	"net/http"
 	"os"
 	"path/filepath"
 
@@ -14,13 +13,11 @@ import (
 // It can be used to Store or Delete FBC in the host's filesystem
 type Storage struct {
 	RootDirectory string
-	ServerMux     *http.ServeMux
 }
 
-func NewStorage(rootDir string, mux *http.ServeMux) Storage {
+func NewStorage(rootDir string) Storage {
 	return Storage{
 		RootDirectory: rootDir,
-		ServerMux:     mux,
 	}
 }
 
@@ -34,9 +31,6 @@ func (s *Storage) Store(owner string, fbc *declcfg.DeclarativeConfig) error {
 	if err := declcfg.WriteJSON(*fbc, fbcFile); err != nil {
 		return err
 	}
-	s.ServerMux.HandleFunc(fmt.Sprintf("/catalogs/%s", owner), func(w http.ResponseWriter, r *http.Request) {
-		http.ServeFile(w, r, filepath.Join(s.RootDirectory, fmt.Sprintf("%s.json", owner)))
-	})
 	return nil
 }