Prepare for v0.2.0 release

* Adjust styling to match internal patterns
* Add some test cases for Reply kinds

CHANGES.md

@@ -4,9 +4,11 @@ Notable changes between versions.
 
 ## Latest
 
+## v0.2.0
+
 * Add Prometheus `/metrics` endpoint ([#4](https://github.com/poseidon/fleetlock/pull/4))
+* Add JSON error responses ([#9](https://github.com/poseidon/fleetlock/pull/9))
 * Fix `-version` command output ([#6](https://github.com/poseidon/fleetlock/pull/6))
-* Use JSON output for error responses to match fleetlock specification ([#9](https://github.com/poseidon/fleetlock/pull/9))
 
 ## v0.1.0
 

examples/k8s/deployment.yaml

@@ -17,7 +17,7 @@ spec:
       serviceAccountName: fleetlock
       containers:
         - name: fleetlock
-          image: quay.io/poseidon/fleetlock:v0.1.0
+          image: quay.io/poseidon/fleetlock:v0.2.0
           ports:
             - name: http
               containerPort: 8080

internal/encode.go

@@ -0,0 +1,56 @@
+package fleetlock
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+)
+
+// List of ReplyKind
+const (
+	KindMethodNotAllowed ReplyKind = "method_not_allowed"
+	KindMissingHeader    ReplyKind = "missing_header"
+	KindDecodeError      ReplyKind = "decode_error"
+	KindInternalError    ReplyKind = "internal_error"
+	KindLockHeld         ReplyKind = "lock_held"
+)
+
+// ReplyKind is used as a Zincati metrics label.
+type ReplyKind string
+
+// Reply represents a Fleetlock protocol reply.
+type Reply struct {
+	// reply identifier
+	Kind ReplyKind `json:"kind"`
+	// human-friendly reply message
+	Value string `json:"value"`
+}
+
+// NewReply creates an Reply with a specific kind and a formatted message.
+func NewReply(kind ReplyKind, format string, a ...interface{}) Reply {
+	return Reply{
+		Kind:  kind,
+		Value: fmt.Sprintf(format, a...),
+	}
+}
+
+// encodeReply writes response with the given Reply and HTTP code.
+func encodeReply(w http.ResponseWriter, reply Reply) error {
+	w.Header().Set("Content-Type", "application/json")
+	w.Header().Set("X-Content-Type-Options", "nosniff")
+
+	switch reply.Kind {
+	case KindMethodNotAllowed:
+		w.WriteHeader(http.StatusMethodNotAllowed)
+	case KindDecodeError, KindMissingHeader:
+		w.WriteHeader(http.StatusBadRequest)
+	case KindInternalError:
+		w.WriteHeader(http.StatusInternalServerError)
+	case KindLockHeld:
+		w.WriteHeader(http.StatusLocked)
+	default:
+		w.WriteHeader(http.StatusOK)
+	}
+
+	return json.NewEncoder(w).Encode(reply)
+}

internal/encode_test.go

@@ -0,0 +1,60 @@
+package fleetlock
+
+import (
+	"fmt"
+	"io/ioutil"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestEncodeReply(t *testing.T) {
+	cases := []struct {
+		reply            Reply
+		expectedStatus   int
+		expectedResponse string
+	}{
+		{
+			reply:            NewReply(KindMethodNotAllowed, "required method POST"),
+			expectedStatus:   405,
+			expectedResponse: `{"kind": "method_not_allowed", "value": "required method POST"}`,
+		},
+		{
+			reply:            NewReply(KindMissingHeader, "missing required header %s: %s", fleetLockHeaderKey, "true"),
+			expectedStatus:   400,
+			expectedResponse: `{"kind": "missing_header", "value": "missing required header fleet-lock-protocol: true"}`,
+		},
+		{
+			reply:            NewReply(KindDecodeError, "error decoding message"),
+			expectedStatus:   400,
+			expectedResponse: `{"kind": "decode_error", "value": "error decoding message"}`,
+		},
+		{
+			reply:            NewReply(KindInternalError, "error getting reboot lease"),
+			expectedStatus:   500,
+			expectedResponse: `{"kind": "internal_error", "value": "error getting reboot lease"}`,
+		},
+		{
+			reply:            NewReply(KindLockHeld, "reboot lease unavailable, held by %s", "e0f3745b108f471cbd4883c6fbed8cdd"),
+			expectedStatus:   423,
+			expectedResponse: `{"kind": "lock_held", "value": "reboot lease unavailable, held by e0f3745b108f471cbd4883c6fbed8cdd"}`,
+		},
+		{
+			reply:            NewReply("other", "message"),
+			expectedStatus:   200,
+			expectedResponse: `{"kind": "other", "value": "message"}`,
+		},
+	}
+	for _, c := range cases {
+		t.Run(fmt.Sprintf("%v-%v", c.reply.Kind, c.reply.Value), func(t *testing.T) {
+			w := httptest.NewRecorder()
+			encodeReply(w, c.reply)
+
+			assert.Equal(t, c.expectedStatus, w.Code, "Expected status code %v", c.expectedStatus)
+
+			body, _ := ioutil.ReadAll(w.Body)
+			assert.JSONEq(t, c.expectedResponse, string(body), "Unexpected JSON output in response")
+		})
+	}
+}

internal/handlers.go

@@ -12,7 +12,7 @@ const (
 func POSTHandler(next http.Handler) http.Handler {
 	fn := func(w http.ResponseWriter, req *http.Request) {
 		if req.Method != http.MethodPost {
-			encodeReply(w, NewReply(ErrorMethodNotAllowed, "required method POST"))
+			encodeReply(w, NewReply(KindMethodNotAllowed, "required method POST"))
 			return
 		}
 		next.ServeHTTP(w, req)
@@ -24,7 +24,7 @@ func POSTHandler(next http.Handler) http.Handler {
 func HeaderHandler(key, value string, next http.Handler) http.Handler {
 	fn := func(w http.ResponseWriter, req *http.Request) {
 		if req.Header.Get(key) != value {
-			encodeReply(w, NewReply(ErrorMissingHeader, "missing required header %s: %s", key, value))
+			encodeReply(w, NewReply(KindMissingHeader, "missing required header %s: %s", key, value))
 			return
 		}
 		next.ServeHTTP(w, req)

internal/server.go

@@ -104,7 +104,7 @@ func (s *Server) lock(w http.ResponseWriter, req *http.Request) {
 	msg, err := decodeMessage(req)
 	if err != nil {
 		s.log.Errorf("fleetlock: error decoding message: %v", err)
-		encodeReply(w, NewReply(ErrorDecodingRequest, "error decoding message"))
+		encodeReply(w, NewReply(KindDecodeError, "error decoding message"))
 	}
 	id := msg.ClientParmas.ID
 	group := msg.ClientParmas.Group
@@ -124,7 +124,7 @@ func (s *Server) lock(w http.ResponseWriter, req *http.Request) {
 	if err != nil {
 		if !errors.IsNotFound(err) {
 			s.log.Errorf("fleetlock: error getting reboot lease %s: %v", rebootLease.Name(), err)
-			encodeReply(w, NewReply(ErrorInternal, "error getting reboot lease"))
+			encodeReply(w, NewReply(KindInternalError, "error getting reboot lease"))
 			return
 		}
 	}
@@ -158,7 +158,7 @@ func (s *Server) lock(w http.ResponseWriter, req *http.Request) {
 
 	// reboot lease held by different node
 	s.log.WithFields(fields).Info("fleetlock: reboot lease unavailable")
-	encodeReply(w, NewReply(ErrorLocked, "reboot lease unavailable, held by %s", lock.Holder))
+	encodeReply(w, NewReply(KindLockHeld, "reboot lease unavailable, held by %s", lock.Holder))
 }
 
 // unlock attempts to release a reboot lease lock.
@@ -167,7 +167,7 @@ func (s *Server) unlock(w http.ResponseWriter, req *http.Request) {
 	msg, err := decodeMessage(req)
 	if err != nil {
 		s.log.Errorf("fleetlock: error decoding message: %v", err)
-		encodeReply(w, NewReply(ErrorDecodingRequest, "error decoding message"))
+		encodeReply(w, NewReply(KindDecodeError, "error decoding message"))
 		return
 	}
 	id := msg.ClientParmas.ID
@@ -188,7 +188,7 @@ func (s *Server) unlock(w http.ResponseWriter, req *http.Request) {
 	if err != nil {
 		if !errors.IsNotFound(err) {
 			s.log.Errorf("fleetlock: error getting reboot lease %s: %v", rebootLease.Name(), err)
-			encodeReply(w, NewReply(ErrorInternal, "error getting reboot lease"))
+			encodeReply(w, NewReply(KindInternalError, "error getting reboot lease"))
 			return
 		}
 	}
@@ -204,7 +204,7 @@ func (s *Server) unlock(w http.ResponseWriter, req *http.Request) {
 		err = rebootLease.Update(ctx, update)
 		if err != nil {
 			s.log.WithFields(fields).Errorf("fleetlock: error unlocking reboot lease: %v", err)
-			encodeReply(w, NewReply(ErrorInternal, "error unlocking reboot lease"))
+			encodeReply(w, NewReply(KindInternalError, "error unlocking reboot lease"))
 			return
 		}
 		s.metrics.lockState.With(prometheus.Labels{"group": group}).Set(0)