Merge pull request #92 from glothriel/90-better-error-when-key-mismatch

#90: Improved error on key mismatch

README.md

@@ -57,14 +57,18 @@ If you'll use DNS, you can install the server in one step (replace 0.0.0.0 with
 ```
 kubectl create namespace wormhole
 
+LATEST_VERSION=$(curl -s https://api.github.com/repos/glothriel/wormhole/releases/latest | grep -Po '"tag_name": "\K.*?(?=")')
+
+echo "Latest version is $LATEST_VERSION"
+
 # Replace 1.0.0 with latest version from the releases page
-helm install -n wormhole wh oci://ghcr.io/glothriel/wormhole/wormhole --version 1.1.0 --set server.enabled=true --set server.service.type=LoadBalancer --set server.wg.publicHost="0.0.0.0"
+helm install -n wormhole wh oci://ghcr.io/glothriel/wormhole/wormhole --version $LATEST_VERSION --set server.enabled=true --set server.service.type=LoadBalancer --set server.wg.publicHost="0.0.0.0"
 
 # Wait for the LoadBalancer to get an IP
 kubectl get svc -n wormhole
 
 # Update the server with the IP
-helm upgrade -n wormhole wh oci://ghcr.io/glothriel/wormhole/wormhole --version 1.1.0 --reuse-values --set server.wg.publicHost="<the new IP>"
+helm upgrade -n wormhole wh oci://ghcr.io/glothriel/wormhole/wormhole --version $LATEST_VERSION --reuse-values --set server.wg.publicHost="<the new IP>"
 ```
 
 ### Install client
@@ -182,6 +186,21 @@ No body or query parameters are required.
 |200 Ok | Returned when request was successful |
 |500 Internal server error | Returned when the peers could not be fetched for unknown reasons. |
 
+### DELETE /api/peers/v1/{name}
+
+This endpoint is only available on the server. It allows removing a peer from the server. The peer will be disconnected and all the apps exposed by the peer will be removed.
+
+#### Request
+
+No body or query parameters are required.
+
+#### Response
+
+| Code | Description |
+|:-----|:------------|
+|204 No content | Returned when request was successful |
+|500 Internal server error | Returned when the peer could not be deleted from unknown reason. |
+
 ## Local development
 
 ### Development environment

docker/goDockerfile

@@ -10,14 +10,14 @@ ARG GROUP_ID
 # If the group doesn't exist, use 'go' as the group name
 # This is required because MacOS has strage behavior with GIDs (Default user has GID ~20)
 RUN if ! getent group ${GROUP_ID} > /dev/null 2>&1; then \
-        GROUP_NAME=go; \
-        addgroup -g ${GROUP_ID} ${GROUP_NAME}; \
-        echo $GROUP_NAME > /tmp/groupfile; \
+    GROUP_NAME=go; \
+    addgroup -g ${GROUP_ID} ${GROUP_NAME}; \
+    echo $GROUP_NAME > /tmp/groupfile; \
     else \
-        GROUP_NAME=$(getent group ${GROUP_ID} | cut -d: -f1); \
-        echo $GROUP_NAME > /tmp/groupfile; \
+    GROUP_NAME=$(getent group ${GROUP_ID} | cut -d: -f1); \
+    echo $GROUP_NAME > /tmp/groupfile; \
     fi
-    
+
 # Add the 'go' user with the specified USER_ID and add to the determined group
 RUN groupadd -g ${GROUP_ID} go
 RUN useradd -u ${USER_ID} -g ${GROUP_ID} -m go
@@ -31,14 +31,14 @@ COPY ${PROJECT}/main.go ./main.go
 
 
 # A stage that downloadcs watchexec, made separate to avoid redownloading on every change
-FROM golang:${GO_VERSION}-bookworm as we
+FROM golang:${GO_VERSION}-bookworm AS we
 RUN apt update  && apt install -y xz-utils
 ADD https://github.com/watchexec/watchexec/releases/download/v1.25.1/watchexec-1.25.1-x86_64-unknown-linux-gnu.tar.xz .
 RUN tar -xvf watchexec-1.25.1-x86_64-unknown-linux-gnu.tar.xz
 RUN mv watchexec-1.25.1-x86_64-unknown-linux-gnu/watchexec /usr/local/bin
 
 # Dev stage used when executing the container in development mode, for Tilt
-FROM base as dev
+FROM base AS dev
 WORKDIR /tmp
 USER root
 COPY --from=we /usr/local/bin/watchexec /usr/local/bin/watchexec
@@ -51,7 +51,7 @@ ENV HOME=/home/go
 ENTRYPOINT ["/dev-entrypoint.sh"]
 
 # Build stage, uses the base stage and builds binary, used for docker images built for production and e2e tests
-FROM base as build
+FROM base AS build
 USER root
 WORKDIR /src
 ENV GOCACHE=/root/.cache/go-build

pkg/api/peers.go

@@ -6,31 +6,32 @@ import (
 	"github.com/glothriel/wormhole/pkg/wg"
 )
 
