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 <wbrowne@users.noreply.github.com>

* 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 <jack.westbrook@gmail.com>
Co-authored-by: Will Browne <wbrowne@users.noreply.github.com>

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"},
 		},

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"},
 		}

pkg/plugins/manager/loader/assetpath/assetpath_test.go

@@ -1,6 +1,7 @@
 package assetpath
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/grafana/grafana/pkg/plugins"
@@ -14,8 +15,26 @@ func extPath(pluginID string) string {
 }
 
 func TestService(t *testing.T) {
+	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: "https://cdn.example.com/{id}/{version}/public/plugins/{id}/{assetPath}",
+				PluginsCDNURLTemplate: tc.cdnBaseURL,
 				PluginSettings: map[string]map[string]string{
 					"one": {"cdn": "true"},
 					"two": {},
@@ -30,6 +49,12 @@ func TestService(t *testing.T) {
 				"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)
@@ -75,7 +100,7 @@ func TestService(t *testing.T) {
 
 				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)
+				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)
@@ -85,4 +110,6 @@ func TestService(t *testing.T) {
 				require.NoError(t, err)
 				require.Equal(t, "default", u)
 			})
+		})
+	}
 }

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"},
 				},

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,

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",

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);
     });
   });

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;