Commit ea56e1bd authored by Matthias Käppler's avatar Matthias Käppler 🚴🏿 Committed by Jacob Vosmaer (GitLab)
Browse files

Image scaler binary with pure golang lib

parent 07bbad76
......@@ -2,6 +2,7 @@ testdata/data
testdata/scratch
testdata/public
/gitlab-workhorse
/gitlab-resize-image
/gitlab-zip-cat
/gitlab-zip-metadata
/_build
......
......@@ -38,7 +38,7 @@ changelog:
GITALY_ADDRESS: "tcp://gitaly:8075"
script:
- go version
- apt-get update && apt-get -y install libimage-exiftool-perl graphicsmagick
- apt-get update && apt-get -y install libimage-exiftool-perl
- make test
 
test using go 1.13:
......
......@@ -11,7 +11,7 @@ VERSION_STRING := v$(shell cat VERSION)
endif
BUILD_TIME := $(shell date -u +%Y%m%d.%H%M%S)
GOBUILD := go build -ldflags "-X main.Version=$(VERSION_STRING) -X main.BuildTime=$(BUILD_TIME)"
EXE_ALL := gitlab-zip-cat gitlab-zip-metadata gitlab-workhorse
EXE_ALL := gitlab-resize-image gitlab-zip-cat gitlab-zip-metadata gitlab-workhorse
INSTALL := install
BUILD_TAGS := tracer_static tracer_static_jaeger continuous_profiler_stackdriver
 
......@@ -40,6 +40,10 @@ $(TARGET_SETUP):
mkdir -p "$(TARGET_DIR)"
touch "$(TARGET_SETUP)"
 
gitlab-resize-image: $(TARGET_SETUP) $(shell find cmd/gitlab-resize-image/ -name '*.go')
$(call message,Building $@)
$(GOBUILD) -tags "$(BUILD_TAGS)" -o $(BUILD_DIR)/$@ $(PKG)/cmd/$@
gitlab-zip-cat: $(TARGET_SETUP) $(shell find cmd/gitlab-zip-cat/ -name '*.go')
$(call message,Building $@)
$(GOBUILD) -tags "$(BUILD_TAGS)" -o $(BUILD_DIR)/$@ $(PKG)/cmd/$@
......@@ -53,7 +57,7 @@ gitlab-workhorse: $(TARGET_SETUP) $(shell find . -name '*.go' | grep -v '^\./_')
$(GOBUILD) -tags "$(BUILD_TAGS)" -o $(BUILD_DIR)/$@ $(PKG)
 
