From bada552348d0e3d508fcd1a19fa1d774f817101d Mon Sep 17 00:00:00 2001 From: Brandon Ewing Date: Tue, 10 Dec 2024 13:50:23 -0600 Subject: [PATCH 1/2] Add flag to support ClusterIP exposed CoreDNS Some environments (Calico bare-metal, etc) may allow direct client reachability to the Service CIDR, bypassing the need to assign and use LoadBalancerIPs. A new config flag and environment variable allows selection/return of the ClusterIPs in the assistant rather than externally assigned LoadBalancer IPs. Signed-off-by: Brandon Ewing --- controllers/depresolver/depresolver.go | 2 ++ controllers/mocks/assistant_mock.go | 15 +++++++++++ controllers/providers/assistant/assistant.go | 2 ++ controllers/providers/assistant/gslb.go | 28 ++++++++++++++++++-- controllers/providers/dns/external.go | 6 ++++- controllers/providers/dns/infoblox.go | 6 ++++- 6 files changed, 55 insertions(+), 4 deletions(-) diff --git a/controllers/depresolver/depresolver.go b/controllers/depresolver/depresolver.go index d4b20f1fe7..fd3ad39d23 100644 --- a/controllers/depresolver/depresolver.go +++ b/controllers/depresolver/depresolver.go @@ -144,6 +144,8 @@ type Config struct { Infoblox Infoblox // CoreDNSExposed flag CoreDNSExposed bool `env:"COREDNS_EXPOSED, default=false"` + // CoreDNSClusterIPs flag + CoreDNSClusterIPs bool `env:"COREDNS_CLUSTERIPS, default=false"` // Log configuration Log Log // MetricsAddress in format address:port where address can be empty, IP address, or hostname, default: 0.0.0.0:8080 diff --git a/controllers/mocks/assistant_mock.go b/controllers/mocks/assistant_mock.go index ffa837c62c..85722b148e 100644 --- a/controllers/mocks/assistant_mock.go +++ b/controllers/mocks/assistant_mock.go @@ -59,6 +59,21 @@ func (m *MockAssistant) EXPECT() *MockAssistantMockRecorder { return m.recorder } +// CoreDNSClusterIPs mocks base method. +func (m *MockAssistant) CoreDNSClusterIPs() ([]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CoreDNSClusterIPs") + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CoreDNSClusterIPs indicates an expected call of CoreDNSClusterIPs. +func (mr *MockAssistantMockRecorder) CoreDNSClusterIPs() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CoreDNSClusterIPs", reflect.TypeOf((*MockAssistant)(nil).CoreDNSClusterIPs)) +} + // CoreDNSExposedIPs mocks base method. func (m *MockAssistant) CoreDNSExposedIPs() ([]string, error) { m.ctrl.T.Helper() diff --git a/controllers/providers/assistant/assistant.go b/controllers/providers/assistant/assistant.go index 196555c050..f7cb2640ae 100644 --- a/controllers/providers/assistant/assistant.go +++ b/controllers/providers/assistant/assistant.go @@ -25,6 +25,8 @@ import ( ) type Assistant interface { + //CoreDNSClusterIPs retrieves a list of ClusterIPs assigned to CoreDNS + CoreDNSClusterIPs() ([]string, error) // CoreDNSExposedIPs retrieves list of exposed IP by CoreDNS CoreDNSExposedIPs() ([]string, error) // GetExternalTargets retrieves slice of targets from external clusters diff --git a/controllers/providers/assistant/gslb.go b/controllers/providers/assistant/gslb.go index 1332a2afb8..b39e44e6d8 100644 --- a/controllers/providers/assistant/gslb.go +++ b/controllers/providers/assistant/gslb.go @@ -58,8 +58,7 @@ func NewGslbAssistant(client client.Client, k8gbNamespace string, edgeDNSServers } } -// CoreDNSExposedIPs retrieves list of IP's exposed by CoreDNS -func (r *Gslb) CoreDNSExposedIPs() ([]string, error) { +func (r *Gslb) GetCoreDNSService() (*corev1.Service, error) { serviceList := &corev1.ServiceList{} sel, err := labels.Parse(coreDNSServiceLabel) if err != nil { @@ -88,7 +87,32 @@ func (r *Gslb) CoreDNSExposedIPs() ([]string, error) { return nil, err } coreDNSService := &serviceList.Items[0] + return coreDNSService, nil +} +// CoreDNSClusterIPs retrieves list of ClusterIPs assigned to CoreDNS +func (r *Gslb) CoreDNSClusterIPs() ([]string, error) { + coreDNSService, err := r.GetCoreDNSService() + if err != nil { + return nil, err + } + if len(coreDNSService.Spec.ClusterIPs) == 0 { + errMessage := "no ClusterIPs found" + log.Warn(). + Str("serviceName", coreDNSService.Name). + Msg(errMessage) + err := coreerrors.New(errMessage) + return nil, err + } + return coreDNSService.Spec.ClusterIPs, nil +} + +// CoreDNSExposedIPs retrieves list of IP's exposed by CoreDNS +func (r *Gslb) CoreDNSExposedIPs() ([]string, error) { + coreDNSService, err := r.GetCoreDNSService() + if err != nil { + return nil, err + } var lb corev1.LoadBalancerIngress if len(coreDNSService.Status.LoadBalancer.Ingress) == 0 { errMessage := "no LoadBalancer ExternalIPs are found" diff --git a/controllers/providers/dns/external.go b/controllers/providers/dns/external.go index b4aa210e58..83ebba96d4 100644 --- a/controllers/providers/dns/external.go +++ b/controllers/providers/dns/external.go @@ -66,7 +66,11 @@ func (p *ExternalDNSProvider) CreateZoneDelegationForExternalDNS(gslb *k8gbv1bet var NSServerIPs []string var err error if p.config.CoreDNSExposed { - NSServerIPs, err = p.assistant.CoreDNSExposedIPs() + if p.config.CoreDNSClusterIPs { + NSServerIPs, err = p.assistant.CoreDNSClusterIPs() + } else { + NSServerIPs, err = p.assistant.CoreDNSExposedIPs() + } } else { if len(gslb.Status.LoadBalancer.ExposedIPs) == 0 { // do not update DNS Endpoint for External DNS if no IPs are exposed diff --git a/controllers/providers/dns/infoblox.go b/controllers/providers/dns/infoblox.go index 05120b2956..a30abc856b 100644 --- a/controllers/providers/dns/infoblox.go +++ b/controllers/providers/dns/infoblox.go @@ -70,7 +70,11 @@ func (p *InfobloxProvider) CreateZoneDelegationForExternalDNS(gslb *k8gbv1beta1. var addresses []string if p.config.CoreDNSExposed { - addresses, err = p.assistant.CoreDNSExposedIPs() + if p.config.CoreDNSClusterIPs { + addresses, err = p.assistant.CoreDNSClusterIPs() + } else { + addresses, err = p.assistant.CoreDNSExposedIPs() + } } else { addresses = gslb.Status.LoadBalancer.ExposedIPs } From 7e5d320fcd67227e43003b886b575196d4512eea Mon Sep 17 00:00:00 2001 From: Brandon Ewing Date: Wed, 18 Dec 2024 09:29:52 -0600 Subject: [PATCH 2/2] Remove config flag and refactor assistant Instead of using a config flag to select ClusterIPs for the CoreDNS service, inspect the service type to determine which set of IPs to return. Signed-off-by: Brandon Ewing --- controllers/depresolver/depresolver.go | 2 -- controllers/mocks/assistant_mock.go | 27 +++++++++--------- controllers/providers/assistant/assistant.go | 5 ++-- controllers/providers/assistant/gslb.go | 30 +++++++++----------- controllers/providers/dns/external.go | 6 +--- controllers/providers/dns/infoblox.go | 6 +--- 6 files changed, 32 insertions(+), 44 deletions(-) diff --git a/controllers/depresolver/depresolver.go b/controllers/depresolver/depresolver.go index fd3ad39d23..d4b20f1fe7 100644 --- a/controllers/depresolver/depresolver.go +++ b/controllers/depresolver/depresolver.go @@ -144,8 +144,6 @@ type Config struct { Infoblox Infoblox // CoreDNSExposed flag CoreDNSExposed bool `env:"COREDNS_EXPOSED, default=false"` - // CoreDNSClusterIPs flag - CoreDNSClusterIPs bool `env:"COREDNS_CLUSTERIPS, default=false"` // Log configuration Log Log // MetricsAddress in format address:port where address can be empty, IP address, or hostname, default: 0.0.0.0:8080 diff --git a/controllers/mocks/assistant_mock.go b/controllers/mocks/assistant_mock.go index 85722b148e..f84c1e6076 100644 --- a/controllers/mocks/assistant_mock.go +++ b/controllers/mocks/assistant_mock.go @@ -33,6 +33,7 @@ import ( assistant "github.com/k8gb-io/k8gb/controllers/providers/assistant" gomock "go.uber.org/mock/gomock" + v1 "k8s.io/api/core/v1" endpoint "sigs.k8s.io/external-dns/endpoint" ) @@ -59,34 +60,34 @@ func (m *MockAssistant) EXPECT() *MockAssistantMockRecorder { return m.recorder } -// CoreDNSClusterIPs mocks base method. -func (m *MockAssistant) CoreDNSClusterIPs() ([]string, error) { +// CoreDNSExposedIPs mocks base method. +func (m *MockAssistant) CoreDNSExposedIPs() ([]string, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CoreDNSClusterIPs") + ret := m.ctrl.Call(m, "CoreDNSExposedIPs") ret0, _ := ret[0].([]string) ret1, _ := ret[1].(error) return ret0, ret1 } -// CoreDNSClusterIPs indicates an expected call of CoreDNSClusterIPs. -func (mr *MockAssistantMockRecorder) CoreDNSClusterIPs() *gomock.Call { +// CoreDNSExposedIPs indicates an expected call of CoreDNSExposedIPs. +func (mr *MockAssistantMockRecorder) CoreDNSExposedIPs() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CoreDNSClusterIPs", reflect.TypeOf((*MockAssistant)(nil).CoreDNSClusterIPs)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CoreDNSExposedIPs", reflect.TypeOf((*MockAssistant)(nil).CoreDNSExposedIPs)) } -// CoreDNSExposedIPs mocks base method. -func (m *MockAssistant) CoreDNSExposedIPs() ([]string, error) { +// GetCoreDNSService mocks base method. +func (m *MockAssistant) GetCoreDNSService() (*v1.Service, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CoreDNSExposedIPs") - ret0, _ := ret[0].([]string) + ret := m.ctrl.Call(m, "GetCoreDNSService") + ret0, _ := ret[0].(*v1.Service) ret1, _ := ret[1].(error) return ret0, ret1 } -// CoreDNSExposedIPs indicates an expected call of CoreDNSExposedIPs. -func (mr *MockAssistantMockRecorder) CoreDNSExposedIPs() *gomock.Call { +// GetCoreDNSService indicates an expected call of GetCoreDNSService. +func (mr *MockAssistantMockRecorder) GetCoreDNSService() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CoreDNSExposedIPs", reflect.TypeOf((*MockAssistant)(nil).CoreDNSExposedIPs)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCoreDNSService", reflect.TypeOf((*MockAssistant)(nil).GetCoreDNSService)) } // GetExternalTargets mocks base method. diff --git a/controllers/providers/assistant/assistant.go b/controllers/providers/assistant/assistant.go index f7cb2640ae..faef7a2aba 100644 --- a/controllers/providers/assistant/assistant.go +++ b/controllers/providers/assistant/assistant.go @@ -21,12 +21,13 @@ Generated by GoLic, for more details see: https://github.com/AbsaOSS/golic import ( "time" + corev1 "k8s.io/api/core/v1" externaldns "sigs.k8s.io/external-dns/endpoint" ) type Assistant interface { - //CoreDNSClusterIPs retrieves a list of ClusterIPs assigned to CoreDNS - CoreDNSClusterIPs() ([]string, error) + // GetCoreDNSService returns the CoreDNS Service + GetCoreDNSService() (*corev1.Service, error) // CoreDNSExposedIPs retrieves list of exposed IP by CoreDNS CoreDNSExposedIPs() ([]string, error) // GetExternalTargets retrieves slice of targets from external clusters diff --git a/controllers/providers/assistant/gslb.go b/controllers/providers/assistant/gslb.go index b39e44e6d8..059a8db3ac 100644 --- a/controllers/providers/assistant/gslb.go +++ b/controllers/providers/assistant/gslb.go @@ -58,6 +58,7 @@ func NewGslbAssistant(client client.Client, k8gbNamespace string, edgeDNSServers } } +// GetCoreDNSService returns the CoreDNS Service func (r *Gslb) GetCoreDNSService() (*corev1.Service, error) { serviceList := &corev1.ServiceList{} sel, err := labels.Parse(coreDNSServiceLabel) @@ -90,29 +91,24 @@ func (r *Gslb) GetCoreDNSService() (*corev1.Service, error) { return coreDNSService, nil } -// CoreDNSClusterIPs retrieves list of ClusterIPs assigned to CoreDNS -func (r *Gslb) CoreDNSClusterIPs() ([]string, error) { - coreDNSService, err := r.GetCoreDNSService() - if err != nil { - return nil, err - } - if len(coreDNSService.Spec.ClusterIPs) == 0 { - errMessage := "no ClusterIPs found" - log.Warn(). - Str("serviceName", coreDNSService.Name). - Msg(errMessage) - err := coreerrors.New(errMessage) - return nil, err - } - return coreDNSService.Spec.ClusterIPs, nil -} - // CoreDNSExposedIPs retrieves list of IP's exposed by CoreDNS func (r *Gslb) CoreDNSExposedIPs() ([]string, error) { coreDNSService, err := r.GetCoreDNSService() if err != nil { return nil, err } + if coreDNSService.Spec.Type == "ClusterIP" { + if len(coreDNSService.Spec.ClusterIPs) == 0 { + errMessage := "no ClusterIPs found" + log.Warn(). + Str("serviceName", coreDNSService.Name). + Msg(errMessage) + err := coreerrors.New(errMessage) + return nil, err + } + return coreDNSService.Spec.ClusterIPs, nil + } + // LoadBalancer / ExternalName / NodePort service var lb corev1.LoadBalancerIngress if len(coreDNSService.Status.LoadBalancer.Ingress) == 0 { errMessage := "no LoadBalancer ExternalIPs are found" diff --git a/controllers/providers/dns/external.go b/controllers/providers/dns/external.go index 83ebba96d4..b4aa210e58 100644 --- a/controllers/providers/dns/external.go +++ b/controllers/providers/dns/external.go @@ -66,11 +66,7 @@ func (p *ExternalDNSProvider) CreateZoneDelegationForExternalDNS(gslb *k8gbv1bet var NSServerIPs []string var err error if p.config.CoreDNSExposed { - if p.config.CoreDNSClusterIPs { - NSServerIPs, err = p.assistant.CoreDNSClusterIPs() - } else { - NSServerIPs, err = p.assistant.CoreDNSExposedIPs() - } + NSServerIPs, err = p.assistant.CoreDNSExposedIPs() } else { if len(gslb.Status.LoadBalancer.ExposedIPs) == 0 { // do not update DNS Endpoint for External DNS if no IPs are exposed diff --git a/controllers/providers/dns/infoblox.go b/controllers/providers/dns/infoblox.go index a30abc856b..05120b2956 100644 --- a/controllers/providers/dns/infoblox.go +++ b/controllers/providers/dns/infoblox.go @@ -70,11 +70,7 @@ func (p *InfobloxProvider) CreateZoneDelegationForExternalDNS(gslb *k8gbv1beta1. var addresses []string if p.config.CoreDNSExposed { - if p.config.CoreDNSClusterIPs { - addresses, err = p.assistant.CoreDNSClusterIPs() - } else { - addresses, err = p.assistant.CoreDNSExposedIPs() - } + addresses, err = p.assistant.CoreDNSExposedIPs() } else { addresses = gslb.Status.LoadBalancer.ExposedIPs }