From dda4998261398e6e8c4a8f82f66c93dc342797b8 Mon Sep 17 00:00:00 2001
From: velsinki <40809145+velsinki@users.noreply.github.com>
Date: Sat, 19 Nov 2022 12:06:51 +0100
Subject: [PATCH] Change healtcheck port into address format, redo #1197
 (#1423)

As discussed in the comments in PR #1197. Also add documenation
accordingly.

One thing I'm not sure about is the simple check in health.go if the
address is usable in the GET request or not. From reading
https://pkg.go.dev/net#Dial it seems that the only non-standard address
format that would work in the `net` package but not in a GET url would
likely only be `:port`, as the others listed here are actually also
valid urls:

`For TCP, UDP and IP networks, if the host is empty or a literal
unspecified IP address, as in ":80", "0.0.0.0:80" or "[::]:80" for TCP
and UDP, "", "0.0.0.0" or "::" for IP, the local system is assumed.`

One additional thing I noticed is that while `WOODPECKER_SERVER_ADDR`
and `WOODPECKER_SERVER_ADDR` use the default value format of `:PORT`,
`WOODPECKER_SERVER` actually uses `localhost:9000`. I guess it makes a
bit of sense, considering the server might not be local to the agent,
but it looks a bit inconsistent this way. I don't think it would hurt to
make the `WOODPECKER_HEALTHCHECK_ADDR` in this format too, but then it's
different from the server flags again... :-)
---
 cmd/agent/agent.go                             |  5 ++---
 cmd/agent/flags.go                             | 10 +++++-----
 cmd/agent/health.go                            |  8 +++++++-
 docs/docs/30-administration/15-agent-config.md |  5 +++++
 4 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/cmd/agent/agent.go b/cmd/agent/agent.go
index 6b4af32dc..225db73e0 100644
--- a/cmd/agent/agent.go
+++ b/cmd/agent/agent.go
@@ -17,7 +17,6 @@ package main
 import (
 	"context"
 	"crypto/tls"
-	"fmt"
 	"net/http"
 	"os"
 	"runtime"
@@ -90,8 +89,8 @@ func loop(c *cli.Context) error {
 
 	if c.Bool("healthcheck") {
 		go func() {
-			if err := http.ListenAndServe(fmt.Sprintf(":%d", c.Int("healthcheck-port")), nil); err != nil {
-				log.Error().Msgf("can not listen on port 3000: %v", err)
+			if err := http.ListenAndServe(c.String("healthcheck-addr"), nil); err != nil {
+				log.Error().Msgf("cannot listen on address %s: %v", c.String("healthcheck-addr"), err)
 			}
 		}()
 	}
diff --git a/cmd/agent/flags.go b/cmd/agent/flags.go
index 23c5e4775..8740d71e9 100644
--- a/cmd/agent/flags.go
+++ b/cmd/agent/flags.go
@@ -90,11 +90,11 @@ var flags = []cli.Flag{
 		Usage:   "enable healthcheck endpoint",
 		Value:   true,
 	},
-	&cli.IntFlag{
-		EnvVars: []string{"WOODPECKER_HEALTHCHECK_PORT"},
-		Name:    "healthcheck-port",
-		Usage:   "port used for healthcheck endpoint",
-		Value:   3000,
+	&cli.StringFlag{
+		EnvVars: []string{"WOODPECKER_HEALTHCHECK_ADDR"},
+		Name:    "healthcheck-addr",
+		Usage:   "healthcheck endpoint address",
+		Value:   ":3000",
 	},
 	&cli.DurationFlag{
 		EnvVars: []string{"WOODPECKER_KEEPALIVE_TIME"},
diff --git a/cmd/agent/health.go b/cmd/agent/health.go
index 3645509ef..a80926375 100644
--- a/cmd/agent/health.go
+++ b/cmd/agent/health.go
@@ -18,6 +18,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"strings"
 
 	"github.com/rs/zerolog/log"
 	"github.com/urfave/cli/v2"
@@ -78,7 +79,12 @@ var counter = &agent.State{
 // handles pinging the endpoint and returns an error if the
 // agent is in an unhealthy state.
 func pinger(c *cli.Context) error {
-	resp, err := http.Get("http://localhost:3000/healthz")
+	healthcheckAddress := c.String("healthcheck-addr")
+	if strings.HasPrefix(healthcheckAddress, ":") {
+		// this seems sufficient according to https://pkg.go.dev/net#Dial
+		healthcheckAddress = "localhost" + healthcheckAddress
+	}
+	resp, err := http.Get("http://" + healthcheckAddress + "/healthz")
 	if err != nil {
 		return err
 	}
diff --git a/docs/docs/30-administration/15-agent-config.md b/docs/docs/30-administration/15-agent-config.md
index f8a34e234..4c31af700 100644
--- a/docs/docs/30-administration/15-agent-config.md
+++ b/docs/docs/30-administration/15-agent-config.md
@@ -95,6 +95,11 @@ Configures labels to filter pipeline pick up. Use a list of key-value pairs like
 
 Enable healthcheck endpoint.
 
+### `WOODPECKER_HEALTHCHECK_ADDR`
+> Default: `:3000`
+
+Configures healthcheck endpoint address.
+
 ### `WOODPECKER_KEEPALIVE_TIME`
 > Default: empty