.PHONY: install
install: gitlab-workhorse gitlab-zip-cat gitlab-zip-metadata
install: gitlab-workhorse gitlab-resize-image gitlab-zip-cat gitlab-zip-metadata
$(call message,$@)
mkdir -p $(DESTDIR)$(PREFIX)/bin/
cd $(BUILD_DIR) && $(INSTALL) gitlab-workhorse gitlab-zip-cat gitlab-zip-metadata $(DESTDIR)$(PREFIX)/bin/
......
......@@ -212,28 +212,6 @@ images. If you installed GitLab:
sudo yum install perl-Image-ExifTool
```
 
### GraphicsMagick (**experimental**)
Workhorse has an experimental feature that allows us to rescale images on-the-fly.
If you do not run Workhorse in a container where the `gm` tool is already installed,
you will have to install it on your host machine instead:
#### macOS
```sh
brew install graphicsmagick
```
#### Debian/Ubuntu
```sh
sudo apt-get install graphicsmagick
```
For installation on other platforms, please consult http://www.graphicsmagick.org/README.html.
Note that Omnibus containers already come with `gm` installed.
## Error tracking
 
GitLab-Workhorse supports remote error tracking with
......
---
title: Switch image scaler to a Go-only solution
merge_request: 603
author:
type: changed
package main
import (
"fmt"
"image"
"mime"
"os"
"strconv"
"github.com/disintegration/imaging"
)
func main() {
if err := _main(); err != nil {
fmt.Fprintf(os.Stderr, "%s: fatal: %v\n", os.Args[0], err)
os.Exit(1)
}
}
func _main() error {
widthParam := os.Getenv("GL_RESIZE_IMAGE_WIDTH")
requestedWidth, err := strconv.Atoi(widthParam)
if err != nil {
return fmt.Errorf("GL_RESIZE_IMAGE_WIDTH: %w", err)
}
contentType := os.Getenv("GL_RESIZE_IMAGE_CONTENT_TYPE")
if contentType == "" {
return fmt.Errorf("GL_RESIZE_IMAGE_CONTENT_TYPE is empty")
}
src, extension, err := image.Decode(os.Stdin)
if err != nil {
return fmt.Errorf("decode: %w", err)
}
if detectedType := mime.TypeByExtension("." + extension); detectedType != contentType {
return fmt.Errorf("MIME types do not match; requested: %s; actual: %s", contentType, detectedType)
}
format, err := imaging.FormatFromExtension(extension)
if err != nil {
return fmt.Errorf("find imaging format: %w", err)
}
image := imaging.Resize(src, requestedWidth, 0, imaging.Lanczos)
return imaging.Encode(os.Stdout, image, format)
}
......@@ -155,6 +155,8 @@ github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZm
github.com/dgryski/go-farm v0.0.0-20190423205320-6a90982ecee2/go.mod h1:SqUrOPUnsFjfmXRMNPybcSiG0BgUW2AuFH8PAnS2iTw=
github.com/dimchansky/utfbom v1.1.0 h1:FcM3g+nofKgUteL8dm/UpdRXNC9KmADgTpLKsu0TRo4=
github.com/dimchansky/utfbom v1.1.0/go.mod h1:rO41eb7gLfo8SF1jd9F8HplJm1Fewwi4mQvIirEdv+8=
github.com/disintegration/imaging v1.6.2 h1:w1LecBlG2Lnp8B3jk5zSuNqd7b4DXhcjwek1ei82L+c=
github.com/disintegration/imaging v1.6.2/go.mod h1:44/5580QXChDfwIclfc/PCwrr44amcmDAg8hxG0Ewe4=
github.com/dlclark/regexp2 v1.2.0 h1:8sAhBGEM0dRWogWqWyQeIJnxjWO6oIjl8FKqREDsGfk=
github.com/dlclark/regexp2 v1.2.0/go.mod h1:2pZnwuY/m+8K6iRw6wQdMtk+rH5tNGR1i55kozfMjCc=
github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
......@@ -536,6 +538,8 @@ golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EH
golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
golang.org/x/image v0.0.0-20191009234506-e7c1f5e7dbb8 h1:hVwzHzIUGRjiF7EcUjqNxk3NCfkPxbDKRdnNE1Rpg0U=
golang.org/x/image v0.0.0-20191009234506-e7c1f5e7dbb8/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
golang.org/x/lint v0.0.0-20180702182130-06c8688daad7 h1:00BeQWmeaGazuOrq8Q5K5d3/cHaGuFrZzpaHBXfrsUA=
golang.org/x/lint v0.0.0-20180702182130-06c8688daad7/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
......
......@@ -8,6 +8,7 @@ import (
"net/http"
"os"
"os/exec"
"strconv"
"strings"
"sync/atomic"
"syscall"
......@@ -40,7 +41,10 @@ type processCounter struct {
 
var numScalerProcs processCounter
 
const maxImageScalerProcs = 100
const (
maxImageScalerProcs = 100
maxAllowedFileSizeBytes = 250 * 1000 // 250kB
)
 
// Images might be located remotely in object storage, in which case we need to stream
// it via http(s)
......@@ -88,7 +92,7 @@ func init() {
prometheus.MustRegister(imageResizeCompleted)
}
 
// This Injecter forks into graphicsmagick to resize an image identified by path or URL
// This Injecter forks into a dedicated scaler process to resize an image identified by path or URL
// and streams the resized image back to the client
func (r *resizer) Inject(w http.ResponseWriter, req *http.Request, paramsData string) {
start := time.Now()
......@@ -101,7 +105,7 @@ func (r *resizer) Inject(w http.ResponseWriter, req *http.Request, paramsData st
return
}
 
sourceImageReader, filesize, err := openSourceImage(params.Location)
sourceImageReader, fileSize, err := openSourceImage(params.Location)
if err != nil {
// This means we cannot even read the input image; fail fast.
helper.Fail500(w, req, fmt.Errorf("ImageResizer: Failed opening image data stream: %v", err))
......@@ -115,13 +119,13 @@ func (r *resizer) Inject(w http.ResponseWriter, req *http.Request, paramsData st
"duration_s": time.Since(start).Seconds(),
"target_width": params.Width,
"content_type": params.ContentType,
"original_filesize": filesize,
"original_filesize": fileSize,
}
}
 
// We first attempt to rescale the image; if this should fail for any reason, we
// simply fail over to rendering out the original image unchanged.
imageReader, resizeCmd, err := tryResizeImage(req, sourceImageReader, logger.Writer(), params)
// We first attempt to rescale the image; if this should fail for any reason, imageReader
// will point to the original image, i.e. we render it unchanged.
imageReader, resizeCmd, err := tryResizeImage(req, sourceImageReader, logger.Writer(), params, fileSize)
if err != nil {
// something failed, but we can still write out the original image, do don't return early
helper.LogErrorWithFields(req, err, *logFields(0))
......@@ -130,24 +134,39 @@ func (r *resizer) Inject(w http.ResponseWriter, req *http.Request, paramsData st
imageResizeCompleted.Inc()
 
w.Header().Del("Content-Length")
bytesWritten, err := io.Copy(w, imageReader)
bytesWritten, err := serveImage(imageReader, w, resizeCmd)
if err != nil {
handleFailedCommand(w, req, bytesWritten, err, logFields(bytesWritten))
} else if err = resizeCmd.Wait(); err != nil {
handleFailedCommand(w, req, bytesWritten, err, logFields(bytesWritten))
} else {
logger.WithFields(*logFields(bytesWritten)).Printf("ImageResizer: Success")
return
}
logger.WithFields(*logFields(bytesWritten)).Printf("ImageResizer: Success")
}
 
func handleFailedCommand(w http.ResponseWriter, req *http.Request, bytesWritten int64, err error, logFields *log.Fields) {
// Streams image data from the given reader to the given writer and returns the number of bytes written.
// Errors are either served to the caller or merely logged, depending on whether any image data had
// already been transmitted or not.
func serveImage(r io.Reader, w io.Writer, resizeCmd *exec.Cmd) (int64, error) {
bytesWritten, err := io.Copy(w, r)
if err != nil {
if bytesWritten <= 0 {
helper.Fail500(w, req, err)
} else {
helper.LogErrorWithFields(req, err, *logFields)
return bytesWritten, err
}
if resizeCmd != nil {
if err = resizeCmd.Wait(); err != nil {
return bytesWritten, err
}
}
return bytesWritten, nil
}
func handleFailedCommand(w http.ResponseWriter, req *http.Request, bytesWritten int64, err error, logFields *log.Fields) {
if bytesWritten <= 0 {
helper.Fail500(w, req, err)
} else {
helper.LogErrorWithFields(req, err, *logFields)
}
}
 
func (r *resizer) unpackParameters(paramsData string) (*resizeParams, error) {
......@@ -161,14 +180,18 @@ func (r *resizer) unpackParameters(paramsData string) (*resizeParams, error) {
}
 
if params.ContentType == "" {
return nil, fmt.Errorf("ImageResizer: Image MIME type must be set")
return nil, fmt.Errorf("ImageResizer: ContentType must be set")
}
 
return &params, nil
}
 
// Attempts to rescale the given image data, or in case of errors, falls back to the original image.
func tryResizeImage(req *http.Request, r io.Reader, errorWriter io.Writer, params *resizeParams) (io.Reader, *exec.Cmd, error) {
func tryResizeImage(req *http.Request, r io.Reader, errorWriter io.Writer, params *resizeParams, fileSize int64) (io.Reader, *exec.Cmd, error) {
if fileSize > maxAllowedFileSizeBytes {
return r, nil, fmt.Errorf("ImageResizer: %db exceeds maximum file size of %db", fileSize, maxAllowedFileSizeBytes)
}
if !numScalerProcs.tryIncrement() {
return r, nil, fmt.Errorf("ImageResizer: too many running scaler processes")
}
......@@ -179,35 +202,22 @@ func tryResizeImage(req *http.Request, r io.Reader, errorWriter io.Writer, param
numScalerProcs.decrement()
}()
 
width := params.Width
gmFileSpec := determineFilePrefix(params.ContentType)
if gmFileSpec == "" {
return r, nil, fmt.Errorf("ImageResizer: unexpected MIME type: %s", params.ContentType)
}
resizeCmd, resizedImageReader, err := startResizeImageCommand(ctx, r, errorWriter, width, gmFileSpec)
resizeCmd, resizedImageReader, err := startResizeImageCommand(ctx, r, errorWriter, params)
if err != nil {
return r, nil, fmt.Errorf("ImageResizer: failed forking into graphicsmagick")
return r, nil, fmt.Errorf("ImageResizer: failed forking into scaler process: %w", err)
}
return resizedImageReader, resizeCmd, nil
}
 
func determineFilePrefix(contentType string) string {
switch contentType {
case "image/png":
return "png:"
case "image/jpeg":
return "jpg:"
default:
return ""
}
}
func startResizeImageCommand(ctx context.Context, imageReader io.Reader, errorWriter io.Writer, width uint, gmFileSpec string) (*exec.Cmd, io.ReadCloser, error) {
cmd := exec.CommandContext(ctx, "gm", "convert", "-resize", fmt.Sprintf("%dx", width), gmFileSpec+"-", "-")
func startResizeImageCommand(ctx context.Context, imageReader io.Reader, errorWriter io.Writer, params *resizeParams) (*exec.Cmd, io.ReadCloser, error) {
cmd := exec.CommandContext(ctx, "gitlab-resize-image")
cmd.Stdin = imageReader
cmd.Stderr = errorWriter
cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
cmd.Env = []string{
"GL_RESIZE_IMAGE_WIDTH=" + strconv.Itoa(int(params.Width)),
"GL_RESIZE_IMAGE_CONTENT_TYPE=" + params.ContentType,
}
 
stdout, err := cmd.StdoutPipe()
if err != nil {
......@@ -227,13 +237,13 @@ func isURL(location string) bool {
 
func openSourceImage(location string) (io.ReadCloser, int64, error) {
if isURL(location) {
return openFromUrl(location)
return openFromURL(location)
}
 
return openFromFile(location)
}
 
func openFromUrl(location string) (io.ReadCloser, int64, error) {
func openFromURL(location string) (io.ReadCloser, int64, error) {
res, err := httpClient.Get(location)
if err != nil {
return nil, 0, err
......
package imageresizer
 
import (
"bytes"
"encoding/base64"
"encoding/json"
"net/http"
"os"
"testing"
 
"gitlab.com/gitlab-org/labkit/log"
"github.com/stretchr/testify/require"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/testhelper"
)
 
var r = resizer{}
 
func TestMain(m *testing.M) {
if err := testhelper.BuildExecutables(); err != nil {
log.WithError(err).Fatal()
}
os.Exit(m.Run())
}
func TestUnpackParametersReturnsParamsInstanceForValidInput(t *testing.T) {
inParams := resizeParams{Location: "/path/to/img", Width: 64, ContentType: "image/png"}
 
......@@ -37,19 +50,13 @@ func TestUnpackParametersReturnsErrorWhenContentTypeBlank(t *testing.T) {
require.Error(t, err, "expected error when ContentType is blank")
}
 
func TestDetermineFilePrefixFromMimeType(t *testing.T) {
require.Equal(t, "png:", determineFilePrefix("image/png"))
require.Equal(t, "jpg:", determineFilePrefix("image/jpeg"))
require.Equal(t, "", determineFilePrefix("unsupported"))
}
func TestTryResizeImageSuccess(t *testing.T) {
inParams := resizeParams{Location: "/path/to/img", Width: 64, ContentType: "image/png"}
inFile := testImage(t)
req, err := http.NewRequest("GET", "/foo", nil)
require.NoError(t, err)
 
reader, cmd, err := tryResizeImage(req, inFile, os.Stderr, &inParams)
reader, cmd, err := tryResizeImage(req, inFile, os.Stderr, &inParams, maxAllowedFileSizeBytes)
 
require.NoError(t, err)
require.NotNil(t, cmd)
......@@ -57,29 +64,40 @@ func TestTryResizeImageSuccess(t *testing.T) {
require.NotEqual(t, inFile, reader)
}
 
func TestTryResizeImageFailsOverToOriginalImageWhenContentTypeNotSupported(t *testing.T) {
inParams := resizeParams{Location: "/path/to/img", Width: 64, ContentType: "not supported"}
func TestTryResizeImageSkipsResizeWhenSourceImageTooLarge(t *testing.T) {
inParams := resizeParams{Location: "/path/to/img", Width: 64, ContentType: "image/png"}
inFile := testImage(t)
req, err := http.NewRequest("GET", "/foo", nil)
require.NoError(t, err)
 
reader, cmd, err := tryResizeImage(req, inFile, os.Stderr, &inParams)
reader, cmd, err := tryResizeImage(req, inFile, os.Stderr, &inParams, maxAllowedFileSizeBytes+1)
 
require.Error(t, err)
require.Nil(t, cmd)
require.Equal(t, inFile, reader)
require.Equal(t, inFile, reader, "Expected output streams to match")
}
 
func TestGraphicsMagickFailsWhenContentTypeNotMatchingFileContents(t *testing.T) {
func TestTryResizeImageFailsWhenContentTypeNotMatchingFileContents(t *testing.T) {
inParams := resizeParams{Location: "/path/to/img", Width: 64, ContentType: "image/jpeg"}
inFile := testImage(t) // this is PNG file; gm should fail fast in this case
inFile := testImage(t) // this is a PNG file; the image scaler should fail fast in this case
req, err := http.NewRequest("GET", "/foo", nil)
require.NoError(t, err)
 
_, cmd, err := tryResizeImage(req, inFile, os.Stderr, &inParams)
_, cmd, err := tryResizeImage(req, inFile, os.Stderr, &inParams, maxAllowedFileSizeBytes)
require.NoError(t, err)
require.Error(t, cmd.Wait(), "Expected to fail due to content-type mismatch")
}
func TestServeImage(t *testing.T) {
inFile := testImage(t)
var writer bytes.Buffer
bytesWritten, err := serveImage(inFile, &writer, nil)
 
require.NoError(t, err)
require.Error(t, cmd.Wait())
require.Greater(t, bytesWritten, int64(0))
require.Equal(t, int64(len(writer.Bytes())), bytesWritten)
}
 
// The Rails applications sends a Base64 encoded JSON string carrying
......
......@@ -74,7 +74,7 @@ func TestServerWithHandler(url *regexp.Regexp, handler http.HandlerFunc) *httpte
}))
}
 
var workhorseExecutables = []string{"gitlab-workhorse", "gitlab-zip-cat", "gitlab-zip-metadata"}
var workhorseExecutables = []string{"gitlab-workhorse", "gitlab-zip-cat", "gitlab-zip-metadata", "gitlab-resize-image"}
 
func BuildExecutables() error {
rootDir := RootDir()
......
......@@ -377,7 +377,7 @@ func TestImageResizing(t *testing.T) {
 
resp, body, err := doSendDataRequest(resourcePath, "send-scaled-img", jsonParams)
require.NoError(t, err, "send resize request")
require.Equal(t, 200, resp.StatusCode, "GET %q: status code", resourcePath)
require.Equal(t, 200, resp.StatusCode, "GET %q: body: %s", resourcePath, body)
 
img, err := png.Decode(bytes.NewReader(body))
require.NoError(t, err, "decode resized image")
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment