From 6fe5a1b225e45355d1df8db1bd9b73d56f951b61 Mon Sep 17 00:00:00 2001 From: Pablo Acevedo Montserrat Date: Fri, 27 Jan 2023 11:13:42 +0100 Subject: [PATCH 1/5] USHIFT-716: KAS advertise address match the service network ip Adds an internal IP to kubernetes service. Pods will use an internal IP to connect to KAS, while external clients will use the node IP. Solves ambiguity problem when selecting which certificate to return to a client, based on the destination IP (KAS). Configures the service IP to the loopback interface so that the endpoint slice for kubernetes service matches the same IP. --- pkg/cmd/init.go | 1 + pkg/config/config.go | 22 ++++++----- pkg/controllers/kube-apiserver.go | 65 +++++++++++++++++++++++++++++-- 3 files changed, 75 insertions(+), 13 deletions(-) diff --git a/pkg/cmd/init.go b/pkg/cmd/init.go index 063ad4bd0d..c000082c3a 100644 --- a/pkg/cmd/init.go +++ b/pkg/cmd/init.go @@ -235,6 +235,7 @@ func certSetup(cfg *config.MicroshiftConfig) (*certchains.CertificateChains, err cfg.SubjectAltNames, cfg.NodeName, "api."+cfg.BaseDomain, + cfg.NodeIP, ), }, ), diff --git a/pkg/config/config.go b/pkg/config/config.go index dd94b4cc3b..20e589c4d4 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -59,11 +59,12 @@ type IngressConfig struct { type MicroshiftConfig struct { LogVLevel int `json:"logVLevel"` - SubjectAltNames []string `json:"subjectAltNames"` - NodeName string `json:"nodeName"` - NodeIP string `json:"nodeIP"` - BaseDomain string `json:"baseDomain"` - Cluster ClusterConfig `json:"cluster"` + SubjectAltNames []string `json:"subjectAltNames"` + NodeName string `json:"nodeName"` + NodeIP string `json:"nodeIP"` + AdvertiseKASAddress string `json:"advertiseKASAddress"` + BaseDomain string `json:"baseDomain"` + Cluster ClusterConfig `json:"cluster"` Ingress IngressConfig `json:"-"` } @@ -199,11 +200,12 @@ func NewMicroshiftConfig() *MicroshiftConfig { } return &MicroshiftConfig{ - LogVLevel: 2, - SubjectAltNames: subjectAltNames, - NodeName: nodeName, - NodeIP: nodeIP, - BaseDomain: "example.com", + LogVLevel: 2, + SubjectAltNames: subjectAltNames, + NodeName: nodeName, + NodeIP: nodeIP, + AdvertiseKASAddress: "10.43.0.1", + BaseDomain: "example.com", Cluster: ClusterConfig{ URL: "https://127.0.0.1:6443", ClusterCIDR: "10.42.0.0/16", diff --git a/pkg/controllers/kube-apiserver.go b/pkg/controllers/kube-apiserver.go index fa3415563a..7cf71eda60 100644 --- a/pkg/controllers/kube-apiserver.go +++ b/pkg/controllers/kube-apiserver.go @@ -43,11 +43,13 @@ import ( embedded "github.com/openshift/microshift/assets" "github.com/openshift/microshift/pkg/config" "github.com/openshift/microshift/pkg/util/cryptomaterial" + "github.com/vishvananda/netlink" hostassignmentv1 "k8s.io/kubernetes/openshift-kube-apiserver/admission/route/apis/hostassignment/v1" ) const ( kubeAPIStartupTimeout = 60 + loopbackInterface = "lo" ) var baseKubeAPIServerConfigs = [][]byte{ @@ -71,8 +73,9 @@ type KubeAPIServer struct { verbosity int configureErr error // todo: report configuration errors immediately - masterURL string - servingCAPath string + masterURL string + servingCAPath string + advertiseAddress string } func NewKubeAPIServer(cfg *config.MicroshiftConfig) *KubeAPIServer { @@ -112,10 +115,11 @@ func (s *KubeAPIServer) configure(cfg *config.MicroshiftConfig) error { s.masterURL = cfg.Cluster.URL s.servingCAPath = cryptomaterial.ServiceAccountTokenCABundlePath(certsDir) + s.advertiseAddress = cfg.AdvertiseKASAddress overrides := &kubecontrolplanev1.KubeAPIServerConfig{ APIServerArguments: map[string]kubecontrolplanev1.Arguments{ - "advertise-address": {cfg.NodeIP}, + "advertise-address": {s.advertiseAddress}, "audit-policy-file": {microshiftDataDir + "/resources/kube-apiserver-audit-policies/default.yaml"}, "client-ca-file": {clientCABundlePath}, "etcd-cafile": {cryptomaterial.CACertPath(cryptomaterial.EtcdSignerDir(certsDir))}, @@ -293,11 +297,24 @@ func (s *KubeAPIServer) Run(ctx context.Context, ready chan<- struct{}, stopped return fmt.Errorf("configuration failed: %w", s.configureErr) } + if err := s.addServiceIPLoopback(); err != nil { + return err + } + defer close(stopped) errorChannel := make(chan error, 1) ctx, cancel := context.WithCancel(ctx) defer cancel() + go func() { + select { + case <-ctx.Done(): + if err := s.removeServiceIPLoopback(); err != nil { + klog.Warningf("failed to remove IP from interface: %v", err) + } + } + }() + // run readiness check go func() { err := wait.PollImmediateWithContext(ctx, time.Second, kubeAPIStartupTimeout*time.Second, func(ctx context.Context) (bool, error) { @@ -372,3 +389,45 @@ func (s *KubeAPIServer) Run(ctx context.Context, ready chan<- struct{}, stopped }() return <-errorChannel } + +func (s *KubeAPIServer) addServiceIPLoopback() error { + link, err := netlink.LinkByName(loopbackInterface) + if err != nil { + return err + } + address, err := netlink.ParseAddr(fmt.Sprintf("%s/32", s.advertiseAddress)) + if err != nil { + return err + } + existing, err := netlink.AddrList(link, netlink.FAMILY_ALL) + if err != nil { + return err + } + for _, existingAddress := range existing { + if address.Equal(existingAddress) { + return nil + } + } + return netlink.AddrAdd(link, address) +} + +func (s *KubeAPIServer) removeServiceIPLoopback() error { + link, err := netlink.LinkByName(loopbackInterface) + if err != nil { + return err + } + address, err := netlink.ParseAddr(fmt.Sprintf("%s/32", s.advertiseAddress)) + if err != nil { + return err + } + existing, err := netlink.AddrList(link, netlink.FAMILY_ALL) + if err != nil { + return err + } + for _, existingAddress := range existing { + if address.Equal(existingAddress) { + return netlink.AddrDel(link, address) + } + } + return nil +} From a9b49e6e2bf6f475a9ce55cfd90e9b5e17aedff7 Mon Sep 17 00:00:00 2001 From: Pablo Acevedo Montserrat Date: Fri, 27 Jan 2023 11:53:28 +0100 Subject: [PATCH 2/5] USHIFT-716: add config options to alter KAS advertise address To follow service CIDR configuration options, and to allow forcing a different IP, a new configuration parameter is introduced. This will come in handy when setting extra nodes. Also excludes the node IP in the external serving certificate if the KAS advertise address matches the node IP. --- docs/howto_config.md | 2 +- pkg/cmd/init.go | 22 +++++++++++++----- pkg/config/config.go | 21 +++++++++++++----- pkg/config/config_test.go | 37 ++++++++++++++++++------------- pkg/controllers/kube-apiserver.go | 2 +- 5 files changed, 56 insertions(+), 28 deletions(-) diff --git a/docs/howto_config.md b/docs/howto_config.md index ce635c6528..7edd953575 100644 --- a/docs/howto_config.md +++ b/docs/howto_config.md @@ -39,7 +39,7 @@ The configuration settings alongside with the supported command line arguments a | nodeIP | --node-ip | MICROSHIFT_NODEIP | The IP address of the node, defaults to IP of the default route | hostnameOverride | --hostname-override | MICROSHIFT_HOSTNAMEOVERRIDE | The name of the node, defaults to hostname | logLevel | --v | MICROSHIFT_LOGVLEVEL | Log verbosity (Normal, Debug, Trace, TraceAll) -| subjectAltNames | --subject-alt-names | MICROSHIFT_SUBJECTALTNAMES | Subject Alternative Names for apiserver certificates +| subjectAltNames | --subject-alt-names | MICROSHIFT_SUBJECTALTNAMES | Subject Alternative Names for apiserver certificates ## Default Settings diff --git a/pkg/cmd/init.go b/pkg/cmd/init.go index c000082c3a..ea151892b4 100644 --- a/pkg/cmd/init.go +++ b/pkg/cmd/init.go @@ -70,6 +70,21 @@ func certSetup(cfg *config.MicroshiftConfig) (*certchains.CertificateChains, err return nil, err } + externalCertNames := []string{ + cfg.NodeName, + "api." + cfg.BaseDomain, + } + externalCertNames = append(externalCertNames, cfg.SubjectAltNames...) + // When Kube apiserver advertise address matches the node IP we can not add + // it to the certificates or else the internal pod access to apiserver is + // broken. Because of client-go not using SNI and the way apiserver handles + // which certificate to serve which destination IP, internal pods start + // getting the external certificate, which is signed by a different CA and + // does not match the hostname. + if cfg.KASAdvertiseAddress != cfg.NodeIP { + externalCertNames = append(externalCertNames, cfg.NodeIP) + } + certsDir := cryptomaterial.CertsDirectory(microshiftDataDir) certChains, err := certchains.NewCertificateChains( @@ -231,12 +246,7 @@ func certSetup(cfg *config.MicroshiftConfig) (*certchains.CertificateChains, err Name: "kube-external-serving", ValidityDays: cryptomaterial.ShortLivedCertificateValidityDays, }, - Hostnames: append( - cfg.SubjectAltNames, - cfg.NodeName, - "api."+cfg.BaseDomain, - cfg.NodeIP, - ), + Hostnames: externalCertNames, }, ), diff --git a/pkg/config/config.go b/pkg/config/config.go index 20e589c4d4..812db76986 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -59,10 +59,15 @@ type IngressConfig struct { type MicroshiftConfig struct { LogVLevel int `json:"logVLevel"` - SubjectAltNames []string `json:"subjectAltNames"` - NodeName string `json:"nodeName"` - NodeIP string `json:"nodeIP"` - AdvertiseKASAddress string `json:"advertiseKASAddress"` + SubjectAltNames []string `json:"subjectAltNames"` + NodeName string `json:"nodeName"` + NodeIP string `json:"nodeIP"` + // Kube apiserver advertise address to work around the certificates issue + // when requiring external access using the node IP. This will turn into + // the IP configured in the endpoint slice for kubernetes service. Must be + // a reachable IP from pods. Defaults to service network CIDR first + // address. + KASAdvertiseAddress string `json:"kasAdvertiseAddress"` BaseDomain string `json:"baseDomain"` Cluster ClusterConfig `json:"cluster"` @@ -119,6 +124,9 @@ type DNS struct { type ApiServer struct { // SubjectAltNames added to API server certs SubjectAltNames []string `json:"subjectAltNames"` + // AdvertiseAddress for endpoint slices in kubernetes service. Developer + // only parameter, wont show in show-config commands or docs. + AdvertiseAddress string `json:"advertiseAddress,omitempty"` } type Node struct { @@ -204,7 +212,7 @@ func NewMicroshiftConfig() *MicroshiftConfig { SubjectAltNames: subjectAltNames, NodeName: nodeName, NodeIP: nodeIP, - AdvertiseKASAddress: "10.43.0.1", + KASAdvertiseAddress: "10.43.0.1", BaseDomain: "example.com", Cluster: ClusterConfig{ URL: "https://127.0.0.1:6443", @@ -365,6 +373,9 @@ func (c *MicroshiftConfig) ReadFromConfigFile(configFile string) error { if len(config.ApiServer.SubjectAltNames) > 0 { c.SubjectAltNames = config.ApiServer.SubjectAltNames } + if len(config.ApiServer.AdvertiseAddress) > 0 { + c.KASAdvertiseAddress = config.ApiServer.AdvertiseAddress + } return nil } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 4cbaee8c60..7a2d2037d4 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -58,11 +58,12 @@ func TestCommandLineConfig(t *testing.T) { }{ { config: &MicroshiftConfig{ - LogVLevel: 4, - SubjectAltNames: []string{"node1"}, - NodeName: "node1", - NodeIP: "1.2.3.4", - BaseDomain: "example.com", + LogVLevel: 4, + SubjectAltNames: []string{"node1"}, + NodeName: "node1", + NodeIP: "1.2.3.4", + KASAdvertiseAddress: "6.7.8.9", + BaseDomain: "example.com", Cluster: ClusterConfig{ URL: "https://127.0.0.1:6443", ClusterCIDR: "10.20.30.40/16", @@ -87,6 +88,7 @@ func TestCommandLineConfig(t *testing.T) { flags.String("service-cidr", config.Cluster.ServiceCIDR, "") flags.String("service-node-port-range", config.Cluster.ServiceNodePortRange, "") flags.String("base-domain", config.BaseDomain, "") + flags.String("kas-advertise-address", config.BaseDomain, "") // parse the flags var err error @@ -99,6 +101,7 @@ func TestCommandLineConfig(t *testing.T) { "--service-cidr=" + tt.config.Cluster.ServiceCIDR, "--service-node-port-range=" + tt.config.Cluster.ServiceNodePortRange, "--base-domain=" + tt.config.BaseDomain, + "--kas-advertise-address=" + tt.config.KASAdvertiseAddress, }) if err != nil { t.Errorf("failed to parse command line flags: %s", err) @@ -128,11 +131,12 @@ func TestEnvironmentVariableConfig(t *testing.T) { }{ { desiredMicroShiftConfig: &MicroshiftConfig{ - LogVLevel: 23, - SubjectAltNames: []string{"node1", "node2"}, - NodeName: "node1", - NodeIP: "1.2.3.4", - BaseDomain: "example.com", + LogVLevel: 23, + SubjectAltNames: []string{"node1", "node2"}, + NodeName: "node1", + NodeIP: "1.2.3.4", + KASAdvertiseAddress: "6.7.8.9", + BaseDomain: "example.com", Cluster: ClusterConfig{ URL: "https://cluster.com:4343/endpoint", ClusterCIDR: "10.20.30.40/16", @@ -149,6 +153,7 @@ func TestEnvironmentVariableConfig(t *testing.T) { {"MICROSHIFT_NODENAME", "node1"}, {"MICROSHIFT_SUBJECTALTNAMES", "node1,node2"}, {"MICROSHIFT_NODEIP", "1.2.3.4"}, + {"MICROSHIFT_KASADVERTISEADDRESS", "6.7.8.9"}, {"MICROSHIFT_BASEDOMAIN", "example.com"}, {"MICROSHIFT_CLUSTER_URL", "https://cluster.com:4343/endpoint"}, {"MICROSHIFT_CLUSTER_CLUSTERCIDR", "10.20.30.40/16"}, @@ -158,11 +163,12 @@ func TestEnvironmentVariableConfig(t *testing.T) { }, { desiredMicroShiftConfig: &MicroshiftConfig{ - LogVLevel: 23, - SubjectAltNames: []string{"node1"}, - NodeName: "node1", - NodeIP: "1.2.3.4", - BaseDomain: "another.example.com", + LogVLevel: 23, + SubjectAltNames: []string{"node1"}, + NodeName: "node1", + NodeIP: "1.2.3.4", + KASAdvertiseAddress: "6.7.8.9", + BaseDomain: "another.example.com", Cluster: ClusterConfig{ URL: "https://cluster.com:4343/endpoint", ClusterCIDR: "10.20.30.40/16", @@ -179,6 +185,7 @@ func TestEnvironmentVariableConfig(t *testing.T) { {"MICROSHIFT_NODENAME", "node1"}, {"MICROSHIFT_SUBJECTALTNAMES", "node1"}, {"MICROSHIFT_NODEIP", "1.2.3.4"}, + {"MICROSHIFT_KASADVERTISEADDRESS", "6.7.8.9"}, {"MICROSHIFT_BASEDOMAIN", "another.example.com"}, {"MICROSHIFT_CLUSTER_URL", "https://cluster.com:4343/endpoint"}, {"MICROSHIFT_CLUSTER_CLUSTERCIDR", "10.20.30.40/16"}, diff --git a/pkg/controllers/kube-apiserver.go b/pkg/controllers/kube-apiserver.go index 7cf71eda60..b1afd35084 100644 --- a/pkg/controllers/kube-apiserver.go +++ b/pkg/controllers/kube-apiserver.go @@ -115,7 +115,7 @@ func (s *KubeAPIServer) configure(cfg *config.MicroshiftConfig) error { s.masterURL = cfg.Cluster.URL s.servingCAPath = cryptomaterial.ServiceAccountTokenCABundlePath(certsDir) - s.advertiseAddress = cfg.AdvertiseKASAddress + s.advertiseAddress = cfg.KASAdvertiseAddress overrides := &kubecontrolplanev1.KubeAPIServerConfig{ APIServerArguments: map[string]kubecontrolplanev1.Arguments{ From e036609d6254696dc21fd57a0b43c2d668af8919 Mon Sep 17 00:00:00 2001 From: Pablo Acevedo Montserrat Date: Fri, 27 Jan 2023 17:52:32 +0100 Subject: [PATCH 3/5] USHIFT-716: Default KAS advertise address to follow service CIDR Defaults KAS advertise address to always follow the service CIDR from configuration. If this CIDR is changed before starting MicroShift, the KAS advertise address (and therefore the secondary IP to lo interface) will be in that range. --- pkg/config/config.go | 31 ++++++++++++++++++------------- pkg/config/config_test.go | 37 +++++++++++++++---------------------- 2 files changed, 33 insertions(+), 35 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 812db76986..e6f97cdace 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -208,12 +208,11 @@ func NewMicroshiftConfig() *MicroshiftConfig { } return &MicroshiftConfig{ - LogVLevel: 2, - SubjectAltNames: subjectAltNames, - NodeName: nodeName, - NodeIP: nodeIP, - KASAdvertiseAddress: "10.43.0.1", - BaseDomain: "example.com", + LogVLevel: 2, + SubjectAltNames: subjectAltNames, + NodeName: nodeName, + NodeIP: nodeIP, + BaseDomain: "example.com", Cluster: ClusterConfig{ URL: "https://127.0.0.1:6443", ClusterCIDR: "10.42.0.0/16", @@ -438,6 +437,18 @@ func (c *MicroshiftConfig) ReadAndValidate(configFile string, flags *pflag.FlagS } c.Cluster.DNS = clusterDNS + // If KAS advertise address is not configured then grab it from the service + // CIDR automatically. + if len(c.KASAdvertiseAddress) == 0 { + // unchecked error because this was done when getting cluster DNS + _, svcNet, _ := net.ParseCIDR(c.Cluster.ServiceCIDR) + _, apiServerServiceIP, err := ctrl.ServiceIPRange(*svcNet) + if err != nil { + return fmt.Errorf("error getting apiserver IP: %v", err) + } + c.KASAdvertiseAddress = apiServerServiceIP.String() + } + if len(c.SubjectAltNames) > 0 { // Any entry in SubjectAltNames will be included in the external access certificates. // Any of the hostnames and IPs (except the node IP) listed below conflicts with @@ -468,12 +479,6 @@ func (c *MicroshiftConfig) ReadAndValidate(configFile string, flags *pflag.FlagS } } - // unchecked error because this was done when getting cluster DNS - _, svcNet, _ := net.ParseCIDR(c.Cluster.ServiceCIDR) - _, apiServerServiceIP, err := ctrl.ServiceIPRange(*svcNet) - if err != nil { - return fmt.Errorf("error getting apiserver IP: %v", err) - } if stringSliceContains( c.SubjectAltNames, "kubernetes", @@ -484,7 +489,7 @@ func (c *MicroshiftConfig) ReadAndValidate(configFile string, flags *pflag.FlagS "openshift.default", "openshift.default.svc", "openshift.default.svc.cluster.local", - apiServerServiceIP.String(), + c.KASAdvertiseAddress, ) { return fmt.Errorf("subjectAltNames must not contain apiserver kubernetes service names or IPs") } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 7a2d2037d4..4cbaee8c60 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -58,12 +58,11 @@ func TestCommandLineConfig(t *testing.T) { }{ { config: &MicroshiftConfig{ - LogVLevel: 4, - SubjectAltNames: []string{"node1"}, - NodeName: "node1", - NodeIP: "1.2.3.4", - KASAdvertiseAddress: "6.7.8.9", - BaseDomain: "example.com", + LogVLevel: 4, + SubjectAltNames: []string{"node1"}, + NodeName: "node1", + NodeIP: "1.2.3.4", + BaseDomain: "example.com", Cluster: ClusterConfig{ URL: "https://127.0.0.1:6443", ClusterCIDR: "10.20.30.40/16", @@ -88,7 +87,6 @@ func TestCommandLineConfig(t *testing.T) { flags.String("service-cidr", config.Cluster.ServiceCIDR, "") flags.String("service-node-port-range", config.Cluster.ServiceNodePortRange, "") flags.String("base-domain", config.BaseDomain, "") - flags.String("kas-advertise-address", config.BaseDomain, "") // parse the flags var err error @@ -101,7 +99,6 @@ func TestCommandLineConfig(t *testing.T) { "--service-cidr=" + tt.config.Cluster.ServiceCIDR, "--service-node-port-range=" + tt.config.Cluster.ServiceNodePortRange, "--base-domain=" + tt.config.BaseDomain, - "--kas-advertise-address=" + tt.config.KASAdvertiseAddress, }) if err != nil { t.Errorf("failed to parse command line flags: %s", err) @@ -131,12 +128,11 @@ func TestEnvironmentVariableConfig(t *testing.T) { }{ { desiredMicroShiftConfig: &MicroshiftConfig{ - LogVLevel: 23, - SubjectAltNames: []string{"node1", "node2"}, - NodeName: "node1", - NodeIP: "1.2.3.4", - KASAdvertiseAddress: "6.7.8.9", - BaseDomain: "example.com", + LogVLevel: 23, + SubjectAltNames: []string{"node1", "node2"}, + NodeName: "node1", + NodeIP: "1.2.3.4", + BaseDomain: "example.com", Cluster: ClusterConfig{ URL: "https://cluster.com:4343/endpoint", ClusterCIDR: "10.20.30.40/16", @@ -153,7 +149,6 @@ func TestEnvironmentVariableConfig(t *testing.T) { {"MICROSHIFT_NODENAME", "node1"}, {"MICROSHIFT_SUBJECTALTNAMES", "node1,node2"}, {"MICROSHIFT_NODEIP", "1.2.3.4"}, - {"MICROSHIFT_KASADVERTISEADDRESS", "6.7.8.9"}, {"MICROSHIFT_BASEDOMAIN", "example.com"}, {"MICROSHIFT_CLUSTER_URL", "https://cluster.com:4343/endpoint"}, {"MICROSHIFT_CLUSTER_CLUSTERCIDR", "10.20.30.40/16"}, @@ -163,12 +158,11 @@ func TestEnvironmentVariableConfig(t *testing.T) { }, { desiredMicroShiftConfig: &MicroshiftConfig{ - LogVLevel: 23, - SubjectAltNames: []string{"node1"}, - NodeName: "node1", - NodeIP: "1.2.3.4", - KASAdvertiseAddress: "6.7.8.9", - BaseDomain: "another.example.com", + LogVLevel: 23, + SubjectAltNames: []string{"node1"}, + NodeName: "node1", + NodeIP: "1.2.3.4", + BaseDomain: "another.example.com", Cluster: ClusterConfig{ URL: "https://cluster.com:4343/endpoint", ClusterCIDR: "10.20.30.40/16", @@ -185,7 +179,6 @@ func TestEnvironmentVariableConfig(t *testing.T) { {"MICROSHIFT_NODENAME", "node1"}, {"MICROSHIFT_SUBJECTALTNAMES", "node1"}, {"MICROSHIFT_NODEIP", "1.2.3.4"}, - {"MICROSHIFT_KASADVERTISEADDRESS", "6.7.8.9"}, {"MICROSHIFT_BASEDOMAIN", "another.example.com"}, {"MICROSHIFT_CLUSTER_URL", "https://cluster.com:4343/endpoint"}, {"MICROSHIFT_CLUSTER_CLUSTERCIDR", "10.20.30.40/16"}, From 290209579db43422d80f6b598c002eaaf65705d2 Mon Sep 17 00:00:00 2001 From: Pablo Acevedo Montserrat Date: Mon, 30 Jan 2023 22:47:31 +0100 Subject: [PATCH 4/5] USHIFT-716: avoid interface address add when not default Avoid adding the secondary IP to lo interface when KAS advertise address has been forced to an IP in a different range than service network CIDR. --- pkg/config/config.go | 12 +++++++++--- pkg/controllers/kube-apiserver.go | 25 ++++++++++++++----------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index e6f97cdace..04b5060d63 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -67,9 +67,12 @@ type MicroshiftConfig struct { // the IP configured in the endpoint slice for kubernetes service. Must be // a reachable IP from pods. Defaults to service network CIDR first // address. - KASAdvertiseAddress string `json:"kasAdvertiseAddress"` - BaseDomain string `json:"baseDomain"` - Cluster ClusterConfig `json:"cluster"` + KASAdvertiseAddress string `json:"kasAdvertiseAddress"` + // Determines if kube-apiserver controller should configure the + // KASAdvertiseAddress in the loopback interface. Automatically computed. + SkipKASInterface bool `json:"-"` + BaseDomain string `json:"baseDomain"` + Cluster ClusterConfig `json:"cluster"` Ingress IngressConfig `json:"-"` } @@ -447,6 +450,9 @@ func (c *MicroshiftConfig) ReadAndValidate(configFile string, flags *pflag.FlagS return fmt.Errorf("error getting apiserver IP: %v", err) } c.KASAdvertiseAddress = apiServerServiceIP.String() + c.SkipKASInterface = false + } else { + c.SkipKASInterface = true } if len(c.SubjectAltNames) > 0 { diff --git a/pkg/controllers/kube-apiserver.go b/pkg/controllers/kube-apiserver.go index b1afd35084..35a27f4407 100644 --- a/pkg/controllers/kube-apiserver.go +++ b/pkg/controllers/kube-apiserver.go @@ -76,6 +76,7 @@ type KubeAPIServer struct { masterURL string servingCAPath string advertiseAddress string + skipLoInterface bool } func NewKubeAPIServer(cfg *config.MicroshiftConfig) *KubeAPIServer { @@ -116,6 +117,7 @@ func (s *KubeAPIServer) configure(cfg *config.MicroshiftConfig) error { s.masterURL = cfg.Cluster.URL s.servingCAPath = cryptomaterial.ServiceAccountTokenCABundlePath(certsDir) s.advertiseAddress = cfg.KASAdvertiseAddress + s.skipLoInterface = cfg.SkipKASInterface overrides := &kubecontrolplanev1.KubeAPIServerConfig{ APIServerArguments: map[string]kubecontrolplanev1.Arguments{ @@ -297,23 +299,24 @@ func (s *KubeAPIServer) Run(ctx context.Context, ready chan<- struct{}, stopped return fmt.Errorf("configuration failed: %w", s.configureErr) } - if err := s.addServiceIPLoopback(); err != nil { - return err - } - defer close(stopped) errorChannel := make(chan error, 1) ctx, cancel := context.WithCancel(ctx) defer cancel() - go func() { - select { - case <-ctx.Done(): - if err := s.removeServiceIPLoopback(); err != nil { - klog.Warningf("failed to remove IP from interface: %v", err) - } + if !s.skipLoInterface { + if err := s.addServiceIPLoopback(); err != nil { + return err } - }() + go func() { + select { + case <-ctx.Done(): + if err := s.removeServiceIPLoopback(); err != nil { + klog.Warningf("failed to remove IP from interface: %v", err) + } + } + }() + } // run readiness check go func() { From aacf9e0551a6b17b9e5be8c6ec9dbb1c82ffbca3 Mon Sep 17 00:00:00 2001 From: Pablo Acevedo Montserrat Date: Wed, 1 Feb 2023 23:01:44 +0100 Subject: [PATCH 5/5] USHIFT-716: New service to configure network with KAS address Use a new service to configure loopback interface in the node if required by the KAS advertise address. Kube apiserver service will depend on this one before starting. --- pkg/cmd/run.go | 1 + pkg/controllers/kube-apiserver.go | 62 +-------------- pkg/node/netconfig.go | 120 ++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 61 deletions(-) create mode 100644 pkg/node/netconfig.go diff --git a/pkg/cmd/run.go b/pkg/cmd/run.go index 2e1e41f54a..dd013f4c7b 100644 --- a/pkg/cmd/run.go +++ b/pkg/cmd/run.go @@ -93,6 +93,7 @@ func RunMicroshift(cfg *config.MicroshiftConfig, flags *pflag.FlagSet) error { } m := servicemanager.NewServiceManager() + util.Must(m.AddService(node.NewNetworkConfiguration(cfg))) util.Must(m.AddService(controllers.NewEtcd(cfg))) util.Must(m.AddService(sysconfwatch.NewSysConfWatchController(cfg))) util.Must(m.AddService(controllers.NewKubeAPIServer(cfg))) diff --git a/pkg/controllers/kube-apiserver.go b/pkg/controllers/kube-apiserver.go index 35a27f4407..d1ee3cb126 100644 --- a/pkg/controllers/kube-apiserver.go +++ b/pkg/controllers/kube-apiserver.go @@ -43,13 +43,11 @@ import ( embedded "github.com/openshift/microshift/assets" "github.com/openshift/microshift/pkg/config" "github.com/openshift/microshift/pkg/util/cryptomaterial" - "github.com/vishvananda/netlink" hostassignmentv1 "k8s.io/kubernetes/openshift-kube-apiserver/admission/route/apis/hostassignment/v1" ) const ( kubeAPIStartupTimeout = 60 - loopbackInterface = "lo" ) var baseKubeAPIServerConfigs = [][]byte{ @@ -76,7 +74,6 @@ type KubeAPIServer struct { masterURL string servingCAPath string advertiseAddress string - skipLoInterface bool } func NewKubeAPIServer(cfg *config.MicroshiftConfig) *KubeAPIServer { @@ -88,7 +85,7 @@ func NewKubeAPIServer(cfg *config.MicroshiftConfig) *KubeAPIServer { } func (s *KubeAPIServer) Name() string { return "kube-apiserver" } -func (s *KubeAPIServer) Dependencies() []string { return []string{"etcd"} } +func (s *KubeAPIServer) Dependencies() []string { return []string{"etcd", "network-configuration"} } func (s *KubeAPIServer) configure(cfg *config.MicroshiftConfig) error { s.verbosity = cfg.LogVLevel @@ -117,7 +114,6 @@ func (s *KubeAPIServer) configure(cfg *config.MicroshiftConfig) error { s.masterURL = cfg.Cluster.URL s.servingCAPath = cryptomaterial.ServiceAccountTokenCABundlePath(certsDir) s.advertiseAddress = cfg.KASAdvertiseAddress - s.skipLoInterface = cfg.SkipKASInterface overrides := &kubecontrolplanev1.KubeAPIServerConfig{ APIServerArguments: map[string]kubecontrolplanev1.Arguments{ @@ -304,20 +300,6 @@ func (s *KubeAPIServer) Run(ctx context.Context, ready chan<- struct{}, stopped ctx, cancel := context.WithCancel(ctx) defer cancel() - if !s.skipLoInterface { - if err := s.addServiceIPLoopback(); err != nil { - return err - } - go func() { - select { - case <-ctx.Done(): - if err := s.removeServiceIPLoopback(); err != nil { - klog.Warningf("failed to remove IP from interface: %v", err) - } - } - }() - } - // run readiness check go func() { err := wait.PollImmediateWithContext(ctx, time.Second, kubeAPIStartupTimeout*time.Second, func(ctx context.Context) (bool, error) { @@ -392,45 +374,3 @@ func (s *KubeAPIServer) Run(ctx context.Context, ready chan<- struct{}, stopped }() return <-errorChannel } - -func (s *KubeAPIServer) addServiceIPLoopback() error { - link, err := netlink.LinkByName(loopbackInterface) - if err != nil { - return err - } - address, err := netlink.ParseAddr(fmt.Sprintf("%s/32", s.advertiseAddress)) - if err != nil { - return err - } - existing, err := netlink.AddrList(link, netlink.FAMILY_ALL) - if err != nil { - return err - } - for _, existingAddress := range existing { - if address.Equal(existingAddress) { - return nil - } - } - return netlink.AddrAdd(link, address) -} - -func (s *KubeAPIServer) removeServiceIPLoopback() error { - link, err := netlink.LinkByName(loopbackInterface) - if err != nil { - return err - } - address, err := netlink.ParseAddr(fmt.Sprintf("%s/32", s.advertiseAddress)) - if err != nil { - return err - } - existing, err := netlink.AddrList(link, netlink.FAMILY_ALL) - if err != nil { - return err - } - for _, existingAddress := range existing { - if address.Equal(existingAddress) { - return netlink.AddrDel(link, address) - } - } - return nil -} diff --git a/pkg/node/netconfig.go b/pkg/node/netconfig.go new file mode 100644 index 0000000000..e43813d7f8 --- /dev/null +++ b/pkg/node/netconfig.go @@ -0,0 +1,120 @@ +/* +Copyright © 2023 MicroShift Contributors + +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 node + +import ( + "context" + "fmt" + + "k8s.io/klog/v2" + + "github.com/vishvananda/netlink" + + "github.com/openshift/microshift/pkg/config" +) + +const ( + // Network configuration component name + componentNetworkConfiguration = "network-configuration" + // Interface name where to add service IP + loopbackInterface = "lo" +) + +type NetworkConfiguration struct { + kasAdvertiseAddress string + skipInterfaceConfiguration bool +} + +func NewNetworkConfiguration(cfg *config.MicroshiftConfig) *NetworkConfiguration { + n := &NetworkConfiguration{} + n.configure(cfg) + return n +} + +func (n *NetworkConfiguration) Name() string { return componentNetworkConfiguration } +func (n *NetworkConfiguration) Dependencies() []string { return []string{} } + +func (n *NetworkConfiguration) configure(cfg *config.MicroshiftConfig) { + n.kasAdvertiseAddress = cfg.KASAdvertiseAddress + n.skipInterfaceConfiguration = cfg.SkipKASInterface +} + +func (n *NetworkConfiguration) Run(ctx context.Context, ready chan<- struct{}, stopped chan<- struct{}) error { + defer close(stopped) + + stopChan := make(chan struct{}) + + if !n.skipInterfaceConfiguration { + if err := n.addServiceIPLoopback(); err != nil { + return err + } + go func() { + select { + case <-ctx.Done(): + if err := n.removeServiceIPLoopback(); err != nil { + klog.Warningf("failed to remove IP from interface: %v", err) + } + close(stopChan) + } + }() + } + klog.Infof("%q is ready", n.Name()) + close(ready) + <-stopChan + return ctx.Err() +} + +func (n *NetworkConfiguration) addServiceIPLoopback() error { + link, err := netlink.LinkByName(loopbackInterface) + if err != nil { + return err + } + address, err := netlink.ParseAddr(fmt.Sprintf("%s/32", n.kasAdvertiseAddress)) + if err != nil { + return err + } + existing, err := netlink.AddrList(link, netlink.FAMILY_ALL) + if err != nil { + return err + } + for _, existingAddress := range existing { + if address.Equal(existingAddress) { + return nil + } + } + return netlink.AddrAdd(link, address) +} + +func (n *NetworkConfiguration) removeServiceIPLoopback() error { + link, err := netlink.LinkByName(loopbackInterface) + if err != nil { + return err + } + address, err := netlink.ParseAddr(fmt.Sprintf("%s/32", n.kasAdvertiseAddress)) + if err != nil { + return err + } + existing, err := netlink.AddrList(link, netlink.FAMILY_ALL) + if err != nil { + return err + } + for _, existingAddress := range existing { + if address.Equal(existingAddress) { + return netlink.AddrDel(link, address) + } + } + return nil +}