From bac03e19f154c3ebb40faac9e140016a3e318bbf 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 aa003343c2b1d794c0e5716d8f58cf1350f4d345 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 e847bc1954f38b4e245bbfaa8e5af0d86407a657 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 a6ca7f5458aba1e1ff06ea6fed0c33a62ad6f6a7 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 c4c11cc3fc7ae7656e4b04c3f13e326a4e98615a 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 +}