From 45bca07ec2ad6af2443a2635f3cdc6bf7c0bb82d Mon Sep 17 00:00:00 2001
From: Simon Howe <simon@weave.works>
Date: Wed, 13 Sep 2023 09:32:13 +0200
Subject: [PATCH 1/2] Expose OSS static asset handling so we unify OSS/EE
 serving

- Asset handling is growing more logic like injecting <base> tags
- We should have the same code in OSS/EE
---
 cmd/gitops-server/cmd/cmd.go          |  70 ++-------------
 pkg/server/static_assets.go           |  61 +++++++++++++
 pkg/server/static_assets_test.go      | 119 ++++++++++++++++++++++++++
 pkg/server/testdata/public/index.html |   6 ++
 4 files changed, 192 insertions(+), 64 deletions(-)
 create mode 100644 pkg/server/static_assets.go
 create mode 100644 pkg/server/static_assets_test.go
 create mode 100644 pkg/server/testdata/public/index.html

diff --git a/cmd/gitops-server/cmd/cmd.go b/cmd/gitops-server/cmd/cmd.go
index 81dbc262c1..a4f2e0bb5f 100644
--- a/cmd/gitops-server/cmd/cmd.go
+++ b/cmd/gitops-server/cmd/cmd.go
@@ -11,7 +11,6 @@ import (
 	"os"
 	"os/signal"
 	"path"
-	"path/filepath"
 	"strings"
 	"syscall"
 	"time"
@@ -157,9 +156,6 @@ func runCmd(cmd *cobra.Command, args []string) error {
 		}
 	}))
 
-	assetFS := getAssets()
-	assetHandler := http.FileServer(http.FS(assetFS))
-	redirector := createRedirector(assetFS, log, options.RoutePrefix)
 	clusterName := kube.InClusterConfigClusterName()
 
 	rest, err := config.GetConfig()
