Repository: gotenberg/gotenberg (go)

Issue:
Configure listen IP
For now, we can only set --api-port (or via the env var), but this only allows to set the port, not the listen IP, and gotenberg always listen on 0.0.0.0.
It'd be useful to configure the IP on which the API listen. One use case I have is running gotenberg on a Nomad cluster, and exposing it through the Consul service mesh. When using the mesh, all the security relies on the fact that services are only listening on 127.0.0.1, so they can be accessed though an envoy proxy in the same allocation (= pod in Nomad's terminology).
With gotenberg listening on 0.0.0.0, random other workloads which are happening to run on the same host can access gotenberg, bypassing the service mesh, and the ACL applied by the mesh.

Pull request:
feat(api): add flag --api-bind-ip
Closes #1008.

Patch:
diff --git a/Makefile b/Makefile
index 2b3c23b..67529de 100644
--- a/Makefile
+++ b/Makefile
@@ -32,6 +32,7 @@ build: ## Build the Gotenberg's Docker image
 GOTENBERG_GRACEFUL_SHUTDOWN_DURATION=30s
 API_PORT=3000
 API_PORT_FROM_ENV=
+API_BIND_IP=
 API_START_TIMEOUT=30s
 API_TIMEOUT=30s
 API_BODY_LIMIT=
@@ -97,6 +98,7 @@ run: ## Start a Gotenberg container
 	--gotenberg-graceful-shutdown-duration=$(GOTENBERG_GRACEFUL_SHUTDOWN_DURATION) \
 	--api-port=$(API_PORT) \
 	--api-port-from-env=$(API_PORT_FROM_ENV) \
+	--api-bind-ip=$(API_BIND_IP) \
 	--api-start-timeout=$(API_START_TIMEOUT) \
 	--api-timeout=$(API_TIMEOUT) \
 	--api-body-limit="$(API_BODY_LIMIT)" \
@@ -104,9 +106,9 @@ run: ## Start a Gotenberg container
 	--api-trace-header=$(API_TRACE_HEADER) \
 	--api-enable-basic-auth=$(API_ENABLE_BASIC_AUTH) \
 	--api-download-from-allow-list=$(API-DOWNLOAD-FROM-ALLOW-LIST) \
-    --api-download-from-deny-list=$(API-DOWNLOAD-FROM-DENY-LIST) \
-    --api-download-from-max-retry=$(API-DOWNLOAD-FROM-FROM-MAX-RETRY) \
-    --api-disable-download-from=$(API-DISABLE-DOWNLOAD-FROM) \
+	--api-download-from-deny-list=$(API-DOWNLOAD-FROM-DENY-LIST) \
+	--api-download-from-max-retry=$(API-DOWNLOAD-FROM-FROM-MAX-RETRY) \
+	--api-disable-download-from=$(API-DISABLE-DOWNLOAD-FROM) \
 	--api-disable-health-check-logging=$(API_DISABLE_HEALTH_CHECK_LOGGING) \
 	--chromium-restart-after=$(CHROMIUM_RESTART_AFTER) \
 	--chromium-auto-start=$(CHROMIUM_AUTO_START) \
diff --git a/go.mod b/go.mod
index cc3688e..837d542 100644
--- a/go.mod
+++ b/go.mod
@@ -59,5 +59,5 @@ require (
 	github.com/valyala/fasttemplate v1.2.2 // indirect
 	github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8 // indirect
 	golang.org/x/time v0.7.0 // indirect
-	google.golang.org/protobuf v1.34.2 // indirect
+	google.golang.org/protobuf v1.35.1 // indirect
 )
diff --git a/go.sum b/go.sum
index 0e05859..6412b97 100644
--- a/go.sum
+++ b/go.sum
@@ -145,7 +145,7 @@ golang.org/x/text v0.19.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY=
 golang.org/x/time v0.7.0 h1:ntUhktv3OPE6TgYxXWv9vKvUSJyIFJlyohwbkEwPrKQ=
 golang.org/x/time v0.7.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
-google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6hg=
-google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWniOlNbLDw=
+google.golang.org/protobuf v1.35.1 h1:m3LfL6/Ca+fqnjnlqQXNpFPABW1UD7mjh8KO2mKFytA=
+google.golang.org/protobuf v1.35.1/go.mod h1:9fA7Ob0pmnwhb644+1+CVWFRbNajQ6iRojtC/QF5bRE=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
diff --git a/pkg/modules/api/api.go b/pkg/modules/api/api.go
index 227e7ea..3d6cb45 100644
--- a/pkg/modules/api/api.go
+++ b/pkg/modules/api/api.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net"
 	"net/http"
 	"sort"
 	"strings"
