diff --git a/pkg/util/patroni/patroni.go b/pkg/util/patroni/patroni.go index 23260f277..bdd96f048 100644 --- a/pkg/util/patroni/patroni.go +++ b/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 { - return fmt.Sprintf("http://%s:%d", masterPod.Status.PodIP, apiPort) +func apiURL(masterPod *v1.Pod) (string, error) { + 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) } diff --git a/pkg/util/patroni/patroni_test.go b/pkg/util/patroni/patroni_test.go new file mode 100644 index 000000000..388120ae5 --- /dev/null +++ b/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) + } + } + } +}