util/httpm: add new package for prettier HTTP method constants

See package doc.

Change-Id: Ibbfc8e1f98294217c56f3a9452bd93ffa3103572
Signed-off-by: Brad Fitzpatrick <[email protected]>

client/tailscale/tailnet.go

@@ -11,6 +11,8 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
+
+	"tailscale.com/util/httpm"
 )
 
 // TailnetDeleteRequest handles sending a DELETE request for a tailnet to control.
@@ -22,7 +24,7 @@ func (c *Client) TailnetDeleteRequest(ctx context.Context, tailnetID string) (er
 	}()
 
 	path := fmt.Sprintf("%s/api/v2/tailnet/%s", c.baseURL(), url.PathEscape(string(tailnetID)))
-	req, err := http.NewRequestWithContext(ctx, http.MethodDelete, path, nil)
+	req, err := http.NewRequestWithContext(ctx, httpm.DELETE, path, nil)
 	if err != nil {
 		return err
 	}

cmd/derper/depaware.txt

@@ -78,6 +78,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa
    W    tailscale.com/util/cmpver                                    from tailscale.com/net/tshttpproxy
    L 💣 tailscale.com/util/dirwalk                                   from tailscale.com/metrics
         tailscale.com/util/dnsname                                   from tailscale.com/hostinfo+
+        tailscale.com/util/httpm                                     from tailscale.com/client/tailscale
         tailscale.com/util/lineread                                  from tailscale.com/hostinfo+
         tailscale.com/util/mak                                       from tailscale.com/syncs
         tailscale.com/util/singleflight                              from tailscale.com/net/dnscache

cmd/gitops-pusher/gitops-pusher.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/peterbourgon/ff/v3/ffcli"
 	"github.com/tailscale/hujson"
+	"tailscale.com/util/httpm"
 )
 
 var (
@@ -235,7 +236,7 @@ func applyNewACL(ctx context.Context, tailnet, apiKey, policyFname, oldEtag stri
 	}
 	defer fin.Close()
 
-	req, err := http.NewRequestWithContext(ctx, http.MethodPost, fmt.Sprintf("https://%s/api/v2/tailnet/%s/acl", *apiServer, tailnet), fin)
+	req, err := http.NewRequestWithContext(ctx, httpm.POST, fmt.Sprintf("https://%s/api/v2/tailnet/%s/acl", *apiServer, tailnet), fin)
 	if err != nil {
 		return err
 	}
@@ -275,7 +276,7 @@ func testNewACLs(ctx context.Context, tailnet, apiKey, policyFname string) error
 		return err
 	}
 
-	req, err := http.NewRequestWithContext(ctx, http.MethodPost, fmt.Sprintf("https://%s/api/v2/tailnet/%s/acl/validate", *apiServer, tailnet), bytes.NewBuffer(data))
+	req, err := http.NewRequestWithContext(ctx, httpm.POST, fmt.Sprintf("https://%s/api/v2/tailnet/%s/acl/validate", *apiServer, tailnet), bytes.NewBuffer(data))
 	if err != nil {
 		return err
 	}
@@ -347,7 +348,7 @@ type ACLTestErrorDetail struct {
 }
 
 func getACLETag(ctx context.Context, tailnet, apiKey string) (string, error) {
-	req, err := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf("https://%s/api/v2/tailnet/%s/acl", *apiServer, tailnet), nil)
+	req, err := http.NewRequestWithContext(ctx, httpm.GET, fmt.Sprintf("https://%s/api/v2/tailnet/%s/acl", *apiServer, tailnet), nil)
 	if err != nil {
 		return "", err
 	}

cmd/tailscale/depaware.txt

@@ -108,6 +108,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
    L 💣 tailscale.com/util/dirwalk                                   from tailscale.com/metrics
         tailscale.com/util/dnsname                                   from tailscale.com/cmd/tailscale/cli+
         tailscale.com/util/groupmember                               from tailscale.com/cmd/tailscale/cli