@@ -29,6 +30,7 @@ func init() {
 // middlewares or health checks.
 type Api struct {
 	port                      int
+	bindIp                    string
 	tlsCertFile               string
 	tlsKeyFile                string
 	startTimeout              time.Duration
@@ -171,11 +173,12 @@ func (a *Api) Descriptor() gotenberg.ModuleDescriptor {
 			fs := flag.NewFlagSet("api", flag.ExitOnError)
 			fs.Int("api-port", 3000, "Set the port on which the API should listen")
 			fs.String("api-port-from-env", "", "Set the environment variable with the port on which the API should listen - override the default port")
+			fs.String("api-bind-ip", "", "Set the IP address the API should bind to for incoming connections")
 			fs.String("api-tls-cert-file", "", "Path to the TLS/SSL certificate file - for HTTPS support")
 			fs.String("api-tls-key-file", "", "Path to the TLS/SSL key file - for HTTPS support")
 			fs.Duration("api-start-timeout", time.Duration(30)*time.Second, "Set the time limit for the API to start")
 			fs.Duration("api-timeout", time.Duration(30)*time.Second, "Set the time limit for requests")
-			fs.String("api-body-limit", "", "Set the body limit for multipart/form-data requests")
+			fs.String("api-body-limit", "", "Set the body limit for multipart/form-data requests - it accepts values like 5MB, 1GB, etc")
 			fs.String("api-root-path", "/", "Set the root path of the API - for service discovery via URL paths")
 			fs.String("api-trace-header", "Gotenberg-Trace", "Set the header name to use for identifying requests")
 			fs.Bool("api-enable-basic-auth", false, "Enable basic authentication - will look for the GOTENBERG_API_BASIC_AUTH_USERNAME and GOTENBERG_API_BASIC_AUTH_PASSWORD environment variables")
@@ -194,6 +197,7 @@ func (a *Api) Descriptor() gotenberg.ModuleDescriptor {
 func (a *Api) Provision(ctx *gotenberg.Context) error {
 	flags := ctx.ParsedFlags()
 	a.port = flags.MustInt("api-port")
+	a.bindIp = flags.MustString("api-bind-ip")
 	a.tlsCertFile = flags.MustString("api-tls-cert-file")
 	a.tlsKeyFile = flags.MustString("api-tls-key-file")
 	a.startTimeout = flags.MustDuration("api-start-timeout")
@@ -329,6 +333,10 @@ func (a *Api) Validate() error {
 		)
 	}
 
+	if a.bindIp != "" && net.ParseIP(a.bindIp) == nil {
+		err = multierr.Append(err, errors.New("IP must be a valid IP address"))
+	}
+
 	if (a.tlsCertFile != "" && a.tlsKeyFile == "") || (a.tlsCertFile == "" && a.tlsKeyFile != "") {
 		err = multierr.Append(err,
 			errors.New("both TLS certificate and key files must be set"),
@@ -522,11 +530,11 @@ func (a *Api) Start() error {
 		var err error
 		if a.tlsCertFile != "" && a.tlsKeyFile != "" {
 			// Start an HTTPS server (supports HTTP/2).
-			err = a.srv.StartTLS(fmt.Sprintf(":%d", a.port), a.tlsCertFile, a.tlsKeyFile)
+			err = a.srv.StartTLS(fmt.Sprintf("%s:%d", a.bindIp, a.port), a.tlsCertFile, a.tlsKeyFile)
 		} else {
 			// Start an HTTP/2 Cleartext (non-HTTPS) server.
 			server := &http2.Server{}
-			err = a.srv.StartH2CServer(fmt.Sprintf(":%d", a.port), server)
+			err = a.srv.StartH2CServer(fmt.Sprintf("%s:%d", a.bindIp, a.port), server)
 		}
 		if !errors.Is(err, http.ErrServerClosed) {
 			a.logger.Fatal(err.Error())
@@ -538,7 +546,11 @@ func (a *Api) Start() error {
 
 // StartupMessage returns a custom startup message.
 func (a *Api) StartupMessage() string {
-	return fmt.Sprintf("server listening on port %d", a.port)
+	ip := a.bindIp
+	if a.bindIp == "" {
+		ip = "[::]"
+	}
+	return fmt.Sprintf("server started on %s:%d", ip, a.port)
 }
 
 // Stop stops the HTTP server.


Test patch:
diff --git a/pkg/modules/api/api_test.go b/pkg/modules/api/api_test.go
index 8e8a461..885076a 100644
--- a/pkg/modules/api/api_test.go
+++ b/pkg/modules/api/api_test.go
@@ -58,10 +58,10 @@ func TestApi_Provision(t *testing.T) {
 			expectError: true,
 		},
 		{
-			scenario: "basic auth: non-existing GOTENBERG_API_BASIC_AUTH_USERNAME environment variable",
+			scenario: "port from env: invalid environment variable value",
 			ctx: func() *gotenberg.Context {
 				fs := new(Api).Descriptor().FlagSet
-				err := fs.Parse([]string{"--api-enable-basic-auth=true"})
+				err := fs.Parse([]string{"--api-port-from-env=PORT"})
 				if err != nil {
 					t.Fatalf("expected no error but got: %v", err)
 				}
@@ -73,10 +73,16 @@ func TestApi_Provision(t *testing.T) {
 					nil,
 				)
 			}(),
+			setEnv: func() {
+				err := os.Setenv("PORT", "foo")
+				if err != nil {
+					t.Fatalf("expected no error but got: %v", err)
+				}
+			},
 			expectError: true,
 		},
 		{
-			scenario: "basic auth: non-existing GOTENBERG_API_BASIC_AUTH_PASSWORD environment variable",
+			scenario: "basic auth: non-existing GOTENBERG_API_BASIC_AUTH_USERNAME environment variable",
 			ctx: func() *gotenberg.Context {
 				fs := new(Api).Descriptor().FlagSet
 				err := fs.Parse([]string{"--api-enable-basic-auth=true"})
@@ -91,19 +97,13 @@ func TestApi_Provision(t *testing.T) {
 					nil,
 				)
 			}(),
-			setEnv: func() {
-				err := os.Setenv("GOTENBERG_API_BASIC_AUTH_USERNAME", "foo")
-				if err != nil {
-					t.Fatalf("expected no error but got: %v", err)
-				}
-			},
 			expectError: true,
 		},
 		{
-			scenario: "port from env: invalid environment variable value",
+			scenario: "basic auth: non-existing GOTENBERG_API_BASIC_AUTH_PASSWORD environment variable",
 			ctx: func() *gotenberg.Context {
 				fs := new(Api).Descriptor().FlagSet
-				err := fs.Parse([]string{"--api-port-from-env=PORT"})
+				err := fs.Parse([]string{"--api-enable-basic-auth=true"})
 				if err != nil {
 					t.Fatalf("expected no error but got: %v", err)
 				}
@@ -116,7 +116,7 @@ func TestApi_Provision(t *testing.T) {
 				)
 			}(),
 			setEnv: func() {
-				err := os.Setenv("PORT", "foo")
+				err := os.Setenv("GOTENBERG_API_BASIC_AUTH_USERNAME", "foo")
 				if err != nil {
 					t.Fatalf("expected no error but got: %v", err)
 				}
@@ -462,6 +462,7 @@ func TestApi_Validate(t *testing.T) {
 	for _, tc := range []struct {
 		scenario    string
 		port        int
+		bindIp      string
 		tlsCertFile string
 		tlsKeyFile  string
 		rootPath    string
@@ -473,6 +474,7 @@ func TestApi_Validate(t *testing.T) {
 		{
 			scenario:    "invalid port (< 1)",
 			port:        0,
+			bindIp:      "127.0.0.1",
 			rootPath:    "/foo/",
 			traceHeader: "foo",
 			routes:      nil,
@@ -482,6 +484,17 @@ func TestApi_Validate(t *testing.T) {
 		{
 			scenario:    "invalid port (> 65535)",
 			port:        65536,
+			bindIp:      "127.0.0.1",
+			rootPath:    "/foo/",
+			traceHeader: "foo",
+			routes:      nil,
+			middlewares: nil,
+			expectError: true,
+		},
+		{
+			scenario:    "invalid IP",
+			port:        10,
+			bindIp:      "foo",
 			rootPath:    "/foo/",
 			traceHeader: "foo",
 			routes:      nil,
@@ -491,6 +504,7 @@ func TestApi_Validate(t *testing.T) {
 		{
 			scenario:    "invalid TLS files: only cert file provided",
 			port:        10,
+			bindIp:      "127.0.0.1",
 			tlsCertFile: "cert.pem",
 			rootPath:    "/foo/",
 			traceHeader: "foo",
@@ -501,6 +515,7 @@ func TestApi_Validate(t *testing.T) {
 		{
 			scenario:    "invalid TLS files: only key file provided",
 			port:        10,
+			bindIp:      "127.0.0.1",
 			tlsKeyFile:  "key.pem",
 			rootPath:    "/foo/",
 			traceHeader: "foo",
@@ -511,6 +526,7 @@ func TestApi_Validate(t *testing.T) {
 		{
 			scenario:    "invalid root path: missing / prefix",
 			port:        10,
+			bindIp:      "127.0.0.1",
 			rootPath:    "foo/",
 			traceHeader: "foo",
 			routes:      nil,
@@ -520,6 +536,7 @@ func TestApi_Validate(t *testing.T) {
 		{
 			scenario:    "invalid root path: missing / suffix",
 			port:        10,
+			bindIp:      "127.0.0.1",
 			rootPath:    "/foo",
 			traceHeader: "foo",
 			routes:      nil,
@@ -529,6 +546,7 @@ func TestApi_Validate(t *testing.T) {
 		{
 			scenario:    "invalid trace header",
 			port:        10,
+			bindIp:      "127.0.0.1",
 			rootPath:    "/foo/",
 			traceHeader: "",
 			routes:      nil,
@@ -538,6 +556,7 @@ func TestApi_Validate(t *testing.T) {
 		{
 			scenario:    "invalid route: empty path",
 			port:        10,
+			bindIp:      "127.0.0.1",
 			rootPath:    "/foo/",
 			traceHeader: "foo",
 			routes: []Route{
@@ -551,6 +570,7 @@ func TestApi_Validate(t *testing.T) {
 		{
 			scenario:    "invalid route: missing / prefix in path",
 			port:        10,
+			bindIp:      "127.0.0.1",
 			rootPath:    "/foo/",
 			traceHeader: "foo",
 			routes: []Route{
@@ -564,6 +584,7 @@ func TestApi_Validate(t *testing.T) {
 		{
 			scenario:    "invalid multipart route: no /forms prefix in path",
 			port:        10,
+			bindIp:      "127.0.0.1",
 			rootPath:    "/foo/",
 			traceHeader: "foo",
 			routes: []Route{
@@ -578,6 +599,7 @@ func TestApi_Validate(t *testing.T) {
 		{
 			scenario:    "invalid route: no method",
 			port:        10,
+			bindIp:      "127.0.0.1",
 			rootPath:    "/foo/",
 			traceHeader: "foo",
 			routes: []Route{
@@ -592,6 +614,7 @@ func TestApi_Validate(t *testing.T) {
 		{
 			scenario:    "invalid route: nil handler",
 			port:        10,
+			bindIp:      "127.0.0.1",
 			rootPath:    "/foo/",
 			traceHeader: "foo",
 			routes: []Route{
@@ -607,6 +630,7 @@ func TestApi_Validate(t *testing.T) {
 		{
 			scenario:    "invalid route: path already existing",
 			port:        10,
+			bindIp:      "127.0.0.1",
 			rootPath:    "/foo/",
 			traceHeader: "foo",
 			routes: []Route{
@@ -627,6 +651,7 @@ func TestApi_Validate(t *testing.T) {
 		{
 			scenario:    "invalid middleware: nil handler",
 			port:        10,
+			bindIp:      "127.0.0.1",
 			rootPath:    "/foo/",
 			traceHeader: "foo",
 			routes:      nil,
@@ -641,6 +666,7 @@ func TestApi_Validate(t *testing.T) {
 		{
 			scenario:    "success",
 			port:        10,
+			bindIp:      "127.0.0.1",
 			rootPath:    "/foo/",
 			traceHeader: "foo",
 			routes: []Route{
@@ -694,6 +720,7 @@ func TestApi_Validate(t *testing.T) {
 		t.Run(tc.scenario, func(t *testing.T) {
 			mod := Api{
 				port:                tc.port,
+				bindIp:              tc.bindIp,
 				tlsCertFile:         tc.tlsCertFile,
 				tlsKeyFile:          tc.tlsKeyFile,
 				rootPath:            tc.rootPath,
@@ -918,15 +945,36 @@ func TestApi_Start(t *testing.T) {
 }
 
 func TestApi_StartupMessage(t *testing.T) {
-	mod := Api{
-		port: 3000,
-	}
-
-	actual := mod.StartupMessage()
-	expect := "server listening on port 3000"
+	for _, tc := range []struct {
+		scenario      string
+		port          int
+		bindIp        string
+		expectMessage string
+	}{
+		{
+			scenario:      "no custom IP",
+			port:          3000,
+			bindIp:        "",
+			expectMessage: "server started on [::]:3000",
+		},
+		{
+			scenario:      "custom IP",
+			port:          3000,
+			bindIp:        "127.0.0.1",
+			expectMessage: "server started on 127.0.0.1:3000",
+		},
+	} {
+		t.Run(tc.scenario, func(t *testing.T) {
+			mod := Api{
+				port:   tc.port,
+				bindIp: tc.bindIp,
+			}
 
-	if actual != expect {
-		t.Errorf("expected '%s' but got '%s'", expect, actual)
+			actual := mod.StartupMessage()
+			if actual != tc.expectMessage {
+				t.Errorf("expected '%s' but got '%s'", tc.expectMessage, actual)
+			}
+		})
 	}
 }