Merge pull request #3211 from pacevedom/USHIFT-2443

USHIFT-2443: Introduce router IP address listening configuration

cmd/generate-config/config/config-openapi-spec.json

@@ -101,11 +101,19 @@
     "ingress": {
       "type": "object",
       "required": [
+        "listenAddress",
         "ports",
         "routeAdmissionPolicy",
         "status"
       ],
       "properties": {
+        "listenAddress": {
+          "description": "List of IP addresses and NIC names where the router will be listening. The NIC names get translated to all their configured IPs dynamically. Defaults to the configured IPs in the host at MicroShift start.",
+          "type": "array",
+          "items": {
+            "type": "string"
+          }
+        },
         "ports": {
           "type": "object",
           "required": [

docs/user/howto_config.md

@@ -23,6 +23,8 @@ dns:
 etcd:
     memoryLimitMB: 0
 ingress:
+    listenAddress:
+        - ""
     ports:
         http: 0
         https: 0
@@ -71,6 +73,8 @@ dns:
 etcd:
     memoryLimitMB: 0
 ingress:
+    listenAddress:
+        - ""
     ports:
         http: 80
         https: 443

packaging/microshift/config.yaml

@@ -24,6 +24,9 @@ etcd:
     # Set a memory limit on the etcd process; etcd will begin paging memory when it gets to this value. 0 means no limit.
     memoryLimitMB: 0
 ingress:
+    # List of IP addresses and NIC names where the router will be listening. The NIC names get translated to all their configured IPs dynamically. Defaults to the configured IPs in the host at MicroShift start.
+    listenAddress:
+        - ""
     ports:
         # Default router http port. Must be in range 1-65535.
         http: 80

pkg/config/config.go

@@ -10,6 +10,7 @@ import (
 	"net/url"
 	"os"
 	"os/exec"
+	"slices"
 	"strings"
 	"time"
 
@@ -19,13 +20,21 @@ import (
 
 	"github.com/apparentlymart/go-cidr/cidr"
 	"github.com/openshift/microshift/pkg/util"
+	"github.com/vishvananda/netlink"
 )
 
 const (
 	// default DNS resolve file when systemd-resolved is used
 	DefaultSystemdResolvedFile = "/run/systemd/resolve/resolv.conf"
 )
 
+var (
+	defaultRouterForbiddenCIDRs = []string{
+		"127.0.0.0/8",
+		"169.254.0.0/16",
+	}
+)
+
 type Config struct {
 	DNS       DNS           `json:"dns"`
 	Network   Network       `json:"network"`
@@ -213,6 +222,10 @@ func (c *Config) incorporateUserSettings(u *Config) {
 		c.Ingress.Ports.Https = ptr.To[int](*u.Ingress.Ports.Https)
 	}
 
+	if len(u.Ingress.ListenAddress) != 0 {
+		c.Ingress.ListenAddress = u.Ingress.ListenAddress
+	}
+
 	if len(u.ApiServer.NamedCertificates) != 0 {
 		c.ApiServer.NamedCertificates = u.ApiServer.NamedCertificates
 	}
@@ -253,6 +266,21 @@ func (c *Config) updateComputedValues() error {
 		c.ApiServer.AdvertiseAddress = firstValidIP.String()
 	}
 
+	if len(c.Ingress.ListenAddress) == 0 {
+		// If the listenAddress is not configured we need to include all of the host addresses
+		// to preserve previous behavior. However, if the apiserver advertise address has
+		// not been configured, it will do so in a later stage and we also need to
+		// include it here.
+		addresses, err := AllowedListeningIPAddresses()
+		if err != nil {
+			return fmt.Errorf("unable to compute default listening addresses: %v", err)
+		}
+		if !c.ApiServer.SkipInterface {
+			addresses = append(addresses, c.ApiServer.AdvertiseAddress)
+		}
+		c.Ingress.ListenAddress = addresses
+	}
+
 	c.computeLoggingSetting()
 
 	return nil
@@ -337,6 +365,12 @@ func (c *Config) validate() error {
 		return fmt.Errorf("unsupported value %v for ingress.ports.https", *c.Ingress.Ports.Https)
 	}
 
+	if len(c.Ingress.ListenAddress) != 0 {
+		if err := validateRouterListenAddress(c.Ingress.ListenAddress, c.ApiServer.AdvertiseAddress, c.ApiServer.SkipInterface); err != nil {
+			return fmt.Errorf("error validating ingress.listenAddress: %w", err)
+		}
+	}
+
 	return nil
 }
 
@@ -392,3 +426,108 @@ func checkAdvertiseAddressConfigured(advertiseAddress string) error {
 	}
 	return fmt.Errorf("Advertise address: %s not present in any interface", advertiseAddress)
 }
+
+func validateRouterListenAddress(ingressListenAddresses []string, advertiseAddress string, skipInterface bool) error {
+	addresses, err := AllowedListeningIPAddresses()
+	if err != nil {
+		return err
+	}
+	nicNames, err := AllowedNICNames()
+	if err != nil {
+		return err
+	}
+	for _, entry := range ingressListenAddresses {
+		if net.ParseIP(entry) != nil {
+			if entry == advertiseAddress && !skipInterface {
+				continue
+			}
+			if !slices.Contains(addresses, entry) {
+				return fmt.Errorf("IP %v not present in any of the host's interfaces", entry)
+			}
+		} else if slices.Contains(nicNames, entry) {
+			continue
+		} else {
+			return fmt.Errorf("interface %v not present in the host", entry)
+		}
+	}
+	return nil
+}
+
+func getForbiddenIPs() ([]*net.IPNet, error) {
+	banned := make([]*net.IPNet, 0)
+	for _, entry := range defaultRouterForbiddenCIDRs {
+		_, netIP, err := net.ParseCIDR(entry)
+		if err != nil {
+			return nil, err
+		}
+		banned = append(banned, netIP)
+	}
+	return banned, nil
+}
+
+func getHostAddresses() ([]net.IP, error) {
+	handle, err := netlink.NewHandle()
+	if err != nil {
+		return nil, err
+	}
+	links, err := handle.LinkList()
+	if err != nil {
+		return nil, err
+	}
+	addresses := make([]net.IP, 0, len(links)*2)
+	for _, link := range links {
+		// Filter out slave NICs. These include ovs/ovn created interfaces, in case of a restart.
+		if link.Attrs().ParentIndex != 0 || link.Attrs().MasterIndex != 0 {
+			continue
+		}
+		addressList, err := handle.AddrList(link, netlink.FAMILY_V4)
+		if err != nil {
+			return nil, err
+		}
+		for _, addr := range addressList {
+			addresses = append(addresses, addr.IP)
+		}
+	}
+	return addresses, nil
+}
+
+func AllowedListeningIPAddresses() ([]string, error) {
+	bannedAddresses, err := getForbiddenIPs()
+	if err != nil {
+		return nil, err
+	}
+	hostAddresses, err := getHostAddresses()
+	if err != nil {
+		return nil, err
+	}
+	addressList := make([]string, 0, len(hostAddresses))
+	for _, addr := range hostAddresses {
+		skip := false
+		for _, banned := range bannedAddresses {
+			if banned.Contains(addr) {
+				skip = true
+			}
+		}
+		if skip {
+			continue
+		}
+		addressList = append(addressList, addr.String())
+	}
+	return addressList, nil
+}
+
+func AllowedNICNames() ([]string, error) {
+	handle, err := netlink.NewHandle()
+	if err != nil {
+		return nil, err
+	}
+	links, err := handle.LinkList()
+	if err != nil {
+		return nil, err
+	}
+	names := make([]string, 0, len(links))
+	for _, link := range links {
+		names = append(names, link.Attrs().Name)
+	}
+	return names, nil
+}

pkg/config/config_test.go

@@ -95,6 +95,7 @@ func TestGetActiveConfigFromYAML(t *testing.T) {
 				c.Network.ServiceNetwork = []string{"40.30.20.10/16"}
 				c.Network.ServiceNodePortRange = "1024-32767"
 				c.ApiServer.AdvertiseAddress = ""           // force value to be recomputed
+				c.Ingress.ListenAddress = nil               // force value to be recomputed
 				assert.NoError(t, c.updateComputedValues()) // recomputes DNS field
 				return c
 			}(),
@@ -169,6 +170,8 @@ func TestGetActiveConfigFromYAML(t *testing.T) {
 				c := mkDefaultConfig()
 				c.ApiServer.AdvertiseAddress = "127.0.0.1"
 				c.ApiServer.SkipInterface = true
+				c.Ingress.ListenAddress = nil
+				assert.NoError(t, c.updateComputedValues()) // recomputes ingress listenAddress field
 				return c
 			}(),
 		},
@@ -314,6 +317,19 @@ func TestGetActiveConfigFromYAML(t *testing.T) {
 				return c
 			}(),
 		},
+		{
+			name: "ingress-listen-address-nic",
+			config: dedent(`
+            ingress:
+              listenAddress:
+                - lo
+            `),
+			expected: func() *Config {
+				c := mkDefaultConfig()
+				c.Ingress.ListenAddress = []string{"lo"}
+				return c
+			}(),
+		},
 	}
 
 	for _, tt := range ttests {
@@ -437,6 +453,15 @@ func TestValidate(t *testing.T) {
 			}(),
 			expectErr: true,
 		},
+		{
+			name: "ingress-listen-address-ip-forbidden",
+			config: func() *Config {
+				c := mkDefaultConfig()
+				c.Ingress.ListenAddress = []string{"127.0.0.1", "169.255.169.254"}
+				return c
+			}(),
+			expectErr: true,
+		},
 		{
 			name: "ingress-ports-http-invalid-value-2",
 			config: func() *Config {
@@ -446,6 +471,15 @@ func TestValidate(t *testing.T) {
 			}(),
 			expectErr: true,
 		},
+		{
+			name: "ingress-listen-address-ip-not-present",
+			config: func() *Config {
+				c := mkDefaultConfig()
+				c.Ingress.ListenAddress = []string{"1.2.3.4"}
+				return c
+			}(),
+			expectErr: true,
+		},
 		{
 			name: "ingress-ports-https-invalid-value-1",
 			config: func() *Config {
@@ -464,6 +498,15 @@ func TestValidate(t *testing.T) {
 			}(),
 			expectErr: true,
 		},
+		{
+			name: "ingress-listen-address-nic-not-present",
+			config: func() *Config {
+				c := mkDefaultConfig()
+				c.Ingress.ListenAddress = []string{"dummyinterface"}
+				return c
+			}(),
+			expectErr: true,
+		},
 	}
 	for _, tt := range ttests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/config/ingress.go

@@ -13,11 +13,15 @@ type IngressStatusEnum string
 type IngressConfig struct {
 	// Default router status, can be Managed or Removed.
 	// +kubebuilder:default=Managed
-	Status             IngressStatusEnum    `json:"status"`
-	AdmissionPolicy    RouteAdmissionPolicy `json:"routeAdmissionPolicy"`
-	Ports              IngressPortsConfig   `json:"ports"`
-	ServingCertificate []byte               `json:"-"`
-	ServingKey         []byte               `json:"-"`
+	Status          IngressStatusEnum    `json:"status"`
+	AdmissionPolicy RouteAdmissionPolicy `json:"routeAdmissionPolicy"`
+	Ports           IngressPortsConfig   `json:"ports"`
+	// List of IP addresses and NIC names where the router will be listening. The NIC
+	// names get translated to all their configured IPs dynamically. Defaults to the
+	// configured IPs in the host at MicroShift start.
+	ListenAddress      []string `json:"listenAddress"`
+	ServingCertificate []byte   `json:"-"`
+	ServingKey         []byte   `json:"-"`
 }
 
 type RouteAdmissionPolicy struct {