Skip to content

Commit

Permalink
Support IPv6 in controller GetMetricsURL (kubevirt#3165)
Browse files Browse the repository at this point in the history
Joining hostname+port with string concatenation leads to wrong URLs
when the hostame is an IPv6:

HOST    PORT   Sprintf    CORRECT
::1     8000   ::1:8000   [::1]:8000

To avoid this issue, it's best to use net.JoinHostPort. I added a test
that verifies this fix.

This type of issue can be discovered running the following command:

    golangci-lint run ./... --enable nosprintfhostport

Signed-off-by: Edu Gómez Escandell <egomez@redhat.com>
  • Loading branch information
EduardGomezEscandell authored Apr 8, 2024
1 parent afb2697 commit eb6b76a
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 1 deletion.
4 changes: 3 additions & 1 deletion pkg/controller/common/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"fmt"
"io"
"math"
"net"
"net/http"
"reflect"
"regexp"
Expand Down Expand Up @@ -1641,7 +1642,8 @@ func GetMetricsURL(pod *corev1.Pod) (string, error) {
if err != nil || pod.Status.PodIP == "" {
return "", err
}
url := fmt.Sprintf("https://%s:%d/metrics", pod.Status.PodIP, port)
domain := net.JoinHostPort(pod.Status.PodIP, fmt.Sprint(port))
url := fmt.Sprintf("https://%s/metrics", domain)
return url, nil
}

Expand Down
46 changes: 46 additions & 0 deletions pkg/controller/common/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,52 @@ var _ = Describe("Rebind", func() {
})
})

var _ = Describe("GetMetricsURL", func() {
makePod := func(ip string, withMetrics bool) *v1.Pod {
pod := &v1.Pod{
Status: v1.PodStatus{
PodIP: ip,
},
}

if !withMetrics {
return pod
}

pod.Spec = v1.PodSpec{
Containers: []v1.Container{
{
Ports: []v1.ContainerPort{
{Name: "metrics", ContainerPort: 8080},
},
},
},
}

return pod
}

It("Should suceed with IPv4", func() {
pod := makePod("127.0.0.1", true)
url, err := GetMetricsURL(pod)
Expect(err).ToNot(HaveOccurred())
Expect(url).To(Equal("https://127.0.0.1:8080/metrics"))
})

It("Should suceed with IPv6", func() {
pod := makePod("::1", true)
url, err := GetMetricsURL(pod)
Expect(err).ToNot(HaveOccurred())
Expect(url).To(Equal("https://[::1]:8080/metrics"))
})

It("Should fail when there is no metrics port", func() {
pod := makePod("127.0.0.1", false)
_, err := GetMetricsURL(pod)
Expect(err).To(HaveOccurred())
})
})

func createPvcNoSize(name, ns string, annotations, labels map[string]string) *v1.PersistentVolumeClaim {
return &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Expand Down

0 comments on commit eb6b76a

Please sign in to comment.