Merge pull request #5393 from juanluisvaladas/etcd-cplb-4664

Ensure iface.FirstPublicAddress isn't a secondary IP
k0sproject · Jan 8, 2025 · 0a8fafb · 0a8fafb
2 parents 0147fbd + 5a69092
commit 0a8fafb
Show file tree

Hide file tree

Showing 4 changed files with 116 additions and 30 deletions.
diff --git a/internal/pkg/iface/iface.go b/internal/pkg/iface/iface.go
@@ -82,19 +82,20 @@ func FirstPublicAddress() (string, error) {
 		case strings.HasPrefix(i.Name, "cali"):
 			continue
 		}
-		addresses, err := i.Addrs()
+
+		addresses, err := interfaceAddrs(i)
 		if err != nil {
-			logrus.Warnf("failed to get addresses for interface %s: %s", i.Name, err.Error())
+			logrus.WithError(err).Warn("Skipping network interface ", i.Name)
 			continue
 		}
-		for _, a := range addresses {
+		for a := range addresses {
 			// check the address type and skip if loopback
-			if ipnet, ok := a.(*net.IPNet); ok && !ipnet.IP.IsLoopback() {
-				if ipnet.IP.To4() != nil {
-					return ipnet.IP.String(), nil
+			if a != nil && !a.IP.IsLoopback() {
+				if a.IP.To4() != nil {
+					return a.IP.String(), nil
 				}
-				if ipnet.IP.To16() != nil && ipv6addr == "" {
-					ipv6addr = ipnet.IP.String()
+				if a.IP.To16() != nil && ipv6addr == "" {
+					ipv6addr = a.IP.String()
 				}
 			}
 		}

diff --git a/internal/pkg/iface/iface_linux.go b/internal/pkg/iface/iface_linux.go
@@ -0,0 +1,54 @@
+/*
+Copyright 2025 k0s authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package iface
+
+import (
+	"fmt"
+	"iter"
+	"net"
+
+	"github.com/vishvananda/netlink"
+	"golang.org/x/sys/unix"
+)
+
+func interfaceAddrs(i net.Interface) (iter.Seq[*net.IPNet], error) {
+	link, err := netlink.LinkByName(i.Name)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get link by name: %w", err)
+	}
+
+	addresses, err := netlink.AddrList(link, netlink.FAMILY_ALL)
+	if err != nil {
+		return nil, fmt.Errorf("failed to list IP addresses: %w", err)
+	}
+
+	return func(yield func(*net.IPNet) bool) {
+		for _, a := range addresses {
+			// skip secondary addresses. This is to avoid returning VIPs as the public address
+			// https://github.com/k0sproject/k0s/issues/4664
+			if a.Flags&unix.IFA_F_SECONDARY != 0 {
+				continue
+			}
+
+			if a.IPNet != nil {
+				if !yield(a.IPNet) {
+					return
+				}
+			}
+		}
+	}, nil
+}
diff --git a/internal/pkg/iface/iface_other.go b/internal/pkg/iface/iface_other.go
@@ -0,0 +1,42 @@
+//go:build !linux
+
+/*
+Copyright 2025 k0s authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package iface
+
+import (
+	"fmt"
+	"iter"
+	"net"
+)
+
+func interfaceAddrs(i net.Interface) (iter.Seq[*net.IPNet], error) {
+	addresses, err := i.Addrs()
+	if err != nil {
+		return nil, fmt.Errorf("failed to list interface addresses: %w", err)
+	}
+
+	return func(yield func(*net.IPNet) bool) {
+		for _, a := range addresses {
+			if ipnet, ok := a.(*net.IPNet); ok && ipnet != nil {
+				if !yield(ipnet) {
+					return
+				}
+			}
+		}
+	}, nil
+}
diff --git a/inttest/backup/backup_test.go b/inttest/backup/backup_test.go
@@ -76,11 +76,8 @@ func (s *BackupSuite) TestK0sGetsUp() {
 	s.Require().NoError(s.InitController(1, token, "--config=/tmp/k0s.yaml"))
 	s.Require().NoError(s.WaitJoinAPI(s.ControllerNode(1)))
 
-	err = s.WaitForNodeReady(s.WorkerNode(0), kc)
-	s.Require().NoError(err)
-
-	err = s.WaitForNodeReady(s.WorkerNode(1), kc)
-	s.Require().NoError(err)
+	s.Require().NoError(s.WaitForNodeReady(s.WorkerNode(0), kc))
+	s.Require().NoError(s.WaitForNodeReady(s.WorkerNode(1), kc))
 
 	s.AssertSomeKubeSystemPods(kc)
 
@@ -91,8 +88,6 @@ func (s *BackupSuite) TestK0sGetsUp() {
 
 	snapshot := s.makeSnapshot(kc)
 
-	s.Require().NoError(err)
-
 	s.Require().NoError(s.StopController(s.ControllerNode(0)))
 	_ = s.StopController(s.ControllerNode(1)) // No error check as k0s might have actually exited since etcd is not really happy
 
@@ -106,16 +101,10 @@ func (s *BackupSuite) TestK0sGetsUp() {
 	// Join the second controller as normally
 	s.Require().NoError(s.InitController(1, "--enable-worker", token))
 
-	s.Require().NoError(err)
-
-	err = s.WaitForNodeReady(s.WorkerNode(0), kc)
-	s.Require().NoError(err)
-
-	err = s.WaitForNodeReady(s.WorkerNode(1), kc)
-	s.Require().NoError(err)
+	s.Require().NoError(s.WaitForNodeReady(s.WorkerNode(0), kc))
+	s.Require().NoError(s.WaitForNodeReady(s.WorkerNode(1), kc))
 
 	snapshotAfterBackup := s.makeSnapshot(kc)
-	s.Require().NoError(err)
 	// Matching object UIDs after restore guarantees we got the full state restored
 	s.Require().Equal(snapshot, snapshotAfterBackup)
 
@@ -191,7 +180,7 @@ func (s *BackupSuite) takeBackup() error {
 	}
 	defer ssh.Disconnect()
 
-	out, err := ssh.ExecWithOutput(s.Context(), "/usr/local/bin/k0s backup --save-path /root/")
+	out, err := ssh.ExecWithOutput(s.Context(), "/usr/local/bin/k0s backup --debug --save-path /root/")
 	if !s.NoErrorf(err, "backup failed with output: %s", out) {
 		return err
 	}
@@ -206,7 +195,7 @@ func (s *BackupSuite) takeBackupStdout() error {
 	}
 	defer ssh.Disconnect()
 
-	out, err := ssh.ExecWithOutput(s.Context(), "/usr/local/bin/k0s backup --save-path - > backup.tar.gz")
+	out, err := ssh.ExecWithOutput(s.Context(), "/usr/local/bin/k0s backup --debug --save-path - > backup.tar.gz")
 	if !s.NoErrorf(err, "backup failed with output: %s", out) {
 		return err
 	}
@@ -216,7 +205,7 @@ func (s *BackupSuite) takeBackupStdout() error {
 		return err
 	}
 
-	s.T().Logf("backup taken succesfully with output:\n%s", out)
+	s.T().Logf("backup taken successfully with output:\n%s", out)
 	return nil
 }
 
@@ -229,11 +218,11 @@ func (s *BackupSuite) restoreBackup() error {
 
 	s.T().Log("restoring controller from file")
 
-	out, err := ssh.ExecWithOutput(s.Context(), "/usr/local/bin/k0s restore $(ls /root/k0s_backup_*.tar.gz)")
+	out, err := ssh.ExecWithOutput(s.Context(), "/usr/local/bin/k0s restore --debug $(ls /root/k0s_backup_*.tar.gz)")
 	if !s.NoErrorf(err, "restore failed with output: %s", out) {
 		return err
 	}
-	s.T().Logf("restored succesfully with output:\n%s", out)
+	s.T().Logf("restored successfully with output:\n%s", out)
 
 	return nil
 }
@@ -247,11 +236,11 @@ func (s *BackupSuite) restoreBackupStdin() error {
 
 	s.T().Log("restoring controller from stdin")
 
-	out, err := ssh.ExecWithOutput(s.Context(), "cat backup.tar.gz | /usr/local/bin/k0s restore -")
+	out, err := ssh.ExecWithOutput(s.Context(), "cat backup.tar.gz | /usr/local/bin/k0s restore --debug -")
 	if !s.NoErrorf(err, "restore failed with output: %s", out) {
 		return err
 	}
-	s.T().Logf("restored succesfully with output:\n%s", out)
+	s.T().Logf("restored successfully with output:\n%s", out)
 
 	return nil
 }