From 20dd618a673f2602d67f55e1b71c7c8ad924d7ae Mon Sep 17 00:00:00 2001
From: Will McCutchen <will@mccutch.org>
Date: Mon, 1 Jul 2019 17:26:45 -0700
Subject: [PATCH] =?UTF-8?q?Support=20cmd/go-httpbin=20(=F0=9F=91=8D)=20and?=
 =?UTF-8?q?=20cmd/go=5Fhttpbin=20(=F0=9F=91=8E)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In #17, we switched to deploying httpbingo.org on Google App Engine. To
meet their naming requirements, we had to rename cmd/go-httpbin to
cmd/go_httpbin, but this broke any existing uses of, e.g.,

    go get github.com/mccutchen/go-httpbin/cmd/go-httpbin

as suggested in the README and as used in various places (including my
employer).

Here's a dumb ass fix for that dumb ass problem; I'm not happy about it,
but it seems to work.
---
 Makefile               |   8 +--
 README.md              |   2 +-
 cmd/README.md          |  38 ++++++++++++++
 cmd/go-httpbin/main.go |   7 +++
 cmd/go_httpbin/main.go | 107 +--------------------------------------
 cmd/maincmd/main.go    | 111 +++++++++++++++++++++++++++++++++++++++++
 6 files changed, 164 insertions(+), 109 deletions(-)
 create mode 100644 cmd/README.md
 create mode 100644 cmd/go-httpbin/main.go
 create mode 100644 cmd/maincmd/main.go

diff --git a/Makefile b/Makefile
index 9a571e4..4885c36 100644
--- a/Makefile
+++ b/Makefile
@@ -10,14 +10,16 @@ BIN_DIR   := $(GOPATH)/bin
 GOLINT    := $(BIN_DIR)/golint
 GOBINDATA := $(BIN_DIR)/go-bindata
 
