public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Renderer: Fix regression on callback URL in plugin mode (#103787)

Browse Source 
* Renderer: Fix regression on callback URL in plugin mode

* cleanup

* refactor to not mutate cfg object

* apply review feedback
This commit is contained in:
Agnès Toulet
2025-04-11 15:34:10 +02:00
committed by GitHub
parent f1625d9e56
commit d7c554c25e
6 changed files with 133 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/rendering/capabilities_test.go
+1 -1
View File
@@ -121,7 +121,7 @@ func TestCapabilities(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
rs.Cfg.RendererUrl = tt.rendererUrl
rs.Cfg.RendererServerUrl = tt.rendererUrl
rs.version = tt.rendererVersion
res, err := rs.HasCapability(context.Background(), tt.capabilityName)
pkg/services/rendering/http_mode.go
+2 -2
View File
@@ -71,7 +71,7 @@ func (rs *RenderingService) renderCSVViaHTTP(ctx context.Context, renderKey stri
}
func (rs *RenderingService) generateImageRendererURL(renderType RenderType, opts Opts, renderKey string) (*url.URL, error) {
rendererUrl := rs.Cfg.RendererUrl
rendererUrl := rs.Cfg.RendererServerUrl
if renderType == RenderCSV {
rendererUrl += "/csv"
}
@@ -242,7 +242,7 @@ func (rs *RenderingService) getRemotePluginVersionWithRetry(callback func(string
}
func (rs *RenderingService) getRemotePluginVersion() (string, error) {
rendererURL, err := url.Parse(rs.Cfg.RendererUrl + "/version")
rendererURL, err := url.Parse(rs.Cfg.RendererServerUrl + "/version")
if err != nil {
return "", err
}
pkg/services/rendering/rendering.go
+38 -24
View File
@@ -27,18 +27,19 @@ import (
var _ Service = (*RenderingService)(nil)
type RenderingService struct {
log log.Logger
plugin Plugin
renderAction renderFunc
renderCSVAction renderCSVFunc
sanitizeSVGAction sanitizeFunc
sanitizeURL string
domain string
inProgressCount int32
version string
versionMutex sync.RWMutex
capabilities []Capability
pluginAvailable bool
log log.Logger
plugin Plugin
renderAction renderFunc
renderCSVAction renderCSVFunc
sanitizeSVGAction sanitizeFunc
sanitizeURL string
domain string
inProgressCount int32
version string
versionMutex sync.RWMutex
capabilities []Capability
pluginAvailable bool
rendererCallbackURL string
perRequestRenderKeyProvider renderKeyProvider
Cfg *setting.Cfg
@@ -80,9 +81,24 @@ func ProvideService(cfg *setting.Cfg, features featuremgmt.FeatureToggles, remot
// value used for domain attribute of renderKey cookie
var domain string
// value used by the image renderer to make requests to Grafana
rendererCallbackURL := cfg.RendererCallbackUrl
if cfg.RendererServerUrl != "" {
sanitizeURL = getSanitizerURL(cfg.RendererServerUrl)
// Default value for callback URL using a remote renderer should be AppURL
if rendererCallbackURL == "" {
rendererCallbackURL = cfg.AppURL
}
}
switch {
case cfg.RendererCallbackUrl != "":
u, err := url.Parse(cfg.RendererCallbackUrl)
case rendererCallbackURL != "":
if rendererCallbackURL[len(rendererCallbackURL)-1] != '/' {
rendererCallbackURL += "/"
}
u, err := url.Parse(rendererCallbackURL)
if err != nil {
logger.Warn("Image renderer callback url is not valid. " +
"Please provide a valid RendererCallbackUrl. " +
@@ -96,10 +112,6 @@ func ProvideService(cfg *setting.Cfg, features featuremgmt.FeatureToggles, remot
domain = "localhost"
}
if cfg.RendererUrl != "" {
sanitizeURL = getSanitizerURL(cfg.RendererUrl)
}
var renderKeyProvider renderKeyProvider
if features.IsEnabledGlobally(featuremgmt.FlagRenderAuthJWT) {
renderKeyProvider = &jwtRenderKeyProvider{
@@ -145,6 +157,7 @@ func ProvideService(cfg *setting.Cfg, features featuremgmt.FeatureToggles, remot
domain: domain,
sanitizeURL: sanitizeURL,
pluginAvailable: exists,
rendererCallbackURL: rendererCallbackURL,
}
gob.Register(&RenderUser{})
@@ -215,7 +228,7 @@ func (rs *RenderingService) Run(ctx context.Context) error {
}
func (rs *RenderingService) remoteAvailable() bool {
return rs.Cfg.RendererUrl != ""
return rs.Cfg.RendererServerUrl != ""
}
func (rs *RenderingService) IsAvailable(ctx context.Context) bool {
@@ -424,13 +437,14 @@ func (rs *RenderingService) getNewFilePath(rt RenderType) (string, error) {
// getGrafanaCallbackURL creates a URL to send to the image rendering as callback for rendering a Grafana resource
func (rs *RenderingService) getGrafanaCallbackURL(path string) string {
if rs.Cfg.RendererUrl != "" || rs.Cfg.RendererCallbackUrl != "" {
// The backend rendering service can potentially be remote.
// So we need to use the root_url to ensure the rendering service
// can reach this Grafana instance.
if rs.rendererCallbackURL != "" {
// rendererCallbackURL should be set if:
// - the backend rendering service is remote (default value is cfg.AppURL
// and set when initializing the service)
// - the service is a plugin and Grafana is running behind a proxy changing its domain
// &render=1 signals to the legacy redirect layer to
return fmt.Sprintf("%s%s&render=1", rs.Cfg.RendererCallbackUrl, path)
return fmt.Sprintf("%s%s&render=1", rs.rendererCallbackURL, path)
}
protocol := rs.Cfg.Protocol
pkg/services/rendering/rendering_test.go
+87 -13
View File
@@ -14,6 +14,7 @@ import (
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/setting"
)
@@ -25,21 +26,21 @@ func TestGetUrl(t *testing.T) {
}
t.Run("When renderer and callback url configured should return callback url plus path", func(t *testing.T) {
rs.Cfg.RendererUrl = "http://localhost:8081/render"
rs.Cfg.RendererCallbackUrl = "http://public-grafana.com/"
rs.Cfg.RendererServerUrl = "http://localhost:8081/render"
rs.rendererCallbackURL = "http://public-grafana.com/"
url := rs.getGrafanaCallbackURL(path)
require.Equal(t, rs.Cfg.RendererCallbackUrl+path+"&render=1", url)
require.Equal(t, rs.rendererCallbackURL+path+"&render=1", url)
})
t.Run("When callback url is configured and https should return domain of callback url plus path", func(t *testing.T) {
rs.Cfg.RendererCallbackUrl = "https://public-grafana.com/"
rs.rendererCallbackURL = "https://public-grafana.com/"
url := rs.getGrafanaCallbackURL(path)
require.Equal(t, rs.Cfg.RendererCallbackUrl+path+"&render=1", url)
require.Equal(t, rs.rendererCallbackURL+path+"&render=1", url)
})
t.Run("When renderer url not configured", func(t *testing.T) {
rs.Cfg.RendererUrl = ""
rs.Cfg.RendererCallbackUrl = ""
rs.Cfg.RendererServerUrl = ""
rs.rendererCallbackURL = ""
rs.domain = "localhost"
rs.Cfg.HTTPPort = "3000"
@@ -128,8 +129,8 @@ func TestRenderLimitImage(t *testing.T) {
rs := RenderingService{
Cfg: &setting.Cfg{
HomePath: path,
RendererUrl: "http://localhost:8081/render",
HomePath: path,
RendererServerUrl: "http://localhost:8081/render",
},
inProgressCount: 2,
log: log.New("test"),
@@ -170,7 +171,7 @@ func TestRenderLimitImage(t *testing.T) {
func TestRenderLimitImageError(t *testing.T) {
rs := RenderingService{
Cfg: &setting.Cfg{
RendererUrl: "http://localhost:8081/render",
RendererServerUrl: "http://localhost:8081/render",
},
inProgressCount: 2,
log: log.New("test"),
@@ -201,7 +202,7 @@ func TestRenderingServiceGetRemotePluginVersion(t *testing.T) {
}))
defer server.Close()
rs.Cfg.RendererUrl = server.URL + "/render"
rs.Cfg.RendererServerUrl = server.URL + "/render"
version, err := rs.getRemotePluginVersion()
require.NoError(t, err)
@@ -214,7 +215,7 @@ func TestRenderingServiceGetRemotePluginVersion(t *testing.T) {
}))
defer server.Close()
rs.Cfg.RendererUrl = server.URL + "/render"
rs.Cfg.RendererServerUrl = server.URL + "/render"
version, err := rs.getRemotePluginVersion()
require.NoError(t, err)
@@ -239,7 +240,7 @@ func TestRenderingServiceGetRemotePluginVersion(t *testing.T) {
}))
defer server.Close()
rs.Cfg.RendererUrl = server.URL + "/render"
rs.Cfg.RendererServerUrl = server.URL + "/render"
remoteVersionFetchInterval = time.Millisecond
remoteVersionFetchRetries = 5
go func() {
@@ -249,3 +250,76 @@ func TestRenderingServiceGetRemotePluginVersion(t *testing.T) {
require.Eventually(t, func() bool { return rs.Version() == "3.1.4159" }, time.Second, time.Millisecond)
})
}
func TestProvideService(t *testing.T) {
cfg := setting.NewCfg()
cfg.AppURL = "http://app-url"
cfg.ImagesDir = filepath.Join(t.TempDir(), "images")
cfg.CSVsDir = filepath.Join(t.TempDir(), "csvs")
cfg.PDFsDir = filepath.Join(t.TempDir(), "pdfs")
t.Run("Default configuration values", func(t *testing.T) {
rs, err := ProvideService(cfg, featuremgmt.WithFeatures(), nil, &dummyPluginManager{})
require.NoError(t, err)
require.Equal(t, "", rs.Cfg.RendererServerUrl)
require.Equal(t, "", rs.rendererCallbackURL)
require.Equal(t, "", rs.domain)
})
t.Run("RendererURL is set but not RendererCallbackUrl", func(t *testing.T) {
cfg.RendererServerUrl = "http://custom-renderer:8081"
cfg.RendererCallbackUrl = ""
rs, err := ProvideService(cfg, featuremgmt.WithFeatures(), nil, &dummyPluginManager{})
require.NoError(t, err)
require.Equal(t, "http://custom-renderer:8081", rs.Cfg.RendererServerUrl)
require.Equal(t, "http://app-url/", rs.rendererCallbackURL)
require.Equal(t, "app-url", rs.domain)
})
t.Run("RendererURL and RendererCallbackUrl are set", func(t *testing.T) {
cfg.RendererServerUrl = "http://custom-renderer:8081"
cfg.RendererCallbackUrl = "http://public-grafana.com/"
rs, err := ProvideService(cfg, featuremgmt.WithFeatures(), nil, &dummyPluginManager{})
require.NoError(t, err)
require.Equal(t, "http://custom-renderer:8081", rs.Cfg.RendererServerUrl)
require.Equal(t, "http://public-grafana.com/", rs.rendererCallbackURL)
require.Equal(t, "public-grafana.com", rs.domain)
})
t.Run("RendererURL is not set but RendererCallbackUrl is set", func(t *testing.T) {
cfg.RendererServerUrl = ""
cfg.RendererCallbackUrl = "https://public-grafana.com/"
rs, err := ProvideService(cfg, featuremgmt.WithFeatures(), nil, &dummyPluginManager{})
require.NoError(t, err)
require.Equal(t, "", rs.Cfg.RendererServerUrl)
require.Equal(t, "https://public-grafana.com/", rs.rendererCallbackURL)
require.Equal(t, "public-grafana.com", rs.domain)
})
t.Run("RendererCallbackURL is missing trailing slash", func(t *testing.T) {
cfg.RendererServerUrl = ""
cfg.RendererCallbackUrl = "https://public-grafana.com"
rs, err := ProvideService(cfg, featuremgmt.WithFeatures(), nil, &dummyPluginManager{})
require.NoError(t, err)
require.Equal(t, "", rs.Cfg.RendererServerUrl)
require.Equal(t, "https://public-grafana.com/", rs.rendererCallbackURL)
require.Equal(t, "public-grafana.com", rs.domain)
})
t.Run("RendererCallbackURL is invalid", func(t *testing.T) {
cfg.RendererServerUrl = ""
cfg.RendererCallbackUrl = "http://public{grafana"
_, err := ProvideService(cfg, featuremgmt.WithFeatures(), nil, &dummyPluginManager{})
require.Error(t, err)
})
}
pkg/setting/setting.go
+4 -22
View File
@@ -144,7 +144,7 @@ type Cfg struct {
ImagesDir string
CSVsDir string
PDFsDir string
RendererUrl string
RendererServerUrl string
RendererCallbackUrl string
RendererAuthToken string
RendererConcurrentRequestLimit int
@@ -1168,9 +1168,7 @@ func (cfg *Cfg) parseINIFile(iniFile *ini.File) error {
cfg.readZanzanaSettings()
if err := cfg.readRenderingSettings(iniFile); err != nil {
return err
}
cfg.readRenderingSettings(iniFile)
cfg.TempDataLifetime = iniFile.Section("paths").Key("temp_data_lifetime").MustDuration(time.Second * 3600 * 24)
cfg.MetricsEndpointEnabled = iniFile.Section("metrics").Key("enabled").MustBool(true)
@@ -1796,26 +1794,12 @@ func readServiceAccountSettings(iniFile *ini.File, cfg *Cfg) error {
return nil
}
func (cfg *Cfg) readRenderingSettings(iniFile *ini.File) error {
func (cfg *Cfg) readRenderingSettings(iniFile *ini.File) {
renderSec := iniFile.Section("rendering")
cfg.RendererUrl = valueAsString(renderSec, "server_url", "")
cfg.RendererServerUrl = valueAsString(renderSec, "server_url", "")
cfg.RendererCallbackUrl = valueAsString(renderSec, "callback_url", "")
cfg.RendererAuthToken = valueAsString(renderSec, "renderer_token", "-")
if cfg.RendererCallbackUrl == "" {
cfg.RendererCallbackUrl = AppUrl
} else {
if cfg.RendererCallbackUrl[len(cfg.RendererCallbackUrl)-1] != '/' {
cfg.RendererCallbackUrl += "/"
}
_, err := url.Parse(cfg.RendererCallbackUrl)
if err != nil {
// XXX: Should return an error?
cfg.Logger.Error("Invalid callback_url.", "url", cfg.RendererCallbackUrl, "error", err)
os.Exit(1)
}
}
cfg.RendererConcurrentRequestLimit = renderSec.Key("concurrent_render_request_limit").MustInt(30)
cfg.RendererRenderKeyLifeTime = renderSec.Key("render_key_lifetime").MustDuration(5 * time.Minute)
cfg.RendererDefaultImageWidth = renderSec.Key("default_image_width").MustInt(1000)
@@ -1824,8 +1808,6 @@ func (cfg *Cfg) readRenderingSettings(iniFile *ini.File) error {
cfg.ImagesDir = filepath.Join(cfg.DataPath, "png")
cfg.CSVsDir = filepath.Join(cfg.DataPath, "csv")
cfg.PDFsDir = filepath.Join(cfg.DataPath, "pdf")
return nil
}
func (cfg *Cfg) readAlertingSettings(iniFile *ini.File) error {
pkg/setting/setting_test.go
+1 -12
View File
@@ -33,7 +33,7 @@ func TestLoadingSettings(t *testing.T) {
require.Nil(t, err)
require.Equal(t, "admin", cfg.AdminUser)
require.Equal(t, "http://localhost:3000/", cfg.RendererCallbackUrl)
require.Equal(t, "", cfg.RendererCallbackUrl)
require.Equal(t, "TLS1.2", cfg.MinTLSVersion)
})
@@ -255,17 +255,6 @@ func TestLoadingSettings(t *testing.T) {
require.Equal(t, hostname, cfg.InstanceName)
})
t.Run("Reading callback_url should add trailing slash", func(t *testing.T) {
cfg := NewCfg()
err := cfg.Load(CommandLineArgs{
HomePath: "../../",
Args: []string{"cfg:rendering.callback_url=http://myserver/renderer"},
})
require.Nil(t, err)
require.Equal(t, "http://myserver/renderer/", cfg.RendererCallbackUrl)
})
t.Run("Only sync_ttl should return the value sync_ttl", func(t *testing.T) {
cfg := NewCfg()
err := cfg.Load(CommandLineArgs{