Skip to content

[Cleanup] remove embedded assets #1040

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sourishkrout merged 1 commit into from
Feb 12, 2026
Merged

[Cleanup] remove embedded assets #1040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 7 additions & 62 deletions pkg/agent/server/assets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package server

import (
"bytes"
"embed"
"io/fs"
"net/http"
"net/url"
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down
85 changes: 6 additions & 79 deletions pkg/agent/server/assets_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package server

import (
"embed"
"io/fs"
"os"
"path/filepath"
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
},
Expand Down Expand Up @@ -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
}

Expand Down
Loading