+        tailscale.com/util/httpm                                     from tailscale.com/client/tailscale
         tailscale.com/util/lineread                                  from tailscale.com/net/interfaces+
         tailscale.com/util/mak                                       from tailscale.com/net/netcheck+
         tailscale.com/util/multierr                                  from tailscale.com/control/controlhttp

cmd/tailscaled/depaware.txt

@@ -293,6 +293,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/util/goroutines                                from tailscale.com/control/controlclient+
         tailscale.com/util/groupmember                               from tailscale.com/ipn/ipnauth
      💣 tailscale.com/util/hashx                                     from tailscale.com/util/deephash
+        tailscale.com/util/httpm                                     from tailscale.com/client/tailscale+
         tailscale.com/util/lineread                                  from tailscale.com/hostinfo+
         tailscale.com/util/mak                                       from tailscale.com/control/controlclient+
         tailscale.com/util/multierr                                  from tailscale.com/control/controlclient+

ipn/localapi/localapi.go

@@ -43,6 +43,7 @@ import (
 	"tailscale.com/types/logger"
 	"tailscale.com/types/ptr"
 	"tailscale.com/util/clientmetric"
+	"tailscale.com/util/httpm"
 	"tailscale.com/util/mak"
 	"tailscale.com/util/strs"
 	"tailscale.com/version"
@@ -1235,7 +1236,7 @@ func (h *Handler) serveTKAStatus(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "lock status access denied", http.StatusForbidden)
 		return
 	}
-	if r.Method != http.MethodGet {
+	if r.Method != httpm.GET {
 		http.Error(w, "use GET", http.StatusMethodNotAllowed)
 		return
 	}
@@ -1254,7 +1255,7 @@ func (h *Handler) serveTKASign(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "lock status access denied", http.StatusForbidden)
 		return
 	}
-	if r.Method != http.MethodPost {
+	if r.Method != httpm.POST {
 		http.Error(w, "use POST", http.StatusMethodNotAllowed)
 		return
 	}
@@ -1282,7 +1283,7 @@ func (h *Handler) serveTKAInit(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "lock init access denied", http.StatusForbidden)
 		return
 	}
-	if r.Method != http.MethodPost {
+	if r.Method != httpm.POST {
 		http.Error(w, "use POST", http.StatusMethodNotAllowed)
 		return
 	}
@@ -1317,7 +1318,7 @@ func (h *Handler) serveTKAModify(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "network-lock modify access denied", http.StatusForbidden)
 		return
 	}
-	if r.Method != http.MethodPost {
+	if r.Method != httpm.POST {
 		http.Error(w, "use POST", http.StatusMethodNotAllowed)
 		return
 	}
@@ -1344,7 +1345,7 @@ func (h *Handler) serveTKADisable(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "network-lock modify access denied", http.StatusForbidden)
 		return
 	}
-	if r.Method != http.MethodPost {
+	if r.Method != httpm.POST {
 		http.Error(w, "use POST", http.StatusMethodNotAllowed)
 		return
 	}
@@ -1368,7 +1369,7 @@ func (h *Handler) serveTKALocalDisable(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "network-lock modify access denied", http.StatusForbidden)
 		return
 	}
