From 1e1db901d9de74f1c79c243c282ecb0276e89c2b Mon Sep 17 00:00:00 2001 From: Andre Baptista Aguas Date: Sun, 25 Aug 2024 21:09:31 +0200 Subject: [PATCH 1/3] Add reverse proxy support Problem K8GB reads IP addresses from `Ingress.Status.LoadBalancer.Ingress` or from `Service.Status.LoadBalancer.Ingress` for ingress configured with Kubernetes Ingress and Istio Virtual Service, respectively. The IP addresses exposed by these resources are the IP addresses exposed by the Kubernetes Cluster. However, in some setups the clients do not route their traffic to these IP addresses because the cluster is behind a reverse proxy. Solution To support this setup, K8GB should expose DNS records with the IP address of the reverse proxy. Since the address is unknown to the cluster the K8GB administrator must provide it via configuration. This PR adds to K8GB the capability to read IP address from an annotation `k8gb.io/external-ips` on the GSLB resource. Example ``` apiVersion: k8gb.absa.oss/v1beta1 kind: Gslb metadata: labels: app: ingress annotations: k8gb.io/external-ips: "185.199.110.153" ``` Fixes #1275 Signed-off-by: Andre Baptista Aguas --- .../testdata/invalid_omitempty_empty.yaml | 2 -- controllers/gslb_controller_reconciliation.go | 2 +- controllers/mocks/refresolver_mock.go | 8 +++---- controllers/refresolver/ingress/ingress.go | 15 ++++++++++-- .../refresolver/ingress/ingress_test.go | 24 ++++++++++++++++++- .../istiovirtualservice.go | 14 +++++++++-- .../istiovirtualservice_test.go | 24 ++++++++++++++++++- controllers/refresolver/refresolver.go | 2 +- 8 files changed, 77 insertions(+), 14 deletions(-) diff --git a/controllers/depresolver/testdata/invalid_omitempty_empty.yaml b/controllers/depresolver/testdata/invalid_omitempty_empty.yaml index 7ee8f9801b..981b89b7e3 100644 --- a/controllers/depresolver/testdata/invalid_omitempty_empty.yaml +++ b/controllers/depresolver/testdata/invalid_omitempty_empty.yaml @@ -37,5 +37,3 @@ spec: type: roundRobin # Use a round robin load balancing strategy, when deciding which downstream clusters to route clients too splitBrainThresholdSeconds: weight: - - diff --git a/controllers/gslb_controller_reconciliation.go b/controllers/gslb_controller_reconciliation.go index c626d74e0b..4488e5b886 100644 --- a/controllers/gslb_controller_reconciliation.go +++ b/controllers/gslb_controller_reconciliation.go @@ -178,7 +178,7 @@ func (r *GslbReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. } gslb.Status.Servers = servers - loadBalancerExposedIPs, err := refResolver.GetGslbExposedIPs(r.Config.EdgeDNSServers) + loadBalancerExposedIPs, err := refResolver.GetGslbExposedIPs(gslb.Annotations, r.Config.EdgeDNSServers) if err != nil { m.IncrementError(gslb) return result.RequeueError(fmt.Errorf("getting load balancer exposed IPs (%s)", err)) diff --git a/controllers/mocks/refresolver_mock.go b/controllers/mocks/refresolver_mock.go index 2b5ce011f8..6694e0d3c3 100644 --- a/controllers/mocks/refresolver_mock.go +++ b/controllers/mocks/refresolver_mock.go @@ -59,18 +59,18 @@ func (m *MockGslbReferenceResolver) EXPECT() *MockGslbReferenceResolverMockRecor } // GetGslbExposedIPs mocks base method. -func (m *MockGslbReferenceResolver) GetGslbExposedIPs(arg0 utils.DNSList) ([]string, error) { +func (m *MockGslbReferenceResolver) GetGslbExposedIPs(gslbAnnotations map[string]string, edgeDNSServers utils.DNSList) ([]string, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetGslbExposedIPs", arg0) + ret := m.ctrl.Call(m, "GetGslbExposedIPs", gslbAnnotations, edgeDNSServers) ret0, _ := ret[0].([]string) ret1, _ := ret[1].(error) return ret0, ret1 } // GetGslbExposedIPs indicates an expected call of GetGslbExposedIPs. -func (mr *MockGslbReferenceResolverMockRecorder) GetGslbExposedIPs(arg0 any) *gomock.Call { +func (mr *MockGslbReferenceResolverMockRecorder) GetGslbExposedIPs(gslbAnnotations, edgeDNSServers any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGslbExposedIPs", reflect.TypeOf((*MockGslbReferenceResolver)(nil).GetGslbExposedIPs), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetGslbExposedIPs", reflect.TypeOf((*MockGslbReferenceResolver)(nil).GetGslbExposedIPs), gslbAnnotations, edgeDNSServers) } // GetServers mocks base method. diff --git a/controllers/refresolver/ingress/ingress.go b/controllers/refresolver/ingress/ingress.go index 8c4a4c5034..93141adff2 100644 --- a/controllers/refresolver/ingress/ingress.go +++ b/controllers/refresolver/ingress/ingress.go @@ -22,6 +22,7 @@ import ( "context" "fmt" "reflect" + "strings" k8gbv1beta1 "github.com/k8gb-io/k8gb/api/v1beta1" "github.com/k8gb-io/k8gb/controllers/logging" @@ -35,6 +36,11 @@ import ( var log = logging.Logger() +const ( + // comma separated list of external IP addresses + externalIPsAnnotation = "k8gb.io/exposed-ip-addresses" +) + type ReferenceResolver struct { ingress *netv1.Ingress } @@ -158,9 +164,14 @@ func (rr *ReferenceResolver) GetServers() ([]*k8gbv1beta1.Server, error) { } // GetGslbExposedIPs retrieves the load balancer IP address of the GSLB -func (rr *ReferenceResolver) GetGslbExposedIPs(edgeDNSServers utils.DNSList) ([]string, error) { - gslbIngressIPs := []string{} +func (rr *ReferenceResolver) GetGslbExposedIPs(gslbAnnotations map[string]string, edgeDNSServers utils.DNSList) ([]string, error) { + // fetch the IP addresses of the reverse proxy from an annotation if it exists + if ingressIPsFromAnnotation, ok := gslbAnnotations[externalIPsAnnotation]; ok { + return strings.Split(ingressIPsFromAnnotation, ","), nil + } + // if there is no annotation -> fetch the IP addresses from the Status of the Ingress resource + gslbIngressIPs := []string{} for _, ip := range rr.ingress.Status.LoadBalancer.Ingress { if len(ip.IP) > 0 { gslbIngressIPs = append(gslbIngressIPs, ip.IP) diff --git a/controllers/refresolver/ingress/ingress_test.go b/controllers/refresolver/ingress/ingress_test.go index e288101e81..c6b3d07ce1 100644 --- a/controllers/refresolver/ingress/ingress_test.go +++ b/controllers/refresolver/ingress/ingress_test.go @@ -97,24 +97,46 @@ func TestGetServers(t *testing.T) { func TestGetGslbExposedIPs(t *testing.T) { var tests = []struct { name string + annotations map[string]string ingressYaml string expectedIPs []string }{ { name: "no exposed IPs", + annotations: map[string]string{}, ingressYaml: "./testdata/ingress_no_ips.yaml", expectedIPs: []string{}, }, { name: "single exposed IP", + annotations: map[string]string{}, ingressYaml: "../testdata/ingress_referenced.yaml", expectedIPs: []string{"10.0.0.1"}, }, { name: "multiple exposed IPs", + annotations: map[string]string{}, ingressYaml: "./testdata/ingress_multiple_ips.yaml", expectedIPs: []string{"10.0.0.1", "10.0.0.2"}, }, + { + name: "annotation with no exposed IPs", + annotations: map[string]string{"k8gb.io/exposed-ip-addresses": ""}, + ingressYaml: "./testdata/ingress_multiple_ips.yaml", + expectedIPs: []string{""}, + }, + { + name: "annotation with single exposed IP", + annotations: map[string]string{"k8gb.io/exposed-ip-addresses": "185.199.110.153"}, + ingressYaml: "./testdata/ingress_multiple_ips.yaml", + expectedIPs: []string{"185.199.110.153"}, + }, + { + name: "annotation with multiple exposed IPs", + annotations: map[string]string{"k8gb.io/exposed-ip-addresses": "185.199.110.153,185.199.109.153"}, + ingressYaml: "./testdata/ingress_multiple_ips.yaml", + expectedIPs: []string{"185.199.110.153", "185.199.109.153"}, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -125,7 +147,7 @@ func TestGetGslbExposedIPs(t *testing.T) { } // act - IPs, err := resolver.GetGslbExposedIPs([]utils.DNSServer{}) + IPs, err := resolver.GetGslbExposedIPs(test.annotations, []utils.DNSServer{}) assert.NoError(t, err) // assert diff --git a/controllers/refresolver/istiovirtualservice/istiovirtualservice.go b/controllers/refresolver/istiovirtualservice/istiovirtualservice.go index 93f4050f68..6405d3ca5a 100644 --- a/controllers/refresolver/istiovirtualservice/istiovirtualservice.go +++ b/controllers/refresolver/istiovirtualservice/istiovirtualservice.go @@ -37,6 +37,11 @@ import ( var log = logging.Logger() +const ( + // comma separated list of external IP addresses + externalIPsAnnotation = "k8gb.io/exposed-ip-addresses" +) + type ReferenceResolver struct { virtualService *istio.VirtualService lbService *corev1.Service @@ -191,9 +196,14 @@ func (rr *ReferenceResolver) GetServers() ([]*k8gbv1beta1.Server, error) { } // GetGslbExposedIPs retrieves the load balancer IP address of the GSLB -func (rr *ReferenceResolver) GetGslbExposedIPs(edgeDNSServers utils.DNSList) ([]string, error) { - gslbIngressIPs := []string{} +func (rr *ReferenceResolver) GetGslbExposedIPs(gslbAnnotations map[string]string, edgeDNSServers utils.DNSList) ([]string, error) { + // fetch the IP addresses of the reverse proxy from an annotation if it exists + if ingressIPsFromAnnotation, ok := gslbAnnotations[externalIPsAnnotation]; ok { + return strings.Split(ingressIPsFromAnnotation, ","), nil + } + // if there is no annotation -> fetch the IP addresses from the Status of the Ingress resource + gslbIngressIPs := []string{} for _, ip := range rr.lbService.Status.LoadBalancer.Ingress { if len(ip.IP) > 0 { gslbIngressIPs = append(gslbIngressIPs, ip.IP) diff --git a/controllers/refresolver/istiovirtualservice/istiovirtualservice_test.go b/controllers/refresolver/istiovirtualservice/istiovirtualservice_test.go index 8938e6dd10..8f0e78465a 100644 --- a/controllers/refresolver/istiovirtualservice/istiovirtualservice_test.go +++ b/controllers/refresolver/istiovirtualservice/istiovirtualservice_test.go @@ -112,24 +112,46 @@ func TestGetServers(t *testing.T) { func TestGetGslbExposedIPs(t *testing.T) { var tests = []struct { name string + annotations map[string]string serviceYaml string expectedIPs []string }{ { name: "no exposed IPs", serviceYaml: "./testdata/istio_service_no_ips.yaml", + annotations: map[string]string{}, expectedIPs: []string{}, }, { name: "single exposed IP", + annotations: map[string]string{}, serviceYaml: "../testdata/istio_service.yaml", expectedIPs: []string{"10.0.0.1"}, }, { name: "multiple exposed IPs", + annotations: map[string]string{}, serviceYaml: "./testdata/istio_service_multiple_ips.yaml", expectedIPs: []string{"10.0.0.1", "10.0.0.2"}, }, + { + name: "annotation with no exposed IPs", + annotations: map[string]string{"k8gb.io/exposed-ip-addresses": ""}, + serviceYaml: "./testdata/istio_service_multiple_ips.yaml", + expectedIPs: []string{""}, + }, + { + name: "annotation with single exposed IP", + annotations: map[string]string{"k8gb.io/exposed-ip-addresses": "185.199.110.153"}, + serviceYaml: "./testdata/istio_service_multiple_ips.yaml", + expectedIPs: []string{"185.199.110.153"}, + }, + { + name: "annotation with multiple exposed IPs", + annotations: map[string]string{"k8gb.io/exposed-ip-addresses": "185.199.110.153,185.199.109.153"}, + serviceYaml: "./testdata/istio_service_multiple_ips.yaml", + expectedIPs: []string{"185.199.110.153", "185.199.109.153"}, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -140,7 +162,7 @@ func TestGetGslbExposedIPs(t *testing.T) { } // act - IPs, err := resolver.GetGslbExposedIPs([]utils.DNSServer{}) + IPs, err := resolver.GetGslbExposedIPs(test.annotations, []utils.DNSServer{}) assert.NoError(t, err) // assert diff --git a/controllers/refresolver/refresolver.go b/controllers/refresolver/refresolver.go index 6b9067f1a2..460de7f815 100644 --- a/controllers/refresolver/refresolver.go +++ b/controllers/refresolver/refresolver.go @@ -34,7 +34,7 @@ type GslbReferenceResolver interface { // GetServers retrieves GSLB the server configuration GetServers() ([]*k8gbv1beta1.Server, error) // GetGslbExposedIPs retrieves the load balancer IP address of the GSLB - GetGslbExposedIPs(utils.DNSList) ([]string, error) + GetGslbExposedIPs(gslbAnnotations map[string]string, edgeDNSServers utils.DNSList) ([]string, error) } // New creates a new GSLBReferenceResolver From 960e7e9e6f34d7ae38fbe1bb8a8ea837e7778f85 Mon Sep 17 00:00:00 2001 From: Andre Aguas Date: Thu, 31 Oct 2024 15:17:03 +0100 Subject: [PATCH 2/3] Validate k8gb.io/exposed-ip-addresses annotation Signed-off-by: Andre Aguas --- .../templates/validatingadmissionpolicy.yaml | 32 +++++++ chart/k8gb/values.schema.json | 12 +++ chart/k8gb/values.yaml | 3 + controllers/refresolver/ingress/ingress.go | 3 +- .../refresolver/ingress/ingress_test.go | 86 +++++++++++-------- .../istiovirtualservice.go | 2 +- .../istiovirtualservice_test.go | 79 +++++++++-------- controllers/utils/ips.go | 39 +++++++++ controllers/utils/ips_test.go | 85 ++++++++++++++++++ 9 files changed, 270 insertions(+), 71 deletions(-) create mode 100644 chart/k8gb/templates/validatingadmissionpolicy.yaml create mode 100644 controllers/utils/ips.go create mode 100644 controllers/utils/ips_test.go diff --git a/chart/k8gb/templates/validatingadmissionpolicy.yaml b/chart/k8gb/templates/validatingadmissionpolicy.yaml new file mode 100644 index 0000000000..50f59aa43c --- /dev/null +++ b/chart/k8gb/templates/validatingadmissionpolicy.yaml @@ -0,0 +1,32 @@ +{{ if .Values.k8gb.validatingAdmissionPolicy.enabled -}} +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicyBinding +metadata: + name: k8gb-exposed-ip-annotation +spec: + policyName: k8gb-exposed-ip-annotation + matchResources: + namespaceSelector: {} + validationActions: + - Deny +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicy +metadata: + name: k8gb-exposed-ip-annotation +spec: + validations: + - expression: object.metadata.annotations['k8gb.io/exposed-ip-addresses'].matches('^((25[0-5]|2[0-4][0-9]|1[0-9]{2}|[1-9]?[0-9])\\.){3}(25[0-5]|2[0-4][0-9]|1[0-9]{2}|[1-9]?[0-9])$') + message: The annotation 'k8gb.io/exposed-ip-addresses' must contain a valid IPv4 address + matchConditions: + - name: hasExposedIPAddressesAnnotation + expression: "has(object.metadata.annotations) && 'k8gb.io/exposed-ip-addresses' in object.metadata.annotations" + matchConstraints: + resourceRules: + - apiGroups: ["k8gb.absa.oss"] + apiVersions: ["v1beta1"] + operations: ["CREATE", "UPDATE"] + resources: ["gslbs"] + failurePolicy: Fail +{{ end -}} diff --git a/chart/k8gb/values.schema.json b/chart/k8gb/values.schema.json index e2f3865f68..8b40c2193f 100644 --- a/chart/k8gb/values.schema.json +++ b/chart/k8gb/values.schema.json @@ -324,6 +324,9 @@ "serviceMonitor": { "$ref": "#/definitions/k8gbServiceMonitor" }, + "validatingAdmissionPolicy": { + "$ref": "#/definitions/k8gbValidatingAdmissionPolicy" + }, "podAnnotations": { "type": "object" }, @@ -409,6 +412,15 @@ } } }, + "k8gbValidatingAdmissionPolicy": { + "type": "object", + "additionalProperties": false, + "properties": { + "enabled": { + "type": "boolean" + } + } + }, "Ns1": { "type": "object", "additionalProperties": false, diff --git a/chart/k8gb/values.yaml b/chart/k8gb/values.yaml index d42d32671e..9daee858da 100644 --- a/chart/k8gb/values.yaml +++ b/chart/k8gb/values.yaml @@ -56,6 +56,9 @@ k8gb: # -- enable ServiceMonitor serviceMonitor: enabled: false + # -- enable validating admission policies + validatingAdmissionPolicy: + enabled: true # -- pod annotations podAnnotations: {} # -- pod labels diff --git a/controllers/refresolver/ingress/ingress.go b/controllers/refresolver/ingress/ingress.go index 93141adff2..61a5a60145 100644 --- a/controllers/refresolver/ingress/ingress.go +++ b/controllers/refresolver/ingress/ingress.go @@ -22,7 +22,6 @@ import ( "context" "fmt" "reflect" - "strings" k8gbv1beta1 "github.com/k8gb-io/k8gb/api/v1beta1" "github.com/k8gb-io/k8gb/controllers/logging" @@ -167,7 +166,7 @@ func (rr *ReferenceResolver) GetServers() ([]*k8gbv1beta1.Server, error) { func (rr *ReferenceResolver) GetGslbExposedIPs(gslbAnnotations map[string]string, edgeDNSServers utils.DNSList) ([]string, error) { // fetch the IP addresses of the reverse proxy from an annotation if it exists if ingressIPsFromAnnotation, ok := gslbAnnotations[externalIPsAnnotation]; ok { - return strings.Split(ingressIPsFromAnnotation, ","), nil + return utils.ParseIPAddresses(ingressIPsFromAnnotation) } // if there is no annotation -> fetch the IP addresses from the Status of the Ingress resource diff --git a/controllers/refresolver/ingress/ingress_test.go b/controllers/refresolver/ingress/ingress_test.go index c6b3d07ce1..229f19ebb7 100644 --- a/controllers/refresolver/ingress/ingress_test.go +++ b/controllers/refresolver/ingress/ingress_test.go @@ -96,62 +96,80 @@ func TestGetServers(t *testing.T) { func TestGetGslbExposedIPs(t *testing.T) { var tests = []struct { - name string - annotations map[string]string - ingressYaml string - expectedIPs []string + name string + annotations map[string]string + ingressYaml string + expectedIPs []string + expectedError bool }{ { - name: "no exposed IPs", - annotations: map[string]string{}, - ingressYaml: "./testdata/ingress_no_ips.yaml", - expectedIPs: []string{}, + name: "no exposed IPs", + annotations: map[string]string{}, + ingressYaml: "./testdata/ingress_no_ips.yaml", + expectedIPs: []string{}, + expectedError: false, }, { - name: "single exposed IP", - annotations: map[string]string{}, - ingressYaml: "../testdata/ingress_referenced.yaml", - expectedIPs: []string{"10.0.0.1"}, + name: "single exposed IP", + annotations: map[string]string{}, + ingressYaml: "../testdata/ingress_referenced.yaml", + expectedIPs: []string{"10.0.0.1"}, + expectedError: false, }, { - name: "multiple exposed IPs", - annotations: map[string]string{}, - ingressYaml: "./testdata/ingress_multiple_ips.yaml", - expectedIPs: []string{"10.0.0.1", "10.0.0.2"}, + name: "multiple exposed IPs", + annotations: map[string]string{}, + ingressYaml: "./testdata/ingress_multiple_ips.yaml", + expectedIPs: []string{"10.0.0.1", "10.0.0.2"}, + expectedError: false, }, { - name: "annotation with no exposed IPs", - annotations: map[string]string{"k8gb.io/exposed-ip-addresses": ""}, - ingressYaml: "./testdata/ingress_multiple_ips.yaml", - expectedIPs: []string{""}, + name: "annotation with no exposed IPs", + annotations: map[string]string{"k8gb.io/exposed-ip-addresses": ""}, + ingressYaml: "./testdata/ingress_multiple_ips.yaml", + expectedIPs: []string{}, + expectedError: true, }, { - name: "annotation with single exposed IP", - annotations: map[string]string{"k8gb.io/exposed-ip-addresses": "185.199.110.153"}, - ingressYaml: "./testdata/ingress_multiple_ips.yaml", - expectedIPs: []string{"185.199.110.153"}, + name: "annotation with single exposed IP", + annotations: map[string]string{"k8gb.io/exposed-ip-addresses": "185.199.110.153"}, + ingressYaml: "./testdata/ingress_multiple_ips.yaml", + expectedIPs: []string{"185.199.110.153"}, + expectedError: false, }, { - name: "annotation with multiple exposed IPs", - annotations: map[string]string{"k8gb.io/exposed-ip-addresses": "185.199.110.153,185.199.109.153"}, - ingressYaml: "./testdata/ingress_multiple_ips.yaml", - expectedIPs: []string{"185.199.110.153", "185.199.109.153"}, + name: "annotation with multiple exposed IPs", + annotations: map[string]string{"k8gb.io/exposed-ip-addresses": "185.199.110.153,185.199.109.153"}, + ingressYaml: "./testdata/ingress_multiple_ips.yaml", + expectedIPs: []string{"185.199.110.153", "185.199.109.153"}, + expectedError: false, + }, + { + name: "annotation with invalid IP", + annotations: map[string]string{"k8gb.io/exposed-ip-addresses": "192.169.0.test"}, + ingressYaml: "./testdata/ingress_multiple_ips.yaml", + expectedIPs: []string{}, + expectedError: true, }, } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { // arrange - ingress := utils.FileToIngress(test.ingressYaml) + ingress := utils.FileToIngress(tt.ingressYaml) resolver := ReferenceResolver{ ingress: ingress, } // act - IPs, err := resolver.GetGslbExposedIPs(test.annotations, []utils.DNSServer{}) - assert.NoError(t, err) + IPs, err := resolver.GetGslbExposedIPs(tt.annotations, []utils.DNSServer{}) + if tt.expectedError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } // assert - assert.Equal(t, test.expectedIPs, IPs) + assert.Equal(t, tt.expectedIPs, IPs) }) } } diff --git a/controllers/refresolver/istiovirtualservice/istiovirtualservice.go b/controllers/refresolver/istiovirtualservice/istiovirtualservice.go index 6405d3ca5a..4674e0a703 100644 --- a/controllers/refresolver/istiovirtualservice/istiovirtualservice.go +++ b/controllers/refresolver/istiovirtualservice/istiovirtualservice.go @@ -199,7 +199,7 @@ func (rr *ReferenceResolver) GetServers() ([]*k8gbv1beta1.Server, error) { func (rr *ReferenceResolver) GetGslbExposedIPs(gslbAnnotations map[string]string, edgeDNSServers utils.DNSList) ([]string, error) { // fetch the IP addresses of the reverse proxy from an annotation if it exists if ingressIPsFromAnnotation, ok := gslbAnnotations[externalIPsAnnotation]; ok { - return strings.Split(ingressIPsFromAnnotation, ","), nil + return utils.ParseIPAddresses(ingressIPsFromAnnotation) } // if there is no annotation -> fetch the IP addresses from the Status of the Ingress resource diff --git a/controllers/refresolver/istiovirtualservice/istiovirtualservice_test.go b/controllers/refresolver/istiovirtualservice/istiovirtualservice_test.go index 8f0e78465a..0d997a7a47 100644 --- a/controllers/refresolver/istiovirtualservice/istiovirtualservice_test.go +++ b/controllers/refresolver/istiovirtualservice/istiovirtualservice_test.go @@ -111,62 +111,73 @@ func TestGetServers(t *testing.T) { func TestGetGslbExposedIPs(t *testing.T) { var tests = []struct { - name string - annotations map[string]string - serviceYaml string - expectedIPs []string + name string + annotations map[string]string + serviceYaml string + expectedIPs []string + expectedError bool }{ { - name: "no exposed IPs", - serviceYaml: "./testdata/istio_service_no_ips.yaml", - annotations: map[string]string{}, - expectedIPs: []string{}, + name: "no exposed IPs", + serviceYaml: "./testdata/istio_service_no_ips.yaml", + annotations: map[string]string{}, + expectedIPs: []string{}, + expectedError: false, }, { - name: "single exposed IP", - annotations: map[string]string{}, - serviceYaml: "../testdata/istio_service.yaml", - expectedIPs: []string{"10.0.0.1"}, + name: "single exposed IP", + annotations: map[string]string{}, + serviceYaml: "../testdata/istio_service.yaml", + expectedIPs: []string{"10.0.0.1"}, + expectedError: false, }, { - name: "multiple exposed IPs", - annotations: map[string]string{}, - serviceYaml: "./testdata/istio_service_multiple_ips.yaml", - expectedIPs: []string{"10.0.0.1", "10.0.0.2"}, + name: "multiple exposed IPs", + annotations: map[string]string{}, + serviceYaml: "./testdata/istio_service_multiple_ips.yaml", + expectedIPs: []string{"10.0.0.1", "10.0.0.2"}, + expectedError: false, }, { - name: "annotation with no exposed IPs", - annotations: map[string]string{"k8gb.io/exposed-ip-addresses": ""}, - serviceYaml: "./testdata/istio_service_multiple_ips.yaml", - expectedIPs: []string{""}, + name: "annotation with no exposed IPs", + annotations: map[string]string{"k8gb.io/exposed-ip-addresses": ""}, + serviceYaml: "./testdata/istio_service_multiple_ips.yaml", + expectedIPs: []string{}, + expectedError: true, }, { - name: "annotation with single exposed IP", - annotations: map[string]string{"k8gb.io/exposed-ip-addresses": "185.199.110.153"}, - serviceYaml: "./testdata/istio_service_multiple_ips.yaml", - expectedIPs: []string{"185.199.110.153"}, + name: "annotation with single exposed IP", + annotations: map[string]string{"k8gb.io/exposed-ip-addresses": "185.199.110.153"}, + serviceYaml: "./testdata/istio_service_multiple_ips.yaml", + expectedIPs: []string{"185.199.110.153"}, + expectedError: false, }, { - name: "annotation with multiple exposed IPs", - annotations: map[string]string{"k8gb.io/exposed-ip-addresses": "185.199.110.153,185.199.109.153"}, - serviceYaml: "./testdata/istio_service_multiple_ips.yaml", - expectedIPs: []string{"185.199.110.153", "185.199.109.153"}, + name: "annotation with invalid IP", + annotations: map[string]string{"k8gb.io/exposed-ip-addresses": "192.169.0.test"}, + serviceYaml: "./testdata/istio_service_multiple_ips.yaml", + expectedIPs: []string{}, + expectedError: true, }, } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { // arrange - svc := utils.FileToService(test.serviceYaml) + svc := utils.FileToService(tt.serviceYaml) resolver := ReferenceResolver{ lbService: svc, } // act - IPs, err := resolver.GetGslbExposedIPs(test.annotations, []utils.DNSServer{}) - assert.NoError(t, err) + IPs, err := resolver.GetGslbExposedIPs(tt.annotations, []utils.DNSServer{}) + if tt.expectedError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } // assert - assert.Equal(t, test.expectedIPs, IPs) + assert.Equal(t, tt.expectedIPs, IPs) }) } } diff --git a/controllers/utils/ips.go b/controllers/utils/ips.go new file mode 100644 index 0000000000..f948c4e6c4 --- /dev/null +++ b/controllers/utils/ips.go @@ -0,0 +1,39 @@ +package utils + +/* +Copyright 2022 The k8gb 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. + +Generated by GoLic, for more details see: https://github.com/AbsaOSS/golic +*/ + +import ( + "fmt" + "net" + "strings" +) + +// ParseIPAddresses parse a comma separated string of ip addresses into a list of net.IP +func ParseIPAddresses(ipsStr string) ([]string, error) { + IPs := []string{} + for _, ipStr := range strings.Split(ipsStr, ",") { + ipAddr := net.ParseIP(ipStr) + if ipAddr == nil { + return []string{}, fmt.Errorf("invalid IP address: %s", ipStr) + } + IPs = append(IPs, ipAddr.String()) + } + + return IPs, nil +} diff --git a/controllers/utils/ips_test.go b/controllers/utils/ips_test.go new file mode 100644 index 0000000000..bb952b652c --- /dev/null +++ b/controllers/utils/ips_test.go @@ -0,0 +1,85 @@ +package utils + +/* +Copyright 2022 The k8gb 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. + +Generated by GoLic, for more details see: https://github.com/AbsaOSS/golic +*/ + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParseIPAddresses(t *testing.T) { + tests := []struct { + name string + input string + expected []string + expectedError bool + }{ + { + "single valid IP address", + "10.0.0.1", + []string{"10.0.0.1"}, + false, + }, + { + "multiple valid IP addresses", + "10.0.0.1,10.0.0.2", + []string{"10.0.0.1", "10.0.0.2"}, + false, + }, + { + "out of 0-255 range", + "256.256.256.256", + []string{}, + true, + }, + { + "invalid characters", + "test.gslb.test.gslb", + []string{}, + true, + }, + { + "valid and invalid IP addresses", + "10.0.0.1,test.gslb.test.gslb", + []string{}, + true, + }, + { + "multiple valid IP addresses with additional comma", + "10.0.0.1,10.0.0.2,", + []string{}, + true, + }, + } + + // Run each test case + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ipAddresses, err := ParseIPAddresses(tt.input) + if tt.expectedError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + + assert.Equal(t, tt.expected, ipAddresses) + }) + } +} From 541b81fbc8d50b2555c86492acd691a4b7f5d1ce Mon Sep 17 00:00:00 2001 From: Andre Aguas Date: Mon, 13 Jan 2025 22:12:42 +0100 Subject: [PATCH 3/3] set the admission policy to false by default Signed-off-by: Andre Aguas --- chart/k8gb/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chart/k8gb/values.yaml b/chart/k8gb/values.yaml index 9daee858da..ac07904cd1 100644 --- a/chart/k8gb/values.yaml +++ b/chart/k8gb/values.yaml @@ -58,7 +58,7 @@ k8gb: enabled: false # -- enable validating admission policies validatingAdmissionPolicy: - enabled: true + enabled: false # -- pod annotations podAnnotations: {} # -- pod labels