-type peerController struct {
+// PeerController is a controller for managing peers
+type PeerController struct {
 	peers              pairing.PeerStorage
 	wgConfig           *wg.Config
 	watcher            *wg.Watcher
 	enablePeerDeletion bool
 }
 
-func (p *peerController) deletePeer(name string) error {
+func (p *PeerController) deletePeer(name string) error {
 	peerInfo, err := p.peers.GetByName(name)
 	if err != nil {
 		return err
 	}
-	err = p.peers.DeleteByName(name)
+	p.wgConfig.DeleteByPublicKey(peerInfo.PublicKey)
+	err = p.watcher.Update(*p.wgConfig)
 	if err != nil {
 		return err
 	}
-	p.wgConfig.DeleteByPublicKey(peerInfo.PublicKey)
-	err = p.watcher.Update(*p.wgConfig)
+	err = p.peers.DeleteByName(name)
 	if err != nil {
 		return err
 	}
 	return nil
 }
 
-func (p *peerController) registerRoutes(r *gin.Engine) {
+func (p *PeerController) registerRoutes(r *gin.Engine) {
 	r.GET("/api/peers/v1", func(c *gin.Context) {
 		peerList, err := p.peers.List()
 		if err != nil {
@@ -65,13 +66,30 @@ func (p *peerController) registerRoutes(r *gin.Engine) {
 	})
 }
 
+// PeerControllerSettings is a type for setting up the PeerController
+type PeerControllerSettings func(*PeerController)
+
+// EnablePeerDeletion enables peer deletion
+func EnablePeerDeletion(controller *PeerController) {
+	controller.enablePeerDeletion = true
+}
+
 // NewPeersController allows querying and manipulation of the connected peers
-func NewPeersController(peers pairing.PeerStorage, wgConfig *wg.Config, watcher *wg.Watcher) Controller {
-	return &peerController{
+func NewPeersController(
+	peers pairing.PeerStorage,
+	wgConfig *wg.Config,
+	watcher *wg.Watcher,
+	settings ...PeerControllerSettings,
+) Controller {
+	theController := &PeerController{
 		peers:    peers,
 		wgConfig: wgConfig,
 		watcher:  watcher,
 		// We currently don't have authorization in place, disabling peer deletion
 		enablePeerDeletion: false,
 	}
+	for _, setting := range settings {
+		setting(theController)
+	}
+	return theController
 }

pkg/cmd/flags.go

@@ -17,6 +17,11 @@ var peerStorageDBFlag *cli.StringFlag = &cli.StringFlag{
 	Value: "",
 }
 
+var peerControllerEnableDeletionFlag *cli.BoolFlag = &cli.BoolFlag{
+	Name:  "peer-controller-enable-deletion",
+	Value: false,
+}
+
 var keyStorageDBFlag *cli.StringFlag = &cli.StringFlag{
 	Name:  "key-storage-db",
 	Value: "",

pkg/cmd/server.go

@@ -64,6 +64,7 @@ var serverCommand *cli.Command = &cli.Command{
 		kubernetesLabelsFlag,
 		enableNetworkPoliciesFlag,
 		peerStorageDBFlag,
+		peerControllerEnableDeletionFlag,
 		peerNameFlag,
 		wgAddressFlag,
 		wgSubnetFlag,
@@ -189,9 +190,13 @@ var serverCommand *cli.Command = &cli.Command{
 		)
 		go ss.Start()
 		go func() {
+			peerControllerSettings := []api.PeerControllerSettings{}
+			if c.Bool(peerControllerEnableDeletionFlag.Name) {
+				peerControllerSettings = append(peerControllerSettings, api.EnablePeerDeletion)
+			}
 			err := api.NewAdminAPI([]api.Controller{
 				api.NewAppsController(appsExposedFromRemote),
-				api.NewPeersController(peerStorage, wgConfig, watcher),
+				api.NewPeersController(peerStorage, wgConfig, watcher, peerControllerSettings...),
 			}, c.Bool(debugFlag.Name)).Run(":8082")
 			if err != nil {
 				logrus.Fatalf("Failed to start admin API: %v", err)

pkg/pairing/server.go

@@ -1,6 +1,7 @@
 package pairing
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/glothriel/wormhole/pkg/wg"
@@ -62,7 +63,12 @@ func (s *Server) Start() { // nolint: funlen, gocognit
 		} else {
 			if existingPeer.PublicKey != request.Wireguard.PublicKey {
 				logrus.Errorf(
-					"attempted peering from peer %s: error, public key mismatch", request.Name,
+					"attempted peering from peer `%s`: error, public key mismatch. "+
+						"There's existing peer `%s` with a different public key.",
+					request.Name, existingPeer.Name,
+				)
+				incomingRequest.Err <- NewServerError(
+					errors.New("please see the server log for error details"),
 				)
 				continue
 			}

tests/fixtures.py

@@ -147,7 +147,6 @@ def create(self):
         @retry(tries=60, delay=2)
         def wait_for_cluster_availability():
             Kubectl(self).run(["get", "namespaces"])
-
         wait_for_cluster_availability()
 
     def delete(self):