-	if r.Method != http.MethodPost {
+	if r.Method != httpm.POST {
 		http.Error(w, "use POST", http.StatusMethodNotAllowed)
 		return
 	}
@@ -1388,7 +1389,7 @@ func (h *Handler) serveTKALocalDisable(w http.ResponseWriter, r *http.Request) {
 }
 
 func (h *Handler) serveTKALog(w http.ResponseWriter, r *http.Request) {
-	if r.Method != http.MethodGet {
+	if r.Method != httpm.GET {
 		http.Error(w, "use GET", http.StatusMethodNotAllowed)
 		return
 	}
@@ -1439,10 +1440,10 @@ func (h *Handler) serveProfiles(w http.ResponseWriter, r *http.Request) {
 	}
 	if suffix == "" {
 		switch r.Method {
-		case http.MethodGet:
+		case httpm.GET:
 			w.Header().Set("Content-Type", "application/json")
 			json.NewEncoder(w).Encode(h.b.ListProfiles())
-		case http.MethodPut:
+		case httpm.PUT:
 			err := h.b.NewProfile()
 			if err != nil {
 				http.Error(w, err.Error(), http.StatusInternalServerError)
@@ -1461,7 +1462,7 @@ func (h *Handler) serveProfiles(w http.ResponseWriter, r *http.Request) {
 	}
 	if suffix == "current" {
 		switch r.Method {
-		case http.MethodGet:
+		case httpm.GET:
 			w.Header().Set("Content-Type", "application/json")
 			json.NewEncoder(w).Encode(h.b.CurrentProfile())
 		default:
@@ -1472,7 +1473,7 @@ func (h *Handler) serveProfiles(w http.ResponseWriter, r *http.Request) {
 
 	profileID := ipn.ProfileID(suffix)
 	switch r.Method {
-	case http.MethodGet:
+	case httpm.GET:
 		profiles := h.b.ListProfiles()
 		profileIndex := slices.IndexFunc(profiles, func(p ipn.LoginProfile) bool {
 			return p.ID == profileID
@@ -1483,14 +1484,14 @@ func (h *Handler) serveProfiles(w http.ResponseWriter, r *http.Request) {
 		}
 		w.Header().Set("Content-Type", "application/json")
 		json.NewEncoder(w).Encode(profiles[profileIndex])
-	case http.MethodPost:
+	case httpm.POST:
 		err := h.b.SwitchProfile(profileID)
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusInternalServerError)
 			return
 		}
 		w.WriteHeader(http.StatusNoContent)
-	case http.MethodDelete:
+	case httpm.DELETE:
 		err := h.b.DeleteProfile(profileID)
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusInternalServerError)

prober/notification.go

@@ -17,6 +17,7 @@ import (
 	"time"
 
 	"github.com/google/uuid"
+	"tailscale.com/util/httpm"
 )
 
 var (
@@ -59,7 +60,7 @@ func SendAlert(summary, details string) error {
 		return errors.New("no SQUADCAST_WEBHOOK configured")
 	}
 
-	req, err := http.NewRequest(http.MethodPost, webhookUrl, bytes.NewBuffer(sqBody))
+	req, err := http.NewRequest(httpm.POST, webhookUrl, bytes.NewBuffer(sqBody))
 	if err != nil {
 		alertFailed.Add(1)
 		return err

util/httpm/httpm.go

@@ -0,0 +1,36 @@
+// Copyright (c) 2023 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package httpm has shorter names for HTTP method constants.
+//
+// Some background: originally Go didn't have http.MethodGet, http.MethodPost
+// and life was good and people just wrote readable "GET" and "POST". But then
+// in a moment of weakness Brad and others maintaining net/http caved and let
+// the http.MethodFoo constants be added and code's been less readable since.
+// Now the substance of the method name is hidden away at the end after
+// "http.Method" and they all blend together and it's hard to read code using
+// them.
+//
+// This package is a compromise. It provides constants, but shorter and closer
+// to how it used to look. It does violate Go style
+// (https://github.com/golang/go/wiki/CodeReviewComments#mixed-caps) that says
+// constants shouldn't be SCREAM_CASE. But this isn't INT_MAX; it's GET and
+// POST, which are already defined as all caps.
+//
+// It would be tempting to make these constants be typed but then they wouldn't
+// be assignable to things in net/http that just want string. Oh well.
+package httpm
+
+const (
+	GET       = "GET"
+	HEAD      = "HEAD"
+	POST      = "POST"
+	PUT       = "PUT"
+	PATCH     = "PATCH"
+	DELETE    = "DELETE"
+	CONNECT   = "CONNECT"
+	OPTIONS   = "OPTIONS"
+	TRACE     = "TRACE"
+	SPACEJUMP = "SPACEJUMP" // https://www.w3.org/Protocols/HTTP/Methods/SpaceJump.html
+)