From 454380431d8ecc677f65714a87500d55639035f1 Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Mon, 8 Sep 2025 16:57:03 +0100 Subject: [PATCH] FS: Get CDN prefix from configuration (#110615) * FS: Get CDN prefix from configuration * undo logger change * fix tests * add unused property * tests * fix tests * remove dead comment --- pkg/api/api.go | 7 -- .../assets-manifest.json} | 0 pkg/api/webassets/webassets.go | 4 +- pkg/api/webassets/webassets_test.go | 2 +- pkg/services/frontend/frontend_service.go | 8 +- .../frontend/frontend_service_test.go | 4 + pkg/services/frontend/index.go | 24 ++---- pkg/services/frontend/index_test.go | 22 ------ pkg/services/frontend/webassets/webassets.go | 49 ++++++++++++ .../frontend/webassets/webassets_test.go | 75 +++++++++++++++++++ 10 files changed, 143 insertions(+), 52 deletions(-) rename pkg/api/webassets/testdata/{sample-assets-manifest.json => build/assets-manifest.json} (100%) create mode 100644 pkg/services/frontend/webassets/webassets.go create mode 100644 pkg/services/frontend/webassets/webassets_test.go diff --git a/pkg/api/api.go b/pkg/api/api.go index 5248740b5ae..a1ba8591044 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -47,7 +47,6 @@ import ( "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/featuremgmt" - "github.com/grafana/grafana/pkg/services/frontend" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginaccesscontrol" publicdashboardsapi "github.com/grafana/grafana/pkg/services/publicdashboards/api" @@ -86,12 +85,6 @@ func (hs *HTTPServer) registerRoutes() { r.Get("/invite/:code", hs.Index) if hs.Features.IsEnabledGlobally(featuremgmt.FlagMultiTenantFrontend) { - index, err := frontend.NewIndexProvider(hs.Cfg, hs.License) - if err != nil { - panic(err) // ??? - } - r.Get("/femt", index.HandleRequest) - // Temporarily expose the full bootdata via API r.Get("/bootdata", reqNoAuth, hs.GetBootdata) } diff --git a/pkg/api/webassets/testdata/sample-assets-manifest.json b/pkg/api/webassets/testdata/build/assets-manifest.json similarity index 100% rename from pkg/api/webassets/testdata/sample-assets-manifest.json rename to pkg/api/webassets/testdata/build/assets-manifest.json diff --git a/pkg/api/webassets/webassets.go b/pkg/api/webassets/webassets.go index 39b8f62ddba..3ca86af446e 100644 --- a/pkg/api/webassets/webassets.go +++ b/pkg/api/webassets/webassets.go @@ -60,7 +60,7 @@ func GetWebAssets(ctx context.Context, cfg *setting.Cfg, license licensing.Licen } if result == nil { - result, err = readWebAssetsFromFile(filepath.Join(cfg.StaticRootPath, "build", "assets-manifest.json")) + result, err = ReadWebAssetsFromFile(filepath.Join(cfg.StaticRootPath, "build", "assets-manifest.json")) if err == nil { cdn, _ = cfg.GetContentDeliveryURL(license.ContentDeliveryPrefix()) if cdn != "" { @@ -73,7 +73,7 @@ func GetWebAssets(ctx context.Context, cfg *setting.Cfg, license licensing.Licen return entryPointAssetsCache, err } -func readWebAssetsFromFile(manifestpath string) (*dtos.EntryPointAssets, error) { +func ReadWebAssetsFromFile(manifestpath string) (*dtos.EntryPointAssets, error) { //nolint:gosec f, err := os.Open(manifestpath) if err != nil { diff --git a/pkg/api/webassets/webassets_test.go b/pkg/api/webassets/webassets_test.go index b4bb5c25d70..dbf9dd91ab8 100644 --- a/pkg/api/webassets/webassets_test.go +++ b/pkg/api/webassets/webassets_test.go @@ -10,7 +10,7 @@ import ( ) func TestReadWebassets(t *testing.T) { - assets, err := readWebAssetsFromFile("testdata/sample-assets-manifest.json") + assets, err := ReadWebAssetsFromFile("testdata/build/assets-manifest.json") require.NoError(t, err) dto, err := json.MarshalIndent(assets, "", " ") diff --git a/pkg/services/frontend/frontend_service.go b/pkg/services/frontend/frontend_service.go index 0852a7a2147..3a7230f9d1b 100644 --- a/pkg/services/frontend/frontend_service.go +++ b/pkg/services/frontend/frontend_service.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/middleware/loggermw" "github.com/grafana/grafana/pkg/middleware/requestmeta" "github.com/grafana/grafana/pkg/services/featuremgmt" + fswebassets "github.com/grafana/grafana/pkg/services/frontend/webassets" "github.com/grafana/grafana/pkg/services/licensing" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web" @@ -40,7 +41,12 @@ type frontendService struct { } func ProvideFrontendService(cfg *setting.Cfg, features featuremgmt.FeatureToggles, promGatherer prometheus.Gatherer, promRegister prometheus.Registerer, license licensing.Licensing) (*frontendService, error) { - index, err := NewIndexProvider(cfg, license) + assetsManifest, err := fswebassets.GetWebAssets(cfg, license) + if err != nil { + return nil, err + } + + index, err := NewIndexProvider(cfg, assetsManifest) if err != nil { return nil, err } diff --git a/pkg/services/frontend/frontend_service_test.go b/pkg/services/frontend/frontend_service_test.go index 348531f4b2c..05490ae63ab 100644 --- a/pkg/services/frontend/frontend_service_test.go +++ b/pkg/services/frontend/frontend_service_test.go @@ -28,6 +28,10 @@ func createTestService(t *testing.T, cfg *setting.Cfg) *frontendService { var promRegister prometheus.Registerer = prometheus.NewRegistry() promGatherer := promRegister.(*prometheus.Registry) + if cfg.BuildVersion == "" { + cfg.BuildVersion = "10.3.0" + } + service, err := ProvideFrontendService(cfg, features, promGatherer, promRegister, license) require.NoError(t, err) diff --git a/pkg/services/frontend/index.go b/pkg/services/frontend/index.go index ff18e6e5b7d..18b422c987b 100644 --- a/pkg/services/frontend/index.go +++ b/pkg/services/frontend/index.go @@ -1,7 +1,6 @@ package frontend import ( - "context" "embed" "errors" "fmt" @@ -11,9 +10,7 @@ import ( "github.com/grafana/grafana-app-sdk/logging" "github.com/grafana/grafana/pkg/api/dtos" - "github.com/grafana/grafana/pkg/api/webassets" "github.com/grafana/grafana/pkg/middleware" - "github.com/grafana/grafana/pkg/services/licensing" "github.com/grafana/grafana/pkg/setting" ) @@ -29,15 +26,14 @@ type IndexViewData struct { CSPEnabled bool IsDevelopmentEnv bool - Config *setting.Cfg - License licensing.Licensing + Config *setting.Cfg AppSubUrl string BuildVersion string BuildCommit string AppTitle string - Assets *dtos.EntryPointAssets // Includes CDN info + Assets dtos.EntryPointAssets // Includes CDN info // Nonce is a cryptographic identifier for use with Content Security Policy. Nonce string @@ -52,7 +48,7 @@ var ( htmlTemplates = template.Must(template.New("html").Delims("[[", "]]").ParseFS(templatesFS, `*.html`)) ) -func NewIndexProvider(cfg *setting.Cfg, license licensing.Licensing) (*IndexProvider, error) { +func NewIndexProvider(cfg *setting.Cfg, assetsManifest dtos.EntryPointAssets) (*IndexProvider, error) { t := htmlTemplates.Lookup("index.html") if t == nil { return nil, fmt.Errorf("missing index template") @@ -67,13 +63,14 @@ func NewIndexProvider(cfg *setting.Cfg, license licensing.Licensing) (*IndexProv BuildVersion: cfg.BuildVersion, BuildCommit: cfg.BuildCommit, Config: cfg, - License: license, CSPEnabled: cfg.CSPEnabled, CSPContent: cfg.CSPTemplate, CSPReportOnlyContent: cfg.CSPReportOnlyTemplate, IsDevelopmentEnv: cfg.Env == setting.Dev, + + Assets: assetsManifest, }, }, nil } @@ -106,17 +103,6 @@ func (p *IndexProvider) HandleRequest(writer http.ResponseWriter, request *http. writer.Header().Set("Content-Security-Policy-Report-Only", policy) } - // TODO: moved to request handler to prevent stale assets during dev, - // but should we do this differently? - assets, err := webassets.GetWebAssets(context.Background(), data.Config, data.License) - if err != nil { - p.log.Error("error getting assets", "err", err) - writer.WriteHeader(500) - return - } - - data.Assets = assets - writer.Header().Set("Content-Type", "text/html; charset=UTF-8") writer.WriteHeader(200) if err := p.index.Execute(writer, &data); err != nil { diff --git a/pkg/services/frontend/index_test.go b/pkg/services/frontend/index_test.go index eaee0e8ff50..3f0b76ac4f4 100644 --- a/pkg/services/frontend/index_test.go +++ b/pkg/services/frontend/index_test.go @@ -98,26 +98,4 @@ func TestFrontendService_WebAssets(t *testing.T) { assert.Contains(t, body, "src=\"public/build/runtime.js\" type=\"text/javascript\"") assert.Contains(t, body, "src=\"public/build/app.js\" type=\"text/javascript\"") }) - - t.Run("should handle missing assets manifest gracefully", func(t *testing.T) { - cfg := &setting.Cfg{ - HTTPPort: "3000", - StaticRootPath: "/dev/null", // No build directory or manifest - Env: setting.Dev, // needs to be dev to bypass the cache - } - service := createTestService(t, cfg) - - mux := web.New() - service.addMiddlewares(mux) - service.registerRoutes(mux) - - // Test index route which should fail to load web assets - req := httptest.NewRequest("GET", "/", nil) - recorder := httptest.NewRecorder() - - mux.ServeHTTP(recorder, req) - - // Should return 500 due to missing assets manifest - assert.Equal(t, 500, recorder.Code) - }) } diff --git a/pkg/services/frontend/webassets/webassets.go b/pkg/services/frontend/webassets/webassets.go new file mode 100644 index 00000000000..295406e84e4 --- /dev/null +++ b/pkg/services/frontend/webassets/webassets.go @@ -0,0 +1,49 @@ +package fswebassets + +import ( + "path/filepath" + + "github.com/grafana/grafana/pkg/api/dtos" + "github.com/grafana/grafana/pkg/api/webassets" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/licensing" + "github.com/grafana/grafana/pkg/setting" +) + +var logger = log.New("webassets") + +func getCDNRoot(cfg *setting.Cfg, license licensing.Licensing) string { + if cfg.CDNRootURL == nil { + return "" + } + + // We prefer to set the prefix from config, but make this backwards compatible + // taking it from the license instead + var prefix string + if cfg.CDNRootURL.Path == "" { + prefix = license.ContentDeliveryPrefix() + } + + cdnRoot, err := cfg.GetContentDeliveryURL(prefix) + if err != nil { + logger.Error("error getting cdn url from config", "error", err) + return "" + } + + return cdnRoot +} + +// New codepath for retrieving web assets URLs for the frontend-service +func GetWebAssets(cfg *setting.Cfg, license licensing.Licensing) (dtos.EntryPointAssets, error) { + assetsManifest, err := webassets.ReadWebAssetsFromFile(filepath.Join(cfg.StaticRootPath, "build", "assets-manifest.json")) + if err != nil { + return dtos.EntryPointAssets{}, err + } + + cdnRoot := getCDNRoot(cfg, license) + if cdnRoot != "" { + assetsManifest.SetContentDeliveryURL(cdnRoot) + } + + return *assetsManifest, nil +} diff --git a/pkg/services/frontend/webassets/webassets_test.go b/pkg/services/frontend/webassets/webassets_test.go new file mode 100644 index 00000000000..bd169835e70 --- /dev/null +++ b/pkg/services/frontend/webassets/webassets_test.go @@ -0,0 +1,75 @@ +package fswebassets_test + +import ( + "net/url" + "testing" + + fswebassets "github.com/grafana/grafana/pkg/services/frontend/webassets" + "github.com/grafana/grafana/pkg/services/licensing/licensingtest" + "github.com/grafana/grafana/pkg/setting" + "github.com/stretchr/testify/assert" +) + +func TestGetWebAssets_WithoutCDNConfigured(t *testing.T) { + cfg := &setting.Cfg{ + StaticRootPath: "../../../api/webassets/testdata", + } + license := licensingtest.NewFakeLicensing() + license.On("ContentDeliveryPrefix").Return("grafana") + + assets, err := fswebassets.GetWebAssets(cfg, license) + assert.NoError(t, err) + assert.NotNil(t, assets) + + assert.Equal(t, "public/build/runtime.js", assets.JSFiles[0].FilePath) +} + +func TestGetWebAssets_PrefixFromLicense(t *testing.T) { + cdnConfigUrl, _ := url.Parse("http://example.com") + cfg := &setting.Cfg{ + StaticRootPath: "../../../api/webassets/testdata", + CDNRootURL: cdnConfigUrl, + BuildVersion: "10.3.0", + } + license := licensingtest.NewFakeLicensing() + license.On("ContentDeliveryPrefix").Return("grafana-pro-max") + + assets, err := fswebassets.GetWebAssets(cfg, license) + assert.NoError(t, err) + assert.NotNil(t, assets) + + assert.Equal(t, "http://example.com/grafana-pro-max/10.3.0/public/build/runtime.js", assets.JSFiles[0].FilePath) +} +func TestGetWebAssets_PrefixFromConfig(t *testing.T) { + cdnConfigUrl, _ := url.Parse("http://example.com/grafana-super-plus") + cfg := &setting.Cfg{ + StaticRootPath: "../../../api/webassets/testdata", + CDNRootURL: cdnConfigUrl, + BuildVersion: "10.3.0", + } + license := licensingtest.NewFakeLicensing() + license.On("ContentDeliveryPrefix").Return("should-not-be-used") + + assets, err := fswebassets.GetWebAssets(cfg, license) + assert.NoError(t, err) + assert.NotNil(t, assets) + + assert.Equal(t, "http://example.com/grafana-super-plus/10.3.0/public/build/runtime.js", assets.JSFiles[0].FilePath) +} + +func TestGetWebAssets_PrefixFromConfigTrailingSlash(t *testing.T) { + cdnConfigUrl, _ := url.Parse("http://example.com/grafana-mega/") + cfg := &setting.Cfg{ + StaticRootPath: "../../../api/webassets/testdata", + CDNRootURL: cdnConfigUrl, + BuildVersion: "10.3.0", + } + license := licensingtest.NewFakeLicensing() + license.On("ContentDeliveryPrefix").Return("should-not-be-used") + + assets, err := fswebassets.GetWebAssets(cfg, license) + assert.NoError(t, err) + assert.NotNil(t, assets) + + assert.Equal(t, "http://example.com/grafana-mega/10.3.0/public/build/runtime.js", assets.JSFiles[0].FilePath) +}