Handle IPv6 k8s pods in Patroni URLs (#671)

* Handle IPv6 Patroni URLs

pkg/util/patroni/patroni.go

@@ -5,7 +5,9 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
+	"net"
 	"net/http"
+	"strconv"
 	"time"
 
 	"github.com/sirupsen/logrus"
@@ -43,8 +45,19 @@ func New(logger *logrus.Entry) *Patroni {
 	}
 }
 
-func apiURL(masterPod *v1.Pod) string {
+func apiURL(masterPod *v1.Pod) (string, error) {
-	return fmt.Sprintf("http://%s:%d", masterPod.Status.PodIP, apiPort)
+	ip := net.ParseIP(masterPod.Status.PodIP)
+	if ip == nil {
+		return "", fmt.Errorf("%s is not a valid IP", masterPod.Status.PodIP)
+	}
+	// Sanity check PodIP
+	if ip.To4() == nil {
+		if ip.To16() == nil {
+			// Shouldn't ever get here, but library states it's possible.
+			return "", fmt.Errorf("%s is not a valid IPv4/IPv6 address", masterPod.Status.PodIP)
+		}
+	}
+	return fmt.Sprintf("http://%s", net.JoinHostPort(ip.String(), strconv.Itoa(apiPort))), nil
 }
 
 func (p *Patroni) httpPostOrPatch(method string, url string, body *bytes.Buffer) (err error) {
@@ -88,7 +101,11 @@ func (p *Patroni) Switchover(master *v1.Pod, candidate string) error {
 	if err != nil {
 		return fmt.Errorf("could not encode json: %v", err)
 	}
-	return p.httpPostOrPatch(http.MethodPost, apiURL(master)+failoverPath, buf)
+	apiURLString, err := apiURL(master)
+	if err != nil {
+		return err
+	}
+	return p.httpPostOrPatch(http.MethodPost, apiURLString+failoverPath, buf)
 }
 
 //TODO: add an option call /patroni to check if it is necessary to restart the server
@@ -100,5 +117,9 @@ func (p *Patroni) SetPostgresParameters(server *v1.Pod, parameters map[string]st
 	if err != nil {
 		return fmt.Errorf("could not encode json: %v", err)
 	}
-	return p.httpPostOrPatch(http.MethodPatch, apiURL(server)+configPath, buf)
+	apiURLString, err := apiURL(server)
+	if err != nil {
+		return err
+	}
+	return p.httpPostOrPatch(http.MethodPatch, apiURLString+configPath, buf)
 }

pkg/util/patroni/patroni_test.go

@@ -0,0 +1,74 @@
+package patroni
+
+import (
+	"errors"
+	"fmt"
+	"k8s.io/api/core/v1"
+	"testing"
+)
+
+func newMockPod(ip string) *v1.Pod {
+	return &v1.Pod{
+		Status: v1.PodStatus{
+			PodIP: ip,
+		},
+	}
+}
+
+func TestApiURL(t *testing.T) {
+	var testTable = []struct {
+		podIP            string
+		expectedResponse string
+		expectedError    error
+	}{
+		{
+			"127.0.0.1",
+			fmt.Sprintf("http://127.0.0.1:%d", apiPort),
+			nil,
+		},
+		{
+			"0000:0000:0000:0000:0000:0000:0000:0001",
+			fmt.Sprintf("http://[::1]:%d", apiPort),
+			nil,
+		},
+		{
+			"::1",
+			fmt.Sprintf("http://[::1]:%d", apiPort),
+			nil,
+		},
+		{
+			"",
+			"",
+			errors.New(" is not a valid IP"),
+		},
+		{
+			"foobar",
+			"",
+			errors.New("foobar is not a valid IP"),
+		},
+		{
+			"127.0.1",
+			"",
+			errors.New("127.0.1 is not a valid IP"),
+		},
+		{
+			":::",
+			"",
+			errors.New("::: is not a valid IP"),
+		},
+	}
+	for _, test := range testTable {
+		resp, err := apiURL(newMockPod(test.podIP))
+		if resp != test.expectedResponse {
+			t.Errorf("expected response %v does not match the actual %v", test.expectedResponse, resp)
+		}
+		if err != test.expectedError {
+			if err == nil || test.expectedError == nil {
+				t.Errorf("expected error '%v' does not match the actual error '%v'", test.expectedError, err)
+			}
+			if err != nil && test.expectedError != nil && err.Error() != test.expectedError.Error() {
+				t.Errorf("expected error '%v' does not match the actual error '%v'", test.expectedError, err)
+			}
+		}
+	}
+}