github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-20 10:20:51 +00:00
Code Issues Projects Releases Wiki Activity

Replace use of Sprintf with net.JoinHostPort

Browse Source 
On IPv6 clusters, one of the most frequent problems I encounter is
assumptions that one can build a URL with a host and port simply by
using Sprintf, like this:

```go
fmt.Sprintf("http://%s:%d/foo", host, port)
```

When `host` is an IPv6 address, this produces an invalid URL as it must
be bracketed, like this:

```
http://[2001:4860:4860::8888]:9443
```

This change fixes the occurences of joining a host and port with the
purpose built `net.JoinHostPort` function.

I encounter this problem often enough that I started to [write a linter
for it](https://github.com/stbenjam/go-sprintf-host-port).  I don't
think the linter is quite ready for wide use yet, but I did run it
against the Kube codebase and found these.  While the host portion in
some of these changes may always be an FQDN or IPv4 IP today, it's an
easy thing that can break later on.
This commit is contained in:
Stephen Benjamin 2022-04-06 18:16:02 -04:00
parent 3bef1692ef
commit b351745c1c
8 changed files with 28 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
pkg/volume/portworx/portworx.go
View File

@ -18,7 +18,9 @@ package portworx
import (
"fmt"
"net"
"os"
"strconv"
"k8s.io/klog/v2"
"k8s.io/mount-utils"
@ -71,7 +73,7 @@ func (plugin *portworxVolumePlugin) IsMigratedToCSI() bool {
func (plugin *portworxVolumePlugin) Init(host volume.VolumeHost) error {
client, err := volumeclient.NewDriverClient(
fmt.Sprintf("http://%s:%d", host.GetHostName(), osdMgmtDefaultPort),
fmt.Sprintf("http://%s", net.JoinHostPort(host.GetHostName(), strconv.Itoa(osdMgmtDefaultPort))),
pxdDriverName, osdDriverVersion, pxDriverName)
if err != nil {
return err

5
pkg/volume/portworx/portworx_util.go
View File

@ -19,6 +19,8 @@ package portworx
import (
"context"
"fmt"
"net"
"strconv"
osdapi "github.com/libopenstorage/openstorage/api"
osdclient "github.com/libopenstorage/openstorage/api/client"
@ -30,6 +32,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
volumehelpers "k8s.io/cloud-provider/volume/helpers"
"k8s.io/klog/v2"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/volume"
)
@ -265,7 +268,7 @@ func isClientValid(client *osdclient.Client) (bool, error) {
}
func createDriverClient(hostname string, port int32) (*osdclient.Client, error) {
client, err := volumeclient.NewDriverClient(fmt.Sprintf("http://%s:%d", hostname, port),
client, err := volumeclient.NewDriverClient(fmt.Sprintf("http://%s", net.JoinHostPort(hostname, strconv.Itoa(int(port)))),
pxdDriverName, osdDriverVersion, pxDriverName)
if err != nil {
return nil, err

3
test/e2e/storage/vsphere/connection.go
View File

@ -19,6 +19,7 @@ package vsphere
import (
"context"
"fmt"
"net"
neturl "net/url"
"sync"
@ -72,7 +73,7 @@ func Connect(ctx context.Context, vs *VSphere) error {
// NewClient creates a new client for vSphere connection
func NewClient(ctx context.Context, vs *VSphere) (*govmomi.Client, error) {
url, err := neturl.Parse(fmt.Sprintf("https://%s:%s/sdk", vs.Config.Hostname, vs.Config.Port))
url, err := neturl.Parse(fmt.Sprintf("https://%s/sdk", net.JoinHostPort(vs.Config.Hostname, vs.Config.Port)))
if err != nil {
klog.Errorf("Failed to parse URL: %s. err: %+v", url, err)
return nil, err

6
test/e2e/upgrades/apps/cassandra.go
View File

@ -21,6 +21,7 @@ import (
"encoding/json"
"fmt"
"io"
"net"
"net/http"
"path/filepath"
"sync"
@ -32,6 +33,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/version"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/kubernetes/test/e2e/framework"
e2estatefulset "k8s.io/kubernetes/test/e2e/framework/statefulset"
e2etestfiles "k8s.io/kubernetes/test/e2e/framework/testfiles"
@ -118,7 +120,7 @@ func (t *CassandraUpgradeTest) Setup(f *framework.Framework) {
// listUsers gets a list of users from the db via the tester service.
func (t *CassandraUpgradeTest) listUsers() ([]string, error) {
r, err := http.Get(fmt.Sprintf("http://%s:8080/list", t.ip))
r, err := http.Get(fmt.Sprintf("http://%s/list", net.JoinHostPort(t.ip, "8080")))
if err != nil {
return nil, err
}
@ -140,7 +142,7 @@ func (t *CassandraUpgradeTest) listUsers() ([]string, error) {
// addUser adds a user to the db via the tester services.
func (t *CassandraUpgradeTest) addUser(name string) error {
val := map[string][]string{"name": {name}}
r, err := http.PostForm(fmt.Sprintf("http://%s:8080/add", t.ip), val)
r, err := http.PostForm(fmt.Sprintf("http://%s/add", net.JoinHostPort(t.ip, "8080")), val)
if err != nil {
return err
}

6
test/e2e/upgrades/apps/etcd.go
View File

@ -21,6 +21,7 @@ import (
"encoding/json"
"fmt"
"io"
"net"
"net/http"
"path/filepath"
"sync"
@ -32,6 +33,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/version"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/kubernetes/test/e2e/framework"
e2estatefulset "k8s.io/kubernetes/test/e2e/framework/statefulset"
e2etestfiles "k8s.io/kubernetes/test/e2e/framework/testfiles"
@ -112,7 +114,7 @@ func (t *EtcdUpgradeTest) Setup(f *framework.Framework) {
}
func (t *EtcdUpgradeTest) listUsers() ([]string, error) {
r, err := http.Get(fmt.Sprintf("http://%s:8080/list", t.ip))
r, err := http.Get(fmt.Sprintf("http://%s/list", net.JoinHostPort(t.ip, "8080")))
if err != nil {
return nil, err
}
@ -133,7 +135,7 @@ func (t *EtcdUpgradeTest) listUsers() ([]string, error) {
func (t *EtcdUpgradeTest) addUser(name string) error {
val := map[string][]string{"name": {name}}
r, err := http.PostForm(fmt.Sprintf("http://%s:8080/add", t.ip), val)
r, err := http.PostForm(fmt.Sprintf("http://%s/add", net.JoinHostPort(t.ip, "8080")), val)
if err != nil {
return err
}

6
test/e2e/upgrades/apps/mysql.go
View File

@ -21,6 +21,7 @@ import (
"encoding/json"
"fmt"
"io"
"net"
"net/http"
"path/filepath"
"strconv"
@ -32,6 +33,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/version"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/kubernetes/test/e2e/framework"
e2estatefulset "k8s.io/kubernetes/test/e2e/framework/statefulset"
e2etestfiles "k8s.io/kubernetes/test/e2e/framework/testfiles"
@ -181,7 +183,7 @@ func (t *MySQLUpgradeTest) Teardown(f *framework.Framework) {
func (t *MySQLUpgradeTest) addName(name string) error {
val := map[string][]string{"name": {name}}
t.nextWrite++
r, err := http.PostForm(fmt.Sprintf("http://%s:8080/addName", t.ip), val)
r, err := http.PostForm(fmt.Sprintf("http://%s/addName", net.JoinHostPort(t.ip, "8080")), val)
if err != nil {
return err
}
@ -199,7 +201,7 @@ func (t *MySQLUpgradeTest) addName(name string) error {
// countNames checks to make sure the values in testing.users are available, and returns
// the count of them.
func (t *MySQLUpgradeTest) countNames() (int, error) {
r, err := http.Get(fmt.Sprintf("http://%s:8080/countNames", t.ip))
r, err := http.Get(fmt.Sprintf("http://%s/countNames", net.JoinHostPort(t.ip, "8080")))
if err != nil {
return 0, err
}

5
test/e2e/windows/service.go
View File

@ -18,9 +18,12 @@ package windows
import (
"fmt"
"net"
"strconv"
v1 "k8s.io/api/core/v1"
clientset "k8s.io/client-go/kubernetes"
"k8s.io/kubernetes/test/e2e/framework"
e2enode "k8s.io/kubernetes/test/e2e/framework/node"
e2eservice "k8s.io/kubernetes/test/e2e/framework/service"
@ -77,7 +80,7 @@ var _ = SIGDescribe("Services", func() {
framework.ExpectEqual(testPod.Spec.NodeSelector["kubernetes.io/os"], "windows")
ginkgo.By(fmt.Sprintf("checking connectivity Pod to curl http://%s:%d", nodeIP, nodePort))
assertConsistentConnectivity(f, testPod.ObjectMeta.Name, windowsOS, windowsCheck(fmt.Sprintf("http://%s:%d", nodeIP, nodePort)))
assertConsistentConnectivity(f, testPod.ObjectMeta.Name, windowsOS, windowsCheck(fmt.Sprintf("http://%s", net.JoinHostPort(nodeIP, strconv.Itoa(nodePort)))))
})

4
test/e2e_node/util.go
View File

@ -23,9 +23,11 @@ import (
"flag"
"fmt"
"io"
"net"
"net/http"
"os/exec"
"regexp"
"strconv"
"strings"
"time"
@ -83,7 +85,7 @@ func getNodeSummary() (*stats.Summary, error) {
if err != nil {
return nil, fmt.Errorf("failed to get current kubelet config")
}
req, err := http.NewRequest("GET", fmt.Sprintf("http://%s:%d/stats/summary", kubeletConfig.Address, kubeletConfig.ReadOnlyPort), nil)
req, err := http.NewRequest("GET", fmt.Sprintf("http://%s/stats/summary", net.JoinHostPort(kubeletConfig.Address, strconv.Itoa(int(kubeletConfig.ReadOnlyPort)))), nil)
if err != nil {
return nil, fmt.Errorf("failed to build http request: %v", err)
}