@@ -271,18 +267,12 @@ func runCmd(cmd *cobra.Command, args []string) error {
 
 	mux.Handle("/v1/", gziphandler.GzipHandler(appAndProfilesHandlers))
 
-	mux.Handle("/", gziphandler.GzipHandler(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		// Assume anything with a file extension in the name is a static asset.
-		extension := filepath.Ext(req.URL.Path)
-		// We use the golang http.FileServer for static file requests.
-		// This will return a 404 on normal page requests, ie /some-page.
-		// Redirect all non-file requests to index.html, where the JS routing will take over.
-		if extension == "" {
-			redirector(w, req)
-			return
-		}
-		assetHandler.ServeHTTP(w, req)
-	})))
+	// Static asset handling
+	assetFS := getAssets()
+	assertFSHandler := http.FileServer(http.FS(assetFS))
+	redirectHandler := server.IndexHTMLHandler(assetFS, log, options.RoutePrefix)
+	assetHandler := server.AssetHandler(assertFSHandler, redirectHandler)
+	mux.Handle("/", gziphandler.GzipHandler(assetHandler))
 
 	if options.RoutePrefix != "" {
 		mux = server.WithRoutePrefix(mux, options.RoutePrefix)
@@ -405,51 +395,3 @@ func getAssets() fs.FS {
 
 	return f
 }
-
-// A redirector ensures that index.html always gets served.
-// The JS router will take care of actual navigation once the index.html page lands.
-func createRedirector(fsys fs.FS, log logr.Logger, routePrefix string) http.HandlerFunc {
-	baseHref := server.GetBaseHref(routePrefix)
-	log.Info("Creating redirector", "routePrefix", routePrefix, "baseHref", baseHref)
-
-	return func(w http.ResponseWriter, r *http.Request) {
-		indexPage, err := fsys.Open("index.html")
-
-		if err != nil {
-			log.Error(err, "could not open index.html page")
-			w.WriteHeader(http.StatusInternalServerError)
-
-			return
-		}
-
-		stat, err := indexPage.Stat()
-		if err != nil {
-			log.Error(err, "could not get index.html stat")
-			w.WriteHeader(http.StatusInternalServerError)
-
-			return
-		}
-
-		bt := make([]byte, stat.Size())
-		_, err = indexPage.Read(bt)
-
-		if err != nil {
-			log.Error(err, "could not read index.html")
-			w.WriteHeader(http.StatusInternalServerError)
-
-			return
-		}
-
-		// inject base tag into index.html
-		bt = server.InjectHTMLBaseTag(bt, baseHref)
-
-		_, err = w.Write(bt)
-
-		if err != nil {
-			log.Error(err, "error writing index.html")
-			w.WriteHeader(http.StatusInternalServerError)
-
-			return
-		}
-	}
-}
diff --git a/pkg/server/static_assets.go b/pkg/server/static_assets.go
new file mode 100644
index 0000000000..887c1b5a2f
--- /dev/null
+++ b/pkg/server/static_assets.go
@@ -0,0 +1,61 @@
+package server
+
+import (
+	"io"
+	"io/fs"
+	"net/http"
+	"path/filepath"
+
+	"github.com/go-logr/logr"
+)
+
+// AssetHandler returns a http.Handler that serves static assets from the provided fs.FS.
+// It also redirects all non-file requests to index.html.
+func AssetHandler(assetHandler http.Handler, redirectHandler http.Handler) http.HandlerFunc {
+	return func(w http.ResponseWriter, req *http.Request) {
+		// Assume anything with a file extension in the name is a static asset.
+		extension := filepath.Ext(req.URL.Path)
+		// We use the golang http.FileServer for static file requests.
+		// This will return a 404 on normal page requests, ie /some-page.
+		// Redirect all non-file requests to index.html, where the JS routing will take over.
+		if extension == "" {
+			redirectHandler.ServeHTTP(w, req)
+			return
+		}
+		assetHandler.ServeHTTP(w, req)
+	}
+}
+
+// IndexHTMLHandler ensures that index.html always gets served.
+// The JS router will take care of actual navigation once the index.html page lands.
+func IndexHTMLHandler(fsys fs.FS, log logr.Logger, routePrefix string) http.HandlerFunc {
+	baseHref := GetBaseHref(routePrefix)
+	log.Info("Creating redirector", "routePrefix", routePrefix, "baseHref", baseHref)
+
+	return func(w http.ResponseWriter, r *http.Request) {
+		indexPage, err := fsys.Open("index.html")
+		if err != nil {
+			log.Error(err, "could not open index.html page")
+			http.Error(w, "could not open index.html page", http.StatusInternalServerError)
+			return
+		}
+		defer indexPage.Close()
+
+		bt, err := io.ReadAll(indexPage)
+		if err != nil {
+			log.Error(err, "could not read index.html")
+			http.Error(w, "could not read index.html", http.StatusInternalServerError)
+			return
+		}
+
+		// inject base tag into index.html
+		bt = InjectHTMLBaseTag(bt, baseHref)
+
+		_, err = w.Write(bt)
+		if err != nil {
+			log.Error(err, "error writing index.html")
+			http.Error(w, "error writing index.html", http.StatusInternalServerError)
+			return
+		}
+	}
+}
diff --git a/pkg/server/static_assets_test.go b/pkg/server/static_assets_test.go
new file mode 100644
index 0000000000..91362cfa4a
--- /dev/null
+++ b/pkg/server/static_assets_test.go
@@ -0,0 +1,119 @@
+package server
+
+import (
+	"io"
+	"io/fs"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/go-logr/logr"
+)
+
+func TestCreateRedirector(t *testing.T) {
+	log := logr.Discard()
+	// Easiest way to create a filesystem..
+	fsys, err := fs.Sub(os.DirFS("testdata"), "public")
+	if err != nil {
+		t.Fatalf("failed to create fs: %v", err)
+	}
+
+	t.Run("We read the index.html and inject base", func(t *testing.T) {
+		req := httptest.NewRequest("GET", "http://example.com/foo", nil)
+		w := httptest.NewRecorder()
+		handler := IndexHTMLHandler(fsys, log, "/prefix")
+
+		handler.ServeHTTP(w, req)
+
+		resp := w.Result()
+		body, _ := io.ReadAll(resp.Body)
+
+		// Check the status code
+		if resp.StatusCode != http.StatusOK {
+			t.Errorf("expected status OK; got %v", resp.StatusCode)
+		}
+
+		// Check that the base tag was injected
+		if !strings.Contains(string(body), `<base href="/prefix/">`) {
+			t.Errorf("base tag not injected correctly: %v", string(body))
+		}
+	})
+
+	t.Run("file not found", func(t *testing.T) {
+		brokenFS, err := fs.Sub(os.DirFS("testdata"), "nonexistent")
+		if err != nil {
+			t.Fatalf("failed to create fs: %v", err)
+		}
+		req := httptest.NewRequest("GET", "http://example.com/foo", nil)
+		w := httptest.NewRecorder()
+		handler := IndexHTMLHandler(brokenFS, log, "/prefix")
+
+		handler.ServeHTTP(w, req)
+
+		resp := w.Result()
+
+		// Check the status code
+		if resp.StatusCode != http.StatusInternalServerError {
+			t.Errorf("expected status InternalServerError; got %v", resp.StatusCode)
+		}
+	})
+
+}
+
+func TestAssetHandlerFunc(t *testing.T) {
+	// Mock assetHandler to just record that it was called and with what request
+	assetHandler := http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		w.Write([]byte("assetHandler called"))
+	})
+
+	// Mock redirector to just record that it was called and with what request
+	redirector := http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		w.Write([]byte("redirector called"))
+	})
+
+	handler := AssetHandler(assetHandler, redirector)
+
+	tests := []struct {
+		name       string
+		requestURI string
+		wantStatus int
+		wantBody   string
+	}{
+		{
+			name:       "Asset request with extension",
+			requestURI: "/static/somefile.js",
+			wantStatus: http.StatusOK,
+			wantBody:   "assetHandler called",
+		},
+		{
+			name:       "Non-asset request",
+			requestURI: "/some-page",
+			wantStatus: http.StatusOK,
+			wantBody:   "redirector called",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req, err := http.NewRequest("GET", tt.requestURI, nil)
+			if err != nil {
+				t.Fatalf("could not create request: %v", err)
+			}
+
+			rr := httptest.NewRecorder()
+			handler(rr, req)
+
+			if rr.Code != tt.wantStatus {
+				t.Errorf("handler returned wrong status code: got %v want %v",
+					rr.Code, tt.wantStatus)
+			}
+
+			if rr.Body.String() != tt.wantBody {
+				t.Errorf("handler returned unexpected body: got %v want %v",
+					rr.Body.String(), tt.wantBody)
+			}
+		})
+	}
+}
diff --git a/pkg/server/testdata/public/index.html b/pkg/server/testdata/public/index.html
new file mode 100644
index 0000000000..227213fc58
--- /dev/null
+++ b/pkg/server/testdata/public/index.html
@@ -0,0 +1,6 @@
+<html>
+  <head></head>
+  <body>
+    <h1>Index</h1>
+  </body>
+</html>

