diff --git a/pkg/api/rendercsv.go b/pkg/api/rendercsv.go index 2d9e3535..b533fa66 100644 --- a/pkg/api/rendercsv.go +++ b/pkg/api/rendercsv.go @@ -5,7 +5,10 @@ import ( "errors" "fmt" "log/slog" + "mime" "net/http" + "path" + "path/filepath" "strconv" "strings" "time" @@ -79,7 +82,7 @@ func HandleGetRenderCSV(browser *service.BrowserService) http.Handler { attribute.String("renderKeyDomain", domain)) start := time.Now() - contents, err := browser.RenderCSV(ctx, url, renderKey, domain, acceptLanguage) + contents, fileName, err := browser.RenderCSV(ctx, url, renderKey, domain, acceptLanguage) if err != nil { MetricRenderCSVDuration.WithLabelValues("error").Observe(time.Since(start).Seconds()) span.SetStatus(codes.Error, "csv rendering failed") @@ -97,8 +100,24 @@ func HandleGetRenderCSV(browser *service.BrowserService) http.Handler { MetricRenderCSVDuration.WithLabelValues("success").Observe(time.Since(start).Seconds()) span.SetStatus(codes.Ok, "csv rendered successfully") + requestedFilePath := r.URL.Query().Get("filePath") + requestedFilePath = filepath.ToSlash(requestedFilePath) + requestedFilePath = strings.TrimPrefix(requestedFilePath, "/") + requestedFilePath = path.Base(requestedFilePath) + if requestedFilePath == "." || requestedFilePath == "/" || requestedFilePath == "" { + requestedFilePath = fileName + } + if requestedFilePath == "" { + requestedFilePath = "data.csv" + } + if !strings.HasSuffix(strings.ToLower(requestedFilePath), ".csv") { + requestedFilePath += ".csv" + } + w.Header().Set("Content-Type", "text/csv") - w.Header().Set("Content-Disposition", `attachment; filename="data.csv"`) + w.Header().Set("Content-Disposition", mime.FormatMediaType("attachment", map[string]string{ + "filename": requestedFilePath, + })) _, _ = w.Write(contents) }) } diff --git a/pkg/service/browser.go b/pkg/service/browser.go index e0f70d2e..3002e918 100644 --- a/pkg/service/browser.go +++ b/pkg/service/browser.go @@ -279,19 +279,19 @@ func (s *BrowserService) Render(ctx context.Context, url string, printer Printer // You may be thinking: what the hell are we doing? Why are we using a browser for this? // The CSV endpoint just returns HTML. The actual query is done by the browser, and then a script _in the webpage_ downloads it as a CSV file. // This SHOULD be replaced at some point, such that the Grafana server does all the work; this is just not acceptable behaviour... -func (s *BrowserService) RenderCSV(ctx context.Context, url, renderKey, domain, acceptLanguage string) ([]byte, error) { +func (s *BrowserService) RenderCSV(ctx context.Context, url, renderKey, domain, acceptLanguage string) ([]byte, string, error) { tracer := tracer(ctx) ctx, span := tracer.Start(ctx, "BrowserService.RenderCSV") defer span.End() start := time.Now() if url == "" { - return nil, fmt.Errorf("url must not be empty") + return nil, "", fmt.Errorf("url must not be empty") } allocatorOptions, err := s.createAllocatorOptions(s.cfg) if err != nil { - return nil, fmt.Errorf("failed to create allocator options: %w", err) + return nil, "", fmt.Errorf("failed to create allocator options: %w", err) } allocatorCtx, cancelAllocator := chromedp.NewExecAllocator(ctx, allocatorOptions...) defer cancelAllocator() @@ -301,7 +301,7 @@ func (s *BrowserService) RenderCSV(ctx context.Context, url, renderKey, domain, tmpDir, err := os.MkdirTemp("", "gir-csv-*") if err != nil { - return nil, fmt.Errorf("failed to create temporary directory: %w", err) + return nil, "", fmt.Errorf("failed to create temporary directory: %w", err) } defer func() { if err := os.RemoveAll(tmpDir); err != nil { @@ -337,7 +337,7 @@ func (s *BrowserService) RenderCSV(ctx context.Context, url, renderKey, domain, MetricBrowserInstancesActive.Inc() defer MetricBrowserInstancesActive.Dec() if err := chromedp.Run(browserCtx, actions...); err != nil { - return nil, fmt.Errorf("failed to run browser: %w", err) + return nil, "", fmt.Errorf("failed to run browser: %w", err) } span.AddEvent("actions completed") @@ -345,7 +345,7 @@ func (s *BrowserService) RenderCSV(ctx context.Context, url, renderKey, domain, var entries []os.DirEntry for { if err := ctx.Err(); err != nil { - return nil, err + return nil, "", err } entries, err = os.ReadDir(tmpDir) @@ -355,7 +355,7 @@ func (s *BrowserService) RenderCSV(ctx context.Context, url, renderKey, domain, select { case <-ctx.Done(): - return nil, ctx.Err() + return nil, "", ctx.Err() case <-time.After(100 * time.Millisecond): // try again } @@ -364,11 +364,11 @@ func (s *BrowserService) RenderCSV(ctx context.Context, url, renderKey, domain, fileContents, err := os.ReadFile(filepath.Join(tmpDir, entries[0].Name())) if err != nil { - return nil, fmt.Errorf("failed to read temporary file: %w", err) + return nil, "", fmt.Errorf("failed to read temporary file: %w", err) } MetricBrowserRenderCSVDuration.Observe(time.Since(start).Seconds()) - return fileContents, nil + return fileContents, filepath.Base(entries[0].Name()), nil } func (s *BrowserService) createAllocatorOptions(cfg config.BrowserConfig) ([]chromedp.ExecAllocatorOption, error) { diff --git a/tests/acceptance/rendering_grafana_test.go b/tests/acceptance/rendering_grafana_test.go index 44000b2b..d5d917b5 100644 --- a/tests/acceptance/rendering_grafana_test.go +++ b/tests/acceptance/rendering_grafana_test.go @@ -114,7 +114,7 @@ func TestRenderingGrafana(t *testing.T) { contentDisposition := resp.Header.Get("Content-Disposition") _, params, err := mime.ParseMediaType(contentDisposition) require.NoError(t, err, "could not parse Content-Disposition header") - require.NotEmpty(t, params["filename"], "no filename in Content-Disposition header") + require.Regexp(t, `^Prometheus_ 1-data-.*\.csv$`, params["filename"]) body := ReadBody(t, resp.Body) const fixture = "render-prometheus-defaults.csv"