improve remote image rendering (#13102)

* improve remote image rendering

- determine "domain" during Init() so we are not re-parsing settings
  on every request
- if using http-mode via a rednererUrl, then use the AppUrl for the
  page that the renderer loads.  When in http-mode the renderer is likely
  running on another server so trying to use the localhost or even the
  specific IP:PORT grafana is listening on wont work.
- apply the request timeout via a context rather then directly on the http client.
- use a global http client so we can take advantage of connection re-use
- log and handle errors better.

* ensure imagesDir exists

* allow users to define callback_url for remote rendering

- allow users to define the url that a remote rendering service
  should use for connecting back to the grafana instance.
  By default the "root_url" is used.

* improve remote image rendering

- determine "domain" during Init() so we are not re-parsing settings
  on every request
- if using http-mode via a rednererUrl, then use the AppUrl for the
  page that the renderer loads.  When in http-mode the renderer is likely
  running on another server so trying to use the localhost or even the
  specific IP:PORT grafana is listening on wont work.
- apply the request timeout via a context rather then directly on the http client.
- use a global http client so we can take advantage of connection re-use
- log and handle errors better.

* ensure imagesDir exists

* allow users to define callback_url for remote rendering

- allow users to define the url that a remote rendering service
  should use for connecting back to the grafana instance.
  By default the "root_url" is used.

* rendering: fixed issue with renderKey where userId and orgId was in mixed up, added test for RenderCallbackUrl reading logic

conf/defaults.ini

@@ -538,3 +538,8 @@ container_name =
 
 [external_image_storage.local]
 # does not require any configuration
+
+[rendering]
+# Options to configure external image rendering server like https://github.com/grafana/grafana-image-renderer
+server_url =
+callback_url =

conf/sample.ini

@@ -460,3 +460,8 @@ log_queries =
 
 [external_image_storage.local]
 # does not require any configuration
+
+[rendering]
+# Options to configure external image rendering server like https://github.com/grafana/grafana-image-renderer
+;server_url =
+;callback_url =

pkg/services/rendering/http_mode.go

@@ -2,6 +2,7 @@ package rendering
 
 import (
 	"context"
+	"fmt"
 	"io"
 	"net"
 	"net/http"
@@ -20,14 +21,13 @@ var netTransport = &http.Transport{
 	TLSHandshakeTimeout: 5 * time.Second,
 }
 
+var netClient = &http.Client{
+	Transport: netTransport,
+}
+
 func (rs *RenderingService) renderViaHttp(ctx context.Context, opts Opts) (*RenderResult, error) {
 	filePath := rs.getFilePathForNewImage()
 
-	var netClient = &http.Client{
-		Timeout:   opts.Timeout,
-		Transport: netTransport,
-	}
-
 	rendererUrl, err := url.Parse(rs.Cfg.RendererUrl)
 	if err != nil {
 		return nil, err
@@ -35,10 +35,10 @@ func (rs *RenderingService) renderViaHttp(ctx context.Context, opts Opts) (*Rend
 
 	queryParams := rendererUrl.Query()
 	queryParams.Add("url", rs.getURL(opts.Path))
-	queryParams.Add("renderKey", rs.getRenderKey(opts.UserId, opts.OrgId, opts.OrgRole))
+	queryParams.Add("renderKey", rs.getRenderKey(opts.OrgId, opts.UserId, opts.OrgRole))
 	queryParams.Add("width", strconv.Itoa(opts.Width))
 	queryParams.Add("height", strconv.Itoa(opts.Height))
-	queryParams.Add("domain", rs.getLocalDomain())
+	queryParams.Add("domain", rs.domain)
 	queryParams.Add("timezone", isoTimeOffsetToPosixTz(opts.Timezone))
 	queryParams.Add("encoding", opts.Encoding)
 	queryParams.Add("timeout", strconv.Itoa(int(opts.Timeout.Seconds())))
@@ -49,20 +49,48 @@ func (rs *RenderingService) renderViaHttp(ctx context.Context, opts Opts) (*Rend
 		return nil, err
 	}
 
+	reqContext, cancel := context.WithTimeout(ctx, opts.Timeout+time.Second*2)
+	defer cancel()
+
+	req = req.WithContext(reqContext)
+
 	// make request to renderer server
 	resp, err := netClient.Do(req)
 	if err != nil {
-		return nil, err
+		rs.log.Error("Failed to send request to remote rendering service.", "error", err)
+		return nil, fmt.Errorf("Failed to send request to remote rendering service. %s", err)
 	}
 
 	// save response to file
 	defer resp.Body.Close()
+
+	// check for timeout first
+	if reqContext.Err() == context.DeadlineExceeded {
+		rs.log.Info("Rendering timed out")
+		return nil, ErrTimeout
+	}
+
+	// if we didnt get a 200 response, something went wrong.
+	if resp.StatusCode != http.StatusOK {
+		rs.log.Error("Remote rendering request failed", "error", resp.Status)
+		return nil, fmt.Errorf("Remote rendering request failed. %d: %s", resp.StatusCode, resp.Status)
+	}
+
 	out, err := os.Create(filePath)
 	if err != nil {
 		return nil, err
 	}
 	defer out.Close()
-	io.Copy(out, resp.Body)
+	_, err = io.Copy(out, resp.Body)
+	if err != nil {
+		// check that we didnt timeout while receiving the response.
+		if reqContext.Err() == context.DeadlineExceeded {
+			rs.log.Info("Rendering timed out")
+			return nil, ErrTimeout
+		}
+		rs.log.Error("Remote rendering request failed", "error", err)
+		return nil, fmt.Errorf("Remote rendering request failed.  %s", err)
+	}
 
 	return &RenderResult{FilePath: filePath}, err
 }

pkg/services/rendering/phantomjs.go

@@ -49,7 +49,7 @@ func (rs *RenderingService) renderViaPhantomJS(ctx context.Context, opts Opts) (
 		fmt.Sprintf("width=%v", opts.Width),
 		fmt.Sprintf("height=%v", opts.Height),
 		fmt.Sprintf("png=%v", pngPath),
-		fmt.Sprintf("domain=%v", rs.getLocalDomain()),
+		fmt.Sprintf("domain=%v", rs.domain),
 		fmt.Sprintf("timeout=%v", opts.Timeout.Seconds()),
 		fmt.Sprintf("renderKey=%v", renderKey),
 	}

pkg/services/rendering/plugin_mode.go

@@ -77,10 +77,10 @@ func (rs *RenderingService) renderViaPlugin(ctx context.Context, opts Opts) (*Re
 		Height:    int32(opts.Height),
 		FilePath:  pngPath,
 		Timeout:   int32(opts.Timeout.Seconds()),
-		RenderKey: rs.getRenderKey(opts.UserId, opts.OrgId, opts.OrgRole),
+		RenderKey: rs.getRenderKey(opts.OrgId, opts.UserId, opts.OrgRole),
 		Encoding:  opts.Encoding,
 		Timezone:  isoTimeOffsetToPosixTz(opts.Timezone),
-		Domain:    rs.getLocalDomain(),
+		Domain:    rs.domain,
 	})
 
 	if err != nil {

pkg/services/rendering/rendering.go

@@ -3,6 +3,8 @@ package rendering
 import (
 	"context"
 	"fmt"
+	"net/url"
+	"os"
 	"path/filepath"
 
 	plugin "github.com/hashicorp/go-plugin"
@@ -27,12 +29,31 @@ type RenderingService struct {
 	grpcPlugin   pluginModel.RendererPlugin
 	pluginInfo   *plugins.RendererPlugin
 	renderAction renderFunc
+	domain       string
 
 	Cfg *setting.Cfg `inject:""`
 }
 
 func (rs *RenderingService) Init() error {
 	rs.log = log.New("rendering")
+
+	// ensure ImagesDir exists
+	err := os.MkdirAll(rs.Cfg.ImagesDir, 0700)
+	if err != nil {
+		return err
+	}
+
+	// set value used for domain attribute of renderKey cookie
+	if rs.Cfg.RendererUrl != "" {
+		// RendererCallbackUrl has already been passed, it wont generate an error.
+		u, _ := url.Parse(rs.Cfg.RendererCallbackUrl)
+		rs.domain = u.Hostname()
+	} else if setting.HttpAddr != setting.DEFAULT_HTTP_ADDR {
+		rs.domain = setting.HttpAddr
+	} else {
+		rs.domain = "localhost"
+	}
+
 	return nil
 }
 
@@ -82,16 +103,17 @@ func (rs *RenderingService) getFilePathForNewImage() string {
 }
 
 func (rs *RenderingService) getURL(path string) string {
-	// &render=1 signals to the legacy redirect layer to
-	return fmt.Sprintf("%s://%s:%s/%s&render=1", setting.Protocol, rs.getLocalDomain(), setting.HttpPort, path)
-}
+	if rs.Cfg.RendererUrl != "" {
+		// 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.
 
-func (rs *RenderingService) getLocalDomain() string {
-	if setting.HttpAddr != setting.DEFAULT_HTTP_ADDR {
-		return setting.HttpAddr
-	}
+		// &render=1 signals to the legacy redirect layer to
+		return fmt.Sprintf("%s%s&render=1", rs.Cfg.RendererCallbackUrl, path)
 
-	return "localhost"
+	}
+	// &render=1 signals to the legacy redirect layer to
+	return fmt.Sprintf("%s://%s:%s/%s&render=1", setting.Protocol, rs.domain, setting.HttpPort, path)
 }
 
 func (rs *RenderingService) getRenderKey(orgId, userId int64, orgRole models.RoleType) string {

pkg/setting/setting.go

@@ -197,6 +197,7 @@ type Cfg struct {
 	ImagesDir                        string
 	PhantomDir                       string
 	RendererUrl                      string
+	RendererCallbackUrl              string
 	DisableBruteForceLoginProtection bool
 
 	TempDataLifetime time.Duration
@@ -641,6 +642,18 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error {
 	// Rendering
 	renderSec := iniFile.Section("rendering")
 	cfg.RendererUrl = renderSec.Key("server_url").String()
+	cfg.RendererCallbackUrl = renderSec.Key("callback_url").String()
+	if cfg.RendererCallbackUrl == "" {
+		cfg.RendererCallbackUrl = AppUrl
+	} else {
+		if cfg.RendererCallbackUrl[len(cfg.RendererCallbackUrl)-1] != '/' {
+			cfg.RendererCallbackUrl += "/"
+		}
+		_, err := url.Parse(cfg.RendererCallbackUrl)
+		if err != nil {
+			log.Fatal(4, "Invalid callback_url(%s): %s", cfg.RendererCallbackUrl, err)
+		}
+	}
 	cfg.ImagesDir = filepath.Join(DataPath, "png")
 	cfg.PhantomDir = filepath.Join(HomePath, "tools/phantomjs")
 	cfg.TempDataLifetime = iniFile.Section("paths").Key("temp_data_lifetime").MustDuration(time.Second * 3600 * 24)

pkg/setting/setting_test.go

@@ -20,6 +20,7 @@ func TestLoadingSettings(t *testing.T) {
 			So(err, ShouldBeNil)
 
 			So(AdminUser, ShouldEqual, "admin")
+			So(cfg.RendererCallbackUrl, ShouldEqual, "http://localhost:3000/")
 		})
 
 		Convey("Should be able to override via environment variables", func() {
@@ -178,5 +179,15 @@ func TestLoadingSettings(t *testing.T) {
 			So(InstanceName, ShouldEqual, hostname)
 		})
 
+		Convey("Reading callback_url should add trailing slash", func() {
+			cfg := NewCfg()
+			cfg.Load(&CommandLineArgs{
+				HomePath: "../../",
+				Args:     []string{"cfg:rendering.callback_url=http://myserver/renderer"},
+			})
+
+			So(cfg.RendererCallbackUrl, ShouldEqual, "http://myserver/renderer/")
+		})
+
 	})
 }