[Cleanup] remove embedded assets (#1040)

Signed-off-by: Jeremy lewi <jeremy@lewi.us>

Author: jlewi

pkg/agent/server/assets.go

@@ -2,7 +2,6 @@ package server
 
 import (
 	"bytes"
-	"embed"
 	"io/fs"
 	"net/http"
 	"net/url"
@@ -20,14 +19,6 @@ import (
 	"google.golang.org/protobuf/encoding/protojson"
 )
 
-// TODO(jlewi): I think we should get rid of embedded assets. Now that we publish and download assets
-// via OCI not sure its worth it to have another code path. The problem with embedded assets is that its
-// difficult to layer on additional static assets like configs. If we pull them from the filesystem then
-// users can just add their own assets to subdirectories.
-
-//go:embed dist/*
-var embeddedAssets embed.FS
-
 // AssetsFileSystemProvider is an interface for providing asset filesystems.
 // This allows the server to be decoupled from how assets are constructed.
 type AssetsFileSystemProvider interface {
@@ -57,38 +48,6 @@ func (s *staticAssetsFileSystemProvider) GetAssetsFileSystem() (fs.FS, error) {
 	return os.DirFS(s.staticAssets), nil
 }
 
-// embeddedAssetsFileSystemProvider provides assets from embedded files.
-type embeddedAssetsFileSystemProvider struct {
-	embeddedFS embed.FS
-	subPath    string
-}
-
-// NewEmbeddedAssetsFileSystemProvider creates a provider that serves assets from embedded files.
-// The subPath parameter specifies the subdirectory within the embedded filesystem (e.g., "dist").
-func NewEmbeddedAssetsFileSystemProvider(embeddedFS embed.FS, subPath string) AssetsFileSystemProvider {
-	return &embeddedAssetsFileSystemProvider{
-		embeddedFS: embeddedFS,
-		subPath:    subPath,
-	}
-}
-
-// GetAssetsFileSystem implements AssetsFileSystemProvider by returning a filesystem
-// for the embedded assets.
-func (e *embeddedAssetsFileSystemProvider) GetAssetsFileSystem() (fs.FS, error) {
-	log := zapr.NewLogger(zap.L())
-	distFS, err := fs.Sub(e.embeddedFS, e.subPath)
-	if err != nil {
-		return nil, errors.Wrapf(err, "failed to create sub filesystem for embedded assets")
-	}
-	// Verify that index.html exists
-	_, err = distFS.Open("index.html")
-	if err != nil {
-		return nil, errors.Wrapf(err, "embedded assets not available: index.html not found")
-	}
-	log.Info("Serving embedded assets")
-	return distFS, nil
-}
-
 // fallbackAssetsFileSystemProvider tries multiple providers in order until one succeeds.
 type fallbackAssetsFileSystemProvider struct {
 	providers []AssetsFileSystemProvider
@@ -119,39 +78,25 @@ func (f *fallbackAssetsFileSystemProvider) GetAssetsFileSystem() (fs.FS, error)
 	return nil, errors.Wrapf(lastErr, "all asset providers failed")
 }
 
-// defaultAssetsFileSystemProvider is the default implementation that tries
-// static assets first, then falls back to embedded assets.
+// defaultAssetsFileSystemProvider is the default implementation that serves
+// assets from the static assets directory.
 type defaultAssetsFileSystemProvider struct {
 	staticAssets string
 }
 
 // NewDefaultAssetsFileSystemProvider creates a new default asset filesystem provider
-// that tries static assets first, then embedded assets as a fallback.
-// This preserves the original behavior of getAssetsFileSystem.
+// that serves assets from the static assets directory.
 func NewDefaultAssetsFileSystemProvider(staticAssets string) AssetsFileSystemProvider {
 	return &defaultAssetsFileSystemProvider{
 		staticAssets: staticAssets,
 	}
 }
 
-// GetAssetsFileSystem implements AssetsFileSystemProvider by trying static assets first,
-// then falling back to embedded assets. This preserves the original behavior.
+// GetAssetsFileSystem implements AssetsFileSystemProvider by serving static assets.
 func (d *defaultAssetsFileSystemProvider) GetAssetsFileSystem() (fs.FS, error) {
-	var providers []AssetsFileSystemProvider
-
-	// If static assets directory is specified, try it first
-	if d.staticAssets != "" {
-		providers = append(providers, NewStaticAssetsFileSystemProvider(d.staticAssets))
-	}
-
-	// Always try embedded assets as fallback
-	providers = append(providers, NewEmbeddedAssetsFileSystemProvider(embeddedAssets, "dist"))
-
-	// Use fallback provider to try them in order
-	fallback := NewFallbackAssetsFileSystemProvider(providers...)
-	fs, err := fallback.GetAssetsFileSystem()
+	fs, err := NewStaticAssetsFileSystemProvider(d.staticAssets).GetAssetsFileSystem()
 	if err != nil {
-		return nil, errors.New("no assets available: neither staticAssets directory is configured nor embedded assets could be found")
+		return nil, errors.New("no assets available: staticAssets directory is not configured")
 	}
 	return fs, nil
 }
@@ -200,7 +145,7 @@ func (s *Server) processIndexHTMLWithConfig(assetsFS fs.FS) ([]byte, error) {
 	return content, nil
 }
 
-// singlePageAppHandler serves a single-page app from static or embedded assets,
+// singlePageAppHandler serves a single-page app from static assets,
 // falling back to index for client-side routing when files don't exist.
 func (s *Server) singlePageAppHandler() (http.Handler, error) {
 	if s.assetsFS == nil {

pkg/agent/server/assets_test.go

@@ -1,7 +1,6 @@
 package server
 
 import (
-	"embed"
 	"io/fs"
 	"os"
 	"path/filepath"
@@ -11,9 +10,6 @@ import (
 	"github.com/pkg/errors"
 )
 
-//go:embed testdata/assets_test/*
-var testEmbeddedAssets embed.FS
-
 func TestStaticAssetsFileSystemProvider(t *testing.T) {
 	tests := []struct {
 		name         string
@@ -102,68 +98,6 @@ func TestStaticAssetsFileSystemProvider(t *testing.T) {
 	}
 }
 
-func TestEmbeddedAssetsFileSystemProvider(t *testing.T) {
-	tests := []struct {
-		name        string
-		embeddedFS  embed.FS
-		subPath     string
-		wantErr     bool
-		errContains string
-	}{
-		{
-			name:       "valid embedded FS with valid subpath",
-			embeddedFS: testEmbeddedAssets,
-			subPath:    "testdata/assets_test",
-			wantErr:    false,
-		},
-		{
-			name:        "invalid subpath",
-			embeddedFS:  testEmbeddedAssets,
-			subPath:     "nonexistent",
-			wantErr:     true,
-			errContains: "index.html not found",
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			provider := NewEmbeddedAssetsFileSystemProvider(tt.embeddedFS, tt.subPath)
-			fs, err := provider.GetAssetsFileSystem()
-
-			if tt.wantErr {
-				if err == nil {
-					t.Errorf("Expected error but got none")
-					return
-				}
-				if tt.errContains != "" && !strings.Contains(err.Error(), tt.errContains) {
-					t.Errorf("Expected error to contain %q, got %q", tt.errContains, err.Error())
-				}
-				return
-			}
-
-			if err != nil {
-				t.Errorf("Unexpected error: %v", err)
-				return
-			}
-
-			if fs == nil {
-				t.Errorf("Expected filesystem but got nil")
-				return
-			}
-
-			// Try to read index.html if it should exist
-			if tt.name == "valid embedded FS with valid subpath" {
-				file, err := fs.Open("index.html")
-				if err != nil {
-					t.Errorf("Failed to open index.html: %v", err)
-				} else {
-					file.Close()
-				}
-			}
-		})
-	}
-}
-
 func TestFallbackAssetsFileSystemProvider(t *testing.T) {
 	tests := []struct {
 		name        string
@@ -266,14 +200,15 @@ func TestDefaultAssetsFileSystemProvider(t *testing.T) {
 			},
 		},
 		{
-			name:         "without static assets, should fallback to embedded",
+			name:         "without static assets, should fail",
 			staticAssets: "",
-			wantErr:      false, // Should succeed if embedded assets exist
+			wantErr:      true,
+			errContains:  "no assets available: staticAssets directory is not configured",
 		},
 		{
-			name:         "with non-existent static assets, should fallback to embedded",
+			name:         "with non-existent static assets",
 			staticAssets: "",
-			wantErr:      false, // Should succeed if embedded assets exist
+			wantErr:      false, // os.DirFS doesn't validate path existence until read-time
 			setupDir: func(t *testing.T) string {
 				return filepath.Join(t.TempDir(), "nonexistent")
 			},
@@ -302,16 +237,8 @@ func TestDefaultAssetsFileSystemProvider(t *testing.T) {
 				return
 			}
 
-			// Note: This test may fail if embedded assets don't exist in the test environment
-			// That's okay - it means the test is working correctly
 			if err != nil {
-				// If we have a static assets dir, it should have been tried first
-				if dir != "" {
-					// The error might be from embedded assets fallback failing
-					if !strings.Contains(err.Error(), "no assets available") {
-						t.Logf("Got error (may be expected if embedded assets missing): %v", err)
-					}
-				}
+				t.Errorf("Unexpected error: %v", err)
 				return
 			}