REST API error response body for invalid /pems/ requests.

pkg/rest/handlers.go

@@ -29,9 +29,12 @@
 package rest
 
 import (
+	"encoding/json"
 	"net/http"
 	"regexp"
+	"strconv"
 	"strings"
+	"sync/atomic"
 	"time"
 
 	"code.uplex.de/k8s/k8s-crt-dnldr/pkg/crt"
@@ -45,6 +48,14 @@ const (
 	verParam = "version"
 )
 
+// ErrorDetails stores the fixed strings used in the bodies of error
+// responses.
+type ErrorDetails struct {
+	Type   string
+	Title  string
+	Detail string
+}
+
 var (
 	pemsRegex = regexp.MustCompile("^" + pemsPfx + "([^/]+)/([^/]+)$")
 
@@ -62,8 +73,27 @@ var (
 		http.MethodPut:    struct{}{},
 	}
 	allowPems = "GET, HEAD, PUT, POST, DELETE"
+
+	errCtr             = uint64(0)
+	problemContentType = "application/problem+json"
+
+	errPemsPattern = ErrorDetails{
+		Type:  "/errors/pems/urlPattern",
+		Title: "Invalid /v1/pems/ URL path",
+		Detail: "/v1/pems/ URL path does not match " +
+			"/v1/pems/{namespace}/{name}",
+	}
 )
 
+// Problem Details object per RFC7807
+type Problem struct {
+	Type     string `json:"type"`
+	Title    string `json:"title"`
+	Status   int    `json:"status"`
+	Detail   string `json:"detail"`
+	Instance string `json:"instance"`
+}
+
 // Date format for Common Log Format
 const common = "02/Jan/2006:15:04:05 -0700"
 
@@ -87,6 +117,11 @@ func errLog(log *logrus.Logger, req *http.Request, err error) {
 		req.RequestURI, req.Proto, err)
 }
 
+func getErrInstance() string {
+	n := atomic.AddUint64(&errCtr, 1)
+	return "/log/errors/" + strconv.FormatUint(n, 36)
+}
+
 type healthzHndlr struct {
 	log     *logrus.Logger
 	version string
@@ -119,6 +154,45 @@ type pemsHndlr struct {
 	crtGetter *crt.Getter
 }
 
+func (h *pemsHndlr) errorResponse(
+	resp http.ResponseWriter,
+	req *http.Request,
+	now time.Time,
+	status int,
+	details ErrorDetails,
+	err error,
+) {
+	problem := Problem{
+		Type:     details.Type,
+		Title:    details.Title,
+		Detail:   details.Detail,
+		Status:   status,
+		Instance: getErrInstance(),
+	}
+	if err != nil {
+		problem.Detail = err.Error()
+	}
+	h.log.Errorf("%s %s: %+v", req.Method, req.RequestURI, problem)
+
+	body, err := json.Marshal(problem)
+	if err != nil {
+		resp.WriteHeader(status)
+		h.log.Errorf("%s %s: cannot marshal %+v: %v", req.Method,
+			req.RequestURI, problem, err)
+		reqLog(h.log, req, now, status, 0)
+		return
+	}
+	resp.Header().Set("Content-Type", problemContentType)
+	resp.Header().Set("Content-Length", strconv.Itoa(len(body)))
+	resp.WriteHeader(status)
+	n, err := resp.Write(body)
+	if err != nil {
+		h.log.Errorf("%s %s: cannot write body: %v", req.Method,
+			req.RequestURI, err)
+	}
+	reqLog(h.log, req, now, status, n)
+}
+
 func (h *pemsHndlr) allPems(
 	resp http.ResponseWriter,
 	req *http.Request,
@@ -143,9 +217,8 @@ func (h *pemsHndlr) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
 
 	matches := pemsRegex.FindStringSubmatch(req.URL.Path)
 	if matches == nil {
-		status = http.StatusNotFound
-		resp.WriteHeader(status)
-		reqLog(h.log, req, now, status, bytes)
+		h.errorResponse(resp, req, now, http.StatusNotFound,
+			errPemsPattern, nil)
 		return
 	}
 

pkg/rest/handlers_test.go

@@ -31,10 +31,13 @@ package rest
 import (
 	"bytes"
 	"context"
+	"encoding/json"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"regexp"
+	"strconv"
 	"testing"
 
 	"code.uplex.de/k8s/k8s-crt-dnldr/pkg/crt"
@@ -813,3 +816,67 @@ func TestAllPem(t *testing.T) {
 		}
 	}
 }
+
+var errInstancePattern = regexp.MustCompile(`^/log/errors/\d+$`)
+
+func TestErrors(t *testing.T) {
+	client := fake.NewSimpleClientset()
+	lister := setupSecretLister(client)
+	getter := crt.NewGetter(lister)
+	base, err := getTempDir()
+	if err != nil {
+		t.Fatalf("ioutil.TempDir(): %+v", err)
+	}
+	defer os.RemoveAll(base)
+	files, err := pem.NewFiles(base, -1, getter)
+	if err != nil {
+		t.Fatalf("NewFiles(): %v", err)
+	}
+
+	hndlr := &pemsHndlr{
+		log:       &logrus.Logger{Out: ioutil.Discard},
+		files:     files,
+		crtGetter: getter,
+	}
+
+	req := httptest.NewRequest(http.MethodGet, "/v1/pems/foo", nil)
+	rr := httptest.NewRecorder()
+	hndlr.ServeHTTP(rr, req)
+	if rr.Code != http.StatusNotFound {
+		t.Errorf("GET /v1/pems/foo status: got %d want %d", rr.Code,
+			http.StatusNotFound)
+	}
+	if rr.Header().Get("Content-Type") != problemContentType {
+		t.Errorf("GET /v1/pems/foo Content-Type: got %s want %s",
+			rr.Header().Get("Content-Type"), problemContentType)
+	}
+	bodylen := len(rr.Body.String())
+	if rr.Header().Get("Content-Length") != strconv.Itoa(bodylen) {
+		t.Errorf("GET /v1/pems/foo Content-Length: got %s want %d",
+			rr.Header().Get("Content-Length"), bodylen)
+	}
+	problem := &Problem{}
+	if err = json.Unmarshal(rr.Body.Bytes(), problem); err != nil {
+		t.Fatalf("GET /v1/pems/foo body unmarshal: %v", err)
+	}
+	if problem.Type != errPemsPattern.Type {
+		t.Errorf("GET /v1/pems/foo problem type: got %s want %s",
+			problem.Type, errPemsPattern.Type)
+	}
+	if problem.Title != errPemsPattern.Title {
+		t.Errorf("GET /v1/pems/foo problem title: got %s want %s",
+			problem.Title, errPemsPattern.Title)
+	}
+	if problem.Detail != errPemsPattern.Detail {
+		t.Errorf("GET /v1/pems/foo problem title: got %s want %s",
+			problem.Detail, errPemsPattern.Title)
+	}
+	if problem.Status != http.StatusNotFound {
+		t.Errorf("GET /v1/pems/foo problem status: got %d want %d",
+			problem.Status, http.StatusNotFound)
+	}
+	if !errInstancePattern.Match([]byte(problem.Instance)) {
+		t.Errorf("GET /v1/pems/foo problem instance: "+
+			"got %s want /log/errors/N", problem.Instance)
+	}
+}