Repository: mccutchen/go-httpbin
Pull request #189 (2024-09-21 08:36:40)

feat: add /env endpoint to allow exposing operator-controlled info from the server
Proposal of implementation of the feature request
- #114

----

I've submitted this for an initial round of review. 
I still have to add more cases to the test and add the endpoint to the index page, but first I'd like to receive feedback if this implementation goes in the right direction.

----

```bash
make run

curl http://127.0.0.1:8080/env
{
  "env": {}
}

export HTTPBIN_A=a
export HTTPBIN_B=b
export HTTPBIN_123=123

make run

curl http://127.0.0.1:8080/env
{
  "env": {
    "HTTPBIN_123": "123",
    "HTTPBIN_A": "a",
    "HTTPBIN_B": "b"
  }
}
```


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/mccutchen/go-httpbin/pull/189?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Will+McCutchen) Report
Attention: Patch coverage is `90.90909%` with `1 line` in your changes missing coverage. Please review.
> Project coverage is 95.17%. Comparing base [(`34a21a3`)](https://app.codecov.io/gh/mccutchen/go-httpbin/commit/34a21a3ef81df5aa42185fcae2e5a608404ac6a0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Will+McCutchen) to head [(`add87e1`)](https://app.codecov.io/gh/mccutchen/go-httpbin/commit/add87e1051dc924d73ac9286c1fb9d41882c6001?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Will+McCutchen).

| [Files with missing lines](https://app.codecov.io/gh/mccutchen/go-httpbin/pull/189?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Will+McCutchen) | Patch % | Lines |
|---|---|---|
| [httpbin/handlers.go](https://app.codecov.io/gh/mccutchen/go-httpbin/pull/189?src=pr&el=tree&filepath=httpbin%2Fhandlers.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Will+McCutchen#diff-aHR0cGJpbi9oYW5kbGVycy5nbw==) | 90.00% | [1 Missing :warning: ](https://app.codecov.io/gh/mccutchen/go-httpbin/pull/189?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Will+McCutchen) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #189      +/-   ##
==========================================
- Coverage   95.20%   95.17%   -0.03%     
==========================================
  Files          10       10              
  Lines        1750     1761      +11     
==========================================
+ Hits         1666     1676      +10     
- Misses         48       49       +1     
  Partials       36       36              
```

| [Files with missing lines](https://app.codecov.io/gh/mccutchen/go-httpbin/pull/189?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Will+McCutchen) | Coverage Δ | |
|---|---|---|
| [httpbin/httpbin.go](https://app.codecov.io/gh/mccutchen/go-httpbin/pull/189?src=pr&el=tree&filepath=httpbin%2Fhttpbin.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Will+McCutchen#diff-aHR0cGJpbi9odHRwYmluLmdv) | `100.00% <100.00%> (ø)` | |
| [httpbin/handlers.go](https://app.codecov.io/gh/mccutchen/go-httpbin/pull/189?src=pr&el=tree&filepath=httpbin%2Fhandlers.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Will+McCutchen#diff-aHR0cGJpbi9oYW5kbGVycy5nbw==) | `99.11% <90.00%> (-0.12%)` | :arrow_down: |

</details>
mloskot: @mccutchen 
> I think I'd prefer a slightly different approach where the environment is resolved at process startup and passed explicitly to `httpbin.New()` via a new `httpbin.WithEnv(env map[string]string)` option func.

This is a very good suggestion indeed. I will update this PR with the process startup approach.


mloskot: @mccutchen I've refatored and the `HTTPBIN_` vars are now collected at the startup. I'll welcome any feedback - I'm available to implement any suggestions you may have.

Patch:
diff --git a/httpbin/cmd/cmd.go b/httpbin/cmd/cmd.go
index 08a9846..cdda27b 100644
--- a/httpbin/cmd/cmd.go
+++ b/httpbin/cmd/cmd.go
@@ -25,6 +25,7 @@ const (
 	defaultListenHost = "0.0.0.0"
 	defaultListenPort = 8080
 	defaultLogFormat  = "text"
+	defaultEnvPrefix  = "HTTPBIN_ENV_"
 
 	// Reasonable defaults for our http server
 	srvReadTimeout       = 5 * time.Second
@@ -35,13 +36,13 @@ const (
 // Main is the main entrypoint for the go-httpbin binary. See loadConfig() for
 // command line argument parsing.
 func Main() int {
-	return mainImpl(os.Args[1:], os.Getenv, os.Hostname, os.Stderr)
+	return mainImpl(os.Args[1:], os.Getenv, os.Environ, os.Hostname, os.Stderr)
 }
 
 // mainImpl is the real implementation of Main(), extracted for better
 // testability.
-func mainImpl(args []string, getEnv func(string) string, getHostname func() (string, error), out io.Writer) int {
-	cfg, err := loadConfig(args, getEnv, getHostname)
+func mainImpl(args []string, getEnvVal func(string) string, getEnviron func() []string, getHostname func() (string, error), out io.Writer) int {
+	cfg, err := loadConfig(args, getEnvVal, getEnviron, getHostname)
 	if err != nil {
 		if cfgErr, ok := err.(ConfigError); ok {
 			// for -h/-help, just print usage and exit without error
@@ -75,6 +76,7 @@ func mainImpl(args []string, getEnv func(string) string, getHostname func() (str
 	}
 
 	opts := []httpbin.OptionFunc{
+		httpbin.WithEnv(cfg.Env),
 		httpbin.WithMaxBodySize(cfg.MaxBodySize),
 		httpbin.WithMaxDuration(cfg.MaxDuration),
 		httpbin.WithObserver(httpbin.StdLogObserver(logger)),
@@ -110,6 +112,7 @@ func mainImpl(args []string, getEnv func(string) string, getHostname func() (str
 // config holds the configuration needed to initialize and run go-httpbin as a
 // standalone server.
 type config struct {
+	Env                    map[string]string
 	AllowedRedirectDomains []string
 	ListenHost             string
 	ExcludeHeaders         string
@@ -144,7 +147,7 @@ func (e ConfigError) Error() string {
 
 // loadConfig parses command line arguments and env vars into a fully resolved
 // Config struct. Command line arguments take precedence over env vars.
-func loadConfig(args []string, getEnv func(string) string, getHostname func() (string, error)) (*config, error) {
+func loadConfig(args []string, getEnvVal func(string) string, getEnviron func() []string, getHostname func() (string, error)) (*config, error) {
 	cfg := &config{}
 
 	fs := flag.NewFlagSet("go-httpbin", flag.ContinueOnError)
@@ -192,24 +195,24 @@ func loadConfig(args []string, getEnv func(string) string, getHostname func() (s
 	// Command line flags take precedence over environment vars, so we only
 	// check for environment vars if we have default values for our command
 	// line flags.
-	if cfg.MaxBodySize == httpbin.DefaultMaxBodySize && getEnv("MAX_BODY_SIZE") != "" {
-		cfg.MaxBodySize, err = strconv.ParseInt(getEnv("MAX_BODY_SIZE"), 10, 64)
+	if cfg.MaxBodySize == httpbin.DefaultMaxBodySize && getEnvVal("MAX_BODY_SIZE") != "" {
+		cfg.MaxBodySize, err = strconv.ParseInt(getEnvVal("MAX_BODY_SIZE"), 10, 64)
 		if err != nil {
-			return nil, configErr("invalid value %#v for env var MAX_BODY_SIZE: parse error", getEnv("MAX_BODY_SIZE"))
+			return nil, configErr("invalid value %#v for env var MAX_BODY_SIZE: parse error", getEnvVal("MAX_BODY_SIZE"))
 		}
 	}
 
-	if cfg.MaxDuration == httpbin.DefaultMaxDuration && getEnv("MAX_DURATION") != "" {
-		cfg.MaxDuration, err = time.ParseDuration(getEnv("MAX_DURATION"))
+	if cfg.MaxDuration == httpbin.DefaultMaxDuration && getEnvVal("MAX_DURATION") != "" {
+		cfg.MaxDuration, err = time.ParseDuration(getEnvVal("MAX_DURATION"))
 		if err != nil {
-			return nil, configErr("invalid value %#v for env var MAX_DURATION: parse error", getEnv("MAX_DURATION"))
+			return nil, configErr("invalid value %#v for env var MAX_DURATION: parse error", getEnvVal("MAX_DURATION"))
 		}
 	}
-	if cfg.ListenHost == defaultListenHost && getEnv("HOST") != "" {
-		cfg.ListenHost = getEnv("HOST")
+	if cfg.ListenHost == defaultListenHost && getEnvVal("HOST") != "" {
+		cfg.ListenHost = getEnvVal("HOST")
 	}
 	if cfg.Prefix == "" {
-		if prefix := getEnv("PREFIX"); prefix != "" {
+		if prefix := getEnvVal("PREFIX"); prefix != "" {
 			cfg.Prefix = prefix
 		}
 	}
@@ -221,29 +224,29 @@ func loadConfig(args []string, getEnv func(string) string, getHostname func() (s
 			return nil, configErr("Prefix %#v must not end with a slash", cfg.Prefix)
 		}
 	}
-	if cfg.ExcludeHeaders == "" && getEnv("EXCLUDE_HEADERS") != "" {
-		cfg.ExcludeHeaders = getEnv("EXCLUDE_HEADERS")
+	if cfg.ExcludeHeaders == "" && getEnvVal("EXCLUDE_HEADERS") != "" {
+		cfg.ExcludeHeaders = getEnvVal("EXCLUDE_HEADERS")
 	}
-	if cfg.ListenPort == defaultListenPort && getEnv("PORT") != "" {
-		cfg.ListenPort, err = strconv.Atoi(getEnv("PORT"))
+	if cfg.ListenPort == defaultListenPort && getEnvVal("PORT") != "" {
+		cfg.ListenPort, err = strconv.Atoi(getEnvVal("PORT"))
 		if err != nil {
-			return nil, configErr("invalid value %#v for env var PORT: parse error", getEnv("PORT"))
+			return nil, configErr("invalid value %#v for env var PORT: parse error", getEnvVal("PORT"))
 		}
 	}
 
-	if cfg.TLSCertFile == "" && getEnv("HTTPS_CERT_FILE") != "" {
-		cfg.TLSCertFile = getEnv("HTTPS_CERT_FILE")
+	if cfg.TLSCertFile == "" && getEnvVal("HTTPS_CERT_FILE") != "" {
+		cfg.TLSCertFile = getEnvVal("HTTPS_CERT_FILE")
 	}
-	if cfg.TLSKeyFile == "" && getEnv("HTTPS_KEY_FILE") != "" {
-		cfg.TLSKeyFile = getEnv("HTTPS_KEY_FILE")
+	if cfg.TLSKeyFile == "" && getEnvVal("HTTPS_KEY_FILE") != "" {
+		cfg.TLSKeyFile = getEnvVal("HTTPS_KEY_FILE")
 	}
 	if cfg.TLSCertFile != "" || cfg.TLSKeyFile != "" {
 		if cfg.TLSCertFile == "" || cfg.TLSKeyFile == "" {
 			return nil, configErr("https cert and key must both be provided")
 		}
 	}
-	if cfg.LogFormat == defaultLogFormat && getEnv("LOG_FORMAT") != "" {
-		cfg.LogFormat = getEnv("LOG_FORMAT")
+	if cfg.LogFormat == defaultLogFormat && getEnvVal("LOG_FORMAT") != "" {
+		cfg.LogFormat = getEnvVal("LOG_FORMAT")
 	}
 	if cfg.LogFormat != "text" && cfg.LogFormat != "json" {
 		return nil, configErr(`invalid log format %q, must be "text" or "json"`, cfg.LogFormat)
@@ -252,7 +255,7 @@ func loadConfig(args []string, getEnv func(string) string, getHostname func() (s
 	// useRealHostname will be true if either the `-use-real-hostname`
 	// arg is given on the command line or if the USE_REAL_HOSTNAME env var
 	// is one of "1" or "true".
-	if useRealHostnameEnv := getEnv("USE_REAL_HOSTNAME"); useRealHostnameEnv == "1" || useRealHostnameEnv == "true" {
+	if useRealHostnameEnv := getEnvVal("USE_REAL_HOSTNAME"); useRealHostnameEnv == "1" || useRealHostnameEnv == "true" {
 		cfg.rawUseRealHostname = true
 	}
 	if cfg.rawUseRealHostname {
@@ -263,8 +266,8 @@ func loadConfig(args []string, getEnv func(string) string, getHostname func() (s
 	}
 
 	// split comma-separated list of domains into a slice, if given
-	if cfg.rawAllowedRedirectDomains == "" && getEnv("ALLOWED_REDIRECT_DOMAINS") != "" {
-		cfg.rawAllowedRedirectDomains = getEnv("ALLOWED_REDIRECT_DOMAINS")
+	if cfg.rawAllowedRedirectDomains == "" && getEnvVal("ALLOWED_REDIRECT_DOMAINS") != "" {
+		cfg.rawAllowedRedirectDomains = getEnvVal("ALLOWED_REDIRECT_DOMAINS")
 	}
 	for _, domain := range strings.Split(cfg.rawAllowedRedirectDomains, ",") {
 		if strings.TrimSpace(domain) != "" {
@@ -275,6 +278,18 @@ func loadConfig(args []string, getEnv func(string) string, getHostname func() (s
 	// reset temporary fields to their zero values
 	cfg.rawAllowedRedirectDomains = ""
 	cfg.rawUseRealHostname = false
+
+	for _, envVar := range getEnviron() {
+		name, value, _ := strings.Cut(envVar, "=")
+		if !strings.HasPrefix(name, defaultEnvPrefix) {
+			continue
+		}
+		if cfg.Env == nil {
+			cfg.Env = make(map[string]string)
+		}
+		cfg.Env[name] = value
+	}
+
 	return cfg, nil
 }
 
diff --git a/httpbin/handlers.go b/httpbin/handlers.go
index 16503f6..fbd3d46 100644
--- a/httpbin/handlers.go
+++ b/httpbin/handlers.go
@@ -35,6 +35,13 @@ func (h *HTTPBin) Index(w http.ResponseWriter, r *http.Request) {
 	writeHTML(w, h.indexHTML, http.StatusOK)
 }
 
+// Env - returns environment variables with HTTPBIN_ prefix, if any pre-configured by operator
+func (h *HTTPBin) Env(w http.ResponseWriter, _ *http.Request) {
+	writeJSON(http.StatusOK, w, &envResponse{
+		Env: h.env,
+	})
+}
+
 // FormsPost renders an HTML form that submits a request to the /post endpoint
 func (h *HTTPBin) FormsPost(w http.ResponseWriter, _ *http.Request) {
 	writeHTML(w, h.formsPostHTML, http.StatusOK)
diff --git a/httpbin/httpbin.go b/httpbin/httpbin.go
index 82ae6e6..7c60924 100644
--- a/httpbin/httpbin.go
+++ b/httpbin/httpbin.go
@@ -57,6 +57,10 @@ type HTTPBin struct {
 	// Set of hosts to which the /redirect-to endpoint will allow redirects
 	AllowedRedirectDomains map[string]struct{}
 
+	// The operator-controlled environment variables filtered from
+	// the process environment, based on named HTTPBIN_ prefix.
+	env map[string]string
+
 	// Pre-computed error message for the /redirect-to endpoint, based on
 	// -allowed-redirect-domains/ALLOWED_REDIRECT_DOMAINS
 	forbiddenRedirectError string
@@ -159,6 +163,7 @@ func (h *HTTPBin) Handler() http.Handler {
 	mux.HandleFunc("/digest-auth/{qop}/{user}/{password}/{algorithm}", h.DigestAuth)
 	mux.HandleFunc("/drip", h.Drip)
 	mux.HandleFunc("/dump/request", h.DumpRequest)
+	mux.HandleFunc("/env", h.Env)
 	mux.HandleFunc("/etag/{etag}", h.ETag)
 	mux.HandleFunc("/gzip", h.Gzip)
 	mux.HandleFunc("/headers", h.Headers)
diff --git a/httpbin/options.go b/httpbin/options.go
index e697a1c..15f6090 100644
--- a/httpbin/options.go
+++ b/httpbin/options.go
@@ -46,6 +46,14 @@ func WithObserver(o Observer) OptionFunc {
 	}
 }
 
+// WithEnv sets the HTTPBIN_-prefixed environment variables reported
+// by the /env endpoint.
+func WithEnv(env map[string]string) OptionFunc {
+	return func(h *HTTPBin) {
+		h.env = env
+	}
+}
+
 // WithExcludeHeaders sets the headers to exclude in outgoing responses, to
 // prevent possible information leakage.
 func WithExcludeHeaders(excludeHeaders string) OptionFunc {
diff --git a/httpbin/responses.go b/httpbin/responses.go
index 6b1e82b..d05d525 100644
--- a/httpbin/responses.go
+++ b/httpbin/responses.go
@@ -13,6 +13,10 @@ const (
 	textContentType   = "text/plain; charset=utf-8"
 )
 
+type envResponse struct {
+	Env map[string]string `json:"env"`
+}
+
 type headersResponse struct {
 	Headers http.Header `json:"headers"`
 }
diff --git a/httpbin/static/index.html.tmpl b/httpbin/static/index.html.tmpl
index d70030a..0cf8b77 100644
--- a/httpbin/static/index.html.tmpl
+++ b/httpbin/static/index.html.tmpl
@@ -82,6 +82,7 @@
 <li><a href="{{.Prefix}}/drip?code=200&amp;numbytes=5&amp;duration=5"><code>{{.Prefix}}/drip?numbytes=n&amp;duration=s&amp;delay=s&amp;code=code</code></a> Drips data over the given duration after an optional initial delay, simulating a slow HTTP server.</li>
 <li><a href="{{.Prefix}}/dump/request"><code>{{.Prefix}}/dump/request</code></a> Returns the given request in its HTTP/1.x wire approximate representation.</li>
 <li><a href="{{.Prefix}}/encoding/utf8"><code>{{.Prefix}}/encoding/utf8</code></a> Returns page containing UTF-8 data.</li>
+<li><a href="{{.Prefix}}/env"><code>{{.Prefix}}/env</code></a> Returns all environment variables named with <code>HTTPBIN_ENV_</code> prefix.</li>
 <li><a href="{{.Prefix}}/etag/etag"><code>{{.Prefix}}/etag/:etag</code></a> Assumes the resource has the given etag and responds to If-None-Match header with a 200 or 304 and If-Match with a 200 or 412 as appropriate.</li>
 <li><a href="{{.Prefix}}/forms/post"><code>{{.Prefix}}/forms/post</code></a> HTML form that submits to <em>{{.Prefix}}/post</em></li>
 <li><a href="{{.Prefix}}/get"><code>{{.Prefix}}/get</code></a> Returns GET data.</li>


Test patch:
diff --git a/httpbin/cmd/cmd_test.go b/httpbin/cmd/cmd_test.go
index a061987..ecf7065 100644
--- a/httpbin/cmd/cmd_test.go
+++ b/httpbin/cmd/cmd_test.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"errors"
 	"flag"
+	"fmt"
 	"os"
 	"reflect"
 	"testing"
@@ -77,6 +78,49 @@ func TestLoadConfig(t *testing.T) {
 			wantErr: flag.ErrHelp,
 		},
 
+		// env
+		"ok env with empty variables": {
+			env: map[string]string{},
+			wantCfg: &config{
+				Env:         nil,
+				ListenHost:  "0.0.0.0",
+				ListenPort:  8080,
+				MaxBodySize: httpbin.DefaultMaxBodySize,
+				MaxDuration: httpbin.DefaultMaxDuration,
+				LogFormat:   defaultLogFormat,
+			},
+		},
+		"ok env with recognized variables": {
+			env: map[string]string{
+				fmt.Sprintf("%sFOO", defaultEnvPrefix):                     "foo",
+				fmt.Sprintf("%s%sBAR", defaultEnvPrefix, defaultEnvPrefix): "bar",
+				fmt.Sprintf("%s123", defaultEnvPrefix):                     "123",
+			},
+			wantCfg: &config{
+				Env: map[string]string{
+					fmt.Sprintf("%sFOO", defaultEnvPrefix):                     "foo",
+					fmt.Sprintf("%s%sBAR", defaultEnvPrefix, defaultEnvPrefix): "bar",
+					fmt.Sprintf("%s123", defaultEnvPrefix):                     "123",
+				},
+				ListenHost:  "0.0.0.0",
+				ListenPort:  8080,
+				MaxBodySize: httpbin.DefaultMaxBodySize,
+				MaxDuration: httpbin.DefaultMaxDuration,
+				LogFormat:   defaultLogFormat,
+			},
+		},
+		"ok env with unrecognized variables": {
+			env: map[string]string{"HTTPBIN_FOO": "foo", "BAR": "bar"},
+			wantCfg: &config{
+				Env:         nil,
+				ListenHost:  "0.0.0.0",
+				ListenPort:  8080,
+				MaxBodySize: httpbin.DefaultMaxBodySize,
+				MaxDuration: httpbin.DefaultMaxDuration,
+				LogFormat:   defaultLogFormat,
+			},
+		},
+
 		// max body size
 		"invalid -max-body-size": {
 			args:    []string{"-max-body-size", "foo"},
@@ -515,7 +559,7 @@ func TestLoadConfig(t *testing.T) {
 			if tc.getHostname == nil {
 				tc.getHostname = getHostnameDefault
 			}
-			cfg, err := loadConfig(tc.args, func(key string) string { return tc.env[key] }, tc.getHostname)
+			cfg, err := loadConfig(tc.args, func(key string) string { return tc.env[key] }, func() []string { return environSlice(tc.env) }, tc.getHostname)
 
 			switch {
 			case tc.wantErr != nil && err != nil:
@@ -606,7 +650,7 @@ func TestMainImpl(t *testing.T) {
 			}
 
 			buf := &bytes.Buffer{}
-			gotCode := mainImpl(tc.args, func(key string) string { return tc.env[key] }, tc.getHostname, buf)
+			gotCode := mainImpl(tc.args, func(key string) string { return tc.env[key] }, func() []string { return environSlice(tc.env) }, tc.getHostname, buf)
 			out := buf.String()
 
 			if gotCode != tc.wantCode {
@@ -625,3 +669,11 @@ func TestMainImpl(t *testing.T) {
 		})
 	}
 }
+
+func environSlice(env map[string]string) []string {
+	envStrings := make([]string, 0, len(env))
+	for name, value := range env {
+		envStrings = append(envStrings, fmt.Sprintf("%s=%s", name, value))
+	}
+	return envStrings
+}
diff --git a/httpbin/handlers_test.go b/httpbin/handlers_test.go
index 048c684..27b1a5a 100644
--- a/httpbin/handlers_test.go
+++ b/httpbin/handlers_test.go
@@ -120,6 +120,16 @@ func TestIndex(t *testing.T) {
 	}
 }
 
+func TestEnv(t *testing.T) {
+	t.Run("default environment", func(t *testing.T) {
+		t.Parallel()
+		req := newTestRequest(t, "GET", "/env")
+		resp := must.DoReq(t, client, req)
+		result := mustParseResponse[envResponse](t, resp)
+		assert.Equal(t, len(result.Env), 0, "environment variables unexpected")
+	})
+}
+
 func TestFormsPost(t *testing.T) {
 	t.Parallel()