+GO_SOURCES = $(wildcard **/*.go)
+
 # =============================================================================
 # build
 # =============================================================================
 build: dist/go-httpbin
 
-dist/go-httpbin: assets cmd/go_httpbin/*.go httpbin/*.go go.mod
+dist/go-httpbin: assets $(GO_SOURCES)
 	mkdir -p dist
-	go build -o dist/go-httpbin ./cmd/go_httpbin
+	go build -o dist/go-httpbin ./cmd/go-httpbin
 
 assets: $(GENERATED_ASSETS_PATH)
 
@@ -67,7 +69,7 @@ run: build
 # =============================================================================
 # docker images
 # =============================================================================
-image: assets cmd/go_httpbin/*.go httpbin/*.go
+image: build
 	docker build -t mccutchen/go-httpbin:$(VERSION) .
 
 imagepush: image
diff --git a/README.md b/README.md
index a3a3aa2..7555eaf 100644
--- a/README.md
+++ b/README.md
@@ -88,7 +88,7 @@ func TestSlowResponse(t *testing.T) {
 ## Installation
 
 ```
-go get github.com/mccutchen/go-httpbin/...
+go get github.com/mccutchen/go-httpbin/cmd/go-httpbin
 ```
 
 
diff --git a/cmd/README.md b/cmd/README.md
new file mode 100644
index 0000000..267f207
--- /dev/null
+++ b/cmd/README.md
@@ -0,0 +1,38 @@
+# What is going on here?
+
+## TL;DR
+
+ * `cmd/maincmd` package exposes all of this app's command line functionality in a `Main()`
+
+ * `cmd/go-httpbin` and `cmd/go_httpbin` build identical binaries using the
+   `maincmd` package for backwards compatibility reasons explained below
+
+## Why tho
+
+Originally, this project exposed only one command:
+
+    cmd/go-httpbin/main.go
+
+But the dash in that path was incompatible with Google App Engine's naming
+restrictions, so in [moving httpbingo.org onto Google App Engine][pr17], that
+path was (carelessly) renamed to
+
+    cmd/go_httpbin/main.go
+
+_That_ change had a number of unintended consequences:
+
+ * It broke existing workflows built around `go get github.com/mccutchen/go-httpbin/cmd/go-httpbin`,
+   as suggested in the README
+
+ * It broke the Makefile, which was still looking for `cmd/go-httpbin`
+
+ * It broke the absolute aesthetic truth that CLI binaries should use dashes
+   instead of underscores for word separators
+
+So, to restore the former behavior while maintaining support for deploying to
+App Engine, the actual main functionality was extracted into the `cmd/maincmd`
+package here and shared between the other two.
+
+(This is pretty dumb, I know, but it seems to work.)
+
+[pr17]: https://github.com/mccutchen/go-httpbin/pull/17
diff --git a/cmd/go-httpbin/main.go b/cmd/go-httpbin/main.go
new file mode 100644
index 0000000..71c0bb9
--- /dev/null
+++ b/cmd/go-httpbin/main.go
@@ -0,0 +1,7 @@
+package main
+
+import "github.com/mccutchen/go-httpbin/cmd/maincmd"
+
+func main() {
+	maincmd.Main()
+}
diff --git a/cmd/go_httpbin/main.go b/cmd/go_httpbin/main.go
index 310ac96..71c0bb9 100644
--- a/cmd/go_httpbin/main.go
+++ b/cmd/go_httpbin/main.go
@@ -1,110 +1,7 @@
 package main
 
-import (
-	"crypto/tls"
-	"flag"
-	"fmt"
-	"log"
-	"net"
-	"net/http"
-	"os"
-	"strconv"
-	"time"
-
-	"github.com/mccutchen/go-httpbin/httpbin"
-)
-
-const defaultHost = "0.0.0.0"
-const defaultPort = 8080
-
-var (
-	host          string
-	port          int
-	maxBodySize   int64
-	maxDuration   time.Duration
-	httpsCertFile string
-	httpsKeyFile  string
-)
+import "github.com/mccutchen/go-httpbin/cmd/maincmd"
 
 func main() {
-	flag.StringVar(&host, "host", defaultHost, "Host to listen on")
-	flag.IntVar(&port, "port", defaultPort, "Port to listen on")
-	flag.StringVar(&httpsCertFile, "https-cert-file", "", "HTTPS Server certificate file")
-	flag.StringVar(&httpsKeyFile, "https-key-file", "", "HTTPS Server private key file")
-	flag.Int64Var(&maxBodySize, "max-body-size", httpbin.DefaultMaxBodySize, "Maximum size of request or response, in bytes")
-	flag.DurationVar(&maxDuration, "max-duration", httpbin.DefaultMaxDuration, "Maximum duration a response may take")
-	flag.Parse()
-
-	// 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.
-	var err error
-	if maxBodySize == httpbin.DefaultMaxBodySize && os.Getenv("MAX_BODY_SIZE") != "" {
-		maxBodySize, err = strconv.ParseInt(os.Getenv("MAX_BODY_SIZE"), 10, 64)
-		if err != nil {
-			fmt.Printf("invalid value %#v for env var MAX_BODY_SIZE: %s\n", os.Getenv("MAX_BODY_SIZE"), err)
-			flag.Usage()
-			os.Exit(1)
-		}
-	}
-	if maxDuration == httpbin.DefaultMaxDuration && os.Getenv("MAX_DURATION") != "" {
-		maxDuration, err = time.ParseDuration(os.Getenv("MAX_DURATION"))
-		if err != nil {
-			fmt.Printf("invalid value %#v for env var MAX_DURATION: %s\n", os.Getenv("MAX_DURATION"), err)
-			flag.Usage()
-			os.Exit(1)
-		}
-	}
-	if host == defaultHost && os.Getenv("HOST") != "" {
-		host = os.Getenv("HOST")
-	}
-	if port == defaultPort && os.Getenv("PORT") != "" {
-		port, err = strconv.Atoi(os.Getenv("PORT"))
-		if err != nil {
-			fmt.Printf("invalid value %#v for env var PORT: %s\n", os.Getenv("PORT"), err)
-			flag.Usage()
-			os.Exit(1)
-		}
-	}
-
-	if httpsCertFile == "" && os.Getenv("HTTPS_CERT_FILE") != "" {
-		httpsCertFile = os.Getenv("HTTPS_CERT_FILE")
-	}
-	if httpsKeyFile == "" && os.Getenv("HTTPS_KEY_FILE") != "" {
-		httpsKeyFile = os.Getenv("HTTPS_KEY_FILE")
-	}
-
-	logger := log.New(os.Stderr, "", 0)
-
-	h := httpbin.New(
-		httpbin.WithMaxBodySize(maxBodySize),
-		httpbin.WithMaxDuration(maxDuration),
-		httpbin.WithObserver(httpbin.StdLogObserver(logger)),
-	)
-
-	listenAddr := net.JoinHostPort(host, strconv.Itoa(port))
-
-	server := &http.Server{
-		Addr:    listenAddr,
-		Handler: h.Handler(),
-	}
-
-	var listenErr error
-	if httpsCertFile != "" && httpsKeyFile != "" {
-		cert, err := tls.LoadX509KeyPair(httpsCertFile, httpsKeyFile)
-		if err != nil {
-			logger.Fatal("Failed to generate https key pair: ", err)
-		}
-		server.TLSConfig = &tls.Config{
-			Certificates: []tls.Certificate{cert},
-		}
-		logger.Printf("go-httpbin listening on https://%s", listenAddr)
-		listenErr = server.ListenAndServeTLS("", "")
-	} else {
-		logger.Printf("go-httpbin listening on http://%s", listenAddr)
-		listenErr = server.ListenAndServe()
-	}
-	if listenErr != nil {
-		logger.Fatalf("Failed to listen: %s", listenErr)
-	}
+	maincmd.Main()
 }
diff --git a/cmd/maincmd/main.go b/cmd/maincmd/main.go
new file mode 100644
index 0000000..7393bfe
--- /dev/null
+++ b/cmd/maincmd/main.go
@@ -0,0 +1,111 @@
+package maincmd
+
+import (
+	"crypto/tls"
+	"flag"
+	"fmt"
+	"log"
+	"net"
+	"net/http"
+	"os"
+	"strconv"
+	"time"
+
+	"github.com/mccutchen/go-httpbin/httpbin"
+)
+
+const defaultHost = "0.0.0.0"
+const defaultPort = 8080
+
+var (
+	host          string
+	port          int
+	maxBodySize   int64
+	maxDuration   time.Duration
+	httpsCertFile string
+	httpsKeyFile  string
+)
+
+// Main implements the go-httpbin CLI's main() function in a reusable way
+func Main() {
+	flag.StringVar(&host, "host", defaultHost, "Host to listen on")
+	flag.IntVar(&port, "port", defaultPort, "Port to listen on")
+	flag.StringVar(&httpsCertFile, "https-cert-file", "", "HTTPS Server certificate file")
+	flag.StringVar(&httpsKeyFile, "https-key-file", "", "HTTPS Server private key file")
+	flag.Int64Var(&maxBodySize, "max-body-size", httpbin.DefaultMaxBodySize, "Maximum size of request or response, in bytes")
+	flag.DurationVar(&maxDuration, "max-duration", httpbin.DefaultMaxDuration, "Maximum duration a response may take")
+	flag.Parse()
+
+	// 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.
+	var err error
+	if maxBodySize == httpbin.DefaultMaxBodySize && os.Getenv("MAX_BODY_SIZE") != "" {
+		maxBodySize, err = strconv.ParseInt(os.Getenv("MAX_BODY_SIZE"), 10, 64)
+		if err != nil {
+			fmt.Printf("invalid value %#v for env var MAX_BODY_SIZE: %s\n", os.Getenv("MAX_BODY_SIZE"), err)
+			flag.Usage()
+			os.Exit(1)
+		}
+	}
+	if maxDuration == httpbin.DefaultMaxDuration && os.Getenv("MAX_DURATION") != "" {
+		maxDuration, err = time.ParseDuration(os.Getenv("MAX_DURATION"))
+		if err != nil {
+			fmt.Printf("invalid value %#v for env var MAX_DURATION: %s\n", os.Getenv("MAX_DURATION"), err)
+			flag.Usage()
+			os.Exit(1)
+		}
+	}
+	if host == defaultHost && os.Getenv("HOST") != "" {
+		host = os.Getenv("HOST")
+	}
+	if port == defaultPort && os.Getenv("PORT") != "" {
+		port, err = strconv.Atoi(os.Getenv("PORT"))
+		if err != nil {
+			fmt.Printf("invalid value %#v for env var PORT: %s\n", os.Getenv("PORT"), err)
+			flag.Usage()
+			os.Exit(1)
+		}
+	}
+
+	if httpsCertFile == "" && os.Getenv("HTTPS_CERT_FILE") != "" {
+		httpsCertFile = os.Getenv("HTTPS_CERT_FILE")
+	}
+	if httpsKeyFile == "" && os.Getenv("HTTPS_KEY_FILE") != "" {
+		httpsKeyFile = os.Getenv("HTTPS_KEY_FILE")
+	}
+
+	logger := log.New(os.Stderr, "", 0)
+
+	h := httpbin.New(
+		httpbin.WithMaxBodySize(maxBodySize),
+		httpbin.WithMaxDuration(maxDuration),
+		httpbin.WithObserver(httpbin.StdLogObserver(logger)),
+	)
+
+	listenAddr := net.JoinHostPort(host, strconv.Itoa(port))
+
+	server := &http.Server{
+		Addr:    listenAddr,
+		Handler: h.Handler(),
+	}
+
+	var listenErr error
+	if httpsCertFile != "" && httpsKeyFile != "" {
+		cert, err := tls.LoadX509KeyPair(httpsCertFile, httpsKeyFile)
+		if err != nil {
+			logger.Fatal("Failed to generate https key pair: ", err)
+		}
+		server.TLSConfig = &tls.Config{
+			Certificates: []tls.Certificate{cert},
+		}
+		logger.Printf("go-httpbin listening on https://%s", listenAddr)
+		listenErr = server.ListenAndServeTLS("", "")
+	} else {
+		logger.Printf("go-httpbin listening on http://%s", listenAddr)
+		listenErr = server.ListenAndServe()
+	}
+	if listenErr != nil {
+		logger.Fatalf("Failed to listen: %s", listenErr)
+	}
+}
-- 
GitLab