From 39d22c55715fc203b3aa43400fd882d2903dcbc9 Mon Sep 17 00:00:00 2001
From: AlinaGoaga <goaga.alina@gmail.com>
Date: Thu, 23 Nov 2023 15:55:18 +0100
Subject: [PATCH 2/2] Implement CI suggestions

---
 pkg/server/static_assets.go      | 2 +-
 pkg/server/static_assets_test.go | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/pkg/server/static_assets.go b/pkg/server/static_assets.go
index 887c1b5a2f..8652e8fd22 100644
--- a/pkg/server/static_assets.go
+++ b/pkg/server/static_assets.go
@@ -11,7 +11,7 @@ import (
 
 // AssetHandler returns a http.Handler that serves static assets from the provided fs.FS.
 // It also redirects all non-file requests to index.html.
-func AssetHandler(assetHandler http.Handler, redirectHandler http.Handler) http.HandlerFunc {
+func AssetHandler(assetHandler, redirectHandler http.Handler) http.HandlerFunc {
 	return func(w http.ResponseWriter, req *http.Request) {
 		// Assume anything with a file extension in the name is a static asset.
 		extension := filepath.Ext(req.URL.Path)
diff --git a/pkg/server/static_assets_test.go b/pkg/server/static_assets_test.go
index 91362cfa4a..84d691c075 100644
--- a/pkg/server/static_assets_test.go
+++ b/pkg/server/static_assets_test.go
@@ -59,7 +59,6 @@ func TestCreateRedirector(t *testing.T) {
 			t.Errorf("expected status InternalServerError; got %v", resp.StatusCode)
 		}
 	})
-
 }
 
 func TestAssetHandlerFunc(t *testing.T) {