diff --git a/chart/k8gb/templates/validatingadmissionpolicy.yaml b/chart/k8gb/templates/validatingadmissionpolicy.yaml new file mode 100644 index 0000000000..7d7bfd0e1f --- /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 -}} \ No newline at end of file diff --git a/chart/k8gb/values.schema.json b/chart/k8gb/values.schema.json index 8eec0b7954..e12066dbc7 100644 --- a/chart/k8gb/values.schema.json +++ b/chart/k8gb/values.schema.json @@ -320,6 +320,9 @@ "serviceMonitor": { "$ref": "#/definitions/k8gbServiceMonitor" }, + "validatingAdmissionPolicy": { + "$ref": "#/definitions/k8gbValidatingAdmissionPolicy" + }, "podAnnotations": { "type": "object" }, @@ -405,6 +408,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 2cff9ad713..37cc59d54b 100644 --- a/chart/k8gb/values.yaml +++ b/chart/k8gb/values.yaml @@ -54,6 +54,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 5348e552f1..fdfb60904d 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 0c9b88178b..79f9abc630 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..58e17bf7d1 --- /dev/null +++ b/controllers/utils/ips.go @@ -0,0 +1,35 @@ +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..510aee7be9 --- /dev/null +++ b/controllers/utils/ips_test.go @@ -0,0 +1,81 @@ +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) + }) + } +}