From b25b56b4f53e6dcff0bed80de9726224bab4f2a3 Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Tue, 12 Nov 2024 16:30:44 +0100 Subject: [PATCH] Enable nosprintfhostport linter ... and fix lints on the way. Inline the IsIPV6String function, as it had only one use left, which was inlined as a one-liner. Signed-off-by: Tom Wieczorek --- .golangci.yml | 29 +++++++++++++------------ inttest/customports/customports_test.go | 7 ++++-- pkg/apis/k0s/v1beta1/api.go | 14 +++--------- pkg/apis/k0s/v1beta1/network.go | 9 ++++---- 4 files changed, 27 insertions(+), 32 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 91bf5f6d21d7..2b75b3a143c9 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -10,20 +10,21 @@ run: linters: enable: - - copyloopvar # Detects unnecessary copies of loop variables for Go >= 1.22 - - depguard # Checks if package imports are in a list of acceptable packages - - dupword # Finds word repetitions - - errorlint # Find code that will cause problems with Go's error wrapping scheme - - gofmt # Checks whether code was gofmt-ed - - goheader # Checks is file headers matche a given pattern - - intrange # Checking for loops that could use an integer range - - perfsprint # Checks for faster fmt.Sprintf alternatives - - nolintlint # Find ill-formed or insufficiently explained nolint directives - - revive # Stricter drop-in replacement for golint - - testifylint # Checks usage of github.com/stretchr/testify - - unconvert # Checks for unnecessary type conversions - - unparam # Checks for unused function parameters - - usestdlibvars # Checks for things that are provided by the standard library + - copyloopvar # Detects unnecessary copies of loop variables for Go >= 1.22 + - depguard # Checks if package imports are in a list of acceptable packages + - dupword # Finds word repetitions + - errorlint # Find code that will cause problems with Go's error wrapping scheme + - gofmt # Checks whether code was gofmt-ed + - goheader # Checks is file headers matche a given pattern + - intrange # Checking for loops that could use an integer range + - perfsprint # Checks for faster fmt.Sprintf alternatives + - nolintlint # Find ill-formed or insufficiently explained nolint directives + - nosprintfhostport # Detects misuses of Sprintf to construct hosts with ports in a URL + - revive # Stricter drop-in replacement for golint + - testifylint # Checks usage of github.com/stretchr/testify + - unconvert # Checks for unnecessary type conversions + - unparam # Checks for unused function parameters + - usestdlibvars # Checks for things that are provided by the standard library linters-settings: depguard: diff --git a/inttest/customports/customports_test.go b/inttest/customports/customports_test.go index c973d0ca5c7e..9ff005449485 100644 --- a/inttest/customports/customports_test.go +++ b/inttest/customports/customports_test.go @@ -18,9 +18,11 @@ package customports import ( "bytes" - "fmt" "html/template" + "net" + "net/url" "os" + "strconv" "testing" "github.com/k0sproject/k0s/inttest/common" @@ -138,12 +140,13 @@ func (s *customPortsSuite) TestControllerJoinsWithCustomPort() { // https://github.com/k0sproject/k0s/issues/1202 s.Run("kubeconfigIncludesExternalAddress", func() { + expectedURL := url.URL{Scheme: "https", Host: net.JoinHostPort(ipAddress, strconv.Itoa(kubeAPIPort))} ssh, err := s.SSH(s.Context(), s.ControllerNode(0)) s.Require().NoError(err) defer ssh.Disconnect() out, err := ssh.ExecWithOutput(s.Context(), "/usr/local/bin/k0s kubeconfig create user | awk '$1 == \"server:\" {print $2}'") s.Require().NoError(err) - s.Require().Equal(fmt.Sprintf("https://%s:%d", ipAddress, kubeAPIPort), out) + s.Require().Equal(expectedURL.String(), out) }) } diff --git a/pkg/apis/k0s/v1beta1/api.go b/pkg/apis/k0s/v1beta1/api.go index cb043fb70152..ebfa7e4d181f 100644 --- a/pkg/apis/k0s/v1beta1/api.go +++ b/pkg/apis/k0s/v1beta1/api.go @@ -18,8 +18,9 @@ package v1beta1 import ( "encoding/json" - "fmt" "net" + "net/url" + "strconv" "github.com/k0sproject/k0s/internal/pkg/iface" "github.com/k0sproject/k0s/internal/pkg/stringslice" @@ -84,12 +85,6 @@ func (a *APISpec) APIAddressURL() string { return a.getExternalURIForPort(a.Port) } -// IsIPv6String returns if ip is IPv6. -func IsIPv6String(ip string) bool { - netIP := net.ParseIP(ip) - return netIP != nil && netIP.To4() == nil -} - // K0sControlPlaneAPIAddress returns the controller join APIs address func (a *APISpec) K0sControlPlaneAPIAddress() string { return a.getExternalURIForPort(a.K0sAPIPort) @@ -100,10 +95,7 @@ func (a *APISpec) getExternalURIForPort(port int) string { if a.ExternalAddress != "" { addr = a.ExternalAddress } - if IsIPv6String(addr) { - return fmt.Sprintf("https://[%s]:%d", addr, port) - } - return fmt.Sprintf("https://%s:%d", addr, port) + return (&url.URL{Scheme: "https", Host: net.JoinHostPort(addr, strconv.Itoa(port))}).String() } // Sans return the given SANS plus all local addresses and externalAddress if given diff --git a/pkg/apis/k0s/v1beta1/network.go b/pkg/apis/k0s/v1beta1/network.go index 893b4b84e98b..70ffcad91ab4 100644 --- a/pkg/apis/k0s/v1beta1/network.go +++ b/pkg/apis/k0s/v1beta1/network.go @@ -216,11 +216,10 @@ func (n *Network) BuildServiceCIDR(addr string) string { if !n.DualStack.Enabled { return n.ServiceCIDR } - // because in the dual-stack mode k8s - // relies on the ordering of the given CIDRs - // we need to first give family on which - // api server listens - if IsIPv6String(addr) { + // Because Kubernetes relies on the order of the given CIDRs in dual-stack + // mode, the CIDR whose version matches the version of the IP address the + // API server is listening on must be specified first. + if ip := net.ParseIP(addr); ip != nil && ip.To4() == nil { return n.DualStack.IPv6ServiceCIDR + "," + n.ServiceCIDR } return n.ServiceCIDR + "," + n.DualStack.IPv6ServiceCIDR