From 966bcd35459348279442118aeb8a0c7a9bd0ac90 Mon Sep 17 00:00:00 2001 From: Giuseppe Guerra Date: Fri, 24 Feb 2023 14:28:13 +0100 Subject: [PATCH] Plugins: Fix plugins CDN not working when plugins are not in domain's root path (#63202) * Plugins CDN: Add support for different CDN root path * Plugins CDN: Make frontendsettings return the correct CDN base path * Update comments * Fix version detection * Undo frontend changes * Fix system.js asset path construction * fix(plugins): translate all plugin css asset paths loaded via cdn * refactor(plugins): rename extractPluginNameVersionFromUrl and add comments * Fix typo in comment Co-authored-by: Will Browne * Hardcode CDN URL structure /{id}/{version}/public/plugins/{id}/{assetPath} is not required anymore in the cdn url template config --------- Co-authored-by: Jack Westbrook Co-authored-by: Will Browne --- pkg/api/frontendsettings_test.go | 2 +- pkg/api/plugins_test.go | 2 +- .../loader/assetpath/assetpath_test.go | 161 ++++++++++-------- pkg/plugins/manager/loader/loader_test.go | 2 +- pkg/plugins/pluginscdn/pluginscdn.go | 30 ++-- pkg/plugins/pluginscdn/pluginscdn_test.go | 6 +- .../plugins/systemjsPlugins/pluginCDN.test.ts | 17 +- .../plugins/systemjsPlugins/pluginCDN.ts | 32 +++- 8 files changed, 153 insertions(+), 99 deletions(-) diff --git a/pkg/api/frontendsettings_test.go b/pkg/api/frontendsettings_test.go index f1aca2ae700..42fe5fa83e6 100644 --- a/pkg/api/frontendsettings_test.go +++ b/pkg/api/frontendsettings_test.go @@ -166,7 +166,7 @@ func TestHTTPServer_GetFrontendSettings_pluginsCDNBaseURL(t *testing.T) { { desc: "With CDN", mutateCfg: func(cfg *setting.Cfg) { - cfg.PluginsCDNURLTemplate = "https://cdn.example.com/{id}/{version}/public/plugins/{id}/{assetPath}" + cfg.PluginsCDNURLTemplate = "https://cdn.example.com" }, expected: settings{PluginsCDNBaseURL: "https://cdn.example.com"}, }, diff --git a/pkg/api/plugins_test.go b/pkg/api/plugins_test.go index f14e44a79d4..db1d1075ac9 100644 --- a/pkg/api/plugins_test.go +++ b/pkg/api/plugins_test.go @@ -174,7 +174,7 @@ func Test_GetPluginAssetCDNRedirect(t *testing.T) { }, } cfg := setting.NewCfg() - cfg.PluginsCDNURLTemplate = "https://cdn.example.com/{id}/{version}/public/plugins/{id}/{assetPath}" + cfg.PluginsCDNURLTemplate = "https://cdn.example.com" cfg.PluginSettings = map[string]map[string]string{ cdnPluginID: {"cdn": "true"}, } diff --git a/pkg/plugins/manager/loader/assetpath/assetpath_test.go b/pkg/plugins/manager/loader/assetpath/assetpath_test.go index 1551aa57146..91fd0b58443 100644 --- a/pkg/plugins/manager/loader/assetpath/assetpath_test.go +++ b/pkg/plugins/manager/loader/assetpath/assetpath_test.go @@ -1,6 +1,7 @@ package assetpath import ( + "strings" "testing" "github.com/grafana/grafana/pkg/plugins" @@ -14,75 +15,101 @@ func extPath(pluginID string) string { } func TestService(t *testing.T) { - svc := ProvideService(pluginscdn.ProvideService(&config.Cfg{ - PluginsCDNURLTemplate: "https://cdn.example.com/{id}/{version}/public/plugins/{id}/{assetPath}", - PluginSettings: map[string]map[string]string{ - "one": {"cdn": "true"}, - "two": {}, + for _, tc := range []struct { + name string + cdnBaseURL string + }{ + { + name: "Simple", + cdnBaseURL: "https://cdn.example.com", }, - })) + { + name: "Not root", + cdnBaseURL: "https://cdn.example.com/plugins", + }, + { + name: "End slashes", + cdnBaseURL: "https://cdn.example.com/plugins////", + }, + } { + t.Run(tc.name, func(t *testing.T) { + svc := ProvideService(pluginscdn.ProvideService(&config.Cfg{ + PluginsCDNURLTemplate: tc.cdnBaseURL, + PluginSettings: map[string]map[string]string{ + "one": {"cdn": "true"}, + "two": {}, + }, + })) - const tableOldPath = "/grafana/public/app/plugins/panel/table-old" - jsonData := map[string]plugins.JSONData{ - "table-old": {ID: "table-old", Info: plugins.Info{Version: "1.0.0"}}, + const tableOldPath = "/grafana/public/app/plugins/panel/table-old" + jsonData := map[string]plugins.JSONData{ + "table-old": {ID: "table-old", Info: plugins.Info{Version: "1.0.0"}}, - "one": {ID: "one", Info: plugins.Info{Version: "1.0.0"}}, - "two": {ID: "two", Info: plugins.Info{Version: "2.0.0"}}, + "one": {ID: "one", Info: plugins.Info{Version: "1.0.0"}}, + "two": {ID: "two", Info: plugins.Info{Version: "2.0.0"}}, + } + + t.Run("CDN Base URL", func(t *testing.T) { + base, err := svc.cdn.BaseURL() + require.NoError(t, err) + require.Equal(t, tc.cdnBaseURL, base) + }) + + t.Run("Base", func(t *testing.T) { + base, err := svc.Base(jsonData["one"], plugins.External, extPath("one")) + require.NoError(t, err) + require.Equal(t, "plugin-cdn/one/1.0.0/public/plugins/one", base) + + base, err = svc.Base(jsonData["two"], plugins.External, extPath("two")) + require.NoError(t, err) + require.Equal(t, "public/plugins/two", base) + + base, err = svc.Base(jsonData["table-old"], plugins.Core, tableOldPath) + require.NoError(t, err) + require.Equal(t, "public/app/plugins/table-old", base) + }) + + t.Run("Module", func(t *testing.T) { + module, err := svc.Module(jsonData["one"], plugins.External, extPath("one")) + require.NoError(t, err) + require.Equal(t, "plugin-cdn/one/1.0.0/public/plugins/one/module", module) + + module, err = svc.Module(jsonData["two"], plugins.External, extPath("two")) + require.NoError(t, err) + require.Equal(t, "plugins/two/module", module) + + module, err = svc.Module(jsonData["table-old"], plugins.Core, tableOldPath) + require.NoError(t, err) + require.Equal(t, "app/plugins/table-old/module", module) + }) + + t.Run("RelativeURL", func(t *testing.T) { + pluginsMap := map[string]*plugins.Plugin{ + "one": { + JSONData: plugins.JSONData{ID: "one", Info: plugins.Info{Version: "1.0.0"}}, + BaseURL: "plugin-cdn/one/1.0.0/public/pluginsMap/one", + }, + "two": { + JSONData: plugins.JSONData{ID: "two", Info: plugins.Info{Version: "2.0.0"}}, + BaseURL: "public/pluginsMap/two", + }, + } + u, err := svc.RelativeURL(pluginsMap["one"], "", "default") + require.NoError(t, err) + require.Equal(t, "default", u) + + u, err = svc.RelativeURL(pluginsMap["one"], "path/to/file.txt", "default") + require.NoError(t, err) + require.Equal(t, strings.TrimRight(tc.cdnBaseURL, "/")+"/one/1.0.0/public/plugins/one/path/to/file.txt", u) + + u, err = svc.RelativeURL(pluginsMap["two"], "path/to/file.txt", "default") + require.NoError(t, err) + require.Equal(t, "public/pluginsMap/two/path/to/file.txt", u) + + u, err = svc.RelativeURL(pluginsMap["two"], "default", "default") + require.NoError(t, err) + require.Equal(t, "default", u) + }) + }) } - - t.Run("Base", func(t *testing.T) { - base, err := svc.Base(jsonData["one"], plugins.External, extPath("one")) - require.NoError(t, err) - require.Equal(t, "plugin-cdn/one/1.0.0/public/plugins/one", base) - - base, err = svc.Base(jsonData["two"], plugins.External, extPath("two")) - require.NoError(t, err) - require.Equal(t, "public/plugins/two", base) - - base, err = svc.Base(jsonData["table-old"], plugins.Core, tableOldPath) - require.NoError(t, err) - require.Equal(t, "public/app/plugins/table-old", base) - }) - - t.Run("Module", func(t *testing.T) { - module, err := svc.Module(jsonData["one"], plugins.External, extPath("one")) - require.NoError(t, err) - require.Equal(t, "plugin-cdn/one/1.0.0/public/plugins/one/module", module) - - module, err = svc.Module(jsonData["two"], plugins.External, extPath("two")) - require.NoError(t, err) - require.Equal(t, "plugins/two/module", module) - - module, err = svc.Module(jsonData["table-old"], plugins.Core, tableOldPath) - require.NoError(t, err) - require.Equal(t, "app/plugins/table-old/module", module) - }) - - t.Run("RelativeURL", func(t *testing.T) { - pluginsMap := map[string]*plugins.Plugin{ - "one": { - JSONData: plugins.JSONData{ID: "one", Info: plugins.Info{Version: "1.0.0"}}, - BaseURL: "plugin-cdn/one/1.0.0/public/pluginsMap/one", - }, - "two": { - JSONData: plugins.JSONData{ID: "two", Info: plugins.Info{Version: "2.0.0"}}, - BaseURL: "public/pluginsMap/two", - }, - } - u, err := svc.RelativeURL(pluginsMap["one"], "", "default") - require.NoError(t, err) - require.Equal(t, "default", u) - - u, err = svc.RelativeURL(pluginsMap["one"], "path/to/file.txt", "default") - require.NoError(t, err) - require.Equal(t, "https://cdn.example.com/one/1.0.0/public/plugins/one/path/to/file.txt", u) - - u, err = svc.RelativeURL(pluginsMap["two"], "path/to/file.txt", "default") - require.NoError(t, err) - require.Equal(t, "public/pluginsMap/two/path/to/file.txt", u) - - u, err = svc.RelativeURL(pluginsMap["two"], "default", "default") - require.NoError(t, err) - require.Equal(t, "default", u) - }) } diff --git a/pkg/plugins/manager/loader/loader_test.go b/pkg/plugins/manager/loader/loader_test.go index b48df204b70..d30b5b7b76f 100644 --- a/pkg/plugins/manager/loader/loader_test.go +++ b/pkg/plugins/manager/loader/loader_test.go @@ -412,7 +412,7 @@ func TestLoader_Load(t *testing.T) { name: "Load CDN plugin", class: plugins.External, cfg: &config.Cfg{ - PluginsCDNURLTemplate: "https://cdn.example.com/{id}/{version}/public/plugins/{id}/{assetPath}", + PluginsCDNURLTemplate: "https://cdn.example.com", PluginSettings: setting.PluginSettings{ "grafana-worldmap-panel": {"cdn": "true"}, }, diff --git a/pkg/plugins/pluginscdn/pluginscdn.go b/pkg/plugins/pluginscdn/pluginscdn.go index 3539d9ac765..da21336523b 100644 --- a/pkg/plugins/pluginscdn/pluginscdn.go +++ b/pkg/plugins/pluginscdn/pluginscdn.go @@ -2,16 +2,17 @@ package pluginscdn import ( "errors" - "fmt" - "net/url" - "path" + "strings" "github.com/grafana/grafana/pkg/plugins/config" ) const ( - // systemJSCDNKeyword is the path prefix used by system.js to identify the plugins CDN. - systemJSCDNKeyword = "plugin-cdn" + // cdnAssetPathTemplate is the relative path template used to locate plugin CDN assets + cdnAssetPathTemplate = "{id}/{version}/public/plugins/{id}/{assetPath}" + + // systemJSCDNURLTemplate is a special path template used by system.js to identify plugin CDN assets + systemJSCDNURLTemplate = "plugin-cdn/" + cdnAssetPathTemplate ) var ErrPluginNotCDN = errors.New("plugin is not a cdn plugin") @@ -30,7 +31,7 @@ func ProvideService(cfg *config.Cfg) *Service { // and invalid base url. func (s *Service) NewCDNURLConstructor(pluginID, pluginVersion string) URLConstructor { return URLConstructor{ - cdnURLTemplate: s.cfg.PluginsCDNURLTemplate, + cdnURLTemplate: strings.TrimRight(s.cfg.PluginsCDNURLTemplate, "/") + "/" + cdnAssetPathTemplate, pluginID: pluginID, pluginVersion: pluginVersion, } @@ -47,27 +48,28 @@ func (s *Service) PluginSupported(pluginID string) bool { } // BaseURL returns the absolute base URL of the plugins CDN. +// This is the "fixed" part of the URL (protocol + host + root url). // If the plugins CDN is disabled, it returns an empty string. func (s *Service) BaseURL() (string, error) { if !s.IsEnabled() { return "", nil } - u, err := url.Parse(s.cfg.PluginsCDNURLTemplate) - if err != nil { - return "", fmt.Errorf("url parse: %w", err) - } - return u.Scheme + "://" + u.Host, nil + return s.cfg.PluginsCDNURLTemplate, nil } // SystemJSAssetPath returns a system-js path for the specified asset on the plugins CDN. -// It replaces the base path of the CDN with systemJSCDNKeyword. +// The returned path will follow the template specified in systemJSCDNURLTemplate. // If assetPath is an empty string, the base path for the plugin is returned. func (s *Service) SystemJSAssetPath(pluginID, pluginVersion, assetPath string) (string, error) { - u, err := s.NewCDNURLConstructor(pluginID, pluginVersion).Path(assetPath) + u, err := URLConstructor{ + cdnURLTemplate: systemJSCDNURLTemplate, + pluginID: pluginID, + pluginVersion: pluginVersion, + }.Path(assetPath) if err != nil { return "", err } - return path.Join(systemJSCDNKeyword, u.Path), nil + return u.String(), nil } // AssetURL returns the URL of a CDN asset for a CDN plugin. If the specified plugin is not a CDN plugin, diff --git a/pkg/plugins/pluginscdn/pluginscdn_test.go b/pkg/plugins/pluginscdn/pluginscdn_test.go index b473aeb4b54..08cae194aae 100644 --- a/pkg/plugins/pluginscdn/pluginscdn_test.go +++ b/pkg/plugins/pluginscdn/pluginscdn_test.go @@ -9,7 +9,7 @@ import ( func TestService(t *testing.T) { svc := ProvideService(&config.Cfg{ - PluginsCDNURLTemplate: "https://cdn.example.com/{id}/{version}/public/plugins/{id}/{assetPath}", + PluginsCDNURLTemplate: "https://cdn.example.com", PluginSettings: map[string]map[string]string{ "one": {"cdn": "true"}, "two": {}, @@ -30,8 +30,8 @@ func TestService(t *testing.T) { }{ { name: "valid", - cfgURL: "https://grafana-assets.grafana.net/plugin-cdn-test/plugin-cdn/{id}/{version}/public/plugins/{id}/{assetPath}", - expBaseURL: "https://grafana-assets.grafana.net", + cfgURL: "https://grafana-assets.grafana.net/plugin-cdn-test/plugin-cdn", + expBaseURL: "https://grafana-assets.grafana.net/plugin-cdn-test/plugin-cdn", }, { name: "empty", diff --git a/public/app/features/plugins/systemjsPlugins/pluginCDN.test.ts b/public/app/features/plugins/systemjsPlugins/pluginCDN.test.ts index 6abb0699e7d..d2c2bcfa8d3 100644 --- a/public/app/features/plugins/systemjsPlugins/pluginCDN.test.ts +++ b/public/app/features/plugins/systemjsPlugins/pluginCDN.test.ts @@ -1,6 +1,6 @@ import { config } from '@grafana/runtime'; -import { translateForCDN, extractPluginNameVersionFromUrl } from './pluginCDN'; +import { translateForCDN, extractPluginIdVersionFromUrl } from './pluginCDN'; describe('Plugin CDN', () => { describe('translateForCDN', () => { const load = { @@ -88,17 +88,24 @@ describe('Plugin CDN', () => { const translatedLoad = translateForCDN({ ...load, source }); expect(translatedLoad).toBe(expectedSource); }); + + it('should replace css paths', () => { + const source = `(0,o.loadPluginCss)({dark:"plugins/grafana-worldmap-panel/css/worldmap.dark.css",light:"plugins/grafana-worldmap-panel/css/worldmap.light.css"}),`; + const expectedSource = `(0,o.loadPluginCss)({dark:"http://my-host.com/grafana-worldmap-panel/0.3.3/public/plugins/grafana-worldmap-panel/css/worldmap.dark.css",light:"http://my-host.com/grafana-worldmap-panel/0.3.3/public/plugins/grafana-worldmap-panel/css/worldmap.light.css"}),`; + const translatedLoad = translateForCDN({ ...load, source }); + expect(translatedLoad).toBe(expectedSource); + }); }); - describe('extractPluginNameVersionFromUrl', () => { - it('should extract the plugin name and version from a path', () => { + describe('extractPluginIdVersionFromUrl', () => { + it('should extract the plugin id and version from a path', () => { const source = 'http://localhost:3000/public/plugin-cdn/grafana-worldmap-panel/0.3.3/public/plugins/grafana-worldmap-panel/module.js'; const expected = { - name: 'grafana-worldmap-panel', + id: 'grafana-worldmap-panel', version: '0.3.3', }; - const expectedExtractedPluginDeets = extractPluginNameVersionFromUrl(source); + const expectedExtractedPluginDeets = extractPluginIdVersionFromUrl(source); expect(expectedExtractedPluginDeets).toEqual(expected); }); }); diff --git a/public/app/features/plugins/systemjsPlugins/pluginCDN.ts b/public/app/features/plugins/systemjsPlugins/pluginCDN.ts index 22e33f0e1ef..27d2b5382f4 100644 --- a/public/app/features/plugins/systemjsPlugins/pluginCDN.ts +++ b/public/app/features/plugins/systemjsPlugins/pluginCDN.ts @@ -2,27 +2,45 @@ import { config } from '@grafana/runtime'; import type { SystemJSLoad } from './types'; -export function extractPluginNameVersionFromUrl(address: string) { +/* + Given an "expected" address of `http://localhost/public/plugin-cdn/{pluginId}/{version}/public/plugins/{pluginId}` + this function will return the plugin id and version. + */ +export function extractPluginIdVersionFromUrl(address: string) { const path = new URL(address).pathname; const match = path.split('/'); - return { name: match[3], version: match[4] }; + return { id: match[3], version: match[4] }; } +/* + Locate: Overrides the location of the plugin resource + Plugins loaded via CDN fall into this plugin via the `plugin-cdn` keyword. + Systemjs first resolves to an origin on the local filesystem + (e.g. http://localhost/public/plugin-cdn/{pluginId}/{version}/public/plugins/{pluginId}) + we then split this url and prefix with the CDN base url giving us the correct asset location. + */ export function locateFromCDN(load: SystemJSLoad) { const { address } = load; const pluginPath = address.split('/public/plugin-cdn/'); return `${config.pluginsCDNBaseURL}/${pluginPath[1]}`; } +/* + Translate: Returns the translated source from load.source, can also set load.metadata.sourceMap for full source maps support. + Plugins that require loading via a CDN need to have their asset paths translated to point to the configured CDN. + e.g. public/plugins/my-plugin/data/ -> http://my-host.com/my-plugin/0.3.3/public/plugins/my-plugin/data/ + */ export function translateForCDN(load: SystemJSLoad) { - const { name, version } = extractPluginNameVersionFromUrl(load.name); - const baseAddress = `${config.pluginsCDNBaseURL}/${name}/${version}`; - + const { id, version } = extractPluginIdVersionFromUrl(load.name); + const baseAddress = `${config.pluginsCDNBaseURL}/${id}/${version}`; + // handle basic asset paths that include public/plugins load.source = load.source.replace(/(\/?)(public\/plugins)/g, `${baseAddress}/$2`); - load.source = load.source.replace(/(["|'])(plugins\/.+.css)(["|'])/g, `$1${baseAddress}/public/$2$3`); + // handle custom plugin css (light and dark themes) + load.source = load.source.replace(/(["|'])(plugins\/.+?.css)(["|'])/g, `$1${baseAddress}/public/$2$3`); + // handle external sourcemap links load.source = load.source.replace( /(\/\/#\ssourceMappingURL=)(.+)\.map/g, - `$1${baseAddress}/public/plugins/${name}/$2.map` + `$1${baseAddress}/public/plugins/${id}/$2.map` ); return load.source;