Merge branch 'main' into feat/missing-client-cert-secret-propagation

Kong · Nov 7, 2022 · 1609bdf · 1609bdf
2 parents c14ccbb + a3c40fc
commit 1609bdf
Show file tree

Hide file tree

Showing 11 changed files with 212 additions and 148 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -128,6 +128,9 @@ Adding a new version? You'll need three changes:
  [#3130](https:/Kong/kubernetes-ingress-controller/pull/3130)
 - Warning events are recorded when a service's referred client-cert does not exist. 
  [#3137](https:/Kong/kubernetes-ingress-controller/pull/3137)
+- CRDs' validations improvements: `UDPIngressRule.Port`, `IngressRule.Port` and `IngressBackend.ServiceName`
+ instead of being validated in the Parser, are validated by the Kubernetes API now.
+ [#3136](https:/Kong/kubernetes-ingress-controller/pull/3136)
 
 ### Fixed
 

diff --git a/config/crd/bases/configuration.konghq.com_tcpingresses.yaml b/config/crd/bases/configuration.konghq.com_tcpingresses.yaml
@@ -59,6 +59,7 @@ spec:
  properties:
  serviceName:
  description: Specifies the name of the referenced service.
+ minLength: 1
  type: string
  servicePort:
  description: Specifies the port of the referenced service.
@@ -87,6 +88,7 @@ spec:
  type: integer
  required:
  - backend
+ - port
  type: object
  type: array
  tls:

diff --git a/config/crd/bases/configuration.konghq.com_udpingresses.yaml b/config/crd/bases/configuration.konghq.com_udpingresses.yaml
@@ -59,6 +59,7 @@ spec:
  properties:
  serviceName:
  description: Specifies the name of the referenced service.
+ minLength: 1
  type: string
  servicePort:
  description: Specifies the port of the referenced service.
@@ -74,6 +75,9 @@ spec:
  description: Port indicates the port for the Kong proxy to accept
  incoming traffic on, which will then be routed to the service
  Backend.
+ format: int32
+ maximum: 65535
+ minimum: 1
  type: integer
  required:
  - backend

diff --git a/deploy/single/all-in-one-dbless-k4k8s-enterprise.yaml b/deploy/single/all-in-one-dbless-k4k8s-enterprise.yaml
@@ -787,6 +787,7 @@ spec:
  properties:
  serviceName:
  description: Specifies the name of the referenced service.
+ minLength: 1
  type: string
  servicePort:
  description: Specifies the port of the referenced service.
@@ -815,6 +816,7 @@ spec:
  type: integer
  required:
  - backend
+ - port
  type: object
  type: array
  tls:
@@ -969,6 +971,7 @@ spec:
  properties:
  serviceName:
  description: Specifies the name of the referenced service.
+ minLength: 1
  type: string
  servicePort:
  description: Specifies the port of the referenced service.
@@ -984,6 +987,9 @@ spec:
  description: Port indicates the port for the Kong proxy to accept
  incoming traffic on, which will then be routed to the service
  Backend.
+ format: int32
+ maximum: 65535
+ minimum: 1
  type: integer
  required:
  - backend

diff --git a/deploy/single/all-in-one-dbless.yaml b/deploy/single/all-in-one-dbless.yaml
@@ -787,6 +787,7 @@ spec:
  properties:
  serviceName:
  description: Specifies the name of the referenced service.
+ minLength: 1
  type: string
  servicePort:
  description: Specifies the port of the referenced service.
@@ -815,6 +816,7 @@ spec:
  type: integer
  required:
  - backend
+ - port
  type: object
  type: array
  tls:
@@ -969,6 +971,7 @@ spec:
  properties:
  serviceName:
  description: Specifies the name of the referenced service.
+ minLength: 1
  type: string
  servicePort:
  description: Specifies the port of the referenced service.
@@ -984,6 +987,9 @@ spec:
  description: Port indicates the port for the Kong proxy to accept
  incoming traffic on, which will then be routed to the service
  Backend.
+ format: int32
+ maximum: 65535
+ minimum: 1
  type: integer
  required:
  - backend

diff --git a/deploy/single/all-in-one-postgres-enterprise.yaml b/deploy/single/all-in-one-postgres-enterprise.yaml
@@ -787,6 +787,7 @@ spec:
  properties:
  serviceName:
  description: Specifies the name of the referenced service.
+ minLength: 1
  type: string
  servicePort:
  description: Specifies the port of the referenced service.
@@ -815,6 +816,7 @@ spec:
  type: integer
  required:
  - backend
+ - port
  type: object
  type: array
  tls:
@@ -969,6 +971,7 @@ spec:
  properties:
  serviceName:
  description: Specifies the name of the referenced service.
+ minLength: 1
  type: string
  servicePort:
  description: Specifies the port of the referenced service.
@@ -984,6 +987,9 @@ spec:
  description: Port indicates the port for the Kong proxy to accept
  incoming traffic on, which will then be routed to the service
  Backend.
+ format: int32
+ maximum: 65535
+ minimum: 1
  type: integer
  required:
  - backend

diff --git a/deploy/single/all-in-one-postgres.yaml b/deploy/single/all-in-one-postgres.yaml
@@ -787,6 +787,7 @@ spec:
  properties:
  serviceName:
  description: Specifies the name of the referenced service.
+ minLength: 1
  type: string
  servicePort:
  description: Specifies the port of the referenced service.
@@ -815,6 +816,7 @@ spec:
  type: integer
  required:
  - backend
+ - port
  type: object
  type: array
  tls:
@@ -969,6 +971,7 @@ spec:
  properties:
  serviceName:
  description: Specifies the name of the referenced service.
+ minLength: 1
  type: string
  servicePort:
  description: Specifies the port of the referenced service.
@@ -984,6 +987,9 @@ spec:
  description: Port indicates the port for the Kong proxy to accept
  incoming traffic on, which will then be routed to the service
  Backend.
+ format: int32
+ maximum: 65535
+ minimum: 1
  type: integer
  required:
  - backend

diff --git a/internal/dataplane/parser/translate_kong_l4.go b/internal/dataplane/parser/translate_kong_l4.go
@@ -6,7 +6,6 @@ import (
  "strconv"
 
  "github.com/kong/go-kong/kong"
- "github.com/sirupsen/logrus"
 
  "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate"
  "github.com/kong/kubernetes-ingress-controller/v2/internal/util"
@@ -29,19 +28,10 @@ func (p *Parser) ingressRulesFromTCPIngressV1beta1() ingressRules {
  for _, ingress := range ingressList {
  ingressSpec := ingress.Spec
 
- log := p.logger.WithFields(logrus.Fields{
- "tcpingress_namespace": ingress.Namespace,
- "tcpingress_name": ingress.Name,
- })
-
  result.SecretNameToSNIs.addFromIngressV1beta1TLS(tcpIngressToNetworkingTLS(ingressSpec.TLS), ingress.Namespace)
 
  var objectSuccessfullyParsed bool
  for i, rule := range ingressSpec.Rules {
- if !util.IsValidPort(rule.Port) {
- log.Errorf("invalid TCPIngress: invalid port: %v", rule.Port)
- continue
- }
  r := kongstate.Route{
  Ingress: util.FromK8sObject(ingress),
  Route: kong.Route{
@@ -58,14 +48,6 @@ func (p *Parser) ingressRulesFromTCPIngressV1beta1() ingressRules {
  if host != "" {
  r.SNIs = kong.StringSlice(host)
  }
- if rule.Backend.ServiceName == "" {
- log.Errorf("invalid TCPIngress: empty serviceName")
- continue
- }
- if !util.IsValidPort(rule.Backend.ServicePort) {
- log.Errorf("invalid TCPIngress: invalid servicePort: %v", rule.Backend.ServicePort)
- continue
- }
 
  serviceName := fmt.Sprintf("%s.%s.%d", ingress.Namespace, rule.Backend.ServiceName, rule.Backend.ServicePort)
  service, ok := result.ServiceNameToServices[serviceName]
@@ -118,27 +100,8 @@ func (p *Parser) ingressRulesFromUDPIngressV1beta1() ingressRules {
  for _, ingress := range ingressList {
  ingressSpec := ingress.Spec
 
- log := p.logger.WithFields(logrus.Fields{
- "udpingress_namespace": ingress.Namespace,
- "udpingress_name": ingress.Name,
- })
-
  var objectSuccessfullyParsed bool
  for i, rule := range ingressSpec.Rules {
- // validate the ports and servicenames for the rule
- if !util.IsValidPort(rule.Port) {
- log.Errorf("invalid UDPIngress: invalid port: %d", rule.Port)
- continue
- }
- if rule.Backend.ServiceName == "" {
- log.Errorf("invalid UDPIngress: empty serviceName")
- continue
- }
- if !util.IsValidPort(rule.Backend.ServicePort) {
- log.Errorf("invalid UDPIngress: invalid servicePort: %d", rule.Backend.ServicePort)
- continue
- }
-
  // generate the kong Route based on the listen port
  route := kongstate.Route{
  Ingress: util.FromK8sObject(ingress),

diff --git a/internal/dataplane/parser/translate_kong_l4_test.go b/internal/dataplane/parser/translate_kong_l4_test.go
@@ -97,69 +97,6 @@ func TestFromTCPIngressV1beta1(t *testing.T) {
  },
  },
  },
- // 4
- {
- ObjectMeta: metav1.ObjectMeta{
- Name: "foo",
- Namespace: "default",
- Annotations: map[string]string{
- annotations.IngressClassKey: annotations.DefaultIngressClass,
- },
- },
- Spec: configurationv1beta1.TCPIngressSpec{
- Rules: []configurationv1beta1.IngressRule{
- {
- Port: 9000,
- Backend: configurationv1beta1.IngressBackend{
- ServiceName: "",
- ServicePort: 80,
- },
- },
- },
- },
- },
- // 5
- {
- ObjectMeta: metav1.ObjectMeta{
- Name: "foo",
- Namespace: "default",
- Annotations: map[string]string{
- annotations.IngressClassKey: annotations.DefaultIngressClass,
- },
- },
- Spec: configurationv1beta1.TCPIngressSpec{
- Rules: []configurationv1beta1.IngressRule{
- {
- Port: 0,
- Backend: configurationv1beta1.IngressBackend{
- ServiceName: "foo-svc",
- ServicePort: 80,
- },
- },
- },
- },
- },
- // 6
- {
- ObjectMeta: metav1.ObjectMeta{
- Name: "foo",
- Namespace: "default",
- Annotations: map[string]string{
- annotations.IngressClassKey: annotations.DefaultIngressClass,
- },
- },
- Spec: configurationv1beta1.TCPIngressSpec{
- Rules: []configurationv1beta1.IngressRule{
- {
- Port: 9000,
- Backend: configurationv1beta1.IngressBackend{
- ServiceName: "foo-svc",
- ServicePort: 0,
- },
- },
- },
- },
- },
  }
  t.Run("no TCPIngress returns empty info", func(t *testing.T) {
  store, err := store.NewFakeStore(store.FakeObjects{
@@ -260,49 +197,4 @@ func TestFromTCPIngressV1beta1(t *testing.T) {
  assert.Equal(2, len(parsedInfo.SecretNameToSNIs["default/sooper-secret"]))
  assert.Equal(2, len(parsedInfo.SecretNameToSNIs["default/sooper-secret2"]))
  })
- t.Run("TCPIngress without service name returns empty info", func(t *testing.T) {
- store, err := store.NewFakeStore(store.FakeObjects{
- TCPIngresses: []*configurationv1beta1.TCPIngress{
- tcpIngressList[4],
- },
- })
- assert.NoError(err)
- p := mustNewParser(t, store)
-
- parsedInfo := p.ingressRulesFromTCPIngressV1beta1()
- assert.Equal(ingressRules{
- ServiceNameToServices: make(map[string]kongstate.Service),
- SecretNameToSNIs: make(map[string][]string),
- }, parsedInfo)
- })
- t.Run("TCPIngress with invalid port returns empty info", func(t *testing.T) {
- store, err := store.NewFakeStore(store.FakeObjects{
- TCPIngresses: []*configurationv1beta1.TCPIngress{
- tcpIngressList[5],
- },
- })
- assert.NoError(err)
- p := mustNewParser(t, store)
-
- parsedInfo := p.ingressRulesFromTCPIngressV1beta1()
- assert.Equal(ingressRules{
- ServiceNameToServices: make(map[string]kongstate.Service),
- SecretNameToSNIs: make(map[string][]string),
- }, parsedInfo)
- })
- t.Run("empty TCPIngress with invalid service port returns empty info", func(t *testing.T) {
- store, err := store.NewFakeStore(store.FakeObjects{
- TCPIngresses: []*configurationv1beta1.TCPIngress{
- tcpIngressList[6],
- },
- })
- assert.NoError(err)
- p := mustNewParser(t, store)
-
- parsedInfo := p.ingressRulesFromTCPIngressV1beta1()
- assert.Equal(ingressRules{
- ServiceNameToServices: make(map[string]kongstate.Service),
- SecretNameToSNIs: make(map[string][]string),
- }, parsedInfo)
- })
 }