feat(#3097): propagate translation failures for gateway listeners ref…

…erred secrets (#3147)

Adds propagation of translation failures for Gateway.Listeners that:
- specify more than one certReference,
- refer to a secret that has no valid TLS key-pair embedded.

CHANGELOG.md

@@ -137,6 +137,9 @@ Adding a new version? You'll need three changes:
 - Warning events are recorded when one of `netv1.Ingress` related issues occurs
   (e.g. backing Kubernetes service couldn't be found, matching Kubernetes service port couldn't be found).
   [#3138](https://github.com/Kong/kubernetes-ingress-controller/pull/3138)
+- Warning events are recorded when a Gateway Listener has more than one CertificateRef specified
+  or refers to a Secret that has no valid TLS key-pair.
+  [#3147](https://github.com/Kong/kubernetes-ingress-controller/pull/3147)
 
 ### Fixed
 

internal/controllers/gateway/gateway_controller_test.go

@@ -557,6 +557,11 @@ func TestGetReferenceGrantConditionReason(t *testing.T) {
 		referenceGrants  []gatewayv1alpha2.ReferenceGrant
 		expectedReason   string
 	}{
+		{
+			name:           "empty reference",
+			certRef:        gatewayv1beta1.SecretObjectReference{},
+			expectedReason: string(gatewayv1alpha2.ListenerReasonResolvedRefs),
+		},
 		{
 			name:             "no need for reference",
 			gatewayNamespace: "test",
@@ -567,7 +572,7 @@ func TestGetReferenceGrantConditionReason(t *testing.T) {
 			expectedReason: string(gatewayv1alpha2.ListenerReasonResolvedRefs),
 		},
 		{
-			name:             "reference not granted",
+			name:             "reference not granted - secret name not matching",
 			gatewayNamespace: "test",
 			certRef: gatewayv1beta1.SecretObjectReference{
 				Kind:      util.StringToGatewayAPIKindPtr("Secret"),
@@ -599,6 +604,16 @@ func TestGetReferenceGrantConditionReason(t *testing.T) {
 			},
 			expectedReason: string(gatewayv1alpha2.ListenerReasonRefNotPermitted),
 		},
+		{
+			name:             "reference not granted - no grants specified",
+			gatewayNamespace: "test",
+			certRef: gatewayv1beta1.SecretObjectReference{
+				Kind:      util.StringToGatewayAPIKindPtr("Secret"),
+				Name:      "testSecret",
+				Namespace: (*Namespace)(pointer.StringPtr("otherNamespace")),
+			},
+			expectedReason: string(gatewayv1alpha2.ListenerReasonRefNotPermitted),
+		},
 		{
 			name:             "reference granted, secret name not specified",
 			gatewayNamespace: "test",

internal/dataplane/parser/parser.go

@@ -121,7 +121,7 @@ func (p *Parser) Build() (*kongstate.KongState, []TranslationFailure) {
 
 	// generate Certificates and SNIs
 	ingressCerts := getCerts(p.logger, p.storer, ingressRules.SecretNameToSNIs)
-	gatewayCerts := getGatewayCerts(p.logger, p.storer)
+	gatewayCerts := p.getGatewayCerts()
 	// note that ingress-derived certificates will take precedence over gateway-derived certificates for SNI assignment
 	result.Certificates = mergeCerts(p.logger, ingressCerts, gatewayCerts)
 
@@ -386,18 +386,15 @@ type certWrapper struct {
 	CreationTimestamp metav1.Time
 }
 
-func getGatewayCerts(log logrus.FieldLogger, s store.Storer) []certWrapper {
+func (p *Parser) getGatewayCerts() []certWrapper {
+	log := p.logger
+	s := p.storer
 	certs := []certWrapper{}
 	gateways, err := s.ListGateways()
 	if err != nil {
 		log.WithError(err).Error("failed to list Gateways")
 		return certs
 	}
-	grants, err := s.ListReferenceGrants()
-	if err != nil {
-		log.WithError(err).Error("failed to list ReferenceGrants")
-		return certs
-	}
 	for _, gateway := range gateways {
 		statuses := make(map[gatewayv1beta1.SectionName]gatewayv1beta1.ListenerStatus, len(gateway.Status.Listeners))
 		for _, status := range gateway.Status.Listeners {
@@ -431,48 +428,16 @@ func getGatewayCerts(log logrus.FieldLogger, s store.Storer) []certWrapper {
 					if len(listener.TLS.CertificateRefs) > 1 {
 						// TODO support cert_alt and key_alt if there are 2 SecretObjectReferences
 						// https://github.com/Kong/kubernetes-ingress-controller/issues/2604
-						log.WithFields(logrus.Fields{
-							"gateway":  gateway.Name,
-							"listener": listener.Name,
-						}).Error("Gateway Listeners with more than one certificateRef are not supported")
+						p.registerTranslationFailure("listener '%s' has more than one certificateRef, it's not supported", gateway)
 						continue
 					}
 
+					// determine the Secret Namespace
 					ref := listener.TLS.CertificateRefs[0]
-
-					// SecretObjectReference is a misnomer; it can reference any Group+Kind, but defaults to Secret
-					if ref.Kind != nil {
-						if string(*ref.Kind) != "Secret" {
-							log.WithFields(logrus.Fields{
-								"gateway":  gateway.Name,
-								"listener": listener.Name,
-							}).Error("CertificateRefs to kinds other than Secret are not supported")
-						}
-					}
-
-					// determine the Secret Namespace and validate against ReferenceGrant if needed
 					namespace := gateway.Namespace
 					if ref.Namespace != nil {
 						namespace = string(*ref.Namespace)
 					}
-					if namespace != gateway.Namespace {
-						allowed := getPermittedForReferenceGrantFrom(gatewayv1alpha2.ReferenceGrantFrom{
-							Group:     gatewayv1alpha2.Group(gateway.GetObjectKind().GroupVersionKind().Group),
-							Kind:      gatewayv1alpha2.Kind(gateway.GetObjectKind().GroupVersionKind().Kind),
-							Namespace: gatewayv1alpha2.Namespace(gateway.GetNamespace()),
-						}, grants)
-
-						if !newRefChecker(ref).IsRefAllowedByGrant(allowed) {
-							log.WithFields(logrus.Fields{
-								"gateway":           gateway.Name,
-								"gateway_namespace": gateway.Namespace,
-								"listener":          listener.Name,
-								"secret_name":       string(ref.Name),
-								"secret_namespace":  namespace,
-							}).WithError(err).Error("secret reference not allowed by ReferenceGrant")
-							continue
-						}
-					}
 
 					// retrieve the Secret and extract the PEM strings
 					secret, err := s.GetSecret(namespace, string(ref.Name))
@@ -487,12 +452,7 @@ func getGatewayCerts(log logrus.FieldLogger, s store.Storer) []certWrapper {
 					}
 					cert, key, err := getCertFromSecret(secret)
 					if err != nil {
-						log.WithFields(logrus.Fields{
-							"gateway":          gateway.Name,
-							"listener":         listener.Name,
-							"secret_name":      string(ref.Name),
-							"secret_namespace": namespace,
-						}).WithError(err).Error("failed to construct certificate from secret")
+						p.registerTranslationFailure("failed to construct certificate from secret", secret, gateway)
 						continue
 					}
 

internal/dataplane/parser/translate_utils_test.go

@@ -178,120 +178,6 @@ func TestConvertGatewayMatchHeadersToKongRouteMatchHeaders(t *testing.T) {
 	}
 }
 
-func TestIsRefAllowedByGrant(t *testing.T) {
-	fitrat := gatewayv1beta1.Namespace("fitrat")
-	cholpon := gatewayv1beta1.Namespace("cholpon")
-	behbudiy := gatewayv1beta1.Namespace("behbudiy")
-
-	group := gatewayv1beta1.Group("fake.example.com")
-	kind := gatewayv1beta1.Kind("fakeKind")
-	badKind := gatewayv1beta1.Kind("badFakeKind")
-	cholponName := gatewayv1beta1.ObjectName("cholpon")
-
-	refGrantGroup := gatewayv1alpha2.Group(group)
-	refGrantKind := gatewayv1alpha2.Kind(kind)
-	refGrantBadKind := gatewayv1alpha2.Kind(badKind)
-	refGrantName := gatewayv1alpha2.ObjectName(cholponName)
-
-	fakeMap := map[gatewayv1beta1.Namespace][]gatewayv1alpha2.ReferenceGrantTo{
-		fitrat: {
-			{Group: refGrantGroup, Kind: refGrantKind},
-			{Group: gatewayv1alpha2.Group("extra.example"), Kind: refGrantBadKind},
-		},
-		cholpon: {
-			{Group: refGrantGroup, Kind: refGrantKind, Name: &refGrantName},
-		},
-		behbudiy: {},
-	}
-	tests := []struct {
-		msg    string
-		ref    gatewayv1beta1.BackendRef
-		result bool
-	}{
-		{
-			msg:    "empty",
-			ref:    gatewayv1beta1.BackendRef{},
-			result: true,
-		},
-		{
-			msg: "no namespace",
-			ref: gatewayv1beta1.BackendRef{
-				BackendObjectReference: gatewayv1beta1.BackendObjectReference{
-					Name: gatewayv1beta1.ObjectName("foo"),
-				},
-			},
-			result: true,
-		},
-		{
-			msg: "valid namespace+group+kind",
-			ref: gatewayv1beta1.BackendRef{
-				BackendObjectReference: gatewayv1beta1.BackendObjectReference{
-					Name:      gatewayv1beta1.ObjectName("foo"),
-					Group:     &group,
-					Kind:      &kind,
-					Namespace: &fitrat,
-				},
-			},
-			result: true,
-		},
-		{
-			msg: "valid namespace+group+kind+name",
-			ref: gatewayv1beta1.BackendRef{
-				BackendObjectReference: gatewayv1beta1.BackendObjectReference{
-					Name:      cholponName,
-					Group:     &group,
-					Kind:      &kind,
-					Namespace: &cholpon,
-				},
-			},
-			result: true,
-		},
-		{
-			msg: "invalid namespace+group+kind",
-			ref: gatewayv1beta1.BackendRef{
-				BackendObjectReference: gatewayv1beta1.BackendObjectReference{
-					Name:      gatewayv1beta1.ObjectName("foo"),
-					Group:     &group,
-					Kind:      &badKind,
-					Namespace: &fitrat,
-				},
-			},
-			result: false,
-		},
-		{
-			msg: "invalid namespace+group+kind+name",
-			ref: gatewayv1beta1.BackendRef{
-				BackendObjectReference: gatewayv1beta1.BackendObjectReference{
-					Name:      gatewayv1beta1.ObjectName("sadness"),
-					Group:     &group,
-					Kind:      &kind,
-					Namespace: &cholpon,
-				},
-			},
-			result: false,
-		},
-		{
-			msg: "no grants in target namespace",
-			ref: gatewayv1beta1.BackendRef{
-				BackendObjectReference: gatewayv1beta1.BackendObjectReference{
-					Name:      gatewayv1beta1.ObjectName("foo"),
-					Group:     &group,
-					Kind:      &kind,
-					Namespace: &behbudiy,
-				},
-			},
-			result: false,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.msg, func(t *testing.T) {
-			result := newRefChecker(tt.ref).IsRefAllowedByGrant(fakeMap)
-
-			assert.Equal(t, tt.result, result)
-		})
-	}
-}
-
 func TestGetPermittedForReferenceGrantFrom(t *testing.T) {
 	grants := []*gatewayv1alpha2.ReferenceGrant{
 		{

test/integration/translation_failures_test.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/kong/go-kong/kong"
 	"github.com/kong/kubernetes-testing-framework/pkg/clusters"
+	ktfkong "github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/kong"
 	"github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/generators"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -187,6 +188,48 @@ func TestTranslationFailures(t *testing.T) {
 				}
 			},
 		},
+		{
+			name: "more than one certificate ref specified for a gateway listener",
+			translationFailureTrigger: func(t *testing.T, cleaner *clusters.Cleaner, ns string) expectedTranslationFailure {
+				secret1 := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{
+					Name: testutils.RandomName(testTranslationFailuresObjectsPrefix),
+				}}
+				secret2 := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{
+					Name: testutils.RandomName(testTranslationFailuresObjectsPrefix),
+				}}
+				secret1, err := env.Cluster().Client().CoreV1().Secrets(ns).Create(ctx, secret1, metav1.CreateOptions{})
+				require.NoError(t, err)
+				cleaner.Add(secret1)
+				secret2, err = env.Cluster().Client().CoreV1().Secrets(ns).Create(ctx, secret2, metav1.CreateOptions{})
+				require.NoError(t, err)
+				cleaner.Add(secret2)
+
+				gateway := deployGatewayReferringSecrets(t, cleaner, ns, secret1, secret2)
+
+				return expectedTranslationFailure{
+					causingObjects: []client.Object{gateway},
+					reasonContains: "more than one certificateRef",
+				}
+			},
+		},
+		{
+			name: "invalid secret referred by a gateway listener",
+			translationFailureTrigger: func(t *testing.T, cleaner *clusters.Cleaner, ns string) expectedTranslationFailure {
+				emptySecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{
+					Name: testutils.RandomName(testTranslationFailuresObjectsPrefix),
+				}}
+				emptySecret, err := env.Cluster().Client().CoreV1().Secrets(ns).Create(ctx, emptySecret, metav1.CreateOptions{})
+				require.NoError(t, err)
+				cleaner.Add(emptySecret)
+
+				gateway := deployGatewayReferringSecrets(t, cleaner, ns, emptySecret)
+
+				return expectedTranslationFailure{
+					causingObjects: []client.Object{gateway, emptySecret},
+					reasonContains: "failed to construct certificate from secret",
+				}
+			},
+		},
 	}
 
 	for _, tt := range testCases {
@@ -395,3 +438,38 @@ func validService() *corev1.Service {
 		},
 	}
 }
+
+func deployGatewayReferringSecrets(t *testing.T, cleaner *clusters.Cleaner, ns string, secrets ...*corev1.Secret) *gatewayv1beta1.Gateway {
+	gatewayClient, err := gatewayclient.NewForConfig(env.Cluster().Config())
+	require.NoError(t, err)
+
+	gatewayClassName := testutils.RandomName(testTranslationFailuresObjectsPrefix)
+	gwc, err := DeployGatewayClass(ctx, gatewayClient, gatewayClassName)
+	require.NoError(t, err)
+	cleaner.Add(gwc)
+
+	certificateRefs := make([]gatewayv1beta1.SecretObjectReference, 0, len(secrets))
+	for _, s := range secrets {
+		sn := gatewayv1beta1.Namespace(s.GetNamespace())
+		certificateRefs = append(certificateRefs, gatewayv1beta1.SecretObjectReference{
+			Name:      gatewayv1beta1.ObjectName(s.GetName()),
+			Namespace: &sn,
+		})
+	}
+
+	gatewayName := testutils.RandomName(testTranslationFailuresObjectsPrefix)
+	hostname := gatewayv1beta1.Hostname(tlsRouteHostname)
+	gateway, err := DeployGateway(ctx, gatewayClient, ns, gatewayClassName, func(gw *gatewayv1beta1.Gateway) {
+		gw.Name = gatewayName
+		gw.Spec.Listeners = []gatewayv1beta1.Listener{{
+			Name:     gatewayv1beta1.SectionName(testutils.RandomName(testTranslationFailuresObjectsPrefix)),
+			Protocol: gatewayv1beta1.TLSProtocolType,
+			Port:     gatewayv1beta1.PortNumber(ktfkong.DefaultTLSServicePort),
+			Hostname: &hostname,
+			TLS:      &gatewayv1beta1.GatewayTLSConfig{CertificateRefs: certificateRefs},
+		}}
+	})
+	require.NoError(t, err)
+	cleaner.Add(gateway)
+	return gateway
+}